From c7a6429bae0c4e5d931aed022f1b7e702868f94a Mon Sep 17 00:00:00 2001 From: Mathias Date: Sun, 4 Oct 2026 22:17:44 +0200 Subject: [PATCH 1/2] fix(graph,wiki): read the brain through brainfs (#157, slice 4) graphsync.IndexDoc/BackfillFromBrainDir and wiki.LoadInventory/ RebuildIndex read notes by path (os.ReadFile, os.Open, os.ReadDir + open). A symlinked note put the target's title, aliases and wikilinks into the knowledge graph, the wiki inventory (and the LLM prompts built from it) and wiki/index.md; a FIFO hung each read. - graphsync: IndexDoc and the backfill read through brainfs; the backfill walks with WalkRegular, skips a missing root and aborts on any other walk error, including a symlinked root. - wiki: LoadInventory lists type dirs with the new brainfs.ReadDirRegular (every component a real dir, regular files only) and reads frontmatter through the root; a symlinked type dir is an error. RebuildIndex reads first sentences through the root. - brainfstest's outside file now carries the sentinel in frontmatter title and aliases, a heading and a wikilink, so extractors that ignore the body are tested too. Co-Authored-By: Claude Opus 5.5 --- ingestion/internal/brainfs/brainfs.go | 22 ++++++ .../brainfs/brainfstest/brainfstest.go | 7 +- ingestion/internal/graphsync/graphsync.go | 58 +++++++++------ .../internal/graphsync/graphsync_root_test.go | 72 +++++++++++++++++++ ingestion/internal/wiki/index.go | 15 ++-- ingestion/internal/wiki/inventory.go | 38 ++++++---- ingestion/internal/wiki/root_test.go | 71 ++++++++++++++++++ 7 files changed, 241 insertions(+), 42 deletions(-) create mode 100644 ingestion/internal/graphsync/graphsync_root_test.go create mode 100644 ingestion/internal/wiki/root_test.go diff --git a/ingestion/internal/brainfs/brainfs.go b/ingestion/internal/brainfs/brainfs.go index 5419b43..3b1cb5c 100644 --- a/ingestion/internal/brainfs/brainfs.go +++ b/ingestion/internal/brainfs/brainfs.go @@ -82,6 +82,28 @@ func (b *Brain) realDir(rel string) error { return nil } +// ReadDirRegular lists the regular files directly in a brain-relative +// directory, by name. Every component of relDir must be a real directory +// (ErrNotRegular otherwise); symlinks, FIFOs, devices and subdirectories in +// it are left out. +func (b *Brain) ReadDirRegular(relDir string) ([]string, error) { + relDir = filepath.ToSlash(relDir) + if err := b.realDir(relDir); err != nil { + return nil, err + } + ents, err := fs.ReadDir(b.root.FS(), relDir) + if err != nil { + return nil, err + } + var names []string + for _, e := range ents { + if e.Type().IsRegular() { + names = append(names, e.Name()) + } + } + return names, nil +} + // ReadRegular reads a brain-relative path, refusing anything that is not a // regular file, both before the open and on the opened handle. func (b *Brain) ReadRegular(rel string) ([]byte, error) { diff --git a/ingestion/internal/brainfs/brainfstest/brainfstest.go b/ingestion/internal/brainfs/brainfstest/brainfstest.go index 3cf4e14..45a6afe 100644 --- a/ingestion/internal/brainfs/brainfstest/brainfstest.go +++ b/ingestion/internal/brainfs/brainfstest/brainfstest.go @@ -20,11 +20,16 @@ import ( // Sentinel marks the outside file's contents. const Sentinel = "zqbrainfssentinelonefiftyseven" +// outsideContent carries Sentinel in every field a reader might extract: the +// frontmatter title and aliases, a heading, a wikilink and the body. +const outsideContent = "---\ntitle: " + Sentinel + "\naliases:\n - " + Sentinel + "\n---\n# " + Sentinel + + "\n\n" + Sentinel + " [[" + Sentinel + "]]\n" + // Outside writes a file outside any brain dir containing Sentinel. func Outside(t *testing.T) string { t.Helper() p := filepath.Join(t.TempDir(), "secret.md") - if err := os.WriteFile(p, []byte("# secret\n\n"+Sentinel), 0o644); err != nil { + if err := os.WriteFile(p, []byte(outsideContent), 0o644); err != nil { t.Fatal(err) } return p diff --git a/ingestion/internal/graphsync/graphsync.go b/ingestion/internal/graphsync/graphsync.go index 4677515..763400e 100644 --- a/ingestion/internal/graphsync/graphsync.go +++ b/ingestion/internal/graphsync/graphsync.go @@ -12,10 +12,12 @@ package graphsync import ( "context" + "errors" "fmt" - "os" - "path/filepath" + "io/fs" + "path" + "github.com/mathiasbq/hyperguild/ingestion/internal/brainfs" "github.com/mathiasbq/hyperguild/ingestion/internal/graph" "github.com/mathiasbq/hyperguild/ingestion/internal/graphstore" ) @@ -46,8 +48,19 @@ func IndexDoc(ctx context.Context, store Store, brainDir, relPath string) error if relPath == "" { return nil } - abs := filepath.Join(brainDir, filepath.FromSlash(relPath)) - content, err := os.ReadFile(abs) + bfs, err := brainfs.Open(brainDir) + if err != nil { + return fmt.Errorf("open brain: %w", err) + } + defer func() { _ = bfs.Close() }() + return indexDoc(ctx, store, bfs, relPath) +} + +// indexDoc reads relPath through the brain root, regular files only +// (hyperguild#157): the brain is a Syncthing folder, and os.ReadFile +// followed a synced symlink into the graph and blocked on a FIFO. +func indexDoc(ctx context.Context, store Store, bfs *brainfs.Brain, relPath string) error { + content, err := bfs.ReadRegular(relPath) if err != nil { return fmt.Errorf("read %q: %w", relPath, err) } @@ -77,33 +90,34 @@ func BackfillFromBrainDir(ctx context.Context, store Store, brainDir string) (in if store == nil { return 0, nil } - roots := []string{"wiki", "knowledge"} - for _, root := range roots { - base := filepath.Join(brainDir, root) - if _, err := os.Stat(base); os.IsNotExist(err) { + bfs, err := brainfs.Open(brainDir) + if err != nil { + return 0, fmt.Errorf("open brain: %w", err) + } + defer func() { _ = bfs.Close() }() + for _, root := range []string{"wiki", "knowledge"} { + if _, err := bfs.Lstat(root); errors.Is(err, fs.ErrNotExist) { continue } - err := filepath.WalkDir(base, func(path string, d os.DirEntry, walkErr error) error { - if walkErr != nil { - return walkErr - } - if d.IsDir() { + // Any walk error, including a root that is a symlink, aborts. + var walkErr error + err := bfs.WalkRegular(root, func(rel string) error { + if path.Ext(rel) != ".md" { return nil } - if filepath.Ext(path) != ".md" { - return nil - } - rel, relErr := filepath.Rel(brainDir, path) - if relErr != nil { - return fmt.Errorf("rel %q: %w", path, relErr) - } - rel = filepath.ToSlash(rel) - if err := IndexDoc(ctx, store, brainDir, rel); err != nil { + if err := indexDoc(ctx, store, bfs, rel); err != nil { return fmt.Errorf("index %q: %w", rel, err) } indexed++ return nil + }, func(rel string, err error) { + if walkErr == nil { + walkErr = fmt.Errorf("%s: %w", rel, err) + } }) + if err == nil { + err = walkErr + } if err != nil { return indexed, fmt.Errorf("walk %s: %w", root, err) } diff --git a/ingestion/internal/graphsync/graphsync_root_test.go b/ingestion/internal/graphsync/graphsync_root_test.go new file mode 100644 index 0000000..84f11e6 --- /dev/null +++ b/ingestion/internal/graphsync/graphsync_root_test.go @@ -0,0 +1,72 @@ +package graphsync + +import ( + "context" + "fmt" + "os" + "path/filepath" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/mathiasbq/hyperguild/ingestion/internal/brainfs/brainfstest" +) + +func (f *fakeStore) dump() string { + f.mu.Lock() + defer f.mu.Unlock() + return fmt.Sprintf("%+v %+v", f.upserts, f.replaces) +} + +// hyperguild#157 slice 4: IndexDoc and BackfillFromBrainDir read notes with +// os.ReadFile, so a symlinked note put the target's title and wikilinks into +// the graph, and a FIFO hung the read. +func TestIndexDocRefusesSymlinksAndSurvivesSwapRaces(t *testing.T) { + brain := t.TempDir() + facts := filepath.Join(brain, "wiki", "w", "facts") + require.NoError(t, os.MkdirAll(facts, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(facts, "ok.md"), []byte("---\ntitle: zqinside\n---\nbody [[other]]\n"), 0o644)) + require.NoError(t, os.Symlink(brainfstest.Outside(t), filepath.Join(facts, "leak.md"))) + + store := newFakeStore() + require.NoError(t, IndexDoc(context.Background(), store, brain, "wiki/w/facts/ok.md")) + assert.Contains(t, store.dump(), "zqinside", "positive control: a regular note is indexed") + assert.Error(t, IndexDoc(context.Background(), store, brain, "wiki/w/facts/leak.md")) + assert.NotContains(t, store.dump(), brainfstest.Sentinel) + + p := filepath.Join(facts, "r.md") + read := func() string { + s := newFakeStore() + _ = IndexDoc(context.Background(), s, brain, "wiki/w/facts/r.md") + return s.dump() + } + brainfstest.AssertNoLeak(t, p, "---\ntitle: zqinside\n---\n", 1500*time.Millisecond, read) + brainfstest.AssertNoHang(t, p, "---\ntitle: zqinside\n---\n", 1500*time.Millisecond, 6*time.Second, func() { _ = read() }) +} + +func TestBackfillSkipsSymlinkedNotesAndASymlinkedRoot(t *testing.T) { + brain := t.TempDir() + facts := filepath.Join(brain, "wiki", "w", "facts") + require.NoError(t, os.MkdirAll(facts, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(facts, "ok.md"), []byte("---\ntitle: zqinside\n---\n"), 0o644)) + require.NoError(t, os.Symlink(brainfstest.Outside(t), filepath.Join(facts, "leak.md"))) + require.NoError(t, os.MkdirAll(filepath.Join(brain, "raw"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(brain, "raw", "r.md"), []byte("---\ntitle: zqrawonly\n---\n"), 0o644)) + require.NoError(t, os.Symlink(".", filepath.Join(brain, "knowledge"))) + + store := newFakeStore() + _, err := BackfillFromBrainDir(context.Background(), store, brain) + assert.Error(t, err, "a symlinked root aborts the backfill") + assert.NotContains(t, store.dump(), brainfstest.Sentinel) + assert.NotContains(t, store.dump(), "zqrawonly") + + require.NoError(t, os.Remove(filepath.Join(brain, "knowledge"))) + store = newFakeStore() + n, err := BackfillFromBrainDir(context.Background(), store, brain) + require.NoError(t, err) + assert.Equal(t, 1, n, "only the regular note is indexed") + assert.Contains(t, store.dump(), "zqinside") + assert.NotContains(t, store.dump(), brainfstest.Sentinel) +} diff --git a/ingestion/internal/wiki/index.go b/ingestion/internal/wiki/index.go index 3a0ea2f..0bab797 100644 --- a/ingestion/internal/wiki/index.go +++ b/ingestion/internal/wiki/index.go @@ -7,6 +7,7 @@ import ( "path/filepath" "strings" + "github.com/mathiasbq/hyperguild/ingestion/internal/brainfs" "github.com/mathiasbq/hyperguild/ingestion/internal/textutil" ) @@ -16,6 +17,11 @@ func RebuildIndex(brainDir, date string) error { if err != nil { return fmt.Errorf("load inventory: %w", err) } + bfs, err := brainfs.Open(brainDir) + if err != nil { + return fmt.Errorf("open brain: %w", err) + } + defer func() { _ = bfs.Close() }() total := len(inv[PageTypeConcept]) + len(inv[PageTypeEntity]) + len(inv[PageTypeSource]) var sb strings.Builder @@ -34,7 +40,7 @@ func RebuildIndex(brainDir, date string) error { label := strings.ToUpper(string(pt)[:1]) + string(pt)[1:] fmt.Fprintf(&sb, "## %s\n\n", label) for _, e := range entries { - summary := pageFirstSentence(brainDir, e) + summary := pageFirstSentence(bfs, e) if summary != "" { fmt.Fprintf(&sb, "- [[%s|%s]] — %s\n", e.Slug, e.Title, summary) } else { @@ -48,9 +54,10 @@ func RebuildIndex(brainDir, date string) error { return os.WriteFile(dest, []byte(sb.String()), 0o644) } -func pageFirstSentence(brainDir string, e Entry) string { - path := filepath.Join(brainDir, "wiki", string(e.Type), e.Slug+".md") - content, err := os.ReadFile(path) +// pageFirstSentence reads the page through the brain root, regular files +// only (hyperguild#157). +func pageFirstSentence(bfs *brainfs.Brain, e Entry) string { + content, err := bfs.ReadRegular("wiki/" + string(e.Type) + "/" + e.Slug + ".md") if err != nil { return "" } diff --git a/ingestion/internal/wiki/inventory.go b/ingestion/internal/wiki/inventory.go index 0d83c31..00b67cd 100644 --- a/ingestion/internal/wiki/inventory.go +++ b/ingestion/internal/wiki/inventory.go @@ -3,36 +3,45 @@ package wiki import ( "bufio" + "bytes" + "errors" "fmt" - "os" - "path/filepath" + "io/fs" "strings" + + "github.com/mathiasbq/hyperguild/ingestion/internal/brainfs" ) // LoadInventory walks brain/wiki/ and returns all pages grouped by type. -// Missing subdirectories are silently skipped. +// Missing subdirectories are silently skipped. Pages are listed and read +// through the brain root, regular files only (hyperguild#157); a type +// directory that is a symlink is an error. func LoadInventory(brainDir string) (map[PageType][]Entry, error) { result := map[PageType][]Entry{ PageTypeConcept: {}, PageTypeEntity: {}, PageTypeSource: {}, } + bfs, err := brainfs.Open(brainDir) + if err != nil { + return nil, fmt.Errorf("open brain: %w", err) + } + defer func() { _ = bfs.Close() }() for pt := range result { - dir := filepath.Join(brainDir, "wiki", string(pt)) - entries, err := os.ReadDir(dir) - if os.IsNotExist(err) { + dir := "wiki/" + string(pt) + names, err := bfs.ReadDirRegular(dir) + if errors.Is(err, fs.ErrNotExist) { continue } if err != nil { return nil, fmt.Errorf("read dir %s: %w", dir, err) } - for _, e := range entries { - if e.IsDir() || !strings.HasSuffix(e.Name(), ".md") { + for _, name := range names { + if !strings.HasSuffix(name, ".md") { continue } - slug := strings.TrimSuffix(e.Name(), ".md") - path := filepath.Join(dir, e.Name()) - title, aliases := readFrontmatter(path, slug) + slug := strings.TrimSuffix(name, ".md") + title, aliases := readFrontmatter(bfs, dir+"/"+name, slug) result[pt] = append(result[pt], Entry{Slug: slug, Title: title, Aliases: aliases, Type: pt}) } } @@ -41,15 +50,14 @@ func LoadInventory(brainDir string) (map[PageType][]Entry, error) { // readFrontmatter extracts title and aliases from YAML frontmatter. // Falls back to slug for title and empty aliases on any error. -func readFrontmatter(path, fallbackSlug string) (title string, aliases []string) { +func readFrontmatter(bfs *brainfs.Brain, rel, fallbackSlug string) (title string, aliases []string) { title = fallbackSlug - f, err := os.Open(path) + content, err := bfs.ReadRegular(rel) if err != nil { return } - defer f.Close() //nolint:errcheck - scanner := bufio.NewScanner(f) + scanner := bufio.NewScanner(bytes.NewReader(content)) inFM := false inAliases := false for scanner.Scan() { diff --git a/ingestion/internal/wiki/root_test.go b/ingestion/internal/wiki/root_test.go new file mode 100644 index 0000000..63af2cb --- /dev/null +++ b/ingestion/internal/wiki/root_test.go @@ -0,0 +1,71 @@ +package wiki + +import ( + "fmt" + "os" + "path/filepath" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/mathiasbq/hyperguild/ingestion/internal/brainfs/brainfstest" +) + +// hyperguild#157 slice 4: LoadInventory listed wiki// with os.ReadDir +// and opened each .md by path, so a symlinked page put the target's title and +// aliases into the inventory (and the LLM prompts built from it), and a FIFO +// hung it. RebuildIndex read first sentences the same way. +func TestLoadInventoryRefusesSymlinksAndSurvivesSwapRaces(t *testing.T) { + brain := t.TempDir() + dir := filepath.Join(brain, "wiki", "concepts") + require.NoError(t, os.MkdirAll(dir, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "ok.md"), []byte("---\ntitle: zqinside\n---\n"), 0o644)) + require.NoError(t, os.Symlink(brainfstest.Outside(t), filepath.Join(dir, "leak.md"))) + + inv, err := LoadInventory(brain) + require.NoError(t, err) + got := fmt.Sprintf("%+v", inv) + assert.Contains(t, got, "zqinside", "positive control") + assert.NotContains(t, got, brainfstest.Sentinel) + assert.NotContains(t, got, "leak", "a symlinked page is not listed") + + p := filepath.Join(dir, "r.md") + read := func() string { + inv, _ := LoadInventory(brain) + return fmt.Sprintf("%+v", inv) + } + brainfstest.AssertNoLeak(t, p, "---\ntitle: zqinside\n---\n", 1500*time.Millisecond, read) + brainfstest.AssertNoHang(t, p, "---\ntitle: zqinside\n---\n", 1500*time.Millisecond, 6*time.Second, func() { _ = read() }) +} + +func TestLoadInventorySkipsASymlinkedTypeDir(t *testing.T) { + brain := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(brain, "wiki"), 0o755)) + require.NoError(t, os.MkdirAll(filepath.Join(brain, "raw"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(brain, "raw", "r.md"), []byte("---\ntitle: zqrawonly\n---\n"), 0o644)) + require.NoError(t, os.Symlink(filepath.Join("..", "raw"), filepath.Join(brain, "wiki", "concepts"))) + inv, _ := LoadInventory(brain) + assert.NotContains(t, fmt.Sprintf("%+v", inv), "zqrawonly") +} + +func TestRebuildIndexDoesNotReadAPageSwappedForASymlink(t *testing.T) { + brain := t.TempDir() + dir := filepath.Join(brain, "wiki", "concepts") + require.NoError(t, os.MkdirAll(dir, 0o755)) + p := filepath.Join(dir, "r.md") + require.NoError(t, os.WriteFile(p, []byte("---\ntitle: R\n---\nzqinside sentence.\n"), 0o644)) + require.NoError(t, RebuildIndex(brain, "2026-10-04")) + idx, err := os.ReadFile(filepath.Join(brain, "wiki", "index.md")) + require.NoError(t, err) + require.Contains(t, string(idx), "zqinside", "positive control: the summary is read") + + read := func() string { + _ = RebuildIndex(brain, "2026-10-04") + b, _ := os.ReadFile(filepath.Join(brain, "wiki", "index.md")) + return string(b) + } + brainfstest.AssertNoLeak(t, p, "---\ntitle: R\n---\nzqinside sentence.\n", 1500*time.Millisecond, read) + brainfstest.AssertNoHang(t, p, "---\ntitle: R\n---\nzqinside sentence.\n", 1500*time.Millisecond, 6*time.Second, func() { _ = read() }) +} -- 2.54.0 From d90ce66fb282923fa4bfccfadba482ee6a8bb5fa Mon Sep 17 00:00:00 2001 From: Mathias Date: Sun, 4 Oct 2026 22:34:46 +0200 Subject: [PATCH 2/2] fix(brainfs): refuse an in-brain file swapped in behind a symlink; in-brain test targets (#157) Review round 1 on PR #168: - ReadRegular/ReadRegularMax Lstat the path, then open it again by path; os.Root follows an in-root final symlink even with O_NOFOLLOW, so a symlink to another brain file swapped in between was read (8k-109k raw/ leaks in RebuildIndex, LoadInventory, IndexDoc). The opened handle must now be the same file as the Lstat (os.SameFile). This covers every ReadRegular caller, slices 1-3 included. - Every symlink test pointed outside the brain, which os.Root refuses on its own. Each now has an in-brain case too: a static in-brain link per reader, AssertNoLeakFrom with an in-brain target, and brainfs unit tests for ReadDirRegular and the in-brain swap. - brainfstest: swap links are relative (os.Root refuses every absolute link, so an absolute in-brain target tested nothing), and each swapper has its own temp name (two swappers on one path deadlocked on each other's FIFO). - IndexDoc indexes only a clean wiki/ or knowledge/ path (callers pass caller-supplied paths). - Rule 9: TestLoadInventoryReturnsErrNotRegularForASymlinkedTypeDir now asserts the error; the backfill test says it aborts on a symlinked root. Co-Authored-By: Claude Opus 5.5 --- ingestion/internal/brainfs/brainfs.go | 15 +++- ingestion/internal/brainfs/brainfs_test.go | 73 +++++++++++++++++++ .../brainfs/brainfstest/brainfstest.go | 41 ++++++++++- ingestion/internal/graphsync/graphsync.go | 8 ++ .../internal/graphsync/graphsync_root_test.go | 22 +++++- ingestion/internal/wiki/root_test.go | 12 ++- 6 files changed, 160 insertions(+), 11 deletions(-) diff --git a/ingestion/internal/brainfs/brainfs.go b/ingestion/internal/brainfs/brainfs.go index 3b1cb5c..fb1b76c 100644 --- a/ingestion/internal/brainfs/brainfs.go +++ b/ingestion/internal/brainfs/brainfs.go @@ -114,7 +114,7 @@ func (b *Brain) ReadRegular(rel string) ([]byte, error) { if !fi.Mode().IsRegular() { return nil, fmt.Errorf("%s: %w", rel, ErrNotRegular) } - return b.readOpened(rel) + return b.readOpenedMax(rel, -1, fi) } // readOpened opens rel non-blocking and reads it only if the OPENED handle is @@ -128,7 +128,7 @@ func (b *Brain) ReadRegular(rel string) ([]byte, error) { // cannot create one; fs.protected_hardlinks=1 and the pod's secrets are on a // different filesystem from the brain volume, so a local caller cannot link // them in. Refusing Nlink > 1 is the next step if either changes. -func (b *Brain) readOpened(rel string) ([]byte, error) { return b.readOpenedMax(rel, -1) } +func (b *Brain) readOpened(rel string) ([]byte, error) { return b.readOpenedMax(rel, -1, nil) } // ReadRegularMax is ReadRegular with a size limit, checked on the opened // handle before reading (max < 0 means no limit). @@ -140,10 +140,14 @@ func (b *Brain) ReadRegularMax(rel string, max int64) ([]byte, error) { if !fi.Mode().IsRegular() { return nil, fmt.Errorf("%s: %w", rel, ErrNotRegular) } - return b.readOpenedMax(rel, max) + return b.readOpenedMax(rel, max, fi) } -func (b *Brain) readOpenedMax(rel string, max int64) ([]byte, error) { +// readOpenedMax opens rel and reads it if the opened handle is a regular file +// and, when lfi is set, the same file the caller Lstat-ed: os.Root follows an +// in-root final symlink even with O_NOFOLLOW, so a symlink to another brain +// file swapped in between the Lstat and the open is refused only here. +func (b *Brain) readOpenedMax(rel string, max int64, lfi fs.FileInfo) ([]byte, error) { f, err := b.root.OpenFile(rel, os.O_RDONLY|syscall.O_NONBLOCK, 0) if err != nil { return nil, err @@ -156,6 +160,9 @@ func (b *Brain) readOpenedMax(rel string, max int64) ([]byte, error) { if !hfi.Mode().IsRegular() { return nil, fmt.Errorf("%s: %w", rel, ErrNotRegular) } + if lfi != nil && !os.SameFile(lfi, hfi) { + return nil, fmt.Errorf("%s: changed while opening: %w", rel, ErrNotRegular) + } if max >= 0 && hfi.Size() > max { return nil, fmt.Errorf("%s is %d bytes (limit %d): %w", rel, hfi.Size(), max, ErrTooLarge) } diff --git a/ingestion/internal/brainfs/brainfs_test.go b/ingestion/internal/brainfs/brainfs_test.go index 57284f7..7146af8 100644 --- a/ingestion/internal/brainfs/brainfs_test.go +++ b/ingestion/internal/brainfs/brainfs_test.go @@ -6,6 +6,8 @@ import ( "io/fs" "os" "path/filepath" + "sync" + "sync/atomic" "syscall" "testing" "time" @@ -189,3 +191,74 @@ func TestWalkRegularReportsASymlinkedIntermediateComponentOfTheWalkRoot(t *testi require.Len(t, errs, 1) assert.ErrorIs(t, errs[0], ErrNotRegular) } + +func TestReadDirRegularListsOnlyRegularFilesAndRefusesSymlinkedComponents(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(dir, "a", "b", "sub"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "a", "b", "n.md"), []byte("n"), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "other.md"), []byte("o"), 0o644)) + require.NoError(t, os.Symlink(filepath.Join("..", "..", "other.md"), filepath.Join(dir, "a", "b", "l.md"))) + require.NoError(t, syscall.Mkfifo(filepath.Join(dir, "a", "b", "f.md"), 0o644)) + require.NoError(t, os.Symlink("a", filepath.Join(dir, "alias"))) + require.NoError(t, os.MkdirAll(filepath.Join(dir, "x"), 0o755)) + require.NoError(t, os.Symlink(filepath.Join("..", "a"), filepath.Join(dir, "x", "mid"))) + b, err := Open(dir) + require.NoError(t, err) + defer func() { _ = b.Close() }() + + names, err := b.ReadDirRegular("a/b") + require.NoError(t, err) + assert.Equal(t, []string{"n.md"}, names) + for _, rel := range []string{"alias/b", "x/mid/b"} { + _, err := b.ReadDirRegular(rel) + assert.ErrorIs(t, err, ErrNotRegular, rel) + } +} + +// A final component swapped for a symlink to another regular file in the +// brain between the Lstat and the open: os.Root follows in-root links, so +// only comparing the two (os.SameFile) refuses it. +func TestReadRegularNeverReturnsAnInBrainFileSwappedInBehindASymlink(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, "s.md"), []byte("zqsamefilesentinel"), 0o644)) + p := filepath.Join(dir, "r.md") + require.NoError(t, os.WriteFile(p, []byte("inside"), 0o644)) + b, err := Open(dir) + require.NoError(t, err) + defer func() { _ = b.Close() }() + + var stop atomic.Bool + var wg sync.WaitGroup + wg.Add(1) + go func() { + defer wg.Done() + tmp := p + ".swap" + for i := 0; !stop.Load(); i++ { + _ = os.Remove(tmp) + if i%2 == 0 { + _ = os.WriteFile(tmp, []byte("inside"), 0o644) + } else { + _ = os.Symlink("s.md", tmp) + } + _ = os.Rename(tmp, p) + } + }() + leaks, ok := 0, 0 + for deadline := time.Now().Add(1500 * time.Millisecond); time.Now().Before(deadline); { + got, err := b.ReadRegular("r.md") + if err == nil { + ok++ + } + if string(got) == "zqsamefilesentinel" { + leaks++ + } + got, _ = b.ReadRegularMax("r.md", 1<<20) + if string(got) == "zqsamefilesentinel" { + leaks++ + } + } + stop.Store(true) + wg.Wait() + assert.Zero(t, leaks) + assert.Positive(t, ok, "positive control: the regular file is read") +} diff --git a/ingestion/internal/brainfs/brainfstest/brainfstest.go b/ingestion/internal/brainfs/brainfstest/brainfstest.go index 45a6afe..fdfe968 100644 --- a/ingestion/internal/brainfs/brainfstest/brainfstest.go +++ b/ingestion/internal/brainfs/brainfstest/brainfstest.go @@ -7,6 +7,7 @@ package brainfstest import ( + "fmt" "os" "path/filepath" "strings" @@ -35,6 +36,8 @@ func Outside(t *testing.T) string { return p } +var swapSeq atomic.Int64 + // StartSwap renames, in a loop, a regular note (content inside) and either // a symlink to outside or a FIFO into path, until the test ends. FIFOs get a // writer held open until the swapper exits, so a reader that skips the @@ -47,7 +50,15 @@ func StartSwap(t *testing.T, path, inside, outside string, fifo bool) { wg.Add(1) go func() { defer wg.Done() - tmp := path + ".swap" + // A name of its own: swappers on one path share a test, and one + // opening another's FIFO for write would block forever. + tmp := fmt.Sprintf("%s.swap%d", path, swapSeq.Add(1)) + // A relative link: os.Root refuses every absolute link on its own, so + // an absolute in-brain target would test nothing. + link := outside + if rel, err := filepath.Rel(filepath.Dir(path), outside); err == nil { + link = rel + } var writers []*os.File defer func() { for _, w := range writers { @@ -66,7 +77,7 @@ func StartSwap(t *testing.T, path, inside, outside string, fifo bool) { } } default: - _ = os.Symlink(outside, tmp) + _ = os.Symlink(link, tmp) } _ = os.Rename(tmp, path) } @@ -74,11 +85,33 @@ func StartSwap(t *testing.T, path, inside, outside string, fifo bool) { t.Cleanup(func() { stop.Store(true); wg.Wait() }) } +// WriteSentinelAt writes a file carrying Sentinel at p (creating its +// directory), for an in-brain symlink target. +func WriteSentinelAt(t *testing.T, p string) string { + t.Helper() + if err := os.MkdirAll(filepath.Dir(p), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(p, []byte(outsideContent), 0o644); err != nil { + t.Fatal(err) + } + return p +} + // AssertNoLeak loops read for d while a symlink swapper runs on path, and -// fails if any output contains Sentinel. +// fails if any output contains Sentinel. The symlink points outside the +// brain; os.Root refuses that on its own, so pair it with AssertNoLeakFrom +// and an in-brain target. func AssertNoLeak(t *testing.T, path, inside string, d time.Duration, read func() string) { t.Helper() - StartSwap(t, path, inside, Outside(t), false) + AssertNoLeakFrom(t, path, inside, Outside(t), d, read) +} + +// AssertNoLeakFrom is AssertNoLeak with the symlink pointing at target, e.g. +// a file elsewhere in the brain written with WriteSentinelAt. +func AssertNoLeakFrom(t *testing.T, path, inside, target string, d time.Duration, read func() string) { + t.Helper() + StartSwap(t, path, inside, target, false) leaks, iters := 0, 0 for deadline := time.Now().Add(d); time.Now().Before(deadline); iters++ { leaks += strings.Count(read(), Sentinel) diff --git a/ingestion/internal/graphsync/graphsync.go b/ingestion/internal/graphsync/graphsync.go index 763400e..882bf99 100644 --- a/ingestion/internal/graphsync/graphsync.go +++ b/ingestion/internal/graphsync/graphsync.go @@ -16,6 +16,8 @@ import ( "fmt" "io/fs" "path" + "path/filepath" + "strings" "github.com/mathiasbq/hyperguild/ingestion/internal/brainfs" "github.com/mathiasbq/hyperguild/ingestion/internal/graph" @@ -48,6 +50,12 @@ func IndexDoc(ctx context.Context, store Store, brainDir, relPath string) error if relPath == "" { return nil } + // Callers pass caller-supplied paths (brain_tunnel, brain_update): index + // only a clean path under wiki/ or knowledge/. + if path.Clean(relPath) != relPath || !filepath.IsLocal(relPath) || + (!strings.HasPrefix(relPath, "wiki/") && !strings.HasPrefix(relPath, "knowledge/")) { + return fmt.Errorf("index %q: not a clean wiki/ or knowledge/ path", relPath) + } bfs, err := brainfs.Open(brainDir) if err != nil { return fmt.Errorf("open brain: %w", err) diff --git a/ingestion/internal/graphsync/graphsync_root_test.go b/ingestion/internal/graphsync/graphsync_root_test.go index 84f11e6..f421de2 100644 --- a/ingestion/internal/graphsync/graphsync_root_test.go +++ b/ingestion/internal/graphsync/graphsync_root_test.go @@ -34,6 +34,11 @@ func TestIndexDocRefusesSymlinksAndSurvivesSwapRaces(t *testing.T) { require.NoError(t, IndexDoc(context.Background(), store, brain, "wiki/w/facts/ok.md")) assert.Contains(t, store.dump(), "zqinside", "positive control: a regular note is indexed") assert.Error(t, IndexDoc(context.Background(), store, brain, "wiki/w/facts/leak.md")) + // A link to a file elsewhere in the brain: os.Root allows it, only the + // regular-file check refuses it. + inBrain := brainfstest.WriteSentinelAt(t, filepath.Join(brain, "raw", "s.md")) + require.NoError(t, os.Symlink(filepath.Join("..", "..", "..", "raw", "s.md"), filepath.Join(facts, "inlink.md"))) + assert.Error(t, IndexDoc(context.Background(), store, brain, "wiki/w/facts/inlink.md")) assert.NotContains(t, store.dump(), brainfstest.Sentinel) p := filepath.Join(facts, "r.md") @@ -43,10 +48,11 @@ func TestIndexDocRefusesSymlinksAndSurvivesSwapRaces(t *testing.T) { return s.dump() } brainfstest.AssertNoLeak(t, p, "---\ntitle: zqinside\n---\n", 1500*time.Millisecond, read) + brainfstest.AssertNoLeakFrom(t, p, "---\ntitle: zqinside\n---\n", inBrain, 1500*time.Millisecond, read) brainfstest.AssertNoHang(t, p, "---\ntitle: zqinside\n---\n", 1500*time.Millisecond, 6*time.Second, func() { _ = read() }) } -func TestBackfillSkipsSymlinkedNotesAndASymlinkedRoot(t *testing.T) { +func TestBackfillSkipsSymlinkedNotesAndAbortsOnASymlinkedRoot(t *testing.T) { brain := t.TempDir() facts := filepath.Join(brain, "wiki", "w", "facts") require.NoError(t, os.MkdirAll(facts, 0o755)) @@ -54,6 +60,7 @@ func TestBackfillSkipsSymlinkedNotesAndASymlinkedRoot(t *testing.T) { require.NoError(t, os.Symlink(brainfstest.Outside(t), filepath.Join(facts, "leak.md"))) require.NoError(t, os.MkdirAll(filepath.Join(brain, "raw"), 0o755)) require.NoError(t, os.WriteFile(filepath.Join(brain, "raw", "r.md"), []byte("---\ntitle: zqrawonly\n---\n"), 0o644)) + require.NoError(t, os.Symlink(filepath.Join("..", "..", "..", "raw", "r.md"), filepath.Join(facts, "inlink.md"))) require.NoError(t, os.Symlink(".", filepath.Join(brain, "knowledge"))) store := newFakeStore() @@ -70,3 +77,16 @@ func TestBackfillSkipsSymlinkedNotesAndASymlinkedRoot(t *testing.T) { assert.Contains(t, store.dump(), "zqinside") assert.NotContains(t, store.dump(), brainfstest.Sentinel) } + +// IndexDoc gets caller-supplied paths (brain_tunnel, brain_update): only a +// clean path under wiki/ or knowledge/ is indexed. +func TestIndexDocRefusesPathsOutsideWikiAndKnowledgeAndUncleanPaths(t *testing.T) { + brain := t.TempDir() + brainfstest.WriteSentinelAt(t, filepath.Join(brain, "raw", "secret.md")) + require.NoError(t, os.MkdirAll(filepath.Join(brain, "wiki", "w"), 0o755)) + for _, p := range []string{"raw/secret.md", "wiki/w/../../raw/secret.md", "./raw/secret.md", "/raw/secret.md"} { + store := newFakeStore() + assert.Error(t, IndexDoc(context.Background(), store, brain, p), p) + assert.NotContains(t, store.dump(), brainfstest.Sentinel, p) + } +} diff --git a/ingestion/internal/wiki/root_test.go b/ingestion/internal/wiki/root_test.go index 63af2cb..5baecce 100644 --- a/ingestion/internal/wiki/root_test.go +++ b/ingestion/internal/wiki/root_test.go @@ -10,6 +10,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/mathiasbq/hyperguild/ingestion/internal/brainfs" "github.com/mathiasbq/hyperguild/ingestion/internal/brainfs/brainfstest" ) @@ -23,6 +24,8 @@ func TestLoadInventoryRefusesSymlinksAndSurvivesSwapRaces(t *testing.T) { require.NoError(t, os.MkdirAll(dir, 0o755)) require.NoError(t, os.WriteFile(filepath.Join(dir, "ok.md"), []byte("---\ntitle: zqinside\n---\n"), 0o644)) require.NoError(t, os.Symlink(brainfstest.Outside(t), filepath.Join(dir, "leak.md"))) + inBrain := brainfstest.WriteSentinelAt(t, filepath.Join(brain, "raw", "s.md")) + require.NoError(t, os.Symlink(filepath.Join("..", "..", "raw", "s.md"), filepath.Join(dir, "inlink.md"))) inv, err := LoadInventory(brain) require.NoError(t, err) @@ -30,6 +33,7 @@ func TestLoadInventoryRefusesSymlinksAndSurvivesSwapRaces(t *testing.T) { assert.Contains(t, got, "zqinside", "positive control") assert.NotContains(t, got, brainfstest.Sentinel) assert.NotContains(t, got, "leak", "a symlinked page is not listed") + assert.NotContains(t, got, "inlink", "a page linked to another brain file is not listed") p := filepath.Join(dir, "r.md") read := func() string { @@ -37,16 +41,18 @@ func TestLoadInventoryRefusesSymlinksAndSurvivesSwapRaces(t *testing.T) { return fmt.Sprintf("%+v", inv) } brainfstest.AssertNoLeak(t, p, "---\ntitle: zqinside\n---\n", 1500*time.Millisecond, read) + brainfstest.AssertNoLeakFrom(t, p, "---\ntitle: zqinside\n---\n", inBrain, 1500*time.Millisecond, read) brainfstest.AssertNoHang(t, p, "---\ntitle: zqinside\n---\n", 1500*time.Millisecond, 6*time.Second, func() { _ = read() }) } -func TestLoadInventorySkipsASymlinkedTypeDir(t *testing.T) { +func TestLoadInventoryReturnsErrNotRegularForASymlinkedTypeDir(t *testing.T) { brain := t.TempDir() require.NoError(t, os.MkdirAll(filepath.Join(brain, "wiki"), 0o755)) require.NoError(t, os.MkdirAll(filepath.Join(brain, "raw"), 0o755)) require.NoError(t, os.WriteFile(filepath.Join(brain, "raw", "r.md"), []byte("---\ntitle: zqrawonly\n---\n"), 0o644)) require.NoError(t, os.Symlink(filepath.Join("..", "raw"), filepath.Join(brain, "wiki", "concepts"))) - inv, _ := LoadInventory(brain) + inv, err := LoadInventory(brain) + require.ErrorIs(t, err, brainfs.ErrNotRegular) assert.NotContains(t, fmt.Sprintf("%+v", inv), "zqrawonly") } @@ -67,5 +73,7 @@ func TestRebuildIndexDoesNotReadAPageSwappedForASymlink(t *testing.T) { return string(b) } brainfstest.AssertNoLeak(t, p, "---\ntitle: R\n---\nzqinside sentence.\n", 1500*time.Millisecond, read) + inBrain := brainfstest.WriteSentinelAt(t, filepath.Join(brain, "raw", "s.md")) + brainfstest.AssertNoLeakFrom(t, p, "---\ntitle: R\n---\nzqinside sentence.\n", inBrain, 1500*time.Millisecond, read) brainfstest.AssertNoHang(t, p, "---\ntitle: R\n---\nzqinside sentence.\n", 1500*time.Millisecond, 6*time.Second, func() { _ = read() }) } -- 2.54.0