Compare commits

...
1 Commits
Author SHA1 Message Date
mathiasandClaude Opus 5.5 a1e036168a fix(ingestion): build /mcp's bearer and the metrics wrap inside buildMux
CI / Lint / Test / Vet (pull_request) Successful in 56s
CI / Mirror to GitHub (pull_request) Skipped
/mcp's bearer was built in main.go and the buildMux tests used stubs, so
dropping it would have kept CI green. buildMux now takes the MCP handler
and wraps it with the same BearerMiddleware and credential as the REST
API. "/mcp" and "GET /metrics" left the public allowlist, so passing
either as a public route is refused. buildMux also registers GET
/metrics and returns the metrics-wrapped handler, so main passes its
result to ListenAndServe unchanged.

Flip-tested: registering /mcp unwrapped turns
TestBuildMuxRefusesMCPWithoutAValidToken red, and dropping the metrics
wrap turns the four metrics tests red.

Comments now claim only what is tested. The ListenAndServe line and the
symlink tripwire wiring in main (#154 comment) are still untested and
tracked in #182.

Closes #154

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-05 09:41:30 +02:00
5 changed files with 109 additions and 53 deletions
+4 -1
View File
@@ -12,6 +12,8 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/mathiasbq/hyperguild/ingestion/internal/metrics"
)
// fakeIssuer answers OIDC discovery with 502 for the first fail requests, as
@@ -73,7 +75,8 @@ func TestBuildMuxAnswers503ToAJWTWhileTheIssuerIsUnresolved(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), nil)
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, "GET", "/pending", "Bearer aaa.bbb.ccc")
assert.Equal(t, http.StatusServiceUnavailable, code)
+12 -17
View File
@@ -516,8 +516,6 @@ func main() {
}
}
public["/mcp"] = chassisauth.BearerMiddleware(mcpToken, jwtValidator, "brain", resourceMetadataURL, mcpSrv)
// POST /capture (#53/#54): the uniform capture REST door. Needs a ticket
// tracker to file action items, so it only mounts when Gitea is
// configured. It reuses the MCP server's graph-wired brain store (one
@@ -541,7 +539,7 @@ func main() {
public["POST /capture"] = captureH
// Same use-case behind the MCP `capture` tool (#55 relay) so MCP-native
// harnesses (claude.ai, Crush, Pi, LLM Council) reach capture through
// the existing /mcp OAuth connector. mcpSrv is already wrapped above;
// the existing /mcp OAuth connector. buildMux wraps this same mcpSrv;
// WithCapture mutates the same instance, so the tool appears live.
mcpSrv.WithCapture(captureSvc, jwtValidator, mcpToken, "local-cli", resolver)
// hyperguild#97: a governance-file rewrite gets the same trail a
@@ -583,19 +581,16 @@ func main() {
os.Exit(1)
}
// /metrics — unauthenticated Prometheus endpoint. kube-prometheus-stack
// scrapes it via the ServiceMonitor in k3s/apps/supervisor/. The metrics
// middleware below wraps every other registered handler so it observes
// real request latency. /metrics itself is excluded from its own
// observation by registering it on the outer mux (post-wrap).
reg := metrics.New()
public["GET /metrics"] = http.HandlerFunc(reg.Handler())
// The whole router, built in one place so the test of buildMux is a test
// of production wiring (PR #152 review). The REST API's credential is the
// same as /mcp's.
mux, err := buildMux(restAuth{token: mcpToken, validator: jwtValidator, realm: "brain",
resourceMetadataURL: resourceMetadataURL}, h, public)
// /metrics — unauthenticated Prometheus endpoint, scraped via the
// ServiceMonitor in k3s/apps/supervisor/. buildMux registers it and wraps
// the whole router in the registry's middleware.
//
// 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.
handler, err := buildMux(restAuth{token: mcpToken, validator: jwtValidator, realm: "brain",
resourceMetadataURL: resourceMetadataURL}, h, mcpSrv, metrics.New(), public)
if err != nil {
logger.Error("build router", "err", err)
os.Exit(1)
@@ -616,7 +611,7 @@ func main() {
"watch_interval", watchIntervalLog,
"mcp_enabled", true,
)
if err := http.ListenAndServe(addr, reg.Middleware(mux)); err != nil {
if err := http.ListenAndServe(addr, handler); err != nil {
logger.Error("server stopped", "err", err)
os.Exit(1)
}
+31 -18
View File
@@ -8,22 +8,22 @@ import (
chassisauth "git.d-ma.be/mathias/mcp-chassis/auth"
"github.com/mathiasbq/hyperguild/ingestion/internal/api"
"github.com/mathiasbq/hyperguild/ingestion/internal/metrics"
)
// publicRoutes is the complete set of patterns allowed on the outer mux,
// outside the REST API's auth (hyperguild#149). Each authenticates itself or is
// meant to be open: /mcp (bearer/JWT), /capture (bearer/JWT), the webhook
// (HMAC), /oauth/token (client secret), OAuth metadata and /metrics (open).
// A REST pattern added here would beat the authenticated catch-all by
// specificity and ship open, which is why buildMux refuses anything else.
// publicRoutes is the complete set of patterns a caller may register on the
// outer mux, outside the REST API's auth (hyperguild#149). Each authenticates
// itself or is meant to be open: /capture (bearer/JWT), the webhook (HMAC),
// /oauth/token (client secret) and OAuth metadata (open). /mcp and /metrics
// are not here: buildMux builds them itself (hyperguild#154). A REST pattern
// added here would beat the authenticated catch-all by specificity and ship
// open, which is why buildMux refuses anything else.
var publicRoutes = map[string]bool{
"/mcp": true,
"POST /capture": true,
"POST /webhooks/brain-sync": true,
"GET /.well-known/oauth-protected-resource": true,
"GET /.well-known/oauth-authorization-server": true,
"POST /oauth/token": true,
"GET /metrics": true,
}
// restAuth is the credential the REST API checks: the same as /mcp's.
@@ -34,16 +34,24 @@ type restAuth struct {
resourceMetadataURL string
}
// buildMux is the server's whole router. The REST API is mounted at "/"
// behind bearer auth that buildMux constructs itself, so a caller cannot pass
// a no-op; 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 afterwards and
// bypass the allowlist.
func buildMux(auth restAuth, h *api.Handler, public map[string]http.Handler) (http.Handler, error) {
// buildMux builds the handler main passes to ListenAndServe unchanged. The
// REST API is mounted at "/" and /mcp is registered, both behind bearer auth
// that buildMux constructs itself from auth (one credential), so a caller
// cannot pass a no-op. GET /metrics serves reg, and the whole router is
// 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.
func buildMux(
auth restAuth, h *api.Handler, mcpHandler http.Handler, reg *metrics.Registry, public map[string]http.Handler,
) (http.Handler, error) {
if auth.token == "" {
return nil, errors.New("buildMux: REST auth needs a token")
}
if mcpHandler == nil || reg == nil {
return nil, errors.New("buildMux: needs the MCP handler and the metrics registry")
}
mux := http.NewServeMux()
for pattern, handler := range public {
if !publicRoutes[pattern] {
@@ -51,10 +59,15 @@ func buildMux(auth restAuth, h *api.Handler, public map[string]http.Handler) (ht
}
mux.Handle(pattern, handler)
}
mountREST(mux, h, func(next http.Handler) http.Handler {
bearer := func(next http.Handler) http.Handler {
return chassisauth.BearerMiddleware(auth.token, auth.validator, auth.realm, auth.resourceMetadataURL, next)
})
return mux, nil
}
mux.Handle("/mcp", bearer(mcpHandler))
// /metrics is registered on the mux and so observed by its own
// middleware, like any route.
mux.Handle("GET /metrics", reg.Handler())
mountREST(mux, h, bearer)
return reg.Middleware(mux), nil
}
// mountREST registers the REST API on its own mux and mounts it at "/" behind
@@ -9,8 +9,6 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/mathiasbq/hyperguild/ingestion/internal/metrics"
)
// hyperguild#155: since #149 every REST route sits behind BearerMiddleware,
@@ -32,8 +30,8 @@ func TestBuildMuxLogsARESTAuthRejection(t *testing.T) {
// An authenticated REST request is labelled with the REST mux's pattern:
// the inner mux sets it on the same *http.Request the outer middleware reads.
func TestMetricsLabelAnAuthenticatedRESTRequestWithItsRoutePattern(t *testing.T) {
reg := metrics.New()
code, _ := do(t, reg.Middleware(testMux(t)), "GET", "/pending", "Bearer "+routesTok)
mux, reg := testMuxAndRegistry(t)
code, _ := do(t, mux, "GET", "/pending", "Bearer "+routesTok)
require.Equal(t, http.StatusOK, code)
rec := httptest.NewRecorder()
@@ -46,8 +44,8 @@ func TestMetricsLabelAnAuthenticatedRESTRequestWithItsRoutePattern(t *testing.T)
// so it was labelled path="/" and a 4xx on /query could not be told from one
// on /write. It is now labelled with the REST pattern it was addressed to.
func TestMetricsLabelARejectedRESTRequestWithItsRoutePattern(t *testing.T) {
reg := metrics.New()
code, _ := do(t, reg.Middleware(testMux(t)), "POST", "/query", "")
mux, reg := testMuxAndRegistry(t)
code, _ := do(t, mux, "POST", "/query", "")
require.Equal(t, http.StatusUnauthorized, code)
rec := httptest.NewRecorder()
@@ -58,8 +56,8 @@ func TestMetricsLabelARejectedRESTRequestWithItsRoutePattern(t *testing.T) {
// A path no REST route matches keeps the catch-all label, so an attacker
// probing random URLs cannot grow the series set.
func TestMetricsLabelARejectedUnknownPathWithTheCatchAll(t *testing.T) {
reg := metrics.New()
code, _ := do(t, reg.Middleware(testMux(t)), "POST", "/no-such-route-zq", "")
mux, reg := testMuxAndRegistry(t)
code, _ := do(t, mux, "POST", "/no-such-route-zq", "")
require.Equal(t, http.StatusUnauthorized, code)
rec := httptest.NewRecorder()
+56 -9
View File
@@ -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"
)
@@ -22,19 +23,26 @@ func okHandler(body string) http.Handler {
}
// 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.
// 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.
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 {
@@ -98,14 +106,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"`)
}