amadan
host: a prepared replay is accepted only as this box's establishment in full (Task 6 fix round 2, finding 6, platform-1e P2)
establishedOver accepted the current env entry on its digest and Deleted flag alone, so a bumped manifest whose env entry carried the expected digest but an object under another location was accepted, StagePlatform skipped the write on the digest match, and the foreign reference stood. It now validates the current manifest's origins by the location rule and every entry against them, and requires the env entry's complete shape: the object key under P at the entry's own sequence and digest (what Publisher.put writes), a sequence after the prepared one and within the manifest's, env's size and mode, not deleted, torn or skipped. The refusal names what differed. Regression: on a prepared replay, a bumped sequence with an env entry of the expected digest under another location, under another generation, of the wrong size, or torn, is refused and left as it was; this box's own establishment is accepted and writes nothing.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
2 files changed,
+89
−14
internal/host/movesfiles.go+39 −14internal/host/movesfiles_test.go+50 −0
diff --git a/internal/host/movesfiles.go b/internal/host/movesfiles.go| index 827a11924..b51137324 100644 |
| --- a/internal/host/movesfiles.go |
| +++ b/internal/host/movesfiles.go |
| @@ -508,27 +508,34 @@ func (f *movesFiles) openPrepared(ctx context.Context, files release.Store, o ho |
| if err != nil || !found { |
| return nil, fmt.Errorf("host: the files manifest %s: %v", key, err) |
| } |
| - if !establishedOver(current, prepared, env) { |
| - return nil, fmt.Errorf("%w: the files manifest %s is neither the prepared manifest %s nor this box's establishment of it", hostbox.ErrChanged, key, pm.Version) |
| + if err := establishedOver(h.Location, prefix, current, prepared, env); err != nil { |
| + return nil, fmt.Errorf("%w: the files manifest %s is neither the prepared manifest %s nor this box's establishment of it: %v", hostbox.ErrChanged, key, pm.Version, err) |
| } |
| } |
| return hostfiles.OpenAt(files, prefix, cur, token) |
| } |
| // establishedOver reports whether cur is an establishment of prepared by |
| -// this box: the same entries and the same platform items apart from the |
| -// env file, which holds exactly env's bytes, and the adoption item, which |
| -// is gone. |
| -func establishedOver(cur, prepared hostfiles.Manifest, env []byte) bool { |
| - if cur.Seq <= prepared.Seq || !reflect.DeepEqual(cur.Entries, prepared.Entries) { |
| - return false |
| - } |
| - e, ok := cur.Platform[hostfiles.EnvItem] |
| - if !ok || e.Deleted || e.SHA256 != hostfilesSum(env) { |
| - return false |
| +// this box, in full (fix round 2, finding 6): a later sequence; the same |
| +// entries and the same platform items apart from the env file and the |
| +// adoption item; the adoption item gone; cur's origins holding the |
| +// location rule and every entry validating against them; and the env |
| +// entry with the complete shape of the object this box's establishment |
| +// wrote, not merely its digest: the object key under P at the entry's own |
| +// sequence and digest (what Publisher.put writes), a sequence after the |
| +// prepared one, env's size and mode, not deleted, torn or skipped. A |
| +// manifest whose env entry names an object elsewhere, or a different |
| +// object under P, is not this box's establishment, however its digest |
| +// reads. |
| +func establishedOver(location, prefix string, cur, prepared hostfiles.Manifest, env []byte) error { |
| + if cur.Seq <= prepared.Seq { |
| + return fmt.Errorf("sequence %d is not after the prepared %d", cur.Seq, prepared.Seq) |
| + } |
| + if !reflect.DeepEqual(cur.Entries, prepared.Entries) { |
| + return errors.New("the entries are not the prepared manifest's") |
| } |
| if _, has := cur.Platform[instancekey.AdoptionItem]; has { |
| - return false |
| + return errors.New("the adoption item is still carried") |
| } |
| rest := func(m hostfiles.Manifest) map[string]hostfiles.Entry { |
| out := map[string]hostfiles.Entry{} |
| @@ -539,7 +546,25 @@ func establishedOver(cur, prepared hostfiles.Manifest, env []byte) bool { |
| } |
| return out |
| } |
| - return reflect.DeepEqual(rest(cur), rest(prepared)) |
| + if !reflect.DeepEqual(rest(cur), rest(prepared)) { |
| + return errors.New("the platform items are not the prepared manifest's") |
| + } |
| + origins, err := movesFilesOrigins(location, prefix, cur) |
| + if err != nil { |
| + return err |
| + } |
| + if err := hostfiles.ValidateManifest(cur, origins); err != nil { |
| + return err |
| + } |
| + e, ok := cur.Platform[hostfiles.EnvItem] |
| + if !ok { |
| + return errors.New("no env item") |
| + } |
| + want := hostfiles.Entry{Object: fmt.Sprintf("%sobjects/%016d-%s", prefix, e.Seq, hostfilesSum(env)), SHA256: hostfilesSum(env), Size: int64(len(env)), Mode: 0o600, Seq: e.Seq} |
| + if e != want || e.Seq <= prepared.Seq || e.Seq > cur.Seq { |
| + return fmt.Errorf("the env item %+v is not the object this box's establishment writes (%+v)", e, want) |
| + } |
| + return nil |
| } |
| func hostfilesSum(body []byte) string { |
diff --git a/internal/host/movesfiles_test.go b/internal/host/movesfiles_test.go| index 187eb44ce..1877753fd 100644 |
| --- a/internal/host/movesfiles_test.go |
| +++ b/internal/host/movesfiles_test.go |
| @@ -627,6 +627,56 @@ func TestEstablishPublishesOnlyOverTheBytesItChecked(t *testing.T) { |
| } |
| } |
| +// A prepared replay is accepted only as this box's own establishment, in |
| +// full (fix round 2, finding 6): the current manifest's origins hold the |
| +// location rule and the env entry has the complete shape of the object |
| +// the establishment wrote (under P, at the expected sequence and size, |
| +// not torn, not skipped), not merely the digest. A bumped sequence whose |
| +// env entry carries the expected digest but an object under another |
| +// location, or the wrong size, is refused and left as it is. |
| +func TestEstablishRefusesAForeignEnvEntryOnPreparedReplay(t *testing.T) { |
| + x, pm, o, in := preparedFixture(t) |
| + ctx := context.Background() |
| + envSHA := sha([]byte("K=1\nCARLOS_INSTANCE_KEY=abc\n")) |
| + raw, err := x.bucket.GetVersion(ctx, "data", pm.Key, pm.Version) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + var prepared hostfiles.Manifest |
| + if err := json.Unmarshal(raw, &prepared); err != nil { |
| + t.Fatal(err) |
| + } |
| + for name, entry := range map[string]hostfiles.Entry{ |
| + "another location": {Object: "instances/other/files/g-mv1/objects/0000000000000001-" + envSHA, SHA256: envSHA, Size: 30, Mode: 0o600, Seq: 1}, |
| + "another generation": {Object: movesFilesPrefix(filesLoc, "cur") + "objects/0000000000000001-" + envSHA, SHA256: envSHA, Size: 30, Mode: 0o600, Seq: 1}, |
| + "the wrong size": {Object: movesFilesPrefix(filesLoc, "mv1") + "objects/0000000000000001-" + envSHA, SHA256: envSHA, Size: 3, Mode: 0o600, Seq: 1}, |
| + "torn": {Object: movesFilesPrefix(filesLoc, "mv1") + "objects/0000000000000001-" + envSHA, SHA256: envSHA, Size: 30, Mode: 0o600, Seq: 1, Torn: true}, |
| + } { |
| + bumped := hostfiles.Manifest{V: hostfiles.ManifestVersion, Seq: 1, Entries: prepared.Entries, Platform: map[string]hostfiles.Entry{}} |
| + for k, v := range prepared.Platform { |
| + bumped.Platform[k] = v |
| + } |
| + bumped.Platform[hostfiles.EnvItem] = entry |
| + body := mustJSON(t, bumped) |
| + x.bucket.Put("data", pm.Key, body) |
| + if err := x.f.Establish(ctx, o, in, x); err == nil { |
| + t.Fatalf("%s: a foreign env entry was accepted as this box's establishment", name) |
| + } |
| + if cur, _ := x.bucket.Current("data", pm.Key); string(cur) != string(body) { |
| + t.Fatalf("%s: the manifest was replaced: %s", name, cur) |
| + } |
| + } |
| + // This box's own establishment, in full, is accepted and left as it is. |
| + x.bucket.Put("data", pm.Key, raw) |
| + if err := x.f.Establish(ctx, o, in, x); err != nil { |
| + t.Fatal(err) |
| + } |
| + seq1 := x.bucket.LatestID("data", pm.Key) |
| + if err := x.f.Establish(ctx, o, in, x); err != nil || x.bucket.LatestID("data", pm.Key) != seq1 { |
| + t.Fatalf("the genuine replay: %v, revision moved %v", err, x.bucket.LatestID("data", pm.Key) != seq1) |
| + } |
| +} |
| + |
| // stallingStore holds one key's write until released, signalling that it |
| // is stalled: a slow S3 request. |
| type stallingStore struct { |