Skip to content

Fix: Env-mediate expressions in run blocks - #104

Merged
tykeal merged 1 commit into
lfreleng-actions:mainfrom
modeseven-lfreleng-actions:fix/zizmor-template-injection
Jul 28, 2026
Merged

Fix: Env-mediate expressions in run blocks#104
tykeal merged 1 commit into
lfreleng-actions:mainfrom
modeseven-lfreleng-actions:fix/zizmor-template-injection

Conversation

@ModeSevenIndustrialSolutions

Copy link
Copy Markdown
Contributor

Why

The org-wide zizmor workflow (lfreleng-actions/.github/workflows/zizmor.yaml) was changed today:

2026-07-28T10:46  Feat: Surface all zizmor findings at any level

It runs --persona auditor --min-severity informational and now fails a run on any finding at any level. This repo carries 10 pre-existing template-injection findings — the most of any repo I've looked at — so every new PR here is blocked until they're cleared, currently #103.

What

.github/workflows/testing.yaml expanded generate-sbom outputs directly inside run: blocks. GitHub expands ${{ }} before the shell sees the script, so a crafted value becomes shell source text rather than data. Routing through env: makes the value opaque to the shell parser.

Two steps were affected — the debug dump and the validation step. The validation step already declared an env: block for its matrix values, so the SBOM outputs simply join it:

env:
  MATRIX_DESCRIPTION: ${{ matrix.description }}
  MATRIX_REPOSITORY: ${{ matrix.repository }}
  EXPECTED_MANAGER: ${{ matrix.expected_manager }}
  SBOM_OUTCOME: ${{ steps.generate-sbom.outcome }}
  SBOM_MANAGER: ${{ steps.generate-sbom.outputs.dependency_manager }}
  ...

Same shape python-workflows uses (see its "Fix: Env-mediate expressions in run blocks"), and the env:-before-run: ordering already used elsewhere here.

On naming: python-workflows used SBOM_COMPONENT_COUNT / SBOM_DEPENDENCY_MANAGER. Here the step id is generate-sbom rather than sbom, which pushes those lines past the 80-column limit yamllint enforces in this repo. I shortened to SBOM_COUNT / SBOM_MANAGER rather than scatter # yamllint disable-line suppressions.

⚠️ Why this was written by hand

zizmor offers an auto-fix for this audit. It is marked unsafe, and on inspection it is: it rewrites expressions in place while leaving them inside their original quoting, and the shell does not expand ${...} inside single quotes. The original code was correct precisely because GitHub expanded the template before the shell ran; once the value moves to env:, expansion becomes the shell's job and the quoting has to change with it.

This repo's call sites happened to use double quotes so the auto-fix would have survived here — but it silently breaks sibling repos, so all of these were done by hand and re-verified.

The auto-fix also emitted a duplicated env: block on one step, which is a second reason not to take its output verbatim.

Validation

  • zizmor --persona auditor --min-severity informationalNo findings to report
  • pre-commit run --all-files → all hooks pass, including yamllint and actionlint
  • No behavioural change: the same values reach the same commands

Unblocks #103.

zizmor's auditor persona reported 10 template-injection findings in
testing.yaml: GitHub expressions expanded directly inside 'run:'
blocks, where expansion happens before the shell sees the script and a
crafted value could inject commands.

Two steps were affected, both reading outputs of the generate-sbom
step: the debug dump and the validation step.  The validation step
already declared an 'env:' block for its matrix values, so the SBOM
outputs join it there.

Names are kept short (SBOM_MANAGER, SBOM_COUNT) because the step id is
long enough that the fuller names push the lines past the 80-column
limit yamllint enforces here.

The org-wide zizmor workflow now fails a run on any finding at any
level, so these informational findings block every new pull request
in this repository.

zizmor offers an auto-fix for this audit, but it is marked unsafe and
on inspection it is: it rewrites the expression in place, leaving it
inside the original single quotes, where the shell will not expand it.
The original was correct because GitHub expanded the template before
the shell ever saw it.  These edits were therefore written by hand,
with double quotes, and follow the env-before-run shape already used
elsewhere in this repository.

No behavioural change: the same values reach the same commands.
Zizmor now reports no findings for the repository.

Co-authored-by: Claude <claude@anthropic.com>
Signed-off-by: Matthew Watkins <mwatkins@linuxfoundation.org>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR hardens the CI workflow against GitHub Actions template-injection findings by ensuring steps.generate-sbom.* expressions are not expanded directly inside run: scripts, instead passing them through step-scoped env: variables.

Changes:

  • Added step env: mappings for generate-sbom outcome and outputs in the SBOM debug step.
  • Added the same SBOM-related env: mappings to the validation step (joining its existing matrix env: block).
  • Updated the bash scripts to reference $SBOM_* variables instead of ${{ ... }} expressions.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@zxiiro zxiiro left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Auto-approved by agent: reviewed workflow/code change for security and CI/CD impact, found low risk. Mediates GitHub Actions expressions (step outputs) through env: vars instead of direct ${{ }} interpolation in shell run: steps, in test/CI workflow only; standard injection-hardening pattern, no new permissions or secrets.

@tykeal
tykeal merged commit ab83629 into lfreleng-actions:main Jul 28, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants