MDBF-1227: Rate limit Buildbot by client IP#982
Merged
RazvanLiviuVarzaru merged 2 commits intoJul 1, 2026
Merged
Conversation
Old behavior used request_uri as the rate-limit key, so the bucket was shared globally per exact URL. One aggressive client hammering a common URL could consume that URL's allowance and cause unrelated users to be rate-limited too. It also did not catch scanners that spread requests across many unique URLs, since each URL had its own bucket. New behavior uses binary_remote_addr as the key and allows 10 requests per second with a burst of 200 before returning 429. This isolates aggressive scanners to their source IP and leaves other client IPs unaffected. The tradeoff is that users behind the same NAT, VPN, or proxy still share one client-IP bucket.
Will probably need future changes if we identify 429 reponses for normal operation. Buildbot pages can generate short request spikes when developers open or refresh builds with many steps, changes, avatars, and websocket/API calls. Treat those page-load clusters as burst traffic rather than raising the sustained request rate too far. Use the request rate as the long-term per-client ceiling, and use burst to absorb normal UI fan-out from heavy build pages or a small number of tabs. This keeps the developer workflow responsive while still rejecting clients that continue sending requests above the sustained rate.
3db71cb to
7d56c49
Compare
fauust
approved these changes
Jun 30, 2026
fauust
left a comment
Collaborator
There was a problem hiding this comment.
LGTM, but make sure to communicate with our DEVs about this change so that they can report any problem.
The tradeoff is that users behind the same NAT, VPN, or proxy still share one client-IP bucket.
This is probably OK but maybe you can quickly ask around at PLC if that's going to be a problem?
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.
@fauust TLDR: We will probably need to experiment with the values. I allowed bursts for dev's rapidly navigating through bb pages. The per-URL bucket made aggressive scanners block genuine users.
Old behavior used request_uri as the rate-limit key, so the bucket was shared globally per exact URL. One aggressive client hammering a common URL could consume that URL's allowance and cause unrelated users to be rate-limited too. It also did not catch scanners that spread requests across many unique URLs, since each URL had its own bucket.
New behavior uses binary_remote_addr as the key.
This isolates aggressive scanners to their source IP and leaves other client IPs unaffected.
The tradeoff is that users behind the same NAT, VPN, or proxy still share one client-IP bucket.