Skip to content

ci: fix zizmor template-injection findings blocking all open PRs - #3003

Closed
mstruebing wants to merge 1 commit into
mainfrom
ci/fix-zizmor-template-injection
Closed

ci: fix zizmor template-injection findings blocking all open PRs#3003
mstruebing wants to merge 1 commit into
mainfrom
ci/fix-zizmor-template-injection

Conversation

@mstruebing

Copy link
Copy Markdown
Member

The zizmorcore/zizmor-action step fails on every open dependabot PR (#3002, #2997, #2996). The bumps are not the cause — the findings are pre-existing on main and reproduce against an unmodified checkout.

Cause

release.yml interpolates ${{ github.event.inputs.releaseVersion }} and ${{ github.ref_name }} directly into run: blocks (lines 77 and 84, added in #2911). zizmor's template-injection audit rates both high, so the job exits 14.

Those findings reached main because the zizmor job lived in test.yml behind a paths: filter covering only src/**, testdata/**, the package/tsconfig/eslint files and test.yml itself. A change to release.yml therefore could not trigger the lint that audits release.yml, and the regression merged green. Every later PR that does touch a filtered path — i.e. every dependabot npm PR — then failed on findings it did not introduce.

The differing errors between PRs are the same root cause seen against different bases: #2996/#2997 ran while deploy-docs-v2.yml still existed and additionally reported excessive-permissions on its workflow-level contents: write; that file has since been removed, so #3002 shows only the two release.yml findings.

Changes

  • release.yml — pass both expressions through env: and reference them as shell variables, matching the pattern already used by the Tag release and Push tag steps in the same file. No behaviour change.
  • zizmor.yml (new) — move the job out of test.yml and run it on all pushes and pull requests with no paths: filter, so a workflow change can no longer skip the lint that guards it. The job name is unchanged, and only EasyCLA is a required status context, so no required check is affected.

Verification

Ran the exact image CI uses (zizmor v1.29.0, --persona pedantic --min-severity medium) against the tree before and after:

before: 15 findings (13 ignored, 2 unsafe fixes): 0 medium, 2 high  → exit 14
after:  No findings to report. Good job! (14 ignored)               → exit 0

Notes for the reviewer

  • The commit message at release.yml:84 uses github.ref_name, which on a workflow_dispatch run is the dispatched branch — so the message reads docs: snapshot version main rather than the release version. Preserved as-is here since it is a behaviour question, not a lint one, but it looks unintended.
  • zizmor-action still resolves the zizmor binary as version: latest (its default), so a future zizmor release adding audits can surface new findings on unrelated PRs. Pinning would trade that off against the lint going stale; left unchanged, but worth a decision.

🤖 Generated with Claude Code

…ippable

The release workflow interpolated `${{ github.event.inputs.releaseVersion }}`
and `${{ github.ref_name }}` directly into `run:` blocks, which zizmor's
template-injection audit reports as high severity. Pass both through `env:`
and reference them as shell variables instead, matching the pattern already
used by the "Tag release" and "Push tag" steps in the same file.

These findings reached main because the zizmor job lived in test.yml behind a
`paths:` filter that did not cover the workflow files zizmor audits, so the
commit that introduced them never ran the lint. Every later PR touching a
filtered path then failed on the pre-existing findings.

Move the job to its own workflow that runs on all pushes and pull requests
with no `paths:` filter, so a change to a workflow can no longer skip the
lint that guards it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@mstruebing mstruebing self-assigned this Aug 12, 2026
@kubernetes-prow kubernetes-prow Bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Aug 12, 2026
@linux-foundation-easycla

Copy link
Copy Markdown

CLA Missing ID

  • ✅ login: mstruebing / name: Max Strübing (be59233)
  • ❌ The email address for the commit (be59233) is not linked to the GitHub account, preventing the EasyCLA check. Consult this Help Article and GitHub Help to resolve. (To view the commit's email address, add .patch at the end of this PR page's URL.) For further assistance with EasyCLA, please visit our EasyCLA portal and chat with our support bot.

One or more co-authors of this pull request were not found. You must specify co-authors in commit message trailer via:

Co-authored-by: name <email>

Supported Co-authored-by: formats include:

  1. Anything <id+login@users.noreply.github.com> - it will locate your GitHub user by id part.
  2. Anything <login@users.noreply.github.com> - it will locate your GitHub user by login part.
  3. Anything <public-email> - it will locate your GitHub user by public-email part. Note that this email must be made public on Github.
  4. Anything <other-email> - it will locate your GitHub user by other-email part but only if that email was used before for any other CLA as a main commit author.
  5. login <any-valid-email> - it will locate your GitHub user by login part, note that login part must be at least 3 characters long.

Alternatively, if the co-author should not be included, remove the Co-authored-by: line from the commit message.

Please update your commit message(s) by doing git commit --amend and then git push [--force] and then request re-running CLA check via commenting on this pull request:

/easycla

@kubernetes-prow kubernetes-prow Bot added the cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. label Aug 12, 2026
@kubernetes-prow

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mstruebing

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@kubernetes-prow kubernetes-prow Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 12, 2026
@mstruebing mstruebing closed this Aug 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant