fix(search): walk and read the brain through an os.Root (#157, slice 1)
Option 1 of #157, first slice. Search walked and read the brain by path, so a symlink planted in the Syncthing-shared brain dir put an outside file's contents in brain_query results, and a FIFO swapped into the wiki hung the query forever (reproduced: the FIFO race test blocked for the full 6s timeout before this change). New package internal/brainfs: Open (os.Root on the brain dir), ReadRegular (Lstat + non-blocking open + type check on the handle) and WalkRegular (walks the root's FS, yields regular files only, never follows a link). search.Query and hydrate use it. Tests (red first): a wiki symlink is not followed, a symlinked dir is not descended, hydrate refuses a symlink, and race tests swap a symlink or a FIFO into a wiki note while Query loops; any leaked byte or a hang fails. Refs #157 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,124 @@
|
||||
package search
|
||||
|
||||
import (
|
||||
"os"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"sync/atomic"
|
||||
"syscall"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
// hyperguild#157 option 1: search walked and read the brain by path, so a
|
||||
// symlink anywhere in the Syncthing-shared brain dir (wiki/<w>/<h>/leak.md ->
|
||||
// /any/file) put the target's contents in brain_query results (reproduced in
|
||||
// the PR #156 review). Search now walks and reads through an os.Root on the
|
||||
// brain dir and only reads regular files.
|
||||
|
||||
const sentinel157 = "zqsentinelonefiftyseven"
|
||||
|
||||
func searchBrain(t *testing.T) (brain, facts, outside string) {
|
||||
t.Helper()
|
||||
brain = t.TempDir()
|
||||
facts = filepath.Join(brain, "wiki", "w", "facts")
|
||||
require.NoError(t, os.MkdirAll(facts, 0o755))
|
||||
require.NoError(t, os.MkdirAll(filepath.Join(brain, "knowledge"), 0o755))
|
||||
outside = filepath.Join(t.TempDir(), "secret.md")
|
||||
require.NoError(t, os.WriteFile(outside, []byte("# secret\n\n"+sentinel157), 0o644))
|
||||
return brain, facts, outside
|
||||
}
|
||||
|
||||
func leaked(rs []Result) int {
|
||||
n := 0
|
||||
for _, r := range rs {
|
||||
n += strings.Count(r.Excerpt+r.Title, sentinel157)
|
||||
}
|
||||
return n
|
||||
}
|
||||
|
||||
func TestQueryDoesNotFollowASymlinkInTheWiki(t *testing.T) {
|
||||
brain, facts, outside := searchBrain(t)
|
||||
require.NoError(t, os.Symlink(outside, filepath.Join(facts, "leak.md")))
|
||||
require.NoError(t, os.WriteFile(filepath.Join(facts, "real.md"), []byte("# real\n\nzqrealnote"), 0o644))
|
||||
rs, err := Query(brain, QueryOptions{Query: sentinel157, Limit: 10})
|
||||
require.NoError(t, err)
|
||||
assert.Zero(t, leaked(rs))
|
||||
rs, err = Query(brain, QueryOptions{Query: "zqrealnote", Limit: 10})
|
||||
require.NoError(t, err)
|
||||
assert.Len(t, rs, 1, "regular notes are still found")
|
||||
}
|
||||
|
||||
func TestQueryDoesNotFollowASymlinkedDirectory(t *testing.T) {
|
||||
brain, _, outside := searchBrain(t)
|
||||
require.NoError(t, os.Symlink(filepath.Dir(outside), filepath.Join(brain, "wiki", "linked")))
|
||||
rs, err := Query(brain, QueryOptions{Query: sentinel157, Limit: 10})
|
||||
require.NoError(t, err)
|
||||
assert.Zero(t, leaked(rs))
|
||||
}
|
||||
|
||||
func swapInto(path, outside string, fifo bool, stop *atomic.Bool) {
|
||||
tmp := path + ".swap"
|
||||
for i := 0; !stop.Load(); i++ {
|
||||
_ = os.Remove(tmp)
|
||||
switch {
|
||||
case i%2 == 0:
|
||||
_ = os.WriteFile(tmp, []byte("# r\n\nzqinside "+"zqracenote"), 0o644)
|
||||
case fifo:
|
||||
_ = syscall.Mkfifo(tmp, 0o644)
|
||||
default:
|
||||
_ = os.Symlink(outside, tmp)
|
||||
}
|
||||
_ = os.Rename(tmp, path)
|
||||
}
|
||||
}
|
||||
|
||||
func TestQueryLeaksNothingUnderASymlinkSwapRace(t *testing.T) {
|
||||
brain, facts, outside := searchBrain(t)
|
||||
var stop atomic.Bool
|
||||
go swapInto(filepath.Join(facts, "r.md"), outside, false, &stop)
|
||||
defer stop.Store(true)
|
||||
leaks, iters := 0, 0
|
||||
for deadline := time.Now().Add(1500 * time.Millisecond); time.Now().Before(deadline); iters++ {
|
||||
rs, _ := Query(brain, QueryOptions{Query: sentinel157, Limit: 10})
|
||||
leaks += leaked(rs)
|
||||
}
|
||||
if leaks > 0 {
|
||||
t.Fatalf("outside file leaked %d times in %d iterations", leaks, iters)
|
||||
}
|
||||
}
|
||||
|
||||
func TestQueryDoesNotHangUnderAFIFOSwapRace(t *testing.T) {
|
||||
brain, facts, outside := searchBrain(t)
|
||||
var stop atomic.Bool
|
||||
go swapInto(filepath.Join(facts, "r.md"), outside, true, &stop)
|
||||
done := make(chan struct{})
|
||||
go func() {
|
||||
for deadline := time.Now().Add(1500 * time.Millisecond); time.Now().Before(deadline); {
|
||||
_, _ = Query(brain, QueryOptions{Query: "zqracenote", Limit: 10})
|
||||
}
|
||||
close(done)
|
||||
}()
|
||||
select {
|
||||
case <-done:
|
||||
stop.Store(true)
|
||||
case <-time.After(6 * time.Second):
|
||||
stop.Store(true)
|
||||
if f, err := os.OpenFile(filepath.Join(facts, "r.md"), os.O_WRONLY|syscall.O_NONBLOCK, 0); err == nil {
|
||||
_ = f.Close()
|
||||
}
|
||||
t.Fatal("search hung on a swapped-in FIFO")
|
||||
}
|
||||
}
|
||||
|
||||
// hydrate (vector-only hits) reads by relative path; it must refuse a symlink.
|
||||
func TestHydrateRefusesASymlink(t *testing.T) {
|
||||
brain, facts, outside := searchBrain(t)
|
||||
require.NoError(t, os.Symlink(outside, filepath.Join(facts, "leak.md")))
|
||||
r, err := hydrate(brain, "wiki/w/facts/leak.md")
|
||||
assert.Error(t, err)
|
||||
assert.NotContains(t, r.Excerpt, sentinel157)
|
||||
}
|
||||
@@ -5,6 +5,7 @@ import (
|
||||
"bufio"
|
||||
"context"
|
||||
"fmt"
|
||||
"github.com/mathiasbq/hyperguild/ingestion/internal/brainfs"
|
||||
sharedfm "github.com/mathiasbq/hyperguild/ingestion/internal/frontmatter"
|
||||
"log/slog"
|
||||
"os"
|
||||
@@ -119,20 +120,30 @@ func QueryContext(ctx context.Context, brainDir string, opts QueryOptions) ([]Re
|
||||
return nil, err
|
||||
}
|
||||
|
||||
// Walk and read through an os.Root on the brain dir, regular files only
|
||||
// (hyperguild#157): a symlink planted in the Syncthing-shared brain must
|
||||
// not put an outside file in results, nor a FIFO hang the query.
|
||||
bfs, err := brainfs.Open(brainDir)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("search: %w", err)
|
||||
}
|
||||
defer func() { _ = bfs.Close() }()
|
||||
|
||||
var results []Result
|
||||
for _, dir := range roots {
|
||||
if _, statErr := os.Stat(dir); os.IsNotExist(statErr) {
|
||||
continue
|
||||
}
|
||||
err := filepath.WalkDir(dir, func(path string, d os.DirEntry, err error) error {
|
||||
if err != nil {
|
||||
slog.Warn("search: skipping path", "path", path, "err", err)
|
||||
relDir, relErr := bfs.Rel(dir)
|
||||
if relErr != nil {
|
||||
return nil, fmt.Errorf("search: %w", relErr)
|
||||
}
|
||||
err := bfs.WalkRegular(relDir, func(relPath string) error {
|
||||
if !strings.HasSuffix(relPath, ".md") {
|
||||
return nil
|
||||
}
|
||||
if d.IsDir() || !strings.HasSuffix(path, ".md") {
|
||||
return nil
|
||||
}
|
||||
content, err := os.ReadFile(path)
|
||||
path := filepath.Join(brainDir, filepath.FromSlash(relPath))
|
||||
content, err := bfs.ReadRegular(relPath)
|
||||
if err != nil {
|
||||
slog.Warn("search: skipping unreadable file", "path", path, "err", err)
|
||||
return nil
|
||||
@@ -155,7 +166,7 @@ func QueryContext(ctx context.Context, brainDir string, opts QueryOptions) ([]Re
|
||||
origin, writtenBy := extractProvenance(string(content))
|
||||
results = append(results, Result{
|
||||
Path: rel,
|
||||
Title: extractTitle(string(content), d.Name()),
|
||||
Title: extractTitle(string(content), filepath.Base(relPath)),
|
||||
Excerpt: excerpt(string(content), 300),
|
||||
Score: score,
|
||||
Wing: wing,
|
||||
@@ -166,6 +177,8 @@ func QueryContext(ctx context.Context, brainDir string, opts QueryOptions) ([]Re
|
||||
WrittenBy: writtenBy,
|
||||
})
|
||||
return nil
|
||||
}, func(p string, err error) {
|
||||
slog.Warn("search: skipping path", "path", p, "err", err)
|
||||
})
|
||||
if err != nil {
|
||||
return nil, err
|
||||
@@ -275,8 +288,13 @@ func pathInScope(relPath, wing, hall string) bool {
|
||||
// excerpt, wing, and hall populated. Used for paths that surface only
|
||||
// via vector search.
|
||||
func hydrate(brainDir, relPath string) (Result, error) {
|
||||
full := filepath.Join(brainDir, filepath.FromSlash(relPath))
|
||||
content, err := os.ReadFile(full)
|
||||
// Through the brain root, regular files only (hyperguild#157).
|
||||
bfs, err := brainfs.Open(brainDir)
|
||||
if err != nil {
|
||||
return Result{}, err
|
||||
}
|
||||
defer func() { _ = bfs.Close() }()
|
||||
content, err := bfs.ReadRegular(filepath.ToSlash(relPath))
|
||||
if err != nil {
|
||||
return Result{}, err
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user