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 7146af8..dbc0025 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 fdfe968..fd9ef0d 100644 --- a/ingestion/internal/brainfs/brainfstest/brainfstest.go +++ b/ingestion/internal/brainfs/brainfstest/brainfstest.go @@ -117,7 +117,7 @@ func AssertNoLeakFrom(t *testing.T, path, inside, target string, d time.Duration leaks += strings.Count(read(), Sentinel) } 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) } } 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/search/brainroot_test.go b/ingestion/internal/search/brainroot_test.go index 6acb636..ea5f5d2 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 @@ -172,3 +174,47 @@ 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 TestQueryLeaksNothingFromAnInBrainTargetUnderASymlinkSwapRace(t *testing.T) { + brain, facts, _ := searchBrain(t) + target := inBrainTarget(t, brain) + read := func() string { + rs, _ := Query(brain, QueryOptions{Query: brainfstest.Sentinel, 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 TestHydrateRefusesAnInBrainSymlinkAndLeaksNothingUnderASwapRace(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() }) }