Skip to content

Make the zizmor check blocking - #6274

Merged
ChrisJBurns merged 3 commits into
mainfrom
enable-zizmor-gate
Aug 10, 2026
Merged

Make the zizmor check blocking#6274
ChrisJBurns merged 3 commits into
mainfrom
enable-zizmor-gate

Conversation

@ChrisJBurns

@ChrisJBurns ChrisJBurns commented Aug 10, 2026

Copy link
Copy Markdown
Collaborator

The last item in #6253. main is clear of medium-and-above findings, so the advisory mode can come off.

Summary

  • main has zero findings at medium or above — down from 111 findings overall, of which 33 were high or medium. --no-exit-codes and the job's continue-on-error were there to keep the job green while that backlog was worked through. Both are gone, and the check now fails on anything at medium or higher.
  • Switched to the official zizmorcore/zizmor-action, replacing a hand-rolled pipx run. The action resolves zizmor to a digest-pinned container image, so the artifact is pinned rather than just the version number — worth having for a check that is about to start blocking merges.
  • Gated at medium, not high. The plan said high, but medium is also at zero, so the stricter threshold is free. The three medium rules — secrets-inherit, ref-version-mismatch and excessive-permissions — are all worth blocking on. Dropping to high is a one-word change if it proves too tight.

Why two calls to the action rather than one

They cannot be combined, for a reason that is not obvious:

zizmor suppresses its findings exit code when the output format is SARIF — and SARIF is what advanced-security: true selects. Measured against a checkout of main with one medium finding injected, repeated three times each:

flags exit code
--format=sarif --min-severity=medium 0
--format=plain --min-severity=medium 13
--format=github --min-severity=medium 13

So the action's documented Advanced Security usage reports without gating. That is a sensible default for a scanner, but it means a single call cannot both upload SARIF and fail the build.

Hence:

Step advanced-security Severity Effect
Report findings true (default) all SARIF to the Security tab, cannot fail
Fail on new findings false medium and above human-readable output, gates

The first call reporting at every severity is deliberate: it keeps the low and informational findings visible in the Security tab even though the gate ignores them. The second sets advanced-security: false both to restore the exit code and to avoid uploading a second, severity-filtered SARIF over the first.

What is deliberately not blocked

24 findings remain, all low or informational, all in the four release workflows:

  • artipacked ×7 — checkouts in release jobs, in workflows no pull request can exercise.
  • template-injection ×17 — github.repository and a semver-validated version, neither of which can carry a shell metacharacter.

Lowering the threshold further means dealing with those first — #6275 makes a start. The trade is recorded in the job's comments, not just here.

Closes #6253

Type of change

  • Bug fix
  • New feature
  • Refactoring (no behavior change)
  • Dependency update
  • Documentation
  • Other (describe): CI configuration

Test plan

  • Unit tests (task test)

  • E2E tests (task test-e2e)

  • Linting (task lint-fix)

  • Manual testing (describe below)

  • Established the SARIF exit-code behaviour by measurement, not from documentation — three repeats of each format against a real checkout, as tabled above. It is the reason this is two steps, so it seemed worth being sure of.

  • Verified the gate actually bites. Injected a real regression — a pin with a mismatched version comment, a medium ref-version-mismatch — and confirmed zizmor reports it with file, line, the offending text and a fix hint, and exits 13. Reverted afterwards. A gate that has never been seen to fail is not a gate.

  • Confirmed the reporting run produces valid SARIF containing all 24 findings, so nothing disappears from the Security tab.

  • Confirmed the action pin 3dc1ecc9… is the commit v0.6.2 points to.

  • Confirmed the job no longer carries continue-on-error, and that the steps are ordered checkout → report → gate.

  • Workflow parses as YAML; actionlint clean.

  • This pull request exercises the change on itselfsecurity-scan.yml runs on every pull request, so the gate is running here.

Does this introduce a user-facing change?

Yes, for contributors: a pull request that introduces a medium-or-higher workflow finding will now fail CI rather than reporting quietly. That is the intent.

Special notes for reviewers

  • The threshold is the decision worth making here, not the mechanics. medium costs nothing today; high is more forgiving of future churn. I would rather start strict and relax if it annoys, but it is a team call.
  • The action pulls its zizmor image by digest from a version map inside the action repo, so pinning the action pins the scanner too. Bumping version: is only safe to a version that map knows; the action fails loudly on an unknown one.
  • If the gate ever fires on something that genuinely cannot be fixed, the established pattern is a # zizmor: ignore[rule] comment carrying the reasoning — see claude.yml and api-compat.yml for existing examples.

Generated with Claude Code

main is clear of medium-and-above findings, so the advisory mode can
come off: --no-exit-codes and the job's continue-on-error are gone and
the check fails on anything at medium or higher.

The reporting run is kept separate from the gate. It still emits SARIF
at every severity so the Security tab keeps the low and informational
findings the gate does not block on, and still writes them to file
rather than the log. The gate prints, because anything it reports was
introduced by the change under review and is already visible in its
diff, and a gate that fails without saying why is not worth having.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@ChrisJBurns
ChrisJBurns requested a review from JAORMX as a code owner August 10, 2026 20:42
@github-actions github-actions Bot added the size/XS Extra small PR: < 100 lines changed label Aug 10, 2026
@codecov

codecov Bot commented Aug 10, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 72.71%. Comparing base (06483ba) to head (bf25ff8).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6274      +/-   ##
==========================================
- Coverage   72.73%   72.71%   -0.03%     
==========================================
  Files         742      742              
  Lines       77299    77325      +26     
==========================================
+ Hits        56224    56227       +3     
- Misses      17113    17135      +22     
- Partials     3962     3963       +1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

pipx resolved zizmor from PyPI at run time. The version was pinned but
the artifact behind it was not, which is worth tightening for a check
that is about to start blocking merges.

Both steps now run the official container pinned by digest. A tag can be
repointed; a digest cannot. The digest is the one zizmorcore/zizmor-action
resolves 1.29.0 to, confirmed against the registry and by running the
image, so it is the artifact upstream publishes and vets.

The action itself was considered and not used: it tees its output, which
would put the full finding inventory into a world-readable log, and its
SARIF upload is skipped when the run fails, so findings would not reach
the Security tab on exactly the pull requests that have them.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added size/XS Extra small PR: < 100 lines changed and removed size/XS Extra small PR: < 100 lines changed labels Aug 10, 2026
The hand-rolled container invocation was replaced by two calls to
zizmorcore/zizmor-action, which pins the zizmor image by digest itself,
so the pinning that motivated the container approach is kept without the
bespoke shell.

Reporting and gating stay separate because the action cannot do both in
one call: zizmor suppresses its findings exit code when the format is
SARIF, which is what advanced-security selects. So the first call
reports at every severity and uploads, and cannot fail; the second sets
advanced-security false to restore human-readable output and the exit
code, and gates at medium.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added size/XS Extra small PR: < 100 lines changed and removed size/XS Extra small PR: < 100 lines changed labels Aug 10, 2026
@ChrisJBurns

Copy link
Copy Markdown
Collaborator Author

/retest

@ChrisJBurns
ChrisJBurns merged commit ef5bb19 into main Aug 10, 2026
77 of 78 checks passed
@ChrisJBurns
ChrisJBurns deleted the enable-zizmor-gate branch August 10, 2026 22:32
@github-actions github-actions Bot mentioned this pull request Aug 14, 2026
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/XS Extra small PR: < 100 lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

zizmor findings backlog: pins, expressions and checkout credentials

2 participants