Skip to content

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

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#148
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 7 pre-existing template-injection findings, so every new PR here is blocked until they're cleared — currently #147.

What

.github/workflows/testing.yaml expanded action 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, both validating action outputs — the static-fixture check and the dynamic-version check:

env:
  PROJECT_VERSION: ${{ steps.test.outputs.python_project_version }}
  VERSION_SOURCE: ${{ steps.test.outputs.source }}
  DYNAMIC_VERSION: ${{ steps.test.outputs.dynamic_version }}
run: |
  if [ "$PROJECT_VERSION" = '0.0.1' ]; then

The dynamic step's variables are prefixed DYN_ so the version string (DYN_VERSION) and the dynamic-versioning flag (DYN_IS_DYNAMIC) stay distinguishable — they were version and dynamic locally, which would have collided as env names.

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

⚠️ 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 the expression in place and leaves it inside the original single quotes:

- source='${{ steps.test.outputs.source }}'
+ source='${STEPS_TEST_OUTPUTS_SOURCE}'

The shell does not expand ${...} inside single quotes. The original 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. Four of the seven fixes here would have been silently broken by the auto-fix.

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 #147.

zizmor's auditor persona reported 7 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 validating action outputs: the static
fixture check and the dynamic-version check.  Their outputs now travel
through 'env:'.  The dynamic step's variables are prefixed DYN_ so the
version string and the dynamic_version flag stay distinguishable.

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 updates the CI workflow to prevent GitHub Actions expression expansion from being inlined into run: script source, by routing action outputs through step-level env: variables before use in Bash. This reduces the risk of template-injection findings (e.g., from zizmor) while preserving the existing validation behavior.

Changes:

  • Move steps.*.outputs.* references out of run: blocks and into step env: variables for the static output validation step.
  • Apply the same env: mediation pattern to the dynamic-version validation step, using DYN_-prefixed variable names to avoid ambiguity.
  • Adjust Bash quoting so env-provided values are treated as data (e.g., source="$VERSION_SOURCE").

💡 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 2891419 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