fix(ingestion): bump mcp-chassis to v0.7.1; label refused REST requests by route
CI / Lint / Test / Vet (pull_request) Successful in 54s
CI / Mirror to GitHub (pull_request) Skipped

1. Rejections are logged. chassis v0.2.0 refused without a log line, so
   since #149 an unknown REST caller got silent 401s. v0.7.1 writes
   `mcp auth rejected` (reason, remote, token fingerprint, path) and
   `mcp auth accepted` per authenticated request. Tested through
   buildMux.

2. Fallout: since chassis v0.7 (mcp-chassis#4) NewJWTValidator no longer
   fails on OIDC discovery; the issuer resolves lazily and a JWT gets
   503-and-retry until it does. #83's boot retry loop therefore never
   retried, and its tests failed. buildJWTValidator now builds once and
   logs an issuer still unresolved at boot. Behaviour change: a
   misconfigured issuer no longer exits the process; JWTs get 503, the
   static bearer keeps working. internal/startupretry is now unused.

3. Metrics: authenticated REST requests were already labelled by route
   (the REST mux sets Pattern on the same *http.Request the middleware
   reads). Requests refused by auth never reached the REST mux and were
   labelled path="/". labelRESTPattern now sets the REST route's pattern
   before auth; unmatched paths keep "/", so the label set stays bounded.

Closes #155

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
2026-10-05 09:38:41 +02:00
co-authored by Claude Opus 5.5
parent 0bd0fbf03e
commit 474473c99f
8 changed files with 149 additions and 73 deletions
+13 -12
View File
@@ -2,24 +2,25 @@ package main
import ( import (
"context" "context"
"fmt"
"log/slog" "log/slog"
chassisauth "git.d-ma.be/mathias/mcp-chassis/auth" chassisauth "git.d-ma.be/mathias/mcp-chassis/auth"
"github.com/mathiasbq/hyperguild/ingestion/internal/startupretry"
) )
// buildJWTValidator runs OIDC discovery under policy, so a 502 from an auth // buildJWTValidator builds the JWT validator for issuer (nil when issuer is
// upstream that is not serving yet on a cold boot is retried instead of // empty: JWT auth disabled). Since mcp-chassis v0.7 construction no longer
// exiting (#83). It honours ctx between attempts. // fails when OIDC discovery does (mcp-chassis#4): the issuer resolves lazily,
func buildJWTValidator( // and until it does a JWT is answered 503-and-retry, not 401. That replaces
ctx context.Context, logger *slog.Logger, issuer, audience string, policy startupretry.Policy, // #83's boot-time retry loop (hyperguild#155). An issuer still unresolved at
) (*chassisauth.JWTValidator, error) { // boot is logged, so a misconfigured issuer is visible rather than silent.
return startupretry.Do(ctx, policy, func(ctx context.Context) (*chassisauth.JWTValidator, error) { func buildJWTValidator(ctx context.Context, logger *slog.Logger, issuer, audience string) (*chassisauth.JWTValidator, error) {
v, err := chassisauth.NewJWTValidator(ctx, issuer, audience) v, err := chassisauth.NewJWTValidator(ctx, issuer, audience)
if err != nil { if err != nil {
logger.Warn("oidc discovery not ready, retrying", "err", err) return nil, fmt.Errorf("build jwt validator: %w", err)
} }
return v, err if ok, rerr := v.Ready(ctx); !ok {
}) logger.Warn("jwt issuer not resolved at boot; JWTs answer 503 until it is", "issuer", issuer, "err", rerr)
}
return v, nil
} }
+32 -38
View File
@@ -1,6 +1,7 @@
package main package main
import ( import (
"bytes"
"context" "context"
"io" "io"
"log/slog" "log/slog"
@@ -8,12 +9,9 @@ import (
"net/http/httptest" "net/http/httptest"
"sync/atomic" "sync/atomic"
"testing" "testing"
"time"
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
"github.com/mathiasbq/hyperguild/ingestion/internal/startupretry"
) )
// fakeIssuer answers OIDC discovery with 502 for the first fail requests, as // fakeIssuer answers OIDC discovery with 502 for the first fail requests, as
@@ -41,46 +39,42 @@ func fakeIssuer(t *testing.T, fail int32) (*httptest.Server, *atomic.Int32) {
return srv, &discoveries return srv, &discoveries
} }
var fastPolicy = startupretry.Policy{Attempts: 8, Base: time.Millisecond, Max: 5 * time.Millisecond} // #83 retried discovery at boot because chassis v0.2.0 failed construction on
// a 502. Since chassis v0.7 (mcp-chassis#4) the validator is built anyway and
func TestBuildJWTValidatorRetriesDiscovery502sThenSucceeds(t *testing.T) { // resolves the issuer lazily, so boot no longer waits or exits on it
srv, discoveries := fakeIssuer(t, 3) // (hyperguild#155). An unresolved issuer is logged at boot.
ctx, cancel := context.WithCancel(context.Background()) func TestBuildJWTValidatorReturnsAValidatorWhileDiscovery502s(t *testing.T) {
defer cancel() srv, _ := fakeIssuer(t, 1000)
var buf bytes.Buffer
v, err := buildJWTValidator(ctx, slog.New(slog.DiscardHandler), srv.URL, "brain", fastPolicy) v, err := buildJWTValidator(context.Background(), slog.New(slog.NewTextHandler(&buf, nil)), srv.URL, "brain")
require.NoError(t, err) require.NoError(t, err)
assert.NotNil(t, v) assert.NotNil(t, v)
assert.Equal(t, int32(4), discoveries.Load(), "three 502s, then the successful discovery") assert.Contains(t, buf.String(), "jwt issuer not resolved at boot")
} }
func TestBuildJWTValidatorGivesUpAfterTheBudgetWithTheLastError(t *testing.T) { func TestBuildJWTValidatorLogsNothingAgainstAHealthyIssuer(t *testing.T) {
srv, discoveries := fakeIssuer(t, 1000) srv, _ := fakeIssuer(t, 0)
ctx, cancel := context.WithCancel(context.Background()) var buf bytes.Buffer
defer cancel() v, err := buildJWTValidator(context.Background(), slog.New(slog.NewTextHandler(&buf, nil)), srv.URL, "brain")
require.NoError(t, err)
_, err := buildJWTValidator(ctx, slog.New(slog.DiscardHandler), srv.URL, "brain", fastPolicy) assert.NotNil(t, v)
require.Error(t, err) assert.NotContains(t, buf.String(), "not resolved")
assert.Contains(t, err.Error(), "status 502")
assert.Equal(t, int32(8), discoveries.Load(), "bounded: exactly the attempt budget")
} }
func TestBuildJWTValidatorStopsRetryingWhenTheContextIsCancelled(t *testing.T) { func TestBuildJWTValidatorIsNilWithoutAnIssuer(t *testing.T) {
v, err := buildJWTValidator(context.Background(), slog.New(slog.DiscardHandler), "", "brain")
require.NoError(t, err)
assert.Nil(t, v)
}
// With the issuer unresolved, a JWT on a REST route answers 503 (retry), not
// 401: the token may be fine, the server just cannot check it yet.
func TestBuildMuxAnswers503ToAJWTWhileTheIssuerIsUnresolved(t *testing.T) {
srv, _ := fakeIssuer(t, 1000) srv, _ := fakeIssuer(t, 1000)
ctx, cancel := context.WithCancel(context.Background()) v, err := buildJWTValidator(context.Background(), slog.New(slog.DiscardHandler), srv.URL, "brain")
slow := startupretry.Policy{Attempts: 8, Base: time.Hour, Max: time.Hour} require.NoError(t, err)
mux, err := buildMux(restAuth{token: routesTok, validator: v, realm: "brain"}, testHandler(t), nil)
done := make(chan error, 1) require.NoError(t, err)
go func() { code, _ := do(t, mux, "GET", "/pending", "Bearer aaa.bbb.ccc")
_, err := buildJWTValidator(ctx, slog.New(slog.DiscardHandler), srv.URL, "brain", slow) assert.Equal(t, http.StatusServiceUnavailable, code)
done <- err
}()
time.Sleep(50 * time.Millisecond)
cancel()
select {
case err := <-done:
assert.ErrorIs(t, err, context.Canceled)
case <-time.After(3 * time.Second):
t.Fatal("did not return after the context was cancelled")
}
} }
+8 -15
View File
@@ -32,7 +32,6 @@ import (
"github.com/mathiasbq/hyperguild/ingestion/internal/pipeline" "github.com/mathiasbq/hyperguild/ingestion/internal/pipeline"
"github.com/mathiasbq/hyperguild/ingestion/internal/reranker" "github.com/mathiasbq/hyperguild/ingestion/internal/reranker"
"github.com/mathiasbq/hyperguild/ingestion/internal/search" "github.com/mathiasbq/hyperguild/ingestion/internal/search"
"github.com/mathiasbq/hyperguild/ingestion/internal/startupretry"
"github.com/mathiasbq/hyperguild/ingestion/internal/symlinkguard" "github.com/mathiasbq/hyperguild/ingestion/internal/symlinkguard"
"github.com/mathiasbq/hyperguild/ingestion/internal/vectorstore" "github.com/mathiasbq/hyperguild/ingestion/internal/vectorstore"
"github.com/mathiasbq/hyperguild/ingestion/internal/watcher" "github.com/mathiasbq/hyperguild/ingestion/internal/watcher"
@@ -369,21 +368,15 @@ func main() {
// hyperguild#157: report a symlink that appears in the brain while running. // hyperguild#157: report a symlink that appears in the brain while running.
go symlinkguard.Watch(ctx, brainDir, symlinkScanInterval(os.Getenv), buildNotifier()) go symlinkguard.Watch(ctx, brainDir, symlinkScanInterval(os.Getenv), buildNotifier())
// Built before the watcher, the graph backfill and session ingestion, and // Never fatal on an unreachable issuer (#83): on a cold cluster boot this
// retried rather than fatal (#83). Two separate reasons: // pod can beat dex to serving and get a 502 from ingress, and exiting meant
// // CrashLoopBackOff redoing the backfill. Since mcp-chassis v0.7 the
// - On a cold cluster boot this pod can beat dex to serving and gets a // validator resolves the issuer lazily and answers JWTs 503-and-retry until
// 502 from ingress. Nothing is misconfigured; it simply started first. // it does (hyperguild#155), so no boot retry is needed. A misconfigured
// Exiting there meant CrashLoopBackOff redoing the backfill on every // issuer no longer exits the process: it is logged at boot and every JWT
// attempt, with exponential backoff stretching the brain outage. // gets 503; the static bearer keeps working.
// - Constructing it first means a genuine config error (wrong issuer,
// bad audience) fails in seconds instead of after all that work.
//
// The budget stays bounded: retrying forever would turn a misconfiguration
// into a process that looks alive and never serves.
jwtValidator, err := buildJWTValidator(ctx, logger, jwtValidator, err := buildJWTValidator(ctx, logger,
os.Getenv("DEX_ISSUER_URL"), os.Getenv("MCP_AUDIENCE"), os.Getenv("DEX_ISSUER_URL"), os.Getenv("MCP_AUDIENCE"))
startupretry.Policy{Attempts: 8, Base: time.Second, Max: 30 * time.Second})
if err != nil { if err != nil {
logger.Error("build jwt validator", "err", err) logger.Error("build jwt validator", "err", err)
os.Exit(1) os.Exit(1)
+15 -1
View File
@@ -73,5 +73,19 @@ func mountREST(mux *http.ServeMux, h *api.Handler, auth func(http.Handler) http.
rest.HandleFunc("POST /promote", h.Promote) rest.HandleFunc("POST /promote", h.Promote)
rest.HandleFunc("POST /backfill-embeddings", h.BackfillEmbeddings) rest.HandleFunc("POST /backfill-embeddings", h.BackfillEmbeddings)
rest.HandleFunc("GET /pass-rate", h.PassRate) rest.HandleFunc("GET /pass-rate", h.PassRate)
mux.Handle("/", auth(rest)) mux.Handle("/", labelRESTPattern(rest, auth(rest)))
}
// labelRESTPattern sets the request's Pattern to the REST route it addresses
// 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
// the "/" mount (hyperguild#155). A path no route matches keeps "/", so the
// label set stays bounded by the registered routes.
func labelRESTPattern(rest *http.ServeMux, next http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if _, pattern := rest.Handler(r); pattern != "" {
r.Pattern = pattern
}
next.ServeHTTP(w, r)
})
} }
@@ -0,0 +1,69 @@
package main
import (
"bytes"
"log/slog"
"net/http"
"net/http/httptest"
"testing"
"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,
// and chassis v0.2.0 refused without logging, so an unknown caller got
// silent 401s. The chassis now writes an `mcp auth rejected` line.
func TestBuildMuxLogsARESTAuthRejection(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), "POST", "/query", "Bearer wrong")
require.Equal(t, http.StatusUnauthorized, code)
assert.Contains(t, buf.String(), "mcp auth rejected")
assert.Contains(t, buf.String(), "path=/query")
assert.NotContains(t, buf.String(), "Bearer wrong", "the credential itself is never logged")
}
// 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)
require.Equal(t, http.StatusOK, code)
rec := httptest.NewRecorder()
reg.Handler().ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/metrics", nil))
assert.Contains(t, rec.Body.String(), `path="GET /pending"`)
assert.NotContains(t, rec.Body.String(), `path="/"`)
}
// hyperguild#155: a request refused by REST auth never reaches the REST mux,
// 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", "")
require.Equal(t, http.StatusUnauthorized, code)
rec := httptest.NewRecorder()
reg.Handler().ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/metrics", nil))
assert.Contains(t, rec.Body.String(), `path="POST /query",status="4xx"`)
}
// 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", "")
require.Equal(t, http.StatusUnauthorized, code)
rec := httptest.NewRecorder()
reg.Handler().ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/metrics", nil))
assert.Contains(t, rec.Body.String(), `path="/",status="4xx"`)
assert.NotContains(t, rec.Body.String(), "no-such-route-zq")
}
+6 -1
View File
@@ -27,7 +27,7 @@ func okHandler(body string) http.Handler {
// review showed a test of a COPY of the router let main.go regress unseen. // 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()
h := api.NewHandler(t.TempDir(), slog.New(slog.NewTextHandler(os.Stderr, nil)), pipeline.Config{}) h := testHandler(t)
public := map[string]http.Handler{} public := map[string]http.Handler{}
for p := range publicRoutes { for p := range publicRoutes {
public[p] = okHandler("public " + p) public[p] = okHandler("public " + p)
@@ -37,6 +37,11 @@ func testMux(t *testing.T) http.Handler {
return mux return mux
} }
func testHandler(t *testing.T) *api.Handler {
t.Helper()
return api.NewHandler(t.TempDir(), slog.New(slog.NewTextHandler(os.Stderr, nil)), pipeline.Config{})
}
func do(t *testing.T, h http.Handler, method, path, authz string) (int, string) { func do(t *testing.T, h http.Handler, method, path, authz string) (int, string) {
t.Helper() t.Helper()
req := httptest.NewRequest(method, path, strings.NewReader(`{"query":"x"}`)) req := httptest.NewRequest(method, path, strings.NewReader(`{"query":"x"}`))
+1 -1
View File
@@ -51,7 +51,7 @@ require (
) )
require ( require (
git.d-ma.be/mathias/mcp-chassis v0.2.0 git.d-ma.be/mathias/mcp-chassis v0.7.1
github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect
github.com/decred/dcrd/dcrec/secp256k1/v4 v4.4.0 // indirect github.com/decred/dcrd/dcrec/secp256k1/v4 v4.4.0 // indirect
github.com/goccy/go-json v0.10.3 // indirect github.com/goccy/go-json v0.10.3 // indirect
+2 -2
View File
@@ -1,5 +1,5 @@
git.d-ma.be/mathias/mcp-chassis v0.2.0 h1:6fLmb7xqRa2nNVWsHaUbbfbArgDXJw/gDhb09clBIjo= git.d-ma.be/mathias/mcp-chassis v0.7.1 h1:4h8TOBHTA0L/no4IPcr4tLcQtmKUlhE2/cXyPdNFGmM=
git.d-ma.be/mathias/mcp-chassis v0.2.0/go.mod h1:Ks7EK2UnGAN0H3rJjKUxUagX8/ZBdtLrOlcUbv0RwH8= git.d-ma.be/mathias/mcp-chassis v0.7.1/go.mod h1:Ks7EK2UnGAN0H3rJjKUxUagX8/ZBdtLrOlcUbv0RwH8=
github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E= github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E=
github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=