Skip to content

Suppress four zizmor findings that cannot be fixed - #6262

Merged
ChrisJBurns merged 1 commit into
mainfrom
suppress-mitigated-zizmor-findings
Aug 10, 2026
Merged

Suppress four zizmor findings that cannot be fixed#6262
ChrisJBurns merged 1 commit into
mainfrom
suppress-mitigated-zizmor-findings

Conversation

@ChrisJBurns

Copy link
Copy Markdown
Collaborator

Summary

  • Four findings will never be cleared by a code change, so they would block a gate forever. Two flag a trigger that cannot be dropped; two flag credentials that are deliberately kept. Suppressing them is the only way the gate can ever be turned on honestly.
  • Each suppression carries its reasoning inline, next to the code. A future sweep will see why the line was left alone; nobody reads the tracking issue before editing a workflow.

This changes no behaviour whatsoever — the diff is comments only.

File Rule Why it can't be fixed
pr-size-label-apply.yml dangerous-triggers Applying a label needs write access the calculating workflow deliberately lacks, so workflow_run is required. What made it dangerous — reading the target pull request number and the label out of an artifact a pull request author controls — was fixed in #6259; neither is read from the artifact now.
release-notes.yml dangerous-triggers claude-code-action rejects the release event outright, so workflow_run is the only option. The risk this audit describes is a privileged workflow checking out an event-supplied ref; this one deliberately checks out the default branch, with the reasoning already documented on the step.
claude.yml artipacked The job holds contents: write specifically so Claude can push commits to a pull request, and that push uses the credential actions/checkout leaves behind. persist-credentials: false would break the action.
api-compat.yml artipacked Cannot push (contents: read), so exposure is limited, and it runs git fetch origin refs/tags/... — which succeeds unauthenticated only because this repository is public. It is also the repository's only required status check, so breaking it blocks every pull request. Not worth it for one low-severity finding.

Effect

High Medium Low Info Total
Before 10 4 9 24 47
After 8 4 7 24 43

More useful than the count: all 8 remaining High findings are now in the four release workflowsreleaser.yml (4), helm-publish.yml (2), create-release-pr.yml (1), create-release-tag.yml (1). Nothing High is left anywhere else, so what stands between here and a --min-severity=high gate is now a single, clearly bounded decision.

Part of #6253

Type of change

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

Test plan

  • Unit tests (task test)

  • E2E tests (task test-e2e)

  • Linting (task lint-fix)

  • Manual testing (describe below)

  • Verified the suppression syntax on throwaway copies before writing it into the workflows: each file went from reporting its finding to reporting none.

  • Confirmed the comment placement, which is not where the finding is reported. dangerous-triggers is reported against on: but is only suppressed by a comment on the workflow_run: line beneath it.

  • All workflows parse as YAML; actionlint reports 12 findings before and after, all pre-existing.

  • Confirmed the counts above and that the 8 survivors are the release-workflow findings.

Does this introduce a user-facing change?

No.

Special notes for reviewers

  • The two artipacked suppressions are not strictly required for a --min-severity=high gate, since they are Low. They are here because the reasoning is worth recording where it will actually be read — particularly claude.yml, where the obvious "fix" breaks the action.
  • Worth scrutinising the two dangerous-triggers justifications specifically. A suppression is only as good as its argument, and if either reads as hand-waving it should be challenged rather than merged.

Generated with Claude Code

Two dangerous-triggers findings flag the workflow_run trigger itself,
which neither workflow can drop: applying a label needs write access the
calculating workflow deliberately lacks, and claude-code-action rejects
the release event. What made the first dangerous was trusting the
artifact for the target pull request and the label, and neither is read
from it any more. The second checks out the default branch rather than
an event-supplied ref.

Two artipacked findings are on checkouts that keep their credentials on
purpose. claude.yml has contents: write so Claude can push commits, and
that push uses the credential. api-compat.yml cannot push, but it runs
a git fetch and is the only required status check, so dropping it risks
blocking every pull request to clear one low-severity finding.

Each suppression carries the reasoning inline, so the decision sits next
to the code rather than only in an issue.

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 17:41
@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.69%. Comparing base (d163115) to head (c962332).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6262      +/-   ##
==========================================
- Coverage   72.71%   72.69%   -0.02%     
==========================================
  Files         742      742              
  Lines       77299    77299              
==========================================
- Hits        56209    56194      -15     
- Misses      17126    17142      +16     
+ Partials     3964     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.

@ChrisJBurns
ChrisJBurns merged commit f66d0f5 into main Aug 10, 2026
46 checks passed
@ChrisJBurns
ChrisJBurns deleted the suppress-mitigated-zizmor-findings branch August 10, 2026 19:07
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.

2 participants