From 21a53c4159e9e789da5d877ca772e996506fa96c Mon Sep 17 00:00:00 2001 From: Mathias Date: Sat, 3 Oct 2026 16:17:45 +0200 Subject: [PATCH] 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/") +}