Files
releasekit/docs/roadmap/audit.md

21 KiB
Raw Blame History

Audit remediation target

The target state and policy choices below are accepted. They cover the 27 findings consolidated from the eleven reports in audit/ into 20 work items. Original reports retain reproduction evidence and limitations. The implementation baseline is b5b3f09; only documentation changed since the audits' source baselines. Implementation status and ordered work are owned by the implementation plan.

Accepted scope and priorities

P1 addresses publication integrity, credential confidentiality, and cancellation. P2 covers recovery, completed-output contracts, cleanup, and asset inspection. P3 covers focused internal improvements. P4 retains the accepted deferral of production changes while response sizing and exclusion performance are assessed.

The target preserves the root and buildinfo facades, explicit forge selection, caller ownership of prebuilt artifacts, credential origin checks, and additive hosted delivery. The accepted resource policies are ten followed redirects per asset request and a 1 MiB retained tail for status-only Go diagnostics. A11 and A17 assessment supplies evidence for later policy or optimization decisions.

ID Priority Target outcome
A01 P1 Exact source-tag publication with submodule recursion disabled.
A02 P1 Consistent original-object release evidence.
A03 P1 Bounded command I/O shutdown and joined pipe ownership.
A04 P1 Credential-safe Git URL diagnostics.
A05 P2 Accurate retained-tag recovery evidence.
A06 P2 Digests on every completed artifact.
A07 P2 Actionable cleanup errors with accurate primary outcomes.
A08 P2 At most ten followed redirects per asset request.
A09 P2 Content comparison bounded by expected size plus one byte.
A10 P3 Bounded status diagnostics and complete existing forwarding.
A11 P4 Response-size assessment; production caps deferred.
A12 P3 Shared Git observations with caller-owned policy.
A13 P3 Clear Go-validation and committed-tree ownership.
A14 P3 One internal normalized build-settings representation.
A15 P3 Private command workflows behind a small dispatcher.
A16 P3 Shared pure artifact-declaration matching.
A17 P4 Exclusion benchmarks; production indexing deferred.
A18 P3 Queued cancellation preserves a healthy reader session.
A19 P3 Tests protect behavior and selected resource concerns.
A20 P3 Unused helpers removed with live coverage preserved.

Accepted outcomes and rationale

A01. Inherited submodule push mode can suppress publication

Sources: correctness §1 and testing §1.

Target: Source publication pushes only the selected annotated tag object, with submodule recursion disabled. A successful publication identifies the exact tag published to the authoritative endpoint.

Rationale: An inherited submodule-only push mode can otherwise return success while leaving the release tag absent.

Considered alternative: Successful-push read-back supplies independent evidence but adds network work and an uncertain outcome when that read fails. The selected remedy controls the owned push operation.

A02. Replacement objects separate validated and published content

Sources: correctness §2 and testing §1.

Target: ReleaseKit-owned Git inspection uses original objects consistently for module identity, notes, candidate files, and source eligibility. Working source is compared with that view before release evidence is accepted. Ordinary dirty builds retain their supported behavior; unrelated replacement refs remain permitted.

Rationale: Replacement interpretation can make validated bytes differ from the original objects transferred to the remote, despite matching recorded IDs.

Considered alternative: Rejecting replacements affecting selected objects is viable but needs additional commit/tree/blob reachability analysis. The accepted original-object view provides one consistent interpretation.

A03. Inherited descriptors defeat cancellation and bounded recovery

Sources: correctness §3, concurrency §1, and testing §2.

Target: Buffered command execution has bounded I/O teardown after cancellation or child exit. Piped exchanges close owned pipes on operation cancellation and join their cancellation work and child wait. Context error identity and the existing bounded publication reconciliation remain meaningful.

Rationale: Descendants can retain descriptors after the immediate child exits or is canceled. Both buffered execution and batch-reader cleanup need ownership of their shutdown paths.

Considered alternative: Process-group termination can stop descendants but introduces platform and ownership policy beyond completing the selected cancellation behavior. Standard subprocess and pipe mechanisms are the accepted scope.

A04. Git URL credentials appear in errors and CLI output

Source: security §1.

Target: ReleaseKit-owned Git URL diagnostics and CLI output mask HTTP(S) userinfo credentials, including username-carried tokens. Transport and exact comparisons use original URLs. PublicationResult.Endpoint retains its raw captured value, with its sensitivity documented.

Rationale: The observed mismatch and failure paths can expose configured credentials to readers of logs or shared diagnostics.

Considered alternative: Sanitizing the public endpoint value reduces accidental logging but changes the captured-URL contract. The accepted choice sanitizes diagnostic boundaries while preserving that contract.

A05. Post-creation tag inspection loses recovery evidence

Sources: error handling and testing §3.

Target: Publication preserves any verified local tag object obtained before a later inspection failure. When its identity is unavailable after creation, the error identifies the ref and possible retained local tag. Failed verification remains a failure, with no push and a not_published remote outcome.

Rationale: Recovery needs accurate partial evidence even when the initial tag command succeeded and a later local command failed.

Considered alternative: Independent post-failure inspection or a new creation-progress type can provide more machine-readable evidence, but adds work or public surface. Existing results and contextual errors are sufficient for the selected fix.

A06. A partial build lists an artifact before hashing completes

Source: interfaces.

Target: Every entry in the completed artifact list has its digest. A hashing failure preserves earlier completed artifacts and identifies the affected path in the error. Generated caller-owned files remain available for recovery.

Rationale: An empty digest in a completed-output entry violates the current partial-result contract even though the overall build returns an error.

Considered alternative: A separate compiled-but-unverified result state is viable if consumers need richer recovery. The accepted choice retains the existing completed-only model.

A07. Temporary cleanup loses paths or silently discards failures

Sources: resources §3 and §4.

Target: Owned temporary paths remain identifiable through allocation, resolution, and cleanup failures. Removal failures are returned with the affected path and any primary cause. Confirmed hosted outcomes remain intact: DeliveryComplete can accompany a local cleanup error, and the CLI distinguishes completed remote work from failed housekeeping. Caller-owned prebuilt artifacts retain their lifecycle.

Rationale: Discarded errors and lost allocation paths can leave storage behind without actionable recovery information.

Considered alternative: Warnings alone preserve primary success conventions but can disappear when an optional writer is absent. Structured cleanup metadata could support future automation; contextual returned errors are the accepted representation.

A08. Manual asset redirects have no finite hop budget

Source: security §2, also referenced by the resource audit.

Target: An asset request follows at most ten redirects. Limit exhaustion is unverifiable content. Exact-cycle detection, response-body closure, and per-origin authorization remain in place. A longer-chain override requires demonstrated consumer need.

Rationale: A finite hop budget bounds requests and retained URL history for distinct-URL chains supplied by external content hosts.

Considered alternative: An operation deadline can bound elapsed time but does not directly bound redirect history and is not automatically supplied by the CLI. The selected protection is a redirect budget.

A09. Mismatching assets are read beyond their expected size

Source: security §3, also referenced by the efficiency and resource audits.

Target: Remote content comparison consumes at most the locally verified expected size plus one byte, with overflow-safe counting. Shorter or longer content conflicts; equal-length content still requires a matching digest. Read, close, and cancellation failures retain their causes.

Rationale: Once size mismatch is established, further download and hashing cannot produce a matching asset and can consume unbounded resources.

Considered alternative: Reported remote size or content length can be absent or inaccurate. A global byte cap would also restrict legitimate binaries. The accepted bound derives from local artifact facts.

A10. Status-only Go output accumulates in memory

Source: resources §1.

Target: Status-only Go command diagnostics retain at most a 1 MiB tail, with an explicit truncation notice on failure. Existing output destinations receive the complete stream; operations without a destination retain their quiet behavior. Tool.Output continues returning complete output. Process and forwarding error identity are preserved.

Rationale: A bounded tail controls retained memory for long-running commands without adding public configuration or temporary-log ownership.

Considered alternative: Full spooling retains complete failure history but requires disk budgets, cleanup, and retrieval semantics. It remains a future option if consumers require full captured failure history.

A11. Forge JSON decoding has no response budget

Source: resources §2.

Target: JSON response caps remain deferred pending a supported response envelope for repository metadata, notes, and unpaginated attachment lists. Assessment records available local sizing evidence and its limits. Context cancellation and uncertain-write reconciliation remain unchanged.

Rationale: Source inspection demonstrates unbounded consumption, while the reports establish neither representative maximum sizes nor a selected protocol limit. Arbitrary caps would introduce a new compatibility restriction.

Considered alternative: Configurable limits with generous defaults provide earlier containment but add policy and may reject supported responses. Sizing evidence precedes that future decision.

A12. Common Git observations have several owners

Sources: complexity §2 and equivalence §1.

Target: The existing internal Git boundary owns common HEAD and repository-wide status observations. Callers retain their observation timing and decisions: clean-main candidate rules, dirty-build eligibility, and tagged delivery checks stay distinct.

Rationale: One mechanics owner reduces duplicated object-view, cancellation, and error propagation handling.

Considered alternative: Separate implementations keep individual repairs small but retain parallel maintenance. A generalized release-state object would combine different policies; narrow observations are the accepted abstraction.

A13. Shared validation mixes sequencing and tree ownership

Source: complexity §1.

Target: Shared validation exposes a readable ordered Go-command sequence and a committed-tree sequence that owns reader lifetime and close-error attribution. Nil/empty selections, check order, failure precedence, and current diagnostic identities are preserved.

Rationale: Separating these ownership responsibilities reduces distant coordinated edits when checks change.

Considered alternative: Explicit blocks in one large function are viable while stable. The accepted simplification uses private functions within existing packages; a plugin registry has no demonstrated consumer need.

A14. Internal build results duplicate normalized settings

Source: complexity §3.

Target: Internal build results have one normalized Settings representation. Public BuildResult and private receipt fields keep their existing contracts and explicit boundary conversions.

Rationale: Removing duplicate internal package/name/target fields prevents drift without changing consumer types.

Considered alternative: A constructor can enforce duplicate-field consistency but preserves redundant state and maintenance. A single internal representation is accepted.

A15. CLI routing contains complete command workflows

Source: complexity §4.

Target: The CLI dispatcher delegates check/publication and build workflows to named private handlers. Profile mapping, receipt invalidation order, diagnostic context, and exit classifications retain their current responsibilities.

Rationale: Command-specific workflows become easier to change and verify without adding a command framework or public interface.

Considered alternative: Keeping the current switch is viable for small repairs but continues interleaving routing with full workflows. Private handlers are the accepted organization.

A16. Receipts and delivery repeat declaration matching

Source: equivalence §2.

Target: Receipts and delivery share pure target/filename declaration matching in internal/artifact. Receipt schema/path/digest rules remain CLI-owned; source-only handling and filesystem/content verification remain with delivery after preflight.

Rationale: The same identity rule has two callers and one natural internal owner. Its independent boundary rules remain explicit.

Considered alternative: Sharing only name generation avoids another helper but duplicates uniqueness and matching logic. A narrow metadata-only helper is accepted.

A17. File-size exclusions scale with files times exclusions

Source: efficiency.

Target: Exclusion indexing remains deferred pending benchmarks varying both selected files and exclusion count. Production matching retains literal component boundaries, module-relative normalization, shared inputs, and first-oversized-file behavior.

Rationale: The O(N × E) scan is established, but practical slowdown has not been measured; common small lists may favor the existing loop.

Considered alternative: A set plus ancestor lookups removes the exclusion-count factor at an allocation/path-walking cost. A trie is viable for deeper paths with additional complexity. Adoption remains an evidence-driven future choice.

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

Source: concurrency §2.

Target: A read canceled while queued is rejected after acquiring the reader mutex and before it can change the healthy batch session. Current sequential validation and mutex-based serialization remain in place.

Rationale: An already-canceled queued request can otherwise install a callback that terminates a healthy child and affects subsequent requests.

Considered alternative: Cancellation-aware acquisition also provides prompt queued cancellation but changes synchronization and expands its effective concurrent-use contract. That expansion remains deferred until a caller needs it.

A19. Tests assert mechanisms beyond the protected behavior

Source: testing §4.

Target: Tests protect parsing and source-relative resolution, artifact integrity and ordering, and exact publication outcomes. Cache pointer identity, exact binary-open counts, and successful-path remote query counts remain implementation details. The documented single reconciliation query after a failed push retains its separate contract.

Rationale: Mechanism assertions can reject correct refactoring while failing to cover the observed correctness defects. Relevant performance concerns are measured through benchmarks.

Considered alternative: A mechanism assertion is appropriate when it protects a demonstrated resource problem that behavioral checks miss. The three reported cases use behavioral evidence and benchmarks instead.

A20. Two internal helpers have no production purpose

Sources: dead code §1 and §2.

Target: The unused FromCommittedSelection wrapper and test-only InspectLocalTag helper are removed. Their live private implementations, selection APIs, candidate tag guard, and useful full-tag/source assertions remain. Tree accessors with meaningful tests or benchmarks remain supported internally.

Rationale: These two helpers have no current production purpose or documented retention need and are outside the public facades.

Considered alternative: A real shared-guard use could justify retaining InspectLocalTag, but adds integration and ordering considerations. The accepted standalone disposition is removal while preserving the live guard.

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 1 → A01; 2 → A02; 3 → A03
02 Efficiency Exclusion scanning → A17; remote-read cross-reference → A09
03 Complexity 1 → A13; 2 → A12; 3 → A14; 4 → A15
04 Concurrency 1 → A03; 2 → A18
05 Resources 1 → A10; 2 → A11; 3 and 4 → A07; cross-referenced shutdown/asset issues → A03/A08/A09
06 Error handling Retained local tag → A05
07 Security 1 → A04; 2 → A08; 3 → A09
08 Testing 1 → A01/A02; 2 → A03; 3 → A05; 4 → A19
09 Interfaces Missing partial-artifact digest → A06
10 Equivalence 1 → A12; 2 → A16
11 Dead code 1 and 2 → A20