From 812c24de9ef1709a9d6456aa94fcafede11b319a Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Mon, 2 Nov 2026 01:27:24 +0000 Subject: [PATCH] Record security audit findings --- docs/roadmap/audit/07-security.md | 200 ++++++++++++++++++++++++++++++ 1 file changed, 200 insertions(+) create mode 100644 docs/roadmap/audit/07-security.md diff --git a/docs/roadmap/audit/07-security.md b/docs/roadmap/audit/07-security.md new file mode 100644 index 0000000..c18c11f --- /dev/null +++ b/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.