fix(ingestion): build /mcp's bearer and the metrics wrap inside buildMux (#154) (#183)

Co-authored-by: Mathias <mthbqv@gmail.com>

ingestion/cmd/server/routes_test.go

@@ -12,6 +12,7 @@ import (
 	"github.com/stretchr/testify/require"
 
 	"github.com/mathiasbq/hyperguild/ingestion/internal/api"
+	"github.com/mathiasbq/hyperguild/ingestion/internal/metrics"
 	"github.com/mathiasbq/hyperguild/ingestion/internal/pipeline"
 )
 
@@ -21,20 +22,29 @@ 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) 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()
-	h := testHandler(t)
+	mux, _ := testMuxAndRegistry(t)
+	return mux
+}
+
+func testMuxAndRegistry(t *testing.T) (http.Handler, *metrics.Registry) {
+	t.Helper()
 	public := map[string]http.Handler{}
 	for p := range publicRoutes {
 		public[p] = okHandler("public " + p)
 	}
-	mux, err := buildMux(restAuth{token: routesTok, realm: "brain"}, h, public)
+	reg := metrics.New()
+	mux, err := buildMux(restAuth{token: routesTok, realm: "brain"}, testHandler(t), okHandler("mcp"), reg, public)
 	require.NoError(t, err)
-	return mux
+	return mux, reg
 }
 
 func testHandler(t *testing.T) *api.Handler {
@@ -79,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 {
@@ -98,14 +109,53 @@ func TestBuildMuxDoesNotWrapPublicRoutes(t *testing.T) {
 // over the authenticated catch-all (more specific pattern), so it is refused.
 func TestBuildMuxRefusesAPublicPatternOffTheAllowlist(t *testing.T) {
 	h := api.NewHandler(t.TempDir(), slog.New(slog.NewTextHandler(os.Stderr, nil)), pipeline.Config{})
-	for _, p := range []string{"POST /query", "GET /pending", "/", "POST /metrics"} {
-		_, err := buildMux(restAuth{token: routesTok, realm: "brain"}, h, map[string]http.Handler{p: okHandler("x")})
+	// "/mcp" and "GET /metrics" are built by buildMux itself (hyperguild#154):
+	// passing either as public would register them unauthenticated or twice.
+	for _, p := range []string{"POST /query", "GET /pending", "/", "POST /metrics", "/mcp", "GET /metrics"} {
+		_, err := buildMux(restAuth{token: routesTok, realm: "brain"}, h, okHandler("mcp"), metrics.New(),
+			map[string]http.Handler{p: okHandler("x")})
 		assert.Error(t, err, "public pattern %q must be refused", p)
 	}
 }
 
 func TestBuildMuxRefusesAnEmptyToken(t *testing.T) {
 	h := api.NewHandler(t.TempDir(), slog.New(slog.NewTextHandler(os.Stderr, nil)), pipeline.Config{})
-	_, err := buildMux(restAuth{token: "", realm: "brain"}, h, nil)
+	_, err := buildMux(restAuth{token: "", realm: "brain"}, h, okHandler("mcp"), metrics.New(), nil)
 	assert.Error(t, err)
 }
+
+// hyperguild#154: /mcp's bearer was built in main.go, untested; the buildMux
+// tests used stubs. buildMux now wraps the MCP handler itself, with the same
+// credential as the REST API.
+func TestBuildMuxRefusesMCPWithoutAValidToken(t *testing.T) {
+	r := testMux(t)
+	for _, authz := range []string{"", "Bearer wrong"} {
+		code, body := do(t, r, "POST", "/mcp", authz)
+		assert.Equal(t, http.StatusUnauthorized, code, "authz %q", authz)
+		assert.NotEqual(t, "mcp", body)
+	}
+	code, body := do(t, r, "POST", "/mcp", "Bearer "+routesTok)
+	assert.Equal(t, http.StatusOK, code)
+	assert.Equal(t, "mcp", body)
+}
+
+func TestBuildMuxRefusesANilMCPHandlerOrRegistry(t *testing.T) {
+	auth := restAuth{token: routesTok, realm: "brain"}
+	_, err := buildMux(auth, testHandler(t), nil, metrics.New(), nil)
+	assert.Error(t, err)
+	_, err = buildMux(auth, testHandler(t), okHandler("mcp"), nil, nil)
+	assert.Error(t, err)
+}
+
+// hyperguild#154: main wrapped buildMux's result in the metrics middleware
+// before ListenAndServe, untested. buildMux now returns the handler main
+// serves as-is, metrics included: a request through it is observed and
+// exposed on its own GET /metrics.
+func TestBuildMuxObservesRequestsAndServesMetrics(t *testing.T) {
+	r := testMux(t)
+	code, _ := do(t, r, "GET", "/pending", "Bearer "+routesTok)
+	require.Equal(t, http.StatusOK, code)
+	code, body := do(t, r, "GET", "/metrics", "")
+	require.Equal(t, http.StatusOK, code)
+	assert.Contains(t, body, `path="GET /pending"`)
+}