diff --git a/docs/roadmap/remediation.md b/docs/roadmap/remediation.md new file mode 100644 index 0000000..4c174ae --- /dev/null +++ b/docs/roadmap/remediation.md @@ -0,0 +1,285 @@ +# Audit remediation roadmap + +## Status and baseline + +Proposed on 2026-11-01 against `899e7f1`. Implementation has not started. + +This roadmap consolidates all three findings in the +[functional correctness audit](correctness.md) and all five findings in the +[interface audit](interfaces.md). Those documents retain the reproduction +circumstances and evidence. This document owns the proposed corrections, +implementation sequence, and completion criteria. + +The baseline has separate orchestration and runtime facades, internal candidate, +validation, build, preparation, and publication services, and shared delivery +policy behind the neutral forge boundary. The corrections fit those existing +owners. The ordinary and race-enabled offline suites and compiled boundary +checks passed during the audits; they do not cover all identified defects. + +## Targeted end state + +- Incomplete authoritative tag observations retain an unknown publication + outcome after a failed push acknowledgment. +- Dependency preparation uses ordinary module discovery from its selected + directory despite an inherited or persistent `GOFLAGS=-modfile=...` setting. +- Candidate checking establishes that its derived release tag is a valid Git + ref before accepting it as an unused local tag. +- Cancellation during build source inspection remains identifiable through + the returned error chain. +- Asset operations classify a missing service-provided release identity as a + malformed observation. Existing page-only reads retain their current minimal + response requirements. +- Preparation path anchoring is explicit in the public documentation. The + recommended interpretation retains the supplied-directory anchor; the + alternative is discussed under Open Decisions. +- Delivery documentation accurately describes identity requirements for empty + prebuilt results and credential preflight before full artifact verification. + +These corrections preserve the existing public types, receipt schema, release +identity conventions, and publication/delivery model. Work stays focused on the +reported cases: Git supplies ref-name rules; local builds retain their current +filesystem choices; preparation retains network access and caller operational +settings; and page-only reads retain their current field requirements. Existing +source-verification observations and preflight evidence limits continue to +apply. + +## Finding coverage + +| Audit finding | Work unit | +| --- | --- | +| Correctness 3: incomplete reconciliation becomes conflict | 1 | +| Correctness 2: ambient module-file redirection | 2 | +| Correctness 1: invalid derived tag accepted by Check | 3 | +| Interfaces 1: source-inspection cancellation identity | 4 | +| Interfaces 2: unusable successful release handle | 5 | +| Interfaces 3: preparation repository anchor | 6 | +| Interfaces 4: empty-prebuilt GoDoc | 7 | +| Interfaces 5: hashing before every hosted request | 8 | + +## Implementation sequence + +Each unit includes its relevant validation and canonical documentation updates. +Complete the units as focused changes using the repository's ordinary commit +policy. Record completion here and update the corresponding audit finding's +status when its correction is verified. The sequence requires local repository +work and automated checks; it adds no pilot, intermediate release, or external +review step. + +### 1. Preserve uncertainty for incomplete remote tag observations + +**Status:** open. **Source:** correctness finding 3. + +**Owners:** `internal/git` ref parsing and `internal/publish` recovery, with CLI +outcome verification. + +Finalize tag observations after the entire query response has been parsed. +A peeled record without its exact tag-object record is an inconsistent or +unreadable observation. Use the existing ref-state/error vocabulary so +publication reconciliation retains `PublicationUnknown` and the original push +cause. Keep valid records usable in either order; a peeled record may precede +its exact tag record. + +**Completion evidence:** a local Git recovery fixture performs the intended +push, loses its acknowledgment, and supplies only the peeled record. The public +result is unknown, the cause remains inspectable, and the CLI uses status 3. +Existing exact-match, absent-tag, and different-object recovery outcomes still +work, along with complete records in either order. Query finalization retains +both genuinely absent results and valid exact-object-only results. + +### 2. Override the redirecting module-file flag during preparation + +**Status:** open. **Source:** correctness finding 2. + +**Owners:** `internal/gotool` dependency preparation and its CLI integration. + +Give `go mod download` an explicit empty `-modfile` argument. Command-line flag +precedence overrides an inherited or persistent alternate module-file selection +and restores Go's ordinary module discovery. A local Go 1.26.10 probe verified +this behavior from both a module root and its package subdirectory, with each +flag source. + +This is narrower than replacing all `GOFLAGS` or disabling the persistent Go +environment. Preserve the other effective flags and the current proxy, +private-module, authentication, checksum, cache, workspace, and local-toolchain +behavior. Preserve Go's existing checksum-update behavior and directory +selection; this unit adds no requirement for an immediate `go.mod` in a supplied +package directory. + +**Completion evidence:** exercise both inherited and persistent `-modfile` +settings against different module graphs. Preparation consumes the ordinary +selected graph, including when Go discovers its declaration above the working +directory. Verify the existing network/private-module/cache settings remain +available and that unrelated flags retain their behavior. Use test-owned +`GOMODCACHE` directories for every synthetic-module download. Keep the existing +binary CI preparation ordering and cancellation/error propagation covered. + +Document the preparation flag override in its GoDoc and the +[Go environment guide](../internal/gotool.md). Update command fixtures for the +intentional invocation change; use graph-selection behavior as the regression +assertion rather than relying only on argument matching. + +### 3. Validate derived tag names during candidate capture + +**Status:** open. **Source:** correctness finding 1. + +**Owners:** `internal/candidate` and the shared Git ref-validation boundary. + +Apply the existing Git ref-name check to the complete derived tag before local +tag absence is accepted during candidate capture. Keep name validity distinct +from the existence query: exit status 1 from `show-ref` alone cannot establish +both. Reuse the same rule at any candidate recheck boundary that needs it, +without duplicating Git's character rules in ReleaseKit. + +**Completion evidence:** public `Check` fails for derived tags containing the +reported space and `.lock` directory components, before validation callbacks +run. Ordinary root, nested, slash-containing, and supported major-version tags +still work. Preserve the captured identity in failure results and existing +publication guards. Standalone `Build` retains its current local-build behavior. + +Update candidate guidance only as needed to explain the local failure; this +unit introduces no new version syntax or blanket directory-name policy. + +### 4. Preserve cancellation during build source-state inspection + +**Status:** open. **Source:** interfaces finding 1. + +**Owner:** `cleanSource` in `internal/build/build.go`. + +When the Git status command fails and its operation context is canceled or has +expired, retain that context error in the returned chain, following the +neighboring build helpers or the existing process boundary. Ordinary Git errors +retain their current useful diagnostics. Keep the partial build result intact. + +**Completion evidence:** interrupt source inspection before compilation and +after completed artifacts exist. `errors.Is` identifies cancellation, and a +focused deadline case identifies deadline expiration. The latter build result +retains its completed artifacts. Retain existing managed-delivery cancellation +and temporary-output cleanup coverage; add an integration case only if the +helper-level/public-build coverage leaves a distinct propagation risk. + +### 5. Classify incomplete Gitea handles at asset use + +**Status:** open. **Source:** interfaces finding 2. + +**Owner:** Gitea backend handle interpretation in `internal/gitea/backend.go`. + +Keep `FindRelease` usable for page-only reads with the existing minimal response +fields. When an asset operation needs the positive release ID and a +backend-produced handle lacks it, return the existing malformed-response +classification with a useful missing-identity diagnostic. Distinguish this from +a handle of the wrong type, which remains an invalid request. Apply the +distinction consistently to listing and uploading; an upload rejected locally +retains not-attempted write evidence. + +**Completion evidence:** feed a lookup response with a missing, zero, or negative +ID through the backend. Page-only matching still succeeds under its existing +rules; binary asset use fails with the malformed classification before an +asset request. Wrong-type handles remain invalid requests, valid IDs continue +to work, and existing page/write-progress outcomes remain accurate. Retain +`TestBackendFindReleaseDoesNotRequireWriteResponseFields` or equivalent +behavioral coverage. Update the integration guide's error description as needed. + +### 6. Clarify dependency preparation's path anchor + +**Status:** open; targeted interpretation follows Open Decision 1. +**Source:** interfaces finding 3. + +**Owners:** `DependencyPreparationRequest` GoDoc and consumer preparation guidance; +selection code only if the alternative is chosen. + +Under the recommendation, document `RepositoryDir` as the supplied base +directory for preparation, `ModuleDir` as relative to that base, and empty +`ModuleDir` as selecting that base. Explain that callers sharing a location with +other orchestration APIs should supply the resolved checkout root when they +intend repository-root-relative selection. Keep standalone directory preparation +and the CLI's existing root normalization. + +**Completion evidence:** compare the wording and examples with current root, +nested, symlink-resolved, and standalone-directory behavior. Reuse existing +selection tests for a documentation-only correction. If anchoring behavior +changes instead, add behavioral coverage for calls from within a checkout and +for standalone preparation, including failed discovery behavior, and document +the changed interpretation. + +### 7. Clarify empty prebuilt results + +**Status:** open. **Source:** interfaces finding 4. + +**Owners:** `DeliveryRequest` GoDoc and +[delivery consumer guidance](../consumers/delivery.md#go-api). + +Describe a page-only prebuilt result as having no targets or artifacts while +still identifying the same repository, module, tag, commit, and version. +Present nil `Build` and nil `Prebuilt` as the ordinary page-only request. +Keep the existing identity checks and matching-empty-result behavior. + +**Completion evidence:** the exported GoDoc and guide agree with +`validateBuildIdentity`, and no wording implies that `&BuildResult{}` is accepted. +Review existing page-only and prebuilt identity coverage; this wording correction +does not require a new zero-result compatibility path or additional tests. + +### 8. Correct the documented artifact-verification order + +**Status:** open. **Source:** interfaces finding 5. + +**Owner:** the prebuilt paragraph in +[delivery consumer guidance](../consumers/delivery.md#go-api). + +Replace the claim that digest checks precede every hosted request with the +selected order: cheap local/source checks, read-only credential preflight, +full artifact verification, then hosted writes. Keep the concise statement that +local verification completes before hosted writes consistent across GoDoc and +current guidance. + +**Completion evidence:** affected descriptions agree with the implementation, +[ADR 0006](../adr/0006-neutral-forge-delivery-and-preflight.md), and existing +`TestAutomaticPreflightFailurePrecedesPrebuiltArtifactHashing` coverage. This +unit changes wording and preserves the current operation order. + +## Final verification and completion + +Run focused regression checks with each behavioral correction. Once the units +are complete, run the ordinary offline suite and compiled boundary check: + +```sh +GOTOOLCHAIN=local GOPROXY=off GOSUMDB=off go test ./... +GOTOOLCHAIN=local GOPROXY=off GOSUMDB=off go run ./internal/apicheck +``` + +Use race-enabled checks for affected subprocess/cancellation and adapter paths +where the change warrants them. The audit's existing full race-suite result is +baseline evidence; repeat broader checks when implementation changes or failures +justify it. Keep fixtures local, deterministic, and independent of real +credentials, with isolated synthetic module caches. + +Review public errors/results, CLI statuses, and canonical documentation against +the completion evidence above. Validate affected links and prose whitespace. +Mark this roadmap complete when all eight work units and their corresponding +audit findings have been addressed and verified, including the selected path-anchor +interpretation. The initial pass that creates this roadmap changes only this +document. + +## Open Decisions + +### 1. Dependency preparation's repository anchor + +**Recommendation:** retain the supplied `RepositoryDir` as the preparation base +and clarify its meaning in GoDoc and consumer guidance. This corrects the +ambiguity without changing existing selection behavior, preserves standalone +preparation, and keeps the current CLI experience. The tradeoff is that API +callers reusing a directory within a checkout must supply its resolved root +when composing root-relative preparation with `Check`, `Build`, or `Deliver`. +This is a recommended resolution, not a claim that the audit established a +requirement to retain that difference. + +**Viable alternative:** resolve the containing checkout root when the supplied +directory belongs to a Git checkout, and preserve supplied-directory preparation +for standalone use. This aligns ordinary checkout-based calls with the other +facades and reduces caller normalization. It changes the interpretation of +existing calls from checkout subdirectories and adds discovery behavior. The +implementation would need to distinguish standalone use from discovery failures +without silently treating every Git error as permission to use another anchor. +Adopting it would require focused selection tests and updated consumer guidance. + +A Git-checkout prerequisite is unnecessary for either correction; the current +standalone use remains part of the baseline considered by both choices.