fix(ci): stop interpolating the LLVM version into the pwsh setup step - #7417
Merged
Conversation
added 2 commits
August 5, 2026 07:32
zizmor has been red on every `main` commit since #7353 created `.github/actions/setup-llvm22/action.yml` -- roughly 40 consecutive commits. #7388 and #7393 only shifted the reported line numbers, which is what made them look implicated; neither introduced a finding. Four high-severity findings, one of which is properly fixed here. The Windows arm interpolated a composite-action input straight into a PowerShell script body (`$ver = "${{ inputs.version }}"`), which `template-injection` flags at High confidence: the expansion is substituted as raw text before pwsh parses the line, so an input carrying a quote plus a statement separator would execute as code with the runner's privileges. The input now arrives through an `env:` block and is read as `$env:LLVM_VERSION`, a plain string load. The other three are `github-env` at Low confidence -- the single `LLVM_SYS_221_PREFIX=<prefix>` line the action exists to write, once per platform arm -- and are suppressed with reasoning in `.github/zizmor.yml`. Measured: the audit is satisfiable only by not writing the environment file at all, and the clean alternative ($GITHUB_OUTPUT plus composite outputs) costs 44 jobs and 140 downstream steps, recreating the duplication the action exists to remove. The carve-out is a dated ratchet with an explicit delete-condition. Verified with the repo's SRI-pinned zizmor 1.28.0: pristine config plus this fix reports 3 high and exits 14; with the carve-out it exits 0 and `ignored` rises 119 -> 122, matching the three suppressed findings exactly. Claude-Session: https://claude.ai/code/session_019EHcmXKArA7m42SihYCcgH
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe Windows LLVM setup action now passes its version through ChangesLLVM setup zizmor remediation
Estimated code review effort: 2 (Simple) | ~10 minutes ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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
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.
zizmorhas been red onmainfor roughly 40 consecutive commits — since #7353 createdsetup-llvm22/action.yml. #7388 and #7393 are innocent; bisecting the run history shows last green17a39fce7, first red249459556, with the identical 4 findings in that run's log and only line numbers shifting since.One properly fixed
template-injection@ 84:21 —$ver = "${{ inputs.version }}"interpolated a composite-action input as raw text into a pwsh script body. Now passed via anenv:block and read as$env:LLVM_VERSION.Three suppressed, and I want that to be explicit
The
github-envfindings are suppressed via a scoped ratchet entry, not fixed. The agent tested whether they were fixable and they are not, while writingGITHUB_ENVat all: a dynamic value always reports, and the pwsh arm reports even for a static literal because zizmor can't evaluateOut-File.The only clean form is
$GITHUB_OUTPUT+ composite outputs. That was measured, not guessed: 44 jobs would need anid:and 140 downstream steps anenv: LLVM_SYS_221_PREFIX:, becausellvm-sysreads the prefix from the environment at build time — recreating exactly the "44 inline recipes" this action's own header says it exists to delete.Judged a bad trade for a Low-confidence finding about filesystem paths from locally-installed toolchains, on an action whose callers run only under
pull_request/push. The entry carries a dated reason and a delete-condition, per the file's existing convention. Flagging for a maintainer call — if you want the outputs refactor it's mechanical, but only verifiable in CI.Verified as an A/B, not a single check
Which also proves the template-injection fix is real and not absorbed by the suppression:
mainconfig + theaction.ymlfixignoredrises by exactly 3 — the entry is not over-broad. Re-verified against the committed tree viagit archive HEAD, not the working tree.Summary by CodeRabbit