From 7eafe317ccd2f5644819e41ab0f52722ba9639fd Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Tue, 9 Jun 2026 18:52:57 -0400 Subject: [PATCH] =?UTF-8?q?=F0=9F=97=9D=EF=B8=8F=20fix:=20Resolve=20MCP=20?= =?UTF-8?q?Runtime=20User=20and=20Request=20Placeholders=20(#13626)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: Resolve MCP Runtime User Placeholders * fix: Harden MCP Runtime Placeholder Connections * fix: Update MCP Source Tag Test Expectations * fix: Complete MCP Runtime Placeholder Reinit * fix: Harden MCP Request Scoped Runtime Configs * fix: Align MCP OAuth Tests With Domain Policy * fix: Harden MCP Runtime Resolution Edges * fix: Avoid MCP Runtime Reprocessing Pitfalls * fix: Reuse MCP Request Scoped Tool Discovery * fix: Validate MCP Body Runtime Fields * 🛡️ refactor: Harden runtime placeholder edges from review - Warn at inspection when a trusted server URL contains runtime placeholders but no domain allowlist restricts the resolved target - Document the three resolution sites that must stay in sync so the validated config always matches the connected one - Note the per-call connect cost of ephemeral GRAPH/BODY connections - Drop the no-op removeUserConnection in callTool's ephemeral cleanup; ephemeral connections are never stored, and removing the entry could orphan a still-connected cached connection after a config change * 🪪 fix: Cover oauth_headers, Graph URL gating, and request-scoped reconnects Address Codex review: - Resolve runtime placeholders in oauth_headers (processMCPEnv + Graph pre-pass) and include the field in placeholder detection, so OAuth discovery/token requests no longer send literals; consolidate the detection field lists into one helper - Defer the early domain gate when the URL still carries a Graph placeholder (resolved async later); the authoritative assertResolvedRuntimeConfigAllowed check still enforces policy - Bypass the 10s reconnect throttle for request-scoped servers, which re-fetch tool definitions on every message by design --- api/app/clients/tools/util/handleTools.js | 9 +- .../clients/tools/util/handleTools.test.js | 116 ++++ api/server/services/MCP.js | 114 +++- api/server/services/MCP.spec.js | 245 ++++++++ api/server/services/ToolService.js | 7 +- api/server/services/Tools/mcp.js | 28 +- api/server/services/Tools/mcp.spec.js | 84 ++- .../services/__tests__/ToolService.spec.js | 40 ++ packages/api/src/mcp/MCPConnectionFactory.ts | 30 +- packages/api/src/mcp/MCPManager.ts | 78 ++- packages/api/src/mcp/UserConnectionManager.ts | 231 +++++++- .../__tests__/MCPConnectionFactory.test.ts | 52 ++ .../api/src/mcp/__tests__/MCPManager.test.ts | 522 +++++++++++++++++- .../__tests__/MCPOAuthRaceCondition.test.ts | 4 + packages/api/src/mcp/__tests__/mcp.spec.ts | 38 ++ packages/api/src/mcp/__tests__/utils.test.ts | 232 +++++++- .../src/mcp/registry/MCPServerInspector.ts | 27 +- .../src/mcp/registry/MCPServersRegistry.ts | 15 +- .../__tests__/MCPServerInspector.test.ts | 46 ++ .../__tests__/MCPServersInitializer.test.ts | 14 +- .../__tests__/MCPServersRegistry.test.ts | 37 +- .../__tests__/ensureConfigServers.test.ts | 4 +- packages/api/src/mcp/tools.spec.ts | 27 +- packages/api/src/mcp/tools.ts | 11 +- packages/api/src/mcp/types/index.ts | 4 + packages/api/src/mcp/utils.ts | 167 +++++- packages/api/src/utils/env.spec.ts | 36 ++ packages/api/src/utils/env.ts | 16 + packages/api/src/utils/graph.ts | 86 ++- 29 files changed, 2235 insertions(+), 85 deletions(-) diff --git a/api/app/clients/tools/util/handleTools.js b/api/app/clients/tools/util/handleTools.js index 45623f9a9e..3698ce49c2 100644 --- a/api/app/clients/tools/util/handleTools.js +++ b/api/app/clients/tools/util/handleTools.js @@ -9,6 +9,7 @@ const { getCodeApiAuthHeaders, buildImageToolContext, buildWebSearchContext, + requiresEphemeralUserConnection, buildWebSearchDynamicContext, } = require('@librechat/api'); const { @@ -468,6 +469,7 @@ const loadTools = async ({ user: safeUser, userMCPAuthMap, configServers, + requestBody: options.req?.body, res: options.res, streamId: options.req?._resumableStreamId || null, model: agent?.model ?? model, @@ -488,7 +490,9 @@ const loadTools = async ({ } if (!availableTools) { try { - availableTools = await getMCPServerTools(safeUser.id, serverName); + availableTools = requiresEphemeralUserConnection(config.config) + ? null + : await getMCPServerTools(safeUser.id, serverName); } catch (error) { logger.error(`Error fetching available tools for MCP server ${serverName}:`, error); } @@ -502,6 +506,9 @@ const loadTools = async ({ ...mcpParams, availableTools, toolKey: config.toolKey, + onAvailableTools: (tools) => { + availableTools = tools; + }, }); if (Array.isArray(mcpTool)) { diff --git a/api/app/clients/tools/util/handleTools.test.js b/api/app/clients/tools/util/handleTools.test.js index 1adda45c35..3889ae9622 100644 --- a/api/app/clients/tools/util/handleTools.test.js +++ b/api/app/clients/tools/util/handleTools.test.js @@ -6,6 +6,10 @@ const mockPluginService = { deleteUserPluginAuth: jest.fn(), getUserPluginAuthValue: jest.fn(), }; +const mockGetMCPServerTools = jest.fn(); +const mockCreateMCPTool = jest.fn(); +const mockCreateMCPTools = jest.fn(); +const mockGetServerConfig = jest.fn(); jest.mock('~/server/services/PluginService', () => mockPluginService); @@ -28,9 +32,26 @@ jest.mock('~/server/services/Config', () => ({ }, }, }), + getMCPServerTools: (...args) => mockGetMCPServerTools(...args), +})); + +jest.mock('~/server/services/MCP', () => ({ + createMCPTool: (...args) => mockCreateMCPTool(...args), + createMCPTools: (...args) => mockCreateMCPTools(...args), + createMCPPermissionContext: jest.fn(() => ({ + canUseServers: jest.fn().mockResolvedValue(true), + })), + resolveConfigServers: jest.fn().mockResolvedValue({}), +})); + +jest.mock('~/config', () => ({ + getMCPServersRegistry: jest.fn(() => ({ + getServerConfig: (...args) => mockGetServerConfig(...args), + })), })); const { Calculator } = require('@librechat/agents'); +const { Constants } = require('librechat-data-provider'); const { User } = require('~/db/models'); const PluginService = require('~/server/services/PluginService'); @@ -282,5 +303,100 @@ describe('Tool Handlers', () => { expect(structuredTool).toBeInstanceOf(StructuredSD); delete process.env.SD_WEBUI_URL; }); + + it('passes request body to chat MCP tool creation and skips stale cache for BODY-scoped servers', async () => { + const serverName = 'body-scoped'; + const toolKey = `search${Constants.mcp_delimiter}${serverName}`; + const requestBody = { conversationId: 'conv-123', messageId: 'msg-123' }; + const serverConfig = { + type: 'streamable-http', + url: 'https://api.example.com/messages/{{LIBRECHAT_BODY_MESSAGEID}}/mcp', + source: 'yaml', + }; + + mockGetServerConfig.mockResolvedValue(serverConfig); + mockCreateMCPTool.mockResolvedValue({ name: 'loaded-mcp-tool' }); + + const result = await loadTools({ + user: fakeUser._id.toString(), + tools: [toolKey], + options: { + req: { + user: { id: fakeUser._id.toString(), role: 'USER' }, + body: requestBody, + }, + }, + }); + + expect(result.loadedTools).toEqual([{ name: 'loaded-mcp-tool' }]); + expect(mockGetMCPServerTools).not.toHaveBeenCalled(); + expect(mockCreateMCPTool).toHaveBeenCalledWith( + expect.objectContaining({ + requestBody, + toolKey, + config: serverConfig, + }), + ); + }); + + it('reuses discovered request-scoped MCP tool definitions within a server loop', async () => { + const serverName = 'body-scoped'; + const firstToolKey = `search${Constants.mcp_delimiter}${serverName}`; + const secondToolKey = `lookup${Constants.mcp_delimiter}${serverName}`; + const requestBody = { conversationId: 'conv-123', messageId: 'msg-123' }; + const serverConfig = { + type: 'streamable-http', + url: 'https://api.example.com/messages/{{LIBRECHAT_BODY_MESSAGEID}}/mcp', + source: 'yaml', + }; + const discoveredTools = { + [firstToolKey]: { + function: { + description: 'Search', + parameters: { type: 'object', properties: {} }, + }, + }, + [secondToolKey]: { + function: { + description: 'Lookup', + parameters: { type: 'object', properties: {} }, + }, + }, + }; + + mockGetServerConfig.mockResolvedValue(serverConfig); + mockCreateMCPTool + .mockImplementationOnce(async ({ onAvailableTools }) => { + onAvailableTools(discoveredTools); + return { name: 'search-tool' }; + }) + .mockImplementationOnce(async ({ availableTools }) => { + expect(availableTools).toBe(discoveredTools); + return { name: 'lookup-tool' }; + }); + + const result = await loadTools({ + user: fakeUser._id.toString(), + tools: [firstToolKey, secondToolKey], + options: { + req: { + user: { id: fakeUser._id.toString(), role: 'USER' }, + body: requestBody, + }, + }, + }); + + expect(result.loadedTools).toEqual([{ name: 'search-tool' }, { name: 'lookup-tool' }]); + expect(mockGetMCPServerTools).not.toHaveBeenCalled(); + expect(mockCreateMCPTool).toHaveBeenCalledTimes(2); + expect(mockCreateMCPTool).toHaveBeenNthCalledWith( + 2, + expect.objectContaining({ + availableTools: discoveredTools, + requestBody, + toolKey: secondToolKey, + }), + ); + }); }); }); diff --git a/api/server/services/MCP.js b/api/server/services/MCP.js index a837901bc0..2da3744f44 100644 --- a/api/server/services/MCP.js +++ b/api/server/services/MCP.js @@ -13,10 +13,14 @@ const { sanitizeGeminiSchema, buildMCPAuthStepId, buildMCPAuthToolCall, + processMCPEnv, buildMCPAuthRunStepEvent, buildMCPAuthRunStepDeltaEvent, buildMCPAuthRunStepEndDeltaEvent, + isUserSourced, checkAccessWithRequestCache, + requiresEphemeralUserConnection, + containsGraphTokenPlaceholder, } = require('@librechat/api'); const { Time, @@ -158,6 +162,41 @@ async function resolveAllMcpConfigs(userId, user) { return await registry.getAllServerConfigs(userId, configServers); } +function getServerCustomUserVars(userMCPAuthMap, serverName) { + return userMCPAuthMap?.[`${Constants.mcp_prefix}${serverName}`]; +} + +/** + * Best-effort early gate; the authoritative check is + * `assertResolvedRuntimeConfigAllowed` in `@librechat/api`, whose resolution + * this must mirror. Graph placeholders resolve later (async), so a URL still + * carrying one defers to the authoritative check instead of rejecting here. + */ +async function isEarlyDomainAllowed({ + serverConfig, + user, + requestBody, + userMCPAuthMap, + serverName, + allowedDomains, + allowedAddresses, +}) { + const validationConfig = processMCPEnv({ + user, + body: requestBody, + dbSourced: isUserSourced(serverConfig), + options: serverConfig, + customUserVars: getServerCustomUserVars(userMCPAuthMap, serverName), + }); + if ( + typeof validationConfig?.url === 'string' && + containsGraphTokenPlaceholder(validationConfig.url) + ) { + return true; + } + return await isMCPDomainAllowed(validationConfig, allowedDomains, allowedAddresses); +} + /** * @param {string} toolName * @param {string} serverName @@ -340,6 +379,7 @@ function createOAuthCallback({ runStepEmitter, runStepDeltaEmitter }) { * @param {number} [params.index] * @param {string | null} [params.streamId] - The stream ID for resumable mode. * @param {Record>} [params.userMCPAuthMap] + * @param {import('@librechat/api').ParsedServerConfig} [params.serverConfig] - Used to bypass reconnect throttling for request-scoped servers. * @returns { Promise unknown}>> } An object with `_call` method to execute the tool input. */ async function reconnectServer({ @@ -348,23 +388,30 @@ async function reconnectServer({ index, signal, serverName, + serverConfig, configServers, userMCPAuthMap, + requestBody, streamId = null, }) { logger.debug( `[MCP][reconnectServer] serverName: ${serverName}, user: ${user?.id}, hasUserMCPAuthMap: ${!!userMCPAuthMap}`, ); - const throttleKey = `${user.id}:${serverName}`; - const now = Date.now(); - const lastAttempt = lastReconnectAttempts.get(throttleKey) ?? 0; - if (now - lastAttempt < RECONNECT_THROTTLE_MS) { - logger.debug(`[MCP][reconnectServer] Throttled reconnect for ${serverName}`); - return null; + // Request-scoped servers reconnect on every message by design; throttling them + // would stub out healthy tools for messages sent within the throttle window. + const requestScoped = serverConfig ? requiresEphemeralUserConnection(serverConfig) : false; + if (!requestScoped) { + const throttleKey = `${user.id}:${serverName}`; + const now = Date.now(); + const lastAttempt = lastReconnectAttempts.get(throttleKey) ?? 0; + if (now - lastAttempt < RECONNECT_THROTTLE_MS) { + logger.debug(`[MCP][reconnectServer] Throttled reconnect for ${serverName}`); + return null; + } + lastReconnectAttempts.set(throttleKey, now); + evictStale(lastReconnectAttempts, RECONNECT_THROTTLE_MS); } - lastReconnectAttempts.set(throttleKey, now); - evictStale(lastReconnectAttempts, RECONNECT_THROTTLE_MS); const runId = Constants.USE_PRELIM_RESPONSE_MESSAGE_ID; const flowId = `${user.id}:${serverName}:${Date.now()}`; @@ -420,6 +467,7 @@ async function reconnectServer({ oauthStart, flowManager, userMCPAuthMap, + requestBody, forceNew: true, returnOnOAuth: false, connectionTimeout: Time.THIRTY_SECONDS, @@ -449,6 +497,7 @@ async function reconnectServer({ * @param {AbortSignal} [params.signal] * @param {string | null} [params.streamId] - The stream ID for resumable mode. * @param {import('@librechat/api').ParsedServerConfig} [params.config] + * @param {import('@librechat/api').RequestBody} [params.requestBody] * @param {Record>} [params.userMCPAuthMap] * @returns { Promise unknown}>> } An object with `_call` method to execute the tool input. */ @@ -463,10 +512,12 @@ async function createMCPTools({ serverName, configServers, userMCPAuthMap, + requestBody, streamId = null, }) { const serverConfig = config ?? (await getMCPServersRegistry().getServerConfig(serverName, user?.id, configServers)); + if (serverConfig?.url) { const appConfig = await getAppConfig({ role: user?.role, @@ -475,11 +526,15 @@ async function createMCPTools({ }); const allowedDomains = appConfig?.mcpSettings?.allowedDomains; const allowedAddresses = appConfig?.mcpSettings?.allowedAddresses; - const isDomainAllowed = await isMCPDomainAllowed( + const isDomainAllowed = await isEarlyDomainAllowed({ serverConfig, + user, + requestBody, + userMCPAuthMap, + serverName, allowedDomains, allowedAddresses, - ); + }); if (!isDomainAllowed) { logger.warn(`[MCP][${serverName}] Domain not allowed, skipping all tools`); return []; @@ -492,8 +547,10 @@ async function createMCPTools({ index, signal, serverName, + serverConfig, configServers, userMCPAuthMap, + requestBody, streamId, }); if (result === null) { @@ -517,6 +574,7 @@ async function createMCPTools({ streamId, availableTools: result.availableTools, toolKey: `${tool.name}${Constants.mcp_delimiter}${serverName}`, + requestBody, config: serverConfig, }); if (toolInstance) { @@ -540,8 +598,10 @@ async function createMCPTools({ * @param {string | null} [params.streamId] - The stream ID for resumable mode. * @param {Providers | EModelEndpoint} params.provider - The provider for the tool. * @param {LCAvailableTools} [params.availableTools] + * @param {import('@librechat/api').RequestBody} [params.requestBody] * @param {Record>} [params.userMCPAuthMap] * @param {import('@librechat/api').ParsedServerConfig} [params.config] + * @param {(availableTools: LCAvailableTools) => void} [params.onAvailableTools] * @returns { Promise unknown}> } An object with `_call` method to execute the tool input. */ async function createMCPTool({ @@ -554,14 +614,19 @@ async function createMCPTool({ provider, userMCPAuthMap, availableTools, + requestBody, config, configServers, + onAvailableTools, streamId = null, }) { const [toolName, serverName] = toolKey.split(Constants.mcp_delimiter); const serverConfig = config ?? (await getMCPServersRegistry().getServerConfig(serverName, user?.id, configServers)); + const requestScopedTools = serverConfig ? requiresEphemeralUserConnection(serverConfig) : false; + const useMissingToolCache = !requestScopedTools; + if (serverConfig?.url) { const appConfig = await getAppConfig({ role: user?.role, @@ -570,11 +635,15 @@ async function createMCPTool({ }); const allowedDomains = appConfig?.mcpSettings?.allowedDomains; const allowedAddresses = appConfig?.mcpSettings?.allowedAddresses; - const isDomainAllowed = await isMCPDomainAllowed( + const isDomainAllowed = await isEarlyDomainAllowed({ serverConfig, + user, + requestBody, + userMCPAuthMap, + serverName, allowedDomains, allowedAddresses, - ); + }); if (!isDomainAllowed) { logger.warn(`[MCP][${serverName}] Domain no longer allowed, skipping tool: ${toolName}`); return undefined; @@ -584,7 +653,7 @@ async function createMCPTool({ /** @type {LCTool | undefined} */ let toolDefinition = availableTools?.[toolKey]?.function; if (!toolDefinition) { - const cachedAt = missingToolCache.get(toolKey); + const cachedAt = useMissingToolCache ? missingToolCache.get(toolKey) : undefined; if (cachedAt && Date.now() - cachedAt < MISSING_TOOL_TTL_MS) { logger.debug( `[MCP][${serverName}][${toolName}] Tool in negative cache, returning unavailable stub.`, @@ -601,13 +670,18 @@ async function createMCPTool({ index, signal, serverName, + serverConfig, configServers, userMCPAuthMap, + requestBody, streamId, }); + if (result?.availableTools) { + onAvailableTools?.(result.availableTools); + } toolDefinition = result?.availableTools?.[toolKey]?.function; - if (!toolDefinition) { + if (!toolDefinition && useMissingToolCache) { missingToolCache.set(toolKey, Date.now()); evictStale(missingToolCache, MISSING_TOOL_TTL_MS); } @@ -624,6 +698,7 @@ async function createMCPTool({ res, mcpPermissionContext, user, + requestBody, provider, toolName, serverName, @@ -637,6 +712,7 @@ function createToolInstance({ res, mcpPermissionContext, user: capturedUser = null, + requestBody: capturedRequestBody, toolName, serverName, serverConfig: capturedServerConfig, @@ -670,9 +746,9 @@ function createToolInstance({ /** @type {(toolArguments: Object | string, config?: GraphRunnableConfig) => Promise} */ const _call = async (toolArguments, config) => { - const permissionUser = config?.configurable?.user ?? capturedUser; - const userId = - config?.configurable?.user?.id || config?.configurable?.user_id || capturedUser?.id; + const effectiveUser = config?.configurable?.user ?? capturedUser; + const permissionUser = effectiveUser; + const userId = effectiveUser?.id || config?.configurable?.user_id || capturedUser?.id; /** @type {ReturnType} */ let abortHandler = null; /** @type {AbortSignal} */ @@ -728,8 +804,8 @@ function createToolInstance({ options: { signal: derivedSignal, }, - user: config?.configurable?.user, - requestBody: config?.configurable?.requestBody, + user: effectiveUser, + requestBody: config?.configurable?.requestBody ?? capturedRequestBody, customUserVars, flowManager, tokenMethods: { diff --git a/api/server/services/MCP.spec.js b/api/server/services/MCP.spec.js index 5ac6f825d6..564aca74b5 100644 --- a/api/server/services/MCP.spec.js +++ b/api/server/services/MCP.spec.js @@ -873,6 +873,37 @@ describe('User parameter passing tests', () => { expect(mockReinitMCPServer.mock.calls[0][0].user).toBe(mockUser); }); + it('should report available tools discovered during single tool reinit', async () => { + const mockUser = { id: 'user-discovery-callback', role: 'USER' }; + const mockRes = { write: jest.fn(), flush: jest.fn() }; + const onAvailableTools = jest.fn(); + const discoveredTools = { + [`my-tool${D}my-server`]: { + function: { description: 'My Tool', parameters: {} }, + }, + [`other-tool${D}my-server`]: { + function: { description: 'Other Tool', parameters: {} }, + }, + }; + + mockReinitMCPServer.mockResolvedValue({ + availableTools: discoveredTools, + }); + + const result = await createMCPTool({ + res: mockRes, + user: mockUser, + toolKey: `my-tool${D}my-server`, + provider: 'openai', + userMCPAuthMap: {}, + availableTools: undefined, + onAvailableTools, + }); + + expect(result).toBeDefined(); + expect(onAvailableTools).toHaveBeenCalledWith(discoveredTools); + }); + it('should not call reinitMCPServer when tool is in cache', async () => { const mockUser = { id: 'test-user-789' }; const mockRes = { write: jest.fn(), flush: jest.fn() }; @@ -1017,6 +1048,126 @@ describe('User parameter passing tests', () => { expect(getRoleByName).toHaveBeenCalledTimes(1); expect(mockCallTool).toHaveBeenCalledTimes(2); }); + + it('should pass the captured user to MCPManager.callTool when invocation config omits configurable.user', async () => { + const mockUser = { id: 'captured-user', email: 'captured@example.com', role: 'USER' }; + const mockRes = { write: jest.fn(), flush: jest.fn() }; + const { getRoleByName } = require('~/models'); + getRoleByName.mockResolvedValue({ + permissions: { + [PermissionTypes.MCP_SERVERS]: { + [Permissions.USE]: true, + }, + }, + }); + + const mockCallTool = jest.fn().mockResolvedValue(['ok', null]); + mockGetMCPManager.mockReturnValue({ + callTool: mockCallTool, + }); + + const mcpTool = await createMCPTool({ + res: mockRes, + user: mockUser, + toolKey: `test-tool${D}test-server`, + provider: 'openai', + userMCPAuthMap: {}, + availableTools: { + [`test-tool${D}test-server`]: { + function: { + description: 'Cached tool', + parameters: { type: 'object', properties: {} }, + }, + }, + }, + }); + + await expect( + mcpTool.invoke( + {}, + { + configurable: { + user_id: mockUser.id, + }, + metadata: { + provider: 'openai', + thread_id: 'thread-1', + run_id: 'run-1', + }, + toolCall: {}, + }, + ), + ).resolves.toBe('ok'); + + expect(mockCallTool).toHaveBeenCalledWith( + expect.objectContaining({ + serverName: 'test-server', + toolName: 'test-tool', + user: mockUser, + }), + ); + }); + + it('should pass captured request body when invocation config omits requestBody', async () => { + const mockUser = { id: 'captured-body-user', email: 'captured@example.com', role: 'USER' }; + const requestBody = { conversationId: 'conv-123', messageId: 'msg-123' }; + const mockRes = { write: jest.fn(), flush: jest.fn() }; + const { getRoleByName } = require('~/models'); + getRoleByName.mockResolvedValue({ + permissions: { + [PermissionTypes.MCP_SERVERS]: { + [Permissions.USE]: true, + }, + }, + }); + + const mockCallTool = jest.fn().mockResolvedValue(['ok', null]); + mockGetMCPManager.mockReturnValue({ + callTool: mockCallTool, + }); + + const mcpTool = await createMCPTool({ + res: mockRes, + user: mockUser, + requestBody, + toolKey: `test-tool${D}test-server`, + provider: 'openai', + userMCPAuthMap: {}, + availableTools: { + [`test-tool${D}test-server`]: { + function: { + description: 'Cached tool', + parameters: { type: 'object', properties: {} }, + }, + }, + }, + }); + + await expect( + mcpTool.invoke( + {}, + { + configurable: { + user: mockUser, + }, + metadata: { + provider: 'openai', + thread_id: 'thread-1', + run_id: 'run-1', + }, + toolCall: {}, + }, + ), + ).resolves.toBe('ok'); + + expect(mockCallTool).toHaveBeenCalledWith( + expect.objectContaining({ + serverName: 'test-server', + toolName: 'test-tool', + requestBody, + }), + ); + }); }); describe('reinitMCPServer (via reconnectServer)', () => { @@ -1187,6 +1338,50 @@ describe('User parameter passing tests', () => { }); }); + it('should validate the resolved runtime URL for tool creation', async () => { + const mockUser = { id: 'runtime-domain-user', role: 'user' }; + const requestBody = { conversationId: 'tenant-a' }; + const mockRes = { write: jest.fn(), flush: jest.fn() }; + + mockRegistryInstance.getServerConfig.mockResolvedValue({ + type: 'streamable-http', + url: 'https://{{LIBRECHAT_BODY_CONVERSATIONID}}.example.com/sse', + source: 'yaml', + }); + + mockGetAppConfig.mockResolvedValue({ + mcpSettings: { allowedDomains: ['*.example.com'] }, + }); + + mockIsMCPDomainAllowed.mockResolvedValueOnce(true); + + const result = await createMCPTool({ + res: mockRes, + user: mockUser, + requestBody, + toolKey: `test-tool${D}test-server`, + provider: 'openai', + userMCPAuthMap: {}, + availableTools: { + [`test-tool${D}test-server`]: { + function: { + description: 'Test tool', + parameters: { type: 'object', properties: {} }, + }, + }, + }, + }); + + expect(result).toBeDefined(); + expect(mockIsMCPDomainAllowed).toHaveBeenCalledWith( + expect.objectContaining({ + url: 'https://tenant-a.example.com/sse', + }), + ['*.example.com'], + undefined, + ); + }); + it('should skip domain validation for stdio transports (no URL)', async () => { const mockUser = { id: 'stdio-test-user' }; const mockRes = { write: jest.fn(), flush: jest.fn() }; @@ -1470,6 +1665,56 @@ describe('User parameter passing tests', () => { // Still only 1 real reconnect — user B was protected by the cache expect(mockReinitMCPServer).toHaveBeenCalledTimes(1); }); + + it('should bypass the negative cache for request-scoped tools', async () => { + const userA = { id: 'request-scoped-user-A' }; + const userB = { id: 'request-scoped-user-B' }; + const mockRes = { write: jest.fn(), flush: jest.fn() }; + const serverName = 'request-scoped-server'; + const toolKey = `tenant-tool${D}${serverName}`; + + mockRegistryInstance.getServerConfig.mockResolvedValue({ + type: 'streamable-http', + url: 'https://api.example.com/{{LIBRECHAT_BODY_MESSAGEID}}/mcp', + source: 'yaml', + }); + + mockReinitMCPServer + .mockResolvedValueOnce({ + availableTools: {}, + }) + .mockResolvedValueOnce({ + availableTools: { + [toolKey]: { + function: { description: 'Tenant tool', parameters: {} }, + }, + }, + }); + + await createMCPTool({ + res: mockRes, + user: userA, + requestBody: { messageId: 'message-a' }, + toolKey, + provider: 'openai', + userMCPAuthMap: {}, + availableTools: undefined, + }); + + const result = await createMCPTool({ + res: mockRes, + user: userB, + requestBody: { messageId: 'message-b' }, + toolKey, + provider: 'openai', + userMCPAuthMap: {}, + availableTools: undefined, + }); + + expect(result).toBeDefined(); + expect(result.name).toContain('tenant-tool'); + expect(mockReinitMCPServer).toHaveBeenCalledTimes(2); + }); }); describe('createMCPTools throttle handling', () => { diff --git a/api/server/services/ToolService.js b/api/server/services/ToolService.js index 06b963e783..36d9cd78fb 100644 --- a/api/server/services/ToolService.js +++ b/api/server/services/ToolService.js @@ -16,6 +16,7 @@ const { isActionDomainAllowed, buildWebSearchContext, buildImageToolContext, + requiresEphemeralUserConnection, buildToolClassification, getMissingCustomUserVars, buildWebSearchDynamicContext, @@ -743,7 +744,9 @@ async function loadToolDefinitionsWrapper({ req, res, agent, streamId = null, to return null; } - const cached = await getMCPServerTools(userId, serverName); + const cached = requiresEphemeralUserConnection(serverConfig) + ? null + : await getMCPServerTools(userId, serverName); if (cached) { await addPendingOAuthServer(); return cached; @@ -767,6 +770,7 @@ async function loadToolDefinitionsWrapper({ req, res, agent, streamId = null, to serverName, configServers, userMCPAuthMap, + requestBody: req.body, }); return result?.availableTools || null; @@ -885,6 +889,7 @@ async function loadToolDefinitionsWrapper({ req, res, agent, streamId = null, to configServers, userMCPAuthMap, flowManager, + requestBody: req.body, returnOnOAuth: false, oauthStart, oauthEnd: createOAuthEndEmitter(serverName), diff --git a/api/server/services/Tools/mcp.js b/api/server/services/Tools/mcp.js index 512641b7c0..d0b37a0b87 100644 --- a/api/server/services/Tools/mcp.js +++ b/api/server/services/Tools/mcp.js @@ -1,8 +1,9 @@ const { logger } = require('@librechat/data-schemas'); -const { getMissingCustomUserVars } = require('@librechat/api'); +const { getMissingCustomUserVars, requiresEphemeralUserConnection } = require('@librechat/api'); const { CacheKeys, Constants } = require('librechat-data-provider'); const { getMCPManager, getMCPServersRegistry, getFlowStateManager } = require('~/config'); const { findToken, createToken, updateToken, deleteTokens } = require('~/models'); +const { getGraphApiToken } = require('~/server/services/GraphTokenService'); const { exchangeOboToken } = require('~/server/services/OboTokenService'); const { createOboTrustChecker } = require('~/server/services/OboPolicyService'); const { updateMCPServerTools } = require('~/server/services/Config'); @@ -22,6 +23,7 @@ const { getLogStores } = require('~/cache'); * @param {FlowStateManager} [params.flowManager] * @param {(authURL: string, options?: { expiresAt?: number }) => Promise} [params.oauthStart] * @param {() => Promise} [params.oauthEnd] + * @param {import('@librechat/api').RequestBody} [params.requestBody] * @param {Record>} [params.userMCPAuthMap] */ async function reinitMCPServer({ @@ -36,21 +38,25 @@ async function reinitMCPServer({ oauthStart: _oauthStart, flowManager: _flowManager, serverConfig: providedConfig, + requestBody, oauthEnd, }) { /** @type {MCPConnection | null} */ let connection = null; + let serverConfig = providedConfig; /** @type {LCAvailableTools | null} */ let availableTools = null; /** @type {ReturnType | null} */ let tools = null; let oauthRequired = false; let oauthUrl = null; + let ephemeralServer = false; try { const registry = getMCPServersRegistry(); - const serverConfig = - providedConfig ?? (await registry.getServerConfig(serverName, user?.id, configServers)); + serverConfig = + serverConfig ?? (await registry.getServerConfig(serverName, user?.id, configServers)); + ephemeralServer = serverConfig ? requiresEphemeralUserConnection(serverConfig) : false; if (serverConfig?.inspectionFailed) { if (serverConfig.source === 'config') { logger.info( @@ -137,8 +143,10 @@ async function reinitMCPServer({ returnOnOAuth, oauthEnd, customUserVars, + requestBody, connectionTimeout, serverConfig, + graphTokenResolver: getGraphApiToken, oboTokenResolver: exchangeOboToken, oboTrustChecker: createOboTrustChecker(), }); @@ -172,8 +180,10 @@ async function reinitMCPServer({ tokenMethods, oauthStart, customUserVars, + requestBody, connectionTimeout, configServers, + graphTokenResolver: getGraphApiToken, oboTokenResolver: exchangeOboToken, oboTrustChecker: createOboTrustChecker(), }); @@ -206,6 +216,7 @@ async function reinitMCPServer({ userId: user.id, serverName, tools, + skipCache: ephemeralServer, }); } @@ -253,6 +264,17 @@ async function reinitMCPServer({ '[MCP Reinitialize] Error loading MCP Tools, servers may still be initializing:', error, ); + } finally { + if (connection && ephemeralServer) { + try { + await connection.disconnect(); + } catch (error) { + logger.warn( + `[MCP Reinitialize] Failed to disconnect ephemeral server ${serverName}`, + error, + ); + } + } } } diff --git a/api/server/services/Tools/mcp.spec.js b/api/server/services/Tools/mcp.spec.js index bce51aa29a..2dc92ebd9b 100644 --- a/api/server/services/Tools/mcp.spec.js +++ b/api/server/services/Tools/mcp.spec.js @@ -1,9 +1,15 @@ const { Constants } = require('librechat-data-provider'); const mockGetConnection = jest.fn(); +const mockDiscoverServerTools = jest.fn(); +const mockGetGraphApiToken = jest.fn(); +const mockUpdateMCPServerTools = jest.fn(); jest.mock('~/config', () => ({ - getMCPManager: jest.fn(() => ({ getConnection: mockGetConnection })), + getMCPManager: jest.fn(() => ({ + getConnection: mockGetConnection, + discoverServerTools: mockDiscoverServerTools, + })), getMCPServersRegistry: jest.fn(() => ({ getServerConfig: jest.fn() })), getFlowStateManager: jest.fn(() => ({})), })); @@ -14,7 +20,10 @@ jest.mock('~/models', () => ({ deleteTokens: jest.fn(), })); jest.mock('~/server/services/Config', () => ({ - updateMCPServerTools: jest.fn(), + updateMCPServerTools: mockUpdateMCPServerTools, +})); +jest.mock('~/server/services/GraphTokenService', () => ({ + getGraphApiToken: mockGetGraphApiToken, })); jest.mock('~/cache', () => ({ getLogStores: jest.fn(() => ({})), @@ -35,6 +44,7 @@ describe('reinitMCPServer — customUserVars gating (issue #10969)', () => { beforeEach(() => { jest.clearAllMocks(); + mockUpdateMCPServerTools.mockResolvedValue({}); }); it('does not connect and exposes no tools when a required customUserVar is unset', async () => { @@ -90,6 +100,76 @@ describe('reinitMCPServer — customUserVars gating (issue #10969)', () => { ); }); + it('passes request body and Graph resolver into connection creation', async () => { + mockGetConnection.mockResolvedValue({ fetchTools: jest.fn().mockResolvedValue([]) }); + const requestBody = { conversationId: 'conv-123', messageId: 'msg-123' }; + + await reinitMCPServer({ + user, + serverName, + serverConfig: { type: 'streamable-http', url: 'https://thingy.example.com/mcp' }, + requestBody, + userMCPAuthMap: undefined, + }); + + expect(mockGetConnection).toHaveBeenCalledWith( + expect.objectContaining({ + requestBody, + graphTokenResolver: mockGetGraphApiToken, + }), + ); + }); + + it('passes request body and Graph resolver into OAuth discovery fallback', async () => { + mockGetConnection.mockRejectedValue(new Error('OAuth authentication required')); + mockDiscoverServerTools.mockResolvedValue({ tools: [], oauthRequired: true, oauthUrl: null }); + const requestBody = { conversationId: 'conv-456', messageId: 'msg-456' }; + + await reinitMCPServer({ + user, + serverName, + serverConfig: { type: 'streamable-http', url: 'https://thingy.example.com/mcp' }, + requestBody, + userMCPAuthMap: undefined, + }); + + expect(mockDiscoverServerTools).toHaveBeenCalledWith( + expect.objectContaining({ + requestBody, + graphTokenResolver: mockGetGraphApiToken, + }), + ); + }); + + it('disconnects ephemeral BODY-scoped connections after loading tools', async () => { + const disconnect = jest.fn().mockResolvedValue(undefined); + const tools = [{ name: 'search', inputSchema: { type: 'object', properties: {} } }]; + mockGetConnection.mockResolvedValue({ + disconnect, + fetchTools: jest.fn().mockResolvedValue(tools), + }); + + await reinitMCPServer({ + user, + serverName, + serverConfig: { + type: 'streamable-http', + url: 'https://thingy.example.com/messages/{{LIBRECHAT_BODY_MESSAGEID}}/mcp', + source: 'yaml', + }, + requestBody: { messageId: 'msg-789' }, + userMCPAuthMap: undefined, + }); + + expect(disconnect).toHaveBeenCalledTimes(1); + expect(mockUpdateMCPServerTools).toHaveBeenCalledWith( + expect.objectContaining({ + tools, + skipCache: true, + }), + ); + }); + it('proceeds to connect when the server declares no customUserVars', async () => { mockGetConnection.mockResolvedValue({ fetchTools: jest.fn().mockResolvedValue([]) }); diff --git a/api/server/services/__tests__/ToolService.spec.js b/api/server/services/__tests__/ToolService.spec.js index 2718558938..bb6e4e8df1 100644 --- a/api/server/services/__tests__/ToolService.spec.js +++ b/api/server/services/__tests__/ToolService.spec.js @@ -719,6 +719,46 @@ describe('ToolService - Action Capability Gating', () => { ); }); + it('should pass request body context into MCP tool definition reinitialization', async () => { + const serverName = 'Body-Scoped'; + const mcpTool = `search${Constants.mcp_delimiter}${serverName}`; + const capabilities = [AgentCapabilities.tools]; + const req = createMockReq(capabilities); + req.body = { conversationId: 'conv-123', messageId: 'msg-123' }; + + mockGetEndpointsConfig.mockResolvedValue(createEndpointsConfig(capabilities)); + mockGetServerConfig.mockResolvedValue({ + type: 'streamable-http', + url: 'https://demo.librechat.ai/messages/{{LIBRECHAT_BODY_MESSAGEID}}/mcp', + source: 'yaml', + }); + mockGetMCPServerTools.mockResolvedValue(null); + mockFlowManager.getFlowState.mockResolvedValue(null); + mockLoadToolDefinitions.mockImplementation(async (params, deps) => { + await deps.getOrFetchMCPServerTools(params.userId, serverName); + return { + toolDefinitions: [], + toolRegistry: new Map(), + hasDeferredTools: false, + }; + }); + reinitMCPServer.mockResolvedValue({ availableTools: null }); + + await loadAgentTools({ + req, + agent: { id: 'agent_123', tools: [mcpTool] }, + definitionsOnly: true, + }); + + expect(reinitMCPServer).toHaveBeenCalledWith( + expect.objectContaining({ + serverName, + requestBody: req.body, + }), + ); + expect(mockGetMCPServerTools).not.toHaveBeenCalled(); + }); + it('should preserve pending-flow expiry for OAuth URLs captured during discovery', async () => { const serverName = 'Google-Workspace'; const authorizationUrl = 'https://auth.example.com/Google-Workspace'; diff --git a/packages/api/src/mcp/MCPConnectionFactory.ts b/packages/api/src/mcp/MCPConnectionFactory.ts index 3905ac1d8a..7669c451ba 100644 --- a/packages/api/src/mcp/MCPConnectionFactory.ts +++ b/packages/api/src/mcp/MCPConnectionFactory.ts @@ -15,6 +15,7 @@ import { } from '~/mcp/oauth'; import { sanitizeUrlForLogging, isClientRejectionMessage, isOAuthServer } from './utils'; import { PENDING_STALE_MS, normalizeExpiresAt } from '~/flow/manager'; +import { preProcessGraphTokens } from '~/utils/graph'; import { withTimeout } from '~/utils/promise'; import { MCPConnection } from './connection'; import { processMCPEnv } from '~/utils'; @@ -59,7 +60,7 @@ export class MCPConnectionFactory { basic: t.BasicConnectionOptions, oauth?: t.OAuthConnectionOptions | t.UserConnectionContext, ): Promise { - const factory = new this(basic, oauth); + const factory = new this(await this.prepareBasicConnectionOptions(basic, oauth), oauth); return factory.createConnection(); } @@ -72,14 +73,37 @@ export class MCPConnectionFactory { basic: t.BasicConnectionOptions, options?: Omit | t.UserConnectionContext, ): Promise { + const preparedBasic = await this.prepareBasicConnectionOptions(basic, options); if (options != null && 'useOAuth' in options) { - const factory = new this(basic, { ...options, returnOnOAuth: true }); + const factory = new this(preparedBasic, { ...options, returnOnOAuth: true }); return factory.discoverToolsInternal(); } - const factory = new this(basic, options); + const factory = new this(preparedBasic, options); return factory.discoverToolsInternal(); } + /** + * Together with the constructor's processMCPEnv pass, this mirrors + * UserConnectionManager.resolveRuntimeConfig — keep them in sync so the + * config validated there matches the one connected with here. + */ + private static async prepareBasicConnectionOptions( + basic: t.BasicConnectionOptions, + options?: t.OAuthConnectionOptions | t.UserConnectionContext, + ): Promise { + if (basic.dbSourced || !options?.graphTokenResolver) { + return basic; + } + + const serverConfig = await preProcessGraphTokens(basic.serverConfig, { + user: options.user, + graphTokenResolver: options.graphTokenResolver, + scopes: process.env.GRAPH_API_SCOPES, + }); + + return serverConfig === basic.serverConfig ? basic : { ...basic, serverConfig }; + } + protected async discoverToolsInternal(): Promise { const oauthUrl: string | null = null; let oauthRequired = false; diff --git a/packages/api/src/mcp/MCPManager.ts b/packages/api/src/mcp/MCPManager.ts index 3504af21c4..417bfa00a6 100644 --- a/packages/api/src/mcp/MCPManager.ts +++ b/packages/api/src/mcp/MCPManager.ts @@ -10,7 +10,13 @@ import type { FlowStateManager } from '~/flow/manager'; import type { MCPOAuthTokens } from './oauth'; import type { RequestBody } from '~/types'; import type * as t from './types'; -import { isUserSourced, requiresOAuthMachinery, requiresUserScopedConnection } from './utils'; +import { + getMissingRuntimeBodyPlaceholderFields, + isUserSourced, + requiresEphemeralUserConnection, + requiresOAuthMachinery, + requiresUserScopedConnection, +} from './utils'; import { MCPServersInitializer } from './registry/MCPServersInitializer'; import { OboTokenResolutionError, resolveOboToken } from '~/mcp/oauth'; import { MCPServerInspector } from './registry/MCPServerInspector'; @@ -138,12 +144,33 @@ export class MCPManager extends UserConnectionManager { return { tools: null, oauthRequired: false, oauthUrl: null }; } - const useOAuth = requiresOAuthMachinery(serverConfig); + const missingBodyFields = getMissingRuntimeBodyPlaceholderFields( + serverConfig, + args.requestBody, + ); + if (missingBodyFields.length > 0) { + logger.warn( + `${logPrefix} [Discovery] Request body field(s) required to resolve runtime MCP placeholders: ${missingBodyFields.join(', ')}`, + ); + return { tools: null, oauthRequired: false, oauthUrl: null }; + } const registry = MCPServersRegistry.getInstance(); const useSSRFProtection = registry.shouldEnableSSRFProtection(); const allowedDomains = registry.getAllowedDomains(); const allowedAddresses = registry.getAllowedAddresses(); + await this.assertResolvedRuntimeConfigAllowed({ + config: serverConfig, + user, + customUserVars: args.customUserVars, + requestBody: args.requestBody, + graphTokenResolver: args.graphTokenResolver, + allowedDomains, + allowedAddresses, + logPrefix: `${logPrefix} [Discovery]`, + }); + + const useOAuth = requiresOAuthMachinery(serverConfig); const dbSourced = isUserSourced(serverConfig); const basic: t.BasicConnectionOptions = { dbSourced, @@ -154,18 +181,32 @@ export class MCPManager extends UserConnectionManager { allowedAddresses, }; - if (!useOAuth) { - const result = await MCPConnectionFactory.discoverTools(basic, { - user: args.user, - customUserVars: args.customUserVars, - requestBody: args.requestBody, - connectionTimeout: args.connectionTimeout, - }); + const finalizeDiscoveryResult = async ( + result: Awaited>, + ): Promise => { + if (result.connection) { + try { + await result.connection.disconnect(); + } catch (error) { + logger.warn(`${logPrefix} [Discovery] Failed to disconnect discovery connection`, error); + } + } return { tools: result.tools, oauthRequired: result.oauthRequired, oauthUrl: result.oauthUrl, }; + }; + + if (!useOAuth) { + const result = await MCPConnectionFactory.discoverTools(basic, { + user: args.user, + customUserVars: args.customUserVars, + requestBody: args.requestBody, + graphTokenResolver: args.graphTokenResolver, + connectionTimeout: args.connectionTimeout, + }); + return finalizeDiscoveryResult(result); } if (!user || !args.flowManager) { @@ -182,12 +223,13 @@ export class MCPManager extends UserConnectionManager { oauthStart: args.oauthStart, customUserVars: args.customUserVars, requestBody: args.requestBody, + graphTokenResolver: args.graphTokenResolver, connectionTimeout: args.connectionTimeout, oboTokenResolver: args.oboTokenResolver, oboTrustChecker: args.oboTrustChecker, }); - return { tools: result.tools, oauthRequired: result.oauthRequired, oauthUrl: result.oauthUrl }; + return finalizeDiscoveryResult(result); } /** Returns all available tool functions from app-level connections */ @@ -335,6 +377,7 @@ Please follow these instructions when using tools from the respective MCP server }): Promise { /** User-specific connection */ let connection: MCPConnection | undefined; + let disconnectAfterCall = false; const userId = user?.id; const logPrefix = userId ? `[MCP][User: ${userId}][${serverName}]` : `[MCP][${serverName}]`; @@ -350,6 +393,7 @@ Please follow these instructions when using tools from the respective MCP server oauthEnd, oboTokenResolver, oboTrustChecker, + graphTokenResolver, signal: options?.signal, customUserVars, requestBody, @@ -374,6 +418,7 @@ Please follow these instructions when using tools from the respective MCP server ); } const isDbSourced = isUserSourced(rawConfig); + disconnectAfterCall = !!userId && requiresEphemeralUserConnection(rawConfig); /** Pre-process Graph token placeholders (async) before the synchronous processMCPEnv pass */ const graphProcessedConfig = isDbSourced @@ -462,6 +507,19 @@ Please follow these instructions when using tools from the respective MCP server logger.error(`${logPrefix}[${toolName}] Tool call failed`, error); // Rethrowing allows the caller (createMCPTool) to handle the final user message throw error; + } finally { + // Ephemeral connections are never stored in userConnections, so disconnecting + // is the only cleanup needed; removing the map entry here could orphan a + // still-connected cached connection from before a config change. + if (disconnectAfterCall && connection) { + try { + await connection.disconnect(); + } catch (disconnectError) { + logger.warn(`${logPrefix}[${toolName}] Failed to disconnect ephemeral connection`, { + error: disconnectError, + }); + } + } } } } diff --git a/packages/api/src/mcp/UserConnectionManager.ts b/packages/api/src/mcp/UserConnectionManager.ts index 634dddf849..2c801ef11b 100644 --- a/packages/api/src/mcp/UserConnectionManager.ts +++ b/packages/api/src/mcp/UserConnectionManager.ts @@ -3,13 +3,22 @@ import { ErrorCode, McpError } from '@modelcontextprotocol/sdk/types.js'; import type { MCPOAuthFlowMetadata } from '~/mcp/oauth'; import type { FlowState } from '~/flow/types'; import type * as t from './types'; +import { + getMissingRuntimeBodyPlaceholderFields, + hasRuntimeUrlPlaceholders, + isUserSourced, + requiresEphemeralUserConnection, + requiresOAuthMachinery, +} from './utils'; import { MCPServersRegistry } from '~/mcp/registry/MCPServersRegistry'; +import { detectOAuthRequirement, MCPOAuthHandler } from '~/mcp/oauth'; import { ConnectionsRepository } from '~/mcp/ConnectionsRepository'; import { MCPConnectionFactory } from '~/mcp/MCPConnectionFactory'; -import { isUserSourced, requiresOAuthMachinery } from './utils'; +import { preProcessGraphTokens } from '~/utils/graph'; +import { isMCPDomainAllowed } from '~/auth/domain'; import { PENDING_STALE_MS } from '~/flow/manager'; -import { MCPOAuthHandler } from '~/mcp/oauth'; import { MCPConnection } from './connection'; +import { processMCPEnv } from '~/utils/env'; import { mcpConfig } from './mcpConfig'; type PendingOAuthStart = { @@ -63,9 +72,25 @@ export abstract class UserConnectionManager { throw new McpError(ErrorCode.InvalidRequest, `[MCP] User object missing id property`); } + const config = + opts.serverConfig ?? + (await MCPServersRegistry.getInstance().getServerConfig(serverName, userId)); + const missingBodyFields = config + ? getMissingRuntimeBodyPlaceholderFields(config, opts.requestBody) + : []; + if (missingBodyFields.length > 0) { + throw new McpError( + ErrorCode.InvalidRequest, + `[MCP][User: ${userId}][${serverName}] Request body field(s) required to resolve runtime MCP placeholders: ${missingBodyFields.join(', ')}.`, + ); + } + const ephemeralConnection = config ? requiresEphemeralUserConnection(config) : false; + const forceNewConnection = forceNew || ephemeralConnection; + const clearCooldown = forceNew === true; + const lockKey = `${userId}:${serverName}`; - if (!forceNew) { + if (!forceNewConnection) { const pending = this.pendingConnections.get(lockKey); if (pending) { logger.debug(`[MCP][User: ${userId}][${serverName}] Joining in-flight connection attempt`); @@ -78,19 +103,26 @@ export abstract class UserConnectionManager { const connectionPromise = this.createUserConnectionInternal( { ...opts, + forceNew: forceNewConnection, + ephemeralConnection, + serverConfig: config, oauthStart: this.createPendingOAuthStart(serverName, userId, pendingOAuth), }, userId, + clearCooldown, ); - if (!forceNew) { + if (!forceNewConnection) { this.pendingConnections.set(lockKey, { promise: connectionPromise, oauth: pendingOAuth }); } try { return await connectionPromise; } finally { - if (!forceNew && this.pendingConnections.get(lockKey)?.promise === connectionPromise) { + if ( + !forceNewConnection && + this.pendingConnections.get(lockKey)?.promise === connectionPromise + ) { this.pendingConnections.delete(lockKey); } } @@ -276,9 +308,12 @@ export abstract class UserConnectionManager { signal, returnOnOAuth = false, connectionTimeout, + graphTokenResolver, + ephemeralConnection = false, serverConfig: providedConfig, }: t.UserMCPConnectionOptions, userId: string, + clearCooldown: boolean, ): Promise { if (await this.appConnections!.has(serverName)) { throw new McpError( @@ -293,7 +328,7 @@ export abstract class UserConnectionManager { const userServerMap = this.userConnections.get(userId); let connection = forceNew ? undefined : userServerMap?.get(serverName); - if (forceNew) { + if (clearCooldown) { MCPConnection.clearCooldown(serverName); } const now = Date.now(); @@ -344,17 +379,36 @@ export abstract class UserConnectionManager { logger.info(`[MCP][User: ${userId}][${serverName}] Establishing new connection`); try { + const runtimeConfig = await this.applyRuntimeOAuthDetection({ + config, + user, + customUserVars, + requestBody, + graphTokenResolver, + }); const registry = MCPServersRegistry.getInstance(); + const allowedDomains = registry.getAllowedDomains(); + const allowedAddresses = registry.getAllowedAddresses(); + await this.assertResolvedRuntimeConfigAllowed({ + config: runtimeConfig, + user, + customUserVars, + requestBody, + graphTokenResolver, + allowedDomains, + allowedAddresses, + logPrefix: `[MCP][User: ${userId}][${serverName}]`, + }); const basic: t.BasicConnectionOptions = { - serverConfig: config, + serverConfig: runtimeConfig, serverName: serverName, - dbSourced: isUserSourced(config), + dbSourced: isUserSourced(runtimeConfig), useSSRFProtection: registry.shouldEnableSSRFProtection(), - allowedDomains: registry.getAllowedDomains(), - allowedAddresses: registry.getAllowedAddresses(), + allowedDomains, + allowedAddresses, }; - const useOAuth = requiresOAuthMachinery(config); + const useOAuth = requiresOAuthMachinery(runtimeConfig); let connectionOptions: t.OAuthConnectionOptions | t.UserConnectionContext; if (useOAuth) { if (!flowManager) { @@ -375,6 +429,7 @@ export abstract class UserConnectionManager { oauthEnd: oauthEnd, oboTokenResolver: oboTokenResolver, oboTrustChecker: oboTrustChecker, + graphTokenResolver, returnOnOAuth: returnOnOAuth, requestBody: requestBody, connectionTimeout: connectionTimeout, @@ -384,6 +439,7 @@ export abstract class UserConnectionManager { user, customUserVars, requestBody, + graphTokenResolver, connectionTimeout, }; } @@ -394,10 +450,12 @@ export abstract class UserConnectionManager { throw new Error('Failed to establish connection after initialization attempt.'); } - if (!this.userConnections.has(userId)) { - this.userConnections.set(userId, new Map()); + if (!ephemeralConnection) { + if (!this.userConnections.has(userId)) { + this.userConnections.set(userId, new Map()); + } + this.userConnections.get(userId)?.set(serverName, connection); } - this.userConnections.get(userId)?.set(serverName, connection); logger.info(`[MCP][User: ${userId}][${serverName}] Connection successfully established`); // Update timestamp on creation @@ -418,6 +476,151 @@ export abstract class UserConnectionManager { } } + /** + * Mirrors the resolution MCPConnectionFactory performs internally + * (preProcessGraphTokens + processMCPEnv). Both must stay in sync so the + * config validated here matches the one the factory actually connects with. + */ + protected async resolveRuntimeConfig({ + config, + user, + customUserVars, + requestBody, + graphTokenResolver, + }: { + config: t.ParsedServerConfig; + user?: t.UserMCPConnectionOptions['user']; + customUserVars?: Record; + requestBody?: t.UserMCPConnectionOptions['requestBody']; + graphTokenResolver?: t.UserMCPConnectionOptions['graphTokenResolver']; + }): Promise { + const dbSourced = isUserSourced(config); + const graphProcessedConfig = dbSourced + ? config + : await preProcessGraphTokens(config, { + user, + graphTokenResolver, + scopes: process.env.GRAPH_API_SCOPES, + }); + + return processMCPEnv({ + user, + body: requestBody, + dbSourced, + options: graphProcessedConfig, + customUserVars, + }) as t.ParsedServerConfig; + } + + protected async assertResolvedRuntimeConfigAllowed({ + config, + user, + customUserVars, + requestBody, + graphTokenResolver, + allowedDomains, + allowedAddresses, + logPrefix, + }: { + config: t.ParsedServerConfig; + user?: t.UserMCPConnectionOptions['user']; + customUserVars?: Record; + requestBody?: t.UserMCPConnectionOptions['requestBody']; + graphTokenResolver?: t.UserMCPConnectionOptions['graphTokenResolver']; + allowedDomains?: string[] | null; + allowedAddresses?: string[] | null; + logPrefix: string; + }): Promise { + const resolvedConfig = await this.resolveRuntimeConfig({ + config, + user, + customUserVars, + requestBody, + graphTokenResolver, + }); + + if (!resolvedConfig.url) { + return resolvedConfig; + } + + if (hasRuntimeUrlPlaceholders(resolvedConfig)) { + throw new McpError( + ErrorCode.InvalidRequest, + `${logPrefix} Runtime URL still contains unresolved MCP placeholders after resolution.`, + ); + } + + const allowed = await isMCPDomainAllowed(resolvedConfig, allowedDomains, allowedAddresses); + if (!allowed) { + throw new McpError( + ErrorCode.InvalidRequest, + `${logPrefix} Resolved MCP server URL is not allowed by the configured domain policy.`, + ); + } + + return resolvedConfig; + } + + private async applyRuntimeOAuthDetection({ + config, + user, + customUserVars, + requestBody, + graphTokenResolver, + }: { + config: t.ParsedServerConfig; + user?: t.UserMCPConnectionOptions['user']; + customUserVars?: Record; + requestBody?: t.UserMCPConnectionOptions['requestBody']; + graphTokenResolver?: t.UserMCPConnectionOptions['graphTokenResolver']; + }): Promise { + if ( + config.requiresOAuth != null || + config.apiKey?.source === 'admin' || + !hasRuntimeUrlPlaceholders(config) + ) { + return config; + } + + const resolvedConfig = await this.resolveRuntimeConfig({ + config, + user, + customUserVars, + requestBody, + graphTokenResolver, + }); + + if (!resolvedConfig.url || hasRuntimeUrlPlaceholders(resolvedConfig)) { + logger.warn( + `[MCP][User: ${user?.id}][${config.url}] Runtime URL still contains placeholders after resolution; skipping OAuth detection`, + ); + return config; + } + + const registry = MCPServersRegistry.getInstance(); + const allowedDomains = registry.getAllowedDomains(); + const allowedAddresses = registry.getAllowedAddresses(); + const allowed = await isMCPDomainAllowed(resolvedConfig, allowedDomains, allowedAddresses); + if (!allowed) { + throw new McpError( + ErrorCode.InvalidRequest, + `[MCP][User: ${user?.id}][${config.url}] Resolved MCP server URL is not allowed by the configured domain policy.`, + ); + } + + const result = await detectOAuthRequirement( + resolvedConfig.url, + allowedDomains, + allowedAddresses, + ); + + return { + ...config, + requiresOAuth: result.requiresOAuth, + oauthMetadata: result.metadata, + }; + } + /** Returns all connections for a specific user */ public getUserConnections(userId: string): Map | undefined { return this.userConnections.get(userId); diff --git a/packages/api/src/mcp/__tests__/MCPConnectionFactory.test.ts b/packages/api/src/mcp/__tests__/MCPConnectionFactory.test.ts index 1fcff17cd9..f49bc9acda 100644 --- a/packages/api/src/mcp/__tests__/MCPConnectionFactory.test.ts +++ b/packages/api/src/mcp/__tests__/MCPConnectionFactory.test.ts @@ -5,12 +5,17 @@ import type { MCPOAuthTokens } from '~/mcp/oauth'; import type * as t from '~/mcp/types'; import { MCPConnectionFactory } from '~/mcp/MCPConnectionFactory'; import { MCPOAuthHandler, MCPTokenStorage } from '~/mcp/oauth'; +import { preProcessGraphTokens } from '~/utils/graph'; import { PENDING_STALE_MS } from '~/flow/manager'; import { MCPConnection } from '~/mcp/connection'; import { processMCPEnv } from '~/utils'; jest.mock('~/mcp/connection'); jest.mock('~/mcp/oauth'); +jest.mock('~/utils/graph', () => ({ + ...jest.requireActual('~/utils/graph'), + preProcessGraphTokens: jest.fn(async (options) => options), +})); jest.mock('~/utils'); jest.mock('@librechat/data-schemas', () => ({ logger: { @@ -24,6 +29,9 @@ jest.mock('@librechat/data-schemas', () => ({ const mockLogger = logger as jest.Mocked; const mockProcessMCPEnv = processMCPEnv as jest.MockedFunction; +const mockPreProcessGraphTokens = preProcessGraphTokens as jest.MockedFunction< + typeof preProcessGraphTokens +>; const mockMCPConnection = MCPConnection as jest.MockedClass; const mockMCPOAuthHandler = MCPOAuthHandler as jest.Mocked; const mockMCPTokenStorage = MCPTokenStorage as jest.Mocked; @@ -67,6 +75,7 @@ describe('MCPConnectionFactory', () => { } as unknown as jest.Mocked; mockMCPConnection.mockImplementation(() => mockConnectionInstance); + mockPreProcessGraphTokens.mockImplementation(async (options) => options); mockProcessMCPEnv.mockReturnValue(mockServerConfig); }); @@ -96,6 +105,49 @@ describe('MCPConnectionFactory', () => { expect(mockConnectionInstance.connect).toHaveBeenCalled(); }); + it('should pre-process Graph placeholders before connection config resolution', async () => { + const graphTokenResolver = jest.fn(); + const serverConfig: t.MCPOptions = { + type: 'streamable-http', + url: 'https://api.example.com/mcp?token={{LIBRECHAT_GRAPH_ACCESS_TOKEN}}', + }; + const graphProcessedConfig: t.MCPOptions = { + ...serverConfig, + url: 'https://api.example.com/mcp?token=resolved-graph-token', + }; + const basicOptions = { + serverName: 'test-server', + serverConfig, + }; + + mockPreProcessGraphTokens.mockResolvedValue(graphProcessedConfig); + mockProcessMCPEnv.mockReturnValue(graphProcessedConfig); + mockConnectionInstance.isConnected.mockResolvedValue(true); + + await MCPConnectionFactory.create(basicOptions, { + user: mockUser, + graphTokenResolver, + }); + + expect(mockPreProcessGraphTokens).toHaveBeenCalledWith( + serverConfig, + expect.objectContaining({ + user: mockUser, + graphTokenResolver, + }), + ); + expect(mockProcessMCPEnv).toHaveBeenCalledWith( + expect.objectContaining({ + options: graphProcessedConfig, + }), + ); + expect(mockMCPConnection).toHaveBeenCalledWith( + expect.objectContaining({ + serverConfig: graphProcessedConfig, + }), + ); + }); + it('should register fallback oauthRequired handler for non-OAuth connections', async () => { const basicOptions = { serverName: 'test-server', diff --git a/packages/api/src/mcp/__tests__/MCPManager.test.ts b/packages/api/src/mcp/__tests__/MCPManager.test.ts index b8792fb8ef..202785f13c 100644 --- a/packages/api/src/mcp/__tests__/MCPManager.test.ts +++ b/packages/api/src/mcp/__tests__/MCPManager.test.ts @@ -2,14 +2,16 @@ import { logger } from '@librechat/data-schemas'; import type { IUser } from '@librechat/data-schemas'; import type { GraphTokenResolver } from '~/utils/graph'; import type * as t from '~/mcp/types'; +import { OboTokenResolutionError, detectOAuthRequirement, resolveOboToken } from '~/mcp/oauth'; import { MCPServersInitializer } from '~/mcp/registry/MCPServersInitializer'; import { MCPServerInspector } from '~/mcp/registry/MCPServerInspector'; -import { MCPConnectionFactory } from '~/mcp/MCPConnectionFactory'; import { ConnectionsRepository } from '~/mcp/ConnectionsRepository'; +import { MCPConnectionFactory } from '~/mcp/MCPConnectionFactory'; +import { isMCPDomainAllowed } from '~/auth/domain'; import { MCPConnection } from '~/mcp/connection'; import { MCPManager } from '~/mcp/MCPManager'; -import { OboTokenResolutionError, resolveOboToken } from '~/mcp/oauth'; import * as graphUtils from '~/utils/graph'; +import { processMCPEnv } from '~/utils/env'; // Mock external dependencies jest.mock('@librechat/data-schemas', () => ({ @@ -28,6 +30,7 @@ jest.mock('~/utils/graph', () => ({ jest.mock('~/mcp/oauth', () => ({ ...jest.requireActual('~/mcp/oauth'), + detectOAuthRequirement: jest.fn(), resolveOboToken: jest.fn(), })); @@ -35,6 +38,10 @@ jest.mock('~/utils/env', () => ({ processMCPEnv: jest.fn((params) => params.options), })); +jest.mock('~/auth/domain', () => ({ + isMCPDomainAllowed: jest.fn().mockResolvedValue(true), +})); + const mockRegistryInstance = { getServerConfig: jest.fn(), getAllServerConfigs: jest.fn(), @@ -62,6 +69,11 @@ jest.mock('~/mcp/MCPConnectionFactory'); const mockLogger = logger as jest.Mocked; const mockResolveOboToken = resolveOboToken as jest.MockedFunction; +const mockDetectOAuthRequirement = detectOAuthRequirement as jest.MockedFunction< + typeof detectOAuthRequirement +>; +const mockProcessMCPEnv = processMCPEnv as jest.MockedFunction; +const mockIsMCPDomainAllowed = isMCPDomainAllowed as jest.MockedFunction; describe('MCPManager', () => { const userId = 'test-user-123'; @@ -75,6 +87,15 @@ describe('MCPManager', () => { // Set up default mock implementations (MCPServersInitializer.initialize as jest.Mock).mockResolvedValue(undefined); (mockRegistryInstance.getAllServerConfigs as jest.Mock).mockResolvedValue({}); + (mockRegistryInstance.shouldEnableSSRFProtection as jest.Mock).mockReturnValue(false); + (mockRegistryInstance.getAllowedDomains as jest.Mock).mockReturnValue(null); + (mockRegistryInstance.getAllowedAddresses as jest.Mock).mockReturnValue(null); + mockProcessMCPEnv.mockImplementation((params) => params.options); + mockIsMCPDomainAllowed.mockResolvedValue(true); + mockDetectOAuthRequirement.mockResolvedValue({ + requiresOAuth: false, + method: 'no-metadata-found', + }); }); function mockAppConnections( @@ -478,6 +499,7 @@ describe('MCPManager', () => { return options; }, ); + (MCPConnectionFactory.create as jest.Mock).mockResolvedValue(mockConnection); }); it('should call preProcessGraphTokens with graphTokenResolver when provided', async () => { @@ -541,6 +563,44 @@ describe('MCPManager', () => { ); }); + it('should leave graph token placeholders sandboxed for user-sourced configs', async () => { + const serverConfig: t.ParsedServerConfig = { + type: 'sse', + url: 'https://api.example.com', + headers: { + Authorization: 'Bearer {{LIBRECHAT_GRAPH_ACCESS_TOKEN}}', + }, + source: 'user', + dbId: 'user-server-id', + }; + + mockAppConnections({ + get: jest.fn().mockResolvedValue(mockConnection), + }); + + (mockRegistryInstance.getServerConfig as jest.Mock).mockResolvedValue(serverConfig); + + const manager = await MCPManager.createInstance(newMCPServersConfig()); + + await manager.callTool({ + user: mockUser as IUser, + serverName, + toolName: 'test_tool', + provider: 'openai', + flowManager: mockFlowManager as unknown as Parameters< + typeof manager.callTool + >[0]['flowManager'], + graphTokenResolver: mockGraphTokenResolver, + }); + + expect(graphUtils.preProcessGraphTokens).not.toHaveBeenCalled(); + expect(mockConnection.setRequestHeaders).toHaveBeenCalledWith( + expect.objectContaining({ + Authorization: 'Bearer {{LIBRECHAT_GRAPH_ACCESS_TOKEN}}', + }), + ); + }); + it('should pass options unchanged when no graphTokenResolver is provided', async () => { const serverConfig: t.SSEOptions = { type: 'sse', @@ -1088,6 +1148,49 @@ describe('MCPManager', () => { expect(appConnections.get).toHaveBeenCalledWith(serverName); expect(getUserConnectionSpy).not.toHaveBeenCalled(); }); + + it('should use user-scoped connections for trusted runtime context placeholders', async () => { + const appConnection = { + isConnected: jest.fn().mockResolvedValue(true), + } as unknown as MCPConnection; + const userConnection = { + isConnected: jest.fn().mockResolvedValue(true), + } as unknown as MCPConnection; + const appConnections = { + get: jest.fn().mockResolvedValue(appConnection), + }; + const runtimeHeaderConfig: t.ParsedServerConfig = { + type: 'streamable-http', + url: 'https://api.example.com/mcp', + source: 'yaml', + headers: { + 'X-LibreChat-User-Email': '{{LIBRECHAT_USER_EMAIL}}', + }, + }; + + mockAppConnections(appConnections); + (mockRegistryInstance.getServerConfig as jest.Mock).mockResolvedValue(runtimeHeaderConfig); + + const manager = await MCPManager.createInstance(newMCPServersConfig()); + const getUserConnectionSpy = jest + .spyOn(manager, 'getUserConnection') + .mockResolvedValue(userConnection); + + const connection = await manager.getConnection({ + serverName, + user: mockUser as IUser, + }); + + expect(connection).toBe(userConnection); + expect(appConnections.get).not.toHaveBeenCalled(); + expect(getUserConnectionSpy).toHaveBeenCalledWith( + expect.objectContaining({ + serverName, + serverConfig: runtimeHeaderConfig, + user: mockUser, + }), + ); + }); }); describe('discoverServerTools', () => { @@ -1099,6 +1202,7 @@ describe('MCPManager', () => { const mockConnection = { isConnected: jest.fn().mockResolvedValue(true), fetchTools: jest.fn().mockResolvedValue(mockTools), + disconnect: jest.fn().mockResolvedValue(undefined), } as unknown as MCPConnection; beforeEach(() => { @@ -1120,6 +1224,9 @@ describe('MCPManager', () => { }); it('should use MCPConnectionFactory.discoverTools when no app connection available', async () => { + const discoveryConnection = { + disconnect: jest.fn().mockResolvedValue(undefined), + } as unknown as MCPConnection; mockAppConnections({ get: jest.fn().mockResolvedValue(null), }); @@ -1132,7 +1239,7 @@ describe('MCPManager', () => { (MCPConnectionFactory.discoverTools as jest.Mock).mockResolvedValue({ tools: mockTools, - connection: null, + connection: discoveryConnection, oauthRequired: false, oauthUrl: null, }); @@ -1143,11 +1250,13 @@ describe('MCPManager', () => { expect(result.tools).toEqual(mockTools); expect(result.oauthRequired).toBe(false); expect(MCPConnectionFactory.discoverTools).toHaveBeenCalled(); + expect(discoveryConnection.disconnect).toHaveBeenCalledTimes(1); }); - it('should forward user, customUserVars, requestBody, and connectionTimeout to discoverTools in the non-OAuth path', async () => { + it('should forward runtime context to discoverTools in the non-OAuth path', async () => { const mockUser = { id: 'user123', email: 'test@example.com' } as unknown as IUser; const customUserVars = { MY_CUSTOM_KEY: 'c527bd0abc123' }; + const graphTokenResolver = jest.fn(); mockAppConnections({ get: jest.fn().mockResolvedValue(null), @@ -1171,20 +1280,49 @@ describe('MCPManager', () => { user: mockUser, customUserVars, requestBody: { conversationId: 'conv-123' } as t.ToolDiscoveryOptions['requestBody'], + graphTokenResolver, connectionTimeout: 10000, }); expect(MCPConnectionFactory.discoverTools).toHaveBeenCalledWith( - expect.objectContaining({ serverName }), + expect.objectContaining({ + serverName, + serverConfig: expect.objectContaining({ + url: 'https://my-mcp.server.com?key={{MY_CUSTOM_KEY}}', + }), + }), expect.objectContaining({ user: mockUser, customUserVars, requestBody: { conversationId: 'conv-123' }, + graphTokenResolver, connectionTimeout: 10000, }), ); }); + it('should not discover BODY-scoped servers without request body context', async () => { + mockAppConnections({ + get: jest.fn().mockResolvedValue(null), + }); + + (mockRegistryInstance.getServerConfig as jest.Mock).mockResolvedValue({ + type: 'streamable-http', + url: 'https://api.example.com/messages/{{LIBRECHAT_BODY_MESSAGEID}}/mcp', + source: 'yaml', + }); + + const manager = await MCPManager.createInstance(newMCPServersConfig()); + const result = await manager.discoverServerTools({ serverName }); + + expect(result).toEqual({ tools: null, oauthRequired: false, oauthUrl: null }); + expect(MCPConnectionFactory.discoverTools).not.toHaveBeenCalled(); + expect(mockLogger.warn).toHaveBeenCalledWith( + expect.stringContaining('Request body field(s) required'), + ); + expect(mockLogger.warn).toHaveBeenCalledWith(expect.stringContaining('messageId')); + }); + it('should return null tools when server config not found', async () => { mockAppConnections({ get: jest.fn().mockResolvedValue(null), @@ -1275,6 +1413,7 @@ describe('MCPManager', () => { serverName, user: mockUser, flowManager: mockFlowManager as unknown as t.ToolDiscoveryOptions['flowManager'], + graphTokenResolver: jest.fn(), }); expect(result.tools).toEqual(mockTools); @@ -1282,7 +1421,11 @@ describe('MCPManager', () => { expect(result.oauthUrl).toBe('https://auth.example.com/authorize'); expect(MCPConnectionFactory.discoverTools).toHaveBeenCalledWith( expect.objectContaining({ serverName }), - expect.objectContaining({ user: mockUser, useOAuth: true }), + expect.objectContaining({ + user: mockUser, + useOAuth: true, + graphTokenResolver: expect.any(Function), + }), ); }); }); @@ -1358,6 +1501,373 @@ describe('MCPManager', () => { ); }); + it('should detect OAuth after resolving trusted runtime URL placeholders', async () => { + const runtimeUrlConfig: t.ParsedServerConfig = { + type: 'streamable-http', + url: 'https://api.example.com/users/{{LIBRECHAT_USER_ID}}/mcp', + source: 'yaml', + }; + mockAppConnections({ + has: jest.fn().mockResolvedValue(false), + }); + (mockRegistryInstance.getServerConfig as jest.Mock).mockResolvedValue(runtimeUrlConfig); + mockProcessMCPEnv.mockImplementation(({ options, user }) => ({ + ...options, + ...('url' in options && { + url: options.url?.replace('{{LIBRECHAT_USER_ID}}', user?.id ?? ''), + }), + })); + mockDetectOAuthRequirement.mockResolvedValue({ + requiresOAuth: true, + method: 'protected-resource-metadata', + }); + (MCPConnectionFactory.create as jest.Mock).mockResolvedValue(mockConnection); + + const manager = await MCPManager.createInstance(newMCPServersConfig()); + await manager.getUserConnection({ + serverName, + user: mockUser, + flowManager: mockFlowManager as unknown as t.UserMCPConnectionOptions['flowManager'], + }); + + expect(mockDetectOAuthRequirement).toHaveBeenCalledWith( + 'https://api.example.com/users/test-user-123/mcp', + null, + null, + ); + expect(MCPConnectionFactory.create).toHaveBeenCalledWith( + expect.objectContaining({ + serverConfig: expect.objectContaining({ + requiresOAuth: true, + }), + }), + expect.objectContaining({ useOAuth: true }), + ); + }); + + it('should reject disallowed runtime URLs before OAuth detection probes them', async () => { + const runtimeUrlConfig: t.ParsedServerConfig = { + type: 'streamable-http', + url: 'https://{{LIBRECHAT_BODY_CONVERSATIONID}}.example.com/mcp', + source: 'yaml', + }; + mockAppConnections({ + has: jest.fn().mockResolvedValue(false), + }); + (mockRegistryInstance.getServerConfig as jest.Mock).mockResolvedValue(runtimeUrlConfig); + (mockRegistryInstance.getAllowedDomains as jest.Mock).mockReturnValue(['*.example.com']); + mockProcessMCPEnv.mockImplementation(({ options, body }) => ({ + ...options, + ...('url' in options && { + url: options.url?.replace( + '{{LIBRECHAT_BODY_CONVERSATIONID}}', + body?.conversationId ?? '', + ), + }), + })); + mockIsMCPDomainAllowed.mockResolvedValue(false); + + const manager = await MCPManager.createInstance(newMCPServersConfig()); + await expect( + manager.getUserConnection({ + serverName, + user: mockUser, + requestBody: { conversationId: 'evil.com/path' }, + flowManager: mockFlowManager as unknown as t.UserMCPConnectionOptions['flowManager'], + }), + ).rejects.toThrow('not allowed by the configured domain policy'); + + expect(mockDetectOAuthRequirement).not.toHaveBeenCalled(); + expect(MCPConnectionFactory.create).not.toHaveBeenCalled(); + }); + + it('should reject resolved runtime URLs that fail MCP domain policy', async () => { + const runtimeUrlConfig: t.ParsedServerConfig = { + type: 'streamable-http', + url: 'https://{{LIBRECHAT_BODY_CONVERSATIONID}}.example.com/mcp', + source: 'yaml', + requiresOAuth: false, + }; + mockAppConnections({ + has: jest.fn().mockResolvedValue(false), + }); + (mockRegistryInstance.getServerConfig as jest.Mock).mockResolvedValue(runtimeUrlConfig); + (mockRegistryInstance.getAllowedDomains as jest.Mock).mockReturnValue(['*.example.com']); + mockProcessMCPEnv.mockImplementation(({ options, body }) => ({ + ...options, + ...('url' in options && { + url: options.url?.replace( + '{{LIBRECHAT_BODY_CONVERSATIONID}}', + body?.conversationId ?? '', + ), + }), + })); + mockIsMCPDomainAllowed.mockResolvedValue(false); + + const manager = await MCPManager.createInstance(newMCPServersConfig()); + await expect( + manager.getUserConnection({ + serverName, + user: mockUser, + requestBody: { conversationId: 'evil.com/path' }, + }), + ).rejects.toThrow('not allowed by the configured domain policy'); + + expect(mockIsMCPDomainAllowed).toHaveBeenCalledWith( + expect.objectContaining({ + url: 'https://evil.com/path.example.com/mcp', + }), + ['*.example.com'], + null, + ); + expect(MCPConnectionFactory.create).not.toHaveBeenCalled(); + }); + + it('should validate resolved runtime URLs without passing resolved configs to the factory', async () => { + const runtimeUrlConfig: t.ParsedServerConfig = { + type: 'streamable-http', + url: 'https://{{LIBRECHAT_BODY_CONVERSATIONID}}.example.com/mcp', + source: 'yaml', + requiresOAuth: false, + }; + mockAppConnections({ + has: jest.fn().mockResolvedValue(false), + }); + (mockRegistryInstance.getServerConfig as jest.Mock).mockResolvedValue(runtimeUrlConfig); + (mockRegistryInstance.getAllowedDomains as jest.Mock).mockReturnValue(['*.example.com']); + mockProcessMCPEnv.mockImplementation(({ options, body }) => ({ + ...options, + ...('url' in options && { + url: options.url?.replace( + '{{LIBRECHAT_BODY_CONVERSATIONID}}', + body?.conversationId ?? '', + ), + }), + })); + mockIsMCPDomainAllowed.mockResolvedValue(true); + (MCPConnectionFactory.create as jest.Mock).mockResolvedValue(mockConnection); + + const manager = await MCPManager.createInstance(newMCPServersConfig()); + await manager.getUserConnection({ + serverName, + user: mockUser, + requestBody: { conversationId: 'tenant-a' }, + }); + + expect(mockIsMCPDomainAllowed).toHaveBeenCalledWith( + expect.objectContaining({ + url: 'https://tenant-a.example.com/mcp', + }), + ['*.example.com'], + null, + ); + expect(MCPConnectionFactory.create).toHaveBeenCalledWith( + expect.objectContaining({ + serverConfig: expect.objectContaining({ + url: 'https://{{LIBRECHAT_BODY_CONVERSATIONID}}.example.com/mcp', + }), + }), + expect.objectContaining({ + requestBody: { conversationId: 'tenant-a' }, + }), + ); + }); + + it('should keep graph placeholders unresolved for user-sourced connection configs', async () => { + const graphConfig: t.ParsedServerConfig = { + type: 'streamable-http', + url: 'https://api.example.com/mcp', + source: 'user', + dbId: 'user-server-id', + requiresOAuth: false, + headers: { + Authorization: 'Bearer {{LIBRECHAT_GRAPH_ACCESS_TOKEN}}', + }, + }; + mockAppConnections({ + has: jest.fn().mockResolvedValue(false), + }); + (mockRegistryInstance.getServerConfig as jest.Mock).mockResolvedValue(graphConfig); + (MCPConnectionFactory.create as jest.Mock).mockResolvedValue(mockConnection); + + const manager = await MCPManager.createInstance(newMCPServersConfig()); + await manager.getUserConnection({ + serverName, + user: mockUser, + graphTokenResolver: jest.fn(), + }); + + expect(graphUtils.preProcessGraphTokens).not.toHaveBeenCalled(); + expect(MCPConnectionFactory.create).toHaveBeenCalledWith( + expect.objectContaining({ + serverConfig: expect.objectContaining({ + headers: { + Authorization: 'Bearer {{LIBRECHAT_GRAPH_ACCESS_TOKEN}}', + }, + }), + }), + expect.any(Object), + ); + }); + + it('should not cache connections when request body placeholders affect the URL', async () => { + const bodyUrlConfig: t.ParsedServerConfig = { + type: 'streamable-http', + url: 'https://api.example.com/messages/{{LIBRECHAT_BODY_MESSAGEID}}/mcp', + source: 'yaml', + requiresOAuth: false, + }; + const firstConnection = { + isConnected: jest.fn().mockResolvedValue(true), + } as unknown as MCPConnection; + const secondConnection = { + isConnected: jest.fn().mockResolvedValue(true), + } as unknown as MCPConnection; + + mockAppConnections({ + has: jest.fn().mockResolvedValue(false), + }); + (mockRegistryInstance.getServerConfig as jest.Mock).mockResolvedValue(bodyUrlConfig); + mockProcessMCPEnv.mockImplementation(({ options, body }) => ({ + ...options, + ...('url' in options && { + url: options.url?.replace('{{LIBRECHAT_BODY_MESSAGEID}}', body?.messageId ?? ''), + }), + })); + (MCPConnectionFactory.create as jest.Mock) + .mockResolvedValueOnce(firstConnection) + .mockResolvedValueOnce(secondConnection); + + const manager = await MCPManager.createInstance(newMCPServersConfig()); + const first = await manager.getUserConnection({ + serverName, + user: mockUser, + requestBody: { messageId: 'message-1' }, + }); + const second = await manager.getUserConnection({ + serverName, + user: mockUser, + requestBody: { messageId: 'message-2' }, + }); + + expect(first).toBe(firstConnection); + expect(second).toBe(secondConnection); + expect(MCPConnectionFactory.create).toHaveBeenCalledTimes(2); + }); + + it('should not clear server cooldowns for ephemeral runtime connections', async () => { + const bodyUrlConfig: t.ParsedServerConfig = { + type: 'streamable-http', + url: 'https://api.example.com/messages/{{LIBRECHAT_BODY_MESSAGEID}}/mcp', + source: 'yaml', + requiresOAuth: false, + }; + const clearCooldownSpy = jest.spyOn(MCPConnection, 'clearCooldown'); + + mockAppConnections({ + has: jest.fn().mockResolvedValue(false), + }); + (mockRegistryInstance.getServerConfig as jest.Mock).mockResolvedValue(bodyUrlConfig); + mockProcessMCPEnv.mockImplementation(({ options, body }) => ({ + ...options, + ...('url' in options && { + url: options.url?.replace('{{LIBRECHAT_BODY_MESSAGEID}}', body?.messageId ?? ''), + }), + })); + (MCPConnectionFactory.create as jest.Mock).mockResolvedValue(mockConnection); + + try { + const manager = await MCPManager.createInstance(newMCPServersConfig()); + await manager.getUserConnection({ + serverName, + user: mockUser, + requestBody: { messageId: 'message-1' }, + }); + + expect(clearCooldownSpy).not.toHaveBeenCalled(); + } finally { + clearCooldownSpy.mockRestore(); + } + }); + + it('should still clear server cooldowns for explicit forceNew connections', async () => { + const staticConfig: t.ParsedServerConfig = { + type: 'streamable-http', + url: 'https://api.example.com/mcp', + source: 'yaml', + requiresOAuth: false, + }; + const clearCooldownSpy = jest.spyOn(MCPConnection, 'clearCooldown'); + + mockAppConnections({ + has: jest.fn().mockResolvedValue(false), + }); + (mockRegistryInstance.getServerConfig as jest.Mock).mockResolvedValue(staticConfig); + (MCPConnectionFactory.create as jest.Mock).mockResolvedValue(mockConnection); + + try { + const manager = await MCPManager.createInstance(newMCPServersConfig()); + await manager.getUserConnection({ + serverName, + user: mockUser, + forceNew: true, + }); + + expect(clearCooldownSpy).toHaveBeenCalledWith(serverName); + } finally { + clearCooldownSpy.mockRestore(); + } + }); + + it('should reject BODY-scoped connections without request body context', async () => { + const bodyUrlConfig: t.ParsedServerConfig = { + type: 'streamable-http', + url: 'https://api.example.com/messages/{{LIBRECHAT_BODY_MESSAGEID}}/mcp', + source: 'yaml', + requiresOAuth: false, + }; + + mockAppConnections({ + has: jest.fn().mockResolvedValue(false), + }); + (mockRegistryInstance.getServerConfig as jest.Mock).mockResolvedValue(bodyUrlConfig); + + const manager = await MCPManager.createInstance(newMCPServersConfig()); + await expect( + manager.getUserConnection({ + serverName, + user: mockUser, + }), + ).rejects.toThrow('Request body field(s) required'); + + expect(MCPConnectionFactory.create).not.toHaveBeenCalled(); + }); + + it('should reject BODY-scoped connections when a referenced body field is missing', async () => { + const bodyUrlConfig: t.ParsedServerConfig = { + type: 'streamable-http', + url: 'https://api.example.com/messages/{{LIBRECHAT_BODY_MESSAGEID}}/mcp', + source: 'yaml', + requiresOAuth: false, + }; + + mockAppConnections({ + has: jest.fn().mockResolvedValue(false), + }); + (mockRegistryInstance.getServerConfig as jest.Mock).mockResolvedValue(bodyUrlConfig); + + const manager = await MCPManager.createInstance(newMCPServersConfig()); + await expect( + manager.getUserConnection({ + serverName, + user: mockUser, + requestBody: { conversationId: 'conv-123' }, + }), + ).rejects.toThrow('messageId'); + + expect(MCPConnectionFactory.create).not.toHaveBeenCalled(); + }); + it('should throw when OAuth server lacks flowManager', async () => { mockAppConnections({ has: jest.fn().mockResolvedValue(false), diff --git a/packages/api/src/mcp/__tests__/MCPOAuthRaceCondition.test.ts b/packages/api/src/mcp/__tests__/MCPOAuthRaceCondition.test.ts index bee80bd52b..adf97fe980 100644 --- a/packages/api/src/mcp/__tests__/MCPOAuthRaceCondition.test.ts +++ b/packages/api/src/mcp/__tests__/MCPOAuthRaceCondition.test.ts @@ -36,6 +36,10 @@ jest.mock('~/auth', () => ({ resolveHostnameSSRF: jest.fn(async () => false), })); +jest.mock('~/auth/domain', () => ({ + isMCPDomainAllowed: jest.fn().mockResolvedValue(true), +})); + jest.mock('~/mcp/mcpConfig', () => ({ mcpConfig: { CONNECTION_CHECK_TTL: 0, USER_CONNECTION_IDLE_TIMEOUT: 30 * 60 * 1000 }, })); diff --git a/packages/api/src/mcp/__tests__/mcp.spec.ts b/packages/api/src/mcp/__tests__/mcp.spec.ts index de2839371d..539c04f6b9 100644 --- a/packages/api/src/mcp/__tests__/mcp.spec.ts +++ b/packages/api/src/mcp/__tests__/mcp.spec.ts @@ -1027,6 +1027,44 @@ describe('Environment Variable Extraction (MCP)', () => { ]); }); + it('should resolve graph tokens in args and oauth fields before processMCPEnv', async () => { + const user = createTestUser({ + id: 'user-args-oauth', + provider: 'openid', + }) as unknown as IUser; + + const options: MCPOptions = { + type: 'stdio', + command: 'node', + args: ['server.js', '--graph-token={{LIBRECHAT_GRAPH_ACCESS_TOKEN}}'], + oauth: { + client_id: 'client-id', + client_secret: '{{LIBRECHAT_GRAPH_ACCESS_TOKEN}}', + scope: 'profile {{LIBRECHAT_GRAPH_ACCESS_TOKEN}}', + }, + }; + + const graphProcessedConfig = await preProcessGraphTokens(options, { + user, + graphTokenResolver: mockGraphTokenResolver, + }); + + const finalConfig = processMCPEnv({ + options: graphProcessedConfig, + user, + }); + + expect('args' in finalConfig && finalConfig.args).toEqual([ + 'server.js', + '--graph-token=resolved-graph-api-token', + ]); + expect(finalConfig.oauth).toEqual({ + client_id: 'client-id', + client_secret: 'resolved-graph-api-token', + scope: 'profile resolved-graph-api-token', + }); + }); + it('should resolve graph tokens in URL alongside other placeholders', async () => { const user = createTestUser({ id: 'user-789', diff --git a/packages/api/src/mcp/__tests__/utils.test.ts b/packages/api/src/mcp/__tests__/utils.test.ts index c40273440d..199f259434 100644 --- a/packages/api/src/mcp/__tests__/utils.test.ts +++ b/packages/api/src/mcp/__tests__/utils.test.ts @@ -1,3 +1,4 @@ +import type { ParsedServerConfig } from '~/mcp/types'; import { buildOAuthToolCallName, normalizeServerName, @@ -8,9 +9,14 @@ import { isClientRejectionMessage, getMissingCustomUserVars, hasCustomUserVars, + hasRuntimeUrlPlaceholders, + hasRuntimeBodyPlaceholders, + hasRuntimeContextPlaceholders, + getRuntimeBodyPlaceholderFields, + getMissingRuntimeBodyPlaceholderFields, isUserSourced, + requiresEphemeralUserConnection, } from '~/mcp/utils'; -import type { ParsedServerConfig } from '~/mcp/types'; describe('normalizeServerName', () => { it('should not modify server names that already match the pattern', () => { @@ -378,6 +384,29 @@ describe('requiresUserScopedConnection', () => { ).toBe(true); }); + it('returns true for trusted config with runtime user placeholders', () => { + expect( + requiresUserScopedConnection({ + source: 'yaml', + headers: { + 'X-LibreChat-User-Email': '{{LIBRECHAT_USER_EMAIL}}', + }, + }), + ).toBe(true); + }); + + it('returns false for user-sourced config with runtime user placeholders', () => { + expect( + requiresUserScopedConnection({ + source: 'user', + dbId: 'server-123', + headers: { + 'X-LibreChat-User-Email': '{{LIBRECHAT_USER_EMAIL}}', + }, + }), + ).toBe(false); + }); + it('returns false for app-shareable servers', () => { expect( requiresUserScopedConnection({ @@ -388,6 +417,207 @@ describe('requiresUserScopedConnection', () => { }); }); +describe('hasRuntimeContextPlaceholders', () => { + it('detects trusted runtime placeholders across connection fields', () => { + expect( + hasRuntimeContextPlaceholders({ + source: 'config', + url: 'https://example.com/{{LIBRECHAT_BODY_MESSAGEID}}/mcp', + headers: { + Authorization: 'Bearer {{LIBRECHAT_OPENID_ID_TOKEN}}', + 'X-Graph-Access-Token': '{{LIBRECHAT_GRAPH_ACCESS_TOKEN}}', + }, + }), + ).toBe(true); + }); + + it('detects trusted runtime placeholders in oauth_headers', () => { + expect( + hasRuntimeContextPlaceholders({ + source: 'yaml', + url: 'https://example.com/mcp', + oauth_headers: { + 'X-User': '{{LIBRECHAT_USER_ID}}', + }, + }), + ).toBe(true); + }); + + it('ignores custom user variable placeholders', () => { + expect( + hasRuntimeContextPlaceholders({ + source: 'yaml', + headers: { + Authorization: 'Bearer {{MCP_API_KEY}}', + }, + }), + ).toBe(false); + }); + + it('ignores runtime placeholders in user-sourced configs', () => { + expect( + hasRuntimeContextPlaceholders({ + source: 'user', + dbId: 'server-123', + headers: { + Authorization: 'Bearer {{LIBRECHAT_OPENID_ID_TOKEN}}', + }, + }), + ).toBe(false); + }); +}); + +describe('hasRuntimeUrlPlaceholders', () => { + it('detects trusted runtime placeholders in the server URL', () => { + expect( + hasRuntimeUrlPlaceholders({ + source: 'yaml', + url: 'https://example.com/users/{{LIBRECHAT_USER_USERNAME}}/mcp', + }), + ).toBe(true); + }); + + it('ignores runtime URL placeholders in user-sourced configs', () => { + expect( + hasRuntimeUrlPlaceholders({ + source: 'user', + dbId: 'server-123', + url: 'https://example.com/users/{{LIBRECHAT_USER_USERNAME}}/mcp', + }), + ).toBe(false); + }); +}); + +describe('hasRuntimeBodyPlaceholders', () => { + it('detects trusted runtime BODY placeholders across connection fields', () => { + expect( + hasRuntimeBodyPlaceholders({ + source: 'yaml', + url: 'https://example.com/conversations/{{LIBRECHAT_BODY_CONVERSATIONID}}/mcp', + }), + ).toBe(true); + + expect( + hasRuntimeBodyPlaceholders({ + source: 'config', + headers: { + 'X-Message': '{{LIBRECHAT_BODY_MESSAGEID}}', + }, + }), + ).toBe(true); + }); + + it('ignores BODY placeholders in user-sourced configs', () => { + expect( + hasRuntimeBodyPlaceholders({ + source: 'user', + dbId: 'server-123', + url: 'https://example.com/{{LIBRECHAT_BODY_MESSAGEID}}/mcp', + }), + ).toBe(false); + }); +}); + +describe('getMissingRuntimeBodyPlaceholderFields', () => { + const config = { + source: 'yaml', + url: 'https://example.com/conversations/{{LIBRECHAT_BODY_CONVERSATIONID}}/mcp', + headers: { + 'X-Message': '{{LIBRECHAT_BODY_MESSAGEID}}', + 'X-Parent': '{{LIBRECHAT_BODY_PARENTMESSAGEID}}', + }, + } as const; + + it('returns the request body fields required by trusted runtime placeholders', () => { + expect(getRuntimeBodyPlaceholderFields(config)).toEqual([ + 'messageId', + 'parentMessageId', + 'conversationId', + ]); + }); + + it('returns missing or blank request body fields', () => { + expect( + getMissingRuntimeBodyPlaceholderFields(config, { + conversationId: 'conv-123', + messageId: ' ', + }), + ).toEqual(['messageId', 'parentMessageId']); + }); + + it('ignores BODY placeholders in user-sourced configs', () => { + expect( + getMissingRuntimeBodyPlaceholderFields({ + source: 'user', + dbId: 'server-123', + url: 'https://example.com/{{LIBRECHAT_BODY_MESSAGEID}}/mcp', + }), + ).toEqual([]); + }); +}); + +describe('requiresEphemeralUserConnection', () => { + it('returns true when request-varying placeholders affect oauth_headers', () => { + expect( + requiresEphemeralUserConnection({ + source: 'yaml', + url: 'https://example.com/mcp', + oauth_headers: { + Authorization: 'Bearer {{LIBRECHAT_OPENID_ACCESS_TOKEN}}', + }, + }), + ).toBe(true); + }); + + it('returns true when request-varying placeholders affect connection fields', () => { + expect( + requiresEphemeralUserConnection({ + source: 'yaml', + url: 'https://example.com/messages/{{LIBRECHAT_BODY_MESSAGEID}}/mcp', + }), + ).toBe(true); + + expect( + requiresEphemeralUserConnection({ + source: 'config', + env: { + GRAPH_TOKEN: '{{LIBRECHAT_GRAPH_ACCESS_TOKEN}}', + }, + }), + ).toBe(true); + }); + + it('returns true when OpenID token placeholders affect connection fields', () => { + expect( + requiresEphemeralUserConnection({ + source: 'yaml', + args: ['--id-token={{LIBRECHAT_OPENID_ID_TOKEN}}'], + }), + ).toBe(true); + + expect( + requiresEphemeralUserConnection({ + source: 'yaml', + headers: { + Authorization: 'Bearer {{LIBRECHAT_OPENID_ACCESS_TOKEN}}', + }, + }), + ).toBe(true); + }); + + it('returns true when request-varying placeholders affect remote transport headers', () => { + expect( + requiresEphemeralUserConnection({ + source: 'yaml', + headers: { + 'X-Message': '{{LIBRECHAT_BODY_MESSAGEID}}', + 'X-Graph': '{{LIBRECHAT_GRAPH_ACCESS_TOKEN}}', + }, + }), + ).toBe(true); + }); +}); + describe('getMissingCustomUserVars', () => { const configWithVars = (keys: string[]): Pick => ({ customUserVars: Object.fromEntries( diff --git a/packages/api/src/mcp/registry/MCPServerInspector.ts b/packages/api/src/mcp/registry/MCPServerInspector.ts index 2ec4723085..2f41678d98 100644 --- a/packages/api/src/mcp/registry/MCPServerInspector.ts +++ b/packages/api/src/mcp/registry/MCPServerInspector.ts @@ -1,10 +1,16 @@ +import { logger } from '@librechat/data-schemas'; import { Constants } from 'librechat-data-provider'; import type { JsonSchemaType } from '@librechat/data-schemas'; import type { MCPConnection } from '~/mcp/connection'; import type * as t from '~/mcp/types'; +import { + hasCustomUserVars, + hasRuntimeContextPlaceholders, + hasRuntimeUrlPlaceholders, + isUserSourced, +} from '~/mcp/utils'; import { isMCPDomainAllowed, extractMCPServerDomain } from '~/auth/domain'; import { MCPConnectionFactory } from '~/mcp/MCPConnectionFactory'; -import { hasCustomUserVars, isUserSourced } from '~/mcp/utils'; import { MCPDomainNotAllowedError } from '~/mcp/errors'; import { detectOAuthRequirement } from '~/mcp/oauth'; import { isEnabled } from '~/utils'; @@ -63,12 +69,14 @@ export class MCPServerInspector { } private async inspectServer(): Promise { + this.warnOnUnrestrictedRuntimeUrl(); await this.detectOAuth(); if ( this.config.startup !== false && !this.config.requiresOAuth && !hasCustomUserVars(this.config) && + !hasRuntimeContextPlaceholders(this.config) && !this.config.obo ) { let tempConnection = false; @@ -94,8 +102,25 @@ export class MCPServerInspector { } } + /** + * Runtime placeholders in the URL make the resolved connection target partially + * user/request-controlled. The resolved URL is validated against the domain + * allowlist at request time, but without one only private-range SSRF protection + * limits where it can point. + */ + private warnOnUnrestrictedRuntimeUrl(): void { + if (!hasRuntimeUrlPlaceholders(this.config)) return; + if (Array.isArray(this.allowedDomains) && this.allowedDomains.length > 0) return; + + logger.warn( + `[MCP][${this.serverName}] Server URL contains runtime placeholders but no domain allowlist is configured; ` + + 'the resolved URL is partially user/request-controlled. Set mcpSettings.allowedDomains to restrict targets.', + ); + } + private async detectOAuth(): Promise { if (this.config.requiresOAuth != null) return; + if (hasRuntimeUrlPlaceholders(this.config)) return; if (this.config.url == null || this.config.startup === false) { this.config.requiresOAuth = false; return; diff --git a/packages/api/src/mcp/registry/MCPServersRegistry.ts b/packages/api/src/mcp/registry/MCPServersRegistry.ts index 6c0e95b6ad..06e583608a 100644 --- a/packages/api/src/mcp/registry/MCPServersRegistry.ts +++ b/packages/api/src/mcp/registry/MCPServersRegistry.ts @@ -339,11 +339,13 @@ export class MCPServersRegistry { reservedServerNames?: Iterable, ): Promise { const configRepo = this.getConfigRepository(storageLocation); + const source = (storageLocation === 'CACHE' ? 'yaml' : 'user') as t.MCPServerSource; + const configForInspection = { ...config, source } as t.ParsedServerConfig; let parsedConfig: t.ParsedServerConfig; try { parsedConfig = await MCPServerInspector.inspect( serverName, - config, + configForInspection, undefined, this.allowedDomains, this.allowedAddresses, @@ -357,7 +359,7 @@ export class MCPServersRegistry { } const tagged = { ...parsedConfig, - source: (storageLocation === 'CACHE' ? 'yaml' : 'user') as t.MCPServerSource, + source, }; const result = storageLocation === 'DB' @@ -426,6 +428,7 @@ export class MCPServersRegistry { userId?: string, ): Promise { const configRepo = this.getConfigRepository(storageLocation); + const source = (storageLocation === 'CACHE' ? 'yaml' : 'user') as t.MCPServerSource; // Merge existing admin API key if not provided in update (needed for inspection) let configForInspection = { ...config }; @@ -446,7 +449,7 @@ export class MCPServersRegistry { try { parsedConfig = await MCPServerInspector.inspect( serverName, - configForInspection, + { ...configForInspection, source } as t.ParsedServerConfig, undefined, this.allowedDomains, this.allowedAddresses, @@ -578,10 +581,14 @@ export class MCPServersRegistry { logger.info(`${prefix} Lazy-initializing config-source server`); try { + const configForInspection = { + ...rawConfig, + source: 'config' as const, + } as t.ParsedServerConfig; const inspected = await withTimeout( MCPServerInspector.inspect( serverName, - rawConfig, + configForInspection, undefined, this.allowedDomains, this.allowedAddresses, diff --git a/packages/api/src/mcp/registry/__tests__/MCPServerInspector.test.ts b/packages/api/src/mcp/registry/__tests__/MCPServerInspector.test.ts index 3937d700f0..dd64c3e5bd 100644 --- a/packages/api/src/mcp/registry/__tests__/MCPServerInspector.test.ts +++ b/packages/api/src/mcp/registry/__tests__/MCPServerInspector.test.ts @@ -148,6 +148,52 @@ describe('MCPServerInspector', () => { expect(result.toolFunctions).toBeUndefined(); }); + it('should skip capabilities fetch when trusted config needs runtime user context', async () => { + mockDetectOAuthRequirement.mockResolvedValue({ + requiresOAuth: false, + method: 'no-metadata-found', + }); + + const rawConfig: t.MCPOptions = { + type: 'streamable-http', + url: 'https://mcp-server.example.com/mcp', + headers: { + 'X-LibreChat-User-Email': '{{LIBRECHAT_USER_EMAIL}}', + }, + }; + + const result = await MCPServerInspector.inspect('test_server', rawConfig); + + expect(result).toEqual({ + type: 'streamable-http', + url: 'https://mcp-server.example.com/mcp', + headers: { + 'X-LibreChat-User-Email': '{{LIBRECHAT_USER_EMAIL}}', + }, + requiresOAuth: false, + oauthMetadata: undefined, + initDuration: expect.any(Number), + }); + expect(MCPConnectionFactory.create).not.toHaveBeenCalled(); + }); + + it('should skip OAuth detection when trusted URL needs runtime user context', async () => { + const rawConfig: t.MCPOptions = { + type: 'streamable-http', + url: 'https://mcp-server.example.com/users/{{LIBRECHAT_USER_USERNAME}}/mcp', + }; + + const result = await MCPServerInspector.inspect('test_server', rawConfig); + + expect(result).toEqual({ + type: 'streamable-http', + url: 'https://mcp-server.example.com/users/{{LIBRECHAT_USER_USERNAME}}/mcp', + initDuration: expect.any(Number), + }); + expect(mockDetectOAuthRequirement).not.toHaveBeenCalled(); + expect(MCPConnectionFactory.create).not.toHaveBeenCalled(); + }); + it('should skip capabilities fetch when obo is configured', async () => { // OBO servers mint per-user delegated tokens at tool-call time; an // unauthenticated probe at inspection has no valid bearer to attach, diff --git a/packages/api/src/mcp/registry/__tests__/MCPServersInitializer.test.ts b/packages/api/src/mcp/registry/__tests__/MCPServersInitializer.test.ts index 1e705cb79b..8964c15c19 100644 --- a/packages/api/src/mcp/registry/__tests__/MCPServersInitializer.test.ts +++ b/packages/api/src/mcp/registry/__tests__/MCPServersInitializer.test.ts @@ -1,12 +1,12 @@ import { logger } from '@librechat/data-schemas'; -import * as t from '~/mcp/types'; -import { isLeader } from '~/cluster'; import { registryStatusCache } from '~/mcp/registry/cache/RegistryStatusCache'; import { MCPServersInitializer } from '~/mcp/registry/MCPServersInitializer'; import { MCPServerInspector } from '~/mcp/registry/MCPServerInspector'; import { MCPServersRegistry } from '~/mcp/registry/MCPServersRegistry'; import { MCPConnectionFactory } from '~/mcp/MCPConnectionFactory'; import { MCPConnection } from '~/mcp/connection'; +import { isLeader } from '~/cluster'; +import * as t from '~/mcp/types'; const FIXED_TIME = 1699564800000; const originalDateNow = Date.now; @@ -367,35 +367,35 @@ describe('MCPServersInitializer', () => { expect(mockInspect).toHaveBeenCalledTimes(5); expect(mockInspect).toHaveBeenCalledWith( 'disabled_server', - testConfigs.disabled_server, + { ...testConfigs.disabled_server, source: 'yaml' }, undefined, undefined, undefined, ); expect(mockInspect).toHaveBeenCalledWith( 'oauth_server', - testConfigs.oauth_server, + { ...testConfigs.oauth_server, source: 'yaml' }, undefined, undefined, undefined, ); expect(mockInspect).toHaveBeenCalledWith( 'file_tools_server', - testConfigs.file_tools_server, + { ...testConfigs.file_tools_server, source: 'yaml' }, undefined, undefined, undefined, ); expect(mockInspect).toHaveBeenCalledWith( 'search_tools_server', - testConfigs.search_tools_server, + { ...testConfigs.search_tools_server, source: 'yaml' }, undefined, undefined, undefined, ); expect(mockInspect).toHaveBeenCalledWith( 'remote_no_oauth_server', - testConfigs.remote_no_oauth_server, + { ...testConfigs.remote_no_oauth_server, source: 'yaml' }, undefined, undefined, undefined, diff --git a/packages/api/src/mcp/registry/__tests__/MCPServersRegistry.test.ts b/packages/api/src/mcp/registry/__tests__/MCPServersRegistry.test.ts index b1a86864b0..706d34ed2a 100644 --- a/packages/api/src/mcp/registry/__tests__/MCPServersRegistry.test.ts +++ b/packages/api/src/mcp/registry/__tests__/MCPServersRegistry.test.ts @@ -1,5 +1,5 @@ -import type * as t from '~/mcp/types'; import { logger } from '@librechat/data-schemas'; +import type * as t from '~/mcp/types'; import { MCPServersRegistry } from '~/mcp/registry/MCPServersRegistry'; import { MCPServerInspector } from '~/mcp/registry/MCPServerInspector'; @@ -164,6 +164,36 @@ describe('MCPServersRegistry', () => { }); describe('addServer', () => { + it('should pass user source to inspector before storing DB servers', async () => { + const inspectSpy = jest.spyOn(MCPServerInspector, 'inspect'); + + await registry.addServer( + 'user_runtime_server', + { + type: 'streamable-http', + url: 'https://api.example.com/mcp', + headers: { + 'X-LibreChat-User-Email': '{{LIBRECHAT_USER_EMAIL}}', + }, + }, + 'DB', + 'user-1', + ); + + expect(inspectSpy).toHaveBeenCalledWith( + 'user_runtime_server', + expect.objectContaining({ + source: 'user', + headers: { + 'X-LibreChat-User-Email': '{{LIBRECHAT_USER_EMAIL}}', + }, + }), + undefined, + undefined, + undefined, + ); + }); + it('should reserve YAML and current config server names when creating DB servers', async () => { await registry.addServer('slack', { ...testParsedConfig, title: 'Slack' }, 'CACHE'); await registry['configCacheRepo'].upsert('other_tenant:hash', { @@ -488,7 +518,10 @@ describe('MCPServersRegistry', () => { expect(inspectSpy).toHaveBeenCalledTimes(1); expect(inspectSpy).toHaveBeenCalledWith( 'config-only-server', - configOnlyRawConfig, + { + ...configOnlyRawConfig, + source: 'config', + }, undefined, undefined, undefined, diff --git a/packages/api/src/mcp/registry/__tests__/ensureConfigServers.test.ts b/packages/api/src/mcp/registry/__tests__/ensureConfigServers.test.ts index 5db4218a27..933ff7cec2 100644 --- a/packages/api/src/mcp/registry/__tests__/ensureConfigServers.test.ts +++ b/packages/api/src/mcp/registry/__tests__/ensureConfigServers.test.ts @@ -102,7 +102,7 @@ describe('MCPServersRegistry — ensureConfigServers', () => { expect(inspectSpy).toHaveBeenCalledTimes(1); expect(inspectSpy).toHaveBeenCalledWith( 'config_server', - sseConfig, + { ...sseConfig, source: 'config' }, undefined, undefined, undefined, @@ -195,7 +195,7 @@ describe('MCPServersRegistry — ensureConfigServers', () => { expect(inspectSpy).toHaveBeenCalledTimes(1); expect(inspectSpy).toHaveBeenCalledWith( 'my_server', - sseConfig, + { ...sseConfig, source: 'config' }, undefined, undefined, undefined, diff --git a/packages/api/src/mcp/tools.spec.ts b/packages/api/src/mcp/tools.spec.ts index 2a5d201df8..fe9d87bb3c 100644 --- a/packages/api/src/mcp/tools.spec.ts +++ b/packages/api/src/mcp/tools.spec.ts @@ -1,7 +1,7 @@ import { Constants } from 'librechat-data-provider'; -import { createMCPToolCacheService } from './tools'; -import type { LCAvailableTools } from './types'; import type { MCPToolInput, MCPToolCacheDeps } from './tools'; +import type { LCAvailableTools } from './types'; +import { createMCPToolCacheService } from './tools'; function createMockDeps(overrides: Partial = {}): MCPToolCacheDeps { return { @@ -57,6 +57,29 @@ describe('createMCPToolCacheService', () => { }); }); + it('constructs tool names without caching when skipCache is true', async () => { + const deps = createMockDeps(); + const { updateMCPServerTools } = createMCPToolCacheService(deps); + const tools: MCPToolInput[] = [ + { + name: 'search', + description: 'Search request-scoped docs', + inputSchema: { type: 'object', properties: {} }, + }, + ]; + + const result = await updateMCPServerTools({ + userId: 'u1', + serverName: 'body-scoped', + tools, + skipCache: true, + }); + + const expectedKey = `search${Constants.mcp_delimiter}body-scoped`; + expect(result[expectedKey]).toBeDefined(); + expect(deps.setCachedTools).not.toHaveBeenCalled(); + }); + it('propagates setCachedTools errors', async () => { const deps = createMockDeps({ setCachedTools: jest.fn().mockRejectedValue(new Error('Redis down')), diff --git a/packages/api/src/mcp/tools.ts b/packages/api/src/mcp/tools.ts index ccdb48e19a..fe0f582315 100644 --- a/packages/api/src/mcp/tools.ts +++ b/packages/api/src/mcp/tools.ts @@ -25,6 +25,7 @@ export function createMCPToolCacheService(deps: MCPToolCacheDeps): { userId: string; serverName: string; tools: MCPToolInput[] | null; + skipCache?: boolean; }) => Promise; mergeAppTools: (appTools: LCAvailableTools) => Promise; cacheMCPServerTools: (params: { @@ -39,8 +40,9 @@ export function createMCPToolCacheService(deps: MCPToolCacheDeps): { userId: string; serverName: string; tools: MCPToolInput[] | null; + skipCache?: boolean; }): Promise { - const { userId, serverName, tools } = params; + const { userId, serverName, tools, skipCache = false } = params; try { const serverTools: LCAvailableTools = {}; const mcpDelimiter = Constants.mcp_delimiter; @@ -63,6 +65,13 @@ export function createMCPToolCacheService(deps: MCPToolCacheDeps): { serverTools[name] = entry; } + if (skipCache) { + logger.debug( + `[MCP Cache] Built ${tools.length} tools for request-scoped server ${serverName} (user: ${userId}) without caching`, + ); + return serverTools; + } + await setCachedTools(serverTools, { userId, serverName }); logger.debug( `[MCP Cache] Updated ${tools.length} tools for server ${serverName} (user: ${userId})`, diff --git a/packages/api/src/mcp/types/index.ts b/packages/api/src/mcp/types/index.ts index 2b8fa66c7f..25573609b7 100644 --- a/packages/api/src/mcp/types/index.ts +++ b/packages/api/src/mcp/types/index.ts @@ -20,6 +20,7 @@ import type { SearchResultData, UIResource, TPlugin } from 'librechat-data-provi import type { TokenMethods, IUser } from '@librechat/data-schemas'; import type { LCTool } from '@librechat/agents'; import type { OboTokenResolver, OboTrustChecker } from '~/mcp/oauth/obo'; +import type { GraphTokenResolver } from '~/utils/graph'; import type { FlowStateManager } from '~/flow/manager'; import type { RequestBody } from '~/types/http'; import type * as o from '~/mcp/oauth/types'; @@ -198,6 +199,7 @@ export interface UserConnectionContext { user?: IUser; customUserVars?: Record; requestBody?: RequestBody; + graphTokenResolver?: GraphTokenResolver; connectionTimeout?: number; } @@ -223,6 +225,7 @@ export interface OAuthConnectionOptions extends UserConnectionContext { export interface UserMCPConnectionOptions extends UserConnectionContext { serverName: string; forceNew?: boolean; + ephemeralConnection?: boolean; serverConfig?: ParsedServerConfig; flowManager?: FlowStateManager; tokenMethods?: TokenMethods; @@ -243,6 +246,7 @@ export interface ToolDiscoveryOptions { oauthStart?: OAuthStartHandler; customUserVars?: Record; requestBody?: RequestBody; + graphTokenResolver?: GraphTokenResolver; connectionTimeout?: number; /** Pre-resolved config-source servers for tenant-scoped lookup */ configServers?: Record; diff --git a/packages/api/src/mcp/utils.ts b/packages/api/src/mcp/utils.ts index f862badc31..9d58c10358 100644 --- a/packages/api/src/mcp/utils.ts +++ b/packages/api/src/mcp/utils.ts @@ -1,8 +1,45 @@ import { Constants } from 'librechat-data-provider'; import type { ParsedServerConfig } from '~/mcp/types'; +import type { RequestBody } from '~/types'; export const mcpToolPattern: RegExp = new RegExp(`^.+${Constants.mcp_delimiter}.+$`); +const RUNTIME_CONTEXT_PLACEHOLDER_PATTERN = /\{\{LIBRECHAT_(?:USER|OPENID|GRAPH|BODY)_[^}]+\}\}/; +const EPHEMERAL_CONNECTION_PLACEHOLDER_PATTERN = /\{\{LIBRECHAT_(?:OPENID|GRAPH|BODY)_[^}]+\}\}/; +const RUNTIME_BODY_PLACEHOLDER_PATTERN = /\{\{LIBRECHAT_BODY_[^}]+\}\}/; +const RUNTIME_BODY_PLACEHOLDER_CAPTURE_PATTERN = /\{\{LIBRECHAT_BODY_([^}]+)\}\}/g; + +const BODY_PLACEHOLDER_FIELDS: Record = { + CONVERSATIONID: 'conversationId', + PARENTMESSAGEID: 'parentMessageId', + MESSAGEID: 'messageId', +}; + +type PlaceholderValue = + | string + | number + | boolean + | null + | undefined + | readonly PlaceholderValue[] + | { readonly [key: string]: PlaceholderValue }; + +type UserScopedConnectionConfig = Pick< + ParsedServerConfig, + 'requiresOAuth' | 'customUserVars' | 'obo' | 'source' | 'dbId' +> & { + args?: string[]; + env?: Record; + headers?: Record; + oauth?: PlaceholderValue; + oauth_headers?: Record; + url?: string; +}; + +function placeholderBearingFields(config: UserScopedConnectionConfig): PlaceholderValue[] { + return [config.args, config.env, config.headers, config.oauth, config.oauth_headers, config.url]; +} + /** Whether a server should use MCP OAuth handling. */ export function isOAuthServer( config: Pick, @@ -35,14 +72,136 @@ export function hasCustomUserVars(config: Pick 0; } +function hasRuntimeContextPlaceholder(value: PlaceholderValue): boolean { + return hasPlaceholder(value, RUNTIME_CONTEXT_PLACEHOLDER_PATTERN); +} + +function hasEphemeralConnectionPlaceholder(value: PlaceholderValue): boolean { + return hasPlaceholder(value, EPHEMERAL_CONNECTION_PLACEHOLDER_PATTERN); +} + +function hasPlaceholder(value: PlaceholderValue, pattern: RegExp): boolean { + if (typeof value === 'string') { + return pattern.test(value); + } + if (Array.isArray(value)) { + return value.some((item) => hasPlaceholder(item, pattern)); + } + + if (value == null || typeof value !== 'object') { + return false; + } + + return Object.values(value).some((item) => hasPlaceholder(item, pattern)); +} + +function addRuntimeBodyPlaceholderFields(value: PlaceholderValue, fields: Set): void { + if (typeof value === 'string') { + for (const match of value.matchAll(RUNTIME_BODY_PLACEHOLDER_CAPTURE_PATTERN)) { + const placeholderKey = match[1]; + if (placeholderKey) { + fields.add(BODY_PLACEHOLDER_FIELDS[placeholderKey] ?? placeholderKey); + } + } + return; + } + + if (Array.isArray(value)) { + for (const item of value) { + addRuntimeBodyPlaceholderFields(item, fields); + } + return; + } + + if (value == null || typeof value !== 'object') { + return; + } + + for (const item of Object.values(value)) { + addRuntimeBodyPlaceholderFields(item, fields); + } +} + +/** + * Trusted YAML/config servers may use per-user/request placeholders that can + * only be resolved once a real request context exists. User-sourced DB servers + * deliberately stay sandboxed and only resolve customUserVars. + */ +export function hasRuntimeContextPlaceholders(config: UserScopedConnectionConfig): boolean { + if (isUserSourced(config)) { + return false; + } + + return placeholderBearingFields(config).some(hasRuntimeContextPlaceholder); +} + +export function hasRuntimeUrlPlaceholders(config: UserScopedConnectionConfig): boolean { + if (isUserSourced(config)) { + return false; + } + + return hasRuntimeContextPlaceholder(config.url); +} + +export function hasRuntimeBodyPlaceholders(config: UserScopedConnectionConfig): boolean { + if (isUserSourced(config)) { + return false; + } + + return placeholderBearingFields(config).some((value) => + hasPlaceholder(value, RUNTIME_BODY_PLACEHOLDER_PATTERN), + ); +} + +export function getRuntimeBodyPlaceholderFields(config: UserScopedConnectionConfig): string[] { + if (isUserSourced(config)) { + return []; + } + + const fields = new Set(); + for (const value of placeholderBearingFields(config)) { + addRuntimeBodyPlaceholderFields(value, fields); + } + return Array.from(fields); +} + +export function getMissingRuntimeBodyPlaceholderFields( + config: UserScopedConnectionConfig, + requestBody?: RequestBody, +): string[] { + return getRuntimeBodyPlaceholderFields(config).filter((field) => { + const value = requestBody?.[field as keyof RequestBody]; + return value == null || (typeof value === 'string' && value.trim() === ''); + }); +} + +/** + * `GRAPH` and `BODY` placeholders can change per request. If they affect the + * connection-defining parts of a config, the normal userId:serverName cache + * would reuse a connection built with stale request context. + * + * Ephemeral connections are created and torn down per tool call — configs using + * these placeholders pay a full connect + initialize on every invocation. + */ +export function requiresEphemeralUserConnection(config: UserScopedConnectionConfig): boolean { + if (isUserSourced(config)) { + return false; + } + + return placeholderBearingFields(config).some(hasEphemeralConnectionPlaceholder); +} + /** * Returns true when a server requires a per-user connection instead of an * app-shared connection. */ -export function requiresUserScopedConnection( - config: Pick, -): boolean { - return config.requiresOAuth === true || config.obo != null || hasCustomUserVars(config); +export function requiresUserScopedConnection(config: UserScopedConnectionConfig): boolean { + return ( + config.requiresOAuth === true || + config.obo != null || + hasCustomUserVars(config) || + hasRuntimeContextPlaceholders(config) + ); } /** diff --git a/packages/api/src/utils/env.spec.ts b/packages/api/src/utils/env.spec.ts index 52013d943d..ccfe42c93d 100644 --- a/packages/api/src/utils/env.spec.ts +++ b/packages/api/src/utils/env.spec.ts @@ -1126,6 +1126,42 @@ describe('processMCPEnv', () => { }); }); + it('should process user placeholders in oauth_headers', () => { + const user = createTestUser({ id: 'user-123', email: 'test@example.com' }); + const options: MCPOptions = { + type: 'streamable-http', + url: 'https://mcp.example.com/api', + oauth_headers: { + 'X-User-Id': '{{LIBRECHAT_USER_ID}}', + 'X-Static': 'static-value', + }, + }; + + const result = processMCPEnv({ options, user }); + + expect('oauth_headers' in result! && result.oauth_headers).toEqual({ + 'X-User-Id': 'user-123', + 'X-Static': 'static-value', + }); + }); + + it('should NOT resolve user placeholders in oauth_headers when dbSourced', () => { + const user = createTestUser({ id: 'user-123' }); + const options: MCPOptions = { + type: 'streamable-http', + url: 'https://mcp.example.com/api', + oauth_headers: { + 'X-User-Id': '{{LIBRECHAT_USER_ID}}', + }, + }; + + const result = processMCPEnv({ options, user, dbSourced: true }); + + expect('oauth_headers' in result! && result.oauth_headers).toEqual({ + 'X-User-Id': '{{LIBRECHAT_USER_ID}}', + }); + }); + it('should process user field placeholders in all fields', () => { const user = createTestUser({ id: 'user-123', diff --git a/packages/api/src/utils/env.ts b/packages/api/src/utils/env.ts index 316a28afcc..7d331ada95 100644 --- a/packages/api/src/utils/env.ts +++ b/packages/api/src/utils/env.ts @@ -379,6 +379,22 @@ export function processMCPEnv(params: { newObj.headers = processedHeaders; } + // Process OAuth headers if they exist; sent on OAuth discovery/token requests + if ('oauth_headers' in newObj && newObj.oauth_headers) { + const processedOAuthHeaders: Record = {}; + for (const [key, originalValue] of Object.entries(newObj.oauth_headers)) { + processedOAuthHeaders[key] = processSingleValue({ + user, + body, + dbSourced, + originalValue, + customUserVars, + isHeader: true, + }); + } + newObj.oauth_headers = processedOAuthHeaders; + } + // Process URL if it exists (for WebSocket, SSE, StreamableHTTP types) if ('url' in newObj && newObj.url) { newObj.url = processSingleValue({ diff --git a/packages/api/src/utils/graph.ts b/packages/api/src/utils/graph.ts index 3bb3d2bf4c..3bce8ee468 100644 --- a/packages/api/src/utils/graph.ts +++ b/packages/api/src/utils/graph.ts @@ -13,6 +13,15 @@ import { */ const GRAPH_TOKEN_REGEX = new RegExp(GRAPH_TOKEN_PLACEHOLDER.replace(/[{}]/g, '\\$&'), 'g'); +type GraphTokenResolvable = + | string + | string[] + | boolean + | number + | null + | undefined + | Record; + /** * Response from a Graph API token exchange. */ @@ -67,26 +76,48 @@ export function recordContainsGraphTokenPlaceholder( return Object.values(record).some(containsGraphTokenPlaceholder); } +function valueContainsGraphTokenPlaceholder(value: GraphTokenResolvable): boolean { + if (typeof value === 'string') { + return containsGraphTokenPlaceholder(value); + } + if (Array.isArray(value)) { + return value.some(containsGraphTokenPlaceholder); + } + if (value == null || typeof value !== 'object') { + return false; + } + return Object.values(value).some(valueContainsGraphTokenPlaceholder); +} + /** - * Checks if MCP options contain the Graph token placeholder in headers, env, or url. + * Checks if MCP options contain the Graph token placeholder in connection fields. * @param options - The MCP options object * @returns True if any field contains the placeholder */ export function mcpOptionsContainGraphTokenPlaceholder(options: { + args?: string[]; headers?: Record; env?: Record; + oauth?: Record; + oauth_headers?: Record; url?: string; }): boolean { if (options.url && containsGraphTokenPlaceholder(options.url)) { return true; } + if (options.args?.some(containsGraphTokenPlaceholder)) { + return true; + } if (recordContainsGraphTokenPlaceholder(options.headers)) { return true; } if (recordContainsGraphTokenPlaceholder(options.env)) { return true; } - return false; + if (recordContainsGraphTokenPlaceholder(options.oauth_headers)) { + return true; + } + return valueContainsGraphTokenPlaceholder(options.oauth); } /** @@ -176,6 +207,42 @@ export async function resolveGraphTokensInRecord( return resolved; } +async function resolveGraphTokensInArray( + values: string[] | undefined, + options: GraphTokenOptions, +): Promise { + if (!values || !values.some(containsGraphTokenPlaceholder)) { + return values; + } + + const resolved: string[] = []; + for (const value of values) { + resolved.push(await resolveGraphTokenPlaceholder(value, options)); + } + return resolved; +} + +async function resolveGraphTokensInOAuth( + oauth: Record | undefined, + options: GraphTokenOptions, +): Promise | undefined> { + if (!oauth || !valueContainsGraphTokenPlaceholder(oauth)) { + return oauth; + } + + const resolved: Record = {}; + for (const [key, value] of Object.entries(oauth)) { + if (typeof value === 'string') { + resolved[key] = await resolveGraphTokenPlaceholder(value, options); + } else if (Array.isArray(value)) { + resolved[key] = await resolveGraphTokensInArray(value, options); + } else { + resolved[key] = value; + } + } + return resolved; +} + /** * Pre-processes MCP options to resolve Graph token placeholders. * This must be called before processMCPEnv since Graph token resolution is async. @@ -186,8 +253,11 @@ export async function resolveGraphTokensInRecord( */ export async function preProcessGraphTokens< T extends { + args?: string[]; headers?: Record; env?: Record; + oauth?: Record; + oauth_headers?: Record; url?: string; }, >(options: T, graphOptions: GraphTokenOptions): Promise { @@ -201,6 +271,10 @@ export async function preProcessGraphTokens< result.url = await resolveGraphTokenPlaceholder(result.url, graphOptions); } + if (result.args) { + result.args = await resolveGraphTokensInArray(result.args, graphOptions); + } + if (result.headers) { result.headers = await resolveGraphTokensInRecord(result.headers, graphOptions); } @@ -209,5 +283,13 @@ export async function preProcessGraphTokens< result.env = await resolveGraphTokensInRecord(result.env, graphOptions); } + if (result.oauth_headers) { + result.oauth_headers = await resolveGraphTokensInRecord(result.oauth_headers, graphOptions); + } + + if (result.oauth) { + result.oauth = await resolveGraphTokensInOAuth(result.oauth, graphOptions); + } + return result; }