feat(catalog): detect upstream tool-definition drift, and stop the scanner failing open - #523
Conversation
…anner failing open Closes the gap in #521 and meets the substance of #58. The gateway now compares what each upstream server advertises against the approved catalog entry on first contact with that server, using a digest of the semantic triple (description, input schema, output schema). A mismatch fails the call closed by default, writes a catalog_drift audit entry naming the tool and the kind of change, and surfaces in the session TRACE Claim. Three deliberate design points: The enforcing comparison is standard-library only. CatalogScanner is backed by an optional dependency, and a control that stops working when a dependency is missing is not a control. The scanner now classifies the kind of change and is wired at startup so tool fingerprints are registered, but it never decides. The trigger is first contact per server, not notifications/tools/list_changed. Only stdio upstreams could carry that notification today, since HTTP upstreams are request/response here, so a notification-driven check would silently cover one transport and not the other. That asymmetry is what produced #521. CatalogScanResult and DriftResult now carry `available`, and report safe=False with tools_scanned=0 when agent-os-kernel is absent. They previously returned safe=True, so a deployment without the dependency received a clean result it had not earned. The test asserting that behaviour has been rewritten; its name, test_scan_catalog_safe_without_agt, was itself the bug. Also fixes ToolCatalogInfo.drift_detected, which was hardcoded False, so the TRACE Claim carried a drift field that could never be true. LIMITATIONS.md states the three things this does not do: no mid-session detection, no notification subscription, and a server that will not answer tools/list is recorded as unchecked rather than denied.
|
Correcting the last line of the description, which was wrong in a way that matters. I wrote that CI runs Filed as #524, with the counts. 58 conformance tests and 1 integration test are unrun. The gateway is correct on the failing assertion. Per #509 a legacy |
1 participant
Closes #521. Meets the substance of #58, which was closed in June with its acceptance criteria unmet.
What was wrong
SPEC.mdrates P4.2 (rug-pull via upstream tool-definition mutation) as covered. The detector written for it,CatalogScanner.check_drift, was referenced only by its own module, its unit test, and an experiment directory namedclaim3-rug-pull-detection. Nothing insrc/called it. Thecatalog_driftdeny path that does exist compares our own catalog file against itself and never talks to the upstream server.What this does
On first contact with each upstream server, the gateway fetches
tools/listand compares every advertised definition against its approved entry, using a digest of the semantic triple: description, input schema, output schema. Mismatch fails the call closed by default, writes acatalog_driftaudit entry naming the tool and the kind of change (definition_changedorwithdrawn), and surfaces in the session TRACE Claim.catalog.drift_policy: warn_onlyroutes the call and still records the drift, so an operator can measure their own fleet before enforcing. It is not meant as a resting state.Three design points worth reviewing
The enforcing comparison uses only the standard library.
CatalogScanneris backed by an optional dependency. A control that stops working when a dependency is missing is not a control, so the scanner classifies the kind of change and never decides. It is now wired at startup so tool fingerprints get registered, which is what makes classification possible at all.The trigger is first contact per server, not
notifications/tools/list_changed. Only stdio upstreams could carry that notification today, since HTTP upstreams here are plain request/response. A notification-driven check would have silently covered one transport and not the other, which is the same shape of partial coverage that produced #521. First-contact checking covers both identically. This is the one place where I have not met #58's literal first bullet, and it is deliberate.Absence is no longer reported as safety.
CatalogScanResultandDriftResultgainavailable. Withoutagent-os-kernelthey now returnsafe=False, tools_scanned=0, available=Falseinstead ofsafe=True, tools_scanned=len(catalog). The test that asserted the old behaviour has been rewritten, and its name,test_scan_catalog_safe_without_agt, was itself the defect.Also fixes
ToolCatalogInfo.drift_detected, hardcodedFalseatsession/manager.py:256, so the TRACE Claim carried a drift field that could never be true.What it does not do
Stated in
LIMITATIONS.mdrather than left implied: no mid-session detection, no notification subscription, and a server that will not answertools/listis recorded as unchecked rather than denied. The gateway already serves the approved description rather than the live one, so a mutated description does not reach the model even in those windows, but that is a structural property of proxying an approved catalog and not a detection result.SPEC.mdis untouched, per the decision to keep the rating and put the honest detail inLIMITATIONS.md.Testing
1209 passing, 23 skipped. 10 new tests in
tests/unit/test_upstream_catalog_drift.pycovering the digest primitive, camelCase and snake_case agreement, description-only mutation, fail-closed and warn-only, withdrawn tools, the unchecked path, once-per-server caching, and drift being caught with the optional scanner absent.tests/conformance/test_gateway_conformance.py::test_mcp_initializefails onmainas well, an unrelatedprotocolVersionmismatch (2025-11-25against an expected2026-07-28). Deselected locally, not touched here.