fix: remove notification management token from URL query parameters#6229
Conversation
Removes the query parameter fallback (managementToken, token) from getNotificationManagementToken(). Tokens are now only accepted via: - x-notification-token HTTP header - managementToken field in the POST request body URL query parameters are logged by servers, CDNs, and proxies, and leaked via HTTP Referer headers, making them unsuitable for sensitive tokens. Fixes JhaSourav07#6132 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. |
… param The test for DELETE /api/notify was passing the management token via URL query parameter, which is no longer supported after the security fix. Updated the test to pass the token via x-notification-token header and extended makeRequest to accept custom headers. Fixes CI failure for PR JhaSourav07#6229
📦 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.
Great security improvement. Removing the token extraction from URL query parameters in lib/notification-management-token.ts prevents accidental exposure in proxy and server logs. The update to app/api/notify/route.test.ts nicely solidifies this new constraint. 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
Removes the query parameter fallback (
managementToken,token) fromgetNotificationManagementToken(). The notification management token is now only accepted via:x-notification-tokenHTTP headermanagementTokenfield in the POST request bodyProblem
URL query parameters are persisted and exposed through channels outside the application's security perimeter:
Refererheaders — If the page with the token loads any third-party resource, the full URL including the token leaks to that third partyAn attacker with access to any of these sources can obtain any user's management token and modify or delete their notification preferences.
Changes
lib/notification-management-token.ts— RemovedsearchParamsparameter and query parameter extraction logic fromgetNotificationManagementToken()app/api/notify/route.ts— Updated DELETE handler to stop passingsearchParamstogetNotificationManagementToken()lib/notification-management-token.empty-fallback.test.ts— Updated tests to verify query parameters are now ignored (security regression tests)Testing
Fixes #6132
Human Coded