diff --git a/docs/internal/bundle.md b/docs/internal/bundle.md index dc4d5b5..fe9a52c 100644 --- a/docs/internal/bundle.md +++ b/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`. diff --git a/internal/bundle/manifest.go b/internal/bundle/manifest.go index f3522b8..b0a70df 100644 --- a/internal/bundle/manifest.go +++ b/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, diff --git a/internal/bundle/manifest_test.go b/internal/bundle/manifest_test.go index 76e1541..16d8fa9 100644 --- a/internal/bundle/manifest_test.go +++ b/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) diff --git a/internal/bundle/validate.go b/internal/bundle/validate.go index 63b6741..8d6976a 100644 --- a/internal/bundle/validate.go +++ b/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) } diff --git a/internal/bundle/validate_test.go b/internal/bundle/validate_test.go index db4ffdb..1454aec 100644 --- a/internal/bundle/validate_test.go +++ b/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) - writeFakeFile(t, backend, "manifest.json", manifest) + manifest := validFixtureManifest(t) + 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}) diff --git a/internal/state/distributor_test.go b/internal/state/distributor_test.go index b55a1a1..68fecfb 100644 --- a/internal/state/distributor_test.go +++ b/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){ diff --git a/internal/state/validate.go b/internal/state/validate.go index e14292b..cff038d 100644 --- a/internal/state/validate.go +++ b/internal/state/validate.go @@ -45,41 +45,7 @@ func Validate(s DistributorState) error { } func validateEmbeddedManifest(manifest bundle.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 := 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 + return bundle.ValidateManifest(manifest) } func validateOutput(index int, output OutputFile) error {