fix: key rate-limit buckets on X-Forwarded-For behind Spaces ingress

backend/app/main.py

@@ -49,6 +49,19 @@ if os.getenv("PUBLIC_DEPLOY") == "1":
     from slowapi.errors import RateLimitExceeded
     from slowapi.util import get_remote_address
 
+    def client_ip(request: Request) -> str:
+        # Spaces terminates TLS at its ingress, so request.client.host is an
+        # ingress hop, not the user — and the ingress is a fleet, so per-IP
+        # buckets would fragment across hops (verified live: 36 rapid
+        # requests, zero 429s). The real client is in X-Forwarded-For. Use
+        # the RIGHTMOST entry: it's appended by the trusted ingress itself,
+        # while leftmost entries arrive client-controlled — trusting those
+        # would let an attacker mint fresh buckets with a spoofed header.
+        xff = request.headers.get("x-forwarded-for")
+        if xff:
+            return xff.rsplit(",", 1)[-1].strip()
+        return get_remote_address(request)
+
     # One global per-IP budget, NOT per-route @limiter.limit decorators —
     # decorators would force the slowapi import whenever routes.py is
     # imported, which breaks the no-slowapi CI env. application_limits (scope
@@ -56,7 +69,7 @@ if os.getenv("PUBLIC_DEPLOY") == "1":
     # endpoints doesn't multiply the budget by seven. 30/min covers real
     # interactive usage; /api/explain (~50 forward passes per call) is what
     # this protects.
-    limiter = Limiter(key_func=get_remote_address, application_limits=["30/minute"])
+    limiter = Limiter(key_func=client_ip, application_limits=["30/minute"])
     app.state.limiter = limiter
     app.add_exception_handler(RateLimitExceeded, _rate_limit_exceeded_handler)
 

backend/tests/test_public_deploy.py

@@ -63,6 +63,34 @@ def test_public_deploy_rate_limits_api_with_429():
         _restore(["PUBLIC_DEPLOY"])
 
 
+def test_rate_limit_buckets_key_on_rightmost_forwarded_for():
+    """Behind the Spaces ingress the client IP arrives in X-Forwarded-For.
+    Buckets must key on the RIGHTMOST entry (appended by the trusted ingress);
+    leftmost entries are client-controlled, so keying on them would let a
+    spoofed header mint unlimited fresh buckets."""
+    pytest.importorskip("slowapi")
+    module = _reload_with({"PUBLIC_DEPLOY": "1"})
+    try:
+        with TestClient(module.app) as c:
+            exhaust = {"X-Forwarded-For": "6.6.6.6, 9.9.9.9"}
+            statuses = [
+                c.get("/api/health", headers=exhaust).status_code for _ in range(31)
+            ]
+            assert statuses[30] == 429
+
+            # Same trusted (rightmost) hop, different spoofed leftmost entry:
+            # still the SAME bucket — the spoof buys no fresh budget.
+            spoof = {"X-Forwarded-For": "1.2.3.4, 9.9.9.9"}
+            assert c.get("/api/health", headers=spoof).status_code == 429
+
+            # A genuinely different client (different rightmost entry) is
+            # unaffected by the exhausted bucket.
+            other = {"X-Forwarded-For": "6.6.6.6, 7.7.7.7"}
+            assert c.get("/api/health", headers=other).status_code == 200
+    finally:
+        _restore(["PUBLIC_DEPLOY"])
+
+
 def test_default_app_has_no_limiter_or_static_mount():
     """Neither switch set (dev/CI default): no 429s ever, and GET / is a 404 —
     the SPA is served by Vite (dev) or nginx (compose), not FastAPI."""