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 b6e9b12..32d89f1 100644 --- a/cmd/hyperguild/http.go +++ b/cmd/hyperguild/http.go @@ -20,6 +20,7 @@ const defaultBrainURL = "http://koala:30330" // shell-friendly client; MCP framing is unnecessary. type brainClient struct { baseURL string + token string // BRAIN_MCP_TOKEN; the REST API requires it since hyperguild#149 http *http.Client } @@ -30,6 +31,7 @@ func newBrainClient() *brainClient { } return &brainClient{ baseURL: u, + token: os.Getenv("BRAIN_MCP_TOKEN"), http: &http.Client{Timeout: 5 * time.Second}, } } @@ -62,8 +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") - - resp, err := c.http.Do(req) + resp, err := c.do(req) if err != nil { return nil, fmt.Errorf("brain POST /query: %w", err) } @@ -105,8 +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") - - resp, err := c.http.Do(req) + resp, err := c.do(req) if err != nil { return nil, fmt.Errorf("brain POST /write: %w", err) } @@ -143,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) } @@ -158,3 +158,13 @@ func (c *brainClient) PassRate(ctx context.Context, skill, window string) (*Pass } return &out, nil } + +// 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 new file mode 100644 index 0000000..482cfca --- /dev/null +++ b/cmd/hyperguild/http_auth_test.go @@ -0,0 +1,57 @@ +package main + +import ( + "context" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// 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 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")) + 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":[]}`)) + })) + defer srv.Close() + t.Setenv("BRAIN_URL", srv.URL) + t.Setenv("BRAIN_MCP_TOKEN", "cli-test-token") + + c := newBrainClient() + _, err := c.Query(context.Background(), "x", 1) + require.NoError(t, err) + _, err = c.Write(context.Background(), "x", "facts", "y", strings.NewReader("# y")) + require.NoError(t, err) + // 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) { + var auth string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + auth = r.Header.Get("Authorization") + _, _ = w.Write([]byte(`{"results":[]}`)) + })) + defer srv.Close() + t.Setenv("BRAIN_URL", srv.URL) + t.Setenv("BRAIN_MCP_TOKEN", "") + _, err := newBrainClient().Query(context.Background(), "x", 1) + require.NoError(t, err) + assert.Empty(t, auth) +} 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 bc38f11..59def19 100644 --- a/ingestion/cmd/server/main.go +++ b/ingestion/cmd/server/main.go @@ -503,20 +503,10 @@ func main() { logger.Info("embed sync started", "interval_s", embedSyncInterval) } - mux := http.NewServeMux() - mux.HandleFunc("POST /query", h.Query) - mux.HandleFunc("POST /write", h.Write) - mux.HandleFunc("POST /index", h.Index) - mux.HandleFunc("POST /ingest", h.Ingest) - mux.HandleFunc("POST /ingest-path", h.IngestPath) - mux.HandleFunc("POST /ingest-raw", h.IngestRaw) - mux.HandleFunc("POST /backfill-refs", h.BackfillRefs) - mux.HandleFunc("GET /pending", h.Pending) - mux.HandleFunc("POST /promote", h.Promote) - mux.HandleFunc("POST /backfill-embeddings", h.BackfillEmbeddings) - mux.HandleFunc("GET /pass-rate", h.PassRate) + // 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 @@ -524,14 +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)) + 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 @@ -553,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; @@ -583,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, @@ -604,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 new file mode 100644 index 0000000..ca86faf --- /dev/null +++ b/ingestion/cmd/server/routes.go @@ -0,0 +1,77 @@ +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: 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) + rest.HandleFunc("POST /write", h.Write) + rest.HandleFunc("POST /index", h.Index) + rest.HandleFunc("POST /ingest", h.Ingest) + rest.HandleFunc("POST /ingest-path", h.IngestPath) + rest.HandleFunc("POST /ingest-raw", h.IngestRaw) + rest.HandleFunc("POST /backfill-refs", h.BackfillRefs) + rest.HandleFunc("GET /pending", h.Pending) + rest.HandleFunc("POST /promote", h.Promote) + rest.HandleFunc("POST /backfill-embeddings", h.BackfillEmbeddings) + rest.HandleFunc("GET /pass-rate", h.PassRate) + mux.Handle("/", auth(rest)) +} diff --git a/ingestion/cmd/server/routes_test.go b/ingestion/cmd/server/routes_test.go new file mode 100644 index 0000000..8bc2c3e --- /dev/null +++ b/ingestion/cmd/server/routes_test.go @@ -0,0 +1,106 @@ +package main + +import ( + "log/slog" + "net/http" + "net/http/httptest" + "os" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/mathiasbq/hyperguild/ingestion/internal/api" + "github.com/mathiasbq/hyperguild/ingestion/internal/pipeline" +) + +const routesTok = "routes-test-token" + +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{}) + 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, string) { + t.Helper() + req := httptest.NewRequest(method, path, strings.NewReader(`{"query":"x"}`)) + if authz != "" { + req.Header.Set("Authorization", authz) + } + rec := httptest.NewRecorder() + h.ServeHTTP(rec, req) + return rec.Code, rec.Body.String() +} + +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"}, + } { + 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 TestBuildMuxServesRESTRoutesToAValidToken(t *testing.T) { + r := testMux(t) + code, _ := do(t, r, "GET", "/pending", "Bearer "+routesTok) + assert.Equal(t, http.StatusOK, code) +} + +func TestBuildMuxAuthenticatesAnUnregisteredPath(t *testing.T) { + code, _ := do(t, testMux(t), "POST", "/some-future-route", "") + assert.Equal(t, http.StatusUnauthorized, code) +} + +// 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/handler.go b/ingestion/internal/api/handler.go index 6359f74..ad49182 100644 --- a/ingestion/internal/api/handler.go +++ b/ingestion/internal/api/handler.go @@ -447,6 +447,12 @@ func (h *Handler) IngestPath(w http.ResponseWriter, r *http.Request) { writeError(w, http.StatusBadRequest, "path is required") return } + resolved, err := ResolveIngestPath(h.brainDir, req.Path) + if err != nil { + writeError(w, http.StatusBadRequest, err.Error()) + return + } + req.Path = resolved info, err := os.Stat(req.Path) if err != nil { @@ -469,6 +475,12 @@ func (h *Handler) IngestPath(w http.ResponseWriter, r *http.Request) { if !isSupportedExtension(ext) { return nil } + // Each walked file is re-resolved: a symlinked file inside raw/ + // would otherwise lead extract.Text out of the inbox (#149). + if _, rerr := ResolveIngestPath(h.brainDir, path); rerr != nil { + allWarnings = append(allWarnings, fmt.Sprintf("skipped %s: %v", path, rerr)) + return nil + } content, readErr := extract.Text(path) if readErr != nil { allWarnings = append(allWarnings, fmt.Sprintf("extract %s: %v", path, readErr)) diff --git a/ingestion/internal/api/handler_test.go b/ingestion/internal/api/handler_test.go index b5131cd..3354ba0 100644 --- a/ingestion/internal/api/handler_test.go +++ b/ingestion/internal/api/handler_test.go @@ -361,10 +361,11 @@ func TestIngestPath_MissingPath(t *testing.T) { } func TestIngestPath_File(t *testing.T) { - _, h := setup(t) + brainDir, h := setup(t) // Create a temp file with content - dir := t.TempDir() + dir := filepath.Join(brainDir, "raw", "fixture") // ingest paths must live in brain/raw/ (#149) + require.NoError(t, os.MkdirAll(dir, 0o755)) f := filepath.Join(dir, "doc.md") require.NoError(t, os.WriteFile(f, []byte("# Hello\nThis is markdown content."), 0o644)) @@ -468,10 +469,11 @@ func TestIngestRaw_DryRun(t *testing.T) { } func TestIngestPath_Directory(t *testing.T) { - _, h := setup(t) + brainDir, h := setup(t) // Create a temp dir with one .md file - dir := t.TempDir() + dir := filepath.Join(brainDir, "raw", "fixture") // ingest paths must live in brain/raw/ (#149) + require.NoError(t, os.MkdirAll(dir, 0o755)) require.NoError(t, os.WriteFile(filepath.Join(dir, "notes.md"), []byte("# Notes\nSome notes."), 0o644)) body, _ := json.Marshal(map[string]any{ diff --git a/ingestion/internal/api/ingestpath_docmark_test.go b/ingestion/internal/api/ingestpath_docmark_test.go index 40da3ac..7df464e 100644 --- a/ingestion/internal/api/ingestpath_docmark_test.go +++ b/ingestion/internal/api/ingestpath_docmark_test.go @@ -40,9 +40,10 @@ func TestIngestPath_DocxSupportedWhenDocmarkConfigured(t *testing.T) { t.Setenv("DOCMARK_URL", srv.URL+"/mcp") t.Setenv("DOCMARK_BEARER_TOKEN", "tok") - _, h := setup(t) + brainDir, h := setup(t) - dir := t.TempDir() + dir := filepath.Join(brainDir, "raw", "fixture") // ingest paths must live in brain/raw/ (#149) + require.NoError(t, os.MkdirAll(dir, 0o755)) f := filepath.Join(dir, "doc.docx") require.NoError(t, os.WriteFile(f, []byte("fake docx"), 0o644)) diff --git a/ingestion/internal/api/ingestpath_root.go b/ingestion/internal/api/ingestpath_root.go new file mode 100644 index 0000000..fe36a90 --- /dev/null +++ b/ingestion/internal/api/ingestpath_root.go @@ -0,0 +1,38 @@ +package api + +import ( + "errors" + "path/filepath" + "strings" +) + +// 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 +// against that root; symlinks are evaluated before the containment check, so +// 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 "", ErrOutsideIngestRoot + } + root, err := filepath.EvalSymlinks(filepath.Join(brainDir, "raw")) + if err != nil { + return "", ErrOutsideIngestRoot + } + if !filepath.IsAbs(p) { + p = filepath.Join(root, p) + } + real, err := filepath.EvalSymlinks(filepath.Clean(p)) + if err != nil { + return "", ErrOutsideIngestRoot + } + rel, err := filepath.Rel(root, real) + if err != nil || rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) || filepath.IsAbs(rel) { + return "", ErrOutsideIngestRoot + } + return real, nil +} diff --git a/ingestion/internal/api/ingestpath_root_test.go b/ingestion/internal/api/ingestpath_root_test.go new file mode 100644 index 0000000..22ca236 --- /dev/null +++ b/ingestion/internal/api/ingestpath_root_test.go @@ -0,0 +1,88 @@ +package api_test + +import ( + "bytes" + "encoding/json" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/mathiasbq/hyperguild/ingestion/internal/api" +) + +// hyperguild#149: /ingest-path and MCP brain_ingest read a caller-chosen +// server path, so any file the pod could read (the ServiceAccount token, +// /etc/passwd) could be pulled into the brain and read back out. Ingest paths +// now resolve inside brain/raw/ — the inbox the watcher already owns — with +// symlinks evaluated, or are refused. +func TestResolveIngestPathConfinesToRawInbox(t *testing.T) { + brain := t.TempDir() + raw := filepath.Join(brain, "raw") + require.NoError(t, os.MkdirAll(filepath.Join(raw, "sub"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(raw, "sub", "ok.md"), []byte("x"), 0o644)) + outside := filepath.Join(t.TempDir(), "secret.txt") + require.NoError(t, os.WriteFile(outside, []byte("s"), 0o644)) + require.NoError(t, os.Symlink(outside, filepath.Join(raw, "link.md"))) + require.NoError(t, os.Symlink(filepath.Dir(outside), filepath.Join(raw, "dirlink"))) + + for _, ok := range []string{filepath.Join(raw, "sub", "ok.md"), "sub/ok.md", raw, "sub"} { + _, err := api.ResolveIngestPath(brain, ok) + assert.NoError(t, err, ok) + } + for _, bad := range []string{ + "/etc/hostname", outside, "../secret.txt", filepath.Join(raw, "..", "wiki"), + filepath.Join(raw, "link.md"), filepath.Join(raw, "dirlink", "secret.txt"), + raw + "-evil/x.md", "", + } { + _, err := api.ResolveIngestPath(brain, bad) + assert.Error(t, err, "must refuse %q", bad) + } +} + +func TestIngestPathRefusesAPathOutsideTheInbox(t *testing.T) { + _, h := setup(t) + body, _ := json.Marshal(map[string]any{"path": "/etc/hostname"}) + rec := httptest.NewRecorder() + h.IngestPath(rec, httptest.NewRequest(http.MethodPost, "/ingest-path", bytes.NewReader(body))) + assert.Equal(t, http.StatusBadRequest, rec.Code) + assert.Contains(t, rec.Body.String(), "raw") +} + +// A directory ingest must not follow a symlinked FILE inside raw/ out of it. +func TestIngestPathDirectoryWalkSkipsSymlinksLeavingTheInbox(t *testing.T) { + dir, h := setup(t) + raw := filepath.Join(dir, "raw", "batch") + require.NoError(t, os.MkdirAll(raw, 0o755)) + outside := filepath.Join(t.TempDir(), "secret.md") + require.NoError(t, os.WriteFile(outside, []byte("# secret\n\nSENTINEL-149-OUTSIDE"), 0o644)) + require.NoError(t, os.Symlink(outside, filepath.Join(raw, "leak.md"))) + body, _ := json.Marshal(map[string]any{"path": "batch", "source": "walk"}) + rec := httptest.NewRecorder() + h.IngestPath(rec, httptest.NewRequest(http.MethodPost, "/ingest-path", bytes.NewReader(body))) + 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) + } +} diff --git a/ingestion/internal/mcp/handlers.go b/ingestion/internal/mcp/handlers.go index ec76a75..e4eeb59 100644 --- a/ingestion/internal/mcp/handlers.go +++ b/ingestion/internal/mcp/handlers.go @@ -543,6 +543,12 @@ func (s *Server) brainIngest(ctx context.Context, args json.RawMessage) (json.Ra } if a.Path != "" { + // Confined to brain/raw/ with symlinks evaluated (hyperguild#149). + resolved, err := api.ResolveIngestPath(s.brainDir, a.Path) + if err != nil { + return nil, err + } + a.Path = resolved text, err := extract.Text(a.Path) if err != nil { return nil, fmt.Errorf("extract: %w", err) diff --git a/ingestion/internal/mcp/ingest_path_test.go b/ingestion/internal/mcp/ingest_path_test.go new file mode 100644 index 0000000..47da037 --- /dev/null +++ b/ingestion/internal/mcp/ingest_path_test.go @@ -0,0 +1,24 @@ +package mcp_test + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/mathiasbq/hyperguild/ingestion/internal/mcp" + "github.com/mathiasbq/hyperguild/ingestion/internal/pipeline" +) + +// hyperguild#149: brain_ingest's `path` read any file the pod could read and +// ingested it into the brain, where brain_query reads it back out. +func TestBrainIngestRefusesAPathOutsideTheRawInbox(t *testing.T) { + stub := pipeline.Config{Complete: func(context.Context, string, string) (string, error) { + return `[{"title":"Leak","type":"concept","content":"x"}]`, nil + }} + srv := mcp.NewServer(newBrainRepo(t), &stub, nil, nil) + resp := authCall(t, srv, "", "brain_ingest", map[string]any{"path": "/etc/hostname"}) + require.NotNil(t, resp["error"], "a path outside brain/raw/ must be refused") + assert.Contains(t, resp["error"].(map[string]any)["message"], "brain/raw/") +} diff --git a/internal/brain/client.go b/internal/brain/client.go index 3a5e6e8..8ccbc0b 100644 --- a/internal/brain/client.go +++ b/internal/brain/client.go @@ -13,6 +13,7 @@ import ( "io" "log/slog" "net/http" + "os" "strings" ) @@ -42,6 +43,10 @@ func Query(ctx context.Context, baseURL, query string, limit int) (string, error return "", nil } req.Header.Set("Content-Type", "application/json") + // The REST API requires a bearer token since hyperguild#149. + if tok := os.Getenv("BRAIN_MCP_TOKEN"); tok != "" { + req.Header.Set("Authorization", "Bearer "+tok) + } resp, err := http.DefaultClient.Do(req) if err != nil { diff --git a/internal/brain/client_test.go b/internal/brain/client_test.go index fb11b98..1936744 100644 --- a/internal/brain/client_test.go +++ b/internal/brain/client_test.go @@ -65,3 +65,18 @@ func TestQueryUnavailableServerReturnsEmpty(t *testing.T) { require.NoError(t, err) assert.Empty(t, result) } + +// hyperguild#149: the REST API requires a bearer token. +func TestQuerySendsBrainMCPToken(t *testing.T) { + var auth string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + auth = r.Header.Get("Authorization") + _, _ = w.Write([]byte(`{"results":[]}`)) + })) + defer srv.Close() + t.Setenv("BRAIN_MCP_TOKEN", "lib-test-token") + _, _ = brain.Query(context.Background(), srv.URL, "x", 1) + if auth != "Bearer lib-test-token" { + t.Fatalf("Authorization = %q, want the bearer token", auth) + } +}