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
7 changed files with 234 additions and 76 deletions
@@ -81,31 +81,3 @@ func TestBuildMuxAnswers503ToAJWTWhileTheIssuerIsUnresolved(t *testing.T) {
code, _ := do(t, mux, "GET", "/pending", "Bearer aaa.bbb.ccc") code, _ := do(t, mux, "GET", "/pending", "Bearer aaa.bbb.ccc")
assert.Equal(t, http.StatusServiceUnavailable, code) 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+`"`)
}
+2 -4
View File
@@ -587,10 +587,8 @@ func main() {
// //
// The whole router, /mcp's bearer included, is built in buildMux so its // The whole router, /mcp's bearer included, is built in buildMux so its
// tests are tests of production wiring (PR #152 review, hyperguild#154). // tests are tests of production wiring (PR #152 review, hyperguild#154).
// The REST API's credential is the same as /mcp's. Untested here: the // The REST API's credential is the same as /mcp's. ListenAndServe below
// restAuth fields and the public map this call passes in (e.g. a nil // must serve its result unchanged: that one line is the untested part.
// 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", handler, err := buildMux(restAuth{token: mcpToken, validator: jwtValidator, realm: "brain",
resourceMetadataURL: resourceMetadataURL}, h, mcpSrv, metrics.New(), public) resourceMetadataURL: resourceMetadataURL}, h, mcpSrv, metrics.New(), public)
if err != nil { if err != nil {
+4 -9
View File
@@ -41,9 +41,8 @@ type restAuth struct {
// wrapped in reg's middleware. Other public handlers are registered as given, // wrapped in reg's middleware. Other public handlers are registered as given,
// and only for patterns on the publicRoutes allowlist. It returns an // and only for patterns on the publicRoutes allowlist. It returns an
// http.Handler, not the *ServeMux, so nothing can register a route on the // http.Handler, not the *ServeMux, so nothing can register a route on the
// result. What stays untested is main's call site: the restAuth fields and // result. What stays untested is main's own line: a handler main built around
// the public map main passes in, and ListenAndServe serving this handler // this one before ListenAndServe would not be seen by these tests.
// unchanged (hyperguild#182). The tests build their own arguments.
func buildMux( func buildMux(
auth restAuth, h *api.Handler, mcpHandler http.Handler, reg *metrics.Registry, public map[string]http.Handler, auth restAuth, h *api.Handler, mcpHandler http.Handler, reg *metrics.Registry, public map[string]http.Handler,
) (http.Handler, error) { ) (http.Handler, error) {
@@ -93,12 +92,8 @@ func mountREST(mux *http.ServeMux, h *api.Handler, auth func(http.Handler) http.
// labelRESTPattern sets the request's Pattern to the REST route it addresses // labelRESTPattern sets the request's Pattern to the REST route it addresses
// before auth runs, so the metrics middleware (which reads Pattern from the // before auth runs, so the metrics middleware (which reads Pattern from the
// same *http.Request) labels a refused request by its route rather than by // same *http.Request) labels a refused request by its route rather than by
// the "/" mount (hyperguild#155). A refused request to a path no route // the "/" mount (hyperguild#155). A path no route matches keeps "/", so the
// matches keeps "/", so for refused requests the label set stays bounded by // label set stays bounded by the registered routes.
// the registered routes. That bound does not hold for authenticated
// requests: an unregistered path reaches the REST mux, which 404s with an
// empty Pattern, and the metrics middleware falls back to the raw URL path
// (hyperguild#185).
func labelRESTPattern(rest *http.ServeMux, next http.Handler) http.Handler { func labelRESTPattern(rest *http.ServeMux, next http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if _, pattern := rest.Handler(r); pattern != "" { if _, pattern := rest.Handler(r); pattern != "" {
@@ -11,41 +11,20 @@ import (
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
) )
// wrongTokSentinel is a presented credential distinctive enough that any
// occurrence in a log line is the raw token, not a coincidence.
const wrongTokSentinel = "wrong-tok-sentinel-q7zx"
// hyperguild#155: since #149 every REST route sits behind BearerMiddleware, // hyperguild#155: since #149 every REST route sits behind BearerMiddleware,
// and chassis v0.2.0 refused without logging, so an unknown caller got // and chassis v0.2.0 refused without logging, so an unknown caller got
// silent 401s. The chassis now writes an `mcp auth rejected` line, and the // silent 401s. The chassis now writes an `mcp auth rejected` line.
// presented token appears nowhere in the log as a bare value (PR #181 func TestBuildMuxLogsARESTAuthRejection(t *testing.T) {
// review: asserting on the "Bearer " header form could never fail, since
// the chassis logs fields, not headers).
func TestBuildMuxRejectionLogLineOmitsTheBarePresentedToken(t *testing.T) {
var buf bytes.Buffer var buf bytes.Buffer
prev := slog.Default() prev := slog.Default()
slog.SetDefault(slog.New(slog.NewTextHandler(&buf, nil))) slog.SetDefault(slog.New(slog.NewTextHandler(&buf, nil)))
t.Cleanup(func() { slog.SetDefault(prev) }) t.Cleanup(func() { slog.SetDefault(prev) })
code, _ := do(t, testMux(t), "POST", "/query", "Bearer "+wrongTokSentinel) code, _ := do(t, testMux(t), "POST", "/query", "Bearer wrong")
require.Equal(t, http.StatusUnauthorized, code) require.Equal(t, http.StatusUnauthorized, code)
assert.Contains(t, buf.String(), "mcp auth rejected") assert.Contains(t, buf.String(), "mcp auth rejected")
assert.Contains(t, buf.String(), "path=/query") assert.Contains(t, buf.String(), "path=/query")
assert.NotContains(t, buf.String(), wrongTokSentinel, "the rejected token is logged as a bare value") assert.NotContains(t, buf.String(), "Bearer wrong", "the credential itself is never logged")
assert.NotContains(t, buf.String(), routesTok, "the valid static token is logged")
}
// The `mcp auth accepted` line leaves out the valid static token.
func TestBuildMuxAcceptedLogLineOmitsTheBareStaticToken(t *testing.T) {
var buf bytes.Buffer
prev := slog.Default()
slog.SetDefault(slog.New(slog.NewTextHandler(&buf, nil)))
t.Cleanup(func() { slog.SetDefault(prev) })
code, _ := do(t, testMux(t), "GET", "/pending", "Bearer "+routesTok)
require.Equal(t, http.StatusOK, code)
assert.Contains(t, buf.String(), "mcp auth accepted")
assert.NotContains(t, buf.String(), routesTok, "the accepted static token is logged as a bare value")
} }
// An authenticated REST request is labelled with the REST mux's pattern: // An authenticated REST request is labelled with the REST mux's pattern:
+7 -10
View File
@@ -22,13 +22,11 @@ func okHandler(body string) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { _, _ = w.Write([]byte(body)) }) return http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { _, _ = w.Write([]byte(body)) })
} }
// testMux builds the router through buildMux, the function main calls, with // testMux builds the router exactly as main does: buildMux with the full
// every allowlisted public pattern (stub handlers), a stub MCP server and a // public set main registers (stub handlers), a stub MCP server, and real
// static token only: no JWT validator and no resourceMetadataURL (the tests // bearer config. hyperguild#149: the REST API is authenticated as a whole;
// in jwt_validator_test.go pass those). It does not use main's own restAuth // the PR #152 review showed a test of a COPY of the router let main.go
// or public map (hyperguild#182). hyperguild#149: the REST API is // regress unseen.
// 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 { func testMux(t *testing.T) http.Handler {
t.Helper() t.Helper()
mux, _ := testMuxAndRegistry(t) mux, _ := testMuxAndRegistry(t)
@@ -89,9 +87,8 @@ func TestBuildMuxAuthenticatesAnUnregisteredPath(t *testing.T) {
} }
// buildMux registers public handlers as given, without adding auth: each // buildMux registers public handlers as given, without adding auth: each
// public route owns its own (bearer/JWT on /capture, HMAC on the webhook, // public route owns its own (bearer on /mcp, HMAC on the webhook, client
// client secret on /oauth/token) or is meant to be open (OAuth metadata). // secret on /oauth/token) or is meant to be open (/metrics, OAuth metadata).
// /mcp and /metrics are not public routes: buildMux builds them itself.
func TestBuildMuxDoesNotWrapPublicRoutes(t *testing.T) { func TestBuildMuxDoesNotWrapPublicRoutes(t *testing.T) {
r := testMux(t) r := testMux(t)
for p := range publicRoutes { for p := range publicRoutes {
@@ -0,0 +1,75 @@
// Package startupretry retries a startup dependency that may not be serving
// yet, instead of exiting on its first failure.
//
// It exists for the cold-boot ordering race (#83): on a cluster restart the
// ingestion pod can reach OIDC discovery before dex is answering, gets a 502
// from ingress, and dies — after having already connected postgres, backfilled
// the graph and ingested session batches, all of which the restart redoes.
// Nothing is misconfigured in that scenario; the pod simply started first.
//
// The budget is bounded on purpose. Retrying forever would turn a genuine
// misconfiguration (wrong issuer URL, bad audience) into a process that looks
// healthy and never serves, which is harder to diagnose than a crash loop.
package startupretry
import (
"context"
"fmt"
"time"
)
// Policy bounds the retry: at most Attempts calls, with the delay doubling
// from Base and capped at Max.
type Policy struct {
Attempts int // total calls, including the first; < 1 means 1
Base time.Duration // delay before the second attempt
Max time.Duration // ceiling for the doubling delay
}
// Do calls fn until it succeeds, the attempt budget is spent, or ctx ends.
//
// The returned error wraps the last failure, so callers keep the diagnosis
// they would have had without the retry.
func Do[T any](ctx context.Context, p Policy, fn func(context.Context) (T, error)) (T, error) {
var zero T
attempts := p.Attempts
if attempts < 1 {
attempts = 1
}
delay := p.Base
var lastErr error
for i := range attempts {
if err := ctx.Err(); err != nil {
return zero, fmt.Errorf("startupretry: %w", err)
}
v, err := fn(ctx)
if err == nil {
return v, nil
}
lastErr = err
if i == attempts-1 {
break
}
if delay <= 0 {
delay = time.Second
}
if p.Max > 0 && delay > p.Max {
delay = p.Max
}
select {
case <-ctx.Done():
return zero, fmt.Errorf("startupretry: %w", ctx.Err())
case <-time.After(delay):
}
delay *= 2
}
return zero, fmt.Errorf("startupretry: gave up after %d attempts: %w", attempts, lastErr)
}
@@ -0,0 +1,142 @@
// ingestion/internal/startupretry/startupretry_test.go
package startupretry
import (
"context"
"errors"
"testing"
"time"
)
func fastPolicy(attempts int) Policy {
return Policy{Attempts: attempts, Base: time.Millisecond, Max: 2 * time.Millisecond}
}
func TestDo_ReturnsImmediatelyOnFirstSuccess(t *testing.T) {
calls := 0
got, err := Do(context.Background(), fastPolicy(5), func(context.Context) (string, error) {
calls++
return "ready", nil
})
if err != nil {
t.Fatalf("Do: %v", err)
}
if got != "ready" {
t.Errorf("got %q, want %q", got, "ready")
}
if calls != 1 {
t.Errorf("calls = %d, want 1", calls)
}
}
func TestDo_SurvivesTransientFailuresThenSucceeds(t *testing.T) {
// The observed cold-boot race: dex is not serving yet, ingress answers
// 502 for the first few attempts, then discovery works.
calls := 0
got, err := Do(context.Background(), fastPolicy(5), func(context.Context) (string, error) {
calls++
if calls < 4 {
return "", errors.New("oidc discovery: status 502")
}
return "ready", nil
})
if err != nil {
t.Fatalf("Do returned %v — a transient 502 must not be fatal", err)
}
if got != "ready" {
t.Errorf("got %q, want %q", got, "ready")
}
if calls != 4 {
t.Errorf("calls = %d, want 4", calls)
}
}
func TestDo_GivesUpAfterTheAttemptBudgetAndReportsTheLastError(t *testing.T) {
// A genuinely broken issuer must still fail — retrying forever would turn
// a config error into a silent hang, which is worse than crash-looping.
last := errors.New("oidc discovery: status 404")
calls := 0
_, err := Do(context.Background(), fastPolicy(3), func(context.Context) (string, error) {
calls++
return "", last
})
if !errors.Is(err, last) {
t.Fatalf("err = %v, want it to wrap the last failure", err)
}
if calls != 3 {
t.Errorf("calls = %d, want 3 (the attempt budget)", calls)
}
}
func TestDo_StopsPromptlyWhenTheContextIsCancelled(t *testing.T) {
ctx, cancel := context.WithCancel(context.Background())
calls := 0
done := make(chan struct{})
var err error
go func() {
defer close(done)
_, err = Do(ctx, Policy{Attempts: 100, Base: 50 * time.Millisecond, Max: time.Second},
func(context.Context) (string, error) {
calls++
if calls == 1 {
cancel()
}
return "", errors.New("still down")
})
}()
select {
case <-done:
case <-time.After(2 * time.Second):
t.Fatal("Do did not return after context cancellation")
}
if !errors.Is(err, context.Canceled) {
t.Errorf("err = %v, want context.Canceled", err)
}
}
func TestDo_BacksOffAndIsCappedByMax(t *testing.T) {
var delays []time.Duration
p := Policy{Attempts: 5, Base: 10 * time.Millisecond, Max: 25 * time.Millisecond}
last := time.Now()
_, _ = Do(context.Background(), p, func(context.Context) (string, error) {
now := time.Now()
delays = append(delays, now.Sub(last))
last = now
return "", errors.New("down")
})
// delays[0] is the first call (no wait); the rest are the sleeps.
if len(delays) != 5 {
t.Fatalf("attempts = %d, want 5", len(delays))
}
if delays[1] >= delays[2] {
t.Errorf("backoff is not growing: %v then %v", delays[1], delays[2])
}
for i, d := range delays[1:] {
if d > 2*p.Max {
t.Errorf("delay %d = %v, exceeds the Max cap %v with slack", i+1, d, p.Max)
}
}
}
func TestPolicy_ZeroValueStillMakesOneAttempt(t *testing.T) {
// A caller that forgets to configure the policy must not silently get a
// no-op that returns a nil validator and a nil error.
calls := 0
_, err := Do(context.Background(), Policy{}, func(context.Context) (string, error) {
calls++
return "", errors.New("down")
})
if calls != 1 {
t.Errorf("calls = %d, want 1", calls)
}
if err == nil {
t.Error("err = nil, want the failure surfaced")
}
}