# Audit remediation roadmap

Reviewed against `91f25c4` on `main`. All eleven reports under
[`audit/`](audit/) were reviewed, with selected implementation and contract
checks. Changes since their earliest baseline affect audit documents only;
the recorded implementation findings remain pending. This document owns the
accepted remediation priorities and approaches. The individual reports retain
detailed evidence and reproduction limits.

The 27 findings across the reports consolidate into 20 work items. Missing
regressions accompany the defects they should protect. Repeated reports of
subprocess shutdown and Git inspection duplication each become one work item.
Temporary-storage fixes and unused-helper removals are grouped by ownership,
with their distinct cases identified below.

## Accepted decisions and status

The approaches and priorities for A01–A20 are accepted. Implementation remains
pending. Each recommendation below records the accepted approach;
alternatives are retained as decision rationale.

- A01–A04 are the first implementation priorities.
- A08 uses an internal limit of ten followed redirects per asset request.
- A10 retains a 1 MiB diagnostic tail for status-only Go commands while
  forwarding complete output and preserving full `Tool.Output` results.
- A11 response caps and A17 exclusion indexing remain deferred pending the
  sizing and benchmark evidence described in those items.
- Internal consolidation and refactoring follow the timing stated in their
  accepted approaches, alongside related repairs or maintenance.

## Priorities

Priority indicates accepted work order, independent of the original audits'
severity labels. P1 addresses release integrity, credential confidentiality,
and cancellation guarantees. P2 improves recovery, completed-output contracts,
cleanup, and bounded asset inspection. P3 covers focused internal improvements.
P4 defers implementation pending sizing or performance evidence.

| ID | Priority | Accepted work and disposition |
| --- | --- | --- |
| A01 | P1 | Force the intended tag push to include the repository itself. |
| A02 | P1 | Use original Git objects consistently for release evidence. |
| A03 | P1 | Complete cancellation and shutdown across buffered and piped commands. |
| A04 | P1 | Redact Git URL credentials in diagnostics. |
| A05 | P2 | Preserve recovery evidence after local tag creation. |
| A06 | P2 | Return completed artifacts only after their digests are available. |
| A07 | P2 | Preserve temporary cleanup paths and report cleanup failures accurately. |
| A08 | P2 | Apply a finite budget to manually followed asset redirects. |
| A09 | P2 | Stop remote mismatch reads at the expected size plus one byte. |
| A10 | P3 | Bound retained status-only Go diagnostics while forwarding complete output. |
| A11 | P4 | Establish supported JSON response sizes before introducing response caps. |
| A12 | P3 | Consolidate common Git observations alongside the related fixes. |
| A13 | P3 | Separate Go validation and committed-tree ownership when touching validation. |
| A14 | P3 | Keep one normalized settings representation in internal build results. |
| A15 | P3 | Extract CLI command handlers when updating the affected workflows. |
| A16 | P3 | Share pure artifact-declaration matching between receipts and delivery. |
| A17 | P4 | Measure exclusion-list scaling before replacing the current scan. |
| A18 | P3 | Recheck queued read cancellation after acquiring the reader mutex. |
| A19 | P3 | Replace incidental test assertions with the behavior they protect. |
| A20 | P3 | Remove the two unused internal helpers, preserving live behavior. |

The first four items deserve the earliest attention: a release can currently
be reported as published without a remote tag, validated source can differ
from published source, canceled operations can remain blocked, and diagnostics
can expose configured credentials. Internal cleanup can follow or accompany
those repairs where it reduces duplicated work. These priorities introduce no
external review or operational gates between work items.

## P1: integrity, confidentiality, and cancellation

### A01. Inherited submodule push mode can suppress publication

Sources: [correctness §1](audit/01-correctness.md#1-high-publication-can-report-success-while-leaving-the-tag-unpublished)
and [testing §1](audit/08-testing.md#1-publication-fixtures-omit-git-settings-that-change-what-is-published).

**Recommendation:** Add `--recurse-submodules=no` to the ReleaseKit-owned exact
tag push in `internal/git`. This directly prevents
`push.recurseSubmodules=only` from suppressing the repository push while Git
returns success. Protect it through a public publication regression using a
real local bare remote and an exact remote tag-object assertion.

**Alternative:** Verify the exact authoritative ref after every successful
push. This supplies independent confirmation and could cover other anomalous
success responses, but adds network work and uncertainty when the verification
request fails after an acknowledged push. It also leaves the inherited push
mode's behavior to be corrected separately. Prefer the targeted command fix;
consider additional successful-push verification only for an established need.

### A02. Replacement objects separate validated and published content

Sources: [correctness §2](audit/01-correctness.md#2-high-git-replacement-objects-can-separate-validated-content-from-published-source)
and [testing §1](audit/08-testing.md#1-publication-fixtures-omit-git-settings-that-change-what-is-published).

**Recommendation:** Apply an original-object view consistently to
ReleaseKit-owned Git inspection, using `GIT_NO_REPLACE_OBJECTS=1` or the
equivalent command option. Cover committed identity/note reads, batch blobs,
source-status comparisons, and direct build inspection. A replacement-derived
worktree should be compared with the original committed tree before release
evidence is accepted. Consistency across those paths matters more than changing
one object reader. Preserve ordinary dirty-build behavior and its source
eligibility reporting.

**Alternative:** Detect replacements affecting the selected commit and contents
and return a contextual error. This makes the unsupported situation explicit
but requires correctly identifying affected commits, trees, and blobs. Prefer
the common original-object view, which also permits unrelated replacement refs.
Regressions should compare actual validated and remote source/note bytes;
equal commit IDs alone did not catch the defect.

### A03. Inherited descriptors defeat cancellation and bounded recovery

Sources: [correctness §3](audit/01-correctness.md#3-medium-inherited-process-pipes-can-defeat-cancellation-and-recovery-deadlines),
[concurrency §1](audit/04-concurrency.md#1-medium-inherited-descriptors-can-keep-canceled-operations-and-reader-shutdown-blocked),
and [testing §2](audit/08-testing.md#2-cancellation-fixtures-do-not-cover-inherited-subprocess-descriptors).

**Recommendation:** Give command I/O a bounded shutdown path after cancellation
or child exit, using standard `exec.Cmd.WaitDelay` where applicable and
explicit ownership of piped-process closure. Verify the batch reader's stdout
drain and child wait together, and cover direct build subprocesses. Preserve
context identity, worker joining, and the existing publication recovery bound
and uncertainty classifications. A03 is one repair spanning process and Git
ownership, rather than separate fixes for each audit report.

**Alternative:** Use platform-specific process-group termination to interrupt
descendants as well. That can help when descendant execution itself must stop,
but brings portability and process-ownership decisions beyond returning from
blocked I/O. Prefer the standard shutdown mechanisms for the observed defect.
Tests should establish a descendant's descriptor ownership before canceling,
then exercise buffered commands, batch-reader cleanup, and a public path.

### A04. Git URL credentials appear in errors and CLI output

Source: [security §1](audit/07-security.md#1-medium-publication-diagnostics-expose-credentials-embedded-in-git-urls).

**Recommendation:** Centralize credential-safe endpoint rendering for
ReleaseKit-owned diagnostics and CLI output, including expected/resolved URL
comparisons and reconciliation errors. Mask HTTP(S) userinfo credentials,
including username-carried tokens, and check wrapped Git diagnostic text.
Keep original URLs for transport and exact comparison. Preserve the documented
raw `PublicationResult.Endpoint` value and explain its sensitivity in GoDoc;
its CLI rendering should use the safe representation. This fixes the observed
log exposure without changing destination identity or the public result field.

**Alternative:** Return only a sanitized endpoint publicly. This reduces the
chance that callers log a secret, but changes the current captured-URL contract
and removes information callers may use for exact comparison. Consider that
only as a deliberate API change. Regressions should verify both safe output and
unchanged exact matching with placeholder credentials.

## P2: recovery, output contracts, and asset handling

### A05. Post-creation tag inspection loses recovery evidence

Sources: [error handling](audit/06-error-handling.md#a-post-creation-inspection-failure-hides-a-retained-local-tag)
and [testing §3](audit/08-testing.md#3-local-tag-creation-lacks-a-post-write-inspection-failure-case).

**Recommendation:** Return any verified tag object already obtained together
with a later inspection error. When the object cannot be established after
creation, identify the local ref and explicitly report that a tag may remain.
Keep the failed verification visible and the remote outcome at `not_published`;
existence alone does not make the tag eligible for push. This supplies useful
recovery information within the existing result and error model.

**Alternative:** Perform a bounded independent local inspection after failure
to gather additional evidence, or add a structured creation-progress result if
callers need it. Those options improve machine-readable recovery but add work
or surface area. Prefer preserving existing evidence and clear partial-state
errors first. A real repository and controlled post-write inspection failure
should verify the retained ref, cause, and absence of a push.

### A06. A partial build lists an artifact before hashing completes

Source: [interfaces](audit/09-interfaces.md#a-partial-build-result-can-contain-an-artifact-without-its-promised-digest).

**Recommendation:** Hash a newly compiled artifact before appending it to the
completed `Artifacts` list. On failure, retain earlier completed artifacts and
name the affected output path in the error. Keep the file available for the
caller. This restores the documented digest/completed-output contract with a
small internal change and consistent CLI presentation.

**Alternative:** Represent compiled but unverified output explicitly in a
separate result state. This supports richer recovery but adds an API distinction
for which the audits found no current consumer need. Prefer completed-only
entries. Verify a post-compilation read failure through the public build path,
including preservation of earlier artifacts and the underlying cause.

### A07. Temporary cleanup loses paths or silently discards failures

Sources: [resources §3](audit/05-resources.md#3-temporary-build-cleanup-failures-are-silently-discarded)
and [§4](audit/05-resources.md#4-a-failed-temporary-path-resolution-loses-the-cleanup-path).

**Recommendation:** Address both cases in one cleanup-focused unit. In
`tempdir.Outside`, retain the original created path until resolution succeeds
and use it on resolution failure, in both allocation branches. At owned build,
tag-message, and receipt cleanup sites, retain removal failures with the path
and join them with an existing cause where useful. This corrects concrete
ownership gaps with ordinary filesystem operations.

After confirmed hosted delivery, preserve `DeliveryComplete` and the known
remote observations while reporting the cleanup error separately in the error
text and CLI presentation. That distinguishes completed remote work from
failed local housekeeping. Caller-owned prebuilt output retains its lifecycle.

**Alternative:** Emit cleanup warnings while returning success. This preserves
the primary operation's success/error convention, but an optional output writer
can leave library callers unaware of leaked storage. Prefer a returned error
with accurate result state. Structured cleanup metadata is another option if
automation demonstrates a need; a new public error taxonomy is not required
for contextual errors. Verify failure attribution and retained paths without
requiring hostile filesystem actors or power-loss guarantees.

### A08. Manual asset redirects have no finite hop budget

Source: [security §2](audit/07-security.md#2-medium-unique-asset-redirects-bypass-the-http-clients-finite-redirect-limit),
also referenced by the resource audit.

**Recommendation:** Allow up to ten followed redirects per asset request as
the initial internal default, matching the ordinary Go client convention, and
report exhaustion as unverifiable content. Preserve exact-cycle detection,
body closure, and per-origin authorization checks. This bounds both request
count and retained URL history. The ten-redirect budget is accepted;
assess a longer-chain override only for demonstrated consumer requirements.

**Alternative:** Rely on an operation deadline. It preserves arbitrary chain
lengths and bounds elapsed time when set, but the CLI currently supplies no
automatic deadline and time alone does not directly bound retained history.
Prefer a hop budget for this specific defect. Test distinct-URL chains and
credential handling at the boundary, using local servers.

### A09. Mismatching assets are read beyond their expected size

Source: [security §3](audit/07-security.md#3-low-remote-content-verification-reads-beyond-the-known-matching-size),
also referenced by the efficiency and resource audits.

**Recommendation:** Pass the locally verified expected size to remote
verification. Read at most that size plus one byte, with overflow-safe counting;
extra or missing bytes establish a conflict. Compare digests for equal-length
streams and retain read, close, and cancellation errors. This bounds excess
work using existing artifact facts and preserves supported artifact sizes.

**Alternative:** Use reported remote size or HTTP content length to reject a
mismatch earlier. Such metadata can be absent or inaccurate; it cannot replace
counting actual bytes. A fixed global download cap would also reject legitimate
large binaries. Prefer the declaration-derived bound. Cover short, equal,
oversized, and interrupted streams plus a public delivery conflict case.

## P3 and P4: resource policy and focused maintenance

### A10. Status-only Go output accumulates in memory

Source: [resources §1](audit/05-resources.md#1-go-command-output-is-retained-for-the-entire-execution).

**Recommendation:** For status-only `Tool.Run` and dependency preparation,
forward the complete stream while retaining a bounded diagnostic tail for
failure reporting. Retain up to 1 MiB internally with an explicit truncation
notice; keep full output for `Tool.Output` consumers. This accepted change to
diagnostic retention will be documented with implementation.
It bounds memory without adding consumer configuration or disk lifecycle work.

**Alternative:** Spool full diagnostics to owned temporary storage. This retains
complete failure history but requires a disk budget, cleanup, and a way for the
caller to retrieve retained output. Choose it if actual consumers require full
failure capture. Preserve forwarding failures and process error identity under
either approach; benchmarks should measure memory with large output.

### A11. Forge JSON decoding has no response budget

Source: [resources §2](audit/05-resources.md#2-forge-json-responses-have-no-resource-bound).

**Recommendation:** Defer immediate size/time caps and first establish the
legitimate response envelope for repository metadata, notes, and unpaginated
attachment lists. The source inspection establishes unbounded consumption, but
the audit supplies no representative sizes or selected protocol maximum. Record
those sizes and then propose explicit per-response byte budgets, with a
distinguishable limit error. Current context cancellation remains applicable.

**Alternative:** Introduce configurable limits now with generous defaults. This
offers earlier containment for faulty forges but adds public or configuration
policy and can reject supported notes or attachment lists. Prefer sizing before
that change. Any eventual create/upload response limit must preserve uncertain
write reconciliation: a rejected response is not proof of a rejected write.

### A12. Common Git observations have several owners

Sources: [complexity §2](audit/03-complexity.md#2-git-source-inspection-is-repeated-across-package-boundaries)
and [equivalence §1](audit/10-equivalence.md#1-source-state-observations-are-independently-implemented).

**Recommendation:** Move small HEAD and worktree-status observations behind the
existing internal Git boundary when repairing A02/A03, or immediately afterward
if a separate change is easier to verify. This reduces duplicated subprocess,
cancellation, and object-view handling. Keep observation timing and decisions
with callers: candidate checking requires clean `main`, builds record dirty
source eligibility, and delivery checks a tag.

**Alternative:** Patch each current implementation separately and retain the
duplication. It minimizes refactoring during an urgent repair but leaves several
places to maintain the same Git mechanics. Prefer narrow shared primitives;
a generalized release-state object would combine genuinely different policies.
Retain independent caller-level behavioral tests.

### A13. Shared validation mixes sequencing and tree ownership

Source: [complexity §1](audit/03-complexity.md#1-shared-validation-combines-several-kinds-of-orchestration).

**Recommendation:** Extract private Go-command and committed-tree sequences
when the owner is next changed. Let the tree sequence own closure and its
attribution, with check ordering readable in one place. This reduces the number
of distant edits needed for a new check without changing the facade.

**Alternative:** Keep the current function with explicit blocks until another
check makes the maintenance cost material. That is viable because complexity
scores alone do not establish a defect. Prefer small extractions during related
work; preserve nil/empty selections, failure precedence, and reader-close
semantics. A generic check registry adds machinery without a demonstrated need.

### A14. Internal build results duplicate normalized settings

Source: [complexity §3](audit/03-complexity.md#3-internal-build-results-retain-duplicate-settings).

**Recommendation:** Retain `Settings` as the internal source of truth and remove
the duplicate internal `MainPackage`, `BinaryName`, and `Targets` fields. Update
build execution and conversions together. Public `BuildResult` and receipt
fields continue to serve their existing contracts. This prevents internal drift
with modest, localized work.

**Alternative:** Keep the duplicate fields and enforce consistency through a
constructor. It can reduce call-site changes but preserves redundant state and
ongoing maintenance. Prefer removal; verify partial outputs, facade conversions,
and prebuilt delivery independently.

### A15. CLI routing contains complete command workflows

Source: [complexity §4](audit/03-complexity.md#4-cli-command-handling-interleaves-routing-and-command-specific-workflows).

**Recommendation:** Extract named private check/publication and build handlers
when changing those workflows. Keep `run` focused on parsing, repository
resolution, and dispatch. Existing delivery handlers demonstrate a sufficient
local pattern. Preserve receipt invalidation order, output context, profile
mapping, and exit classifications.

**Alternative:** Retain the current switch while making only targeted fixes.
This keeps urgent diffs small and is acceptable short term. Prefer extraction
as the command is touched; a new command framework or public interface would
add migration cost without addressing an observed consumer need.

### A16. Receipts and delivery repeat declaration matching

Source: [equivalence §2](audit/10-equivalence.md#2-build-artifact-declarations-are-checked-independently-by-receipts-and-delivery).

**Recommendation:** Extract a small pure target/filename matching helper into
`internal/artifact`, building on `ExpectedBuild`. It should own uniqueness,
counts, and one expected name per target. Keep receipt schema/path/digest rules
in the CLI and filesystem/content checks in delivery after preflight. This
shares one concrete identity rule across existing boundaries.

**Alternative:** Keep the two loops while sharing only name generation, as
today. That avoids another helper and is viable while conventions are stable,
but requires coordinated edits to the same matching rules. Prefer the narrow
helper during build-model maintenance, with concrete independent expected
filenames in tests and unchanged source-only handling.

### A17. File-size exclusions scale with files times exclusions

Source: [efficiency](audit/02-efficiency.md#file-size-exclusions-are-scanned-for-every-selected-file).

**Recommendation:** Defer an implementation change until a benchmark varies
both repository size and exclusion count. The reported O(N × E) scan is real,
but the audit measured no practical slowdown; a small exclusion list may favor
the existing straightforward loop.

**Alternative:** Index normalized exclusions in a set now and check each file
and its ancestors. This removes the E factor for large lists at the cost of
allocation and path walking. Prefer it if measurements justify the tradeoff.
A trie is viable for very deep paths but has greater implementation cost.
Preserve literal component boundaries, module-relative normalization, shared
inputs, and the first oversized-file result under any change.

### A18. A canceled queued read can terminate a healthy blob session

Source: [concurrency §2](audit/04-concurrency.md#2-low-a-canceled-queued-blob-read-can-later-abort-a-healthy-session).

**Recommendation:** Recheck `ctx.Err()` immediately after acquiring the reader
mutex and before touching the session or installing its cancellation callback.
This small change prevents already-canceled queued work from aborting a healthy
child. A controlled helper regression should verify a successful first read
and subsequent uncanceled read after canceling queued work.

**Alternative:** Make acquisition cancellation-aware so queued calls also
return promptly. It supports a stronger concurrent-use contract but changes
the synchronization mechanism. Current validation reads sequentially; defer
that expansion until a real caller needs it. Keep the existing mutex and
operation-local candidate caches for the present fix.

### A19. Tests assert mechanisms beyond the protected behavior

Source: [testing §4](audit/08-testing.md#4-some-assertions-lock-in-incidental-internal-mechanisms).

**Recommendation:** Remove Markdown cache pointer-identity assertions after
retaining unique parsing and source-relative resolution coverage. Replace the
Linux exact-open-count test with existing integrity/ordering coverage and a
read-volume benchmark if that efficiency concern remains useful. Remove the
success-path assertion of exactly three remote queries while retaining exact
remote tag/message and unrelated-ref checks. The documented single query after
a failed push remains a separate recovery contract.

**Alternative:** Retain a mechanism assertion as an explicit internal regression
when it protects a demonstrated resource problem that behavior tests miss.
Explain that purpose and give refactoring an equivalent observable protection.
Prefer behavioral assertions and benchmarks for the three reported cases;
passing existing tests did not detect the audited publication defects.

### A20. Two internal helpers have no production purpose

Sources: [dead code §1](audit/11-deadcode.md#1-committed-selection-identity-wrapper-has-no-callers)
and [§2](audit/11-deadcode.md#2-local-tag-inspection-helper-is-used-only-by-its-own-test).

**Recommendation:** Remove `FromCommittedSelection`, retaining its private
implementation and the live selection APIs. Remove `InspectLocalTag` and its
direct test call, retaining the live candidate guard and the test's other full
tag/source assertions. Current source references support these removals, which
reduce internal surface without changing the public facades. Preserve tree
accessors and constructors whose tests/benchmarks exercise meaningful behavior.

**Alternative:** Use `InspectLocalTag` in the live guard during A12 if it
actually reduces duplicate mechanics while preserving context, exact-object
checks, and guard timing. That gives the helper a real owner but requires
checking its redundant validation and error ordering. Prefer removal as the
smaller standalone cleanup; keep it only if the shared Git change supplies a
concrete production use.

## Coverage of the source reports

Each original finding has a disposition above. Cross-references to findings in
other audits supply supporting context rather than additional work items.

| Report | Original finding → consolidated work |
| --- | --- |
| [01 Correctness](audit/01-correctness.md) | 1 → A01; 2 → A02; 3 → A03 |
| [02 Efficiency](audit/02-efficiency.md) | Exclusion scanning → A17; remote-read cross-reference → A09 |
| [03 Complexity](audit/03-complexity.md) | 1 → A13; 2 → A12; 3 → A14; 4 → A15 |
| [04 Concurrency](audit/04-concurrency.md) | 1 → A03; 2 → A18 |
| [05 Resources](audit/05-resources.md) | 1 → A10; 2 → A11; 3 and 4 → A07; cross-referenced shutdown/asset issues → A03/A08/A09 |
| [06 Error handling](audit/06-error-handling.md) | Retained local tag → A05 |
| [07 Security](audit/07-security.md) | 1 → A04; 2 → A08; 3 → A09 |
| [08 Testing](audit/08-testing.md) | 1 → A01/A02; 2 → A03; 3 → A05; 4 → A19 |
| [09 Interfaces](audit/09-interfaces.md) | Missing partial-artifact digest → A06 |
| [10 Equivalence](audit/10-equivalence.md) | 1 → A12; 2 → A16 |
| [11 Dead code](audit/11-deadcode.md) | 1 and 2 → A20 |

## Verification and scope of follow-up

This roadmap records accepted work; production behavior remains unchanged.
Original reproductions, source inspection findings, coverage figures, and
scanner results retain their stated limits. Existing passing tests establish
a useful baseline; regressions should demonstrate the behavior of each repaired
defect and accompany that repair. Reuse the normal offline suite, focused race
checks for shutdown/read changes, and compiled facade/dependency checks when
implementation changes warrant them.

Keep the two public facades, explicit forge selection, caller ownership of
prebuilt artifacts, original credential origin checks, and additive delivery
semantics. The diagnostic and redirect budgets above are accepted decisions;
JSON limits and exclusion indexing remain deferred as explained above.