Skip to content

D132 — Audit authorization denials; serve RFC 9728 metadata; drop the unwired HTTP handler layer

Decision. Three corrections, found while implementing D128 (issue #122, outside its scope). (1) handleToolsCall (internal/mcpserver/server.go) returned as soon as s.authorize failed, before beginAuditCall ever ran — a denial produced no audit event at all, even though D119 already documents the attempt+completion pair with outcome=unauthorized as part of the contract. The denial is now audited at the point the decision actually happens: on the authorize failure path, handleToolsCall calls Server.auditDenied, which now takes the principal directly (already resolved from the request context per D118) instead of an *http.Request — the dependency existed only to re-derive the principal, redundantly, from the bearer token. (2) docs/transport-auth.md already claimed the server publishes RFC 9728 Protected Resource Metadata, and auth.go's isPublicPath already exempted /.well-known/oauth-protected-resource from authentication, but nothing served it — auth.ProtectedResourceMetadata had no caller outside its own test. The route is now registered in MultiKBServer.Handler(), the handler cmd/cartographer/serve.go actually wraps in OriginGuard(store.Middleware(...)); resource and authorization_servers both name this server's own base URL, reconstructed per-request from Host and X-Forwarded-Proto, since there is no separate OAuth authorization server to plumb through config (this server validates its own static tokens). (3) With both fixed, mcpAccessGuard and the exported FullHTTPHandler, WellKnownHandler, ListenAndServeWithHandler and ListenAndServeHandler (internal/mcpserver/httpserver.go) had no caller in production or in tests — auditDenied was mcpAccessGuard's only caller, and cmd/cartographer/serve.go has always built its handler directly from MultiKBServer.Handler(), never through any of the five. All five are deleted; the handful of tests that only exercised FullHTTPHandler as a third member of a "same behavior across handler constructors" table lost that one entry, not the test.

Rationale. An audit log exists precisely to show what was refused, and mcpAccessGuard's per-KB r/rw body-peek (D45) and service_get/resolve_secrets override (D47) had already been re-implemented in the authorizer (internal/mcpserver/policy.go, D118) — nothing was unenforced, but three comments elsewhere still pointed at the guard as the place access is decided (audit.go, tools_skill.go), so a reader looking for the enforcement point found the wrong function first. Serving the RFC 9728 route was the smaller of the two options available (serve it, or retract the docs claim and the auth exemption): the docs and the exemption already existed and were correct in intent, only the handler was missing. Deriving resource/authorization_servers from the request rather than adding a config field keeps the change to the size the docs already promised, and is honest about what this deployment actually is — its own token validator, not a client of a separate authorization server.

Consequences. A denied tools/call now appears in the audit log exactly like every other call, closing the gap between D119's documented contract and what the code did. The RFC 9728 endpoint is self-describing rather than operator-configured: an operator behind a reverse proxy that does not set X-Forwarded-Proto gets an http:// resource even when the public entry point is HTTPS — no worse than the previous 404, but not authoritative either; a future need for a precise external issuer would need real config, not this inference. Deleting the dead handler layer removes the only place FullHTTPHandler's CORS behavior was exercised at all outside its own tests — CORS for the production path continues to live in OriginGuard, unaffected by this change. Server.HTTPHandler (single-KB, no .well-known route) and the plain Server.ListenAndServe are unaffected: neither is on the production path (serveHTTP always mounts through MultiKBServer, even for one KB) but both remain for direct single-KB embedding and their own tests, which this issue did not ask to remove. (Superseded in part by D166: Server.ListenAndServe was deleted. The "and their own tests" half was already untrue when written — nothing called it, in production or in tests — so the embedding case it was kept for had no exercised path and no user.)