From d3c06052d7f8a19e00927c70c863d690123a9e23 Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Tue, 3 Mar 2026 18:02:37 -0500 Subject: [PATCH] =?UTF-8?q?=F0=9F=97=9D=EF=B8=8F=20feat:=20Credential=20Va?= =?UTF-8?q?riables=20for=20DB-Sourced=20MCP=20Servers=20(#12044)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat: Allow Credential Variables in Headers for DB-sourced MCP Servers - Removed the hasCustomUserVars check from ToolService.js, directly retrieving userMCPAuthMap. - Updated MCPConnectionFactory and related classes to include a dbSourced flag for better handling of database-sourced configurations. - Added integration tests to ensure proper behavior of dbSourced servers, verifying that sensitive placeholders are not resolved while allowing customUserVars. - Adjusted various MCP-related files to accommodate the new dbSourced logic, ensuring consistent handling across the codebase. * chore: MCPConnectionFactory Tests with Additional Flow Metadata for typing - Updated MCPConnectionFactory tests to include new fields in flowMetadata: serverUrl and state. - Enhanced mockFlowData in multiple test cases to reflect the updated structure, ensuring comprehensive coverage of the OAuth flow scenarios. - Added authorization_endpoint to metadata in the test setup for improved validation of the OAuth process. * refactor: Simplify MCPManager Configuration Handling - Removed unnecessary type assertions and streamlined the retrieval of server configuration in MCPManager. - Enhanced the handling of OAuth and database-sourced flags for improved clarity and efficiency. - Updated tests to reflect changes in user object structure and ensure proper processing of MCP environment variables. * refactor: Optimize User MCP Auth Map Retrieval in ToolService - Introduced conditional loading of userMCPAuthMap based on the presence of MCP-delimited tools, improving efficiency by avoiding unnecessary calls. - Updated the loadToolDefinitionsWrapper and loadAgentTools functions to reflect this change, enhancing overall performance and clarity. * test: Add userMCPAuthMap gating tests in ToolService - Introduced new tests to validate the logic for determining if MCP tools are present in the agent's tool list. - Implemented various scenarios to ensure accurate detection of MCP tools, including edge cases for empty, undefined, and null tool lists. - Enhanced clarity and coverage of the ToolService capability checking logic. * refactor: Enhance MCP Environment Variable Processing - Simplified the handling of the dbSourced parameter in the processMCPEnv function. - Introduced a failsafe mechanism to derive dbSourced from options if not explicitly provided, improving robustness and clarity in MCP environment variable processing. * refactor: Update Regex Patterns for Credential Placeholders in ServerConfigsDB - Modified regex patterns to include additional credential/env placeholders that should not be allowed in user-provided configurations. - Clarified comments to emphasize the security risks associated with credential exfiltration when MCP servers are shared between users. * chore: field order * refactor: Clean Up dbSourced Parameter Handling in processMCPEnv - Reintroduced the failsafe mechanism for deriving the dbSourced parameter from options, ensuring clarity and robustness in MCP environment variable processing. - Enhanced code readability by maintaining consistent comment structure. * refactor: Update MCPOptions Type to Include Optional dbId - Modified the processMCPEnv function to extend the MCPOptions type, allowing for an optional dbId property. - Simplified the logic for deriving the dbSourced parameter by directly checking the dbId property, enhancing code clarity and maintainability. --- api/server/services/ToolService.js | 6 +- .../services/__tests__/ToolService.spec.js | 55 +- packages/api/src/app/config.ts | 8 +- packages/api/src/mcp/ConnectionsRepository.ts | 1 + packages/api/src/mcp/MCPConnectionFactory.ts | 5 +- packages/api/src/mcp/MCPManager.ts | 41 +- packages/api/src/mcp/UserConnectionManager.ts | 3 +- .../__tests__/ConnectionsRepository.test.ts | 11 +- .../__tests__/MCPConnectionFactory.test.ts | 30 +- .../__tests__/dbSourced.integration.test.ts | 517 ++++++++++++++++++ .../src/mcp/registry/MCPServerInspector.ts | 3 +- .../__tests__/MCPServerInspector.test.ts | 5 +- .../src/mcp/registry/db/ServerConfigsDB.ts | 15 +- packages/api/src/mcp/types/index.ts | 2 + packages/api/src/utils/env.spec.ts | 381 ++++++++++++- packages/api/src/utils/env.ts | 47 +- 16 files changed, 1060 insertions(+), 70 deletions(-) create mode 100644 packages/api/src/mcp/__tests__/dbSourced.integration.test.ts diff --git a/api/server/services/ToolService.js b/api/server/services/ToolService.js index eedb95bd4d..62499348e6 100644 --- a/api/server/services/ToolService.js +++ b/api/server/services/ToolService.js @@ -12,7 +12,6 @@ const { const { sendEvent, getToolkitKey, - hasCustomUserVars, getUserMCPAuthMap, loadToolDefinitions, GenerationJobManager, @@ -481,7 +480,7 @@ async function loadToolDefinitionsWrapper({ req, res, agent, streamId = null, to /** @type {Record>} */ let userMCPAuthMap; - if (hasCustomUserVars(req.config)) { + if (agent.tools?.some((t) => t.includes(Constants.mcp_delimiter))) { userMCPAuthMap = await getUserMCPAuthMap({ tools: agent.tools, userId: req.user.id, @@ -860,8 +859,7 @@ async function loadAgentTools({ /** @type {Record>} */ let userMCPAuthMap; - //TODO pass config from registry - if (hasCustomUserVars(req.config)) { + if (agent.tools?.some((t) => t.includes(Constants.mcp_delimiter))) { userMCPAuthMap = await getUserMCPAuthMap({ tools: agent.tools, userId: req.user.id, diff --git a/api/server/services/__tests__/ToolService.spec.js b/api/server/services/__tests__/ToolService.spec.js index 2f00bbc3d6..c44298b09c 100644 --- a/api/server/services/__tests__/ToolService.spec.js +++ b/api/server/services/__tests__/ToolService.spec.js @@ -1,4 +1,8 @@ -const { AgentCapabilities, defaultAgentCapabilities } = require('librechat-data-provider'); +const { + Constants, + AgentCapabilities, + defaultAgentCapabilities, +} = require('librechat-data-provider'); /** * Tests for ToolService capability checking logic. @@ -119,6 +123,55 @@ describe('ToolService - Capability Checking', () => { }); }); + describe('userMCPAuthMap gating', () => { + /** + * Simulates the guard condition used in both loadToolDefinitionsWrapper + * and loadAgentTools to decide whether getUserMCPAuthMap should be called. + */ + const shouldFetchMCPAuth = (tools) => + tools?.some((t) => t.includes(Constants.mcp_delimiter)) ?? false; + + it('should return true when agent has MCP tools', () => { + const tools = ['web_search', `search${Constants.mcp_delimiter}my-mcp-server`, 'calculator']; + expect(shouldFetchMCPAuth(tools)).toBe(true); + }); + + it('should return false when agent has no MCP tools', () => { + const tools = ['web_search', 'calculator', 'code_interpreter']; + expect(shouldFetchMCPAuth(tools)).toBe(false); + }); + + it('should return false when tools is empty', () => { + expect(shouldFetchMCPAuth([])).toBe(false); + }); + + it('should return false when tools is undefined', () => { + expect(shouldFetchMCPAuth(undefined)).toBe(false); + }); + + it('should return false when tools is null', () => { + expect(shouldFetchMCPAuth(null)).toBe(false); + }); + + it('should detect MCP tools with different server names', () => { + const tools = [ + `listFiles${Constants.mcp_delimiter}file-server`, + `query${Constants.mcp_delimiter}db-server`, + ]; + expect(shouldFetchMCPAuth(tools)).toBe(true); + }); + + it('should return true even when only one tool is MCP', () => { + const tools = [ + 'web_search', + 'calculator', + 'code_interpreter', + `echo${Constants.mcp_delimiter}test-server`, + ]; + expect(shouldFetchMCPAuth(tools)).toBe(true); + }); + }); + describe('deferredToolsEnabled integration', () => { it('should correctly determine deferredToolsEnabled from capabilities set', () => { const createCheckCapability = (enabledCapabilities) => { diff --git a/packages/api/src/app/config.ts b/packages/api/src/app/config.ts index 0a2fb3e6f9..db5917da09 100644 --- a/packages/api/src/app/config.ts +++ b/packages/api/src/app/config.ts @@ -64,11 +64,7 @@ export const getCustomEndpointConfig = ({ const customEndpoints = appConfig.endpoints?.[EModelEndpoint.custom] ?? []; return customEndpoints.find( - (endpointConfig) => normalizeEndpointName(endpointConfig.name) === normalizeEndpointName(endpoint), + (endpointConfig) => + normalizeEndpointName(endpointConfig.name) === normalizeEndpointName(endpoint), ); }; - -export function hasCustomUserVars(appConfig?: AppConfig): boolean { - const mcpServers = appConfig?.mcpConfig; - return Object.values(mcpServers ?? {}).some((server) => server?.customUserVars); -} diff --git a/packages/api/src/mcp/ConnectionsRepository.ts b/packages/api/src/mcp/ConnectionsRepository.ts index 3e0c2aca2d..edd1eabc2e 100644 --- a/packages/api/src/mcp/ConnectionsRepository.ts +++ b/packages/api/src/mcp/ConnectionsRepository.ts @@ -80,6 +80,7 @@ export class ConnectionsRepository { { serverName, serverConfig, + dbSourced: !!(serverConfig as t.ParsedServerConfig).dbId, useSSRFProtection: MCPServersRegistry.getInstance().shouldEnableSSRFProtection(), }, this.oauthOpts, diff --git a/packages/api/src/mcp/MCPConnectionFactory.ts b/packages/api/src/mcp/MCPConnectionFactory.ts index 2bf02c7a3f..03131b659b 100644 --- a/packages/api/src/mcp/MCPConnectionFactory.ts +++ b/packages/api/src/mcp/MCPConnectionFactory.ts @@ -185,10 +185,11 @@ export class MCPConnectionFactory { protected constructor(basic: t.BasicConnectionOptions, oauth?: t.OAuthConnectionOptions) { this.serverConfig = processMCPEnv({ - options: basic.serverConfig, user: oauth?.user, - customUserVars: oauth?.customUserVars, body: oauth?.requestBody, + dbSourced: basic.dbSourced, + options: basic.serverConfig, + customUserVars: oauth?.customUserVars, }); this.serverName = basic.serverName; this.useOAuth = !!oauth?.useOAuth; diff --git a/packages/api/src/mcp/MCPManager.ts b/packages/api/src/mcp/MCPManager.ts index cab495774a..6fdf45c27a 100644 --- a/packages/api/src/mcp/MCPManager.ts +++ b/packages/api/src/mcp/MCPManager.ts @@ -88,22 +88,26 @@ export class MCPManager extends UserConnectionManager { logger.debug(`${logPrefix} [Discovery] App connection not available, trying discovery mode`); } - const serverConfig = (await MCPServersRegistry.getInstance().getServerConfig( + const serverConfig = await MCPServersRegistry.getInstance().getServerConfig( serverName, user?.id, - )) as t.MCPOptions | null; + ); if (!serverConfig) { logger.warn(`${logPrefix} [Discovery] Server config not found`); return { tools: null, oauthRequired: false, oauthUrl: null }; } - const useOAuth = Boolean( - serverConfig.requiresOAuth || (serverConfig as t.ParsedServerConfig).oauthMetadata, - ); + const useOAuth = Boolean(serverConfig.requiresOAuth || serverConfig.oauthMetadata); const useSSRFProtection = MCPServersRegistry.getInstance().shouldEnableSSRFProtection(); - const basic: t.BasicConnectionOptions = { serverName, serverConfig, useSSRFProtection }; + const dbSourced = !!serverConfig.dbId; + const basic: t.BasicConnectionOptions = { + dbSourced, + serverName, + serverConfig, + useSSRFProtection, + }; if (!useOAuth) { const result = await MCPConnectionFactory.discoverTools(basic); @@ -290,22 +294,23 @@ Please follow these instructions when using tools from the respective MCP server ); } - const rawConfig = (await MCPServersRegistry.getInstance().getServerConfig( - serverName, - userId, - )) as t.MCPOptions; + const rawConfig = await MCPServersRegistry.getInstance().getServerConfig(serverName, userId); + const isDbSourced = !!rawConfig?.dbId; - // Pre-process Graph token placeholders (async) before sync processMCPEnv - const graphProcessedConfig = await preProcessGraphTokens(rawConfig, { - user, - graphTokenResolver, - scopes: process.env.GRAPH_API_SCOPES, - }); + /** Pre-process Graph token placeholders (async) before the synchronous processMCPEnv pass */ + const graphProcessedConfig = isDbSourced + ? (rawConfig as t.MCPOptions) + : await preProcessGraphTokens(rawConfig as t.MCPOptions, { + user, + graphTokenResolver, + scopes: process.env.GRAPH_API_SCOPES, + }); const currentOptions = processMCPEnv({ user, - options: graphProcessedConfig, - customUserVars: customUserVars, body: requestBody, + dbSourced: isDbSourced, + options: graphProcessedConfig, + customUserVars, }); if ('headers' in currentOptions) { connection.setRequestHeaders(currentOptions.headers || {}); diff --git a/packages/api/src/mcp/UserConnectionManager.ts b/packages/api/src/mcp/UserConnectionManager.ts index 1b90072618..c0ecd18fe2 100644 --- a/packages/api/src/mcp/UserConnectionManager.ts +++ b/packages/api/src/mcp/UserConnectionManager.ts @@ -115,8 +115,9 @@ export abstract class UserConnectionManager { try { connection = await MCPConnectionFactory.create( { - serverName: serverName, serverConfig: config, + serverName: serverName, + dbSourced: !!config.dbId, useSSRFProtection: MCPServersRegistry.getInstance().shouldEnableSSRFProtection(), }, { diff --git a/packages/api/src/mcp/__tests__/ConnectionsRepository.test.ts b/packages/api/src/mcp/__tests__/ConnectionsRepository.test.ts index 4240ba12d6..3b827774d0 100644 --- a/packages/api/src/mcp/__tests__/ConnectionsRepository.test.ts +++ b/packages/api/src/mcp/__tests__/ConnectionsRepository.test.ts @@ -1,8 +1,8 @@ import { logger } from '@librechat/data-schemas'; -import { ConnectionsRepository } from '../ConnectionsRepository'; -import { MCPConnectionFactory } from '../MCPConnectionFactory'; -import { MCPConnection } from '../connection'; -import type * as t from '../types'; +import type * as t from '~/mcp/types'; +import { ConnectionsRepository } from '~/mcp/ConnectionsRepository'; +import { MCPConnectionFactory } from '~/mcp/MCPConnectionFactory'; +import { MCPConnection } from '~/mcp/connection'; // Mock external dependencies jest.mock('@librechat/data-schemas', () => ({ @@ -110,6 +110,7 @@ describe('ConnectionsRepository', () => { serverName: 'server1', serverConfig: mockServerConfigs.server1, useSSRFProtection: false, + dbSourced: false, }, undefined, ); @@ -132,6 +133,7 @@ describe('ConnectionsRepository', () => { serverName: 'server1', serverConfig: mockServerConfigs.server1, useSSRFProtection: false, + dbSourced: false, }, undefined, ); @@ -171,6 +173,7 @@ describe('ConnectionsRepository', () => { serverName: 'server1', serverConfig: configWithCachedAt, useSSRFProtection: false, + dbSourced: false, }, undefined, ); diff --git a/packages/api/src/mcp/__tests__/MCPConnectionFactory.test.ts b/packages/api/src/mcp/__tests__/MCPConnectionFactory.test.ts index c8b3a4b04f..de18e27e89 100644 --- a/packages/api/src/mcp/__tests__/MCPConnectionFactory.test.ts +++ b/packages/api/src/mcp/__tests__/MCPConnectionFactory.test.ts @@ -79,7 +79,10 @@ describe('MCPConnectionFactory', () => { const connection = await MCPConnectionFactory.create(basicOptions); expect(connection).toBe(mockConnectionInstance); - expect(mockProcessMCPEnv).toHaveBeenCalledWith({ options: mockServerConfig }); + expect(mockProcessMCPEnv).toHaveBeenCalledWith({ + options: mockServerConfig, + dbSourced: undefined, + }); expect(mockMCPConnection).toHaveBeenCalledWith({ serverName: 'test-server', serverConfig: mockServerConfig, @@ -121,7 +124,11 @@ describe('MCPConnectionFactory', () => { const connection = await MCPConnectionFactory.create(basicOptions, oauthOptions); expect(connection).toBe(mockConnectionInstance); - expect(mockProcessMCPEnv).toHaveBeenCalledWith({ options: mockServerConfig, user: mockUser }); + expect(mockProcessMCPEnv).toHaveBeenCalledWith({ + options: mockServerConfig, + user: mockUser, + dbSourced: undefined, + }); expect(mockMCPConnection).toHaveBeenCalledWith({ serverName: 'test-server', serverConfig: mockServerConfig, @@ -358,7 +365,12 @@ describe('MCPConnectionFactory', () => { const mockFlowData = { authorizationUrl: 'https://auth.example.com', flowId: 'flow123', - flowMetadata: { serverName: 'test-server', userId: 'user123' }, + flowMetadata: { + serverName: 'test-server', + userId: 'user123', + serverUrl: 'https://api.example.com', + state: 'state123', + }, }; mockMCPOAuthHandler.initiateOAuthFlow.mockResolvedValue(mockFlowData); @@ -419,7 +431,12 @@ describe('MCPConnectionFactory', () => { const mockFlowData = { authorizationUrl: 'https://auth.example.com', flowId: 'flow123', - flowMetadata: { serverName: 'test-server', userId: 'user123' }, + flowMetadata: { + serverName: 'test-server', + userId: 'user123', + serverUrl: 'https://api.example.com', + state: 'state123', + }, }; mockMCPOAuthHandler.initiateOAuthFlow.mockResolvedValue(mockFlowData); @@ -491,7 +508,10 @@ describe('MCPConnectionFactory', () => { serverUrl: 'https://api.example.com', state: 'random-state', clientInfo: { client_id: 'client123' }, - metadata: { token_endpoint: 'https://auth.example.com/token' }, + metadata: { + token_endpoint: 'https://auth.example.com/token', + authorization_endpoint: 'https://auth.example.com/authorize', + }, }, }; diff --git a/packages/api/src/mcp/__tests__/dbSourced.integration.test.ts b/packages/api/src/mcp/__tests__/dbSourced.integration.test.ts new file mode 100644 index 0000000000..5866fa1a08 --- /dev/null +++ b/packages/api/src/mcp/__tests__/dbSourced.integration.test.ts @@ -0,0 +1,517 @@ +/** + * Integration tests for the `dbSourced` security boundary. + * + * These tests spin up real in-process MCP servers using the SDK's + * StreamableHTTPServerTransport, then exercise the full + * processMCPEnv → MCPConnection → HTTP request pipeline to verify: + * + * 1. DB-sourced servers resolve `{{MCP_API_KEY}}` via customUserVars. + * 2. DB-sourced servers do NOT leak `${ENV_VAR}` from process.env. + * 3. DB-sourced servers do NOT resolve `{{LIBRECHAT_USER_*}}` placeholders. + * 4. DB-sourced servers do NOT resolve `{{LIBRECHAT_BODY_*}}` placeholders. + * 5. YAML-sourced servers (dbSourced=false) resolve ALL placeholder types. + * 6. Mixed headers: some placeholders resolve, others are blocked. + */ + +import * as net from 'net'; +import * as http from 'http'; +import { Agent } from 'undici'; +import { Types } from 'mongoose'; +import { randomUUID } from 'crypto'; +import { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'; +import { StreamableHTTPServerTransport } from '@modelcontextprotocol/sdk/server/streamableHttp.js'; +import type { MCPOptions } from 'librechat-data-provider'; +import type { IUser } from '@librechat/data-schemas'; +import type { Socket } from 'net'; +import { MCPConnection } from '~/mcp/connection'; +import { processMCPEnv } from '~/utils/env'; + +jest.mock('@librechat/data-schemas', () => ({ + logger: { + info: jest.fn(), + warn: jest.fn(), + error: jest.fn(), + debug: jest.fn(), + }, +})); + +jest.mock('~/auth', () => ({ + createSSRFSafeUndiciConnect: jest.fn(() => undefined), + resolveHostnameSSRF: jest.fn(async () => false), +})); + +jest.mock('~/mcp/mcpConfig', () => ({ + mcpConfig: { CONNECTION_CHECK_TTL: 0 }, +})); + +/** Track all Agents for cleanup */ +const allAgentsCreated: Agent[] = []; +const OriginalAgent = Agent; +const PatchedAgent = new Proxy(OriginalAgent, { + construct(target, args) { + const instance = new target(...(args as [Agent.Options?])); + allAgentsCreated.push(instance); + return instance; + }, +}); +(global as Record).__undiciAgent = PatchedAgent; + +afterAll(async () => { + const destroying = allAgentsCreated.map((a) => { + if (!a.destroyed && !a.closed) { + return a.destroy().catch(() => undefined); + } + return Promise.resolve(); + }); + allAgentsCreated.length = 0; + await Promise.all(destroying); +}); + +async function safeDisconnect(conn: MCPConnection | null): Promise { + if (!conn) return; + (conn as unknown as { shouldStopReconnecting: boolean }).shouldStopReconnecting = true; + conn.removeAllListeners(); + await conn.disconnect(); +} + +function getFreePort(): Promise { + return new Promise((resolve, reject) => { + const srv = net.createServer(); + srv.listen(0, '127.0.0.1', () => { + const addr = srv.address() as net.AddressInfo; + srv.close((err) => (err ? reject(err) : resolve(addr.port))); + }); + }); +} + +function trackSockets(httpServer: http.Server): () => Promise { + const sockets = new Set(); + httpServer.on('connection', (socket: Socket) => { + sockets.add(socket); + socket.once('close', () => sockets.delete(socket)); + }); + return () => + new Promise((resolve) => { + for (const socket of sockets) socket.destroy(); + sockets.clear(); + httpServer.close(() => resolve()); + }); +} + +interface TestServer { + url: string; + close: () => Promise; + /** Returns the most recently captured request headers */ + getLastHeaders: () => Record; +} + +function createTestUser(overrides: Partial = {}): IUser { + return { + _id: new Types.ObjectId(), + id: new Types.ObjectId().toString(), + username: 'testuser', + email: 'test@example.com', + name: 'Test User', + avatar: 'https://example.com/avatar.png', + provider: 'email', + role: 'user', + createdAt: new Date('2021-01-01'), + updatedAt: new Date('2021-01-01'), + emailVerified: true, + ...overrides, + } as IUser; +} + +/** + * Creates a Streamable HTTP MCP server that captures incoming request headers. + * The server registers a dummy tool so `fetchTools()` makes a real request + * through the transport, allowing us to capture the headers from that request. + */ +async function createHeaderCapturingServer(): Promise { + const sessions = new Map(); + let lastHeaders: Record = {}; + + const httpServer = http.createServer(async (req, res) => { + // Capture headers from every POST request (the tool-listing / tool-call requests) + if (req.method === 'POST') { + lastHeaders = {}; + for (const [key, value] of Object.entries(req.headers)) { + if (typeof value === 'string') { + lastHeaders[key] = value; + } else if (Array.isArray(value)) { + lastHeaders[key] = value.join(', '); + } + } + } + + const sid = req.headers['mcp-session-id'] as string | undefined; + let transport = sid ? sessions.get(sid) : undefined; + + if (!transport) { + transport = new StreamableHTTPServerTransport({ sessionIdGenerator: () => randomUUID() }); + const mcp = new McpServer({ name: 'header-capture-server', version: '0.0.1' }); + mcp.tool('echo', 'Echo tool for testing', {}, async () => ({ + content: [{ type: 'text', text: 'ok' }], + })); + await mcp.connect(transport); + } + + await transport.handleRequest(req, res); + + if (transport.sessionId && !sessions.has(transport.sessionId)) { + sessions.set(transport.sessionId, transport); + transport.onclose = () => sessions.delete(transport!.sessionId!); + } + }); + + const destroySockets = trackSockets(httpServer); + const port = await getFreePort(); + await new Promise((resolve) => httpServer.listen(port, '127.0.0.1', resolve)); + + return { + url: `http://127.0.0.1:${port}/`, + getLastHeaders: () => ({ ...lastHeaders }), + close: async () => { + const closing = [...sessions.values()].map((t) => t.close().catch(() => undefined)); + sessions.clear(); + await Promise.all(closing); + await destroySockets(); + }, + }; +} + +describe('dbSourced header security – integration', () => { + let server: TestServer; + let conn: MCPConnection | null; + + beforeEach(async () => { + server = await createHeaderCapturingServer(); + conn = null; + process.env.SECRET_DB_URL = 'mongodb://admin:password@prod-host:27017/secret'; + process.env.INTERNAL_API_KEY = 'internal-key-do-not-leak'; + }); + + afterEach(async () => { + await safeDisconnect(conn); + conn = null; + jest.restoreAllMocks(); + await server.close(); + delete process.env.SECRET_DB_URL; + delete process.env.INTERNAL_API_KEY; + }); + + it('DB-sourced: resolves {{MCP_API_KEY}} via customUserVars', async () => { + const options: MCPOptions = { + type: 'streamable-http', + url: server.url, + headers: { + Authorization: 'Bearer {{MCP_API_KEY}}', + }, + }; + + const resolved = processMCPEnv({ + options, + dbSourced: true, + customUserVars: { MCP_API_KEY: 'user-provided-secret' }, + }); + + conn = new MCPConnection({ + serverName: 'db-test', + serverConfig: resolved, + useSSRFProtection: false, + }); + + if ('headers' in resolved) { + conn.setRequestHeaders(resolved.headers || {}); + } + + await conn.connect(); + await conn.fetchTools(); + + const captured = server.getLastHeaders(); + expect(captured['authorization']).toBe('Bearer user-provided-secret'); + }); + + it('DB-sourced: does NOT resolve ${ENV_VAR} — literal placeholder sent as header', async () => { + const options: MCPOptions = { + type: 'streamable-http', + url: server.url, + headers: { + 'X-Leaked-DB': '${SECRET_DB_URL}', + 'X-Leaked-Key': '${INTERNAL_API_KEY}', + }, + }; + + const resolved = processMCPEnv({ options, dbSourced: true }); + + conn = new MCPConnection({ + serverName: 'db-test', + serverConfig: resolved, + useSSRFProtection: false, + }); + + if ('headers' in resolved) { + conn.setRequestHeaders(resolved.headers || {}); + } + + await conn.connect(); + await conn.fetchTools(); + + const captured = server.getLastHeaders(); + // The literal placeholders must be sent, NOT the env values + expect(captured['x-leaked-db']).toBe('${SECRET_DB_URL}'); + expect(captured['x-leaked-key']).toBe('${INTERNAL_API_KEY}'); + // Double-check env vars were NOT injected + expect(captured['x-leaked-db']).not.toContain('mongodb://'); + expect(captured['x-leaked-key']).not.toBe('internal-key-do-not-leak'); + }); + + it('DB-sourced: does NOT resolve {{LIBRECHAT_USER_*}} placeholders', async () => { + const user = createTestUser({ id: 'user-secret-id', email: 'private@corp.com' }); + const options: MCPOptions = { + type: 'streamable-http', + url: server.url, + headers: { + 'X-User-Id': '{{LIBRECHAT_USER_ID}}', + 'X-User-Email': '{{LIBRECHAT_USER_EMAIL}}', + }, + }; + + const resolved = processMCPEnv({ options, user, dbSourced: true }); + + conn = new MCPConnection({ + serverName: 'db-test', + serverConfig: resolved, + useSSRFProtection: false, + }); + + if ('headers' in resolved) { + conn.setRequestHeaders(resolved.headers || {}); + } + + await conn.connect(); + await conn.fetchTools(); + + const captured = server.getLastHeaders(); + expect(captured['x-user-id']).toBe('{{LIBRECHAT_USER_ID}}'); + expect(captured['x-user-email']).toBe('{{LIBRECHAT_USER_EMAIL}}'); + expect(captured['x-user-id']).not.toBe('user-secret-id'); + }); + + it('DB-sourced: does NOT resolve {{LIBRECHAT_BODY_*}} placeholders', async () => { + const body = { + conversationId: 'conv-secret-123', + parentMessageId: 'parent-456', + messageId: 'msg-789', + }; + const options: MCPOptions = { + type: 'streamable-http', + url: server.url, + headers: { + 'X-Conv-Id': '{{LIBRECHAT_BODY_CONVERSATIONID}}', + }, + }; + + const resolved = processMCPEnv({ options, body, dbSourced: true }); + + conn = new MCPConnection({ + serverName: 'db-test', + serverConfig: resolved, + useSSRFProtection: false, + }); + + if ('headers' in resolved) { + conn.setRequestHeaders(resolved.headers || {}); + } + + await conn.connect(); + await conn.fetchTools(); + + const captured = server.getLastHeaders(); + expect(captured['x-conv-id']).toBe('{{LIBRECHAT_BODY_CONVERSATIONID}}'); + expect(captured['x-conv-id']).not.toBe('conv-secret-123'); + }); + + it('DB-sourced: mixed headers — customUserVars resolve, everything else blocked', async () => { + const user = createTestUser({ id: 'user-id-value' }); + const body = { conversationId: 'conv-id-value', parentMessageId: 'p-1', messageId: 'm-1' }; + const options: MCPOptions = { + type: 'streamable-http', + url: server.url, + headers: { + Authorization: 'Bearer {{MCP_API_KEY}}', + 'X-Env-Leak': '${SECRET_DB_URL}', + 'X-User-Id': '{{LIBRECHAT_USER_ID}}', + 'X-Conv-Id': '{{LIBRECHAT_BODY_CONVERSATIONID}}', + 'X-Static': 'plain-value', + }, + }; + + const resolved = processMCPEnv({ + options, + user, + body, + dbSourced: true, + customUserVars: { MCP_API_KEY: 'my-api-key-value' }, + }); + + conn = new MCPConnection({ + serverName: 'db-test', + serverConfig: resolved, + useSSRFProtection: false, + }); + + if ('headers' in resolved) { + conn.setRequestHeaders(resolved.headers || {}); + } + + await conn.connect(); + await conn.fetchTools(); + + const captured = server.getLastHeaders(); + // customUserVars resolved + expect(captured['authorization']).toBe('Bearer my-api-key-value'); + // env var blocked + expect(captured['x-env-leak']).toBe('${SECRET_DB_URL}'); + // user placeholder blocked + expect(captured['x-user-id']).toBe('{{LIBRECHAT_USER_ID}}'); + // body placeholder blocked + expect(captured['x-conv-id']).toBe('{{LIBRECHAT_BODY_CONVERSATIONID}}'); + // static value unchanged + expect(captured['x-static']).toBe('plain-value'); + }); + + it('YAML-sourced (default): resolves ALL placeholder types', async () => { + const user = createTestUser({ id: 'yaml-user-id', email: 'yaml@example.com' }); + const body = { conversationId: 'yaml-conv-id', parentMessageId: 'p-1', messageId: 'm-1' }; + const options: MCPOptions = { + type: 'streamable-http', + url: server.url, + headers: { + Authorization: 'Bearer {{MY_CUSTOM_KEY}}', + 'X-Env': '${INTERNAL_API_KEY}', + 'X-User-Id': '{{LIBRECHAT_USER_ID}}', + 'X-Conv-Id': '{{LIBRECHAT_BODY_CONVERSATIONID}}', + }, + }; + + const resolved = processMCPEnv({ + options, + user, + body, + dbSourced: false, + customUserVars: { MY_CUSTOM_KEY: 'yaml-custom-val' }, + }); + + conn = new MCPConnection({ + serverName: 'yaml-test', + serverConfig: resolved, + useSSRFProtection: false, + }); + + if ('headers' in resolved) { + conn.setRequestHeaders(resolved.headers || {}); + } + + await conn.connect(); + await conn.fetchTools(); + + const captured = server.getLastHeaders(); + // All placeholder types resolved + expect(captured['authorization']).toBe('Bearer yaml-custom-val'); + expect(captured['x-env']).toBe('internal-key-do-not-leak'); + expect(captured['x-user-id']).toBe('yaml-user-id'); + expect(captured['x-conv-id']).toBe('yaml-conv-id'); + }); + + it('DB-sourced: URL placeholder ${ENV_VAR} is NOT resolved', () => { + const options: MCPOptions = { + type: 'streamable-http', + url: '${SECRET_DB_URL}/mcp', + headers: {}, + }; + + const resolved = processMCPEnv({ options, dbSourced: true }); + expect((resolved as { url?: string }).url).toBe('${SECRET_DB_URL}/mcp'); + }); + + it('YAML-sourced: URL placeholder ${ENV_VAR} IS resolved', () => { + const options: MCPOptions = { + type: 'streamable-http', + url: '${INTERNAL_API_KEY}/endpoint', + headers: {}, + }; + + const resolved = processMCPEnv({ options, dbSourced: false }); + expect((resolved as { url?: string }).url).toBe('internal-key-do-not-leak/endpoint'); + }); + + it('DB-sourced: multiple customUserVars resolve correctly', async () => { + const options: MCPOptions = { + type: 'streamable-http', + url: server.url, + headers: { + Authorization: 'Bearer {{API_TOKEN}}', + 'X-Workspace': '{{WORKSPACE_ID}}', + 'X-Region': '{{REGION}}', + }, + }; + + const resolved = processMCPEnv({ + options, + dbSourced: true, + customUserVars: { + API_TOKEN: 'tok-abc123', + WORKSPACE_ID: 'ws-def456', + REGION: 'us-east-1', + }, + }); + + conn = new MCPConnection({ + serverName: 'db-multi-var', + serverConfig: resolved, + useSSRFProtection: false, + }); + + if ('headers' in resolved) { + conn.setRequestHeaders(resolved.headers || {}); + } + + await conn.connect(); + await conn.fetchTools(); + + const captured = server.getLastHeaders(); + expect(captured['authorization']).toBe('Bearer tok-abc123'); + expect(captured['x-workspace']).toBe('ws-def456'); + expect(captured['x-region']).toBe('us-east-1'); + }); + + it('DB-sourced: absent customUserVars leaves placeholder unresolved', async () => { + const options: MCPOptions = { + type: 'streamable-http', + url: server.url, + headers: { + Authorization: 'Bearer {{MCP_API_KEY}}', + }, + }; + + // No customUserVars provided at all + const resolved = processMCPEnv({ options, dbSourced: true }); + + conn = new MCPConnection({ + serverName: 'db-no-vars', + serverConfig: resolved, + useSSRFProtection: false, + }); + + if ('headers' in resolved) { + conn.setRequestHeaders(resolved.headers || {}); + } + + await conn.connect(); + await conn.fetchTools(); + + const captured = server.getLastHeaders(); + expect(captured['authorization']).toBe('Bearer {{MCP_API_KEY}}'); + }); +}); diff --git a/packages/api/src/mcp/registry/MCPServerInspector.ts b/packages/api/src/mcp/registry/MCPServerInspector.ts index 50da9cdc25..eea52bbf2e 100644 --- a/packages/api/src/mcp/registry/MCPServerInspector.ts +++ b/packages/api/src/mcp/registry/MCPServerInspector.ts @@ -59,8 +59,9 @@ export class MCPServerInspector { if (!this.connection) { tempConnection = true; this.connection = await MCPConnectionFactory.create({ - serverName: this.serverName, serverConfig: this.config, + serverName: this.serverName, + dbSourced: !!this.config.dbId, useSSRFProtection: this.useSSRFProtection, }); } diff --git a/packages/api/src/mcp/registry/__tests__/MCPServerInspector.test.ts b/packages/api/src/mcp/registry/__tests__/MCPServerInspector.test.ts index 42dc4d2005..b79f2d044a 100644 --- a/packages/api/src/mcp/registry/__tests__/MCPServerInspector.test.ts +++ b/packages/api/src/mcp/registry/__tests__/MCPServerInspector.test.ts @@ -1,9 +1,9 @@ import type { MCPConnection } from '~/mcp/connection'; import type * as t from '~/mcp/types'; import { MCPServerInspector } from '~/mcp/registry/MCPServerInspector'; -import { detectOAuthRequirement } from '~/mcp/oauth'; -import { MCPConnectionFactory } from '~/mcp/MCPConnectionFactory'; import { createMockConnection } from './mcpConnectionsMock.helper'; +import { MCPConnectionFactory } from '~/mcp/MCPConnectionFactory'; +import { detectOAuthRequirement } from '~/mcp/oauth'; // Mock external dependencies jest.mock('../../oauth/detectOAuth'); @@ -277,6 +277,7 @@ describe('MCPServerInspector', () => { serverName: 'test_server', serverConfig: expect.objectContaining({ type: 'stdio', command: 'node' }), useSSRFProtection: true, + dbSourced: false, }); // Verify temporary connection was disconnected diff --git a/packages/api/src/mcp/registry/db/ServerConfigsDB.ts b/packages/api/src/mcp/registry/db/ServerConfigsDB.ts index f6f6d90858..50db81b831 100644 --- a/packages/api/src/mcp/registry/db/ServerConfigsDB.ts +++ b/packages/api/src/mcp/registry/db/ServerConfigsDB.ts @@ -12,15 +12,18 @@ import type { ParsedServerConfig, AddServerResult } from '~/mcp/types'; import { AccessControlService } from '~/acl/accessControlService'; /** - * Regex patterns for credential placeholders that should not be allowed in user-provided headers. - * These placeholders would substitute the CALLING user's credentials, creating a security risk - * when MCP servers are shared between users (credential exfiltration). + * Regex patterns for credential/env placeholders that should not be allowed in user-provided configs. + * These would substitute server credentials or the CALLING user's data, creating exfiltration risks + * when MCP servers are shared between users. * * Safe placeholders like {{MCP_API_KEY}} are allowed as they resolve from the user's own plugin auth. */ const DANGEROUS_CREDENTIAL_PATTERNS = [ + /\$\{[^}]+\}/g, /\{\{LIBRECHAT_OPENID_[^}]+\}\}/g, /\{\{LIBRECHAT_USER_[^}]+\}\}/g, + /\{\{LIBRECHAT_GRAPH_[^}]+\}\}/g, + /\{\{LIBRECHAT_BODY_[^}]+\}\}/g, ]; /** @@ -457,7 +460,7 @@ export class ServerConfigsDB implements IServerConfigsRepositoryInterface { }; // Remove key field since it's user-provided (destructure to omit, not set to undefined) - // eslint-disable-next-line @typescript-eslint/no-unused-vars + const { key: _removed, ...apiKeyWithoutKey } = result.apiKey!; result.apiKey = apiKeyWithoutKey; @@ -521,7 +524,7 @@ export class ServerConfigsDB implements IServerConfigsRepositoryInterface { '[ServerConfigsDB.decryptConfig] Failed to decrypt apiKey.key, returning config without key', error, ); - // eslint-disable-next-line @typescript-eslint/no-unused-vars + const { key: _removedKey, ...apiKeyWithoutKey } = result.apiKey; result.apiKey = apiKeyWithoutKey; } @@ -542,7 +545,7 @@ export class ServerConfigsDB implements IServerConfigsRepositoryInterface { '[ServerConfigsDB.decryptConfig] Failed to decrypt client_secret, returning config without secret', error, ); - // eslint-disable-next-line @typescript-eslint/no-unused-vars + const { client_secret: _removed, ...oauthWithoutSecret } = oauthConfig; result = { ...result, diff --git a/packages/api/src/mcp/types/index.ts b/packages/api/src/mcp/types/index.ts index 270131036b..353bcb9c19 100644 --- a/packages/api/src/mcp/types/index.ts +++ b/packages/api/src/mcp/types/index.ts @@ -167,6 +167,8 @@ export interface BasicConnectionOptions { serverName: string; serverConfig: MCPOptions; useSSRFProtection?: boolean; + /** When true, only resolve customUserVars in processMCPEnv (for DB-stored servers) */ + dbSourced?: boolean; } export interface OAuthConnectionOptions { diff --git a/packages/api/src/utils/env.spec.ts b/packages/api/src/utils/env.spec.ts index eec15c1c25..c241cb2b51 100644 --- a/packages/api/src/utils/env.spec.ts +++ b/packages/api/src/utils/env.spec.ts @@ -1,13 +1,8 @@ +import { Types } from 'mongoose'; import { TokenExchangeMethodEnum } from 'librechat-data-provider'; -import { - resolveHeaders, - resolveNestedObject, - processMCPEnv, - encodeHeaderValue, -} from './env'; import type { MCPOptions } from 'librechat-data-provider'; import type { IUser } from '@librechat/data-schemas'; -import { Types } from 'mongoose'; +import { resolveHeaders, resolveNestedObject, processMCPEnv, encodeHeaderValue } from './env'; function isStdioOptions(options: MCPOptions): options is Extract { return !options.type || options.type === 'stdio'; @@ -43,15 +38,14 @@ describe('encodeHeaderValue', () => { }); it('should return empty string for null/undefined coerced to empty string', () => { - // TypeScript would prevent these, but testing runtime behavior - expect(encodeHeaderValue(null as any)).toBe(''); - expect(encodeHeaderValue(undefined as any)).toBe(''); + expect(encodeHeaderValue(null as unknown as string)).toBe(''); + expect(encodeHeaderValue(undefined as unknown as string)).toBe(''); }); it('should return empty string for non-string values', () => { - expect(encodeHeaderValue(123 as any)).toBe(''); - expect(encodeHeaderValue(false as any)).toBe(''); - expect(encodeHeaderValue({} as any)).toBe(''); + expect(encodeHeaderValue(123 as unknown as string)).toBe(''); + expect(encodeHeaderValue(false as unknown as string)).toBe(''); + expect(encodeHeaderValue({} as unknown as string)).toBe(''); }); it('should pass through ASCII characters (0-127) unchanged', () => { @@ -1612,4 +1606,365 @@ describe('processMCPEnv', () => { } }); }); + + describe('dbSourced flag', () => { + beforeEach(() => { + process.env.TEST_API_KEY = 'test-api-key-value'; + process.env.DATABASE_URL = 'mongodb://secret-host:27017/db'; + }); + + afterEach(() => { + delete process.env.TEST_API_KEY; + delete process.env.DATABASE_URL; + }); + + it('should resolve customUserVars when dbSourced is true', () => { + const options: MCPOptions = { + type: 'streamable-http', + url: 'https://api.example.com', + headers: { + Authorization: 'Bearer {{MCP_API_KEY}}', + }, + }; + + const result = processMCPEnv({ + options, + dbSourced: true, + customUserVars: { MCP_API_KEY: 'user-secret-key' }, + }); + + if (isStreamableHTTPOptions(result)) { + expect(result.headers?.Authorization).toBe('Bearer user-secret-key'); + } else { + throw new Error('Expected streamable-http options'); + } + }); + + it('should NOT resolve ${ENV_VAR} when dbSourced is true', () => { + const options: MCPOptions = { + type: 'streamable-http', + url: 'https://api.example.com', + headers: { + 'X-Leaked': '${DATABASE_URL}', + 'X-Key': '${TEST_API_KEY}', + }, + }; + + const result = processMCPEnv({ options, dbSourced: true }); + + if (isStreamableHTTPOptions(result)) { + expect(result.headers?.['X-Leaked']).toBe('${DATABASE_URL}'); + expect(result.headers?.['X-Key']).toBe('${TEST_API_KEY}'); + } else { + throw new Error('Expected streamable-http options'); + } + }); + + it('should NOT resolve {{LIBRECHAT_USER_*}} when dbSourced is true', () => { + const user = createTestUser({ id: 'user-123', email: 'test@example.com' }); + const options: MCPOptions = { + type: 'streamable-http', + url: 'https://api.example.com', + headers: { + 'X-User-Id': '{{LIBRECHAT_USER_ID}}', + 'X-User-Email': '{{LIBRECHAT_USER_EMAIL}}', + }, + }; + + const result = processMCPEnv({ options, user, dbSourced: true }); + + if (isStreamableHTTPOptions(result)) { + expect(result.headers?.['X-User-Id']).toBe('{{LIBRECHAT_USER_ID}}'); + expect(result.headers?.['X-User-Email']).toBe('{{LIBRECHAT_USER_EMAIL}}'); + } else { + throw new Error('Expected streamable-http options'); + } + }); + + it('should NOT resolve {{LIBRECHAT_OPENID_*}} when dbSourced is true', () => { + const user = { + ...createTestUser({ id: 'user-123', provider: 'openid' }), + federatedTokens: { + access_token: 'oidc-access-token', + id_token: 'oidc-id-token', + refresh_token: 'oidc-refresh-token', + token_type: 'Bearer', + expires_at: Date.now() + 3600000, + }, + }; + const options: MCPOptions = { + type: 'streamable-http', + url: 'https://api.example.com', + headers: { + Authorization: 'Bearer {{LIBRECHAT_OPENID_ACCESS_TOKEN}}', + }, + }; + + const result = processMCPEnv({ options, user, dbSourced: true }); + + if (isStreamableHTTPOptions(result)) { + expect(result.headers?.Authorization).toBe('Bearer {{LIBRECHAT_OPENID_ACCESS_TOKEN}}'); + } else { + throw new Error('Expected streamable-http options'); + } + }); + + it('should NOT resolve {{LIBRECHAT_BODY_*}} when dbSourced is true', () => { + const body = { + conversationId: 'conv-123', + parentMessageId: 'parent-456', + messageId: 'msg-789', + }; + const options: MCPOptions = { + type: 'streamable-http', + url: 'https://api.example.com', + headers: { + 'X-Conversation': '{{LIBRECHAT_BODY_CONVERSATIONID}}', + }, + }; + + const result = processMCPEnv({ options, body, dbSourced: true }); + + if (isStreamableHTTPOptions(result)) { + expect(result.headers?.['X-Conversation']).toBe('{{LIBRECHAT_BODY_CONVERSATIONID}}'); + } else { + throw new Error('Expected streamable-http options'); + } + }); + + it('should resolve customUserVars but block all other placeholders when dbSourced is true', () => { + const user = createTestUser({ id: 'user-123' }); + const body = { conversationId: 'conv-123', parentMessageId: 'p-1', messageId: 'm-1' }; + const options: MCPOptions = { + type: 'streamable-http', + url: '${DATABASE_URL}', + headers: { + Authorization: 'Bearer {{MCP_API_KEY}}', + 'X-Env-Leak': '${TEST_API_KEY}', + 'X-User-Id': '{{LIBRECHAT_USER_ID}}', + 'X-Body': '{{LIBRECHAT_BODY_CONVERSATIONID}}', + }, + }; + + const result = processMCPEnv({ + options, + user, + body, + dbSourced: true, + customUserVars: { MCP_API_KEY: 'user-key-value' }, + }); + + if (isStreamableHTTPOptions(result)) { + expect(result.headers?.Authorization).toBe('Bearer user-key-value'); + expect(result.headers?.['X-Env-Leak']).toBe('${TEST_API_KEY}'); + expect(result.headers?.['X-User-Id']).toBe('{{LIBRECHAT_USER_ID}}'); + expect(result.headers?.['X-Body']).toBe('{{LIBRECHAT_BODY_CONVERSATIONID}}'); + expect(result.url).toBe('${DATABASE_URL}'); + } else { + throw new Error('Expected streamable-http options'); + } + }); + + it('should resolve all placeholders when dbSourced is false (default)', () => { + const user = createTestUser({ id: 'user-123' }); + const options: MCPOptions = { + type: 'streamable-http', + url: 'https://api.example.com', + headers: { + Authorization: 'Bearer {{MCP_API_KEY}}', + 'X-Env': '${TEST_API_KEY}', + 'X-User-Id': '{{LIBRECHAT_USER_ID}}', + }, + }; + + const result = processMCPEnv({ + options, + user, + dbSourced: false, + customUserVars: { MCP_API_KEY: 'user-key-value' }, + }); + + if (isStreamableHTTPOptions(result)) { + expect(result.headers?.Authorization).toBe('Bearer user-key-value'); + expect(result.headers?.['X-Env']).toBe('test-api-key-value'); + expect(result.headers?.['X-User-Id']).toBe('user-123'); + } else { + throw new Error('Expected streamable-http options'); + } + }); + + it('should apply dbSourced to env, args, and URL — not just headers', () => { + const options: MCPOptions = { + type: 'stdio', + command: 'mcp-server', + args: ['--key', '${TEST_API_KEY}', '--custom', '{{MY_VAR}}'], + env: { + SECRET: '${DATABASE_URL}', + CUSTOM: '{{MY_VAR}}', + }, + }; + + const result = processMCPEnv({ + options, + dbSourced: true, + customUserVars: { MY_VAR: 'resolved-value' }, + }); + + if (isStdioOptions(result)) { + expect(result.env?.SECRET).toBe('${DATABASE_URL}'); + expect(result.env?.CUSTOM).toBe('resolved-value'); + expect(result.args?.[1]).toBe('${TEST_API_KEY}'); + expect(result.args?.[3]).toBe('resolved-value'); + } else { + throw new Error('Expected stdio options'); + } + }); + + it('should still apply admin API key header injection when dbSourced is true', () => { + const options: MCPOptions = { + type: 'streamable-http', + url: 'https://api.example.com', + apiKey: { + source: 'admin', + authorization_type: 'bearer', + key: 'admin-managed-key', + }, + }; + + const result = processMCPEnv({ options, dbSourced: true }); + + if (isStreamableHTTPOptions(result)) { + expect(result.headers?.Authorization).toBe('Bearer admin-managed-key'); + } else { + throw new Error('Expected streamable-http options'); + } + }); + + it('should block env vars in OAuth config when dbSourced is true', () => { + const options: MCPOptions = { + type: 'streamable-http', + url: 'https://api.example.com', + oauth: { + client_id: '${TEST_API_KEY}', + client_secret: '${DATABASE_URL}', + token_url: 'https://auth.example.com/token', + token_exchange_method: TokenExchangeMethodEnum.DefaultPost, + }, + }; + + const result = processMCPEnv({ options, dbSourced: true }); + + const oauth = (result as { oauth?: Record }).oauth; + expect(oauth?.client_id).toBe('${TEST_API_KEY}'); + expect(oauth?.client_secret).toBe('${DATABASE_URL}'); + }); + + it('should resolve customUserVars in OAuth config when dbSourced is true', () => { + const options: MCPOptions = { + type: 'streamable-http', + url: 'https://api.example.com', + oauth: { + client_id: '{{MY_CLIENT_ID}}', + client_secret: '{{MY_CLIENT_SECRET}}', + token_url: 'https://auth.example.com/token', + token_exchange_method: TokenExchangeMethodEnum.DefaultPost, + }, + }; + + const result = processMCPEnv({ + options, + dbSourced: true, + customUserVars: { MY_CLIENT_ID: 'resolved-client', MY_CLIENT_SECRET: 'resolved-secret' }, + }); + + const oauth = (result as { oauth?: Record }).oauth; + expect(oauth?.client_id).toBe('resolved-client'); + expect(oauth?.client_secret).toBe('resolved-secret'); + }); + + it('should leave unresolved customUserVars as literal placeholders', () => { + const options: MCPOptions = { + type: 'streamable-http', + url: 'https://api.example.com', + headers: { + Authorization: 'Bearer {{MCP_API_KEY}}', + }, + }; + + // No customUserVars provided — placeholder should remain + const result = processMCPEnv({ options, dbSourced: true }); + + if (isStreamableHTTPOptions(result)) { + expect(result.headers?.Authorization).toBe('Bearer {{MCP_API_KEY}}'); + } else { + throw new Error('Expected streamable-http options'); + } + }); + + it('should not modify the original options when dbSourced is true', () => { + const options: MCPOptions = { + type: 'streamable-http', + url: '${DATABASE_URL}', + headers: { + Authorization: 'Bearer {{MCP_API_KEY}}', + 'X-Env': '${TEST_API_KEY}', + }, + }; + + const originalUrl = options.url; + const originalAuth = (options as { headers: Record }).headers.Authorization; + + processMCPEnv({ + options, + dbSourced: true, + customUserVars: { MCP_API_KEY: 'resolved' }, + }); + + expect(options.url).toBe(originalUrl); + expect((options as { headers: Record }).headers.Authorization).toBe( + originalAuth, + ); + }); + + it('should handle empty customUserVars object without errors', () => { + const options: MCPOptions = { + type: 'streamable-http', + url: 'https://api.example.com', + headers: { + 'X-Key': '${TEST_API_KEY}', + 'X-Custom': '{{MCP_API_KEY}}', + }, + }; + + const result = processMCPEnv({ options, dbSourced: true, customUserVars: {} }); + + if (isStreamableHTTPOptions(result)) { + expect(result.headers?.['X-Key']).toBe('${TEST_API_KEY}'); + expect(result.headers?.['X-Custom']).toBe('{{MCP_API_KEY}}'); + } else { + throw new Error('Expected streamable-http options'); + } + }); + + it('dbSourced undefined should behave like false (resolve everything)', () => { + const user = createTestUser({ id: 'user-abc' }); + const options: MCPOptions = { + type: 'streamable-http', + url: 'https://api.example.com', + headers: { + 'X-Env': '${TEST_API_KEY}', + 'X-User': '{{LIBRECHAT_USER_ID}}', + }, + }; + + const result = processMCPEnv({ options, user }); + + if (isStreamableHTTPOptions(result)) { + expect(result.headers?.['X-Env']).toBe('test-api-key-value'); + expect(result.headers?.['X-User']).toBe('user-abc'); + } else { + throw new Error('Expected streamable-http options'); + } + }); + }); }); diff --git a/packages/api/src/utils/env.ts b/packages/api/src/utils/env.ts index f4fd2b1c78..78d6f9ebdf 100644 --- a/packages/api/src/utils/env.ts +++ b/packages/api/src/utils/env.ts @@ -209,12 +209,15 @@ function processSingleValue({ user, body = undefined, isHeader = false, + dbSourced = false, }: { originalValue: string; customUserVars?: Record; user?: Partial; body?: RequestBody; isHeader?: boolean; + /** When true, only resolve customUserVars — skip env vars, user/OpenID/body placeholders */ + dbSourced?: boolean; }): string { // Type guard: ensure we're working with a string if (typeof originalValue !== 'string') { @@ -232,6 +235,10 @@ function processSingleValue({ } } + if (dbSourced) { + return value; + } + value = processUserPlaceholders(value, user, isHeader); const openidTokenInfo = extractOpenIDTokenInfo(user); @@ -258,10 +265,12 @@ function processSingleValue({ * @returns - The processed object with environment variables replaced */ export function processMCPEnv(params: { - options: Readonly; + options: Readonly & { dbId?: string }; user?: Partial; customUserVars?: Record; body?: RequestBody; + /** When true, only resolve customUserVars — skip env vars, user/OpenID/body placeholders (for DB-stored servers) */ + dbSourced?: boolean; }): MCPOptions { const { options, user, customUserVars, body } = params; @@ -269,6 +278,9 @@ export function processMCPEnv(params: { return options; } + /** Derive dbSourced from explicit param OR from dbId on the options (failsafe for callers that forget the flag) */ + const dbSourced = params.dbSourced ?? !!options.dbId; + const newObj: MCPOptions = structuredClone(options); // Apply admin-provided API key to headers at runtime @@ -306,7 +318,13 @@ export function processMCPEnv(params: { if ('env' in newObj && newObj.env) { const processedEnv: Record = {}; for (const [key, originalValue] of Object.entries(newObj.env)) { - processedEnv[key] = processSingleValue({ originalValue, customUserVars, user, body }); + processedEnv[key] = processSingleValue({ + user, + body, + dbSourced, + originalValue, + customUserVars, + }); } newObj.env = processedEnv; } @@ -314,7 +332,9 @@ export function processMCPEnv(params: { if ('args' in newObj && newObj.args) { const processedArgs: string[] = []; for (const originalValue of newObj.args) { - processedArgs.push(processSingleValue({ originalValue, customUserVars, user, body })); + processedArgs.push( + processSingleValue({ originalValue, customUserVars, user, body, dbSourced }), + ); } newObj.args = processedArgs; } @@ -325,10 +345,11 @@ export function processMCPEnv(params: { const processedHeaders: Record = {}; for (const [key, originalValue] of Object.entries(newObj.headers)) { processedHeaders[key] = processSingleValue({ - originalValue, - customUserVars, user, body, + dbSourced, + originalValue, + customUserVars, isHeader: true, // Important: Enable header encoding }); } @@ -337,7 +358,13 @@ export function processMCPEnv(params: { // Process URL if it exists (for WebSocket, SSE, StreamableHTTP types) if ('url' in newObj && newObj.url) { - newObj.url = processSingleValue({ originalValue: newObj.url, customUserVars, user, body }); + newObj.url = processSingleValue({ + user, + body, + dbSourced, + customUserVars, + originalValue: newObj.url, + }); } // Process OAuth configuration if it exists (for all transport types) @@ -347,7 +374,13 @@ export function processMCPEnv(params: { // Only process string values for environment variables // token_exchange_method is an enum and shouldn't be processed if (typeof originalValue === 'string') { - processedOAuth[key] = processSingleValue({ originalValue, customUserVars, user, body }); + processedOAuth[key] = processSingleValue({ + user, + body, + dbSourced, + originalValue, + customUserVars, + }); } else { processedOAuth[key] = originalValue; }