Are environment variables from previous steps insecure?

[nedbat]

In a complex workflow, I have steps that put values into the environment to be used by later steps. Zizmor warns those later uses are insecure, but are they? Is there a way for an attacker to change them?

A shortened example:

      - name: "Compute info for later steps"
        id: info
        run: |
          export SHA10=$(echo ${{ github.sha }} | cut -c 1-10)
          export SLUG=$(date +'%Y%m%d')_$SHA10
          echo "url=https://htmlpreview.github.io/?https://github.com/nedbat/coverage-reports/blob/main/reports/$SLUG/htmlcov/index.html" >> $GITHUB_ENV

      - name: "Push to report repo"
        run: |
          git push
          echo '[${{ env.url }}](${{ env.url }})' >> $GITHUB_STEP_SUMMARY

I get a warning that env.url could be attacker-controlled:

help[template-injection]: code injection via template expansion
   --> /Users/ned/coverage/trunk/.github/workflows/coverage.yml:243:9
    |
243 |         - name: "Push to report repo"
    |           --------------------------- help: this step
244 |           if: |
...
248 |             COMMIT_MESSAGE: ${{ github.event.head_commit.message }}
249 |           run: |
    |  _________-
250 | |           set -xe
...   |
266 | |           git push
267 | |           echo '[${{ env.url }}](${{ env.url }})' >> $GITHUB_STEP_SUMMARY
    | |_________________________________________________________________________- help: env.url may expand into attacker-controllable code
    |

[woodruffw]

Thanks for the report!

In this case they can't be attacker controlled, but it's hard for zizmor to prove that in the general case (since we'd have to parse the shell steps and make sure there's no attacker-controlled dataflows into those environment variable values).

With that being said, I think in this case there are two appropriate fixes:

Option 1: use $URL directly, since that'll interpolate with normal rules:

      - name: "Push to report repo"
        run: |
          git push
          echo "[${URL}](${URL})" >> $GITHUB_STEP_SUMMARY

I'm pretty sure that will work, since IIUC each step loads from $GITHUB_ENV before running. But if that doesn't work, Option 2 is to plumb env.URL via env: like so:

      - name: "Push to report repo"
        run: |
          git push
          echo "[${URL}](${URL})" >> $GITHUB_STEP_SUMMARY
        env:
          URL: ${{ env.URL }}

...but that shouldn't be necessary, since Option 1 should work.

[nedbat]

it's hard for zizmor to prove that in the general case (since we'd have to parse the shell steps and make sure there's no attacker-controlled dataflows into those environment variable values).

Wouldn't those earlier steps be flagged by zizmor if they had risky interpolations in them?

I tried using the env var directly by duplicating a line:

      - name: "Summarize"
        run: |
          echo "### TOTAL coverage: ${TOTAL}%" >> $GITHUB_STEP_SUMMARY
          echo '### Total coverage: ${{ env.total }}%' >> $GITHUB_STEP_SUMMARY

The result I got was:

TOTAL coverage: %
Total coverage: 95.341%

[woodruffw]

Wouldn't those earlier steps be flagged by zizmor if they had risky interpolations in them?

Not necessarily, since the attacker dataflow is not necessarily from another context expansion, but from any other source that could have shell-interpreted characters in it.

Here's a contrived example:

- run: |
    echo "blah=$(something)" >> "${GITHUB_ENV}"

- run: |
    echo "${{ env.blah }}"

if something is anything the attacker can control (even indirectly), then env.blah becomes an uncontrolled expansion. For example, if something was a curl to the GitHub API controlled by github.event.number, then we don't know which fields of that API response are used and so the whole thing is potentially tainted.

I tried using the env var directly by duplicating a line:

      - name: "Summarize"
        run: |
          echo "### TOTAL coverage: ${TOTAL}%" >> $GITHUB_STEP_SUMMARY
          echo '### Total coverage: ${{ env.total }}%' >> $GITHUB_STEP_SUMMARY

Could you try ${total}? GitHub Actions doesn't auto-capitalize the inputs to ${GITHUB_ENV}, so if you named it total then it needs to be that as an envvar as well :slightly_smiling_face:

For reference, here's an example of a GITHUB_ENV-set envvar being accessed directly:

https://github.com/sigstore/sigstore-python/blob/0ac33eeaeb62ca466cef2708ca1dd5864382a008/.github/workflows/pin-requirements.yml#L42-L56

[nedbat]

Could you try ${total}? GitHub Actions doesn't auto-capitalize the inputs to ${GITHUB_ENV}, so if you named it total then it needs to be that as an envvar as well 🙂

You're right, ${total} works, thanks.