From 02a1e035a5fa648a5aada97d853bd1881ca6241f Mon Sep 17 00:00:00 2001
From: Amr Gaber <amrtgaber@gmail.com>
Date: Mon, 27 Jul 2026 22:52:31 -0500
Subject: [PATCH] Rate limit all routes with a global default

Auth endpoints kept their strict per-path limits but every other route
was unlimited. Apply a 300/min per-client-IP default to all non-exempt
routes through the existing in-house middleware (immune to the FastAPI
>=0.137 / slowapi issue #281 SlowAPIMiddleware regression), exempting
the infrastructure paths probes and docs rely on. The limit is consumed
regardless of auth outcome, so unauthenticated traffic is capped too.
Add canary and exemption tests plus a limiter-reset fixture so request
counts don't leak across tests.
---
 app/main.py                 | 43 +++++++++++++++++++++++++++++++------
 tests/conftest.py           | 11 ++++++++++
 tests/test_rate_limiting.py | 29 +++++++++++++++++++++++++
 3 files changed, 76 insertions(+), 7 deletions(-)
 create mode 100644 tests/test_rate_limiting.py

diff --git a/app/main.py b/app/main.py
index c6a6ad1..bae5678 100644
--- a/app/main.py
+++ b/app/main.py
@@ -79,19 +79,48 @@ async def get_current_user(user: User = Depends(current_active_user)):
 # OAuth endpoints use GET instead of POST
 _OAUTH_PATHS = {"/auth/google/authorize", "/auth/google/callback"}
 
+# Global default for every other route, matching the per-client-IP keying of
+# the auth limits. Enforced here in-house (limits via limiter._limiter)
+# rather than through slowapi's SlowAPIMiddleware, whose default_limits
+# enforcement FastAPI >=0.137 breaks for routes mounted via include_router
+# (slowapi issue #281).
+_DEFAULT_RATE_LIMIT: RateLimitItem = parse("300/minute")
+
+# Infrastructure paths that must never 429: probes, docs, and the security
+# contact file.
+_RATE_LIMIT_EXEMPT_PATHS = {
+    "/",
+    "/health",
+    "/docs",
+    "/openapi.json",
+    "/.well-known/security.txt",
+}
+
 
 @app.middleware("http")
-async def rate_limit_auth(request: Request, call_next) -> Response:
-    """Apply rate limits to auth endpoints."""
-    rate_limit = _AUTH_RATE_LIMITS.get(request.url.path)
-    is_oauth = request.url.path in _OAUTH_PATHS
-    if rate_limit and (request.method == "POST" or is_oauth):
+async def rate_limit(request: Request, call_next) -> Response:
+    """Apply rate limits.
+
+    Auth endpoints get their strict per-path limits; every other route gets
+    the global default except the exempt infrastructure paths. A request
+    that consumed an auth limit does not also consume the default bucket.
+    """
+    path = request.url.path
+    auth_limit = _AUTH_RATE_LIMITS.get(path)
+    is_oauth = path in _OAUTH_PATHS
+    if auth_limit and (request.method == "POST" or is_oauth):
+        rate_limit_item: RateLimitItem | None = auth_limit
+    elif path in _RATE_LIMIT_EXEMPT_PATHS:
+        rate_limit_item = None
+    else:
+        rate_limit_item = _DEFAULT_RATE_LIMIT
+    if rate_limit_item is not None:
         key = get_remote_address(request)
-        if not limiter._limiter.hit(rate_limit, key):
+        if not limiter._limiter.hit(rate_limit_item, key):
             log_security_event(
                 SecurityEvent.RATE_LIMIT_HIT,
                 request=request,
-                detail=f"path={request.url.path}",
+                detail=f"path={path}",
             )
             return JSONResponse(
                 status_code=429,
diff --git a/tests/conftest.py b/tests/conftest.py
index 7e56b48..ac417c2 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -17,6 +17,17 @@
 async_session_maker = async_sessionmaker(engine, class_=AsyncSession, expire_on_commit=False)
 
 
+@pytest.fixture(autouse=True)
+def reset_rate_limiter():
+    """Clear rate-limit buckets between tests so request counts don't leak
+    across tests (the limiter storage is process-global)."""
+    from app.main import limiter
+
+    limiter._storage.reset()
+    yield
+    limiter._storage.reset()
+
+
 @pytest.fixture(autouse=True)
 async def setup_database():
     """Create tables before each test, drop after."""
diff --git a/tests/test_rate_limiting.py b/tests/test_rate_limiting.py
new file mode 100644
index 0000000..5fd3669
--- /dev/null
+++ b/tests/test_rate_limiting.py
@@ -0,0 +1,29 @@
+"""Global rate limiting behaviour.
+
+Every route outside the exempt infrastructure set gets the in-house
+300/min default (auth endpoints keep their stricter per-path limits).
+The default is enforced by our own middleware calling `limits` directly,
+so it is immune to the FastAPI >=0.137 / slowapi issue #281 regression
+that breaks SlowAPIMiddleware-based default_limits.
+"""
+
+from httpx import AsyncClient
+
+
+class TestGlobalRateLimit:
+    async def test_default_limit_applies_to_non_auth_routes(self, client: AsyncClient):
+        # /collections is an ordinary API route with no per-path limit, so
+        # it gets the 300/min default (matches _DEFAULT_RATE_LIMIT in
+        # app/main.py — bump both together if that changes). The limit is
+        # consumed regardless of auth outcome, so unauthenticated traffic
+        # cannot hammer the route either.
+        for _ in range(300):
+            assert (await client.get("/collections")).status_code != 429
+        assert (await client.get("/collections")).status_code == 429
+
+    async def test_infrastructure_routes_are_exempt(self, client: AsyncClient):
+        # / and /health must never 429 — probes and uptime checks hit them
+        # far more often than any human client.
+        for _ in range(310):
+            assert (await client.get("/health")).status_code == 200
+        assert (await client.get("/")).status_code == 200