fix(search): walk and read the brain through an os.Root (#157, slice 1) (#161)

Co-authored-by: Mathias <mthbqv@gmail.com>

ingestion/internal/search/brainroot_test.go

@@ -0,0 +1,149 @@
+package search
+
+import (
+	"os"
+	"path/filepath"
+	"strings"
+	"sync"
+	"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))
+}
+
+// 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
+// that skips the handle check pass (PR #161 round 2: with a held writer, a
+// bypass of readOpened hung Query after 5 iterations).
+// startSwap runs swapInto until the test ends and waits for it before
+// t.TempDir's cleanup (hyperguild#162).
+func startSwap(t *testing.T, path, outside string, fifo bool) {
+	t.Helper()
+	var stop atomic.Bool
+	var wg sync.WaitGroup
+	wg.Add(1)
+	go func() { defer wg.Done(); swapInto(path, outside, fifo, &stop) }()
+	t.Cleanup(func() { stop.Store(true); wg.Wait() })
+}
+
+func swapInto(path, outside string, fifo bool, stop *atomic.Bool) {
+	tmp := path + ".swap"
+	// Writers stay open until the swapper exits, so a reader parked on a
+	// FIFO stays parked (closing one would hand the reader EOF and hide the
+	// hang). Past the fd limit, new FIFOs simply have no writer.
+	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("# r\n\nzqinside "+"zqracenote"), 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)
+	}
+}
+
+func TestQueryLeaksNothingUnderASymlinkSwapRace(t *testing.T) {
+	brain, facts, outside := searchBrain(t)
+	startSwap(t, filepath.Join(facts, "r.md"), outside, false)
+	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)
+	startSwap(t, filepath.Join(facts, "r.md"), outside, true)
+	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:
+	case <-time.After(6 * time.Second):
+		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)
+}