diff --git a/ingestion/internal/api/readnote_root_test.go b/ingestion/internal/api/readnote_root_test.go index 94fedff..24ac216 100644 --- a/ingestion/internal/api/readnote_root_test.go +++ b/ingestion/internal/api/readnote_root_test.go @@ -23,6 +23,13 @@ func TestReadNoteRefusesSymlinksAndSurvivesSwapRaces(t *testing.T) { _, body, _, err := api.ReadNote(brain, "wiki/w/facts/leak.md") assert.Error(t, err) assert.NotContains(t, body, brainfstest.Sentinel) + // A link to a file elsewhere in the brain: os.Root allows it, only the + // regular-file check refuses it (hyperguild#171). + brainfstest.WriteSentinelAt(t, filepath.Join(brain, "raw", "s.md")) + require.NoError(t, os.Symlink(filepath.Join("..", "..", "..", "raw", "s.md"), filepath.Join(facts, "inlink.md"))) + _, body, _, err = api.ReadNote(brain, "wiki/w/facts/inlink.md") + assert.Error(t, err) + assert.NotContains(t, body, brainfstest.Sentinel) p := filepath.Join(facts, "r.md") require.NoError(t, os.WriteFile(p, []byte("---\nx: y\n---\ninside"), 0o644)) @@ -39,5 +46,7 @@ func TestReadNoteRefusesSymlinksAndSurvivesSwapRaces(t *testing.T) { return s } brainfstest.AssertNoLeak(t, p, "---\nx: y\n---\ninside", 1500*time.Millisecond, read) + inBrain := brainfstest.WriteSentinelAt(t, filepath.Join(brain, "raw", "s.md")) + brainfstest.AssertNoLeakFrom(t, p, "---\nx: y\n---\ninside", inBrain, 1500*time.Millisecond, read) brainfstest.AssertNoHang(t, p, "---\nx: y\n---\ninside", 1500*time.Millisecond, 6*time.Second, func() { _ = read() }) } diff --git a/ingestion/internal/brainfs/brainfs_test.go b/ingestion/internal/brainfs/brainfs_test.go index 76ea7d1..2b0dbf5 100644 --- a/ingestion/internal/brainfs/brainfs_test.go +++ b/ingestion/internal/brainfs/brainfs_test.go @@ -218,7 +218,7 @@ func TestReadDirRegularListsOnlyRegularFilesAndRefusesSymlinkedComponents(t *tes // 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) { +func TestReadRegularDoesNotReturnAnInBrainFileSwappedInBehindASymlink(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") diff --git a/ingestion/internal/brainfs/brainfstest/brainfstest.go b/ingestion/internal/brainfs/brainfstest/brainfstest.go index c352194..8a59fc8 100644 --- a/ingestion/internal/brainfs/brainfstest/brainfstest.go +++ b/ingestion/internal/brainfs/brainfstest/brainfstest.go @@ -1,8 +1,10 @@ // Package brainfstest is the shared harness for hyperguild#157's swap-race -// tests: it keeps swapping a regular note, a symlink to an outside file, or a -// FIFO (with a writer held open) into a path while a reader under test loops. -// It observes two things: no sentinel byte from the outside file reaches the -// reader's output, and the reader returns within a timeout. It does not cover +// tests: it keeps swapping a regular note, a symlink to a sentinel target file +// (outside the brain, or inside it for hyperguild#171), or a FIFO (with a +// writer held open) into a path while a reader under test loops. +// It observes two things, over a sampled window only: no sentinel byte from +// the target reaches the reader's output, and the reader returns within a +// timeout. It does not cover // hard links, other paths into the reader, or races it does not generate. package brainfstest @@ -18,7 +20,7 @@ import ( "time" ) -// Sentinel marks the outside file's contents. +// Sentinel marks the target file's contents. const Sentinel = "zqbrainfssentinelonefiftyseven" // outsideContent carries Sentinel in every field a reader might extract: the @@ -141,7 +143,7 @@ func AssertNoLeakFrom(t testing.TB, path, inside, target string, d time.Duration } } if leaks > 0 { - t.Fatalf("outside file leaked %d times in %d iterations", leaks, iters) + t.Fatalf("symlink target %s leaked %d times in %d iterations", target, leaks, iters) } if n := swaps.Load(); n == 0 { t.Fatalf("no symlink was swapped into %s in %d iterations: the race did not run", path, iters) diff --git a/ingestion/internal/mcp/readexcerpt_root_test.go b/ingestion/internal/mcp/readexcerpt_root_test.go index 9ba1748..73df8a2 100644 --- a/ingestion/internal/mcp/readexcerpt_root_test.go +++ b/ingestion/internal/mcp/readexcerpt_root_test.go @@ -21,6 +21,11 @@ func TestReadExcerptRefusesSymlinksAndSurvivesSwapRaces(t *testing.T) { require.NoError(t, os.MkdirAll(facts, 0o755)) require.NoError(t, os.Symlink(brainfstest.Outside(t), filepath.Join(facts, "leak.md"))) assert.NotContains(t, readExcerpt(brain, "wiki/w/facts/leak.md", 500), brainfstest.Sentinel) + // A link to a file elsewhere in the brain: os.Root allows it, only the + // regular-file check refuses it (hyperguild#171). + 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.NotContains(t, readExcerpt(brain, "wiki/w/facts/inlink.md", 500), brainfstest.Sentinel) p := filepath.Join(facts, "r.md") require.NoError(t, os.WriteFile(p, []byte("---\nx: y\n---\ninside"), 0o644)) @@ -28,6 +33,9 @@ func TestReadExcerptRefusesSymlinksAndSurvivesSwapRaces(t *testing.T) { brainfstest.AssertNoLeak(t, p, "---\nx: y\n---\ninside", 1500*time.Millisecond, func() string { return readExcerpt(brain, "wiki/w/facts/r.md", 500) }) + brainfstest.AssertNoLeakFrom(t, p, "---\nx: y\n---\ninside", inBrain, 1500*time.Millisecond, func() string { + return readExcerpt(brain, "wiki/w/facts/r.md", 500) + }) brainfstest.AssertNoHang(t, p, "---\nx: y\n---\ninside", 1500*time.Millisecond, 6*time.Second, func() { _ = readExcerpt(brain, "wiki/w/facts/r.md", 500) }) diff --git a/ingestion/internal/notes/read_root_test.go b/ingestion/internal/notes/read_root_test.go index a699fd4..eaa970b 100644 --- a/ingestion/internal/notes/read_root_test.go +++ b/ingestion/internal/notes/read_root_test.go @@ -38,3 +38,31 @@ func TestReadScopedRefusesANoteOverMaxNoteBytes(t *testing.T) { _, err := ReadScoped(brain, "wiki/w/facts/big.md", ScopeNotes) assert.ErrorIs(t, err, ErrTooLarge) } + +// hyperguild#171 (review of #172, finding 1): os.Root alone refuses links that +// leave the brain, so the outside-target race above stays green for a reader +// that skips brainfs's Lstat and SameFile checks. An in-brain target (raw/, +// outside ScopeNotes) is what tells them apart. +func TestReadScopedDoesNotReturnAnInBrainSymlinkTargetStaticallyOrDuringASampledSwapRace(t *testing.T) { + brain := t.TempDir() + facts := filepath.Join(brain, "wiki", "w", "facts") + require.NoError(t, os.MkdirAll(facts, 0o755)) + target := brainfstest.WriteSentinelAt(t, filepath.Join(brain, "raw", "s.md")) + + require.NoError(t, os.Symlink(filepath.Join("..", "..", "..", "raw", "s.md"), filepath.Join(facts, "inlink.md"))) + b, err := ReadScoped(brain, "wiki/w/facts/inlink.md", ScopeNotes) + assert.Error(t, err, "static in-brain link") + assert.NotContains(t, string(b), brainfstest.Sentinel, "static in-brain link") + + p := filepath.Join(facts, "r.md") + require.NoError(t, os.WriteFile(p, []byte("inside"), 0o644)) + b, err = ReadScoped(brain, "wiki/w/facts/r.md", ScopeNotes) + require.NoError(t, err) + assert.Equal(t, "inside", string(b), "positive control: a regular note reads") + + read := func() string { + b, _ := ReadScoped(brain, "wiki/w/facts/r.md", ScopeNotes) + return string(b) + } + brainfstest.AssertNoLeakFrom(t, p, "inside", target, 1500*time.Millisecond, read) +} diff --git a/ingestion/internal/search/brainroot_test.go b/ingestion/internal/search/brainroot_test.go index 7c70ad0..a94a4a9 100644 --- a/ingestion/internal/search/brainroot_test.go +++ b/ingestion/internal/search/brainroot_test.go @@ -12,6 +12,8 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + "github.com/mathiasbq/hyperguild/ingestion/internal/brainfs/brainfstest" ) // hyperguild#157 option 1: search walked and read the brain by path, so a @@ -189,3 +191,49 @@ func TestHydrateRefusesASymlink(t *testing.T) { assert.Error(t, err) assert.NotContains(t, r.Excerpt, sentinel157) } + +// hyperguild#171: os.Root refuses links that leave the brain on its own, so +// the outside-target cases above would stay green for a caller that read +// without brainfs's Lstat and SameFile checks. These targets are inside the +// brain (raw/, which is not a search root). +func inBrainTarget(t *testing.T, brain string) string { + t.Helper() + return brainfstest.WriteSentinelAt(t, filepath.Join(brain, "raw", "s.md")) +} + +func TestQueryDoesNotReturnAnInBrainSymlinkTargetDuringASampledSwapRace(t *testing.T) { + brain, facts, _ := searchBrain(t) + target := inBrainTarget(t, brain) + read := func() string { + // Both terms, so the regular note is a possible hit: a reader that + // never returns the inside note would pass vacuously (#177 floor). + rs, _ := Query(brain, QueryOptions{Query: brainfstest.Sentinel + " zqinside", Limit: 10}) + s := "" + for _, r := range rs { + s += r.Title + r.Excerpt + } + return s + } + brainfstest.AssertNoLeakFrom(t, filepath.Join(facts, "r.md"), "# r\n\nzqinside", target, 1500*time.Millisecond, read) + require.NoError(t, os.Symlink(filepath.Join("..", "..", "..", "raw", "s.md"), filepath.Join(facts, "inlink.md"))) + assert.NotContains(t, read(), brainfstest.Sentinel, "static in-brain link") +} + +func TestHydrateRefusesAnInBrainSymlinkAndDoesNotReturnItsTargetDuringASampledSwapRace(t *testing.T) { + brain, facts, _ := searchBrain(t) + target := inBrainTarget(t, brain) + require.NoError(t, os.Symlink(filepath.Join("..", "..", "..", "raw", "s.md"), filepath.Join(facts, "inlink.md"))) + r, err := hydrate(brain, "wiki/w/facts/inlink.md") + assert.Error(t, err) + assert.NotContains(t, r.Excerpt+r.Title, brainfstest.Sentinel) + + p := filepath.Join(facts, "r.md") + require.NoError(t, os.WriteFile(p, []byte("# r\n\nzqinside"), 0o644)) + r, err = hydrate(brain, "wiki/w/facts/r.md") + require.NoError(t, err) + assert.Contains(t, r.Excerpt, "zqinside", "positive control: a regular note hydrates") + brainfstest.AssertNoLeakFrom(t, p, "# r\n\nzqinside", target, 1500*time.Millisecond, func() string { + r, _ := hydrate(brain, "wiki/w/facts/r.md") + return r.Title + r.Excerpt + }) +} diff --git a/ingestion/internal/vectorstore/sync_root_test.go b/ingestion/internal/vectorstore/sync_root_test.go index 8f83f74..63512d2 100644 --- a/ingestion/internal/vectorstore/sync_root_test.go +++ b/ingestion/internal/vectorstore/sync_root_test.go @@ -42,6 +42,11 @@ func TestSyncRefusesSymlinkedNotesAndSurvivesSwapRaces(t *testing.T) { brain := t.TempDir() writeNote(t, brain, "wiki/w/facts/ok.md", "inside note") require.NoError(t, os.Symlink(brainfstest.Outside(t), filepath.Join(brain, "wiki", "w", "facts", "leak.md"))) + // A link to a file elsewhere in the brain: os.Root allows it, only the + // regular-file checks refuse it (hyperguild#171). raw/ is not a scan root, + // so its sentinel can only arrive through the link. + inBrain := brainfstest.WriteSentinelAt(t, filepath.Join(brain, "raw", "s.md")) + require.NoError(t, os.Symlink(filepath.Join("..", "..", "..", "raw", "s.md"), filepath.Join(brain, "wiki", "w", "facts", "inlink.md"))) emb := &recordingEmbedder{} store := &stubStore{} @@ -51,6 +56,7 @@ func TestSyncRefusesSymlinkedNotesAndSurvivesSwapRaces(t *testing.T) { assert.NotContains(t, emb.all(), brainfstest.Sentinel) for p := range store.upserts { assert.NotContains(t, p, "leak.md") + assert.NotContains(t, p, "inlink.md") } p := filepath.Join(brain, "knowledge", "r.md") @@ -61,6 +67,7 @@ func TestSyncRefusesSymlinkedNotesAndSurvivesSwapRaces(t *testing.T) { return e.all() } brainfstest.AssertNoLeak(t, p, "inside", 1500*time.Millisecond, sync) + brainfstest.AssertNoLeakFrom(t, p, "inside", inBrain, 1500*time.Millisecond, sync) brainfstest.AssertNoHang(t, p, "inside", 1500*time.Millisecond, 6*time.Second, func() { _ = sync() }) }