GitHub Action tj-actions/verify-changed-files is vulnerable to command injection in output filenames

@@ -39,17 +39,26 @@ fi
 
 CHANGED_FILES=""
 
-# Function to concatenate non-empty strings with a separator
+# Function to concatenate non-empty file names with a separator
 concatenate() {
   local separator=$1
   shift
   local result=""
-  for str in "$@"; do
-    if [[ -n $str ]]; then
+  for filename in "$@"; do
+    if [[ "$INPUT_SAFE_OUTPUT" == "true" ]]; then
+      filename=${filename//$/\\$} # Replace $ with \$
+      filename=${filename//\(/\\\(} # Replace ( with \(
+      filename=${filename//\)/\\\)} # Replace ) with \)
+      filename=${filename//\`/\\\`} # Replace ` with \`
+      filename=${filename//|/\\|} # Replace | with \|
+      filename=${filename//&/\\&} # Replace & with \&
+      filename=${filename//;/\\;} # Replace ; with \;
+    fi
+    if [[ -n $filename ]]; then
       if [[ -n $result ]]; then
-        result+="$separator$str"
+        result+="$separator$filename"
       else
-        result="$str"
+        result="$filename"
       fi
     fi
   done
@@ -62,20 +71,10 @@ CHANGED_FILES=$(concatenate "|" "$TRACKED_FILES" "$UNTRACKED_OR_IGNORED_FILES" "
 CHANGED_FILES=$(echo "$CHANGED_FILES"  | awk '{gsub(/\|/,"\n"); print $0;}' | sort -u | awk -v d="|" '{s=(NR==1?s:s d)$0}END{print s}')
 
 if [[ -n "$CHANGED_FILES" ]]; then
-  echo "Found uncommited changes"
+  echo "Found uncommitted changes"
 
   CHANGED_FILES=$(echo "$CHANGED_FILES" | awk '{gsub(/\|/,"\n"); print $0;}' | awk -v d="$INPUT_SEPARATOR" '{s=(NR==1?s:s d)$0}END{print s}')
 
-  if [[ "$INPUT_SAFE_OUTPUT" == "true" ]]; then
-    CHANGED_FILES=${CHANGED_FILES//$/\\$} # Replace $ with \$
-    CHANGED_FILES=${CHANGED_FILES//\(/\\\(}} # Replace ( with \(
-    CHANGED_FILES=${CHANGED_FILES//\)/\\\)}} # Replace ) with \)
-    CHANGED_FILES=${CHANGED_FILES//\`/\\\`} # Replace ` with \`
-    CHANGED_FILES=${CHANGED_FILES//|/\\|} # Replace | with \|
-    CHANGED_FILES=${CHANGED_FILES//&/\\&} # Replace & with \&
-    CHANGED_FILES=${CHANGED_FILES//;/\\;} # Replace ; with \;
-  fi
-
   echo "files_changed=true" >> "$GITHUB_OUTPUT"
   echo "changed_files=$CHANGED_FILES" >> "$GITHUB_OUTPUT"
 
@@ -85,7 +84,6 @@ if [[ -n "$CHANGED_FILES" ]]; then
     fi
     exit 1
   fi
-
 else
   echo "No changes found."
   echo "files_changed=false" >> "$GITHUB_OUTPUT"

@@ -25,6 +25,10 @@ inputs:
     description: 'Message to display when files have changed and the `fail-if-changed` input is set to `true`.'
     default: "Files have changed."
     required: false
+  safe_output:
+    description: "Apply sanitization to output filenames before being set as output."
+    required: false
+    default: "true"
 
 outputs:
   files_changed:
@@ -61,6 +65,7 @@ runs:
         INPUT_MATCH_GITIGNORE_FILES: ${{ inputs.match-gitignore-files }}
         INPUT_FAIL_IF_CHANGED: ${{ inputs.fail-if-changed }}
         INPUT_FAIL_MSG: ${{ inputs.fail-message }}
+        INPUT_SAFE_OUTPUT: ${{ inputs.safe_output }}
 
 branding:
   icon: file-text

@@ -66,6 +66,16 @@ if [[ -n "$CHANGED_FILES" ]]; then
 
   CHANGED_FILES=$(echo "$CHANGED_FILES" | awk '{gsub(/\|/,"\n"); print $0;}' | awk -v d="$INPUT_SEPARATOR" '{s=(NR==1?s:s d)$0}END{print s}')
 
+  if [[ "$INPUT_SAFE_OUTPUT" == "true" ]]; then
+    CHANGED_FILES=${CHANGED_FILES//$/\\$} # Replace $ with \$
+    CHANGED_FILES=${CHANGED_FILES//\(/\\\(}} # Replace ( with \(
+    CHANGED_FILES=${CHANGED_FILES//\)/\\\)}} # Replace ) with \)
+    CHANGED_FILES=${CHANGED_FILES//\`/\\\`} # Replace ` with \`
+    CHANGED_FILES=${CHANGED_FILES//|/\\|} # Replace | with \|
+    CHANGED_FILES=${CHANGED_FILES//&/\\&} # Replace & with \&
+    CHANGED_FILES=${CHANGED_FILES//;/\\;} # Replace ; with \;
+  fi
+
   echo "files_changed=true" >> "$GITHUB_OUTPUT"
   echo "changed_files=$CHANGED_FILES" >> "$GITHUB_OUTPUT"
 

@@ -60,6 +60,7 @@ Verify that certain files or directories did or did not change during the workfl
         uses: tj-actions/verify-changed-files@v16
         id: verify-changed-files
         with:
+          safe_output: false # true by default, set to false because we are using an environment variable to store the output and avoid command injection.
           files: |
              *.txt
              test_directory
@@ -69,8 +70,11 @@ Verify that certain files or directories did or did not change during the workfl
 
       - name: Run step only when any of the above files change.
         if: steps.verify-changed-files.outputs.files_changed == 'true'
+        env:
+          FILES_CHANGED: |-
+            ${{ steps.verify-changed-files.outputs.changed_files }}
         run: |
-          echo "Changed files: ${{ steps.verify-changed-files.outputs.changed_files }}"
+          echo "Changed files: $FILES_CHANGED"
         # Outputs: "Changed files: new.txt test_directory/new.txt"
 ```
 
@@ -82,6 +86,7 @@ Verify that certain files or directories did or did not change during the workfl
         uses: tj-actions/verify-changed-files@v16
         id: verify-changed-files
         with:
+          safe_output: false
           files: |
              new.txt
              test_directory
@@ -99,10 +104,15 @@ Verify that certain files or directories did or did not change during the workfl
       - name: Verify Changed files
         uses: tj-actions/verify-changed-files@v16
         id: verify-changed-files
+        with:
+          safe_output: false
       
       - name: List all changed files tracked and untracked files
+        env:
+          FILES_CHANGED: |-
+            ${{ steps.verify-changed-files.outputs.changed_files }}
         run: |
-          echo "Changed files: ${{ steps.verify-changed-files.outputs.changed_files }}"
+          echo "Changed files: $FILES_CHANGED"
 ```
 
 If you feel generous and want to show some extra appreciation: