Write the staged audit remediation plan
This commit is contained in:
436
docs/roadmap/implementation.md
Normal file
436
docs/roadmap/implementation.md
Normal file
@@ -0,0 +1,436 @@
|
||||
# Audit remediation implementation plan
|
||||
|
||||
## Baseline and execution
|
||||
|
||||
This plan targets `59bb074` on 2026-11-01. The worktree was clean when planning
|
||||
began. The production code still contains the findings recorded in
|
||||
[correctness.md](correctness.md) and [interfaces.md](interfaces.md).
|
||||
[remediation.md](remediation.md) defines the agreed behavior and scope; this
|
||||
plan defines how to reach it.
|
||||
|
||||
Luna should execute Stage 1 through Stage 10 in order in one end-to-end sprint.
|
||||
The stage boundaries divide implementation prompts. Intermediate stages may
|
||||
leave dependent code or tests incomplete; compilation at every boundary is not
|
||||
a requirement. Record deferred checks and carry them forward. Stage 10 requires
|
||||
the complete, coherent result. There are no intermediate release, live-service,
|
||||
operator-action, or human-review gates.
|
||||
|
||||
Read [development.md](../development.md) and the applicable policies before
|
||||
work. Use the repository's code graph for code discovery, with targeted source
|
||||
reads where the graph is incomplete; the build package may be excluded from
|
||||
its index. Follow the standing commit policy for completed reviewable units.
|
||||
Update stage status and evidence here as work progresses; a deferred regression
|
||||
check remains outstanding until it has run successfully.
|
||||
|
||||
## Implementation decisions
|
||||
|
||||
- Keep both public facades, public signatures, receipt schema, and package
|
||||
dependency direction. New helpers belong under the existing internal owners.
|
||||
- Resolve a checkout root automatically for preparation; retain standalone
|
||||
supplied-directory preparation. Git discovery supplies the root, while the
|
||||
existing selection resolver supplies relative-path, symlink, and containment
|
||||
behavior.
|
||||
- Override only alternate module-file selection with the explicit argument
|
||||
`-modfile=` on dependency downloads. Preserve other effective Go settings.
|
||||
Local Go 1.26.10 probes verified precedence over inherited and persistent
|
||||
`GOFLAGS` without preventing normal upward module discovery.
|
||||
- Represent a peeled-only remote ref as `RefConflict` after parsing the whole
|
||||
response. Existing publication reconciliation already maps that inconsistent
|
||||
observation to `PublicationUnknown`.
|
||||
- Classify a Gitea handle of the expected private type with a nonpositive ID as
|
||||
`forge.FailureMalformed` at asset use. Preserve minimal page-only lookup fields
|
||||
and wrong-type handle classification.
|
||||
- Keep empty-prebuilt identity checking and the existing preflight-first order.
|
||||
Their remediation is documentation, rather than a new request interpretation.
|
||||
|
||||
These decisions cover the reported cases. Preserve existing recovery bounds,
|
||||
write evidence, source observations, and preflight evidence limits. No new
|
||||
public options, directory-name policy, response-field requirements, or build
|
||||
reproducibility guarantees are part of this work.
|
||||
|
||||
## Coverage map
|
||||
|
||||
| Finding | Implementation stages |
|
||||
| --- | --- |
|
||||
| Correctness 3: incomplete remote tag reconciliation | 1 |
|
||||
| Interfaces 3: preparation path anchoring | 2, 3, 5 |
|
||||
| Correctness 2: ambient module-file redirection | 4, 5 |
|
||||
| Correctness 1: invalid derived candidate tag | 6 |
|
||||
| Interfaces 1: source-inspection cancellation | 7 |
|
||||
| Interfaces 2: incomplete Gitea release handle | 8 |
|
||||
| Interfaces 4: empty-prebuilt documentation | 9 |
|
||||
| Interfaces 5: documented hashing order | 9 |
|
||||
| Complete integration and evidence | 10 |
|
||||
|
||||
## Stage 1: Finalize remote ref observations and verify recovery
|
||||
|
||||
**Status:** pending.
|
||||
|
||||
**Files:** `internal/git/remote.go`, `internal/git/tag.go`, their tests,
|
||||
`publish_test.go`, and a focused CLI publication test under `cmd/releasekit/`.
|
||||
|
||||
1. Add a small ref-finalization helper beside `mergeRef`. After parsing a whole
|
||||
response, a `RefPresent` value with an empty `Object` and a populated `Peeled`
|
||||
becomes `RefConflict`. Retain its available fields for diagnostics.
|
||||
2. Apply finalization to the tag observation returned by both `Endpoint.Query`
|
||||
and `Endpoint.QueryTag`. Do not finalize after each record: a peeled record
|
||||
followed by its exact object is valid. Preserve existing duplicate-record and
|
||||
malformed-line handling.
|
||||
3. Keep `internal/publish.reconcilePush`'s existing treatment of `RefConflict`:
|
||||
unknown outcome, reconciliation phase, and the original push error retained.
|
||||
Its existing path should suffice once the observation is classified correctly;
|
||||
improve a diagnostic only if necessary to explain incomplete evidence.
|
||||
4. Extend the existing local Git wrapper/fixture in `publish_test.go`, starting
|
||||
with `TestPublishUninterpretableReconciliationRemainsUnknown` and
|
||||
`installGitWrapperWithMutationMode`. Add a mode that performs the real push,
|
||||
returns a failed acknowledgment, and supplies only the selected peeled tag
|
||||
record on the recovery query. Keep endpoint URL inspection functional and
|
||||
alter only the recovery response, not the pre-publication guards.
|
||||
5. Verify the public result is `PublicationUnknown` in
|
||||
`PublicationReconciliation`, retains `TagObject` and the push cause, and
|
||||
leaves the local tag intact. Independently inspect the local bare remote to
|
||||
prove the intended exact object was actually published.
|
||||
6. Exercise the CLI's `run` publication path with the same failure shape in a
|
||||
package-local fixture: status 3, unknown-state diagnostic, and no success
|
||||
message. Retain the current mapping; do not add a new exit category or shared
|
||||
test framework solely to share fixtures across packages.
|
||||
|
||||
**Verification:** exercise both query entry points and the following response
|
||||
shapes: empty response, exact object only, exact object plus peeled commit in
|
||||
both orders, peeled record only, and the already covered duplicate/conflicting
|
||||
records. Preserve successful recovery, confirmed absence, actual different-object
|
||||
conflict, and the existing single recovery query.
|
||||
|
||||
**Focused checks:** `go test ./internal/git`; the `TestPublish` tests at the root
|
||||
and CLI publication tests. Use the offline environment specified in Stage 10.
|
||||
|
||||
## Stage 2: Add optional checkout-root discovery with precise failure handling
|
||||
|
||||
**Status:** pending.
|
||||
|
||||
**Files:** `internal/git/source.go` or a focused companion file, Git tests, and
|
||||
`internal/process/process.go` only if an existing invocation capability proves
|
||||
insufficient. Its current stdout/stderr capture and environment overlay should
|
||||
already suffice.
|
||||
|
||||
1. Add an internal Git function such as
|
||||
`DiscoverRepositoryRoot(ctx, directory) (root string, found bool, err error)`.
|
||||
Success returns a canonical containing checkout root and `found=true`.
|
||||
Confirmed standalone use returns `found=false` and nil error. Operational
|
||||
failures return their wrapped cause.
|
||||
2. Use Git's own `rev-parse --show-toplevel` discovery, preserving normal Git
|
||||
environment and worktree behavior. Capture stdout and stderr separately
|
||||
through `internal/process`. Apply `LC_ALL=C` to this child only so the known
|
||||
no-repository diagnostics can be recognized without depending on operator
|
||||
locale. Do not change process-global locale or configuration.
|
||||
3. Git uses exit status 128 for both absence and other failures. Classify absence
|
||||
only when that status accompanies its recognized no-parent-repository
|
||||
diagnostic: `fatal: not a git repository (or any of the parent directories): .git`,
|
||||
or the form beginning `fatal: not a git repository (or any parent up to mount point `
|
||||
and ending `)`, followed by
|
||||
`Stopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not set).`.
|
||||
Match the fixed text around the variable mount path. Keep the
|
||||
classifier small, private, and covered by diagnostic samples. Do not accept
|
||||
every message starting with `not a git repository` as standalone: an invalid
|
||||
explicit `GIT_DIR` has a different diagnostic.
|
||||
4. Treat cancellation, inaccessible working directories, broken Git metadata,
|
||||
ownership errors, bare-repository worktree errors, and unrecognized
|
||||
diagnostics as errors. Preserve `errors.Is`/`errors.As` causes and useful
|
||||
stderr. An arbitrary error is never permission to choose another base.
|
||||
5. Reuse the existing successful-root normalization with `filepath.EvalSymlinks`.
|
||||
Keep strict `RepositoryRoot`'s current public-to-internal behavior: its
|
||||
callers still require a checkout. Share private mechanics where useful while
|
||||
retaining the original error cause for that strict operation.
|
||||
6. Preserve the existing Go-only standalone use when the Git executable is
|
||||
unavailable. Only for `exec.ErrNotFound`, check the supplied physical
|
||||
directory and its ancestors for `.git` entries with standard filesystem
|
||||
operations. With no such entry and no explicit `GIT_DIR` or `GIT_WORK_TREE`,
|
||||
return confirmed standalone use. If a marker or explicit Git context exists,
|
||||
return the missing-Git error; checkout discovery is unavailable. Propagate
|
||||
filesystem inspection failures instead of treating them as absence. This
|
||||
limited presence check establishes standalone use only; Git remains the
|
||||
authority for selecting an actual checkout root. Do not apply this fallback
|
||||
to the strict `RepositoryRoot` function.
|
||||
|
||||
**Verification:** real temporary repositories for root/subdirectory discovery,
|
||||
filesystem aliases, and an independent worktree with a `.git` file; real
|
||||
standalone directories for absence. Use controlled Git-process failures for
|
||||
permission/ownership errors and cancellation, rather than relying on host user
|
||||
privileges. Test both no-repository diagnostic forms and a status-128 failure
|
||||
that must remain an error. A broken `.git` file must not trigger standalone
|
||||
fallback. Existing strict-root tests continue to pass.
|
||||
Also cover a Go-only executable path in a marker-free standalone directory:
|
||||
preparation can still run there. With that same executable path inside a
|
||||
checkout, discovery fails with the missing-Git cause before Go runs.
|
||||
|
||||
**Focused check:** `go test ./internal/git`.
|
||||
|
||||
## Stage 3: Apply checkout-root selection to dependency preparation
|
||||
|
||||
**Status:** pending. **Depends on:** Stage 2.
|
||||
|
||||
**Files:** `preparation.go`, `preparation_test.go`, and
|
||||
`docs/consumers/delivery.md`'s preparation guidance.
|
||||
|
||||
1. Retain the existing rejection of an empty/blank `RepositoryDir`.
|
||||
2. Discover the optional checkout root from the supplied directory. If found,
|
||||
pass that root and the request's unchanged `ModuleDir` into
|
||||
`releaseidentity.ResolveSelection`. Otherwise pass the supplied directory
|
||||
and `ModuleDir` into that same resolver. Propagate discovery failures before
|
||||
invoking Go.
|
||||
3. Continue calling `internal/preparation.PrepareBuildDependencies` with the
|
||||
selected absolute directory. Preserve the current selection resolver's
|
||||
handling of symlinks before parent traversal and its containment checks.
|
||||
4. Leave CLI root normalization and `runCIDelivery`'s preflight/preparation order
|
||||
in place. They already supply the containing root; no new CLI flag or profile
|
||||
field is needed.
|
||||
5. Update request/function GoDoc and the preparation guide: checkout-relative
|
||||
selection inside Git, supplied-base selection for standalone use, and the
|
||||
changed interpretation of calls from checkout subdirectories.
|
||||
|
||||
**Verification:** extend `TestPrepareBuildDependenciesUsesSelectedModuleDirectory`
|
||||
and related public tests. Include checkout root and child-directory inputs with
|
||||
empty and explicit nested `ModuleDir`, standalone selection, symlink/`link/..`
|
||||
selection, and discovery failure before any Go invocation. Use the existing Go
|
||||
recording shim to observe the chosen working directory. Keep the current
|
||||
standalone tests, which intentionally have no Git repository; do not introduce
|
||||
an immediate-go.mod requirement just to simplify fixtures. Stage 5 verifies
|
||||
composition with ordinary build identity and ambient flags.
|
||||
|
||||
**Focused checks:** public `TestPrepareBuildDependencies` tests and existing CLI
|
||||
`ci-deliver` preparation tests.
|
||||
|
||||
## Stage 4: Override alternate module-file selection during downloads
|
||||
|
||||
**Status:** pending.
|
||||
|
||||
**Files:** `internal/gotool/gotool.go`, `internal/gotool/gotool_test.go`,
|
||||
`preparation.go` GoDoc, and `docs/internal/gotool.md`.
|
||||
|
||||
1. Change the download argument vector to
|
||||
`go mod download -modfile=`. The explicit empty flag overrides the alternate
|
||||
module file selected through `GOFLAGS` and restores ordinary discovery.
|
||||
2. Keep the existing `GOWORK=off` and `GOTOOLCHAIN=local` overlays. Leave other
|
||||
effective flags, proxy/private-module/checksum/authentication/cache settings,
|
||||
and persistent Go configuration available to this network-capable operation.
|
||||
3. Preserve combined diagnostics, wrapped command/cancellation errors, and
|
||||
permission for the Go command to update its normal `go.sum`.
|
||||
4. Update `TestPrepareBuildDependenciesPreservesPreparationEnvironment`'s
|
||||
recording fixture for the intentional argument change and retain its
|
||||
environment assertions. Keep command failure and cancellation tests.
|
||||
5. Document only this preparation-specific flag override. Build/validation
|
||||
environments and caller-controlled bootstrap commands keep their current
|
||||
responsibilities. Actual graph-selection regression evidence follows in
|
||||
Stage 5; argument recording alone is insufficient.
|
||||
|
||||
**Focused check:** `go test ./internal/gotool`.
|
||||
|
||||
## Stage 5: Verify selected-module preparation and build composition
|
||||
|
||||
**Status:** pending. **Depends on:** Stages 3 and 4.
|
||||
|
||||
**Files:** `preparation_test.go` and small test-local helpers beside it. The
|
||||
existing local module-proxy construction in `cmd/releasekit/woodpecker_test.go`
|
||||
can inform fixture layout, but its package-local helpers cannot be imported.
|
||||
|
||||
1. Create a real local repository with root and nested Go modules using distinct
|
||||
synthetic dependency graphs, plus an alternate dependency-free module file.
|
||||
Serve the synthetic dependency metadata and archives through a temporary
|
||||
file proxy or `httptest` server. Use the standard library and a small fixture,
|
||||
not a new general-purpose testing package.
|
||||
2. For each preparation attempt, supply a fresh test-owned `GOMODCACHE` and
|
||||
fixture-scoped proxy/checksum/private-module settings. Ensure that a synthetic
|
||||
`example.invalid` module cannot escape to a public proxy or VCS lookup.
|
||||
3. Cover `GOFLAGS=-modfile=...` from the inherited environment and from a
|
||||
temporary `GOENV` file with no process override. Do not treat `GOFLAGS=""`
|
||||
as sufficient isolation from persistent settings. The audit demonstrated
|
||||
that Go can still use the persistent value.
|
||||
4. Call public `PrepareBuildDependencies` from checkout subdirectories for both
|
||||
default root selection and explicit nested selection. Prove that the chosen
|
||||
module's dependency was downloaded into its owned cache and that the
|
||||
alternate graph did not satisfy the request. Retain a local negative control
|
||||
showing the ordinary uncorrected Go command would select the alternate file.
|
||||
5. Call public `Build` with the same `RepositoryDir` and `ModuleDir`, using an
|
||||
empty target list, and compare its repository/module identity with the graph
|
||||
prepared above. This checks path composition without adding compiler or
|
||||
buildinfo dependencies to the synthetic proxy. Existing binary build and CI
|
||||
tests remain the owners of actual compilation/version-injection behavior.
|
||||
6. Add a standalone package-subdirectory case proving Go still discovers its
|
||||
parent declaration, and retain an unrelated effective flag such as `-x` in
|
||||
a representative case to prove the fix did not replace all caller flags.
|
||||
Keep changed checksum files within the disposable fixture.
|
||||
|
||||
**Verification:** observed dependency metadata/cache files and public build
|
||||
identity are the assertions. Avoid only checking command counts or argument
|
||||
spelling. Inspect fixture subprocess settings to ensure every synthetic module
|
||||
consumption uses an owned cache.
|
||||
|
||||
**Focused checks:** the new root preparation tests and existing CLI binary
|
||||
`ci-deliver` tests. Preserve their credential-before-download sequencing.
|
||||
|
||||
## Stage 6: Reject invalid derived candidate tags before validation
|
||||
|
||||
**Status:** pending.
|
||||
|
||||
**Files:** `internal/git/remote.go` and its current tag-helper callers/tests,
|
||||
`internal/candidate/check.go`, public candidate tests, and candidate guidance
|
||||
only where its explanation needs adjustment.
|
||||
|
||||
1. Make the existing internal Git `checkTagRef` helper available to the candidate
|
||||
package as `git.CheckTagRef`, updating its internal callers and GoDoc. Retain
|
||||
the existing `git check-ref-format refs/tags/<tag>` behavior. This is an
|
||||
internal-package export, not a new facade API.
|
||||
2. In `candidate.Capture`, validate the fully derived tag before accepting its
|
||||
absence through `hasLocalTag`. Return a contextual local-candidate error
|
||||
while retaining the captured module/tag identity. A malformed derived ref is
|
||||
not an `ErrInvalidVersion` syntax failure and needs no new public error type.
|
||||
3. Keep name validity separate from `show-ref`'s existence result. The captured
|
||||
tag is then immutable across ordinary rechecks; reuse the shared validator
|
||||
if another entry path needs it, without adding a separate character parser.
|
||||
4. Keep working-tree identity derivation and standalone `Build` behavior intact.
|
||||
Do not reject module directories globally or invent alternate tag encodings.
|
||||
|
||||
**Verification:** public `Check` rejects the reported `module space` and
|
||||
`module.lock` cases before Go validation and additional/build callbacks. Keep
|
||||
ordinary root/nested/major-version and slash-containing tags accepted when
|
||||
otherwise valid. A Go marker shim can prove validation did not start. Existing
|
||||
publication guard and local-tag conflict tests continue to cover those paths.
|
||||
|
||||
**Focused checks:** `go test ./internal/git ./internal/candidate` and the public
|
||||
candidate regression tests.
|
||||
|
||||
## Stage 7: Preserve build source-inspection cancellation causes
|
||||
|
||||
**Status:** pending.
|
||||
|
||||
**Files:** `internal/build/build.go` and focused public tests in `build_test.go`
|
||||
or a companion root test file.
|
||||
|
||||
1. In `cleanSource`, when the Git command fails, inspect `ctx.Err()` as neighboring
|
||||
`gitCommit` and `checkVersion` already do. Wrap the context error when present;
|
||||
otherwise retain the Git command failure. Keep the existing diagnostic
|
||||
context and partial result behavior. No new public error type is needed.
|
||||
2. Reuse `newBuildConsumer` and the existing cancellation fixture style. Add a
|
||||
Git wrapper that delegates ordinary operations and signals when the selected
|
||||
build-time `status` invocation is entered before blocking. Use an explicit
|
||||
readiness signal to cancel; do not rely on a guessed startup sleep.
|
||||
3. Cover cancellation in the initial inspection, which needs no compilation,
|
||||
and in the final inspection after a host-target binary has been built and
|
||||
smoke-checked. Verify `errors.Is(err, context.Canceled)` and retention of
|
||||
completed artifact paths/digests for the latter case.
|
||||
4. Add one focused deadline-expiration variant with synchronized readiness and
|
||||
a bounded wait. Verify `context.DeadlineExceeded`, rather than asserting an
|
||||
exact killed-process error string. Join test goroutines and reap children.
|
||||
|
||||
**Verification:** ordinary source-inspection Git errors retain their causes;
|
||||
`SourceVerified` is not reported true after an interrupted build. Retain
|
||||
`TestBuildCancellationReturnsCompletedArtifacts` and managed-delivery temporary
|
||||
cleanup coverage. Add another delivery test only if a distinct propagation or
|
||||
cleanup risk remains uncovered.
|
||||
|
||||
**Focused checks:** root build/cancellation tests and affected managed-delivery
|
||||
cancellation tests; run these with the race detector after they are coherent.
|
||||
|
||||
## Stage 8: Classify missing Gitea release IDs at asset use
|
||||
|
||||
**Status:** pending.
|
||||
|
||||
**Files:** `internal/gitea/backend.go`, its tests, a focused root delivery test,
|
||||
and `docs/integrations/gitea.md`.
|
||||
|
||||
1. Split handle validation in `ListAssets` and `UploadAsset`: a failed private
|
||||
`releaseHandle` type assertion remains `forge.FailureInvalid`; a correctly
|
||||
typed handle with `id <= 0` becomes `forge.FailureMalformed` with a useful
|
||||
missing-release-identity diagnostic. A small private validation helper is
|
||||
sufficient if it avoids duplicating this distinction.
|
||||
2. Perform these checks before making an asset HTTP request. For a locally
|
||||
rejected upload, return `Attempted=false` and `WriteNotAttempted` exactly as
|
||||
other local upload failures do. Do not pass a bad ID into the lower client
|
||||
and reclassify its invalid-request error afterward.
|
||||
3. Keep `FindRelease`'s minimal read response and `CreateRelease`'s existing
|
||||
positive-ID acknowledgment check. No extra lookup, retry, public failure
|
||||
type, handle-provenance framework, or mandatory page URL is needed.
|
||||
4. Drive lookup responses with missing, zero, and negative IDs through the real
|
||||
backend into both asset operations. Assert malformed neutral failures and
|
||||
absence of asset requests. Test wrong-type handles and valid IDs as controls.
|
||||
5. Retain `TestBackendFindReleaseDoesNotRequireWriteResponseFields`. Add a paired
|
||||
public delivery regression using the existing root fixture: the same matching
|
||||
existing page without an ID completes page-only delivery, while a request
|
||||
with a prebuilt artifact fails asset use with `DeliveryNotAttempted` and no
|
||||
new hosted writes. Preserve existing page creation progress and write-recovery
|
||||
behavior.
|
||||
|
||||
**Focused checks:** `go test ./internal/gitea ./internal/delivery` and affected
|
||||
root delivery tests. Describe the corrected observation classification in the
|
||||
integration guide without strengthening page-only read requirements.
|
||||
|
||||
## Stage 9: Align delivery documentation with existing behavior
|
||||
|
||||
**Status:** pending.
|
||||
|
||||
**Files:** `delivery.go` GoDoc and `docs/consumers/delivery.md`; review related
|
||||
current descriptions for consistency. This stage changes documentation only.
|
||||
|
||||
1. Change `DeliveryRequest` GoDoc to distinguish a matching prebuilt result with
|
||||
no targets/artifacts from the zero `BuildResult`. A supplied result still
|
||||
needs matching repository, module directory/path, tag, commit, and version.
|
||||
Binary-only source/artifact requirements remain conditional on binary work.
|
||||
2. Present nil `Build` and nil `Prebuilt` as the ordinary page-only API use.
|
||||
Preserve matching-empty-result behavior, current identity validation, and
|
||||
mutual exclusion of managed and prebuilt sources. Add no compatibility shim.
|
||||
3. In the prebuilt consumer paragraph, replace verification before every hosted
|
||||
request with verification before hosted writes. Describe the current order:
|
||||
cheap local/source checks, read-only preflight, full local verification,
|
||||
then hosted writes. Keep available-access-evidence limits clear.
|
||||
4. Review preparation wording delivered in Stages 3 and 4, plus current CLI and
|
||||
integration descriptions, for agreement. Update only canonical owners that
|
||||
actually need correction. Accepted ADR decision content remains intact.
|
||||
|
||||
**Verification:** compare documentation with `validateBuildIdentity`, automatic
|
||||
preflight, and existing page-only/prebuilt tests. Reuse
|
||||
`TestAutomaticPreflightFailurePrecedesPrebuiltArtifactHashing`. These wording
|
||||
fixes require no new tests that merely repeat existing implementation behavior.
|
||||
Validate affected local links/fragments and prose whitespace.
|
||||
|
||||
## Stage 10: Verify the complete sprint and close the findings
|
||||
|
||||
**Status:** pending. **Depends on:** all preceding stages.
|
||||
|
||||
1. Resolve every deferred check, format changed Go files, and inspect the final
|
||||
diff for changes outside the agreed remediation scope. Verify all eight
|
||||
findings against the coverage map and target behavior in `remediation.md`.
|
||||
2. Run the complete offline suite and compiled API/dependency-boundary check:
|
||||
|
||||
```sh
|
||||
GOTOOLCHAIN=local GOPROXY=off GOSUMDB=off go test ./...
|
||||
GOTOOLCHAIN=local GOPROXY=off GOSUMDB=off go run ./internal/apicheck
|
||||
```
|
||||
|
||||
3. Run race-enabled tests for the changed Git/subprocess, Go preparation,
|
||||
Gitea/delivery, and public cancellation paths. A single
|
||||
`GOTOOLCHAIN=local GOPROXY=off GOSUMDB=off go test -race ./...` is an acceptable
|
||||
combined check; targeted race runs are sufficient when they cover the affected
|
||||
paths. Repeat tests only when changes, failures, or unresolved concerns
|
||||
justify it.
|
||||
4. Confirm public types and receipt schema are unchanged; strict root lookup
|
||||
still requires a checkout; standalone preparation still works; default and
|
||||
explicit module selections agree across preparation/build; and publication
|
||||
uncertainty and write evidence retain their selected meanings.
|
||||
5. Review fixture isolation, cancellation readiness/cleanup, useful error chains,
|
||||
and CLI status assertions. Keep synthetic module caches test-owned and live
|
||||
infrastructure out of default tests.
|
||||
6. Validate changed Markdown links/fragments and run the documented whitespace
|
||||
checks. Review current consumer/GoDoc/integration owners together so the
|
||||
delivered behavior has one consistent description.
|
||||
7. Mark verified stage statuses complete here and add completion markers with
|
||||
relevant test/commit evidence to the corresponding findings in
|
||||
`correctness.md` and `interfaces.md`. Retain their audit evidence. Set
|
||||
`remediation.md`'s status to complete only after every finding is addressed.
|
||||
8. Follow the repository's normal commit discipline for completed work and
|
||||
report the implementation changes, validation, remaining limitations, and
|
||||
worktree status. No tag, push, hosted release, or pilot is part of this plan.
|
||||
|
||||
Completion means the final repository implements the agreed target, the
|
||||
regressions and existing relevant checks pass, and every finding has verified
|
||||
completion evidence. No unresolved product or policy question remains from the
|
||||
planning review.
|
||||
@@ -1,269 +1,108 @@
|
||||
# Audit remediation roadmap
|
||||
|
||||
## Status and baseline
|
||||
## Status and scope
|
||||
|
||||
Proposed on 2026-11-01 against `899e7f1`. Implementation has not started.
|
||||
Agreed remediation target as of 2026-11-01. Implementation is pending against
|
||||
baseline `59bb074`.
|
||||
|
||||
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.
|
||||
This roadmap defines the desired behavior and scope for all three findings in
|
||||
the [functional correctness audit](correctness.md) and all five findings in the
|
||||
[interface audit](interfaces.md). Those documents retain their reproduction
|
||||
circumstances and evidence. The [implementation plan](implementation.md) owns
|
||||
staging, code changes, regression checks, and completion tracking.
|
||||
|
||||
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.
|
||||
## Purpose and architectural scope
|
||||
|
||||
## Targeted end state
|
||||
ReleaseKit owns routine release mechanics so consumers supply product facts and
|
||||
product-specific checks with little release-related code. As recorded in
|
||||
[ADR 0002](../adr/0002-routine-releases.md), internal complexity is justified
|
||||
when it reduces caller-owned release work. In particular, callers can reuse a
|
||||
repository location across preparation and subsequent orchestration without
|
||||
normalization code or another configuration option.
|
||||
|
||||
- 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 resolves the containing checkout root automatically, matching
|
||||
other orchestration operations. Standalone preparation retains the supplied
|
||||
directory as its base.
|
||||
- Delivery documentation accurately describes identity requirements for empty
|
||||
prebuilt results and credential preflight before full artifact verification.
|
||||
The existing public orchestration and runtime facades and internal package
|
||||
boundaries remain appropriate. These corrections use the existing public types,
|
||||
receipt schema, module/tag conventions, and publication/delivery model.
|
||||
|
||||
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.
|
||||
## Desired behavior
|
||||
|
||||
## Finding coverage
|
||||
### Incomplete publication observations
|
||||
|
||||
| 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 |
|
||||
After a failed push acknowledgment, a query containing only a peeled tag record
|
||||
provides insufficient evidence about the exact annotated tag object.
|
||||
Publication remains unknown and retains the original push cause. A complete
|
||||
response works in either record order. Existing exact-object matches, genuinely
|
||||
absent tags, different objects, and exact-object-only responses retain their
|
||||
current outcomes.
|
||||
|
||||
## Implementation sequence
|
||||
### Dependency preparation and ambient module files
|
||||
|
||||
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.
|
||||
Preparation uses ordinary Go module discovery from its selected directory even
|
||||
when inherited or persistent `GOFLAGS` selects an alternate module file. It
|
||||
retains other effective flags and existing proxy, private-module,
|
||||
authentication, checksum, cache, workspace, and local-toolchain behavior.
|
||||
Go retains its existing checksum-update behavior, including upward declaration
|
||||
discovery for standalone package-directory preparation.
|
||||
|
||||
### 1. Preserve uncertainty for incomplete remote tag observations
|
||||
### Valid candidate tags
|
||||
|
||||
**Status:** open. **Source:** correctness finding 3.
|
||||
Candidate checking establishes that the complete derived release tag is a valid
|
||||
Git ref before accepting it as unused. Git's own ref rules remain the authority.
|
||||
The correction concerns release candidate validity; standalone local builds
|
||||
retain their existing filesystem choices and version rules.
|
||||
|
||||
**Owners:** `internal/git` ref parsing and `internal/publish` recovery, with CLI
|
||||
outcome verification.
|
||||
### Identifiable build cancellation
|
||||
|
||||
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.
|
||||
Cancellation or deadline expiration during build source inspection remains
|
||||
identifiable through the error chain. Ordinary Git failures retain useful
|
||||
diagnostics, and partial results retain already completed artifacts.
|
||||
|
||||
**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.
|
||||
### Gitea release identity at asset use
|
||||
|
||||
### 2. Override the redirecting module-file flag during preparation
|
||||
A backend-produced handle with a missing or nonpositive service release ID is a
|
||||
malformed observation when an asset operation needs that ID. A handle of the
|
||||
wrong type remains an invalid request. Locally rejected uploads retain
|
||||
not-attempted write evidence.
|
||||
|
||||
**Status:** open. **Source:** correctness finding 2.
|
||||
Existing page-only reads retain their current minimal response requirements.
|
||||
This correction adds no universal positive-ID or page-URL requirement to a
|
||||
successful release lookup and preserves existing delivery progress semantics.
|
||||
|
||||
**Owners:** `internal/gotool` dependency preparation and its CLI integration.
|
||||
### Consistent preparation paths
|
||||
|
||||
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.
|
||||
Within a Git checkout, preparation resolves its containing root automatically
|
||||
and interprets `ModuleDir` relative to that root. Empty selection means the
|
||||
root module, consistently with `Check`, `Build`, and `Deliver`. This includes
|
||||
calls made from checkout subdirectories and filesystem aliases.
|
||||
|
||||
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. Directory selection follows the checkout-root resolution in unit 6;
|
||||
within the resolved selection, preserve Go's existing checksum-update behavior
|
||||
and ordinary module discovery. Standalone package-directory preparation retains
|
||||
upward declaration discovery.
|
||||
Outside a Git checkout, preparation retains the supplied directory as its base
|
||||
and the existing relative module selection. Standalone use remains supported.
|
||||
It retains its existing Go-only use when Git is unavailable and the directory
|
||||
can be identified as outside a checkout.
|
||||
Discovery distinguishes this case from cancellation, permission failures, and
|
||||
other Git errors, which remain failures. Existing symlink and containment rules
|
||||
and the CLI's root normalization continue to apply.
|
||||
|
||||
**Completion evidence:** exercise both inherited and persistent `-modfile`
|
||||
settings against different module graphs. Preparation consumes the ordinary
|
||||
selected graph, including standalone use where 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.
|
||||
### Empty prebuilt results
|
||||
|
||||
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.
|
||||
Page-only callers ordinarily omit both `Build` and `Prebuilt`. A supplied
|
||||
prebuilt result with no targets or artifacts still identifies the matching
|
||||
repository, module, tag, commit, and version. Documentation states that
|
||||
requirement clearly. A zero `BuildResult` gains no new compatibility behavior.
|
||||
|
||||
### 3. Validate derived tag names during candidate capture
|
||||
### Artifact-verification order
|
||||
|
||||
**Status:** open. **Source:** correctness finding 1.
|
||||
Documentation consistently describes cheap local/source checks, read-only
|
||||
credential preflight, full local artifact verification, and then hosted writes.
|
||||
Digest verification completes before hosted writes. Credential preflight can
|
||||
precede that verification and confirms only the access evidence exposed by the
|
||||
backend.
|
||||
|
||||
**Owners:** `internal/candidate` and the shared Git ref-validation boundary.
|
||||
## Scope boundaries
|
||||
|
||||
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. Resolve preparation's checkout root automatically
|
||||
|
||||
**Status:** open. **Source:** interfaces finding 3.
|
||||
|
||||
**Owners:** the preparation facade, shared Git/selection boundaries,
|
||||
`DependencyPreparationRequest` GoDoc, and consumer preparation guidance.
|
||||
|
||||
Resolve the containing checkout root from `RepositoryDir` before applying
|
||||
`ModuleDir`. Within a checkout, interpret `ModuleDir` relative to that root;
|
||||
empty selection means the root module, consistently with `Check`, `Build`, and
|
||||
`Deliver`. For standalone use outside a Git checkout, retain the supplied
|
||||
directory as the base and apply the existing relative module selection there.
|
||||
|
||||
Keep this normalization inside ReleaseKit so callers can reuse a repository
|
||||
location across operations without normalization code or a new setting. This
|
||||
applies [ADR 0002's priority](../adr/0002-routine-releases.md) of minimizing
|
||||
caller-owned release work. Retain the CLI's existing root normalization and
|
||||
shared symlink/containment rules. Distinguish standalone use from discovery
|
||||
failures; cancellation, permission failures, and other Git errors remain
|
||||
errors rather than silently selecting another base.
|
||||
|
||||
**Completion evidence:** cover calls from the checkout root and its
|
||||
subdirectories, default and explicit nested-module selections, symlink-resolved
|
||||
paths, standalone directories, and discovery failures. Include combined coverage
|
||||
with unit 2 showing that preparation and subsequent orchestration select the
|
||||
same module from the same repository location and `ModuleDir`, even with an
|
||||
ambient alternate module file. Preserve standalone package-directory discovery
|
||||
and the existing CLI behavior. Update GoDoc and consumer guidance to describe
|
||||
the selected interpretation and the change for checkout-subdirectory callers.
|
||||
|
||||
### 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.
|
||||
The corrections retain the current source-verification observations, bounded
|
||||
publication recovery, explicit forge selection, and additive hosted delivery
|
||||
policy. They introduce no new public configuration knobs, compatibility shims,
|
||||
release procedure, pilot release, or intermediate human-review requirement.
|
||||
The targeted sprint reaches this complete end state through local repository
|
||||
work and automated validation.
|
||||
|
||||
Reference in New Issue
Block a user