Skip to content

fix(security): prevent GitHub Actions expression injection in build workflows (CWE-94) - #174

Open
ganeshkumarashok wants to merge 1 commit into
mainfrom
fix/actions-expression-injection
Open

fix(security): prevent GitHub Actions expression injection in build workflows (CWE-94)#174
ganeshkumarashok wants to merge 1 commit into
mainfrom
fix/actions-expression-injection

Conversation

@ganeshkumarashok

Copy link
Copy Markdown
Collaborator

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 of run: bodies into step-level env: 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 — the Load config steps now validate the driver version and URL read from driver_config.yml and fail the job on anything unexpected. This also prevents newline-based injection of extra $GITHUB_OUTPUT entries.
  • auto_update.py — validates 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 — same env: indirection for the azure/cli inline script, plus quoted $GITHUB_OUTPUT writes.

The resulting shape at each build step:

- name: 'Build and Push'
  env:
    DRIVER_URL: ${{ steps.load_config.outputs.grid_url }}
    DRIVER_VERSION: ${{ steps.load_config.outputs.grid_version }}
    REGISTRY_SERVER: ${{ secrets.AZURE_REGISTRY_SERVER }}
    VERSION: ${{ steps.semver.outputs.version }}
  run: |
    image_ref="${REGISTRY_SERVER}/public/aks/${IMAGE_REPO}:${VERSION}"
    docker buildx build --build-arg "DRIVER_URL=${DRIVER_URL}" ... -t "$image_ref" .

Note on the validation anchors

Validation uses whole-string matching — \A...\Z in Python and bash [[ =~ ]] in the workflows — because Python's end-of-string anchor also matches before a trailing newline, and grep -E matches 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 main confirms the job/step graph is equivalent apart from run: bodies and added env: blocks.

azure/cli@v3 forwards step-level env: into the container it runs the script in (verified against the action source — the omit list contains only runner/system variables), so AZURE_KV_NAME / APP_PRIVATE_KEY_SECRET_NAME resolve as before.

Validation

  • actionlint clean on all three workflows.
  • Programmatic scan confirms zero ${{ }} remain inside any run: or inlineScript block.
  • Structural diff vs main: job/step structure identical for all three workflows.
  • Build commands compared before/after with substituted values: image tag, push target, --platform, --build-arg, and cache flags are equivalent for all six build jobs.
  • yq lookups verified against the real driver_config.yml for all four config keys (cuda, cuda_lts, grid, grid_v20).
  • Validation exercised against every driver version and URL currently published upstream: all are accepted.
  • auto_update.py run end to end: real data validates and produces an unchanged driver_config.yml.

Supersedes #172 (same commit, re-opened from a branch on this repo so CI has access to repository secrets).

… (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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants