From 2b23574ecee2c019263018e1b5fd72b32fc6670d Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Mon, 2 Nov 2026 03:29:59 +0000 Subject: [PATCH] Bound remote asset content verification --- delivery_test.go | 28 ++++++++ docs/integrations/gitea.md | 2 + docs/roadmap/implementation.md | 2 +- internal/delivery/remote_assets.go | 40 +++++++++--- internal/delivery/remote_assets_test.go | 87 +++++++++++++++++++++++++ 5 files changed, 149 insertions(+), 10 deletions(-) diff --git a/delivery_test.go b/delivery_test.go index 0a7aec7..e6b5534 100644 --- a/delivery_test.go +++ b/delivery_test.go @@ -307,6 +307,34 @@ func TestDeliverPrebuiltArtifactsRemainCallerOwned(t *testing.T) { } } +func TestDeliverRejectsOversizedExistingPrebuiltAsset(t *testing.T) { + repository := newTaggedDeliveryConsumer(t) + prebuilt, err := releasekit.Build(context.Background(), releasekit.BuildRequest{ + RepositoryDir: repository, Version: "v1.2.3", OutputDir: t.TempDir(), + Spec: releasekit.BuildSpec{MainPackage: "./cmd/app", BinaryName: "fixture", Targets: []releasekit.Target{{OS: runtime.GOOS, Arch: runtime.GOARCH}}}, + }) + if err != nil { + t.Fatal(err) + } + content, err := os.ReadFile(prebuilt.Artifacts[0].Path) + if err != nil { + t.Fatal(err) + } + server, remote := newFacadeDeliveryServer(t, repository) + defer server.Close() + remote.update(func(data *facadeDeliveryData) { + data.pages["v1.2.3"] = true + data.assets[prebuilt.Artifacts[0].Filename] = append(content, 'x') + }) + result, err := releasekit.Deliver(context.Background(), releasekit.DeliveryRequest{ + RepositoryDir: repository, Product: "Fixture", Version: "v1.2.3", + Destination: facadeDeliveryDestination(server.URL), Prebuilt: &prebuilt, + }) + if err == nil || result.State != releasekit.DeliveryConflict || len(result.Conflicting) != 1 || result.Conflicting[0].Filename != prebuilt.Artifacts[0].Filename || remote.snapshot().writes != 0 { + t.Fatalf("delivery = %#v, error = %v, writes = %d", result, err, remote.snapshot().writes) + } +} + func TestDeliverRejectsSameSizePrebuiltTamperingBeforeHostedWrites(t *testing.T) { repository := newTaggedDeliveryConsumer(t) prebuilt, err := releasekit.Build(context.Background(), releasekit.BuildRequest{ diff --git a/docs/integrations/gitea.md b/docs/integrations/gitea.md index 03a4606..ed30470 100644 --- a/docs/integrations/gitea.md +++ b/docs/integrations/gitea.md @@ -89,6 +89,8 @@ Attachment downloads and redirects carry the Gitea authorization token only when their scheme, host, and port match the configured service. Cross-origin content is downloaded without that token and verified by its digest. Downloads follow at most ten redirects; an additional redirect is unverifiable. +Comparison reads no more than the declared local size plus one byte; shorter +or longer content conflicts, regardless of the server's reported size. Shared delivery uploads only after inspection finds no conflicts or unverifiable assets. It uploads missing binaries in filename order and then the diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index 01c0db8..3bfe46f 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -48,7 +48,7 @@ the agent policy; an intermediate compilation dependency is recorded accurately. | 10 | A07: allocation and temporary metadata | Complete — allocation resolution, joined cleanup causes, tag evidence, and receipt installation checks passed. | | 11 | A07: build and delivery outcomes | Complete — managed and native cleanup tests cover success, failure, cancellation, hosted completion, and CLI reporting. | | 12 | A08 | Complete — redirect boundary, origin authorization, cycle, and delivery outcome tests passed. | -| 13 | A09 | Pending | +| 13 | A09 | Complete — bounded reads, size and digest mismatches, stream failures, cancellation, reconciliation, and public prebuilt delivery checks passed. | | 14 | A10 | Pending | | 15 | A13 | Pending | | 16 | A15 | Pending | diff --git a/internal/delivery/remote_assets.go b/internal/delivery/remote_assets.go index b85346c..3ccf773 100644 --- a/internal/delivery/remote_assets.go +++ b/internal/delivery/remote_assets.go @@ -77,11 +77,11 @@ func inspectRemoteAssets(ctx context.Context, backend forge.Backend, releaseHand case 0: observation.State = AssetMissing case 1: - digest, err := remoteAssetDigest(ctx, backend, matches[0].Handle) + digest, sizeMatches, err := remoteAssetDigest(ctx, backend, matches[0].Handle, item.Size) if err != nil { observation.State = AssetUnverifiable observation.Err = err - } else if digest == item.SHA256 { + } else if sizeMatches && digest == item.SHA256 { observation.State = AssetMatching } else { observation.State = AssetConflicting @@ -99,18 +99,40 @@ func inspectRemoteAssets(ctx context.Context, backend forge.Backend, releaseHand return inspection, nil } -func remoteAssetDigest(ctx context.Context, backend forge.Backend, handle any) (string, error) { +func remoteAssetDigest(ctx context.Context, backend forge.Backend, handle any, expectedSize int64) (string, bool, error) { + if err := ctx.Err(); err != nil { + return "", false, err + } stream, err := backend.OpenAsset(ctx, handle) if err != nil { - return "", err + return "", false, err } hasher := sha256.New() - _, readErr := io.Copy(hasher, stream) - closeErr := stream.Close() - if err := errors.Join(readErr, closeErr); err != nil { - return "", &forge.Failure{Kind: forge.FailureUnverifiable, Err: err} + read, readErr := io.Copy(hasher, io.LimitReader(stream, expectedSize)) + excess := false + if readErr == nil && read == expectedSize { + var probe [1]byte + for attempts := 0; ; attempts++ { + var n int + n, readErr = stream.Read(probe[:]) + excess = n != 0 + if readErr != nil || excess { + if readErr == io.EOF { + readErr = nil + } + break + } + if attempts == 99 { + readErr = io.ErrNoProgress + break + } + } } - return hex.EncodeToString(hasher.Sum(nil)), nil + closeErr := stream.Close() + if err := errors.Join(readErr, closeErr, ctx.Err()); err != nil { + return "", false, &forge.Failure{Kind: forge.FailureUnverifiable, Err: err} + } + return hex.EncodeToString(hasher.Sum(nil)), read == expectedSize && !excess, nil } func uploadMissingAssets(ctx context.Context, backend forge.Backend, releaseHandle any, inspection Inspection) (result UploadResult, err error) { diff --git a/internal/delivery/remote_assets_test.go b/internal/delivery/remote_assets_test.go index 8867adc..3fa0f5a 100644 --- a/internal/delivery/remote_assets_test.go +++ b/internal/delivery/remote_assets_test.go @@ -53,6 +53,88 @@ func TestInspectRemoteAssetsComparesContentAndPreservesUnrelatedAssets(t *testin } } +type controlledAssetStream struct { + *bytes.Reader + readErr error + errorAfter int + closeErr error + cancel context.CancelFunc + read int + closed bool +} + +func (stream *controlledAssetStream) Read(p []byte) (int, error) { + n, err := stream.Reader.Read(p) + stream.read += n + if stream.cancel != nil { + stream.cancel() + stream.cancel = nil + } + if stream.readErr != nil && n > 0 && stream.read > stream.errorAfter { + err, stream.readErr = stream.readErr, nil + } + return n, err +} + +func (stream *controlledAssetStream) Close() error { + stream.closed = true + return stream.closeErr +} + +type controlledAssetBackend struct { + pageBackend + stream *controlledAssetStream +} + +func (backend *controlledAssetBackend) ListAssets(context.Context, any) ([]forge.Asset, error) { + return []forge.Asset{{Filename: "asset.bin", Size: 999, Handle: "asset"}}, nil +} + +func (backend *controlledAssetBackend) OpenAsset(context.Context, any) (io.ReadCloser, error) { + return backend.stream, nil +} + +func TestInspectRemoteAssetBoundsContentAndPreservesErrors(t *testing.T) { + readErr := errors.New("read failed") + closeErr := errors.New("close failed") + for _, test := range []struct { + name, expected, actual string + readErr, closeErr error + cancel bool + errorAfter int + state AssetState + }{ + {"empty", "", "", nil, nil, false, 0, AssetMatching}, + {"shorter", "abcd", "abc", nil, nil, false, 0, AssetConflicting}, + {"matching", "abcd", "abcd", nil, nil, false, 0, AssetMatching}, + {"different", "abcd", "wxyz", nil, nil, false, 0, AssetConflicting}, + {"oversized", "abcd", "abcdefghijk", nil, nil, false, 0, AssetConflicting}, + {"empty oversized", "", "abc", nil, nil, false, 0, AssetConflicting}, + {"data and error", "abcd", "abcd", readErr, nil, false, 0, AssetUnverifiable}, + {"probe data and error", "abcd", "abcde", readErr, nil, false, 4, AssetUnverifiable}, + {"close failure", "abcd", "abcdefgh", nil, closeErr, false, 0, AssetUnverifiable}, + {"cancellation", "abcd", "abcd", nil, nil, true, 0, AssetUnverifiable}, + } { + t.Run(test.name, func(t *testing.T) { + ctx, cancel := context.WithCancel(t.Context()) + defer cancel() + stream := &controlledAssetStream{Reader: bytes.NewReader([]byte(test.actual)), readErr: test.readErr, errorAfter: test.errorAfter, closeErr: test.closeErr} + if test.cancel { + stream.cancel = cancel + } + backend := &controlledAssetBackend{stream: stream} + inspection, err := inspectRemoteAssets(ctx, backend, "release", []artifact.ExpectedAsset{expectedAsset("asset.bin", []byte(test.expected))}) + if err != nil || len(inspection.Assets) != 1 || inspection.Assets[0].State != test.state || !stream.closed || stream.read > len(test.expected)+1 { + t.Fatalf("inspection = %#v, error = %v, bytes = %d, closed = %t", inspection, err, stream.read, stream.closed) + } + cause := inspection.Assets[0].Err + if test.readErr != nil && !errors.Is(cause, test.readErr) || test.closeErr != nil && !errors.Is(cause, test.closeErr) || test.cancel && !errors.Is(cause, context.Canceled) { + t.Fatalf("inspection lost cause: %v", cause) + } + }) + } +} + func TestInspectRemoteAssetsListingFailureDoesNotBecomeUnknownWrite(t *testing.T) { assets := []artifact.ExpectedAsset{expectedAsset("a.bin", []byte("a")), expectedAsset("b.bin", []byte("b"))} listErr := &forge.Failure{Kind: forge.FailureTransport, Err: errors.New("service unavailable")} @@ -230,6 +312,11 @@ func TestUploadMissingAssetsReconcilesOnceAndKeepsLaterAssetsPending(t *testing. followupAssets: []forge.Asset{{Filename: "a.bin", Handle: "a"}}, followupData: map[string][]byte{"a": []byte("x")}, wantState: Conflict, wantCategory: "conflicting", }, + { + name: "oversized bytes conflict", disposition: forge.WriteUncertain, + followupAssets: []forge.Asset{{Filename: "a.bin", Handle: "a"}}, followupData: map[string][]byte{"a": []byte("oversized")}, + wantState: Conflict, wantCategory: "conflicting", + }, { name: "missing after conflict remains conflict", disposition: forge.WriteConflict, wantState: Conflict, wantCategory: "conflicting",