fix(server): rate-limit requests before token verification - #2372
Conversation
Invalid or missing tokens used to skip the rate limiter entirely, so a caller could send an endless stream of bogus tokens and force a full argon2 verification for each one without ever hitting the limiter. On a publicly reachable instance that is a cheap way to pin the server's CPU. The limiter now runs before verification, keyed on the client IP when a request is present, so unauthenticated callers are throttled too. This matches the intent already documented at the page-content endpoint, where verification is performed before the parameters are read so that malformed requests from unauthenticated callers count against the rate limiter.
felladrin
left a comment
felladrin
left a comment
There was a problem hiding this comment.
Ordering fix is correct: the rate-limit point is consumed before the missing-token check and before argon2, so invalid/missing tokens are throttled and no argon2 work happens once the limiter trips. Rate-limit key is unchanged on the normal path (client IP), valid tokens still consume exactly one point per request, and the verified-token cache is untouched. The three new tests fail on the pre-fix code for the right reason. Tests pass locally (19/19) and CI is green.
Non-blocking follow-ups for later: (1) the per-request argon2 cost is still attacker-chosen - the client sends an encoded argon2id hash and argon2Verify reads m=/t=/p= from that caller-supplied string, so a caller can buy ~500x the legitimate work per request; this PR caps the rate but not the cost, so a shape/param-bounds guard before argon2Verify is worth a separate PR; (2) the no-request fallback key (token ?? "anonymous") gives each bogus token its own bucket - unreachable today since all call sites pass request, but it fails open for a future call site; (3) /search/ still runs its zod parse before handleTokenVerification, so malformed unauthenticated requests there consume no point (only a zod parse, not a DoS lever, but it contradicts the /page-content principle this PR cites); (4) behind a reverse proxy with TRUST_PROXY off, unauthenticated noise now drains the shared per-proxy-IP bucket and can 429 real users - right trade, but worth a deployment-docs line.
2 participants
Description
The rate limiter was only applied after a token passed verification, so requests carrying an invalid or missing token were rejected without ever consuming a rate-limit point. Each rejected request still paid for a full argon2 verification though - and that verification is the expensive part - which means a caller could stream bogus tokens and keep the server busy doing memory-heavy checks with no throttling. On a publicly reachable instance that is a cheap way to pin the CPU.
verifyTokenAndRateLimit now consumes a rate-limit point before doing anything else, keyed on the client IP when a request is present. Invalid and missing tokens are throttled like every other request, and the verification is skipped entirely once the limiter has said no. This also lines up with the intent already documented at the /page-content endpoint, which verifies before reading the parameters so that malformed requests from unauthenticated callers count against the rate limiter.
How to test