Rate limiter: per-IP bucketing and low /api and /auth ceilings won't hold at scale #1

New issue

Open

opened 2026-09-26 17:16:24 +02:00 by eofredj · 1 comment

eofredj commented

2026-09-26 17:16:24 +02:00

Owner

Problem

backend/app/middleware/rate_limit.py limits are tight enough to cause real, user-visible 429s once usage grows past a handful of concurrent users, independent of the nginx-layer limits in frontend/nginx.conf.template.

Current backend middleware limits (per client IP, sliding 60s window):

/api/v1/*: 100 req/60s
/auth/* and /api/v1/auth/* (except /api/v1/auth/me, carved out to the 100/60s bucket): 10 req/60s

nginx sits in front with its own, more generous, per-second-smoothed limits (10 r/s / burst 20 for api, 5 r/s / burst 10 for auth — roughly 600/min and 300/min respectively), so in practice the backend middleware is the real bottleneck.

Why this bites at scale

Bucketed by IP, not by authenticated user (_get_client_ip uses X-Real-IP). Any users sharing an IP (office network, VPN, CGNAT) share one bucket and can lock each other out.
100 req/60s is low if some views fire on the order of tens of API requests. A single active user bouncing between 2-3 chatty views inside a minute can exhaust the whole budget alone — this doesn't require multiple users to reproduce.
In-memory, per-process buckets. uvicorn currently runs with no --workers flag, so there's one bucket store today, but the module's own docstring already flags that adding workers or replicas later silently multiplies the effective limit unevenly (no shared store, e.g. Redis).

Suggested directions (not mutually exclusive, needs a decision before implementing — this touches auth-adjacent security logic)

Raise /api/v1/* from 100 to something closer to nginx's own ceiling (e.g. 300-600/60s), since nginx's docstring already states it's meant to be the authoritative layer and the backend middleware is defense-in-depth.
Bucket authenticated requests by user ID instead of/in addition to IP, falling back to IP only for anonymous/auth endpoints — fixes the shared-IP collision directly.
Add a Redis-backed (or similar shared) store before ever scaling to multiple backend workers/replicas, to avoid silent effective-limit drift.

How this was found

Surfaced while regenerating manual/landing-page screenshots against a local dev instance seeded with demo data — the seeding script hit 429s from normal API usage patterns that mimic real multi-view navigation, which is what prompted a closer look at the limiter's actual ceilings.

## Problem `backend/app/middleware/rate_limit.py` limits are tight enough to cause real, user-visible 429s once usage grows past a handful of concurrent users, independent of the nginx-layer limits in `frontend/nginx.conf.template`. Current backend middleware limits (per client IP, sliding 60s window): - `/api/v1/*`: 100 req/60s - `/auth/*` and `/api/v1/auth/*` (except `/api/v1/auth/me`, carved out to the 100/60s bucket): 10 req/60s nginx sits in front with its own, more generous, per-second-smoothed limits (10 r/s / burst 20 for `api`, 5 r/s / burst 10 for `auth` — roughly 600/min and 300/min respectively), so in practice the backend middleware is the real bottleneck. ## Why this bites at scale 1. **Bucketed by IP, not by authenticated user** (`_get_client_ip` uses `X-Real-IP`). Any users sharing an IP (office network, VPN, CGNAT) share one bucket and can lock each other out. 2. **100 req/60s is low if some views fire on the order of tens of API requests.** A single active user bouncing between 2-3 chatty views inside a minute can exhaust the whole budget alone — this doesn't require multiple users to reproduce. 3. **In-memory, per-process buckets.** `uvicorn` currently runs with no `--workers` flag, so there's one bucket store today, but the module's own docstring already flags that adding workers or replicas later silently multiplies the effective limit unevenly (no shared store, e.g. Redis). ## Suggested directions (not mutually exclusive, needs a decision before implementing — this touches auth-adjacent security logic) - Raise `/api/v1/*` from 100 to something closer to nginx's own ceiling (e.g. 300-600/60s), since nginx's docstring already states it's meant to be the authoritative layer and the backend middleware is defense-in-depth. - Bucket authenticated requests by user ID instead of/in addition to IP, falling back to IP only for anonymous/auth endpoints — fixes the shared-IP collision directly. - Add a Redis-backed (or similar shared) store before ever scaling to multiple backend workers/replicas, to avoid silent effective-limit drift. ## How this was found Surfaced while regenerating manual/landing-page screenshots against a local dev instance seeded with demo data — the seeding script hit 429s from normal API usage patterns that mimic real multi-view navigation, which is what prompted a closer look at the limiter's actual ceilings.

eofredj self-assigned this

2026-09-26 17:41:40 +02:00

eofredj commented

2026-09-26 17:50:49 +02:00

Author

Owner

Addressed the first two directions in commit bb31d11a:

Raised /api/v1/* from 100 to 400 req/60s, closer to nginx's own ceiling, so a normal user bouncing between a couple of chatty views no longer exhausts the budget alone.
Bucket authenticated requests by user, not IP, when the request carries a verifiable local-JWT access_token cookie (checked synchronously via validate_local_token, bucketed by sub). This fixes the shared-IP collision problem for the common local-auth case. API tokens, OIDC sessions, and /auth/* itself still fall back to IP — see the docstrings in rate_limit.py for why each of those can't be safely/cheaply verified at this layer.
- Note: an earlier version of this change bucketed API-token requests (Authorization: Bearer ptr_...) by the raw, unverified header value, which a background security review flagged as a HIGH-severity bypass (any attacker could send a fresh fake token per request and get an unbounded new bucket every time). That branch was removed before landing — API tokens fall back to IP like before.

Still open, not attempted here: Redis-backed (or similar shared) bucket store before ever scaling to multiple backend workers/replicas — today's in-memory buckets are per-process, so the module docstring's own caveat about effective-limit drift under multiple workers still applies. Leaving this open since it's a bigger infrastructure change than the other two.

Docs updated in the same commit: docs/architecture/middleware.md, docs/security/overview.md. Backend test suite (2478 tests) passes; new/updated tests cover same-IP-different-user bucketing and the fallback-to-IP-on-invalid-token case.

Addressed the first two directions in commit bb31d11a: - **Raised `/api/v1/*` from 100 to 400 req/60s**, closer to nginx's own ceiling, so a normal user bouncing between a couple of chatty views no longer exhausts the budget alone. - **Bucket authenticated requests by user, not IP**, when the request carries a *verifiable* local-JWT `access_token` cookie (checked synchronously via `validate_local_token`, bucketed by `sub`). This fixes the shared-IP collision problem for the common local-auth case. API tokens, OIDC sessions, and `/auth/*` itself still fall back to IP — see the docstrings in `rate_limit.py` for why each of those can't be safely/cheaply verified at this layer. - Note: an earlier version of this change bucketed API-token requests (`Authorization: Bearer ptr_...`) by the raw, unverified header value, which a background security review flagged as a HIGH-severity bypass (any attacker could send a fresh fake token per request and get an unbounded new bucket every time). That branch was removed before landing — API tokens fall back to IP like before. Still open, not attempted here: **Redis-backed (or similar shared) bucket store** before ever scaling to multiple backend workers/replicas — today's in-memory buckets are per-process, so the module docstring's own caveat about effective-limit drift under multiple workers still applies. Leaving this open since it's a bigger infrastructure change than the other two. Docs updated in the same commit: `docs/architecture/middleware.md`, `docs/security/overview.md`. Backend test suite (2478 tests) passes; new/updated tests cover same-IP-different-user bucketing and the fallback-to-IP-on-invalid-token case.

eofredj referenced this issue from a commit

2026-09-26 18:47:48 +02:00

fix(rate-limit): bucket authenticated requests by user, raise API ceiling