diff --git a/cmd/releasekit/destination_test.go b/cmd/releasekit/destination_test.go new file mode 100644 index 0000000..f155bb4 --- /dev/null +++ b/cmd/releasekit/destination_test.go @@ -0,0 +1,64 @@ +package main + +import ( + "bytes" + "context" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "testing" + + "go.maximumdirect.net/releasekit/cmd/releasekit/internal/cli" +) + +func TestInvalidDeliveryDestinationStopsBeforePreparation(t *testing.T) { + for _, command := range []string{"deliver", "ci-deliver"} { + for _, setting := range []string{"URL", "owner", "repository"} { + t.Run(command+"/"+setting, func(t *testing.T) { + repository := newDeliveryBuildRepository(t) + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + t.Errorf("invalid destination made a hosted request: %s", r.URL.Path) + http.Error(w, "unexpected request", http.StatusInternalServerError) + })) + defer server.Close() + destination := &cli.DeliverySettings{Forge: "gitea", BaseURL: server.URL, Owner: "owner", Repository: "example", TokenEnv: "RELEASEKIT_TEST_DELIVERY_TOKEN"} + switch setting { + case "URL": + destination.BaseURL = "ssh://forge.example" + case "owner": + destination.Owner = "owner/child" + case "repository": + destination.Repository = "repo/child" + } + writeDeliveryProfile(t, repository, cli.Settings{ + Product: "Example Product", Delivery: destination, + Build: &cli.BuildSettings{MainPackage: ".", BinaryName: "helper", Targets: []string{"linux/amd64"}}, + }) + commit := pushTaggedCLIRepository(t, repository) + t.Setenv("RELEASEKIT_TEST_DELIVERY_TOKEN", "fixture-token") + shimDir := t.TempDir() + marker := filepath.Join(shimDir, "go-invoked") + if err := os.WriteFile(filepath.Join(shimDir, "go"), []byte("#!/bin/sh\n: > \"$RELEASEKIT_GO_MARKER\"\nexit 99\n"), 0o700); err != nil { + t.Fatal(err) + } + t.Setenv("RELEASEKIT_GO_MARKER", marker) + t.Setenv("PATH", shimDir+string(os.PathListSeparator)+os.Getenv("PATH")) + args := []string{command, "--repo", repository} + if command == "ci-deliver" { + args = append(args, "--prepared", "--commit", commit) + } + args = append(args, "v1.2.3") + var stdout, stderr bytes.Buffer + status := run(context.Background(), args, &stdout, &stderr) + if status != 2 || !strings.Contains(stderr.String(), "invalid_destination") { + t.Fatalf("status=%d stderr=%q, want destination configuration error", status, stderr.String()) + } + if _, err := os.Stat(marker); !os.IsNotExist(err) { + t.Fatalf("preparation/build ran for invalid destination: %v", err) + } + }) + } + } +} diff --git a/cmd/releasekit/main.go b/cmd/releasekit/main.go index 42843e3..f15636f 100644 --- a/cmd/releasekit/main.go +++ b/cmd/releasekit/main.go @@ -446,7 +446,8 @@ func deliveryPreflightExitStatus(err error) int { return 1 } switch preflightErr.Kind { - case releasekit.DeliveryPreflightMissingCredential, + case releasekit.DeliveryPreflightInvalidDestination, + releasekit.DeliveryPreflightMissingCredential, releasekit.DeliveryPreflightInvalidCredential, releasekit.DeliveryPreflightInsufficientPermissions, releasekit.DeliveryPreflightInaccessibleDestination: diff --git a/delivery.go b/delivery.go index 059590f..ee7ee1c 100644 --- a/delivery.go +++ b/delivery.go @@ -48,11 +48,13 @@ type DeliveryDestination struct { Token string } -// DeliveryPreflightFailureKind classifies a failure while checking a delivery -// destination's credentials and repository access. +// DeliveryPreflightFailureKind classifies a failure while checking destination +// settings, credentials, and repository access. type DeliveryPreflightFailureKind string const ( + // DeliveryPreflightInvalidDestination means the destination settings fail local validation. + DeliveryPreflightInvalidDestination DeliveryPreflightFailureKind = "invalid_destination" // DeliveryPreflightMissingCredential means no nonblank credential was supplied. DeliveryPreflightMissingCredential DeliveryPreflightFailureKind = "missing_credential" // DeliveryPreflightInvalidCredential means the selected service rejected the credential. @@ -221,6 +223,8 @@ func publicPreflightError(err error) error { } kind := DeliveryPreflightUnavailable switch failure.Kind { + case forge.PreflightInvalidDestination: + kind = DeliveryPreflightInvalidDestination case forge.PreflightMissingCredential: kind = DeliveryPreflightMissingCredential case forge.PreflightInvalidCredential: diff --git a/delivery_destination_test.go b/delivery_destination_test.go new file mode 100644 index 0000000..4d3f474 --- /dev/null +++ b/delivery_destination_test.go @@ -0,0 +1,53 @@ +package releasekit_test + +import ( + "context" + "errors" + "net/http" + "net/http/httptest" + "strings" + "testing" + + releasekit "go.maximumdirect.net/releasekit" +) + +func TestInvalidDestinationFailsStandaloneAndAutomaticPreflight(t *testing.T) { + repository := newTaggedDeliveryConsumer(t) + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + t.Errorf("invalid destination made a hosted request: %s", r.URL.Path) + http.Error(w, "unexpected request", http.StatusInternalServerError) + })) + defer server.Close() + for _, test := range []struct { + name, setting string + change func(*releasekit.DeliveryDestination) + }{ + {"URL", "base URL", func(d *releasekit.DeliveryDestination) { d.BaseURL = "ssh://forge.example" }}, + {"owner", "owner", func(d *releasekit.DeliveryDestination) { d.Owner = "owner/child" }}, + {"repository", "repository", func(d *releasekit.DeliveryDestination) { d.Repository = "repo/child" }}, + } { + t.Run(test.name, func(t *testing.T) { + destination := facadeDeliveryDestination(server.URL) + test.change(&destination) + assertFailure := func(err error) { + t.Helper() + var preflight *releasekit.DeliveryPreflightError + if !errors.As(err, &preflight) || preflight.Kind != releasekit.DeliveryPreflightInvalidDestination || preflight.Err == nil { + t.Fatalf("error = %#v, want invalid destination with cause", err) + } + if !strings.Contains(err.Error(), test.setting) { + t.Fatalf("error = %v, want invalid setting %q", err, test.setting) + } + } + assertFailure(releasekit.PreflightDelivery(context.Background(), destination)) + result, err := releasekit.Deliver(context.Background(), releasekit.DeliveryRequest{ + RepositoryDir: repository, Product: "Fixture", Version: "v1.2.3", Destination: destination, + Build: &releasekit.BuildSpec{MainPackage: "./missing", BinaryName: "fixture", Targets: []releasekit.Target{{OS: "linux", Arch: "amd64"}}}, + }) + assertFailure(err) + if result.State != releasekit.DeliveryNotAttempted { + t.Fatalf("delivery state = %s, want not attempted", result.State) + } + }) + } +} diff --git a/docs/config.md b/docs/config.md index 2420ccf..cf53646 100644 --- a/docs/config.md +++ b/docs/config.md @@ -170,9 +170,10 @@ both credential sources, returns status 2. Omission of `token_file` retains environment-only loading. After resolving the token, delivery performs an authenticated, read-only -repository access check. Missing or rejected credentials, known insufficient -permissions, and inaccessible destinations are configuration/credential -failures. An unavailable service or canceled check is an operational failure. +repository access check. Invalid destination settings, missing or rejected +credentials, known insufficient permissions, and inaccessible destinations are +configuration/credential failures. An unavailable service or canceled check is +an operational failure. See the [CLI contract](cli.md) for exact exit statuses. Successful preflight confirms authentication and repository access and accounts for any exposed delivery-permission deficiency. It does not guarantee later release or asset diff --git a/docs/consumers/delivery.md b/docs/consumers/delivery.md index f104078..607d09a 100644 --- a/docs/consumers/delivery.md +++ b/docs/consumers/delivery.md @@ -168,9 +168,10 @@ should follow credential verification. The check needs no checkout or published tag and performs no test release or upload. For Gitea it verifies authenticated repository access; exposed metadata can identify known deficiencies, but successful preflight does not guarantee later release or asset writes. A -`DeliveryPreflightError` identifies missing credentials, rejected credentials, -known insufficient permissions, an inaccessible destination, or unavailable -verification. Its `Unwrap` method preserves the cause for `errors.Is` and +`DeliveryPreflightError` identifies invalid destination settings, missing or +rejected credentials, known insufficient permissions, an inaccessible +destination, or unavailable verification. Its `Unwrap` method preserves the +cause for `errors.Is` and `errors.As`. A 403 means access was denied, not necessarily that a token expired; a 404 can mean a missing repository or hidden access. diff --git a/docs/roadmap/remediation.md b/docs/roadmap/remediation.md index 3203097..466ffcc 100644 --- a/docs/roadmap/remediation.md +++ b/docs/roadmap/remediation.md @@ -1,6 +1,6 @@ # Remediation: forge boundary and credential preflight review -Status: open; the findings below are unaddressed. +Status: remediation in progress; completion is recorded per issue below. ## Review baseline and scope @@ -31,7 +31,14 @@ separate Git authentication and completion semantics. ## 1. Complete local destination preflight error handling — P2 -Status: open. +Status: complete. + +**Completed:** Added the facade-owned `DeliveryPreflightInvalidDestination` +classification, preserved the adapter's validation cause, and mapped it to CLI +status 2 for ordinary and CI delivery. Updated consumer/configuration guidance. +Focused standalone, automatic, and CLI regressions cover invalid URLs, owners, +and repository names, early termination, and absence of hosted/build work. +Focused preflight tests and the compiled API/dependency check passed. **Evidence:** [Gitea preflight](../../internal/gitea/backend.go), [public error conversion](../../delivery.go), and diff --git a/internal/forge/forge.go b/internal/forge/forge.go index 30e3721..e12b3ed 100644 --- a/internal/forge/forge.go +++ b/internal/forge/forge.go @@ -66,11 +66,12 @@ type Asset struct { Handle any } -// PreflightFailureKind classifies a failure to verify credentials or access to -// a delivery destination. +// PreflightFailureKind classifies a failure to verify destination settings, +// credentials, or repository access. type PreflightFailureKind string const ( + PreflightInvalidDestination PreflightFailureKind = "invalid_destination" PreflightMissingCredential PreflightFailureKind = "missing_credential" PreflightInvalidCredential PreflightFailureKind = "invalid_credential" PreflightInsufficientPermissions PreflightFailureKind = "insufficient_permissions" diff --git a/internal/gitea/backend.go b/internal/gitea/backend.go index 3be5749..3ccfd55 100644 --- a/internal/gitea/backend.go +++ b/internal/gitea/backend.go @@ -50,7 +50,7 @@ func (b *Backend) Preflight(ctx context.Context) error { } base, apiRoot, err := b.destinationAPI("preflight repository access") if err != nil { - return err + return &forge.PreflightFailure{Kind: forge.PreflightInvalidDestination, Err: err} } client := b.client.httpClient() var resolved repositoryResponse