Repository navigation
fix(monitors): key rate-limit buckets by caller identity, not IP - #108
Merged
Merged
Conversation
The Next.js server proxies all browser traffic (server components, actions, live polling), so every backend request arrives from a single IP. With the global limiter keyed by IP, one user spamming "check now" burned the shared 100/min bucket and 429'd every read — the dashboard rendered empty because pages fall back to [] on failed fetches. The same collapse would happen behind Caddy in production, where all users share the proxy's IP. Key buckets by bearer token / API key instead, falling back to IP only for anonymous routes (login/signup brute force keeps its IP keying). Route-level overrides (auth, check-now) inherit the keyGenerator, so the 10/min check budget is now per-user too. Global budget raised to 300/min per identity to fit a dashboard that polls live state every 5s and re-renders on every action. Verified: 14 rapid check requests → 1×200 + 13×429, while workspace, monitor, live and stats reads on the same token all stayed 200.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Incident
Follow-up to #107, found by spam-testing it: exceeding the check-now rate limit blanked the entire dashboard — no workspaces, no monitors — until the 1-minute window rolled over and the page was refreshed.
Root cause
The Next.js server proxies all browser traffic to the API — server components, server actions, and the 5s live poll (
/api/monitors/live). The backend therefore sees a single IP for every user. With the global limiter keyed by IP (100/min), one user spamming "check now" (each click = 1 POST + arevalidatePathre-render firing several GETs, on top of the live poll) burned the shared bucket, and every subsequent read 429'd. Pages fall back to[]on failed fetches, so the dashboard rendered as "no data" instead of an error.The same collapse would hit production behind Caddy, where all users share the proxy's IP — one abusive client could 429 everyone.
Fix
Key rate-limit buckets by caller identity instead of IP:
keyGenerator, so the 10/min check budget is now per-user rather than per-shared-IPVerified against the running stack
1×200 + 13×429(cooldown + route limit from feat(monitors): rate-limit the on-demand check-now path #107 intact)GET /workspaces,/monitors,/monitors/liveand/monitors/:id/statson the same token all stayed200— the dashboard no longer self-DoSes