feat: fail the Security Suite on error-severity findings by dkastl · Pull Request #93 · geolonia/.github

dkastl · 2026-07-06T01:43:28Z

Closes the "it's weird to pass with an error" gap: zizmor was fully warn-only, so a PR could pass the suite with a zizmor error.

Change

The report job becomes the single gate. It now exits 1 on any error-severity condition — a hard-gate fail/error (bumblebee/betterleaks), or a zizmor error-severity finding — after posting the summary comment so the result stays visible. Warnings and notices (and warn-only pinact, and zizmor warning/notice findings) never block.

zizmor's step keeps continue-on-error (still emits annotations + the error/warning/notice counts); the report job decides blocking.
bumblebee/betterleaks already block via their own jobs (fail-on-findings defaults to true); this doesn't change them.
Docs: added a "What blocks a PR" table to security-suite.md.

⚠️ Rollout (Option A, per your call)

Once released, this blocks any repo that still has a zizmor error org-wide until it's hardened (same pattern as #87 / infra-cdk#152). You're driving that cleanup across the ~10 repos you manage. Known next: geolonia-operations (~10 error-level findings).

After merge

Cut v1.33.0 (lightweight tag) → @v1 moves → the gate is live. Verify on a consumer PR that a zizmor-error repo now goes red and a clean one stays green.

Validation: pinact --check OK, zizmor self-clean, no em-dashes.

Summary by CodeRabbit

Bug Fixes
- The Security Suite check now properly blocks pull requests when high-severity zizmor findings are detected.
- Warning and notice-level findings still appear in the report without failing the check.
Documentation
- Added clearer guidance on which security scanner results fail the PR gate versus only creating warnings.
- Updated notes on how to handle allowed security findings.

Previously zizmor was fully warn-only, so a PR could 'pass' the suite with a zizmor error. Make the report job the single gate: it now exits 1 on any error-severity condition (a hard-gate fail/error, or a zizmor error-severity finding), after posting the summary comment so the result stays visible. - report job: track SUITE_FAILED in the two [!CAUTION] branches, exit 1 at the end; update the zizmor callout to say error-severity findings block. - zizmor step keeps continue-on-error (still emits annotations + counts); the report job decides blocking. Warnings/notices (and warn-only pinact) never block; bumblebee/betterleaks already block via their own jobs. - docs: add a 'What blocks a PR' table to security-suite.md. Note: once released, this blocks any repo that still has a zizmor error (org-wide) until hardened (see #87 / infra-cdk#152 for the pattern).

coderabbitai · 2026-07-06T01:43:37Z

Important

Review skipped

No new commits to review since the last review.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 92a1cb5f-e46a-463e-812b-5f38b88eb309

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

🔍 Trigger review

Walkthrough

This PR modifies the reusable security-suite workflow so the report job now blocks pull requests on zizmor error-severity findings by setting a SUITE_FAILED flag and exiting 1, updates related comments, and documents the new blocking behavior in security-suite.md.

Changes

Zizmor Error-Severity PR Gating

Layer / File(s)	Summary
Workflow comment updates for gating contract `.github/workflows/reusable-security-suite.yml`	Comments at the suite-level gate, actions-audit outputs, and zizmor run step are updated to describe the report job as the single gate for zizmor error-severity findings.
SUITE_FAILED flag and final exit gate `.github/workflows/reusable-security-suite.yml`	Sets `SUITE_FAILED=1` when `ZIZMOR_SEV` is `error`, updates callout text to state findings block the PR, and adds a final `exit 1` after posting the consolidated comment when `SUITE_FAILED=1`.
Security suite documentation update `docs/workflows/security-suite.md`	Adds a "What blocks a PR" section detailing which scanner findings (bumblebee, betterleaks, zizmor, pinact) block vs. warn, and explains the report job's gating logic and ignore guidance.

Estimated code review effort: 2 (Simple) | ~10 minutes

Sequence Diagram(s)

sequenceDiagram
  participant ZizmorStep
  participant ReportJob
  participant PRComment
  participant CIExit

  ZizmorStep->>ReportJob: emit SARIF severity counts (continue-on-error)
  ReportJob->>ReportJob: set SUITE_FAILED=1 if ZIZMOR_SEV is error
  ReportJob->>PRComment: post consolidated comment with blocking callout text
  ReportJob->>CIExit: exit 1 if SUITE_FAILED=1

Possibly related issues

geolonia/geolonia-operations#195: Both touch the same reusable-security-suite.yml enforcement and docs for the security workflow.
geolonia/geolonia-operations#196: Introduces the fail-or-warn behavior for the same zizmor suite that this PR updates.

Possibly related PRs

geolonia/.github#81: Both modify the report job's Zizmor severity interpretation logic; this PR adds the blocking SUITE_FAILED/exit 1 gate on top.
geolonia/.github#83: Both modify the same report logic for mapping/flagging zizmor error-severity findings to determine blocking behavior.
geolonia/.github#85: Both modify the zizmor/report logic in the same workflow file to interpret error-severity findings for PR blocking.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely summarizes the main change: Security Suite now fails on error-severity findings.
Description check	✅ Passed	It covers the core change, rollout, and validation details; the optional checklist and related issues sections are not critical.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch feat/suite-fail-on-error

_{Comment @coderabbitai help to get the list of available commands.}

github-actions · 2026-07-06T01:43:52Z

🛡️ Security suite

Check	Result
✅ Supply chain · `bumblebee`	No exposure matches
✅ Secrets · `betterleaks`	No secrets in diff
✅ Action pinning · `pinact`	All actions pinned
✅ Actions audit · `zizmor`	No findings

Note

All security checks passed.

_{Updated for 40e90e8 · workflow run}

dkastl · 2026-07-06T01:44:28Z

@coderabbitai review

coderabbitai · 2026-07-06T01:44:33Z

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

coderabbitai

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

.github/workflows/reusable-security-suite.yml (1)
222-225: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Stale "warn-only" wording now contradicts the new blocking behavior.

ZIZMOR_BREAKDOWN still hardcodes "(warn-only; see annotations)" and the comment above still states zizmor is warn-only. This string is used verbatim as ZIZMOR_MSG in the error branch (line 234), so the posted PR comment will show something like "N error(s)... (warn-only; see annotations)" even though this PR makes error-severity findings block the PR via the new SUITE_FAILED gate. This directly contradicts the new gating contract this PR introduces and will confuse readers of the summary comment.
🛠️ Proposed fix
-          # zizmor is warn-only. Show the full severity breakdown (from the
-          # SARIF count pass) so errors are never understated as warnings.
+          # Show the full severity breakdown (from the SARIF count pass) so
+          # errors are never understated as warnings; only error-severity
+          # findings actually block (see SUITE_FAILED below).
           ZE="${ZIZMOR_ERRORS:-0}"; ZW="${ZIZMOR_WARNINGS:-0}"; ZN="${ZIZMOR_NOTICES:-0}"
-          ZIZMOR_BREAKDOWN="${ZE} error(s), ${ZW} warning(s), ${ZN} notice(s) (warn-only; see annotations)"
+          ZIZMOR_BREAKDOWN="${ZE} error(s), ${ZW} warning(s), ${ZN} notice(s) (see annotations)"
Then adjust the warn-branch message (line 236) if you still want to call out that warnings/notices specifically don't block.
Also applies to: 233-234
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/reusable-security-suite.yml around lines 222 - 225, Update
the Zizmor summary text in the reusable security workflow so it no longer says
“warn-only” in the error path, since `ZIZMOR_BREAKDOWN` is reused as
`ZIZMOR_MSG` when `SUITE_FAILED` is set and now represents blocking
error-severity findings. Edit the comment and the `ZIZMOR_BREAKDOWN` string near
the severity aggregation logic so the wording reflects the new gating behavior,
and keep any “warnings/notices don’t block” note only in the warn branch message
if needed.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In @.github/workflows/reusable-security-suite.yml:
- Around line 222-225: Update the Zizmor summary text in the reusable security
workflow so it no longer says “warn-only” in the error path, since
`ZIZMOR_BREAKDOWN` is reused as `ZIZMOR_MSG` when `SUITE_FAILED` is set and now
represents blocking error-severity findings. Edit the comment and the
`ZIZMOR_BREAKDOWN` string near the severity aggregation logic so the wording
reflects the new gating behavior, and keep any “warnings/notices don’t block”
note only in the warn branch message if needed.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 1094f4f9-6b5c-478b-9c14-1037ea156a73

📥 Commits

Reviewing files that changed from the base of the PR and between bc7186f and 40e90e8.

📒 Files selected for processing (2)

.github/workflows/reusable-security-suite.yml
docs/workflows/security-suite.md

dkastl marked this pull request as ready for review July 6, 2026 01:44

coderabbitai Bot reviewed Jul 6, 2026

View reviewed changes

dkastl merged commit 0d06c91 into main Jul 6, 2026
6 checks passed

dkastl deleted the feat/suite-fail-on-error branch July 6, 2026 01:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat: fail the Security Suite on error-severity findings#93

feat: fail the Security Suite on error-severity findings#93
dkastl merged 1 commit into
mainfrom
feat/suite-fail-on-error

dkastl commented Jul 6, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jul 6, 2026 •

edited

Loading

Review skipped

Uh oh!

github-actions Bot commented Jul 6, 2026

Uh oh!

dkastl commented Jul 6, 2026

Uh oh!

coderabbitai Bot commented Jul 6, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

dkastl commented Jul 6, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Change

⚠️ Rollout (Option A, per your call)

After merge

Summary by CodeRabbit

Uh oh!

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

Uh oh!

Review skipped

Walkthrough

Changes

Sequence Diagram(s)

Uh oh!

github-actions Bot commented Jul 6, 2026

🛡️ Security suite

Uh oh!

dkastl commented Jul 6, 2026

Uh oh!

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

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

dkastl commented Jul 6, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jul 6, 2026 •

edited

Loading

coderabbitai Bot commented Jul 6, 2026 •

edited

Loading