diff --git a/.context/PROJECT.md b/.context/PROJECT.md index a0ad2b7..17acf0b 100644 --- a/.context/PROJECT.md +++ b/.context/PROJECT.md @@ -64,7 +64,11 @@ the routing pod; brain tools moved to the brain MCP. The brain HTTP REST API (`/query`, `/write`, `/ingest`, `/ingest-raw`, `/ingest-path`, `/backfill-refs`, `/pass-rate`) remains available on port 3300 -for shell scripts and non-MCP clients. +for shell scripts and non-MCP clients, **with the same bearer token as `/mcp`** +(`BRAIN_MCP_TOKEN`; hyperguild#149). It is not served on the public host +(infra#517): use the tailnet (`koala:8443`) or in-cluster +`ingestion.supervisor.svc:3300`. `/ingest-path` and `brain_ingest` `path` +accept only paths inside `brain/raw/`. `brain_answer(query)` performs BM25 retrieval + LLM synthesis (berget.ai gemma4:31b → iguana fallback). `brain_classify(text)` infers doc type, title, diff --git a/.context/system-prompt.txt b/.context/system-prompt.txt index 7dbec86..9a949ee 100644 --- a/.context/system-prompt.txt +++ b/.context/system-prompt.txt @@ -304,7 +304,7 @@ entries that age well are about *why*, *how to avoid*, and *what to do when*. |---------|-------|-------| | **Claude Code, Claude Desktop** | `brain_query` (BM25), `brain_answer` (LLM-synth + sources) MCP tools | `brain_write` MCP tool | | **Crush, Pi, Antigravity, other MCP-capable** | same MCP server: `ingestion-brain` (via the `mcp__*_brain__*` namespace once authenticated) | same | -| **Anything HTTP-only (curl, scripts)** | `POST https://brain-mcp.d-ma.be/query` with `{"query":"..."}` (auth via `BRAIN_MCP_TOKEN`) | `POST .../write` with `{"content":"...","filename":"..."}` | +| **Anything HTTP-only (curl, scripts)** | Tailnet only: `POST https://koala.tail30472e.ts.net:8443/query` with `{"query":"..."}`, sending `BRAIN_MCP_TOKEN` (not yet enforced on REST, hyperguild#149). The public `brain-mcp.d-ma.be` serves `/mcp` only (infra#517). | `POST .../write` with `{"content":"...","wing":"...","hall":"..."}` | | **Browser / human inspection** | `https://git.d-ma.be/mathias/hyperguild` → `knowledge/` and `wiki/` markdown files | - **Scoping — the brain holds no confidential client content.** Nothing on the read path @@ -493,7 +493,11 @@ the routing pod; brain tools moved to the brain MCP. The brain HTTP REST API (`/query`, `/write`, `/ingest`, `/ingest-raw`, `/ingest-path`, `/backfill-refs`, `/pass-rate`) remains available on port 3300 -for shell scripts and non-MCP clients. +for shell scripts and non-MCP clients, **with the same bearer token as `/mcp`** +(`BRAIN_MCP_TOKEN`; hyperguild#149). It is not served on the public host +(infra#517): use the tailnet (`koala:8443`) or in-cluster +`ingestion.supervisor.svc:3300`. `/ingest-path` and `brain_ingest` `path` +accept only paths inside `brain/raw/`. `brain_answer(query)` performs BM25 retrieval + LLM synthesis (berget.ai gemma4:31b → iguana fallback). `brain_classify(text)` infers doc type, title, diff --git a/AGENTS.md b/AGENTS.md index 25498db..c6ba986 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -299,7 +299,7 @@ entries that age well are about *why*, *how to avoid*, and *what to do when*. |---------|-------|-------| | **Claude Code, Claude Desktop** | `brain_query` (BM25), `brain_answer` (LLM-synth + sources) MCP tools | `brain_write` MCP tool | | **Crush, Pi, Antigravity, other MCP-capable** | same MCP server: `ingestion-brain` (via the `mcp__*_brain__*` namespace once authenticated) | same | -| **Anything HTTP-only (curl, scripts)** | `POST https://brain-mcp.d-ma.be/query` with `{"query":"..."}` (auth via `BRAIN_MCP_TOKEN`) | `POST .../write` with `{"content":"...","filename":"..."}` | +| **Anything HTTP-only (curl, scripts)** | Tailnet only: `POST https://koala.tail30472e.ts.net:8443/query` with `{"query":"..."}`, sending `BRAIN_MCP_TOKEN` (not yet enforced on REST, hyperguild#149). The public `brain-mcp.d-ma.be` serves `/mcp` only (infra#517). | `POST .../write` with `{"content":"...","wing":"...","hall":"..."}` | | **Browser / human inspection** | `https://git.d-ma.be/mathias/hyperguild` → `knowledge/` and `wiki/` markdown files | - **Scoping — the brain holds no confidential client content.** Nothing on the read path @@ -488,7 +488,11 @@ the routing pod; brain tools moved to the brain MCP. The brain HTTP REST API (`/query`, `/write`, `/ingest`, `/ingest-raw`, `/ingest-path`, `/backfill-refs`, `/pass-rate`) remains available on port 3300 -for shell scripts and non-MCP clients. +for shell scripts and non-MCP clients, **with the same bearer token as `/mcp`** +(`BRAIN_MCP_TOKEN`; hyperguild#149). It is not served on the public host +(infra#517): use the tailnet (`koala:8443`) or in-cluster +`ingestion.supervisor.svc:3300`. `/ingest-path` and `brain_ingest` `path` +accept only paths inside `brain/raw/`. `brain_answer(query)` performs BM25 retrieval + LLM synthesis (berget.ai gemma4:31b → iguana fallback). `brain_classify(text)` infers doc type, title, diff --git a/CLAUDE.md b/CLAUDE.md index a0ad2b7..17acf0b 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -64,7 +64,11 @@ the routing pod; brain tools moved to the brain MCP. The brain HTTP REST API (`/query`, `/write`, `/ingest`, `/ingest-raw`, `/ingest-path`, `/backfill-refs`, `/pass-rate`) remains available on port 3300 -for shell scripts and non-MCP clients. +for shell scripts and non-MCP clients, **with the same bearer token as `/mcp`** +(`BRAIN_MCP_TOKEN`; hyperguild#149). It is not served on the public host +(infra#517): use the tailnet (`koala:8443`) or in-cluster +`ingestion.supervisor.svc:3300`. `/ingest-path` and `brain_ingest` `path` +accept only paths inside `brain/raw/`. `brain_answer(query)` performs BM25 retrieval + LLM synthesis (berget.ai gemma4:31b → iguana fallback). `brain_classify(text)` infers doc type, title, diff --git a/cmd/hyperguild/http.go b/cmd/hyperguild/http.go index 1205c1e..32d89f1 100644 --- a/cmd/hyperguild/http.go +++ b/cmd/hyperguild/http.go @@ -64,9 +64,7 @@ func (c *brainClient) Query(ctx context.Context, topic string, limit int) (*Quer return nil, fmt.Errorf("build request: %w", err) } req.Header.Set("Content-Type", "application/json") - c.authorize(req) - - resp, err := c.http.Do(req) + resp, err := c.do(req) if err != nil { return nil, fmt.Errorf("brain POST /query: %w", err) } @@ -108,9 +106,7 @@ func (c *brainClient) Write(ctx context.Context, wing, hall, filename string, co return nil, fmt.Errorf("build request: %w", err) } req.Header.Set("Content-Type", "application/json") - c.authorize(req) - - resp, err := c.http.Do(req) + resp, err := c.do(req) if err != nil { return nil, fmt.Errorf("brain POST /write: %w", err) } @@ -147,7 +143,7 @@ func (c *brainClient) PassRate(ctx context.Context, skill, window string) (*Pass if err != nil { return nil, fmt.Errorf("build request: %w", err) } - resp, err := c.http.Do(req) + resp, err := c.do(req) if err != nil { return nil, fmt.Errorf("brain GET /pass-rate: %w", err) } @@ -163,9 +159,12 @@ func (c *brainClient) PassRate(ctx context.Context, skill, window string) (*Pass return &out, nil } -// authorize adds the bearer token when one is configured. -func (c *brainClient) authorize(req *http.Request) { +// do sends every brain request, adding the bearer token when one is +// configured. All methods go through here so none can forget it (PassRate +// did, PR #152 review). +func (c *brainClient) do(req *http.Request) (*http.Response, error) { if c.token != "" { req.Header.Set("Authorization", "Bearer "+c.token) } + return c.http.Do(req) } diff --git a/cmd/hyperguild/http_auth_test.go b/cmd/hyperguild/http_auth_test.go index 97a8e56..482cfca 100644 --- a/cmd/hyperguild/http_auth_test.go +++ b/cmd/hyperguild/http_auth_test.go @@ -13,13 +13,17 @@ import ( // hyperguild#149: the ingestion REST API now requires a bearer token; the CLI // sends BRAIN_MCP_TOKEN, or it gets 401 on every brain call. -func TestBrainClientSendsBrainMCPToken(t *testing.T) { +func TestBrainClientSendsBrainMCPTokenOnQueryWriteAndPassRate(t *testing.T) { var got []string srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { got = append(got, r.URL.Path+" "+r.Header.Get("Authorization")) - if r.URL.Path == "/write" { + switch r.URL.Path { + case "/write": _, _ = w.Write([]byte(`{"path":"wiki/x/facts/y.md"}`)) return + case "/pass-rate": + _, _ = w.Write([]byte(`{}`)) + return } _, _ = w.Write([]byte(`{"results":[]}`)) })) @@ -32,7 +36,10 @@ func TestBrainClientSendsBrainMCPToken(t *testing.T) { require.NoError(t, err) _, err = c.Write(context.Background(), "x", "facts", "y", strings.NewReader("# y")) require.NoError(t, err) - assert.Equal(t, []string{"/query Bearer cli-test-token", "/write Bearer cli-test-token"}, got) + // PR #152 review: PassRate sent no token and would 401 in production. + _, _ = c.PassRate(context.Background(), "tdd", "7d") + assert.Equal(t, []string{"/query Bearer cli-test-token", "/write Bearer cli-test-token", + "/pass-rate Bearer cli-test-token"}, got) } func TestBrainClientSendsNoAuthorizationWhenTokenUnset(t *testing.T) { diff --git a/docs/runbooks/exercising-pass-rate-traffic.md b/docs/runbooks/exercising-pass-rate-traffic.md index 2f13e76..1d30e8e 100644 --- a/docs/runbooks/exercising-pass-rate-traffic.md +++ b/docs/runbooks/exercising-pass-rate-traffic.md @@ -24,7 +24,7 @@ Only a `tools/call` to the routing pod records a pass/fail. |---------|-----|------| | Routing MCP (local, Tailscale) | `http://koala:30310/mcp` | Bearer `ROUTING_MCP_TOKEN` | | Routing MCP (remote) | `https://routing-mcp.d-ma.be/mcp` | OAuth via `auth.d-ma.be` (audience `claude-ai`) | -| Pass-rate readout | `http://koala:30330/pass-rate?skill=` | none (read-only) | +| Pass-rate readout | `http://koala:30330/pass-rate?skill=` | Bearer `BRAIN_MCP_TOKEN` (since hyperguild#149) | Tools advertised: **`review`**, **`debug`** (the two the #35 gate measures), plus `session_log`, `retrospective`, `trainer`. diff --git a/ingestion/cmd/server/main.go b/ingestion/cmd/server/main.go index b0193d0..59def19 100644 --- a/ingestion/cmd/server/main.go +++ b/ingestion/cmd/server/main.go @@ -503,9 +503,10 @@ func main() { logger.Info("embed sync started", "interval_s", embedSyncInterval) } - mux := http.NewServeMux() + // Public routes only; buildMux mounts the authenticated REST API (#149). + public := map[string]http.Handler{} if webhookHandler != nil { - mux.Handle("POST /webhooks/brain-sync", webhookHandler) + public["POST /webhooks/brain-sync"] = webhookHandler } // Resource-metadata URL is only emitted on 401 when Dex OAuth is // configured. Static-Bearer-only deployments leave this empty so @@ -513,21 +514,14 @@ func main() { var resourceMetadataURL string if dexURL := os.Getenv("DEX_ISSUER_URL"); dexURL != "" { resourceURL := os.Getenv("MCP_RESOURCE_URL") - mux.HandleFunc("GET /.well-known/oauth-protected-resource", + public["GET /.well-known/oauth-protected-resource"] = http.HandlerFunc( chassisauth.ProtectedResourceHandler(resourceURL, dexURL)) if resourceURL != "" { resourceMetadataURL = strings.TrimRight(resourceURL, "/") + "/.well-known/oauth-protected-resource" } } - mux.Handle("/mcp", chassisauth.BearerMiddleware(mcpToken, jwtValidator, "brain", resourceMetadataURL, mcpSrv)) - // The REST API, authenticated as a whole (hyperguild#149). Same credential - // as /mcp: callers (brain-ui, brain-ingest-ntfy, the hyperguild CLI) send - // BRAIN_MCP_TOKEN. Registered after the public routes on purpose only for - // reading order; ServeMux precedence is by pattern specificity. - mountREST(mux, h, func(next http.Handler) http.Handler { - return chassisauth.BearerMiddleware(mcpToken, jwtValidator, "brain", resourceMetadataURL, next) - }) + 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 @@ -549,7 +543,7 @@ func main() { sovereign := splitList(os.Getenv("BRAIN_CAPTURE_SOVEREIGN_PRINCIPALS")) resolver := capturehttp.NewOriginResolver(sovereign) captureH := capturehttp.New(captureSvc, jwtValidator, mcpToken, "local-cli", resolver) - mux.Handle("POST /capture", captureH) + 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; @@ -579,9 +573,9 @@ func main() { logger.Error("OAUTH_CLIENT_ID/SECRET set but MCP_RESOURCE_URL is empty; cannot derive issuer") os.Exit(1) } - mux.HandleFunc("GET /.well-known/oauth-authorization-server", + public["GET /.well-known/oauth-authorization-server"] = http.HandlerFunc( oauth.MetadataHandler(issuer)) - mux.HandleFunc("POST /oauth/token", oauth.TokenHandler(oauth.TokenConfig{ + public["POST /oauth/token"] = http.HandlerFunc(oauth.TokenHandler(oauth.TokenConfig{ ClientID: oauthID, ClientSecret: oauthSecret, AccessToken: mcpToken, @@ -600,7 +594,17 @@ func main() { // real request latency. /metrics itself is excluded from its own // observation by registering it on the outer mux (post-wrap). reg := metrics.New() - mux.HandleFunc("GET /metrics", reg.Handler()) + 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) + if err != nil { + logger.Error("build router", "err", err) + os.Exit(1) + } logger.Info("metrics endpoint registered", "path", "/metrics") addr := ":" + port diff --git a/ingestion/cmd/server/routes.go b/ingestion/cmd/server/routes.go index b4584db..ca86faf 100644 --- a/ingestion/cmd/server/routes.go +++ b/ingestion/cmd/server/routes.go @@ -1,18 +1,65 @@ package main import ( + "errors" + "fmt" "net/http" + chassisauth "git.d-ma.be/mathias/mcp-chassis/auth" + "github.com/mathiasbq/hyperguild/ingestion/internal/api" ) +// 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. +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. +type restAuth struct { + token string + validator *chassisauth.JWTValidator + realm string + 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) { + if auth.token == "" { + return nil, errors.New("buildMux: REST auth needs a token") + } + mux := http.NewServeMux() + for pattern, handler := range public { + if !publicRoutes[pattern] { + return nil, fmt.Errorf("buildMux: %q is not an allowed public route", pattern) + } + mux.Handle(pattern, handler) + } + mountREST(mux, h, func(next http.Handler) http.Handler { + return chassisauth.BearerMiddleware(auth.token, auth.validator, auth.realm, auth.resourceMetadataURL, next) + }) + return mux, nil +} + // mountREST registers the REST API on its own mux and mounts it at "/" behind -// auth (hyperguild#149). Authentication is a property of the mux, not of each -// route: anything not registered on the outer mux as explicitly public (/mcp, -// /capture, the HMAC webhook, OAuth metadata, /metrics) lands here and is -// authenticated, including routes added later. Before this, only /mcp was -// wrapped, and the REST routes beside it were open; infra#517 found them on -// the internet. +// auth: anything not registered as public lands here and is authenticated, +// including routes added later. func mountREST(mux *http.ServeMux, h *api.Handler, auth func(http.Handler) http.Handler) { rest := http.NewServeMux() rest.HandleFunc("POST /query", h.Query) diff --git a/ingestion/cmd/server/routes_test.go b/ingestion/cmd/server/routes_test.go index 4e24265..8bc2c3e 100644 --- a/ingestion/cmd/server/routes_test.go +++ b/ingestion/cmd/server/routes_test.go @@ -9,8 +9,7 @@ import ( "testing" "github.com/stretchr/testify/assert" - - chassisauth "git.d-ma.be/mathias/mcp-chassis/auth" + "github.com/stretchr/testify/require" "github.com/mathiasbq/hyperguild/ingestion/internal/api" "github.com/mathiasbq/hyperguild/ingestion/internal/pipeline" @@ -18,22 +17,27 @@ import ( const routesTok = "routes-test-token" -// hyperguild#149: only /mcp was authenticated; every REST route on the same -// mux was open, and infra#517 found them on the internet. The REST API is now -// mounted at "/" behind the bearer middleware, so any route not registered -// as explicitly public is authenticated, including routes added later. -func testRouter(t *testing.T) http.Handler { +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. +func testMux(t *testing.T) http.Handler { t.Helper() h := api.NewHandler(t.TempDir(), slog.New(slog.NewTextHandler(os.Stderr, nil)), pipeline.Config{}) - mux := http.NewServeMux() - mux.HandleFunc("GET /metrics", func(w http.ResponseWriter, _ *http.Request) { _, _ = w.Write([]byte("ok")) }) - mountREST(mux, h, func(next http.Handler) http.Handler { - return chassisauth.BearerMiddleware(routesTok, nil, "brain", "", next) - }) + public := map[string]http.Handler{} + for p := range publicRoutes { + public[p] = okHandler("public " + p) + } + mux, err := buildMux(restAuth{token: routesTok, realm: "brain"}, h, public) + require.NoError(t, err) return mux } -func do(t *testing.T, h http.Handler, method, path, authz string) int { +func do(t *testing.T, h http.Handler, method, path, authz string) (int, string) { t.Helper() req := httptest.NewRequest(method, path, strings.NewReader(`{"query":"x"}`)) if authz != "" { @@ -41,34 +45,62 @@ func do(t *testing.T, h http.Handler, method, path, authz string) int { } rec := httptest.NewRecorder() h.ServeHTTP(rec, req) - return rec.Code + return rec.Code, rec.Body.String() } -func TestRESTRoutesRefuseRequestsWithoutAToken(t *testing.T) { - r := testRouter(t) +func TestBuildMuxRefusesRESTRoutesWithoutAValidToken(t *testing.T) { + r := testMux(t) for _, rt := range []struct{ method, path string }{ {"POST", "/query"}, {"POST", "/write"}, {"POST", "/index"}, {"POST", "/ingest"}, {"POST", "/ingest-path"}, {"POST", "/ingest-raw"}, {"POST", "/backfill-refs"}, {"GET", "/pending"}, {"POST", "/promote"}, {"POST", "/backfill-embeddings"}, {"GET", "/pass-rate"}, } { - assert.Equal(t, http.StatusUnauthorized, do(t, r, rt.method, rt.path, ""), "%s %s without a token", rt.method, rt.path) - assert.Equal(t, http.StatusUnauthorized, do(t, r, rt.method, rt.path, "Bearer wrong"), "%s %s with a wrong token", rt.method, rt.path) + code, _ := do(t, r, rt.method, rt.path, "") + assert.Equal(t, http.StatusUnauthorized, code, "%s %s without a token", rt.method, rt.path) + code, _ = do(t, r, rt.method, rt.path, "Bearer wrong") + assert.Equal(t, http.StatusUnauthorized, code, "%s %s with a wrong token", rt.method, rt.path) } } -func TestRESTRoutesServeAValidToken(t *testing.T) { - r := testRouter(t) - assert.NotEqual(t, http.StatusUnauthorized, do(t, r, "POST", "/query", "Bearer "+routesTok)) - assert.Equal(t, http.StatusOK, do(t, r, "GET", "/pending", "Bearer "+routesTok)) +func TestBuildMuxServesRESTRoutesToAValidToken(t *testing.T) { + r := testMux(t) + code, _ := do(t, r, "GET", "/pending", "Bearer "+routesTok) + assert.Equal(t, http.StatusOK, code) } -// Default-deny: a path nobody registered is still behind auth, so a route -// added to the REST mux later cannot ship open by omission. -func TestUnregisteredPathIsAuthenticatedByDefault(t *testing.T) { - assert.Equal(t, http.StatusUnauthorized, do(t, testRouter(t), "POST", "/some-future-route", "")) +func TestBuildMuxAuthenticatesAnUnregisteredPath(t *testing.T) { + code, _ := do(t, testMux(t), "POST", "/some-future-route", "") + assert.Equal(t, http.StatusUnauthorized, code) } -// Explicitly public routes registered on the outer mux keep working. -func TestMetricsStaysOpenForPrometheus(t *testing.T) { - assert.Equal(t, http.StatusOK, do(t, testRouter(t), "GET", "/metrics", "")) +// 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). +func TestBuildMuxDoesNotWrapPublicRoutes(t *testing.T) { + r := testMux(t) + for p := range publicRoutes { + method, path, ok := strings.Cut(p, " ") + if !ok { + method, path = "GET", p + } + code, body := do(t, r, method, path, "") + assert.Equal(t, http.StatusOK, code, p) + assert.Equal(t, "public "+p, body, p) + } +} + +// The allowlist is the control: a REST pattern registered as public would win +// 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")}) + 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) + assert.Error(t, err) } diff --git a/ingestion/internal/api/ingestpath_root.go b/ingestion/internal/api/ingestpath_root.go index 59a963d..fe36a90 100644 --- a/ingestion/internal/api/ingestpath_root.go +++ b/ingestion/internal/api/ingestpath_root.go @@ -2,13 +2,14 @@ package api import ( "errors" - "fmt" "path/filepath" "strings" ) -// ErrOutsideIngestRoot means an ingest path does not resolve inside brain/raw/. -var ErrOutsideIngestRoot = errors.New("ingest path must be inside brain/raw/") +// ErrOutsideIngestRoot is the one refusal for every bad ingest path. It never +// carries the underlying error: "no such file" versus "permission denied" +// would let a caller probe which server files exist (PR #152 review). +var ErrOutsideIngestRoot = errors.New("ingest path must be an existing file or directory inside brain/raw/") // ResolveIngestPath confines a caller-supplied ingest path to brain/raw/, the // inbox the watcher already owns (hyperguild#149). Relative paths resolve @@ -16,18 +17,18 @@ var ErrOutsideIngestRoot = errors.New("ingest path must be inside brain/raw/") // a link inside raw/ that points elsewhere is refused. The path must exist. func ResolveIngestPath(brainDir, p string) (string, error) { if strings.TrimSpace(p) == "" { - return "", fmt.Errorf("%w: path is empty", ErrOutsideIngestRoot) + return "", ErrOutsideIngestRoot } root, err := filepath.EvalSymlinks(filepath.Join(brainDir, "raw")) if err != nil { - return "", fmt.Errorf("%w: inbox unavailable: %v", ErrOutsideIngestRoot, err) + return "", ErrOutsideIngestRoot } if !filepath.IsAbs(p) { p = filepath.Join(root, p) } real, err := filepath.EvalSymlinks(filepath.Clean(p)) if err != nil { - return "", fmt.Errorf("%w: %v", ErrOutsideIngestRoot, err) + return "", ErrOutsideIngestRoot } rel, err := filepath.Rel(root, real) if err != nil || rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) || filepath.IsAbs(rel) { diff --git a/ingestion/internal/api/ingestpath_root_test.go b/ingestion/internal/api/ingestpath_root_test.go index c6f8dee..22ca236 100644 --- a/ingestion/internal/api/ingestpath_root_test.go +++ b/ingestion/internal/api/ingestpath_root_test.go @@ -67,3 +67,22 @@ func TestIngestPathDirectoryWalkSkipsSymlinksLeavingTheInbox(t *testing.T) { assert.Contains(t, rec.Body.String(), "skipped") assert.NotContains(t, rec.Body.String(), "SENTINEL-149-OUTSIDE") } + +// PR #152 review: the raw EvalSymlinks error told an authenticated caller +// whether any server file existed ("no such file") or was unreadable +// ("permission denied"), and leaked the brain directory. Every refusal now +// reads the same. +func TestResolveIngestPathRefusalsAreIndistinguishable(t *testing.T) { + brain := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(brain, "raw"), 0o755)) + var msgs []string + for _, p := range []string{"/etc/hostname", "/etc/definitely-not-here-149", "../x", "nope/missing.md", ""} { + _, err := api.ResolveIngestPath(brain, p) + require.Error(t, err, p) + msgs = append(msgs, err.Error()) + assert.NotContains(t, err.Error(), brain, "refusal for %q leaks the brain dir", p) + } + for _, m := range msgs[1:] { + assert.Equal(t, msgs[0], m) + } +}