fix(security): prevent GitHub Actions expression injection in build workflows (CWE-94) - #172
Conversation
… (CWE-94)
Several `run:` blocks interpolated ${{ }} expressions directly into shell
commands. GitHub substitutes these expressions as literal text into the script
before the shell runs, so a value containing shell metacharacters is evaluated
as code rather than treated as data. Some of the interpolated values originate
outside this repository, so they should not be treated as trusted.
This is a well-known Actions anti-pattern; the recommended remedy is to pass
values through `env:` and reference them as quoted shell variables.
- main.yaml / ci.yaml: pass all expressions through `env:` and reference them
as quoted shell variables, so values are always treated as data rather than
code. Applied to every build job, since the pattern was repeated throughout.
- main.yaml / ci.yaml: validate the driver version and URL read from
driver_config.yml in the "Load config" steps and fail the job on anything
unexpected, which also prevents newline-based injection of extra
$GITHUB_OUTPUT entries.
- auto_update.py: validate the driver version and URL before writing them to
driver_config.yml, so unexpected values are rejected at the point they enter
the repo rather than at the point they are consumed. The URL must be https on
the expected download host.
- update_grid_driver.yaml: apply the same env indirection to the azure/cli
inline script and quote $GITHUB_OUTPUT writes.
Validation uses whole-string matching (`\A...\Z` in Python, bash `[[ =~ ]]` in
the workflows) because `$` in Python's re matches before a trailing newline and
`grep -E` matches line by line, so an initial version using `^...$` was not
equivalent. The patterns are deliberately narrow: if the upstream driver
location ever changes shape, the job fails loudly and a human reviews the
change. Every version and URL currently published upstream satisfies them.
No job, step, action version or build argument was changed; only the way values
reach the shell. actionlint reports no issues and no `${{ }}` remains inside any
`run:` or `inlineScript` block.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 8cd619fe-2501-4bcc-97ea-4a895088ffc2
1acfd03 to
7fafe47
Compare
|
The red checks here are pre-existing and unrelated to this PR. This is a fork PR, so it receives no secrets — #173 fixes that separately (with A/B evidence from a secret-less fork run). Suggested order: merge #173 first, then I will rebase this PR — the two touch the same |
|
Superseded by #174 — same commit, re-opened from a branch on this repo so CI has access to repository secrets. (A PR's head branch can't be changed after creation, hence the new PR.) |
Summary
Hardens the build workflows against GitHub Actions expression injection (CWE-94).
Several
run:blocks interpolated${{ }}expressions directly into shell commands. GitHub substitutes these expressions as literal text into the script before the shell runs, so a value containing shell metacharacters is evaluated as code rather than treated as data. Some of the interpolated values originate outside this repository, so they should not be treated as trusted.This is a well-known Actions anti-pattern; the recommended remedy is to pass values through
env:and reference them as quoted shell variables.Changes
main.yaml/ci.yaml— all${{ }}expressions moved out ofrun:bodies into step-levelenv:blocks and referenced as quoted shell variables, so values are always data and never code. Applied to every build job, since the pattern was repeated throughout.main.yaml/ci.yaml— theLoad configsteps now validate the driver version and URL read fromdriver_config.ymland fail the job on anything unexpected. This also prevents newline-based injection of extra$GITHUB_OUTPUTentries.auto_update.py— validates the driver version and URL before writing them todriver_config.yml, so unexpected values are rejected at the point they enter the repo rather than at the point they are consumed. The URL must behttpson the expected download host.update_grid_driver.yaml— sameenv:indirection for theazure/cliinline script, plus quoted$GITHUB_OUTPUTwrites.The resulting shape at each build step:
Note on the validation anchors
Validation uses whole-string matching —
\A...\Zin Python and bash[[ =~ ]]in the workflows — because Python's end-of-string anchor also matches before a trailing newline, andgrep -Ematches line by line. An initial version using^...$was not equivalent; that was caught during testing and corrected.Note on validation strictness
The version and URL patterns are deliberately narrow. If the upstream driver location ever changes shape, the job fails loudly and a human reviews the change rather than the pipeline accepting it automatically. Every version and URL currently published upstream satisfies the patterns, so this is not expected to trigger in normal operation.
Compatibility
No job, step, action version, or build argument was changed — only the way values reach the shell. A structural diff against
mainconfirms the job/step graph is equivalent apart fromrun:bodies and addedenv:blocks.azure/cli@v3forwards step-levelenv:into the container it runs the script in (verified against the action source — the omit list contains only runner/system variables), soAZURE_KV_NAME/APP_PRIVATE_KEY_SECRET_NAMEresolve as before.Validation
actionlintclean on all three workflows.${{ }}remain inside anyrun:orinlineScriptblock.main: job/step structure identical for all three workflows.--platform,--build-arg, and cache flags are equivalent for all six build jobs.yqlookups verified against the realdriver_config.ymlfor all four config keys (cuda,cuda_lts,grid,grid_v20).auto_update.pyrun end to end: real data validates and produces an unchangeddriver_config.yml.