mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-03-13 11:26:18 +01:00
🗝️ feat: Credential Variables for DB-Sourced MCP Servers (#12044)
* feat: Allow Credential Variables in Headers for DB-sourced MCP Servers - Removed the hasCustomUserVars check from ToolService.js, directly retrieving userMCPAuthMap. - Updated MCPConnectionFactory and related classes to include a dbSourced flag for better handling of database-sourced configurations. - Added integration tests to ensure proper behavior of dbSourced servers, verifying that sensitive placeholders are not resolved while allowing customUserVars. - Adjusted various MCP-related files to accommodate the new dbSourced logic, ensuring consistent handling across the codebase. * chore: MCPConnectionFactory Tests with Additional Flow Metadata for typing - Updated MCPConnectionFactory tests to include new fields in flowMetadata: serverUrl and state. - Enhanced mockFlowData in multiple test cases to reflect the updated structure, ensuring comprehensive coverage of the OAuth flow scenarios. - Added authorization_endpoint to metadata in the test setup for improved validation of the OAuth process. * refactor: Simplify MCPManager Configuration Handling - Removed unnecessary type assertions and streamlined the retrieval of server configuration in MCPManager. - Enhanced the handling of OAuth and database-sourced flags for improved clarity and efficiency. - Updated tests to reflect changes in user object structure and ensure proper processing of MCP environment variables. * refactor: Optimize User MCP Auth Map Retrieval in ToolService - Introduced conditional loading of userMCPAuthMap based on the presence of MCP-delimited tools, improving efficiency by avoiding unnecessary calls. - Updated the loadToolDefinitionsWrapper and loadAgentTools functions to reflect this change, enhancing overall performance and clarity. * test: Add userMCPAuthMap gating tests in ToolService - Introduced new tests to validate the logic for determining if MCP tools are present in the agent's tool list. - Implemented various scenarios to ensure accurate detection of MCP tools, including edge cases for empty, undefined, and null tool lists. - Enhanced clarity and coverage of the ToolService capability checking logic. * refactor: Enhance MCP Environment Variable Processing - Simplified the handling of the dbSourced parameter in the processMCPEnv function. - Introduced a failsafe mechanism to derive dbSourced from options if not explicitly provided, improving robustness and clarity in MCP environment variable processing. * refactor: Update Regex Patterns for Credential Placeholders in ServerConfigsDB - Modified regex patterns to include additional credential/env placeholders that should not be allowed in user-provided configurations. - Clarified comments to emphasize the security risks associated with credential exfiltration when MCP servers are shared between users. * chore: field order * refactor: Clean Up dbSourced Parameter Handling in processMCPEnv - Reintroduced the failsafe mechanism for deriving the dbSourced parameter from options, ensuring clarity and robustness in MCP environment variable processing. - Enhanced code readability by maintaining consistent comment structure. * refactor: Update MCPOptions Type to Include Optional dbId - Modified the processMCPEnv function to extend the MCPOptions type, allowing for an optional dbId property. - Simplified the logic for deriving the dbSourced parameter by directly checking the dbId property, enhancing code clarity and maintainability.
This commit is contained in:
parent
a2a09b556a
commit
d3c06052d7
16 changed files with 1060 additions and 70 deletions
|
|
@ -80,6 +80,7 @@ export class ConnectionsRepository {
|
|||
{
|
||||
serverName,
|
||||
serverConfig,
|
||||
dbSourced: !!(serverConfig as t.ParsedServerConfig).dbId,
|
||||
useSSRFProtection: MCPServersRegistry.getInstance().shouldEnableSSRFProtection(),
|
||||
},
|
||||
this.oauthOpts,
|
||||
|
|
|
|||
|
|
@ -185,10 +185,11 @@ export class MCPConnectionFactory {
|
|||
|
||||
protected constructor(basic: t.BasicConnectionOptions, oauth?: t.OAuthConnectionOptions) {
|
||||
this.serverConfig = processMCPEnv({
|
||||
options: basic.serverConfig,
|
||||
user: oauth?.user,
|
||||
customUserVars: oauth?.customUserVars,
|
||||
body: oauth?.requestBody,
|
||||
dbSourced: basic.dbSourced,
|
||||
options: basic.serverConfig,
|
||||
customUserVars: oauth?.customUserVars,
|
||||
});
|
||||
this.serverName = basic.serverName;
|
||||
this.useOAuth = !!oauth?.useOAuth;
|
||||
|
|
|
|||
|
|
@ -88,22 +88,26 @@ export class MCPManager extends UserConnectionManager {
|
|||
logger.debug(`${logPrefix} [Discovery] App connection not available, trying discovery mode`);
|
||||
}
|
||||
|
||||
const serverConfig = (await MCPServersRegistry.getInstance().getServerConfig(
|
||||
const serverConfig = await MCPServersRegistry.getInstance().getServerConfig(
|
||||
serverName,
|
||||
user?.id,
|
||||
)) as t.MCPOptions | null;
|
||||
);
|
||||
|
||||
if (!serverConfig) {
|
||||
logger.warn(`${logPrefix} [Discovery] Server config not found`);
|
||||
return { tools: null, oauthRequired: false, oauthUrl: null };
|
||||
}
|
||||
|
||||
const useOAuth = Boolean(
|
||||
serverConfig.requiresOAuth || (serverConfig as t.ParsedServerConfig).oauthMetadata,
|
||||
);
|
||||
const useOAuth = Boolean(serverConfig.requiresOAuth || serverConfig.oauthMetadata);
|
||||
|
||||
const useSSRFProtection = MCPServersRegistry.getInstance().shouldEnableSSRFProtection();
|
||||
const basic: t.BasicConnectionOptions = { serverName, serverConfig, useSSRFProtection };
|
||||
const dbSourced = !!serverConfig.dbId;
|
||||
const basic: t.BasicConnectionOptions = {
|
||||
dbSourced,
|
||||
serverName,
|
||||
serverConfig,
|
||||
useSSRFProtection,
|
||||
};
|
||||
|
||||
if (!useOAuth) {
|
||||
const result = await MCPConnectionFactory.discoverTools(basic);
|
||||
|
|
@ -290,22 +294,23 @@ Please follow these instructions when using tools from the respective MCP server
|
|||
);
|
||||
}
|
||||
|
||||
const rawConfig = (await MCPServersRegistry.getInstance().getServerConfig(
|
||||
serverName,
|
||||
userId,
|
||||
)) as t.MCPOptions;
|
||||
const rawConfig = await MCPServersRegistry.getInstance().getServerConfig(serverName, userId);
|
||||
const isDbSourced = !!rawConfig?.dbId;
|
||||
|
||||
// Pre-process Graph token placeholders (async) before sync processMCPEnv
|
||||
const graphProcessedConfig = await preProcessGraphTokens(rawConfig, {
|
||||
user,
|
||||
graphTokenResolver,
|
||||
scopes: process.env.GRAPH_API_SCOPES,
|
||||
});
|
||||
/** Pre-process Graph token placeholders (async) before the synchronous processMCPEnv pass */
|
||||
const graphProcessedConfig = isDbSourced
|
||||
? (rawConfig as t.MCPOptions)
|
||||
: await preProcessGraphTokens(rawConfig as t.MCPOptions, {
|
||||
user,
|
||||
graphTokenResolver,
|
||||
scopes: process.env.GRAPH_API_SCOPES,
|
||||
});
|
||||
const currentOptions = processMCPEnv({
|
||||
user,
|
||||
options: graphProcessedConfig,
|
||||
customUserVars: customUserVars,
|
||||
body: requestBody,
|
||||
dbSourced: isDbSourced,
|
||||
options: graphProcessedConfig,
|
||||
customUserVars,
|
||||
});
|
||||
if ('headers' in currentOptions) {
|
||||
connection.setRequestHeaders(currentOptions.headers || {});
|
||||
|
|
|
|||
|
|
@ -115,8 +115,9 @@ export abstract class UserConnectionManager {
|
|||
try {
|
||||
connection = await MCPConnectionFactory.create(
|
||||
{
|
||||
serverName: serverName,
|
||||
serverConfig: config,
|
||||
serverName: serverName,
|
||||
dbSourced: !!config.dbId,
|
||||
useSSRFProtection: MCPServersRegistry.getInstance().shouldEnableSSRFProtection(),
|
||||
},
|
||||
{
|
||||
|
|
|
|||
|
|
@ -1,8 +1,8 @@
|
|||
import { logger } from '@librechat/data-schemas';
|
||||
import { ConnectionsRepository } from '../ConnectionsRepository';
|
||||
import { MCPConnectionFactory } from '../MCPConnectionFactory';
|
||||
import { MCPConnection } from '../connection';
|
||||
import type * as t from '../types';
|
||||
import type * as t from '~/mcp/types';
|
||||
import { ConnectionsRepository } from '~/mcp/ConnectionsRepository';
|
||||
import { MCPConnectionFactory } from '~/mcp/MCPConnectionFactory';
|
||||
import { MCPConnection } from '~/mcp/connection';
|
||||
|
||||
// Mock external dependencies
|
||||
jest.mock('@librechat/data-schemas', () => ({
|
||||
|
|
@ -110,6 +110,7 @@ describe('ConnectionsRepository', () => {
|
|||
serverName: 'server1',
|
||||
serverConfig: mockServerConfigs.server1,
|
||||
useSSRFProtection: false,
|
||||
dbSourced: false,
|
||||
},
|
||||
undefined,
|
||||
);
|
||||
|
|
@ -132,6 +133,7 @@ describe('ConnectionsRepository', () => {
|
|||
serverName: 'server1',
|
||||
serverConfig: mockServerConfigs.server1,
|
||||
useSSRFProtection: false,
|
||||
dbSourced: false,
|
||||
},
|
||||
undefined,
|
||||
);
|
||||
|
|
@ -171,6 +173,7 @@ describe('ConnectionsRepository', () => {
|
|||
serverName: 'server1',
|
||||
serverConfig: configWithCachedAt,
|
||||
useSSRFProtection: false,
|
||||
dbSourced: false,
|
||||
},
|
||||
undefined,
|
||||
);
|
||||
|
|
|
|||
|
|
@ -79,7 +79,10 @@ describe('MCPConnectionFactory', () => {
|
|||
const connection = await MCPConnectionFactory.create(basicOptions);
|
||||
|
||||
expect(connection).toBe(mockConnectionInstance);
|
||||
expect(mockProcessMCPEnv).toHaveBeenCalledWith({ options: mockServerConfig });
|
||||
expect(mockProcessMCPEnv).toHaveBeenCalledWith({
|
||||
options: mockServerConfig,
|
||||
dbSourced: undefined,
|
||||
});
|
||||
expect(mockMCPConnection).toHaveBeenCalledWith({
|
||||
serverName: 'test-server',
|
||||
serverConfig: mockServerConfig,
|
||||
|
|
@ -121,7 +124,11 @@ describe('MCPConnectionFactory', () => {
|
|||
const connection = await MCPConnectionFactory.create(basicOptions, oauthOptions);
|
||||
|
||||
expect(connection).toBe(mockConnectionInstance);
|
||||
expect(mockProcessMCPEnv).toHaveBeenCalledWith({ options: mockServerConfig, user: mockUser });
|
||||
expect(mockProcessMCPEnv).toHaveBeenCalledWith({
|
||||
options: mockServerConfig,
|
||||
user: mockUser,
|
||||
dbSourced: undefined,
|
||||
});
|
||||
expect(mockMCPConnection).toHaveBeenCalledWith({
|
||||
serverName: 'test-server',
|
||||
serverConfig: mockServerConfig,
|
||||
|
|
@ -358,7 +365,12 @@ describe('MCPConnectionFactory', () => {
|
|||
const mockFlowData = {
|
||||
authorizationUrl: 'https://auth.example.com',
|
||||
flowId: 'flow123',
|
||||
flowMetadata: { serverName: 'test-server', userId: 'user123' },
|
||||
flowMetadata: {
|
||||
serverName: 'test-server',
|
||||
userId: 'user123',
|
||||
serverUrl: 'https://api.example.com',
|
||||
state: 'state123',
|
||||
},
|
||||
};
|
||||
|
||||
mockMCPOAuthHandler.initiateOAuthFlow.mockResolvedValue(mockFlowData);
|
||||
|
|
@ -419,7 +431,12 @@ describe('MCPConnectionFactory', () => {
|
|||
const mockFlowData = {
|
||||
authorizationUrl: 'https://auth.example.com',
|
||||
flowId: 'flow123',
|
||||
flowMetadata: { serverName: 'test-server', userId: 'user123' },
|
||||
flowMetadata: {
|
||||
serverName: 'test-server',
|
||||
userId: 'user123',
|
||||
serverUrl: 'https://api.example.com',
|
||||
state: 'state123',
|
||||
},
|
||||
};
|
||||
|
||||
mockMCPOAuthHandler.initiateOAuthFlow.mockResolvedValue(mockFlowData);
|
||||
|
|
@ -491,7 +508,10 @@ describe('MCPConnectionFactory', () => {
|
|||
serverUrl: 'https://api.example.com',
|
||||
state: 'random-state',
|
||||
clientInfo: { client_id: 'client123' },
|
||||
metadata: { token_endpoint: 'https://auth.example.com/token' },
|
||||
metadata: {
|
||||
token_endpoint: 'https://auth.example.com/token',
|
||||
authorization_endpoint: 'https://auth.example.com/authorize',
|
||||
},
|
||||
},
|
||||
};
|
||||
|
||||
|
|
|
|||
517
packages/api/src/mcp/__tests__/dbSourced.integration.test.ts
Normal file
517
packages/api/src/mcp/__tests__/dbSourced.integration.test.ts
Normal file
|
|
@ -0,0 +1,517 @@
|
|||
/**
|
||||
* Integration tests for the `dbSourced` security boundary.
|
||||
*
|
||||
* These tests spin up real in-process MCP servers using the SDK's
|
||||
* StreamableHTTPServerTransport, then exercise the full
|
||||
* processMCPEnv → MCPConnection → HTTP request pipeline to verify:
|
||||
*
|
||||
* 1. DB-sourced servers resolve `{{MCP_API_KEY}}` via customUserVars.
|
||||
* 2. DB-sourced servers do NOT leak `${ENV_VAR}` from process.env.
|
||||
* 3. DB-sourced servers do NOT resolve `{{LIBRECHAT_USER_*}}` placeholders.
|
||||
* 4. DB-sourced servers do NOT resolve `{{LIBRECHAT_BODY_*}}` placeholders.
|
||||
* 5. YAML-sourced servers (dbSourced=false) resolve ALL placeholder types.
|
||||
* 6. Mixed headers: some placeholders resolve, others are blocked.
|
||||
*/
|
||||
|
||||
import * as net from 'net';
|
||||
import * as http from 'http';
|
||||
import { Agent } from 'undici';
|
||||
import { Types } from 'mongoose';
|
||||
import { randomUUID } from 'crypto';
|
||||
import { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js';
|
||||
import { StreamableHTTPServerTransport } from '@modelcontextprotocol/sdk/server/streamableHttp.js';
|
||||
import type { MCPOptions } from 'librechat-data-provider';
|
||||
import type { IUser } from '@librechat/data-schemas';
|
||||
import type { Socket } from 'net';
|
||||
import { MCPConnection } from '~/mcp/connection';
|
||||
import { processMCPEnv } from '~/utils/env';
|
||||
|
||||
jest.mock('@librechat/data-schemas', () => ({
|
||||
logger: {
|
||||
info: jest.fn(),
|
||||
warn: jest.fn(),
|
||||
error: jest.fn(),
|
||||
debug: jest.fn(),
|
||||
},
|
||||
}));
|
||||
|
||||
jest.mock('~/auth', () => ({
|
||||
createSSRFSafeUndiciConnect: jest.fn(() => undefined),
|
||||
resolveHostnameSSRF: jest.fn(async () => false),
|
||||
}));
|
||||
|
||||
jest.mock('~/mcp/mcpConfig', () => ({
|
||||
mcpConfig: { CONNECTION_CHECK_TTL: 0 },
|
||||
}));
|
||||
|
||||
/** Track all Agents for cleanup */
|
||||
const allAgentsCreated: Agent[] = [];
|
||||
const OriginalAgent = Agent;
|
||||
const PatchedAgent = new Proxy(OriginalAgent, {
|
||||
construct(target, args) {
|
||||
const instance = new target(...(args as [Agent.Options?]));
|
||||
allAgentsCreated.push(instance);
|
||||
return instance;
|
||||
},
|
||||
});
|
||||
(global as Record<string, unknown>).__undiciAgent = PatchedAgent;
|
||||
|
||||
afterAll(async () => {
|
||||
const destroying = allAgentsCreated.map((a) => {
|
||||
if (!a.destroyed && !a.closed) {
|
||||
return a.destroy().catch(() => undefined);
|
||||
}
|
||||
return Promise.resolve();
|
||||
});
|
||||
allAgentsCreated.length = 0;
|
||||
await Promise.all(destroying);
|
||||
});
|
||||
|
||||
async function safeDisconnect(conn: MCPConnection | null): Promise<void> {
|
||||
if (!conn) return;
|
||||
(conn as unknown as { shouldStopReconnecting: boolean }).shouldStopReconnecting = true;
|
||||
conn.removeAllListeners();
|
||||
await conn.disconnect();
|
||||
}
|
||||
|
||||
function getFreePort(): Promise<number> {
|
||||
return new Promise((resolve, reject) => {
|
||||
const srv = net.createServer();
|
||||
srv.listen(0, '127.0.0.1', () => {
|
||||
const addr = srv.address() as net.AddressInfo;
|
||||
srv.close((err) => (err ? reject(err) : resolve(addr.port)));
|
||||
});
|
||||
});
|
||||
}
|
||||
|
||||
function trackSockets(httpServer: http.Server): () => Promise<void> {
|
||||
const sockets = new Set<Socket>();
|
||||
httpServer.on('connection', (socket: Socket) => {
|
||||
sockets.add(socket);
|
||||
socket.once('close', () => sockets.delete(socket));
|
||||
});
|
||||
return () =>
|
||||
new Promise<void>((resolve) => {
|
||||
for (const socket of sockets) socket.destroy();
|
||||
sockets.clear();
|
||||
httpServer.close(() => resolve());
|
||||
});
|
||||
}
|
||||
|
||||
interface TestServer {
|
||||
url: string;
|
||||
close: () => Promise<void>;
|
||||
/** Returns the most recently captured request headers */
|
||||
getLastHeaders: () => Record<string, string>;
|
||||
}
|
||||
|
||||
function createTestUser(overrides: Partial<IUser> = {}): IUser {
|
||||
return {
|
||||
_id: new Types.ObjectId(),
|
||||
id: new Types.ObjectId().toString(),
|
||||
username: 'testuser',
|
||||
email: 'test@example.com',
|
||||
name: 'Test User',
|
||||
avatar: 'https://example.com/avatar.png',
|
||||
provider: 'email',
|
||||
role: 'user',
|
||||
createdAt: new Date('2021-01-01'),
|
||||
updatedAt: new Date('2021-01-01'),
|
||||
emailVerified: true,
|
||||
...overrides,
|
||||
} as IUser;
|
||||
}
|
||||
|
||||
/**
|
||||
* Creates a Streamable HTTP MCP server that captures incoming request headers.
|
||||
* The server registers a dummy tool so `fetchTools()` makes a real request
|
||||
* through the transport, allowing us to capture the headers from that request.
|
||||
*/
|
||||
async function createHeaderCapturingServer(): Promise<TestServer> {
|
||||
const sessions = new Map<string, StreamableHTTPServerTransport>();
|
||||
let lastHeaders: Record<string, string> = {};
|
||||
|
||||
const httpServer = http.createServer(async (req, res) => {
|
||||
// Capture headers from every POST request (the tool-listing / tool-call requests)
|
||||
if (req.method === 'POST') {
|
||||
lastHeaders = {};
|
||||
for (const [key, value] of Object.entries(req.headers)) {
|
||||
if (typeof value === 'string') {
|
||||
lastHeaders[key] = value;
|
||||
} else if (Array.isArray(value)) {
|
||||
lastHeaders[key] = value.join(', ');
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
const sid = req.headers['mcp-session-id'] as string | undefined;
|
||||
let transport = sid ? sessions.get(sid) : undefined;
|
||||
|
||||
if (!transport) {
|
||||
transport = new StreamableHTTPServerTransport({ sessionIdGenerator: () => randomUUID() });
|
||||
const mcp = new McpServer({ name: 'header-capture-server', version: '0.0.1' });
|
||||
mcp.tool('echo', 'Echo tool for testing', {}, async () => ({
|
||||
content: [{ type: 'text', text: 'ok' }],
|
||||
}));
|
||||
await mcp.connect(transport);
|
||||
}
|
||||
|
||||
await transport.handleRequest(req, res);
|
||||
|
||||
if (transport.sessionId && !sessions.has(transport.sessionId)) {
|
||||
sessions.set(transport.sessionId, transport);
|
||||
transport.onclose = () => sessions.delete(transport!.sessionId!);
|
||||
}
|
||||
});
|
||||
|
||||
const destroySockets = trackSockets(httpServer);
|
||||
const port = await getFreePort();
|
||||
await new Promise<void>((resolve) => httpServer.listen(port, '127.0.0.1', resolve));
|
||||
|
||||
return {
|
||||
url: `http://127.0.0.1:${port}/`,
|
||||
getLastHeaders: () => ({ ...lastHeaders }),
|
||||
close: async () => {
|
||||
const closing = [...sessions.values()].map((t) => t.close().catch(() => undefined));
|
||||
sessions.clear();
|
||||
await Promise.all(closing);
|
||||
await destroySockets();
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
describe('dbSourced header security – integration', () => {
|
||||
let server: TestServer;
|
||||
let conn: MCPConnection | null;
|
||||
|
||||
beforeEach(async () => {
|
||||
server = await createHeaderCapturingServer();
|
||||
conn = null;
|
||||
process.env.SECRET_DB_URL = 'mongodb://admin:password@prod-host:27017/secret';
|
||||
process.env.INTERNAL_API_KEY = 'internal-key-do-not-leak';
|
||||
});
|
||||
|
||||
afterEach(async () => {
|
||||
await safeDisconnect(conn);
|
||||
conn = null;
|
||||
jest.restoreAllMocks();
|
||||
await server.close();
|
||||
delete process.env.SECRET_DB_URL;
|
||||
delete process.env.INTERNAL_API_KEY;
|
||||
});
|
||||
|
||||
it('DB-sourced: resolves {{MCP_API_KEY}} via customUserVars', async () => {
|
||||
const options: MCPOptions = {
|
||||
type: 'streamable-http',
|
||||
url: server.url,
|
||||
headers: {
|
||||
Authorization: 'Bearer {{MCP_API_KEY}}',
|
||||
},
|
||||
};
|
||||
|
||||
const resolved = processMCPEnv({
|
||||
options,
|
||||
dbSourced: true,
|
||||
customUserVars: { MCP_API_KEY: 'user-provided-secret' },
|
||||
});
|
||||
|
||||
conn = new MCPConnection({
|
||||
serverName: 'db-test',
|
||||
serverConfig: resolved,
|
||||
useSSRFProtection: false,
|
||||
});
|
||||
|
||||
if ('headers' in resolved) {
|
||||
conn.setRequestHeaders(resolved.headers || {});
|
||||
}
|
||||
|
||||
await conn.connect();
|
||||
await conn.fetchTools();
|
||||
|
||||
const captured = server.getLastHeaders();
|
||||
expect(captured['authorization']).toBe('Bearer user-provided-secret');
|
||||
});
|
||||
|
||||
it('DB-sourced: does NOT resolve ${ENV_VAR} — literal placeholder sent as header', async () => {
|
||||
const options: MCPOptions = {
|
||||
type: 'streamable-http',
|
||||
url: server.url,
|
||||
headers: {
|
||||
'X-Leaked-DB': '${SECRET_DB_URL}',
|
||||
'X-Leaked-Key': '${INTERNAL_API_KEY}',
|
||||
},
|
||||
};
|
||||
|
||||
const resolved = processMCPEnv({ options, dbSourced: true });
|
||||
|
||||
conn = new MCPConnection({
|
||||
serverName: 'db-test',
|
||||
serverConfig: resolved,
|
||||
useSSRFProtection: false,
|
||||
});
|
||||
|
||||
if ('headers' in resolved) {
|
||||
conn.setRequestHeaders(resolved.headers || {});
|
||||
}
|
||||
|
||||
await conn.connect();
|
||||
await conn.fetchTools();
|
||||
|
||||
const captured = server.getLastHeaders();
|
||||
// The literal placeholders must be sent, NOT the env values
|
||||
expect(captured['x-leaked-db']).toBe('${SECRET_DB_URL}');
|
||||
expect(captured['x-leaked-key']).toBe('${INTERNAL_API_KEY}');
|
||||
// Double-check env vars were NOT injected
|
||||
expect(captured['x-leaked-db']).not.toContain('mongodb://');
|
||||
expect(captured['x-leaked-key']).not.toBe('internal-key-do-not-leak');
|
||||
});
|
||||
|
||||
it('DB-sourced: does NOT resolve {{LIBRECHAT_USER_*}} placeholders', async () => {
|
||||
const user = createTestUser({ id: 'user-secret-id', email: 'private@corp.com' });
|
||||
const options: MCPOptions = {
|
||||
type: 'streamable-http',
|
||||
url: server.url,
|
||||
headers: {
|
||||
'X-User-Id': '{{LIBRECHAT_USER_ID}}',
|
||||
'X-User-Email': '{{LIBRECHAT_USER_EMAIL}}',
|
||||
},
|
||||
};
|
||||
|
||||
const resolved = processMCPEnv({ options, user, dbSourced: true });
|
||||
|
||||
conn = new MCPConnection({
|
||||
serverName: 'db-test',
|
||||
serverConfig: resolved,
|
||||
useSSRFProtection: false,
|
||||
});
|
||||
|
||||
if ('headers' in resolved) {
|
||||
conn.setRequestHeaders(resolved.headers || {});
|
||||
}
|
||||
|
||||
await conn.connect();
|
||||
await conn.fetchTools();
|
||||
|
||||
const captured = server.getLastHeaders();
|
||||
expect(captured['x-user-id']).toBe('{{LIBRECHAT_USER_ID}}');
|
||||
expect(captured['x-user-email']).toBe('{{LIBRECHAT_USER_EMAIL}}');
|
||||
expect(captured['x-user-id']).not.toBe('user-secret-id');
|
||||
});
|
||||
|
||||
it('DB-sourced: does NOT resolve {{LIBRECHAT_BODY_*}} placeholders', async () => {
|
||||
const body = {
|
||||
conversationId: 'conv-secret-123',
|
||||
parentMessageId: 'parent-456',
|
||||
messageId: 'msg-789',
|
||||
};
|
||||
const options: MCPOptions = {
|
||||
type: 'streamable-http',
|
||||
url: server.url,
|
||||
headers: {
|
||||
'X-Conv-Id': '{{LIBRECHAT_BODY_CONVERSATIONID}}',
|
||||
},
|
||||
};
|
||||
|
||||
const resolved = processMCPEnv({ options, body, dbSourced: true });
|
||||
|
||||
conn = new MCPConnection({
|
||||
serverName: 'db-test',
|
||||
serverConfig: resolved,
|
||||
useSSRFProtection: false,
|
||||
});
|
||||
|
||||
if ('headers' in resolved) {
|
||||
conn.setRequestHeaders(resolved.headers || {});
|
||||
}
|
||||
|
||||
await conn.connect();
|
||||
await conn.fetchTools();
|
||||
|
||||
const captured = server.getLastHeaders();
|
||||
expect(captured['x-conv-id']).toBe('{{LIBRECHAT_BODY_CONVERSATIONID}}');
|
||||
expect(captured['x-conv-id']).not.toBe('conv-secret-123');
|
||||
});
|
||||
|
||||
it('DB-sourced: mixed headers — customUserVars resolve, everything else blocked', async () => {
|
||||
const user = createTestUser({ id: 'user-id-value' });
|
||||
const body = { conversationId: 'conv-id-value', parentMessageId: 'p-1', messageId: 'm-1' };
|
||||
const options: MCPOptions = {
|
||||
type: 'streamable-http',
|
||||
url: server.url,
|
||||
headers: {
|
||||
Authorization: 'Bearer {{MCP_API_KEY}}',
|
||||
'X-Env-Leak': '${SECRET_DB_URL}',
|
||||
'X-User-Id': '{{LIBRECHAT_USER_ID}}',
|
||||
'X-Conv-Id': '{{LIBRECHAT_BODY_CONVERSATIONID}}',
|
||||
'X-Static': 'plain-value',
|
||||
},
|
||||
};
|
||||
|
||||
const resolved = processMCPEnv({
|
||||
options,
|
||||
user,
|
||||
body,
|
||||
dbSourced: true,
|
||||
customUserVars: { MCP_API_KEY: 'my-api-key-value' },
|
||||
});
|
||||
|
||||
conn = new MCPConnection({
|
||||
serverName: 'db-test',
|
||||
serverConfig: resolved,
|
||||
useSSRFProtection: false,
|
||||
});
|
||||
|
||||
if ('headers' in resolved) {
|
||||
conn.setRequestHeaders(resolved.headers || {});
|
||||
}
|
||||
|
||||
await conn.connect();
|
||||
await conn.fetchTools();
|
||||
|
||||
const captured = server.getLastHeaders();
|
||||
// customUserVars resolved
|
||||
expect(captured['authorization']).toBe('Bearer my-api-key-value');
|
||||
// env var blocked
|
||||
expect(captured['x-env-leak']).toBe('${SECRET_DB_URL}');
|
||||
// user placeholder blocked
|
||||
expect(captured['x-user-id']).toBe('{{LIBRECHAT_USER_ID}}');
|
||||
// body placeholder blocked
|
||||
expect(captured['x-conv-id']).toBe('{{LIBRECHAT_BODY_CONVERSATIONID}}');
|
||||
// static value unchanged
|
||||
expect(captured['x-static']).toBe('plain-value');
|
||||
});
|
||||
|
||||
it('YAML-sourced (default): resolves ALL placeholder types', async () => {
|
||||
const user = createTestUser({ id: 'yaml-user-id', email: 'yaml@example.com' });
|
||||
const body = { conversationId: 'yaml-conv-id', parentMessageId: 'p-1', messageId: 'm-1' };
|
||||
const options: MCPOptions = {
|
||||
type: 'streamable-http',
|
||||
url: server.url,
|
||||
headers: {
|
||||
Authorization: 'Bearer {{MY_CUSTOM_KEY}}',
|
||||
'X-Env': '${INTERNAL_API_KEY}',
|
||||
'X-User-Id': '{{LIBRECHAT_USER_ID}}',
|
||||
'X-Conv-Id': '{{LIBRECHAT_BODY_CONVERSATIONID}}',
|
||||
},
|
||||
};
|
||||
|
||||
const resolved = processMCPEnv({
|
||||
options,
|
||||
user,
|
||||
body,
|
||||
dbSourced: false,
|
||||
customUserVars: { MY_CUSTOM_KEY: 'yaml-custom-val' },
|
||||
});
|
||||
|
||||
conn = new MCPConnection({
|
||||
serverName: 'yaml-test',
|
||||
serverConfig: resolved,
|
||||
useSSRFProtection: false,
|
||||
});
|
||||
|
||||
if ('headers' in resolved) {
|
||||
conn.setRequestHeaders(resolved.headers || {});
|
||||
}
|
||||
|
||||
await conn.connect();
|
||||
await conn.fetchTools();
|
||||
|
||||
const captured = server.getLastHeaders();
|
||||
// All placeholder types resolved
|
||||
expect(captured['authorization']).toBe('Bearer yaml-custom-val');
|
||||
expect(captured['x-env']).toBe('internal-key-do-not-leak');
|
||||
expect(captured['x-user-id']).toBe('yaml-user-id');
|
||||
expect(captured['x-conv-id']).toBe('yaml-conv-id');
|
||||
});
|
||||
|
||||
it('DB-sourced: URL placeholder ${ENV_VAR} is NOT resolved', () => {
|
||||
const options: MCPOptions = {
|
||||
type: 'streamable-http',
|
||||
url: '${SECRET_DB_URL}/mcp',
|
||||
headers: {},
|
||||
};
|
||||
|
||||
const resolved = processMCPEnv({ options, dbSourced: true });
|
||||
expect((resolved as { url?: string }).url).toBe('${SECRET_DB_URL}/mcp');
|
||||
});
|
||||
|
||||
it('YAML-sourced: URL placeholder ${ENV_VAR} IS resolved', () => {
|
||||
const options: MCPOptions = {
|
||||
type: 'streamable-http',
|
||||
url: '${INTERNAL_API_KEY}/endpoint',
|
||||
headers: {},
|
||||
};
|
||||
|
||||
const resolved = processMCPEnv({ options, dbSourced: false });
|
||||
expect((resolved as { url?: string }).url).toBe('internal-key-do-not-leak/endpoint');
|
||||
});
|
||||
|
||||
it('DB-sourced: multiple customUserVars resolve correctly', async () => {
|
||||
const options: MCPOptions = {
|
||||
type: 'streamable-http',
|
||||
url: server.url,
|
||||
headers: {
|
||||
Authorization: 'Bearer {{API_TOKEN}}',
|
||||
'X-Workspace': '{{WORKSPACE_ID}}',
|
||||
'X-Region': '{{REGION}}',
|
||||
},
|
||||
};
|
||||
|
||||
const resolved = processMCPEnv({
|
||||
options,
|
||||
dbSourced: true,
|
||||
customUserVars: {
|
||||
API_TOKEN: 'tok-abc123',
|
||||
WORKSPACE_ID: 'ws-def456',
|
||||
REGION: 'us-east-1',
|
||||
},
|
||||
});
|
||||
|
||||
conn = new MCPConnection({
|
||||
serverName: 'db-multi-var',
|
||||
serverConfig: resolved,
|
||||
useSSRFProtection: false,
|
||||
});
|
||||
|
||||
if ('headers' in resolved) {
|
||||
conn.setRequestHeaders(resolved.headers || {});
|
||||
}
|
||||
|
||||
await conn.connect();
|
||||
await conn.fetchTools();
|
||||
|
||||
const captured = server.getLastHeaders();
|
||||
expect(captured['authorization']).toBe('Bearer tok-abc123');
|
||||
expect(captured['x-workspace']).toBe('ws-def456');
|
||||
expect(captured['x-region']).toBe('us-east-1');
|
||||
});
|
||||
|
||||
it('DB-sourced: absent customUserVars leaves placeholder unresolved', async () => {
|
||||
const options: MCPOptions = {
|
||||
type: 'streamable-http',
|
||||
url: server.url,
|
||||
headers: {
|
||||
Authorization: 'Bearer {{MCP_API_KEY}}',
|
||||
},
|
||||
};
|
||||
|
||||
// No customUserVars provided at all
|
||||
const resolved = processMCPEnv({ options, dbSourced: true });
|
||||
|
||||
conn = new MCPConnection({
|
||||
serverName: 'db-no-vars',
|
||||
serverConfig: resolved,
|
||||
useSSRFProtection: false,
|
||||
});
|
||||
|
||||
if ('headers' in resolved) {
|
||||
conn.setRequestHeaders(resolved.headers || {});
|
||||
}
|
||||
|
||||
await conn.connect();
|
||||
await conn.fetchTools();
|
||||
|
||||
const captured = server.getLastHeaders();
|
||||
expect(captured['authorization']).toBe('Bearer {{MCP_API_KEY}}');
|
||||
});
|
||||
});
|
||||
|
|
@ -59,8 +59,9 @@ export class MCPServerInspector {
|
|||
if (!this.connection) {
|
||||
tempConnection = true;
|
||||
this.connection = await MCPConnectionFactory.create({
|
||||
serverName: this.serverName,
|
||||
serverConfig: this.config,
|
||||
serverName: this.serverName,
|
||||
dbSourced: !!this.config.dbId,
|
||||
useSSRFProtection: this.useSSRFProtection,
|
||||
});
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,9 +1,9 @@
|
|||
import type { MCPConnection } from '~/mcp/connection';
|
||||
import type * as t from '~/mcp/types';
|
||||
import { MCPServerInspector } from '~/mcp/registry/MCPServerInspector';
|
||||
import { detectOAuthRequirement } from '~/mcp/oauth';
|
||||
import { MCPConnectionFactory } from '~/mcp/MCPConnectionFactory';
|
||||
import { createMockConnection } from './mcpConnectionsMock.helper';
|
||||
import { MCPConnectionFactory } from '~/mcp/MCPConnectionFactory';
|
||||
import { detectOAuthRequirement } from '~/mcp/oauth';
|
||||
|
||||
// Mock external dependencies
|
||||
jest.mock('../../oauth/detectOAuth');
|
||||
|
|
@ -277,6 +277,7 @@ describe('MCPServerInspector', () => {
|
|||
serverName: 'test_server',
|
||||
serverConfig: expect.objectContaining({ type: 'stdio', command: 'node' }),
|
||||
useSSRFProtection: true,
|
||||
dbSourced: false,
|
||||
});
|
||||
|
||||
// Verify temporary connection was disconnected
|
||||
|
|
|
|||
|
|
@ -12,15 +12,18 @@ import type { ParsedServerConfig, AddServerResult } from '~/mcp/types';
|
|||
import { AccessControlService } from '~/acl/accessControlService';
|
||||
|
||||
/**
|
||||
* Regex patterns for credential placeholders that should not be allowed in user-provided headers.
|
||||
* These placeholders would substitute the CALLING user's credentials, creating a security risk
|
||||
* when MCP servers are shared between users (credential exfiltration).
|
||||
* Regex patterns for credential/env placeholders that should not be allowed in user-provided configs.
|
||||
* These would substitute server credentials or the CALLING user's data, creating exfiltration risks
|
||||
* when MCP servers are shared between users.
|
||||
*
|
||||
* Safe placeholders like {{MCP_API_KEY}} are allowed as they resolve from the user's own plugin auth.
|
||||
*/
|
||||
const DANGEROUS_CREDENTIAL_PATTERNS = [
|
||||
/\$\{[^}]+\}/g,
|
||||
/\{\{LIBRECHAT_OPENID_[^}]+\}\}/g,
|
||||
/\{\{LIBRECHAT_USER_[^}]+\}\}/g,
|
||||
/\{\{LIBRECHAT_GRAPH_[^}]+\}\}/g,
|
||||
/\{\{LIBRECHAT_BODY_[^}]+\}\}/g,
|
||||
];
|
||||
|
||||
/**
|
||||
|
|
@ -457,7 +460,7 @@ export class ServerConfigsDB implements IServerConfigsRepositoryInterface {
|
|||
};
|
||||
|
||||
// Remove key field since it's user-provided (destructure to omit, not set to undefined)
|
||||
// eslint-disable-next-line @typescript-eslint/no-unused-vars
|
||||
|
||||
const { key: _removed, ...apiKeyWithoutKey } = result.apiKey!;
|
||||
result.apiKey = apiKeyWithoutKey;
|
||||
|
||||
|
|
@ -521,7 +524,7 @@ export class ServerConfigsDB implements IServerConfigsRepositoryInterface {
|
|||
'[ServerConfigsDB.decryptConfig] Failed to decrypt apiKey.key, returning config without key',
|
||||
error,
|
||||
);
|
||||
// eslint-disable-next-line @typescript-eslint/no-unused-vars
|
||||
|
||||
const { key: _removedKey, ...apiKeyWithoutKey } = result.apiKey;
|
||||
result.apiKey = apiKeyWithoutKey;
|
||||
}
|
||||
|
|
@ -542,7 +545,7 @@ export class ServerConfigsDB implements IServerConfigsRepositoryInterface {
|
|||
'[ServerConfigsDB.decryptConfig] Failed to decrypt client_secret, returning config without secret',
|
||||
error,
|
||||
);
|
||||
// eslint-disable-next-line @typescript-eslint/no-unused-vars
|
||||
|
||||
const { client_secret: _removed, ...oauthWithoutSecret } = oauthConfig;
|
||||
result = {
|
||||
...result,
|
||||
|
|
|
|||
|
|
@ -167,6 +167,8 @@ export interface BasicConnectionOptions {
|
|||
serverName: string;
|
||||
serverConfig: MCPOptions;
|
||||
useSSRFProtection?: boolean;
|
||||
/** When true, only resolve customUserVars in processMCPEnv (for DB-stored servers) */
|
||||
dbSourced?: boolean;
|
||||
}
|
||||
|
||||
export interface OAuthConnectionOptions {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue