From b04a34837c77d0ffc2c643e689506246b007c63d Mon Sep 17 00:00:00 2001 From: Mathias Date: Mon, 5 Oct 2026 16:34:16 +0200 Subject: [PATCH] test(ingestion): /mcp's JWT validator and resource_metadata; narrow comments PR #183 review (comment 15474): - TestBuildMuxAnswers503ToAJWTOnMCPWhileTheIssuerIsUnresolved: building /mcp's middleware with a nil validator turns it red. - TestBuildMuxMCP401CarriesResourceMetadataInWWWAuthenticate: building it with resourceMetadataURL "" turns it red. - main.go, routes.go and the testMux comment now say exactly what is untested: main's restAuth fields and public map passed to buildMux, and ListenAndServe (hyperguild#182). Stale public-route list in the TestBuildMuxDoesNotWrapPublicRoutes comment corrected. Co-Authored-By: Claude Opus 5.5 --- ingestion/cmd/server/jwt_validator_test.go | 28 ++++++++++++++++++++++ ingestion/cmd/server/main.go | 6 +++-- ingestion/cmd/server/routes.go | 5 ++-- ingestion/cmd/server/routes_test.go | 17 +++++++------ 4 files changed, 45 insertions(+), 11 deletions(-) diff --git a/ingestion/cmd/server/jwt_validator_test.go b/ingestion/cmd/server/jwt_validator_test.go index 448673d..9793a00 100644 --- a/ingestion/cmd/server/jwt_validator_test.go +++ b/ingestion/cmd/server/jwt_validator_test.go @@ -81,3 +81,31 @@ func TestBuildMuxAnswers503ToAJWTWhileTheIssuerIsUnresolved(t *testing.T) { code, _ := do(t, mux, "GET", "/pending", "Bearer aaa.bbb.ccc") assert.Equal(t, http.StatusServiceUnavailable, code) } + +// PR #183 review: /mcp shares REST's bearer, but only the static token was +// asserted on it. With the issuer unresolved, a JWT on /mcp answers 503 too, +// so /mcp's middleware was built with the JWT validator. +func TestBuildMuxAnswers503ToAJWTOnMCPWhileTheIssuerIsUnresolved(t *testing.T) { + srv, _ := fakeIssuer(t, 1000) + v, err := buildJWTValidator(context.Background(), slog.New(slog.DiscardHandler), srv.URL, "brain") + require.NoError(t, err) + mux, err := buildMux(restAuth{token: routesTok, validator: v, realm: "brain"}, testHandler(t), + okHandler("mcp"), metrics.New(), nil) + require.NoError(t, err) + code, _ := do(t, mux, "POST", "/mcp", "Bearer aaa.bbb.ccc") + assert.Equal(t, http.StatusServiceUnavailable, code) +} + +// PR #183 review: a 401 on /mcp names the protected-resource metadata in +// WWW-Authenticate (what the claude.ai OAuth connector discovers from), so +// /mcp's middleware was built with resourceMetadataURL. +func TestBuildMuxMCP401CarriesResourceMetadataInWWWAuthenticate(t *testing.T) { + const rm = "https://brain.example/.well-known/oauth-protected-resource" + mux, err := buildMux(restAuth{token: routesTok, realm: "brain", resourceMetadataURL: rm}, testHandler(t), + okHandler("mcp"), metrics.New(), nil) + require.NoError(t, err) + rec := httptest.NewRecorder() + mux.ServeHTTP(rec, httptest.NewRequest(http.MethodPost, "/mcp", nil)) + require.Equal(t, http.StatusUnauthorized, rec.Code) + assert.Contains(t, rec.Header().Get("WWW-Authenticate"), `resource_metadata="`+rm+`"`) +} diff --git a/ingestion/cmd/server/main.go b/ingestion/cmd/server/main.go index 65c48eb..db721e4 100644 --- a/ingestion/cmd/server/main.go +++ b/ingestion/cmd/server/main.go @@ -587,8 +587,10 @@ func main() { // // The whole router, /mcp's bearer included, is built in buildMux so its // tests are tests of production wiring (PR #152 review, hyperguild#154). - // The REST API's credential is the same as /mcp's. ListenAndServe below - // must serve its result unchanged: that one line is the untested part. + // The REST API's credential is the same as /mcp's. Untested here: the + // restAuth fields and the public map this call passes in (e.g. a nil + // validator would drop JWT auth on /mcp and REST and the suite stays + // green), and ListenAndServe serving the result unchanged (hyperguild#182). handler, err := buildMux(restAuth{token: mcpToken, validator: jwtValidator, realm: "brain", resourceMetadataURL: resourceMetadataURL}, h, mcpSrv, metrics.New(), public) if err != nil { diff --git a/ingestion/cmd/server/routes.go b/ingestion/cmd/server/routes.go index cd76b6f..671d0f4 100644 --- a/ingestion/cmd/server/routes.go +++ b/ingestion/cmd/server/routes.go @@ -41,8 +41,9 @@ type restAuth struct { // wrapped in reg's middleware. Other public handlers are registered as given, // and only for patterns on the publicRoutes allowlist. It returns an // http.Handler, not the *ServeMux, so nothing can register a route on the -// result. What stays untested is main's own line: a handler main built around -// this one before ListenAndServe would not be seen by these tests. +// result. What stays untested is main's call site: the restAuth fields and +// the public map main passes in, and ListenAndServe serving this handler +// unchanged (hyperguild#182). The tests build their own arguments. func buildMux( auth restAuth, h *api.Handler, mcpHandler http.Handler, reg *metrics.Registry, public map[string]http.Handler, ) (http.Handler, error) { diff --git a/ingestion/cmd/server/routes_test.go b/ingestion/cmd/server/routes_test.go index ae9674e..be538fb 100644 --- a/ingestion/cmd/server/routes_test.go +++ b/ingestion/cmd/server/routes_test.go @@ -22,11 +22,13 @@ func okHandler(body string) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { _, _ = w.Write([]byte(body)) }) } -// testMux builds the router exactly as main does: buildMux with the full -// public set main registers (stub handlers), a stub MCP server, and real -// bearer config. hyperguild#149: the REST API is authenticated as a whole; -// the PR #152 review showed a test of a COPY of the router let main.go -// regress unseen. +// testMux builds the router through buildMux, the function main calls, with +// every allowlisted public pattern (stub handlers), a stub MCP server and a +// static token only: no JWT validator and no resourceMetadataURL (the tests +// in jwt_validator_test.go pass those). It does not use main's own restAuth +// or public map (hyperguild#182). hyperguild#149: the REST API is +// authenticated as a whole; the PR #152 review showed a test of a COPY of +// the router let main.go regress unseen. func testMux(t *testing.T) http.Handler { t.Helper() mux, _ := testMuxAndRegistry(t) @@ -87,8 +89,9 @@ func TestBuildMuxAuthenticatesAnUnregisteredPath(t *testing.T) { } // buildMux registers public handlers as given, without adding auth: each -// public route owns its own (bearer on /mcp, HMAC on the webhook, client -// secret on /oauth/token) or is meant to be open (/metrics, OAuth metadata). +// public route owns its own (bearer/JWT on /capture, HMAC on the webhook, +// client secret on /oauth/token) or is meant to be open (OAuth metadata). +// /mcp and /metrics are not public routes: buildMux builds them itself. func TestBuildMuxDoesNotWrapPublicRoutes(t *testing.T) { r := testMux(t) for p := range publicRoutes {