diff --git a/packages/api/src/mcp/ConnectionsRepository.ts b/packages/api/src/mcp/ConnectionsRepository.ts index e629934dda..970e7ea4b9 100644 --- a/packages/api/src/mcp/ConnectionsRepository.ts +++ b/packages/api/src/mcp/ConnectionsRepository.ts @@ -1,8 +1,9 @@ import { logger } from '@librechat/data-schemas'; -import { MCPConnectionFactory } from '~/mcp/MCPConnectionFactory'; -import { MCPConnection } from './connection'; -import { MCPServersRegistry } from '~/mcp/registry/MCPServersRegistry'; import type * as t from './types'; +import { MCPServersRegistry } from '~/mcp/registry/MCPServersRegistry'; +import { MCPConnectionFactory } from '~/mcp/MCPConnectionFactory'; +import { hasCustomUserVars } from './utils'; +import { MCPConnection } from './connection'; const CONNECT_CONCURRENCY = 3; @@ -139,12 +140,19 @@ export class ConnectionsRepository { return `[MCP][${serverName}]`; } + /** + * App-level (shared) connections cannot serve servers that need per-user context: + * env/header placeholders like `{{MY_KEY}}` are only resolved by `processMCPEnv()` + * when real `customUserVars` values exist — which requires a user-level connection. + */ private isAllowedToConnectToServer(config: t.ParsedServerConfig) { if (config.inspectionFailed) { return false; } - //the repository is not allowed to be connected in case the Connection repository is shared (ownerId is undefined/null) and the server requires Auth or startup false. - if (this.ownerId === undefined && (config.startup === false || config.requiresOAuth)) { + if ( + this.ownerId === undefined && + (config.startup === false || config.requiresOAuth || hasCustomUserVars(config)) + ) { return false; } return true; diff --git a/packages/api/src/mcp/__tests__/ConnectionsRepository.test.ts b/packages/api/src/mcp/__tests__/ConnectionsRepository.test.ts index 3b827774d0..98e15eca18 100644 --- a/packages/api/src/mcp/__tests__/ConnectionsRepository.test.ts +++ b/packages/api/src/mcp/__tests__/ConnectionsRepository.test.ts @@ -392,6 +392,36 @@ describe('ConnectionsRepository', () => { expect(await repository.has('oauthDisabledServer')).toBe(false); }); + it('should NOT allow connection to servers with customUserVars', async () => { + mockServerConfigs.customVarServer = { + type: 'stdio', + command: 'npx', + args: ['-y', '@test/mcp-stdio-server'], + env: { API_KEY: '{{MY_KEY}}' }, + customUserVars: { + MY_KEY: { title: 'API Key', description: 'Your API key' }, + }, + }; + + expect(await repository.has('customVarServer')).toBe(false); + }); + + it('should NOT allow connection when customUserVars is defined, even when startup is explicitly true', async () => { + mockServerConfigs.customVarStartupServer = { + type: 'stdio', + command: 'npx', + args: ['-y', '@test/mcp-stdio-server'], + env: { TOKEN: '{{USER_TOKEN}}' }, + startup: true, + requiresOAuth: false, + customUserVars: { + USER_TOKEN: { title: 'Token', description: 'Your token' }, + }, + }; + + expect(await repository.has('customVarStartupServer')).toBe(false); + }); + it('should disconnect existing connection when server becomes not allowed', async () => { // Initially setup as regular server mockServerConfigs.changingServer = { @@ -471,6 +501,20 @@ describe('ConnectionsRepository', () => { expect(await repository.has('oauthDisabledServer')).toBe(true); }); + it('should allow connection to servers with customUserVars', async () => { + mockServerConfigs.customVarServer = { + type: 'stdio', + command: 'npx', + args: ['-y', '@test/mcp-stdio-server'], + env: { API_KEY: '{{MY_KEY}}' }, + customUserVars: { + MY_KEY: { title: 'API Key', description: 'Your API key' }, + }, + }; + + expect(await repository.has('customVarServer')).toBe(true); + }); + it('should return null from get() when server config does not exist', async () => { const connection = await repository.get('nonexistent'); expect(connection).toBeNull(); diff --git a/packages/api/src/mcp/registry/MCPServerInspector.ts b/packages/api/src/mcp/registry/MCPServerInspector.ts index eea52bbf2e..a477d9b412 100644 --- a/packages/api/src/mcp/registry/MCPServerInspector.ts +++ b/packages/api/src/mcp/registry/MCPServerInspector.ts @@ -6,6 +6,7 @@ import { isMCPDomainAllowed, extractMCPServerDomain } from '~/auth/domain'; import { MCPConnectionFactory } from '~/mcp/MCPConnectionFactory'; import { MCPDomainNotAllowedError } from '~/mcp/errors'; import { detectOAuthRequirement } from '~/mcp/oauth'; +import { hasCustomUserVars } from '~/mcp/utils'; import { isEnabled } from '~/utils'; /** @@ -54,7 +55,11 @@ export class MCPServerInspector { private async inspectServer(): Promise { await this.detectOAuth(); - if (this.config.startup !== false && !this.config.requiresOAuth) { + if ( + this.config.startup !== false && + !this.config.requiresOAuth && + !hasCustomUserVars(this.config) + ) { let tempConnection = false; if (!this.connection) { tempConnection = true; diff --git a/packages/api/src/mcp/registry/__tests__/MCPServerInspector.test.ts b/packages/api/src/mcp/registry/__tests__/MCPServerInspector.test.ts index b79f2d044a..f0ab75c9b4 100644 --- a/packages/api/src/mcp/registry/__tests__/MCPServerInspector.test.ts +++ b/packages/api/src/mcp/registry/__tests__/MCPServerInspector.test.ts @@ -100,6 +100,54 @@ describe('MCPServerInspector', () => { }); }); + it('should skip capabilities fetch when customUserVars is defined', async () => { + const rawConfig: t.MCPOptions = { + type: 'stdio', + command: 'npx', + args: ['-y', '@test/mcp-stdio-server'], + env: { API_KEY: '{{MY_KEY}}' }, + customUserVars: { + MY_KEY: { title: 'API Key', description: 'Your API key' }, + }, + }; + + const result = await MCPServerInspector.inspect('test_server', rawConfig, mockConnection); + + expect(result).toEqual({ + type: 'stdio', + command: 'npx', + args: ['-y', '@test/mcp-stdio-server'], + env: { API_KEY: '{{MY_KEY}}' }, + customUserVars: { + MY_KEY: { title: 'API Key', description: 'Your API key' }, + }, + requiresOAuth: false, + initDuration: expect.any(Number), + }); + + expect(MCPConnectionFactory.create).not.toHaveBeenCalled(); + expect(mockConnection.disconnect).not.toHaveBeenCalled(); + }); + + it('should NOT create a temp connection when customUserVars is defined and no connection is provided', async () => { + const rawConfig: t.MCPOptions = { + type: 'stdio', + command: 'npx', + args: ['-y', '@test/mcp-stdio-server'], + env: { API_KEY: '{{MY_KEY}}' }, + customUserVars: { + MY_KEY: { title: 'API Key', description: 'Your API key' }, + }, + }; + + const result = await MCPServerInspector.inspect('test_server', rawConfig); + + expect(MCPConnectionFactory.create).not.toHaveBeenCalled(); + expect(result.requiresOAuth).toBe(false); + expect(result.capabilities).toBeUndefined(); + expect(result.toolFunctions).toBeUndefined(); + }); + it('should keep custom serverInstructions string and not fetch from server', async () => { const rawConfig: t.MCPOptions = { type: 'stdio', diff --git a/packages/api/src/mcp/utils.ts b/packages/api/src/mcp/utils.ts index c517388a76..ff367725fc 100644 --- a/packages/api/src/mcp/utils.ts +++ b/packages/api/src/mcp/utils.ts @@ -3,6 +3,11 @@ import type { ParsedServerConfig } from '~/mcp/types'; export const mcpToolPattern = new RegExp(`^.+${Constants.mcp_delimiter}.+$`); +/** Checks that `customUserVars` is present AND non-empty (guards against truthy `{}`) */ +export function hasCustomUserVars(config: Pick): boolean { + return !!config.customUserVars && Object.keys(config.customUserVars).length > 0; +} + /** * Allowlist-based sanitization for API responses. Only explicitly listed fields are included; * new fields added to ParsedServerConfig are excluded by default until allowlisted here.