Skip to content

fix(review): six more — reporting honesty, surface parity, two vacuous tests #35

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
henleda merged 1 commit into from
Aug 5, 2026
Merged

fix(review): six more — reporting honesty, surface parity, two vacuous tests #35

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions BACKLOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 <typo>` 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 `<span cla
- *Repro:* `/Users/d.henley/demos/virtual-patch-copilot/.venv/bin/python -c "
import json,re,sys,tempfile; sys.path.insert(0,'src')
from vpcopilot.schemas import PolicySimulation, SimulationRe`
- *Why the suite misses it:* tests/test_simulate.py exercises `_score`/`write_result` at the model level and asserts on the dict; tests/test_console_simulate.py exercises the /api/simulate JSON. Nothing asserts on the HTML `_blast_radius_html` produces, so no test ever compares what report.py renders against what the PolicySimu
- [ ] **MEDIUM** `report.py:119` — A `dependency_upgrade` remediation makes the finding card claim "✓ code fix drafted", contradicting the hero on the same page which correctly reports 0 code-fix PRs and 1 upgrade to ship
- [x] **MEDIUM** `report.py:119` — A `dependency_upgrade` remediation makes the finding card claim "✓ code fix drafted", contradicting the hero on the same page which correctly reports 0 code-fix PRs and 1 upgrade to ship
- *Fails when:* `_finding_card` builds `rem` from remediations.json without looking at `kind` (report.py:119-120: `if rem: ba.append('✓ code fix drafted')`). pipeline._write_out splits the two kinds correctly into summary['code_fix_prs'] and summary['dependency_upgrades'], and both the hero and the run-summary chips honour that split. But remediations.json holds both kinds, so for an advisory finding (--cve / --m
- *Repro:* `/Users/d.henley/demos/virtual-patch-copilot/.venv/bin/python -c "
import json,re,sys,tempfile,pathlib; sys.path.insert(0,'src')
Expand Down Expand Up @@ -251,7 +251,7 @@ d=pathlib.Path(tempfile`

### E. Surface parity

- [ ] **HIGH** `mcp.py:640` — MCP `simulate` ignores VPCOPILOT_SIM_THRESHOLD — the operator's tightened blast-radius threshold is silently replaced by the hardcoded 0.01, and the wrong verdict is persisted to simulation.json where promotion_gate reads it on all three surfaces
- [x] **HIGH** `mcp.py:640` — MCP `simulate` ignores VPCOPILOT_SIM_THRESHOLD — the operator's tightened blast-radius threshold is silently replaced by the hardcoded 0.01, and the wrong verdict is persisted to simulation.json where promotion_gate reads it on all three surfaces
- *Fails when:* `simulate_policies` hardcodes `threshold: float = DEFAULT_THRESHOLD` (0.01, simulate.py:182) and never reads the environment; cli.py:822 and console/app.py:279-280 each resolve `VPCOPILOT_SIM_THRESHOLD` themselves and pass it. mcp.py:640 does `kw = {} if threshold is None else {"threshold": threshold}` — so when a caller omits `threshold` (the documented path) the env var is dropped, even though t
- *Repro:* `VPCOPILOT_SIM_THRESHOLD=0.001 /Users/d.henley/demos/virtual-patch-copilot/.venv/bin/python /tmp/probe_threshold.py # records the threshold simulate_policies is actually handed by`
- *Why the suite misses it:* tests/test_simulate.py:76 (`test_threshold_is_configurable`) passes `threshold=` explicitly to `simulate_policies`, which is the one call shape that cannot expose this. Nothing in tests/test_mcp.py sets VPCOPILOT_SIM_THRESHOLD or inspects the threshold `_tool_simulate` forwards — grepping tests/test
Expand All @@ -262,11 +262,11 @@ d=pathlib.Path(tempfile`
- *Fails when:* The test builds `httpx.Response(500, text="boom")`, so `"s3cr3t-pw" not in str(e.value)` holds whether or not `_redact` does anything — `BigIPError` is constructed from `f"{method} {path} -> {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
26 changes: 13 additions & 13 deletions demo/out/audit.log
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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}
Loading
Loading