mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-08-04 14:57:42 +00:00
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 `<link rel="modulepreload">` 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 <style> untouched.
This commit is contained in:
Danny Avila
parent
4d5f577e83
commit
b5757cfeac
7 changed files with 174 additions and 51 deletions
|
|
@ -108,10 +108,18 @@ TRUST_PROXY=1
|
|||
# Nonce-based CSP for the SPA HTML response. Off by default so existing
|
||||
# deployments are unaffected. Turn it on in report-only mode first, review the
|
||||
# violations your deployment actually produces, then set CSP_REPORT_ONLY=false.
|
||||
# Only an explicit false/off/0/no enforces; anything unrecognized warns and stays
|
||||
# report-only, so a typo cannot silently start blocking scripts.
|
||||
# CSP_ENABLED=false
|
||||
# CSP_REPORT_ONLY=true
|
||||
# CSP_REPORT_URI=
|
||||
|
||||
# The default policy accommodates what LibreChat actually loads at runtime:
|
||||
# script-src 'wasm-unsafe-eval' HEIC image conversion compiles WebAssembly
|
||||
# worker-src data: Monaco's loader bootstraps workers from data:
|
||||
# Both are narrower than 'unsafe-eval'. Drop them via CSP_ADDITIONAL_DIRECTIVES
|
||||
# only if your deployment uses neither the artifact code editor nor HEIC uploads.
|
||||
|
||||
# Add deployment-specific sources on top of LibreChat's defaults; they are
|
||||
# appended, never replacing them. Comma- or space-separated. Quote values
|
||||
# containing spaces.
|
||||
|
|
|
|||
|
|
@ -4,11 +4,17 @@ const request = require('supertest');
|
|||
const { MongoMemoryServer } = require('mongodb-memory-server');
|
||||
const mongoose = require('mongoose');
|
||||
|
||||
/** Mirrors the SPA shell: an inline style, an inline script, and a bundled script. */
|
||||
/**
|
||||
* Mirrors what a production `client/dist/index.html` actually contains: inline
|
||||
* style, inline script, a module entry, and the module preloads Vite emits.
|
||||
*/
|
||||
const INDEX_HTML =
|
||||
'<!DOCTYPE html><html lang="en-US"><head><title>LibreChat</title>' +
|
||||
'<style>body{margin:0}</style>' +
|
||||
'<script>window.theme="dark";</script>' +
|
||||
'<link rel="modulepreload" crossorigin href="./assets/chunk.js">' +
|
||||
'<link rel="stylesheet" crossorigin href="./assets/app.css">' +
|
||||
'<script type="module" crossorigin src="./assets/index.js"></script>' +
|
||||
'<script defer src="/assets/app.js"></script>' +
|
||||
'</head><body><div id="root"></div></body></html>';
|
||||
|
||||
|
|
@ -112,15 +118,31 @@ describe('Content Security Policy', () => {
|
|||
expect(response.text).toContain(`<script nonce="${nonce}" defer src="/assets/app.js">`);
|
||||
});
|
||||
|
||||
it('leaves style tags unstamped so unsafe-inline keeps working', async () => {
|
||||
it('leaves style tags and stylesheet links unstamped', async () => {
|
||||
const response = await request(app).get('/');
|
||||
|
||||
expect(response.text).toContain('<style>body{margin:0}</style>');
|
||||
expect(response.text).toContain('<link rel="stylesheet" crossorigin href="./assets/app.css">');
|
||||
expect(response.headers['content-security-policy']).toContain(
|
||||
"style-src 'self' 'unsafe-inline'",
|
||||
);
|
||||
});
|
||||
|
||||
it("stamps module preloads, which 'strict-dynamic' does not cover", async () => {
|
||||
const response = await request(app).get('/');
|
||||
const nonce = response.headers['content-security-policy']?.match(
|
||||
/script-src 'nonce-([^']+)'/,
|
||||
)?.[1];
|
||||
|
||||
expect(nonce).toBeTruthy();
|
||||
expect(response.text).toContain(
|
||||
`<link nonce="${nonce}" rel="modulepreload" crossorigin href="./assets/chunk.js">`,
|
||||
);
|
||||
expect(response.text).toContain(
|
||||
`<script nonce="${nonce}" type="module" crossorigin src="./assets/index.js">`,
|
||||
);
|
||||
});
|
||||
|
||||
it('stamps scripts injected after the shell is read', async () => {
|
||||
const response = await request(app).get('/').set('x-librechat-enable-query-devtools', '1');
|
||||
const nonce = response.headers['content-security-policy']?.match(
|
||||
|
|
|
|||
|
|
@ -29,6 +29,20 @@ describe('createCspPolicy', () => {
|
|||
);
|
||||
});
|
||||
|
||||
it('only enforces on a recognized false value, never on a typo', () => {
|
||||
for (const value of ['1', 'yes', 'on', 'treu', 'report-only', 'maybe']) {
|
||||
expect(createCspPolicy({ CSP_ENABLED: 'true', CSP_REPORT_ONLY: value })?.headerName).toBe(
|
||||
'Content-Security-Policy-Report-Only',
|
||||
);
|
||||
}
|
||||
|
||||
for (const value of ['false', 'off', '0', 'no']) {
|
||||
expect(createCspPolicy({ CSP_ENABLED: 'true', CSP_REPORT_ONLY: value })?.headerName).toBe(
|
||||
'Content-Security-Policy',
|
||||
);
|
||||
}
|
||||
});
|
||||
|
||||
it('mints a fresh nonce per response', () => {
|
||||
const policy = createCspPolicy({ CSP_ENABLED: 'true' });
|
||||
if (!policy) {
|
||||
|
|
@ -56,6 +70,13 @@ describe('policy directives', () => {
|
|||
expect(header).toContain("frame-ancestors 'self'");
|
||||
});
|
||||
|
||||
it('permits the runtime dependencies the app actually ships', () => {
|
||||
const header = headerFor({});
|
||||
|
||||
expect(header).toContain("'wasm-unsafe-eval'");
|
||||
expect(header).toContain("worker-src 'self' blob: data:");
|
||||
});
|
||||
|
||||
it('keeps styles on unsafe-inline with no nonce', () => {
|
||||
const header = headerFor({});
|
||||
const styleSrc = header.split('; ').find((directive) => directive.startsWith('style-src'));
|
||||
|
|
@ -69,7 +90,9 @@ describe('policy directives', () => {
|
|||
|
||||
expect(header).not.toContain("'strict-dynamic'");
|
||||
expect(header).toContain('https://scripts.example.com');
|
||||
expect(header).toMatch(/script-src 'nonce-[^']+' 'self' https:\/\/scripts\.example\.com/);
|
||||
expect(header).toMatch(
|
||||
/script-src 'nonce-[^']+' 'wasm-unsafe-eval' 'self' https:\/\/scripts\.example\.com/,
|
||||
);
|
||||
});
|
||||
|
||||
it('adds deployment-specific sources without dropping the safe defaults', () => {
|
||||
|
|
@ -115,12 +138,11 @@ describe('policy directives', () => {
|
|||
});
|
||||
|
||||
describe('applyCspNonce', () => {
|
||||
it('stamps script tags, preserves existing nonces, and leaves styles alone', () => {
|
||||
it('stamps script tags and leaves styles alone', () => {
|
||||
const html = [
|
||||
'<style>body { margin: 0; }</style>',
|
||||
'<script>window.theme = "dark";</script>',
|
||||
'<script defer src="/assets/app.js"></script>',
|
||||
'<script nonce="existing">window.ok = true;</script>',
|
||||
].join('');
|
||||
|
||||
expect(applyCspNonce(html, 'abc123')).toBe(
|
||||
|
|
@ -128,7 +150,31 @@ describe('applyCspNonce', () => {
|
|||
'<style>body { margin: 0; }</style>',
|
||||
'<script nonce="abc123">window.theme = "dark";</script>',
|
||||
'<script nonce="abc123" defer src="/assets/app.js"></script>',
|
||||
'<script nonce="existing">window.ok = true;</script>',
|
||||
].join(''),
|
||||
);
|
||||
});
|
||||
|
||||
it('replaces a stale nonce rather than preserving it', () => {
|
||||
const html = '<script nonce="from-the-build">window.ok = true;</script>';
|
||||
|
||||
expect(applyCspNonce(html, 'abc123')).toBe('<script nonce="abc123">window.ok = true;</script>');
|
||||
expect(applyCspNonce(html, 'abc123')).not.toContain('from-the-build');
|
||||
});
|
||||
|
||||
it('stamps module preloads, which strict-dynamic does not cover', () => {
|
||||
const html = [
|
||||
'<link rel="modulepreload" crossorigin href="/assets/chunk.js">',
|
||||
'<link rel="preload" as="script" href="/assets/other.js">',
|
||||
'<link rel="stylesheet" crossorigin href="/assets/app.css">',
|
||||
'<link rel="icon" type="image/png" href="/favicon.png">',
|
||||
].join('');
|
||||
|
||||
expect(applyCspNonce(html, 'abc123')).toBe(
|
||||
[
|
||||
'<link nonce="abc123" rel="modulepreload" crossorigin href="/assets/chunk.js">',
|
||||
'<link nonce="abc123" rel="preload" as="script" href="/assets/other.js">',
|
||||
'<link rel="stylesheet" crossorigin href="/assets/app.css">',
|
||||
'<link rel="icon" type="image/png" href="/favicon.png">',
|
||||
].join(''),
|
||||
);
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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 = /<script\b([^>]*)>/gi;
|
||||
const NONCE_ATTRIBUTE_PATTERN = /\snonce\s*=/i;
|
||||
/** `<link>` 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 {
|
|||
};
|
||||
}
|
||||
|
||||
/** `<link>` 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 `<script>` element in the SPA shell. Styles are left
|
||||
* alone; see `defaultDirectives` for why they stay on `'unsafe-inline'`.
|
||||
* Stamps the response nonce onto the shell's `<script>` elements and onto the
|
||||
* module preloads a production build emits, which are parser-inserted and so are
|
||||
* not covered by `'strict-dynamic'`. Any nonce already in the markup is replaced,
|
||||
* never kept: only the current response's nonce is authorized by the header.
|
||||
*
|
||||
* Styles are deliberately untouched; see `defaultDirectives`.
|
||||
*/
|
||||
export function applyCspNonce(html: string, nonce: string): string {
|
||||
if (!nonce) {
|
||||
return html;
|
||||
}
|
||||
|
||||
return html.replace(SCRIPT_TAG_PATTERN, (match, attributes: string) => {
|
||||
if (NONCE_ATTRIBUTE_PATTERN.test(attributes)) {
|
||||
return html.replace(NONCEABLE_TAG_PATTERN, (match, tag: string, attributes: string) => {
|
||||
const isScript = tag.toLowerCase() === 'script';
|
||||
if (!isScript && !isScriptPreload(attributes)) {
|
||||
return match;
|
||||
}
|
||||
return `<script nonce="${nonce}"${attributes}>`;
|
||||
|
||||
const stripped = attributes.replace(NONCE_ATTRIBUTE_PATTERN, '');
|
||||
return `<${tag} nonce="${nonce}"${stripped}>`;
|
||||
});
|
||||
}
|
||||
|
|
|
|||
36
packages/api/src/security/env.ts
Normal file
36
packages/api/src/security/env.ts
Normal file
|
|
@ -0,0 +1,36 @@
|
|||
import { logger } from '@librechat/data-schemas';
|
||||
|
||||
const TRUTHY = new Set(['true', '1', 'yes', 'on', 'enabled']);
|
||||
const FALSY = new Set(['false', '0', 'no', 'off', 'disabled', 'none']);
|
||||
|
||||
export function normalizeEnvValue(value: string | undefined): string {
|
||||
return value == null ? '' : value.trim().toLowerCase();
|
||||
}
|
||||
|
||||
export function isFalsyEnvValue(value: string): boolean {
|
||||
return FALSY.has(value);
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolves a security toggle. Unrecognized values warn and fall back rather than
|
||||
* being treated as the opposite of the default, so a typo cannot silently flip a
|
||||
* deployment into the riskier setting.
|
||||
*/
|
||||
export function parseEnvSwitch(
|
||||
name: string,
|
||||
value: string | undefined,
|
||||
fallback: boolean,
|
||||
): boolean {
|
||||
const normalized = normalizeEnvValue(value);
|
||||
if (normalized === '') {
|
||||
return fallback;
|
||||
}
|
||||
if (TRUTHY.has(normalized)) {
|
||||
return true;
|
||||
}
|
||||
if (FALSY.has(normalized)) {
|
||||
return false;
|
||||
}
|
||||
logger.warn(`[SecurityHeaders] Ignoring invalid ${name}="${value}"; using ${fallback}.`);
|
||||
return fallback;
|
||||
}
|
||||
|
|
@ -3,6 +3,8 @@ import { logger } from '@librechat/data-schemas';
|
|||
|
||||
import type { RequestHandler } from 'express';
|
||||
|
||||
import { parseEnvSwitch, normalizeEnvValue, isFalsyEnvValue } from './env';
|
||||
|
||||
const DEFAULT_HSTS_MAX_AGE = 31536000;
|
||||
|
||||
export type FrameOptionsAction = 'deny' | 'sameorigin';
|
||||
|
|
@ -37,9 +39,6 @@ export interface SecurityHeaderOptions {
|
|||
referrerPolicy: { policy: ReferrerPolicyToken } | false;
|
||||
}
|
||||
|
||||
const TRUTHY = new Set(['true', '1', 'yes', 'on', 'enabled']);
|
||||
const FALSY = new Set(['false', '0', 'no', 'off', 'disabled', 'none']);
|
||||
|
||||
const FRAME_ACTIONS = new Set<FrameOptionsAction>(['deny', 'sameorigin']);
|
||||
const OPENER_POLICIES = new Set<OpenerPolicy>([
|
||||
'same-origin',
|
||||
|
|
@ -59,27 +58,8 @@ const REFERRER_TOKENS = new Set<ReferrerPolicyToken>([
|
|||
'unsafe-url',
|
||||
]);
|
||||
|
||||
function normalize(value: string | undefined): string {
|
||||
return value == null ? '' : value.trim().toLowerCase();
|
||||
}
|
||||
|
||||
function parseSwitch(name: string, value: string | undefined, fallback: boolean): boolean {
|
||||
const normalized = normalize(value);
|
||||
if (normalized === '') {
|
||||
return fallback;
|
||||
}
|
||||
if (TRUTHY.has(normalized)) {
|
||||
return true;
|
||||
}
|
||||
if (FALSY.has(normalized)) {
|
||||
return false;
|
||||
}
|
||||
logger.warn(`[SecurityHeaders] Ignoring invalid ${name}="${value}"; using ${fallback}.`);
|
||||
return fallback;
|
||||
}
|
||||
|
||||
function parseMaxAge(value: string | undefined, fallback: number): number {
|
||||
const normalized = normalize(value);
|
||||
const normalized = normalizeEnvValue(value);
|
||||
if (normalized === '') {
|
||||
return fallback;
|
||||
}
|
||||
|
|
@ -101,11 +81,11 @@ function parsePolicy<T extends string>(
|
|||
allowed: ReadonlySet<T>,
|
||||
fallback: T,
|
||||
): T | false {
|
||||
const normalized = normalize(value);
|
||||
const normalized = normalizeEnvValue(value);
|
||||
if (normalized === '') {
|
||||
return fallback;
|
||||
}
|
||||
if (FALSY.has(normalized)) {
|
||||
if (isFalsyEnvValue(normalized)) {
|
||||
return false;
|
||||
}
|
||||
if (allowed.has(normalized as T)) {
|
||||
|
|
@ -116,7 +96,7 @@ function parsePolicy<T extends string>(
|
|||
}
|
||||
|
||||
function buildHsts(env: NodeJS.ProcessEnv): HstsOptions | false {
|
||||
if (!parseSwitch('HSTS_ENABLED', env.HSTS_ENABLED, true)) {
|
||||
if (!parseEnvSwitch('HSTS_ENABLED', env.HSTS_ENABLED, true)) {
|
||||
return false;
|
||||
}
|
||||
return {
|
||||
|
|
@ -125,8 +105,12 @@ function buildHsts(env: NodeJS.ProcessEnv): HstsOptions | false {
|
|||
* deployment would otherwise pin every sibling subdomain to HTTPS for a year in
|
||||
* every visitor's browser, and reversing that means serving `max-age=0` from each
|
||||
* affected host. */
|
||||
includeSubDomains: parseSwitch('HSTS_INCLUDE_SUBDOMAINS', env.HSTS_INCLUDE_SUBDOMAINS, false),
|
||||
preload: parseSwitch('HSTS_PRELOAD', env.HSTS_PRELOAD, false),
|
||||
includeSubDomains: parseEnvSwitch(
|
||||
'HSTS_INCLUDE_SUBDOMAINS',
|
||||
env.HSTS_INCLUDE_SUBDOMAINS,
|
||||
false,
|
||||
),
|
||||
preload: parseEnvSwitch('HSTS_PRELOAD', env.HSTS_PRELOAD, false),
|
||||
};
|
||||
}
|
||||
|
||||
|
|
@ -138,7 +122,7 @@ function buildHsts(env: NodeJS.ProcessEnv): HstsOptions | false {
|
|||
export function buildSecurityHeaderOptions(
|
||||
env: NodeJS.ProcessEnv = process.env,
|
||||
): SecurityHeaderOptions | null {
|
||||
if (!parseSwitch('SECURITY_HEADERS', env.SECURITY_HEADERS, true)) {
|
||||
if (!parseEnvSwitch('SECURITY_HEADERS', env.SECURITY_HEADERS, true)) {
|
||||
return null;
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -1,2 +1,3 @@
|
|||
export * from './env';
|
||||
export * from './headers';
|
||||
export * from './csp';
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue