Merge pull request from GHSA-mcph-m25j-8j63

jorgectf · web-flow · commit 0102c07446a3 · 2023-12-22T14:07:32.000-07:00
* feat: add `safe_output` input enabled by default

* fix: migrate README to safe uses of interpolation

* fix: README `uses` typo

* fix: README examples to account for newlines

* fix: README examples missing `safe_output`

* fix: remove sanitization of `'`

* fix: also sanitize `|&amp;;`
diff --git a/README.md b/README.md
@@ -123,14 +123,19 @@ jobs:
       - name: Get changed files
         id: changed-files
         uses: tj-actions/changed-files@v40
+        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.
         
         # To compare changes between the current commit and the last pushed remote commit set `since_last_remote_commit: true`. e.g
         # with:
         #   since_last_remote_commit: true 
 
       - name: List all changed files
+        env:
+          ALL_CHANGED_FILES: |-
+            ${{ steps.changed-files.outputs.all_changed_files }}
         run: |
-          for file in ${{ steps.changed-files.outputs.all_changed_files }}; do
+          for file in "$ALL_CHANGED_FILES"; do
             echo "$file was changed"
           done
 
@@ -139,14 +144,18 @@ jobs:
         id: changed-markdown-files
         uses: tj-actions/changed-files@v40
         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.
           # Avoid using single or double quotes for multiline patterns
           files: |
              **.md
 
       - name: List all changed files markdown files
         if: steps.changed-markdown-files.outputs.any_changed == 'true'
+        env:
+          ALL_CHANGED_FILES: |-
+            ${{ steps.changed-markdown-files.outputs.all_changed_files }}
         run: |
-          for file in ${{ steps.changed-markdown-files.outputs.all_changed_files }}; do
+          for file in "$ALL_CHANGED_FILES"; do
             echo "$file was changed"
           done
 
@@ -155,6 +164,7 @@ jobs:
         id: changed-files-yaml
         uses: tj-actions/changed-files@v40
         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_yaml: |
             doc:
               - '**.md'
@@ -170,29 +180,39 @@ jobs:
       - name: Run step if test file(s) change
         # NOTE: Ensure all outputs are prefixed by the same key used above e.g. `test_(...)` | `doc_(...)` | `src_(...)` when trying to access the `any_changed` output.
         if: steps.changed-files-yaml.outputs.test_any_changed == 'true'  
+        env:
+          TEST_ALL_CHANGED_FILES: |-
+            ${{ steps.changed-files-yaml.outputs.test_all_changed_files }}
         run: |
           echo "One or more test file(s) has changed."
-          echo "List all the files that have changed: ${{ steps.changed-files-yaml.outputs.test_all_changed_files }}"
+          echo "List all the files that have changed: $TEST_ALL_CHANGED_FILES"
       
       - name: Run step if doc file(s) change
         if: steps.changed-files-yaml.outputs.doc_any_changed == 'true'
+        env:
+          DOC_ALL_CHANGED_FILES: |-
+            ${{ steps.changed-files-yaml.outputs.doc_all_changed_files }}
         run: |
           echo "One or more doc file(s) has changed."
-          echo "List all the files that have changed: ${{ steps.changed-files-yaml.outputs.doc_all_changed_files }}"
+          echo "List all the files that have changed: $DOC_ALL_CHANGED_FILES"
 
       # Example 3
       - name: Get changed files in the docs folder
         id: changed-files-specific
         uses: tj-actions/changed-files@v40
         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: docs/*.{js,html}  # Alternatively using: `docs/**`
           files_ignore: docs/static.js
 
       - name: Run step if any file(s) in the docs folder change
         if: steps.changed-files-specific.outputs.any_changed == 'true'
+        env:
+          ALL_CHANGED_FILES: |-
+            ${{ steps.changed-files-specific.outputs.all_changed_files }}
         run: |
           echo "One or more files in the docs folder has changed."
-          echo "List all the files that have changed: ${{ steps.changed-files-specific.outputs.all_changed_files }}"
+          echo "List all the files that have changed: $ALL_CHANGED_FILES"
 ```
 
 #### Using Github's API :octocat:
@@ -224,10 +244,15 @@ jobs:
       - name: Get changed files
         id: changed-files
         uses: tj-actions/changed-files@v40
+        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.
 
       - name: List all changed files
+        env:
+          ALL_CHANGED_FILES: |-
+            ${{ steps.changed-files.outputs.all_changed_files }}
         run: |
-          for file in ${{ steps.changed-files.outputs.all_changed_files }}; do
+          for file in "$ALL_CHANGED_FILES"; do
             echo "$file was changed"
           done
 ```
@@ -265,12 +290,17 @@ jobs:
       - name: Get changed files
         id: changed-files
         uses: tj-actions/changed-files@v40
+        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.
 
       # NOTE: `since_last_remote_commit: true` is implied by default and falls back to the previous local commit.
 
       - name: List all changed files
+        env:
+          ALL_CHANGED_FILES: |-
+            ${{ steps.changed-files.outputs.all_changed_files }}
         run: |
-          for file in ${{ steps.changed-files.outputs.all_changed_files }}; do
+          for file in "$ALL_CHANGED_FILES"; do
             echo "$file was changed"
           done
       ...
@@ -715,10 +745,15 @@ See [inputs](#inputs) for more information.
     - name: Get changed files
       id: changed-files
       uses: tj-actions/changed-files@v40
+      with:
+        safe_output: false
 
     - name: List all added files
+      env:
+        ADDED_FILES: |-
+          ${{ steps.changed-files.outputs.added_files }}
       run: |
-        for file in ${{ steps.changed-files.outputs.added_files }}; do
+        for file in "$ADDED_FILES"; do
           echo "$file was added"
         done
 ...
@@ -736,6 +771,8 @@ See [outputs](#outputs) for a list of all available outputs.
     - name: Get changed files
       id: changed-files
       uses: tj-actions/changed-files@v40
+      with:
+        safe_output: false
 
     - name: Run a step if my-file.txt was modified
       if: contains(steps.changed-files.outputs.modified_files, 'my-file.txt')
@@ -756,8 +793,9 @@ See [outputs](#outputs) for a list of all available outputs.
 
    - name: Get changed files and write the outputs to a Txt file
      id: changed-files-write-output-files-txt
-     uses: ./
+     uses: tj-actions/changed-files@v40
      with:
+       safe_output: false
        write_output_files: true
 
    - name: Verify the contents of the .github/outputs/added_files.txt file
@@ -775,8 +813,9 @@ See [outputs](#outputs) for a list of all available outputs.
 ...
    - name: Get changed files and write the outputs to a JSON file
      id: changed-files-write-output-files-json
-     uses: ./
+     uses: tj-actions/changed-files@v40
      with:
+       safe_output: false
        json: true
        write_output_files: true
 
@@ -820,6 +859,7 @@ See [inputs](#inputs) for more information.
       id: changed-files-specific
       uses: tj-actions/changed-files@v40
       with:
+        safe_output: false
         files: |
           my-file.txt
           *.sh
@@ -840,15 +880,21 @@ See [inputs](#inputs) for more information.
 
     - name: Run step if any of the listed files above is deleted
       if: steps.changed-files-specific.outputs.any_deleted == 'true'
+      env:
+        DELETED_FILES: |-
+          ${{ steps.changed-files-specific.outputs.deleted_files }}
       run: |
-        for file in ${{ steps.changed-files-specific.outputs.deleted_files }}; do
+        for file in "$DELETED_FILES"; do
           echo "$file was deleted"
         done
 
     - name: Run step if all listed files above have been deleted
       if: steps.changed-files-specific.outputs.only_deleted == 'true'
+      env:
+        DELETED_FILES: |-
+          ${{ steps.changed-files-specific.outputs.deleted_files }}
       run: |
-        for file in ${{ steps.changed-files-specific.outputs.deleted_files }}; do
+        for file in "$DELETED_FILES"; do
           echo "$file was deleted"
         done
 ...
@@ -958,14 +1004,18 @@ jobs:
         id: changed-files-specific
         uses: tj-actions/changed-files@v40
         with:
+          safe_output: false
           base_sha: ${{ steps.get-base-sha.outputs.base_sha }}
           files: .github/**
 
       - name: Run step if any file(s) in the .github folder change
         if: steps.changed-files-specific.outputs.any_changed == 'true'
+        env:
+          ALL_CHANGED_FILES: |-
+            ${{ steps.changed-files-specific.outputs.all_changed_files }}
         run: |
           echo "One or more files in the .github folder has changed."
-          echo "List all the files that have changed: ${{ steps.changed-files-specific.outputs.all_changed_files }}"
+          echo "List all the files that have changed: $ALL_CHANGED_FILES"
 ...
 ```
 
@@ -988,11 +1038,15 @@ See [inputs](#inputs) for more information.
       id: changed-files-for-dir1
       uses: tj-actions/changed-files@v40
       with:
+        safe_output: false
         path: dir1
 
     - name: List all added files in dir1
+      env:
+        ADDED_FILES: |-
+          ${{ steps.changed-files-for-dir1.outputs.added_files }}
       run: |
-        for file in ${{ steps.changed-files-for-dir1.outputs.added_files }}; do
+        for file in "$ADDED_FILES"; do
           echo "$file was added"
         done
 ...
@@ -1015,7 +1069,7 @@ See [inputs](#inputs) for more information.
 
     - name: Run changed-files with quotepath disabled for a specified list of file(s)
       id: changed-files-quotepath-specific
-      uses: ./
+      uses: tj-actions/changed-files@v40
       with:
         files: test/test-&egrave;.txt
         quotepath: "false"
diff --git a/action.yml b/action.yml
@@ -134,6 +134,10 @@ inputs:
     description: "Escape JSON output."
     required: false
     default: "true"
+  safe_output:
+    description: "Apply sanitization to output filenames before being set as output."
+    required: false
+    default: "true"
   fetch_depth:
     description: "Depth of additional branch history fetched. NOTE: This can be adjusted to resolve errors with insufficient history."
     required: false
diff --git a/src/changedFilesOutput.ts b/src/changedFilesOutput.ts
@@ -43,7 +43,8 @@ export const setOutputsAndGetModifiedAndChangedFilesStatus = async ({
     writeOutputFiles: inputs.writeOutputFiles,
     outputDir: inputs.outputDir,
     json: inputs.json,
-    shouldEscape: inputs.escapeJson
+    shouldEscape: inputs.escapeJson,
+    safeOutput: inputs.safeOutput
   })
   await setOutput({
     key: getOutputKey('added_files_count', outputPrefix),
@@ -64,7 +65,8 @@ export const setOutputsAndGetModifiedAndChangedFilesStatus = async ({
     writeOutputFiles: inputs.writeOutputFiles,
     outputDir: inputs.outputDir,
     json: inputs.json,
-    shouldEscape: inputs.escapeJson
+    shouldEscape: inputs.escapeJson,
+    safeOutput: inputs.safeOutput
   })
 
   await setOutput({
@@ -86,7 +88,8 @@ export const setOutputsAndGetModifiedAndChangedFilesStatus = async ({
     writeOutputFiles: inputs.writeOutputFiles,
     outputDir: inputs.outputDir,
     json: inputs.json,
-    shouldEscape: inputs.escapeJson
+    shouldEscape: inputs.escapeJson,
+    safeOutput: inputs.safeOutput
   })
 
   await setOutput({
@@ -108,7 +111,8 @@ export const setOutputsAndGetModifiedAndChangedFilesStatus = async ({
     writeOutputFiles: inputs.writeOutputFiles,
     outputDir: inputs.outputDir,
     json: inputs.json,
-    shouldEscape: inputs.escapeJson
+    shouldEscape: inputs.escapeJson,
+    safeOutput: inputs.safeOutput
   })
 
   await setOutput({
@@ -130,7 +134,8 @@ export const setOutputsAndGetModifiedAndChangedFilesStatus = async ({
     writeOutputFiles: inputs.writeOutputFiles,
     outputDir: inputs.outputDir,
     json: inputs.json,
-    shouldEscape: inputs.escapeJson
+    shouldEscape: inputs.escapeJson,
+    safeOutput: inputs.safeOutput
   })
 
   await setOutput({
@@ -152,7 +157,8 @@ export const setOutputsAndGetModifiedAndChangedFilesStatus = async ({
     writeOutputFiles: inputs.writeOutputFiles,
     outputDir: inputs.outputDir,
     json: inputs.json,
-    shouldEscape: inputs.escapeJson
+    shouldEscape: inputs.escapeJson,
+    safeOutput: inputs.safeOutput
   })
 
   await setOutput({
@@ -174,7 +180,8 @@ export const setOutputsAndGetModifiedAndChangedFilesStatus = async ({
     writeOutputFiles: inputs.writeOutputFiles,
     outputDir: inputs.outputDir,
     json: inputs.json,
-    shouldEscape: inputs.escapeJson
+    shouldEscape: inputs.escapeJson,
+    safeOutput: inputs.safeOutput
   })
 
   await setOutput({
@@ -199,7 +206,8 @@ export const setOutputsAndGetModifiedAndChangedFilesStatus = async ({
     writeOutputFiles: inputs.writeOutputFiles,
     outputDir: inputs.outputDir,
     json: inputs.json,
-    shouldEscape: inputs.escapeJson
+    shouldEscape: inputs.escapeJson,
+    safeOutput: inputs.safeOutput
   })
 
   await setOutput({
@@ -226,7 +234,8 @@ export const setOutputsAndGetModifiedAndChangedFilesStatus = async ({
     writeOutputFiles: inputs.writeOutputFiles,
     outputDir: inputs.outputDir,
     json: inputs.json,
-    shouldEscape: inputs.escapeJson
+    shouldEscape: inputs.escapeJson,
+    safeOutput: inputs.safeOutput
   })
 
   await setOutput({
@@ -314,7 +323,8 @@ export const setOutputsAndGetModifiedAndChangedFilesStatus = async ({
     writeOutputFiles: inputs.writeOutputFiles,
     outputDir: inputs.outputDir,
     json: inputs.json,
-    shouldEscape: inputs.escapeJson
+    shouldEscape: inputs.escapeJson,
+    safeOutput: inputs.safeOutput
   })
 
   await setOutput({
@@ -419,7 +429,8 @@ export const setOutputsAndGetModifiedAndChangedFilesStatus = async ({
     writeOutputFiles: inputs.writeOutputFiles,
     outputDir: inputs.outputDir,
     json: inputs.json,
-    shouldEscape: inputs.escapeJson
+    shouldEscape: inputs.escapeJson,
+    safeOutput: inputs.safeOutput
   })
 
   await setOutput({
diff --git a/src/inputs.ts b/src/inputs.ts
@@ -34,6 +34,7 @@ export type Inputs = {
   dirNamesDeletedFilesIncludeOnlyDeletedDirs: boolean
   json: boolean
   escapeJson: boolean
+  safeOutput: boolean
   fetchDepth?: number
   fetchSubmoduleHistory: boolean
   sinceLastRemoteCommit: boolean
@@ -154,6 +155,7 @@ export const getInputs = (): Inputs => {
   )
   const json = core.getBooleanInput('json', {required: false})
   const escapeJson = core.getBooleanInput('escape_json', {required: false})
+  const safeOutput = core.getBooleanInput('safe_output', {required: false})
   const fetchDepth = core.getInput('fetch_depth', {required: false})
   const sinceLastRemoteCommit = core.getBooleanInput(
     'since_last_remote_commit',
@@ -272,6 +274,7 @@ export const getInputs = (): Inputs => {
     dirNamesIncludeFilesSeparator,
     json,
     escapeJson,
+    safeOutput,
     writeOutputFiles,
     outputDir,
     outputRenamedFilesAsDeletedAndAdded,
diff --git a/src/main.ts b/src/main.ts
diff --git a/src/utils.ts b/src/utils.ts
