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>
This commit is contained in:
@@ -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)
|
||||
}
|
||||
|
||||
@@ -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")
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user