Skip to content

Conversation

Impre-visible
Copy link
Owner

This pull request introduces updates to the CORS configuration in the backend and Docker setup to improve security and flexibility. The changes ensure that the application properly handles allowed origins for cross-origin requests.

Backend CORS Configuration Updates:

  • backend/src/main.ts: Modified the origin property in the enableCors method to use a dynamic list of allowed origins, including http://localhost:5173, the value of APP_URL, and any additional origins specified in the CORS_ORIGINS environment variable.

Docker Environment Variable Addition:

  • docker-compose.yml: Added a new CORS_ORIGINS environment variable to specify a comma-separated list of allowed origins for CORS, including http://localhost:5173 and https://invoicerr.example.com.

@Impre-visible Impre-visible linked an issue Jul 31, 2025 that may be closed by this pull request
@Impre-visible Impre-visible requested a review from Copilot July 31, 2025 08:45
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR enhances CORS security by replacing a permissive "allow all origins" configuration with a more restrictive allowlist approach. The changes enable dynamic configuration of allowed origins through environment variables while maintaining development and production URL support.

  • Replaced permissive CORS configuration with specific allowlist-based origin checking
  • Added configurable CORS_ORIGINS environment variable for additional allowed origins
  • Updated Docker configuration to include default CORS origins for localhost and production

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
backend/src/main.ts Replaced permissive CORS origin callback with allowlist array including localhost, APP_URL, and configurable CORS_ORIGINS
docker-compose.yml Added CORS_ORIGINS environment variable with default localhost and production origins

Comment on lines 10 to 12
origin: ['http://localhost:5173', process.env.APP_URL, ...(process.env.CORS_ORIGINS?.split(',').map(o => o.trim()).filter(val => !!val) || [])],
});
app.use(cookieParser());
Copy link
Preview

Copilot AI Jul 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] The complex inline array construction makes this line difficult to read and maintain. Consider extracting the origin building logic into a separate function or variable for better readability.

Suggested change
origin: ['http://localhost:5173', process.env.APP_URL, ...(process.env.CORS_ORIGINS?.split(',').map(o => o.trim()).filter(val => !!val) || [])],
});
app.use(cookieParser());
origin: getCorsOrigins(),
});
app.use(cookieParser());
function getCorsOrigins(): string[] {
const staticOrigins = ['http://localhost:5173', process.env.APP_URL];
const dynamicOrigins = process.env.CORS_ORIGINS?.split(',')
.map(o => o.trim())
.filter(val => !!val) || [];
return [...staticOrigins, ...dynamicOrigins];
}

Copilot uses AI. Check for mistakes.

@Impre-visible Impre-visible merged commit e60009c into main Jul 31, 2025
@Impre-visible Impre-visible deleted the feat/add-CORS_URLS-env-variable/111 branch August 1, 2025 11:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

the new version docker image is not working
1 participant