mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-07-02 04:12:36 +00:00
fix(mcp): address Codex P1/P2 findings — visibility filter, header clobber, base path, inline text
App-only tools (visibility: ['app']) were not filtered in MCPServerInspector.getToolFunctions,
so initializeMCPs → getAppToolFunctions → mergeAppTools was silently exposing them to the LLM
tool cache at startup, bypassing the filter that updateMCPServerTools correctly applies.
Applied the same visibility guard that updateMCPServerTools uses.
appToolCall was calling processMCPEnv without customUserVars for DB-sourced servers, then
setRequestHeaders — overwriting the connection's already-correctly-resolved headers with
unresolved {{MCP_API_KEY}} placeholders. Skipped the re-resolve for DB-sourced servers
since the connection carries valid headers from the original callTool setup.
callMCPAppTool and readMCPResource used hardcoded /api/... paths without the apiBaseUrl()
prefix; subdirectory deployments would miss those routes. apiBaseUrl was already imported for
getMCPSandboxUrl — extended it to both API calls for consistency.
MCPAppCard (carousel) and MCPAppView (ToolCall) both checked toolName && serverName first
when deciding to use the app bridge, but parsers.ts now sets those fields on all UIResources
including inline ui:// resources with text content. Resources with text were therefore silently
routed through the app bridge instead of being rendered directly as srcDoc iframes.
Added !resource.text / !app.text guard so inline HTML resources take the correct path.
This commit is contained in:
parent
d143bb10cc
commit
cc45641d7e
5 changed files with 40 additions and 11 deletions
|
|
@ -70,6 +70,19 @@ const MCPAppView = React.memo(function MCPAppView({
|
|||
|
||||
useAppBridge(iframeRef, app, toolArgs, toolResult, handleSizeChanged);
|
||||
|
||||
if (app.text && (app.mimeType ?? 'text/html').includes('html')) {
|
||||
return (
|
||||
<div className="my-2">
|
||||
<iframe
|
||||
srcDoc={app.text}
|
||||
sandbox="allow-scripts allow-forms"
|
||||
style={{ width: '100%', minHeight: '200px', border: 'none' }}
|
||||
title={app.uri}
|
||||
/>
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
return (
|
||||
<div className="my-2" style={height ? { height } : { minHeight: 100 }}>
|
||||
{!loaded && !timedOut && (
|
||||
|
|
|
|||
|
|
@ -50,7 +50,7 @@ function MCPAppCard({
|
|||
handleSizeChanged,
|
||||
);
|
||||
|
||||
if (resource.toolName && resource.serverName) {
|
||||
if (resource.toolName && resource.serverName && !resource.text) {
|
||||
return (
|
||||
<>
|
||||
{!loaded && (
|
||||
|
|
|
|||
|
|
@ -11,7 +11,7 @@ export async function callMCPAppTool(
|
|||
toolName: string,
|
||||
args: Record<string, unknown>,
|
||||
) {
|
||||
return request.post('/api/mcp/app-tool-call', {
|
||||
return request.post(`${apiBaseUrl()}/api/mcp/app-tool-call`, {
|
||||
serverName,
|
||||
toolName,
|
||||
arguments: args,
|
||||
|
|
@ -40,7 +40,7 @@ export async function readMCPResource(serverName: string, uri: string, userId?:
|
|||
}
|
||||
}
|
||||
|
||||
const promise = request.post('/api/mcp/resources/read', { serverName, uri });
|
||||
const promise = request.post(`${apiBaseUrl()}/api/mcp/resources/read`, { serverName, uri });
|
||||
resourceCache.set(key, { promise, ts: now });
|
||||
promise.catch(() => resourceCache.delete(key));
|
||||
return promise;
|
||||
|
|
|
|||
|
|
@ -747,14 +747,19 @@ Please follow these instructions when using tools from the respective MCP server
|
|||
`${logPrefix} Server "${serverName}" requires Graph API token resolution which is not supported for app tool calls.`,
|
||||
);
|
||||
}
|
||||
const currentOptions = processMCPEnv({
|
||||
user,
|
||||
dbSourced: isDbSourced,
|
||||
options: rawConfig as t.MCPOptions,
|
||||
});
|
||||
const resolvedHeaders: Record<string, string> =
|
||||
'headers' in currentOptions ? { ...(currentOptions.headers || {}) } : {};
|
||||
connection.setRequestHeaders(resolvedHeaders);
|
||||
// DB-sourced servers have their customUserVars (e.g. {{MCP_API_KEY}}) resolved during
|
||||
// the original callTool setup. Re-processing without customUserVars would overwrite
|
||||
// those resolved headers with unresolved placeholders, so skip for DB-sourced servers.
|
||||
if (!isDbSourced) {
|
||||
const currentOptions = processMCPEnv({
|
||||
user,
|
||||
dbSourced: false,
|
||||
options: rawConfig as t.MCPOptions,
|
||||
});
|
||||
const resolvedHeaders: Record<string, string> =
|
||||
'headers' in currentOptions ? { ...(currentOptions.headers || {}) } : {};
|
||||
connection.setRequestHeaders(resolvedHeaders);
|
||||
}
|
||||
}
|
||||
|
||||
const result = await connection.client.request(
|
||||
|
|
|
|||
|
|
@ -175,6 +175,17 @@ export class MCPServerInspector {
|
|||
|
||||
const toolFunctions: t.LCAvailableTools = {};
|
||||
tools.forEach((tool) => {
|
||||
const uiMeta = (tool._meta as Record<string, unknown>)?.ui as
|
||||
| Record<string, unknown>
|
||||
| undefined;
|
||||
const visibility = uiMeta?.visibility as string[] | undefined;
|
||||
if (
|
||||
Array.isArray(visibility) &&
|
||||
visibility.includes('app') &&
|
||||
!visibility.includes('model')
|
||||
) {
|
||||
return;
|
||||
}
|
||||
const name = `${tool.name}${Constants.mcp_delimiter}${serverName}`;
|
||||
toolFunctions[name] = {
|
||||
type: 'function',
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue