CI: fix template injection in workflow run: blocks#6445
Closed
brtnfld wants to merge 10 commits into
Closed
Conversation
Move ${{ inputs.* }} and ${{ steps.*.outputs.* }} expressions from
run: shell blocks into env: blocks on the same step, then reference
them as plain shell/$env:PSVar variables. This eliminates the
template-injection vector that zizmor flags: expressions in env: are
expanded safely before the shell sees them, while expressions inlined
directly in run: are expanded as literal text into the script and can
be dangerous if the value contains shell metacharacters.
brtnfld
requested review from
glennsong09,
hyoklee,
jhendersonHDF and
lrknox
as code owners
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
zizmor found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
Contributor
Review ChecklistThis PR touches the following areas. Each needs at least one
|
Complete the template-injection fix across all affected workflow files.
Moves ${{ inputs.* }}, ${{ steps.*.outputs.* }}, ${{ matrix.* }},
${{ vars.* }}, ${{ secrets.* }}, ${{ runner.* }}, and
${{ github.event.repository.* }} expressions from run: shell blocks
into env: blocks on the same step, then references them as plain shell
variables. 44 files changed, 0 error[template-injection] findings
remain.
…njection fix script
- main.yml, vfd-ros3.yml: rename invalid YAML key
'INPUT_FORCE_JAVA_IMPLEMENTATION == '\''JNI'\''' → ENABLE_JNI; fix
shell references accordingly
- main.yml: rename unreadable '_MATRIX_OSTYPE_____WINDOWS__' → IS_NOT_WINDOWS
- maven-deploy.yml: rename invalid YAML key 'GPG_PRIVATE_KEY != '\'''\''' → HAS_GPG_KEY;
fix shell reference
- maven-build-test.yml: rename verbose auto-generated
'FORMAT__HTTPS___MAVEN_PKG_GITHUB_COM__0____GITHUB_REPOSITORY_' → MAVEN_PKG_URL
- ctest.yml: fix PowerShell env var syntax $RUNNER_WORKSPACE → $env:RUNNER_WORKSPACE
- analysis.yml: fix bash variable boundary $REPO_FULL_NAME_COV → ${REPO_FULL_NAME}_COV
RUNNER_WORKSPACE is a built-in GitHub Actions environment variable
automatically available on all runners. Explicitly re-mapping it via
env: RUNNER_WORKSPACE: ${{ runner.workspace }} in every step is pure
boilerplate. Remove it from 33 files (170 lines total); run scripts
already reference $RUNNER_WORKSPACE / $env:RUNNER_WORKSPACE and will
continue to work unchanged.
…eanup Removing the RUNNER_WORKSPACE mappings in the previous commit left 88 bare env: lines (no key-value pairs) across 16 workflow files. Empty env: blocks are invalid per the GitHub Actions schema and cause actionlint warnings. Strip them all.
…SPACE cleanup" This reverts commit 3b335a9.
…workflows" This reverts commit 7466e19.
The earlier global sed incorrectly converted $RUNNER_WORKSPACE to $env:RUNNER_WORKSPACE in bash steps as well as PowerShell steps. $env:RUNNER_WORKSPACE is PowerShell-only syntax; bash steps must use $RUNNER_WORKSPACE. Restore 58 bash-step references while keeping $env:RUNNER_WORKSPACE in the 31 shell: pwsh blocks where it is correct.
Collaborator
Author
|
Closing in favor of a targeted fix. The reusable workflow files fixed here receive their inputs from the calling workflow (call-workflows.yml), which is clean — so those fixes were unnecessary. A new PR will address only the genuine risks: workflow_dispatch inputs used directly in run: blocks in java-implementation-test.yml, maven-staging.yml, maven-build-test.yml, test-maven-packages.yml, publish-branch.yml, and test-binary-installation.yml. |
github-project-automation
Bot
moved this from To be triaged
to Done
in HDF5 - TRIAGE & TRACK
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
${{ inputs.* }}and${{ steps.*.outputs.* }}expressions used directly insiderun:shell blocks are a template-injection vector: the expression is expanded as literal text into the shell script before the shell parses it, so values containing shell metacharacters could alter the commandenv:blocks on the same step and reference them as plain shell variables ($VAR) or PowerShell variables ($env:VAR)env:are expanded safely by the Actions runner before the shell sees them, eliminating the injection pathtemplate-injectionfindings across 6 files:setup-jextract/action.yml,symlink-ctest-scripts/action.yml,abi-report.yml,ctest.yml,maven-deploy.yml,maven-staging.yml