Record security audit findings

docs/roadmap/audit/07-security.md

@@ -0,0 +1,200 @@
+# Security audit
+
+Reviewed baseline: `1b39e71` on `main`, using Go 1.26.10 and Git 2.53.0 on
+Linux. The three findings below remain pending.
+
+## Architecture and threat scope
+
+The root facade coordinates candidate validation, source publication, builds,
+preparation, and delivery. `buildinfo` supplies independent runtime version
+access. Internal packages own Git, Go execution, filesystem operations,
+artifact verification, shared delivery policy, and the Gitea protocol adapter.
+The CLI owns configuration, credential loading, output, and exit statuses.
+
+Repository source and release profiles are trusted execution inputs: candidate
+checks run Go tests, configured checks invoke selected programs, and binary
+builds execute the application's version command. ReleaseKit runs with the
+operator's permissions. Its offline Go settings control module/toolchain
+downloads. Repository commands retain the operator's filesystem and network
+permissions.
+This review assessed that implemented operating model under the
+[architecture policy](../../policy/architecture.md#trust-boundaries-and-threat-scope).
+
+Security-relevant external inputs include remote Git observations, forge HTTP
+responses, attachment download URLs and redirects, and downloaded bytes.
+Cross-origin asset downloads are an explicitly supported
+[integration behavior](../../integrations/gitea.md). The audit evaluated both
+credential origin checks and resource consumption while following those
+externally supplied URLs.
+
+The review covered credential supply and diagnostics, HTTP authentication and
+redirects, preflight evidence, command arguments, committed path/symlink
+resolution, build and receipt paths, artifact integrity, temporary-file
+ownership, CI bootstrap examples, and dependency advisories. Local privileged
+attackers and malicious code already authorized to run as the operator were
+outside the assessed boundary.
+
+## Verification and observed protections
+
+The following checks passed:
+
+```sh
+GOTOOLCHAIN=local go run golang.org/x/vuln/cmd/govulncheck@v1.9.0 ./...
+GOTOOLCHAIN=local GOPROXY=off GOSUMDB=off go test -count=1 \
+    ./internal/gitea ./internal/artifact ./cmd/releasekit/internal/cli
+```
+
+`govulncheck` reported `No vulnerabilities found.` for the host build using the
+current Go vulnerability database. This is a known-vulnerability result for
+the scanned build; it does not establish the security of every supported
+platform or older compatible toolchain. See the Go project's
+[vulnerability management documentation](https://go.dev/doc/security/vuln/).
+
+The preceding audits' full ordinary/race suites and facade-boundary checks
+cover the same unchanged production code. A targeted check of 191 tracked files
+at this baseline found no private-key headers or credential-bearing HTTP URL
+patterns. Ignored credential files were outside that content scan.
+
+Isolated local programs reproduced the findings below. Their credentials were
+placeholders; the Git credential probe stopped before any network operation.
+HTTP probes used local servers. Production code and repository tests remain
+unchanged.
+
+The review confirmed useful existing protections: direct argument vectors for
+commands, committed-tree path boundary handling, strict receipt declarations,
+artifact hashing before hosted writes, explicit destination selection, and
+read-only preflight with documented permission-evidence limits. Asset redirects
+are handled manually, and the Gitea token was withheld from the separate asset
+origin in the redirect probe. The findings concern other aspects of those
+boundaries.
+
+## 1. Medium: publication diagnostics expose credentials embedded in Git URLs
+
+**Relevant code:** [publish.Publish](../../../internal/publish/publish.go),
+lines 106–108; publication failure output in
+[cmd/releasekit/main.go](../../../cmd/releasekit/main.go), line 63; and URL-bearing
+errors in [internal/git/remote.go](../../../internal/git/remote.go),
+[internal/git/tag.go](../../../internal/git/tag.go), and
+[publication reconciliation](../../../internal/publish/publish.go).
+
+**Vulnerability and circumstances:** Git supports HTTPS remotes containing
+userinfo credentials, including an access token in the password position.
+ReleaseKit retains that complete effective URL and interpolates it directly
+into errors. The CLI also prints `PublicationResult.Endpoint` verbatim on a
+publication failure. An ordinary URL mismatch, unavailable remote, or later
+publication failure can therefore copy the credential from local configuration
+into terminal transcripts, CI logs, or a shared diagnostic report.
+
+**Reproduction:** A temporary clean candidate used an HTTPS `origin` containing
+a placeholder password, with a different credential-free `ExpectedPushURL`.
+A public `Publish` call returned `ErrUnexpectedPushURL`. Both the error text
+and returned endpoint contained the complete placeholder password. This
+happened before validation or network access. Inspection of the CLI's failure
+formatter confirms that both values flow into its output.
+
+**Impact:** Readers of diagnostic output can obtain a working Git password or
+access token even when they have no access to the operator's local repository
+configuration. The token's permissions determine the resulting repository
+access. This exposure concerns Git URL credentials; the probe did not find a
+leak of the separately supplied Gitea delivery token.
+
+**Potential remediation:** Introduce a credential-safe URL representation at
+diagnostic boundaries and use it consistently in library errors and CLI output.
+The standard library's [URL.Redacted](https://pkg.go.dev/net/url#URL.Redacted)
+handles passwords; review username-carried tokens when choosing userinfo
+redaction. Check wrapped Git diagnostics as well as ReleaseKit-owned messages.
+Retain the original URL for transport and exact destination comparison, and
+preserve error identity. `PublicationResult.Endpoint` currently promises the
+captured effective URL: keep that contract explicit, document its sensitivity,
+and sanitize its CLI rendering rather than silently changing transport or
+comparison semantics. A regression should exercise a credential-bearing
+remote and verify safe errors/output while exact comparison remains intact.
+
+## 2. Medium: unique asset redirects bypass the HTTP client's finite redirect limit
+
+**Relevant code:** [Client.httpClient](../../../internal/gitea/release.go),
+which disables automatic redirects, and
+[Client.openAsset](../../../internal/gitea/assets.go), lines 58–85.
+
+**Vulnerability and circumstances:** The manual redirect loop retains every
+visited URL and rejects an exact repeated URL. It has no hop limit. A content
+server can continually redirect to a new query value or path, so every request
+passes cycle detection and adds another retained string. The ordinary Go
+client's redirect bound is bypassed because each hop is issued as a separate
+`Do` call with automatic redirection disabled. The CLI uses a signal-driven
+context without an automatic delivery deadline, and the default HTTP client
+has no total timeout.
+
+A compromised external attachment host, or an accidentally recursive redirect
+rule that keeps changing the URL, can trigger this during asset inspection.
+This scenario uses the supported cross-origin download path; it requires no
+ability to execute repository code or obtain the delivery token.
+
+**Reproduction:** A local forge fixture returned an attachment URL on a second
+local server. That server issued 64 redirects with distinct query values and
+then returned content. `Backend.OpenAsset` completed after 65 asset requests.
+An ordinary Go HTTP client rejected the same chain under its default redirect
+policy. The fixture separately confirmed that no Gitea authorization header
+reached the second origin. The standard client's default policy is documented
+under [http.Client](https://pkg.go.dev/net/http#Client).
+
+**Impact:** A hostile or faulty content host can occupy the delivery process
+and generate requests indefinitely. The visited-URL map grows with the chain,
+creating a memory-exhaustion path as well as network and runner-time costs.
+The probe demonstrated excess redirects, not an out-of-memory crash.
+
+**Potential remediation:** Apply a finite request-local redirect budget while
+retaining the current per-origin credential checks and response-body cleanup.
+The normal Go limit of ten redirects is a proportionate starting point.
+Choose and document the budget deliberately during remediation. If actual
+consumer workflows need longer chains, evaluate an explicit override alongside
+that decision. A configured deadline can supplement this, while a hop budget
+also bounds retained URL history. Regression coverage should include distinct-URL
+chains, ordinary redirects, exact cycles, and authorization across origins.
+This finding does not call for prohibiting supported external content hosts.
+
+## 3. Low: remote content verification reads beyond the known matching size
+
+**Relevant code:** [inspectRemoteAssets and remoteAssetDigest](../../../internal/delivery/remote_assets.go),
+particularly lines 80 and 102–113. Each expected artifact already carries its
+local verified `Size`, but the download comparison passes only the remote
+handle and hashes the stream to EOF.
+
+**Vulnerability and circumstances:** An attachment response can be much larger
+than the declared local asset, or continuously supply bytes without ending.
+After the response exceeds the expected file size, it cannot match the local
+declaration, but ReleaseKit continues downloading and hashing it. An external
+content host controlling this response can prolong useful-looking activity and
+consume bandwidth and CPU for a result that is already known to conflict.
+
+**Reproduction:** A public `Deliver` call used a valid temporary tagged checkout
+and a caller-owned three-byte prebuilt artifact. The local forge fixture listed
+the expected filename and a reported size of three bytes, while its download
+endpoint returned 65,536 different bytes. A counting response-body wrapper
+confirmed that delivery read all 65,536 bytes before reporting `conflict`.
+No hosted writes occurred. The stream was finite to keep the probe bounded.
+
+**Impact:** A small expected asset can induce arbitrarily larger downloads and
+hashing work. This is a resource-consumption weakness; the finite probe still
+reported the content conflict correctly. It is lower priority than the
+credential exposure and redirect-history growth.
+
+**Potential remediation:** Pass the locally verified expected size into remote
+content verification and consume at most that size plus one byte. Detecting an
+extra byte establishes a conflict; a shorter stream also conflicts. Keep the
+digest comparison for equal-length streams, close the stream on every path,
+and retain inspection errors/cancellation causes. Derive the bound from the
+local declaration, preserving the supported artifact sizes and avoiding an
+arbitrary global download cap. This addresses excess bytes; a stalled server
+before that bound still follows the operation's cancellation/timeout behavior.
+
+## Related work and recommendation
+
+The [correctness audit](01-correctness.md) tracks publication integrity issues,
+and the [concurrency audit](04-concurrency.md) tracks subprocess shutdown and
+queued-reader cancellation. Their remediation remains relevant to release
+integrity and availability.
+
+Prioritize credential-safe diagnostics, then bound manual redirect handling
+and remote mismatch reads. Retain the existing source/profile trust model,
+explicit forge selection, token origin checks, and additive delivery policy.