fix(security): sanitize error messages to prevent leaking internal details - #7346
Conversation
…ck with useSearchParams The mock router.replace was not updating useSearchParams, causing the auto-compare useEffect to race with manual setData(json) and wipe data. Added realistic router mock that mirrors Next.js behavior, and disabled window.caches to prevent stale Cache API interference between tests.
📦 Next.js Bundle Size Report (Gzipped Sizes)✨ No significant bundle size changes detected. 📊 Summary of Totals
|
Aamod007
left a comment
There was a problem hiding this comment.
Hey! Thanks for fixing the error message sanitization logic in the streak API. Leaking internal GitHub API errors or unhandled assertions is a real security risk, and preserving only intentional user validation messages is exactly what we need.
I'm assigning level:advanced, type:security, and quality:clean.
Great work, approving!
|
🎉 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! 💻✨ |
Fixes #7263
Problem
The sanitizeErrorMessage function in app/api/streak/route.ts only filtered messages containing ZodError/zod/schema patterns. All other error messages from GitHub API failures, internal assertion errors, or unexpected exceptions were returned verbatim to the client in both JSON and SVG error responses.
This could expose:
Solution
app/api/streak/route.ts
Default to generic messages: sanitizeErrorMessage now returns a generic message for all unrecognized errors instead of passing through the raw message.
Whitelist safe validation messages: Preserved specific user-facing messages for known client errors: org validation, multi-user validation, and rate limit quota warnings.
Classify before sanitizing: Updated buildErrorResponse to detect error type (not-found, rate-limit, validation) using the raw message before sanitization, ensuring correct HTTP status codes while returning sanitized messages to clients.
app/api/streak/route.test.ts
Updated the test assertion for the multi-user validation error to match the line-wrapped SVG output.
Testing