Skip to content

Harden against resource exhaustion: partial blobs, artifact size, concurrent diffs - #472

Merged
andrew merged 4 commits into
git-pkgs:mainfrom
simonchrz:fix/artifact-size-limits
Oct 14, 2026
Merged

andrew merged 4 commits into
git-pkgs:mainfrom
simonchrz:fix/artifact-size-limits

Conversation

@simonchrz

Copy link
Copy Markdown
Contributor

Three hardening changes against resource exhaustion. Any client that can reach the proxy can trigger these, and /ui has no authentication. Each change is its own commit.

Changes

Abort blob writes when the download fails

  • gocloud commits whatever a writer has received when it is closed, unless its context was cancelled first.
  • Store closed the writer after a failed copy, so every download that broke off midway left a partial object at its fetch path. That object was never served and never deleted.
  • Store now cancels the writer's context before closing it.

Add a size limit for upstream artifacts

  • New artifact_max_size / PROXY_ARTIFACT_MAX_SIZE. Unlimited by default, so existing setups behave as before. Negative or unparseable values fail validation.
  • A declared Content-Length over the limit is refused before the body is read. An artifact of unknown size fails once the download passes the limit. Nothing is cached and the client gets a 502.
  • The limit is applied as a fetcher wrapper (handler.LimitArtifactSize), so stored, streamed and mirrored artifacts all honour it.
  • Documented in docs/configuration.md and config.example.yaml.

Limit how many version diffs run at once

  • compareDiff buffers both archives whole, up to 512 MB each, so a few parallel compares of large cached versions could exhaust memory.
  • At most two diffs run at once. Further requests get an immediate 503 with code BUSY and Retry-After: 5 rather than queueing.
  • The 512 MB per-archive cap stays, because legitimately large packages need it.
  • Swagger docs are regenerated for the new response.

Testing

Each new test was first run against a no-op stub, to check it fails on the missing behaviour:

  • TestBlobStoreFailedCopyLeavesNoObject: "failed Store left a partial object behind".
  • Size limit, declared, undeclared, stored and streamed cases: e.g. "Fetch error = , want ErrArtifactTooLarge".
  • Compare limiter: "status with all slots taken = 200, want 503", plus a test that a finished comparison frees its slot.

go test -race ./..., go vet and golangci-lint v2.13.1 pass.

simonchrz and others added 4 commits October 14, 2026 08:36
gocloud commits whatever a writer received when it is closed, unless its
context was cancelled first. Store closed the writer after a failed copy,
so every download that broke off midway left a partial object under its
fetch path. No database row points at it, so it was never served, but
nothing deleted it either and the orphans kept taking disk space.

- Cancel the writer's context before closing it when the copy fails
- Add a test storing from a reader that errors midway
Artifacts had no size limit. A fetch was bounded only by the server's write
timeout, and storage.max_size evicts only after the bytes are written, so
any client could fill storage or keep fetches busy by requesting huge
artifacts.

- Add artifact_max_size (PROXY_ARTIFACT_MAX_SIZE), unlimited by default
- Refuse an artifact whose declared size is over the limit before reading
  it, and fail one of unknown size once the download passes the limit
- Apply it in the fetcher so stored, streamed and mirrored artifacts all
  honour it; nothing is cached and clients get 502
The compare endpoint buffers both archives whole, up to 512 MB each, and
anyone who can reach the UI can call it. A handful of parallel requests for
large cached versions was enough to exhaust the proxy's memory.

- Run at most two diffs at once; refuse more with 503, code BUSY and
  Retry-After instead of queueing them
- Keep the per-archive cap, which legitimately large packages need
- Move the compare test's setup into a helper shared by the new tests
@andrew
andrew merged commit 784402e into git-pkgs:main Oct 14, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants