Robert Helewka
5c178bb7bd
Blanket per-endpoint limits would have been the wrong shape here. Every REST/WS/MCP surface is owner-only — an unauthenticated request is rejected by resolve_bearer/is_owner before any handler runs — so limiting them would mostly throttle the single legitimate operator, and real spend control for outbound calls is already max_concurrent_calls in gateway.make_call. What is genuinely exposed is the handful of /auth/* routes that must answer before an identity exists. /auth/callback and /auth/refresh-callback each make an outbound token exchange with Casdoor on every request; /auth/me opens a DB session and runs a token lookup. All are free to trigger and none are cheap to serve. /auth/logout is left unlimited — it builds a redirect URL and does no I/O. Not a defence against credential guessing: PATs are secrets.token_urlsafe(32) (256 bits) compared by SHA-256 digest, so brute force was never the threat. This is about unauthenticated work an attacker controls. Fixed-window, in-process, no new dependency — one operator and one process make a shared counter store infrastructure without a purpose. The bucket store is bounded and evicts oldest-first, since an unbounded map keyed by source address would itself be the exhaustion vector. The limiter keys on the socket peer and deliberately ignores X-Forwarded-For. That header is attacker-controlled unless a trusted proxy overwrites it, and this app establishes no such trust; keying on it would let one client present as thousands and make the limiter worse than useless. Behind the estate's reverse proxy the limit is therefore per-proxy, not per-caller — correct for exhaustion and honest about what it can enforce. Per-caller limits need an explicit trusted-proxy config, noted in CLAUDE.md so it isn't added silently. Verified against a real server: exactly 30 requests pass, then 429 with Retry-After: 60, while an owner-gated route serves 40/40. The 429s appear in the JSON access log with queryable status_code and client_addr, so an attack is visible in Loki. The wiring test identifies the dependency by qualname rather than string search, and was mutation-checked by removing the limit from /auth/me. Also documents 401/403/429 in the API reference — 401 and 403 have existed since auth landed but were never in the status-code table. Phase 4 is now complete. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
133 lines
5.1 KiB
Python
133 lines
5.1 KiB
Python
"""
|
||
Rate limiting for the unauthenticated edge.
|
||
|
||
**Scope, and why it is narrow.** Every REST/WS/MCP surface is owner-only: an
|
||
unauthenticated request is rejected by `resolve_bearer`/`is_owner` before any
|
||
handler runs, and real spend control for outbound calls is
|
||
`max_concurrent_calls` in `gateway.make_call`. Blanket per-endpoint limits would
|
||
therefore mostly rate-limit the single legitimate operator. What is genuinely
|
||
exposed is the handful of `/auth/*` routes that must answer before an identity
|
||
exists — those are limited here, and nothing else.
|
||
|
||
**What this defends against.** Not credential guessing: PATs are
|
||
`secrets.token_urlsafe(32)` (256 bits) compared by SHA-256 digest, so brute
|
||
force is not a practical threat. The concern is *unauthenticated work an
|
||
attacker controls*: `/auth/callback` makes an outbound token-exchange round-trip
|
||
to Casdoor on every request, and `/auth/me` opens a DB session and runs a token
|
||
lookup. Both are free to trigger and neither is cheap to serve.
|
||
|
||
**Fixed-window, in-process, no dependency.** One operator and a single process
|
||
mean a shared counter store would be infrastructure without a purpose. The
|
||
trade-off of a fixed window is a burst of up to 2× the limit across a boundary;
|
||
that is irrelevant at these thresholds, and it costs one dict lookup with no
|
||
background task.
|
||
|
||
**Client identity is the socket peer, deliberately.** `X-Forwarded-For` is
|
||
attacker-controlled unless a trusted proxy overwrites it, and this app does not
|
||
currently establish that trust (`_public_base_url` reads forwarded headers, but
|
||
only to build URLs). Keying on a spoofable header would let one client present
|
||
as thousands and make the limiter worse than useless. Behind the estate's
|
||
reverse proxy this means the limit applies per-proxy rather than per-caller —
|
||
correct for exhaustion, and honest about what it can enforce. If per-caller
|
||
limits are ever needed, that needs an explicit trusted-proxy config, not a
|
||
silent `X-Forwarded-For` read.
|
||
"""
|
||
|
||
import time
|
||
|
||
from fastapi import HTTPException, Request
|
||
|
||
# Requests allowed per window, per client, per route. Sized to be invisible to a
|
||
# human — a dashboard load touches /auth/me once — while capping automated
|
||
# hammering.
|
||
DEFAULT_LIMIT = 30
|
||
DEFAULT_WINDOW_SECONDS = 60
|
||
|
||
# Stop the bucket store growing without bound under a spray of source addresses.
|
||
# Eviction is oldest-first and only runs when the cap is exceeded.
|
||
MAX_TRACKED_CLIENTS = 10_000
|
||
|
||
|
||
class RateLimiter:
|
||
"""Fixed-window request counter, keyed by (route, client)."""
|
||
|
||
def __init__(
|
||
self,
|
||
limit: int = DEFAULT_LIMIT,
|
||
window_seconds: int = DEFAULT_WINDOW_SECONDS,
|
||
max_clients: int = MAX_TRACKED_CLIENTS,
|
||
):
|
||
self.limit = limit
|
||
self.window_seconds = window_seconds
|
||
self.max_clients = max_clients
|
||
# key -> [window_start, count]. Insertion-ordered, which is what makes
|
||
# oldest-first eviction a cheap `next(iter(...))`.
|
||
self._buckets: dict[str, list[float]] = {}
|
||
|
||
def check(
|
||
self, key: str, limit: int | None = None, now: float | None = None
|
||
) -> tuple[bool, int]:
|
||
"""Record a hit. Returns (allowed, retry_after_seconds).
|
||
|
||
`retry_after` is 0 when allowed, and the seconds remaining in the
|
||
current window when not. `limit` overrides the instance default for
|
||
routes that want a tighter cap.
|
||
"""
|
||
now = time.monotonic() if now is None else now
|
||
effective = self.limit if limit is None else limit
|
||
bucket = self._buckets.get(key)
|
||
|
||
if bucket is None or now - bucket[0] >= self.window_seconds:
|
||
self._buckets.pop(key, None) # re-insert so ordering tracks recency
|
||
self._buckets[key] = [now, 1]
|
||
self._evict_if_needed()
|
||
return True, 0
|
||
|
||
bucket[1] += 1
|
||
if bucket[1] > effective:
|
||
remaining = self.window_seconds - (now - bucket[0])
|
||
return False, max(1, int(remaining) + 1)
|
||
return True, 0
|
||
|
||
def _evict_if_needed(self) -> None:
|
||
while len(self._buckets) > self.max_clients:
|
||
self._buckets.pop(next(iter(self._buckets)))
|
||
|
||
def reset(self) -> None:
|
||
self._buckets.clear()
|
||
|
||
|
||
def client_key(request: Request, scope: str) -> str:
|
||
"""Identify the caller for limiting purposes.
|
||
|
||
Uses the socket peer, never a forwarded header — see the module docstring.
|
||
"""
|
||
client = request.client.host if request.client else "unknown"
|
||
return f"{scope}:{client}"
|
||
|
||
|
||
_limiter = RateLimiter()
|
||
|
||
|
||
def get_limiter() -> RateLimiter:
|
||
return _limiter
|
||
|
||
|
||
def rate_limit(scope: str, limit: int | None = None):
|
||
"""FastAPI dependency factory limiting one route.
|
||
|
||
Applied per-route rather than as middleware so the authenticated surfaces —
|
||
which are already owner-gated — pay nothing.
|
||
"""
|
||
|
||
async def _dependency(request: Request) -> None:
|
||
allowed, retry_after = get_limiter().check(client_key(request, scope), limit=limit)
|
||
if not allowed:
|
||
raise HTTPException(
|
||
status_code=429,
|
||
detail="Too many requests",
|
||
headers={"Retry-After": str(retry_after)},
|
||
)
|
||
|
||
return _dependency
|