fix(snapshot): preserve manifest version errors
- commit
- 816fe328bdc7c9ab4a5482cfeb65b878f615503c
- author
- brice · · 2026-07-27 13:11 UTC
- parent
- fcb4c06
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
2 files · +100 -1
M internal/adapters/snapshot/direct_load_test.go +96 -0
| @@ -3,6 +3,7 @@ package snapshot | |||
| 3 | 3 | import ( | |
| 4 | 4 | "context" | |
| 5 | 5 | "crypto/sha256" | |
| 6 | + | "encoding/binary" | |
| 6 | 7 | "errors" | |
| 7 | 8 | "os" | |
| 8 | 9 | "path/filepath" | |
| @@ -61,6 +62,101 @@ func TestDirectCheckpointLoad(t *testing.T) { | |||
| 61 | 62 | } | |
| 62 | 63 | } | |
| 63 | 64 | ||
| 65 | + | func TestLoadCheckpointPreservesManifestVersionError(t *testing.T) { | |
| 66 | + | for _, tt := range []struct { | |
| 67 | + | name string | |
| 68 | + | version uint16 | |
| 69 | + | }{ | |
| 70 | + | {name: "old", version: codec.ManifestVersion - 1}, | |
| 71 | + | {name: "future", version: codec.ManifestVersion + 1}, | |
| 72 | + | } { | |
| 73 | + | t.Run(tt.name, func(t *testing.T) { | |
| 74 | + | repo := NewRepository(privateDir(t)) | |
| 75 | + | id := domain.IncarnationID{1} | |
| 76 | + | pub := incarnationPublication(t, id, "work", 1, nil) | |
| 77 | + | require.NoError(t, repo.Publish(t.Context(), pub)) | |
| 78 | + | ||
| 79 | + | manifest := append([]byte(nil), pub.Manifest...) | |
| 80 | + | binary.BigEndian.PutUint16(manifest[4:6], tt.version) | |
| 81 | + | require.NoError(t, os.WriteFile(repo.manifestPath(id, 1), manifest, 0o600)) | |
| 82 | + | ref := domain.CheckpointRef{Generation: 1, ManifestDigest: sha256.Sum256(manifest)} | |
| 83 | + | ||
| 84 | + | _, err := repo.LoadCheckpoint(t.Context(), id, "work", ref) | |
| 85 | + | require.ErrorIs(t, err, codec.ErrBadVersion) | |
| 86 | + | }) | |
| 87 | + | } | |
| 88 | + | } | |
| 89 | + | ||
| 90 | + | func TestLoadCheckpointDoesNotClassifyCorruptionAsManifestVersion(t *testing.T) { | |
| 91 | + | for _, tt := range []struct { | |
| 92 | + | name string | |
| 93 | + | mutate func(*testing.T, *Repository, ports.SnapshotPublication, *domain.CheckpointRef) | |
| 94 | + | }{ | |
| 95 | + | { | |
| 96 | + | name: "digest mismatch precedes version decode", | |
| 97 | + | mutate: func(t *testing.T, repo *Repository, pub ports.SnapshotPublication, _ *domain.CheckpointRef) { | |
| 98 | + | t.Helper() | |
| 99 | + | manifest := append([]byte(nil), pub.Manifest...) | |
| 100 | + | binary.BigEndian.PutUint16(manifest[4:6], codec.ManifestVersion-1) | |
| 101 | + | require.NoError(t, os.WriteFile(repo.manifestPath(pub.IncarnationID, pub.Generation), manifest, 0o600)) | |
| 102 | + | }, | |
| 103 | + | }, | |
| 104 | + | { | |
| 105 | + | name: "bad magic", | |
| 106 | + | mutate: func(t *testing.T, repo *Repository, pub ports.SnapshotPublication, ref *domain.CheckpointRef) { | |
| 107 | + | t.Helper() | |
| 108 | + | manifest := append([]byte(nil), pub.Manifest...) | |
| 109 | + | manifest[0] ^= 1 | |
| 110 | + | ref.ManifestDigest = sha256.Sum256(manifest) | |
| 111 | + | require.NoError(t, os.WriteFile(repo.manifestPath(pub.IncarnationID, pub.Generation), manifest, 0o600)) | |
| 112 | + | }, | |
| 113 | + | }, | |
| 114 | + | { | |
| 115 | + | name: "bad crc", | |
| 116 | + | mutate: func(t *testing.T, repo *Repository, pub ports.SnapshotPublication, ref *domain.CheckpointRef) { | |
| 117 | + | t.Helper() | |
| 118 | + | manifest := append([]byte(nil), pub.Manifest...) | |
| 119 | + | manifest[12] ^= 1 | |
| 120 | + | ref.ManifestDigest = sha256.Sum256(manifest) | |
| 121 | + | require.NoError(t, os.WriteFile(repo.manifestPath(pub.IncarnationID, pub.Generation), manifest, 0o600)) | |
| 122 | + | }, | |
| 123 | + | }, | |
| 124 | + | { | |
| 125 | + | name: "wrong identity", | |
| 126 | + | mutate: func(t *testing.T, repo *Repository, pub ports.SnapshotPublication, ref *domain.CheckpointRef) { | |
| 127 | + | t.Helper() | |
| 128 | + | manifest, err := codec.UnmarshalManifest(pub.Manifest) | |
| 129 | + | require.NoError(t, err) | |
| 130 | + | manifest.IncarnationID = domain.IncarnationID{2} | |
| 131 | + | encoded, err := codec.MarshalManifest(manifest) | |
| 132 | + | require.NoError(t, err) | |
| 133 | + | ref.ManifestDigest = sha256.Sum256(encoded) | |
| 134 | + | require.NoError(t, os.WriteFile(repo.manifestPath(pub.IncarnationID, pub.Generation), encoded, 0o600)) | |
| 135 | + | }, | |
| 136 | + | }, | |
| 137 | + | { | |
| 138 | + | name: "invalid object", | |
| 139 | + | mutate: func(t *testing.T, repo *Repository, pub ports.SnapshotPublication, _ *domain.CheckpointRef) { | |
| 140 | + | t.Helper() | |
| 141 | + | require.NoError(t, os.WriteFile(repo.objectPath(pub.IncarnationID, pub.Objects[0].Digest), []byte("corrupt"), 0o600)) | |
| 142 | + | }, | |
| 143 | + | }, | |
| 144 | + | } { | |
| 145 | + | t.Run(tt.name, func(t *testing.T) { | |
| 146 | + | repo := NewRepository(privateDir(t)) | |
| 147 | + | id := domain.IncarnationID{1} | |
| 148 | + | pub := incarnationPublication(t, id, "work", 1, nil) | |
| 149 | + | require.NoError(t, repo.Publish(t.Context(), pub)) | |
| 150 | + | ref := domain.CheckpointRef{Generation: 1, ManifestDigest: sha256.Sum256(pub.Manifest)} | |
| 151 | + | tt.mutate(t, repo, pub, &ref) | |
| 152 | + | ||
| 153 | + | _, err := repo.LoadCheckpoint(t.Context(), id, "work", ref) | |
| 154 | + | require.Error(t, err) | |
| 155 | + | require.NotErrorIs(t, err, codec.ErrBadVersion) | |
| 156 | + | }) | |
| 157 | + | } | |
| 158 | + | } | |
| 159 | + | ||
| 64 | 160 | func TestReconcileCheckpoint(t *testing.T) { | |
| 65 | 161 | repo := NewRepository(privateDir(t)) | |
| 66 | 162 | id := domain.IncarnationID{1} | |
| @@ -3,6 +3,7 @@ package snapshot | |||||
| 3 | import ( | 3 | import ( | ||
| 4 | "context" | 4 | "context" | ||
| 5 | "crypto/sha256" | 5 | "crypto/sha256" | ||
| 6 | + | "encoding/binary" | |||
| 6 | "errors" | 7 | "errors" | ||
| 7 | "os" | 8 | "os" | ||
| 8 | "path/filepath" | 9 | "path/filepath" | ||
| @@ -61,6 +62,101 @@ func TestDirectCheckpointLoad(t *testing.T) { | |||||
| 61 | } | 62 | } | ||
| 62 | } | 63 | } | ||
| 63 | 64 | ||||
| 65 | + | func TestLoadCheckpointPreservesManifestVersionError(t *testing.T) { | |||
| 66 | + | for _, tt := range []struct { | |||
| 67 | + | name string | |||
| 68 | + | version uint16 | |||
| 69 | + | }{ | |||
| 70 | + | {name: "old", version: codec.ManifestVersion - 1}, | |||
| 71 | + | {name: "future", version: codec.ManifestVersion + 1}, | |||
| 72 | + | } { | |||
| 73 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 74 | + | repo := NewRepository(privateDir(t)) | |||
| 75 | + | id := domain.IncarnationID{1} | |||
| 76 | + | pub := incarnationPublication(t, id, "work", 1, nil) | |||
| 77 | + | require.NoError(t, repo.Publish(t.Context(), pub)) | |||
| 78 | + | ||||
| 79 | + | manifest := append([]byte(nil), pub.Manifest...) | |||
| 80 | + | binary.BigEndian.PutUint16(manifest[4:6], tt.version) | |||
| 81 | + | require.NoError(t, os.WriteFile(repo.manifestPath(id, 1), manifest, 0o600)) | |||
| 82 | + | ref := domain.CheckpointRef{Generation: 1, ManifestDigest: sha256.Sum256(manifest)} | |||
| 83 | + | ||||
| 84 | + | _, err := repo.LoadCheckpoint(t.Context(), id, "work", ref) | |||
| 85 | + | require.ErrorIs(t, err, codec.ErrBadVersion) | |||
| 86 | + | }) | |||
| 87 | + | } | |||
| 88 | + | } | |||
| 89 | + | ||||
| 90 | + | func TestLoadCheckpointDoesNotClassifyCorruptionAsManifestVersion(t *testing.T) { | |||
| 91 | + | for _, tt := range []struct { | |||
| 92 | + | name string | |||
| 93 | + | mutate func(*testing.T, *Repository, ports.SnapshotPublication, *domain.CheckpointRef) | |||
| 94 | + | }{ | |||
| 95 | + | { | |||
| 96 | + | name: "digest mismatch precedes version decode", | |||
| 97 | + | mutate: func(t *testing.T, repo *Repository, pub ports.SnapshotPublication, _ *domain.CheckpointRef) { | |||
| 98 | + | t.Helper() | |||
| 99 | + | manifest := append([]byte(nil), pub.Manifest...) | |||
| 100 | + | binary.BigEndian.PutUint16(manifest[4:6], codec.ManifestVersion-1) | |||
| 101 | + | require.NoError(t, os.WriteFile(repo.manifestPath(pub.IncarnationID, pub.Generation), manifest, 0o600)) | |||
| 102 | + | }, | |||
| 103 | + | }, | |||
| 104 | + | { | |||
| 105 | + | name: "bad magic", | |||
| 106 | + | mutate: func(t *testing.T, repo *Repository, pub ports.SnapshotPublication, ref *domain.CheckpointRef) { | |||
| 107 | + | t.Helper() | |||
| 108 | + | manifest := append([]byte(nil), pub.Manifest...) | |||
| 109 | + | manifest[0] ^= 1 | |||
| 110 | + | ref.ManifestDigest = sha256.Sum256(manifest) | |||
| 111 | + | require.NoError(t, os.WriteFile(repo.manifestPath(pub.IncarnationID, pub.Generation), manifest, 0o600)) | |||
| 112 | + | }, | |||
| 113 | + | }, | |||
| 114 | + | { | |||
| 115 | + | name: "bad crc", | |||
| 116 | + | mutate: func(t *testing.T, repo *Repository, pub ports.SnapshotPublication, ref *domain.CheckpointRef) { | |||
| 117 | + | t.Helper() | |||
| 118 | + | manifest := append([]byte(nil), pub.Manifest...) | |||
| 119 | + | manifest[12] ^= 1 | |||
| 120 | + | ref.ManifestDigest = sha256.Sum256(manifest) | |||
| 121 | + | require.NoError(t, os.WriteFile(repo.manifestPath(pub.IncarnationID, pub.Generation), manifest, 0o600)) | |||
| 122 | + | }, | |||
| 123 | + | }, | |||
| 124 | + | { | |||
| 125 | + | name: "wrong identity", | |||
| 126 | + | mutate: func(t *testing.T, repo *Repository, pub ports.SnapshotPublication, ref *domain.CheckpointRef) { | |||
| 127 | + | t.Helper() | |||
| 128 | + | manifest, err := codec.UnmarshalManifest(pub.Manifest) | |||
| 129 | + | require.NoError(t, err) | |||
| 130 | + | manifest.IncarnationID = domain.IncarnationID{2} | |||
| 131 | + | encoded, err := codec.MarshalManifest(manifest) | |||
| 132 | + | require.NoError(t, err) | |||
| 133 | + | ref.ManifestDigest = sha256.Sum256(encoded) | |||
| 134 | + | require.NoError(t, os.WriteFile(repo.manifestPath(pub.IncarnationID, pub.Generation), encoded, 0o600)) | |||
| 135 | + | }, | |||
| 136 | + | }, | |||
| 137 | + | { | |||
| 138 | + | name: "invalid object", | |||
| 139 | + | mutate: func(t *testing.T, repo *Repository, pub ports.SnapshotPublication, _ *domain.CheckpointRef) { | |||
| 140 | + | t.Helper() | |||
| 141 | + | require.NoError(t, os.WriteFile(repo.objectPath(pub.IncarnationID, pub.Objects[0].Digest), []byte("corrupt"), 0o600)) | |||
| 142 | + | }, | |||
| 143 | + | }, | |||
| 144 | + | } { | |||
| 145 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 146 | + | repo := NewRepository(privateDir(t)) | |||
| 147 | + | id := domain.IncarnationID{1} | |||
| 148 | + | pub := incarnationPublication(t, id, "work", 1, nil) | |||
| 149 | + | require.NoError(t, repo.Publish(t.Context(), pub)) | |||
| 150 | + | ref := domain.CheckpointRef{Generation: 1, ManifestDigest: sha256.Sum256(pub.Manifest)} | |||
| 151 | + | tt.mutate(t, repo, pub, &ref) | |||
| 152 | + | ||||
| 153 | + | _, err := repo.LoadCheckpoint(t.Context(), id, "work", ref) | |||
| 154 | + | require.Error(t, err) | |||
| 155 | + | require.NotErrorIs(t, err, codec.ErrBadVersion) | |||
| 156 | + | }) | |||
| 157 | + | } | |||
| 158 | + | } | |||
| 159 | + | ||||
| 64 | func TestReconcileCheckpoint(t *testing.T) { | 160 | func TestReconcileCheckpoint(t *testing.T) { | ||
| 65 | repo := NewRepository(privateDir(t)) | 161 | repo := NewRepository(privateDir(t)) | ||
| 66 | id := domain.IncarnationID{1} | 162 | id := domain.IncarnationID{1} | ||
M internal/adapters/snapshot/load.go +4 -1
| @@ -41,7 +41,10 @@ func (r *Repository) loadCheckpointLocked(ctx context.Context, id domain.Incarna | |||
| 41 | 41 | return ports.SnapshotGeneration{}, codec.Manifest{}, fmt.Errorf("manifest digest mismatch") | |
| 42 | 42 | } | |
| 43 | 43 | manifest, err := codec.UnmarshalManifest(data) | |
| 44 | - | if err != nil || manifest.IncarnationID != id || manifest.Generation != ref.Generation { | |
| 44 | + | if err != nil { | |
| 45 | + | return ports.SnapshotGeneration{}, codec.Manifest{}, fmt.Errorf("invalid manifest: %w", err) | |
| 46 | + | } | |
| 47 | + | if manifest.IncarnationID != id || manifest.Generation != ref.Generation { | |
| 45 | 48 | return ports.SnapshotGeneration{}, codec.Manifest{}, fmt.Errorf("invalid manifest") | |
| 46 | 49 | } | |
| 47 | 50 | refs := manifestRefs(manifest) | |
| @@ -41,7 +41,10 @@ func (r *Repository) loadCheckpointLocked(ctx context.Context, id domain.Incarna | |||||
| 41 | return ports.SnapshotGeneration{}, codec.Manifest{}, fmt.Errorf("manifest digest mismatch") | 41 | return ports.SnapshotGeneration{}, codec.Manifest{}, fmt.Errorf("manifest digest mismatch") | ||
| 42 | } | 42 | } | ||
| 43 | manifest, err := codec.UnmarshalManifest(data) | 43 | manifest, err := codec.UnmarshalManifest(data) | ||
| 44 | - | if err != nil || manifest.IncarnationID != id || manifest.Generation != ref.Generation { | 44 | + | if err != nil { |
| 45 | + | return ports.SnapshotGeneration{}, codec.Manifest{}, fmt.Errorf("invalid manifest: %w", err) | |||
| 46 | + | } | |||
| 47 | + | if manifest.IncarnationID != id || manifest.Generation != ref.Generation { | |||
| 45 | return ports.SnapshotGeneration{}, codec.Manifest{}, fmt.Errorf("invalid manifest") | 48 | return ports.SnapshotGeneration{}, codec.Manifest{}, fmt.Errorf("invalid manifest") | ||
| 46 | } | 49 | } | ||
| 47 | refs := manifestRefs(manifest) | 50 | refs := manifestRefs(manifest) | ||