201 lines
11 KiB
Markdown
201 lines
11 KiB
Markdown
# 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.
|