amadan

carlos / platform Public

hostfiles: one 4095-byte path cap, with no depth cap, is enforced by every writer (Stage, StageSkipped, StageDelete, Commit, the watcher) as by the readers; the watcher skips an over-cap path and logs it once.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Paul Campbell pushed by claude 400a33029c5b016b6078d9ceda1406cebdf6e621 parent 3682ec7
7 files changed, +213 −26
  • internal/host/relocationflush_saferead_linux_test.go +1 −1
  • internal/hostfiles/fix3_linux_test.go +9 −9
  • internal/hostfiles/fix4_linux_test.go +154 −0
  • internal/hostfiles/publisher.go +18 −0
  • internal/hostfiles/run.go +15 −0
  • internal/hostfiles/sync.go +14 −14
  • internal/hostfiles/walk_unix.go +2 −2

CI cancelled — superseded by a newer push — run details

CI log 
=== gate ===
cd launchctx && go vet ./... && go test ./... -count=1
ok  	amadan.net/carlos/platform.git/launchctx	0.011s
?   	amadan.net/carlos/platform.git/launchctx/controlname	[no test files]
?   	amadan.net/carlos/platform.git/launchctx/wireid	[no test files]
go vet ./...
gofmt -l .: clean
go test ./... -count=1 -timeout 30m

amadan: job cancelled — a newer push superseded it
diff --git a/internal/host/relocationflush_saferead_linux_test.go b/internal/host/relocationflush_saferead_linux_test.go
index 4f0982aa3..78d3beab6 100644
--- a/internal/host/relocationflush_saferead_linux_test.go
+++ b/internal/host/relocationflush_saferead_linux_test.go
@@ -392,7 +392,7 @@ func TestReleaseFlushRefusesATreePastTheWalksCaps(t *testing.T) {
t.Fatal(err)
}
defer root.Close()
- if err := root.MkdirAll(strings.TrimSuffix(strings.Repeat("d/", hostfiles.MaxDepth+1), "/"), 0o700); err != nil {
+ if err := root.MkdirAll(strings.TrimSuffix(strings.Repeat("d/", hostfiles.MaxPathBytes/2+2), "/"), 0o700); err != nil {
t.Fatal(err)
}
_, err = x.flush(t)
diff --git a/internal/hostfiles/fix3_linux_test.go b/internal/hostfiles/fix3_linux_test.go
index f48b518c4..1b5a4b993 100644
--- a/internal/hostfiles/fix3_linux_test.go
+++ b/internal/hostfiles/fix3_linux_test.go
@@ -86,8 +86,9 @@ func TestPresentNeverReadsAVanishingDirectoryAsNoEnvFile(t *testing.T) {
<-flapped
}
-// Opus I1: the walk refuses a path past MaxDepth components or
-// MaxPathBytes bytes, at the boundary exactly, and a chain far past it is
+// Opus I1 (Fix 4: the byte cap alone): the walk refuses a path past
+// MaxPathBytes bytes, at the boundary exactly, accepts a deep tree within
+// it, and a chain far past it is
// refused quickly with bounded allocation.
func TestSyncDirRefusesATreePastItsCaps(t *testing.T) {
build := func(t *testing.T, comps []string, file bool) string {
@@ -132,14 +133,13 @@ func TestSyncDirRefusesATreePastItsCaps(t *testing.T) {
file bool
ok bool
}{
- {"a file at MaxDepth components", repeat(MaxDepth, "d"), true, true},
- {"a file at MaxDepth+1 components", repeat(MaxDepth+1, "d"), true, false},
- {"directories MaxDepth+1 deep, no file", repeat(MaxDepth+1, "d"), false, false},
+ {"a file at 257 one-byte components", repeat(257, "d"), true, true},
+ {"directories past the byte cap, no file", repeat(2100, "d"), false, false},
{"a path of MaxPathBytes", strings.Split(pathOfBytes(MaxPathBytes), "/"), true, true},
{"a path of MaxPathBytes+1", strings.Split(pathOfBytes(MaxPathBytes+1), "/"), true, false},
} {
t.Run(tc.name, func(t *testing.T) {
- if got := len(strings.Join(tc.comps, "/")); strings.Contains(tc.name, "Bytes") && got != MaxPathBytes && got != MaxPathBytes+1 {
+ if got := len(strings.Join(tc.comps, "/")); strings.Contains(tc.name, "MaxPathBytes") && got != MaxPathBytes && got != MaxPathBytes+1 {
t.Fatalf("built a %d-byte path", got)
}
err := sync(t, context.Background(), build(t, tc.comps, tc.file))
@@ -191,7 +191,7 @@ func TestRestoreRefusesAManifestPathPastTheCaps(t *testing.T) {
path string
ok bool
}{
- {deep(MaxDepth), true}, {deep(MaxDepth + 1), false},
+ {deep(257), true}, {deep(2048), true}, {deep(2049), false},
{pathOfBytes(MaxPathBytes), true},
{pathOfBytes(MaxPathBytes + 1), false},
} {
@@ -201,7 +201,7 @@ func TestRestoreRefusesAManifestPathPastTheCaps(t *testing.T) {
}
}
ctx, s := context.Background(), release.NewDirStore(t.TempDir())
- body, err := json.Marshal(man(deep(MaxDepth + 1)))
+ body, err := json.Marshal(man(pathOfBytes(MaxPathBytes + 1)))
if err != nil {
t.Fatal(err)
}
@@ -218,7 +218,7 @@ func TestRestoreRefusesAManifestPathPastTheCaps(t *testing.T) {
}
// pathOfBytes is a relative path of exactly n bytes in components of at
-// most 200 bytes, so its depth stays far under MaxDepth.
+// most 200 bytes.
func pathOfBytes(n int) string {
var b strings.Builder
for b.Len() < n {
diff --git a/internal/hostfiles/fix4_linux_test.go b/internal/hostfiles/fix4_linux_test.go
new file mode 100644
index 000000000..5924e8715
--- /dev/null
+++ b/internal/hostfiles/fix4_linux_test.go
@@ -0,0 +1,154 @@
+//go:build linux
+
+package hostfiles
+
+import (
+ "context"
+ "errors"
+ "os"
+ "path/filepath"
+ "strings"
+ "testing"
+ "time"
+
+ "github.com/carlosframework/platform/internal/release"
+)
+
+// Fix 4 (Astra confirming P2): a deep path within the byte cap (257
+// one-byte components, 513 bytes) travels the whole way: published by a
+// walk, accepted by every reader, restored and found present.
+func TestADeepPathWithinTheCapPublishesAndRestores(t *testing.T) {
+ ctx := context.Background()
+ rel := strings.Repeat("d/", 256) + "f"
+ src := t.TempDir()
+ root, err := os.OpenRoot(src)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if err := root.MkdirAll(strings.TrimSuffix(strings.Repeat("d/", 256), "/"), 0o700); err != nil {
+ t.Fatal(err)
+ }
+ if err := root.WriteFile(rel, []byte("deep"), 0o600); err != nil {
+ t.Fatal(err)
+ }
+ root.Close()
+ s := release.NewDirStore(t.TempDir())
+ p := open(t, s)
+ if _, err := SyncDir(ctx, p, src, nil); err != nil {
+ t.Fatal(err)
+ }
+ commit(t, p)
+ m, _, err := Load(ctx, s, pfx)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if err := ValidateManifest(m, []string{pfx}); err != nil {
+ t.Fatalf("ValidateManifest refused what the writer published: %v", err)
+ }
+ dst := t.TempDir()
+ if _, st, err := Restore(ctx, s, pfx, dst, nil); err != nil || st.Written != 1 {
+ t.Fatalf("Restore: %+v %v", st, err)
+ }
+ r2, err := os.OpenRoot(dst)
+ if err != nil {
+ t.Fatal(err)
+ }
+ defer r2.Close()
+ if b, err := r2.ReadFile(rel); err != nil || string(b) != "deep" {
+ t.Fatalf("restored %q %v", b, err)
+ }
+ if ok, err := Present(dst, filepath.Join(t.TempDir(), "shop.example.env"), m, true); err != nil || !ok {
+ t.Fatalf("Present: %v %v", ok, err)
+ }
+}
+
+// Every writer refuses a path past the byte cap, exactly where the readers
+// do, and accepts one at it; Commit never writes a manifest that holds one.
+func TestTheWritersRefuseAPathPastTheCapAsTheReadersDo(t *testing.T) {
+ ctx := context.Background()
+ at, past := pathOfBytes(MaxPathBytes), pathOfBytes(MaxPathBytes+1)
+ p := open(t, release.NewDirStore(t.TempDir()))
+ if err := p.Stage(ctx, at, []byte("x"), 0o600, false); err != nil {
+ t.Fatalf("Stage at the cap: %v", err)
+ }
+ if err := p.Stage(ctx, past, []byte("x"), 0o600, false); !errors.Is(err, ErrTooDeep) {
+ t.Fatalf("Stage past the cap: %v", err)
+ }
+ if err := p.StageSkipped(past, "too-large"); !errors.Is(err, ErrTooDeep) {
+ t.Fatalf("StageSkipped past the cap: %v", err)
+ }
+ if err := p.StageDelete(past); !errors.Is(err, ErrTooDeep) {
+ t.Fatalf("StageDelete past the cap: %v", err)
+ }
+ // A manifest the publisher continued that already holds such a path
+ // (written before the cap) is refused at Commit, never written again.
+ s := release.NewDirStore(t.TempDir())
+ q := open(t, s)
+ q.mu.Lock()
+ q.next++
+ q.man.Entries[past], q.dirty = Entry{Seq: q.next, Skipped: "too-large"}, true
+ q.mu.Unlock()
+ if _, err := q.Commit(ctx); !errors.Is(err, ErrTooDeep) {
+ t.Fatalf("Commit of a manifest holding a path past the cap: %v", err)
+ }
+ if _, found, err := Load(ctx, s, pfx); found || err != nil {
+ t.Fatalf("a refused Commit wrote a manifest: %v %v", found, err)
+ }
+ // And the readers draw the line at the same byte.
+ sha := strings.Repeat("a", 64)
+ e := Entry{Object: pfx + objectKey(1, sha), SHA256: sha, Seq: 1, Size: 1}
+ for path, ok := range map[string]bool{at: true, past: false} {
+ err := ValidateManifest(Manifest{V: ManifestVersion, Seq: 1, Entries: map[string]Entry{path: e}, Platform: map[string]Entry{}}, []string{pfx})
+ if ok != (err == nil) || !ok && !errors.Is(err, ErrTooDeep) {
+ t.Fatalf("ValidateManifest of %d bytes: %v", len(path), err)
+ }
+ }
+}
+
+// The watcher skips a path past the cap for good: it is never staged, so
+// no manifest a reader refuses is committed; it is not retried; and it
+// costs one log line however often it is touched. Other paths publish.
+func TestRunSkipsAPathPastTheCapAndLogsItOnce(t *testing.T) {
+ logs := captureLog(t)
+ s, dir, w := release.NewDirStore(t.TempDir()), t.TempDir(), newFake()
+ p := open(t, s)
+ stop := startRun(t, p, dir, w, eventsOnly)
+ past := pathOfBytes(MaxPathBytes + 1)
+ root, err := os.OpenRoot(dir)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if err := root.MkdirAll(filepath.Dir(past), 0o700); err != nil {
+ t.Fatal(err)
+ }
+ if err := root.WriteFile(past, []byte("x"), 0o600); err != nil {
+ t.Fatal(err)
+ }
+ root.Close()
+ w.ch <- event{rel: past}
+ write(t, dir, "ok", "ok")
+ w.ch <- event{rel: "ok"}
+ eventually(t, "ok published", func() bool { return manifestHas(s, "ok", sum([]byte("ok"))) })
+ w.ch <- event{rel: past}
+ w.ch <- event{rel: "ok"}
+ eventually(t, "the second touch handled", func() bool { return logs.count("past the 4095-byte cap") >= 1 })
+ // Long enough for a retry at every deadline to have logged again.
+ time.Sleep(10 * eventsOnly.Deadline)
+ stopErr := stop()
+ if !errors.Is(stopErr, context.Canceled) {
+ t.Fatalf("run ended with %v; a path past the cap must be skipped, not fatal", stopErr)
+ }
+ if got := logs.count("past the 4095-byte cap"); got != 1 {
+ t.Fatalf("logged %d times, want once", got)
+ }
+ m, _, err := Load(context.Background(), s, pfx)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if _, ok := m.Entries[past]; ok {
+ t.Fatal("the watcher published a path past the cap")
+ }
+ if err := ValidateManifest(m, []string{pfx}); err != nil {
+ t.Fatalf("the watcher committed a manifest a reader refuses: %v", err)
+ }
+}
diff --git a/internal/hostfiles/publisher.go b/internal/hostfiles/publisher.go
index a55621888..be3e72c86 100644
--- a/internal/hostfiles/publisher.go
+++ b/internal/hostfiles/publisher.go
@@ -78,6 +78,9 @@ func (p *Publisher) Stage(ctx context.Context, path string, body []byte, mode ui
if err != nil {
return err
}
+ if err := withinTreeCap(clean); err != nil {
+ return err
+ }
p.mu.Lock()
defer p.mu.Unlock()
if cur, ok := p.man.Entries[clean]; ok && !cur.Deleted && cur.Skipped == "" &&
@@ -98,6 +101,9 @@ func (p *Publisher) StageSkipped(path, why string) error {
if err != nil {
return err
}
+ if err := withinTreeCap(clean); err != nil {
+ return err
+ }
p.mu.Lock()
defer p.mu.Unlock()
if cur, ok := p.man.Entries[clean]; ok && cur.Skipped == why {
@@ -115,6 +121,9 @@ func (p *Publisher) StageDelete(path string) error {
if err != nil {
return err
}
+ if err := withinTreeCap(clean); err != nil {
+ return err
+ }
p.mu.Lock()
defer p.mu.Unlock()
if cur, ok := p.man.Entries[clean]; !ok || cur.Deleted {
@@ -155,6 +164,15 @@ func (p *Publisher) Commit(ctx context.Context) (int64, error) {
if !p.dirty {
return p.man.Seq, nil
}
+ // The readers refuse a manifest with a path past the cap
+ // (checkManifest), so none is written: Stage refuses one, and a
+ // manifest this publisher continued that already holds one is refused
+ // here rather than written again.
+ for path := range p.man.Entries {
+ if err := withinTreeCap(path); err != nil {
+ return 0, err
+ }
+ }
out := p.man
out.Seq = p.next
body, err := json.Marshal(out)
diff --git a/internal/hostfiles/run.go b/internal/hostfiles/run.go
index 4f9c18245..2d4375d00 100644
--- a/internal/hostfiles/run.go
+++ b/internal/hostfiles/run.go
@@ -78,6 +78,7 @@ func run(ctx context.Context, p *Publisher, dir string, o Options, w watcher) er
needScan := true // the first pass is a full scan: changes made while nothing watched
var lastScan, lastScanTry, lastRepair time.Time
coverageLost, scanFailing := false, false // for logging on transitions only
+ loggedTooDeep := false // publishOne's over-cap skip, logged once
for {
select {
case <-ctx.Done():
@@ -147,6 +148,17 @@ func run(ctx context.Context, p *Publisher, dir string, o Options, w watcher) er
}
for _, rel := range q.Due(now) {
again, err := publishOne(ctx, p, root, rel, o.Skip)
+ if errors.Is(err, ErrTooDeep) {
+ // Past the path cap: never staged, so never in a manifest a
+ // reader would refuse, and not retried (it cannot shrink).
+ // Logged once per run, so neither the log nor a set of
+ // paths grows with a tenant's supply of them.
+ if !loggedTooDeep {
+ loggedTooDeep = true
+ log.Printf("hostfiles: %s: a path past the %d-byte cap is not published (further ones are skipped without a line): %v", dir, MaxPathBytes, err)
+ }
+ continue
+ }
if err != nil {
log.Printf("hostfiles: %s: %v", rel, err)
q.Touch(rel, now) // try again at the next deadline
@@ -179,6 +191,9 @@ func publishOne(ctx context.Context, p *Publisher, root *dirHandle, rel string,
if err != nil {
return false, nil // a path the watcher should never have produced: skip it
}
+ if err := withinTreeCap(clean); err != nil {
+ return false, err // the caller skips it for good: no reader would accept it
+ }
if skipped(skip, clean, false) {
return false, nil
}
diff --git a/internal/hostfiles/sync.go b/internal/hostfiles/sync.go
index acf3cb425..65158cf8b 100644
--- a/internal/hostfiles/sync.go
+++ b/internal/hostfiles/sync.go
@@ -8,7 +8,6 @@ import (
"os"
"path"
"sort"
- "strings"
)
type fileState int
@@ -76,23 +75,24 @@ func SyncDir(ctx context.Context, p *Publisher, dir string, skip func(rel string
}
// MaxPathBytes caps a path in a files tree, relative to its root: PATH_MAX
-// (4096) less its terminating NUL. Every path a walk publishes can then be
-// opened by one openat2 beneath the restored root (Present), and no
-// manifest a restore accepts names a longer one.
+// (4096) less its terminating NUL, as moves' archive contract has it. It is
+// the one cap every writer (Stage, StageSkipped, StageDelete, Commit, the
+// watcher's publishOne, a walk) and every reader (checkManifest: Restore,
+// CopyManifest, ValidateManifest) applies, so no manifest is written that
+// a reader refuses. Every path it admits can be opened by one openat2
+// beneath the restored root (Present). It also bounds depth (about 2048
+// components of one byte), and with it a walk's open directories and its
+// path strings.
const MaxPathBytes = 4095
-// MaxDepth caps the components in such a path. With MaxPathBytes it
-// bounds a walk's open directories and its path strings.
-const MaxDepth = 256
+// ErrTooDeep is a path past MaxPathBytes: a writer refuses to stage it,
+// and the walk or manifest that holds it is refused whole.
+var ErrTooDeep = errors.New("hostfiles: a path is past the tree's length cap")
-// ErrTooDeep is a path past MaxPathBytes or MaxDepth: the walk that met
-// it, or the manifest that names it, is refused whole.
-var ErrTooDeep = errors.New("hostfiles: a path is past the tree's length or depth cap")
-
-// withinTreeCap is the one cap a walk and checkManifest share.
+// withinTreeCap is the one cap the writers and readers share.
func withinTreeCap(rel string) error {
- if n, d := len(rel), strings.Count(rel, "/")+1; n > MaxPathBytes || d > MaxDepth {
- return fmt.Errorf("%w: %d bytes in %d components (caps %d and %d)", ErrTooDeep, n, d, MaxPathBytes, MaxDepth)
+ if n := len(rel); n > MaxPathBytes {
+ return fmt.Errorf("%w: %d bytes (cap %d), starting %q", ErrTooDeep, n, MaxPathBytes, rel[:64])
}
return nil
}
diff --git a/internal/hostfiles/walk_unix.go b/internal/hostfiles/walk_unix.go
index 33afcde26..b63daa462 100644
--- a/internal/hostfiles/walk_unix.go
+++ b/internal/hostfiles/walk_unix.go
@@ -15,8 +15,8 @@ import (
// symlink and never blocks on a FIFO: a directory is entered only through
// openDir.
//
-// The walk is bounded (withinTreeCap): a path past MaxPathBytes or
-// MaxDepth components refuses the whole walk with ErrTooDeep, never a
+// The walk is bounded (withinTreeCap): a path past MaxPathBytes refuses
+// the whole walk with ErrTooDeep, never a
// silent skip, so a tree no restore could recreate is never published as
// complete, and a chain of directories cannot make the walk's memory grow
// with the square of its depth. ctx is checked on entering every