Compare commits
2
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
a1e036168a | ||
|
|
474473c99f |
@@ -2,24 +2,25 @@ package main
|
||||
|
||||
import (
|
||||
"context"
|
||||
"fmt"
|
||||
"log/slog"
|
||||
|
||||
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
|
||||
// upstream that is not serving yet on a cold boot is retried instead of
|
||||
// exiting (#83). It honours ctx between attempts.
|
||||
func buildJWTValidator(
|
||||
ctx context.Context, logger *slog.Logger, issuer, audience string, policy startupretry.Policy,
|
||||
) (*chassisauth.JWTValidator, error) {
|
||||
return startupretry.Do(ctx, policy, func(ctx context.Context) (*chassisauth.JWTValidator, error) {
|
||||
v, err := chassisauth.NewJWTValidator(ctx, issuer, audience)
|
||||
if err != nil {
|
||||
logger.Warn("oidc discovery not ready, retrying", "err", err)
|
||||
}
|
||||
return v, err
|
||||
})
|
||||
// buildJWTValidator builds the JWT validator for issuer (nil when issuer is
|
||||
// empty: JWT auth disabled). Since mcp-chassis v0.7 construction no longer
|
||||
// fails when OIDC discovery does (mcp-chassis#4): the issuer resolves lazily,
|
||||
// and until it does a JWT is answered 503-and-retry, not 401. That replaces
|
||||
// #83's boot-time retry loop (hyperguild#155). An issuer still unresolved at
|
||||
// boot is logged, so a misconfigured issuer is visible rather than silent.
|
||||
func buildJWTValidator(ctx context.Context, logger *slog.Logger, issuer, audience string) (*chassisauth.JWTValidator, error) {
|
||||
v, err := chassisauth.NewJWTValidator(ctx, issuer, audience)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("build jwt validator: %w", 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
|
||||
}
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
package main
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"context"
|
||||
"io"
|
||||
"log/slog"
|
||||
@@ -8,12 +9,11 @@ import (
|
||||
"net/http/httptest"
|
||||
"sync/atomic"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
|
||||
"github.com/mathiasbq/hyperguild/ingestion/internal/startupretry"
|
||||
"github.com/mathiasbq/hyperguild/ingestion/internal/metrics"
|
||||
)
|
||||
|
||||
// fakeIssuer answers OIDC discovery with 502 for the first fail requests, as
|
||||
@@ -41,46 +41,43 @@ func fakeIssuer(t *testing.T, fail int32) (*httptest.Server, *atomic.Int32) {
|
||||
return srv, &discoveries
|
||||
}
|
||||
|
||||
var fastPolicy = startupretry.Policy{Attempts: 8, Base: time.Millisecond, Max: 5 * time.Millisecond}
|
||||
|
||||
func TestBuildJWTValidatorRetriesDiscovery502sThenSucceeds(t *testing.T) {
|
||||
srv, discoveries := fakeIssuer(t, 3)
|
||||
ctx, cancel := context.WithCancel(context.Background())
|
||||
defer cancel()
|
||||
|
||||
v, err := buildJWTValidator(ctx, slog.New(slog.DiscardHandler), srv.URL, "brain", fastPolicy)
|
||||
// #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
|
||||
// resolves the issuer lazily, so boot no longer waits or exits on it
|
||||
// (hyperguild#155). An unresolved issuer is logged at boot.
|
||||
func TestBuildJWTValidatorReturnsAValidatorWhileDiscovery502s(t *testing.T) {
|
||||
srv, _ := fakeIssuer(t, 1000)
|
||||
var buf bytes.Buffer
|
||||
v, err := buildJWTValidator(context.Background(), slog.New(slog.NewTextHandler(&buf, nil)), srv.URL, "brain")
|
||||
require.NoError(t, err)
|
||||
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) {
|
||||
srv, discoveries := fakeIssuer(t, 1000)
|
||||
ctx, cancel := context.WithCancel(context.Background())
|
||||
defer cancel()
|
||||
|
||||
_, err := buildJWTValidator(ctx, slog.New(slog.DiscardHandler), srv.URL, "brain", fastPolicy)
|
||||
require.Error(t, err)
|
||||
assert.Contains(t, err.Error(), "status 502")
|
||||
assert.Equal(t, int32(8), discoveries.Load(), "bounded: exactly the attempt budget")
|
||||
func TestBuildJWTValidatorLogsNothingAgainstAHealthyIssuer(t *testing.T) {
|
||||
srv, _ := fakeIssuer(t, 0)
|
||||
var buf bytes.Buffer
|
||||
v, err := buildJWTValidator(context.Background(), slog.New(slog.NewTextHandler(&buf, nil)), srv.URL, "brain")
|
||||
require.NoError(t, err)
|
||||
assert.NotNil(t, v)
|
||||
assert.NotContains(t, buf.String(), "not resolved")
|
||||
}
|
||||
|
||||
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)
|
||||
ctx, cancel := context.WithCancel(context.Background())
|
||||
slow := startupretry.Policy{Attempts: 8, Base: time.Hour, Max: time.Hour}
|
||||
|
||||
done := make(chan error, 1)
|
||||
go func() {
|
||||
_, err := buildJWTValidator(ctx, slog.New(slog.DiscardHandler), srv.URL, "brain", slow)
|
||||
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")
|
||||
}
|
||||
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, "GET", "/pending", "Bearer aaa.bbb.ccc")
|
||||
assert.Equal(t, http.StatusServiceUnavailable, code)
|
||||
}
|
||||
|
||||
@@ -32,7 +32,6 @@ import (
|
||||
"github.com/mathiasbq/hyperguild/ingestion/internal/pipeline"
|
||||
"github.com/mathiasbq/hyperguild/ingestion/internal/reranker"
|
||||
"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/vectorstore"
|
||||
"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.
|
||||
go symlinkguard.Watch(ctx, brainDir, symlinkScanInterval(os.Getenv), buildNotifier())
|
||||
|
||||
// Built before the watcher, the graph backfill and session ingestion, and
|
||||
// retried rather than fatal (#83). Two separate reasons:
|
||||
//
|
||||
// - On a cold cluster boot this pod can beat dex to serving and gets a
|
||||
// 502 from ingress. Nothing is misconfigured; it simply started first.
|
||||
// Exiting there meant CrashLoopBackOff redoing the backfill on every
|
||||
// attempt, with exponential backoff stretching the brain outage.
|
||||
// - 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.
|
||||
// Never fatal on an unreachable issuer (#83): on a cold cluster boot this
|
||||
// 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
|
||||
// validator resolves the issuer lazily and answers JWTs 503-and-retry until
|
||||
// it does (hyperguild#155), so no boot retry is needed. A misconfigured
|
||||
// issuer no longer exits the process: it is logged at boot and every JWT
|
||||
// gets 503; the static bearer keeps working.
|
||||
jwtValidator, err := buildJWTValidator(ctx, logger,
|
||||
os.Getenv("DEX_ISSUER_URL"), os.Getenv("MCP_AUDIENCE"),
|
||||
startupretry.Policy{Attempts: 8, Base: time.Second, Max: 30 * time.Second})
|
||||
os.Getenv("DEX_ISSUER_URL"), os.Getenv("MCP_AUDIENCE"))
|
||||
if err != nil {
|
||||
logger.Error("build jwt validator", "err", err)
|
||||
os.Exit(1)
|
||||
@@ -523,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
|
||||
@@ -548,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
|
||||
@@ -590,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)
|
||||
@@ -623,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)
|
||||
}
|
||||
|
||||
@@ -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
|
||||
@@ -73,5 +86,19 @@ func mountREST(mux *http.ServeMux, h *api.Handler, auth func(http.Handler) http.
|
||||
rest.HandleFunc("POST /promote", h.Promote)
|
||||
rest.HandleFunc("POST /backfill-embeddings", h.BackfillEmbeddings)
|
||||
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,67 @@
|
||||
package main
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"log/slog"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
// 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) {
|
||||
mux, reg := testMuxAndRegistry(t)
|
||||
code, _ := do(t, mux, "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) {
|
||||
mux, reg := testMuxAndRegistry(t)
|
||||
code, _ := do(t, mux, "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) {
|
||||
mux, reg := testMuxAndRegistry(t)
|
||||
code, _ := do(t, mux, "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")
|
||||
}
|
||||
@@ -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,31 @@ 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 := api.NewHandler(t.TempDir(), slog.New(slog.NewTextHandler(os.Stderr, nil)), pipeline.Config{})
|
||||
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 {
|
||||
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) {
|
||||
@@ -93,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"`)
|
||||
}
|
||||
|
||||
+1
-1
@@ -51,7 +51,7 @@ 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/decred/dcrd/dcrec/secp256k1/v4 v4.4.0 // indirect
|
||||
github.com/goccy/go-json v0.10.3 // indirect
|
||||
|
||||
+2
-2
@@ -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.2.0/go.mod h1:Ks7EK2UnGAN0H3rJjKUxUagX8/ZBdtLrOlcUbv0RwH8=
|
||||
git.d-ma.be/mathias/mcp-chassis v0.7.1 h1:4h8TOBHTA0L/no4IPcr4tLcQtmKUlhE2/cXyPdNFGmM=
|
||||
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/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=
|
||||
|
||||
@@ -23,13 +23,6 @@ func TestReadNoteRefusesSymlinksAndSurvivesSwapRaces(t *testing.T) {
|
||||
_, body, _, err := api.ReadNote(brain, "wiki/w/facts/leak.md")
|
||||
assert.Error(t, err)
|
||||
assert.NotContains(t, body, brainfstest.Sentinel)
|
||||
// A link to a file elsewhere in the brain: os.Root allows it, only the
|
||||
// regular-file check refuses it (hyperguild#171).
|
||||
brainfstest.WriteSentinelAt(t, filepath.Join(brain, "raw", "s.md"))
|
||||
require.NoError(t, os.Symlink(filepath.Join("..", "..", "..", "raw", "s.md"), filepath.Join(facts, "inlink.md")))
|
||||
_, body, _, err = api.ReadNote(brain, "wiki/w/facts/inlink.md")
|
||||
assert.Error(t, err)
|
||||
assert.NotContains(t, body, brainfstest.Sentinel)
|
||||
|
||||
p := filepath.Join(facts, "r.md")
|
||||
require.NoError(t, os.WriteFile(p, []byte("---\nx: y\n---\ninside"), 0o644))
|
||||
@@ -46,7 +39,5 @@ func TestReadNoteRefusesSymlinksAndSurvivesSwapRaces(t *testing.T) {
|
||||
return s
|
||||
}
|
||||
brainfstest.AssertNoLeak(t, p, "---\nx: y\n---\ninside", 1500*time.Millisecond, read)
|
||||
inBrain := brainfstest.WriteSentinelAt(t, filepath.Join(brain, "raw", "s.md"))
|
||||
brainfstest.AssertNoLeakFrom(t, p, "---\nx: y\n---\ninside", inBrain, 1500*time.Millisecond, read)
|
||||
brainfstest.AssertNoHang(t, p, "---\nx: y\n---\ninside", 1500*time.Millisecond, 6*time.Second, func() { _ = read() })
|
||||
}
|
||||
|
||||
@@ -218,7 +218,7 @@ func TestReadDirRegularListsOnlyRegularFilesAndRefusesSymlinkedComponents(t *tes
|
||||
// A final component swapped for a symlink to another regular file in the
|
||||
// brain between the Lstat and the open: os.Root follows in-root links, so
|
||||
// only comparing the two (os.SameFile) refuses it.
|
||||
func TestReadRegularDoesNotReturnAnInBrainFileSwappedInBehindASymlink(t *testing.T) {
|
||||
func TestReadRegularNeverReturnsAnInBrainFileSwappedInBehindASymlink(t *testing.T) {
|
||||
dir := t.TempDir()
|
||||
require.NoError(t, os.WriteFile(filepath.Join(dir, "s.md"), []byte("zqsamefilesentinel"), 0o644))
|
||||
p := filepath.Join(dir, "r.md")
|
||||
|
||||
@@ -1,10 +1,8 @@
|
||||
// Package brainfstest is the shared harness for hyperguild#157's swap-race
|
||||
// tests: it keeps swapping a regular note, a symlink to a sentinel target file
|
||||
// (outside the brain, or inside it for hyperguild#171), or a FIFO (with a
|
||||
// writer held open) into a path while a reader under test loops.
|
||||
// It observes two things, over a sampled window only: no sentinel byte from
|
||||
// the target reaches the reader's output, and the reader returns within a
|
||||
// timeout. It does not cover
|
||||
// tests: it keeps swapping a regular note, a symlink to an outside file, or a
|
||||
// FIFO (with a writer held open) into a path while a reader under test loops.
|
||||
// It observes two things: no sentinel byte from the outside file reaches the
|
||||
// reader's output, and the reader returns within a timeout. It does not cover
|
||||
// hard links, other paths into the reader, or races it does not generate.
|
||||
package brainfstest
|
||||
|
||||
@@ -20,7 +18,7 @@ import (
|
||||
"time"
|
||||
)
|
||||
|
||||
// Sentinel marks the target file's contents.
|
||||
// Sentinel marks the outside file's contents.
|
||||
const Sentinel = "zqbrainfssentinelonefiftyseven"
|
||||
|
||||
// outsideContent carries Sentinel in every field a reader might extract: the
|
||||
@@ -143,7 +141,7 @@ func AssertNoLeakFrom(t testing.TB, path, inside, target string, d time.Duration
|
||||
}
|
||||
}
|
||||
if leaks > 0 {
|
||||
t.Fatalf("symlink target %s leaked %d times in %d iterations", target, leaks, iters)
|
||||
t.Fatalf("outside file leaked %d times in %d iterations", leaks, iters)
|
||||
}
|
||||
if n := swaps.Load(); n == 0 {
|
||||
t.Fatalf("no symlink was swapped into %s in %d iterations: the race did not run", path, iters)
|
||||
|
||||
@@ -21,11 +21,6 @@ func TestReadExcerptRefusesSymlinksAndSurvivesSwapRaces(t *testing.T) {
|
||||
require.NoError(t, os.MkdirAll(facts, 0o755))
|
||||
require.NoError(t, os.Symlink(brainfstest.Outside(t), filepath.Join(facts, "leak.md")))
|
||||
assert.NotContains(t, readExcerpt(brain, "wiki/w/facts/leak.md", 500), brainfstest.Sentinel)
|
||||
// A link to a file elsewhere in the brain: os.Root allows it, only the
|
||||
// regular-file check refuses it (hyperguild#171).
|
||||
inBrain := brainfstest.WriteSentinelAt(t, filepath.Join(brain, "raw", "s.md"))
|
||||
require.NoError(t, os.Symlink(filepath.Join("..", "..", "..", "raw", "s.md"), filepath.Join(facts, "inlink.md")))
|
||||
assert.NotContains(t, readExcerpt(brain, "wiki/w/facts/inlink.md", 500), brainfstest.Sentinel)
|
||||
|
||||
p := filepath.Join(facts, "r.md")
|
||||
require.NoError(t, os.WriteFile(p, []byte("---\nx: y\n---\ninside"), 0o644))
|
||||
@@ -33,9 +28,6 @@ func TestReadExcerptRefusesSymlinksAndSurvivesSwapRaces(t *testing.T) {
|
||||
brainfstest.AssertNoLeak(t, p, "---\nx: y\n---\ninside", 1500*time.Millisecond, func() string {
|
||||
return readExcerpt(brain, "wiki/w/facts/r.md", 500)
|
||||
})
|
||||
brainfstest.AssertNoLeakFrom(t, p, "---\nx: y\n---\ninside", inBrain, 1500*time.Millisecond, func() string {
|
||||
return readExcerpt(brain, "wiki/w/facts/r.md", 500)
|
||||
})
|
||||
brainfstest.AssertNoHang(t, p, "---\nx: y\n---\ninside", 1500*time.Millisecond, 6*time.Second, func() {
|
||||
_ = readExcerpt(brain, "wiki/w/facts/r.md", 500)
|
||||
})
|
||||
|
||||
@@ -38,31 +38,3 @@ func TestReadScopedRefusesANoteOverMaxNoteBytes(t *testing.T) {
|
||||
_, err := ReadScoped(brain, "wiki/w/facts/big.md", ScopeNotes)
|
||||
assert.ErrorIs(t, err, ErrTooLarge)
|
||||
}
|
||||
|
||||
// hyperguild#171 (review of #172, finding 1): os.Root alone refuses links that
|
||||
// leave the brain, so the outside-target race above stays green for a reader
|
||||
// that skips brainfs's Lstat and SameFile checks. An in-brain target (raw/,
|
||||
// outside ScopeNotes) is what tells them apart.
|
||||
func TestReadScopedDoesNotReturnAnInBrainSymlinkTargetStaticallyOrDuringASampledSwapRace(t *testing.T) {
|
||||
brain := t.TempDir()
|
||||
facts := filepath.Join(brain, "wiki", "w", "facts")
|
||||
require.NoError(t, os.MkdirAll(facts, 0o755))
|
||||
target := brainfstest.WriteSentinelAt(t, filepath.Join(brain, "raw", "s.md"))
|
||||
|
||||
require.NoError(t, os.Symlink(filepath.Join("..", "..", "..", "raw", "s.md"), filepath.Join(facts, "inlink.md")))
|
||||
b, err := ReadScoped(brain, "wiki/w/facts/inlink.md", ScopeNotes)
|
||||
assert.Error(t, err, "static in-brain link")
|
||||
assert.NotContains(t, string(b), brainfstest.Sentinel, "static in-brain link")
|
||||
|
||||
p := filepath.Join(facts, "r.md")
|
||||
require.NoError(t, os.WriteFile(p, []byte("inside"), 0o644))
|
||||
b, err = ReadScoped(brain, "wiki/w/facts/r.md", ScopeNotes)
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, "inside", string(b), "positive control: a regular note reads")
|
||||
|
||||
read := func() string {
|
||||
b, _ := ReadScoped(brain, "wiki/w/facts/r.md", ScopeNotes)
|
||||
return string(b)
|
||||
}
|
||||
brainfstest.AssertNoLeakFrom(t, p, "inside", target, 1500*time.Millisecond, read)
|
||||
}
|
||||
|
||||
@@ -12,8 +12,6 @@ import (
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
|
||||
"github.com/mathiasbq/hyperguild/ingestion/internal/brainfs/brainfstest"
|
||||
)
|
||||
|
||||
// hyperguild#157 option 1: search walked and read the brain by path, so a
|
||||
@@ -191,49 +189,3 @@ func TestHydrateRefusesASymlink(t *testing.T) {
|
||||
assert.Error(t, err)
|
||||
assert.NotContains(t, r.Excerpt, sentinel157)
|
||||
}
|
||||
|
||||
// hyperguild#171: os.Root refuses links that leave the brain on its own, so
|
||||
// the outside-target cases above would stay green for a caller that read
|
||||
// without brainfs's Lstat and SameFile checks. These targets are inside the
|
||||
// brain (raw/, which is not a search root).
|
||||
func inBrainTarget(t *testing.T, brain string) string {
|
||||
t.Helper()
|
||||
return brainfstest.WriteSentinelAt(t, filepath.Join(brain, "raw", "s.md"))
|
||||
}
|
||||
|
||||
func TestQueryDoesNotReturnAnInBrainSymlinkTargetDuringASampledSwapRace(t *testing.T) {
|
||||
brain, facts, _ := searchBrain(t)
|
||||
target := inBrainTarget(t, brain)
|
||||
read := func() string {
|
||||
// Both terms, so the regular note is a possible hit: a reader that
|
||||
// never returns the inside note would pass vacuously (#177 floor).
|
||||
rs, _ := Query(brain, QueryOptions{Query: brainfstest.Sentinel + " zqinside", Limit: 10})
|
||||
s := ""
|
||||
for _, r := range rs {
|
||||
s += r.Title + r.Excerpt
|
||||
}
|
||||
return s
|
||||
}
|
||||
brainfstest.AssertNoLeakFrom(t, filepath.Join(facts, "r.md"), "# r\n\nzqinside", target, 1500*time.Millisecond, read)
|
||||
require.NoError(t, os.Symlink(filepath.Join("..", "..", "..", "raw", "s.md"), filepath.Join(facts, "inlink.md")))
|
||||
assert.NotContains(t, read(), brainfstest.Sentinel, "static in-brain link")
|
||||
}
|
||||
|
||||
func TestHydrateRefusesAnInBrainSymlinkAndDoesNotReturnItsTargetDuringASampledSwapRace(t *testing.T) {
|
||||
brain, facts, _ := searchBrain(t)
|
||||
target := inBrainTarget(t, brain)
|
||||
require.NoError(t, os.Symlink(filepath.Join("..", "..", "..", "raw", "s.md"), filepath.Join(facts, "inlink.md")))
|
||||
r, err := hydrate(brain, "wiki/w/facts/inlink.md")
|
||||
assert.Error(t, err)
|
||||
assert.NotContains(t, r.Excerpt+r.Title, brainfstest.Sentinel)
|
||||
|
||||
p := filepath.Join(facts, "r.md")
|
||||
require.NoError(t, os.WriteFile(p, []byte("# r\n\nzqinside"), 0o644))
|
||||
r, err = hydrate(brain, "wiki/w/facts/r.md")
|
||||
require.NoError(t, err)
|
||||
assert.Contains(t, r.Excerpt, "zqinside", "positive control: a regular note hydrates")
|
||||
brainfstest.AssertNoLeakFrom(t, p, "# r\n\nzqinside", target, 1500*time.Millisecond, func() string {
|
||||
r, _ := hydrate(brain, "wiki/w/facts/r.md")
|
||||
return r.Title + r.Excerpt
|
||||
})
|
||||
}
|
||||
|
||||
@@ -42,11 +42,6 @@ func TestSyncRefusesSymlinkedNotesAndSurvivesSwapRaces(t *testing.T) {
|
||||
brain := t.TempDir()
|
||||
writeNote(t, brain, "wiki/w/facts/ok.md", "inside note")
|
||||
require.NoError(t, os.Symlink(brainfstest.Outside(t), filepath.Join(brain, "wiki", "w", "facts", "leak.md")))
|
||||
// A link to a file elsewhere in the brain: os.Root allows it, only the
|
||||
// regular-file checks refuse it (hyperguild#171). raw/ is not a scan root,
|
||||
// so its sentinel can only arrive through the link.
|
||||
inBrain := brainfstest.WriteSentinelAt(t, filepath.Join(brain, "raw", "s.md"))
|
||||
require.NoError(t, os.Symlink(filepath.Join("..", "..", "..", "raw", "s.md"), filepath.Join(brain, "wiki", "w", "facts", "inlink.md")))
|
||||
|
||||
emb := &recordingEmbedder{}
|
||||
store := &stubStore{}
|
||||
@@ -56,7 +51,6 @@ func TestSyncRefusesSymlinkedNotesAndSurvivesSwapRaces(t *testing.T) {
|
||||
assert.NotContains(t, emb.all(), brainfstest.Sentinel)
|
||||
for p := range store.upserts {
|
||||
assert.NotContains(t, p, "leak.md")
|
||||
assert.NotContains(t, p, "inlink.md")
|
||||
}
|
||||
|
||||
p := filepath.Join(brain, "knowledge", "r.md")
|
||||
@@ -67,7 +61,6 @@ func TestSyncRefusesSymlinkedNotesAndSurvivesSwapRaces(t *testing.T) {
|
||||
return e.all()
|
||||
}
|
||||
brainfstest.AssertNoLeak(t, p, "inside", 1500*time.Millisecond, sync)
|
||||
brainfstest.AssertNoLeakFrom(t, p, "inside", inBrain, 1500*time.Millisecond, sync)
|
||||
brainfstest.AssertNoHang(t, p, "inside", 1500*time.Millisecond, 6*time.Second, func() { _ = sync() })
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user