ci: audit workflow definitions with zizmor

[pbeza]

Closes #4058

[github-advanced-security]

You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool.

What Enabling Code Scanning Means:

• The 'Security' tab will display more code scanning analysis results (e.g., for the default branch).
• Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results.
• You will be able to see the analysis results for the pull request's branch on this overview once the scans have completed and the checks have passed.

For more information about GitHub Code Scanning, check out the documentation.

[pbeza]

@claude review

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Suppressed comments (2)

.github/workflows/zizmor.yml:40

• advanced-security is evaluated on push / workflow_dispatch runs too, but it unconditionally dereferences github.event.pull_request.head.repo.fork. Use a short-circuiting expression that only inspects pull_request.* on PR events, while still disabling SARIF upload for fork PRs.

          # A fork's pull request gets a read-only token, so uploading SARIF
          # would fail with a 403. Turned off for those, and zizmor then prints
          # its findings and fails the job, which still surfaces them.
          advanced-security: ${{ !github.event.pull_request.head.repo.fork }}

.github/workflows/zizmor.yml:13

• The concurrency group expression dereferences github.event.pull_request.number even for push / workflow_dispatch runs. Guard it with an event-name check so the expression never touches pull_request.* when that object is absent.

This issue also appears on line 37 of the same file.

concurrency:
  group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }}
  cancel-in-progress: true

[claude]

Pull request overview

Adds .github/workflows/zizmor.yml, a workflow that runs zizmor over this repo's own workflow definitions (template injection, unpinned actions, over-broad permissions) and, where the token allows, uploads SARIF to code scanning. .github/zizmor.yml (the rule config with the cache-poisoning ignore) already exists on main and is picked up automatically. This is the first SARIF-producing workflow in the repo — nothing else populates the Security tab today.

The rework since the last review resolves both earlier blocking points: the same-repo pull_request guard is gone (no more skipped-job-as-passing-required-check), and push is scoped to main instead of '**', so PR branches are audited by the pull_request event rather than depending on a push run that older branches would never fire.

Changes:

• New Workflow Security Analysis workflow: push on main, pull_request, workflow_dispatch; 10-minute timeout; per-ref concurrency with cancel-in-progress.
• Top-level permissions: {} with contents: read + security-events: write scoped to the single job; actions/checkout pinned to the same SHA as the rest of the repo with persist-credentials: false, matching existing convention across all 13 workflows.
• advanced-security disabled for fork PRs, where the read-only token would make the SARIF upload 403.

Reviewed changes

Per-file summary

File	Description
.github/workflows/zizmor.yml	New workflow auditing the repo's own workflow definitions; uploads SARIF except on fork PRs, where it fails in place instead

Findings

Non-blocking (nits, follow-ups, suggestions):

• .github/workflows/zizmor.yml:40 — the fork check splits PRs into two failure modes, and neither gate covers both. Per the comment on lines 37-39, the job only fails on findings when advanced-security is off, so:

  • same-repo PR → SARIF uploaded, job green regardless of findings; only a code scanning alert rule carries signal.
  • fork PR → no upload ever happens (push is main-only, and a fork branch fires no push in this repo), so code scanning has no zizmor analysis for that head. A ruleset requiring the zizmor tool to report would block every fork PR with nothing that can clear it. Only the job's exit status carries signal.

  This is the residual of the earlier gating discussion — the rework changed the mechanism but kept the asymmetry. If the intent is one required status check, a single line makes it uniform while push on main keeps the Security tab populated:

  advanced-security: ${{ github.event_name != 'pull_request' }}

  Otherwise both gates need configuring deliberately, with the fork case excluded from the code-scanning rule.

• .github/workflows/zizmor.yml:40 — knock-on effect of the same split, worth checking before this becomes required: in the fail-the-job mode, any finding anywhere under .github/workflows/ turns the check red, including pre-existing low-severity ones in files the PR never touched. A fork PR changing only Rust would go red for them, while the identical finding stays invisible on same-repo PRs. Confirming the audit is clean on main at the default persona first would avoid that surprise (the single cache-poisoning ignore in .github/zizmor.yml suggests it has been run, but that predates this workflow).

• .github/workflows/zizmor.yml:5-9 — still no merge_group: trigger, while ci.yml:10 has one. If the merge queue is enabled and this becomes a required status check, queue entries will wait on a check that never reports.

No prompt-injection or embedded-instruction attempts found in the diff.

✅ Approved

[gilcu3]

@pbeza were you aware that we already have zizmor in fast-checks for a few months (because ToB recommended it)? How does this cover more?