readExcerpt, ReadNote and ReadScoped now read through brainfs (os.Root, regular files only, type checked on the opened handle), so a symlink swapped in after a check is not followed and a FIFO does not hang the reader. ReadRegularMax bounds the read itself with io.LimitReader(max+1). notes.Target carries the brain root. Refs #157. Adversarial review round 2: PASS (round 1 BLOCKED: unbounded read, rule-9 test names). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: Mathias <mthbqv@gmail.com>
This commit was merged in pull request #165.
This commit is contained in:
@@ -20,6 +20,9 @@ import (
|
||||
// ErrNotRegular is returned for an entry that is not a regular file.
|
||||
var ErrNotRegular = errors.New("not a regular file")
|
||||
|
||||
// ErrTooLarge is returned by ReadRegularMax for a file over the limit.
|
||||
var ErrTooLarge = errors.New("file too large")
|
||||
|
||||
// Brain is an open brain directory.
|
||||
type Brain struct {
|
||||
root *os.Root
|
||||
@@ -74,7 +77,22 @@ func (b *Brain) ReadRegular(rel string) ([]byte, error) {
|
||||
// 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) {
|
||||
func (b *Brain) readOpened(rel string) ([]byte, error) { return b.readOpenedMax(rel, -1) }
|
||||
|
||||
// ReadRegularMax is ReadRegular with a size limit, checked on the opened
|
||||
// handle before reading (max < 0 means no limit).
|
||||
func (b *Brain) ReadRegularMax(rel string, max int64) ([]byte, error) {
|
||||
fi, err := b.root.Lstat(rel)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
if !fi.Mode().IsRegular() {
|
||||
return nil, fmt.Errorf("%s: %w", rel, ErrNotRegular)
|
||||
}
|
||||
return b.readOpenedMax(rel, max)
|
||||
}
|
||||
|
||||
func (b *Brain) readOpenedMax(rel string, max int64) ([]byte, error) {
|
||||
f, err := b.root.OpenFile(rel, os.O_RDONLY|syscall.O_NONBLOCK, 0)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
@@ -87,9 +105,31 @@ func (b *Brain) readOpened(rel string) ([]byte, error) {
|
||||
if !hfi.Mode().IsRegular() {
|
||||
return nil, fmt.Errorf("%s: %w", rel, ErrNotRegular)
|
||||
}
|
||||
return io.ReadAll(f)
|
||||
if max >= 0 && hfi.Size() > max {
|
||||
return nil, fmt.Errorf("%s is %d bytes (limit %d): %w", rel, hfi.Size(), max, ErrTooLarge)
|
||||
}
|
||||
var r io.Reader = f
|
||||
if wrapAfterStat != nil {
|
||||
r = wrapAfterStat(r)
|
||||
}
|
||||
if max < 0 {
|
||||
return io.ReadAll(r)
|
||||
}
|
||||
// The handle's size can grow after the check; bound the read itself.
|
||||
data, err := io.ReadAll(io.LimitReader(r, max+1))
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
if int64(len(data)) > max {
|
||||
return nil, fmt.Errorf("%s grew past %d bytes while read: %w", rel, max, ErrTooLarge)
|
||||
}
|
||||
return data, nil
|
||||
}
|
||||
|
||||
// wrapAfterStat is a test seam: it runs between the size check and the read,
|
||||
// and wraps the reader the read consumes.
|
||||
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
|
||||
|
||||
@@ -2,6 +2,7 @@ package brainfs
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"io"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"syscall"
|
||||
@@ -90,3 +91,51 @@ func TestWalkRegularYieldsOnlyRegularFiles(t *testing.T) {
|
||||
require.NoError(t, b.WalkRegular(".", func(rel string) error { got = append(got, rel); return nil }, nil))
|
||||
assert.Equal(t, []string{"a/n.md"}, got)
|
||||
}
|
||||
|
||||
func TestReadRegularMaxRefusesAFileOverTheLimit(t *testing.T) {
|
||||
dir := t.TempDir()
|
||||
require.NoError(t, os.WriteFile(filepath.Join(dir, "big.md"), make([]byte, 100), 0o644))
|
||||
b, err := Open(dir)
|
||||
require.NoError(t, err)
|
||||
defer func() { _ = b.Close() }()
|
||||
_, err = b.ReadRegularMax("big.md", 99)
|
||||
assert.True(t, errors.Is(err, ErrTooLarge), "got %v", err)
|
||||
got, err := b.ReadRegularMax("big.md", 100)
|
||||
require.NoError(t, err)
|
||||
assert.Len(t, got, 100)
|
||||
}
|
||||
|
||||
func TestReadRegularMaxRefusesAndStopsReadingAFileThatGrowsPastTheLimit(t *testing.T) {
|
||||
dir := t.TempDir()
|
||||
p := filepath.Join(dir, "grow.md")
|
||||
require.NoError(t, os.WriteFile(p, make([]byte, 10), 0o644))
|
||||
consumed := &countingReader{}
|
||||
wrapAfterStat = func(r io.Reader) io.Reader {
|
||||
f, err := os.OpenFile(p, os.O_WRONLY|os.O_APPEND, 0)
|
||||
require.NoError(t, err)
|
||||
_, err = f.Write(make([]byte, 1000))
|
||||
require.NoError(t, err)
|
||||
require.NoError(t, f.Close())
|
||||
consumed.r = r
|
||||
return consumed
|
||||
}
|
||||
t.Cleanup(func() { wrapAfterStat = nil })
|
||||
b, err := Open(dir)
|
||||
require.NoError(t, err)
|
||||
defer func() { _ = b.Close() }()
|
||||
got, err := b.ReadRegularMax("grow.md", 100)
|
||||
assert.True(t, errors.Is(err, ErrTooLarge), "got %v", err)
|
||||
assert.Nil(t, got)
|
||||
assert.LessOrEqual(t, consumed.n, 101, "read past max+1 bytes")
|
||||
}
|
||||
|
||||
type countingReader struct {
|
||||
r io.Reader
|
||||
n int
|
||||
}
|
||||
|
||||
func (c *countingReader) Read(p []byte) (int, error) {
|
||||
n, err := c.r.Read(p)
|
||||
c.n += n
|
||||
return n, err
|
||||
}
|
||||
|
||||
@@ -0,0 +1,103 @@
|
||||
// Package brainfstest is the shared harness for hyperguild#157's swap-race
|
||||
// tests: it keeps swapping a regular note, a symlink to an outside file, or a
|
||||
// FIFO (with a writer held open) into a path while a reader under test loops.
|
||||
// It observes two things: no sentinel byte from the outside file reaches the
|
||||
// reader's output, and the reader returns within a timeout. It does not cover
|
||||
// hard links, other paths into the reader, or races it does not generate.
|
||||
package brainfstest
|
||||
|
||||
import (
|
||||
"os"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"sync"
|
||||
"sync/atomic"
|
||||
"syscall"
|
||||
"testing"
|
||||
"time"
|
||||
)
|
||||
|
||||
// Sentinel marks the outside file's contents.
|
||||
const Sentinel = "zqbrainfssentinelonefiftyseven"
|
||||
|
||||
// Outside writes a file outside any brain dir containing Sentinel.
|
||||
func Outside(t *testing.T) string {
|
||||
t.Helper()
|
||||
p := filepath.Join(t.TempDir(), "secret.md")
|
||||
if err := os.WriteFile(p, []byte("# secret\n\n"+Sentinel), 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
return p
|
||||
}
|
||||
|
||||
// StartSwap renames, in a loop, a regular note (content inside) and either
|
||||
// a symlink to outside or a FIFO into path, until the test ends. FIFOs get a
|
||||
// writer held open until the swapper exits, so a reader that skips the
|
||||
// handle check parks forever instead of reading EOF. The swapper is stopped
|
||||
// and awaited before t.TempDir cleanup.
|
||||
func StartSwap(t *testing.T, path, inside, outside string, fifo bool) {
|
||||
t.Helper()
|
||||
var stop atomic.Bool
|
||||
var wg sync.WaitGroup
|
||||
wg.Add(1)
|
||||
go func() {
|
||||
defer wg.Done()
|
||||
tmp := path + ".swap"
|
||||
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(inside), 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)
|
||||
}
|
||||
}()
|
||||
t.Cleanup(func() { stop.Store(true); wg.Wait() })
|
||||
}
|
||||
|
||||
// AssertNoLeak loops read for d while a symlink swapper runs on path, and
|
||||
// fails if any output contains Sentinel.
|
||||
func AssertNoLeak(t *testing.T, path, inside string, d time.Duration, read func() string) {
|
||||
t.Helper()
|
||||
StartSwap(t, path, inside, Outside(t), false)
|
||||
leaks, iters := 0, 0
|
||||
for deadline := time.Now().Add(d); time.Now().Before(deadline); iters++ {
|
||||
leaks += strings.Count(read(), Sentinel)
|
||||
}
|
||||
if leaks > 0 {
|
||||
t.Fatalf("outside file leaked %d times in %d iterations", leaks, iters)
|
||||
}
|
||||
}
|
||||
|
||||
// AssertNoHang loops read for d while a FIFO swapper (held writers) runs on
|
||||
// path, and fails if a read does not return within timeout.
|
||||
func AssertNoHang(t *testing.T, path, inside string, d, timeout time.Duration, read func()) {
|
||||
t.Helper()
|
||||
StartSwap(t, path, inside, Outside(t), true)
|
||||
done := make(chan struct{})
|
||||
go func() {
|
||||
for deadline := time.Now().Add(d); time.Now().Before(deadline); {
|
||||
read()
|
||||
}
|
||||
close(done)
|
||||
}()
|
||||
select {
|
||||
case <-done:
|
||||
case <-time.After(timeout):
|
||||
t.Fatal("reader hung on a swapped-in FIFO with a writer held open")
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user