From ada643f0e2abd9b91a5e48c1d0f5209a2bc41c29 Mon Sep 17 00:00:00 2001 From: Ilango Date: Mon, 4 May 2026 14:25:50 +0530 Subject: [PATCH] fix: cors handling (#6304) * fix: cors handling * update: use app url instead of new env var * fix: review feedback * add: tests and validation for cors configuration --- packages/server/src/index.ts | 3 +- packages/server/src/utils/XSS.test.ts | 84 ++++++++++++++++++++++++++- packages/server/src/utils/XSS.ts | 54 ++++++++++++++++- 3 files changed, 137 insertions(+), 4 deletions(-) diff --git a/packages/server/src/index.ts b/packages/server/src/index.ts index ee1d6f6dc..1355a6bbb 100644 --- a/packages/server/src/index.ts +++ b/packages/server/src/index.ts @@ -34,7 +34,7 @@ import { RateLimiterManager } from './utils/rateLimit' import { SSEStreamer } from './utils/SSEStreamer' import { Telemetry } from './utils/telemetry' import { validateAPIKey } from './utils/validateKey' -import { getCorsOptions, getIframeSecurityHeaders, sanitizeMiddleware } from './utils/XSS' +import { getCorsOptions, getIframeSecurityHeaders, sanitizeMiddleware, validateCorsConfig } from './utils/XSS' declare global { namespace Express { @@ -187,6 +187,7 @@ export class App { this.app.set('trust proxy', trustProxy) // Allow access from specified domains + validateCorsConfig() this.app.use(cors(getCorsOptions())) // Parse cookies diff --git a/packages/server/src/utils/XSS.test.ts b/packages/server/src/utils/XSS.test.ts index 8fd5cd197..78035d06d 100644 --- a/packages/server/src/utils/XSS.test.ts +++ b/packages/server/src/utils/XSS.test.ts @@ -6,7 +6,89 @@ jest.mock('./domainValidation', () => ({ validateChatflowDomain: jest.fn() })) -import { getAllowedIframeOrigins, getIframeSecurityHeaders } from './XSS' +jest.mock('./logger', () => ({ + __esModule: true, + default: { warn: jest.fn(), info: jest.fn(), error: jest.fn() } +})) + +import logger from './logger' +import { getAllowedIframeOrigins, getCorsOptions, getIframeSecurityHeaders, validateCorsConfig } from './XSS' + +// --------------------------------------------------------------------------- +// getCorsOptions +// --------------------------------------------------------------------------- + +describe('getCorsOptions', () => { + const SAVED_CORS_ORIGINS = process.env.CORS_ORIGINS + const SAVED_CORS_ALLOW_CREDENTIALS = process.env.CORS_ALLOW_CREDENTIALS + + afterEach(() => { + if (SAVED_CORS_ORIGINS !== undefined) process.env.CORS_ORIGINS = SAVED_CORS_ORIGINS + else delete process.env.CORS_ORIGINS + if (SAVED_CORS_ALLOW_CREDENTIALS !== undefined) process.env.CORS_ALLOW_CREDENTIALS = SAVED_CORS_ALLOW_CREDENTIALS + else delete process.env.CORS_ALLOW_CREDENTIALS + }) + + function getCredentials(corsOrigins: string | undefined, corsAllowCredentials: string | undefined): boolean { + if (corsOrigins === undefined) delete process.env.CORS_ORIGINS + else process.env.CORS_ORIGINS = corsOrigins + if (corsAllowCredentials === undefined) delete process.env.CORS_ALLOW_CREDENTIALS + else process.env.CORS_ALLOW_CREDENTIALS = corsAllowCredentials + + let captured: any + getCorsOptions()({ url: '/api/v1/test' }, (_err: any, options: any) => { + captured = options + }) + return captured.credentials + } + + describe('wildcard + credentials guard', () => { + it('forces credentials to false when CORS_ORIGINS=* and CORS_ALLOW_CREDENTIALS=true', () => { + expect(getCredentials('*', 'true')).toBe(false) + }) + + it('leaves credentials false when CORS_ORIGINS=* and CORS_ALLOW_CREDENTIALS is unset', () => { + expect(getCredentials('*', undefined)).toBe(false) + }) + + it('allows credentials when CORS_ORIGINS is an explicit list', () => { + expect(getCredentials('https://trusted.example.com', 'true')).toBe(true) + }) + + it('allows credentials when CORS_ORIGINS has multiple explicit origins', () => { + expect(getCredentials('https://app.example.com,https://admin.example.com', 'true')).toBe(true) + }) + + it('uses credentials=false when CORS_ALLOW_CREDENTIALS is unset regardless of CORS_ORIGINS', () => { + expect(getCredentials('https://trusted.example.com', undefined)).toBe(false) + }) + }) + + describe('validateCorsConfig', () => { + beforeEach(() => jest.clearAllMocks()) + + it('warns when CORS_ORIGINS=* and CORS_ALLOW_CREDENTIALS=true', () => { + process.env.CORS_ORIGINS = '*' + process.env.CORS_ALLOW_CREDENTIALS = 'true' + validateCorsConfig() + expect(logger.warn).toHaveBeenCalledWith(expect.stringContaining('[CORS]')) + }) + + it('does not warn when CORS_ORIGINS=* without CORS_ALLOW_CREDENTIALS', () => { + process.env.CORS_ORIGINS = '*' + delete process.env.CORS_ALLOW_CREDENTIALS + validateCorsConfig() + expect(logger.warn).not.toHaveBeenCalled() + }) + + it('does not warn when CORS_ORIGINS is an explicit list with CORS_ALLOW_CREDENTIALS=true', () => { + process.env.CORS_ORIGINS = 'https://trusted.example.com' + process.env.CORS_ALLOW_CREDENTIALS = 'true' + validateCorsConfig() + expect(logger.warn).not.toHaveBeenCalled() + }) + }) +}) describe('getAllowedIframeOrigins', () => { const originalEnv = process.env.IFRAME_ORIGINS diff --git a/packages/server/src/utils/XSS.ts b/packages/server/src/utils/XSS.ts index 3cbacd78a..54ffaf8f0 100644 --- a/packages/server/src/utils/XSS.ts +++ b/packages/server/src/utils/XSS.ts @@ -1,6 +1,7 @@ import { NextFunction, Request, Response } from 'express' import sanitizeHtml from 'sanitize-html' import { extractChatflowId, isPublicChatflowRequest, isTTSGenerateRequest, validateChatflowDomain } from './domainValidation' +import logger from './logger' export function sanitizeMiddleware(req: Request, res: Response, next: NextFunction): void { // decoding is necessary as the url is encoded by the browser @@ -29,6 +30,32 @@ export function getAllowCredentials(): boolean { return process.env.CORS_ALLOW_CREDENTIALS === 'true' } +export function getAllowedAuthCorsOrigins(): string[] { + const appUrl = process.env.APP_URL?.trim() + if (!appUrl) return [] + try { + return [new URL(appUrl).origin.toLowerCase()] + } catch { + return [] + } +} + +// Endpoints that issue or refresh session tokens — must not accept wildcard origins +const SESSION_ENDPOINTS = [ + '/api/v1/auth/login', + '/api/v1/auth/refreshtoken', + '/api/v1/account/register', + '/api/v1/azure/callback', + '/api/v1/google/callback', + '/api/v1/auth0/callback', + '/api/v1/github/callback' +] + +function isSessionEndpoint(url: string): boolean { + const path = url.split('?')[0].toLowerCase() + return SESSION_ENDPOINTS.some((ep) => path === ep || path.startsWith(ep + '/')) +} + function parseAllowedOrigins(allowedOrigins: string): string[] { if (!allowedOrigins) { return [] @@ -42,12 +69,26 @@ function parseAllowedOrigins(allowedOrigins: string): string[] { .filter((origin) => origin.length > 0) } +export function validateCorsConfig(): void { + if (getAllowedCorsOrigins() === '*' && getAllowCredentials()) { + logger.warn( + '[CORS] Unsafe configuration detected: CORS_ORIGINS=* cannot be combined with ' + + 'CORS_ALLOW_CREDENTIALS=true. Access-Control-Allow-Credentials has been forced to false. ' + + 'To allow credentialed cross-origin requests, set CORS_ORIGINS to an explicit comma-separated ' + + 'list of trusted origins instead of *.' + ) + } +} + export function getCorsOptions(): any { return (req: any, callback: (err: Error | null, options?: any) => void) => { + const allowedOrigins = getAllowedCorsOrigins() + const requestedCredentials = getAllowCredentials() + const credentials = allowedOrigins === '*' ? false : requestedCredentials + const corsOptions = { - credentials: getAllowCredentials(), + credentials, origin: async (origin: string | undefined, originCallback: (err: Error | null, allow?: boolean) => void) => { - const allowedOrigins = getAllowedCorsOrigins() const isPublicChatflowReq = isPublicChatflowRequest(req.url) const isTTSReq = isTTSGenerateRequest(req.url) const allowedList = parseAllowedOrigins(allowedOrigins) @@ -56,6 +97,15 @@ export function getCorsOptions(): any { // Always allow no-Origin requests (same-origin, server-to-server) if (!originLc) return originCallback(null, true) + // Block null origins (sandboxed iframes, data: URIs, file:// pages) + if (originLc === 'null') return originCallback(null, false) + + // Session-issuing endpoints: ignore global wildcard, use APP_URL origin or explicit CORS_ORIGINS list + if (isSessionEndpoint(req.url)) { + const authList = getAllowedAuthCorsOrigins() + return originCallback(null, authList.includes(originLc) || allowedList.includes(originLc)) + } + // Global allow: '*' or exact match const globallyAllowed = allowedOrigins === '*' || allowedList.includes(originLc)