From 99fedeaa984bf5cc3af64c0f1da1463750cddec4 Mon Sep 17 00:00:00 2001 From: Mathias Date: Sat, 3 Oct 2026 16:17:45 +0200 Subject: [PATCH 1/3] fix(ingestion): authenticate the REST API as a whole, not route by route Only /mcp was wrapped in BearerMiddleware; /query, /write, /ingest*, /promote, /pending and the rest on the same mux had no auth, and infra#517 found them on the internet (mitigated there by narrowing the ingress). They are still open on the tailnet and LAN NodePort 30330. mountREST puts the REST routes on their own mux, mounted at "/" behind the same bearer middleware and credential as /mcp. Routes registered on the outer mux stay explicitly public (/mcp has its own auth, /capture, the HMAC webhook, OAuth metadata, /metrics); anything else, including a route added later, is authenticated by default. The hyperguild CLI (cmd/hyperguild and internal/brain) now sends BRAIN_MCP_TOKEN, which it never did. brain-ui and brain-ingest-ntfy already send theirs. Refs #149 Co-Authored-By: Claude Opus 5.5 --- cmd/hyperguild/http.go | 11 +++++ cmd/hyperguild/http_auth_test.go | 50 +++++++++++++++++++ ingestion/cmd/server/main.go | 18 +++---- ingestion/cmd/server/routes.go | 30 ++++++++++++ ingestion/cmd/server/routes_test.go | 74 +++++++++++++++++++++++++++++ internal/brain/client.go | 5 ++ internal/brain/client_test.go | 15 ++++++ 7 files changed, 192 insertions(+), 11 deletions(-) create mode 100644 cmd/hyperguild/http_auth_test.go create mode 100644 ingestion/cmd/server/routes.go create mode 100644 ingestion/cmd/server/routes_test.go diff --git a/cmd/hyperguild/http.go b/cmd/hyperguild/http.go index b6e9b12..1205c1e 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,6 +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) if err != nil { @@ -105,6 +108,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) if err != nil { @@ -158,3 +162,10 @@ 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) { + if c.token != "" { + req.Header.Set("Authorization", "Bearer "+c.token) + } +} diff --git a/cmd/hyperguild/http_auth_test.go b/cmd/hyperguild/http_auth_test.go new file mode 100644 index 0000000..97a8e56 --- /dev/null +++ b/cmd/hyperguild/http_auth_test.go @@ -0,0 +1,50 @@ +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 TestBrainClientSendsBrainMCPToken(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" { + _, _ = w.Write([]byte(`{"path":"wiki/x/facts/y.md"}`)) + 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) + assert.Equal(t, []string{"/query Bearer cli-test-token", "/write 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/ingestion/cmd/server/main.go b/ingestion/cmd/server/main.go index bc38f11..b0193d0 100644 --- a/ingestion/cmd/server/main.go +++ b/ingestion/cmd/server/main.go @@ -504,17 +504,6 @@ func main() { } 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) if webhookHandler != nil { mux.Handle("POST /webhooks/brain-sync", webhookHandler) } @@ -532,6 +521,13 @@ func main() { } 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) + }) // POST /capture (#53/#54): the uniform capture REST door. Needs a ticket // tracker to file action items, so it only mounts when Gitea is diff --git a/ingestion/cmd/server/routes.go b/ingestion/cmd/server/routes.go new file mode 100644 index 0000000..b4584db --- /dev/null +++ b/ingestion/cmd/server/routes.go @@ -0,0 +1,30 @@ +package main + +import ( + "net/http" + + "github.com/mathiasbq/hyperguild/ingestion/internal/api" +) + +// 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. +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..4e24265 --- /dev/null +++ b/ingestion/cmd/server/routes_test.go @@ -0,0 +1,74 @@ +package main + +import ( + "log/slog" + "net/http" + "net/http/httptest" + "os" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + + chassisauth "git.d-ma.be/mathias/mcp-chassis/auth" + + "github.com/mathiasbq/hyperguild/ingestion/internal/api" + "github.com/mathiasbq/hyperguild/ingestion/internal/pipeline" +) + +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 { + 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) + }) + return mux +} + +func do(t *testing.T, h http.Handler, method, path, authz string) int { + 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 +} + +func TestRESTRoutesRefuseRequestsWithoutAToken(t *testing.T) { + r := testRouter(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) + } +} + +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)) +} + +// 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", "")) +} + +// 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", "")) +} 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) + } +} -- 2.54.0 From 21a53c4159e9e789da5d877ca772e996506fa96c Mon Sep 17 00:00:00 2001 From: Mathias Date: Sat, 3 Oct 2026 16:17:45 +0200 Subject: [PATCH 2/3] fix(ingestion): confine /ingest-path and brain_ingest paths to brain/raw/ Both read a caller-chosen server path with extract.Text, so any file the pod could read (the ServiceAccount token, /etc/passwd) could be ingested into the brain and read back with brain_query. ResolveIngestPath resolves relative paths against brain/raw/ (the inbox the watcher already owns), evaluates symlinks, and refuses anything that lands outside. A directory ingest re-resolves every walked file, so a symlinked file inside raw/ cannot lead out. Existing ingest-path tests moved their fixtures under brain/raw/. Refs #149 Co-Authored-By: Claude Opus 5.5 --- ingestion/internal/api/handler.go | 12 ++++ ingestion/internal/api/handler_test.go | 10 +-- .../internal/api/ingestpath_docmark_test.go | 5 +- ingestion/internal/api/ingestpath_root.go | 37 ++++++++++ .../internal/api/ingestpath_root_test.go | 69 +++++++++++++++++++ ingestion/internal/mcp/handlers.go | 6 ++ ingestion/internal/mcp/ingest_path_test.go | 24 +++++++ 7 files changed, 157 insertions(+), 6 deletions(-) create mode 100644 ingestion/internal/api/ingestpath_root.go create mode 100644 ingestion/internal/api/ingestpath_root_test.go create mode 100644 ingestion/internal/mcp/ingest_path_test.go 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..59a963d --- /dev/null +++ b/ingestion/internal/api/ingestpath_root.go @@ -0,0 +1,37 @@ +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/") + +// 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 "", fmt.Errorf("%w: path is empty", ErrOutsideIngestRoot) + } + root, err := filepath.EvalSymlinks(filepath.Join(brainDir, "raw")) + if err != nil { + return "", fmt.Errorf("%w: inbox unavailable: %v", ErrOutsideIngestRoot, err) + } + 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) + } + 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..c6f8dee --- /dev/null +++ b/ingestion/internal/api/ingestpath_root_test.go @@ -0,0 +1,69 @@ +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") +} 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/") +} -- 2.54.0 From b5c227179fc1fe0c8bf64f82d532ee44d74d4f84 Mon Sep 17 00:00:00 2001 From: Mathias Date: Sat, 3 Oct 2026 16:32:20 +0200 Subject: [PATCH 3/3] fix(ingestion): close the PR #152 review findings on REST auth - main.go's wiring was untested (the test built its own copy of the router, so dropping auth in main or re-registering POST /query as public stayed green). Routing is now one function, buildMux, which main calls and the tests call: it constructs the bearer middleware itself, registers public handlers only for patterns on a fixed allowlist (a public REST pattern would beat the authenticated catch-all by specificity), refuses an empty token, and returns an http.Handler so nothing can add a route to the result. - hyperguild CLI: PassRate sent no token and would 401. Every request now goes through brainClient.do, which adds it; the test covers Query, Write and PassRate and is named for that. - ResolveIngestPath returns one refusal for every bad path; the raw EvalSymlinks error let a caller probe which server files exist and leaked the brain directory. - PROJECT.md (and derived adapters) and the pass-rate runbook state that the REST API needs BRAIN_MCP_TOKEN and is tailnet/in-cluster. Refs #149 Co-Authored-By: Claude Opus 5.5 --- .context/PROJECT.md | 6 +- .context/system-prompt.txt | 8 +- AGENTS.md | 8 +- CLAUDE.md | 6 +- cmd/hyperguild/http.go | 17 ++-- cmd/hyperguild/http_auth_test.go | 13 ++- docs/runbooks/exercising-pass-rate-traffic.md | 2 +- ingestion/cmd/server/main.go | 34 +++---- ingestion/cmd/server/routes.go | 59 ++++++++++-- ingestion/cmd/server/routes_test.go | 90 +++++++++++++------ ingestion/internal/api/ingestpath_root.go | 13 +-- .../internal/api/ingestpath_root_test.go | 19 ++++ 12 files changed, 200 insertions(+), 75 deletions(-) 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) + } +} -- 2.54.0