[don-petry]

Summary

Fixes #1436

The min-severity input of agent-shield-reusable.yml now sets the failure threshold. The scanner's --min-severity only filters its report, and its exit code only reflects critical findings, so a report with only high findings passed the job.

• The scan step writes the scanner's JSON report to $RUNNER_TEMP. Exit 0 and 2 continue, because 2 only means critical findings exist. Any other exit stops the job with the scanner's code.
• A new step runs agent_shield_severity_gate, a pure lib in .github/scripts/agent-shield/lib/severity-gate.sh, checked out at job.workflow_sha (same pattern as feature-ideation v1-next: pip step fails — tooling_ref default 'v1' skews from reusable (blocks .github-private, will break fleet on promotion) #839). It fails at or above the threshold.
• The gate fails closed on an unknown threshold, or a report that is missing, not one JSON document, or off-schema.
• The job name, inputs and caller stubs do not change. Log output is now severity, title, location and description only.

JSON schema

Read from the ecc-agentshield package source for 1.4.0 (the pinned default) and 1.6.0 (the latest): calculateScore returns findings[] with severity, title, description, file, line, and summary.totalFindings. Both versions print it with --format json and exit 2 only when critical findings exist. I did not run the scanner locally. This PR's own agent-shield / AgentShield check runs the real scanner through the new steps, because this repo calls the reusable by local ref.

Sequencing with #1419

Merge after #1419. That PR changes the same scan step, and this branch does not repeat its changes.

This PR is based on main rather than stacked because the checks only run on PRs targeting main. A trial merge with #1419's branch is clean, and on that merged tree both this suite and #1419's scan-step test pass.

Problem

min-severity did not act as a failure threshold.

Risk

Medium. The check becomes stricter. agent-shield / AgentShield is a required check, so once a channel carries this, a repo with existing findings at or above its threshold fails that check until they are fixed or its threshold is changed. This repo has none, per its latest main run. Other repos pick this up only when their channels move. This PR also adds require_confirmation to agent-shield's ring1->stable hop, so the release waits for a deliberate confirm before reaching repos that are only on stable.

Test plan

• bats test/workflows/agent-shield/: 18/18 pass, including the reusable's real run blocks against a stubbed scanner.
• bats test/workflows/dependency-hardening.bats, shellcheck -x and actionlint: clean.
• New workflow agent-shield-tests.yml runs shellcheck and the suite on changes to these files.

Rollback

Revert this PR, or move the affected channel back to its prior release. No migrations or external state.

Monitoring

Watch agent-shield / AgentShield on this PR and on the first ring to receive it.

🤖 Generated with Claude Code

[chatgpt-codex-connector]

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.

[don-petry]

Maintainer disposition of chatgpt-codex-connector's status comment, posted by a Claude Code session with the maintainer's approval:

The Codex review failed at 0a8becb and posted no findings: this PR has no inline review comments and the summary lists none. Its hidden state field reports has_any_findings, which could not be matched to any posted finding. The code was reviewed by CodeRabbit instead.

[don-petry]

Maintainer disposition of coderabbitai's status comment, posted by a Claude Code session with the maintainer's approval:

A review summary with no findings: it states that no actionable comments were generated.

[don-petry]

Maintainer disposition of coderabbitai's status comment, posted by a Claude Code session with the maintainer's approval:

The comment now holds a review-limit notice with no findings. CodeRabbit edited it after the previous disposition.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[don-petry]

No description provided.

[donpetry-bot]

No description provided.

[don-petry]

No description provided.

[don-petry]

Re-read the current (edited 2026-11-03T00:05:50Z) CodeRabbit summary. It has a rate-limit notice (code review throttled, next review in ~28 minutes) and a recent-review section that says "No actionable comments were generated." It has no Security Architecture section, no retained concerns, no hardening proposals, no outside-diff comments and no nitpicks, so there is nothing to act on. No code change.

[don-petry]

Dev-Lead — fix-reviews (no-changes)

Agent reasoning

Addressed 0 threads. There were no open review threads, and I made no code changes or commits.
- **Phase 0:** No CI check failed, and the only review (CodeRabbit, `DISMISSED`) is not a change request. There are no Tier 1 blockers.
- **Stale disposition:** I re-read the current body of CodeRabbit's summary comment `IC_kwDORyesfc8AAAABhbI6sA`. It holds only a rate-limit notice and "No actionable comments were generated". It has no architecture-review, retained-concern, hardening, outside-diff or nitpick sections. I posted a fresh `informational` disposition at https://github.com/combsmith/.github/pull/1438#issuecomment-6592019073.
- **Tests:** I didn't run the test suite, since nothing changed. Every CI check on the PR passed or was skipped.
Files changed: none.

[donpetry-bot]

Automated review — APPROVED ✓

Risk: MEDIUM
Reviewed commit: 530ffa5cf6cb8775fb715d4d6d10524852a6f473
Review mode: triage-approved (single reviewer)

Summary

I agree with triage: MEDIUM. This is a logic change to a reusable workflow that backs the required agent-shield / AgentShield check, and 8 downstream consumers pin it. It makes the check stricter and adds no new place where secrets are handled.

What changes:

• The scan step writes the scanner's report as JSON (--format json) to $RUNNER_TEMP. Exit codes 0 and 2 continue. Any other exit stops the job with the scanner's code.
• A new step checks out .github/scripts/agent-shield at job.workflow_sha, with persist-credentials: false and the action pinned by SHA.
• agent_shield_severity_gate then decides pass or fail. It fails closed on an unknown threshold. It also fails closed on a report that is missing, more than one JSON document, or off-schema: the summary count doesn't match, or a severity is unknown.
• inputs.min-severity reaches run: through env:, not ${{ }} interpolation, so there is no injection path.
• Finding text is cleaned before it is logged: control characters are flattened, and :: and ##[ are broken up. evidence is never printed. A bats case covers forged workflow commands.
• The job name is unchanged, so the required-check context is kept. The thin caller stub agent-shield.yml is not touched, as AGENTS.md requires.
• Agent-shield's ring1->stable hop gets require_confirmation: true, and tests/canary_rollout.bats is updated to match.

Linked issue analysis

#1436 asks that min-severity set the failure threshold, using the scanner's JSON output, and that the schema be checked first. The PR does this. The job now fails on any finding at or above the threshold, and the threshold is inclusive at every level (tested).

The schema check is partly done. The author read it from the ecc-agentshield 1.4.0 and 1.6.0 source and did not run the scanner locally. The real scanner does run in this PR's own agent-shield / AgentShield check, which passed. I could not read that job's log, so I can't confirm which code paths it exercised.

Findings

No blocking findings. Four non-blocking notes:

1. The summary-count check depends on an unverified scanner detail (unconfirmed). The gate rejects a report where summary.totalFindings != findings | length. The scan still passes --min-severity. If the scanner filters findings[] but counts totalFindings before filtering, then any repo with findings below its threshold would fail with "cannot trust the scan report". That would be a false failure of a required check. I can't verify which way the scanner works from here. This repo reportedly has no findings at or above its threshold, so its passing run may not cover this case. Canary rollout plus the new ring1->stable confirm should catch it before stable. Before confirming that hop, check that the ring repos have below-threshold findings and that their checks still pass.
2. Cone-mode sparse checkout also brings in root files (unconfirmed impact). actions/checkout defaults to sparse-checkout-cone-mode: true. In that mode, git also checks out every top-level file of combsmith/.github (for example AGENTS.md and CLAUDE.md) into .agent-shield-tooling/. The comment says the checkout is "sparse so later checks ignore it". But if the later "Org-specific structural checks" in this job glob the workspace recursively, they could find these files. Those steps are not in the diff, so I couldn't check. Options: set sparse-checkout-cone-mode: false, or rm -rf .agent-shield-tooling after the gate.
3. job.workflow_sha is not tested across repos (unconfirmed). This repo calls the reusable by local ref. If the expression were empty, actions/checkout would fall back to the PR's own ref, and the check would still pass. So this PR's green run does not prove that the ref resolves to the pinned SHA for callers in other repos. The PR cites the #839 precedent. Watch the first downstream ring.
4. Merge order. The description asks for this to merge after #1419, which edits the same scan step. I can't see #1419's state. Whoever merges should check the order. The author reports a clean trial merge.

Minor: the require_confirmation opt-in is permanent for every future agent-shield release, not only this one. The _confirm_note explains why. Drop it later if that wasn't intended.

Not run: the run_secret_scanning MCP tool was not available in this run. The gitleaks CI check passed. I did not run shellcheck or bats myself. I'm relying on the green AgentShield Gate Tests / Lint and bats and CI / ShellCheck checks. The bats file has no negated commands that could never fail.

CI status

All checks completed. The five required checks passed: SonarCloud, CodeQL, agent-shield / AgentShield, dependency-audit / Detect ecosystems and duplicate-decl-gate. The new AgentShield Gate Tests / Lint and bats, Canary-rollout Tests, CI / ShellCheck, Secret scan (gitleaks) and Dependency Hardening Tests also passed. SonarCloud's Quality Gate passed with 0 new issues and 0 hotspots. The remaining checks were skipped (dependabot, unused audit ecosystems, idle dev-lead jobs), and CodeRabbit is NEUTRAL (rate-limited, not required). There are no open review threads, no CHANGES_REQUESTED, and no human questions waiting for an answer. Merge is still BLOCKED until a review from org-leads.

---

Reviewed automatically by the PR-review agent (single-reviewer mode: opus 5.5 [opus 4.8, opus 4.7]). Reply if you need a human review.

[donpetry-bot]

pr-review approved on PARTIAL advisory evidence: 2/5 required advisory bots reported before the gate's head-age-timeout fallback proceeded. Recorded for the miss-rate metric (#1596).