From ceb7e2847a7d8d62cdfc3370a8f0d6cb0dc3daf0 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Tue, 6 Oct 2026 23:43:58 +0000 Subject: [PATCH] Record follow-up audit findings --- docs/roadmap/remediation.md | 134 ++++++++++++++++++++++++++++++++++++ 1 file changed, 134 insertions(+) create mode 100644 docs/roadmap/remediation.md diff --git a/docs/roadmap/remediation.md b/docs/roadmap/remediation.md new file mode 100644 index 0000000..0983128 --- /dev/null +++ b/docs/roadmap/remediation.md @@ -0,0 +1,134 @@ +# ReleaseKit audit follow-up review + +Date: 2026-10-06. +Reviewed source: `55333b0`. +Status: Two findings require remediation. + +## Scope and assessment + +Reviewed the changes since `536a579` against the [audit](audit.md), completed +[implementation plan](implementation.md), [architecture policy](../policy/architecture.md), +and [routine-release decision](../adr/0002-routine-releases.md). The worktree +was clean at the start of the review. + +The selected architecture is substantially implemented. One Go module exposes +the root orchestration facade and lightweight `buildinfo` runtime facade. +Internal packages own candidate validation, Git publication and recovery, +Go environments, artifact declarations, managed delivery builds, Gitea effects, +and explicit CI preparation. The CLI adapts profiles, credentials, output, and +exit statuses. Standard validation precedes additive product checks; ordinary +delivery uses one facade-owned build-and-deliver operation. The maintained +consumer workflow invokes the shared versioned CI command. + +The audit's F1–F8 changes are present, with behavioral tests for their selected +contracts. R1 below identifies a new defect in the shared Go-environment owner; +R2 identifies increased test cost and coupling introduced by additive validation. +The review found no additional public-type ownership or dependency-direction +gap. The deliberate additive-check, baseline-build, managed-delivery, and CI +preparation decisions remain appropriate. The findings call for implementation +repairs within those decisions. + +## R1 — Medium: Go discovery treats successful diagnostics as invalid configuration + +Status: Open. +Category: Correctness and an accidental environment prerequisite. +Introduced by: `0809312`. + +**Evidence:** [Go configuration discovery](../../internal/gotool/gotool.go) +routes both stdout and stderr into the same buffer in `discover` (lines +189–205). `New` parses that combined output as JSON (lines 59–81). A successful +`go env -json` invocation with a warning therefore becomes a decoding failure. +This affects candidate validation and nonempty builds through their shared owner. + +**Reproduction:** Create a clean `main` repository with a Go 1.25 module, a +passing test, a matching committed note, and a source-only product profile. +Set `GOPATH` to the installed Go toolchain's `GOROOT`. On Go 1.26.8, +`go test -mod=readonly -count=1 ./...` succeeds with a warning on stderr. +`releasekit check --repo DIR v1.2.3` fails before running that suite: + +```text +prepare Go toolchain: decode Go host settings: invalid character 'w' looking for beginning of value +``` + +The warning-producing configuration is discouraged by Go, but the Go command +accepts it. ReleaseKit's selected environment contract preserves effective +operational settings; it establishes no requirement that successful discovery +produce an empty diagnostic stream. The same decoding failure follows from any +successful discovery command that emits stderr alongside valid stdout JSON. + +**Required remediation:** Capture configuration stdout and diagnostics +separately. Parse stdout as JSON and retain stderr for command-failure reporting +or ordinary diagnostics. Preserve subprocess error identity, cancellation, +effective operational settings, and the selected validation/build environments. + +**Focused verification:** A controlled Go-command fixture returns valid JSON on +stdout, a diagnostic on stderr, and exit 0; discovery succeeds and preserves +the settings. A failing command retains useful stderr and its cause. Existing +ambient/persistent flag, host-target, cache, and architecture-tuning regressions +continue to pass. These checks add no warning-free configuration requirement. + +## R2 — Medium: Wrapper regression testing repeats the project suite inside itself + +Status: Open. +Category: Test maintainability and development cost. +Introduced by: `3e9b5c1`. + +**Evidence:** [The wrapper regression](../../cmd/releasekit/release_wrapper_test.go) +clones the complete repository for each exit-status scenario, overlays a +hand-maintained list of production and test files (lines 32–45), and invokes +the source publication wrapper. The success, rejected-push, and unknown-push +scenarios each run the project's default suite inside the parent suite. +An inherited `RELEASEKIT_SKIP_WRAPPER_TEST` flag prevents recursive execution +of this one test, while the other publication, delivery, build, and workflow +tests run again in each clone. + +During this review, process inspection confirmed the nested native publisher, +`go test ./...`, and cloned test binaries. The uncached offline suite passed; +the root package took 40.751 seconds and the CLI package took 115.533 seconds +on this host. These timings are observations, not proposed test-duration limits. + +The extra executions provide little distinct evidence about wrapper exit-code +preservation. The file overlay also couples the fixture to unrelated package +changes and supplies only selected uncommitted files, so development checks can +exercise a mixture of the current worktree and committed source. + +**Required remediation:** Keep the real source-bootstrap wrapper and local Git +exit-status scenarios, while giving their candidate validation a small, +self-contained test fixture. Assemble the tool source consistently from the +version being tested. Replace the manually maintained production/test overlay +and suite-recursion skip mechanism with a fixture that exercises wrapper +behavior without rerunning ReleaseKit's complete test suite. Retain the actual +publication and recovery behavior already protected by this regression. + +**Focused verification:** The real wrapper still preserves success 0, known +failure 1, invalid input 2, and uncertain publication 3. The candidate fixture +runs its standard suite, and captured-endpoint recovery still finds the +intended object with separate fetch and push repositories. The outer default +suite supplies the project's broader behavioral evidence once. Use test scope +and command observations for verification rather than a wall-clock gate. + +## Validation and remaining scope + +The following checks passed on the reviewed source with Go 1.26.8 on +Linux/amd64: + +```sh +GOWORK=off GOPROXY=off GONOPROXY=none GOSUMDB=off GOTOOLCHAIN=local \ + go test -mod=readonly -count=1 ./... +GOWORK=off GOPROXY=off GOTOOLCHAIN=local go run ./internal/apicheck +``` + +The R1 reproduction used a temporary committed consumer repository and a native +CLI built from the reviewed source. Publication and delivery suite checks used +local Git and HTTP fixtures. No consuming repository was changed, release was +published, deployment was activated, or live hosted delivery was performed. +This review did not rerun minimum-Go-1.25 validation; the completed implementation +plan records that earlier verification. Real installation, consumer migration, +credentials, and live CI/Gitea verification remain adoption/deployment work in +the [product roadmap](releasekit.md). + +R1's implicit warning-free discovery prerequisite is the additional restriction +identified by this review. The review found no further strengthened guarantee +requiring remediation. Preserve existing published-content immutability and the +ordinary source-eligibility observations; the findings require no reproducible +build promise, stronger concurrency guarantee, or additional release gate.