fix(brainfs): bound the read in ReadRegularMax; honest test names (#157)
Review round 1 on PR #165: - ReadRegularMax checked the size on the opened handle, then read with an unbounded io.ReadAll, so a file growing after the check returned past the limit. The read now goes through io.LimitReader(max+1). A test seam grows the file between the check and the read and counts bytes consumed. - New: ReadScoped returns ErrTooLarge for a note over maxNoteBytes; red if readContained drops the limit. - notes.Target carries the brain root instead of deriving it by TrimSuffix. - Rule 9: reader tests renamed to <Reader>RefusesSymlinksAndSurvivesSwapRaces; ReadNote test has a positive control; brainfstest comment states what the harness observes and what it does not. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -75,8 +75,7 @@ func readContained(t Target) ([]byte, error) {
|
||||
// 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.
|
||||
brainDir := strings.TrimSuffix(t.Abs, string(filepath.Separator)+filepath.FromSlash(t.Rel))
|
||||
bfs, err := brainfs.Open(brainDir)
|
||||
bfs, err := brainfs.Open(t.Brain)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("notes: %w", err)
|
||||
}
|
||||
|
||||
@@ -58,8 +58,9 @@ var (
|
||||
|
||||
// Target is a resolved, contained path.
|
||||
type Target struct {
|
||||
Rel string // brain-relative, cleaned
|
||||
Abs string // absolute, symlink-checked
|
||||
Rel string // brain-relative, cleaned
|
||||
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,
|
||||
@@ -109,7 +110,7 @@ func Resolve(brainDir, path string, scope Scope) (Target, error) {
|
||||
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
|
||||
|
||||
@@ -15,7 +15,7 @@ import (
|
||||
// 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 TestReadScopedIsConfinedToRegularFilesInTheBrain(t *testing.T) {
|
||||
func TestReadScopedRefusesSymlinksAndSurvivesSwapRaces(t *testing.T) {
|
||||
brain := t.TempDir()
|
||||
facts := filepath.Join(brain, "wiki", "w", "facts")
|
||||
require.NoError(t, os.MkdirAll(facts, 0o755))
|
||||
@@ -29,3 +29,12 @@ func TestReadScopedIsConfinedToRegularFilesInTheBrain(t *testing.T) {
|
||||
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)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user