fix(security): trusted-proxy-aware client IP resolution for rate limiting

api/app/app/config.py

@@ -39,6 +39,13 @@ class Settings(BaseSettings):
 
     frontend_url: str = "http://localhost:5173"
 
+    # Comma-separated CIDRs (e.g. "10.0.0.0/8,172.16.0.0/12") of reverse
+    # proxies/load balancers allowed to set X-Forwarded-For. Left empty by
+    # default: a client-supplied XFF is never trusted unless the direct
+    # TCP peer is in this list, so rate limiting can't be bypassed by
+    # sending an arbitrary XFF header directly to an exposed API.
+    trusted_proxy_cidrs: str = ""
+
     email_backend: Literal["console", "smtp"] = "console"
     smtp_host: str = ""
     smtp_port: int = 587

api/app/app/core/client_ip.py

@@ -0,0 +1,63 @@
+"""Trusted-proxy-aware client IP resolution, shared by every rate limiter.
+
+X-Forwarded-For is attacker-controlled on any request that reaches this
+process directly (no proxy in front, or the proxy isn't configured to
+strip/overwrite it). Trusting it unconditionally lets a client reset its
+own rate-limit bucket on every request just by sending a fresh header
+value. We only honor it when the immediate TCP peer is a configured
+trusted proxy, and even then only take the right-most hop that isn't
+itself one of those proxies.
+"""
+
+import ipaddress
+
+from fastapi import Request
+
+from app.config import settings
+
+_IPNetwork = ipaddress.IPv4Network | ipaddress.IPv6Network
+
+
+def _trusted_networks() -> tuple[_IPNetwork, ...]:
+    # Not cached: settings.trusted_proxy_cidrs can change between tests,
+    # and parsing a short comma list is cheap enough to redo per call.
+    nets = []
+    for raw in settings.trusted_proxy_cidrs.split(","):
+        raw = raw.strip()
+        if not raw:
+            continue
+        try:
+            nets.append(ipaddress.ip_network(raw, strict=False))
+        except ValueError:
+            continue
+    return tuple(nets)
+
+
+def _is_trusted(ip: str) -> bool:
+    try:
+        addr = ipaddress.ip_address(ip)
+    except ValueError:
+        return False
+    return any(addr in net for net in _trusted_networks())
+
+
+def get_client_ip(request: Request) -> str:
+    peer = request.client.host if request.client else "unknown"
+
+    if not _trusted_networks() or not _is_trusted(peer):
+        # No trusted proxies configured, or this request didn't come
+        # from one: never honor a client-supplied X-Forwarded-For.
+        return peer
+
+    xff = request.headers.get("x-forwarded-for")
+    if not xff:
+        return peer
+
+    hops = [h.strip() for h in xff.split(",") if h.strip()]
+    # Walk right-to-left (closest to us first) and return the first hop
+    # that isn't itself one of our trusted proxies -- that's the real
+    # client, since each trusted proxy only ever appends its own address.
+    for hop in reversed(hops):
+        if not _is_trusted(hop):
+            return hop
+    return peer

api/app/app/core/login_rate_limit.py

@@ -2,6 +2,7 @@
 
 from fastapi import HTTPException, Request, status
 
+from app.core.client_ip import get_client_ip
 from app.core.redis import get_redis
 
 logger = logging.getLogger(__name__)
@@ -11,17 +12,8 @@
 LOGIN_RL_PREFIX = "login_rl"
 
 
-def _client_ip(request: Request) -> str:
-    xff = request.headers.get("x-forwarded-for")
-    if xff:
-        first = xff.split(",", 1)[0].strip()
-        if first:
-            return first
-    return request.client.host if request.client else "unknown"
-
-
 def enforce_login_rate_limit(request: Request) -> None:
-    ip = _client_ip(request)
+    ip = get_client_ip(request)
     key = f"{LOGIN_RL_PREFIX}:{ip}"
     try:
         redis = get_redis()

api/app/app/core/rate_limit.py

@@ -5,6 +5,7 @@
 from fastapi import Request, Response
 from starlette.middleware.base import BaseHTTPMiddleware
 
+from app.core.client_ip import get_client_ip
 from app.core.redis import get_redis
 
 logger = logging.getLogger(__name__)
@@ -31,7 +32,7 @@ async def dispatch(
     ) -> Response:
         try:
             redis = get_redis()
-            client_ip = request.client.host if request.client else "unknown"
+            client_ip = get_client_ip(request)
             key = f"{self.key_prefix}:{client_ip}"
 
             pipe = redis.pipeline()

api/app/tests/test_client_ip.py

@@ -0,0 +1,85 @@
+"""get_client_ip must never trust X-Forwarded-For from a client that
+isn't a configured trusted proxy -- otherwise rate limiting is trivially
+bypassed by sending a fresh header value on every request."""
+
+from typing import Any, cast
+
+import pytest
+from fastapi import Request
+
+from app.config import settings
+from app.core.client_ip import get_client_ip
+
+
+class FakeClient:
+    def __init__(self, host: str) -> None:
+        self.host = host
+
+
+class FakeRequest:
+    def __init__(self, peer: str, headers: dict[str, str] | None = None) -> None:
+        self.client = FakeClient(peer)
+        self.headers = headers or {}
+
+
+def _req(peer: str, headers: dict[str, str] | None = None) -> Request:
+    return cast(Request, FakeRequest(peer, headers))
+
+
+@pytest.fixture(autouse=True)
+def _reset_trusted_proxies(monkeypatch: Any) -> Any:
+    monkeypatch.setattr(settings, "trusted_proxy_cidrs", "")
+    yield
+
+
+def test_xff_ignored_when_no_trusted_proxies_configured() -> None:
+    req = _req("203.0.113.9", {"x-forwarded-for": "1.2.3.4"})
+    assert get_client_ip(req) == "203.0.113.9"
+
+
+def test_xff_ignored_when_peer_is_not_a_trusted_proxy(monkeypatch: Any) -> None:
+    monkeypatch.setattr(settings, "trusted_proxy_cidrs", "10.0.0.0/8")
+    # Direct, untrusted client spoofing the header -- must be ignored.
+    req = _req("203.0.113.9", {"x-forwarded-for": "9.9.9.9"})
+    assert get_client_ip(req) == "203.0.113.9"
+
+
+def test_xff_honored_when_peer_is_a_trusted_proxy(monkeypatch: Any) -> None:
+    monkeypatch.setattr(settings, "trusted_proxy_cidrs", "10.0.0.0/8")
+    req = _req("10.0.0.5", {"x-forwarded-for": "198.51.100.7"})
+    assert get_client_ip(req) == "198.51.100.7"
+
+
+def test_xff_right_most_untrusted_hop_used_with_chained_proxies(
+    monkeypatch: Any,
+) -> None:
+    monkeypatch.setattr(settings, "trusted_proxy_cidrs", "10.0.0.0/8")
+    # Real client, then two trusted proxy hops appended their own IPs.
+    # Walking right-to-left skips both trusted hops and lands on the
+    # real client at the left.
+    req = _req(
+        "10.0.0.5",
+        {"x-forwarded-for": "198.51.100.7, 10.0.0.1, 10.0.0.2"},
+    )
+    assert get_client_ip(req) == "198.51.100.7"
+
+
+def test_attacker_cannot_spoof_a_fake_prefix_hop(monkeypatch: Any) -> None:
+    monkeypatch.setattr(settings, "trusted_proxy_cidrs", "10.0.0.0/8")
+    # An attacker connecting directly to our trusted reverse proxy sends
+    # X-Forwarded-For: 6.6.6.6 (trying to impersonate a different IP).
+    # A correctly configured proxy *appends* the real connecting IP
+    # rather than trusting/replacing the client's value, so the header
+    # we actually receive is "6.6.6.6, <attacker's real IP>". Walking
+    # right-to-left must land on the attacker's real IP, not their
+    # spoofed prefix.
+    req = _req("10.0.0.5", {"x-forwarded-for": "6.6.6.6, 198.51.100.42"})
+    assert get_client_ip(req) == "198.51.100.42"
+
+
+def test_no_xff_header_falls_back_to_peer_even_when_trusted(
+    monkeypatch: Any,
+) -> None:
+    monkeypatch.setattr(settings, "trusted_proxy_cidrs", "10.0.0.0/8")
+    req = _req("10.0.0.5")
+    assert get_client_ip(req) == "10.0.0.5"