Make git exclusions in rate limit rules unconditional again
What does this MR do and why?
This MR is a draft, posted for review discussion before it merges.
Rack::Attack rate-limit rules are true or false checks with built-in exclusions. The unauthenticated web throttle, for example, always skips git paths, no matter what setting is on. Labkit re-expressed some exclusions as a separate claim rule instead of stating them directly. That breaks under one common setting. It caused a Severity 2 incident on a GitLab Dedicated tenant: CI job token Git and LFS requests got HTTP 429s, over 588,000 rejected, 69% of CI Git requests, in the 90 minutes before mitigation.
How the bug works
A git rule counts the request, then a claim stops the web throttle from counting it again. That claim reuses the git rule's match, which carries a setting fact like setting_unauthenticated_git_http. When that git throttle is off, the default, the claim never fires. The request falls through to the web throttle instead and gets counted per IP. Rack::Attack never did that.
The codebase already states the rule this breaks, in the comment above throttle_authenticated_dependency_proxy? in lib/gitlab/rack_attack/request.rb: a setting-gated exclusion is meant to become a no-op when its setting is off, so the request falls back to the web throttle. An unconditional exclusion is not. Claims model the first correctly. They cannot model the second.
Three rules were missing their exclusion:
throttle_unauthenticated_webhad no!git_path?, the rule behind the incident. CI job tokens have no requester_id at the rack layer, so they look unauthenticated, and CI runners share egress IPs.throttle_authenticated_webhad no!(git_path? && !git_lfs_path?).throttle_authenticated_git_httphad no!git_lfs_path?, previously unreported.
throttle_authenticated_git_lfs_enabled defaults to false. A post-migration change in 18.7, db/post_migrate/20251125102233_backfill_throttle_authenticated_git_http_settings.rb, turns on throttle_authenticated_git_http_enabled wherever throttle_authenticated_web_enabled was already on. That makes rule 3 reachable, and has been the default since 18.7. The same migration keeps rule 2 dormant.
GitLab.com is not affected. Its unauthenticated Git HTTP throttle is on, at about 3,199 rule evaluations per second, with the claim firing at the same rate, so Git traffic never reaches the web rule.
The fix
Add a git_http_non_lfs fact to the request classifier: git_path? && !git_lfs_path?. A git_http fact already existed. Each rule now states its exclusion directly, instead of relying on another rule's claim:
throttle_unauthenticated_webgainsgit_http: falsethrottle_authenticated_webgainsgit_http_non_lfs: falsethrottle_authenticated_git_httpchangespath: gittogit_http_non_lfs: true, which already implies a git path
The classifier computes git_path? once and shares it between the two git facts. The route regex is unanchored, so it scans the whole path, and matches? memoizes nothing.
A labkit match is an AND of positive conditions, so "git and not lfs" cannot come from two separate true or false facts. That is why the derived fact exists.
This asymmetry is deliberate. It matches Rack::Attack: the unauthenticated rule excludes every git path, the authenticated rule excludes only non-LFS git, since Rack::Attack treats authenticated LFS as web traffic. repository_git_lfs_route_regex is repository_git_route_regex plus a suffix, so LFS is a strict subset of git and the two exclusions cannot be collapsed into one.
Note that this removes throttling rather than adding it. Unauthenticated Git with the Git HTTP throttle off is now uncounted, which is what Rack::Attack did at every setting. The admin-controlled Git throttle is the intended control for that traffic, and the HAProxy edge limits are unchanged.
Test plan
spec/lib/gitlab/rack_attack/labkit_rate_limit/limiters_spec.rb now carries the new facts in its classification table. Six examples were added, a pair per rule: one checks the claiming throttle still fires while its setting is on, one checks where the request goes once that setting is off.
Each pair names the wider throttle's own setting, so the fall-through it asserts is reachable. The two web rules assert nothing is counted. The LFS pair asserts the request lands on throttle_authenticated_web, which is what Rack::Attack does when the LFS throttle is off and the authenticated web throttle is on. Reverting throttle_authenticated_git_http to path: git fails that example with expected: "authenticated_web", got: "authenticated_git_http".
Full directory run: 134 examples, 0 failures. RuboCop is clean.
Related
- Fixes gitlab-org/gitlab#630787
- Backport to 19.3 required
- !256308 removes the cohort feature flags and must not merge before this lands, because the incident workaround depends on those flags