"""The rate-limit bucket must be the caller, not the proxy in front of them. `get_remote_address` reads request.client.host. In staging and production the backend has no published ports and its only caller is the Next proxy, so that host is the Next container — one bucket for every browser user on the site. Measured before this fix: 70 concurrent requests, 60 served and 10 refused. """ from starlette.datastructures import Headers from backend.app import client_key class _Req: """Enough of a Request for the key function: headers and a client host.""" def __init__(self, headers: dict, host: str = "10.0.0.9"): self.headers = Headers(headers) self.client = type("C", (), {"host": host})() self.scope = {"type": "http", "client": (host, 0), "headers": [(k.lower().encode(), v.encode()) for k, v in headers.items()]} def test_cloudflare_header_wins(): # Cloudflare sets CF-Connecting-IP and overwrites any client-supplied # value, so it is trustworthy in a way a parsed XFF chain is not. assert client_key(_Req({"cf-connecting-ip": "203.0.113.7"})) == "203.0.113.7" def test_forwarded_for_is_the_fallback_and_takes_the_first_entry(): # Left-most is the original client; everything after it is proxies. assert client_key( _Req({"x-forwarded-for": "203.0.113.7, 10.0.0.2"})) == "203.0.113.7" def test_remote_address_is_the_last_resort(): assert client_key(_Req({}, host="10.0.0.9")) == "10.0.0.9" def test_cloudflare_header_beats_forwarded_for(): key = client_key(_Req({"cf-connecting-ip": "203.0.113.7", "x-forwarded-for": "198.51.100.1"})) assert key == "203.0.113.7" def test_two_callers_get_two_buckets(): # The whole point: one user exhausting their limit must not refuse another. a = client_key(_Req({"cf-connecting-ip": "203.0.113.7"})) b = client_key(_Req({"cf-connecting-ip": "203.0.113.8"})) assert a != b def test_whitespace_is_stripped(): # "a, b" split on comma leaves a leading space on every entry but the # first; an unstripped key silently creates a second bucket per client. assert client_key(_Req({"x-forwarded-for": " 203.0.113.7 ,10.0.0.2"})) \ == "203.0.113.7" # --------------------------------------------------------------------------- # The ceiling that header rotation cannot raise. # --------------------------------------------------------------------------- import pytest from fastapi.testclient import TestClient @pytest.fixture() def api(monkeypatch): from backend import app as app_module from backend.config import settings monkeypatch.setattr(settings, "global_rate_limit_per_minute", 5) monkeypatch.setattr(app_module, "_global_window", None) return TestClient(app_module.app, raise_server_exceptions=False) def _ceiling_req(path: str, host: str): """Enough of a Request for exempt_from_ceiling: a path and a peer host.""" return type("R", (), { "url": type("U", (), {"path": path})(), "client": type("C", (), {"host": host})(), })() def _get(client, path="/api/flags", cf=None): headers = {"cf-connecting-ip": cf} if cf else {} return client.get(path, headers=headers) def test_rotating_the_cloudflare_header_cannot_buy_unlimited_requests(api): """The attack the per-client keying opened up. client_key trusts CF-Connecting-IP, and nothing in this process can tell an edge-set header from an attacker-set one — that distinction can only be made at Cloudflare, with Authenticated Origin Pulls or an origin firewall. A caller reaching the origin directly can therefore mint a fresh rate-limit bucket per request and evade per-client limits entirely. Per-client fairness is still the right default; this is the backstop that bounds what evading it can achieve. Without it, correct keying would be a net regression against abuse compared with the shared bucket it replaced. """ codes = [_get(api, cf=f"203.0.113.{i}").status_code for i in range(8)] assert codes.count(200) == 5 assert codes.count(429) == 3 def test_the_ceiling_says_which_limit_was_hit(api): # Distinguishable from slowapi's per-client 429, or an operator reading # logs cannot tell "one noisy client" from "the origin is saturated". for i in range(5): _get(api, cf=f"203.0.113.{i}") refused = _get(api, cf="203.0.113.99") assert refused.status_code == 429 assert "capacity" in refused.json()["detail"].lower() assert refused.headers.get("retry-after") def test_traffic_below_the_ceiling_is_untouched(api): codes = [_get(api, cf=f"203.0.113.{i}").status_code for i in range(5)] assert codes == [200] * 5 def test_the_container_healthcheck_is_exempt(api): """The healthcheck runs `curl http://localhost:80/api/data-info` inside the container. If the ceiling could starve it, saturation would fail the healthcheck, restart the container, and turn a load spike into an outage loop — the ceiling has to protect the origin, not kill it. """ from backend.app import exempt_from_ceiling assert exempt_from_ceiling(_ceiling_req("/api/data-info", "127.0.0.1")) assert exempt_from_ceiling(_ceiling_req("/api/data-info", "::1")) # Everyone else is counted. assert not exempt_from_ceiling(_ceiling_req("/api/data-info", "10.0.0.9")) def test_the_ceiling_ignores_non_api_paths(): # Sitemaps and robots.txt are served by this app too, and a crawler # fetching them must not be refused because the API is busy. from backend.app import exempt_from_ceiling assert exempt_from_ceiling(_ceiling_req("/sitemap.xml", "10.0.0.9")) assert exempt_from_ceiling(_ceiling_req("/robots.txt", "10.0.0.9"))