diff --git a/packages/api/src/mcp/MCPConnectionFactory.ts b/packages/api/src/mcp/MCPConnectionFactory.ts index 2c16da0760..337662c812 100644 --- a/packages/api/src/mcp/MCPConnectionFactory.ts +++ b/packages/api/src/mcp/MCPConnectionFactory.ts @@ -58,9 +58,13 @@ export class MCPConnectionFactory { */ static async discoverTools( basic: t.BasicConnectionOptions, - oauth?: Omit, + options?: Omit | t.UserConnectionContext, ): Promise { - const factory = new this(basic, oauth ? { ...oauth, returnOnOAuth: true } : undefined); + if (options != null && 'useOAuth' in options) { + const factory = new this(basic, { ...options, returnOnOAuth: true }); + return factory.discoverToolsInternal(); + } + const factory = new this(basic, options); return factory.discoverToolsInternal(); } @@ -187,31 +191,36 @@ export class MCPConnectionFactory { return null; } - protected constructor(basic: t.BasicConnectionOptions, oauth?: t.OAuthConnectionOptions) { + protected constructor( + basic: t.BasicConnectionOptions, + options?: t.OAuthConnectionOptions | t.UserConnectionContext, + ) { this.serverConfig = processMCPEnv({ - user: oauth?.user, - body: oauth?.requestBody, + user: options?.user, + body: options?.requestBody, dbSourced: basic.dbSourced, options: basic.serverConfig, - customUserVars: oauth?.customUserVars, + customUserVars: options?.customUserVars, }); this.serverName = basic.serverName; - this.useOAuth = !!oauth?.useOAuth; this.useSSRFProtection = basic.useSSRFProtection === true; this.allowedDomains = basic.allowedDomains; - this.connectionTimeout = oauth?.connectionTimeout; - this.logPrefix = oauth?.user - ? `[MCP][${basic.serverName}][${oauth.user.id}]` + this.connectionTimeout = options?.connectionTimeout; + this.logPrefix = options?.user + ? `[MCP][${basic.serverName}][${options.user.id}]` : `[MCP][${basic.serverName}]`; - if (oauth?.useOAuth) { - this.userId = oauth.user?.id; - this.flowManager = oauth.flowManager; - this.tokenMethods = oauth.tokenMethods; - this.signal = oauth.signal; - this.oauthStart = oauth.oauthStart; - this.oauthEnd = oauth.oauthEnd; - this.returnOnOAuth = oauth.returnOnOAuth; + if (options != null && 'useOAuth' in options) { + this.useOAuth = true; + this.userId = options.user?.id; + this.flowManager = options.flowManager; + this.tokenMethods = options.tokenMethods; + this.signal = options.signal; + this.oauthStart = options.oauthStart; + this.oauthEnd = options.oauthEnd; + this.returnOnOAuth = options.returnOnOAuth; + } else { + this.useOAuth = false; } } diff --git a/packages/api/src/mcp/MCPManager.ts b/packages/api/src/mcp/MCPManager.ts index afb6c68796..935307fa49 100644 --- a/packages/api/src/mcp/MCPManager.ts +++ b/packages/api/src/mcp/MCPManager.ts @@ -113,7 +113,12 @@ export class MCPManager extends UserConnectionManager { }; if (!useOAuth) { - const result = await MCPConnectionFactory.discoverTools(basic); + const result = await MCPConnectionFactory.discoverTools(basic, { + user: args.user, + customUserVars: args.customUserVars, + requestBody: args.requestBody, + connectionTimeout: args.connectionTimeout, + }); return { tools: result.tools, oauthRequired: result.oauthRequired, diff --git a/packages/api/src/mcp/__tests__/MCPConnectionFactory.test.ts b/packages/api/src/mcp/__tests__/MCPConnectionFactory.test.ts index 23bfa89d56..75d7b4321d 100644 --- a/packages/api/src/mcp/__tests__/MCPConnectionFactory.test.ts +++ b/packages/api/src/mcp/__tests__/MCPConnectionFactory.test.ts @@ -764,6 +764,39 @@ describe('MCPConnectionFactory', () => { expect(result.connection).toBe(mockConnectionInstance); }); + it('should forward user context to processMCPEnv for non-OAuth discovery', async () => { + const serverConfig: t.MCPOptions = { + type: 'streamable-http', + url: 'https://my-mcp.server.com?key={{MY_CUSTOM_KEY}}', + } as t.MCPOptions; + + const basicOptions = { + serverName: 'test-server', + serverConfig, + }; + + const userContext = { + user: mockUser, + customUserVars: { MY_CUSTOM_KEY: 'c527bd0abc123' }, + connectionTimeout: 10000, + }; + + mockConnectionInstance.connect.mockResolvedValue(undefined); + mockConnectionInstance.isConnected.mockResolvedValue(true); + mockConnectionInstance.fetchTools = jest.fn().mockResolvedValue(mockTools); + + const result = await MCPConnectionFactory.discoverTools(basicOptions, userContext); + + expect(result.tools).toEqual(mockTools); + expect(mockProcessMCPEnv).toHaveBeenCalledWith( + expect.objectContaining({ + user: mockUser, + options: serverConfig, + customUserVars: { MY_CUSTOM_KEY: 'c527bd0abc123' }, + }), + ); + }); + it('should detect OAuth required without generating URL in discovery mode', 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 dd1ead0dd9..ba5b0b3b8e 100644 --- a/packages/api/src/mcp/__tests__/MCPManager.test.ts +++ b/packages/api/src/mcp/__tests__/MCPManager.test.ts @@ -847,6 +847,46 @@ describe('MCPManager', () => { expect(MCPConnectionFactory.discoverTools).toHaveBeenCalled(); }); + it('should forward user, customUserVars, requestBody, and connectionTimeout 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' }; + + mockAppConnections({ + get: jest.fn().mockResolvedValue(null), + }); + + (mockRegistryInstance.getServerConfig as jest.Mock).mockResolvedValue({ + type: 'streamable-http', + url: 'https://my-mcp.server.com?key={{MY_CUSTOM_KEY}}', + }); + + (MCPConnectionFactory.discoverTools as jest.Mock).mockResolvedValue({ + tools: mockTools, + connection: null, + oauthRequired: false, + oauthUrl: null, + }); + + const manager = await MCPManager.createInstance(newMCPServersConfig()); + await manager.discoverServerTools({ + serverName, + user: mockUser, + customUserVars, + requestBody: { conversationId: 'conv-123' } as t.ToolDiscoveryOptions['requestBody'], + connectionTimeout: 10000, + }); + + expect(MCPConnectionFactory.discoverTools).toHaveBeenCalledWith( + expect.objectContaining({ serverName }), + expect.objectContaining({ + user: mockUser, + customUserVars, + requestBody: { conversationId: 'conv-123' }, + connectionTimeout: 10000, + }), + ); + }); + it('should return null tools when server config not found', async () => { mockAppConnections({ get: jest.fn().mockResolvedValue(null), diff --git a/packages/api/src/mcp/__tests__/customUserVars.integration.test.ts b/packages/api/src/mcp/__tests__/customUserVars.integration.test.ts new file mode 100644 index 0000000000..35e087be04 --- /dev/null +++ b/packages/api/src/mcp/__tests__/customUserVars.integration.test.ts @@ -0,0 +1,74 @@ +/** + * Integration test exercising real processMCPEnv for the non-OAuth + * customUserVars scenario: a streamable-http server whose URL contains + * a {{PLACEHOLDER}} that must be resolved from per-user custom variables. + * + * This is the exact bug scenario from PR #12348 — without the fix, + * the literal string `{{MY_CUSTOM_KEY}}` would be sent to the MCP + * server endpoint instead of the substituted value. + */ +import type { IUser } from '@librechat/data-schemas'; +import type * as t from '~/mcp/types'; +import { processMCPEnv } from '~/utils/env'; + +describe('processMCPEnv — customUserVars placeholder resolution', () => { + const mockUser = { id: 'user-abc', email: 'test@example.com' } as IUser; + + it('should resolve {{CUSTOM_VAR}} in a streamable-http URL', () => { + const serverConfig: t.MCPOptions = { + type: 'streamable-http', + url: 'https://my-mcp.server.com/server?key={{MY_CUSTOM_KEY}}', + } as t.MCPOptions; + + const result = processMCPEnv({ + options: serverConfig, + user: mockUser, + customUserVars: { MY_CUSTOM_KEY: 'c527bd0abc123' }, + }); + + expect((result as t.StreamableHTTPOptions).url).toBe( + 'https://my-mcp.server.com/server?key=c527bd0abc123', + ); + }); + + it('should resolve multiple placeholders in URL and headers simultaneously', () => { + const serverConfig: t.MCPOptions = { + type: 'streamable-http', + url: 'https://my-mcp.server.com/server?key={{API_KEY}}&project={{PROJECT_ID}}', + headers: { + Authorization: 'Bearer {{AUTH_TOKEN}}', + 'X-Project': '{{PROJECT_ID}}', + }, + } as t.MCPOptions; + + const result = processMCPEnv({ + options: serverConfig, + user: mockUser, + customUserVars: { + API_KEY: 'key-123', + PROJECT_ID: 'proj-456', + AUTH_TOKEN: 'tok-789', + }, + }); + + const typed = result as t.StreamableHTTPOptions; + expect(typed.url).toBe('https://my-mcp.server.com/server?key=key-123&project=proj-456'); + expect(typed.headers).toEqual({ + Authorization: 'Bearer tok-789', + 'X-Project': 'proj-456', + }); + }); + + it('should leave unmatched placeholders as literal strings when customUserVars is undefined', () => { + const serverConfig: t.MCPOptions = { + type: 'streamable-http', + url: 'https://my-mcp.server.com/server?key={{MY_CUSTOM_KEY}}', + } as t.MCPOptions; + + const result = processMCPEnv({ + options: serverConfig, + }); + + expect((result as t.StreamableHTTPOptions).url).toContain('{{MY_CUSTOM_KEY}}'); + }); +}); diff --git a/packages/api/src/mcp/types/index.ts b/packages/api/src/mcp/types/index.ts index 0af10c7399..9d43aa543d 100644 --- a/packages/api/src/mcp/types/index.ts +++ b/packages/api/src/mcp/types/index.ts @@ -174,18 +174,22 @@ export interface BasicConnectionOptions { dbSourced?: boolean; } -export interface OAuthConnectionOptions { +/** User context for placeholder resolution in MCP connections (non-OAuth and OAuth alike) */ +export interface UserConnectionContext { user?: IUser; - useOAuth: true; - requestBody?: RequestBody; customUserVars?: Record; + requestBody?: RequestBody; + connectionTimeout?: number; +} + +export interface OAuthConnectionOptions extends UserConnectionContext { + useOAuth: true; flowManager: FlowStateManager; tokenMethods?: TokenMethods; signal?: AbortSignal; oauthStart?: (authURL: string) => Promise; oauthEnd?: () => Promise; returnOnOAuth?: boolean; - connectionTimeout?: number; } export interface ToolDiscoveryOptions {