fix(brainfs): check every component of the walk root (#157)
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>
This commit is contained in:
@@ -13,7 +13,9 @@ import (
|
||||
"io"
|
||||
"io/fs"
|
||||
"os"
|
||||
"path"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"syscall"
|
||||
)
|
||||
|
||||
@@ -59,6 +61,27 @@ 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
|
||||
// regular file, both before the open and on the opened handle.
|
||||
func (b *Brain) ReadRegular(rel string) ([]byte, error) {
|
||||
@@ -138,18 +161,22 @@ func (b *Brain) readOpenedMax(rel string, max int64) ([]byte, error) {
|
||||
var wrapAfterStat func(io.Reader) io.Reader
|
||||
|
||||
// WalkRegular walks a brain-relative directory through the root and calls fn
|
||||
// for each regular file (directories are descended; symlinks, FIFOs and
|
||||
// devices are skipped, never followed). An entry error is passed to onErr and
|
||||
// the walk continues; a nil onErr ignores it.
|
||||
// for each regular file. Directories are descended; symlinks, FIFOs and
|
||||
// devices found by the walk are skipped. A walk root with any component that
|
||||
// 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 {
|
||||
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 a real directory first.
|
||||
fi, err := b.root.Lstat(relDir)
|
||||
if err == nil && !fi.IsDir() {
|
||||
err = fmt.Errorf("%s: walk root is not a directory: %w", relDir, ErrNotRegular)
|
||||
}
|
||||
// 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)
|
||||
|
||||
@@ -172,3 +172,20 @@ func TestWalkRegularReportsASymlinkedWalkRootAndYieldsNothing(t *testing.T) {
|
||||
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)
|
||||
}
|
||||
|
||||
@@ -64,7 +64,7 @@ func TestQueryDoesNotFollowASymlinkedDirectory(t *testing.T) {
|
||||
// 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 TestQueryDoesNotFollowASymlinkedSearchRootInsideTheBrain(t *testing.T) {
|
||||
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))
|
||||
@@ -74,6 +74,18 @@ func TestQueryDoesNotFollowASymlinkedSearchRootInsideTheBrain(t *testing.T) {
|
||||
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
|
||||
// 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
|
||||
|
||||
Reference in New Issue
Block a user