carlos / platform Public

hostfiles: the path cap applies to live entries only (LiveEntriesWithinCap), so a manifest written before the cap that holds a tombstone or skipped entry past it stays readable, and a moves recovery that selects it is not stranded (#236, moves' review of a8cebad)

Restore writes neither a tombstone nor a skipped entry: it lists a skipped
path and removes a tombstoned one, so neither needs the cap. A live entry
past the cap is still refused everywhere: by the readers (checkManifest,
ValidateManifest, CopyManifest, Restore), and by Open, OpenAt and Commit.
New writes still refuse a path past the cap: Stage, StageSkipped,
StageDelete and the walk.

LiveEntriesWithinCap is exported as the one spelling of the rule, for
moves' cut preflight (#236 item 2). Open, OpenAt and Commit call it under
its old name, manifestWithinCap, so the bodies the Plan 9 record-write
census and the Plan 14 root-read legacy freeze pin are byte-identical.
The Plan 14 hostfiles envelope moves in a new envelope-only overlay
(plan14-root-reads-hostfiles-cap-236.tsv); the legacy seal stands.

Tests:
- fix4 and fix5 now use a live over-cap entry as the refused case.
- New: TestATombstoneOrSkippedEntryPastTheCapStaysReadable. It covers
  Open, continue and commit, CopyManifest, ValidateManifest and Restore
  over over-cap tombstones and skipped entries, plus the live-entry
  refusal. It fails with the reader cap applied to every entry.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Paul Campbell pushed by claude 1 week ago 28d086d82fa76ce2f5b11538297945098d90c9b9 parent 132cf99

6 files changed, +108 −15

internal/hostfiles/fix4_linux_test.go +4 −3
internal/hostfiles/fix5_linux_test.go +75 −5
internal/hostfiles/publisher.go +15 −4
internal/hostfiles/restore.go +6 −3
internal/provision/plan14_root_reads_test.go +6 −0
internal/provision/testdata/plan14-root-reads-hostfiles-cap-236.tsv +2 −0

CI cancelled — superseded by a newer push — run details

diff --git a/internal/hostfiles/fix4_linux_test.go b/internal/hostfiles/fix4_linux_test.go

index 5924e8715..e093826ea 100644

--- a/internal/hostfiles/fix4_linux_test.go

+++ b/internal/hostfiles/fix4_linux_test.go

@@ -80,13 +80,14 @@ func TestTheWritersRefuseAPathPastTheCapAsTheReadersDo(t *testing.T) {

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.

+ // A manifest the publisher continued that already holds such a live

+ // path (written before the cap) is refused at Commit, never written

+ // again. A tombstone or a skipped entry is exempt (LiveEntriesWithinCap).

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.man.Entries[past], q.dirty = liveEntry(q.next), 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)

diff --git a/internal/hostfiles/fix5_linux_test.go b/internal/hostfiles/fix5_linux_test.go

index b029c6af0..56207dc7e 100644

--- a/internal/hostfiles/fix5_linux_test.go

+++ b/internal/hostfiles/fix5_linux_test.go

@@ -8,6 +8,9 @@ import (

"encoding/json"

"errors"

"os"

+ "path/filepath"

+ "slices"

+ "strings"

"testing"

"time"

@@ -17,7 +20,23 @@ import (

// inheritOverCap writes, under pfx, the manifest a writer from before the

// path cap could have left: a healthy published file beside an entry whose

// path is past MaxPathBytes. It returns the manifest's bytes.

+// liveEntry is a live entry at seq whose object this package would have

+// written under pfx.

+func liveEntry(seq int64) Entry {

+ sha := strings.Repeat("a", 64)

+ return Entry{Object: pfx + objectKey(seq, sha), SHA256: sha, Seq: seq, Size: 1}

+// inheritOverCap writes, over a healthy manifest at pfx, one holding a live

+// path past the cap, as a writer before the cap could have.

func inheritOverCap(t *testing.T, s release.Store) []byte {

+ t.Helper()

+ return inheritPastTheCap(t, s, func(seq int64) map[string]Entry {

+ return map[string]Entry{pathOfBytes(MaxPathBytes + 1): liveEntry(seq)}

+ })

+func inheritPastTheCap(t *testing.T, s release.Store, extra func(seq int64) map[string]Entry) []byte {

t.Helper()

ctx := context.Background()

p := open(t, s)

@@ -28,7 +47,9 @@ func inheritOverCap(t *testing.T, s release.Store) []byte {

t.Fatal(err)

}

m.Seq++

- m.Entries[pathOfBytes(MaxPathBytes+1)] = Entry{Seq: m.Seq, Skipped: "too-large"}

+ for p, e := range extra(m.Seq) {

+ m.Entries[p] = e

+ }

body, err := json.Marshal(m)

if err != nil {

t.Fatal(err)

@@ -43,9 +64,9 @@ func inheritOverCap(t *testing.T, s release.Store) []byte {

return body

}

-// Fix 5 (Astra confirming P2): a manifest inherited with a path past the

-// cap is refused by Open, terminally, and by every reader, and nothing is

-// committed over it.

+// Fix 5 (Astra confirming P2): a manifest inherited with a live path past

+// the cap is refused by Open, terminally, and by every reader, and nothing

+// is committed over it.

func TestAnInheritedManifestPastTheCapIsRefusedByOpenAndTheReaders(t *testing.T) {

ctx := context.Background()

s := release.NewDirStore(t.TempDir())

@@ -91,7 +112,7 @@ func TestRunStopsOnAManifestItCannotCommit(t *testing.T) {

}

p.mu.Lock()

p.next++

- p.man.Entries[pathOfBytes(MaxPathBytes+1)] = Entry{Seq: p.next, Skipped: "too-large"}

+ p.man.Entries[pathOfBytes(MaxPathBytes+1)] = liveEntry(p.next)

p.mu.Unlock()

write(t, dir, "healthy.txt", "changed") // the first scan stages it, and Commit is attempted

ctx, cancel := context.WithCancel(context.Background())

@@ -114,3 +135,52 @@ func TestRunStopsOnAManifestItCannotCommit(t *testing.T) {

t.Fatalf("something was committed: %v", err)

}

+// Tombstones and skipped entries past the cap are exempt (moves' review of

+// a8cebad, #236): Restore writes neither, so a manifest written before the

+// cap that holds them is still opened, continued, validated, copied and

+// restored, and a recovery that selects it is not stranded. The tombstone

+// is removed, as any other tombstone is.

+func TestATombstoneOrSkippedEntryPastTheCapStaysReadable(t *testing.T) {

+ ctx := context.Background()

+ s, dir := release.NewDirStore(t.TempDir()), t.TempDir()

+ dead, skipped := pathOfBytes(MaxPathBytes+1), pathOfBytes(MaxPathBytes+2)

+ inheritPastTheCap(t, s, func(seq int64) map[string]Entry {

+ return map[string]Entry{dead: {Seq: seq, Deleted: true}, skipped: {Seq: seq, Skipped: "too-large"}}

+ })

+ m, _, err := Load(ctx, s, pfx)

+ if err != nil {

+ t.Fatal(err)

+ }

+ if err := LiveEntriesWithinCap(m); err != nil {

+ t.Fatalf("LiveEntriesWithinCap: %v", err)

+ }

+ if err := ValidateManifest(m, []string{pfx}); err != nil {

+ t.Fatalf("ValidateManifest: %v", err)

+ }

+ p, err := Open(ctx, s, pfx)

+ if err != nil {

+ t.Fatalf("Open: %v", err)

+ }

+ stage(t, p, "more.txt", "more")

+ commit(t, p)

+ const to = "instances/shop.test/abc/files/g9/"

+ if _, err := CopyManifest(ctx, s, pfx, to, 0, nil); err != nil {

+ t.Fatalf("CopyManifest: %v", err)

+ }

+ got, st, err := Restore(ctx, s, pfx, dir, nil)

+ if err != nil {

+ t.Fatalf("Restore: %v", err)

+ }

+ if st.Written != 2 || !slices.Equal(st.Skipped, []string{skipped}) || !got.Entries[dead].Deleted {

+ t.Fatalf("restore: %+v", st)

+ }

+ if b, err := os.ReadFile(filepath.Join(dir, "healthy.txt")); err != nil || string(b) != "fine" {

+ t.Fatalf("healthy.txt: %q %v", b, err)

+ }

+ // A live path past the cap is still refused by the same rule.

+ m.Entries[pathOfBytes(MaxPathBytes+3)] = liveEntry(m.Seq)

+ if err := LiveEntriesWithinCap(m); !errors.Is(err, ErrTooDeep) {

+ t.Fatalf("a live path past the cap: %v", err)

+ }

diff --git a/internal/hostfiles/publisher.go b/internal/hostfiles/publisher.go

index 6fa47d036..17cc6e834 100644

--- a/internal/hostfiles/publisher.go

+++ b/internal/hostfiles/publisher.go

@@ -57,11 +57,22 @@ func Open(ctx context.Context, s release.Store, prefix string) (*Publisher, erro

return &Publisher{s: s, prefix: prefix, man: m, etag: etag, next: m.Seq}, nil

}

-// manifestWithinCap holds every entry path of m to withinTreeCap, the cap

-// the readers apply: Open refuses to continue a manifest that fails it,

-// and Commit refuses to write one.

+// LiveEntriesWithinCap holds every live entry path of m to the tree's

+// length cap (withinTreeCap): the one rule the readers (checkManifest),

+// Open, OpenAt and Commit apply, and the check a caller makes before it

+// starts work that will need m restored (a move's cut). A tombstone or a

+// skipped entry is exempt: Restore writes neither, so a manifest written

+// before the cap that holds one past it stays readable and continuable.

+// A live entry past the cap is ErrTooDeep: no restore could recreate it.

+func LiveEntriesWithinCap(m Manifest) error { return manifestWithinCap(m) }

+// manifestWithinCap is LiveEntriesWithinCap. Open, OpenAt and Commit call

+// it by this name, which the root-read and record-write censuses pin.

func manifestWithinCap(m Manifest) error {

- for path := range m.Entries {

+ for path, e := range m.Entries {

+ if e.Deleted || e.Skipped != "" {

+ continue

+ }

if err := withinTreeCap(path); err != nil {

return err

}

diff --git a/internal/hostfiles/restore.go b/internal/hostfiles/restore.go

index acf477529..3fc5af304 100644

--- a/internal/hostfiles/restore.go

+++ b/internal/hostfiles/restore.go

@@ -264,13 +264,16 @@ func checkManifest(m Manifest, origins []string) error {

return fmt.Errorf("hostfiles: platform item %q: %w", name, err)

}

+ // A live path past the cap: no restore could recreate it, so it is

+ // refused before anything is touched (tombstones and skipped entries

+ // are exempt; Restore writes neither).

+ if err := LiveEntriesWithinCap(m); err != nil {

+ return err

+ }

for p, e := range m.Entries {

if _, err := CleanPath(p); err != nil {

return err

}

- if err := withinTreeCap(p); err != nil {

- return err // no restore could recreate it: refused before anything is touched

- }

if err := checkEntry(e, origins); err != nil {

return fmt.Errorf("hostfiles: %s: %w", p, err)

}

diff --git a/internal/provision/plan14_root_reads_test.go b/internal/provision/plan14_root_reads_test.go

index 1c7a2443b..79105e458 100644

--- a/internal/provision/plan14_root_reads_test.go

+++ b/internal/provision/plan14_root_reads_test.go

@@ -691,6 +691,12 @@ func TestPlan14RootReadCensus(t *testing.T) {

if err = loadRootReadInventory(filepath.Join(root, "internal/provision/testdata/plan14-root-reads-census-ruling-275.tsv"), &inv, true); err != nil {

t.Fatal(err)

}

+ // Census overlay hostfiles-cap-236 (amadan #236): envelope-only, for the

+ // hostfiles path cap applying to live entries only. No root-read unit

+ // changed or was added, so the legacy seal above stands.

+ if err = loadRootReadInventory(filepath.Join(root, "internal/provision/testdata/plan14-root-reads-hostfiles-cap-236.tsv"), &inv, true); err != nil {

+ t.Fatal(err)

+ }

errs := append(compareRootReads(got, inv), checkRootReadLegacy(inv, seal)...)

errs = append(errs, checkRootReadDeclarations(got, inv)...)

if len(errs) > 0 {

diff --git a/internal/provision/testdata/plan14-root-reads-hostfiles-cap-236.tsv b/internal/provision/testdata/plan14-root-reads-hostfiles-cap-236.tsv

new file mode 100644

index 000000000..1e1c1c83a

--- /dev/null

+++ b/internal/provision/testdata/plan14-root-reads-hostfiles-cap-236.tsv

@@ -0,0 +1,2 @@

+# Plan 14 census overlay hostfiles-cap-236 (amadan #236, owner region HA, basis moves' review of a8cebad): the hostfiles path cap applies to live entries only (LiveEntriesWithinCap). Tombstones and skipped entries past the cap are exempt, since Restore writes neither, so a manifest written before the cap stays readable and a moves recovery that selects it is not stranded. manifestWithinCap's body and checkManifest change; Open, OpenAt and Commit are byte-identical. Envelope-only: no root-read unit changed or was added, nothing runs as root that did not before, and no registry, /data, release or claim read was added, so the legacy seal stands.

+@coverage internal/hostfiles 1b7630f1bd066784e51a5b9cbb4d300a4c590fbaae4b08d43013373e9d50c65e