From d7a1123c9bb8094d3d3d49548a3e3ade6e150cfa Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Sun, 26 Jul 2026 23:37:35 -0400 Subject: [PATCH] fix: preserve legacy filters and generated files --- librechat.example.yaml | 6 +- package-lock.json | 30 ++++- packages/api/package.json | 3 +- packages/api/src/files/code/preflight.spec.ts | 51 ++++++++ packages/api/src/files/code/preflight.ts | 20 +-- .../api/src/protection/detectors/pattern.ts | 70 ++++++++++- packages/api/src/protection/legacy.spec.ts | 117 +++++++++++++++--- packages/api/src/protection/legacy.ts | 2 +- packages/data-provider/src/filters.spec.ts | 12 ++ 9 files changed, 283 insertions(+), 28 deletions(-) diff --git a/librechat.example.yaml b/librechat.example.yaml index 00f0aaa7c6..5b7ed60a0b 100644 --- a/librechat.example.yaml +++ b/librechat.example.yaml @@ -940,8 +940,10 @@ endpoints: # # enable all starters (sk_prefix, bearer_header, api_key_header). # starterPatterns: [sk_prefix, bearer_header, api_key_header] # # (optional) Operator-defined patterns. Each entry needs id, -# # label, and a regex. Legacy patterns are executed by the same bounded, -# # linear-time engine; unsupported native-only constructs are ignored. +# # label, and a JavaScript-flavor regex. Patterns are checked for unsafe +# # backtracking and fail closed when they cannot be safely evaluated; +# # selected inputs over 64 KiB are also blocked. +# # Prefer `filters.messages` for linear-time patterns and source-aware fields. # customPatterns: # - id: anthropic_api_key # label: Anthropic API key diff --git a/package-lock.json b/package-lock.json index 3f9daf2dd4..7da4284c5c 100644 --- a/package-lock.json +++ b/package-lock.json @@ -38066,6 +38066,33 @@ "node": ">=4" } }, + "node_modules/redos-detector": { + "version": "6.1.4", + "resolved": "https://registry.npmjs.org/redos-detector/-/redos-detector-6.1.4.tgz", + "integrity": "sha512-lPlka1rEH6kK42gtgokvvxMmpAvyc28DcRjQbGxeP5RJsJWgAduBOdGedVCDPdSyCr4ay/JDxq3nGYsJZyt8tA==", + "license": "MIT", + "dependencies": { + "regjsparser": "0.13.0" + }, + "bin": { + "redos-detector": "bin.js" + }, + "engines": { + "node": ">=16.0.0" + } + }, + "node_modules/redos-detector/node_modules/regjsparser": { + "version": "0.13.0", + "resolved": "https://registry.npmjs.org/regjsparser/-/regjsparser-0.13.0.tgz", + "integrity": "sha512-NZQZdC5wOE/H3UT28fVGL+ikOZcEzfMGk/c3iN9UGxzWHMa1op7274oyiUVrAG4B2EuFhus8SvkaYnhvW92p9Q==", + "license": "BSD-2-Clause", + "dependencies": { + "jsesc": "~3.1.0" + }, + "bin": { + "regjsparser": "bin/parser" + } + }, "node_modules/redux": { "version": "4.2.1", "resolved": "https://registry.npmjs.org/redux/-/redux-4.2.1.tgz", @@ -43757,7 +43784,8 @@ "dependencies": { "@langchain/langgraph-checkpoint": "^1.1.2", "@langchain/langgraph-checkpoint-mongodb": "^1.4.0", - "re2js": "^2.8.6" + "re2js": "^2.8.6", + "redos-detector": "^6.1.4" }, "devDependencies": { "@babel/preset-env": "^7.29.5", diff --git a/packages/api/package.json b/packages/api/package.json index e1e9d9bbd5..c61a5badeb 100644 --- a/packages/api/package.json +++ b/packages/api/package.json @@ -170,6 +170,7 @@ "dependencies": { "@langchain/langgraph-checkpoint": "^1.1.2", "@langchain/langgraph-checkpoint-mongodb": "^1.4.0", - "re2js": "^2.8.6" + "re2js": "^2.8.6", + "redos-detector": "^6.1.4" } } diff --git a/packages/api/src/files/code/preflight.spec.ts b/packages/api/src/files/code/preflight.spec.ts index bab11143bd..4741627e50 100644 --- a/packages/api/src/files/code/preflight.spec.ts +++ b/packages/api/src/files/code/preflight.spec.ts @@ -101,6 +101,22 @@ describe('preflightCodeOutputBatch', () => { expect(result.every((entry) => entry.downloadFallback === true)).toBe(true); }); + it('preserves configured files above the inspection count as URL fallbacks', async () => { + const prepare = jest.fn(async (input: PrepareCodeOutputInput) => prepared(input)); + + const result = await preflightCodeOutputBatch({ + artifact: artifact(12), + limits: { ...limits, fileLimit: 12 }, + prepare, + }); + + expect(prepare).not.toHaveBeenCalled(); + expect(result.map((entry) => entry.file.id)).toEqual( + Array.from({ length: 12 }, (_, index) => `file-${index}`), + ); + expect(result.every((entry) => entry.downloadFallback === true)).toBe(true); + }); + it('stops default-off downloads at the aggregate budget and falls back without retrying', async () => { const prepare = jest.fn(async (input: PrepareCodeOutputInput) => prepared(input, { buffer: Buffer.alloc(4) }), @@ -160,6 +176,41 @@ describe('preflightCodeOutputBatch', () => { expect(prepare).not.toHaveBeenCalled(); }); + it('rejects active inspection above the hard count even when the configured limit allows it', async () => { + const filters: FiltersConfig = { + files: { + pii: { + fields: ['content'], + starterPatterns: [], + customPatterns: [BLOCK_PATTERN], + uninspectable: 'allow', + }, + }, + }; + const prepare = jest.fn(async (input: PrepareCodeOutputInput) => prepared(input)); + const inputArtifact = artifact(12); + const overflowFile = inputArtifact.files?.[10]; + if (overflowFile == null) { + throw new Error('Expected an overflow file'); + } + const readOverflowContent = jest.fn(() => 'safe'); + Object.defineProperty(overflowFile, 'content', { get: readOverflowContent }); + + await expect( + preflightCodeOutputBatch({ + filters, + artifact: inputArtifact, + limits: { ...limits, fileLimit: 12 }, + prepare, + }), + ).rejects.toMatchObject({ + code: 'content_filter_uninspectable', + body: { source: 'file', field: 'content' }, + }); + expect(readOverflowContent).not.toHaveBeenCalled(); + expect(prepare).not.toHaveBeenCalled(); + }); + it('rejects a late blocked file only after preparing the complete no-write batch', async () => { const filters: FiltersConfig = { files: { diff --git a/packages/api/src/files/code/preflight.ts b/packages/api/src/files/code/preflight.ts index 0625f683f9..ca4407e826 100644 --- a/packages/api/src/files/code/preflight.ts +++ b/packages/api/src/files/code/preflight.ts @@ -93,11 +93,11 @@ function normalizedPreparedMimeType(prepared: PreparedCodeOutput): string { return prepared.file.type.split(';', 1)[0].trim().toLowerCase(); } -function boundedCountLimit(configured: number, hardLimit: number): number { +function getConfiguredCountLimit(configured: number, fallback: number): number { if (!Number.isFinite(configured) || configured < 0) { - return hardLimit; + return fallback; } - return Math.min(configured, hardLimit); + return configured; } /** @@ -129,29 +129,33 @@ export async function preflightCodeOutputBatch( selectedFields.length > 0 && (hasActivePiiPatterns(pii) || pii?.uninspectable === 'block'); const resourceField = selectedFields[0] ?? 'content'; - const maxCount = boundedCountLimit( + const configuredMaxCount = getConfiguredCountLimit( Math.floor(input.limits.fileLimit), CODE_OUTPUT_PREFLIGHT_MAX_COUNT, ); const maxBytes = getBoundedCodeOutputByteLimit(input.limits.totalSizeLimit); const fileSizeLimit = getBoundedCodeOutputByteLimit(input.limits.fileSizeLimit, maxBytes); const entries: PreparedCodeOutputEntry[] = []; - let countExceeded = false; + let configuredCountExceeded = false; for (const file of input.artifact?.files ?? []) { if (file.inherited === true) { continue; } - if (entries.length >= maxCount) { - countExceeded = true; + if (entries.length >= configuredMaxCount) { + configuredCountExceeded = true; break; } + if (inspectionActive && entries.length >= CODE_OUTPUT_PREFLIGHT_MAX_COUNT) { + throw new UninspectableFileError(resourceField); + } throwIfContentBlocked(input.filters, extractFileContent(file)); entries.push({ file, sessionId: file.storage_session_id ?? input.artifact?.session_id, }); } - if (countExceeded) { + const inspectionCountExceeded = entries.length > CODE_OUTPUT_PREFLIGHT_MAX_COUNT; + if (configuredCountExceeded || inspectionCountExceeded) { if (inspectionActive) { throw new UninspectableFileError(resourceField); } diff --git a/packages/api/src/protection/detectors/pattern.ts b/packages/api/src/protection/detectors/pattern.ts index 3116e1dc7f..71ecca160a 100644 --- a/packages/api/src/protection/detectors/pattern.ts +++ b/packages/api/src/protection/detectors/pattern.ts @@ -1,4 +1,5 @@ import { RE2JS } from 're2js'; +import { isSafePattern } from 'redos-detector'; import { logger } from '@librechat/data-schemas'; import type { MessageFilterPiiConfig, FilterPiiCustomPatternConfig } from 'librechat-data-provider'; import type { ProtectionFinding, TextContentFragment } from '../types'; @@ -36,6 +37,71 @@ const STARTER_PATTERNS: readonly CompiledPattern[] = [ const STARTER_BY_ID = new Map(STARTER_PATTERNS.map((pattern) => [pattern.id, pattern])); const NATIVE_INSPECTOR_CACHE = new WeakMap(); const LINEAR_INSPECTOR_CACHE = new WeakMap(); +const LEGACY_PATTERN_MAX_INPUT_BYTES = 64 * 1024; +const LEGACY_PATTERN_MAX_PATTERN_LENGTH = 512; +const LEGACY_PATTERN_MAX_SCORE = 200; +const LEGACY_PATTERN_ANALYSIS_TIMEOUT_MS = 100; + +/** + * Legacy custom patterns retain JavaScript semantics after bounded static + * analysis. Unsafe, unknown, or oversized cases block instead of running an + * unbounded backtracking match against request content. + */ +class ValidatedNativePattern implements TestablePattern { + private readonly pattern: RegExp; + private warnedOversized = false; + + constructor( + regex: string, + private readonly id: string, + ) { + this.pattern = new RegExp(regex); + } + + test(input: string): boolean { + if (Buffer.byteLength(input, 'utf8') > LEGACY_PATTERN_MAX_INPUT_BYTES) { + if (!this.warnedOversized) { + this.warnedOversized = true; + logger.warn( + `[messageFilter.pii] blocking oversized input for customPattern ${JSON.stringify(this.id)}`, + ); + } + return true; + } + this.pattern.lastIndex = 0; + return this.pattern.test(input); + } +} + +function createValidatedNativePattern(regex: string, id: string): TestablePattern { + if (regex.length > LEGACY_PATTERN_MAX_PATTERN_LENGTH) { + logger.warn( + `[messageFilter.pii] failing closed because customPattern ${JSON.stringify(id)} is too long`, + ); + return { test: () => true }; + } + const pattern = new ValidatedNativePattern(regex, id); + let analysis: ReturnType; + try { + analysis = isSafePattern(regex, { + maxScore: LEGACY_PATTERN_MAX_SCORE, + maxSteps: 20_000, + timeout: LEGACY_PATTERN_ANALYSIS_TIMEOUT_MS, + }); + } catch { + logger.warn( + `[messageFilter.pii] failing closed because customPattern ${JSON.stringify(id)} could not be analyzed`, + ); + return { test: () => true }; + } + if (analysis.safe) { + return pattern; + } + logger.warn( + `[messageFilter.pii] failing closed for unsafe customPattern ${JSON.stringify(id)} (${analysis.error ?? 'unsafe'})`, + ); + return { test: () => true }; +} function selectStarter(ids?: readonly string[]): readonly CompiledPattern[] { if (ids == null) { @@ -100,7 +166,9 @@ export function createPatternContentInspector( id: pattern.id, label: pattern.label, pattern: - options.linearTime === true ? RE2JS.compile(pattern.regex) : new RegExp(pattern.regex), + options.linearTime === true + ? RE2JS.compile(pattern.regex) + : createValidatedNativePattern(pattern.regex, pattern.id), }); } catch (error) { logger.warn( diff --git a/packages/api/src/protection/legacy.spec.ts b/packages/api/src/protection/legacy.spec.ts index 663a7acc5e..2376a3fc87 100644 --- a/packages/api/src/protection/legacy.spec.ts +++ b/packages/api/src/protection/legacy.spec.ts @@ -139,28 +139,117 @@ describe('legacy content protection', () => { ); }); - it('runs legacy custom patterns through the linear-time engine', () => { + it('preserves JavaScript regex compatibility for legacy custom patterns', () => { jest.mocked(logger.warn).mockClear(); - const nestedQuantifier = { + const config = { starterPatterns: [], - customPatterns: [{ id: 'nested', label: 'Nested', regex: '(a+)+$' }], + customPatterns: [ + { id: 'lookahead', label: 'Lookahead', regex: '(?=PRIVATE)PRIVATE' }, + { id: 'backreference', label: 'Backreference', regex: '([A-Z]{3})-\\1' }, + ], } as MessageFilterPiiConfig; - const nativeOnly = { + + expect( + inspectLegacyPii([fragment('external-message.0.content', 'PRIVATE')], config)?.ruleId, + ).toBe('lookahead'); + expect( + inspectLegacyPii([fragment('external-message.0.content', 'ABC-ABC')], config)?.ruleId, + ).toBe('backreference'); + expect( + inspectLegacyPii([fragment('external-message.0.content', 'SECRET\u00a0KEY')], { + starterPatterns: [], + customPatterns: [{ id: 'whitespace', label: 'Whitespace', regex: 'SECRET\\sKEY' }], + })?.ruleId, + ).toBe('whitespace'); + expect(logger.warn).not.toHaveBeenCalled(); + }); + + it('fails closed without executing an unsafe legacy native pattern', () => { + jest.mocked(logger.warn).mockClear(); + const config = { + starterPatterns: [], + customPatterns: [ + { + id: 'unsafe-native', + label: 'Unsafe native', + regex: '(a+)+$', + }, + ], + } as MessageFilterPiiConfig; + + const finding = inspectLegacyPii([fragment('external-message.0.content', 'safe')], config); + + expect(finding?.ruleId).toBe('unsafe-native'); + expect(logger.warn).toHaveBeenCalledWith( + expect.stringContaining( + '[messageFilter.pii] failing closed for unsafe customPattern "unsafe-native"', + ), + ); + }); + + it('fails closed when a valid legacy native pattern cannot be analyzed', () => { + jest.mocked(logger.warn).mockClear(); + const config = { + starterPatterns: [], + customPatterns: [ + { + id: 'named-backreference', + label: 'Named backreference', + regex: '(?[A-Z]+)-\\k', + }, + ], + } as MessageFilterPiiConfig; + + const finding = inspectLegacyPii([fragment('external-message.0.content', 'safe')], config); + + expect(finding?.ruleId).toBe('named-backreference'); + expect(logger.warn).toHaveBeenCalledWith( + expect.stringContaining( + '[messageFilter.pii] failing closed because customPattern "named-backreference" could not be analyzed', + ), + ); + }); + + it('fails closed before compiling an oversized legacy native pattern', () => { + jest.mocked(logger.warn).mockClear(); + const config = { + starterPatterns: [], + customPatterns: [ + { + id: 'oversized-pattern', + label: 'Oversized pattern', + regex: 'A'.repeat(513), + }, + ], + } as MessageFilterPiiConfig; + + const finding = inspectLegacyPii([fragment('external-message.0.content', 'safe')], config); + + expect(finding?.ruleId).toBe('oversized-pattern'); + expect(logger.warn).toHaveBeenCalledWith( + expect.stringContaining( + '[messageFilter.pii] failing closed because customPattern "oversized-pattern" is too long', + ), + ); + }); + + it('fails closed on oversized input before running a legacy native pattern', () => { + jest.mocked(logger.warn).mockClear(); + const config = { starterPatterns: [], customPatterns: [{ id: 'lookahead', label: 'Lookahead', regex: '(?=PRIVATE)PRIVATE' }], } as MessageFilterPiiConfig; - expect( - inspectLegacyPii( - [fragment('external-message.0.content', `${'a'.repeat(50_000)}!`)], - nestedQuantifier, - ), - ).toBeNull(); - expect( - inspectLegacyPii([fragment('external-message.0.content', 'PRIVATE')], nativeOnly), - ).toBeNull(); + const finding = inspectLegacyPii( + [fragment('external-message.0.content', 'a'.repeat(64 * 1024 + 1))], + config, + ); + + expect(finding?.ruleId).toBe('lookahead'); expect(logger.warn).toHaveBeenCalledWith( - expect.stringContaining('[messageFilter.pii] dropping invalid customPattern "lookahead":'), + expect.stringContaining( + '[messageFilter.pii] blocking oversized input for customPattern "lookahead"', + ), ); }); }); diff --git a/packages/api/src/protection/legacy.ts b/packages/api/src/protection/legacy.ts index b00455bf8a..bf7e139e43 100644 --- a/packages/api/src/protection/legacy.ts +++ b/packages/api/src/protection/legacy.ts @@ -46,7 +46,7 @@ export function createLegacyPiiInspector( if (cached != null) { return cached; } - const patternInspector = createPatternContentInspector(config, { linearTime: true }); + const patternInspector = createPatternContentInspector(config); if (!patternInspector.active) { INACTIVE_LEGACY_CONFIGS.add(config); return null; diff --git a/packages/data-provider/src/filters.spec.ts b/packages/data-provider/src/filters.spec.ts index 228cb4e233..8075df947e 100644 --- a/packages/data-provider/src/filters.spec.ts +++ b/packages/data-provider/src/filters.spec.ts @@ -243,6 +243,18 @@ describe('filtersConfigSchema', () => { customPatterns: [{ id: 'legacy-broken', label: 'Broken', regex: '(' }], }).success, ).toBe(false); + expect( + messageFilterPiiSchema.safeParse({ + customPatterns: [{ id: 'legacy-lookahead', label: 'Lookahead', regex: '(?=LEGACY)LEGACY' }], + }).success, + ).toBe(true); + expect( + messageFilterPiiSchema.safeParse({ + customPatterns: [ + { id: 'legacy-backreference', label: 'Backreference', regex: '(LEGACY)-\\1' }, + ], + }).success, + ).toBe(true); expect( messageFilterPiiSchema.parse({ customPatterns: [{ id: 'legacy', label: 'Legacy', regex: 'LEGACY-[0-9]+', ignored: true }],