Conversation
- Restricted CORS origins using ALLOWED_ORIGINS env var - Added robust whitespace handling for comma-separated origins - Defaulted to http://localhost:5173 for local development Co-authored-by: davidraehles <6085055+davidraehles@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
There was a problem hiding this comment.
Pull request overview
This pull request enhances security by replacing a permissive CORS policy that allowed requests from any origin (origin: *) with a whitelist-based approach. The backend now only accepts requests from origins specified in the ALLOWED_ORIGINS environment variable, with a sensible default for local development.
Changes:
- Updated CORS middleware configuration to use an environment-based origin whitelist
- Added robust parsing of comma-separated origins from the
ALLOWED_ORIGINSenvironment variable - Provided a safe default (
http://localhost:5173) for local development when the variable is not set
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| backend/src/server.ts | Modified CORS configuration from permissive origin: * to whitelist-based approach using ALLOWED_ORIGINS environment variable |
| .jules/sentinel.md | Added documentation about the CORS policy fix and key learnings for future reference |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| : ['http://localhost:5173']; | ||
|
|
||
| app.use(cors({ | ||
| origin: allowedOrigins, |
There was a problem hiding this comment.
The CORS configuration is missing the credentials option. Since the application uses JWT tokens sent via Authorization headers and may use cookies in the future, you should add credentials: true to allow browsers to include credentials in cross-origin requests. Without this, authenticated requests from the frontend may fail when deployed to different domains.
| origin: allowedOrigins, | |
| origin: allowedOrigins, | |
| credentials: true, |
There was a problem hiding this comment.
Added credentials: true to the CORS configuration to support cookie-based authentication.
There was a problem hiding this comment.
Done. Added credentials: true to the CORS configuration.
| // Middleware | ||
| app.use(cors()); | ||
| const allowedOrigins = process.env.ALLOWED_ORIGINS | ||
| ? process.env.ALLOWED_ORIGINS.split(',').map(origin => origin.trim()) |
There was a problem hiding this comment.
The environment variable parsing does not handle edge cases properly. If ALLOWED_ORIGINS contains empty strings after splitting (e.g., "http://localhost:5173,"), the resulting array will include empty strings which are invalid origins. Consider filtering out empty strings after trimming to avoid potential security issues or runtime errors.
| ? process.env.ALLOWED_ORIGINS.split(',').map(origin => origin.trim()) | |
| ? process.env.ALLOWED_ORIGINS | |
| .split(',') | |
| .map(origin => origin.trim()) | |
| .filter(origin => origin.length > 0) |
There was a problem hiding this comment.
Added .filter(origin => origin.length > 0) after the trim to prevent empty strings from being included as allowed origins.
There was a problem hiding this comment.
Done. Added .filter(Boolean) to remove any empty strings after trimming.
- Added credentials: true to support cookie-based auth - Added filtering to remove empty strings from allowedOrigins list Co-authored-by: davidraehles <6085055+davidraehles@users.noreply.github.com>
🎯 What: Fixed a permissive CORS policy in
backend/src/server.tsthat defaulted toorigin: *.🛡️ Solution:
backend/src/server.tsto use a whitelist of allowed origins.ALLOWED_ORIGINSenvironment variable.http://localhost:5173) is provided for local development.PR created automatically by Jules for task 8453454130598439823 started by @davidraehles