diff --git a/docs/roadmap/remediation.md b/docs/roadmap/remediation.md index bcd287c..97558de 100644 --- a/docs/roadmap/remediation.md +++ b/docs/roadmap/remediation.md @@ -1,7 +1,8 @@ # Remediation: module release implementation review -Status: complete; all eleven findings addressed and verified. The review examined -`90559fe`, compared with planning baseline `dbfae9e`. +Status: open; issues 1–11 are complete, and issues 12–15 require remediation. +The initial review examined `90559fe`, compared with planning baseline `dbfae9e`. +The follow-up repository audit examined `6b0b8ce`, including those corrections. ## Scope and evidence @@ -408,3 +409,141 @@ Update migration/examples alongside the behavior corrections above. check root/nested examples, note and tag wording, roadmap status, local links, fragments, and Markdown whitespace. Keep exact contract definitions at their canonical owners. + +## Follow-up audit + +The follow-up reviewed module identity, candidate validation and publication, +builds, receipts, delivery, Go/process ownership, committed reference handling, +facade conversions, CLI/profile mapping, maintained workflows, documentation, +and the eleven previous corrections. The complete offline suite and compiled +API/dependency checks pass with Go 1.26.8. The additional findings below were +reproduced with temporary local repositories, a filtered sparse clone, a Git +transport stub, and a local HTTP server. The artifact-read evidence also used +`strace`. This audit changes the remediation document only. + +## Issue 12: Resolve module paths consistently across declaration sources (P2) + +Status: open. Category: inconsistent release-unit selection. + +**Evidence:** [Filesystem selection](../../internal/releaseidentity/identity.go) +cleans the joined module path before following symlinks. Committed selection +uses [component traversal](../../internal/git/path.go), following a symlink +before processing a subsequent `..`. The same selection can consequently +identify different physical modules in an unchanged, clean checkout. + +A fixture contained root and `nested` modules, a tracked directory +`nested/deep`, and `link -> nested/deep`. With `ModuleDir: "link/.."` and +`Version: "v0.1.0"`, public `Build` succeeded with the root module and tag +`v0.1.0`; public `Check` succeeded with the nested module and tag +`nested/v0.1.0`. Working-tree and committed declaration readers independently +confirmed the difference. Dependency preparation uses the filesystem selector, +while candidate and delivery operations use committed selection. + +**Required change:** Give filesystem and committed selection the same component +and symlink semantics for identical paths in an unchanged checkout. Preserve +the agreed declaration source for each operation, support for working-tree +changes and committed sparse selection, checkout boundaries, and progressive +identity evidence. Module paths containing `..` need consistent resolution; +this correction does not require prohibiting those paths. + +**Verification:** Exercise a tracked directory symlink followed by `..` through +standalone build, candidate check, dependency preparation, and representative +delivery. An unchanged checkout must identify the same physical module and tag +across operations. Retain deliberate committed/working-tree declaration +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. + +**Evidence:** [Delivery](../../internal/delivery/delivery.go) calls +`VerifyLocalAssets` before hosted requests. After release and attachment +inspection, [UploadMissing](../../internal/gitea/upload.go) calls +`verifyLocalAsset` again for every missing attachment. Both calls stream and +hash each local binary. The upload then reads it a third time to send its bytes. +Stages 8 and 9 of the [implementation plan](implementation.md) select one +delivery-owned content-verification pass. + +A public prebuilt-delivery fixture completed against a local HTTP server with +one missing binary and its manifest. A system-call trace recorded three complete +reads of the binary: the delivery preflight, the uploader's repeated verification, +and the multipart upload. Receipt loading is already free of artifact hashing; +the remaining duplicate sits across the delivery/Gitea boundary. The current +asset-preflight benchmark ends before that second verification. + +**Required change:** Reuse the completed local verification in the assembled +delivery path so each binary receives one delivery-time digest-and-size pass +before hosted requests, followed by its necessary upload stream. Preserve +validation for independently used internal client operations, build-time +hashing, deterministic manifests, actual remote-content comparison, additive +retry behavior, caller-owned files, and existing failure outcomes. + +**Verification:** Observe file reads across a complete public binary delivery +with missing attachments, covering the uploader as well as preflight. Establish +one verification read plus the upload read. Keep tampered/missing-file rejection +before hosted requests and matching-asset/partial-retry cases. Measure the whole +local path without timing thresholds or additional source guarantees. + +## Issue 14: Keep committed release-note inspection within the local object boundary (P2) + +Status: open. Category: incomplete earlier offline-inspection correction. + +**Evidence:** [ReadCommitFile](../../internal/git/source.go), used by candidate +capture and delivery for release notes, executes `git show` through the ordinary +Git runner. Its child does not receive the lazy-fetch and transport controls +used by `ReadBlob`, committed-path lookup, tree inspection, and the batch reader. +An unavailable note in a partial clone can therefore initiate remote transport +during source inspection. The earlier correction in `0205001` and its partial +clone tests cover the other object readers, leaving this entry point uncovered. + +A clean filtered sparse clone had its root declaration and Go source locally +available, with the committed note omitted and its blob unavailable locally. +Public `Check` resolved the root identity, then invoked a fixture SSH transport +while reading `docs/releases/v0.1.0.md`. The stub recorded the attempt and exited; +the resulting error reported failure to fetch the note from the promisor remote. +The fixture contacted no external service. + +**Required change:** Read committed note bytes through the same local-only object +boundary as the other source-inspection readers. Return an inspection error +when required objects are unavailable locally, preserving path context, +cancellation, underlying Git causes, and exact note bytes. Explicit checkout +and dependency preparation retain their network-capable responsibilities. + +**Verification:** Cover direct committed-file reading and representative +candidate/delivery note inspection in filtered clones with unavailable note +objects. A controlled transport stub must remain unused, and inspection must +leave the object database unchanged. Existing ordinary note reads and explicit +network-capable preparation must continue to work. + +## Issue 15: Limit blob availability requirements to selected validation inputs (P2) + +Status: open. Category: module-scope leakage through eager metadata enrichment. + +**Evidence:** [ReadCandidateTreeForScope](../../internal/validation/tree.go) +calls [ReadTree](../../internal/git/source.go) before constructing module and +shared-file inputs. `ReadTree` asks `populateBlobSizes` for every repository blob +and fails on any unavailable object. Excluded modules and unreferenced files +therefore become availability prerequisites for a selected module's file checks, +even when the selected check needs no blob sizes. + +In a clean filtered sparse clone, the root module's source, declaration, and +note were available; independently scoped `nested` module blobs were omitted. +Public root `Check` succeeded with ordinary validation. Enabling `GoFormat` +with an explicit empty shared-file list made the same check fail during size +enumeration on the unavailable blob for `nested/deep/placeholder`. That file +belongs to the excluded nested module and is neither a formatting input nor a +referenced destination. + +**Required change:** Separate full repository path/type/object metadata from +blob enrichment. Construct the agreed input set before acquiring required blob +sizes or contents, and inspect referenced destinations when a check actually +needs them. Keep full lookup available without requiring every unrelated blob +locally. Preserve selected-input ordering, boundaries, vendor/shared selection, +size semantics, local-only reads, cancellation, and reader ownership. + +**Verification:** Use a filtered sparse fixture with a complete selected module +and unavailable blobs in an excluded sibling or nested module. Representative +formatting, file-size, and explicit shared-file validation must work when their +required inputs are available. A selected missing blob or required referenced +destination must still produce the appropriate inspection error. Verify that +excluded unreferenced blobs cause neither content reads nor transport.