Skip to content

ci(zizmor): opt into the severity gate — block high-severity findings - #225

Merged
kyle-sexton merged 1 commit into
mainfrom
chore/zizmor-opt-into-gate
Jul 21, 2026
Merged

ci(zizmor): opt into the severity gate — block high-severity findings#225
kyle-sexton merged 1 commit into
mainfrom
chore/zizmor-opt-into-gate

Conversation

@kyle-sexton

Copy link
Copy Markdown
Contributor

Adopt the new zizmor SARIF-guard gating shipped in ci-workflows #183. The zizmor lane now runs --format=sarif through the in-lane guard and blocks on any high-severity (SARIF error) workflow-security finding — template-injection, dangerous-triggers, unpinned-uses, etc. — while lower severities still only annotate. Gating rides the existing required ci-status lane: no code-scanning, no security-events grant, no org-wide ruleset change.

Changes

  • ci.yml: bump the zizmor.yml pin to 31a5b76 and set fail-on-severity: high; comment updated.
  • components/runner-policy/policy.json: register the new runner-input contract for zizmor.yml@31a5b76 with fail-on-severity in allowedInputsrunner-policy would otherwise reject the new pin + input (its lane is in ci-status).

Safety

Pre-flight zizmor --persona=regular (online) on this repo → 0 high findings (3 notes: 2 adhoc-packages, 1 use-trusted-publishing). The high gate does not block existing workflows. lint:runner-policy + test:runner-policy (227/227) pass locally.

Related

No related issue: rolls out the consumer-side opt-in for the gating capability added in melodic-software/ci-workflows#183. The 3 remaining note-level findings are a separate follow-up (documented-ignore / OIDC trusted-publishing), below this gate's threshold.

Adopt the new zizmor SARIF-guard gating from ci-workflows (#183). The zizmor
lane now runs `--format=sarif` through the in-lane guard and BLOCKS on any
high-severity (SARIF error) workflow-security finding — template-injection,
dangerous-triggers, unpinned-uses, etc. — while lower severities still only
annotate. Gating rides the existing required ci-status lane: no code-scanning,
no security-events grant, no org-wide ruleset change.

- ci.yml: bump the zizmor.yml pin to 31a5b76 and set `fail-on-severity: high`.
- policy.json: register the new runner-input contract for zizmor.yml@31a5b76
  with `fail-on-severity` in allowedInputs (runner-policy would otherwise
  reject the new pin + input).

Pre-flight `zizmor --persona=regular` (online) on this repo reports 0 high
findings (3 notes: 2 adhoc-packages, 1 use-trusted-publishing), so the gate
does not block existing workflows. lint:runner-policy + test:runner-policy pass.

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

@claude

claude Bot commented Jul 21, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 1m 7s —— View job


PR Review

  • Gather context (CLAUDE.md, REVIEW.md)
  • Read changed files
  • Analyze diff
  • Post findings

The changes are minimal and well-scoped. Both files are updated atomically — the new pin is introduced in ci.yml and immediately registered in policy.json, which is the correct order of operations (runner-policy would reject an unregistered pin/input combination).

No critical or important findings.


🟡 Nit — stale policy.json entry for the old pin

components/runner-policy/policy.json L369–374

The zizmor.yml@90f1c54... entry is now orphaned: ci.yml no longer references that pin, so this allowlist slot will never be exercised. It's not a security problem (an unused allowlist entry is inert), but it's dead config that will accumulate if not pruned on each pin bump. Removing it in a follow-up (or in this PR) keeps policy.json as a live index of what's actually called rather than a historical ledger.

Fix this →


Everything else checks out:

  • Pin format is consistent with the rest of ci.yml (full SHA + short hash + date comment). ✓
  • fail-on-severity: high is explicitly whitelisted in allowedInputs — runner-policy won't reject it. ✓
  • The new policy entry mirrors the existing zizmor entry structure (same routing, runnerInput, allowedSecrets: {}), with fail-on-severity added. ✓
  • contents: read permissions are unchanged — the gating is purely in-lane with no elevated grant needed. ✓
  • The updated comment in ci.yml accurately describes the new behavior and the design constraint (no code-scanning, no security-events grant). ✓

@kyle-sexton
kyle-sexton merged commit 2ba57de into main Jul 21, 2026
42 checks passed
@kyle-sexton
kyle-sexton deleted the chore/zizmor-opt-into-gate branch July 21, 2026 01:26
kyle-sexton added a commit that referenced this pull request Jul 21, 2026
Burn down the note-level zizmor findings that were annotating every PR
(all below the `fail-on-severity: high` gate added in #225).

- `ci.yml:404,589` (`adhoc-packages`): the two exact-pinned `npm install
-g` tsc/biome steps install toolchains on PATH so the contract tests can
drive the tools directly — deliberate and documented. Add inline `#
zizmor: ignore[adhoc-packages] <rationale>`.
- `publish-packages.yml:83` (`use-trusted-publishing`): a real security
recommendation (drop the long-lived token for OIDC). Left visible and
tracked in #226 — the proper fix needs npm-org trusted-publisher config.

After this, `zizmor --persona=regular` reports only the single tracked
note.

## Related
No related issue: annotation-noise burn-down following the gate rollout
(#225, ci-workflows#183). Deferred proper fix tracked in #226.

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
kyle-sexton added a commit that referenced this pull request Jul 23, 2026
…ter v0.8.0 selector (#252)

No related issue: enforcer support for merged
melodic-software/ci-workflows v0.8.0 (#224, #225); unblocks the caller
repin + ancillary-events opt-in in melodic-software/medley.

## Related

ci-workflows#224, ci-workflows#225

## Summary

- Adds `optionalBooleanSelectorInputs` to the runner-policy schema and
enforcer so boolean selector opt-ins can be governed; the only accepted
value is the literal `true` (`booleanTrueMap` uses `const: true`).
- Registers `admits-ancillary-events` and the deprecated alias
`admits-comment-events`. These let a caller opt its no-checkout
`issue_comment` / `pull_request_review` / `pull_request_review_comment`
/ `issues` jobs into self-hosted fleet routing (ci-workflows #224,
#225).
- Registers the ci-workflows v0.8.0 `select-runner` revision
`e77f0126b474144708719f99795e44d0ffe2541d`, owner-scoped to
`melodic-software`, so consumers can repin and activate the opt-in.
- The boolean map merges into `exactCanonicalMap`'s optional-input
argument, reusing the existing fail-closed exact-match; a load-time
duplicate-name guard keeps the optional string and boolean maps
disjoint.

## Security posture

The enforcer *permits*, but does not *verify*, the caller's no-checkout
declaration — that premise is trusted under the same job-step review
that backs a local-routing grant. No existing job-level control is
relaxed (statically-read-only-or-grant, privileged-hosted,
container/service, runner-target all still apply); the opt-in only
widens which event types the selector may route self-hosted. Captured as
a new THREAT-MODEL threats-table row and residual-risk bullet, and in
the component README's selector-input contract. Fleet-admission-logic
security review pending per team lead before merge.

## Test plan

- [x] `npm run test:runner-policy` — 238/238 pass (10 new cases: accept
`true`, accept alias, reject `false`, reject quoted `"true"`, reject
unknown input, owner-scoped v0.8.0 accept, unapproved-revision reject,
schema reject non-`true`, duplicate optional-string name, duplicate
canonical name)
- [x] `npm run lint:runner-policy` — Runner policy passed (dogfood with
`GITHUB_REPOSITORY` owner evidence)
- [x] `npm run lint:md` — 0 errors
- [x] `npm run test:packages` — 14/14 pass
- [x] `npm run lint:hooks` — All good
- [x] `npm run test:lefthook-dotnet` — 12/12 pass
- [x] `bash harness/shell/run-tests.sh harness/shell/lib.test.sh` —
passed
- [x] `npx biome check` on changed JS/JSON — no fixes needed
- [x] Independent security review (fresh reviewer, rationale withheld) —
correct and fail-closed, no P1–P3 findings; the two P4 coverage gaps it
raised (quoted-string value, canonical duplicate-name branch) are
addressed here

Do NOT merge — held for the fleet-admission security review.

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant