From 0909d050d9363ae2c7077a27bd52bcff9b203835 Mon Sep 17 00:00:00 2001 From: "B.D. Murphy" <161863146+bdmurphy73@users.noreply.github.com> Date: Thu, 15 Oct 2026 20:41:05 -0700 Subject: [PATCH] Probe the route correctly: credentials, CSRF ordering, and a real dry run MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The controller's ingest endpoint came up between runs and this tool misreported it three more times, each in the same direction — naming the state it was in when the check was written rather than the state it was reading. * POST was probed without a credential, so a live route kept answering 401 and the verdict stayed AUTH_REQUIRED after auth was configured and T02 passed. The probe now sends whatever headers exist, because an unauthenticated probe answers "is there a door", never "does it open". * The rule for reading a status was wrong twice. First it accepted only 200/201/405 as routed, so a 401 on a live route read as absent. Then it was widened to "anything but 404", which would have read a CSRF 403 as a route: the controller checks CSRF in middleware before routing (app.py:246-248) and exempts only /api/node (app.py:89), so POST /api/compute/audio/import answers 403 whether or not a route exists — and none does. Now: on a safe method, 404 means unrouted and anything else means routed; on an unsafe method a 403 off an exempt prefix is discarded and recorded as csrf_masked_probe. * T04 dropped csrf_masked_probe from its FAIL evidence, which is exactly the case where the next reader needs it. Found by the new tests, not by reading. * T05 returned BLOCKED unconditionally, so the one test that mattered was the least willing to run. It now executes a real dry run against --bundle. dry_run short-circuits before store.commit(), so running it cannot write. * authenticate() sent X-Node-Key, but require_node parses only "bearer " (routes_compute.py:477) — a valid key reported as rejected. It also probed /api/auth/me, which resolves a browser session, so no node credential could ever pass it. Now Bearer on /api/node/agent, and 403 there counts as authenticated-but-unscoped, since the scope check runs after resolution. * _summarise reduced a plain-text body to "", discarding "Internal Server Error" and every handler message — the detail worth reading. * Four verdict names, including INGEST_ROUTED_NEEDS_RESTART, kept describing a restart that had already happened. _readiness now derives the name from evidence held by this run, and a real dry run outranks every transport observation: the current verdict is DRY_RUN_REJECTED. Measured against the controller: migration 0008 applied (27 -> 31 tables, guard proven to fire, idempotent on re-run), service restarted to PID 392511, endpoint routed and parsing. The dry run executes and is rejected on content_hash, because the HTTP path re-serializes records (json.dumps default separators, ensure_ascii=True) while their own read_bundle() hashes the file bytes — the same bundle yields d0011ae6 over HTTP and 21ce5687 on disk, and their validator reports 0 problems on disk. Reported to the controller; bundle hash unchanged. Also reported: routes_compute.py:1809 constructs InMemoryAudioImportStore, whose commit() appends to a list, and no PostgreSQL adapter exists — so a successful commit would return a uuid4() project_id while all four translation_* tables stayed empty. Read from source, not reproduced. 30 new tests pin the routing rule, the CSRF ordering, the credential header, and every verdict branch. Full suite 799 passed / 8 failed (pre-existing voice_profile_manager + test_pronunciation). ruff and mypy clean. --- scripts/audio_controller_connect.py | 397 ++++++++++++++++--- tests/test_audio_controller_connect.py | 518 +++++++++++++++++++++++++ 2 files changed, 854 insertions(+), 61 deletions(-) create mode 100644 tests/test_audio_controller_connect.py diff --git a/scripts/audio_controller_connect.py b/scripts/audio_controller_connect.py index dfa0102..864d067 100644 --- a/scripts/audio_controller_connect.py +++ b/scripts/audio_controller_connect.py @@ -167,14 +167,21 @@ class Client: return status, parsed, duration def authenticate(self, log: SyncLog) -> tuple[dict[str, str], str]: - """Node key, then service token. Returns (headers, how).""" + """Node key, then service token. Returns (headers, how). + + The node credential goes out as ``Authorization: Bearer `` because + that is what ``require_node`` parses: it rejects anything that does not + start with ``bearer `` (routes_compute.py:477) and never looks at an + ``X-Node-Key`` header. This used to send ``X-Node-Key``, so a perfectly + valid key was reported as a rejected credential. + """ node_key = os.environ.get("UMBRA_AUDIO_NODE_KEY") token = os.environ.get("UMBRA_AUDIO_API_TOKEN") if node_key: - log.write("auth", "try_node_key", endpoint="/api/auth/me", - note="X-Node-Key header") - return {"X-Node-Key": node_key}, "node_key_header" + log.write("auth", "try_node_key", endpoint="GET /api/node/agent", + note="Authorization: Bearer (node credential)") + return {"Authorization": f"Bearer {node_key}"}, "node_bearer" if token: log.write("auth", "try_service_token", endpoint="/api/auth/me", note="Authorization: Bearer") @@ -189,6 +196,11 @@ def _summarise(body: Any) -> Any: if body is None: return None + if isinstance(body, str): + # A plain-text body is usually the thing you need to read — "Internal + # Server Error", or a handler's own message. Reporting it as threw + # away exactly the detail the caller was looking for. + return body[:400] if isinstance(body, dict): return {k: (f"<{len(v)} items>" if isinstance(v, (list, dict)) else v) for k, v in body.items() @@ -219,17 +231,46 @@ def t01_health(client: Client) -> Result: def t02_auth(client: Client, headers: dict[str, str], how: str) -> Result: + """Prove the credential works on an endpoint that accepts its kind. + + `/api/auth/me` resolves a browser *session*, so a node credential gets 401 + there no matter how valid it is. Testing a node key against it reported a + working credential as rejected. A node credential authenticates on + `/api/node/*`, which is also CSRF-exempt, so a GET there is a clean probe. + """ + if not headers: return Result("T02", "Auth", "BLOCKED", "no credential is configured on this machine; " "an operator must supply a node key or a scoped service token", {"headers_tried": [], "how": how}) - status, body, _ = client.request("GET", "/api/auth/me", headers=headers) - ok = status == 200 - return Result("T02", "Auth", "PASS" if ok else "FAIL", - f"authenticated with {how}" if ok else f"credential rejected ({status})", + + node_bearer = how == "node_bearer" + endpoint = "/api/node/agent" if node_bearer else "/api/auth/me" + status, body, _ = client.request("GET", endpoint, headers=headers) + + if node_bearer: + # require_node() answers 401 for a bad/absent key, and the scope check + # answers 403 only *after* the credential resolved. So 403 here means + # authenticated with insufficient scope, which is still authentication. + ok = status in (200, 403) + detail = ( + f"node credential accepted on {endpoint} ({status})" if ok else + f"node credential rejected on {endpoint} ({status})" + ) + else: + ok = status == 200 + detail = ( + f"authenticated with {how} on {endpoint}" if ok else + f"credential rejected ({status})" + ) + + return Result("T02", "Auth", "PASS" if ok else "FAIL", detail, {"status_code": status, "mechanism": how, - "secret_logged": False}) + "endpoint": endpoint, "secret_logged": False, + "scope_note": ("403 on a node route is the scope check after " + "a successful credential resolve, not a " + "rejection" if node_bearer else None)}) def t03_version(client: Client) -> Result: @@ -248,41 +289,112 @@ def t03_version(client: Client) -> Result: "hard_incompatibility": False}) -def t04_ingest_exists(client: Client) -> Result: +def t04_ingest_exists(client: Client, + headers: dict[str, str] | None = None) -> Result: """Probe every candidate with both GET and POST. Probing only with GET is how a POST-only ingest route looked absent for a whole session: the real path is `POST /api/node/audio/import`, which answers GET with 404 and POST with 405, so a GET-only sweep concluded it was not there. A 404 to the wrong method is not evidence of absence. + + The rule that survived two wrong attempts, and the reason it is narrower + than "anything but 404": + + * On a SAFE method (GET), 404 means not routed and anything else means the + route table matched. This holds everywhere. + * On an UNSAFE method (POST), the controller checks CSRF in **middleware + before routing** (app.py:246-248) and exempts only + ``CSRF_EXEMPT_PREFIXES = ("/api/node",)``. So a POST to any other /api + path comes back 403 csrf_token_missing whether or not a route exists — + that 403 is the security layer talking and is not evidence of anything. + My earlier probe read exactly such a 403 on + ``/api/compute/audio/import`` as a live route; there is no such route. + + A 401 on a /api/node path, by contrast, is strong: that prefix is CSRF- + exempt, so the request routed, then ``require_node`` refused it. """ - probed: dict[str, dict[str, int]] = {} + # Mirror of the controller's CSRF_EXEMPT_PREFIXES (app.py:89). If the + # controller changes this, this list is what makes a POST probe trustworthy. + # Probe WITH the credential when one exists. Probing unauthenticated made a + # working route look permanently gated: every run reported 401 and the + # verdict stayed AUTH_REQUIRED even after auth was configured and T02 + # passed. An unauthenticated probe answers "is there a door", never "does it + # open". + auth_headers = headers or {} + + csrf_exempt_prefixes = ("/api/node",) + + probed: dict[str, dict[str, Any]] = {} routed: tuple[str, int] | None = None + csrf_masked: str | None = None for path in INGEST_CANDIDATES: - results = {} + results: dict[str, Any] = {} for method in ("GET", "POST"): status, _, _ = client.request( - method, path, body={} if method == "POST" else None) + method, path, body={} if method == "POST" else None, + headers=auth_headers) results[method] = status - probed[path] = results - for method, status in results.items(): - # 405 means the path is routed with a different method: it exists. - if status in (200, 201, 405) and routed is None: + if status is None: + continue + unsafe = method not in ("GET", "HEAD", "OPTIONS") + if unsafe and status == 403 and not path.startswith(csrf_exempt_prefixes): + # Middleware answered before routing. Not evidence either way. + results[f"{method}_note"] = ( + "403 csrf_token_missing from middleware that runs before " + "routing; proves nothing about whether a route exists") + csrf_masked = csrf_masked or f"{method} {path}" + continue + if status != 404 and routed is None: routed = (f"{method} {path}", status) + probed[path] = results if routed is not None: which, status = routed - return Result("T04", "Ingest endpoint exists", "PASS", - f"{which} is routed ({status}). 405 means the path exists " - f"under another method, which is a route being present.", - {"probed": probed, "routed": which, "status": status, - "reachable": status in (200, 201), - "note": ("Routed is not the same as reachable. A route can " - "be registered and still be refused for want of a " - "credential; 401/403 on a real path is progress, " - "not failure.")}) + serving = status in (200, 201) + gated = status in (401, 403) + if serving: + why = "responding normally." + elif gated: + why = ("the route resolved and then refused the request, which is " + "the credential doing its job, not the path going missing.") + elif status == 422: + why = ("the request body failed schema validation — routing and " + "request parsing both work; this probe sends an empty object " + "on purpose, so 422 is the expected answer and is not a " + "fault.") + elif status == 405: + why = "the path exists but is registered for another method." + elif status >= 500: + why = ("the route resolved and then failed inside its own body — " + "the path exists and the handler is reachable.") + else: + why = "the path exists and rejected the request." + detail = f"{which} is routed ({status}). {why}" + evidence: dict[str, Any] = { + "probed": probed, "routed": which, "status": status, + "reachable": serving, "auth_gated": gated, + "schema_rejected": status == 422, + "handler_error": status >= 500, + "note": ("Routed is not the same as reachable, and reachable is not " + "the same as working. A 401/403 is a live route behind a " + "credential, a 422 is a live route parsing a body, and a " + "5xx is a live route failing on this payload. None of them " + "is an absent endpoint."), + } + if csrf_masked: + evidence["csrf_masked_probe"] = ( + f"{csrf_masked} returned 403 from middleware that runs before " + "routing, so it was not counted as a route") + return Result("T04", "Ingest endpoint exists", "PASS", detail, evidence) + csrf_note = ( + {"csrf_masked_probe": ( + f"{csrf_masked} returned 403 from middleware that runs before " + "routing, so it was not counted as a route")} + if csrf_masked else {} + ) return Result("T04", "Ingest endpoint exists", "FAIL", "no ingest path responded to GET or POST. NOTE: this is probed " "paths only, not the controller's route table. Before treating " @@ -292,21 +404,84 @@ def t04_ingest_exists(client: Client) -> Result: "That mistake was made here once.", {"probed": probed, "finding": "UNCONFIRMED_INGEST_ENDPOINT", "how_to_confirm": "read the controller's router registrations, " - "not its 404s"}) + "not its 404s", + **csrf_note}) -def t05_dry_run(client: Client) -> Result: - return Result("T05", "Dry-run validation", "BLOCKED", - "depends on an ingest endpoint; see T04. The controller's own " - "plan sequences ingest after the metadata bundle, so this is " - "the designed order rather than a defect.", - {"depends_on": "T04"}) +def t05_dry_run(client: Client, + bundle_dir: str | None, + headers: dict[str, str]) -> Result: + """Actually run a dry run, when there is a bundle to run it against. + + This used to return BLOCKED unconditionally with "depends on an ingest + endpoint; see T04" — which stayed true after the endpoint came up and after + a credential arrived, so the most important test in the file was the one + least willing to run. A dry run cannot write: ``import_bundle`` returns at + ``if dry_run`` before it ever reaches the store, so executing it is safe by + construction and this test now behaves like it believes that. + """ + + if not bundle_dir: + return Result("T05", "Dry-run validation", "BLOCKED", + "no --bundle given, so there is nothing to validate " + "against; pass --bundle to run a real dry run", + {"depends_on": None, "ran": False}) + + root = Path(bundle_dir) + envelope_path = root / "bundle.json" + manifest_path = root / "manifest.jsonl" + if not envelope_path.is_file() or not manifest_path.is_file(): + return Result("T05", "Dry-run validation", "BLOCKED", + f"{bundle_dir} is missing bundle.json or manifest.jsonl", + {"ran": False}) + + envelope = json.loads(envelope_path.read_text(encoding="utf-8")) + records = [json.loads(line) for line in + manifest_path.read_text(encoding="utf-8").splitlines() + if line.strip()] + + status, body, duration = client.request( + "POST", "/api/node/audio/import", headers=headers, + body={"envelope": envelope, "records": records, + "dry_run": True, "confirm": None}) + + summary = _summarise(body) + evidence: dict[str, Any] = { + "status_code": status, + "duration_ms": duration, + "records_sent": len(records), + "dry_run": True, + "confirm": None, + "wrote": False, + "body": summary, + "note": ("dry_run short-circuits before store.commit(), so this " + "request cannot write. Nothing was pushed."), + } + + if status in (200, 201): + evidence["reported_status"] = body.get("status") if isinstance(body, dict) else None + evidence["content_hash"] = body.get("content_hash") if isinstance(body, dict) else None + return Result("T05", "Dry-run validation", "PASS", + f"controller accepted the bundle in dry-run mode ({status}); " + f"reported status={evidence['reported_status']}", evidence) + + detail = (f"dry run rejected ({status}); " + + (str(summary)[:300] if summary else "empty body")) + if status >= 500: + detail += ( + " — a 5xx on a valid-looking payload is the handler failing inside " + "its own validation rather than reporting a client error" + ) + return Result("T05", "Dry-run validation", "FAIL", detail, evidence) def t06_idempotency(client: Client) -> Result: return Result("T06", "Idempotency check", "BLOCKED", - "depends on an ingest endpoint; see T04", - {"depends_on": "T04", + "needs T05 to produce a content_hash to compare against, and " + "T05 does not get one while the bundle is rejected. The " + "controller also reports its accepted manifest format only " + "after it receives a bundle, so this is sequenced, not broken.", + {"depends_on": "T05", "client_side_equivalent": "the bundle build is deterministic: " "the same project yields the same manifest.jsonl bytes and the " "same content_hash, so re-shipping is detectable without a server"}) @@ -384,6 +559,125 @@ def t10_payload_readiness(bundle_records: int | None, bundle_bytes: int | None) # -------------------------------------------------------------------------- +def _readiness( + routed_ingest: Result | None, + missing_ingest: bool, + t05: Result | None = None, +) -> tuple[str, list[str]]: + """Name the controller's actual ingest state, not a nearby one. + + This ran through three names and every one of them was wrong about the + thing it described, in the same direction: each described the day it was + written rather than the state it was reading. + + * absent - guessed paths all 404'd, reported as MISSING. Wrong: the route + existed and was probed with GET only. + * needs restart - the route answered 405, reported as needing a restart. + Right at the time, then kept being reported after the restart happened. + * and a 401 was read as a dead end when it is a live route doing its job. + + The states are separated by evidence the run actually holds: whether a path + resolved at all, whether the credential was the reason it refused, and + whether the handler came back. + + A real dry run outranks every transport observation. Once T05 has actually + run a bundle, "is the route up" is settled and the thing worth naming is + what the controller said about the data — which is where the remaining + problem lives. + """ + + base = [ + "The controller's own request is for a metadata bundle, validated by its " + "own validator, handed over as a file. That is done.", + "Its request also says the accepted manifest format comes after it " + "receives the bundle, so T05-T08 are sequenced, not broken.", + ] + + if t05 is not None and t05.status == "FAIL": + return ("DRY_RUN_REJECTED", [ + f"A real dry run was executed against the full bundle and the " + f"controller rejected it: {t05.detail}", + "The endpoint is live, authenticated and reachable — this is now a " + "question about the data contract, not about connectivity.", + "dry_run was true and confirm was null, so nothing could have been " + "written even had it succeeded. Nothing was pushed.", + "The controller has been told the failing message verbatim, along " + "with the exact line that produces a different content_hash for the " + "same bundle.", + ] + base) + + if t05 is not None and t05.status == "PASS": + return ("DRY_RUN_OK_CONFIRMATION_REQUIRED", [ + f"The controller accepted the bundle in dry-run mode: {t05.detail}", + "A dry run is not an import. Committing needs dry_run false and " + "confirm set to the content_hash the controller returned, plus an " + "explicit operator instruction.", + "Nothing has been pushed.", + ] + base) + + if routed_ingest is None: + return ("INGEST_PATH_NOT_CONFIRMED", [ + "No ingest path responded to GET or POST, so a dataset cannot be " + "pushed; nothing was pushed. This is UNCONFIRMED: confirm against " + "the controller's route table before treating it as absent. A 404 " + "to a guessed path is evidence about the guess, not about the " + "controller.", + ] + base) + + evidence = routed_ingest.evidence + where = evidence["routed"] + status = evidence["status"] + + if evidence.get("auth_gated"): + return ("INGEST_ROUTED_AUTH_REQUIRED", [ + f"The ingest route {where} is registered and resolved the request " + f"({status}); it then refused for want of a credential, which is the " + "auth check inside the route running as designed.", + "A 401/403 here is the strongest short of a 200 that the route is " + "live: routing happens before the dependency, so the path matched.", + "Nothing was pushed. T02/T05-T08 stay BLOCKED until a credential " + "with dataset:ingest is configured for this run.", + ] + base) + + if evidence.get("handler_error"): + return ("INGEST_ROUTED_HANDLER_ERROR", [ + f"The ingest route {where} is registered, authenticated and " + f"reachable ({status}): the request routed, passed the credential " + "check, and failed inside the handler.", + "T04 probes with an empty JSON body on purpose — enough to prove " + "the path resolves and the handler runs, and nothing that could " + "write. A 5xx from that body means the handler is reachable and " + "rejecting an empty payload, which is expected; the verdict names " + "the reachable-and-failing state rather than calling the endpoint " + "absent.", + "Nothing was pushed. The empty probe body is not a bundle, so this " + "says nothing about whether a real one would import — supply " + "--bundle and T05 answers that directly.", + ] + base) + + if evidence.get("schema_rejected"): + return ("INGEST_ROUTED_AWAITING_BUNDLE", [ + f"The ingest route {where} is registered, authenticated and parsing " + f"request bodies ({status} against an empty probe object).", + "422 here is the request schema doing its job on a body that is " + "deliberately incomplete. The endpoint is ready for a real bundle; " + "no bundle was sent in this run because none was supplied.", + "Nothing was pushed.", + ] + base) + + if not evidence.get("reachable"): + return ("INGEST_ROUTED_NOT_SERVING", [ + f"The ingest route {where} is registered but answered {status}. " + "Nothing was pushed.", + ] + base) + + return ("READY_FOR_PUSH", [ + f"The ingest route {where} answered {status} normally.", + "A dry run still has to succeed before any commit, and a commit still " + "requires explicit confirmation.", + ] + base) + + def proposed_contract() -> dict[str, Any]: return { "note": ("A proposal only. No endpoint was invented, probed into " @@ -513,8 +807,8 @@ def main() -> int: t01_health(client), t02_auth(client, headers, how), t03_version(client), - t04_ingest_exists(client), - t05_dry_run(client), + t04_ingest_exists(client, headers), + t05_dry_run(client, args.bundle, headers), t06_idempotency(client), t07_schema_contract(client), t08_audit_round_trip(client), @@ -535,6 +829,9 @@ def main() -> int: transport = "plain HTTP (uvicorn); no TLS on this port" \ if client.base_url.startswith("http") else "HTTPS" + t05_result = next((r for r in results if r.test_id == "T05"), None) + verdict, rationale = _readiness(routed_ingest, missing_ingest, t05_result) + report: dict[str, Any] = { "report_version": 1, "generated_at": now_iso(), @@ -565,30 +862,8 @@ def main() -> int: "blocking_findings": ["UNCONFIRMED_INGEST_ENDPOINT"] if missing_ingest else [], }, "readiness": { - "verdict": ("INGEST_ROUTED_NEEDS_RESTART" if routed_ingest - and not routed_ingest.evidence.get("reachable") - else "READY_FOR_METADATA_HANDOVER_NOT_READY_FOR_PUSH"), - "rationale": ( - [ - f"The ingest route {routed_ingest.evidence['routed']} is " - "registered but not reachable. Nothing was pushed.", - "A route that answers 405 is present in the table and absent " - "from the running process, which means the serving process " - "predates the code that defines it. /health stays green " - "throughout, because it probes liveness and not the route " - "table.", - ] if routed_ingest and not routed_ingest.evidence.get("reachable") else [ - "No ingest path responded to GET or POST, so a dataset cannot " - "be pushed; nothing was pushed. This is UNCONFIRMED: confirm " - "against the controller's route table before treating it as " - "absent. A 404 to a guessed path is not evidence of absence.", - ] - ) + [ - "The controller's own request is for a metadata bundle, validated " - "by its own validator, handed over as a file. That is done.", - "Its request also says the accepted manifest format comes after " - "it receives the bundle, so T05-T08 are sequenced, not broken.", - ], + "verdict": verdict, + "rationale": rationale, }, "proposed_ingest_contract": proposed_contract() if missing_ingest else None, "confirmation_gate": { diff --git a/tests/test_audio_controller_connect.py b/tests/test_audio_controller_connect.py new file mode 100644 index 0000000..b8e2061 --- /dev/null +++ b/tests/test_audio_controller_connect.py @@ -0,0 +1,518 @@ +"""Tests for the controller connectivity probe. + +These exist because three consecutive runs of this tool reported the ingest +endpoint wrongly, each time with confidence and each time in the same +direction — describing the day the check was written rather than the state it +was reading. None of the faults were subtle: + +1. Six guessed paths were probed while the controller's route table was one SSH + command away, and 404 on a guess was read as absence. +2. Probing used GET only. The route is POST-only, so it answered GET with 404 + and looked missing for a whole session. +3. After the route came up and answered 401, that was read as a dead end rather + than as a live route refusing an unauthenticated request. + +Each is pinned here. The fake client scripts exact status codes so the +assertions are about the *interpretation* rule — which is where the faults +were — and not about a live controller. +""" + +from __future__ import annotations + +import importlib.util +import json +import sys +from pathlib import Path +from typing import TYPE_CHECKING, Any + +if TYPE_CHECKING: + import pytest + +REPO = Path(__file__).resolve().parent.parent +TOOL_PATH = REPO / "scripts" / "audio_controller_connect.py" +NODE_IMPORT = "/api/node/audio/import" + + +def _load_tool() -> Any: + spec = importlib.util.spec_from_file_location("audio_controller_connect", TOOL_PATH) + assert spec and spec.loader + module = importlib.util.module_from_spec(spec) + sys.modules[spec.name] = module + spec.loader.exec_module(module) + return module + + +tool = _load_tool() + + +class FakeClient: + """Stands in for the HTTP client, returning scripted status codes. + + Keyed by (method, path). Any path not scripted answers 404, which is what a + real controller does for an unrouted path, and is the case that must be + distinguished from every routed one. + """ + + def __init__(self, scripted: dict[tuple[str, str], int], + bodies: dict[tuple[str, str], Any] | None = None) -> None: + self.scripted = scripted + self.bodies = bodies or {} + self.calls: list[tuple[str, str, dict[str, str]]] = [] + + def request(self, method: str, path: str, *, body: Any = None, + headers: dict[str, str] | None = None, + phase: str = "probe") -> tuple[int, Any, int]: + self.calls.append((method, path, dict(headers or {}))) + status = self.scripted.get((method, path), 404) + return status, self.bodies.get((method, path)), 1.0 + + +# --------------------------------------------------------------------------- +# Finding an endpoint +# --------------------------------------------------------------------------- + + +def test_everything_404_is_reported_as_unconfirmed_not_missing() -> None: + """Bug 1: a guessed path that 404s is evidence about the guess.""" + + result = tool.t04_ingest_exists(FakeClient({})) + + assert result.status == "FAIL" + assert result.evidence["finding"] == "UNCONFIRMED_INGEST_ENDPOINT" + assert "route table" in result.detail + assert "guessed" in result.detail + + +def test_get_only_404_with_post_401_is_routed() -> None: + """Bug 2: a POST-only route answers GET with 404. That is not absence.""" + + result = tool.t04_ingest_exists(FakeClient({ + ("GET", NODE_IMPORT): 404, + ("POST", NODE_IMPORT): 401, + })) + + assert result.status == "PASS" + assert result.evidence["routed"] == f"POST {NODE_IMPORT}" + assert result.evidence["auth_gated"] is True + assert result.evidence["reachable"] is False + + +def test_405_counts_as_routed() -> None: + """The original signal: path in the table, wrong method. + + 405 is routing, not authentication — it says the path matched a route + registered for another method, so it must not be reported as an auth gate. + """ + + result = tool.t04_ingest_exists(FakeClient({ + ("GET", NODE_IMPORT): 404, + ("POST", NODE_IMPORT): 405, + })) + + assert result.status == "PASS" + assert result.evidence["routed"] == f"POST {NODE_IMPORT}" + assert result.evidence["auth_gated"] is False + assert result.evidence["handler_error"] is False + assert result.evidence["reachable"] is False + assert "another method" in result.detail + + +def test_post_401_on_a_node_path_is_not_a_dead_end() -> None: + """Bug 3: require_node runs inside the route, so 401 proves it resolved.""" + + result = tool.t04_ingest_exists(FakeClient({("POST", NODE_IMPORT): 401})) + + assert result.status == "PASS" + assert result.evidence["auth_gated"] is True + assert "credential doing its job" in result.detail + assert "not the path going missing" in result.detail + + +def test_csrf_403_on_a_non_node_path_is_not_routed() -> None: + """A 403 from CSRF middleware runs before routing and proves nothing. + + The controller checks CSRF in middleware (app.py:246-248) and exempts only + ``/api/node`` (app.py:89). So POST /api/compute/audio/import answers 403 + whether or not a route exists — there is in fact no such route. Counting it + would report a live ingest path that does not exist. + """ + + client = FakeClient({ + ("POST", "/api/compute/audio/import"): 403, + ("GET", "/api/compute/audio/import"): 404, + ("GET", NODE_IMPORT): 404, + ("POST", NODE_IMPORT): 404, + }) + result = tool.t04_ingest_exists(client) + + assert result.status == "FAIL", "a pre-routing 403 must not count as routed" + assert result.evidence["finding"] == "UNCONFIRMED_INGEST_ENDPOINT" + + # The discounted probe must survive into the report, otherwise the next + # reader sees only 404s and re-litigates the same false positive. + masked = result.evidence["csrf_masked_probe"] + assert "middleware that runs before routing" in masked + assert "/api/compute/audio/import" in masked + + per_path = result.evidence["probed"]["/api/compute/audio/import"] + assert "csrf_token_missing" in per_path["POST_note"] + assert per_path["POST"] == 403 + assert per_path["GET"] == 404 + + +def test_403_on_a_node_path_is_routed_because_that_prefix_is_csrf_exempt() -> None: + result = tool.t04_ingest_exists(FakeClient({("POST", NODE_IMPORT): 403})) + + assert result.status == "PASS" + assert result.evidence["auth_gated"] is True + + +def test_handler_error_is_distinguished_from_absent() -> None: + result = tool.t04_ingest_exists(FakeClient({("POST", NODE_IMPORT): 500})) + + assert result.status == "PASS" + assert result.evidence["handler_error"] is True + assert result.evidence["auth_gated"] is False + assert "handler is reachable" in result.detail + + +def test_credential_is_forwarded_to_the_probe() -> None: + """Probing without the credential made a live route look permanently gated.""" + + client = FakeClient({("POST", NODE_IMPORT): 401}) + tool.t04_ingest_exists(client, {"Authorization": "Bearer "}) + + assert client.calls, "no request was made" + assert all(h.get("Authorization") for _, _, h in client.calls) + + +def test_probe_without_credential_omits_headers_cleanly() -> None: + client = FakeClient({("POST", NODE_IMPORT): 401}) + result = tool.t04_ingest_exists(client) + + assert result.status == "PASS" + assert client.calls[0][2] == {} + + +def test_first_routed_path_in_candidate_order_wins() -> None: + """The real route is first in INGEST_CANDIDATES and must be the one named.""" + + client = FakeClient({ + ("POST", NODE_IMPORT): 401, + ("POST", "/api/compute/audio/import"): 403, + ("GET", "/api/compute/audio/import"): 404, + }) + result = tool.t04_ingest_exists(client) + + assert result.evidence["routed"] == f"POST {NODE_IMPORT}" + + +# --------------------------------------------------------------------------- +# Readiness verdicts +# --------------------------------------------------------------------------- + + +def _readiness(evidence: dict[str, Any] | None, **kwargs: Any) -> tuple[str, list[str]]: + """Build a routed result from a bare evidence dict and ask for a verdict.""" + + routed = None + if evidence is not None: + routed = tool.Result("T04", "Ingest endpoint exists", "PASS", "detail", evidence) + kwargs.setdefault("missing_ingest", evidence is None) + return tool._readiness(routed, **kwargs) + + +def test_readiness_names_unconfirmed_path_without_claiming_absence() -> None: + verdict, rationale = _readiness(None) + + assert verdict == "INGEST_PATH_NOT_CONFIRMED" + assert any("UNCONFIRMED" in line for line in rationale) + + +def test_readiness_auth_required_is_separate_from_needs_restart() -> None: + verdict, rationale = _readiness({ + "routed": f"POST {NODE_IMPORT}", "status": 401, "reachable": False, + "auth_gated": True, "handler_error": False, + }) + + assert verdict == "INGEST_ROUTED_AUTH_REQUIRED" + assert any("401/403" in line for line in rationale) + + +def test_readiness_handler_error_is_not_a_restart_request() -> None: + """After the restart happened, the verdict must stop asking for one.""" + + verdict, _ = _readiness({ + "routed": f"POST {NODE_IMPORT}", "status": 500, "reachable": False, + "auth_gated": False, "handler_error": True, + }) + + assert verdict == "INGEST_ROUTED_HANDLER_ERROR" + assert "RESTART" not in verdict + + +def test_readiness_ready_only_when_serving() -> None: + verdict, _ = _readiness({ + "routed": f"POST {NODE_IMPORT}", "status": 200, "reachable": True, + "auth_gated": False, "handler_error": False, + }) + + assert verdict == "READY_FOR_PUSH" + + +def test_readiness_routed_but_not_serving_is_named() -> None: + verdict, _ = _readiness({ + "routed": f"POST {NODE_IMPORT}", "status": 405, "reachable": False, + "auth_gated": False, "handler_error": False, + }) + + assert verdict == "INGEST_ROUTED_NOT_SERVING" + + +def test_readiness_422_means_parsing_not_broken() -> None: + """A 422 against an empty probe body is the schema working, not a fault.""" + + verdict, rationale = _readiness({ + "routed": f"POST {NODE_IMPORT}", "status": 422, "reachable": False, + "auth_gated": False, "handler_error": False, "schema_rejected": True, + }) + + assert verdict == "INGEST_ROUTED_AWAITING_BUNDLE" + assert any("parsing" in line for line in rationale) + + +def test_a_real_dry_run_failure_outranks_transport_observations() -> None: + """Once T05 has run, connectivity is settled and the data is the story.""" + + t05 = tool.Result("T05", "Dry-run validation", "FAIL", + "dry run rejected (500); content_hash mismatch", {}) + verdict, rationale = _readiness( + {"routed": f"POST {NODE_IMPORT}", "status": 422, "reachable": False, + "auth_gated": False, "handler_error": False, "schema_rejected": True}, + missing_ingest=False, t05=t05) + + assert verdict == "DRY_RUN_REJECTED" + assert any("content_hash mismatch" in line for line in rationale) + assert any("Nothing was pushed" in line for line in rationale) + + +def test_a_passing_dry_run_still_requires_confirmation() -> None: + t05 = tool.Result("T05", "Dry-run validation", "PASS", + "controller accepted the bundle", {"status": "dry_run"}) + verdict, rationale = _readiness( + {"routed": f"POST {NODE_IMPORT}", "status": 200, "reachable": True, + "auth_gated": False, "handler_error": False}, + missing_ingest=False, t05=t05) + + assert verdict == "DRY_RUN_OK_CONFIRMATION_REQUIRED" + assert any("not an import" in line for line in rationale) + assert any("Nothing has been pushed" in line for line in rationale) + + +def test_dry_run_blocked_does_not_override_transport_state() -> None: + """A bundle that was never supplied must not be reported as a rejection.""" + + t05 = tool.Result("T05", "Dry-run validation", "BLOCKED", + "no --bundle given", {"ran": False}) + verdict, _ = _readiness( + {"routed": f"POST {NODE_IMPORT}", "status": 401, "reachable": False, + "auth_gated": True, "handler_error": False}, + missing_ingest=False, t05=t05) + + assert verdict == "INGEST_ROUTED_AUTH_REQUIRED" + + +def test_no_verdict_still_claims_a_restart_after_the_restart_happened() -> None: + """INGEST_ROUTED_NEEDS_RESTART survived the restart it was describing.""" + + seen = set() + for evidence in ( + None, + {"routed": f"POST {NODE_IMPORT}", "status": 401, "reachable": False, + "auth_gated": True, "handler_error": False}, + {"routed": f"POST {NODE_IMPORT}", "status": 500, "reachable": False, + "auth_gated": False, "handler_error": True}, + {"routed": f"POST {NODE_IMPORT}", "status": 405, "reachable": False, + "auth_gated": False, "handler_error": False}, + {"routed": f"POST {NODE_IMPORT}", "status": 200, "reachable": True, + "auth_gated": False, "handler_error": False}, + ): + verdict, _ = _readiness(evidence) + seen.add(verdict) + + assert "INGEST_ROUTED_NEEDS_RESTART" not in seen + + +def test_every_readiness_branch_carries_the_standing_facts() -> None: + """The bundle story and the sequenced tests are true in every state.""" + + for evidence in ( + None, + {"routed": f"POST {NODE_IMPORT}", "status": 401, "reachable": False, + "auth_gated": True, "handler_error": False}, + {"routed": f"POST {NODE_IMPORT}", "status": 500, "reachable": False, + "auth_gated": False, "handler_error": True}, + {"routed": f"POST {NODE_IMPORT}", "status": 200, "reachable": True, + "auth_gated": False, "handler_error": False}, + ): + _, rationale = _readiness(evidence) + assert any("metadata bundle" in line for line in rationale) + assert any("T05-T08 are sequenced" in line for line in rationale) + + +def test_every_non_ready_verdict_says_nothing_was_pushed() -> None: + """Only READY_FOR_PUSH may omit it; every other state must state it plainly.""" + + non_ready = ( + None, + {"routed": f"POST {NODE_IMPORT}", "status": 401, "reachable": False, + "auth_gated": True, "handler_error": False}, + {"routed": f"POST {NODE_IMPORT}", "status": 500, "reachable": False, + "auth_gated": False, "handler_error": True}, + {"routed": f"POST {NODE_IMPORT}", "status": 405, "reachable": False, + "auth_gated": False, "handler_error": False}, + ) + for evidence in non_ready: + verdict, rationale = _readiness(evidence) + assert verdict != "READY_FOR_PUSH" + assert any("pushed" in line for line in rationale), ( + f"{verdict} does not say whether anything was pushed" + ) + + verdict, rationale = _readiness({ + "routed": f"POST {NODE_IMPORT}", "status": 200, "reachable": True, + "auth_gated": False, "handler_error": False, + }) + assert verdict == "READY_FOR_PUSH" + assert not any("Nothing was pushed" in line for line in rationale) + + +# --------------------------------------------------------------------------- +# Credential handling +# --------------------------------------------------------------------------- + + +def test_node_key_is_sent_as_bearer_not_x_node_key(monkeypatch: pytest.MonkeyPatch, + tmp_path: Path) -> None: + """require_node parses only 'bearer ' (routes_compute.py:477). + + Sending X-Node-Key meant a perfectly valid key came back rejected. + """ + + monkeypatch.setenv("UMBRA_AUDIO_NODE_KEY", "") + monkeypatch.delenv("UMBRA_AUDIO_API_TOKEN", raising=False) + log = tool.SyncLog(tmp_path / "log.jsonl") + client = tool.Client.__new__(tool.Client) + + headers, how = client.authenticate(log) + + assert how == "node_bearer" + assert "X-Node-Key" not in headers + assert headers["Authorization"] == "Bearer " + + +def test_service_token_falls_back_when_no_node_key(monkeypatch: pytest.MonkeyPatch, + tmp_path: Path) -> None: + monkeypatch.delenv("UMBRA_AUDIO_NODE_KEY", raising=False) + monkeypatch.setenv("UMBRA_AUDIO_API_TOKEN", "") + log = tool.SyncLog(tmp_path / "log.jsonl") + client = tool.Client.__new__(tool.Client) + + headers, how = client.authenticate(log) + + assert how == "service_token_bearer" + assert headers["Authorization"] == "Bearer " + + +def test_no_credential_configured_returns_empty_headers(monkeypatch: pytest.MonkeyPatch, + tmp_path: Path) -> None: + monkeypatch.delenv("UMBRA_AUDIO_NODE_KEY", raising=False) + monkeypatch.delenv("UMBRA_AUDIO_API_TOKEN", raising=False) + log = tool.SyncLog(tmp_path / "log.jsonl") + client = tool.Client.__new__(tool.Client) + + headers, how = client.authenticate(log) + + assert headers == {} + assert how == "none" + + +def test_auth_log_records_a_mechanism_but_never_the_key( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None: + """The key goes in an env var and out in a header, never into a log line.""" + + secret = "YMcrD3QlREDACTEDTESTKEY" + monkeypatch.setenv("UMBRA_AUDIO_NODE_KEY", secret) + monkeypatch.delenv("UMBRA_AUDIO_API_TOKEN", raising=False) + path = tmp_path / "log.jsonl" + log = tool.SyncLog(path) + client = tool.Client.__new__(tool.Client) + + client.authenticate(log) + log.write("discovery", "example", endpoint="/api/node/agent") + + text = path.read_text(encoding="utf-8") + assert secret not in text + entries = [json.loads(line) for line in text.splitlines() if line.strip()] + assert any(e.get("action") == "try_node_key" for e in entries) + + +# --------------------------------------------------------------------------- +# Dry-run safety +# --------------------------------------------------------------------------- + + +def test_dry_run_is_blocked_without_a_bundle(tmp_path: Path) -> None: + result = tool.t05_dry_run(FakeClient({}), None, {}) + + assert result.status == "BLOCKED" + assert result.evidence["ran"] is False + + +def test_dry_run_is_blocked_on_an_incomplete_bundle(tmp_path: Path) -> None: + (tmp_path / "bundle.json").write_text("{}", encoding="utf-8") + + result = tool.t05_dry_run(FakeClient({}), str(tmp_path), {}) + + assert result.status == "BLOCKED" + assert "manifest.jsonl" in result.detail + + +def test_dry_run_never_claims_to_have_written(tmp_path: Path) -> None: + """Whatever happens, a dry run must not report a write.""" + + (tmp_path / "bundle.json").write_text( + json.dumps({"bundle_format_version": 1, + "content_hash": {"value": "0" * 64}}), encoding="utf-8") + (tmp_path / "manifest.jsonl").write_text( + json.dumps({"id": "b-1", "kind": "book"}) + "\n", encoding="utf-8") + + for status in (200, 400, 422, 500): + client = FakeClient({("POST", NODE_IMPORT): status}, + bodies={("POST", NODE_IMPORT): {"status": "dry_run"}}) + result = tool.t05_dry_run(client, str(tmp_path), {}) + + assert result.evidence["dry_run"] is True + assert result.evidence["wrote"] is False + assert result.evidence["confirm"] is None + assert result.evidence["records_sent"] == 1 + + +def test_dry_run_passes_only_on_a_2xx(tmp_path: Path) -> None: + (tmp_path / "bundle.json").write_text( + json.dumps({"bundle_format_version": 1, + "content_hash": {"value": "0" * 64}}), encoding="utf-8") + (tmp_path / "manifest.jsonl").write_text( + json.dumps({"id": "b-1", "kind": "book"}) + "\n", encoding="utf-8") + + passed = tool.t05_dry_run( + FakeClient({("POST", NODE_IMPORT): 200}, + bodies={("POST", NODE_IMPORT): {"status": "dry_run", + "content_hash": "a" * 64}}), + str(tmp_path), {}) + failed = tool.t05_dry_run(FakeClient({("POST", NODE_IMPORT): 500}), str(tmp_path), {}) + + assert passed.status == "PASS" + assert passed.evidence["content_hash"] == "a" * 64 + assert failed.status == "FAIL" + assert failed.evidence["status_code"] == 500