Skip to content

Cors Issue Resolved#5

Merged
Shubham4026 merged 1 commit into
mainfrom
dev-release
May 7, 2026
Merged

Cors Issue Resolved#5
Shubham4026 merged 1 commit into
mainfrom
dev-release

Conversation

@Shubham4026
Copy link
Copy Markdown
Collaborator

No description provided.

@Shubham4026 Shubham4026 merged commit 3e4f25d into main May 7, 2026
0 of 2 checks passed
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request updates the CORS configuration to reflect the request origin and enables credential support. The review identifies a critical security vulnerability where allowing credentials while reflecting any origin could lead to unauthorized authenticated requests. Additionally, the explicit inclusion of the OPTIONS method is noted as redundant.

Comment thread src/config/app.config.ts
Comment on lines +8 to +12
if (!process.env.CORS_ORIGINS?.trim()) return true; // true = reflect any origin
const list = process.env.CORS_ORIGINS.split(',')
.map((o) => o.trim())
.filter(Boolean);
return list.length > 0 ? list : ['*'];
return list.length > 0 ? list : true;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

security-high high

Reflecting any origin by returning true is a security risk when credentials: true is enabled in the CORS configuration. This allows any website to make authenticated requests to your API on behalf of a user. It is highly recommended to use a specific whitelist of allowed domains instead of reflecting the request origin.

Comment thread src/main.ts
'tenantid',
'academicyearid',
],
credentials: true,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

security-high high

Enabling credentials: true while the origin is configured to reflect any requester (via true in app.config.ts) creates a security vulnerability. This configuration allows any malicious website to perform authenticated actions on behalf of your users. If credentials are required, the origin must be restricted to a trusted whitelist.

Comment thread src/main.ts
app.enableCors({
origin: corsOrigins,
methods: ['GET', 'POST', 'PUT', 'DELETE', 'PATCH'],
methods: ['GET', 'POST', 'PUT', 'DELETE', 'PATCH', 'OPTIONS'],
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The OPTIONS method is automatically handled by the CORS middleware for preflight requests. Explicitly including it in the methods array is redundant.

Suggested change
methods: ['GET', 'POST', 'PUT', 'DELETE', 'PATCH', 'OPTIONS'],
methods: ['GET', 'POST', 'PUT', 'DELETE', 'PATCH'],

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.

1 participant