fix: isolate rate limit counters between middleware and webhook via namespaces#6232
fix: isolate rate limit counters between middleware and webhook via namespaces#6232atul-upadhyay-7 wants to merge 7 commits into
Conversation
…amespaces
The middleware and webhook handler shared the same rate limit counter
per IP address (cache key: ratelimit:{ip}). High traffic to any
middleware-matched route (e.g., /api/streak) would exhaust the
webhook's 10/min limit, causing legitimate GitHub webhook deliveries
to be rejected with 429.
Adds a namespace parameter to rateLimit() so different route groups
use independent counters:
- Middleware uses namespace 'api' (60 req/min)
- Webhook uses namespace 'webhook' (10 req/min)
Cache keys are now: ratelimit:{namespace}:{ip}
Fixes JhaSourav07#6134
Human Coded
|
@atul-upadhyay-7 is attempting to deploy a commit to the jhasourav07's projects Team on Vercel. A member of the Team first needs to authorize it. |
Aamod-Dev
left a comment
There was a problem hiding this comment.
The namespace implementation for rate limit isolation in lib/rate-limit.ts and app/api/webhook/route.ts looks robust and effectively addresses the cross-route interference. The tests in lib/rate-limit.test.ts provide great coverage. However, since the PR is currently marked as blocked, please resolve the blocking issues so we can proceed with merging.
|
GSSoC 2025 — @Aamod007 — blocked, but also noticed a logic bug: the namespace-aware rateLimit call is added alongside the old non-namespace call in webhook/route.ts, resulting in duplicate declarations and double rate-limit calls. The old lines need to be removed. |
…route - Remove old non-namespaced rateLimit() call that was left alongside the new namespace-aware version - Use getClientIp(req) instead of raw x-forwarded-for header - Fixes double rate-limit counting and IP spoofing vulnerability Fixes JhaSourav07#6134
The middleware passes namespace 'api' to rateLimit(), but tests expected the old 3-argument signature. Updated all assertions to match.
📦 Next.js Bundle Size Report (Gzipped Sizes)✨ No significant bundle size changes detected. 📊 Summary of Totals
|
Aamod-Dev
left a comment
There was a problem hiding this comment.
Hey! Isolating the rate limit counters between the middleware and webhooks via distinct namespaces is a fantastic security patch. It prevents a Denial-of-Service vector where an attacker could exhaust the webhook rate limit by spamming the middleware route. Since the Vercel CI is currently failing globally on this repository, I must request changes to keep this safely blocked. Please update when the build passes!
Aamod-Dev
left a comment
There was a problem hiding this comment.
The Vercel CI failure is a known global issue and can be ignored. Since there are no merge conflicts with main, I am officially approving this PR! Great work! 🚀
Summary
Adds a
namespaceparameter to therateLimit()function so that different route groups use independent counters, preventing cross-route rate limit interference.Problem
The middleware (
middleware.ts) and the webhook handler (app/api/webhook/route.ts) both calledrateLimit(ip, ...)using the same cache key patternratelimit:${ip}. This meant:/api/streak,/api/github, etc.) incremented the shared counterAn attacker could deliberately exhaust the webhook rate limit by making badge/streak API requests from the same IP.
Solution
Added a
namespaceparameter (default:default) torateLimit(). Cache keys are nowratelimit:${namespace}:${ip}, so different namespaces maintain independent counters.apiwebhookChanges
lib/rate-limit.ts— Addednamespaceparameter torateLimit(), updated cache key format for both memory and KV pathsmiddleware.ts— Passesnamespace: apitorateLimit()app/api/webhook/route.ts— Passesnamespace: webhooktorateLimit()lib/rate-limit.test.ts— Updated cache key assertions, added namespace isolation testTesting
Fixes #6134
Human Coded