fix(server): preserve custom iframe allowlists in frame headers (#6232)

* Preserve explicit iframe allowlists when hardening frame headers

The clickjacking fix correctly added frame-ancestors controls, but custom allowlists still emitted X-Frame-Options: DENY. This keeps SAMEORIGIN for 'self' and DENY for 'none', while letting explicit allowlists rely on CSP alone so supported embeds continue to work.

Constraint: Legacy X-Frame-Options cannot express arbitrary allowlists

Rejected: Keep DENY for all non-self values | breaks documented IFRAME_ORIGINS allowlists

Confidence: high

Scope-risk: narrow

Reversibility: clean

Directive: Keep CSP as the source of truth for custom iframe allowlists; do not reintroduce contradictory XFO headers

Tested: Added unit coverage for wildcard, self, none, and custom allowlists; local header reproduction for custom origin

Not-tested: Full browser iframe E2E across legacy browsers

* Avoid malformed iframe CSP values while removing per-request header recomputation

Gemini's review pointed out two low-risk improvements that strengthen the
existing iframe allowlist fix without changing its intent: normalize away empty
CSV tokens before building frame-ancestors, and compute the static header set
once during app startup instead of rebuilding it on every request.

Constraint: IFRAME_ORIGINS is startup configuration, not a dynamic per-request input
Rejected: Larger header-construction refactor | not necessary to address the concrete review feedback
Confidence: high
Scope-risk: narrow
Reversibility: clean
Directive: Keep CSP token normalization aligned with explicit allowlists so empty CSV segments never leak into frame-ancestors
Tested: pnpm --dir packages/server test -- src/utils/XSS.test.ts --runInBand
Not-tested: Full browser iframe embed verification against a running server

---------

Co-authored-by: yau-wd <yau.ong@workday.com>

Author: shaun0927

packages/server/src/index.ts

@@ -33,7 +33,7 @@ import { RateLimiterManager } from './utils/rateLimit'
 import { SSEStreamer } from './utils/SSEStreamer'
 import { Telemetry } from './utils/telemetry'
 import { validateAPIKey } from './utils/validateKey'
-import { getAllowedIframeOrigins, getCorsOptions, sanitizeMiddleware } from './utils/XSS'
+import { getCorsOptions, getIframeSecurityHeaders, sanitizeMiddleware } from './utils/XSS'
 
 declare global {
     namespace Express {
@@ -187,20 +187,10 @@ export class App {
         this.app.use(cookieParser())
 
         // Allow embedding from specified domains.
+        const iframeSecurityHeaders = getIframeSecurityHeaders()
         this.app.use((req, res, next) => {
-            const allowedOrigins = getAllowedIframeOrigins()
-            if (allowedOrigins === '*') {
-                // Explicitly allow all origins (only when user opts in)
-                res.setHeader('Content-Security-Policy', 'frame-ancestors *')
-            } else {
-                const csp = `frame-ancestors ${allowedOrigins}`
-                res.setHeader('Content-Security-Policy', csp)
-                // X-Frame-Options for legacy browser support
-                if (allowedOrigins === "'self'") {
-                    res.setHeader('X-Frame-Options', 'SAMEORIGIN')
-                } else {
-                    res.setHeader('X-Frame-Options', 'DENY')
-                }
+            for (const [headerName, headerValue] of Object.entries(iframeSecurityHeaders)) {
+                res.setHeader(headerName, headerValue)
             }
             next()
         })

packages/server/src/utils/XSS.test.ts

@@ -6,7 +6,7 @@ jest.mock('./domainValidation', () => ({
     validateChatflowDomain: jest.fn()
 }))
 
-import { getAllowedIframeOrigins } from './XSS'
+import { getAllowedIframeOrigins, getIframeSecurityHeaders } from './XSS'
 
 describe('getAllowedIframeOrigins', () => {
     const originalEnv = process.env.IFRAME_ORIGINS
@@ -101,17 +101,17 @@ describe('getAllowedIframeOrigins', () => {
 
         it('should handle trailing comma', () => {
             process.env.IFRAME_ORIGINS = 'https://example.com,'
-            expect(getAllowedIframeOrigins()).toBe('https://example.com ')
+            expect(getAllowedIframeOrigins()).toBe('https://example.com')
         })
 
         it('should handle leading comma', () => {
             process.env.IFRAME_ORIGINS = ',https://example.com'
-            expect(getAllowedIframeOrigins()).toBe(' https://example.com')
+            expect(getAllowedIframeOrigins()).toBe('https://example.com')
         })
 
         it('should handle multiple consecutive commas', () => {
             process.env.IFRAME_ORIGINS = 'https://domain1.com,,https://domain2.com'
-            expect(getAllowedIframeOrigins()).toBe('https://domain1.com  https://domain2.com')
+            expect(getAllowedIframeOrigins()).toBe('https://domain1.com https://domain2.com')
         })
     })
 
@@ -132,3 +132,45 @@ describe('getAllowedIframeOrigins', () => {
         })
     })
 })
+
+describe('getIframeSecurityHeaders', () => {
+    const originalEnv = process.env.IFRAME_ORIGINS
+
+    afterEach(() => {
+        if (originalEnv !== undefined) {
+            process.env.IFRAME_ORIGINS = originalEnv
+        } else {
+            delete process.env.IFRAME_ORIGINS
+        }
+    })
+
+    it('returns CSP only for wildcard allowlists', () => {
+        process.env.IFRAME_ORIGINS = '*'
+        expect(getIframeSecurityHeaders()).toEqual({
+            'Content-Security-Policy': 'frame-ancestors *'
+        })
+    })
+
+    it("returns SAMEORIGIN only for 'self'", () => {
+        process.env.IFRAME_ORIGINS = "'self'"
+        expect(getIframeSecurityHeaders()).toEqual({
+            'Content-Security-Policy': "frame-ancestors 'self'",
+            'X-Frame-Options': 'SAMEORIGIN'
+        })
+    })
+
+    it("returns DENY for 'none'", () => {
+        process.env.IFRAME_ORIGINS = "'none'"
+        expect(getIframeSecurityHeaders()).toEqual({
+            'Content-Security-Policy': "frame-ancestors 'none'",
+            'X-Frame-Options': 'DENY'
+        })
+    })
+
+    it('omits X-Frame-Options for custom allowlists', () => {
+        process.env.IFRAME_ORIGINS = 'https://embed.example.com,https://admin.example.com'
+        expect(getIframeSecurityHeaders()).toEqual({
+            'Content-Security-Policy': 'frame-ancestors https://embed.example.com https://admin.example.com'
+        })
+    })
+})

packages/server/src/utils/XSS.ts

@@ -113,5 +113,34 @@ export function getAllowedIframeOrigins(): string {
     return origins
         .split(',')
         .map((s) => s.trim())
+        .filter(Boolean)
         .join(' ')
 }
+
+export function getIframeSecurityHeaders(): Record<string, string> {
+    const allowedOrigins = getAllowedIframeOrigins()
+
+    if (allowedOrigins === '*') {
+        return {
+            'Content-Security-Policy': 'frame-ancestors *'
+        }
+    }
+
+    if (allowedOrigins === "'self'") {
+        return {
+            'Content-Security-Policy': `frame-ancestors ${allowedOrigins}`,
+            'X-Frame-Options': 'SAMEORIGIN'
+        }
+    }
+
+    if (allowedOrigins === "'none'") {
+        return {
+            'Content-Security-Policy': `frame-ancestors ${allowedOrigins}`,
+            'X-Frame-Options': 'DENY'
+        }
+    }
+
+    return {
+        'Content-Security-Policy': `frame-ancestors ${allowedOrigins}`
+    }
+}