K2: scan a pull request's diff and comment the band-aid

[henleda]

The shift left: a developer sees the F5 XC virtual patch that would hold their new hole closed in the review where they introduced it.

.github/actions/vpcopilot-scan/action.yml   the action
.github/workflows/pr-review.yml             an example workflow
vpcopilot ci-review                          the CLI it shells out to
docs/CI.md                                   the reference

Acceptance, as met

One comment carrying the policy ✅ · a PR with nothing above the threshold posts nothing ✅ · 76 s on the Nimbus repo against a three-minute budget ✅ · never writes to XC ✅, structurally.

Live: a deliberately vulnerable /api/refund route → three findings (critical negative-amount, critical SQLi, high missing-authorization) and four policies across service_policy, waf, malicious_user, rate_limit.

The acceptance criterion contradicted itself

It asked for one comment carrying the policy and the simulation result while also requiring never writes to XC from CI. Both cannot hold: G2's blast-radius measurement creates a throwaway policy, attaches it to a load balancer, replays traffic through it and deletes it — three tenant writes — and there is no offline evaluator, because G1 was deliberately deferred.

So a would-block count is not computed in CI. The comment reports blast radius only from a real tenant run's simulation.json (passed via the simulation-json input), and otherwise says so plainly and names what it would take. Neither silence nor 0% would do: both read as measured and safe. Same precedent as G2's own rewritten criterion, G4's reproducibility, and I2's conflict criterion.

Never writes to XC, by construction

ci.py imports no xc, no apply, no refiner, no simulate — there is no code path from CI to a load balancer, pinned by a test that reads the module's own source, which is the only version of that guarantee that survives someone adding a convenient import later. The action declares no XC inputs, and ci-review says so out loud if it finds an XC credential in its environment.

The defect that would have shipped a silent all-clear

git diff --name-only answers relative to the repository root; collect_files matches relative to repo-path; this project's app fixture lives eight levels down. Compared directly they never match — zero files scanned, no findings, PR told it is clean. Catching it means noticing the absence of an error. rebase_onto() translates the paths and returns a count of changed files outside the scanned directory, which the comment discloses.

Adversarial review before shipping

21 raised across four dimensions; 12 verified — 2 confirmed, 10 refuted, again mostly because they had already been fixed while the review ran. Almost every real finding was one shape: a failure rendering as an all-clear.

• Refuted candidates were being reported as findings. findings.json is the discover contract — every candidate, including ones verify refuted — and the filter compared that set against itself, so it was a no-op. Now built from findings that have a triage decision. Showing developers refuted false positives with a band-aid attached is the fastest way to get a bot ignored.
• A diff with nothing scannable produced an empty comment, so no comment file was written and the step summary fell through to "no findings at or above the threshold" — a clean bill of health for a diff never analysed, and reachable by default (the workflow triggers on any **/*.py change while scanning one fixture directory).
• Truncating an oversized comment deleted exactly the disclosures — GitHub caps a body at 65,536 chars, and cutting from the end removed the "N files were not scanned" lines. The finding list is cut instead.
• A crash exited 1, which the action reads as "findings reported" — so a review that never completed looked like a completed one. Now exit 2, and the summary consults the step outcome.
• Three more ways a meaningless blast-radius number read as safe, each now carried through from G2: enforcement not confirmed (G2's own first live defect — an unenforced policy blocked 0 of 200 and looked harmless), zero requests evaluated, and a sample replayed against XC access logs which carry no request bodies, so a body_matcher policy matches nothing and scores a perfect 0%.
• Plus: git quotes non-ASCII paths ("caf\303\251.py"), so the suffix read as .py" and the file vanished — neither scanned nor counted as outside (-z fixes it); base: origin/main became origin/origin/main; action inputs reached bash by ${{ }} text substitution rather than the environment; --min-severity was unvalidated and silently fell through to high; the header's denominator used the post-filter count, so a fourteen-file diff read as "1 of 1".

Notes for review

• The fixture is in bench/fixtures/ci/, deliberately not in the Nimbus app. bench scans bench/fixtures/nimbus-vuln-lab/app/src/app/api against answer_key.yaml, and a vulnerable route there produces findings in neither expected nor bonus — which bench.py scores as noise, silently degrading G4's committed scorecard and BASELINE.md. A benchmark regression caused by a test fixture. Pinned by a test, with the reason in the fixture README.
• pull_request, never pull_request_target — the latter runs trusted workflow code with secrets against untrusted head code. The consequence, that fork PRs get no review, is documented rather than discovered.
• fail-on-findings defaults to false. The comment is the deliverable; a red check on a finding the team has accepted is how a useful bot gets switched off.
• The Nimbus fixture tree is byte-identical to main (verified), so no benchmark numbers move.

Verification

• 735 tests pass offline, ruff clean, coverage 79%; ci.py at 87%.
• Live end-to-end twice, before and after the review fixes: 77 s and 76 s, exit 1, three findings, blast radius honestly unmeasured, out-of-scope files disclosed.

Roadmap now 12 done / 5 open.

🤖 Generated with Claude Code