diff --git a/.github/workflows/backend-review.yml b/.github/workflows/backend-review.yml index 0c593eac18..7637b8cdc0 100644 --- a/.github/workflows/backend-review.yml +++ b/.github/workflows/backend-review.yml @@ -67,5 +67,8 @@ jobs: - name: Run librechat-data-provider unit tests run: cd packages/data-provider && npm run test:ci - - name: Run librechat-api unit tests + - name: Run @librechat/data-schemas unit tests + run: cd packages/data-schemas && npm run test:ci + + - name: Run @librechat/api unit tests run: cd packages/api && npm run test:ci \ No newline at end of file diff --git a/api/server/controllers/agents/client.js b/api/server/controllers/agents/client.js index 7c8a375a9f..be64d5a0e3 100644 --- a/api/server/controllers/agents/client.js +++ b/api/server/controllers/agents/client.js @@ -671,6 +671,7 @@ class AgentClient extends BaseClient { last_agent_index: this.agentConfigs?.size ?? 0, user_id: this.user ?? this.options.req.user?.id, hide_sequential_outputs: this.options.agent.hide_sequential_outputs, + user: this.options.req.user, }, recursionLimit: agentsEConfig?.recursionLimit, signal: abortController.signal, diff --git a/api/server/services/MCP.js b/api/server/services/MCP.js index dc3d3f0037..357913e519 100644 --- a/api/server/services/MCP.js +++ b/api/server/services/MCP.js @@ -50,9 +50,10 @@ async function createMCPTool({ req, toolKey, provider: _provider }) { /** @type {(toolArguments: Object | string, config?: GraphRunnableConfig) => Promise} */ const _call = async (toolArguments, config) => { + const userId = config?.configurable?.user?.id || config?.configurable?.user_id; try { const derivedSignal = config?.signal ? AbortSignal.any([config.signal]) : undefined; - const mcpManager = getMCPManager(config?.configurable?.user_id); + const mcpManager = getMCPManager(userId); const provider = (config?.metadata?.provider || _provider)?.toLowerCase(); const result = await mcpManager.callTool({ serverName, @@ -60,8 +61,8 @@ async function createMCPTool({ req, toolKey, provider: _provider }) { provider, toolArguments, options: { - userId: config?.configurable?.user_id, signal: derivedSignal, + user: config?.configurable?.user, }, }); @@ -74,7 +75,7 @@ async function createMCPTool({ req, toolKey, provider: _provider }) { return result; } catch (error) { logger.error( - `[MCP][User: ${config?.configurable?.user_id}][${serverName}] Error calling "${toolName}" MCP tool:`, + `[MCP][User: ${userId}][${serverName}] Error calling "${toolName}" MCP tool:`, error, ); throw new Error( diff --git a/packages/api/src/mcp/manager.ts b/packages/api/src/mcp/manager.ts index 93655dd3bb..af8f1632a7 100644 --- a/packages/api/src/mcp/manager.ts +++ b/packages/api/src/mcp/manager.ts @@ -1,6 +1,6 @@ import { CallToolResultSchema, ErrorCode, McpError } from '@modelcontextprotocol/sdk/types.js'; import type { RequestOptions } from '@modelcontextprotocol/sdk/shared/protocol.js'; -import type { JsonSchemaType, MCPOptions } from 'librechat-data-provider'; +import type { JsonSchemaType, MCPOptions, TUser } from 'librechat-data-provider'; import type { Logger } from 'winston'; import type * as t from './types'; import { formatToolContent } from './parsers'; @@ -8,7 +8,7 @@ import { MCPConnection } from './connection'; import { CONSTANTS } from './enum'; export interface CallToolOptions extends RequestOptions { - userId?: string; + user?: TUser; } export class MCPManager { @@ -21,7 +21,7 @@ export class MCPManager { private userLastActivity: Map = new Map(); private readonly USER_CONNECTION_IDLE_TIMEOUT = 15 * 60 * 1000; // 15 minutes (TODO: make configurable) private mcpConfigs: t.MCPServers = {}; - private processMCPEnv?: (obj: MCPOptions, userId?: string) => MCPOptions; // Store the processing function + private processMCPEnv?: (obj: MCPOptions, user?: TUser) => MCPOptions; // Store the processing function /** Store MCP server instructions */ private serverInstructions: Map = new Map(); private logger: Logger; @@ -219,7 +219,12 @@ export class MCPManager { } /** Gets or creates a connection for a specific user */ - public async getUserConnection(userId: string, serverName: string): Promise { + public async getUserConnection(serverName: string, user: TUser): Promise { + const userId = user.id; + if (!userId) { + throw new McpError(ErrorCode.InvalidRequest, `[MCP] User object missing id property`); + } + const userServerMap = this.userConnections.get(userId); let connection = userServerMap?.get(serverName); const now = Date.now(); @@ -267,7 +272,7 @@ export class MCPManager { } if (this.processMCPEnv) { - config = { ...(this.processMCPEnv(config, userId) ?? {}) }; + config = { ...(this.processMCPEnv(config, user) ?? {}) }; } connection = new MCPConnection(serverName, config, this.logger, userId); @@ -462,14 +467,15 @@ export class MCPManager { options?: CallToolOptions; }): Promise { let connection: MCPConnection | undefined; - const { userId, ...callOptions } = options ?? {}; + const { user, ...callOptions } = options ?? {}; + const userId = user?.id; const logPrefix = userId ? `[MCP][User: ${userId}][${serverName}]` : `[MCP][${serverName}]`; try { - if (userId) { + if (userId && user) { this.updateUserLastActivity(userId); // Get or create user-specific connection - connection = await this.getUserConnection(userId, serverName); + connection = await this.getUserConnection(serverName, user); } else { // Use app-level connection connection = this.connections.get(serverName); diff --git a/packages/data-provider/specs/mcp.spec.ts b/packages/data-provider/specs/mcp.spec.ts index b3fb61375c..f2b62c0f89 100644 --- a/packages/data-provider/specs/mcp.spec.ts +++ b/packages/data-provider/specs/mcp.spec.ts @@ -1,4 +1,28 @@ -import { StdioOptionsSchema, StreamableHTTPOptionsSchema, processMCPEnv, MCPOptions } from '../src/mcp'; +import type { TUser } from 'librechat-data-provider'; +import { + StreamableHTTPOptionsSchema, + StdioOptionsSchema, + processMCPEnv, + MCPOptions, +} from '../src/mcp'; + +// Helper function to create test user objects +function createTestUser( + overrides: Partial & Record = {}, +): TUser & Record { + return { + id: 'test-user-id', + 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').toISOString(), + updatedAt: new Date('2021-01-01').toISOString(), + ...overrides, + }; +} describe('Environment Variable Extraction (MCP)', () => { const originalEnv = process.env; @@ -91,13 +115,13 @@ describe('Environment Variable Extraction (MCP)', () => { // Type is now required, so parsing should fail expect(() => StreamableHTTPOptionsSchema.parse(options)).toThrow(); - + // With type provided, it should pass const validOptions = { type: 'streamable-http' as const, url: 'https://example.com/api', }; - + const result = StreamableHTTPOptionsSchema.parse(validOptions); expect(result.type).toBe('streamable-http'); }); @@ -113,7 +137,7 @@ describe('Environment Variable Extraction (MCP)', () => { }; const result = StreamableHTTPOptionsSchema.parse(options); - + expect(result.headers).toEqual(options.headers); }); }); @@ -165,7 +189,7 @@ describe('Environment Variable Extraction (MCP)', () => { }); it('should process user ID in headers field', () => { - const userId = 'test-user-123'; + const user = createTestUser({ id: 'test-user-123' }); const obj: MCPOptions = { type: 'sse', url: 'https://example.com', @@ -176,7 +200,7 @@ describe('Environment Variable Extraction (MCP)', () => { }, }; - const result = processMCPEnv(obj, userId); + const result = processMCPEnv(obj, user); expect('headers' in result && result.headers).toEqual({ Authorization: 'test-api-key-value', @@ -217,15 +241,15 @@ describe('Environment Variable Extraction (MCP)', () => { }; // Process for two different users - const user1Id = 'user-123'; - const user2Id = 'user-456'; + const user1 = createTestUser({ id: 'user-123' }); + const user2 = createTestUser({ id: 'user-456' }); - const resultUser1 = processMCPEnv(baseConfig, user1Id); - const resultUser2 = processMCPEnv(baseConfig, user2Id); + const resultUser1 = processMCPEnv(baseConfig, user1); + const resultUser2 = processMCPEnv(baseConfig, user2); // Verify each has the correct user ID - expect('headers' in resultUser1 && resultUser1.headers?.['User-Id']).toBe(user1Id); - expect('headers' in resultUser2 && resultUser2.headers?.['User-Id']).toBe(user2Id); + expect('headers' in resultUser1 && resultUser1.headers?.['User-Id']).toBe('user-123'); + expect('headers' in resultUser2 && resultUser2.headers?.['User-Id']).toBe('user-456'); // Verify they're different objects expect(resultUser1).not.toBe(resultUser2); @@ -239,11 +263,11 @@ describe('Environment Variable Extraction (MCP)', () => { expect(baseConfig.headers?.['User-Id']).toBe('{{LIBRECHAT_USER_ID}}'); // Second user's config should be unchanged - expect('headers' in resultUser2 && resultUser2.headers?.['User-Id']).toBe(user2Id); + expect('headers' in resultUser2 && resultUser2.headers?.['User-Id']).toBe('user-456'); }); it('should process headers in streamable-http options', () => { - const userId = 'test-user-123'; + const user = createTestUser({ id: 'test-user-123' }); const obj: MCPOptions = { type: 'streamable-http', url: 'https://example.com', @@ -254,7 +278,7 @@ describe('Environment Variable Extraction (MCP)', () => { }, }; - const result = processMCPEnv(obj, userId); + const result = processMCPEnv(obj, user); expect('headers' in result && result.headers).toEqual({ Authorization: 'test-api-key-value', @@ -262,7 +286,7 @@ describe('Environment Variable Extraction (MCP)', () => { 'Content-Type': 'application/json', }); }); - + it('should maintain streamable-http type in processed options', () => { const obj: MCPOptions = { type: 'streamable-http', @@ -273,5 +297,233 @@ describe('Environment Variable Extraction (MCP)', () => { expect(result.type).toBe('streamable-http'); }); + + it('should process dynamic user fields in headers', () => { + const user = createTestUser({ + id: 'user-123', + email: 'test@example.com', + username: 'testuser', + openidId: 'openid-123', + googleId: 'google-456', + emailVerified: true, + role: 'admin', + }); + const obj: MCPOptions = { + type: 'sse', + url: 'https://example.com', + headers: { + 'User-Email': '{{LIBRECHAT_USER_EMAIL}}', + 'User-Name': '{{LIBRECHAT_USER_USERNAME}}', + OpenID: '{{LIBRECHAT_USER_OPENIDID}}', + 'Google-ID': '{{LIBRECHAT_USER_GOOGLEID}}', + 'Email-Verified': '{{LIBRECHAT_USER_EMAILVERIFIED}}', + 'User-Role': '{{LIBRECHAT_USER_ROLE}}', + 'Content-Type': 'application/json', + }, + }; + + const result = processMCPEnv(obj, user); + + expect('headers' in result && result.headers).toEqual({ + 'User-Email': 'test@example.com', + 'User-Name': 'testuser', + OpenID: 'openid-123', + 'Google-ID': 'google-456', + 'Email-Verified': 'true', + 'User-Role': 'admin', + 'Content-Type': 'application/json', + }); + }); + + it('should handle missing user fields gracefully', () => { + const user = createTestUser({ + id: 'user-123', + email: 'test@example.com', + username: undefined, // explicitly set to undefined to test missing field + }); + const obj: MCPOptions = { + type: 'sse', + url: 'https://example.com', + headers: { + 'User-Email': '{{LIBRECHAT_USER_EMAIL}}', + 'User-Name': '{{LIBRECHAT_USER_USERNAME}}', + 'Content-Type': 'application/json', + }, + }; + + const result = processMCPEnv(obj, user); + + expect('headers' in result && result.headers).toEqual({ + 'User-Email': 'test@example.com', + 'User-Name': '', // Empty string for missing field + 'Content-Type': 'application/json', + }); + }); + + it('should process user fields in env variables', () => { + const user = createTestUser({ + id: 'user-123', + email: 'test@example.com', + ldapId: 'ldap-user-123', + }); + const obj: MCPOptions = { + command: 'node', + args: ['server.js'], + env: { + USER_EMAIL: '{{LIBRECHAT_USER_EMAIL}}', + LDAP_ID: '{{LIBRECHAT_USER_LDAPID}}', + API_KEY: '${TEST_API_KEY}', + }, + }; + + const result = processMCPEnv(obj, user); + + expect('env' in result && result.env).toEqual({ + USER_EMAIL: 'test@example.com', + LDAP_ID: 'ldap-user-123', + API_KEY: 'test-api-key-value', + }); + }); + + it('should process user fields in URL', () => { + const user = createTestUser({ + id: 'user-123', + username: 'testuser', + }); + const obj: MCPOptions = { + type: 'sse', + url: 'https://example.com/api/{{LIBRECHAT_USER_USERNAME}}/stream', + }; + + const result = processMCPEnv(obj, user); + + expect('url' in result && result.url).toBe('https://example.com/api/testuser/stream'); + }); + + it('should handle boolean user fields', () => { + const user = createTestUser({ + id: 'user-123', + emailVerified: true, + twoFactorEnabled: false, + termsAccepted: true, + }); + const obj: MCPOptions = { + type: 'sse', + url: 'https://example.com', + headers: { + 'Email-Verified': '{{LIBRECHAT_USER_EMAILVERIFIED}}', + 'Two-Factor': '{{LIBRECHAT_USER_TWOFACTORENABLED}}', + 'Terms-Accepted': '{{LIBRECHAT_USER_TERMSACCEPTED}}', + }, + }; + + const result = processMCPEnv(obj, user); + + expect('headers' in result && result.headers).toEqual({ + 'Email-Verified': 'true', + 'Two-Factor': 'false', + 'Terms-Accepted': 'true', + }); + }); + + it('should not process sensitive fields like password', () => { + const user = createTestUser({ + id: 'user-123', + email: 'test@example.com', + password: 'secret-password', + }); + const obj: MCPOptions = { + type: 'sse', + url: 'https://example.com', + headers: { + 'User-Email': '{{LIBRECHAT_USER_EMAIL}}', + 'User-Password': '{{LIBRECHAT_USER_PASSWORD}}', // This should not be processed + }, + }; + + const result = processMCPEnv(obj, user); + + expect('headers' in result && result.headers).toEqual({ + 'User-Email': 'test@example.com', + 'User-Password': '{{LIBRECHAT_USER_PASSWORD}}', // Unchanged + }); + }); + + it('should handle multiple occurrences of the same placeholder', () => { + const user = createTestUser({ + id: 'user-123', + email: 'test@example.com', + }); + const obj: MCPOptions = { + type: 'sse', + url: 'https://example.com', + headers: { + 'Primary-Email': '{{LIBRECHAT_USER_EMAIL}}', + 'Secondary-Email': '{{LIBRECHAT_USER_EMAIL}}', + 'Backup-Email': '{{LIBRECHAT_USER_EMAIL}}', + }, + }; + + const result = processMCPEnv(obj, user); + + expect('headers' in result && result.headers).toEqual({ + 'Primary-Email': 'test@example.com', + 'Secondary-Email': 'test@example.com', + 'Backup-Email': 'test@example.com', + }); + }); + + it('should support both id and _id properties for LIBRECHAT_USER_ID', () => { + // Test with 'id' property + const userWithId = createTestUser({ + id: 'user-123', + email: 'test@example.com', + }); + const obj1: MCPOptions = { + type: 'sse', + url: 'https://example.com', + headers: { + 'User-Id': '{{LIBRECHAT_USER_ID}}', + }, + }; + + const result1 = processMCPEnv(obj1, userWithId); + expect('headers' in result1 && result1.headers?.['User-Id']).toBe('user-123'); + + // Test with '_id' property only (should not work since we only check 'id') + const userWithUnderscore = createTestUser({ + id: undefined, // Remove default id to test _id + _id: 'user-456', + email: 'test@example.com', + }); + const obj2: MCPOptions = { + type: 'sse', + url: 'https://example.com', + headers: { + 'User-Id': '{{LIBRECHAT_USER_ID}}', + }, + }; + + const result2 = processMCPEnv(obj2, userWithUnderscore); + // Since we don't check _id, the placeholder should remain unchanged + expect('headers' in result2 && result2.headers?.['User-Id']).toBe('{{LIBRECHAT_USER_ID}}'); + + // Test with both properties (id takes precedence) + const userWithBoth = createTestUser({ + id: 'user-789', + _id: 'user-000', + email: 'test@example.com', + }); + const obj3: MCPOptions = { + type: 'sse', + url: 'https://example.com', + headers: { + 'User-Id': '{{LIBRECHAT_USER_ID}}', + }, + }; + + const result3 = processMCPEnv(obj3, userWithBoth); + expect('headers' in result3 && result3.headers?.['User-Id']).toBe('user-789'); + }); }); }); diff --git a/packages/data-provider/src/mcp.ts b/packages/data-provider/src/mcp.ts index 1273274c54..2d7be8458f 100644 --- a/packages/data-provider/src/mcp.ts +++ b/packages/data-provider/src/mcp.ts @@ -1,4 +1,5 @@ import { z } from 'zod'; +import type { TUser } from './types'; import { extractEnvVariable } from './utils'; const BaseOptionsSchema = z.object({ @@ -7,7 +8,7 @@ const BaseOptionsSchema = z.object({ initTimeout: z.number().optional(), /** Controls visibility in chat dropdown menu (MCPSelect) */ chatMenu: z.boolean().optional(), - /** + /** * Controls server instruction behavior: * - undefined/not set: No instructions included (default) * - true: Use server-provided instructions @@ -121,12 +122,58 @@ export const MCPServersSchema = z.record(z.string(), MCPOptionsSchema); export type MCPOptions = z.infer; /** - * Recursively processes an object to replace environment variables in string values - * @param {MCPOptions} obj - The object to process - * @param {string} [userId] - The user ID - * @returns {MCPOptions} - The processed object with environment variables replaced + * List of allowed user fields that can be used in MCP environment variables. + * These are non-sensitive string/boolean fields from the IUser interface. */ -export function processMCPEnv(obj: Readonly, userId?: string): MCPOptions { +const ALLOWED_USER_FIELDS = [ + 'name', + 'username', + 'email', + 'provider', + 'role', + 'googleId', + 'facebookId', + 'openidId', + 'samlId', + 'ldapId', + 'githubId', + 'discordId', + 'appleId', + 'emailVerified', + 'twoFactorEnabled', + 'termsAccepted', +] as const; + +/** + * Processes a string value to replace user field placeholders + * @param value - The string value to process + * @param user - The user object + * @returns The processed string with placeholders replaced + */ +function processUserPlaceholders(value: string, user?: TUser): string { + if (!user || typeof value !== 'string') { + return value; + } + + for (const field of ALLOWED_USER_FIELDS) { + const placeholder = `{{LIBRECHAT_USER_${field.toUpperCase()}}}`; + if (value.includes(placeholder)) { + const fieldValue = user[field as keyof TUser]; + const replacementValue = fieldValue != null ? String(fieldValue) : ''; + value = value.replace(new RegExp(placeholder, 'g'), replacementValue); + } + } + + return value; +} + +/** + * Recursively processes an object to replace environment variables in string values + * @param obj - The object to process + * @param user - The user object containing all user fields + * @returns - The processed object with environment variables replaced + */ +export function processMCPEnv(obj: Readonly, user?: TUser): MCPOptions { if (obj === null || obj === undefined) { return obj; } @@ -136,23 +183,31 @@ export function processMCPEnv(obj: Readonly, userId?: string): MCPOp if ('env' in newObj && newObj.env) { const processedEnv: Record = {}; for (const [key, value] of Object.entries(newObj.env)) { - processedEnv[key] = extractEnvVariable(value); + let processedValue = extractEnvVariable(value); + processedValue = processUserPlaceholders(processedValue, user); + processedEnv[key] = processedValue; } newObj.env = processedEnv; } else if ('headers' in newObj && newObj.headers) { const processedHeaders: Record = {}; for (const [key, value] of Object.entries(newObj.headers)) { - if (value === '{{LIBRECHAT_USER_ID}}' && userId != null && userId) { - processedHeaders[key] = userId; + const userId = user?.id; + if (value === '{{LIBRECHAT_USER_ID}}' && userId != null) { + processedHeaders[key] = String(userId); continue; } - processedHeaders[key] = extractEnvVariable(value); + + let processedValue = extractEnvVariable(value); + processedValue = processUserPlaceholders(processedValue, user); + processedHeaders[key] = processedValue; } newObj.headers = processedHeaders; } if ('url' in newObj && newObj.url) { - newObj.url = extractEnvVariable(newObj.url); + let processedUrl = extractEnvVariable(newObj.url); + processedUrl = processUserPlaceholders(processedUrl, user); + newObj.url = processedUrl; } return newObj;