refactor(snapshot): simplify publication validation
- commit
- 20864cba18d8139199f3f7c7402e3819cb161be8
- author
- brice · · 2026-07-21 20:29 UTC
- parent
- 13c7e30
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
2 files · +12 -13
M internal/adapters/snapshot/publish.go +11 -12
| @@ -18,11 +18,10 @@ func (r *Repository) Publish(ctx context.Context, publication ports.SnapshotPubl | |||
| 18 | 18 | lock.Lock() | |
| 19 | 19 | defer lock.Unlock() | |
| 20 | 20 | ||
| 21 | - | manifest, refs, supplied, err := validatePublication(publication) | |
| 21 | + | refs, supplied, err := validatePublication(publication) | |
| 22 | 22 | if err != nil { | |
| 23 | 23 | return err | |
| 24 | 24 | } | |
| 25 | - | _ = manifest | |
| 26 | 25 | if err := r.ensureSession(key); err != nil { | |
| 27 | 26 | return err | |
| 28 | 27 | } | |
| @@ -106,40 +105,40 @@ func (r *Repository) Publish(ctx context.Context, publication ports.SnapshotPubl | |||
| 106 | 105 | } | |
| 107 | 106 | return nil | |
| 108 | 107 | } | |
| 109 | - | func validatePublication(p ports.SnapshotPublication) (codec.Manifest, map[ports.SnapshotDigest]codec.ObjectRef, map[ports.SnapshotDigest]ports.SnapshotObject, error) { | |
| 108 | + | func validatePublication(p ports.SnapshotPublication) (map[ports.SnapshotDigest]codec.ObjectRef, map[ports.SnapshotDigest]ports.SnapshotObject, error) { | |
| 110 | 109 | manifest, err := codec.UnmarshalManifest(p.Manifest) | |
| 111 | 110 | if err != nil { | |
| 112 | - | return codec.Manifest{}, nil, nil, fmt.Errorf("invalid manifest: %w", err) | |
| 111 | + | return nil, nil, fmt.Errorf("invalid manifest: %w", err) | |
| 113 | 112 | } | |
| 114 | 113 | if manifest.Name != p.Name || manifest.Generation != p.Generation || p.Generation == 0 { | |
| 115 | - | return codec.Manifest{}, nil, nil, fmt.Errorf("manifest name or generation does not match publication") | |
| 114 | + | return nil, nil, fmt.Errorf("manifest name or generation does not match publication") | |
| 116 | 115 | } | |
| 117 | 116 | refs := manifestRefs(manifest) | |
| 118 | 117 | if refs == nil { | |
| 119 | - | return codec.Manifest{}, nil, nil, fmt.Errorf("conflicting manifest references") | |
| 118 | + | return nil, nil, fmt.Errorf("conflicting manifest references") | |
| 120 | 119 | } | |
| 121 | 120 | if !withinGenerationBudget(len(p.Manifest), refs) { | |
| 122 | - | return codec.Manifest{}, nil, nil, fmt.Errorf("snapshot generation too large") | |
| 121 | + | return nil, nil, fmt.Errorf("snapshot generation too large") | |
| 123 | 122 | } | |
| 124 | 123 | objects := make(map[ports.SnapshotDigest]ports.SnapshotObject, len(p.Objects)) | |
| 125 | 124 | for _, object := range p.Objects { | |
| 126 | 125 | if sha256.Sum256(object.Data) != object.Digest { | |
| 127 | - | return codec.Manifest{}, nil, nil, fmt.Errorf("object digest mismatch") | |
| 126 | + | return nil, nil, fmt.Errorf("object digest mismatch") | |
| 128 | 127 | } | |
| 129 | 128 | ref, used := refs[object.Digest] | |
| 130 | 129 | if !used { | |
| 131 | - | return codec.Manifest{}, nil, nil, fmt.Errorf("unreferenced object") | |
| 130 | + | return nil, nil, fmt.Errorf("unreferenced object") | |
| 132 | 131 | } | |
| 133 | 132 | kind, payload, err := codec.PreflightObject(object.Data) | |
| 134 | 133 | if err != nil || kind != ref.Kind || len(object.Data) != int(ref.Size) || len(payload) == 0 { | |
| 135 | - | return codec.Manifest{}, nil, nil, fmt.Errorf("invalid object envelope") | |
| 134 | + | return nil, nil, fmt.Errorf("invalid object envelope") | |
| 136 | 135 | } | |
| 137 | 136 | if old, duplicate := objects[object.Digest]; duplicate && !equalBytes(old.Data, object.Data) { | |
| 138 | - | return codec.Manifest{}, nil, nil, fmt.Errorf("conflicting object") | |
| 137 | + | return nil, nil, fmt.Errorf("conflicting object") | |
| 139 | 138 | } | |
| 140 | 139 | objects[object.Digest] = ports.SnapshotObject{Digest: object.Digest, Data: append([]byte(nil), object.Data...)} | |
| 141 | 140 | } | |
| 142 | - | return manifest, refs, objects, nil | |
| 141 | + | return refs, objects, nil | |
| 143 | 142 | } | |
| 144 | 143 | ||
| 145 | 144 | func withinGenerationBudget(manifestSize int, refs map[ports.SnapshotDigest]codec.ObjectRef) bool { | |
| @@ -18,11 +18,10 @@ func (r *Repository) Publish(ctx context.Context, publication ports.SnapshotPubl | |||||
| 18 | lock.Lock() | 18 | lock.Lock() | ||
| 19 | defer lock.Unlock() | 19 | defer lock.Unlock() | ||
| 20 | 20 | ||||
| 21 | - | manifest, refs, supplied, err := validatePublication(publication) | 21 | + | refs, supplied, err := validatePublication(publication) |
| 22 | if err != nil { | 22 | if err != nil { | ||
| 23 | return err | 23 | return err | ||
| 24 | } | 24 | } | ||
| 25 | - | _ = manifest | |||
| 26 | if err := r.ensureSession(key); err != nil { | 25 | if err := r.ensureSession(key); err != nil { | ||
| 27 | return err | 26 | return err | ||
| 28 | } | 27 | } | ||
| @@ -106,40 +105,40 @@ func (r *Repository) Publish(ctx context.Context, publication ports.SnapshotPubl | |||||
| 106 | } | 105 | } | ||
| 107 | return nil | 106 | return nil | ||
| 108 | } | 107 | } | ||
| 109 | - | func validatePublication(p ports.SnapshotPublication) (codec.Manifest, map[ports.SnapshotDigest]codec.ObjectRef, map[ports.SnapshotDigest]ports.SnapshotObject, error) { | 108 | + | func validatePublication(p ports.SnapshotPublication) (map[ports.SnapshotDigest]codec.ObjectRef, map[ports.SnapshotDigest]ports.SnapshotObject, error) { |
| 110 | manifest, err := codec.UnmarshalManifest(p.Manifest) | 109 | manifest, err := codec.UnmarshalManifest(p.Manifest) | ||
| 111 | if err != nil { | 110 | if err != nil { | ||
| 112 | - | return codec.Manifest{}, nil, nil, fmt.Errorf("invalid manifest: %w", err) | 111 | + | return nil, nil, fmt.Errorf("invalid manifest: %w", err) |
| 113 | } | 112 | } | ||
| 114 | if manifest.Name != p.Name || manifest.Generation != p.Generation || p.Generation == 0 { | 113 | if manifest.Name != p.Name || manifest.Generation != p.Generation || p.Generation == 0 { | ||
| 115 | - | return codec.Manifest{}, nil, nil, fmt.Errorf("manifest name or generation does not match publication") | 114 | + | return nil, nil, fmt.Errorf("manifest name or generation does not match publication") |
| 116 | } | 115 | } | ||
| 117 | refs := manifestRefs(manifest) | 116 | refs := manifestRefs(manifest) | ||
| 118 | if refs == nil { | 117 | if refs == nil { | ||
| 119 | - | return codec.Manifest{}, nil, nil, fmt.Errorf("conflicting manifest references") | 118 | + | return nil, nil, fmt.Errorf("conflicting manifest references") |
| 120 | } | 119 | } | ||
| 121 | if !withinGenerationBudget(len(p.Manifest), refs) { | 120 | if !withinGenerationBudget(len(p.Manifest), refs) { | ||
| 122 | - | return codec.Manifest{}, nil, nil, fmt.Errorf("snapshot generation too large") | 121 | + | return nil, nil, fmt.Errorf("snapshot generation too large") |
| 123 | } | 122 | } | ||
| 124 | objects := make(map[ports.SnapshotDigest]ports.SnapshotObject, len(p.Objects)) | 123 | objects := make(map[ports.SnapshotDigest]ports.SnapshotObject, len(p.Objects)) | ||
| 125 | for _, object := range p.Objects { | 124 | for _, object := range p.Objects { | ||
| 126 | if sha256.Sum256(object.Data) != object.Digest { | 125 | if sha256.Sum256(object.Data) != object.Digest { | ||
| 127 | - | return codec.Manifest{}, nil, nil, fmt.Errorf("object digest mismatch") | 126 | + | return nil, nil, fmt.Errorf("object digest mismatch") |
| 128 | } | 127 | } | ||
| 129 | ref, used := refs[object.Digest] | 128 | ref, used := refs[object.Digest] | ||
| 130 | if !used { | 129 | if !used { | ||
| 131 | - | return codec.Manifest{}, nil, nil, fmt.Errorf("unreferenced object") | 130 | + | return nil, nil, fmt.Errorf("unreferenced object") |
| 132 | } | 131 | } | ||
| 133 | kind, payload, err := codec.PreflightObject(object.Data) | 132 | kind, payload, err := codec.PreflightObject(object.Data) | ||
| 134 | if err != nil || kind != ref.Kind || len(object.Data) != int(ref.Size) || len(payload) == 0 { | 133 | if err != nil || kind != ref.Kind || len(object.Data) != int(ref.Size) || len(payload) == 0 { | ||
| 135 | - | return codec.Manifest{}, nil, nil, fmt.Errorf("invalid object envelope") | 134 | + | return nil, nil, fmt.Errorf("invalid object envelope") |
| 136 | } | 135 | } | ||
| 137 | if old, duplicate := objects[object.Digest]; duplicate && !equalBytes(old.Data, object.Data) { | 136 | if old, duplicate := objects[object.Digest]; duplicate && !equalBytes(old.Data, object.Data) { | ||
| 138 | - | return codec.Manifest{}, nil, nil, fmt.Errorf("conflicting object") | 137 | + | return nil, nil, fmt.Errorf("conflicting object") |
| 139 | } | 138 | } | ||
| 140 | objects[object.Digest] = ports.SnapshotObject{Digest: object.Digest, Data: append([]byte(nil), object.Data...)} | 139 | objects[object.Digest] = ports.SnapshotObject{Digest: object.Digest, Data: append([]byte(nil), object.Data...)} | ||
| 141 | } | 140 | } | ||
| 142 | - | return manifest, refs, objects, nil | 141 | + | return refs, objects, nil |
| 143 | } | 142 | } | ||
| 144 | 143 | ||||
| 145 | func withinGenerationBudget(manifestSize int, refs map[ports.SnapshotDigest]codec.ObjectRef) bool { | 144 | func withinGenerationBudget(manifestSize int, refs map[ports.SnapshotDigest]codec.ObjectRef) bool { | ||
M internal/adapters/snapshot/repository_hardening_test.go +1 -1
| @@ -75,7 +75,7 @@ func TestValidatePublicationRejectsUnloadableAggregate(t *testing.T) { | |||
| 75 | 75 | t.Fatal(err) | |
| 76 | 76 | } | |
| 77 | 77 | pub.Objects = nil // validation permits omitted objects for immutable reuse. | |
| 78 | - | if _, _, _, err := validatePublication(pub); err == nil { | |
| 78 | + | if _, _, err := validatePublication(pub); err == nil { | |
| 79 | 79 | t.Fatal("validatePublication accepted unloadable aggregate") | |
| 80 | 80 | } else if errors.Is(err, os.ErrNotExist) { | |
| 81 | 81 | t.Fatalf("validatePublication returned unrelated error: %v", err) | |
| @@ -75,7 +75,7 @@ func TestValidatePublicationRejectsUnloadableAggregate(t *testing.T) { | |||||
| 75 | t.Fatal(err) | 75 | t.Fatal(err) | ||
| 76 | } | 76 | } | ||
| 77 | pub.Objects = nil // validation permits omitted objects for immutable reuse. | 77 | pub.Objects = nil // validation permits omitted objects for immutable reuse. | ||
| 78 | - | if _, _, _, err := validatePublication(pub); err == nil { | 78 | + | if _, _, err := validatePublication(pub); err == nil { |
| 79 | t.Fatal("validatePublication accepted unloadable aggregate") | 79 | t.Fatal("validatePublication accepted unloadable aggregate") | ||
| 80 | } else if errors.Is(err, os.ErrNotExist) { | 80 | } else if errors.Is(err, os.ErrNotExist) { | ||
| 81 | t.Fatalf("validatePublication returned unrelated error: %v", err) | 81 | t.Fatalf("validatePublication returned unrelated error: %v", err) | ||