Merge remote-tracking branch 'origin/feat/backup-phase0a-keymap-manifest' into feat/backup-phase0a-s3

Author: bootjp

internal/backup/manifest.go

@@ -224,21 +224,8 @@ func ReadManifest(r io.Reader) (Manifest, error) {
 	if err != nil {
 		return Manifest{}, errors.Wrap(ErrInvalidManifest, err.Error())
 	}
-	// Phase 1: probe format_version with a relaxed shape that tolerates
-	// arbitrary types on every other field.
-	var probe struct {
-		FormatVersion uint32 `json:"format_version"`
-	}
-	if err := json.Unmarshal(payload, &probe); err != nil {
-		return Manifest{}, errors.Wrap(ErrInvalidManifest, err.Error())
-	}
-	if probe.FormatVersion == 0 {
-		return Manifest{}, errors.Wrapf(ErrUnsupportedFormatVersion,
-			"format_version is zero")
-	}
-	if probe.FormatVersion > CurrentFormatVersion {
-		return Manifest{}, errors.Wrapf(ErrUnsupportedFormatVersion,
-			"format_version %d > current %d (newer producer)", probe.FormatVersion, CurrentFormatVersion)
+	if err := probeManifestFormatVersion(payload); err != nil {
+		return Manifest{}, err
 	}
 	// Phase 2: strict struct decode on a known-supported version.
 	var m Manifest
@@ -277,6 +264,48 @@ func ReadManifest(r io.Reader) (Manifest, error) {
 	return m, nil
 }
 
+// probeManifestFormatVersion runs the relaxed-shape format_version
+// gate that ReadManifest applies before the strict struct decode.
+// Splitting it into its own function keeps ReadManifest under the
+// project's cyclomatic-complexity ceiling. The contract:
+//
+//   - missing or null `format_version` -> ErrInvalidManifest
+//     (truncated/malformed file; Codex P2 round 8). Without this
+//     branch json.Unmarshal would collapse absence to zero and the
+//     version gate would misclassify as upgrade-required.
+//   - `format_version` = 0 -> ErrUnsupportedFormatVersion (the
+//     reserved sentinel for "no version assigned").
+//   - `format_version` > CurrentFormatVersion ->
+//     ErrUnsupportedFormatVersion (newer producer; upgrade-required).
+//   - within range -> nil; the strict struct decode runs next.
+func probeManifestFormatVersion(payload []byte) error {
+	var top map[string]json.RawMessage
+	if err := json.Unmarshal(payload, &top); err != nil {
+		return errors.Wrap(ErrInvalidManifest, err.Error())
+	}
+	rawFV, hasFV := top["format_version"]
+	if !hasFV {
+		return errors.Wrap(ErrInvalidManifest, "format_version missing")
+	}
+	if bytes.Equal(rawFV, jsonNullLiteral) {
+		return errors.Wrap(ErrInvalidManifest, "format_version is null")
+	}
+	var probe struct {
+		FormatVersion uint32 `json:"format_version"`
+	}
+	if err := json.Unmarshal(payload, &probe); err != nil {
+		return errors.Wrap(ErrInvalidManifest, err.Error())
+	}
+	if probe.FormatVersion == 0 {
+		return errors.Wrap(ErrUnsupportedFormatVersion, "format_version is zero")
+	}
+	if probe.FormatVersion > CurrentFormatVersion {
+		return errors.Wrapf(ErrUnsupportedFormatVersion,
+			"format_version %d > current %d (newer producer)", probe.FormatVersion, CurrentFormatVersion)
+	}
+	return nil
+}
+
 // validateExclusionsFieldsPresent rejects manifests whose `exclusions`
 // section omits any of the required boolean flags. Go's
 // json.Unmarshal silently fills missing booleans with `false`, so a
@@ -339,6 +368,16 @@ func (m Manifest) validateRequiredFields() error {
 	if m.FormatVersion == 0 {
 		return errors.Wrap(ErrInvalidManifest, "format_version is zero")
 	}
+	// WriteManifest must refuse manifests advertising a version this
+	// build cannot produce — without this gate, a caller mutating
+	// `m.FormatVersion = CurrentFormatVersion + 1` would write a
+	// manifest that ReadManifest in the same package then rejects as
+	// ErrUnsupportedFormatVersion, producing self-incompatible
+	// backup metadata. Codex P2 round 8.
+	if m.FormatVersion > CurrentFormatVersion {
+		return errors.Wrapf(ErrInvalidManifest,
+			"format_version %d > current %d (this build cannot produce that)", m.FormatVersion, CurrentFormatVersion)
+	}
 	switch m.Phase {
 	case PhasePhase0SnapshotDecode, PhasePhase1LivePinned:
 	default:

internal/backup/manifest_test.go

@@ -170,6 +170,77 @@ func TestReadManifest_RejectsZeroFormatVersion(t *testing.T) {
 	}
 }
 
+// TestReadManifest_RejectsMissingFormatVersion is the regression for
+// Codex P2 round 8: an absent `format_version` unmarshals into uint32
+// zero, which the version gate would otherwise misclassify as
+// ErrUnsupportedFormatVersion ("upgrade required"). A truncated /
+// malformed manifest that dropped the field belongs in the
+// ErrInvalidManifest branch instead.
+func TestReadManifest_RejectsMissingFormatVersion(t *testing.T) {
+	t.Parallel()
+	body := `{
+		"phase": "phase0-snapshot-decode",
+		"wall_time_iso": "2026-04-29T00:00:00Z",
+		"adapters": {},
+		"exclusions": {"include_incomplete_uploads":false,"include_orphans":false,"preserve_sqs_visibility":false,"include_sqs_side_records":false},
+		"checksum_algorithm": "sha256",
+		"checksum_format": "sha256sum",
+		"encoded_filename_charset": "rfc3986-unreserved-plus-percent",
+		"key_segment_max_bytes": 240,
+		"s3_meta_suffix": ".elastickv-meta.json",
+		"s3_collision_strategy": "leaf-data-suffix",
+		"dynamodb_layout": "per-item"
+	}`
+	_, err := ReadManifest(strings.NewReader(body))
+	if !errors.Is(err, ErrInvalidManifest) {
+		t.Fatalf("err=%v want ErrInvalidManifest", err)
+	}
+	if errors.Is(err, ErrUnsupportedFormatVersion) {
+		t.Fatalf("missing format_version must not surface as upgrade-required: %v", err)
+	}
+}
+
+// TestReadManifest_RejectsNullFormatVersion mirrors the missing-field
+// case for `"format_version": null`.
+func TestReadManifest_RejectsNullFormatVersion(t *testing.T) {
+	t.Parallel()
+	body := `{
+		"format_version": null,
+		"phase": "phase0-snapshot-decode",
+		"wall_time_iso": "2026-04-29T00:00:00Z",
+		"adapters": {},
+		"exclusions": {"include_incomplete_uploads":false,"include_orphans":false,"preserve_sqs_visibility":false,"include_sqs_side_records":false},
+		"checksum_algorithm": "sha256",
+		"checksum_format": "sha256sum",
+		"encoded_filename_charset": "rfc3986-unreserved-plus-percent",
+		"key_segment_max_bytes": 240,
+		"s3_meta_suffix": ".elastickv-meta.json",
+		"s3_collision_strategy": "leaf-data-suffix",
+		"dynamodb_layout": "per-item"
+	}`
+	_, err := ReadManifest(strings.NewReader(body))
+	if !errors.Is(err, ErrInvalidManifest) {
+		t.Fatalf("err=%v want ErrInvalidManifest", err)
+	}
+}
+
+// TestWriteManifest_RejectsFutureFormatVersion is the regression for
+// Codex P2 round 8: WriteManifest must refuse manifests advertising
+// a version this build cannot produce. Without this gate, a caller
+// mutating m.FormatVersion = CurrentFormatVersion + 1 writes a
+// manifest that the same package's ReadManifest then refuses,
+// producing self-incompatible backup metadata.
+func TestWriteManifest_RejectsFutureFormatVersion(t *testing.T) {
+	t.Parallel()
+	m := NewPhase0SnapshotManifest(time.Now())
+	m.FormatVersion = CurrentFormatVersion + 1
+	var buf bytes.Buffer
+	err := WriteManifest(&buf, m)
+	if !errors.Is(err, ErrInvalidManifest) {
+		t.Fatalf("WriteManifest err=%v want ErrInvalidManifest for future format_version", err)
+	}
+}
+
 func TestReadManifest_AcceptsUnknownFieldsForSameMajorMinorEvolution(t *testing.T) {
 	t.Parallel()
 	// Same-major minor evolution: a newer producer adds an optional