diff --git a/packages/api/src/mcp/__tests__/auth.test.ts b/packages/api/src/mcp/__tests__/auth.test.ts index 7bad7227300..2b258a57770 100644 --- a/packages/api/src/mcp/__tests__/auth.test.ts +++ b/packages/api/src/mcp/__tests__/auth.test.ts @@ -1,7 +1,9 @@ -import type { PluginAuthMethods } from '@librechat/data-schemas'; +import type { IUser, PluginAuthMethods } from '@librechat/data-schemas'; +import type { MCPOptions } from 'librechat-data-provider'; import type { GenericTool } from '@librechat/agents'; import { getUserMCPAuthMap, getServerCustomUserVars } from '../auth'; import { getPluginAuthMap } from '~/agents/auth'; +import { processMCPEnv } from '~/utils/env'; jest.mock('~/agents/auth', () => ({ getPluginAuthMap: jest.fn(), @@ -339,3 +341,32 @@ describe('getServerCustomUserVars', () => { expect(getServerCustomUserVars(undefined, 'my-server')).toBeUndefined(); }); }); + +describe('getUserMCPAuthMap -> processMCPEnv: stored auth field with a reserved name', () => { + it('should not let a stored LIBRECHAT_USER_EMAIL auth row override the real user email on a yaml server', async () => { + /** getPluginAuthMap is mocked; this is the map it builds from the row + * {authField:'LIBRECHAT_USER_EMAIL', pluginKey:'mcp_docs', value:'other@example.com'} */ + mockGetPluginAuthMap.mockResolvedValue({ + mcp_docs: { LIBRECHAT_USER_EMAIL: 'other@example.com' }, + }); + + const authMap = await getUserMCPAuthMap({ + userId: 'u1', + servers: ['docs'], + findPluginAuthsByKeys: mockFindPluginAuthsByKeys, + }); + + const options = { + type: 'streamable-http', + url: 'https://api.example.com', + headers: { 'X-User-Email': '{{LIBRECHAT_USER_EMAIL}}' }, + } as MCPOptions; + const result = processMCPEnv({ + options, + user: { id: 'u1', email: 'real@example.com' } as IUser, + customUserVars: getServerCustomUserVars(authMap, 'docs'), + }) as { headers?: Record }; + + expect(result.headers?.['X-User-Email']).toBe('real@example.com'); + }); +}); diff --git a/packages/api/src/mcp/__tests__/mcp.spec.ts b/packages/api/src/mcp/__tests__/mcp.spec.ts index 539c04f6b94..87f1f815c11 100644 --- a/packages/api/src/mcp/__tests__/mcp.spec.ts +++ b/packages/api/src/mcp/__tests__/mcp.spec.ts @@ -737,13 +737,12 @@ describe('Environment Variable Extraction (MCP)', () => { ]); }); - it('should prioritize customUserVars over user fields and system env vars if placeholders are the same (though not recommended)', () => { - // This tests the order of operations: customUserVars -> userFields -> systemEnv - // BUt it's generally not recommended to have overlapping placeholder names. - process.env.LIBRECHAT_USER_EMAIL = 'system-email-should-be-overridden'; - const user = createTestUser({ email: 'user-email-should-be-overridden' }); + it('should resolve reserved LIBRECHAT_* placeholders from the user even when customUserVars reuse the name', () => { + // customUserVars must not pre-empt trusted LIBRECHAT_USER_* values + process.env.LIBRECHAT_USER_EMAIL = 'system-email-should-be-ignored'; + const user = createTestUser({ email: 'user-email-wins' }); const customUserVars = { - LIBRECHAT_USER_EMAIL: 'custom-email-wins', + LIBRECHAT_USER_EMAIL: 'custom-email-should-be-ignored', }; const options: MCPOptions = { type: 'sse', @@ -754,7 +753,7 @@ describe('Environment Variable Extraction (MCP)', () => { }; const result = processMCPEnv({ options, user, customUserVars }); - expect('headers' in result && result.headers?.['Test-Email']).toBe('custom-email-wins'); + expect('headers' in result && result.headers?.['Test-Email']).toBe('user-email-wins'); // Clean up env var delete process.env.LIBRECHAT_USER_EMAIL; diff --git a/packages/api/src/utils/env.spec.ts b/packages/api/src/utils/env.spec.ts index 079043b6e55..e10791fdea9 100644 --- a/packages/api/src/utils/env.spec.ts +++ b/packages/api/src/utils/env.spec.ts @@ -281,7 +281,7 @@ describe('resolveHeaders', () => { }); }); - it('should prioritize custom user variables over user fields', () => { + it('should not let custom user variables override user fields', () => { const user = createTestUser({ id: 'user-123', email: 'user-email@example.com', @@ -297,7 +297,7 @@ describe('resolveHeaders', () => { const result = resolveHeaders({ headers, user, customUserVars }); expect(result).toEqual({ - 'Test-Email': 'custom-email@example.com', + 'Test-Email': 'user-email@example.com', }); }); @@ -2714,3 +2714,54 @@ describe('resolveHeaders with unavailable OpenID tokens for unattended requests' expect(result).toEqual({ 'x-openid-access-token': 'live-access-token' }); }); }); + +describe('processMCPEnv - customUserVars cannot shadow reserved LIBRECHAT_* placeholders (yaml servers)', () => { + const user = createTestUser({ id: 'u1', email: 'real@example.com' }); + + it('should resolve reserved placeholders from the real user, not a customUserVar with the same name', () => { + const options: MCPOptions = { + type: 'streamable-http', + url: 'https://api.example.com', + headers: { + 'X-User-Email': '{{LIBRECHAT_USER_EMAIL}}', + 'X-User-Id': '{{LIBRECHAT_USER_ID}}', + }, + }; + + const result = processMCPEnv({ + options, + user, + customUserVars: { LIBRECHAT_USER_EMAIL: 'other@example.com', LIBRECHAT_USER_ID: 'other-id' }, + }); + + if (!isStreamableHTTPOptions(result)) { + throw new Error('Expected streamable-http options'); + } + expect(result.headers?.['X-User-Email']).toBe('real@example.com'); + expect(result.headers?.['X-User-Id']).toBe('u1'); + }); + + it('should keep declared customUserVars working while reserved placeholders stay real', () => { + const options: MCPOptions = { + type: 'streamable-http', + url: 'https://api.example.com', + customUserVars: { API_KEY: { title: 'API Key', description: 'Your key' } }, + headers: { + 'X-Api-Key': '{{API_KEY}}', + 'X-User-Email': '{{LIBRECHAT_USER_EMAIL}}', + }, + }; + + const result = processMCPEnv({ + options, + user, + customUserVars: { API_KEY: 'k', LIBRECHAT_USER_EMAIL: 'other@example.com' }, + }); + + if (!isStreamableHTTPOptions(result)) { + throw new Error('Expected streamable-http options'); + } + expect(result.headers?.['X-Api-Key']).toBe('k'); + expect(result.headers?.['X-User-Email']).toBe('real@example.com'); + }); +}); diff --git a/packages/api/src/utils/env.ts b/packages/api/src/utils/env.ts index cb89e8e27e3..3e3651efca0 100644 --- a/packages/api/src/utils/env.ts +++ b/packages/api/src/utils/env.ts @@ -347,6 +347,10 @@ function processSingleValue({ /** Runs for both dbSourced and non-dbSourced — it is the only resolution DB-stored servers get */ if (customUserVars) { for (const [varName, varVal] of Object.entries(customUserVars)) { + /** `LIBRECHAT_*` placeholders are resolved from trusted sources below; a stored user var must not pre-empt them */ + if (varName.startsWith('LIBRECHAT_')) { + continue; + } const escapedVarName = varName.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); const placeholderRegex = new RegExp(`\\{\\{${escapedVarName}\\}\\}`, 'g'); value = value.replace(placeholderRegex, varVal);