Record findings from the follow-up repository audit
This commit is contained in:
@@ -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.
|
||||
|
||||
Reference in New Issue
Block a user