fix: add per-IP rate limiting to prevent GitHub quota exhaustion#5957
fix: add per-IP rate limiting to prevent GitHub quota exhaustion#5957taniy8 wants to merge 2 commits into
Conversation
|
@taniy8 is attempting to deploy a commit to the jhasourav07's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
🚨 Hey @taniy8, the CI Pipeline is failing on this PR and it has been marked as Please fix the issues before this can be reviewed. Here's how: 1. Run checks locally before pushing: npm run format:check # Check Prettier formatting
npm run lint # Run ESLint
npm run typecheck # TypeScript type check
npm run test # Run unit tests (Vitest)
npm run build # Verify production build passes2. Auto-fix common issues: npm run format # Auto-fix formatting with Prettier
npm run lint -- --fix # Auto-fix lint errors where possible3. Check the full failure log here: Once you push a fix and the CI passes, the |
|
Labels Applied:
Justification: Status: Blocked |
Aamod-Dev
left a comment
There was a problem hiding this comment.
Review
This PR cannot be approved in its current state due to blocking issues (status:blocked label, merge conflicts, needs-rebase label, and/or failing CI checks). Please resolve the blocking issues and re-request review.
Once unblocked, I'm happy to re-review! 💚
Description
Fixes #5955
Problem
app/api/compare/route.tshad zero rate limiting, the single mostexpensive endpoint in the codebase. It calls
getFullDashboardDataTWICEin parallel (once per compared user):
Since
getFullDashboardDataitself fires 7 parallel GitHub API requests,a single call to /api/compare costs 14 GitHub API calls total with no
RateLimiter and no per-IP check anywhere in the file.
Although /api/compare appears in the proxy.ts middleware matcher list,
that middleware is non-functional due to a separate naming bug (tracked
in a different issue), so no protection is applied at any layer in practice.
Fix
Added per-IP rate limiting (5 requests/minute - the lowest of any route
fixed so far, given the 14x GitHub API cost per request) at the top of
the GET handler, consistent with the pattern used in /api/og, /api/notify,
/api/user-details, /api/ci-analytics, and /api/achievements.
Note
The existing test file fails to load with a pre-existing
next-authmodule resolution error, unrelated to this change (route.ts imports
getUserGitHubToken which pulls in the broken auth chain). Verified the
fix via lint and manual code review.
Pillar
Visual Preview
N/A
Checklist before requesting a review:
CONTRIBUTING.mdfile.localhost:3000/api/streak?user=YOUR_USERNAME).npm run formatandnpm run lintlocally and resolved all errors (CI will fail otherwise).feat(themes): ...,fix(calculate): ...).README.mdif I added a new theme or URL parameter.