fix: use getClientIp in webhook endpoint to prevent IP spoofing (#6014)#6047
Conversation
|
@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.
This is a solid security fix — swapping the raw
eq.headers.get('x-forwarded-for')\ in \�pp/api/webhook/route.ts:38\ for the proper \getClientIp(req)\ helper, with comprehensive test mocks in
oute.test.ts\ for unique IPs per test case. Unfortunately this PR has \status:blocked\ (failing CI), so I can't approve it yet. Get those CI checks green and I'll be happy to approve!
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! 💚
📦 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.
Using getClientIp() instead of directly reading the x-forwarded-for header in app/api/webhook/route.ts is crucial for preventing IP spoofing and rate limit bypasses. Refactoring app/api/webhook/route.test.ts to use a mocked getClientIp properly isolates the tests. Approved!
|
🎉 Congratulations @atul-upadhyay-7! Your PR has been successfully merged. 🚀 Thank you for contributing to CommitPulse. Your work helps us build a better tool for the community.
Keep building! 💻✨ |
Summary
This PR fixes a security vulnerability where the webhook endpoint reads the client IP directly from x-forwarded-for instead of using the secure getClientIp() helper, making rate limiting trivially bypassable.
Changes
Added getClientIp import ():
Replaced direct header reading with getClientIp() ():
Updated tests ():
Security Benefits
Issue
Fixes #6014
Testing