Repository navigation
Harden against resource exhaustion: partial blobs, artifact size, concurrent diffs - #472
Merged
Merged
Conversation
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
approved these changes
Oct 14, 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
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.
Three hardening changes against resource exhaustion. Any client that can reach the proxy can trigger these, and
/uihas no authentication. Each change is its own commit.Changes
Abort blob writes when the download fails
Storeclosed 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.Storenow cancels the writer's context before closing it.Add a size limit for upstream artifacts
artifact_max_size/PROXY_ARTIFACT_MAX_SIZE. Unlimited by default, so existing setups behave as before. Negative or unparseable values fail validation.Content-Lengthover 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.handler.LimitArtifactSize), so stored, streamed and mirrored artifacts all honour it.docs/configuration.mdandconfig.example.yaml.Limit how many version diffs run at once
compareDiffbuffers both archives whole, up to 512 MB each, so a few parallel compares of large cached versions could exhaust memory.503with codeBUSYandRetry-After: 5rather than queueing.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".go test -race ./...,go vetandgolangci-lintv2.13.1 pass.