From b5757cfeacdede8c4e9a9fbfd488f267b2a233c2 Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Sun, 26 Jul 2026 23:39:09 -0400 Subject: [PATCH] fix: address Codex review findings on the CSP defaults All five were real against LibreChat's actual runtime: - CSP_REPORT_ONLY now only enforces on an explicit false/off/0/no. A typo or `1` previously fell through isEnabled() to enforcing, turning a config slip into a blocked SPA. Shares the parse helper with headers.ts via a new security/env.ts. - Module preloads are stamped. A production client/dist/index.html carries 32 parser-inserted `` tags, which 'strict-dynamic' does not cover and 'self' cannot rescue. - Stale nonce attributes are replaced rather than preserved; only the current response's nonce is authorized. - worker-src allows data:, which Monaco's default CDN loader needs to bootstrap its workers (there is no loader.config() in the client). - script-src allows 'wasm-unsafe-eval' for the HEIC upload path, which compiles WebAssembly through heic-to. Narrower than 'unsafe-eval'. Verified against the real built shell: 4 scripts and all 32 preloads nonced, stylesheets/icons/manifest and ' + '' + + '' + + '' + + '' + '' + '
'; @@ -112,15 +118,31 @@ describe('Content Security Policy', () => { expect(response.text).toContain(`', '', - '', ].join(''); expect(applyCspNonce(html, 'abc123')).toBe( @@ -128,7 +150,31 @@ describe('applyCspNonce', () => { '', '', '', - '', + ].join(''), + ); + }); + + it('replaces a stale nonce rather than preserving it', () => { + const html = ''; + + expect(applyCspNonce(html, 'abc123')).toBe(''); + expect(applyCspNonce(html, 'abc123')).not.toContain('from-the-build'); + }); + + it('stamps module preloads, which strict-dynamic does not cover', () => { + const html = [ + '', + '', + '', + '', + ].join(''); + + expect(applyCspNonce(html, 'abc123')).toBe( + [ + '', + '', + '', + '', ].join(''), ); }); diff --git a/packages/api/src/security/csp.ts b/packages/api/src/security/csp.ts index a00ecd4a76..fc8f673059 100644 --- a/packages/api/src/security/csp.ts +++ b/packages/api/src/security/csp.ts @@ -1,14 +1,17 @@ import { randomBytes } from 'crypto'; import { logger } from '@librechat/data-schemas'; - +import { parseEnvSwitch } from './env'; import { isEnabled } from '../utils'; /** Split point for the per-request nonce. Randomized so no env value can collide. */ const NONCE_SLOT = `__csp_nonce_${randomBytes(8).toString('hex')}__`; const DIRECTIVE_NAME_PATTERN = /^[a-z][a-z0-9-]*$/; -const SCRIPT_TAG_PATTERN = /]*)>/gi; -const NONCE_ATTRIBUTE_PATTERN = /\snonce\s*=/i; +/** `` is in here because module preloads are fetched under `script-src`. */ +const NONCEABLE_TAG_PATTERN = /<(script|link)\b([^>]*)>/gi; +const NONCE_ATTRIBUTE_PATTERN = /\snonce\s*=\s*(?:"[^"]*"|'[^']*'|[^\s>]+)/gi; +const REL_PATTERN = /\srel\s*=\s*(?:"([^"]*)"|'([^']*)'|([^\s>]+))/i; +const AS_SCRIPT_PATTERN = /\sas\s*=\s*(?:"script"|'script'|script\b)/i; type CspDirective = [string, string[]]; @@ -48,12 +51,13 @@ function splitSourceList(value: string | undefined): string[] { .filter(Boolean); } +/** + * Only an explicitly recognized false value enforces. A typo or an unrecognized + * truthy spelling stays report-only, so a config slip cannot turn a rollout into + * a blocked SPA. + */ function isReportOnly(env: NodeJS.ProcessEnv): boolean { - const value = env.CSP_REPORT_ONLY; - if (value == null || value.trim() === '') { - return true; - } - return isEnabled(value); + return parseEnvSwitch('CSP_REPORT_ONLY', env.CSP_REPORT_ONLY, true); } /** @@ -62,13 +66,15 @@ function isReportOnly(env: NodeJS.ProcessEnv): boolean { * drop it and let the (now honored) `'self'` plus those hosts govern script loading. */ function scriptSources(scriptExtras: string[]): string[] { + /* 'wasm-unsafe-eval' permits WebAssembly compilation without permitting eval(); + * the HEIC upload path (client/src/utils/heicConverter.ts -> heic-to) needs it. */ if (scriptExtras.length === 0) { - return [`'nonce-${NONCE_SLOT}'`, "'strict-dynamic'", "'self'"]; + return [`'nonce-${NONCE_SLOT}'`, "'strict-dynamic'", "'wasm-unsafe-eval'", "'self'"]; } logger.info( "[CSP] CSP_SCRIPT_SRC_EXTRA is set; omitting 'strict-dynamic' so the configured script hosts take effect.", ); - return [`'nonce-${NONCE_SLOT}'`, "'self'"]; + return [`'nonce-${NONCE_SLOT}'`, "'wasm-unsafe-eval'", "'self'"]; } /** @@ -89,7 +95,10 @@ function defaultDirectives(scriptExtras: string[], frameAncestors: string[]): Cs ['connect-src', ["'self'", 'https:', 'wss:']], ['media-src', ["'self'", 'data:', 'blob:']], ['frame-src', ["'self'", 'https:', 'blob:', 'data:', 'about:']], - ['worker-src', ["'self'", 'blob:']], + /* `data:` is required by Monaco's default CDN loader, which bootstraps its + * workers from a data: URL; without it the artifact editor silently drops to + * running worker tasks on the UI thread. */ + ['worker-src', ["'self'", 'blob:', 'data:']], ['manifest-src', ["'self'"]], ['form-action', ["'self'", 'https:']], /* Replaced wholesale, not appended: merging would turn a deliberate @@ -216,19 +225,36 @@ export function issueCsp(policy: CspPolicy): CspResponse { }; } +/** `` elements that the browser fetches under `script-src`. */ +function isScriptPreload(attributes: string): boolean { + const match = REL_PATTERN.exec(attributes); + const rel = (match?.[1] ?? match?.[2] ?? match?.[3] ?? '').toLowerCase(); + if (rel === 'modulepreload') { + return true; + } + return rel === 'preload' && AS_SCRIPT_PATTERN.test(attributes); +} + /** - * Stamps the nonce onto every `