diff --git a/packages/api/src/mcp/MCPConnectionFactory.ts b/packages/api/src/mcp/MCPConnectionFactory.ts index 8e093cc475..d331178443 100644 --- a/packages/api/src/mcp/MCPConnectionFactory.ts +++ b/packages/api/src/mcp/MCPConnectionFactory.ts @@ -245,13 +245,15 @@ export class MCPConnectionFactory { basic: t.BasicConnectionOptions, options?: t.OAuthConnectionOptions | t.UserConnectionContext, ) { - this.serverConfig = processMCPEnv({ - user: options?.user, - body: options?.requestBody, - dbSourced: basic.dbSourced, - options: basic.serverConfig, - customUserVars: options?.customUserVars, - }); + this.serverConfig = basic.skipEnvProcessing + ? basic.serverConfig + : processMCPEnv({ + user: options?.user, + body: options?.requestBody, + dbSourced: basic.dbSourced, + options: basic.serverConfig, + customUserVars: options?.customUserVars, + }); this.serverName = basic.serverName; this.useSSRFProtection = basic.useSSRFProtection === true; this.allowedDomains = basic.allowedDomains; diff --git a/packages/api/src/mcp/MCPManager.ts b/packages/api/src/mcp/MCPManager.ts index c33fade7aa..a3ce6bc333 100644 --- a/packages/api/src/mcp/MCPManager.ts +++ b/packages/api/src/mcp/MCPManager.ts @@ -355,6 +355,7 @@ Please follow these instructions when using tools from the respective MCP server serverName, serverConfig: currentOptions, dbSourced: isDbSourced, + skipEnvProcessing: true, useSSRFProtection: registry.shouldEnableSSRFProtection(), allowedDomains: registry.getAllowedDomains(), allowedAddresses: registry.getAllowedAddresses(), diff --git a/packages/api/src/mcp/__tests__/MCPConnectionFactory.test.ts b/packages/api/src/mcp/__tests__/MCPConnectionFactory.test.ts index 4a25c3522b..eb3440e708 100644 --- a/packages/api/src/mcp/__tests__/MCPConnectionFactory.test.ts +++ b/packages/api/src/mcp/__tests__/MCPConnectionFactory.test.ts @@ -1648,6 +1648,38 @@ describe('MCPConnectionFactory', () => { expect(mockMCPOAuthHandler.initiateOAuthFlow).toHaveBeenCalled(); }); + it('should not process already resolved configs for request OAuth handlers', () => { + const sseConfig = { + ...mockServerConfig, + url: 'https://api.example.com/${SHOULD_NOT_EXPAND}', + type: 'sse' as const, + } as t.SSEOptions; + mockProcessMCPEnv.mockClear(); + + const cleanup = MCPConnectionFactory.attachRequestOAuthHandler( + { + serverName: 'test-server', + serverConfig: sseConfig, + skipEnvProcessing: true, + }, + { + useOAuth: true, + user: mockUser, + flowManager: mockFlowManager, + oauthStart: jest.fn(), + }, + mockConnectionInstance, + ); + + expect(mockProcessMCPEnv).not.toHaveBeenCalled(); + expect(mockConnectionInstance.on).toHaveBeenCalledWith( + 'oauthReauthenticationRequired', + expect.any(Function), + ); + + cleanup(); + }); + it('should not reuse request-scoped OAuth callbacks after connection is cached', async () => { const sseConfig = { ...mockServerConfig, diff --git a/packages/api/src/mcp/__tests__/MCPManager.test.ts b/packages/api/src/mcp/__tests__/MCPManager.test.ts index cd30a75318..31af50364f 100644 --- a/packages/api/src/mcp/__tests__/MCPManager.test.ts +++ b/packages/api/src/mcp/__tests__/MCPManager.test.ts @@ -9,6 +9,7 @@ import { ConnectionsRepository } from '~/mcp/ConnectionsRepository'; import { MCPConnection } from '~/mcp/connection'; import { MCPManager } from '~/mcp/MCPManager'; import * as graphUtils from '~/utils/graph'; +import { processMCPEnv } from '~/utils/env'; // Mock external dependencies jest.mock('@librechat/data-schemas', () => ({ @@ -55,6 +56,7 @@ jest.mock('~/mcp/ConnectionsRepository'); jest.mock('~/mcp/MCPConnectionFactory'); const mockLogger = logger as jest.Mocked; +const mockProcessMCPEnv = processMCPEnv as jest.MockedFunction; describe('MCPManager', () => { const userId = 'test-user-123'; @@ -534,6 +536,65 @@ describe('MCPManager', () => { ); }); + it('should attach request OAuth handler without reprocessing resolved config', async () => { + const rawServerConfig = { + type: 'sse', + url: 'https://api.example.com/{{LIBRECHAT_USER_ID}}', + headers: { + Authorization: 'Bearer {{USER_TOKEN}}', + }, + requiresOAuth: true, + oauth: { + authorization_url: 'https://auth.example.com/authorize', + }, + } as t.ParsedServerConfig; + const processedServerConfig = { + ...rawServerConfig, + url: 'https://api.example.com/user-123', + headers: { + Authorization: 'Bearer ${SHOULD_NOT_EXPAND}', + }, + }; + const cleanupOAuthHandler = jest.fn(); + + mockProcessMCPEnv.mockReturnValueOnce(processedServerConfig); + (MCPConnectionFactory.attachRequestOAuthHandler as jest.Mock).mockReturnValue( + cleanupOAuthHandler, + ); + mockAppConnections({ + get: jest.fn().mockResolvedValue(mockConnection), + }); + (mockRegistryInstance.getServerConfig as jest.Mock).mockResolvedValue(rawServerConfig); + + const manager = await MCPManager.createInstance(newMCPServersConfig()); + const oauthStart = jest.fn(); + + await manager.callTool({ + user: mockUser as IUser, + serverName, + toolName: 'test_tool', + provider: 'openai', + oauthStart, + flowManager: mockFlowManager as unknown as Parameters< + typeof manager.callTool + >[0]['flowManager'], + }); + + expect(mockProcessMCPEnv).toHaveBeenCalledTimes(1); + expect(MCPConnectionFactory.attachRequestOAuthHandler).toHaveBeenCalledWith( + expect.objectContaining({ + serverConfig: processedServerConfig, + skipEnvProcessing: true, + }), + expect.objectContaining({ + oauthStart, + user: mockUser, + }), + mockConnection, + ); + expect(cleanupOAuthHandler).toHaveBeenCalled(); + }); + it('should pass options unchanged when no graphTokenResolver is provided', async () => { const serverConfig: t.SSEOptions = { type: 'sse', diff --git a/packages/api/src/mcp/types/index.ts b/packages/api/src/mcp/types/index.ts index ed85c13fa9..69322cd4a3 100644 --- a/packages/api/src/mcp/types/index.ts +++ b/packages/api/src/mcp/types/index.ts @@ -184,6 +184,8 @@ export interface BasicConnectionOptions { allowedAddresses?: string[] | null; /** When true, only resolve customUserVars in processMCPEnv (for DB-stored servers) */ dbSourced?: boolean; + /** When true, serverConfig has already gone through processMCPEnv for this request */ + skipEnvProcessing?: boolean; } /** User context for placeholder resolution in MCP connections (non-OAuth and OAuth alike) */