mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-08-27 04:07:05 +00:00
🗝️ fix: Resolve MCP Runtime User and Request Placeholders (#13626)
* fix: Resolve MCP Runtime User Placeholders * fix: Harden MCP Runtime Placeholder Connections * fix: Update MCP Source Tag Test Expectations * fix: Complete MCP Runtime Placeholder Reinit * fix: Harden MCP Request Scoped Runtime Configs * fix: Align MCP OAuth Tests With Domain Policy * fix: Harden MCP Runtime Resolution Edges * fix: Avoid MCP Runtime Reprocessing Pitfalls * fix: Reuse MCP Request Scoped Tool Discovery * fix: Validate MCP Body Runtime Fields * 🛡️ refactor: Harden runtime placeholder edges from review - Warn at inspection when a trusted server URL contains runtime placeholders but no domain allowlist restricts the resolved target - Document the three resolution sites that must stay in sync so the validated config always matches the connected one - Note the per-call connect cost of ephemeral GRAPH/BODY connections - Drop the no-op removeUserConnection in callTool's ephemeral cleanup; ephemeral connections are never stored, and removing the entry could orphan a still-connected cached connection after a config change * 🪪 fix: Cover oauth_headers, Graph URL gating, and request-scoped reconnects Address Codex review: - Resolve runtime placeholders in oauth_headers (processMCPEnv + Graph pre-pass) and include the field in placeholder detection, so OAuth discovery/token requests no longer send literals; consolidate the detection field lists into one helper - Defer the early domain gate when the URL still carries a Graph placeholder (resolved async later); the authoritative assertResolvedRuntimeConfigAllowed check still enforces policy - Bypass the 10s reconnect throttle for request-scoped servers, which re-fetch tool definitions on every message by design
This commit is contained in:
parent
a7f16911b2
commit
7eafe317cc
29 changed files with 2235 additions and 85 deletions
|
|
@ -13,10 +13,14 @@ const {
|
|||
sanitizeGeminiSchema,
|
||||
buildMCPAuthStepId,
|
||||
buildMCPAuthToolCall,
|
||||
processMCPEnv,
|
||||
buildMCPAuthRunStepEvent,
|
||||
buildMCPAuthRunStepDeltaEvent,
|
||||
buildMCPAuthRunStepEndDeltaEvent,
|
||||
isUserSourced,
|
||||
checkAccessWithRequestCache,
|
||||
requiresEphemeralUserConnection,
|
||||
containsGraphTokenPlaceholder,
|
||||
} = require('@librechat/api');
|
||||
const {
|
||||
Time,
|
||||
|
|
@ -158,6 +162,41 @@ async function resolveAllMcpConfigs(userId, user) {
|
|||
return await registry.getAllServerConfigs(userId, configServers);
|
||||
}
|
||||
|
||||
function getServerCustomUserVars(userMCPAuthMap, serverName) {
|
||||
return userMCPAuthMap?.[`${Constants.mcp_prefix}${serverName}`];
|
||||
}
|
||||
|
||||
/**
|
||||
* Best-effort early gate; the authoritative check is
|
||||
* `assertResolvedRuntimeConfigAllowed` in `@librechat/api`, whose resolution
|
||||
* this must mirror. Graph placeholders resolve later (async), so a URL still
|
||||
* carrying one defers to the authoritative check instead of rejecting here.
|
||||
*/
|
||||
async function isEarlyDomainAllowed({
|
||||
serverConfig,
|
||||
user,
|
||||
requestBody,
|
||||
userMCPAuthMap,
|
||||
serverName,
|
||||
allowedDomains,
|
||||
allowedAddresses,
|
||||
}) {
|
||||
const validationConfig = processMCPEnv({
|
||||
user,
|
||||
body: requestBody,
|
||||
dbSourced: isUserSourced(serverConfig),
|
||||
options: serverConfig,
|
||||
customUserVars: getServerCustomUserVars(userMCPAuthMap, serverName),
|
||||
});
|
||||
if (
|
||||
typeof validationConfig?.url === 'string' &&
|
||||
containsGraphTokenPlaceholder(validationConfig.url)
|
||||
) {
|
||||
return true;
|
||||
}
|
||||
return await isMCPDomainAllowed(validationConfig, allowedDomains, allowedAddresses);
|
||||
}
|
||||
|
||||
/**
|
||||
* @param {string} toolName
|
||||
* @param {string} serverName
|
||||
|
|
@ -340,6 +379,7 @@ function createOAuthCallback({ runStepEmitter, runStepDeltaEmitter }) {
|
|||
* @param {number} [params.index]
|
||||
* @param {string | null} [params.streamId] - The stream ID for resumable mode.
|
||||
* @param {Record<string, Record<string, string>>} [params.userMCPAuthMap]
|
||||
* @param {import('@librechat/api').ParsedServerConfig} [params.serverConfig] - Used to bypass reconnect throttling for request-scoped servers.
|
||||
* @returns { Promise<Array<typeof tool | { _call: (toolInput: Object | string) => unknown}>> } An object with `_call` method to execute the tool input.
|
||||
*/
|
||||
async function reconnectServer({
|
||||
|
|
@ -348,23 +388,30 @@ async function reconnectServer({
|
|||
index,
|
||||
signal,
|
||||
serverName,
|
||||
serverConfig,
|
||||
configServers,
|
||||
userMCPAuthMap,
|
||||
requestBody,
|
||||
streamId = null,
|
||||
}) {
|
||||
logger.debug(
|
||||
`[MCP][reconnectServer] serverName: ${serverName}, user: ${user?.id}, hasUserMCPAuthMap: ${!!userMCPAuthMap}`,
|
||||
);
|
||||
|
||||
const throttleKey = `${user.id}:${serverName}`;
|
||||
const now = Date.now();
|
||||
const lastAttempt = lastReconnectAttempts.get(throttleKey) ?? 0;
|
||||
if (now - lastAttempt < RECONNECT_THROTTLE_MS) {
|
||||
logger.debug(`[MCP][reconnectServer] Throttled reconnect for ${serverName}`);
|
||||
return null;
|
||||
// Request-scoped servers reconnect on every message by design; throttling them
|
||||
// would stub out healthy tools for messages sent within the throttle window.
|
||||
const requestScoped = serverConfig ? requiresEphemeralUserConnection(serverConfig) : false;
|
||||
if (!requestScoped) {
|
||||
const throttleKey = `${user.id}:${serverName}`;
|
||||
const now = Date.now();
|
||||
const lastAttempt = lastReconnectAttempts.get(throttleKey) ?? 0;
|
||||
if (now - lastAttempt < RECONNECT_THROTTLE_MS) {
|
||||
logger.debug(`[MCP][reconnectServer] Throttled reconnect for ${serverName}`);
|
||||
return null;
|
||||
}
|
||||
lastReconnectAttempts.set(throttleKey, now);
|
||||
evictStale(lastReconnectAttempts, RECONNECT_THROTTLE_MS);
|
||||
}
|
||||
lastReconnectAttempts.set(throttleKey, now);
|
||||
evictStale(lastReconnectAttempts, RECONNECT_THROTTLE_MS);
|
||||
|
||||
const runId = Constants.USE_PRELIM_RESPONSE_MESSAGE_ID;
|
||||
const flowId = `${user.id}:${serverName}:${Date.now()}`;
|
||||
|
|
@ -420,6 +467,7 @@ async function reconnectServer({
|
|||
oauthStart,
|
||||
flowManager,
|
||||
userMCPAuthMap,
|
||||
requestBody,
|
||||
forceNew: true,
|
||||
returnOnOAuth: false,
|
||||
connectionTimeout: Time.THIRTY_SECONDS,
|
||||
|
|
@ -449,6 +497,7 @@ async function reconnectServer({
|
|||
* @param {AbortSignal} [params.signal]
|
||||
* @param {string | null} [params.streamId] - The stream ID for resumable mode.
|
||||
* @param {import('@librechat/api').ParsedServerConfig} [params.config]
|
||||
* @param {import('@librechat/api').RequestBody} [params.requestBody]
|
||||
* @param {Record<string, Record<string, string>>} [params.userMCPAuthMap]
|
||||
* @returns { Promise<Array<typeof tool | { _call: (toolInput: Object | string) => unknown}>> } An object with `_call` method to execute the tool input.
|
||||
*/
|
||||
|
|
@ -463,10 +512,12 @@ async function createMCPTools({
|
|||
serverName,
|
||||
configServers,
|
||||
userMCPAuthMap,
|
||||
requestBody,
|
||||
streamId = null,
|
||||
}) {
|
||||
const serverConfig =
|
||||
config ?? (await getMCPServersRegistry().getServerConfig(serverName, user?.id, configServers));
|
||||
|
||||
if (serverConfig?.url) {
|
||||
const appConfig = await getAppConfig({
|
||||
role: user?.role,
|
||||
|
|
@ -475,11 +526,15 @@ async function createMCPTools({
|
|||
});
|
||||
const allowedDomains = appConfig?.mcpSettings?.allowedDomains;
|
||||
const allowedAddresses = appConfig?.mcpSettings?.allowedAddresses;
|
||||
const isDomainAllowed = await isMCPDomainAllowed(
|
||||
const isDomainAllowed = await isEarlyDomainAllowed({
|
||||
serverConfig,
|
||||
user,
|
||||
requestBody,
|
||||
userMCPAuthMap,
|
||||
serverName,
|
||||
allowedDomains,
|
||||
allowedAddresses,
|
||||
);
|
||||
});
|
||||
if (!isDomainAllowed) {
|
||||
logger.warn(`[MCP][${serverName}] Domain not allowed, skipping all tools`);
|
||||
return [];
|
||||
|
|
@ -492,8 +547,10 @@ async function createMCPTools({
|
|||
index,
|
||||
signal,
|
||||
serverName,
|
||||
serverConfig,
|
||||
configServers,
|
||||
userMCPAuthMap,
|
||||
requestBody,
|
||||
streamId,
|
||||
});
|
||||
if (result === null) {
|
||||
|
|
@ -517,6 +574,7 @@ async function createMCPTools({
|
|||
streamId,
|
||||
availableTools: result.availableTools,
|
||||
toolKey: `${tool.name}${Constants.mcp_delimiter}${serverName}`,
|
||||
requestBody,
|
||||
config: serverConfig,
|
||||
});
|
||||
if (toolInstance) {
|
||||
|
|
@ -540,8 +598,10 @@ async function createMCPTools({
|
|||
* @param {string | null} [params.streamId] - The stream ID for resumable mode.
|
||||
* @param {Providers | EModelEndpoint} params.provider - The provider for the tool.
|
||||
* @param {LCAvailableTools} [params.availableTools]
|
||||
* @param {import('@librechat/api').RequestBody} [params.requestBody]
|
||||
* @param {Record<string, Record<string, string>>} [params.userMCPAuthMap]
|
||||
* @param {import('@librechat/api').ParsedServerConfig} [params.config]
|
||||
* @param {(availableTools: LCAvailableTools) => void} [params.onAvailableTools]
|
||||
* @returns { Promise<typeof tool | { _call: (toolInput: Object | string) => unknown}> } An object with `_call` method to execute the tool input.
|
||||
*/
|
||||
async function createMCPTool({
|
||||
|
|
@ -554,14 +614,19 @@ async function createMCPTool({
|
|||
provider,
|
||||
userMCPAuthMap,
|
||||
availableTools,
|
||||
requestBody,
|
||||
config,
|
||||
configServers,
|
||||
onAvailableTools,
|
||||
streamId = null,
|
||||
}) {
|
||||
const [toolName, serverName] = toolKey.split(Constants.mcp_delimiter);
|
||||
|
||||
const serverConfig =
|
||||
config ?? (await getMCPServersRegistry().getServerConfig(serverName, user?.id, configServers));
|
||||
const requestScopedTools = serverConfig ? requiresEphemeralUserConnection(serverConfig) : false;
|
||||
const useMissingToolCache = !requestScopedTools;
|
||||
|
||||
if (serverConfig?.url) {
|
||||
const appConfig = await getAppConfig({
|
||||
role: user?.role,
|
||||
|
|
@ -570,11 +635,15 @@ async function createMCPTool({
|
|||
});
|
||||
const allowedDomains = appConfig?.mcpSettings?.allowedDomains;
|
||||
const allowedAddresses = appConfig?.mcpSettings?.allowedAddresses;
|
||||
const isDomainAllowed = await isMCPDomainAllowed(
|
||||
const isDomainAllowed = await isEarlyDomainAllowed({
|
||||
serverConfig,
|
||||
user,
|
||||
requestBody,
|
||||
userMCPAuthMap,
|
||||
serverName,
|
||||
allowedDomains,
|
||||
allowedAddresses,
|
||||
);
|
||||
});
|
||||
if (!isDomainAllowed) {
|
||||
logger.warn(`[MCP][${serverName}] Domain no longer allowed, skipping tool: ${toolName}`);
|
||||
return undefined;
|
||||
|
|
@ -584,7 +653,7 @@ async function createMCPTool({
|
|||
/** @type {LCTool | undefined} */
|
||||
let toolDefinition = availableTools?.[toolKey]?.function;
|
||||
if (!toolDefinition) {
|
||||
const cachedAt = missingToolCache.get(toolKey);
|
||||
const cachedAt = useMissingToolCache ? missingToolCache.get(toolKey) : undefined;
|
||||
if (cachedAt && Date.now() - cachedAt < MISSING_TOOL_TTL_MS) {
|
||||
logger.debug(
|
||||
`[MCP][${serverName}][${toolName}] Tool in negative cache, returning unavailable stub.`,
|
||||
|
|
@ -601,13 +670,18 @@ async function createMCPTool({
|
|||
index,
|
||||
signal,
|
||||
serverName,
|
||||
serverConfig,
|
||||
configServers,
|
||||
userMCPAuthMap,
|
||||
requestBody,
|
||||
streamId,
|
||||
});
|
||||
if (result?.availableTools) {
|
||||
onAvailableTools?.(result.availableTools);
|
||||
}
|
||||
toolDefinition = result?.availableTools?.[toolKey]?.function;
|
||||
|
||||
if (!toolDefinition) {
|
||||
if (!toolDefinition && useMissingToolCache) {
|
||||
missingToolCache.set(toolKey, Date.now());
|
||||
evictStale(missingToolCache, MISSING_TOOL_TTL_MS);
|
||||
}
|
||||
|
|
@ -624,6 +698,7 @@ async function createMCPTool({
|
|||
res,
|
||||
mcpPermissionContext,
|
||||
user,
|
||||
requestBody,
|
||||
provider,
|
||||
toolName,
|
||||
serverName,
|
||||
|
|
@ -637,6 +712,7 @@ function createToolInstance({
|
|||
res,
|
||||
mcpPermissionContext,
|
||||
user: capturedUser = null,
|
||||
requestBody: capturedRequestBody,
|
||||
toolName,
|
||||
serverName,
|
||||
serverConfig: capturedServerConfig,
|
||||
|
|
@ -670,9 +746,9 @@ function createToolInstance({
|
|||
|
||||
/** @type {(toolArguments: Object | string, config?: GraphRunnableConfig) => Promise<unknown>} */
|
||||
const _call = async (toolArguments, config) => {
|
||||
const permissionUser = config?.configurable?.user ?? capturedUser;
|
||||
const userId =
|
||||
config?.configurable?.user?.id || config?.configurable?.user_id || capturedUser?.id;
|
||||
const effectiveUser = config?.configurable?.user ?? capturedUser;
|
||||
const permissionUser = effectiveUser;
|
||||
const userId = effectiveUser?.id || config?.configurable?.user_id || capturedUser?.id;
|
||||
/** @type {ReturnType<typeof createAbortHandler>} */
|
||||
let abortHandler = null;
|
||||
/** @type {AbortSignal} */
|
||||
|
|
@ -728,8 +804,8 @@ function createToolInstance({
|
|||
options: {
|
||||
signal: derivedSignal,
|
||||
},
|
||||
user: config?.configurable?.user,
|
||||
requestBody: config?.configurable?.requestBody,
|
||||
user: effectiveUser,
|
||||
requestBody: config?.configurable?.requestBody ?? capturedRequestBody,
|
||||
customUserVars,
|
||||
flowManager,
|
||||
tokenMethods: {
|
||||
|
|
|
|||
|
|
@ -873,6 +873,37 @@ describe('User parameter passing tests', () => {
|
|||
expect(mockReinitMCPServer.mock.calls[0][0].user).toBe(mockUser);
|
||||
});
|
||||
|
||||
it('should report available tools discovered during single tool reinit', async () => {
|
||||
const mockUser = { id: 'user-discovery-callback', role: 'USER' };
|
||||
const mockRes = { write: jest.fn(), flush: jest.fn() };
|
||||
const onAvailableTools = jest.fn();
|
||||
const discoveredTools = {
|
||||
[`my-tool${D}my-server`]: {
|
||||
function: { description: 'My Tool', parameters: {} },
|
||||
},
|
||||
[`other-tool${D}my-server`]: {
|
||||
function: { description: 'Other Tool', parameters: {} },
|
||||
},
|
||||
};
|
||||
|
||||
mockReinitMCPServer.mockResolvedValue({
|
||||
availableTools: discoveredTools,
|
||||
});
|
||||
|
||||
const result = await createMCPTool({
|
||||
res: mockRes,
|
||||
user: mockUser,
|
||||
toolKey: `my-tool${D}my-server`,
|
||||
provider: 'openai',
|
||||
userMCPAuthMap: {},
|
||||
availableTools: undefined,
|
||||
onAvailableTools,
|
||||
});
|
||||
|
||||
expect(result).toBeDefined();
|
||||
expect(onAvailableTools).toHaveBeenCalledWith(discoveredTools);
|
||||
});
|
||||
|
||||
it('should not call reinitMCPServer when tool is in cache', async () => {
|
||||
const mockUser = { id: 'test-user-789' };
|
||||
const mockRes = { write: jest.fn(), flush: jest.fn() };
|
||||
|
|
@ -1017,6 +1048,126 @@ describe('User parameter passing tests', () => {
|
|||
expect(getRoleByName).toHaveBeenCalledTimes(1);
|
||||
expect(mockCallTool).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
it('should pass the captured user to MCPManager.callTool when invocation config omits configurable.user', async () => {
|
||||
const mockUser = { id: 'captured-user', email: 'captured@example.com', role: 'USER' };
|
||||
const mockRes = { write: jest.fn(), flush: jest.fn() };
|
||||
const { getRoleByName } = require('~/models');
|
||||
getRoleByName.mockResolvedValue({
|
||||
permissions: {
|
||||
[PermissionTypes.MCP_SERVERS]: {
|
||||
[Permissions.USE]: true,
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
const mockCallTool = jest.fn().mockResolvedValue(['ok', null]);
|
||||
mockGetMCPManager.mockReturnValue({
|
||||
callTool: mockCallTool,
|
||||
});
|
||||
|
||||
const mcpTool = await createMCPTool({
|
||||
res: mockRes,
|
||||
user: mockUser,
|
||||
toolKey: `test-tool${D}test-server`,
|
||||
provider: 'openai',
|
||||
userMCPAuthMap: {},
|
||||
availableTools: {
|
||||
[`test-tool${D}test-server`]: {
|
||||
function: {
|
||||
description: 'Cached tool',
|
||||
parameters: { type: 'object', properties: {} },
|
||||
},
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
await expect(
|
||||
mcpTool.invoke(
|
||||
{},
|
||||
{
|
||||
configurable: {
|
||||
user_id: mockUser.id,
|
||||
},
|
||||
metadata: {
|
||||
provider: 'openai',
|
||||
thread_id: 'thread-1',
|
||||
run_id: 'run-1',
|
||||
},
|
||||
toolCall: {},
|
||||
},
|
||||
),
|
||||
).resolves.toBe('ok');
|
||||
|
||||
expect(mockCallTool).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
serverName: 'test-server',
|
||||
toolName: 'test-tool',
|
||||
user: mockUser,
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('should pass captured request body when invocation config omits requestBody', async () => {
|
||||
const mockUser = { id: 'captured-body-user', email: 'captured@example.com', role: 'USER' };
|
||||
const requestBody = { conversationId: 'conv-123', messageId: 'msg-123' };
|
||||
const mockRes = { write: jest.fn(), flush: jest.fn() };
|
||||
const { getRoleByName } = require('~/models');
|
||||
getRoleByName.mockResolvedValue({
|
||||
permissions: {
|
||||
[PermissionTypes.MCP_SERVERS]: {
|
||||
[Permissions.USE]: true,
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
const mockCallTool = jest.fn().mockResolvedValue(['ok', null]);
|
||||
mockGetMCPManager.mockReturnValue({
|
||||
callTool: mockCallTool,
|
||||
});
|
||||
|
||||
const mcpTool = await createMCPTool({
|
||||
res: mockRes,
|
||||
user: mockUser,
|
||||
requestBody,
|
||||
toolKey: `test-tool${D}test-server`,
|
||||
provider: 'openai',
|
||||
userMCPAuthMap: {},
|
||||
availableTools: {
|
||||
[`test-tool${D}test-server`]: {
|
||||
function: {
|
||||
description: 'Cached tool',
|
||||
parameters: { type: 'object', properties: {} },
|
||||
},
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
await expect(
|
||||
mcpTool.invoke(
|
||||
{},
|
||||
{
|
||||
configurable: {
|
||||
user: mockUser,
|
||||
},
|
||||
metadata: {
|
||||
provider: 'openai',
|
||||
thread_id: 'thread-1',
|
||||
run_id: 'run-1',
|
||||
},
|
||||
toolCall: {},
|
||||
},
|
||||
),
|
||||
).resolves.toBe('ok');
|
||||
|
||||
expect(mockCallTool).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
serverName: 'test-server',
|
||||
toolName: 'test-tool',
|
||||
requestBody,
|
||||
}),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('reinitMCPServer (via reconnectServer)', () => {
|
||||
|
|
@ -1187,6 +1338,50 @@ describe('User parameter passing tests', () => {
|
|||
});
|
||||
});
|
||||
|
||||
it('should validate the resolved runtime URL for tool creation', async () => {
|
||||
const mockUser = { id: 'runtime-domain-user', role: 'user' };
|
||||
const requestBody = { conversationId: 'tenant-a' };
|
||||
const mockRes = { write: jest.fn(), flush: jest.fn() };
|
||||
|
||||
mockRegistryInstance.getServerConfig.mockResolvedValue({
|
||||
type: 'streamable-http',
|
||||
url: 'https://{{LIBRECHAT_BODY_CONVERSATIONID}}.example.com/sse',
|
||||
source: 'yaml',
|
||||
});
|
||||
|
||||
mockGetAppConfig.mockResolvedValue({
|
||||
mcpSettings: { allowedDomains: ['*.example.com'] },
|
||||
});
|
||||
|
||||
mockIsMCPDomainAllowed.mockResolvedValueOnce(true);
|
||||
|
||||
const result = await createMCPTool({
|
||||
res: mockRes,
|
||||
user: mockUser,
|
||||
requestBody,
|
||||
toolKey: `test-tool${D}test-server`,
|
||||
provider: 'openai',
|
||||
userMCPAuthMap: {},
|
||||
availableTools: {
|
||||
[`test-tool${D}test-server`]: {
|
||||
function: {
|
||||
description: 'Test tool',
|
||||
parameters: { type: 'object', properties: {} },
|
||||
},
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
expect(result).toBeDefined();
|
||||
expect(mockIsMCPDomainAllowed).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
url: 'https://tenant-a.example.com/sse',
|
||||
}),
|
||||
['*.example.com'],
|
||||
undefined,
|
||||
);
|
||||
});
|
||||
|
||||
it('should skip domain validation for stdio transports (no URL)', async () => {
|
||||
const mockUser = { id: 'stdio-test-user' };
|
||||
const mockRes = { write: jest.fn(), flush: jest.fn() };
|
||||
|
|
@ -1470,6 +1665,56 @@ describe('User parameter passing tests', () => {
|
|||
// Still only 1 real reconnect — user B was protected by the cache
|
||||
expect(mockReinitMCPServer).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('should bypass the negative cache for request-scoped tools', async () => {
|
||||
const userA = { id: 'request-scoped-user-A' };
|
||||
const userB = { id: 'request-scoped-user-B' };
|
||||
const mockRes = { write: jest.fn(), flush: jest.fn() };
|
||||
const serverName = 'request-scoped-server';
|
||||
const toolKey = `tenant-tool${D}${serverName}`;
|
||||
|
||||
mockRegistryInstance.getServerConfig.mockResolvedValue({
|
||||
type: 'streamable-http',
|
||||
url: 'https://api.example.com/{{LIBRECHAT_BODY_MESSAGEID}}/mcp',
|
||||
source: 'yaml',
|
||||
});
|
||||
|
||||
mockReinitMCPServer
|
||||
.mockResolvedValueOnce({
|
||||
availableTools: {},
|
||||
})
|
||||
.mockResolvedValueOnce({
|
||||
availableTools: {
|
||||
[toolKey]: {
|
||||
function: { description: 'Tenant tool', parameters: {} },
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
await createMCPTool({
|
||||
res: mockRes,
|
||||
user: userA,
|
||||
requestBody: { messageId: 'message-a' },
|
||||
toolKey,
|
||||
provider: 'openai',
|
||||
userMCPAuthMap: {},
|
||||
availableTools: undefined,
|
||||
});
|
||||
|
||||
const result = await createMCPTool({
|
||||
res: mockRes,
|
||||
user: userB,
|
||||
requestBody: { messageId: 'message-b' },
|
||||
toolKey,
|
||||
provider: 'openai',
|
||||
userMCPAuthMap: {},
|
||||
availableTools: undefined,
|
||||
});
|
||||
|
||||
expect(result).toBeDefined();
|
||||
expect(result.name).toContain('tenant-tool');
|
||||
expect(mockReinitMCPServer).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
});
|
||||
|
||||
describe('createMCPTools throttle handling', () => {
|
||||
|
|
|
|||
|
|
@ -16,6 +16,7 @@ const {
|
|||
isActionDomainAllowed,
|
||||
buildWebSearchContext,
|
||||
buildImageToolContext,
|
||||
requiresEphemeralUserConnection,
|
||||
buildToolClassification,
|
||||
getMissingCustomUserVars,
|
||||
buildWebSearchDynamicContext,
|
||||
|
|
@ -743,7 +744,9 @@ async function loadToolDefinitionsWrapper({ req, res, agent, streamId = null, to
|
|||
return null;
|
||||
}
|
||||
|
||||
const cached = await getMCPServerTools(userId, serverName);
|
||||
const cached = requiresEphemeralUserConnection(serverConfig)
|
||||
? null
|
||||
: await getMCPServerTools(userId, serverName);
|
||||
if (cached) {
|
||||
await addPendingOAuthServer();
|
||||
return cached;
|
||||
|
|
@ -767,6 +770,7 @@ async function loadToolDefinitionsWrapper({ req, res, agent, streamId = null, to
|
|||
serverName,
|
||||
configServers,
|
||||
userMCPAuthMap,
|
||||
requestBody: req.body,
|
||||
});
|
||||
|
||||
return result?.availableTools || null;
|
||||
|
|
@ -885,6 +889,7 @@ async function loadToolDefinitionsWrapper({ req, res, agent, streamId = null, to
|
|||
configServers,
|
||||
userMCPAuthMap,
|
||||
flowManager,
|
||||
requestBody: req.body,
|
||||
returnOnOAuth: false,
|
||||
oauthStart,
|
||||
oauthEnd: createOAuthEndEmitter(serverName),
|
||||
|
|
|
|||
|
|
@ -1,8 +1,9 @@
|
|||
const { logger } = require('@librechat/data-schemas');
|
||||
const { getMissingCustomUserVars } = require('@librechat/api');
|
||||
const { getMissingCustomUserVars, requiresEphemeralUserConnection } = require('@librechat/api');
|
||||
const { CacheKeys, Constants } = require('librechat-data-provider');
|
||||
const { getMCPManager, getMCPServersRegistry, getFlowStateManager } = require('~/config');
|
||||
const { findToken, createToken, updateToken, deleteTokens } = require('~/models');
|
||||
const { getGraphApiToken } = require('~/server/services/GraphTokenService');
|
||||
const { exchangeOboToken } = require('~/server/services/OboTokenService');
|
||||
const { createOboTrustChecker } = require('~/server/services/OboPolicyService');
|
||||
const { updateMCPServerTools } = require('~/server/services/Config');
|
||||
|
|
@ -22,6 +23,7 @@ const { getLogStores } = require('~/cache');
|
|||
* @param {FlowStateManager<any>} [params.flowManager]
|
||||
* @param {(authURL: string, options?: { expiresAt?: number }) => Promise<void>} [params.oauthStart]
|
||||
* @param {() => Promise<void>} [params.oauthEnd]
|
||||
* @param {import('@librechat/api').RequestBody} [params.requestBody]
|
||||
* @param {Record<string, Record<string, string>>} [params.userMCPAuthMap]
|
||||
*/
|
||||
async function reinitMCPServer({
|
||||
|
|
@ -36,21 +38,25 @@ async function reinitMCPServer({
|
|||
oauthStart: _oauthStart,
|
||||
flowManager: _flowManager,
|
||||
serverConfig: providedConfig,
|
||||
requestBody,
|
||||
oauthEnd,
|
||||
}) {
|
||||
/** @type {MCPConnection | null} */
|
||||
let connection = null;
|
||||
let serverConfig = providedConfig;
|
||||
/** @type {LCAvailableTools | null} */
|
||||
let availableTools = null;
|
||||
/** @type {ReturnType<MCPConnection['fetchTools']> | null} */
|
||||
let tools = null;
|
||||
let oauthRequired = false;
|
||||
let oauthUrl = null;
|
||||
let ephemeralServer = false;
|
||||
|
||||
try {
|
||||
const registry = getMCPServersRegistry();
|
||||
const serverConfig =
|
||||
providedConfig ?? (await registry.getServerConfig(serverName, user?.id, configServers));
|
||||
serverConfig =
|
||||
serverConfig ?? (await registry.getServerConfig(serverName, user?.id, configServers));
|
||||
ephemeralServer = serverConfig ? requiresEphemeralUserConnection(serverConfig) : false;
|
||||
if (serverConfig?.inspectionFailed) {
|
||||
if (serverConfig.source === 'config') {
|
||||
logger.info(
|
||||
|
|
@ -137,8 +143,10 @@ async function reinitMCPServer({
|
|||
returnOnOAuth,
|
||||
oauthEnd,
|
||||
customUserVars,
|
||||
requestBody,
|
||||
connectionTimeout,
|
||||
serverConfig,
|
||||
graphTokenResolver: getGraphApiToken,
|
||||
oboTokenResolver: exchangeOboToken,
|
||||
oboTrustChecker: createOboTrustChecker(),
|
||||
});
|
||||
|
|
@ -172,8 +180,10 @@ async function reinitMCPServer({
|
|||
tokenMethods,
|
||||
oauthStart,
|
||||
customUserVars,
|
||||
requestBody,
|
||||
connectionTimeout,
|
||||
configServers,
|
||||
graphTokenResolver: getGraphApiToken,
|
||||
oboTokenResolver: exchangeOboToken,
|
||||
oboTrustChecker: createOboTrustChecker(),
|
||||
});
|
||||
|
|
@ -206,6 +216,7 @@ async function reinitMCPServer({
|
|||
userId: user.id,
|
||||
serverName,
|
||||
tools,
|
||||
skipCache: ephemeralServer,
|
||||
});
|
||||
}
|
||||
|
||||
|
|
@ -253,6 +264,17 @@ async function reinitMCPServer({
|
|||
'[MCP Reinitialize] Error loading MCP Tools, servers may still be initializing:',
|
||||
error,
|
||||
);
|
||||
} finally {
|
||||
if (connection && ephemeralServer) {
|
||||
try {
|
||||
await connection.disconnect();
|
||||
} catch (error) {
|
||||
logger.warn(
|
||||
`[MCP Reinitialize] Failed to disconnect ephemeral server ${serverName}`,
|
||||
error,
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -1,9 +1,15 @@
|
|||
const { Constants } = require('librechat-data-provider');
|
||||
|
||||
const mockGetConnection = jest.fn();
|
||||
const mockDiscoverServerTools = jest.fn();
|
||||
const mockGetGraphApiToken = jest.fn();
|
||||
const mockUpdateMCPServerTools = jest.fn();
|
||||
|
||||
jest.mock('~/config', () => ({
|
||||
getMCPManager: jest.fn(() => ({ getConnection: mockGetConnection })),
|
||||
getMCPManager: jest.fn(() => ({
|
||||
getConnection: mockGetConnection,
|
||||
discoverServerTools: mockDiscoverServerTools,
|
||||
})),
|
||||
getMCPServersRegistry: jest.fn(() => ({ getServerConfig: jest.fn() })),
|
||||
getFlowStateManager: jest.fn(() => ({})),
|
||||
}));
|
||||
|
|
@ -14,7 +20,10 @@ jest.mock('~/models', () => ({
|
|||
deleteTokens: jest.fn(),
|
||||
}));
|
||||
jest.mock('~/server/services/Config', () => ({
|
||||
updateMCPServerTools: jest.fn(),
|
||||
updateMCPServerTools: mockUpdateMCPServerTools,
|
||||
}));
|
||||
jest.mock('~/server/services/GraphTokenService', () => ({
|
||||
getGraphApiToken: mockGetGraphApiToken,
|
||||
}));
|
||||
jest.mock('~/cache', () => ({
|
||||
getLogStores: jest.fn(() => ({})),
|
||||
|
|
@ -35,6 +44,7 @@ describe('reinitMCPServer — customUserVars gating (issue #10969)', () => {
|
|||
|
||||
beforeEach(() => {
|
||||
jest.clearAllMocks();
|
||||
mockUpdateMCPServerTools.mockResolvedValue({});
|
||||
});
|
||||
|
||||
it('does not connect and exposes no tools when a required customUserVar is unset', async () => {
|
||||
|
|
@ -90,6 +100,76 @@ describe('reinitMCPServer — customUserVars gating (issue #10969)', () => {
|
|||
);
|
||||
});
|
||||
|
||||
it('passes request body and Graph resolver into connection creation', async () => {
|
||||
mockGetConnection.mockResolvedValue({ fetchTools: jest.fn().mockResolvedValue([]) });
|
||||
const requestBody = { conversationId: 'conv-123', messageId: 'msg-123' };
|
||||
|
||||
await reinitMCPServer({
|
||||
user,
|
||||
serverName,
|
||||
serverConfig: { type: 'streamable-http', url: 'https://thingy.example.com/mcp' },
|
||||
requestBody,
|
||||
userMCPAuthMap: undefined,
|
||||
});
|
||||
|
||||
expect(mockGetConnection).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
requestBody,
|
||||
graphTokenResolver: mockGetGraphApiToken,
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('passes request body and Graph resolver into OAuth discovery fallback', async () => {
|
||||
mockGetConnection.mockRejectedValue(new Error('OAuth authentication required'));
|
||||
mockDiscoverServerTools.mockResolvedValue({ tools: [], oauthRequired: true, oauthUrl: null });
|
||||
const requestBody = { conversationId: 'conv-456', messageId: 'msg-456' };
|
||||
|
||||
await reinitMCPServer({
|
||||
user,
|
||||
serverName,
|
||||
serverConfig: { type: 'streamable-http', url: 'https://thingy.example.com/mcp' },
|
||||
requestBody,
|
||||
userMCPAuthMap: undefined,
|
||||
});
|
||||
|
||||
expect(mockDiscoverServerTools).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
requestBody,
|
||||
graphTokenResolver: mockGetGraphApiToken,
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('disconnects ephemeral BODY-scoped connections after loading tools', async () => {
|
||||
const disconnect = jest.fn().mockResolvedValue(undefined);
|
||||
const tools = [{ name: 'search', inputSchema: { type: 'object', properties: {} } }];
|
||||
mockGetConnection.mockResolvedValue({
|
||||
disconnect,
|
||||
fetchTools: jest.fn().mockResolvedValue(tools),
|
||||
});
|
||||
|
||||
await reinitMCPServer({
|
||||
user,
|
||||
serverName,
|
||||
serverConfig: {
|
||||
type: 'streamable-http',
|
||||
url: 'https://thingy.example.com/messages/{{LIBRECHAT_BODY_MESSAGEID}}/mcp',
|
||||
source: 'yaml',
|
||||
},
|
||||
requestBody: { messageId: 'msg-789' },
|
||||
userMCPAuthMap: undefined,
|
||||
});
|
||||
|
||||
expect(disconnect).toHaveBeenCalledTimes(1);
|
||||
expect(mockUpdateMCPServerTools).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
tools,
|
||||
skipCache: true,
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('proceeds to connect when the server declares no customUserVars', async () => {
|
||||
mockGetConnection.mockResolvedValue({ fetchTools: jest.fn().mockResolvedValue([]) });
|
||||
|
||||
|
|
|
|||
|
|
@ -719,6 +719,46 @@ describe('ToolService - Action Capability Gating', () => {
|
|||
);
|
||||
});
|
||||
|
||||
it('should pass request body context into MCP tool definition reinitialization', async () => {
|
||||
const serverName = 'Body-Scoped';
|
||||
const mcpTool = `search${Constants.mcp_delimiter}${serverName}`;
|
||||
const capabilities = [AgentCapabilities.tools];
|
||||
const req = createMockReq(capabilities);
|
||||
req.body = { conversationId: 'conv-123', messageId: 'msg-123' };
|
||||
|
||||
mockGetEndpointsConfig.mockResolvedValue(createEndpointsConfig(capabilities));
|
||||
mockGetServerConfig.mockResolvedValue({
|
||||
type: 'streamable-http',
|
||||
url: 'https://demo.librechat.ai/messages/{{LIBRECHAT_BODY_MESSAGEID}}/mcp',
|
||||
source: 'yaml',
|
||||
});
|
||||
mockGetMCPServerTools.mockResolvedValue(null);
|
||||
mockFlowManager.getFlowState.mockResolvedValue(null);
|
||||
mockLoadToolDefinitions.mockImplementation(async (params, deps) => {
|
||||
await deps.getOrFetchMCPServerTools(params.userId, serverName);
|
||||
return {
|
||||
toolDefinitions: [],
|
||||
toolRegistry: new Map(),
|
||||
hasDeferredTools: false,
|
||||
};
|
||||
});
|
||||
reinitMCPServer.mockResolvedValue({ availableTools: null });
|
||||
|
||||
await loadAgentTools({
|
||||
req,
|
||||
agent: { id: 'agent_123', tools: [mcpTool] },
|
||||
definitionsOnly: true,
|
||||
});
|
||||
|
||||
expect(reinitMCPServer).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
serverName,
|
||||
requestBody: req.body,
|
||||
}),
|
||||
);
|
||||
expect(mockGetMCPServerTools).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should preserve pending-flow expiry for OAuth URLs captured during discovery', async () => {
|
||||
const serverName = 'Google-Workspace';
|
||||
const authorizationUrl = 'https://auth.example.com/Google-Workspace';
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue