diff --git a/delivery_reads_linux_test.go b/delivery_reads_linux_test.go new file mode 100644 index 0000000..5df8d74 --- /dev/null +++ b/delivery_reads_linux_test.go @@ -0,0 +1,66 @@ +package releasekit_test + +import ( + "encoding/binary" + "errors" + "net/http" + "net/http/httptest" + "syscall" + "testing" + + releasekit "gitea.maximumdirect.net/eric/ReleaseKit" +) + +func TestDeliverReadsLocalBinaryOnceForVerificationAndOnceForUpload(t *testing.T) { + repository, build, expected := newDeliveryBenchmarkFixture(t) + watcher, err := syscall.InotifyInit1(syscall.IN_NONBLOCK | syscall.IN_CLOEXEC) + if err != nil { + t.Fatal(err) + } + defer syscall.Close(watcher) + // Close events separate successive opens so inotify cannot coalesce them. + if _, err := syscall.InotifyAddWatch(watcher, build.Artifacts[0].Path, syscall.IN_OPEN|syscall.IN_CLOSE_NOWRITE); err != nil { + t.Fatal(err) + } + state := &deliveryBenchmarkServer{commit: build.CandidateCommit, assets: expected} + state.reset(false) + server := httptest.NewServer(http.HandlerFunc(state.serveHTTP)) + defer server.Close() + state.baseURL = server.URL + result, err := releasekit.Deliver(t.Context(), releasekit.DeliveryRequest{ + RepositoryDir: repository, Product: "Sample App", Version: build.Version, + Destination: facadeDeliveryDestination(server.URL), Prebuilt: &build, + }) + if err != nil || result.State != releasekit.DeliveryComplete || len(result.Uploaded) != len(expected) { + t.Fatalf("delivery = %#v, error = %v", result, err) + } + state.verifyUploads(t) + opens := 0 + var events [256]byte + for { + n, err := syscall.Read(watcher, events[:]) + if errors.Is(err, syscall.EAGAIN) { + break + } + if errors.Is(err, syscall.EINTR) { + continue + } + if err != nil { + t.Fatal(err) + } + for offset := 0; offset < n; { + mask := binary.NativeEndian.Uint32(events[offset+4:]) + if mask&syscall.IN_Q_OVERFLOW != 0 { + t.Fatal("artifact event queue overflowed") + } + if mask&syscall.IN_OPEN != 0 { + opens++ + } + nameLength := binary.NativeEndian.Uint32(events[offset+12:]) + offset += syscall.SizeofInotifyEvent + int(nameLength) + } + } + if opens != 2 { + t.Fatalf("local binary opened %d times; want one verification read and one upload read", opens) + } +} diff --git a/docs/internal/overview.md b/docs/internal/overview.md index 2e3deef..163fd4f 100644 --- a/docs/internal/overview.md +++ b/docs/internal/overview.md @@ -74,6 +74,12 @@ it does not consult neighboring files or traverse submodule checkouts. See the [shared validation guide](validation.md) for checker, Go command, and candidate-file integration mechanics. +Delivery verifies all local binary sizes and digests before hosted requests. +Its Gitea upload path reuses that verification and streams missing files once +for transport. Independent calls to the internal client's `UploadMissing` +verify missing local files before writing. Both paths share remote inspection, +conflict handling, and reconciliation. + ## Capability owners and behavioral evidence The following tests are representative evidence from the current offline suite, diff --git a/docs/roadmap/remediation.md b/docs/roadmap/remediation.md index 440e31a..53b1564 100644 --- a/docs/roadmap/remediation.md +++ b/docs/roadmap/remediation.md @@ -1,6 +1,6 @@ # Remediation: module release implementation review -Status: open; issues 1–12 are complete, and issues 13–15 require remediation. +Status: open; issues 1–13 are complete, and issues 14–15 require remediation. The initial review examined `90559fe`, compared with planning baseline `dbfae9e`. The follow-up repository audit examined `6b0b8ce`, including those corrections. @@ -462,7 +462,17 @@ differences, ordinary aliases, cycles, and outside-checkout failures. ## Issue 13: Reuse local asset verification through the complete delivery path (P2) -Status: open. Category: incomplete verification ownership and redundant I/O. +Status: complete. Category: incomplete verification ownership and redundant I/O. + +**Completion:** Assembled delivery reuses its preflight verification through the +internal upload path; standalone internal uploads retain local verification. +A Linux filesystem-event regression observes one verification read and one +upload read across public delivery and validates uploaded contents. It failed +with three reads before the correction. Delivery/Gitea suites pass offline, +including tampering, caller ownership, matching attachments, and partial retries. +The complete-delivery benchmark now supplies current module identity. Its 32 MiB +fresh-upload case measured about 352 ms before and 254 ms afterward on this host; +these illustrative measurements introduce no timing gate. **Evidence:** [Delivery](../../internal/delivery/delivery.go) calls `VerifyLocalAssets` before hosted requests. After release and attachment diff --git a/internal/delivery/delivery.go b/internal/delivery/delivery.go index b7e808e..43dd184 100644 --- a/internal/delivery/delivery.go +++ b/internal/delivery/delivery.go @@ -183,7 +183,7 @@ func Deliver(ctx context.Context, request Request) (Result, error) { result.Uncertain = assetIdentities(assets) return result, err } - uploadResult, uploadErr := (gitea.Client{}).UploadMissing(ctx, request.Destination, releaseResult.Release.ID, inspection) + uploadResult, uploadErr := (gitea.Client{}).UploadVerifiedMissing(ctx, request.Destination, releaseResult.Release.ID, inspection) result.Matching = uploadResult.Matching result.Uploaded = uploadResult.Uploaded result.Pending = uploadResult.Pending diff --git a/internal/gitea/upload.go b/internal/gitea/upload.go index 920fb8d..b9a714e 100644 --- a/internal/gitea/upload.go +++ b/internal/gitea/upload.go @@ -68,6 +68,17 @@ func VerifyLocalAssets(assets []ExpectedAsset) error { // before the first write if any declared existing asset conflicts or cannot be // verified, and never replaces an existing attachment. func (c Client) UploadMissing(ctx context.Context, destination Destination, releaseID int64, inspection Inspection) (UploadResult, error) { + return c.uploadMissing(ctx, destination, releaseID, inspection, true) +} + +// UploadVerifiedMissing uploads missing assets whose local contents have already +// passed VerifyLocalAssets. It reuses that verification while retaining remote +// inspection, conflict handling, and upload reconciliation. +func (c Client) UploadVerifiedMissing(ctx context.Context, destination Destination, releaseID int64, inspection Inspection) (UploadResult, error) { + return c.uploadMissing(ctx, destination, releaseID, inspection, false) +} + +func (c Client) uploadMissing(ctx context.Context, destination Destination, releaseID int64, inspection Inspection, verifyLocal bool) (UploadResult, error) { if releaseID <= 0 { return UploadResult{State: DeliveryPartial}, failure(FailureInvalid, "upload release assets", 0, errors.New("release ID must be positive")) } @@ -127,12 +138,14 @@ func (c Client) UploadMissing(ctx context.Context, destination Destination, rele missing[observation.Expected.Filename] = observation.Expected } } - for _, item := range expected { - if _, needsUpload := missing[item.Filename]; !needsUpload { - continue - } - if err := verifyLocalAsset(item); err != nil { - return result, err + if verifyLocal { + for _, item := range expected { + if _, needsUpload := missing[item.Filename]; !needsUpload { + continue + } + if err := verifyLocalAsset(item); err != nil { + return result, err + } } } diff --git a/internal/gitea/upload_test.go b/internal/gitea/upload_test.go index 81e9d9e..89316ed 100644 --- a/internal/gitea/upload_test.go +++ b/internal/gitea/upload_test.go @@ -15,6 +15,31 @@ import ( "testing" ) +func TestUploadMissingVerifiesEveryMissingLocalFileBeforeWriting(t *testing.T) { + fixture := &uploadFixture{files: make(map[string][]byte)} + server := httptest.NewServer(fixture) + defer server.Close() + client := testClient(server) + destination := testDestination(server.URL) + assets := localAssets(t, []string{"alpha.bin", "beta.bin"}) + inspection, err := client.InspectAssets(t.Context(), destination, 10, assets) + if err != nil { + t.Fatal(err) + } + contents := readExpectedFile(t, assets[1]) + contents[0] ^= 0xff + if err := os.WriteFile(assets[1].Path, contents, 0o600); err != nil { + t.Fatal(err) + } + result, err := client.UploadMissing(t.Context(), destination, 10, inspection) + if err == nil || result.State != DeliveryPartial { + t.Fatalf("upload = %#v, error = %v; want local verification failure", result, err) + } + if posts := fixture.postNames(); len(posts) != 0 { + t.Fatalf("uploaded %v before rejecting local content", posts) + } +} + func TestUploadMissingPartialFailureThenRerunOnlyMissingAssets(t *testing.T) { fixture := &uploadFixture{files: make(map[string][]byte), failName: "beta.bin"} server := httptest.NewServer(fixture) diff --git a/performance_delivery_test.go b/performance_delivery_test.go index 83569bf..1ad0596 100644 --- a/performance_delivery_test.go +++ b/performance_delivery_test.go @@ -102,7 +102,7 @@ func BenchmarkPerformanceFacadeDeliver(b *testing.B) { }) } -func newDeliveryBenchmarkFixture(b *testing.B) (string, releasekit.BuildResult, []deliveryBenchmarkAsset) { +func newDeliveryBenchmarkFixture(b testing.TB) (string, releasekit.BuildResult, []deliveryBenchmarkAsset) { b.Helper() repository := filepath.Join(b.TempDir(), "repository") if err := os.MkdirAll(repository, 0o755); err != nil { @@ -111,6 +111,7 @@ func newDeliveryBenchmarkFixture(b *testing.B) (string, releasekit.BuildResult, gitRun(b, repository, "init", "-b", "main") gitRun(b, repository, "config", "user.name", "ReleaseKit Benchmark") gitRun(b, repository, "config", "user.email", "benchmark@example.invalid") + writeFile(b, repository, "go.mod", "module example.com/deliveryfixture\n\ngo 1.26.0\n") writeFile(b, repository, "docs/releases/v1.2.3.md", "# Sample App v1.2.3\n\nDelivery fixture.\n") gitRun(b, repository, "add", ".") gitRun(b, repository, "commit", "-m", "prepare delivery benchmark") @@ -129,6 +130,7 @@ func newDeliveryBenchmarkFixture(b *testing.B) (string, releasekit.BuildResult, targets := []releasekit.Target{{OS: "linux", Arch: "amd64"}, {OS: "windows", Arch: "amd64"}} build := releasekit.BuildResult{ RepositoryDir: repository, CandidateCommit: gitText(b, repository, "rev-parse", "HEAD"), + ModuleDir: repository, ModulePath: "example.com/deliveryfixture", Tag: "v1.2.3", SourceVerified: true, Version: "v1.2.3", MainPackage: "./cmd/app", BinaryName: "releasekit", Targets: targets, OutputDir: outputDir, } @@ -173,7 +175,7 @@ func (state *deliveryBenchmarkServer) reset(populated bool) { } } -func (state *deliveryBenchmarkServer) verifyUploads(b *testing.B) { +func (state *deliveryBenchmarkServer) verifyUploads(b testing.TB) { b.Helper() state.mu.Lock() defer state.mu.Unlock()