amadan

carlos / platform Public

hostfiles: Open refuses a manifest inherited with a path past the cap, terminally, and Run stops with one line on a commit no retry can fix.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Paul Campbell pushed by claude 318519f50a10ec3fff2fe561da39737aa5654e47 parent 3662903
3 files changed, +149 −10
  • internal/hostfiles/fix5_linux_test.go +116 −0
  • internal/hostfiles/publisher.go +23 −7
  • internal/hostfiles/run.go +10 −3
diff --git a/internal/hostfiles/fix5_linux_test.go b/internal/hostfiles/fix5_linux_test.go
new file mode 100644
index 000000000..b029c6af0
--- /dev/null
+++ b/internal/hostfiles/fix5_linux_test.go
@@ -0,0 +1,116 @@
+//go:build linux
+
+package hostfiles
+
+import (
+ "bytes"
+ "context"
+ "encoding/json"
+ "errors"
+ "os"
+ "testing"
+ "time"
+
+ "github.com/carlosframework/platform/internal/release"
+)
+
+// 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.
+func inheritOverCap(t *testing.T, s release.Store) []byte {
+ t.Helper()
+ ctx := context.Background()
+ p := open(t, s)
+ stage(t, p, "healthy.txt", "fine")
+ commit(t, p)
+ m, _, err := Load(ctx, s, pfx)
+ if err != nil {
+ t.Fatal(err)
+ }
+ m.Seq++
+ m.Entries[pathOfBytes(MaxPathBytes+1)] = Entry{Seq: m.Seq, Skipped: "too-large"}
+ body, err := json.Marshal(m)
+ if err != nil {
+ t.Fatal(err)
+ }
+ _, etag, err := s.Get(ctx, ManifestKey(pfx))
+ if err != nil {
+ t.Fatal(err)
+ }
+ if _, err := s.Put(ctx, ManifestKey(pfx), body, etag); err != nil {
+ t.Fatal(err)
+ }
+ 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.
+func TestAnInheritedManifestPastTheCapIsRefusedByOpenAndTheReaders(t *testing.T) {
+ ctx := context.Background()
+ s := release.NewDirStore(t.TempDir())
+ before := inheritOverCap(t, s)
+ if p, err := Open(ctx, s, pfx); !errors.Is(err, ErrTooDeep) || p != nil {
+ t.Fatalf("Open of an inherited over-cap manifest: %v", err)
+ }
+ if _, _, err := Restore(ctx, s, pfx, t.TempDir(), nil); !errors.Is(err, ErrTooDeep) {
+ t.Fatalf("Restore: %v", err)
+ }
+ m, _, err := Load(ctx, s, pfx)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if err := ValidateManifest(m, []string{pfx}); !errors.Is(err, ErrTooDeep) {
+ t.Fatalf("ValidateManifest: %v", err)
+ }
+ const to = "instances/shop.test/abc/files/g9/"
+ if _, err := CopyManifest(ctx, s, pfx, to, 0, nil); !errors.Is(err, ErrTooDeep) {
+ t.Fatalf("CopyManifest: %v", err)
+ }
+ if _, _, err := s.Get(ctx, ManifestKey(to)); !errors.Is(err, os.ErrNotExist) {
+ t.Fatalf("CopyManifest wrote its target: %v", err)
+ }
+ after, _, err := s.Get(ctx, ManifestKey(pfx))
+ if err != nil || !bytes.Equal(before, after) {
+ t.Fatalf("the inherited manifest was rewritten: %v", err)
+ }
+}
+
+// Run over a publisher holding such an entry (one continued before Open
+// refused them) stops on its first commit with one line, never retrying
+// at every tick, and commits nothing.
+func TestRunStopsOnAManifestItCannotCommit(t *testing.T) {
+ logs := captureLog(t)
+ s, dir := release.NewDirStore(t.TempDir()), t.TempDir()
+ p := open(t, s)
+ stage(t, p, "healthy.txt", "fine")
+ commit(t, p)
+ before, _, err := s.Get(context.Background(), ManifestKey(pfx))
+ if err != nil {
+ t.Fatal(err)
+ }
+ p.mu.Lock()
+ p.next++
+ p.man.Entries[pathOfBytes(MaxPathBytes+1)] = Entry{Seq: p.next, Skipped: "too-large"}
+ p.mu.Unlock()
+ write(t, dir, "healthy.txt", "changed") // the first scan stages it, and Commit is attempted
+ ctx, cancel := context.WithCancel(context.Background())
+ defer cancel()
+ done := make(chan error, 1)
+ go func() { done <- run(ctx, p, dir, polled, nil) }()
+ select {
+ case err := <-done:
+ if !errors.Is(err, ErrTooDeep) {
+ t.Fatalf("run ended with %v, want ErrTooDeep", err)
+ }
+ case <-time.After(3 * time.Second):
+ t.Fatalf("run kept going on a manifest no commit can write (%d commit lines)", logs.count("commit"))
+ }
+ if got := logs.count("cannot commit"); got != 1 {
+ t.Fatalf("logged %d times, want once", got)
+ }
+ after, _, err := s.Get(context.Background(), ManifestKey(pfx))
+ if err != nil || !bytes.Equal(before, after) {
+ t.Fatalf("something was committed: %v", err)
+ }
+}
diff --git a/internal/hostfiles/publisher.go b/internal/hostfiles/publisher.go
index be3e72c86..2fd6df6e9 100644
--- a/internal/hostfiles/publisher.go
+++ b/internal/hostfiles/publisher.go
@@ -46,9 +46,28 @@ func Open(ctx context.Context, s release.Store, prefix string) (*Publisher, erro
if err != nil {
return nil, err
}
+ // A manifest written before the path cap may hold a path past it. No
+ // reader accepts it (checkManifest), and no Commit could ever write it
+ // again, so it is refused here, terminally, rather than continued into
+ // a publisher whose every commit fails. An operator replaces it.
+ if err := manifestWithinCap(m); err != nil {
+ return nil, fmt.Errorf("hostfiles: open %s: the manifest cannot be continued, an operator must replace it: %w", prefix, err)
+ }
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.
+func manifestWithinCap(m Manifest) error {
+ for path := range m.Entries {
+ if err := withinTreeCap(path); err != nil {
+ return err
+ }
+ }
+ return nil
+}
+
func sum(body []byte) string {
h := sha256.Sum256(body)
return hex.EncodeToString(h[:])
@@ -165,13 +184,10 @@ func (p *Publisher) Commit(ctx context.Context) (int64, error) {
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
- }
+ // (checkManifest), so none is written: Open refuses to continue one and
+ // Stage refuses to add one; this is the last line before the store.
+ if err := manifestWithinCap(p.man); err != nil {
+ return 0, err
}
out := p.man
out.Seq = p.next
diff --git a/internal/hostfiles/run.go b/internal/hostfiles/run.go
index 2d4375d00..066dc4ed9 100644
--- a/internal/hostfiles/run.go
+++ b/internal/hostfiles/run.go
@@ -37,9 +37,10 @@ type watcher interface {
Close() error
}
-// Run publishes dir's changes until ctx ends (it returns ctx.Err()) or
-// another writer supersedes the manifest (ErrSuperseded). Where there is
-// no watcher, or the watcher fails, it polls.
+// Run publishes dir's changes until ctx ends (it returns ctx.Err()),
+// another writer supersedes the manifest (ErrSuperseded), or the manifest
+// holds a path past the cap so no commit can succeed (ErrTooDeep). Where
+// there is no watcher, or the watcher fails, it polls.
func Run(ctx context.Context, p *Publisher, dir string, o Options) error {
w, err := newWatcher(dir, o.Skip)
if err != nil {
@@ -171,6 +172,12 @@ func run(ctx context.Context, p *Publisher, dir string, o Options, w watcher) er
if p.Dirty() {
if _, err := p.Commit(ctx); errors.Is(err, ErrSuperseded) {
return err
+ } else if errors.Is(err, ErrTooDeep) {
+ // The manifest holds a path past the cap: no commit can
+ // ever succeed, so retrying would only spin. One line, and
+ // stop.
+ log.Printf("hostfiles: %s: cannot commit, stopping: %v", dir, err)
+ return err
} else if err != nil {
log.Printf("hostfiles: commit: %v", err) // staged changes stay; the next tick retries
}