Compare commits

..
Author SHA1 Message Date
mathiasandClaude Opus 5.5 31071bc9b1 fix(brainfs): check every component of the walk root (#157)
CI / Lint / Test / Vet (pull_request) Successful in 41s
CI / Mirror to GitHub (pull_request) Skipped
Review round 2 on PR #166: Lstat on the walk root checks only its last
component, so search with Wing+Hall (root wiki/<wing>/<hall>) walked a
symlinked <wing> whose target had a real <hall> directory.

WalkRegular now Lstats every prefix of the root and requires each to be
a real directory. New brainfs and search (Wing+Hall) tests; the Wing-only
search test is renamed to say what it covers.

The doc now records the accepted residual: a directory swapped for a
symlink between listing and read is still followed; os.Root keeps the
read inside the brain but not inside the walk root.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-04 08:16:50 +02:00
mathiasandClaude Opus 5.5 66b6cf8a77 fix(brainfs): refuse a symlinked walk root (#157)
CI / Lint / Test / Vet (pull_request) Successful in 40s
CI / Mirror to GitHub (pull_request) Skipped
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>
2026-10-04 08:11:16 +02:00
mathiasandClaude Opus 5.5 74a9621ba6 fix(vectorstore): walk and read the brain through brainfs (#157, slice 3)
CI / Lint / Test / Vet (pull_request) Successful in 40s
CI / Mirror to GitHub (pull_request) Skipped
Sync walked wiki/ and knowledge/ with filepath.WalkDir and read each .md
with os.ReadFile. A symlinked note (Syncthing syncs symlinks) had its
target embedded into pgvector and served by semantic search; a FIFO
stalled the sync. The walk and read now go through brainfs: regular
files only, type checked on the opened handle.

- Chunks already embedded from a symlinked note are deleted on the next
  sync, because the note is no longer seen.
- A walk error still aborts Sync before the delete pass, so an unreadable
  subdir does not delete chunks of files that exist. A missing scan dir
  is skipped as before.
- brainfs gains Lstat, used for the mtime staleness check.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-04 08:02:16 +02:00
mathiasandClaude Opus 5.5 27a01dd2af fix(brain): context, brain_get and brain_read read through brainfs (#157, slice 2) (#165)
CI / Lint / Test / Vet (push) Successful in 46s
CI / Mirror to GitHub (push) Successful in 4s
readExcerpt, ReadNote and ReadScoped now read through brainfs (os.Root, regular files only, type checked on the opened handle), so a symlink swapped in after a check is not followed and a FIFO does not hang the reader. ReadRegularMax bounds the read itself with io.LimitReader(max+1). notes.Target carries the brain root.

Refs #157. Adversarial review round 2: PASS (round 1 BLOCKED: unbounded read, rule-9 test names).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: Mathias <mthbqv@gmail.com>
2026-10-04 05:59:55 +00:00
13 changed files with 627 additions and 36 deletions
@@ -0,0 +1,43 @@
package api_test
import (
"os"
"path/filepath"
"testing"
"time"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/mathiasbq/hyperguild/ingestion/internal/api"
"github.com/mathiasbq/hyperguild/ingestion/internal/brainfs/brainfstest"
)
// hyperguild#157 slice 2: ReadNote (brain_get, read-after-write) read by
// path; it now reads through brainfs.
func TestReadNoteRefusesSymlinksAndSurvivesSwapRaces(t *testing.T) {
brain := t.TempDir()
facts := filepath.Join(brain, "wiki", "w", "facts")
require.NoError(t, os.MkdirAll(facts, 0o755))
require.NoError(t, os.Symlink(brainfstest.Outside(t), filepath.Join(facts, "leak.md")))
_, body, _, err := api.ReadNote(brain, "wiki/w/facts/leak.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))
fm, body, _, err := api.ReadNote(brain, "wiki/w/facts/r.md")
require.NoError(t, err)
assert.Equal(t, "y", fm["x"])
assert.Contains(t, body, "inside")
read := func() string {
fm, body, _, _ := api.ReadNote(brain, "wiki/w/facts/r.md")
s := body
for k, v := range fm {
s += k + v
}
return s
}
brainfstest.AssertNoLeak(t, p, "---\nx: y\n---\ninside", 1500*time.Millisecond, read)
brainfstest.AssertNoHang(t, p, "---\nx: y\n---\ninside", 1500*time.Millisecond, 6*time.Second, func() { _ = read() })
}
+9 -2
View File
@@ -4,6 +4,7 @@ import (
"crypto/sha256" "crypto/sha256"
"encoding/hex" "encoding/hex"
"fmt" "fmt"
"github.com/mathiasbq/hyperguild/ingestion/internal/brainfs"
"os" "os"
"path/filepath" "path/filepath"
"strings" "strings"
@@ -174,11 +175,17 @@ func UpdateNote(brainDir string, opts UpdateNoteOptions) (relPath, contentHash,
// write primitive behind brain_get: the hash it returns equals the hash // write primitive behind brain_get: the hash it returns equals the hash
// brain_write / brain_update returned for the same bytes. // brain_write / brain_update returned for the same bytes.
func ReadNote(brainDir, relPath string) (fm map[string]string, body, contentHash string, err error) { func ReadNote(brainDir, relPath string) (fm map[string]string, body, contentHash string, err error) {
_, abs, err := resolveWithin(brainDir, relPath) rel, _, err := resolveWithin(brainDir, relPath)
if err != nil { if err != nil {
return nil, "", "", err return nil, "", "", err
} }
raw, err := os.ReadFile(abs) // Through the brain root, regular files only (hyperguild#157).
bfs, err := brainfs.Open(brainDir)
if err != nil {
return nil, "", "", fmt.Errorf("read note: %w", err)
}
defer func() { _ = bfs.Close() }()
raw, err := bfs.ReadRegular(rel)
if err != nil { if err != nil {
if os.IsNotExist(err) { if os.IsNotExist(err) {
return nil, "", "", fmt.Errorf("note %q does not exist", relPath) return nil, "", "", fmt.Errorf("note %q does not exist", relPath)
+94 -6
View File
@@ -13,13 +13,18 @@ import (
"io" "io"
"io/fs" "io/fs"
"os" "os"
"path"
"path/filepath" "path/filepath"
"strings"
"syscall" "syscall"
) )
// ErrNotRegular is returned for an entry that is not a regular file. // ErrNotRegular is returned for an entry that is not a regular file.
var ErrNotRegular = errors.New("not a regular file") var ErrNotRegular = errors.New("not a regular file")
// ErrTooLarge is returned by ReadRegularMax for a file over the limit.
var ErrTooLarge = errors.New("file too large")
// Brain is an open brain directory. // Brain is an open brain directory.
type Brain struct { type Brain struct {
root *os.Root root *os.Root
@@ -50,6 +55,33 @@ func (b *Brain) Rel(path string) (string, error) {
return filepath.ToSlash(rel), nil return filepath.ToSlash(rel), nil
} }
// Lstat stats a brain-relative path through the root without following a
// final symlink.
func (b *Brain) Lstat(rel string) (fs.FileInfo, error) {
return b.root.Lstat(filepath.ToSlash(rel))
}
// realDir returns ErrNotRegular unless every component of rel is a real
// directory. A swap after the check is not caught (accepted residual,
// hyperguild#157).
func (b *Brain) realDir(rel string) error {
if rel == "." {
return nil
}
prefix := ""
for _, part := range strings.Split(rel, "/") {
prefix = path.Join(prefix, part)
fi, err := b.root.Lstat(prefix)
if err != nil {
return err
}
if !fi.IsDir() {
return fmt.Errorf("%s: walk root component %s is not a directory: %w", rel, prefix, ErrNotRegular)
}
}
return nil
}
// ReadRegular reads a brain-relative path, refusing anything that is not a // ReadRegular reads a brain-relative path, refusing anything that is not a
// regular file, both before the open and on the opened handle. // regular file, both before the open and on the opened handle.
func (b *Brain) ReadRegular(rel string) ([]byte, error) { func (b *Brain) ReadRegular(rel string) ([]byte, error) {
@@ -74,7 +106,22 @@ func (b *Brain) ReadRegular(rel string) ([]byte, error) {
// cannot create one; fs.protected_hardlinks=1 and the pod's secrets are on a // 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 // different filesystem from the brain volume, so a local caller cannot link
// them in. Refusing Nlink > 1 is the next step if either changes. // them in. Refusing Nlink > 1 is the next step if either changes.
func (b *Brain) readOpened(rel string) ([]byte, error) { func (b *Brain) readOpened(rel string) ([]byte, error) { return b.readOpenedMax(rel, -1) }
// ReadRegularMax is ReadRegular with a size limit, checked on the opened
// handle before reading (max < 0 means no limit).
func (b *Brain) ReadRegularMax(rel string, max int64) ([]byte, error) {
fi, err := b.root.Lstat(rel)
if err != nil {
return nil, err
}
if !fi.Mode().IsRegular() {
return nil, fmt.Errorf("%s: %w", rel, ErrNotRegular)
}
return b.readOpenedMax(rel, max)
}
func (b *Brain) readOpenedMax(rel string, max int64) ([]byte, error) {
f, err := b.root.OpenFile(rel, os.O_RDONLY|syscall.O_NONBLOCK, 0) f, err := b.root.OpenFile(rel, os.O_RDONLY|syscall.O_NONBLOCK, 0)
if err != nil { if err != nil {
return nil, err return nil, err
@@ -87,15 +134,56 @@ func (b *Brain) readOpened(rel string) ([]byte, error) {
if !hfi.Mode().IsRegular() { if !hfi.Mode().IsRegular() {
return nil, fmt.Errorf("%s: %w", rel, ErrNotRegular) return nil, fmt.Errorf("%s: %w", rel, ErrNotRegular)
} }
return io.ReadAll(f) if max >= 0 && hfi.Size() > max {
return nil, fmt.Errorf("%s is %d bytes (limit %d): %w", rel, hfi.Size(), max, ErrTooLarge)
}
var r io.Reader = f
if wrapAfterStat != nil {
r = wrapAfterStat(r)
}
if max < 0 {
return io.ReadAll(r)
}
// The handle's size can grow after the check; bound the read itself.
data, err := io.ReadAll(io.LimitReader(r, max+1))
if err != nil {
return nil, err
}
if int64(len(data)) > max {
return nil, fmt.Errorf("%s grew past %d bytes while read: %w", rel, max, ErrTooLarge)
}
return data, nil
} }
// wrapAfterStat is a test seam: it runs between the size check and the read,
// 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 // WalkRegular walks a brain-relative directory through the root and calls fn
// for each regular file (directories are descended; symlinks, FIFOs and // for each regular file. Directories are descended; symlinks, FIFOs and
// devices are skipped, never followed). An entry error is passed to onErr and // devices found by the walk are skipped. A walk root with any component that
// the walk continues; a nil onErr ignores it. // is not a real directory goes to onErr as ErrNotRegular and nothing is
// yielded. An entry error is passed to onErr and the walk continues; a nil
// onErr ignores it.
//
// Accepted residual (hyperguild#157): a directory swapped for a symlink
// between the listing and the read is followed. os.Root still keeps the read
// inside the brain, but not inside relDir.
func (b *Brain) WalkRegular(relDir string, fn func(rel string) error, onErr func(rel string, err error)) error { 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 every component of the root to be a real directory
// (Lstat checks only the last one).
err := b.realDir(relDir)
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 err != nil {
if onErr != nil { if onErr != nil {
onErr(p, err) onErr(p, err)
@@ -2,6 +2,8 @@ package brainfs
import ( import (
"errors" "errors"
"io"
"io/fs"
"os" "os"
"path/filepath" "path/filepath"
"syscall" "syscall"
@@ -90,3 +92,100 @@ func TestWalkRegularYieldsOnlyRegularFiles(t *testing.T) {
require.NoError(t, b.WalkRegular(".", func(rel string) error { got = append(got, rel); return nil }, nil)) require.NoError(t, b.WalkRegular(".", func(rel string) error { got = append(got, rel); return nil }, nil))
assert.Equal(t, []string{"a/n.md"}, got) assert.Equal(t, []string{"a/n.md"}, got)
} }
func TestReadRegularMaxRefusesAFileOverTheLimit(t *testing.T) {
dir := t.TempDir()
require.NoError(t, os.WriteFile(filepath.Join(dir, "big.md"), make([]byte, 100), 0o644))
b, err := Open(dir)
require.NoError(t, err)
defer func() { _ = b.Close() }()
_, err = b.ReadRegularMax("big.md", 99)
assert.True(t, errors.Is(err, ErrTooLarge), "got %v", err)
got, err := b.ReadRegularMax("big.md", 100)
require.NoError(t, err)
assert.Len(t, got, 100)
}
func TestReadRegularMaxRefusesAndStopsReadingAFileThatGrowsPastTheLimit(t *testing.T) {
dir := t.TempDir()
p := filepath.Join(dir, "grow.md")
require.NoError(t, os.WriteFile(p, make([]byte, 10), 0o644))
consumed := &countingReader{}
wrapAfterStat = func(r io.Reader) io.Reader {
f, err := os.OpenFile(p, os.O_WRONLY|os.O_APPEND, 0)
require.NoError(t, err)
_, err = f.Write(make([]byte, 1000))
require.NoError(t, err)
require.NoError(t, f.Close())
consumed.r = r
return consumed
}
t.Cleanup(func() { wrapAfterStat = nil })
b, err := Open(dir)
require.NoError(t, err)
defer func() { _ = b.Close() }()
got, err := b.ReadRegularMax("grow.md", 100)
assert.True(t, errors.Is(err, ErrTooLarge), "got %v", err)
assert.Nil(t, got)
assert.LessOrEqual(t, consumed.n, 101, "read past max+1 bytes")
}
type countingReader struct {
r io.Reader
n int
}
func (c *countingReader) Read(p []byte) (int, error) {
n, err := c.r.Read(p)
c.n += n
return n, err
}
func TestLstatDoesNotFollowSymlinksAndRefusesEscapes(t *testing.T) {
dir := t.TempDir()
outside := filepath.Join(t.TempDir(), "o.md")
require.NoError(t, os.WriteFile(outside, []byte("o"), 0o644))
require.NoError(t, os.Symlink(outside, filepath.Join(dir, "l.md")))
b, err := Open(dir)
require.NoError(t, err)
defer func() { _ = b.Close() }()
fi, err := b.Lstat("l.md")
require.NoError(t, err)
assert.Equal(t, fs.ModeSymlink, fi.Mode().Type())
_, 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)
}
func TestWalkRegularReportsASymlinkedIntermediateComponentOfTheWalkRoot(t *testing.T) {
dir := t.TempDir()
require.NoError(t, os.MkdirAll(filepath.Join(dir, "real", "sub"), 0o755))
require.NoError(t, os.WriteFile(filepath.Join(dir, "real", "sub", "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/sub", 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)
}
@@ -0,0 +1,103 @@
// 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
// hard links, other paths into the reader, or races it does not generate.
package brainfstest
import (
"os"
"path/filepath"
"strings"
"sync"
"sync/atomic"
"syscall"
"testing"
"time"
)
// Sentinel marks the outside file's contents.
const Sentinel = "zqbrainfssentinelonefiftyseven"
// 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 {
t.Fatal(err)
}
return p
}
// 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
// handle check parks forever instead of reading EOF. The swapper is stopped
// and awaited before t.TempDir cleanup.
func StartSwap(t *testing.T, path, inside, outside string, fifo bool) {
t.Helper()
var stop atomic.Bool
var wg sync.WaitGroup
wg.Add(1)
go func() {
defer wg.Done()
tmp := path + ".swap"
var writers []*os.File
defer func() {
for _, w := range writers {
_ = w.Close()
}
}()
for i := 0; !stop.Load(); i++ {
_ = os.Remove(tmp)
switch {
case i%2 == 0:
_ = os.WriteFile(tmp, []byte(inside), 0o644)
case fifo:
if err := syscall.Mkfifo(tmp, 0o644); err == nil {
if w, err := os.OpenFile(tmp, os.O_RDWR, 0); err == nil {
writers = append(writers, w)
}
}
default:
_ = os.Symlink(outside, tmp)
}
_ = os.Rename(tmp, path)
}
}()
t.Cleanup(func() { stop.Store(true); wg.Wait() })
}
// AssertNoLeak loops read for d while a symlink swapper runs on path, and
// fails if any output contains Sentinel.
func AssertNoLeak(t *testing.T, path, inside string, d time.Duration, read func() string) {
t.Helper()
StartSwap(t, path, inside, Outside(t), false)
leaks, iters := 0, 0
for deadline := time.Now().Add(d); time.Now().Before(deadline); iters++ {
leaks += strings.Count(read(), Sentinel)
}
if leaks > 0 {
t.Fatalf("outside file leaked %d times in %d iterations", leaks, iters)
}
}
// AssertNoHang loops read for d while a FIFO swapper (held writers) runs on
// path, and fails if a read does not return within timeout.
func AssertNoHang(t *testing.T, path, inside string, d, timeout time.Duration, read func()) {
t.Helper()
StartSwap(t, path, inside, Outside(t), true)
done := make(chan struct{})
go func() {
for deadline := time.Now().Add(d); time.Now().Before(deadline); {
read()
}
close(done)
}()
select {
case <-done:
case <-time.After(timeout):
t.Fatal("reader hung on a swapped-in FIFO with a writer held open")
}
}
@@ -0,0 +1,34 @@
package mcp
import (
"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 2: readExcerpt (brain_context) read by path, so a
// symlink in the Syncthing-shared brain put an outside file in context, and
// a FIFO hung it. It now reads through brainfs.
func TestReadExcerptRefusesSymlinksAndSurvivesSwapRaces(t *testing.T) {
brain := t.TempDir()
facts := filepath.Join(brain, "wiki", "w", "facts")
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)
p := filepath.Join(facts, "r.md")
require.NoError(t, os.WriteFile(p, []byte("---\nx: y\n---\ninside"), 0o644))
assert.Equal(t, "inside", readExcerpt(brain, "wiki/w/facts/r.md", 500))
brainfstest.AssertNoLeak(t, p, "---\nx: y\n---\ninside", 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)
})
}
+8 -3
View File
@@ -4,7 +4,7 @@ import (
"context" "context"
"encoding/json" "encoding/json"
"fmt" "fmt"
"os" "github.com/mathiasbq/hyperguild/ingestion/internal/brainfs"
"path/filepath" "path/filepath"
"sort" "sort"
"strings" "strings"
@@ -181,8 +181,13 @@ func readExcerpt(brainDir, relPath string, maxLen int) string {
if relPath == "" { if relPath == "" {
return "" return ""
} }
full := filepath.Join(brainDir, filepath.FromSlash(relPath)) // Through the brain root, regular files only (hyperguild#157).
content, err := os.ReadFile(full) bfs, err := brainfs.Open(brainDir)
if err != nil {
return ""
}
defer func() { _ = bfs.Close() }()
content, err := bfs.ReadRegular(filepath.ToSlash(relPath))
if err != nil { if err != nil {
return "" return ""
} }
+10 -5
View File
@@ -5,6 +5,7 @@ import (
"encoding/hex" "encoding/hex"
"errors" "errors"
"fmt" "fmt"
"github.com/mathiasbq/hyperguild/ingestion/internal/brainfs"
"os" "os"
"path/filepath" "path/filepath"
"strings" "strings"
@@ -71,14 +72,18 @@ func ReadScoped(brainDir, path string, scope Scope) ([]byte, error) {
} }
func readContained(t Target) ([]byte, error) { func readContained(t Target) ([]byte, error) {
fi, err := os.Stat(t.Abs) // Through the brain root, regular files only, size checked on the opened
// handle (hyperguild#157): Resolve's Lstat could be raced by a swap, and
// os.ReadFile blocked on a FIFO.
bfs, err := brainfs.Open(t.Brain)
if err != nil { if err != nil {
return nil, fmt.Errorf("notes: stat %s: %w", t.Rel, err) return nil, fmt.Errorf("notes: %w", err)
} }
if fi.Size() > maxNoteBytes { defer func() { _ = bfs.Close() }()
return nil, fmt.Errorf("%w: %s is %d bytes (limit %d)", ErrTooLarge, t.Rel, fi.Size(), maxNoteBytes) b, err := bfs.ReadRegularMax(t.Rel, maxNoteBytes)
if errors.Is(err, brainfs.ErrTooLarge) {
return nil, fmt.Errorf("%w: %s (limit %d)", ErrTooLarge, t.Rel, maxNoteBytes)
} }
b, err := os.ReadFile(t.Abs)
if err != nil { if err != nil {
return nil, fmt.Errorf("notes: read %s: %w", t.Rel, err) return nil, fmt.Errorf("notes: read %s: %w", t.Rel, err)
} }
+2 -1
View File
@@ -60,6 +60,7 @@ var (
type Target struct { type Target struct {
Rel string // brain-relative, cleaned Rel string // brain-relative, cleaned
Abs string // absolute, symlink-checked Abs string // absolute, symlink-checked
Brain string // the brain root Rel is relative to
} }
// Resolve canonicalises path, then checks it against the scope's allowlist, // Resolve canonicalises path, then checks it against the scope's allowlist,
@@ -109,7 +110,7 @@ func Resolve(brainDir, path string, scope Scope) (Target, error) {
return Target{}, err return Target{}, err
} }
return Target{Rel: clean, Abs: abs}, nil return Target{Rel: clean, Abs: abs, Brain: brainDir}, nil
} }
// allow enforces the scope's allowlist. The two scopes are DISJOINT, not // allow enforces the scope's allowlist. The two scopes are DISJOINT, not
@@ -0,0 +1,40 @@
package notes
import (
"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 2: ReadScoped (brain_read) Lstat-checked the final
// component, then reopened by name with os.ReadFile: a swap after the check
// was followed, and a FIFO blocked the read. It now reads through brainfs.
func TestReadScopedRefusesSymlinksAndSurvivesSwapRaces(t *testing.T) {
brain := t.TempDir()
facts := filepath.Join(brain, "wiki", "w", "facts")
require.NoError(t, os.MkdirAll(facts, 0o755))
p := filepath.Join(facts, "r.md")
read := func() string {
b, _ := ReadScoped(brain, "wiki/w/facts/r.md", ScopeNotes)
return string(b)
}
require.NoError(t, os.WriteFile(p, []byte("inside"), 0o644))
assert.Equal(t, "inside", read())
brainfstest.AssertNoLeak(t, p, "inside", 1500*time.Millisecond, read)
brainfstest.AssertNoHang(t, p, "inside", 1500*time.Millisecond, 6*time.Second, func() { _ = read() })
}
func TestReadScopedRefusesANoteOverMaxNoteBytes(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, "big.md"), make([]byte, maxNoteBytes+1), 0o644))
_, err := ReadScoped(brain, "wiki/w/facts/big.md", ScopeNotes)
assert.ErrorIs(t, err, ErrTooLarge)
}
@@ -61,6 +61,31 @@ func TestQueryDoesNotFollowASymlinkedDirectory(t *testing.T) {
assert.Zero(t, leaked(rs)) 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 TestQueryDoesNotFollowASymlinkedWingRootInsideTheBrain(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)
}
// With a hall the search root is wiki/<wing>/<hall>; a symlinked <wing> with
// a real <hall> directory under its target was followed (PR #166 round 2).
func TestQueryDoesNotFollowASymlinkedWingUnderAWingAndHallRoot(t *testing.T) {
brain, _, _ := searchBrain(t)
require.NoError(t, os.MkdirAll(filepath.Join(brain, "raw", "facts"), 0o755))
require.NoError(t, os.WriteFile(filepath.Join(brain, "raw", "facts", "r.md"), []byte("# r\n\nzqrawhall"), 0o644))
require.NoError(t, os.Symlink(filepath.Join("..", "raw"), filepath.Join(brain, "wiki", "alias")))
rs, err := Query(brain, QueryOptions{Query: "zqrawhall", Wing: "alias", Hall: "facts", Limit: 10})
require.NoError(t, err)
assert.Empty(t, rs)
}
// swapInto keeps renaming a regular note, and either a symlink to outside or // 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 // 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 // can): a FIFO with no writer reads EOF at once, which would let a reader
+32 -17
View File
@@ -2,12 +2,15 @@ package vectorstore
import ( import (
"context" "context"
"errors"
"fmt" "fmt"
"io/fs"
"log/slog" "log/slog"
"os" "path"
"path/filepath"
"strings" "strings"
"time" "time"
"github.com/mathiasbq/hyperguild/ingestion/internal/brainfs"
) )
// Embedder produces dense vectors. The embed package's Client satisfies // Embedder produces dense vectors. The embed package's Client satisfies
@@ -90,31 +93,38 @@ func Sync(ctx context.Context, brainDir string, store Store, embedder Embedder)
} }
seenParents := make(map[string]struct{}) seenParents := make(map[string]struct{})
// Through the brain root, regular files only (hyperguild#157): the brain
// is a Syncthing folder, and os.ReadFile followed a synced symlink into
// the embeddings and blocked on a FIFO.
bfs, err := brainfs.Open(brainDir)
if err != nil {
return res, fmt.Errorf("open brain: %w", err)
}
defer func() { _ = bfs.Close() }()
for _, sub := range scanDirs { for _, sub := range scanDirs {
root := filepath.Join(brainDir, sub) if _, err := bfs.Lstat(sub); errors.Is(err, fs.ErrNotExist) {
if _, err := os.Stat(root); os.IsNotExist(err) {
continue continue
} }
// Any other walk error, including a scan root that is a symlink,
err = filepath.WalkDir(root, func(path string, d os.DirEntry, err error) error { // aborts the run below.
if err != nil { var walkErr error
return err onErr := func(rel string, err error) {
if walkErr == nil {
walkErr = fmt.Errorf("%s: %w", rel, err)
} }
if d.IsDir() || !strings.HasSuffix(path, ".md") || d.Name() == "_index.md" { }
err = bfs.WalkRegular(sub, func(relSlash string) error {
if !strings.HasSuffix(relSlash, ".md") || path.Base(relSlash) == "_index.md" {
return nil return nil
} }
rel, err := filepath.Rel(brainDir, path)
if err != nil {
return err
}
relSlash := filepath.ToSlash(rel)
seenParents[relSlash] = struct{}{} seenParents[relSlash] = struct{}{}
if ps, ok := parents[relSlash]; ok { if ps, ok := parents[relSlash]; ok {
// File already has chunks in the store. Re-embed only when // File already has chunks in the store. Re-embed only when
// the file has been edited since the oldest chunk was // the file has been edited since the oldest chunk was
// written. Tolerate clock skew with a sub-second grace. // written. Tolerate clock skew with a sub-second grace.
info, statErr := d.Info() info, statErr := bfs.Lstat(relSlash)
if statErr != nil { if statErr != nil {
res.Errors = append(res.Errors, fmt.Errorf("stat %s: %w", relSlash, statErr)) res.Errors = append(res.Errors, fmt.Errorf("stat %s: %w", relSlash, statErr))
return nil return nil
@@ -132,7 +142,7 @@ func Sync(ctx context.Context, brainDir string, store Store, embedder Embedder)
} }
} }
content, readErr := os.ReadFile(path) content, readErr := bfs.ReadRegular(relSlash)
if readErr != nil { if readErr != nil {
res.Errors = append(res.Errors, fmt.Errorf("read %s: %w", relSlash, readErr)) res.Errors = append(res.Errors, fmt.Errorf("read %s: %w", relSlash, readErr))
return nil return nil
@@ -151,7 +161,12 @@ func Sync(ctx context.Context, brainDir string, store Store, embedder Embedder)
res.Added++ res.Added++
} }
return nil return nil
}) }, onErr)
if err == nil {
err = walkErr
}
// Abort before the delete pass: an incomplete walk would leave
// seenParents short and delete chunks of files that still exist.
if err != nil { if err != nil {
return res, fmt.Errorf("walk %s: %w", sub, err) return res, fmt.Errorf("walk %s: %w", sub, err)
} }
@@ -0,0 +1,126 @@
package vectorstore_test
import (
"context"
"os"
"path/filepath"
"strings"
"sync"
"testing"
"time"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/mathiasbq/hyperguild/ingestion/internal/brainfs/brainfstest"
"github.com/mathiasbq/hyperguild/ingestion/internal/vectorstore"
)
// recordingEmbedder keeps every text it was asked to embed.
type recordingEmbedder struct {
mu sync.Mutex
texts []string
}
func (e *recordingEmbedder) Embed(_ context.Context, text string) ([]float32, error) {
e.mu.Lock()
defer e.mu.Unlock()
e.texts = append(e.texts, text)
return []float32{1}, nil
}
func (e *recordingEmbedder) all() string {
e.mu.Lock()
defer e.mu.Unlock()
return strings.Join(e.texts, "\n")
}
// 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 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")))
emb := &recordingEmbedder{}
store := &stubStore{}
_, err := vectorstore.Sync(context.Background(), brain, store, emb)
require.NoError(t, err)
assert.Contains(t, emb.all(), "inside note", "positive control: the regular note is embedded")
assert.NotContains(t, emb.all(), brainfstest.Sentinel)
for p := range store.upserts {
assert.NotContains(t, p, "leak.md")
}
p := filepath.Join(brain, "knowledge", "r.md")
require.NoError(t, os.MkdirAll(filepath.Dir(p), 0o755))
sync := func() string {
e := &recordingEmbedder{}
_, _ = vectorstore.Sync(context.Background(), brain, &stubStore{}, e)
return e.all()
}
brainfstest.AssertNoLeak(t, p, "inside", 1500*time.Millisecond, sync)
brainfstest.AssertNoHang(t, p, "inside", 1500*time.Millisecond, 6*time.Second, func() { _ = sync() })
}
// Chunks embedded from a symlinked note before this fix are deleted: the
// note is no longer seen, so the delete pass drops its rows.
func TestSyncDeletesChunksOfASymlinkedNote(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")))
store := &stubStore{known: map[string]time.Time{
"wiki/w/facts/leak.md#0001": {},
"wiki/w/facts/ok.md#0001": {},
}}
_, err := vectorstore.Sync(context.Background(), brain, store, &recordingEmbedder{})
require.NoError(t, err)
assert.Equal(t, []string{"wiki/w/facts/leak.md#0001"}, store.deletes)
}
// A walk error aborts Sync before the delete pass, so chunks of files the
// walk could not see are kept.
func TestSyncAbortsBeforeDeletingWhenASubdirIsUnreadable(t *testing.T) {
if os.Geteuid() == 0 {
t.Skip("root ignores directory permissions")
}
brain := t.TempDir()
writeNote(t, brain, "wiki/w/facts/ok.md", "inside note")
locked := filepath.Join(brain, "wiki", "w")
require.NoError(t, os.Chmod(locked, 0o000))
t.Cleanup(func() { _ = os.Chmod(locked, 0o755) })
store := &stubStore{known: map[string]time.Time{"wiki/w/facts/ok.md#0001": {}}}
_, err := vectorstore.Sync(context.Background(), brain, store, &recordingEmbedder{})
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")
}