Centralize source manifest validation

docs/internal/bundle.md

@@ -22,7 +22,9 @@ Each file requires `path`, `sha256`, and `size`. Digests must use lowercase `sha
 
 ## Validation
 
-Bundle validation checks source path safety, duplicate file paths, reserved paths, file existence, regular-file type, file size, per-file SHA-256, and the top-level bundle digest.
+`ValidateManifest` owns normalized source manifest semantics: schema version, id, digest format, timestamp presence, file list presence, source path safety, duplicate file paths, reserved paths, file digest format, non-negative file sizes, and the top-level bundle digest.
+
+Storage-backed bundle validation additionally checks file existence, regular-file type, file size, and per-file SHA-256.
 
 The bundle digest is SHA-256 of a deterministic JSON array of file records in manifest order with fields `path`, `sha256`, and `size`.
 

internal/bundle/manifest.go

@@ -59,9 +59,6 @@ func ParseManifest(data []byte) (Manifest, error) {
 		return Manifest{}, fmt.Errorf("manifest schema_version is required")
 	}
 	manifest.SchemaVersion = *raw.SchemaVersion
-	if manifest.SchemaVersion != 1 {
-		return Manifest{}, fmt.Errorf("manifest schema_version must be 1")
-	}
 	if raw.ID == nil || *raw.ID == "" {
 		return Manifest{}, fmt.Errorf("manifest id is required")
 	}
@@ -69,9 +66,6 @@ func ParseManifest(data []byte) (Manifest, error) {
 	if raw.Digest == nil || *raw.Digest == "" {
 		return Manifest{}, fmt.Errorf("manifest digest is required")
 	}
-	if err := ValidateDigest(*raw.Digest); err != nil {
-		return Manifest{}, fmt.Errorf("manifest digest: %w", err)
-	}
 	manifest.Digest = *raw.Digest
 	if raw.Created == nil || *raw.Created == "" {
 		return Manifest{}, fmt.Errorf("manifest created is required")
@@ -85,18 +79,16 @@ func ParseManifest(data []byte) (Manifest, error) {
 		return Manifest{}, fmt.Errorf("manifest files is required")
 	}
 
-	seen := make(map[string]struct{}, len(raw.Files))
 	for index, rawFile := range raw.Files {
 		file, err := parseManifestFile(index, rawFile)
 		if err != nil {
 			return Manifest{}, err
 		}
-		if _, exists := seen[file.Path]; exists {
-			return Manifest{}, fmt.Errorf("manifest files[%d].path duplicates %q", index, file.Path)
-		}
-		seen[file.Path] = struct{}{}
 		manifest.Files = append(manifest.Files, file)
 	}
+	if err := ValidateManifest(manifest); err != nil {
+		return Manifest{}, fmt.Errorf("manifest %w", err)
+	}
 	return manifest, nil
 }
 
@@ -104,21 +96,12 @@ func parseManifestFile(index int, raw rawManifestFile) (ManifestFile, error) {
 	if raw.Path == nil || *raw.Path == "" {
 		return ManifestFile{}, fmt.Errorf("manifest files[%d].path is required", index)
 	}
-	if err := ValidateSourcePath(*raw.Path); err != nil {
-		return ManifestFile{}, fmt.Errorf("manifest files[%d].path: %w", index, err)
-	}
 	if raw.SHA256 == nil || *raw.SHA256 == "" {
 		return ManifestFile{}, fmt.Errorf("manifest files[%d].sha256 is required", index)
 	}
-	if err := ValidateDigest(*raw.SHA256); err != nil {
-		return ManifestFile{}, fmt.Errorf("manifest files[%d].sha256: %w", index, err)
-	}
 	if raw.Size == nil {
 		return ManifestFile{}, fmt.Errorf("manifest files[%d].size is required", index)
 	}
-	if *raw.Size < 0 {
-		return ManifestFile{}, fmt.Errorf("manifest files[%d].size must be non-negative", index)
-	}
 	return ManifestFile{
 		Path:   *raw.Path,
 		SHA256: *raw.SHA256,

internal/bundle/manifest_test.go

@@ -4,6 +4,7 @@ import (
 	"os"
 	"strings"
 	"testing"
+	"time"
 
 	"gitea.maximumdirect.net/eric/distributor/internal/storage"
 )
@@ -92,6 +93,75 @@ func TestParseManifestRejectsDuplicatePaths(t *testing.T) {
 	assertErrorContains(t, err, "duplicates")
 }
 
+func TestValidateManifestAcceptsValidFixture(t *testing.T) {
+	manifest := validFixtureManifest(t)
+	if err := ValidateManifest(manifest); err != nil {
+		t.Fatalf("ValidateManifest() error = %v", err)
+	}
+}
+
+func TestValidateManifestRejectsInvalidManifest(t *testing.T) {
+	tests := map[string]func(Manifest) Manifest{
+		"schema version": func(manifest Manifest) Manifest {
+			manifest.SchemaVersion = 2
+			return manifest
+		},
+		"empty id": func(manifest Manifest) Manifest {
+			manifest.ID = ""
+			return manifest
+		},
+		"bad digest": func(manifest Manifest) Manifest {
+			manifest.Digest = "SHA256:099b205780d2b050024868399961b05731729a548d5d6329c7b06a6740dd75fe"
+			return manifest
+		},
+		"zero created": func(manifest Manifest) Manifest {
+			manifest.Created = time.Time{}
+			return manifest
+		},
+		"empty files": func(manifest Manifest) Manifest {
+			manifest.Files = nil
+			manifest.Digest = BundleDigest(manifest.Files)
+			return manifest
+		},
+		"unsafe path": func(manifest Manifest) Manifest {
+			manifest.Files[0].Path = "../report.md"
+			manifest.Digest = BundleDigest(manifest.Files)
+			return manifest
+		},
+		"duplicate path": func(manifest Manifest) Manifest {
+			manifest.Files[1].Path = manifest.Files[0].Path
+			manifest.Digest = BundleDigest(manifest.Files)
+			return manifest
+		},
+		"negative size": func(manifest Manifest) Manifest {
+			manifest.Files[0].Size = -1
+			manifest.Digest = BundleDigest(manifest.Files)
+			return manifest
+		},
+		"digest mismatch": func(manifest Manifest) Manifest {
+			manifest.Digest = "sha256:0000000000000000000000000000000000000000000000000000000000000000"
+			return manifest
+		},
+	}
+	for name, mutate := range tests {
+		t.Run(name, func(t *testing.T) {
+			err := ValidateManifest(mutate(validFixtureManifest(t)))
+			if err == nil {
+				t.Fatal("ValidateManifest() error = nil, want error")
+			}
+		})
+	}
+}
+
+func validFixtureManifest(t *testing.T) Manifest {
+	t.Helper()
+	manifest, err := ParseManifest(readFixture(t, "testdata/valid_bundle/manifest.json"))
+	if err != nil {
+		t.Fatalf("ParseManifest() error = %v", err)
+	}
+	return manifest
+}
+
 func readFixture(t *testing.T, path string) []byte {
 	t.Helper()
 	data, err := os.ReadFile(path)

internal/bundle/validate.go

@@ -18,6 +18,44 @@ func ValidateSourcePath(path string) error {
 	return nil
 }
 
+func ValidateManifest(manifest Manifest) error {
+	if manifest.SchemaVersion != 1 {
+		return fmt.Errorf("schema_version must be 1")
+	}
+	if manifest.ID == "" {
+		return fmt.Errorf("id is required")
+	}
+	if err := ValidateDigest(manifest.Digest); err != nil {
+		return fmt.Errorf("digest: %w", err)
+	}
+	if manifest.Created.IsZero() {
+		return fmt.Errorf("created is required")
+	}
+	if len(manifest.Files) == 0 {
+		return fmt.Errorf("files is required")
+	}
+	seen := make(map[string]struct{}, len(manifest.Files))
+	for index, file := range manifest.Files {
+		if err := ValidateSourcePath(file.Path); err != nil {
+			return fmt.Errorf("files[%d].path: %w", index, err)
+		}
+		if err := ValidateDigest(file.SHA256); err != nil {
+			return fmt.Errorf("files[%d].sha256: %w", index, err)
+		}
+		if file.Size < 0 {
+			return fmt.Errorf("files[%d].size must be non-negative", index)
+		}
+		if _, exists := seen[file.Path]; exists {
+			return fmt.Errorf("files[%d].path duplicates %q", index, file.Path)
+		}
+		seen[file.Path] = struct{}{}
+	}
+	if actual := BundleDigest(manifest.Files); actual != manifest.Digest {
+		return fmt.Errorf("digest mismatch: got %s want %s", actual, manifest.Digest)
+	}
+	return nil
+}
+
 func Validate(ctx context.Context, backend storage.Backend, bundleRoot string) (Bundle, error) {
 	return validateAt(ctx, backend, bundleRoot, bundleRoot)
 }

internal/bundle/validate_test.go

@@ -2,6 +2,7 @@ package bundle
 
 import (
 	"context"
+	"encoding/json"
 	"strings"
 	"testing"
 
@@ -32,8 +33,10 @@ func TestValidateRejectsMissingFile(t *testing.T) {
 
 func TestValidateRejectsSizeMismatch(t *testing.T) {
 	backend := validFakeBundle(t)
-	manifest := strings.Replace(string(readFixture(t, "testdata/valid_bundle/manifest.json")), `"size": 8`, `"size": 9`, 1)
+	manifest := validFixtureManifest(t)
-	writeFakeFile(t, backend, "manifest.json", manifest)
+	manifest.Files[1].Size = 9
+	manifest.Digest = BundleDigest(manifest.Files)
+	writeManifest(t, backend, manifest)
 	_, err := Validate(context.Background(), backend, "")
 	assertErrorContains(t, err, "size mismatch")
 }
@@ -79,6 +82,15 @@ func writeFakeFile(t *testing.T, backend *fake.Backend, path, data string) {
 	}
 }
 
+func writeManifest(t *testing.T, backend *fake.Backend, manifest Manifest) {
+	t.Helper()
+	data, err := json.MarshalIndent(manifest, "", "  ")
+	if err != nil {
+		t.Fatalf("MarshalIndent() error = %v", err)
+	}
+	writeFakeFile(t, backend, ManifestName, string(append(data, '\n')))
+}
+
 func deleteFakeFile(t *testing.T, backend *fake.Backend, path string) {
 	t.Helper()
 	err := backend.DeleteManagedBundle(context.Background(), "", []string{path}, storage.DeleteOptions{IgnoreMissing: true})

internal/state/distributor_test.go

@@ -70,6 +70,60 @@ func TestParseRejectsInvalidEmbeddedManifest(t *testing.T) {
 	assertStateErrorContains(t, err, "source.manifest")
 }
 
+func TestValidateRejectsInvalidEmbeddedManifest(t *testing.T) {
+	tests := map[string]func(bundle.Manifest) bundle.Manifest{
+		"schema version": func(manifest bundle.Manifest) bundle.Manifest {
+			manifest.SchemaVersion = 2
+			return manifest
+		},
+		"empty id": func(manifest bundle.Manifest) bundle.Manifest {
+			manifest.ID = ""
+			return manifest
+		},
+		"bad digest": func(manifest bundle.Manifest) bundle.Manifest {
+			manifest.Digest = "SHA256:099b205780d2b050024868399961b05731729a548d5d6329c7b06a6740dd75fe"
+			return manifest
+		},
+		"zero created": func(manifest bundle.Manifest) bundle.Manifest {
+			manifest.Created = time.Time{}
+			return manifest
+		},
+		"empty files": func(manifest bundle.Manifest) bundle.Manifest {
+			manifest.Files = nil
+			manifest.Digest = bundle.BundleDigest(manifest.Files)
+			return manifest
+		},
+		"unsafe path": func(manifest bundle.Manifest) bundle.Manifest {
+			manifest.Files[0].Path = "../report.md"
+			manifest.Digest = bundle.BundleDigest(manifest.Files)
+			return manifest
+		},
+		"duplicate path": func(manifest bundle.Manifest) bundle.Manifest {
+			manifest.Files[1].Path = manifest.Files[0].Path
+			manifest.Digest = bundle.BundleDigest(manifest.Files)
+			return manifest
+		},
+		"negative size": func(manifest bundle.Manifest) bundle.Manifest {
+			manifest.Files[0].Size = -1
+			manifest.Digest = bundle.BundleDigest(manifest.Files)
+			return manifest
+		},
+		"digest mismatch": func(manifest bundle.Manifest) bundle.Manifest {
+			manifest.Digest = "sha256:0000000000000000000000000000000000000000000000000000000000000000"
+			return manifest
+		},
+	}
+	for name, mutate := range tests {
+		t.Run(name, func(t *testing.T) {
+			source := mutate(validManifest(t))
+			state := *withState(t, validManifest(t), func(*DistributorState) {})
+			state.Source.Manifest = source
+			err := Validate(state)
+			assertStateErrorContains(t, err, "source.manifest")
+		})
+	}
+}
+
 func TestParseRejectsInvalidOutputMetadata(t *testing.T) {
 	source := validManifest(t)
 	tests := map[string]func(*DistributorState){

internal/state/validate.go

@@ -45,41 +45,7 @@ func Validate(s DistributorState) error {
 }
 
 func validateEmbeddedManifest(manifest bundle.Manifest) error {
-	if manifest.SchemaVersion != 1 {
+	return bundle.ValidateManifest(manifest)
-		return fmt.Errorf("schema_version must be 1")
-	}
-	if manifest.ID == "" {
-		return fmt.Errorf("id is required")
-	}
-	if err := bundle.ValidateDigest(manifest.Digest); err != nil {
-		return fmt.Errorf("digest: %w", err)
-	}
-	if manifest.Created.IsZero() {
-		return fmt.Errorf("created is required")
-	}
-	if len(manifest.Files) == 0 {
-		return fmt.Errorf("files is required")
-	}
-	seen := make(map[string]struct{}, len(manifest.Files))
-	for index, file := range manifest.Files {
-		if err := bundle.ValidateSourcePath(file.Path); err != nil {
-			return fmt.Errorf("files[%d].path: %w", index, err)
-		}
-		if err := bundle.ValidateDigest(file.SHA256); err != nil {
-			return fmt.Errorf("files[%d].sha256: %w", index, err)
-		}
-		if file.Size < 0 {
-			return fmt.Errorf("files[%d].size must be non-negative", index)
-		}
-		if _, exists := seen[file.Path]; exists {
-			return fmt.Errorf("files[%d].path duplicates %q", index, file.Path)
-		}
-		seen[file.Path] = struct{}{}
-	}
-	if actual := bundle.BundleDigest(manifest.Files); actual != manifest.Digest {
-		return fmt.Errorf("digest mismatch: got %s want %s", actual, manifest.Digest)
-	}
-	return nil
 }
 
 func validateOutput(index int, output OutputFile) error {