amadan

carlos / platform Public

Final fix 4: refuse S3's mutable "null" version id in staging

A versioning-suspended bucket answers every write with version id "null",
and the next write to the key overwrites that version. lease.UsableVersionID
refuses it with "": the staging mirror's copy, put, streamed put and
put-if-absent fail closed on it, and a mirror receipt naming it no longer
vouches for a skip (the object is recopied). moves.ArchiveRef.Pinned needs
usable versions for both objects; T5 refuses an unpinned reference before
its verdict cache (MV-N154, pending), and the source's ledger drops a null
archive or move.json version so the archive is restaged.

The copy gate's file inventory gains proofgate.go, refusal.go and
proofroot.go, which the D3-held fix round touched.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
claude 13a42e9f0bbaf9fa1478ecdf83a3ef53929e23fb parent b65dced
11 files changed, +247 −5
  • approved_moves_copy_test.go +4 −1
  • docs/superpowers/plans/2026-10-06-moves-phase1-copy.md +1 −0
  • internal/host/movesnullversion_test.go +75 −0
  • internal/host/movesphase1.go +9 −0
  • internal/host/movesreport.go +7 −0
  • internal/lease/mirror.go +6 −1
  • internal/lease/mirror_test.go +56 −0
  • internal/lease/staging.go +11 −3
  • internal/lease/staging_test.go +51 −0
  • internal/moves/readiness.go +10 −0
  • internal/moves/readiness_test.go +17 −0
diff --git a/approved_moves_copy_test.go b/approved_moves_copy_test.go
index 65002c84b..b6eecce4d 100644
--- a/approved_moves_copy_test.go
+++ b/approved_moves_copy_test.go
@@ -64,7 +64,7 @@ var movesCopyIDs = []string{
"MV-N133", "MV-N134", "MV-N135", "MV-N136", "MV-N137", "MV-N138",
"MV-N139", "MV-N140", "MV-N141", "MV-N142", "MV-N143", "MV-N144",
"MV-N145", "MV-N146", "MV-N147", "MV-N148", "MV-N149", "MV-N150",
- "MV-N151", "MV-N152", "MV-N153",
+ "MV-N151", "MV-N152", "MV-N153", "MV-N154",
}
// copyRow is one row of the record. The File cell is the file the sentence is
@@ -558,6 +558,7 @@ var movesScanFiles = []string{
"internal/host/movesreport.go",
"internal/host/movesstage.go",
"internal/host/movesstore.go",
+ "internal/host/proofroot.go",
"internal/host/systemapply.go",
"internal/hostbox/writer.go",
"internal/hostclaim/move.go",
@@ -592,6 +593,8 @@ var movesScanFiles = []string{
"internal/registry/registry.go",
"internal/release/adopt.go",
"internal/release/materializeheld.go",
+ "internal/release/proofgate.go",
+ "internal/release/refusal.go",
}
// The scan's base is the main commit moves-phase1 last merged: the diff from
diff --git a/docs/superpowers/plans/2026-10-06-moves-phase1-copy.md b/docs/superpowers/plans/2026-10-06-moves-phase1-copy.md
index af56be0d2..57468447e 100644
--- a/docs/superpowers/plans/2026-10-06-moves-phase1-copy.md
+++ b/docs/superpowers/plans/2026-10-06-moves-phase1-copy.md
@@ -193,6 +193,7 @@ Columns: id, file, text, status.
| MV-N151 | internal/host/movesphase1.go | the files beside the database could not be archived | pending |
| MV-N152 | internal/moves/readiness.go | the cut budget could not be priced | pending |
| MV-N153 | internal/host/movesreport.go | this box requires a proof for the source's release and has not accepted one | pending |
+| MV-N154 | internal/host/movesreport.go | the source staged its archive without a fixed version id | pending |
## Plan 2
diff --git a/internal/host/movesnullversion_test.go b/internal/host/movesnullversion_test.go
new file mode 100644
index 000000000..0c1eb17dc
--- /dev/null
+++ b/internal/host/movesnullversion_test.go
@@ -0,0 +1,75 @@
+package host
+
+import (
+ "context"
+ "encoding/json"
+ "testing"
+
+ "github.com/carlosframework/platform/internal/lease"
+ "github.com/carlosframework/platform/internal/moves"
+ "github.com/carlosframework/platform/internal/provision"
+)
+
+// Final review finding 4: a version id of "null" (a versioning-suspended
+// bucket) pins nothing, so a ledger that recorded one is no evidence. The
+// next round restages the archive under real version ids and asks for
+// move.json again, and the source report never carries "null".
+func TestNullVersionsInTheLedgerAreRestaged(t *testing.T) {
+ ctx := context.Background()
+ h := newPhase1Harness(t)
+ h.start(t, "manual")
+ h.pass(t)
+ l := h.ledger(t)
+ if l.ArchiveVersions == nil || !l.ArchiveVersions.Pinned() {
+ t.Fatalf("the first round staged no pinned archive: %+v", l.ArchiveVersions)
+ }
+ ref := *l.ArchiveVersions
+ ref.ManifestVersion, ref.SealedVersion = "null", "null"
+ l.ArchiveVersions, l.MoveJSONVersion = &ref, "null"
+ if err := writeMoveLedger(ctx, h.f, "data-ie", moves.LedgerKey(p1Host, p1ID), l); err != nil {
+ t.Fatal(err)
+ }
+
+ h.pass(t)
+ body, err := h.f.Get(ctx, "data-ie", moves.LedgerKey(p1Host, p1ID))
+ if err != nil {
+ t.Fatal(err)
+ }
+ var raw moveLedger
+ if err := json.Unmarshal(body, &raw); err != nil {
+ t.Fatal(err)
+ }
+ if raw.ArchiveVersions == nil || !raw.ArchiveVersions.Pinned() {
+ t.Fatalf("the null-version archive was kept: %+v", raw.ArchiveVersions)
+ }
+ if raw.MoveJSONVersion != moveJSONPresent {
+ t.Fatalf("move_json_version = %q, want %q (asked again, found present)", raw.MoveJSONVersion, moveJSONPresent)
+ }
+ if sr := h.sourceReport(t); sr.Archive == nil || !sr.Archive.Pinned() {
+ t.Fatalf("the source report carries %+v", sr.Archive)
+ }
+}
+
+// T5 refuses an archive reference that names "null" before it looks in its
+// cache of verdicts: a passing verdict cached for that reference would
+// otherwise vouch for whatever the key holds now.
+func TestT5RefusesANullArchiveVersionBeforeItsCache(t *testing.T) {
+ ctx := context.Background()
+ rec := provision.Record{Host: p1Host, Move: &provision.MoveState{ID: p1ID}}
+ good := moves.ArchiveRef{ManifestKey: "m.json", ManifestVersion: "mv1", SealedObjectKey: "s.sealed", SealedVersion: "sv1", Plain: "sha256:p"}
+ for name, mut := range map[string]func(*moves.ArchiveRef){
+ "manifest": func(r *moves.ArchiveRef) { r.ManifestVersion = "null" },
+ "sealed": func(r *moves.ArchiveRef) { r.SealedVersion = "null" },
+ "both": func(r *moves.ArchiveRef) { r.ManifestVersion, r.SealedVersion = "null", "null" },
+ "empty": func(r *moves.ArchiveRef) { r.SealedVersion = "" },
+ } {
+ ref := good
+ mut(&ref)
+ w := &movesWorker{mirrors: map[string]lease.MirrorStore{}}
+ w.archives = map[string]verifiedArchive{p1ID: {ref: ref, check: moves.Check{OK: true}, plain: ref.Plain}}
+ got := w.checkArchive(ctx, rec, &moves.SourceReport{Archive: &ref})
+ if got.check.OK || got.check.Reason != t5NotPinned {
+ t.Errorf("%s: T5 = %+v, want %q", name, got.check, t5NotPinned)
+ }
+ }
+}
diff --git a/internal/host/movesphase1.go b/internal/host/movesphase1.go
index 1871b0bdd..a68c73296 100644
--- a/internal/host/movesphase1.go
+++ b/internal/host/movesphase1.go
@@ -150,6 +150,15 @@ func readMoveLedger(ctx context.Context, m lease.MirrorStore, bucket, key string
// is recomputed (move.json answers present, the archive is rebuilt).
return moveLedger{V: 1}, nil
}
+ // Nor is a version id that pins nothing (final review finding 4): a
+ // "null" recorded before the staging mirror refused it. The archive is
+ // restaged and move.json asked for again (it answers present).
+ if l.ArchiveVersions != nil && !l.ArchiveVersions.Pinned() {
+ l.ArchivePlain, l.ArchiveSealed, l.ArchiveVersions = "", "", nil
+ }
+ if l.MoveJSONVersion != moveJSONPresent && !lease.UsableVersionID(l.MoveJSONVersion) {
+ l.MoveJSONVersion = ""
+ }
return l, nil
}
diff --git a/internal/host/movesreport.go b/internal/host/movesreport.go
index 21c763f5c..fecff8f80 100644
--- a/internal/host/movesreport.go
+++ b/internal/host/movesreport.go
@@ -42,6 +42,7 @@ const (
t5NoArchive = "the source has published no archive yet" // MV-N73
t5Unreadable = "this box could not read the archive the source published"
t5Refused = "the archive does not open with this box's key or does not match its manifest"
+ t5NotPinned = "the source staged its archive without a fixed version id"
t6Unmeasured = "this box could not measure its free disk space"
t7NoRelease = "the source has not reported its release yet" // MV-N76
t7Waiting = "waiting for the root stage pass" // MV-N75
@@ -316,6 +317,12 @@ func (w *movesWorker) checkArchive(ctx context.Context, rec provision.Record, so
return verifiedArchive{check: moves.Check{Reason: t5NoArchive}}
}
ref := *source.Archive
+ // A reference by "null" (or no) version pins nothing, so it is refused
+ // before the cache: a verdict kept for it would vouch for whatever the
+ // key holds now (final review finding 4).
+ if !ref.Pinned() {
+ return verifiedArchive{ref: ref, check: moves.Check{Reason: t5NotPinned}}
+ }
w.mu.Lock()
cached, ok := w.archives[rec.Move.ID]
w.mu.Unlock()
diff --git a/internal/lease/mirror.go b/internal/lease/mirror.go
index ffc11d3d7..2aeb7bcb6 100644
--- a/internal/lease/mirror.go
+++ b/internal/lease/mirror.go
@@ -248,7 +248,9 @@ func MirrorPrefixKeep(ctx context.Context, m MirrorStore, srcBucket, srcPrefix,
// seal, so a matching receipt has to carry one; without it the
// object is recopied (a fresh version id; the stale version
// stays unreferenced).
- if ok && rcpt.SrcETag == s.ETag && rcpt.SrcSize == s.Size && rcpt.DstETag == d.ETag && d.Size == s.Size && (!versioned || rcpt.DstVersionID != "") {
+ // A receipt naming "null" (UsableVersionID) is no better than
+ // none: a later write overwrote that version.
+ if ok && rcpt.SrcETag == s.ETag && rcpt.SrcSize == s.Size && rcpt.DstETag == d.ETag && d.Size == s.Size && (!versioned || UsableVersionID(rcpt.DstVersionID)) {
if versioned {
round.Versions[r] = rcpt.DstVersionID
}
@@ -273,6 +275,9 @@ func MirrorPrefixKeep(ctx context.Context, m MirrorStore, srcBucket, srcPrefix,
book.save(ctx) // best effort: what already landed keeps its receipts
return round, fmt.Errorf("mirror: copy %s: %w", sk, err)
}
+ if !UsableVersionID(ver) {
+ ver = "" // never recorded or sealed by
+ }
round.Changed++
round.BytesCopied += s.Size
if ver != "" {
diff --git a/internal/lease/mirror_test.go b/internal/lease/mirror_test.go
index 9b25912bc..2e9bae104 100644
--- a/internal/lease/mirror_test.go
+++ b/internal/lease/mirror_test.go
@@ -609,6 +609,62 @@ func TestVersionedMirrorRecordsMultipartVersions(t *testing.T) {
}
}
+// Final review finding 4: a receipt that names the version "null" (a copy
+// into a versioning-suspended bucket, recorded before the staging mirror
+// refused it) vouches for nothing, since a later write overwrites that
+// version. The round recopies the object rather than skip it or report the
+// null version, and a copier that answers "null" has no version recorded.
+func TestVersionedMirrorRecopiesANullVersionReceipt(t *testing.T) {
+ ctx := context.Background()
+ f := &fakeVersionedMirror{fakeMirror: newFakeMirror()}
+ f.putMultipart("src", "instances/h/ltx/1/a.ltx", "v1", 100<<20)
+ rc := testReceipts
+ round := func() MirrorRound {
+ t.Helper()
+ r, err := MirrorPrefix(ctx, f, "src", "instances/h/", "dst", "moves/h/", rc)
+ if err != nil {
+ t.Fatal(err)
+ }
+ return r
+ }
+ r1 := round()
+ book := readReceipts(t, f.fakeMirror)
+ rcpt := book["ltx/1/a.ltx"]
+ rcpt.DstVersionID = "null"
+ book["ltx/1/a.ltx"] = rcpt
+ body, err := json.Marshal(receiptBook{V: 1, SrcBucket: "src", SrcPrefix: "instances/h/", DstBucket: "dst", DstPrefix: "moves/h/", Objects: book})
+ if err != nil {
+ t.Fatal(err)
+ }
+ if err := f.fakeMirror.Put(ctx, rc.Bucket, rc.Key, body); err != nil {
+ t.Fatal(err)
+ }
+ r2 := round()
+ if r2.Changed != 1 || r2.BytesCopied != 100<<20 || r2.Versions["ltx/1/a.ltx"] == "null" || r2.Versions["ltx/1/a.ltx"] == r1.Versions["ltx/1/a.ltx"] {
+ t.Fatalf("round after a null-version receipt: %+v (round 1 %v)", r2, r1.Versions)
+ }
+ n := &nullVersionMirror{fakeMirror: newFakeMirror()}
+ n.putMultipart("src", "instances/h/ltx/1/a.ltx", "v1", 100<<20)
+ r, err := MirrorPrefix(ctx, n, "src", "instances/h/", "dst", "moves/h/", rc)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if v, ok := r.Versions["ltx/1/a.ltx"]; ok {
+ t.Fatalf("a null version was recorded: %q", v)
+ }
+ if got := readReceipts(t, n.fakeMirror)["ltx/1/a.ltx"]; got.DstVersionID != "" {
+ t.Fatalf("a receipt carries the null version: %+v", got)
+ }
+}
+
+// nullVersionMirror is a copier into a versioning-suspended bucket.
+type nullVersionMirror struct{ *fakeMirror }
+
+func (f *nullVersionMirror) CopyVersioned(ctx context.Context, sb, sk, se, db, dk string) (string, string, error) {
+ etag, err := f.fakeMirror.Copy(ctx, sb, sk, se, db, dk)
+ return etag, "null", err
+}
+
// Version collection is opt-in: the production s3Mirror must never enter
// the receipt-required mode, since an unversioned destination can never
// supply a version receipt and every unchanged object would recopy forever
diff --git a/internal/lease/staging.go b/internal/lease/staging.go
index 4152afbab..767eef1e5 100644
--- a/internal/lease/staging.go
+++ b/internal/lease/staging.go
@@ -100,12 +100,13 @@ func newStagingClient() *http.Client {
// versionIDOf is the version id a 2xx answer carries. A staging bucket
// must be versioned, since every artifact is sealed by version (§7.3), so
-// an answer without one fails closed: the write may have landed, and the
+// an answer without one, or with the suspended bucket's "null"
+// (UsableVersionID), fails closed: the write may have landed, and the
// caller learns it cannot be sealed.
func versionIDOf(op string, h http.Header) (string, error) {
v := h.Get("X-Amz-Version-Id")
- if v == "" {
- return "", fmt.Errorf("staging mirror: %s: no version id in the response (the staging bucket must be versioned)", op)
+ if !UsableVersionID(v) {
+ return "", fmt.Errorf("staging mirror: %s: no usable version id in the response (%q; the staging bucket must be versioned)", op, v)
}
return v, nil
}
@@ -403,3 +404,10 @@ func (m *StagingMirror) DeleteAt(ctx context.Context, signAt time.Time, bucket,
}
return nil
}
+
+// UsableVersionID reports whether v names one immutable object version. ""
+// is no version at all. "null" is what a versioning-suspended (or never
+// versioned) bucket answers: the next write to the key overwrites that very
+// version, so it pins nothing and a seal by it is no seal (final review
+// finding 4; AWS, "Adding objects to versioning-suspended buckets").
+func UsableVersionID(v string) bool { return v != "" && v != "null" }
diff --git a/internal/lease/staging_test.go b/internal/lease/staging_test.go
index e67800778..6e808ae3a 100644
--- a/internal/lease/staging_test.go
+++ b/internal/lease/staging_test.go
@@ -345,6 +345,57 @@ func TestStagingMirrorRefusesAMissingVersionID(t *testing.T) {
}
}
+// Final review finding 4: a versioning-suspended bucket answers every write
+// with the version id "null", and the next write to the key overwrites that
+// same version. It names no immutable object, so it is refused like a
+// missing id, for every mutation that returns one.
+func TestStagingMirrorRefusesTheNullVersionID(t *testing.T) {
+ m, bucket, _ := stagingFixture(t, func(w http.ResponseWriter, r *http.Request) bool {
+ if !strings.Contains(r.URL.Path, "suspended") {
+ return false
+ }
+ // Headers first: a body written before them sends them.
+ w.Header().Set("ETag", `"abc"`)
+ w.Header().Set("X-Amz-Version-Id", "null")
+ if r.Header.Get("X-Amz-Copy-Source") != "" {
+ io.WriteString(w, `<CopyObjectResult><ETag>"abc"</ETag></CopyObjectResult>`)
+ }
+ return true
+ })
+ bucket.Put("data-ie", "k", []byte("body"))
+ ctx := context.Background()
+ body := "sealed"
+ sum := sha256.Sum256([]byte(body))
+ for name, call := range map[string]func() (string, error){
+ "copy": func() (string, error) {
+ _, v, err := m.CopyVersionedAt(ctx, signAt, "data-ie", "k", "", "data-uk", "suspended/c")
+ return v, err
+ },
+ "put": func() (string, error) {
+ return m.PutVersionedAt(ctx, signAt, "data-uk", "suspended/p", []byte("x"))
+ },
+ "put-reader": func() (string, error) {
+ return m.PutReaderVersionedAt(ctx, signAt, "data-uk", "suspended/r", strings.NewReader(body), int64(len(body)), hex.EncodeToString(sum[:]))
+ },
+ "put-if-absent": func() (string, error) {
+ return m.PutIfAbsentVersionedAt(ctx, signAt, "data-uk", "suspended/pia", []byte("x"))
+ },
+ } {
+ v, err := call()
+ if err == nil || v != "" || !strings.Contains(err.Error(), "version id") {
+ t.Errorf("%s answered with the null version: %q, %v", name, v, err)
+ }
+ }
+ for _, v := range []string{"", "null"} {
+ if lease.UsableVersionID(v) {
+ t.Errorf("UsableVersionID(%q) = true", v)
+ }
+ }
+ if !lease.UsableVersionID("3HL4kqtJlcpXroDTDmJ+rmSpXd3dIbrHY") {
+ t.Error("a real version id is not usable")
+ }
+}
+
// A redirect is an error, and it is never followed: no second request,
// whatever the status (a 307/308 would resend the mutation, a 301/302/303
// would turn it into a GET).
diff --git a/internal/moves/readiness.go b/internal/moves/readiness.go
index 9d5b8e8aa..7c6222688 100644
--- a/internal/moves/readiness.go
+++ b/internal/moves/readiness.go
@@ -7,6 +7,7 @@ import (
"os"
"time"
+ "github.com/carlosframework/platform/internal/lease"
"github.com/carlosframework/platform/internal/provision"
"github.com/carlosframework/platform/internal/release"
)
@@ -129,6 +130,15 @@ type ArchiveRef struct {
Plain string `json:"plain"`
}
+// Pinned reports whether both objects are named by a version id that pins
+// one immutable version (lease.UsableVersionID): not "", and not the "null"
+// a versioning-suspended bucket answers, which the next write to the key
+// overwrites (final review finding 4). A reference that is not pinned is
+// never verified or trusted from a cache.
+func (r ArchiveRef) Pinned() bool {
+ return lease.UsableVersionID(r.ManifestVersion) && lease.UsableVersionID(r.SealedVersion)
+}
+
// CrossMeasurement is the source's cross-region rate as a measurement. A
// nil report, or one that has measured nothing, gives the zero measurement,
// which Measurements.Rates refuses.
diff --git a/internal/moves/readiness_test.go b/internal/moves/readiness_test.go
index 53669edfb..2e2eecdc1 100644
--- a/internal/moves/readiness_test.go
+++ b/internal/moves/readiness_test.go
@@ -840,3 +840,20 @@ func TestNoFreshRateIsTyped(t *testing.T) {
t.Fatalf("%q", r.Check.Reason)
}
}
+
+// Final review finding 4: an archive reference pins its objects only by
+// version ids that name one immutable version each: neither "" nor the
+// suspended bucket's "null".
+func TestArchiveRefPinnedNeedsImmutableVersions(t *testing.T) {
+ good := ArchiveRef{ManifestKey: "m", ManifestVersion: "mv1", SealedObjectKey: "s", SealedVersion: "sv1"}
+ if !good.Pinned() {
+ t.Fatal("a reference with real version ids is not pinned")
+ }
+ for _, c := range []struct{ manifest, sealed string }{{"null", "sv1"}, {"mv1", "null"}, {"", "sv1"}, {"mv1", ""}, {"null", "null"}} {
+ r := good
+ r.ManifestVersion, r.SealedVersion = c.manifest, c.sealed
+ if r.Pinned() {
+ t.Errorf("versions %q and %q read as pinned", c.manifest, c.sealed)
+ }
+ }
+}