diff --git a/ingestion/internal/brainfs/brainfs.go b/ingestion/internal/brainfs/brainfs.go index 8cbb477..c370d7b 100644 --- a/ingestion/internal/brainfs/brainfs.go +++ b/ingestion/internal/brainfs/brainfs.go @@ -60,6 +60,21 @@ func (b *Brain) ReadRegular(rel string) ([]byte, error) { if !fi.Mode().IsRegular() { return nil, fmt.Errorf("%s: %w", rel, ErrNotRegular) } + return b.readOpened(rel) +} + +// readOpened opens rel non-blocking and reads it only if the OPENED handle is +// a regular file. This is the check that matters under a race: an entry +// swapped for a FIFO after the Lstat, with a writer holding it open, would +// otherwise park io.ReadAll forever (PR #161 review: removing it hung search +// after 16 iterations). +// +// Accepted residual, as in api/rawroot.go: a HARD link to an outside file is +// a regular file and is read. Syncthing syncs contents, not links, so a peer +// 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 +// them in. Refusing Nlink > 1 is the next step if either changes. +func (b *Brain) readOpened(rel string) ([]byte, error) { f, err := b.root.OpenFile(rel, os.O_RDONLY|syscall.O_NONBLOCK, 0) if err != nil { return nil, err diff --git a/ingestion/internal/brainfs/brainfs_test.go b/ingestion/internal/brainfs/brainfs_test.go new file mode 100644 index 0000000..4d4b2dc --- /dev/null +++ b/ingestion/internal/brainfs/brainfs_test.go @@ -0,0 +1,92 @@ +package brainfs + +import ( + "errors" + "os" + "path/filepath" + "syscall" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// The handle check is what stops a FIFO swapped in after the Lstat. A FIFO +// with no writer reads EOF at once, so a race test could not see the check +// missing; here a writer holds the FIFO open, as any local process can, and +// readOpened must refuse it promptly (PR #161 review). +func TestReadOpenedRefusesAFIFOHeldOpenByAWriter(t *testing.T) { + dir := t.TempDir() + fifo := filepath.Join(dir, "p.md") + require.NoError(t, syscall.Mkfifo(fifo, 0o644)) + w, err := os.OpenFile(fifo, os.O_RDWR, 0) + require.NoError(t, err) + defer func() { _ = w.Close() }() + + b, err := Open(dir) + require.NoError(t, err) + defer func() { _ = b.Close() }() + + done := make(chan error, 1) + go func() { _, err := b.readOpened("p.md"); done <- err }() + select { + case err := <-done: + assert.True(t, errors.Is(err, ErrNotRegular), "got %v", err) + case <-time.After(2 * time.Second): + t.Fatal("readOpened blocked on a FIFO with a writer") + } +} + +func TestReadRegularRefusesSymlinksAndReadsRegularFiles(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, "ok.md"), []byte("ok"), 0o644)) + outside := filepath.Join(t.TempDir(), "s.md") + require.NoError(t, os.WriteFile(outside, []byte("secret"), 0o644)) + require.NoError(t, os.Symlink(outside, filepath.Join(dir, "link.md"))) + b, err := Open(dir) + require.NoError(t, err) + defer func() { _ = b.Close() }() + got, err := b.ReadRegular("ok.md") + require.NoError(t, err) + assert.Equal(t, "ok", string(got)) + _, err = b.ReadRegular("link.md") + assert.True(t, errors.Is(err, ErrNotRegular), "got %v", err) + _, err = b.ReadRegular("../" + filepath.Base(outside)) + assert.Error(t, err) +} + +func TestRelAcceptsPathsInsideAndRefusesEscapes(t *testing.T) { + dir := t.TempDir() + b, err := Open(dir) + require.NoError(t, err) + defer func() { _ = b.Close() }() + for in, want := range map[string]string{ + filepath.Join(dir, "wiki", "x.md"): "wiki/x.md", + filepath.Join(dir, "..foo", "x"): "..foo/x", + dir: ".", + } { + got, err := b.Rel(in) + require.NoError(t, err, in) + assert.Equal(t, want, got, in) + } + for _, bad := range []string{filepath.Join(dir, "..", "x"), dir + "-evil/x", "/etc/passwd", filepath.Join(dir, "wiki", "..", "..", "x")} { + _, err := b.Rel(bad) + assert.Error(t, err, bad) + } +} + +func TestWalkRegularYieldsOnlyRegularFiles(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(dir, "a"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "a", "n.md"), []byte("n"), 0o644)) + require.NoError(t, os.Symlink(t.TempDir(), filepath.Join(dir, "a", "dirlink"))) + require.NoError(t, os.Symlink(filepath.Join(dir, "a", "n.md"), filepath.Join(dir, "a", "filelink.md"))) + require.NoError(t, syscall.Mkfifo(filepath.Join(dir, "a", "p.md"), 0o644)) + b, err := Open(dir) + require.NoError(t, err) + defer func() { _ = b.Close() }() + var got []string + require.NoError(t, b.WalkRegular(".", func(rel string) error { got = append(got, rel); return nil }, nil)) + assert.Equal(t, []string{"a/n.md"}, got) +}