fix(brainfs): refuse a symlinked walk root (#157)

Review round 1 on PR #166: fs.WalkDir stats its root with a follow, and
os.Root allows a symlink that stays inside the brain, so knowledge -> .
aliased raw/, sessions/ and root files into the embeddings (and a
wiki/<w> -> .. root into search results). On main, filepath.WalkDir
Lstat'ed the root and did not descend.

- WalkRegular Lstats its root and requires a real directory; anything
  else goes to onErr as ErrNotRegular and nothing is yielded.
- Sync skips a missing scan root via bfs.Lstat before walking, and every
  other walk error, including a symlinked root, aborts before the delete
  pass. This removes the unpinned rel == sub special case.
- Search warns and skips a symlinked root (new test).
- Test renamed to TestSyncRefusesSymlinkedNotesAndSurvivesSwapRaces.
- wrapAfterStat comment: brainfs tests must not use t.Parallel.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

ingestion/internal/brainfs/brainfs.go

@@ -133,7 +133,8 @@ func (b *Brain) readOpenedMax(rel string, max int64) ([]byte, error) {
 }
 
 // wrapAfterStat is a test seam: it runs between the size check and the read,
-// and wraps the reader the read consumes.
+// and wraps the reader the read consumes. A package global: brainfs tests
+// must not use t.Parallel.
 var wrapAfterStat func(io.Reader) io.Reader
 
 // WalkRegular walks a brain-relative directory through the root and calls fn
@@ -141,7 +142,21 @@ var wrapAfterStat func(io.Reader) io.Reader
 // devices are skipped, never followed). An entry error is passed to onErr and
 // the walk continues; a nil onErr ignores it.
 func (b *Brain) WalkRegular(relDir string, fn func(rel string) error, onErr func(rel string, err error)) error {
-	return fs.WalkDir(b.root.FS(), filepath.ToSlash(relDir), func(p string, d fs.DirEntry, err error) error {
+	relDir = filepath.ToSlash(relDir)
+	// fs.WalkDir stats its root with a follow, and os.Root allows a symlink
+	// that stays inside the brain: knowledge -> . would alias the whole
+	// brain. Require a real directory first.
+	fi, err := b.root.Lstat(relDir)
+	if err == nil && !fi.IsDir() {
+		err = fmt.Errorf("%s: walk root is not a directory: %w", relDir, ErrNotRegular)
+	}
+	if err != nil {
+		if onErr != nil {
+			onErr(relDir, err)
+		}
+		return nil
+	}
+	return fs.WalkDir(b.root.FS(), relDir, func(p string, d fs.DirEntry, err error) error {
 		if err != nil {
 			if onErr != nil {
 				onErr(p, err)

ingestion/internal/brainfs/brainfs_test.go

@@ -155,3 +155,20 @@ func TestLstatDoesNotFollowSymlinksAndRefusesEscapes(t *testing.T) {
 	_, err = b.Lstat("../" + filepath.Base(filepath.Dir(outside)) + "/o.md")
 	assert.Error(t, err)
 }
+
+func TestWalkRegularReportsASymlinkedWalkRootAndYieldsNothing(t *testing.T) {
+	dir := t.TempDir()
+	require.NoError(t, os.MkdirAll(filepath.Join(dir, "real"), 0o755))
+	require.NoError(t, os.WriteFile(filepath.Join(dir, "real", "n.md"), []byte("n"), 0o644))
+	require.NoError(t, os.Symlink("real", filepath.Join(dir, "alias")))
+	b, err := Open(dir)
+	require.NoError(t, err)
+	defer func() { _ = b.Close() }()
+	var got []string
+	var errs []error
+	require.NoError(t, b.WalkRegular("alias", func(rel string) error { got = append(got, rel); return nil },
+		func(_ string, err error) { errs = append(errs, err) }))
+	assert.Empty(t, got)
+	require.Len(t, errs, 1)
+	assert.ErrorIs(t, errs[0], ErrNotRegular)
+}

ingestion/internal/search/brainroot_test.go

@@ -61,6 +61,19 @@ func TestQueryDoesNotFollowASymlinkedDirectory(t *testing.T) {
 	assert.Zero(t, leaked(rs))
 }
 
+// A search root that is a symlink staying inside the brain (wiki/<w> -> ..)
+// was followed by fs.WalkDir, aliasing raw/ and root files into results
+// (PR #166 review). The root is now Lstat-checked and skipped.
+func TestQueryDoesNotFollowASymlinkedSearchRootInsideTheBrain(t *testing.T) {
+	brain, _, _ := searchBrain(t)
+	require.NoError(t, os.MkdirAll(filepath.Join(brain, "raw"), 0o755))
+	require.NoError(t, os.WriteFile(filepath.Join(brain, "raw", "r.md"), []byte("# r\n\nzqrawonly"), 0o644))
+	require.NoError(t, os.Symlink("..", filepath.Join(brain, "wiki", "alias")))
+	rs, err := Query(brain, QueryOptions{Query: "zqrawonly", Wing: "alias", Limit: 10})
+	require.NoError(t, err)
+	assert.Empty(t, rs)
+}
+
 // swapInto keeps renaming a regular note, and either a symlink to outside or
 // a FIFO, into path. Each FIFO has a writer held open (as any local process
 // can): a FIFO with no writer reads EOF at once, which would let a reader

ingestion/internal/vectorstore/sync.go

@@ -103,11 +103,13 @@ func Sync(ctx context.Context, brainDir string, store Store, embedder Embedder)
 	defer func() { _ = bfs.Close() }()
 
 	for _, sub := range scanDirs {
+		if _, err := bfs.Lstat(sub); errors.Is(err, fs.ErrNotExist) {
+			continue
+		}
+		// Any other walk error, including a scan root that is a symlink,
+		// aborts the run below.
 		var walkErr error
 		onErr := func(rel string, err error) {
-			if rel == sub && errors.Is(err, fs.ErrNotExist) {
-				return
-			}
 			if walkErr == nil {
 				walkErr = fmt.Errorf("%s: %w", rel, err)
 			}

ingestion/internal/vectorstore/sync_root_test.go

@@ -38,7 +38,7 @@ func (e *recordingEmbedder) all() string {
 // hyperguild#157 slice 3: Sync walked wiki/ and knowledge/ and read each .md
 // with os.ReadFile, so a symlinked note had its target embedded and served by
 // semantic search, and a FIFO stalled the sync.
-func TestSyncRefusesSymlinksAndSurvivesSwapRaces(t *testing.T) {
+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")))
@@ -95,3 +95,32 @@ func TestSyncAbortsBeforeDeletingWhenASubdirIsUnreadable(t *testing.T) {
 	assert.Error(t, err)
 	assert.Empty(t, store.deletes)
 }
+
+// A scan root that is a symlink inside the brain (knowledge -> .) was
+// followed by fs.WalkDir, aliasing raw/, sessions/ and root files into the
+// index. Sync now aborts on it, before embedding or deleting anything.
+func TestSyncAbortsOnASymlinkedScanRoot(t *testing.T) {
+	brain := t.TempDir()
+	writeNote(t, brain, "wiki/w/a.md", "wiki note")
+	writeNote(t, brain, "sessions/s1.md", "session note")
+	require.NoError(t, os.Symlink(".", filepath.Join(brain, "knowledge")))
+	emb := &recordingEmbedder{}
+	store := &stubStore{known: map[string]time.Time{"wiki/w/a.md#0001": {}}}
+	_, err := vectorstore.Sync(context.Background(), brain, store, emb)
+	assert.Error(t, err)
+	assert.NotContains(t, emb.all(), "session note")
+	for p := range store.upserts {
+		assert.False(t, strings.HasPrefix(p, "knowledge/"), "upserted %s", p)
+	}
+	assert.Empty(t, store.deletes)
+}
+
+// A missing scan root is skipped; the other root still syncs.
+func TestSyncSkipsAMissingScanRoot(t *testing.T) {
+	brain := t.TempDir()
+	writeNote(t, brain, "wiki/w/a.md", "wiki note")
+	emb := &recordingEmbedder{}
+	_, err := vectorstore.Sync(context.Background(), brain, &stubStore{}, emb)
+	require.NoError(t, err)
+	assert.Contains(t, emb.all(), "wiki note")
+}