From f41340d4ccd72d58223a000ae1e4d7f1903482c5 Mon Sep 17 00:00:00 2001 From: henleda Date: Wed, 5 Aug 2026 18:47:44 +0530 Subject: [PATCH] =?UTF-8?q?fix(review):=20six=20more=20=E2=80=94=20reporti?= =?UTF-8?q?ng=20honesty,=20surface=20parity,=20two=20vacuous=20tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1. report.html claimed "✓ code fix drafted" for a dependency upgrade. The badge keyed on the mere PRESENCE of a remediation, so a `dependency_upgrade` — where there is no file to patch and pr.py writes no diff — claimed a drafted code fix, contradicting the hero on the same page. Three outcomes now: upgrade (naming package and version), drafted code fix, and "cure planned — no patch drafted" for a plan carrying neither a diff nor patched_content. 2. The blast-radius table reported an unmeasured replay as safe. `simulate` computes evaluated / errored / enforcement_confirmed / reason and the table used NONE of them, so a replay in which every request failed in transit (evaluated=0, errored=12, reason="nothing measurable") rendered as a green "within threshold" at 0.0% — a rate that means "we measured nothing", presented as "safe to promote". Now: not measured / unconfirmed / within threshold / over threshold, with a caveats column carrying the reason. 3. reconcile reported a transport failure as "no runnable probe". `{}` is what "no probe recorded" returns — a PERMANENT condition an operator can only fix by re-scanning. A probe that exists and blew up in transport is TRANSIENT. Reporting the second as the first sends someone to fix the wrong thing. Now its own hold code, saying the next pass will retry. 4. MCP ignored VPCOPILOT_SIM_THRESHOLD. The threshold was a lookup each surface had to REMEMBER. CLI, console and refiner remembered; MCP did not — so an operator's tightened blast-radius threshold was silently replaced by the default for every simulation an agent ran. Now `simulate.effective_threshold`, one resolver all four call, and a malformed value is REFUSED rather than silently defaulted: substituting the default would apply a threshold the operator did not choose to the gate that decides whether a policy is safe to promote. 5. test_safe_rollback_restores_and_verifies never tested "verifies". FakeXC genuinely applies the PUT, so the restore succeeded on its own and `verify` returned True whatever it did — delete the verify call from safe_rollback and the test still passed. It was asserting the behaviour of the fake. A new test uses an appliance that ACCEPTS the rollback PUT and applies nothing (a 200 that changed nothing), which is what a partially degraded control plane looks like, and asserts RollbackError plus the audit record. Without it, safe_rollback reports a clean rollback while the band-aid is still attached to a live LB — the "silent half-rollback" the module docstring calls the worst outcome. 6. The four-place agent registration guard was a whole-file substring search. It asserted `"resolve" in report.py`, which the unrelated word "resolved" already satisfied — so it passed with the agent missing from the very list it guarded. report.py now exposes REPORTED_AGENTS and the test asserts the LIST; a second test asserts all four sites AGREE, which holds for the next agent rather than for this one by name. Also: tests/test_report.py's remediation fixture carried empty `diff` and `patched_content`, so it was asserting the drafted-fix badge for a plan with no patch. Given a real patch — an unrealistic fixture, not a behaviour change. 10 new tests; suite 1067 -> 1077. Mutation-verified: reverting the four source fixes fails 7, and the two vacuous tests fail against the mutations they previously survived. Live suite: 18 passed, estates restored (0 ASM policies, Larkspur balance 48215, vpcopilot-lab at baseline). Co-Authored-By: Claude Opus 5 (1M context) --- BACKLOG.md | 12 +-- demo/out/audit.log | 26 +++--- demo/out/ledger.json | 20 ++--- demo/out/report.html | 4 +- demo/out/run.json | 4 +- demo/out/simulation.json | 2 +- src/vpcopilot/cli.py | 6 +- src/vpcopilot/console/app.py | 7 +- src/vpcopilot/mcp.py | 8 +- src/vpcopilot/reconcile.py | 13 ++- src/vpcopilot/refiner.py | 7 +- src/vpcopilot/report.py | 57 +++++++++++-- src/vpcopilot/simulate.py | 29 +++++++ tests/test_engine.py | 33 +++++++- tests/test_inputs_cve.py | 27 +++++- tests/test_report.py | 10 ++- tests/test_review_tail.py | 154 +++++++++++++++++++++++++++++++++++ 17 files changed, 358 insertions(+), 61 deletions(-) create mode 100644 tests/test_review_tail.py diff --git a/BACKLOG.md b/BACKLOG.md index 2432a5b..ce57c1a 100644 --- a/BACKLOG.md +++ b/BACKLOG.md @@ -186,17 +186,17 @@ d=tempfile.mkdtemp` - *Fails when:* mcp.py:128-137 declines `patches_list`, `ledger` and `impact` for a missing `out`, with the docstring "A typo'd `out` would have produced the most reassuring possible answer." That exact answer is what the other two surfaces give: `vpcopilot patches-list --out ` prints `no live band-aids` in green and exits 0 (cli.py:1016-1018), and the console — whose OUT global is set straight from the Sca - *Repro:* `/Users/d.henley/demos/virtual-patch-copilot/.venv/bin/python -m vpcopilot.cli patches-list --out /nonexistent-run-dir; echo "exit=$?" # then the MCP contrast: .venv/bin/python -c` - *Why the suite misses it:* The guard lives in mcp.py rather than in reconcile.list_patches / ledger.load / impact.impact, so the only test that can see it is tests/test_mcp.py:790 — the sole "no run directory" assertion in the suite, driven through MCP frames. tests/test_console_reconcile.py and the CLI tests always build a r -- [ ] **MEDIUM** `reconcile.py:494` — reconcile reports a probe that blew up in transport as "this finding has no runnable probe" — a permanent, unfixable condition — indistinguishable from a finding that genuinely has no probe recorded +- [x] **MEDIUM** `reconcile.py:494` — reconcile reports a probe that blew up in transport as "this finding has no runnable probe" — a permanent, unfixable condition — indistinguishable from a finding that genuinely has no probe recorded - *Fails when:* `_probe` (reconcile.py:300-304) catches every exception from `probe_from_spec` — DNS failure, connect timeout, TLS error, a transient 5xx at the origin — and returns `{}`, the same value it returns when `probes.json` has no entry for the finding. The caller at reconcile.py:494 tests `not probe or probe.get("exploit_status") is None` and holds with "cure merged, but this finding has no runnable pro - *Repro:* `/Users/d.henley/demos/virtual-patch-copilot/.venv/bin/python /tmp/vpr3/repro_reconcile.py (two run dirs, both cure=merged and origin healthy; A has no probes.json, B has a valid ` - *Why the suite misses it:* tests/test_reconcile.py substitutes the whole of `_probe` via `_fake_probe(monkeypatch, result)` (line 55-56), so the `except Exception -> return {}` arm inside `_probe` is never executed by the suite. `skipped_no_probe` is only ever tested by omitting probes.json, i.e. the branch that is genuinely -- [ ] **MEDIUM** `report.py:336` — report.py's blast-radius table drops `reason`, `errored`, `enforcement_confirmed`, `carried_from` and the whole `caveats` list — a replay where every request failed in transit renders as a green "within threshold" 0.0% +- [x] **MEDIUM** `report.py:336` — report.py's blast-radius table drops `reason`, `errored`, `enforcement_confirmed`, `carried_from` and the whole `caveats` list — a replay where every request failed in transit renders as a green "within threshold" 0.0% - *Fails when:* `simulate._score` (simulate.py:155-157) sets `reason = "nothing measured — every replayed request failed in transit, so this is zero evidence, not a clean result"` when `evaluated == 0`, and leaves `blocked_promotion=False`, `block_rate=0.0`, `error=""`. `_blast_radius_html`'s verdict expression (report.py:332-334) only branches on `blocked_promotion` and `error`, so it falls through to ` {r.status_code}: {r.text[:400]}"`, and none of `method`, `path`, `500` or `boom` can ever contain the password. Replace `src/vpcopilot/bigip.py:68` with `return s`, and both that test and all 1009 offline test - *Repro:* `zsh /private/tmp/claude-502/-Users-d-henley-demos-virtual-patch-copilot/fa4d9adf-b050-4d9d-911d-2130d7c6285b/scratchpad/repro2_bigip.sh # rsyncs to /tmp/vpc-repro2, rewrites bigi` - *Why the suite misses it:* The mock transport returns a fixed body (`"boom"`) that is not derived from the credential under test, so the negative assertion is trivially true. A redaction test must plant the secret in the payload being redacted; this one plants it only in the client constructor. -- [ ] **MEDIUM** `tests/test_inputs_cve.py:323` — `test_the_resolve_agent_is_registered_everywhere_it_has_to_be` checks report.py with a whole-file substring search that unrelated dependency-report text already satisfies +- [x] **MEDIUM** `tests/test_inputs_cve.py:323` — `test_the_resolve_agent_is_registered_everywhere_it_has_to_be` checks report.py with a whole-file substring search that unrelated dependency-report text already satisfies - *Fails when:* The assertion is `assert "resolve" in Path("src/vpcopilot/report.py").read_text()`. report.py contains `"resolve"` in six unrelated places in `_dependencies_html` (`("not_resolved", "not resolved")`, `"Listed, not resolved."`, `"resolved against OSV.dev"`, ...), so the check is satisfied no matter what `_models_html` contains. Delete `"resolve"` from the hardcoded list at src/vpcopilot/report.py:3 - *Repro:* `zsh /private/tmp/claude-502/-Users-d-henley-demos-virtual-patch-copilot/fa4d9adf-b050-4d9d-911d-2130d7c6285b/scratchpad/repro4_report.sh # rsyncs to /tmp/vpc-repro4, drops "resol` - *Why the suite misses it:* report.py's agent list is a module-local literal inside `_models_html`, not an importable constant, so the test reached for a text search instead of a membership test. The search is over the whole file, and the H2 dependency section independently contains the same word. No other test renders the mod ### Ungrouped -- [ ] **MEDIUM** `engine.py:115` — `test_safe_rollback_restores_and_verifies` never proves the "verifies" half — the `verify` callable can be dropped and rollback will report success on an LB that was not restored +- [x] **MEDIUM** `engine.py:115` — `test_safe_rollback_restores_and_verifies` never proves the "verifies" half — the `verify` callable can be dropped and rollback will report success on an LB that was not restored diff --git a/demo/out/audit.log b/demo/out/audit.log index 94140ee..01b8862 100644 --- a/demo/out/audit.log +++ b/demo/out/audit.log @@ -1,13 +1,13 @@ -{"ts": "2026-08-05T01:27:30.048411+00:00", "action": "refine_apply", "run_id": "7f125a9722c5", "actor": "security-oncall", "host": "vpcopilot-demo", "tool_version": "0.1.0", "finding_id": "crapi-sqli-001", "namespace": "crapi-demo", "control": "service_policy", "policy": "deny-login-sqli", "lb": "crapi-lab", "passed": true, "attempts": 2, "before_after": {"before": {"exploit_status": 200, "exploit_blocked": false, "legit_ok": true}, "after": {"exploit_status": 403, "exploit_blocked": true, "legit_ok": true}}} -{"ts": "2026-08-05T01:27:34.048411+00:00", "action": "apply_timing", "run_id": "7f125a9722c5", "actor": "security-oncall", "host": "vpcopilot-demo", "tool_version": "0.1.0", "control": "service_policy", "finding_id": "crapi-sqli-001", "passed": true, "elapsed_s": 48.0, "attempts": 2, "before_after": {"before": {"exploit_status": 200, "exploit_blocked": false, "legit_ok": true}, "after": {"exploit_status": 403, "exploit_blocked": true, "legit_ok": true}}} -{"ts": "2026-08-05T01:28:26.048411+00:00", "action": "create_api_definition", "run_id": "7f125a9722c5", "actor": "security-oncall", "host": "vpcopilot-demo", "tool_version": "0.1.0", "finding_id": "crapi-bola-002", "namespace": "crapi-demo", "name": "crapi-lab-apidef", "swagger": "crapi-lab-swagger"} -{"ts": "2026-08-05T01:28:30.048411+00:00", "action": "apply_api_schema", "run_id": "7f125a9722c5", "actor": "security-oncall", "host": "vpcopilot-demo", "tool_version": "0.1.0", "finding_id": "crapi-bola-002", "namespace": "crapi-demo", "apidef": "crapi-lab-apidef", "lb": "crapi-lab", "passed": true, "kept": true, "before_after": {"before": {"exploit_status": 200, "exploit_blocked": false, "legit_ok": true}, "after": {"exploit_status": 403, "exploit_blocked": true, "legit_ok": true}}} -{"ts": "2026-08-05T01:28:34.048411+00:00", "action": "apply_timing", "run_id": "7f125a9722c5", "actor": "security-oncall", "host": "vpcopilot-demo", "tool_version": "0.1.0", "control": "api_schema", "finding_id": "crapi-bola-002", "passed": true, "elapsed_s": 33.0, "attempts": 1, "before_after": {"before": {"exploit_status": 200, "exploit_blocked": false, "legit_ok": true}, "after": {"exploit_status": 403, "exploit_blocked": true, "legit_ok": true}}} -{"ts": "2026-08-05T01:29:11.048411+00:00", "action": "apply_waf", "run_id": "7f125a9722c5", "actor": "security-oncall", "host": "vpcopilot-demo", "tool_version": "0.1.0", "finding_id": "crapi-mass-003", "namespace": "crapi-demo", "app_firewall": "crapi-lab-waf", "lb": "crapi-lab", "config_enabled": true, "kept": true, "before_after": {"before": {"exploit_status": 200, "exploit_blocked": false, "legit_ok": true}, "after": {"exploit_status": 403, "exploit_blocked": true, "legit_ok": true}}} -{"ts": "2026-08-05T01:29:15.048411+00:00", "action": "apply_timing", "run_id": "7f125a9722c5", "actor": "security-oncall", "host": "vpcopilot-demo", "tool_version": "0.1.0", "control": "waf", "finding_id": "crapi-mass-003", "passed": true, "elapsed_s": 21.0, "attempts": 1, "before_after": {"before": {"exploit_status": 200, "exploit_blocked": false, "legit_ok": true}, "after": {"exploit_status": 403, "exploit_blocked": true, "legit_ok": true}}} -{"ts": "2026-08-05T01:29:40.048411+00:00", "action": "apply_rate_limit", "run_id": "7f125a9722c5", "actor": "security-oncall", "host": "vpcopilot-demo", "tool_version": "0.1.0", "finding_id": "crapi-bruteforce-004", "namespace": "crapi-demo", "rate": "5/MINUTE", "lb": "crapi-lab", "passed": true, "kept": true, "behavioral": {"sent": 30, "limited": 25, "passed": 5, "codes": {"200": 5, "429": 25}}} -{"ts": "2026-08-05T01:29:44.048411+00:00", "action": "apply_timing", "run_id": "7f125a9722c5", "actor": "security-oncall", "host": "vpcopilot-demo", "tool_version": "0.1.0", "control": "rate_limit", "finding_id": "crapi-bruteforce-004", "passed": true, "elapsed_s": 27.0, "attempts": 1} -{"ts": "2026-08-05T01:30:15.048411+00:00", "action": "apply_data_guard", "run_id": "7f125a9722c5", "actor": "security-oncall", "host": "vpcopilot-demo", "tool_version": "0.1.0", "finding_id": "crapi-tokenleak-006", "namespace": "crapi-demo", "app_firewall": "crapi-lab-waf", "lb": "crapi-lab", "enabled": true, "kept": true} -{"ts": "2026-08-05T01:30:19.048411+00:00", "action": "apply_timing", "run_id": "7f125a9722c5", "actor": "security-oncall", "host": "vpcopilot-demo", "tool_version": "0.1.0", "control": "waf_data_guard", "finding_id": "crapi-tokenleak-006", "passed": true, "elapsed_s": 19.0, "attempts": 1} -{"ts": "2026-08-05T01:30:42.048411+00:00", "action": "open_pr", "run_id": "7f125a9722c5", "actor": "security-oncall", "host": "vpcopilot-demo", "tool_version": "0.1.0", "finding_id": "crapi-sqli-001", "finding": "crapi-sqli-001", "repo": "acme/crapi", "url": "https://github.com/acme/crapi/pull/311", "number": 311} -{"ts": "2026-08-05T01:30:46.048411+00:00", "action": "retire", "run_id": "7f125a9722c5", "actor": "security-oncall", "host": "vpcopilot-demo", "tool_version": "0.1.0", "finding_id": "crapi-sqli-001", "namespace": "crapi-demo", "control": "service_policy", "lb": "crapi-lab", "forced": false} +{"ts": "2026-08-05T13:06:09.732288+00:00", "action": "refine_apply", "run_id": "cfc003c6cfb6", "actor": "security-oncall", "host": "vpcopilot-demo", "tool_version": "0.1.0", "finding_id": "crapi-sqli-001", "namespace": "crapi-demo", "control": "service_policy", "policy": "deny-login-sqli", "lb": "crapi-lab", "passed": true, "attempts": 2, "before_after": {"before": {"exploit_status": 200, "exploit_blocked": false, "legit_ok": true}, "after": {"exploit_status": 403, "exploit_blocked": true, "legit_ok": true}}} +{"ts": "2026-08-05T13:06:13.732288+00:00", "action": "apply_timing", "run_id": "cfc003c6cfb6", "actor": "security-oncall", "host": "vpcopilot-demo", "tool_version": "0.1.0", "control": "service_policy", "finding_id": "crapi-sqli-001", "passed": true, "elapsed_s": 48.0, "attempts": 2, "before_after": {"before": {"exploit_status": 200, "exploit_blocked": false, "legit_ok": true}, "after": {"exploit_status": 403, "exploit_blocked": true, "legit_ok": true}}} +{"ts": "2026-08-05T13:07:05.732288+00:00", "action": "create_api_definition", "run_id": "cfc003c6cfb6", "actor": "security-oncall", "host": "vpcopilot-demo", "tool_version": "0.1.0", "finding_id": "crapi-bola-002", "namespace": "crapi-demo", "name": "crapi-lab-apidef", "swagger": "crapi-lab-swagger"} +{"ts": "2026-08-05T13:07:09.732288+00:00", "action": "apply_api_schema", "run_id": "cfc003c6cfb6", "actor": "security-oncall", "host": "vpcopilot-demo", "tool_version": "0.1.0", "finding_id": "crapi-bola-002", "namespace": "crapi-demo", "apidef": "crapi-lab-apidef", "lb": "crapi-lab", "passed": true, "kept": true, "before_after": {"before": {"exploit_status": 200, "exploit_blocked": false, "legit_ok": true}, "after": {"exploit_status": 403, "exploit_blocked": true, "legit_ok": true}}} +{"ts": "2026-08-05T13:07:13.732288+00:00", "action": "apply_timing", "run_id": "cfc003c6cfb6", "actor": "security-oncall", "host": "vpcopilot-demo", "tool_version": "0.1.0", "control": "api_schema", "finding_id": "crapi-bola-002", "passed": true, "elapsed_s": 33.0, "attempts": 1, "before_after": {"before": {"exploit_status": 200, "exploit_blocked": false, "legit_ok": true}, "after": {"exploit_status": 403, "exploit_blocked": true, "legit_ok": true}}} +{"ts": "2026-08-05T13:07:50.732288+00:00", "action": "apply_waf", "run_id": "cfc003c6cfb6", "actor": "security-oncall", "host": "vpcopilot-demo", "tool_version": "0.1.0", "finding_id": "crapi-mass-003", "namespace": "crapi-demo", "app_firewall": "crapi-lab-waf", "lb": "crapi-lab", "config_enabled": true, "kept": true, "before_after": {"before": {"exploit_status": 200, "exploit_blocked": false, "legit_ok": true}, "after": {"exploit_status": 403, "exploit_blocked": true, "legit_ok": true}}} +{"ts": "2026-08-05T13:07:54.732288+00:00", "action": "apply_timing", "run_id": "cfc003c6cfb6", "actor": "security-oncall", "host": "vpcopilot-demo", "tool_version": "0.1.0", "control": "waf", "finding_id": "crapi-mass-003", "passed": true, "elapsed_s": 21.0, "attempts": 1, "before_after": {"before": {"exploit_status": 200, "exploit_blocked": false, "legit_ok": true}, "after": {"exploit_status": 403, "exploit_blocked": true, "legit_ok": true}}} +{"ts": "2026-08-05T13:08:19.732288+00:00", "action": "apply_rate_limit", "run_id": "cfc003c6cfb6", "actor": "security-oncall", "host": "vpcopilot-demo", "tool_version": "0.1.0", "finding_id": "crapi-bruteforce-004", "namespace": "crapi-demo", "rate": "5/MINUTE", "lb": "crapi-lab", "passed": true, "kept": true, "behavioral": {"sent": 30, "limited": 25, "passed": 5, "codes": {"200": 5, "429": 25}}} +{"ts": "2026-08-05T13:08:23.732288+00:00", "action": "apply_timing", "run_id": "cfc003c6cfb6", "actor": "security-oncall", "host": "vpcopilot-demo", "tool_version": "0.1.0", "control": "rate_limit", "finding_id": "crapi-bruteforce-004", "passed": true, "elapsed_s": 27.0, "attempts": 1} +{"ts": "2026-08-05T13:08:54.732288+00:00", "action": "apply_data_guard", "run_id": "cfc003c6cfb6", "actor": "security-oncall", "host": "vpcopilot-demo", "tool_version": "0.1.0", "finding_id": "crapi-tokenleak-006", "namespace": "crapi-demo", "app_firewall": "crapi-lab-waf", "lb": "crapi-lab", "enabled": true, "kept": true} +{"ts": "2026-08-05T13:08:58.732288+00:00", "action": "apply_timing", "run_id": "cfc003c6cfb6", "actor": "security-oncall", "host": "vpcopilot-demo", "tool_version": "0.1.0", "control": "waf_data_guard", "finding_id": "crapi-tokenleak-006", "passed": true, "elapsed_s": 19.0, "attempts": 1} +{"ts": "2026-08-05T13:09:21.732288+00:00", "action": "open_pr", "run_id": "cfc003c6cfb6", "actor": "security-oncall", "host": "vpcopilot-demo", "tool_version": "0.1.0", "finding_id": "crapi-sqli-001", "finding": "crapi-sqli-001", "repo": "acme/crapi", "url": "https://github.com/acme/crapi/pull/311", "number": 311} +{"ts": "2026-08-05T13:09:25.732288+00:00", "action": "retire", "run_id": "cfc003c6cfb6", "actor": "security-oncall", "host": "vpcopilot-demo", "tool_version": "0.1.0", "finding_id": "crapi-sqli-001", "namespace": "crapi-demo", "control": "service_policy", "lb": "crapi-lab", "forced": false} diff --git a/demo/out/ledger.json b/demo/out/ledger.json index dad6085..be54930 100644 --- a/demo/out/ledger.json +++ b/demo/out/ledger.json @@ -24,9 +24,9 @@ "no_bandaid": false, "has_cure": true, "ttl": { - "applied_at": "2026-08-05T01:36:30.033704+00:00", + "applied_at": "2026-08-05T13:15:09.715566+00:00", "ttl_hours": 168, - "expires_at": "2026-08-12T01:36:30.033704+00:00" + "expires_at": "2026-08-12T13:15:09.715566+00:00" } }, "crapi-bola-002": { @@ -54,9 +54,9 @@ "no_bandaid": false, "has_cure": true, "ttl": { - "applied_at": "2026-08-05T01:36:30.034904+00:00", + "applied_at": "2026-08-05T13:15:09.716815+00:00", "ttl_hours": 168, - "expires_at": "2026-08-12T01:36:30.034904+00:00" + "expires_at": "2026-08-12T13:15:09.716815+00:00" } }, "crapi-mass-003": { @@ -81,9 +81,9 @@ "no_bandaid": false, "has_cure": true, "ttl": { - "applied_at": "2026-08-05T01:36:30.036950+00:00", + "applied_at": "2026-08-05T13:15:09.717778+00:00", "ttl_hours": 168, - "expires_at": "2026-08-12T01:36:30.036950+00:00" + "expires_at": "2026-08-12T13:15:09.717778+00:00" } }, "crapi-bruteforce-004": { @@ -108,9 +108,9 @@ "no_bandaid": false, "has_cure": true, "ttl": { - "applied_at": "2026-08-05T01:36:30.038715+00:00", + "applied_at": "2026-08-05T13:15:09.720633+00:00", "ttl_hours": 168, - "expires_at": "2026-08-12T01:36:30.038715+00:00" + "expires_at": "2026-08-12T13:15:09.720633+00:00" } }, "crapi-tokenleak-006": { @@ -138,9 +138,9 @@ "no_bandaid": false, "has_cure": true, "ttl": { - "applied_at": "2026-08-05T01:36:30.039707+00:00", + "applied_at": "2026-08-05T13:15:09.721386+00:00", "ttl_hours": 168, - "expires_at": "2026-08-12T01:36:30.039707+00:00" + "expires_at": "2026-08-12T13:15:09.721386+00:00" } }, "crapi-userenum-005": { diff --git a/demo/out/report.html b/demo/out/report.html index 66fdf3b..1048633 100644 --- a/demo/out/report.html +++ b/demo/out/report.html @@ -62,7 +62,7 @@ .model .a{color:var(--grey)}.model .m{font-family:ui-monospace,Menlo,monospace;color:var(--f5)}

virtual-patch·copilot — scan report

-
target: crapi-lab · generated 2026-08-05 01:36 UTC
+
target: crapi-lab · generated 2026-08-05 13:15 UTC
6exploitable vulns
→
5mitigated live by XC
29.6stime to mitigate · 72,973× faster
vs
25 daysnormal change control
6code-fix PRs (the cure)

Run summary

9candidates
6verified
5band-aided
1code-cure only
5XC policies
6code-fix PRs
@@ -72,7 +72,7 @@

Pipeline metrics

19

Findings & band-aid coverage

criticalSQL injection in logincrapi-sqli-001sqliservices/identity/login.js:42
service_policy · full✓ code fix drafted
Details

Description: The email field is concatenated straight into the auth query.

Exploit: email=" OR 1=1 -- lets any password through and dumps the users table.

Code cure: Fix sql injection in login

highBOLA on vehicle locationcrapi-bola-002broken_object_authzservices/identity/vehicle.js:88
api_schema · full✓ code fix drafted
Details

Description: Any authenticated user can read another user's vehicle GPS by id.

Exploit: Swap {id} to another user's vehicle uuid; server returns their live location.

Code cure: Fix bola on vehicle location

highMass assignment on profilecrapi-mass-003mass_assignmentservices/identity/dashboard.js:61
waf · partial✓ code fix drafted
Details

Description: The update handler binds the whole body, so `role` and `credit` are writable.

Exploit: POST {"role":"admin","available_credit":9999} — privilege + balance escalation.

Code cure: Fix mass assignment on profile

highJWT + card data in responsecrapi-tokenleak-006sensitive_dataservices/workshop/mechanic.js:120
waf_data_guard · partial✓ code fix drafted
Details

Description: The receipt payload echoes the full PAN and a signed service token.

Exploit: GET a receipt; response body contains the 16-digit card number in cleartext.

Code cure: Fix jwt + card data in response

mediumNo rate limit on OTP verifycrapi-bruteforce-004rate_abuseservices/identity/otp.js:30
rate_limit · full✓ code fix drafted
Details

Description: The 4-digit OTP endpoint has no throttle — brute-forceable in minutes.

Exploit: Fire all 10k OTPs; no lockout, no delay.

Code cure: Fix no rate limit on otp verify

mediumUsername enumeration on signupcrapi-userenum-005broken_authservices/identity/signup.js:25
no band-aid — code cure only✓ code fix drafted
Residual risk: no positive-security band-aid fits; ships as code-only.
Details

Description: Distinct errors for taken vs free emails leak which accounts exist.

Exploit: Diff the 'already registered' vs 'ok' responses to enumerate users.

Code cure: Fix username enumeration on signup

Generated XC band-aid policies

api_schema crapi-lab-apidef
rate_limit otp-throttle
service_policy deny-login-sqli
waf crapi-lab-waf
waf_data_guard mask-pan
-

Band-aid impact exploit before → after (live validation)

controlpolicyexploit beforeexploit afterlegitresultwhen
service_policy self-healed ×2deny-login-sqli200 allowed403 blockedokPASS2026-08-05T01:27:30
apply_timing self-healed ×2200 allowed403 blockedokPASS2026-08-05T01:27:34
api_schemacrapi-lab-apidef200 allowed403 blockedokPASS2026-08-05T01:28:30
apply_timing200 allowed403 blockedokPASS2026-08-05T01:28:34
wafcrapi-lab-waf200 allowed403 blockedokfail2026-08-05T01:29:11
apply_timing200 allowed403 blockedokPASS2026-08-05T01:29:15
rate_limit5/MINUTEburst 30 allowed25/30 rate-limited (429)—PASS2026-08-05T01:29:40

Blast radius what each band-aid would block in recorded traffic — 500 of 500 recorded request(s) replayed through crapi-lab · window 2026-07-20T09:00:00Z..2026-07-20T10:00:00Z

policyevaluatedwould blockrateverdicttop blocked paths
deny-login-sqli50030.6%within threshold/identity/api/auth/login ×3
+

Band-aid impact exploit before → after (live validation)

controlpolicyexploit beforeexploit afterlegitresultwhen
service_policy self-healed ×2deny-login-sqli200 allowed403 blockedokPASS2026-08-05T13:06:09
apply_timing self-healed ×2200 allowed403 blockedokPASS2026-08-05T13:06:13
api_schemacrapi-lab-apidef200 allowed403 blockedokPASS2026-08-05T13:07:09
apply_timing200 allowed403 blockedokPASS2026-08-05T13:07:13
wafcrapi-lab-waf200 allowed403 blockedokfail2026-08-05T13:07:50
apply_timing200 allowed403 blockedokPASS2026-08-05T13:07:54
rate_limit5/MINUTEburst 30 allowed25/30 rate-limited (429)—PASS2026-08-05T13:08:19

Blast radius what each band-aid would block in recorded traffic — 500 of 500 recorded request(s) replayed through crapi-lab · window 2026-07-20T09:00:00Z..2026-07-20T10:00:00Z

policyevaluatedwould blockrateverdicttop blocked pathscaveats
deny-login-sqli50030.6%within threshold/identity/api/auth/login ×3—

Remediation ledger found → mitigated → remediated → retired

findingstateband-aidcode cure
crapi-sqli-001retired{'control': 'service_policy', 'policy_name': 'deny-login-sqli', 'lb': 'crapi-lab'}{'pr_url': 'https://github.com/acme/crapi/pull/311', 'pr_number': 311}
crapi-bola-002remediated{'control': 'api_schema', 'policy_name': 'crapi-lab-apidef', 'lb': 'crapi-lab'}{'pr_url': 'https://github.com/acme/crapi/pull/312', 'pr_number': 312}
crapi-mass-003mitigated{'control': 'waf', 'policy_name': 'crapi-lab-waf', 'lb': 'crapi-lab'}—
crapi-bruteforce-004mitigated{'control': 'rate_limit', 'policy_name': 'otp-throttle', 'lb': 'crapi-lab'}—
crapi-tokenleak-006remediated{'control': 'waf_data_guard', 'policy_name': 'mask-pan', 'lb': 'crapi-lab'}{'pr_url': 'https://github.com/acme/crapi/pull/313', 'pr_number': 313}
crapi-userenum-005found——
virtual-patch-copilot 0.1.0 · band-aids are temporary — every finding also gets a code-fix PR
diff --git a/demo/out/run.json b/demo/out/run.json index 4ef8ae6..02fd888 100644 --- a/demo/out/run.json +++ b/demo/out/run.json @@ -1,6 +1,6 @@ { - "run_id": "7f125a9722c5", - "created": "2026-08-05T01:36:30.044247+00:00", + "run_id": "cfc003c6cfb6", + "created": "2026-08-05T13:15:09.728200+00:00", "repo": "/src/crapi", "config_path": "config/agents.yaml", "models": { diff --git a/demo/out/simulation.json b/demo/out/simulation.json index 0d7e8f1..e98faf6 100644 --- a/demo/out/simulation.json +++ b/demo/out/simulation.json @@ -1,5 +1,5 @@ { - "ts": "2026-08-05T01:36:30.049284+00:00", + "ts": "2026-08-05T13:15:09.733157+00:00", "lb": "crapi-lab", "source": "xc:crapi-lab", "records": 500, diff --git a/src/vpcopilot/cli.py b/src/vpcopilot/cli.py index 948cd74..c175ed9 100644 --- a/src/vpcopilot/cli.py +++ b/src/vpcopilot/cli.py @@ -811,8 +811,7 @@ def simulate( """Replay a recorded traffic sample against each generated band-aid and report what it WOULD block, before anything reaches the gate. Read-only against the sample; the spare LB is snapshotted and restored.""" - import os - from .simulate import DEFAULT_THRESHOLD, candidates_from_out, simulate_policies, write_result + from .simulate import candidates_from_out, simulate_policies, write_result cands = candidates_from_out(out, policy) if not cands: @@ -827,7 +826,8 @@ def simulate( + (f"; redacted {sum(v for k, v in redacted.items() if not k.startswith('_'))} value(s)" if redacted else "") + "[/dim]") - thr = threshold if threshold is not None else float(os.environ.get("VPCOPILOT_SIM_THRESHOLD", DEFAULT_THRESHOLD)) + from .simulate import effective_threshold + thr = effective_threshold(threshold) res = simulate_policies(cands, records, lb=lb, url=url, out_dir=out, threshold=thr, max_records=max_records, source=src, window=window, redacted=redacted, log=lambda m: rprint(f"[dim]{m}[/dim]")) diff --git a/src/vpcopilot/console/app.py b/src/vpcopilot/console/app.py index 6fd91b0..8c61a62 100644 --- a/src/vpcopilot/console/app.py +++ b/src/vpcopilot/console/app.py @@ -268,10 +268,9 @@ def _run_simulation(job_id: str, body: SimReq): job = _jobs[job_id] log = lambda m: _append(job["log"], m) # noqa: E731 try: - import os from ..cli import _load_traffic - from ..simulate import DEFAULT_THRESHOLD, candidates_from_out, simulate_policies, write_result + from ..simulate import candidates_from_out, simulate_policies, write_result cands = candidates_from_out(str(OUT), body.policy) if not cands: raise RuntimeError(f"no service_policy artifacts in {OUT} — run a scan first") @@ -280,8 +279,8 @@ def _run_simulation(job_id: str, body: SimReq): if not records: raise RuntimeError("no records ingested — give a traffic file or enable from-tenant") log(f"{len(records)} record(s) from {src}") - thr = body.threshold if body.threshold is not None else float( - os.environ.get("VPCOPILOT_SIM_THRESHOLD", DEFAULT_THRESHOLD)) + from ..simulate import effective_threshold + thr = effective_threshold(body.threshold) res = simulate_policies(cands, records, lb=body.lb, url=body.url, out_dir=str(OUT), threshold=thr, max_records=body.max_records, source=src, window=window, redacted=redacted, log=log) diff --git a/src/vpcopilot/mcp.py b/src/vpcopilot/mcp.py index 0654df1..bf69976 100644 --- a/src/vpcopilot/mcp.py +++ b/src/vpcopilot/mcp.py @@ -637,9 +637,13 @@ def _tool_simulate(policy_name: str | None = None, lb: str = "", url: str = "", records, redacted = load_traffic(logs) if not records: raise Declined(f"no records ingested from {logs!r}") - kw = {} if threshold is None else {"threshold": threshold} + # Resolved through the module, not re-implemented here — this surface used to skip the + # VPCOPILOT_SIM_THRESHOLD lookup entirely, so an operator's tightened threshold was silently + # replaced by the default for every simulation an agent ran. + from .simulate import effective_threshold res = simulate_policies(cands, records, lb=lb, url=url, out_dir=out, max_records=max_records, - source=f"file:{logs}", redacted=redacted, log=log, **kw) + source=f"file:{logs}", redacted=redacted, log=log, + threshold=effective_threshold(threshold)) write_result(out, res) return res.model_dump() if hasattr(res, "model_dump") else dict(res) diff --git a/src/vpcopilot/reconcile.py b/src/vpcopilot/reconcile.py index bc1553d..b6d6f33 100644 --- a/src/vpcopilot/reconcile.py +++ b/src/vpcopilot/reconcile.py @@ -301,7 +301,13 @@ def _probe(entry: dict, out_dir: str, origin: str, *, log: Callable) -> dict: return probe_from_spec(origin, spec, log=log, auth=_reconcile_auth()) except Exception as e: # noqa: BLE001 — a DNS failure on one finding must not end the pass log(f" ⚠ probe failed for {entry.get('finding_id')}: {type(e).__name__}: {e}") - return {} + # NOT `{}`. An empty dict is what "there is no probe recorded for this finding" returns, + # and that is a PERMANENT condition an operator can only fix by re-scanning. A probe that + # exists and blew up in transport is a TRANSIENT one — the origin was unreachable, TLS + # failed, the connection dropped — and it will very likely work on the next pass. Reporting + # the second as the first sends someone to fix the wrong thing, which is the same + # collapse `auth_failed` already exists to prevent one case of. + return {"probe_error": f"{type(e).__name__}: {e}"} def _due_for_probe(entry: dict, now: datetime, *, force: bool) -> bool: @@ -491,6 +497,11 @@ def hold(outcome: str, reason: str, **extra): return hold("skipped_auth_failed", "cure merged, but the probe could not authenticate at origin — check " "VPCOPILOT_PROBE_USER/PASS/LOGIN_PATH; holding the band-aid") + if probe.get("probe_error"): + return hold("skipped_probe_error", + f"cure merged, but the probe could not reach the origin ({probe['probe_error']}) " + f"— this is a transient failure, not a missing probe; the band-aid stays on and " + f"the next pass will retry") if not probe or probe.get("exploit_status") is None: return hold("skipped_no_probe", "cure merged, but this finding has no runnable probe — cannot prove the fix " diff --git a/src/vpcopilot/refiner.py b/src/vpcopilot/refiner.py index 03b0e32..75a42d9 100644 --- a/src/vpcopilot/refiner.py +++ b/src/vpcopilot/refiner.py @@ -39,11 +39,8 @@ def _load_finding(out_dir: str, finding_id: str | None) -> Finding | None: def _sim_threshold_default() -> float: - from .simulate import DEFAULT_THRESHOLD - try: - return float(os.environ.get("VPCOPILOT_SIM_THRESHOLD", DEFAULT_THRESHOLD)) - except ValueError: - return DEFAULT_THRESHOLD + from .simulate import effective_threshold + return effective_threshold() def _resolve_records(records, log: Callable): diff --git a/src/vpcopilot/report.py b/src/vpcopilot/report.py index 178028e..991f17c 100644 --- a/src/vpcopilot/report.py +++ b/src/vpcopilot/report.py @@ -11,6 +11,12 @@ from datetime import datetime, timezone from pathlib import Path +# The agents named in the report's model table. One of the FOUR places a new agent must be +# registered (config.AGENT_NAMES, console.AGENT_ROLES, here, bench_model.AGENTS) — see +# tests/test_inputs_cve.py::test_the_resolve_agent_is_registered_everywhere_it_has_to_be. +REPORTED_AGENTS = ("resolve", "discover", "verify", "triage", "generate", "remediate", + "probe", "refine") + SEV_ORDER = {"critical": 0, "high": 1, "medium": 2, "low": 3} _CSS = """ @@ -117,7 +123,19 @@ def _finding_card(f: dict, decision: dict | None, rem: dict | None) -> str: cov = f' · {_e(b.get("coverage", ""))}' ba.append(f'{_e(b.get("control", ""))}{cov}') if rem: - ba.append('✓ code fix drafted') + # A `dependency_upgrade` is NOT a drafted code fix: there is no file to patch and `pr.py` + # writes no diff for it — the cure is "bump the version in someone else's package". Keying + # the badge on the mere PRESENCE of a remediation made a card claim "✓ code fix drafted" + # while the hero on the SAME page correctly reported 0 code-fix PRs and 1 upgrade to ship. + if (rem.get("kind") or "code_fix") == "dependency_upgrade": + pkg, ver = rem.get("package") or "", rem.get("fixed_version") or "" + detail = f" — {_e(pkg)} → {_e(ver)}" if pkg and ver else "" + ba.append(f'↑ dependency upgrade{detail}') + elif rem.get("patched_content") or rem.get("diff"): + ba.append('✓ code fix drafted') + else: + # A plan with no patch is not a drafted fix. Saying so beats a tick that is not true. + ba.append('cure planned — no patch drafted') ba.append("") parts.append("".join(ba)) @@ -305,7 +323,11 @@ def _models_html() -> str: from .config import load_config import os cfg = load_config(os.environ.get("VPCOPILOT_CONFIG", "config/agents.yaml")) - agents = ["resolve", "discover", "verify", "triage", "generate", "remediate", "probe", "refine"] + # Module-level so a test can assert on the LIST rather than grepping the file. The + # registration guard used a whole-file substring search for "resolve", which the word + # "resolved" elsewhere in this module already satisfied — so the guard passed with the + # agent missing from exactly the list it was guarding. + agents = list(REPORTED_AGENTS) chips = "".join(f'{a} · {_e(cfg.for_agent(a).model)}' for a in agents) except Exception: # noqa: BLE001 @@ -329,13 +351,34 @@ def _blast_radius_html(out_dir: str) -> str: rows = "" for p in pols: rate = f'{(p.get("block_rate") or 0) * 100:.1f}%' - verdict = ('over threshold' if p.get("blocked_promotion") - else ('error' if p.get("error") - else 'within threshold')) + # NOT-MEASURED is a third verdict, not a green one. `simulate` computes `evaluated`, + # `errored`, `enforcement_confirmed` and `reason` and the table used none of them, so a + # replay in which EVERY request failed in transit (evaluated=0, errored=12, + # reason="nothing measurable") rendered as "within threshold" — a 0.0% block rate that + # means "we measured nothing", presented as "this policy is safe to promote". + evaluated = p.get("evaluated") or 0 + errored = p.get("errored") or 0 + if p.get("blocked_promotion"): + verdict = 'over threshold' + elif p.get("error"): + verdict = 'error' + elif not evaluated: + verdict = ('not measured') + rate = "—" + elif p.get("enforcement_confirmed") is False: + verdict = ('unconfirmed') + else: + verdict = 'within threshold' + why = p.get("reason") or "" + if errored: + why = (why + " · " if why else "") + f"{errored} request(s) failed in transit" top = ", ".join(f'{_e(t[0])} ×{_e(t[1])}' for t in (p.get("top_paths") or [])[:3]) or "—" rows += (f'{_e(p.get("policy_name"))}' f'{_e(p.get("evaluated"))}{_e(p.get("would_block"))}' - f'{rate}{verdict}{top}') + f'{rate}{verdict}{top}' + f'{_e(why) or "—"}') meta = (f'{_e(sim.get("records_replayed"))} of {_e(sim.get("records"))} recorded request(s) ' f'replayed through {_e(sim.get("lb"))}') if sim.get("window"): @@ -345,7 +388,7 @@ def _blast_radius_html(out_dir: str) -> str: return ('

Blast radius what each band-aid would block in recorded ' f'traffic — {meta}

' '' - f'{rows}
policyevaluatedwould blockrateverdicttop blocked paths
') + f'verdicttop blocked pathscaveats{rows}') def _dependencies_html(out_dir: str) -> str: diff --git a/src/vpcopilot/simulate.py b/src/vpcopilot/simulate.py index e3ff723..134ab30 100644 --- a/src/vpcopilot/simulate.py +++ b/src/vpcopilot/simulate.py @@ -27,11 +27,40 @@ from .probe import _blocked from .runmeta import utc_now from .schemas import PolicySimulation, RequestRecord, SimulationResult +import os + from .traffic import from_probe_request from .xc import XC MAX_SAMPLES = 10 # blocked requests carried into the report, per policy DEFAULT_THRESHOLD = 0.01 # 1% of the sample — override with --threshold / VPCOPILOT_SIM_THRESHOLD + + +def effective_threshold(explicit: float | None = None) -> float: + """The blast-radius threshold, resolved in ONE place. + + This used to be a lookup each surface had to remember to perform. The CLI, the console and the + refiner all remembered; the MCP server did not — so an operator who had tightened + `VPCOPILOT_SIM_THRESHOLD` got the default silently applied to every simulation an agent ran, + and the number they set was the number they believed was in force. + + A guard that each caller re-implements is a guard that holds in some callers. Making it a + property of the module means a fifth consumer inherits it rather than reimplementing it. + """ + if explicit is not None: + return float(explicit) + raw = os.environ.get("VPCOPILOT_SIM_THRESHOLD", "") + if not raw.strip(): + return DEFAULT_THRESHOLD + try: + return float(raw) + except ValueError: + # Refusing to guess: a malformed override is not the default. Silently substituting the + # default would apply a threshold the operator did not choose and never be told about. + raise RuntimeError( + f"VPCOPILOT_SIM_THRESHOLD is set to {raw!r}, which is not a number. Fix it or unset it " + f"— falling back to the default would silently apply a threshold you did not choose." + ) from None _SP_ONEOF = ("no_service_policies", "active_service_policies", "service_policies_from_namespace") diff --git a/tests/test_engine.py b/tests/test_engine.py index 5d09b0b..370cd13 100644 --- a/tests/test_engine.py +++ b/tests/test_engine.py @@ -45,8 +45,39 @@ def test_safe_rollback_restores_and_verifies(fake_xc, tmp_path, noop_sleep): # simulate an attach: LB now has a band-aid ctx.put({"active_service_policies": {"policies": [{"name": "x"}]}}) assert "active_service_policies" in fake_xc.lb["spec"] - ok = safe_rollback(ctx, verify=lambda back: "active_service_policies" not in back) + seen = [] + ok = safe_rollback(ctx, verify=lambda back: seen.append(back) or + "active_service_policies" not in back) assert ok and fake_xc.lb["spec"] == {"no_service_policies": {}} # back to the snapshot + assert seen, "safe_rollback reported success without calling verify at all" + + +def test_safe_rollback_refuses_to_call_an_unrestored_lb_restored(fake_xc, tmp_path, noop_sleep): + """The "verifies" half of the guarantee, which the test above CANNOT prove on its own. + + `FakeXC` genuinely applies the PUT, so the restore succeeds by itself and `verify` returns True + whatever it does — delete the `verify` call from `safe_rollback` entirely and that test still + passes. It was asserting the behaviour of the fake, not of the code. + + The failure that matters is an appliance that ACCEPTS the rollback PUT and does not apply it — + a 200 that changed nothing, which is exactly what a partially-degraded control plane looks + like. Without the verify step, `safe_rollback` returns True and the caller reports a clean + rollback while the band-aid is still attached to a live load balancer. That is the "silent + half-rollback" the module docstring calls the worst outcome on a live LB.""" + ctx = _ctx(fake_xc, tmp_path, noop_sleep) + ctx.put({"active_service_policies": {"policies": [{"name": "x"}]}}) + + # Accept every subsequent PUT, apply none of them. + fake_xc.put_lb = lambda name, body: {"metadata": {"name": name}, "spec": fake_xc.lb["spec"]} + + with pytest.raises(RollbackError, match="could not restore"): + safe_rollback(ctx, retries=2, verify=lambda back: "active_service_policies" not in back) + + assert "active_service_policies" in fake_xc.lb["spec"], \ + "the LB was never restored — which is precisely why this must raise" + from vpcopilot import audit + assert any(a["action"] == "rollback_failed" for a in audit.load(str(tmp_path))), \ + "a rollback that could not be confirmed must leave a loud, attributable audit record" def test_safe_rollback_raises_when_put_keeps_failing(fake_xc, tmp_path, noop_sleep): diff --git a/tests/test_inputs_cve.py b/tests/test_inputs_cve.py index 329efb4..4aa9265 100644 --- a/tests/test_inputs_cve.py +++ b/tests/test_inputs_cve.py @@ -319,8 +319,31 @@ def test_the_resolve_agent_is_registered_everywhere_it_has_to_be(): from vpcopilot.bench_model import AGENTS from vpcopilot.config import AGENT_NAMES from vpcopilot.console.app import AGENT_ROLES - assert "resolve" in AGENT_NAMES and "resolve" in AGENTS and "resolve" in AGENT_ROLES - assert "resolve" in (__import__("pathlib").Path("src/vpcopilot/report.py").read_text()) + from vpcopilot.report import REPORTED_AGENTS + for site, names in (("config.AGENT_NAMES", AGENT_NAMES), ("bench_model.AGENTS", AGENTS), + ("console.AGENT_ROLES", AGENT_ROLES), + ("report.REPORTED_AGENTS", REPORTED_AGENTS)): + assert "resolve" in names, f"the resolve agent is missing from {site}" + + +def test_the_four_registration_sites_agree_with_each_other(): + """The guard above checked report.py with a whole-file substring search for "resolve", which + the unrelated word "resolved" already satisfied — so it passed with the agent missing from the + very list it was guarding. Asserting on the LIST is the fix; asserting the four lists AGREE is + the guard that actually holds for the next agent too, rather than for this one by name.""" + from vpcopilot.bench_model import AGENTS + from vpcopilot.config import AGENT_NAMES + from vpcopilot.console.app import AGENT_ROLES + from vpcopilot.report import REPORTED_AGENTS + + sites = {"config.AGENT_NAMES": set(AGENT_NAMES), "bench_model.AGENTS": set(AGENTS), + "console.AGENT_ROLES": set(AGENT_ROLES), "report.REPORTED_AGENTS": set(REPORTED_AGENTS)} + everywhere = set.intersection(*sites.values()) + for name, got in sites.items(): + missing = everywhere ^ got + assert not missing, ( + f"{name} disagrees with the other registration sites about {sorted(missing)} — " + f"an agent registered in three of four places is silently absent from the fourth") def test_every_shipped_config_names_the_resolve_agent(): diff --git a/tests/test_report.py b/tests/test_report.py index 6792ba9..16bc23e 100644 --- a/tests/test_report.py +++ b/tests/test_report.py @@ -23,8 +23,14 @@ def _seed(out: Path): "code_cure_required": True}, ])) (out / "remediations.json").write_text(json.dumps([ - {"finding_id": "a-001", "summary": "s", "file": "api/login.js", "diff": "", - "patched_content": "", "pr_title": "Fix SQLi in login", "pr_body": "b"}])) + # `patched_content` is non-empty on purpose: a code_fix with no patch is nothing for + # `pr.py` to write to a branch, and the report now says so rather than claiming a fix was + # drafted. An empty stub here made this fixture assert the badge for a plan that had no + # patch at all. + {"finding_id": "a-001", "summary": "s", "file": "api/login.js", + "diff": "--- a/api/login.js\n+++ b/api/login.js\n", "kind": "code_fix", + "patched_content": "const q = db.prepare('SELECT * FROM users WHERE email = ?');", + "pr_title": "Fix SQLi in login", "pr_body": "b"}])) def test_report_renders_and_is_selfcontained(tmp_path): diff --git a/tests/test_review_tail.py b/tests/test_review_tail.py new file mode 100644 index 0000000..5bfac75 --- /dev/null +++ b/tests/test_review_tail.py @@ -0,0 +1,154 @@ +"""The last of the 2026-08-04 review findings: honesty of reporting, and one surface-parity gap. + +Common thread: a value that means "we could not measure this" rendered as a measurement, or a +lookup that three surfaces remembered to do and the fourth did not. +""" +from __future__ import annotations + +import json +import re + +import pytest + + +# ---------------------------------------------------------------- the cure badge + + +def _card(rem, **finding): + from vpcopilot.report import _finding_card + f = {"id": "x", "title": "t", "severity": "critical", "vuln_class": "other", + "description": "d", **finding} + html = _finding_card(f, {"no_bandaid": True, "bandaids": []}, rem) + m = re.search(r'([^<]*)', html) + return m.group(1) if m else "" + + +def test_a_dependency_upgrade_is_not_reported_as_a_drafted_code_fix(): + """The badge keyed on the mere PRESENCE of a remediation, so a `dependency_upgrade` claimed + "✓ code fix drafted" while the hero on the SAME page correctly reported 0 code-fix PRs and 1 + upgrade to ship. There is no file to patch in someone else's package and `pr.py` writes no + diff for one — the card was describing work that does not exist.""" + got = _card({"kind": "dependency_upgrade", "package": "log4j-core", "fixed_version": "2.17.1"}) + assert "code fix drafted" not in got + assert "dependency upgrade" in got and "log4j-core" in got and "2.17.1" in got, \ + "the upgrade must name the package and version — that is the actionable part" + + +def test_a_real_code_fix_still_says_so(): + assert "code fix drafted" in _card({"kind": "code_fix", "patched_content": "x = 1"}) + + +def test_a_plan_with_no_patch_does_not_claim_a_patch(): + """Third outcome. A RemediationPlan with neither a diff nor patched_content is a plan, not a + drafted fix — `pr.py` has nothing to write to a branch. Saying so beats a tick that is untrue.""" + got = _card({"kind": "code_fix", "patched_content": "", "diff": ""}) + assert "code fix drafted" not in got and "no patch drafted" in got + + +# ---------------------------------------------------------------- blast radius + + +def _blast(tmp_path, policy): + from vpcopilot.report import _blast_radius_html + (tmp_path / "simulation.json").write_text(json.dumps( + {"lb": "lab", "records": 12, "records_replayed": 12, "policies": [policy]})) + return _blast_radius_html(str(tmp_path)) + + +def test_a_replay_where_everything_failed_is_not_reported_within_threshold(tmp_path): + """`simulate` computes `evaluated`, `errored`, `enforcement_confirmed` and `reason`; the table + used none of them. A replay in which every request failed in transit (evaluated=0, errored=12, + reason="nothing measurable") rendered as a green "within threshold" at 0.0% — a block rate that + means "we measured nothing", presented as "safe to promote".""" + html = _blast(tmp_path, {"policy_name": "p", "evaluated": 0, "errored": 12, "would_block": 0, + "block_rate": 0, "enforcement_confirmed": False, + "reason": "nothing measurable"}) + assert "within threshold" not in html, "an unmeasured replay was reported as measured and safe" + assert "not measured" in html + assert "0.0%" not in html, "a rate computed from zero samples must not be shown as a rate" + assert "nothing measurable" in html and "failed in transit" in html, \ + "the caveats the simulation computed must reach the page" + + +def test_a_healthy_replay_still_reads_within_threshold(tmp_path): + """The fix must not paint every row amber — a real measurement still reports as one.""" + html = _blast(tmp_path, {"policy_name": "p", "evaluated": 100, "errored": 0, "would_block": 2, + "block_rate": 0.02, "enforcement_confirmed": True}) + assert "within threshold" in html and "2.0%" in html + + +def test_an_unconfirmed_enforcement_is_its_own_verdict(tmp_path): + """Measured, but the policy was not confirmed to be enforcing during the replay — which is + neither "safe" nor "over threshold".""" + html = _blast(tmp_path, {"policy_name": "p", "evaluated": 100, "errored": 0, "would_block": 0, + "block_rate": 0.0, "enforcement_confirmed": False}) + assert "unconfirmed" in html and "within threshold" not in html + + +# ---------------------------------------------------------------- reconcile + + +def test_a_probe_that_blew_up_in_transport_is_not_reported_as_no_probe(monkeypatch, tmp_path): + """"This finding has no runnable probe" is a PERMANENT condition an operator can only fix by + re-scanning. A probe that exists and failed in transport is TRANSIENT — the origin was + unreachable, TLS failed, the connection dropped — and will very likely work next pass. + Reporting the second as the first sends someone to fix the wrong thing.""" + import httpx + + from vpcopilot import reconcile + monkeypatch.setattr(reconcile, "_load_probe", + lambda out, fid: {"exploit": {"path": "/x", "method": "POST"}}, + raising=False) + monkeypatch.setattr("vpcopilot.apply._load_probe", + lambda out, fid: {"exploit": {"path": "/x", "method": "POST"}}) + monkeypatch.setattr("vpcopilot.probe.probe_from_spec", + lambda *a, **k: (_ for _ in ()).throw(httpx.ReadError("connection reset"))) + got = reconcile._probe({"finding_id": "f1"}, str(tmp_path), "http://origin", log=lambda m: None) + assert got.get("probe_error"), "a transport failure returned the same {} as 'no probe recorded'" + assert "ReadError" in got["probe_error"], "the reason must be carried, not just a flag" + + +def test_the_two_reasons_are_distinct_hold_codes(): + """Rendered on every surface through the hold code, so they cannot read alike.""" + src = (__import__("pathlib").Path(__file__).resolve().parents[1] + / "src/vpcopilot/reconcile.py").read_text() + assert "skipped_probe_error" in src and "skipped_no_probe" in src + assert "transient failure, not a missing probe" in src + + +# ---------------------------------------------------------------- the blast-radius threshold + + +def test_every_surface_honours_the_operators_threshold(monkeypatch): + """`VPCOPILOT_SIM_THRESHOLD` was a lookup each surface had to REMEMBER. The CLI, console and + refiner remembered; the MCP server did not — so an operator who tightened the threshold got + the default silently applied to every simulation an agent ran, and the number they set was the + number they believed was in force. + + Now a property of the module. Asserted on the source of all four callers, because the defect + was one caller not calling it.""" + from pathlib import Path + + from vpcopilot.simulate import effective_threshold + monkeypatch.setenv("VPCOPILOT_SIM_THRESHOLD", "0.05") + assert effective_threshold() == 0.05 + assert effective_threshold(0.2) == 0.2, "an explicit argument still wins" + + root = Path(__file__).resolve().parents[1] / "src/vpcopilot" + for mod in ("cli.py", "mcp.py", "refiner.py", "console/app.py"): + src = (root / mod).read_text() + assert "effective_threshold(" in src, f"{mod} resolves the threshold its own way" + assert 'os.environ.get("VPCOPILOT_SIM_THRESHOLD"' not in src, \ + f"{mod} still re-implements the lookup — that is how one surface drifted" + + +def test_a_malformed_threshold_is_refused_rather_than_defaulted(monkeypatch): + """Refusing to guess. Silently substituting the default would apply a threshold the operator + did not choose, on the gate that decides whether a policy is safe to promote — and they would + never be told.""" + from vpcopilot.simulate import DEFAULT_THRESHOLD, effective_threshold + monkeypatch.setenv("VPCOPILOT_SIM_THRESHOLD", "1%") + with pytest.raises(RuntimeError, match="not a number"): + effective_threshold() + monkeypatch.setenv("VPCOPILOT_SIM_THRESHOLD", "") + assert effective_threshold() == DEFAULT_THRESHOLD, "an EMPTY value is unset, not malformed"