feat(web): opt-in X-Forwarded-For keying for the rate limiter behind a trusted proxy

.env.example

@@ -34,6 +34,11 @@ MEDCHECK_PORT=8080
 # Strongly recommended whenever MEDCHECK_HOST is not 127.0.0.1.
 MEDCHECK_API_KEY=
 
+# Only when a trusted reverse proxy fronts the server: set to 1 so the rate
+# limiter keys on the first X-Forwarded-For hop instead of the proxy's IP.
+# Leave off otherwise — without a proxy the header is client-spoofable.
+MEDCHECK_TRUST_PROXY_HEADERS=
+
 # =============================================================================
 # Default Settings
 # =============================================================================

README.md

@@ -193,6 +193,7 @@ cp .env.example .env
 | `MEDCHECK_PORT` | `8080` | Bind port |
 | `MEDCHECK_API_KEY` | — | When set, `/api` requires an `X-API-Key` header |
 | `MEDCHECK_RATE_LIMIT` | `10` | `POST /api/analyze` requests per IP per minute (`0` = off) |
+| `MEDCHECK_TRUST_PROXY_HEADERS` | off | Key the rate limiter on the first `X-Forwarded-For` hop (`1` — only behind a trusted reverse proxy) |
 | `MEDCHECK_MAX_VISION_IMAGES` | `12` | Max slice images sent to the LLM per analysis |
 | `MEDCHECK_MAX_DOWNLOAD_BYTES` | 2 GiB | Cap on portal exam-ZIP downloads |
 

SECURITY.md

@@ -57,6 +57,13 @@ MedCheck processes medical and health-related queries. Any vulnerability that co
 - **Network exposure is opt-in.** The web server binds to `127.0.0.1` by
   default. When exposing it on the network, set `MEDCHECK_API_KEY` so `/api`
   endpoints require an `X-API-Key` header.
+- **Rate limiting behind a reverse proxy.** The built-in limiter keys on the
+  client socket IP; behind a proxy all clients would share the proxy's IP and
+  one bucket. Set `MEDCHECK_TRUST_PROXY_HEADERS=1` **only** when a trusted
+  proxy fronts the server, so the first `X-Forwarded-For` hop is used instead
+  (client-spoofable without a proxy). In proxied deployments, treat the
+  proxy's own rate limiting as the primary control and the app limiter as a
+  backstop — the in-process limiter is per-worker and not shared state.
 - **Generated reports contain PHI by default.** Reports (JSON/PDF/HTML) embed
   patient name, ID and birth date from the DICOM metadata unless
   `--deidentify` is passed, which replaces them with a stable pseudonym.

src/medcheck/core/config.py

@@ -35,6 +35,9 @@ class Settings:
     host: str = field(default_factory=lambda: os.environ.get("MEDCHECK_HOST", "127.0.0.1"))
     port: int = field(default_factory=lambda: _env_int("MEDCHECK_PORT", 8080))
     api_key: str | None = field(default_factory=lambda: os.environ.get("MEDCHECK_API_KEY"))
+    # Trust X-Forwarded-For from a fronting reverse proxy (rate-limit keying).
+    # Off by default: without a trusted proxy the header is client-spoofable.
+    trust_proxy_headers: bool = field(default_factory=lambda: _env_flag("MEDCHECK_TRUST_PROXY_HEADERS"))
     # Consent gate: patient-derived data is only sent to external cloud LLM APIs
     # when this is explicitly enabled (MEDCHECK_ALLOW_EXTERNAL_LLM=1).
     allow_external_llm: bool = field(default_factory=lambda: _env_flag("MEDCHECK_ALLOW_EXTERNAL_LLM"))

src/medcheck/web/app.py

@@ -155,6 +155,23 @@ def allow(self, key: str) -> bool:
             return True
 
 
+def _client_key(request: Request, trust_proxy_headers: bool) -> str:
+    """Rate-limit bucket key for a request.
+
+    Behind a reverse proxy every request arrives from the proxy's socket IP,
+    which would collapse all clients into one shared bucket. When the operator
+    explicitly declares the proxy trusted (MEDCHECK_TRUST_PROXY_HEADERS), use
+    the first hop of X-Forwarded-For instead. Never trust the header by
+    default — without a proxy in front it is trivially client-spoofable.
+    """
+    if trust_proxy_headers:
+        forwarded = request.headers.get("x-forwarded-for", "")
+        first_hop = forwarded.split(",")[0].strip()
+        if first_hop:
+            return first_hop
+    return request.client.host if request.client else "unknown"
+
+
 def create_app(settings: Settings | None = None) -> FastAPI:
     settings = settings or Settings()
     app = FastAPI(title="MedCheck", version=__version__)
@@ -167,7 +184,7 @@ def create_app(settings: Settings | None = None) -> FastAPI:
     rate_limiter = _RateLimiter(limit=int(os.environ.get("MEDCHECK_RATE_LIMIT", "10")))
 
     def enforce_rate_limit(request: Request) -> None:
-        client_ip = request.client.host if request.client else "unknown"
+        client_ip = _client_key(request, settings.trust_proxy_headers)
         if not rate_limiter.allow(client_ip):
             raise HTTPException(
                 status_code=status.HTTP_429_TOO_MANY_REQUESTS,

tests/unit/test_web.py

@@ -178,6 +178,45 @@ def test_analyze_rate_limited(monkeypatch):
     assert statuses[4] == 429
 
 
+# --- Rate limiting behind a reverse proxy (issue #142) ---
+
+
+def test_rate_limit_ignores_forwarded_header_by_default(monkeypatch):
+    # X-Forwarded-For is client-spoofable; without the trust flag it must NOT
+    # create fresh buckets (else the limiter is trivially bypassed).
+    monkeypatch.setenv("MEDCHECK_RATE_LIMIT", "2")
+    client = TestClient(create_app(Settings(api_key=None, trust_proxy_headers=False)))
+    statuses = [
+        client.post("/api/analyze", json=_VALID_BODY, headers={"X-Forwarded-For": f"10.0.0.{i}"}).status_code
+        for i in range(4)
+    ]
+    assert statuses == [501, 501, 429, 429]
+
+
+def test_rate_limit_keys_on_forwarded_hop_when_proxy_trusted(monkeypatch):
+    # With a trusted proxy, distinct clients (distinct first hops) must not
+    # share one bucket even though every request has the same socket IP.
+    monkeypatch.setenv("MEDCHECK_RATE_LIMIT", "2")
+    client = TestClient(create_app(Settings(api_key=None, trust_proxy_headers=True)))
+    for i in range(4):
+        resp = client.post("/api/analyze", json=_VALID_BODY, headers={"X-Forwarded-For": f"10.0.0.{i}, 172.16.0.1"})
+        assert resp.status_code == 501  # each client has its own bucket
+
+    # ...while one client hammering does get limited.
+    statuses = [
+        client.post("/api/analyze", json=_VALID_BODY, headers={"X-Forwarded-For": "10.0.0.99"}).status_code
+        for _ in range(3)
+    ]
+    assert statuses == [501, 501, 429]
+
+
+def test_rate_limit_trusted_proxy_missing_header_falls_back_to_socket_ip(monkeypatch):
+    monkeypatch.setenv("MEDCHECK_RATE_LIMIT", "2")
+    client = TestClient(create_app(Settings(api_key=None, trust_proxy_headers=True)))
+    statuses = [client.post("/api/analyze", json=_VALID_BODY).status_code for _ in range(3)]
+    assert statuses == [501, 501, 429]
+
+
 # --- Per-request cloud LLM consent (issue #63) ---