Distinguish asset read failures from unknown writes

This commit is contained in:
2026-10-06 00:54:49 +00:00
parent 91712fd900
commit b744346210
5 changed files with 66 additions and 19 deletions

View File

@@ -423,18 +423,19 @@ func writeDeliveryReceipt(t *testing.T, repository string, settings cli.BuildSet
} }
type deliveryServerState struct { type deliveryServerState struct {
malformedCreate bool malformedCreate bool
failAssetListing bool failAssetListing bool
commit string failAssetDownloads bool
product string commit string
remoteCommit string product string
apiRequests int remoteCommit string
pageExists bool apiRequests int
releaseCreates int pageExists bool
assetUploads int releaseCreates int
assetListReads int assetUploads int
noteBody string assetListReads int
assets map[string][]byte noteBody string
assets map[string][]byte
} }
func newDeliveryServer(t *testing.T, state *deliveryServerState) *httptest.Server { func newDeliveryServer(t *testing.T, state *deliveryServerState) *httptest.Server {
@@ -530,6 +531,10 @@ func newDeliveryServer(t *testing.T, state *deliveryServerState) *httptest.Serve
_ = json.NewEncoder(w).Encode(map[string]any{"id": state.assetUploads, "name": name, "size": len(content)}) _ = json.NewEncoder(w).Encode(map[string]any{"id": state.assetUploads, "name": name, "size": len(content)})
default: default:
if strings.HasPrefix(r.URL.Path, "/files/") { if strings.HasPrefix(r.URL.Path, "/files/") {
if state.failAssetDownloads {
http.Error(w, "unavailable", 503)
return
}
name := strings.TrimPrefix(r.URL.Path, "/files/") name := strings.TrimPrefix(r.URL.Path, "/files/")
content, ok := state.assets[name] content, ok := state.assets[name]
if !ok { if !ok {
@@ -704,3 +709,31 @@ func TestRecoveredPageCreatePreservesPartialDeliveryOnInspectionFailure(t *testi
t.Fatalf("status=%d state=%#v %s", status, state, stderr.String()) t.Fatalf("status=%d state=%#v %s", status, state, stderr.String())
} }
} }
func TestUnreadableAssetsDoNotClaimAnUnknownWrite(t *testing.T) {
for _, existingPage := range []bool{false, true} {
t.Run(fmt.Sprintf("existing-page-%t", existingPage), func(t *testing.T) {
repository := newCLIRepository(t)
state := &deliveryServerState{pageExists: existingPage, noteBody: "# Example Product v1.2.3\n\nRelease note.\n", failAssetDownloads: true, assets: map[string][]byte{"helper-v1.2.3-linux-amd64": []byte("fixture artifact")}}
server := newDeliveryServer(t, state)
defer server.Close()
build := cli.BuildSettings{MainPackage: ".", BinaryName: "helper", Targets: []string{"linux/amd64"}}
writeDeliveryProfile(t, repository, cli.Settings{Product: "Example Product", Build: &build, Delivery: &cli.DeliverySettings{BaseURL: server.URL, Owner: "owner", Repository: "example"}})
state.commit = cliGitOutput(t, repository, "rev-parse", "HEAD")
cliGitRun(t, repository, "tag", "v1.2.3")
assets, _ := writeDeliveryReceipt(t, repository, build)
t.Setenv("RELEASEKIT_GITEA_TOKEN", "fixture-token")
var stdout, stderr bytes.Buffer
status := run(context.Background(), []string{"deliver", "--repo", repository, "--assets", assets, "v1.2.3"}, &stdout, &stderr)
expectedState := "partial"
expectedCreates := 1
if existingPage {
expectedState = "not_attempted"
expectedCreates = 0
}
if status != 1 || !strings.Contains(stderr.String(), "state="+expectedState) || state.assetUploads != 0 || state.releaseCreates != expectedCreates {
t.Fatalf("status=%d creates=%d uploads=%d %s", status, state.releaseCreates, state.assetUploads, stderr.String())
}
})
}
}

View File

@@ -43,7 +43,9 @@ missing, conflicting, or unverifiable. Same-name attachments with different
bytes are conflicts even when their reported size matches. An existing bytes are conflicts even when their reported size matches. An existing
`SHA256SUMS` must exactly match the complete expected manifest; a subset or `SHA256SUMS` must exactly match the complete expected manifest; a subset or
different manifest is a conflict. An unreadable attachment is unverifiable, different manifest is a conflict. An unreadable attachment is unverifiable,
not missing. Unrelated attachments are returned separately and are preserved. not missing. A failed read-only inspection reports `not_attempted`; if this
invocation already delivered the page, its outcome retains `partial` progress.
Unknown-write outcomes are reserved for unresolved writes. Unrelated attachments are returned separately and are preserved.
Attachment downloads and same-origin redirects carry authorization only when Attachment downloads and same-origin redirects carry authorization only when
their scheme, host, and port match the configured Gitea service. Cross-origin their scheme, host, and port match the configured Gitea service. Cross-origin

View File

@@ -131,6 +131,8 @@ followed by failed asset inspection. Both affected suites pass.
### R3 — Medium: Read-only asset failures are classified as unknown writes ### R3 — Medium: Read-only asset failures are classified as unknown writes
Status: Resolved.
Category: Divergence from the selected outcome contract. Category: Divergence from the selected outcome contract.
**Evidence:** [UploadMissing](../../internal/gitea/upload.go), around line 102, **Evidence:** [UploadMissing](../../internal/gitea/upload.go), around line 102,
@@ -153,6 +155,12 @@ as selected in the plan. Adjust tests and documentation consistently.
read-only invocation must not return unknown-write state or exit 3. A genuinely read-only invocation must not return unknown-write state or exit 3. A genuinely
ambiguous upload must continue to do so. ambiguous upload must continue to do so.
**Resolution:** Unverifiable preflight assets now return an inspection failure
with `not_attempted`, retaining partial page progress when creation preceded
the read failure. CLI tests cover existing and newly created pages, no asset
writes, and exit 1. Existing ambiguous-upload regressions still verify unknown
write outcomes. Both affected suites pass.
### R4 — Medium: Local artifact regeneration is prohibited ### R4 — Medium: Local artifact regeneration is prohibited
Category: Inadvertently strengthened prohibition. Category: Inadvertently strengthened prohibition.

View File

@@ -124,6 +124,9 @@ func Deliver(ctx context.Context, request Request) (Result, error) {
result.Conflicting = uploadResult.Conflicting result.Conflicting = uploadResult.Conflicting
result.Uncertain = uploadResult.Uncertain result.Uncertain = uploadResult.Uncertain
result.State = State(uploadResult.State) result.State = State(uploadResult.State)
if result.State == NotAttempted && releaseResult.WriteAttempted {
result.State = Partial
}
return result, uploadErr return result, uploadErr
} }

View File

@@ -21,10 +21,11 @@ import (
type DeliveryState string type DeliveryState string
const ( const (
DeliveryComplete DeliveryState = "complete" DeliveryNotAttempted DeliveryState = "not_attempted"
DeliveryPartial DeliveryState = "partial" DeliveryComplete DeliveryState = "complete"
DeliveryConflict DeliveryState = "conflict" DeliveryPartial DeliveryState = "partial"
DeliveryUnknown DeliveryState = "unknown" DeliveryConflict DeliveryState = "conflict"
DeliveryUnknown DeliveryState = "unknown"
) )
type AssetIdentity struct { type AssetIdentity struct {
@@ -100,8 +101,8 @@ func (c Client) UploadMissing(ctx context.Context, destination Destination, rele
return result, failure(FailureConflict, "preflight release assets", 0, errors.New("one or more existing attachments conflict")) return result, failure(FailureConflict, "preflight release assets", 0, errors.New("one or more existing attachments conflict"))
} }
if preflightUnknown { if preflightUnknown {
result.State = DeliveryUnknown result.State = DeliveryNotAttempted
return result, failure(FailureUnknown, "preflight release assets", 0, errors.New("one or more existing attachments could not be verified")) return result, failure(FailureUnverifiable, "preflight release assets", 0, errors.New("one or more existing attachments could not be verified"))
} }
if len(expected) == 0 { if len(expected) == 0 {
result.State = DeliveryComplete result.State = DeliveryComplete