fix: fail closed when SUPABASE_JWT_SECRET is missing (#420)#486
fix: fail closed when SUPABASE_JWT_SECRET is missing (#420)#486ionfwsrijan wants to merge 8 commits into
Conversation
|
@ionfwsrijan is attempting to deploy a commit to the firefistisdead's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
Warning Review limit reached
More reviews will be available in 35 minutes and 47 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughServer initialization now triggers uploads cleanup when the backend starts. Express 404 handler middleware signature changes from three to two arguments. Authentication tests are updated to use real Supabase-style JWTs signed with the secret, test user data expands, and a duplicate schema export is removed. ChangesServer and schema maintenance
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
677e2ae to
42ae762
Compare
- Remove duplicate sessionsLookupSchema key in validators/schemas.js - Generate valid JWT tokens in process-from-url tests instead of plaintext Bearer tokens (tests broken by requireSupabaseAuth change) - Remove unused _credCache variable from server.test.js - Remove unused next parameter from 404 handler in server.js - Start uploadsCleanup interval on server startup
…rl tests Replace Buffer return with Readable stream from Buffer to match the streaming download handler, and mock axios.postForm (not axios.post) since the handler now uses axios.postForm.
- Return Buffer instead of Readable stream (handler uses responseType: 'arraybuffer') - Mock axios.post instead of axios.postForm (handler uses axios.post with FormData)
…en since PR FireFistisDead#427); remove debug CI step
|
@FireFistisDead You may review and merge this |
Summary
requireSupabaseAuthmiddleware only enforced token presence whenSUPABASE_JWT_SECRETwas unset — any bearer token-shaped header passed through without cryptographic verification. In production, a missing or mistyped secret silently became an ingest-capable open gate for arbitrary requesters.Made the middleware return HTTP 500 when the secret is not configured, and added a startup validation that throws in production so the server cannot start in an insecure state.
Related issue
Closes #420
Testing
Checklist:
Screenshots / recordings
N/A
Notes
SUPABASE_JWT_SECRETis not set, before any token checkjwt.verify) now always runs since the secret is guaranteed present past the gateSecurity
Summary by CodeRabbit
Chores
Tests