diff --git a/packages/api/src/utils/env.spec.ts b/packages/api/src/utils/env.spec.ts index c241cb2b51..e1244fa605 100644 --- a/packages/api/src/utils/env.spec.ts +++ b/packages/api/src/utils/env.spec.ts @@ -111,12 +111,12 @@ describe('encodeHeaderValue', () => { describe('resolveHeaders', () => { beforeEach(() => { process.env.TEST_API_KEY = 'test-api-key-value'; - process.env.ANOTHER_SECRET = 'another-secret-value'; + process.env.ANOTHER_VALUE = 'another-test-value'; }); afterEach(() => { delete process.env.TEST_API_KEY; - delete process.env.ANOTHER_SECRET; + delete process.env.ANOTHER_VALUE; }); it('should return empty object when headers is undefined', () => { @@ -139,7 +139,7 @@ describe('resolveHeaders', () => { it('should process environment variables in headers', () => { const headers = { Authorization: '${TEST_API_KEY}', - 'X-Secret': '${ANOTHER_SECRET}', + 'X-Secret': '${ANOTHER_VALUE}', 'Content-Type': 'application/json', }; @@ -147,7 +147,7 @@ describe('resolveHeaders', () => { expect(result).toEqual({ Authorization: 'test-api-key-value', - 'X-Secret': 'another-secret-value', + 'X-Secret': 'another-test-value', 'Content-Type': 'application/json', }); }); @@ -526,6 +526,40 @@ describe('resolveHeaders', () => { expect(result['X-Conversation']).toBe('conv-123'); }); + it('should not resolve env vars introduced via LIBRECHAT_BODY placeholders', () => { + const body = { + conversationId: '${TEST_API_KEY}', + parentMessageId: '${TEST_API_KEY}', + messageId: '${TEST_API_KEY}', + }; + const headers = { + 'X-Conv': '{{LIBRECHAT_BODY_CONVERSATIONID}}', + 'X-Parent': '{{LIBRECHAT_BODY_PARENTMESSAGEID}}', + 'X-Msg': '{{LIBRECHAT_BODY_MESSAGEID}}', + }; + const result = resolveHeaders({ headers, body }); + + expect(result['X-Conv']).toBe('${TEST_API_KEY}'); + expect(result['X-Parent']).toBe('${TEST_API_KEY}'); + expect(result['X-Msg']).toBe('${TEST_API_KEY}'); + }); + + it('should not resolve env vars introduced via LIBRECHAT_USER placeholders', () => { + const user = createTestUser({ name: '${TEST_API_KEY}' }); + const headers = { 'X-Name': '{{LIBRECHAT_USER_NAME}}' }; + const result = resolveHeaders({ headers, user }); + + expect(result['X-Name']).toBe('${TEST_API_KEY}'); + }); + + it('should not resolve env vars introduced via customUserVars', () => { + const customUserVars = { MY_TOKEN: '${TEST_API_KEY}' }; + const headers = { Authorization: 'Bearer {{MY_TOKEN}}' }; + const result = resolveHeaders({ headers, customUserVars }); + + expect(result.Authorization).toBe('Bearer ${TEST_API_KEY}'); + }); + describe('non-string header values (type guard tests)', () => { it('should handle numeric header values without crashing', () => { const headers = { @@ -657,12 +691,12 @@ describe('resolveHeaders', () => { describe('resolveNestedObject', () => { beforeEach(() => { process.env.TEST_API_KEY = 'test-api-key-value'; - process.env.ANOTHER_SECRET = 'another-secret-value'; + process.env.ANOTHER_VALUE = 'another-test-value'; }); afterEach(() => { delete process.env.TEST_API_KEY; - delete process.env.ANOTHER_SECRET; + delete process.env.ANOTHER_VALUE; }); it('should preserve nested object structure', () => { @@ -952,7 +986,7 @@ describe('resolveNestedObject', () => { describe('processMCPEnv', () => { beforeEach(() => { process.env.TEST_API_KEY = 'test-api-key-value'; - process.env.ANOTHER_SECRET = 'another-secret-value'; + process.env.ANOTHER_VALUE = 'another-test-value'; process.env.OAUTH_CLIENT_ID = 'oauth-client-id-value'; process.env.OAUTH_CLIENT_SECRET = 'oauth-client-secret-value'; process.env.MCP_SERVER_URL = 'https://mcp.example.com'; @@ -960,7 +994,7 @@ describe('processMCPEnv', () => { afterEach(() => { delete process.env.TEST_API_KEY; - delete process.env.ANOTHER_SECRET; + delete process.env.ANOTHER_VALUE; delete process.env.OAUTH_CLIENT_ID; delete process.env.OAUTH_CLIENT_SECRET; delete process.env.MCP_SERVER_URL; @@ -977,7 +1011,7 @@ describe('processMCPEnv', () => { command: 'mcp-server', env: { API_KEY: '${TEST_API_KEY}', - SECRET: '${ANOTHER_SECRET}', + SECRET: '${ANOTHER_VALUE}', PLAIN_VALUE: 'plain-text', }, args: ['--key', '${TEST_API_KEY}', '--url', '${MCP_SERVER_URL}'], @@ -990,7 +1024,7 @@ describe('processMCPEnv', () => { command: 'mcp-server', env: { API_KEY: 'test-api-key-value', - SECRET: 'another-secret-value', + SECRET: 'another-test-value', PLAIN_VALUE: 'plain-text', }, args: ['--key', 'test-api-key-value', '--url', 'https://mcp.example.com'], @@ -1137,6 +1171,49 @@ describe('processMCPEnv', () => { }); }); + it('should not resolve env vars introduced via body placeholders in MCP headers', () => { + const body = { + conversationId: '${TEST_API_KEY}', + parentMessageId: '${TEST_API_KEY}', + messageId: '${TEST_API_KEY}', + }; + + const options: MCPOptions = { + type: 'streamable-http', + url: 'https://api.example.com', + headers: { + 'X-Conv': '{{LIBRECHAT_BODY_CONVERSATIONID}}', + 'X-Parent': '{{LIBRECHAT_BODY_PARENTMESSAGEID}}', + }, + }; + + const result = processMCPEnv({ options, body }); + + if (!isStreamableHTTPOptions(result)) { + throw new Error('Expected streamable-http options'); + } + expect(result.headers?.['X-Conv']).toBe('${TEST_API_KEY}'); + expect(result.headers?.['X-Parent']).toBe('${TEST_API_KEY}'); + }); + + it('should not resolve env vars introduced via customUserVars in MCP headers', () => { + const customUserVars = { MY_TOKEN: '${TEST_API_KEY}' }; + const options: MCPOptions = { + type: 'streamable-http', + url: 'https://api.example.com', + headers: { + Authorization: 'Bearer {{MY_TOKEN}}', + }, + }; + + const result = processMCPEnv({ options, customUserVars }); + + if (!isStreamableHTTPOptions(result)) { + throw new Error('Expected streamable-http options'); + } + expect(result.headers?.Authorization).toBe('Bearer ${TEST_API_KEY}'); + }); + it('should handle mixed placeholders in OAuth configuration', () => { const user = createTestUser({ id: 'user-123', diff --git a/packages/api/src/utils/env.ts b/packages/api/src/utils/env.ts index 78d6f9ebdf..adeeb24b34 100644 --- a/packages/api/src/utils/env.ts +++ b/packages/api/src/utils/env.ts @@ -226,9 +226,20 @@ function processSingleValue({ let value = originalValue; + /** + * SECURITY INVARIANT — ordering matters: + * Resolve env vars on the admin-authored template BEFORE any user-controlled + * data is substituted (customUserVars, user fields, OIDC tokens, body placeholders). + * This prevents second-order injection where user values containing ${VAR} + * patterns would otherwise be expanded against process.env. + */ + if (!dbSourced) { + value = extractEnvVariable(value); + } + + /** Runs for both dbSourced and non-dbSourced — it is the only resolution DB-stored servers get */ if (customUserVars) { for (const [varName, varVal] of Object.entries(customUserVars)) { - /** Escaped varName for use in regex to avoid issues with special characters */ const escapedVarName = varName.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); const placeholderRegex = new RegExp(`\\{\\{${escapedVarName}\\}\\}`, 'g'); value = value.replace(placeholderRegex, varVal); @@ -250,8 +261,6 @@ function processSingleValue({ value = processBodyPlaceholders(value, body); } - value = extractEnvVariable(value); - return value; } diff --git a/packages/data-provider/specs/utils.spec.ts b/packages/data-provider/specs/utils.spec.ts index 01c403f4e8..48df6d46c2 100644 --- a/packages/data-provider/specs/utils.spec.ts +++ b/packages/data-provider/specs/utils.spec.ts @@ -1,4 +1,4 @@ -import { extractEnvVariable } from '../src/utils'; +import { extractEnvVariable, isSensitiveEnvVar } from '../src/utils'; describe('Environment Variable Extraction', () => { const originalEnv = process.env; @@ -7,7 +7,7 @@ describe('Environment Variable Extraction', () => { process.env = { ...originalEnv, TEST_API_KEY: 'test-api-key-value', - ANOTHER_SECRET: 'another-secret-value', + ANOTHER_VALUE: 'another-value', }; }); @@ -55,7 +55,7 @@ describe('Environment Variable Extraction', () => { describe('extractEnvVariable function', () => { it('should extract environment variables from exact matches', () => { expect(extractEnvVariable('${TEST_API_KEY}')).toBe('test-api-key-value'); - expect(extractEnvVariable('${ANOTHER_SECRET}')).toBe('another-secret-value'); + expect(extractEnvVariable('${ANOTHER_VALUE}')).toBe('another-value'); }); it('should extract environment variables from strings with prefixes', () => { @@ -82,7 +82,7 @@ describe('Environment Variable Extraction', () => { describe('extractEnvVariable', () => { it('should extract environment variable values', () => { expect(extractEnvVariable('${TEST_API_KEY}')).toBe('test-api-key-value'); - expect(extractEnvVariable('${ANOTHER_SECRET}')).toBe('another-secret-value'); + expect(extractEnvVariable('${ANOTHER_VALUE}')).toBe('another-value'); }); it('should return the original string if environment variable is not found', () => { @@ -126,4 +126,71 @@ describe('Environment Variable Extraction', () => { ); }); }); + + describe('isSensitiveEnvVar', () => { + it('should flag infrastructure secrets', () => { + expect(isSensitiveEnvVar('JWT_SECRET')).toBe(true); + expect(isSensitiveEnvVar('JWT_REFRESH_SECRET')).toBe(true); + expect(isSensitiveEnvVar('CREDS_KEY')).toBe(true); + expect(isSensitiveEnvVar('CREDS_IV')).toBe(true); + expect(isSensitiveEnvVar('MEILI_MASTER_KEY')).toBe(true); + expect(isSensitiveEnvVar('MONGO_URI')).toBe(true); + expect(isSensitiveEnvVar('REDIS_URI')).toBe(true); + expect(isSensitiveEnvVar('REDIS_PASSWORD')).toBe(true); + }); + + it('should allow non-infrastructure vars through (including operator-configured secrets)', () => { + expect(isSensitiveEnvVar('OPENAI_API_KEY')).toBe(false); + expect(isSensitiveEnvVar('ANTHROPIC_API_KEY')).toBe(false); + expect(isSensitiveEnvVar('GOOGLE_KEY')).toBe(false); + expect(isSensitiveEnvVar('PROXY')).toBe(false); + expect(isSensitiveEnvVar('DEBUG_LOGGING')).toBe(false); + expect(isSensitiveEnvVar('DOMAIN_CLIENT')).toBe(false); + expect(isSensitiveEnvVar('APP_TITLE')).toBe(false); + expect(isSensitiveEnvVar('OPENID_CLIENT_SECRET')).toBe(false); + expect(isSensitiveEnvVar('DISCORD_CLIENT_SECRET')).toBe(false); + expect(isSensitiveEnvVar('MY_CUSTOM_SECRET')).toBe(false); + }); + }); + + describe('extractEnvVariable sensitive var blocklist', () => { + beforeEach(() => { + process.env.JWT_SECRET = 'super-secret-jwt'; + process.env.JWT_REFRESH_SECRET = 'super-secret-refresh'; + process.env.CREDS_KEY = 'encryption-key'; + process.env.CREDS_IV = 'encryption-iv'; + process.env.MEILI_MASTER_KEY = 'meili-key'; + process.env.MONGO_URI = 'mongodb://user:pass@host/db'; + process.env.REDIS_URI = 'redis://:pass@host:6379'; + process.env.REDIS_PASSWORD = 'redis-pass'; + process.env.OPENAI_API_KEY = 'sk-legit-key'; + }); + + it('should refuse to resolve sensitive vars (single-match path)', () => { + expect(extractEnvVariable('${JWT_SECRET}')).toBe('${JWT_SECRET}'); + expect(extractEnvVariable('${JWT_REFRESH_SECRET}')).toBe('${JWT_REFRESH_SECRET}'); + expect(extractEnvVariable('${CREDS_KEY}')).toBe('${CREDS_KEY}'); + expect(extractEnvVariable('${CREDS_IV}')).toBe('${CREDS_IV}'); + expect(extractEnvVariable('${MEILI_MASTER_KEY}')).toBe('${MEILI_MASTER_KEY}'); + expect(extractEnvVariable('${MONGO_URI}')).toBe('${MONGO_URI}'); + expect(extractEnvVariable('${REDIS_URI}')).toBe('${REDIS_URI}'); + expect(extractEnvVariable('${REDIS_PASSWORD}')).toBe('${REDIS_PASSWORD}'); + }); + + it('should refuse to resolve sensitive vars in composite strings (multi-match path)', () => { + expect(extractEnvVariable('key=${JWT_SECRET}&more')).toBe('key=${JWT_SECRET}&more'); + expect(extractEnvVariable('db=${MONGO_URI}/extra')).toBe('db=${MONGO_URI}/extra'); + }); + + it('should still resolve non-sensitive vars normally', () => { + expect(extractEnvVariable('${OPENAI_API_KEY}')).toBe('sk-legit-key'); + expect(extractEnvVariable('Bearer ${OPENAI_API_KEY}')).toBe('Bearer sk-legit-key'); + }); + + it('should resolve non-sensitive vars while blocking sensitive ones in the same string', () => { + expect(extractEnvVariable('key=${OPENAI_API_KEY}&secret=${JWT_SECRET}')).toBe( + 'key=sk-legit-key&secret=${JWT_SECRET}', + ); + }); + }); }); diff --git a/packages/data-provider/src/utils.ts b/packages/data-provider/src/utils.ts index 57abbf0495..1eefcff8c4 100644 --- a/packages/data-provider/src/utils.ts +++ b/packages/data-provider/src/utils.ts @@ -1,5 +1,29 @@ export const envVarRegex = /^\${(.+)}$/; +/** + * Infrastructure env vars that must never be resolved via placeholder expansion. + * These are internal secrets whose exposure would compromise the system — + * they have no legitimate reason to appear in outbound headers, MCP env/args, or OAuth config. + * + * Intentionally excludes API keys (operators reference them in config) and + * OAuth/session secrets (referenced in MCP OAuth config via processMCPEnv). + */ +const SENSITIVE_ENV_VARS = new Set([ + 'JWT_SECRET', + 'JWT_REFRESH_SECRET', + 'CREDS_KEY', + 'CREDS_IV', + 'MEILI_MASTER_KEY', + 'MONGO_URI', + 'REDIS_URI', + 'REDIS_PASSWORD', +]); + +/** Returns true when `varName` refers to an infrastructure secret that must not leak. */ +export function isSensitiveEnvVar(varName: string): boolean { + return SENSITIVE_ENV_VARS.has(varName); +} + /** Extracts the environment variable name from a template literal string */ export function extractVariableName(value: string): string | null { if (!value) { @@ -16,21 +40,20 @@ export function extractEnvVariable(value: string) { return value; } - // Trim the input const trimmed = value.trim(); - // Special case: if it's just a single environment variable const singleMatch = trimmed.match(envVarRegex); if (singleMatch) { const varName = singleMatch[1]; + if (isSensitiveEnvVar(varName)) { + return trimmed; + } return process.env[varName] || trimmed; } - // For multiple variables, process them using a regex loop const regex = /\${([^}]+)}/g; let result = trimmed; - // First collect all matches and their positions const matches = []; let match; while ((match = regex.exec(trimmed)) !== null) { @@ -41,12 +64,12 @@ export function extractEnvVariable(value: string) { }); } - // Process matches in reverse order to avoid position shifts for (let i = matches.length - 1; i >= 0; i--) { const { fullMatch, varName, index } = matches[i]; + if (isSensitiveEnvVar(varName)) { + continue; + } const envValue = process.env[varName] || fullMatch; - - // Replace at exact position result = result.substring(0, index) + envValue + result.substring(index + fullMatch.length); }