amadan

carlos / platform Public

host: the prepared build refuses missing inputs instead of an empty image (Task 6 fix round 1, finding 4)

The build takes the install receipt as evidence (files_placed: the
placed tree's identity and the env file's identity, or none when the
archive carried no env file): local refuses when the files directory or
the env file is not the one it placed (sameIdentity), and the build
refuses a missing tree, a missing expected env file, or an env file
nobody placed, before any write. An archive without an env file builds
a manifest without the item; nothing else does.

Tests: the unit refusals and the archive-without-env case; the tree or
the env file vanishing after placement and before the manifest leaves
the record at local with nothing written and no step journalled.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
claude 7527782498c40de9454205c7a4972b0ca8c13bf2 parent b9ca273
4 files changed, +153 −22
  • internal/host/movesfiles.go +29 −16
  • internal/host/movesfiles_test.go +70 −5
  • internal/host/movesinstall.go +19 −1
  • internal/host/movesland_test.go +35 −0
diff --git a/internal/host/movesfiles.go b/internal/host/movesfiles.go
index 60f4de1a6..2e3fb96f0 100644
--- a/internal/host/movesfiles.go
+++ b/internal/host/movesfiles.go
@@ -178,11 +178,11 @@ func (createOnlyStore) Delete(context.Context, string) error {
// buildPreparedManifest is BuildPrepared over a versioned store: the
// install's own (ir.versioned), which needs no compare-and-swap.
-func buildPreparedManifest(ctx context.Context, data lease.VersionedStore, bucket, location, generation, op string, stamp uint64, dir, envPath string, wake hostfiles.WakeState) (preparedManifest, error) {
+func buildPreparedManifest(ctx context.Context, data lease.VersionedStore, bucket, location, generation, op string, stamp uint64, dir, envPath string, wantEnv bool, wake hostfiles.WakeState) (preparedManifest, error) {
if data == nil || bucket == "" {
return preparedManifest{}, hostwrite.ErrUnavailable
}
- return buildPrepared(ctx, createOnlyStore{data: data, bucket: bucket}, location, generation, op, stamp, dir, envPath, wake)
+ return buildPrepared(ctx, createOnlyStore{data: data, bucket: bucket}, location, generation, op, stamp, dir, envPath, wantEnv, wake)
}
// BuildPrepared writes the prepared manifest of generation (the
@@ -191,25 +191,45 @@ func buildPreparedManifest(ctx context.Context, data lease.VersionedStore, bucke
// regular file under dir as a create-only object at sequence 0 (a file
// over hostfiles.MaxFileBytes is recorded as skipped; anything not a
// regular file is left out, as SyncDir leaves it), the env file at envPath
-// as the EnvItem when it exists, the wake state as the WakeItem, then the
-// one create-only sequence-0 manifest. It returns the binding for the op
-// journal: the key, the manifest's digest, op and stamp. A replay reuses
+// as the EnvItem, the wake state as the WakeItem, then the one create-only
+// sequence-0 manifest. It returns the binding for the op journal: the key,
+// the manifest's digest and exact version, op and stamp. A replay reuses
// every object by digest and reads the manifest back by key; a different
// manifest at the key is hostfiles.ErrManifestExists.
//
+// Missing inputs are never an empty image (fix round 1, finding 4): dir
+// must exist (the install placed it), and the env file must exist exactly
+// when wantEnv says the install placed one (the archive's evidence, the
+// install receipt's Env identity). A vanished tree, a vanished env file,
+// or an env file nobody placed, refuses before any write.
+//
// The caller runs it as op.exec in a phase that permits installation,
// after the install's own claim sync and stamp capture (local_).
-func (f *movesFiles) BuildPrepared(ctx context.Context, location, generation, op string, stamp uint64, dir, envPath string, wake hostfiles.WakeState) (preparedManifest, error) {
+func (f *movesFiles) BuildPrepared(ctx context.Context, location, generation, op string, stamp uint64, dir, envPath string, wantEnv bool, wake hostfiles.WakeState) (preparedManifest, error) {
if f == nil {
return preparedManifest{}, hostwrite.ErrUnavailable
}
- return buildPreparedManifest(ctx, f.Data, f.Bucket, location, generation, op, stamp, dir, envPath, wake)
+ return buildPreparedManifest(ctx, f.Data, f.Bucket, location, generation, op, stamp, dir, envPath, wantEnv, wake)
}
-func buildPrepared(ctx context.Context, s release.Store, location, generation, op string, stamp uint64, dir, envPath string, wake hostfiles.WakeState) (preparedManifest, error) {
+func buildPrepared(ctx context.Context, s release.Store, location, generation, op string, stamp uint64, dir, envPath string, wantEnv bool, wake hostfiles.WakeState) (preparedManifest, error) {
if ctx == nil || location == "" || !movesFilesGeneration(generation) || op == "" || stamp == 0 || dir == "" || envPath == "" {
return preparedManifest{}, fmt.Errorf("%w: a prepared manifest names its location, generation, op, stamp, files directory and env path", hostwrite.ErrUnavailable)
}
+ // The inputs first, before any write: the placed tree and the env file
+ // as the install's evidence says.
+ if st, err := os.Lstat(dir); err != nil || !st.IsDir() {
+ return preparedManifest{}, fmt.Errorf("%w: the files directory %s the install placed is not there (%v)", errLandingChanged, dir, err)
+ }
+ env, err := os.ReadFile(envPath)
+ switch {
+ case err == nil && !wantEnv:
+ return preparedManifest{}, fmt.Errorf("%w: an env file is at %s, which the install did not place", errLandingChanged, envPath)
+ case errors.Is(err, fs.ErrNotExist) && wantEnv:
+ return preparedManifest{}, fmt.Errorf("%w: the env file %s the install placed is not there", errLandingChanged, envPath)
+ case err != nil && !errors.Is(err, fs.ErrNotExist):
+ return preparedManifest{}, err
+ }
prefix := movesFilesPrefix(location, generation)
p, err := hostfiles.NewPrepared(s, prefix)
if err != nil {
@@ -217,9 +237,6 @@ func buildPrepared(ctx context.Context, s release.Store, location, generation, o
}
err = filepath.WalkDir(dir, func(path string, d fs.DirEntry, err error) error {
if err != nil {
- if path == dir && errors.Is(err, fs.ErrNotExist) {
- return fs.SkipAll // no files directory: an empty image
- }
return err
}
if path == dir || d.IsDir() || !d.Type().IsRegular() {
@@ -242,14 +259,10 @@ func buildPrepared(ctx context.Context, s release.Store, location, generation, o
if err != nil {
return preparedManifest{}, fmt.Errorf("prepared manifest of %s: %w", dir, err)
}
- env, err := os.ReadFile(envPath)
- switch {
- case err == nil:
+ if wantEnv {
if err := p.StagePlatform(ctx, hostfiles.EnvItem, env); err != nil {
return preparedManifest{}, err
}
- case !errors.Is(err, fs.ErrNotExist):
- return preparedManifest{}, err
}
body, err := json.Marshal(wake)
if err != nil {
diff --git a/internal/host/movesfiles_test.go b/internal/host/movesfiles_test.go
index 4c445553a..9d77aa547 100644
--- a/internal/host/movesfiles_test.go
+++ b/internal/host/movesfiles_test.go
@@ -210,7 +210,7 @@ func TestPreparedManifestIsCreateOnlySequenceZeroAndBound(t *testing.T) {
}
x.writeEnv("K=1\n")
wake := hostfiles.WakeState{V: 1, Host: filesHost, Timers: []hostfiles.WakeTimer{{Name: "tick", Kind: "cron", Spec: "* * * * *", NextAt: 5}}, NextDue: 5, Replication: "hot"}
- pm, err := x.f.BuildPrepared(ctx, filesLoc, "mv1", "mv1", 7, dir, x.envPath(), wake)
+ pm, err := x.f.BuildPrepared(ctx, filesLoc, "mv1", "mv1", 7, dir, x.envPath(), true, wake)
if err != nil {
t.Fatal(err)
}
@@ -248,7 +248,7 @@ func TestPreparedManifestIsCreateOnlySequenceZeroAndBound(t *testing.T) {
if err := os.WriteFile(filepath.Join(dir, "a.txt"), []byte("changed"), 0o640); err != nil {
t.Fatal(err)
}
- if _, err := x.f.BuildPrepared(ctx, filesLoc, "mv1", "mv1", 7, dir, x.envPath(), wake); !errors.Is(err, hostfiles.ErrManifestExists) {
+ if _, err := x.f.BuildPrepared(ctx, filesLoc, "mv1", "mv1", 7, dir, x.envPath(), true, wake); !errors.Is(err, hostfiles.ErrManifestExists) {
t.Fatalf("second build: %v", err)
}
if again, _ := x.bucket.Current("data", pm.Key); sha(again) != pm.SHA256 {
@@ -256,6 +256,71 @@ func TestPreparedManifestIsCreateOnlySequenceZeroAndBound(t *testing.T) {
}
}
+// The build never turns missing inputs into an empty image (fix round 1,
+// finding 4): the files directory the install placed must be there, and
+// the env file must be there exactly when the install placed one (the
+// archive's evidence, wantEnv). A tree that vanished, an expected env file
+// that is gone, or an env file nobody placed, refuses before any write;
+// an install whose archive carried no env file builds a manifest without
+// the item.
+func TestPreparedManifestRefusesMissingInputs(t *testing.T) {
+ x := newFilesFixture(t)
+ ctx := context.Background()
+ dir := filepath.Join(x.dir, filesHost)
+ wake := hostfiles.WakeState{V: 1, Host: filesHost, Timers: []hostfiles.WakeTimer{}}
+ build := func(gen string, wantEnv bool) (preparedManifest, error) {
+ return buildPreparedManifest(ctx, x.bucket, "data", filesLoc, gen, gen, 7, dir, x.envPath(), wantEnv, wake)
+ }
+ written := func() int {
+ vs, err := x.bucket.ListVersions(ctx, "data", strings.TrimSuffix(filesLoc, "/")+"/files/")
+ if err != nil {
+ t.Fatal(err)
+ }
+ return len(vs)
+ }
+ // No tree at all.
+ x.writeEnv("K=1\n")
+ if _, err := build("g1", true); err == nil {
+ t.Fatal("built over a missing files directory")
+ }
+ if err := os.MkdirAll(dir, 0o755); err != nil {
+ t.Fatal(err)
+ }
+ if err := os.WriteFile(filepath.Join(dir, "a.txt"), []byte("A"), 0o640); err != nil {
+ t.Fatal(err)
+ }
+ // The env file the install placed is gone.
+ if err := os.Remove(x.envPath()); err != nil {
+ t.Fatal(err)
+ }
+ if _, err := build("g1", true); err == nil {
+ t.Fatal("built without the env file the install placed")
+ }
+ // An env file nobody placed.
+ x.writeEnv("K=1\n")
+ if _, err := build("g1", false); err == nil {
+ t.Fatal("built over an env file the install did not place")
+ }
+ if n := written(); n != 0 {
+ t.Fatalf("%d objects written by refused builds", n)
+ }
+ // An archive without an env file: a manifest without the item.
+ if err := os.Remove(x.envPath()); err != nil {
+ t.Fatal(err)
+ }
+ pm, err := build("g1", false)
+ if err != nil {
+ t.Fatal(err)
+ }
+ m, found := x.manifest("g1")
+ if !found || len(m.Entries) != 1 {
+ t.Fatalf("manifest: %+v %v", m, found)
+ }
+ if _, has := m.Platform[hostfiles.EnvItem]; has || pm.Key == "" {
+ t.Fatalf("an env item for an archive without one: %+v", m.Platform)
+ }
+}
+
// A replay of the build (a crash after the objects, before or after the
// manifest) reuses every object it finds by digest and the manifest by
// key: the same binding, one version of each object, no new object.
@@ -271,11 +336,11 @@ func TestPreparedManifestReplayReusesObjects(t *testing.T) {
}
x.writeEnv("K=1\n")
wake := hostfiles.WakeState{V: 1, Host: filesHost, Timers: []hostfiles.WakeTimer{}}
- first, err := x.f.BuildPrepared(ctx, filesLoc, "mv1", "mv1", 7, dir, x.envPath(), wake)
+ first, err := x.f.BuildPrepared(ctx, filesLoc, "mv1", "mv1", 7, dir, x.envPath(), true, wake)
if err != nil {
t.Fatal(err)
}
- second, err := x.f.BuildPrepared(ctx, filesLoc, "mv1", "mv1", 7, dir, x.envPath(), wake)
+ second, err := x.f.BuildPrepared(ctx, filesLoc, "mv1", "mv1", 7, dir, x.envPath(), true, wake)
if err != nil {
t.Fatalf("replay: %v", err)
}
@@ -561,7 +626,7 @@ func TestEstablishOnThePreparedGenerationContinuesFromSequenceZero(t *testing.T)
t.Fatal(err)
}
x.writeEnv("K=1\n")
- pm, err := x.f.BuildPrepared(ctx, filesLoc, "mv1", "mv1", 7, dir, x.envPath(), hostfiles.WakeState{V: 1, Host: filesHost, Timers: []hostfiles.WakeTimer{}})
+ pm, err := x.f.BuildPrepared(ctx, filesLoc, "mv1", "mv1", 7, dir, x.envPath(), true, hostfiles.WakeState{V: 1, Host: filesHost, Timers: []hostfiles.WakeTimer{}})
if err != nil {
t.Fatal(err)
}
diff --git a/internal/host/movesinstall.go b/internal/host/movesinstall.go
index 72c5d5293..949958fb3 100644
--- a/internal/host/movesinstall.go
+++ b/internal/host/movesinstall.go
@@ -877,6 +877,24 @@ func (ir *installRun) local_(ctx context.Context) (bool, error) {
// skipped (written before C8 existed) counts as not done (ruling Q3
// item 2). Another manifest at the key is not this install's to
// replace: the refusal (RollbackManifestOccupied).
+ // The image is the install's own placement, by the install receipt
+ // (files_placed: the tree's identity, the env file's identity or
+ // none): a vanished tree or env file refuses, and the manifest
+ // carries an env item exactly when the archive carried one (fix
+ // round 1, finding 4).
+ placed, err := journalledStaged(j, landStepFilesPlaced)
+ if err != nil {
+ return false, err
+ }
+ if ok, err := sameIdentity(dir, placed.Files); err != nil || !ok {
+ return false, fmt.Errorf("%w: %s is not the files directory this %s placed (%v)", errLandingChanged, dir, ir.src.Name, err)
+ }
+ wantEnv := placed.Env != ""
+ if wantEnv {
+ if ok, err := sameIdentity(envPath, placed.Env); err != nil || !ok {
+ return false, fmt.Errorf("%w: %s is not the env file this %s placed (%v)", errLandingChanged, envPath, ir.src.Name, err)
+ }
+ }
rows, err := ir.sealedTimers(ctx, seal, src.TimersSHA256)
if err != nil {
return false, err
@@ -888,7 +906,7 @@ func (ir *installRun) local_(ctx context.Context) (bool, error) {
// Every write re-checks the authority first (installAuthority): a
// stale build never wins the create-only manifest.
authority := installAuthorityStore{VersionedStore: vs, check: ir.installAuthority(ctx, captured, phase)}
- pm, err := buildPreparedManifest(ctx, authority, cfg.OwnBucket, holder.Location, gen, id, captured.Stamp, dir, envPath, hostfiles.WakeStateOf(host, rows, src.NextDue, src.Replication))
+ pm, err := buildPreparedManifest(ctx, authority, cfg.OwnBucket, holder.Location, gen, id, captured.Stamp, dir, envPath, wantEnv, hostfiles.WakeStateOf(host, rows, src.NextDue, src.Replication))
if errors.Is(err, hostfiles.ErrManifestExists) {
log.Printf("moves: %s: the prepared files manifest of %s is not this install's: %v", host, id, err)
return ir.refuse(ctx, refuseManifestOccupied)
diff --git a/internal/host/movesland_test.go b/internal/host/movesland_test.go
index a93862691..d78551d76 100644
--- a/internal/host/movesland_test.go
+++ b/internal/host/movesland_test.go
@@ -1461,6 +1461,41 @@ func TestLocalRefusesADamagedFilesManifestJournal(t *testing.T) {
}
}
+// The placed tree or the placed env file vanishes after placement and
+// before the manifest is built (fix round 1, finding 4): the install
+// refuses rather than publishing an empty or partial image. The record
+// stays at local, nothing is written under the files prefix and the step
+// is not journalled.
+func TestLocalRefusesAVanishedTreeBeforeTheManifest(t *testing.T) {
+ for name, vanish := range map[string]func(h *landingHarness) error{
+ "the files directory": func(h *landingHarness) error { return os.RemoveAll(filepath.Join(h.tdir, p1Host)) },
+ "the env file": func(h *landingHarness) error { return os.Remove(filepath.Join(h.tdir, p1Host+".env")) },
+ } {
+ t.Run(name, func(t *testing.T) {
+ h := newLandingHarness(t)
+ h.flip(t)
+ h.landAt = "after-verify"
+ h.crashLand(t)
+ if err := vanish(h); err != nil {
+ t.Fatal(err)
+ }
+ h.freshTarget()
+ h.land(t)
+ if m := h.read(t, p1Host).Move; m.LandingPhase != provision.LandLocal || m.Reason != "" {
+ t.Fatalf("the record moved: %+v", m)
+ }
+ if _, ok := h.landJournal(t).Steps[landStepFilesManifest]; ok {
+ t.Fatal("the step was journalled over a vanished input")
+ }
+ for k := range h.f.buckets["data-uk"] {
+ if strings.HasPrefix(k, cutRoot+"/files/") {
+ t.Fatalf("written over a vanished input: %s", k)
+ }
+ }
+ })
+ }
+}
+
// A cross-region landing's prepared manifest names destination-local
// objects it materialized from the sealed archive (claim P6-D3): every
// object lies under the manifest's own prefix in the target's bucket, and