From 0eec4a30ed63af1470419ffbdb766374a4e3d3df Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Tue, 6 Oct 2026 17:29:41 +0000 Subject: [PATCH] Consolidate artifact declarations --- cmd/releasekit/internal/cli/receipt.go | 41 ++++++++--------- docs/internal/overview.md | 1 + docs/roadmap/implementation.md | 10 ++++- internal/artifact/artifact.go | 62 ++++++++++++++++++++++++++ internal/artifact/artifact_test.go | 42 +++++++++++++++++ internal/build/build.go | 44 ++++-------------- internal/delivery/assets.go | 30 ++++++------- internal/delivery/assets_test.go | 17 +++++++ internal/gitea/assets.go | 4 +- 9 files changed, 175 insertions(+), 76 deletions(-) create mode 100644 internal/artifact/artifact.go create mode 100644 internal/artifact/artifact_test.go diff --git a/cmd/releasekit/internal/cli/receipt.go b/cmd/releasekit/internal/cli/receipt.go index d0fa759..bb074fe 100644 --- a/cmd/releasekit/internal/cli/receipt.go +++ b/cmd/releasekit/internal/cli/receipt.go @@ -11,6 +11,7 @@ import ( "path/filepath" "strings" + "gitea.maximumdirect.net/eric/ReleaseKit/internal/artifact" "gitea.maximumdirect.net/eric/ReleaseKit/internal/versioncontract" ) @@ -149,7 +150,7 @@ func validateBuildReceipt(receipt BuildReceipt, verifyFiles bool) error { if !validCommitID(receipt.CandidateCommit) { return errors.New("build receipt has invalid candidate commit") } - if receipt.MainPackage == "" || !safeReceiptComponent(receipt.BinaryName) || receipt.Compiler == "" { + if receipt.MainPackage == "" || !artifact.ValidComponent(receipt.BinaryName) || receipt.Compiler == "" { return errors.New("build receipt is missing build identity") } if len(receipt.Targets) == 0 || len(receipt.Artifacts) != len(receipt.Targets) { @@ -157,21 +158,29 @@ func validateBuildReceipt(receipt BuildReceipt, verifyFiles bool) error { } seenTargets := make(map[ReceiptTarget]bool, len(receipt.Targets)) seenArtifacts := make(map[ReceiptTarget]bool, len(receipt.Artifacts)) + targets := make([]artifact.Target, 0, len(receipt.Targets)) for _, target := range receipt.Targets { - if !safeReceiptComponent(target.OS) || !safeReceiptComponent(target.Arch) { + if !artifact.ValidComponent(target.OS) || !artifact.ValidComponent(target.Arch) { return fmt.Errorf("build receipt has unsafe target %q/%q", target.OS, target.Arch) } if seenTargets[target] { return fmt.Errorf("build receipt repeats target %s/%s", target.OS, target.Arch) } seenTargets[target] = true + targets = append(targets, artifact.Target{OS: target.OS, Arch: target.Arch}) + } + declarations := artifact.ExpectedBuild(receipt.BinaryName, receipt.Version, targets) + wantByTarget := make(map[ReceiptTarget]string, len(declarations)) + for _, declaration := range declarations { + target := ReceiptTarget{OS: declaration.Target.OS, Arch: declaration.Target.Arch} + wantByTarget[target] = declaration.Filename } for _, artifact := range receipt.Artifacts { if !seenTargets[artifact.Target] || seenArtifacts[artifact.Target] { return fmt.Errorf("build receipt has an unexpected artifact target %s/%s", artifact.Target.OS, artifact.Target.Arch) } seenArtifacts[artifact.Target] = true - wantName := receiptArtifactFilename(receipt.BinaryName, receipt.Version, artifact.Target) + wantName := wantByTarget[artifact.Target] if artifact.Filename != wantName || filepath.Base(artifact.Filename) != artifact.Filename { return fmt.Errorf("build receipt has invalid artifact filename %q", artifact.Filename) } @@ -215,34 +224,22 @@ func verifyReceiptArtifact(artifact ReceiptArtifact) error { } func normalizeReceiptTargets(targets []string) []ReceiptTarget { - seen := make(map[ReceiptTarget]bool, len(targets)) - result := make([]ReceiptTarget, 0, len(targets)) + parsed := make([]artifact.Target, 0, len(targets)) for _, value := range targets { goos, goarch, ok := strings.Cut(value, "/") if !ok { continue } - target := ReceiptTarget{OS: goos, Arch: goarch} - if !seen[target] { - seen[target] = true - result = append(result, target) - } + parsed = append(parsed, artifact.Target{OS: goos, Arch: goarch}) + } + normalized := artifact.NormalizeTargets(parsed) + result := make([]ReceiptTarget, 0, len(normalized)) + for _, target := range normalized { + result = append(result, ReceiptTarget{OS: target.OS, Arch: target.Arch}) } return result } -func receiptArtifactFilename(binaryName, version string, target ReceiptTarget) string { - filename := fmt.Sprintf("%s-%s-%s-%s", binaryName, version, target.OS, target.Arch) - if target.OS == "windows" { - filename += ".exe" - } - return filename -} - -func safeReceiptComponent(value string) bool { - return value != "" && !strings.ContainsAny(value, "/\\\r\n") -} - func validCommitID(value string) bool { if len(value) != 40 && len(value) != 64 { return false diff --git a/docs/internal/overview.md b/docs/internal/overview.md index ec39cc3..b1f290e 100644 --- a/docs/internal/overview.md +++ b/docs/internal/overview.md @@ -35,6 +35,7 @@ these implemented packages: | `cmd/releasekit` | Executable entry point, command routing, output, and exit status. | | `cmd/releasekit/internal/cli` | Command parsing, strict JSON profile loading, configured checks, and private build receipts. | | `internal/apicheck` | Compiled-export and import-boundary checks run during development. | +| `internal/artifact` | Shared build-target normalization, artifact declarations, filename rules, and checksum-manifest naming. | | `internal/build` | Candidate-build coordination, selected Go target compilation, version injection, native smoke check, and artifact digests. | | `internal/candidate` | Stable-version, note, repository-state, candidate capture, and post-validation checks. | | `internal/delivery` | Tagged-checkout verification, artifact inventory/hash validation, manifest assembly, and coordination of hosted delivery results. | diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index c40a2ae..83b3912 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -1,6 +1,6 @@ # ReleaseKit audit implementation plan -Status: Stages 1–3 complete; Stage 4 not started. +Status: Stages 1–4 complete; Stage 5 not started. Baseline: `e4be7fa` (2026-10-06). The [audit](audit.md) records the evidence, product goals, and selected direction. This document owns implementation order and completion criteria for that work. @@ -236,6 +236,14 @@ and manifests. Add only missing coverage for disagreement between these boundaries. The API/dependency checker confirms that the shared internal owner has not leaked through exported signatures or runtime dependencies. +**Completed:** `internal/artifact` now owns target normalization, filename +components, binary declarations, and checksum-manifest naming. The builder, +receipt adapter, and delivery validation share those rules while retaining +their build, receipt-path/file, and hosted-content responsibilities. Existing +coverage plus a mismatched delivery declaration check preserve cross-boundary +behavior. Focused tests, the full offline suite, API boundary checks, and +documentation whitespace checks pass. + ## Stage 5 — Make delivery outcomes disjoint and actionable **Covers:** F7. **Primary owners:** `internal/gitea/upload.go`, delivery result diff --git a/internal/artifact/artifact.go b/internal/artifact/artifact.go new file mode 100644 index 0000000..92bf1bd --- /dev/null +++ b/internal/artifact/artifact.go @@ -0,0 +1,62 @@ +// Package artifact owns shared build artifact identity rules. +package artifact + +import ( + "fmt" + "strings" +) + +const ManifestFilename = "SHA256SUMS" + +type Target struct { + OS string + Arch string +} + +type Declaration struct { + Target Target + Filename string +} + +// NormalizeTargets removes duplicate targets while preserving their first +// occurrence. +func NormalizeTargets(targets []Target) []Target { + seen := make(map[Target]struct{}, len(targets)) + result := make([]Target, 0, len(targets)) + for _, target := range targets { + if _, ok := seen[target]; ok { + continue + } + seen[target] = struct{}{} + result = append(result, target) + } + return result +} + +// ValidComponent reports whether value is a nonempty filename component under +// the build and receipt contracts. +func ValidComponent(value string) bool { + return value != "" && !strings.ContainsAny(value, "/\\\r\n") +} + +// Filename returns the declared binary filename for one target. +func Filename(binaryName, version string, target Target) string { + filename := fmt.Sprintf("%s-%s-%s-%s", binaryName, version, target.OS, target.Arch) + if target.OS == "windows" { + filename += ".exe" + } + return filename +} + +// ExpectedBuild returns artifact declarations in target order. Callers retain +// responsibility for validating their boundary-specific build identity. +func ExpectedBuild(binaryName, version string, targets []Target) []Declaration { + declarations := make([]Declaration, 0, len(targets)) + for _, target := range targets { + declarations = append(declarations, Declaration{ + Target: target, + Filename: Filename(binaryName, version, target), + }) + } + return declarations +} diff --git a/internal/artifact/artifact_test.go b/internal/artifact/artifact_test.go new file mode 100644 index 0000000..3477dbb --- /dev/null +++ b/internal/artifact/artifact_test.go @@ -0,0 +1,42 @@ +package artifact + +import ( + "reflect" + "testing" +) + +func TestNormalizeTargetsPreservesOrderAndFirstOccurrence(t *testing.T) { + targets := []Target{ + {OS: "linux", Arch: "amd64"}, + {OS: "windows", Arch: "arm64"}, + {OS: "linux", Arch: "amd64"}, + } + want := []Target{{OS: "linux", Arch: "amd64"}, {OS: "windows", Arch: "arm64"}} + if got := NormalizeTargets(targets); !reflect.DeepEqual(got, want) { + t.Fatalf("NormalizeTargets() = %#v, want %#v", got, want) + } +} + +func TestExpectedBuildUsesSharedNamesInTargetOrder(t *testing.T) { + targets := []Target{{OS: "windows", Arch: "arm64"}, {OS: "linux", Arch: "amd64"}} + want := []Declaration{ + {Target: targets[0], Filename: "app-v1.2.3-windows-arm64.exe"}, + {Target: targets[1], Filename: "app-v1.2.3-linux-amd64"}, + } + if got := ExpectedBuild("app", "v1.2.3", targets); !reflect.DeepEqual(got, want) { + t.Fatalf("ExpectedBuild() = %#v, want %#v", got, want) + } +} + +func TestValidComponentPreservesBuildAndReceiptRules(t *testing.T) { + for _, value := range []string{"", "path/name", `path\name`, "line\nbreak", "line\rbreak"} { + if ValidComponent(value) { + t.Errorf("ValidComponent(%q) = true, want false", value) + } + } + for _, value := range []string{"app", "..", "nul\x00byte"} { + if !ValidComponent(value) { + t.Errorf("ValidComponent(%q) = false, want true", value) + } + } +} diff --git a/internal/build/build.go b/internal/build/build.go index fc0d1da..419e936 100644 --- a/internal/build/build.go +++ b/internal/build/build.go @@ -13,15 +13,13 @@ import ( "path/filepath" "strings" + "gitea.maximumdirect.net/eric/ReleaseKit/internal/artifact" "gitea.maximumdirect.net/eric/ReleaseKit/internal/candidate" "gitea.maximumdirect.net/eric/ReleaseKit/internal/gotool" "gitea.maximumdirect.net/eric/ReleaseKit/internal/versioncontract" ) -type Target struct { - OS string - Arch string -} +type Target = artifact.Target type Spec struct { MainPackage string @@ -62,7 +60,7 @@ func Build(ctx context.Context, request Request) (Result, error) { Version: request.Version, MainPackage: request.Spec.MainPackage, BinaryName: request.Spec.BinaryName, - Targets: normalizeTargets(request.Spec.Targets), + Targets: artifact.NormalizeTargets(request.Spec.Targets), } if !versioncontract.ValidStableVersion(request.Version) { return result, fmt.Errorf("%w: %q", candidate.ErrInvalidVersion, request.Version) @@ -74,11 +72,11 @@ func Build(ctx context.Context, request Request) (Result, error) { if request.Spec.MainPackage == "" { return result, errors.New("build requires a main package when targets are selected") } - if !safeComponent(request.Spec.BinaryName) { + if !artifact.ValidComponent(request.Spec.BinaryName) { return result, fmt.Errorf("unsafe binary name %q", request.Spec.BinaryName) } for _, target := range result.Targets { - if !safeComponent(target.OS) || !safeComponent(target.Arch) { + if !artifact.ValidComponent(target.OS) || !artifact.ValidComponent(target.Arch) { return result, fmt.Errorf("unsafe target %q/%q", target.OS, target.Arch) } } @@ -107,9 +105,10 @@ func Build(ctx context.Context, request Request) (Result, error) { } result.OutputDir = resolvedOutputDir(repositoryDir, request.OutputDir) - filenames := make([]string, 0, len(result.Targets)) - for _, target := range result.Targets { - filenames = append(filenames, artifactFilename(request.Spec.BinaryName, request.Version, target)) + declarations := artifact.ExpectedBuild(request.Spec.BinaryName, request.Version, result.Targets) + filenames := make([]string, len(declarations)) + for i, declaration := range declarations { + filenames[i] = declaration.Filename } if err := os.MkdirAll(result.OutputDir, 0o755); err != nil { return result, fmt.Errorf("create build output directory %q: %w", result.OutputDir, err) @@ -188,23 +187,6 @@ func Build(ctx context.Context, request Request) (Result, error) { return result, nil } -func normalizeTargets(targets []Target) []Target { - seen := make(map[Target]struct{}, len(targets)) - result := make([]Target, 0, len(targets)) - for _, target := range targets { - if _, ok := seen[target]; ok { - continue - } - seen[target] = struct{}{} - result = append(result, target) - } - return result -} - -func safeComponent(value string) bool { - return value != "" && !strings.ContainsAny(value, "/\\\r\n") -} - func resolvedOutputDir(repositoryDir, outputDir string) string { if outputDir == "" { outputDir = filepath.Join(repositoryDir, "dist") @@ -218,14 +200,6 @@ func resolvedOutputDir(repositoryDir, outputDir string) string { return resolved } -func artifactFilename(binaryName, version string, target Target) string { - filename := fmt.Sprintf("%s-%s-%s-%s", binaryName, version, target.OS, target.Arch) - if target.OS == "windows" { - filename += ".exe" - } - return filename -} - func gitCommit(ctx context.Context, repositoryDir string) (string, error) { command := exec.CommandContext(ctx, "git", "-C", repositoryDir, "rev-parse", "--verify", "HEAD^{commit}") output, err := command.Output() diff --git a/internal/delivery/assets.go b/internal/delivery/assets.go index b04708d..91d201e 100644 --- a/internal/delivery/assets.go +++ b/internal/delivery/assets.go @@ -7,8 +7,8 @@ import ( "io" "os" "path/filepath" - "strings" + "gitea.maximumdirect.net/eric/ReleaseKit/internal/artifact" internalbuild "gitea.maximumdirect.net/eric/ReleaseKit/internal/build" "gitea.maximumdirect.net/eric/ReleaseKit/internal/gitea" ) @@ -23,30 +23,26 @@ func declaredBuildAssets(result internalbuild.Result) ([]gitea.ExpectedAsset, er if len(result.Artifacts) != len(result.Targets) { return nil, fmt.Errorf("build result must contain one artifact for each selected target") } - wantByTarget := make(map[string]string, len(result.Targets)) - for _, target := range result.Targets { - if target.OS == "" || target.Arch == "" || strings.ContainsAny(target.OS+target.Arch, "/\\\r\n") { + declarations := artifact.ExpectedBuild(result.BinaryName, result.Version, result.Targets) + wantByTarget := make(map[artifact.Target]string, len(declarations)) + for _, declaration := range declarations { + target := declaration.Target + if !artifact.ValidComponent(target.OS) || !artifact.ValidComponent(target.Arch) { return nil, fmt.Errorf("build result contains an invalid target") } - key := target.OS + "/" + target.Arch - if _, exists := wantByTarget[key]; exists { - return nil, fmt.Errorf("build result repeats target %q", key) + if _, exists := wantByTarget[target]; exists { + return nil, fmt.Errorf("build result repeats target %q", target.OS+"/"+target.Arch) } - name := result.BinaryName + "-" + result.Version + "-" + target.OS + "-" + target.Arch - if target.OS == "windows" { - name += ".exe" - } - wantByTarget[key] = name + wantByTarget[target] = declaration.Filename } assets := make([]gitea.ExpectedAsset, 0, len(result.Artifacts)+1) - seenTargets := make(map[string]bool, len(result.Artifacts)) + seenTargets := make(map[artifact.Target]bool, len(result.Artifacts)) for _, artifact := range result.Artifacts { - key := artifact.Target.OS + "/" + artifact.Target.Arch - wantName, ok := wantByTarget[key] - if !ok || seenTargets[key] || artifact.Filename != wantName { + wantName, ok := wantByTarget[artifact.Target] + if !ok || seenTargets[artifact.Target] || artifact.Filename != wantName { return nil, fmt.Errorf("build artifact %q does not match a selected target and its declared name", artifact.Filename) } - seenTargets[key] = true + seenTargets[artifact.Target] = true if filepath.Base(artifact.Path) != artifact.Filename { return nil, fmt.Errorf("build artifact path %q does not end with filename %q", artifact.Path, artifact.Filename) } diff --git a/internal/delivery/assets_test.go b/internal/delivery/assets_test.go index 0bf0c86..96ca04c 100644 --- a/internal/delivery/assets_test.go +++ b/internal/delivery/assets_test.go @@ -61,6 +61,23 @@ func TestDeclaredBuildAssetsRejectsTamperedArtifact(t *testing.T) { } } +func TestDeclaredBuildAssetsRejectsArtifactDeclarationMismatch(t *testing.T) { + path := filepath.Join(t.TempDir(), "app-v1.2.3-linux-arm64") + contents := []byte("built application") + if err := os.WriteFile(path, contents, 0o600); err != nil { + t.Fatal(err) + } + _, err := declaredBuildAssets(internalbuild.Result{ + Version: "v1.2.3", BinaryName: "app", Targets: []internalbuild.Target{{OS: "linux", Arch: "amd64"}}, + Artifacts: []internalbuild.Artifact{{ + Path: path, Filename: filepath.Base(path), Target: internalbuild.Target{OS: "linux", Arch: "amd64"}, SHA256: assetDigest(contents), + }}, + }) + if err == nil || !strings.Contains(err.Error(), "does not match a selected target") { + t.Fatalf("declaredBuildAssets() error = %v, want declaration mismatch", err) + } +} + func TestDeclaredBuildAssetsKeepsSourceOnlyBuildManifestFree(t *testing.T) { assets, err := declaredBuildAssets(internalbuild.Result{}) if err != nil || len(assets) != 0 { diff --git a/internal/gitea/assets.go b/internal/gitea/assets.go index f4c1e1c..db95276 100644 --- a/internal/gitea/assets.go +++ b/internal/gitea/assets.go @@ -13,9 +13,11 @@ import ( "sort" "strconv" "strings" + + "gitea.maximumdirect.net/eric/ReleaseKit/internal/artifact" ) -const ManifestFilename = "SHA256SUMS" +const ManifestFilename = artifact.ManifestFilename type AssetState string