💁 refactor: Better Config UX for MCP STDIO with customUserVars (#12226)

* refactor: Better UX for MCP stdio with Custom User Variables

- Updated the ConnectionsRepository to prevent connections when customUserVars are defined, improving security and access control.
- Modified the MCPServerInspector to skip capabilities fetch when customUserVars are present, streamlining server inspection.
- Added tests to validate connection restrictions with customUserVars, ensuring robust handling of various server configurations.

This change enhances the overall integrity of the connection management process by enforcing stricter rules around custom user variables.

* fix: guard against empty customUserVars and add JSDoc context

- Extract `hasCustomUserVars()` helper to guard against truthy `{}`
  (Zod's `.record().optional()` yields `{}` on empty input, not `undefined`)
- Add JSDoc to `isAllowedToConnectToServer` explaining why customUserVars
  servers are excluded from app-level connections

* test: improve customUserVars test coverage and fixture hygiene

- Add no-connection-provided test for MCPServerInspector (production path)
- Fix test descriptions to match actual fixture values
- Replace real package name with fictional @test/mcp-stdio-server
This commit is contained in:
Danny Avila 2026-03-14 21:22:25 -04:00 committed by GitHub
parent 7bc793b18d
commit 8318446704
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 116 additions and 6 deletions

View file

@ -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;

View file

@ -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();

View file

@ -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<void> {
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;

View file

@ -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',

View file

@ -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<ParsedServerConfig, 'customUserVars'>): 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.