fix(server): restrict CORS to allowed origin from env variable#450
Conversation
✅ Deploy Preview for github-spy ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughWalkthroughThe PR hardens CORS security by replacing wildcard origin configuration with a restricted policy that permits only the frontend client URL and enables credentials support. Environment variable requirements are documented in an example configuration file. ChangesCORS Security Hardening
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes 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)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
backend/server.js (1)
17-20: ⚡ Quick winConsider logging when CLIENT_URL is not explicitly set.
The fallback to
localhost:5173is appropriate for local development, but if CLIENT_URL is accidentally omitted in production, the server will start successfully while CORS silently blocks all legitimate requests. Unlike other environment variables in this file (SESSION_SECRET, MONGO_URI, PORT), CLIENT_URL has a fallback that could mask configuration issues.📋 Suggested enhancement for production safety
Add a startup log to make the CORS origin explicit:
// CORS configuration +const clientURL = process.env.CLIENT_URL || 'http://localhost:5173'; +if (!process.env.CLIENT_URL) { + logger.warn('CLIENT_URL not set, falling back to localhost:5173'); +} app.use(cors({ - origin: process.env.CLIENT_URL || 'http://localhost:5173', + origin: clientURL, credentials: true, })); +logger.info(`CORS enabled for origin: ${clientURL}`);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/server.js` around lines 17 - 20, Add a startup log that explicitly reports which CORS origin is being used so missing CLIENT_URL doesn't silently mask misconfiguration: check process.env.CLIENT_URL when configuring app.use(cors(...)) (or immediately before app.listen) and log a warning if it's undefined while also logging the effective origin value (the process.env.CLIENT_URL or the fallback 'http://localhost:5173') so operators can see whether the environment variable was provided; reference the existing app.use(cors(...)) block and process.env.CLIENT_URL when adding the log.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@backend/server.js`:
- Around line 17-20: Add a startup log that explicitly reports which CORS origin
is being used so missing CLIENT_URL doesn't silently mask misconfiguration:
check process.env.CLIENT_URL when configuring app.use(cors(...)) (or immediately
before app.listen) and log a warning if it's undefined while also logging the
effective origin value (the process.env.CLIENT_URL or the fallback
'http://localhost:5173') so operators can see whether the environment variable
was provided; reference the existing app.use(cors(...)) block and
process.env.CLIENT_URL when adding the log.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a3134437-b045-46b1-85d8-01fbfb581c2c
📒 Files selected for processing (2)
backend/.env.examplebackend/server.js
|
🎉🎉 Thank you for your contribution! Your PR #450 has been merged! 🎉🎉 |
Related Issue
Description
The server used cors() with no origin restriction, allowing any
domain to make credentialed API requests - a security risk.
Replaced with an explicit origin config that reads CLIENT_URL
from environment variables, falling back to localhost:5173 for
local dev. Added credentials: true so session cookies continue
working cross-origin.
backend/server.js: updated CORS configurationbackend/.env.example: added CLIENT_URL variableHow Has This Been Tested?
Screenshots (if applicable)
Type of Change
Summary by CodeRabbit