feat(zizmor): severity-tiered SARIF gate (opt-in, advisory default) by kyle-sexton · Pull Request #183 · melodic-software/ci-workflows

kyle-sexton · 2026-07-21T00:59:35Z

Bring zizmor onto the constellation's SARIF-guard pattern (as used by osv-scanner + govulncheck): run --format=sarif, parse it in a bash guard that emits annotations for every finding and exits nonzero when any finding's severity is at or above a caller-chosen threshold. Lets a consumer make high-severity workflow-security findings BLOCK via their existing ci-status lane — no code-scanning, no security-events: write, no org-wide ruleset change.

What changed

New input fail-on-severity (never|low|medium|high, default never — preserves today's advisory behavior). fail-on-findings: true kept as a back-compat alias (== fail-on-severity: low).
--format=github → --format=sarif; new .github/scripts/zizmor-sarif-guard.sh reuses osv-scan-guard.sh's hardened annotation machinery (base64/escape/normalize_sarif_uri path defense), parameterized by level. Level mapping (source- + empirically-verified at zizmor 1.27.0): high→error, medium→warning, low|info→note.
render-zizmor-sarif-guard.cjs + # BEGIN/END GENERATED ZIZMOR SARIF GUARD markers materialize the guard into zizmor.yml (mirrors render-osv-scan-guard.cjs; --check sync-verified).
zizmor-sarif-guard.test.sh (17 cases) + revised zizmor-native.test.cjs; wired into ci.yml + selector-conformance.yml. README zizmor section corrected.

Verification

zizmor-sarif-guard.test.sh 17/17; node --test 5/5 (zizmor-native); render --check in sync; shellcheck + shfmt clean; actionlint clean.
Independent fresh-context security review: SHIP — fail-open + fail-closed both refuted against zizmor v1.27.0 primary sources; annotation injection neutralized empirically.

Provenance pinning is strong. The SARIF schema validation checks semanticVersion == $version before any finding is processed (zizmor-sarif-guard.sh lines 36–53). Combined with the symlink guard (-L "$ZIZMOR_SARIF") and the fail-closed behavior on a nonzero exit code, the chain resists SARIF file substitution and ensures only the reviewed version's output is trusted.

Unknown SARIF levels fail closed. The provenance validator requires every result to have level ∈ {error, warning, note}. If a future zizmor version introduces a new level (e.g. none, which is valid in the SARIF spec), the guard exits 2 rather than silently treating it as error via the catch-all branch in annotate_findings. This is the right tradeoff — the unknown-level test case confirms it.

Token lifecycle is correct. The GH token is saved to a local variable, ZIZMOR_TOKEN unset, then only forwarded as GH_TOKEN to the checksum-verified binary. unset token runs before the guard block, so the guard never sees the token.

Annotation injection protection matches osv-scan-guard. Base64 encoding in jq, printable-character pre-check, decode_base64_field using the \036 sentinel trick, escape_property/escape_data for workflow command encoding — all identical to the already-reviewed pattern.

Path traversal in normalize_sarif_uri is covered. realpath -e resolves symlinks before the workspace-containment check, the %00 null-byte check, the control-character grep, and the ?/# rejections collectively block the known SARIF URI injection vectors.

Correctness

jq index() at position 0 is truthy. Worth calling out explicitly: select(.level as $l | $levels | index($l)) works correctly even when the matching level is at index 0, because jq treats 0 as truthy (only false and null are falsy). A JavaScript reader might flag this as a bug — it isn't.

Annotation cap vs. blocking count. annotate_findings caps output at 50 (line 165), but blocking_count scans all results. This is correct: the gate is over all findings, annotations are bounded to avoid flooding. However, a run with >50 blocking findings will report "N findings at or above severity X" where N > 50 while the user sees only 50 annotations. The rule IDs in the error message compensate, but it's worth knowing about if zizmor ever produces verbose output. Not a change request — the OSV guard makes the same tradeoff.

effective_severity resolution is correct. fail-on-severity above never wins; fail-on-findings: true maps to low only when fail-on-severity is never. The guard then receives the resolved value, so the second case "$FAIL_ON_SEVERITY" block inside the guard is always reachable from the standalone invocation path and always passes from the inline path — intentional defense-in-depth, not dead code.

The renderer roundtrip is stable. bundledScript maps empty lines to "" (no trailing spaces), uses trimEnd() before splitting, and render slices exactly at the boundary markers. The --check assertion in zizmor-native.test.cjs confirms in-CI that the inlined copy matches the source script.

Minor observations (no change required)

[.runs[0].results[]][:50][] is functionally equivalent to .runs[0].results[:50][]. The extra array constructor is a style artifact from the pattern in osv-scan-guard.sh — consistent, fine to leave as-is.
The # shellcheck shell=bash directive on line 1 of zizmor-sarif-guard.sh appears inside a run: block in the inlined copy. actionlint forwards embedded scripts to shellcheck with the correct shell context, so this is harmless and accurate.
selector-conformance.yml path filters include all three new scripts — no gap.

CLAUDE.md checklist

Rule	Status
No `pull_request_target` / `workflow_run` with secrets	✅
`permissions: contents: read` only, no `write`	✅ (no `security-events: write`)
All `uses:` SHA-pinned with version comment	✅
No credential echo / `ACTIONS_STEP_DEBUG`	✅
No untrusted checkout before action step	✅

Verdict

Ship. The implementation is correct, the security properties are well-reasoned and tested, and the advisory default preserves backward compatibility. The observations above are informational — none warrant a blocking change.

claude · 2026-07-21T01:03:16Z

Claude finished @kyle-sexton's task in 3m 33s —— View job

Security Review — PR #183

Tasks

Load tools
Read PR diff and changed files
Review shell script for injection surfaces
Review SARIF parsing for path traversal / injection
Review GitHub Actions workflow for dangerous patterns
Review permissions, secrets handling
Post findings

Findings

The overall design is well-hardened. The annotation injection defenses (base64 transport, uri_safe/message_safe boolean gates, escape_property, escape_data, normalize_sarif_uri with realpath -e confinement) are correctly applied. Token handling follows least-privilege: ZIZMOR_TOKEN is unset before the curl download and only passed as a scoped env var to the verified binary. No dangerous triggers (pull_request_target, workflow_run+secrets) or permission widening are present.

Two findings, both suggestions:

SUGGESTION — Confidence: Medium

$ids inserted into annotation without escaping

zizmor-sarif-guard.sh L202–L205

ids="$(jq -r --argjson levels "$blocking_levels" '
  [.runs[0].results[] | select(.level as $l | $levels | index($l)) | .ruleId] | unique | join(", ")
' "$ZIZMOR_SARIF")"
echo "::error::zizmor reported $blocking_count finding(s) at or above severity $FAIL_ON_SEVERITY: $ids"

The ruleId values are validated only as non-empty strings (no character-set restriction in the provenance schema check). jq -r expands JSON escape sequences before joining, so a ruleId containing a literal \n in the JSON document would produce a newline in ids. GitHub Actions processes each stdout line independently for workflow commands, so a value like zizmor/foo\n::error::injected would emit a second ::error:: command.

In practice, zizmor's rule IDs are hardcoded constants in the binary (verified by SHA-256), so no attacker-controlled content reaches ruleId. This is therefore theoretical given the current trust model, but does not follow the same defense-in-depth applied to message/file values. Piping ids through escape_data (already defined in scope) would close the gap at zero cost.

Fix this →

SUGGESTION — Confidence: High

persona input accepts arbitrary values without validation

zizmor.yml L42–L44 (input declaration), L168–L169 (use)

args=(
  --format=sarif
  "--persona=$PERSONA"
  ...
)

All other sensitive string inputs (fail-on-severity, version, sha256) are validated with a case statement or regex before use. PERSONA is passed directly to the binary as --persona=$PERSONA with no prior check. The value is correctly inside a bash array element (not shell-split), and zizmor will reject an unrecognised persona at runtime — so there is no injection risk. But a bogus persona silently becomes a zizmor invocation error rather than a clear "invalid input" message from the workflow layer. A case "$PERSONA" in regular|pedantic|auditor) ;; *) echo '::error::...' ; exit 2 ;; esac block, consistent with how fail-on-severity is handled, would give callers a cleaner failure mode and would be consistent with the established validation pattern in this workflow.

Fix this →

No findings related to: trigger misuse, permission widening, secret exposure, path traversal (normalize_sarif_uri is solid), supply-chain (SHA-pinned at both layers), or annotation injection via message/file paths (base64 + escape chain is correct).

claude · 2026-07-21T01:08:15Z

Claude finished @kyle-sexton's task in 3m 19s —— View job

Security Review — PR #183

Load tools
Read PR diff and changed files
Review shell script for injection surfaces
Review SARIF parsing for path traversal / injection
Review GitHub Actions workflow for dangerous patterns
Review permissions, secrets handling
Post findings

Summary

No CRITICAL or IMPORTANT findings. The implementation is well-hardened: trigger is workflow_call only, permissions are narrowed to contents: read, all action uses: lines are SHA-pinned, the binary is SHA-256-verified before execution, token is isolated from the curl/tar environment and unset after use, annotation injection is defended via base64 transport + escape_property/escape_data, and normalize_sarif_uri blocks path traversal via realpath -e workspace confinement. Two SUGGESTION-level gaps below.

SUGGESTION — Confidence: Medium

$ids interpolated into a workflow command without escape_data

zizmor-sarif-guard.sh L202–L205 (inlined: zizmor.yml L406–L409)

ids="$(jq -r ... '[.ruleId] | unique | join(", ")' "$ZIZMOR_SARIF")"
echo "::error::zizmor reported $blocking_count finding(s) at or above severity $FAIL_ON_SEVERITY: $ids"

The SARIF schema check (guard L36–L53) validates .ruleId as a non-empty string but does not restrict the character set. jq -r outputs decoded JSON strings, so a ruleId containing a literal \n in the JSON document produces a newline in $ids. When echo writes a multi-line string to stdout, Actions processes each line independently for workflow commands — a value like zizmor/foo\n::set-env name=X::Y would emit a second ::set-env:: command.

In practice this is theoretical: zizmor's rule IDs are hardcoded constants in the SHA-256-verified binary, so no external data reaches them. The risk is still worth closing — escape_data (defined inside annotate_findings) applies the same percent-encoding already used for message/file values. Moving the $ids echo inside annotate_findings, or hoisting escape_data to the outer scope so the blocking-count branch can call it, would close the gap at zero cost and make the annotation-injection discipline consistent throughout the script.

Fix this →

SUGGESTION — Confidence: High

persona input accepted without validation

zizmor.yml L169

args=(
  --format=sarif
  "--persona=$PERSONA"
  ...
)

PERSONA is placed in a bash array element as "--persona=$PERSONA", which prevents shell injection — the value is passed as a single opaque argument through execve. However, all other inputs are validated before use (version by regex, sha256 by regex, online-audits and fail-on-findings by a combined case check, fail-on-severity by case) while persona skips directly to the binary. An unrecognized persona causes a zizmor runtime error rather than a clear workflow-layer message. A case "$PERSONA" in regular|pedantic|auditor) ;; *) echo '::error::...' ; exit 2 ;; esac block, consistent with the fail-on-severity validation on L124–L130, would provide a cleaner failure mode and close the validation gap.

Fix this →

Checks with no findings

Area	Status
Dangerous triggers (`pull_request_target`, `workflow_run`+secrets)	✅ Not present
Permission widening	✅ `contents: read` only; no `security-events: write`
`github` context injection in `run:` blocks	✅ All inputs enter via `env:`, not inline `${{ }}` interpolation
Supply chain — action SHA pins	✅ SHA-pinned with version comment
Supply chain — binary download	✅ SHA-256 verified; version string compared post-install
Token lifecycle	✅ Scoped local variable; `ZIZMOR_TOKEN` unset before curl; `token` unset after binary run
Annotation injection (message / file paths)	✅ Base64 transport + `escape_data`/`escape_property`/`normalize_sarif_uri` chain correct
Path traversal in `normalize_sarif_uri`	✅ `realpath -e` + workspace containment check; null-byte and control-char guards
`PATHS` word-split injection	✅ Paths enter a bash array; `--` separates zizmor options from positional args
Untrusted checkout before action step	✅ Not present (`persist-credentials: false`)

claude · 2026-07-21T01:08:20Z

Claude finished @kyle-sexton's task in 2m 37s —— View job

Code Review — PR #183

Summary

This is a clean, well-tested port of the osv-scan-guard SARIF-gate pattern onto zizmor. The latest commit (5fac354) is a pure formatting fix aligning render-zizmor-sarif-guard.cjs and zizmor-native.test.cjs to the repo's biome config (2-space indentation) — no logic changes. Two prior security review suggestions remain open and are called out below.

Security

Trigger and permissions are correct. workflow_call only, permissions: contents: read at the top level with no security-events: write. No pull_request_target or workflow_run with secrets.

Token lifecycle is correct. token=$ZIZMOR_TOKEN → unset ZIZMOR_TOKEN → curl download (no token in env) → checksum → extract → verify version → GH_TOKEN="$token" "$binary" → unset token. Token is never in the environment during download or extraction, only forwarded to the already-verified binary.

Binary verification chain. Download → sha256sum --check --strict → tar --extract → exec-bit check → "$binary" --version → string compare against $resolved_version. The checksum-before-extraction ordering is asserted in zizmor-native.test.cjs lines 98–105.

Provenance schema validation is fail-closed. zizmor-sarif-guard.sh L36–53 rejects any SARIF whose tool name, version, or result level doesn't match the expected contract. The all() check over results means a single result with level none (valid SARIF spec but not emitted by zizmor 1.27.0) exits 2 — fail-closed, not fail-open. The unknown-level test case at line 63 confirms this.

Annotation injection chain is correct. jq base64-encodes all string fields before crossing the pipe boundary; uri_safe/message_safe boolean gates reject non-printable content before decoding; escape_property/escape_data encode %, \r, \n, :, , before writing workflow commands. normalize_sarif_uri applies realpath -e containment + null-byte + control-char + ?/# rejections on the decoded path. The located-annotation test at lines 86–104 confirms end-to-end path resolution.

escape_data scope vs. $ids. The previous security review correctly flagged this: zizmor-sarif-guard.sh L202–205 inserts $ids (joined ruleId strings from jq) into the ::error:: workflow command without encoding. escape_data is defined as a nested function inside annotate_findings, so it is not in scope here. A ruleId containing \n in the JSON document (after jq -r expansion) would emit a second workflow command. In practice this can't happen — zizmor's rule IDs are hardcoded constants in the SHA-256-verified binary — but it's the only place in the guard that doesn't follow the base64/encode chain. Fixing it requires either moving escape_data to global scope or duplicating the encoding inline:

Fix this →

persona input not validated before use. zizmor.yml L169 passes "--persona=$PERSONA" directly to the binary with no preceding case guard. No injection risk (it's inside a bash array element), but an invalid persona produces a zizmor CLI error rather than the clean workflow-layer message used for FAIL_ON_SEVERITY, REQUESTED_VERSION, and EXPECTED_SHA256. Consistent with the existing validation pattern would be:

case "$PERSONA" in
  regular|pedantic|auditor) ;;
  *)
    echo '::error::zizmor persona must be regular, pedantic, or auditor.'
    exit 2
    ;;
esac

Fix this →

Correctness

Severity mapping is correct. The SARIF level → blocking_levels mapping:

`fail-on-severity`	`blocking_levels`	Blocks on
`never`	`[]`	nothing
`low`	`["error","warning","note"]`	all findings
`medium`	`["error","warning"]`	medium+
`high`	`["error"]`	high only

This matches zizmor's documented level mapping (error=high, warning=medium, note=low/info).

effective_severity resolution. fail-on-severity above never wins unconditionally; fail-on-findings: true resolves to low only when fail-on-severity is never. The inlined guard validates FAIL_ON_SEVERITY again — defense-in-depth for standalone invocations.

jq index() at position 0 is truthy. select(.level as $l | $levels | index($l)) works correctly even when the matching level is at index 0 — jq treats 0 as truthy (unlike JavaScript where 0 is falsy). Not a bug.

Annotation cap vs. blocking count. annotate_findings caps at 50 results (L165); blocking_count scans all. The exit message still names the rule IDs, so users can find findings beyond the cap. Same tradeoff as osv-scan-guard.sh.

Renderer is stable. bundledScript uses trimEnd() before splitting, maps empty lines to "" (no trailing spaces), and indents non-empty lines with 10 spaces matching the run: block depth. The --check test in zizmor-native.test.cjs line 154–158 asserts the inlined copy matches the source at CI time.

Test coverage

17 guard cases span: all severity × level combinations, infrastructure failures (malformed JSON, wrong name, wrong version, unknown level, nonzero exit code, bad severity), advisory annotation smoke test, and located-annotation with workspace resolution. The selector-contract job in ci.yml and the selector-unit job in selector-conformance.yml both run zizmor-sarif-guard.test.sh — no gap in coverage wiring.

CLAUDE.md checklist

Rule	Status
No `pull_request_target` / `workflow_run` with secrets	✅
`permissions: contents: read` only	✅
All `uses:` SHA-pinned with version comment	✅
No credential echo / `ACTIONS_STEP_DEBUG`	✅
No untrusted checkout before action step	✅

Verdict

Ship. The two suggestions above (escape_data scope for $ids, persona case validation) are the same ones the prior security review surfaced — still worth addressing in a follow-up or this PR, but neither is a blocking correctness or security issue given the verified binary trust model. The latest commit is formatting-only and introduces no regressions.

Bring zizmor onto the constellation's SARIF-guard pattern (as used by osv-scanner + govulncheck): run `--format=sarif`, parse it in a bash guard that emits annotations for every finding and exits nonzero when any finding's severity is at or above a caller-chosen threshold. - New input `fail-on-severity` (never|low|medium|high, default `never`). Default preserves today's advisory behavior. `fail-on-findings: true` kept as a back-compat alias (== `fail-on-severity: low`). - SARIF level mapping (source- and empirically-verified at zizmor 1.27.0): high->error, medium->warning, low|informational->note. Guard blocks on the level set for the threshold; `never`->{} advisory. - New `.github/scripts/zizmor-sarif-guard.sh` reuses osv-scan-guard.sh's hardened annotation machinery (base64/escape/normalize_sarif_uri path defense), parameterized by level. `render-zizmor-sarif-guard.cjs` + `# BEGIN/END GENERATED ZIZMOR SARIF GUARD` markers materialize it into zizmor.yml (mirrors render-osv-scan-guard.cjs; `--check` sync-verified). - `zizmor-sarif-guard.test.sh` (17 cases) + revised static `zizmor-native.test.cjs`; wired into ci.yml + selector-conformance.yml. - README zizmor section corrected (blocking now available; only SARIF *upload* to code scanning stays deferred). No code-scanning, no security-events:write, no org-wide ruleset change — gating rides the caller's existing ci-status lane when opted in. Verified: zizmor-sarif-guard.test.sh 17/17; node --test 5/5 (zizmor-native); render --check in sync; shellcheck + shfmt clean; actionlint clean (YAML/expr). Independent fresh-context security review was in flight at commit time — to be reconciled before the PR is opened. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>

- biome: render-zizmor-sarif-guard.cjs + zizmor-native.test.cjs to the repo's tab indentation (formatting only; rendered guard output unchanged, --check in sync). - exec-bit: zizmor-sarif-guard.test.sh carries a shebang, so mark it executable (100755), matching govulncheck-sarif-guard.test.sh. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>

Prior commit applied biome's DEFAULT config (tabs), which the repo's fixtures/typescript/good/biome.json overrides to 2-space and .editorconfig mandates as spaces. Reformat both .cjs with the repo biome config: the longer zizmor marker string wraps to a continuation line and a multi-line assert.match collapses inline. Rendered guard output unchanged (render --check in sync); no tabs (editorconfig clean); node --test 5/5. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>

claude · 2026-07-21T01:18:22Z

Claude finished @kyle-sexton's task in 2m 23s —— View job

Code Review — PR #183

Gather context and understand the request
Read PR diff (head SHA: 017493d)
Review core guard script (zizmor-sarif-guard.sh)
Review renderer (render-zizmor-sarif-guard.cjs)
Review test files
Review zizmor.yml changes
Check CLAUDE.md alignment
Post review findings

Summary

The latest commit (017493d) is a pure formatting fix — biome config alignment (2-space indentation) in render-zizmor-sarif-guard.cjs and zizmor-native.test.cjs with no logic changes. The substantive work is in the two prior commits and is a clean port of the osv-scan-guard pattern onto zizmor. The architecture is sound and the security boundaries are correctly maintained. Two suggestions from the prior reviews remain open (noted below); neither is blocking.

Security

Trigger and permissions are correct. workflow_call only; permissions: contents: read at the top level; no security-events: write, no pull_request_target, no workflow_run with secrets. ✅

Token lifecycle is correct. token=$ZIZMOR_TOKEN → unset ZIZMOR_TOKEN → curl download (token absent from env) → sha256 check → extract → version verify → GH_TOKEN="$token" "$binary" → unset token. Token is never present in the environment during download or extraction. ✅

Binary verification chain. curl → sha256sum --check --strict → tar --extract → exec-bit check → version string compare. zizmor-native.test.cjs asserts this ordering explicitly at lines 98–105. ✅

Provenance schema validation is fail-closed. The jq -e check at guard L36–53 gates on tool name, semantic version, and every result's level being one of error|warning|note. A single result with level none (valid SARIF spec, not emitted by zizmor) exits 2. The unknown-level test case at test L63 confirms this. ✅

Annotation injection chain is correct. jq base64-encodes all string fields before crossing the pipe boundary; uri_safe/message_safe boolean gates screen out non-printable content; escape_property/escape_data encode %, \r, \n, :, , before writing workflow commands; normalize_sarif_uri applies realpath -e containment + null-byte + control-char + ?/# rejections. ✅

All uses: SHA-pinned with version comments. Confirmed throughout zizmor.yml and the ci.yml/selector-conformance.yml changes. ✅

Two open suggestions (carried from prior reviews — not blocking)

SUGGESTION — Confidence: Medium

$ids interpolated into a workflow command without escape_data

zizmor-sarif-guard.sh L202–205 (inlined: zizmor.yml L406–409)

ids="$(jq -r ... '[.ruleId] | unique | join(", ")' "$ZIZMOR_SARIF")"
echo "::error::zizmor reported $blocking_count finding(s) at or above severity $FAIL_ON_SEVERITY: $ids"

escape_data is defined as a nested function inside annotate_findings and is not in scope here. jq -r expands JSON escape sequences, so a ruleId containing a literal \n in the JSON document would produce a newline in $ids, and Actions processes each stdout line independently for workflow commands. Practical risk is near zero — zizmor's rule IDs are hardcoded constants in the SHA-256-verified binary — but this is the only place in the script that doesn't follow the base64/encode discipline applied everywhere else.

Fix: move escape_data (and escape_property) to global scope above annotate_findings, then pipe $ids through escape_data before the echo. Apply the same fix to the inlined copy in the BEGIN/END GENERATED block in zizmor.yml, then re-run the renderer.

Fix this →

SUGGESTION — Confidence: High

persona input accepted without workflow-layer validation

zizmor.yml L169

args=(
  --format=sarif
  "--persona=$PERSONA"   # ← no case guard
  ...
)

Every other string input is validated before use (version by regex, sha256 by regex, online-audits/fail-on-findings by combined case, fail-on-severity by case at L124–130). PERSONA passes directly to the binary with no prior check. No injection risk (it's inside a bash array element so it's a single opaque execve argument), but an unrecognised persona produces a zizmor CLI error rather than a clean workflow-layer message. Consistent with the existing pattern:

case "$PERSONA" in
  regular|pedantic|auditor) ;;
  *)
    echo '::error::zizmor persona must be regular, pedantic, or auditor.'
    exit 2
    ;;
esac

Fix this →

Correctness

Severity mapping is correct.

`fail-on-severity`	`blocking_levels`	Blocks on
`never`	`[]`	nothing
`low`	`["error","warning","note"]`	all findings
`medium`	`["error","warning"]`	medium+
`high`	`["error"]`	high only

This matches zizmor's documented level mapping (error=high, warning=medium, note=low/info). ✅

effective_severity resolution is correct. fail-on-severity above never wins unconditionally; fail-on-findings: true maps to low only when fail-on-severity is never. The guard validates FAIL_ON_SEVERITY again inside the inlined block — correct defense-in-depth for standalone invocations. ✅

jq index() at position 0 is truthy. select(.level as $l | $levels | index($l)) is correct even when the matching level is at index 0 — jq treats 0 as truthy (unlike JavaScript). Not a bug. ✅

Annotation cap vs. blocking count. annotate_findings caps at 50 results (guard L165); blocking_count scans all results. The exit message names the rule IDs to compensate. Same deliberate tradeoff as osv-scan-guard.sh. ✅

Renderer roundtrip is stable. bundledScript() trims trailing whitespace, splits on \r?\n, maps empty lines to "" (no trailing spaces), and indents to 10-space depth matching the run: block. The --check assertion in zizmor-native.test.cjs at L154–158 asserts the inlined copy matches the source at CI time. ✅

Test coverage

17 guard cases cover all severity × level combinations, infrastructure failures (malformed JSON, wrong tool name, wrong version, unknown level, nonzero exit code, bad severity), advisory annotation smoke test, and located-annotation with workspace resolution. Both ci.yml and selector-conformance.yml wire the test. selector-conformance.yml path filters include all four new/modified script files — no coverage gap. ✅

CLAUDE.md checklist

Rule	Status
No `pull_request_target` / `workflow_run` with secrets	✅
`permissions: contents: read` only; no `security-events: write`	✅
All `uses:` SHA-pinned with version comment	✅
No credential echo / `ACTIONS_STEP_DEBUG`	✅
No untrusted checkout before action step	✅

Verdict

Ship. The two suggestions above (escape_data scope for $ids, persona case validation) are worth addressing — they close a design-consistency gap in the annotation-injection discipline and add a cleaner failure mode for invalid persona inputs — but neither is a blocking correctness or security issue given the verified binary trust model. All three commits together represent a clean, well-tested implementation.

claude · 2026-07-21T01:18:23Z

Claude finished @kyle-sexton's task in 2m 17s —— View job

Security Review — PR #183

Load tools
Read PR diff
Read zizmor-sarif-guard.sh (core guard logic)
Read zizmor.yml (workflow changes)
Read render-zizmor-sarif-guard.cjs (renderer)
Read zizmor-sarif-guard.test.sh (test suite)
Review for injection surfaces, token handling, dangerous Actions patterns, supply-chain risks
Post findings

Summary

No CRITICAL or IMPORTANT findings. The implementation is well-hardened throughout. The trigger is workflow_call only — no pull_request_target or workflow_run with secrets. Permissions are contents: read with no security-events: write. All uses: lines are SHA-pinned with version comments. The binary download is SHA-256 verified before extraction (sha256sum --check --strict, then tar, then exec-bit check, then version string comparison). Token isolation is correct: ZIZMOR_TOKEN is captured into a local variable and unset before curl, then forwarded only to the verified binary as GH_TOKEN, and unset after. All inputs enter run: blocks via the env: map — no inline ${{ }} interpolation inside script bodies. Annotation injection is neutralized by the base64 transport chain, printable-character guards, escape_property/escape_data percent-encoding, and normalize_sarif_uri with realpath -e workspace confinement. SARIF provenance validation fails closed on unknown levels, wrong tool name, or wrong version.

Two SUGGESTION-level findings below.

SUGGESTION — Confidence: Medium

$ids interpolated into ::error:: workflow command without escape_data

zizmor-sarif-guard.sh L202–205 (inlined: zizmor.yml L406–409)

ids="$(jq -r --argjson levels "$blocking_levels" '
  [.runs[0].results[] | select(.level as $l | $levels | index($l)) | .ruleId] | unique | join(", ")
' "$ZIZMOR_SARIF")"
echo "::error::zizmor reported $blocking_count finding(s) at or above severity $FAIL_ON_SEVERITY: $ids"

The provenance schema (L43–49) validates .ruleId as a non-empty string but does not restrict the character set. jq -r expands JSON escape sequences before output, so a ruleId value containing a literal \n in the JSON document produces a newline in $ids. echo writes each line independently to stdout; GitHub Actions processes each stdout line for workflow commands, so a value like zizmor/foo\n::set-env name=X::Y would emit a second ::set-env:: workflow command.

In practice this is theoretical: the binary is SHA-256 verified before execution, so its ruleId constants cannot be attacker-controlled. However, escape_data (defined as a nested function inside annotate_findings) is already the right tool and is applied to every other user-derived value in the script. Moving escape_data and escape_property to the outer scope and applying escape_data to $ids would close the gap at zero cost and make the injection discipline consistent throughout.

Fix this →

SUGGESTION — Confidence: High

persona input used without validating against allowed values

zizmor.yml L169

args=(
  --format=sarif
  "--persona=$PERSONA"   # no prior case-guard
  ...
)

PERSONA is placed in a bash array element as "--persona=$PERSONA", preventing word-split injection — the value reaches zizmor as a single opaque argument. However, every other string input is validated before use: REQUESTED_VERSION by regex (L106–110), EXPECTED_SHA256 by regex (L113–116), ONLINE_AUDITS/FAIL_ON_FINDINGS by a combined case check (L117–123), and FAIL_ON_SEVERITY by case (L124–130). PERSONA skips directly to the binary. An unrecognized value causes a zizmor CLI-layer error rather than the clean workflow-level message that FAIL_ON_SEVERITY etc. produce. The fix is a case guard consistent with the established pattern:

case "$PERSONA" in
  regular|pedantic|auditor) ;;
  *)
    echo '::error::zizmor persona must be regular, pedantic, or auditor.'
    exit 2
    ;;
esac

Fix this →

Checks with no findings

Area	Status
Dangerous triggers (`pull_request_target`, `workflow_run` + secrets)	✅ `workflow_call` only
Permission widening	✅ `contents: read`; no `security-events: write`
`github` context injection in `run:` blocks	✅ All inputs flow through `env:`, not inline `${{ }}` in script bodies
Supply chain — action SHA pins	✅ `actions/checkout` SHA-pinned with version comment
Supply chain — binary download	✅ SHA-256 verified (`sha256sum --check --strict`) before extraction; version string confirmed post-install
Token lifecycle	✅ Scoped to local `token`; `ZIZMOR_TOKEN` unset before curl; forwarded as `GH_TOKEN` to verified binary only; `token` unset after use
Annotation injection (message / file paths)	✅ Base64 transport + `uri_safe`/`message_safe` guards + `escape_data`/`escape_property` chain correct
Path traversal in `normalize_sarif_uri`	✅ `realpath -e` + workspace containment + null-byte + control-char + `?`/`#` rejections
PATHS word-split into binary args	✅ Array + `--` separator; no flag injection possible
SARIF provenance validation fail-closed	✅ Unknown levels, wrong name, wrong version all exit 2
Untrusted checkout before action step	✅ Not present; `persist-credentials: false`
Renderer roundtrip sync	✅ `--check` assertion in `zizmor-native.test.cjs` enforced at CI time

…#225) 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 `allowedInputs` — `runner-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. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>

kyle-sexton and others added 3 commits July 20, 2026 21:16

kyle-sexton force-pushed the feat/zizmor-severity-gate branch from 5fac354 to 017493d Compare July 21, 2026 01:18

kyle-sexton merged commit 31a5b76 into main Jul 21, 2026
40 checks passed

kyle-sexton deleted the feat/zizmor-severity-gate branch July 21, 2026 01:20

kyle-sexton mentioned this pull request Jul 21, 2026

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

Merged

Navigation Menu

Uh oh!

feat(zizmor): severity-tiered SARIF gate (opt-in, advisory default) - #183

feat(zizmor): severity-tiered SARIF gate (opt-in, advisory default)#183
kyle-sexton merged 3 commits into
mainfrom
feat/zizmor-severity-gate

kyle-sexton commented Jul 21, 2026

Uh oh!

chatgpt-codex-connector Bot commented Jul 21, 2026

Uh oh!

claude Bot commented Jul 21, 2026 •

edited

Loading

Uh oh!

claude Bot commented Jul 21, 2026 •

edited

Loading

Uh oh!

claude Bot commented Jul 21, 2026 •

edited

Loading

Uh oh!

claude Bot commented Jul 21, 2026 •

edited

Loading

Uh oh!

claude Bot commented Jul 21, 2026 •

edited

Loading

Uh oh!

claude Bot commented Jul 21, 2026 •

edited

Loading

Uh oh!

claude Bot commented Jul 21, 2026 •

edited

Loading

Uh oh!

claude Bot commented Jul 21, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

kyle-sexton commented Jul 21, 2026

What changed

Verification

Related

Uh oh!

chatgpt-codex-connector Bot commented Jul 21, 2026

Uh oh!

claude Bot commented Jul 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review in Progress

Uh oh!

claude Bot commented Jul 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Security Review — PR #183

Uh oh!

claude Bot commented Jul 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review — PR #183

Summary

Security

Correctness

Minor observations (no change required)

CLAUDE.md checklist

Verdict

Uh oh!

claude Bot commented Jul 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Security Review — PR #183

Findings

SUGGESTION — Confidence: Medium

SUGGESTION — Confidence: High

Uh oh!

claude Bot commented Jul 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Security Review — PR #183

Summary

SUGGESTION — Confidence: Medium

SUGGESTION — Confidence: High

Checks with no findings

Uh oh!

claude Bot commented Jul 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review — PR #183

Summary

Security

Correctness

Test coverage

CLAUDE.md checklist

Verdict

Uh oh!

claude Bot commented Jul 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review — PR #183

Summary

Security

Two open suggestions (carried from prior reviews — not blocking)

SUGGESTION — Confidence: Medium

SUGGESTION — Confidence: High

Correctness

Test coverage

CLAUDE.md checklist

Verdict

Uh oh!

claude Bot commented Jul 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Security Review — PR #183

Summary

SUGGESTION — Confidence: Medium

SUGGESTION — Confidence: High

Checks with no findings

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

claude Bot commented Jul 21, 2026 •

edited

Loading

claude Bot commented Jul 21, 2026 •

edited

Loading

claude Bot commented Jul 21, 2026 •

edited

Loading

claude Bot commented Jul 21, 2026 •

edited

Loading

claude Bot commented Jul 21, 2026 •

edited

Loading

claude Bot commented Jul 21, 2026 •

edited

Loading

claude Bot commented Jul 21, 2026 •

edited

Loading

claude Bot commented Jul 21, 2026 •

edited

Loading