feat: Harden downloads: path traversal, size limits, and injection fixes - #40
Merged
Conversation
Remediations from a security audit of the download and extraction paths. - s3/gcs: constrain directory downloads to the destination directory. Object keys come from the bucket listing (attacker-influenced) and were joined onto tmpDir with no containment check, so a key containing ".." could write outside it. Add internal/safepath and gate both writes on it. A ".." that stays inside the destination is still allowed. - deps: bump golang.org/x/text 0.37.0 -> 0.39.0 and go 1.25.2 -> 1.26.5, clearing the three govulncheck findings reachable from this code (x/text DoS, crypto/tls ECH leak, os symlink root-escape). - downloads/extraction: add WithMaxDownloadBytes (default 10 GiB) bounding a single downloaded file and the total size of an archive extraction, so a decompression bomb or unbounded body cannot exhaust the disk. Applied to http, s3, gcs, the git archive fallback, and the archive extractor. - hg: reject a repo URL whose host begins with "-" and pass "--" before the positional args, closing the ssh -oProxyCommand argument-injection class in the hg subprocess. - ssrf: apply the guard to the connect probe, which dialled the target directly and so could reach (and reveal the reachability of) internal addresses the guarded fetch refuses. - extract: strip setuid/setgid/sticky bits from archive entry modes. - file: add WithAllowedLocalDirectories to restrict file:// sources to a set of directories, resolving symlinks so a link out of an allowed tree is rejected. Useful when grabber may be handed untrusted URLs.
liamg
changed the title
liamg
changed the title
liamcervante
approved these changes
Jul 31, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Remediations from a security audit of the download and extraction paths. All changes preserve existing behaviour for well-formed inputs; the new limits and restrictions are opt-in or set to unrestrictive defaults.
Fixes
S3 / GCS directory traversal (the one real write-outside bug). Directory downloads joined server-supplied object keys onto the temp dir with no containment check, so a bucket returning a key like
../../../home/you/.ssh/authorized_keyscould write outside it. Both paths now go through a newinternal/safepath.Join, which rejects any result that escapes the destination — while still allowing a..that stays inside it (as requested, not a blanket..ban). This matches the guard the tar/zip extractors and the git sparse path already had.Vulnerable dependencies. Bumped
golang.org/x/text0.37.0 → 0.39.0 and the Go directive 1.25.2 → 1.26.5, clearing all three govulncheck findings reachable from this code (x/text infinite-loop DoS,crypto/tlsECH privacy leak,ossymlink root-escape reachable via go-git clone).govulncheck ./...now reports no reachable vulnerabilities.Decompression-bomb / unbounded-download DoS. New
WithMaxDownloadBytes(defaultsettings.DefaultMaxBytes= 10 GiB,0disables) bounds a single downloaded file and the total size of an archive extraction. Enforced in http, s3, gcs, the git archive fallback, and the archive extractor (cumulative across entries). Does not apply to OCI, which pulls through oras's own client.Mercurial argument injection.
hg clonereceived a URL-derived host positionally. A host beginning with-is now rejected, and--is passed before the positional args, closing thessh -oProxyCommandinjection class (grabber no longer relies solely on hg's own protections).SSRF via the connect probe.
ProbeConnectdialled the target directly, bypassing the SSRF guard, so it could reach and reveal the reachability of loopback/link-local/private addresses the guarded fetch refuses. It now honours the guard (via a smallGuard.Control/Guard.HostAllowedrefactor).setuid/setgid bits from untrusted archives. Archive entry modes are masked to permission bits only, so an archive can't land a setuid/setgid file on disk.
Arbitrary local reads via
file::. NewWithAllowedLocalDirectories(dirs ...)restricts the file protocol to a set of directories, resolving symlinks (including intermediate path components) so a link pointing out of an allowed tree is rejected. Empty by default (no restriction) — for callers that may be handed untrusted URLs.Not changed (by design)
WithNoSystemFallbackmode with noKnownHostsconfigured (opt-in; a changed key is still rejected once known_hosts is supplied).md5/sha1remain accepted for checksums (non-default, integrity not secrecy).Tests
Added unit tests for
safepathcontainment,limitiobounds, the extraction size budget (including cumulative-across-entries) and setuid stripping, thefileallowed-dirs restriction (including symlink escape and a symlinked intermediate component), and the hg leading-dash rejection. Full suite passes (go test ./...);go vetandgovulncheckclean.