mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-08-04 14:57:42 +00:00
fix: persist resolved MCP server provenance on agents
Deriving mcpServerNames from the tool key cannot tell a config server's trailing segment from a real DB server name, so a config server named a_mcp_b indexed an unrelated DB server b and shared the agent's viewers into it. Neither string rule works: the suffix guess exposes, and failing closed drops legitimate DB access for gateway-prefixed tools. filterAuthorizedTools already resolves each tool's server against the merged registry config, so it now collects those names and create, update and duplicate persist them. No extra registry queries: the update path unions the newly resolved names with what the agent already had, and duplicate replaces the copied list rather than inheriting the source's servers. Display parsing also takes the configured names, so a real tool call on a delimiter-bearing server renders the right server and icon.
This commit is contained in:
parent
e7c72d57dd
commit
f6fbf08687
5 changed files with 47 additions and 8 deletions
|
|
@ -215,6 +215,7 @@ const filterAuthorizedTools = async ({
|
|||
availableTools,
|
||||
existingTools,
|
||||
configServers,
|
||||
resolvedServerNames,
|
||||
}) => {
|
||||
const filteredTools = [];
|
||||
let mcpServerConfigs;
|
||||
|
|
@ -283,6 +284,7 @@ const filterAuthorizedTools = async ({
|
|||
continue;
|
||||
}
|
||||
|
||||
resolvedServerNames?.add(serverName);
|
||||
filteredTools.push(tool);
|
||||
}
|
||||
|
||||
|
|
@ -433,6 +435,9 @@ const createAgentHandler = async (req, res) => {
|
|||
hasMCPTools ? resolveConfigServers(req) : Promise.resolve(undefined),
|
||||
]);
|
||||
const mcpPermissionContext = createMCPPermissionContext(req);
|
||||
/** Resolved during authorization, so persistence indexes the real server rather
|
||||
* than a suffix guess - see the note on `filterAuthorizedTools`. */
|
||||
const resolvedServerNames = new Set();
|
||||
agentData.tools = await filterAuthorizedTools({
|
||||
tools,
|
||||
userId,
|
||||
|
|
@ -441,7 +446,11 @@ const createAgentHandler = async (req, res) => {
|
|||
mcpPermissionContext,
|
||||
availableTools,
|
||||
configServers,
|
||||
resolvedServerNames,
|
||||
});
|
||||
if (hasMCPTools) {
|
||||
agentData.mcpServerNames = Array.from(resolvedServerNames);
|
||||
}
|
||||
|
||||
const agent = await db.createAgent(agentData);
|
||||
|
||||
|
|
@ -735,6 +744,7 @@ const updateAgentHandler = async (req, res) => {
|
|||
getCachedTools().then((t) => t ?? {}),
|
||||
resolveConfigServers(req),
|
||||
]);
|
||||
const resolvedServerNames = new Set();
|
||||
const approvedNew = await filterAuthorizedTools({
|
||||
tools: newMCPTools,
|
||||
userId: req.user.id,
|
||||
|
|
@ -743,11 +753,19 @@ const updateAgentHandler = async (req, res) => {
|
|||
mcpPermissionContext,
|
||||
availableTools,
|
||||
configServers,
|
||||
resolvedServerNames,
|
||||
});
|
||||
const rejectedSet = new Set(newMCPTools.filter((t) => !approvedNew.includes(t)));
|
||||
if (rejectedSet.size > 0) {
|
||||
updateData.tools = updateData.tools.filter((t) => !rejectedSet.has(t));
|
||||
}
|
||||
/** Union with what the agent already had: the new tools were resolved during
|
||||
* authorization, and re-deriving the rest from their keys would reintroduce
|
||||
* the suffix guess this avoids. */
|
||||
for (const existingName of existingAgent.mcpServerNames ?? []) {
|
||||
resolvedServerNames.add(existingName);
|
||||
}
|
||||
updateData.mcpServerNames = Array.from(resolvedServerNames);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -903,6 +921,9 @@ const duplicateAgentHandler = async (req, res) => {
|
|||
resolveConfigServers(req),
|
||||
]);
|
||||
const mcpPermissionContext = createMCPPermissionContext(req);
|
||||
/** The duplicate carries the source agent's `mcpServerNames`; replace it with what
|
||||
* this user is actually authorized for, or the copy would grant the source's servers. */
|
||||
const resolvedServerNames = new Set();
|
||||
newAgentData.tools = await filterAuthorizedTools({
|
||||
tools: newAgentData.tools,
|
||||
userId,
|
||||
|
|
@ -912,7 +933,9 @@ const duplicateAgentHandler = async (req, res) => {
|
|||
availableTools,
|
||||
existingTools: newAgentData.tools,
|
||||
configServers,
|
||||
resolvedServerNames,
|
||||
});
|
||||
newAgentData.mcpServerNames = Array.from(resolvedServerNames);
|
||||
}
|
||||
|
||||
if (newAgentData.tool_resources) {
|
||||
|
|
|
|||
|
|
@ -12,7 +12,7 @@ import {
|
|||
import type { TAttachment } from 'librechat-data-provider';
|
||||
import { useLocalize, useProgress, useExpandCollapse } from '~/hooks';
|
||||
import { ToolIcon, getToolIconType, isError } from './ToolOutput';
|
||||
import { useMCPIconMap } from '~/hooks/MCP';
|
||||
import { useMCPIconMap, useMCPServerNames } from '~/hooks/MCP';
|
||||
import { AttachmentGroup } from './Parts';
|
||||
import ToolCallInfo from './ToolCallInfo';
|
||||
import ProgressText from './ProgressText';
|
||||
|
|
@ -67,12 +67,13 @@ export default function ToolCall({
|
|||
}
|
||||
}, [auth]);
|
||||
|
||||
const mcpServerNames = useMCPServerNames();
|
||||
const { function_name, domain, isMCPToolCall, mcpServerName } = useMemo(() => {
|
||||
if (typeof name !== 'string') {
|
||||
return { function_name: '', domain: null, isMCPToolCall: false, mcpServerName: '' };
|
||||
}
|
||||
if (name.includes(Constants.mcp_delimiter)) {
|
||||
const [func, server = ''] = splitToolCallName(name);
|
||||
const [func, server = ''] = splitToolCallName(name, mcpServerNames);
|
||||
const displayName = func === 'oauth' ? server : func;
|
||||
return {
|
||||
function_name: displayName || '',
|
||||
|
|
@ -104,7 +105,7 @@ export default function ToolCall({
|
|||
isMCPToolCall: false,
|
||||
mcpServerName: '',
|
||||
};
|
||||
}, [name, parsedAuthUrl]);
|
||||
}, [name, parsedAuthUrl, mcpServerNames]);
|
||||
|
||||
const toolIconType = useMemo(() => getToolIconType(name), [name]);
|
||||
const mcpIconMap = useMCPIconMap();
|
||||
|
|
|
|||
|
|
@ -3,5 +3,5 @@ export * from './useVisibleTools';
|
|||
export * from './useMCPServerManager';
|
||||
export * from './useMCPConnectionStatus';
|
||||
|
||||
export { useMCPIconMap } from './useMCPIconMap';
|
||||
export { useMCPIconMap, useMCPServerNames } from './useMCPIconMap';
|
||||
export { useRemoveMCPTool } from './useRemoveMCPTool';
|
||||
|
|
|
|||
|
|
@ -17,3 +17,9 @@ export function useMCPIconMap(): Map<string, string> {
|
|||
return map;
|
||||
}, [servers]);
|
||||
}
|
||||
|
||||
/** Configured MCP server names, used to resolve the tool-key boundary exactly. */
|
||||
export function useMCPServerNames(): string[] {
|
||||
const { data: servers } = useMCPServersQuery();
|
||||
return useMemo(() => (servers ? Object.keys(servers) : []), [servers]);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -368,7 +368,11 @@ export function createAgentMethods(
|
|||
},
|
||||
],
|
||||
category: (agentData.category as string) || 'general',
|
||||
mcpServerNames: extractMCPServerNames(agentData.tools as string[] | undefined),
|
||||
/** Callers that authorized the tools pass resolved names; deriving from the key
|
||||
* alone cannot tell a config server's suffix from a real DB server name. */
|
||||
mcpServerNames:
|
||||
(agentData.mcpServerNames as string[] | undefined) ??
|
||||
extractMCPServerNames(agentData.tools as string[] | undefined),
|
||||
};
|
||||
|
||||
return (await Agent.create(initialAgentData)).toObject() as IAgent;
|
||||
|
|
@ -523,9 +527,14 @@ export function createAgentMethods(
|
|||
|
||||
// Sync mcpServerNames when tools are updated
|
||||
if ((directUpdates as Record<string, unknown>).tools !== undefined) {
|
||||
const mcpServerNames = extractMCPServerNames(
|
||||
(directUpdates as Record<string, unknown>).tools as string[],
|
||||
);
|
||||
/** Callers that authorized the tools pass resolved names; deriving from the key
|
||||
* alone cannot tell a config server's suffix from a real DB server name. */
|
||||
const supplied = (directUpdates as Record<string, unknown>).mcpServerNames as
|
||||
| string[]
|
||||
| undefined;
|
||||
const mcpServerNames =
|
||||
supplied ??
|
||||
extractMCPServerNames((directUpdates as Record<string, unknown>).tools as string[]);
|
||||
(directUpdates as Record<string, unknown>).mcpServerNames = mcpServerNames;
|
||||
updateData.mcpServerNames = mcpServerNames;
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue