Classify invalid delivery destinations during preflight

This commit is contained in:
2026-11-01 19:24:52 +00:00
parent 627d524aa2
commit e5512d50b0
9 changed files with 146 additions and 14 deletions

View File

@@ -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)
}
})
}
}
}

View File

@@ -446,7 +446,8 @@ func deliveryPreflightExitStatus(err error) int {
return 1 return 1
} }
switch preflightErr.Kind { switch preflightErr.Kind {
case releasekit.DeliveryPreflightMissingCredential, case releasekit.DeliveryPreflightInvalidDestination,
releasekit.DeliveryPreflightMissingCredential,
releasekit.DeliveryPreflightInvalidCredential, releasekit.DeliveryPreflightInvalidCredential,
releasekit.DeliveryPreflightInsufficientPermissions, releasekit.DeliveryPreflightInsufficientPermissions,
releasekit.DeliveryPreflightInaccessibleDestination: releasekit.DeliveryPreflightInaccessibleDestination:

View File

@@ -48,11 +48,13 @@ type DeliveryDestination struct {
Token string Token string
} }
// DeliveryPreflightFailureKind classifies a failure while checking a delivery // DeliveryPreflightFailureKind classifies a failure while checking destination
// destination's credentials and repository access. // settings, credentials, and repository access.
type DeliveryPreflightFailureKind string type DeliveryPreflightFailureKind string
const ( const (
// DeliveryPreflightInvalidDestination means the destination settings fail local validation.
DeliveryPreflightInvalidDestination DeliveryPreflightFailureKind = "invalid_destination"
// DeliveryPreflightMissingCredential means no nonblank credential was supplied. // DeliveryPreflightMissingCredential means no nonblank credential was supplied.
DeliveryPreflightMissingCredential DeliveryPreflightFailureKind = "missing_credential" DeliveryPreflightMissingCredential DeliveryPreflightFailureKind = "missing_credential"
// DeliveryPreflightInvalidCredential means the selected service rejected the credential. // DeliveryPreflightInvalidCredential means the selected service rejected the credential.
@@ -221,6 +223,8 @@ func publicPreflightError(err error) error {
} }
kind := DeliveryPreflightUnavailable kind := DeliveryPreflightUnavailable
switch failure.Kind { switch failure.Kind {
case forge.PreflightInvalidDestination:
kind = DeliveryPreflightInvalidDestination
case forge.PreflightMissingCredential: case forge.PreflightMissingCredential:
kind = DeliveryPreflightMissingCredential kind = DeliveryPreflightMissingCredential
case forge.PreflightInvalidCredential: case forge.PreflightInvalidCredential:

View File

@@ -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)
}
})
}
}

View File

@@ -170,9 +170,10 @@ both credential sources, returns status 2. Omission of `token_file` retains
environment-only loading. environment-only loading.
After resolving the token, delivery performs an authenticated, read-only After resolving the token, delivery performs an authenticated, read-only
repository access check. Missing or rejected credentials, known insufficient repository access check. Invalid destination settings, missing or rejected
permissions, and inaccessible destinations are configuration/credential credentials, known insufficient permissions, and inaccessible destinations are
failures. An unavailable service or canceled check is an operational failure. 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 See the [CLI contract](cli.md) for exact exit statuses. Successful preflight
confirms authentication and repository access and accounts for any exposed confirms authentication and repository access and accounts for any exposed
delivery-permission deficiency. It does not guarantee later release or asset delivery-permission deficiency. It does not guarantee later release or asset

View File

@@ -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 tag and performs no test release or upload. For Gitea it verifies authenticated
repository access; exposed metadata can identify known deficiencies, but repository access; exposed metadata can identify known deficiencies, but
successful preflight does not guarantee later release or asset writes. A successful preflight does not guarantee later release or asset writes. A
`DeliveryPreflightError` identifies missing credentials, rejected credentials, `DeliveryPreflightError` identifies invalid destination settings, missing or
known insufficient permissions, an inaccessible destination, or unavailable rejected credentials, known insufficient permissions, an inaccessible
verification. Its `Unwrap` method preserves the cause for `errors.Is` and 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 `errors.As`. A 403 means access was denied, not necessarily that a token
expired; a 404 can mean a missing repository or hidden access. expired; a 404 can mean a missing repository or hidden access.

View File

@@ -1,6 +1,6 @@
# Remediation: forge boundary and credential preflight review # 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 ## Review baseline and scope
@@ -31,7 +31,14 @@ separate Git authentication and completion semantics.
## 1. Complete local destination preflight error handling — P2 ## 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), **Evidence:** [Gitea preflight](../../internal/gitea/backend.go),
[public error conversion](../../delivery.go), and [public error conversion](../../delivery.go), and

View File

@@ -66,11 +66,12 @@ type Asset struct {
Handle any Handle any
} }
// PreflightFailureKind classifies a failure to verify credentials or access to // PreflightFailureKind classifies a failure to verify destination settings,
// a delivery destination. // credentials, or repository access.
type PreflightFailureKind string type PreflightFailureKind string
const ( const (
PreflightInvalidDestination PreflightFailureKind = "invalid_destination"
PreflightMissingCredential PreflightFailureKind = "missing_credential" PreflightMissingCredential PreflightFailureKind = "missing_credential"
PreflightInvalidCredential PreflightFailureKind = "invalid_credential" PreflightInvalidCredential PreflightFailureKind = "invalid_credential"
PreflightInsufficientPermissions PreflightFailureKind = "insufficient_permissions" PreflightInsufficientPermissions PreflightFailureKind = "insufficient_permissions"

View File

@@ -50,7 +50,7 @@ func (b *Backend) Preflight(ctx context.Context) error {
} }
base, apiRoot, err := b.destinationAPI("preflight repository access") base, apiRoot, err := b.destinationAPI("preflight repository access")
if err != nil { if err != nil {
return err return &forge.PreflightFailure{Kind: forge.PreflightInvalidDestination, Err: err}
} }
client := b.client.httpClient() client := b.client.httpClient()
var resolved repositoryResponse var resolved repositoryResponse