diff --git a/BACKLOG.md b/BACKLOG.md index ce57c1a..d158149 100644 --- a/BACKLOG.md +++ b/BACKLOG.md @@ -156,7 +156,7 @@ how they got here. Ordered within each group by severity. ### A. Honesty of reporting — a blank that reads as a clean answer -- [ ] **HIGH** `pipeline.py:152` — The spec-vs-code orphan comparison is fed framework route-registration lines, not paths, so `served` is empty and every endpoint the code demonstrably serves is reported as "declared in the spec but served by no route in the code" — while `code_only: []` renders as "no undeclared routes" having checked nothing +- [x] **HIGH** `pipeline.py:152` — The spec-vs-code orphan comparison is fed framework route-registration lines, not paths, so `served` is empty and every endpoint the code demonstrably serves is reported as "declared in the spec but served by no route in the code" — while `code_only: []` renders as "no undeclared routes" having checked nothing - *Fails when:* The documented invocation `vpcopilot scan ./app --spec ./openapi.yaml` (docs/USAGE.md:96). `openapi.orphans(spec, repo_routes)` documents its input as lines like `"GET POST /users/v1/register"` — the shape `routes._openapi_paths` emits. But pipeline.py:152 hands it `route_ctx.splitlines()`, and for any repo that does not itself contain an OpenAPI/Swagger file, `collect_route_context` returns only - *Repro:* `/Users/d.henley/demos/virtual-patch-copilot/.venv/bin/python /tmp/vpr1/repro.py (script replays pipeline.py:141 and :152 verbatim against a 3-route Flask app at /tmp/vpr1/app and` - *Why the suite misses it:* tests/test_inputs_openapi.py:112-128 exercises `orphans` only with hand-written path-shaped lists (`["POST /api/pay", "GET /api/legacy"]`) — i.e. it invents the input instead of taking it from the real producer, `collect_route_context`. tests/test_routes.py tests `collect_route_context` in isolation @@ -178,11 +178,11 @@ import json,sys,tempfile; sys.path.insert(0,'src') from vpcopilot import ledger, impact, report d=tempfile.mkdtemp` - *Why the suite misses it:* Every ledger fixture in tests/test_impact.py gives the remediated/retired entries a `mitigation` block (`_seed` line 12-13, `test_controls_live_excludes_retired` line 50). The suite has no entry that reached `remediated` without first being `mitigated`, so `mitigated` and `controls_live` always agre -- [ ] **MEDIUM** `cli.py:70` — The scan input-path existence check exists only on MCP: `vpcopilot scan /does/not/exist` and POST /api/scan both run the pipeline to completion and write a clean-bill-of-health summary.json for a directory that was never read +- [x] **MEDIUM** `cli.py:70` — The scan input-path existence check exists only on MCP: `vpcopilot scan /does/not/exist` and POST /api/scan both run the pipeline to completion and write a clean-bill-of-health summary.json for a directory that was never read - *Fails when:* mcp.py:290-294 checks every repo/spec/manifest path before starting the job, citing run_pipeline's own docstring: "a scan of nothing would write a summary saying nothing was found, which is not the same answer". cli.py:66-75 and console/app.py:847-854 validate the CVE-exclusivity rule and min_severity but never check that the paths exist, and `run_pipeline` (pipeline.py:80-88) validates only that - *Repro:* `/Users/d.henley/demos/virtual-patch-copilot/.venv/bin/python /tmp/probe_surfaces_scan.py # calls scan_start / the typer CLI / POST /api/scan with repo=/does/not/exist/at/all; an` - *Why the suite misses it:* tests/test_mcp.py:801 is the only test that asserts "does not exist" anywhere in the suite, and it drives the MCP frame path exclusively. The CLI scan tests and the console scan tests (test_inputs_openapi.py:168, test_inputs_manifest.py:955) assert the *accepting* half of the input rules — that a sp -- [ ] **MEDIUM** `cli.py:1017` — `_require_run_dir` — "a run directory that does not exist is not an empty one" — is enforced only in mcp.py: the CLI prints a green "no live band-aids" and the console returns all-zero impact for a run directory that isn't there +- [x] **MEDIUM** `cli.py:1017` — `_require_run_dir` — "a run directory that does not exist is not an empty one" — is enforced only in mcp.py: the CLI prints a green "no live band-aids" and the console returns all-zero impact for a run directory that isn't there - *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 diff --git a/src/vpcopilot/console/app.py b/src/vpcopilot/console/app.py index 8c61a62..a8f2a1a 100644 --- a/src/vpcopilot/console/app.py +++ b/src/vpcopilot/console/app.py @@ -919,6 +919,13 @@ def start_scan(body: ScanReq): # into the same out dir. # * after validation: claiming first meant a request that then 400'd left the scanner marked # running forever — one malformed request and the console can never scan again. + # On the REQUEST thread, before the worker is spawned — raising inside the pipeline would + # surface as a job error after this endpoint had already returned 200 "running". + from ..pipeline import validate_scan_inputs + try: + validate_scan_inputs(body.repo or None, body.spec or None, manifests) + except ValueError as e: + raise HTTPException(400, str(e)) from None with _scan_lock: if _scan["state"] == "running": raise HTTPException(409, "a scan is already running") diff --git a/src/vpcopilot/inputs/openapi.py b/src/vpcopilot/inputs/openapi.py index a473abb..9dfaff6 100644 --- a/src/vpcopilot/inputs/openapi.py +++ b/src/vpcopilot/inputs/openapi.py @@ -24,6 +24,8 @@ """ from __future__ import annotations +import re + from pathlib import Path from typing import Any @@ -176,23 +178,60 @@ def _normalize(path: str) -> str: return "/" + p.strip("/").lower() +# A quoted, absolute path inside a route-registration line: @app.route("/api/pay", …), +# path('/users/'), router.get(`/api/x`). Deliberately literal-only — a path built by +# concatenation is not evidence of what is served, and guessing at one would be worse than +# the gap it fills. +_QUOTED_PATH = re.compile(r"""['"`](/[^'"`\s]*)['"`]""") + def orphans(spec: dict, repo_routes: list[str]) -> dict: """Spec vs code, both directions. Pure comparison — no model, no network. - `spec_only`: declared but nothing serves it — dead documentation, or a shadow API someone - forgot to remove. `code_only`: served but undeclared — and this is the one that bites, because - an `api_schema` band-aid built from this spec would start rejecting those routes the moment it - is applied.""" + `code_only`: served but undeclared — the one that bites, because an `api_schema` band-aid + built from this spec would start rejecting those routes the moment it is applied. This is a + PRESENCE claim and a source sweep can establish it. + + `spec_unverified`: declared, and the sweep did not find it in the source. Deliberately NOT + called `spec_only` any more, and deliberately not phrased as "nothing serves this": that is an + ABSENCE claim, and a line-wise sweep cannot establish absence. File-based routing + (`app/api/pay/route.js`) declares a route with no line to match, and a dynamically mounted + router never appears literally. The old key asserted the strong reading and was consumed as + such — it raised a medium finding — so it is gone rather than left empty for someone to + reach for again. + + `swept`: whether any route was extracted at all. A sweep that found nothing is not a spec + whose endpoints are all unserved, and the two used to be indistinguishable.""" declared = {_normalize(op["path"]): op["path"] for op in operations(spec)} served = {} for r in repo_routes or []: - # route context lines look like "GET POST /users/v1/register" or "/users/v1/register" + # Two shapes reach here and only one used to be handled: + # "GET POST /users/v1/register" <- an in-repo spec's paths + # ' app.py:2: @app.route("/api/pay", methods=[…])' <- a framework REGISTRATION line + # The old code took the last whitespace token and kept it if it began with "/". A + # registration line's last token is `methods=["POST"])`, so it kept NONE of them — meaning + # `served` was populated only from an in-repo OpenAPI file and NEVER from code, in any + # configuration. With no in-repo spec every declared endpoint became "declared but served + # by no route", and with one the comparison was spec-vs-spec, so a genuinely served + # undeclared route was reported nowhere. Verified against a 3-route Flask app. parts = r.split() path = parts[-1] if parts else "" if path.startswith("/"): served[_normalize(path)] = path + continue + for quoted in _QUOTED_PATH.findall(r): + served[_normalize(quoted)] = quoted + + # `code_only` is a PRESENCE claim — "the sweep found this route in the source and the spec does + # not declare it" — and a source sweep can establish presence. `spec_only` is an ABSENCE claim + # — "nothing serves this" — and a line-wise sweep cannot establish that: file-based routing + # (app/api/pay/route.js) declares a route with no line to grep, and dynamically mounted routers + # never appear literally. So the unmatched declarations are returned as `spec_unverified`, NOT + # as orphans, whenever the sweep is the only evidence. Publishing them as orphans is a + # confident claim resting on evidence that cannot support it. + unmatched = sorted(declared[k] for k in declared.keys() - served.keys()) return { - "spec_only": sorted(declared[k] for k in declared.keys() - served.keys()), + "spec_unverified": unmatched, + "swept": bool(served), "code_only": sorted(served[k] for k in served.keys() - declared.keys()), "matched": sorted(declared[k] for k in declared.keys() & served.keys()), } diff --git a/src/vpcopilot/pipeline.py b/src/vpcopilot/pipeline.py index ef41384..e922692 100644 --- a/src/vpcopilot/pipeline.py +++ b/src/vpcopilot/pipeline.py @@ -55,6 +55,27 @@ def _dedup_findings(findings, log, counter: dict | None = None): return kept +def validate_scan_inputs(repo_path: str | None = None, spec_path: str | None = None, + manifest_paths: list[str] | None = None) -> None: + """Every input path must exist. Raises ValueError naming the offender. + + This check lived ONLY in mcp.py, so `vpcopilot scan /does/not/exist` and POST /api/scan both + ran to completion and wrote a clean-bill-of-health summary.json for a directory that was never + read — the exact behaviour `run_pipeline`'s own docstring calls "the failure mode not to + extend", extended to two of the three surfaces. + + Public and separate from `run_pipeline` because the console starts scans on a worker thread: + raising inside the pipeline there would surface as a job error AFTER a 200, so the console + calls this on the request thread and refuses up front, as MCP already did. + """ + for label, p in (("repo", repo_path), ("spec", spec_path), + *[("manifest", m) for m in (manifest_paths or [])]): + if p and not Path(p).exists(): + raise ValueError( + f"{label} path {p!r} does not exist — a scan of nothing would write a summary " + f"saying nothing was found, which is not the same answer") + + def run_pipeline( repo_path: str | None = None, out_dir: str = "out", @@ -86,6 +107,7 @@ def run_pipeline( if not (repo_path or advisory or spec_path or manifest_paths): raise ValueError("pass a repo path, an advisory id (--cve), an OpenAPI spec (--spec), " "or a dependency manifest (--manifest)") + validate_scan_inputs(repo_path, spec_path, manifest_paths) h = Harness(config_path) h.warmup() # B6: warm instructor's mode registry before ANY fan-out — both inputs need it t0, started = time.perf_counter(), runmeta.utc_now() @@ -151,8 +173,18 @@ def run_pipeline( if repo_path and route_ctx: o = orphans(load_spec(spec_path), route_ctx.splitlines()) spec_orphans = o - log(f" spec vs code: {len(o['matched'])} matched, " - f"{len(o['spec_only'])} declared-but-unserved, {len(o['code_only'])} undeclared") + # Say what was ESTABLISHED and what was not. A sweep that matched nothing is not + # a spec whose endpoints are all unserved — it is a sweep that failed, and the two + # used to print identically. + if not o["swept"]: + log(" spec vs code: the route sweep found NO routes in the source, so the " + "comparison did not run. This is not 'no orphans' — file-based routing " + "and dynamically mounted routers leave no line to match.") + else: + log(f" spec vs code: {len(o['matched'])} matched, " + f"{len(o['code_only'])} served-but-undeclared, " + f"{len(o['spec_unverified'])} declared endpoint(s) the sweep did not find " + f"(NOT reported as orphaned — a source sweep cannot prove absence)") if manifest_paths: # H2 — a manifest yields findings the same way an advisory does (H1's resolve agent, # H1's deterministic no_bandaid route, H1's OSV-sourced cure), so they are appended to @@ -280,17 +312,22 @@ def _threshold(f): else: refuted += 1 log(f" verify {f.id}: refuted ({v.confidence:.2f})") - if spec_orphans and (spec_orphans["spec_only"] or spec_orphans["code_only"]): + if spec_orphans and spec_orphans.get("code_only"): # Deterministic, no agent: this is a comparison of two documents. Both directions matter and # they fail differently — a declared-but-unserved endpoint is dead documentation or a shadow # API, while a served-but-undeclared route is what an api_schema band-aid built from this # spec would start rejecting the moment it is applied. from .schemas import Finding - so, co = spec_orphans["spec_only"], spec_orphans["code_only"] + # ONLY the served-but-undeclared direction raises a finding now. The other direction is a + # claim of ABSENCE — "nothing serves this endpoint" — and the route sweep is a line-wise + # grep that cannot establish absence: file-based routing (app/api/pay/route.js) declares a + # route with no line to match, and a dynamically mounted router never appears literally. + # This used to raise a medium finding asserting exactly that, on evidence that could not + # support it — and it bypasses verify, so nothing downstream ever challenged it. The + # unmatched declarations are still REPORTED, as `spec_unverified`, phrased as what they + # are: not checked. + so, co = [], spec_orphans["code_only"] bits = [] - if so: - bits.append("declared in the spec but served by no route in the code: " - + ", ".join(so[:15]) + (f" (+{len(so) - 15} more)" if len(so) > 15 else "")) if co: bits.append("served by the code but absent from the spec: " + ", ".join(co[:15]) + (f" (+{len(co) - 15} more)" if len(co) > 15 else "")) diff --git a/tests/test_inputs_manifest.py b/tests/test_inputs_manifest.py index 9a579e9..04cb121 100644 --- a/tests/test_inputs_manifest.py +++ b/tests/test_inputs_manifest.py @@ -828,8 +828,11 @@ def test_the_fallback_only_fires_when_the_recommended_tier_produced_nothing(monk from vpcopilot.harness import Harness monkeypatch.setattr(Harness, "__init__", lambda self, cp=None: None) monkeypatch.setattr(Harness, "warmup", lambda self: None) - run_pipeline(manifest_paths=["x.txt"], out_dir=str(tmp_path), draft_code_fixes=False, - log=lambda m: None) + # The manifest reader is monkeypatched above, so the CONTENTS are irrelevant — but the + # file must exist: the pipeline now refuses a path that does not, on every surface. + (tmp_path / "x.txt").write_text("flask==1.0\n") + run_pipeline(manifest_paths=[str(tmp_path / "x.txt")], out_dir=str(tmp_path), + draft_code_fixes=False, log=lambda m: None) assert made == [("f1", "service_policy")] # the rate_limit alternative was never reached @@ -848,8 +851,11 @@ def test_the_fallback_fires_when_every_recommended_control_was_claimed(monkeypat from vpcopilot.harness import Harness monkeypatch.setattr(Harness, "__init__", lambda self, cp=None: None) monkeypatch.setattr(Harness, "warmup", lambda self: None) - run_pipeline(manifest_paths=["x.txt"], out_dir=str(tmp_path), draft_code_fixes=False, - log=lambda m: None) + # The manifest reader is monkeypatched above, so the CONTENTS are irrelevant — but the + # file must exist: the pipeline now refuses a path that does not, on every surface. + (tmp_path / "x.txt").write_text("flask==1.0\n") + run_pipeline(manifest_paths=[str(tmp_path / "x.txt")], out_dir=str(tmp_path), + draft_code_fixes=False, log=lambda m: None) assert ("owner", "waf") in made assert ("loser", "service_policy") in made # was: nothing at all assert ("loser", "waf") not in made # the LB-wide slot is still shared, not duplicated @@ -881,8 +887,11 @@ def test_an_alternate_never_takes_a_slot_a_later_finding_recommends(monkeypatch, "candidates": []}) monkeypatch.setattr(Harness, "__init__", lambda self, cp=None: None) monkeypatch.setattr(Harness, "warmup", lambda self: None) - run_pipeline(manifest_paths=["x.txt"], out_dir=str(tmp_path), draft_code_fixes=False, - log=lambda m: None) + # The manifest reader is monkeypatched above, so the CONTENTS are irrelevant — but the + # file must exist: the pipeline now refuses a path that does not, on every surface. + (tmp_path / "x.txt").write_text("flask==1.0\n") + run_pipeline(manifest_paths=[str(tmp_path / "x.txt")], out_dir=str(tmp_path), + draft_code_fixes=False, log=lambda m: None) assert ("brute", "rate_limit") in made # was: stolen by loser's alternate, brute got nothing assert ("owner", "waf") in made # the fallback takes ONE alternative, not the whole non-recommended stack @@ -905,8 +914,11 @@ def test_an_lb_wide_correlation_says_whose_exploit_the_policy_was_built_from(mon from vpcopilot.harness import Harness monkeypatch.setattr(Harness, "__init__", lambda self, cp=None: None) monkeypatch.setattr(Harness, "warmup", lambda self: None) - run_pipeline(manifest_paths=["x.txt"], out_dir=str(tmp_path), draft_code_fixes=False, - log=lambda m: None) + # The manifest reader is monkeypatched above, so the CONTENTS are irrelevant — but the + # file must exist: the pipeline now refuses a path that does not, on every surface. + (tmp_path / "x.txt").write_text("flask==1.0\n") + run_pipeline(manifest_paths=[str(tmp_path / "x.txt")], out_dir=str(tmp_path), + draft_code_fixes=False, log=lambda m: None) cor = json.loads((tmp_path / "correlations.json").read_text()) # service_policy is endpoint-scoped, not LB-wide: same endpoint really is one policy for both assert cor[0]["note"] == "same endpoint — one policy covers both" @@ -1037,8 +1049,11 @@ def _pipeline_with(monkeypatch, tmp_path, findings, remediations): "report": {"funnel": {}}, "candidates": []}) monkeypatch.setattr(Harness, "__init__", lambda self, cp=None: None) monkeypatch.setattr(Harness, "warmup", lambda self: None) - run_pipeline(manifest_paths=["x.txt"], out_dir=str(tmp_path), draft_code_fixes=False, - log=lambda m: None) + # The manifest reader is monkeypatched above, so the CONTENTS are irrelevant — but the + # file must exist: the pipeline now refuses a path that does not, on every surface. + (tmp_path / "x.txt").write_text("flask==1.0\n") + run_pipeline(manifest_paths=[str(tmp_path / "x.txt")], out_dir=str(tmp_path), + draft_code_fixes=False, log=lambda m: None) return json.loads((tmp_path / "summary.json").read_text()) diff --git a/tests/test_inputs_openapi.py b/tests/test_inputs_openapi.py index 5208419..4dc95b7 100644 --- a/tests/test_inputs_openapi.py +++ b/tests/test_inputs_openapi.py @@ -112,8 +112,12 @@ def test_a_missing_file_says_so(): def test_orphans_compare_both_directions(tmp_path): o = oa.orphans(oa.load_spec(_write(tmp_path)), ["POST /api/pay", "GET /api/legacy"]) assert o["matched"] == ["/api/pay"] - assert o["spec_only"] == ["/api/users/{id}"] assert o["code_only"] == ["/api/legacy"] + # Was `spec_only`, asserted as "declared but nothing serves it". That is an ABSENCE claim a + # line-wise source sweep cannot establish, so it is reported as unverified and no longer + # raises a finding. See test_spec_vs_code.py for the full reasoning. + assert o["spec_unverified"] == ["/api/users/{id}"] + assert "spec_only" not in o @pytest.mark.parametrize("declared,served", [ @@ -125,7 +129,7 @@ def test_orphans_compare_both_directions(tmp_path): def test_path_parameter_styles_are_treated_as_the_same_endpoint(tmp_path, declared, served): spec = f"openapi: 3.0.3\ninfo: {{title: t, version: '1'}}\npaths:\n {declared}:\n get: {{}}\n" o = oa.orphans(oa.load_spec(_write(tmp_path, spec)), [f"GET {served}"]) - assert o["matched"] and not o["spec_only"] and not o["code_only"] + assert o["matched"] and not o["spec_unverified"] and not o["code_only"] def test_the_scan_input_carries_the_structural_findings_as_hints(tmp_path): diff --git a/tests/test_model_switch.py b/tests/test_model_switch.py index 9ce1b05..71b80a3 100644 --- a/tests/test_model_switch.py +++ b/tests/test_model_switch.py @@ -32,7 +32,12 @@ def test_scan_points_console_at_its_output_dir(tmp_path, monkeypatch): # a scan makes the console read the dir it wrote to (kills the out-claude-vampi mismatch) c = _client() monkeypatch.setattr(A, "_run_scan", lambda *a, **k: None) # don't actually run the pipeline - r = c.post("/api/scan", json={"repo": "x", "out": "out-claude-vampi"}).json() + # A real directory: the console now refuses a repo path that does not exist, on the request + # thread, so `"x"` would 400 before the scan started. That refusal is the point of the check — + # the fixture was relying on its absence. + (tmp_path / "repo").mkdir() + r = c.post("/api/scan", json={"repo": str(tmp_path / "repo"), + "out": "out-claude-vampi"}).json() assert r["out"] == "out-claude-vampi" assert c.get("/api/models").json()["out"] == "out-claude-vampi" diff --git a/tests/test_spec_vs_code.py b/tests/test_spec_vs_code.py new file mode 100644 index 0000000..1693a56 --- /dev/null +++ b/tests/test_spec_vs_code.py @@ -0,0 +1,139 @@ +"""The last three review findings: the spec-vs-code comparison, and two "MCP only" guards. + +The spec-vs-code one is the most consequential defect the review found, because its output does +not go through the verify agent — the pipeline appends it straight to `verified`, so a wrong +answer draws a real `api_schema` band-aid with nothing downstream to challenge it. +""" +from __future__ import annotations + +import textwrap + +import pytest + +from vpcopilot.inputs.openapi import orphans +from vpcopilot.routes import collect_route_context + + +@pytest.fixture +def flask_app(tmp_path): + """Three routes, declared the ordinary way. `collect_route_context` is the real producer — + the existing `orphans` tests hand-write path-shaped lines the caller never actually supplies, + which is precisely why the producer/consumer mismatch survived.""" + (tmp_path / "app.py").write_text(textwrap.dedent(''' + @app.route("/api/pay", methods=["POST"]) + def pay(): ... + @app.route("/api/login", methods=["POST"]) + def login(): ... + @app.route("/api/secret-admin", methods=["GET"]) + def admin(): ... + ''')) + return collect_route_context(str(tmp_path)).splitlines() + + +SPEC = {"paths": {"/api/pay": {"post": {}}, "/api/login": {"post": {}}, "/api/gone": {"get": {}}}} + + +def test_a_served_undeclared_route_is_found(flask_app): + """The direction `orphans`' own docstring calls "the one that bites" — an `api_schema` band-aid + built from the spec would start rejecting it the moment it is applied. + + It was reported NOWHERE. `served` was populated only from an in-repo OpenAPI file and never + from code in any configuration, because a registration line's last whitespace token is + `methods=["POST"])`, not a path.""" + got = orphans(SPEC, flask_app) + assert got["code_only"] == ["/api/secret-admin"] + + +def test_the_routes_the_app_really_serves_are_not_called_unserved(flask_app): + """The inverse false positive, and the louder one: /api/pay and /api/login ARE served, and the + run reported both as "declared in the spec but served by no route in the code".""" + got = orphans(SPEC, flask_app) + assert got["matched"] == ["/api/login", "/api/pay"] + assert "/api/pay" not in got["spec_unverified"] + + +def test_an_unmatched_declaration_is_reported_as_unverified_not_as_an_orphan(flask_app): + """The design decision, and the reason this is not simply "extract paths better". + + `code_only` is a PRESENCE claim — the sweep found this route in the source — and a source + sweep can establish presence. The other direction is an ABSENCE claim: "nothing serves this". + A line-wise grep cannot establish that. File-based routing (`app/api/pay/route.js`) declares a + route with no line to match; a dynamically mounted router never appears literally. So the + unmatched declarations are reported as NOT CHECKED, and the old `spec_only` key is gone rather + than left empty for someone to reach for again.""" + got = orphans(SPEC, flask_app) + assert got["spec_unverified"] == ["/api/gone"] + assert "spec_only" not in got, "the key that asserted absence must not survive as an empty list" + + +def test_a_sweep_that_found_nothing_is_not_a_spec_full_of_orphans(): + """Two states that used to print identically. A repo whose routes the sweep cannot see (file + -based routing, a framework with no literal registration lines) produced `served == {}`, so + EVERY declared endpoint became an orphan — a confident finding generated by a failed sweep.""" + got = orphans(SPEC, ["Route registrations (framework code):"]) + assert got["swept"] is False + assert got["code_only"] == [] + assert sorted(got["spec_unverified"]) == ["/api/gone", "/api/login", "/api/pay"], \ + "the declarations are still reported — as unchecked, not as orphaned" + + +def test_the_pipeline_only_raises_a_finding_for_the_claim_it_can_support(): + """The finding bypasses verify (`findings.append` + `verified.append`), so nothing downstream + challenges it and triage can draw a real api_schema band-aid from it. It must therefore only + ever assert the direction the evidence supports.""" + from pathlib import Path + src = (Path(__file__).resolve().parents[1] / "src/vpcopilot/pipeline.py").read_text() + assert 'if spec_orphans and spec_orphans.get("code_only"):' in src, \ + "the orphan finding still fires on the unsupportable absence claim" + assert "cannot prove absence" in src, "the log must say what was not established" + + +# ---------------------------------------------------------------- input paths, on every surface + + +MISSING = "/does/not/exist/at/all" + + +def test_the_cli_refuses_a_repo_path_that_does_not_exist(): + """`run_pipeline`'s own docstring calls this "the failure mode not to extend" — and the check + existed only in mcp.py, so the CLI and console ran the pipeline to completion and wrote a + clean-bill-of-health summary.json for a directory that was never read.""" + from typer.testing import CliRunner + + from vpcopilot import cli + res = CliRunner().invoke(cli.app, ["scan", MISSING, "--out", "/tmp/vpc-nope"]) + assert res.exit_code != 0 + + +def test_the_console_refuses_it_before_the_scan_starts(monkeypatch): + """On the REQUEST thread. Raising inside the pipeline would surface as a job error after the + endpoint had already returned 200 "running" — the operator would see a scan start, then fail. + + Also asserts the scanner is not left claimed: a refusal that wedges the console is worse than + the thing it refused.""" + from fastapi.testclient import TestClient + + from vpcopilot.console import app as A + A._scan.update(state="idle") + res = TestClient(A.app).post("/api/scan", json={"repo": MISSING, "out": "/tmp/vpc-nope"}) + assert res.status_code == 400 + assert "does not exist" in res.json()["detail"] + assert A._scan["state"] != "running", "the refusal left the scanner claimed" + + +def test_all_three_surfaces_share_one_implementation(): + """The guard lives in `pipeline.validate_scan_inputs`, so a fourth consumer inherits it. The + defect was one surface having a copy and the others having nothing.""" + from pathlib import Path + + from vpcopilot.pipeline import validate_scan_inputs + with pytest.raises(ValueError, match="does not exist"): + validate_scan_inputs(repo_path=MISSING) + with pytest.raises(ValueError, match="spec"): + validate_scan_inputs(spec_path=MISSING) + with pytest.raises(ValueError, match="manifest"): + validate_scan_inputs(manifest_paths=[MISSING]) + validate_scan_inputs(repo_path=None, spec_path=None, manifest_paths=[]) # must not raise + + root = Path(__file__).resolve().parents[1] / "src/vpcopilot" + assert "validate_scan_inputs" in (root / "console/app.py").read_text()