fix(mcp): keep app frames opaque, fail closed on auth lookup, honor blob HTML and role

Always strip allow-same-origin from the blob-loaded app frame. The resource CSP is injected as a
meta policy inside the app document, so it does not bind the sandbox proxy, whose own response sets
only frame-ancestors; a same-origin app could reach the proxy and fetch or submit from there,
escaping the declared connectDomains and form-action. The previous dedicated-origin grant was inert
under sandbox flag inheritance (the outer frame withholds allow-same-origin, forcing an opaque
origin, so storage stayed unavailable), but it would arm if the outer frame ever granted it.

Add an opt-in throwOnError to getUserMCPAuthMap, plumbed to getPluginAuthMap's throwError and
rethrown from its catch, and use it from resolveAppContext. The controller previously caught a
rejection that could never happen, so an auth lookup or decryption failure degraded to an empty map
and app requests continued on a connection with previously resolved headers. Other callers keep the
swallowing default.

Treat a base64 blob resource as persisted inline HTML in the read-only guards and static renders of
ToolCall, MCPUIResource, and UIResourceCarousel via a shared getInlineResourceHtml helper, so
blob-embedded apps are no longer dropped from shared transcripts.

Resolve app follow-up configs through the role-aware getAllServerConfigs so a server or agent shared
to the user's role stays readable, and constrain host-opened app links to the resource's declared
egress domains.
This commit is contained in:
Dustin Healy 2026-08-09 15:16:49 -07:00
parent b9e33e0d08
commit ec52b86a53
14 changed files with 196 additions and 59 deletions

View file

@ -30,11 +30,16 @@ const MCP_INVALID_REQUEST = -32600;
const resolveAppContext = async (req, serverName) => {
const userId = req.user?.id;
// Fail closed on both config and auth resolution: a transient lookup failure must reject rather
// than fall back to the base config (wrong server) or to unresolved/stale credentials. A user
// who genuinely has no vars resolves to undefined without throwing, so that path still proceeds.
// than fall back to the base config (wrong server) or to unresolved/stale credentials. A user who
// genuinely has no vars still resolves to an empty map without throwing, so that path proceeds.
const [configServers, userMCPAuthMap] = await Promise.all([
resolveConfigServers(req, { throwOnError: true }),
getUserMCPAuthMap({ userId, servers: [serverName], findPluginAuthsByKeys }).catch((err) => {
getUserMCPAuthMap({
userId,
servers: [serverName],
findPluginAuthsByKeys,
throwOnError: true,
}).catch((err) => {
logger.error(
`[resolveAppContext] Failed to resolve MCP auth values for user ${userId}, server ${serverName}; failing closed`,
err,

View file

@ -48,9 +48,6 @@
return window.location.origin;
})();
// A dedicated sandbox origin (parentOrigin differs from ours) isolates the inner frame from
// the host origin, so allow-same-origin can be granted; same-origin deployments cannot.
const dedicatedOrigin = window.location.origin !== trustedOrigin;
// Opt-in stricter CSP that drops unsafe-eval/wasm/blob/data from script-src for deployments
// that do not need them.
const strictCsp = new URLSearchParams(window.location.search).get('strictCsp') === '1';
@ -138,18 +135,17 @@
innerFrame = null;
});
// On a dedicated sandbox origin, grant allow-same-origin so storage-backed apps work
// (the spec's dedicated-origin model); the distinct origin keeps it away from the host.
// When the sandbox runs same-origin as the host, allow-same-origin would expose the host
// origin, so it is stripped regardless of what the host requested.
// The app frame stays opaque to this proxy: the resource CSP is injected as a meta policy
// INSIDE the app document, so it does not bind this proxy document, whose own response only
// sets frame-ancestors. A same-origin app could therefore reach the proxy and fetch or
// submit from there, escaping the declared connectDomains/form-action. Strip
// allow-same-origin regardless of what the host requested, on any origin. Granting it (and
// outer-frame parity with the spec's allow-scripts+allow-same-origin Sandbox requirement)
// is only safe once the per-resource CSP moves to the sandbox HTTP response boundary.
const sandboxTokens = new Set(
(params.sandbox || 'allow-scripts allow-forms').split(/\s+/).filter(Boolean),
);
if (dedicatedOrigin) {
sandboxTokens.add('allow-same-origin');
} else {
sandboxTokens.delete('allow-same-origin');
}
sandboxTokens.delete('allow-same-origin');
innerFrame.sandbox = Array.from(sandboxTokens).join(' ') || 'allow-scripts';
const allowParts = [];

View file

@ -11,7 +11,12 @@ import {
splitToolCallName,
} from 'librechat-data-provider';
import type { TAttachment, UIResource } from 'librechat-data-provider';
import { getMCPSandboxUrl, buildAppToolResult, isMcpAppResource } from '~/utils/mcpApps';
import {
getMCPSandboxUrl,
buildAppToolResult,
isMcpAppResource,
getInlineResourceHtml,
} from '~/utils/mcpApps';
import { useMCPIconMap, useAppBridge, useMCPServerNames } from '~/hooks/MCP';
import { useLocalize, useProgress, useExpandCollapse } from '~/hooks';
import { ToolIcon, getToolIconType, isError } from './ToolOutput';
@ -40,6 +45,7 @@ const MCPAppView = React.memo(function MCPAppView({
const [timedOut, setTimedOut] = useState(false);
const [tornDown, setTornDown] = useState(false);
const sandboxUrl = useMemo(() => getMCPSandboxUrl(), []);
const inlineHtml = useMemo(() => getInlineResourceHtml(app), [app]);
useEffect(() => {
if (loaded) return;
@ -80,18 +86,18 @@ const MCPAppView = React.memo(function MCPAppView({
const isAppBacked = isMcpAppResource(app);
// Read-only views don't fetch app HTML, so a resourceUri-only app shows a placeholder.
if (isAppBacked && !app.text && readOnly) {
if (isAppBacked && !inlineHtml && readOnly) {
return (
<div className="my-2 flex items-center gap-2 rounded-lg border border-border-light bg-surface-secondary px-4 py-3 text-sm text-text-secondary">
{localize('com_ui_mcp_app_shared_unavailable')}
</div>
);
}
if (!isAppBacked && app.text) {
if (!isAppBacked && inlineHtml) {
return (
<div className="my-2">
<iframe
srcDoc={app.text}
srcDoc={inlineHtml}
sandbox=""
style={{ width: '100%', minHeight: '200px', border: 'none' }}
title={app.uri}
@ -290,7 +296,9 @@ export default function ToolCall({
?.filter((a) => a.type === Tools.ui_resources)
.flatMap((a) => (a[Tools.ui_resources] ?? []) as UIResource[]) ?? [];
return uiResources.filter(
(r) => isMcpAppResource(r) || (r.text && (r.mimeType ?? 'text/html').includes('html')),
(r) =>
isMcpAppResource(r) ||
(getInlineResourceHtml(r) != null && (r.mimeType ?? 'text/html').includes('html')),
);
}, [attachments]);

View file

@ -1,6 +1,11 @@
import React, { useState } from 'react';
import type { UIResource } from 'librechat-data-provider';
import { getMCPSandboxUrl, buildAppToolResult, isMcpAppResource } from '~/utils/mcpApps';
import {
getMCPSandboxUrl,
buildAppToolResult,
isMcpAppResource,
getInlineResourceHtml,
} from '~/utils/mcpApps';
import { useIsMessagesViewReadOnly } from '~/Providers';
import { useAppBridge } from '~/hooks/MCP';
import { useLocalize } from '~/hooks';
@ -25,6 +30,7 @@ function MCPAppCard({
const [timedOut, setTimedOut] = useState(false);
const [tornDown, setTornDown] = useState(false);
const sandboxUrl = React.useMemo(() => getMCPSandboxUrl(), []);
const inlineHtml = React.useMemo(() => getInlineResourceHtml(resource), [resource]);
React.useEffect(() => {
if (loaded) {
@ -60,7 +66,7 @@ function MCPAppCard({
return null;
}
if (isMcpAppResource(resource) && !resource.text && readOnly) {
if (isMcpAppResource(resource) && !inlineHtml && readOnly) {
return (
<div className="flex h-full w-full items-center justify-center rounded-lg border border-border-light bg-surface-secondary px-4 py-3 text-center text-sm text-text-secondary">
{localize('com_ui_mcp_app_shared_unavailable')}
@ -97,10 +103,10 @@ function MCPAppCard({
);
}
if (resource.text) {
if (inlineHtml) {
return (
<iframe
srcDoc={resource.text}
srcDoc={inlineHtml}
sandbox=""
style={{ width: '100%', height: '100%', border: 'none' }}
title={resource.uri}

View file

@ -25,6 +25,11 @@ jest.mock('~/hooks');
jest.mock('~/hooks/Messages/useConversationUIResources');
jest.mock('~/utils/mcpApps', () => ({
getInlineResourceHtml: (r: any) =>
r?.text ||
(typeof r?.blob === 'string' && r.blob
? Buffer.from(r.blob, 'base64').toString('utf-8')
: undefined),
isMcpAppResource: (r) =>
!!(r && r.toolName && r.serverName) && (r.mimeType ?? '').includes('profile=mcp-app'),
buildAppToolResult: jest.fn(),

View file

@ -13,6 +13,11 @@ jest.mock('~/Providers', () => ({
}));
jest.mock('~/utils/mcpApps', () => ({
getInlineResourceHtml: (r: any) =>
r?.text ||
(typeof r?.blob === 'string' && r.blob
? Buffer.from(r.blob, 'base64').toString('utf-8')
: undefined),
isMcpAppResource: (r) =>
!!(r && r.toolName && r.serverName) && (r.mimeType ?? '').includes('profile=mcp-app'),
buildAppToolResult: jest.fn(),

View file

@ -1,6 +1,11 @@
import React, { useRef, useState, useMemo, useEffect, useCallback } from 'react';
import {
getMCPSandboxUrl,
buildAppToolResult,
isMcpAppResource,
getInlineResourceHtml,
} from '~/utils/mcpApps';
import { useConversationUIResources } from '~/hooks/Messages/useConversationUIResources';
import { getMCPSandboxUrl, buildAppToolResult, isMcpAppResource } from '~/utils/mcpApps';
import { useOptionalMessagesConversation, useIsMessagesViewReadOnly } from '~/Providers';
import { useAppBridge } from '~/hooks/MCP';
import { useLocalize } from '~/hooks';
@ -31,6 +36,10 @@ export function MCPUIResource(props: MCPUIResourceProps) {
const [tornDown, setTornDown] = useState(false);
const [height, setHeight] = useState<number | undefined>(undefined);
const sandboxUrl = useMemo(() => getMCPSandboxUrl(), []);
const inlineHtml = useMemo(
() => (uiResource ? getInlineResourceHtml(uiResource) : undefined),
[uiResource],
);
useEffect(() => {
if (loaded) {
@ -77,7 +86,7 @@ export function MCPUIResource(props: MCPUIResourceProps) {
}
try {
if (isMcpAppResource(uiResource) && !uiResource.text && readOnly) {
if (isMcpAppResource(uiResource) && !inlineHtml && readOnly) {
return (
<span className="mx-1 inline-flex w-full items-center gap-2 rounded-lg border border-border-light bg-surface-secondary px-4 py-3 align-middle text-sm text-text-secondary">
{localize('com_ui_mcp_app_shared_unavailable')}
@ -116,11 +125,11 @@ export function MCPUIResource(props: MCPUIResourceProps) {
);
}
if (uiResource.text) {
if (inlineHtml) {
return (
<span className="mx-1 inline-block w-full align-middle">
<iframe
srcDoc={uiResource.text}
srcDoc={inlineHtml}
sandbox=""
style={{ width: '100%', minHeight: '200px', border: 'none' }}
title={uiResource.uri}

View file

@ -20,6 +20,11 @@ jest.mock('~/hooks/MCP', () => ({
}));
jest.mock('~/utils/mcpApps', () => ({
getInlineResourceHtml: (r: any) =>
r?.text ||
(typeof r?.blob === 'string' && r.blob
? Buffer.from(r.blob, 'base64').toString('utf-8')
: undefined),
isMcpAppResource: (r) =>
!!(r && r.toolName && r.serverName) && (r.mimeType ?? '').includes('profile=mcp-app'),
buildAppToolResult: jest.fn(),

View file

@ -16,7 +16,8 @@ import {
readMCPResource,
listMCPResources,
listMCPResourceTemplates,
decodeBase64Utf8,
getInlineResourceHtml,
isAllowedAppLink,
} from '~/utils/mcpApps';
import { useOptionalMessagesOperations, useIsMessagesViewReadOnly } from '~/Providers';
import { logger } from '~/utils';
@ -141,15 +142,10 @@ export function useAppBridge(
);
bridge.onopenlink = async ({ url }) => {
try {
const { protocol } = new URL(url);
if (protocol === 'http:' || protocol === 'https:') {
window.open(url, '_blank', 'noopener,noreferrer');
} else {
logger.warn('[MCP App] Blocked open-link with unsupported scheme', protocol);
}
} catch {
logger.warn('[MCP App] Blocked malformed open-link url');
if (isAllowedAppLink(url, resource.csp)) {
window.open(url, '_blank', 'noopener,noreferrer');
} else {
logger.warn('[MCP App] Blocked open-link outside the declared egress domains');
}
return {};
};
@ -190,16 +186,7 @@ export function useAppBridge(
return;
}
sandboxReadyHandled = true;
// Inline HTML may arrive as `text` or as a base64 `blob`; decode the blob so blob-embedded
// apps are treated as persisted (rendered in read-only) rather than resourceUri-only.
let inlineHtml = resource.text;
if (!inlineHtml && typeof resource.blob === 'string' && resource.blob) {
try {
inlineHtml = decodeBase64Utf8(resource.blob);
} catch {
inlineHtml = undefined;
}
}
const inlineHtml = getInlineResourceHtml(resource);
// Read-only views must not resolve app HTML from the viewer's MCP server, so only inline
// (persisted) HTML renders here.
if (!inlineHtml && readOnlyRef.current) {

View file

@ -111,6 +111,65 @@ export function decodeBase64Utf8(b64: string): string {
return new TextDecoder('utf-8').decode(bytes);
}
const APP_LINK_HOST_PATTERN =
/^(?:(?:https?|wss?):\/\/)?(\*\.)?([a-zA-Z0-9][a-zA-Z0-9.-]*)(?::\d{1,5})?$/;
function hostMatchesDeclaredDomain(hostname: string, entry: string): boolean {
const match = APP_LINK_HOST_PATTERN.exec(entry.trim());
if (!match) {
return false;
}
const [, wildcard, declaredHost] = match;
const host = hostname.toLowerCase();
const target = declaredHost.toLowerCase();
return wildcard ? host === target || host.endsWith(`.${target}`) : host === target;
}
/**
* A host-opened link is not bound by the sandbox CSP, so an app holding proxied MCP data could
* encode it into a URL and exfiltrate it through the host page. Only hosts the resource declared
* for egress are opened; a resource declaring none gets no host-opened links, matching the
* `connect-src 'none'` default applied inside the sandbox.
*/
export function isAllowedAppLink(url: string, csp: UIResource['csp']): boolean {
let parsed: URL;
try {
parsed = new URL(url);
} catch {
return false;
}
if (parsed.protocol !== 'http:' && parsed.protocol !== 'https:') {
return false;
}
const declared = [
...(csp?.connectDomains ?? []),
...(csp?.resourceDomains ?? []),
...(csp?.frameDomains ?? []),
];
return declared.some(
(entry) => typeof entry === 'string' && hostMatchesDeclaredDomain(parsed.hostname, entry),
);
}
/**
* Inline HTML persisted on a UI resource, carried either as `text` or as a base64 `blob`. Read-only
* views render only inline HTML, so both encodings must count as persisted or blob-embedded apps
* would be dropped from shared transcripts.
*/
export function getInlineResourceHtml(resource: UIResource): string | undefined {
if (typeof resource.text === 'string' && resource.text) {
return resource.text;
}
if (typeof resource.blob === 'string' && resource.blob) {
try {
return decodeBase64Utf8(resource.blob);
} catch {
return undefined;
}
}
return undefined;
}
export async function fetchMCPResourceHtml(
serverName: string,
uri: string,

View file

@ -807,11 +807,16 @@ Please follow these instructions when using tools from the respective MCP server
tokenMethods?: TokenMethods;
}): Promise<MCPConnection> {
const logPrefix = `[MCP][User: ${userId}][${serverName}]`;
const rawConfig = await MCPServersRegistry.getInstance().getServerConfig(
serverName,
// Resolved through the role-aware path (as discovery does) rather than the single-server lookup,
// whose ACL check is user-only: a server or agent shared to the user's role would otherwise look
// inaccessible here and the app's follow-up reads and tool calls would be rejected. Precedence
// matches getServerConfig by contract.
const allConfigs = await MCPServersRegistry.getInstance().getAllServerConfigs(
userId,
configServers,
user?.role,
);
const rawConfig = allConfigs[serverName];
const isDbSourced = rawConfig ? isUserSourced(rawConfig) : false;
if (rawConfig) {
if (rawConfig.obo) {

View file

@ -1268,10 +1268,14 @@ describe('MCPManager', () => {
const mockUser: Partial<IUser> = { id: 'user-123' };
it('rejects when the server config needs request body placeholders unavailable to app calls', async () => {
(mockRegistryInstance.getServerConfig as jest.Mock).mockResolvedValue({
const config = {
source: 'yaml',
type: 'sse',
url: 'https://example.com/{{LIBRECHAT_BODY_CONVERSATIONID}}/mcp',
};
(mockRegistryInstance.getServerConfig as jest.Mock).mockResolvedValue(config);
(mockRegistryInstance.getAllServerConfigs as jest.Mock).mockResolvedValue({
'body-server': config,
});
const manager = await MCPManager.createInstance(newMCPServersConfig());
@ -1288,12 +1292,16 @@ describe('MCPManager', () => {
});
it('preserves resolved headers for customUserVars servers when the route supplies no vars', async () => {
(mockRegistryInstance.getServerConfig as jest.Mock).mockResolvedValue({
const config = {
source: 'yaml',
type: 'sse',
url: 'https://example.com/mcp',
headers: { Authorization: 'Bearer {{API_KEY}}' },
customUserVars: { API_KEY: { title: 'API Key' } },
};
(mockRegistryInstance.getServerConfig as jest.Mock).mockResolvedValue(config);
(mockRegistryInstance.getAllServerConfigs as jest.Mock).mockResolvedValue({
'cuv-server': config,
});
const mockConnection = {
@ -1320,12 +1328,16 @@ describe('MCPManager', () => {
});
it('resolves headers with customUserVars when the app route supplies them', async () => {
(mockRegistryInstance.getServerConfig as jest.Mock).mockResolvedValue({
const config = {
source: 'yaml',
type: 'sse',
url: 'https://example.com/mcp',
headers: { Authorization: 'Bearer {{API_KEY}}' },
customUserVars: { API_KEY: { title: 'API Key' } },
};
(mockRegistryInstance.getServerConfig as jest.Mock).mockResolvedValue(config);
(mockRegistryInstance.getAllServerConfigs as jest.Mock).mockResolvedValue({
'cuv-server': config,
});
mockProcessMCPEnv.mockImplementation((params) => ({
...params.options,
@ -1359,11 +1371,15 @@ describe('MCPManager', () => {
});
});
it('forwards configServers, flowManager, and tokenMethods to getConnection', async () => {
(mockRegistryInstance.getServerConfig as jest.Mock).mockResolvedValue({
it('resolves the config with the user role and forwards flowManager/tokenMethods to getConnection', async () => {
const config = {
source: 'yaml',
type: 'sse',
url: 'https://example.com/mcp',
};
(mockRegistryInstance.getServerConfig as jest.Mock).mockResolvedValue(config);
(mockRegistryInstance.getAllServerConfigs as jest.Mock).mockResolvedValue({
'cfg-server': config,
});
const mockConnection = {
@ -1388,16 +1404,17 @@ describe('MCPManager', () => {
serverName: 'cfg-server',
toolName: 'do_thing',
toolArguments: {},
user: mockUser as IUser,
user: { ...mockUser, role: 'ADMIN' } as IUser,
configServers,
flowManager,
tokenMethods,
});
expect(mockRegistryInstance.getServerConfig).toHaveBeenCalledWith(
'cfg-server',
// Role-aware lookup: a server shared to the user's role must resolve for app follow-ups.
expect(mockRegistryInstance.getAllServerConfigs).toHaveBeenCalledWith(
'user-123',
configServers,
'ADMIN',
);
expect(getConnectionSpy).toHaveBeenCalledWith(
expect.objectContaining({ flowManager, tokenMethods }),

View file

@ -173,6 +173,26 @@ describe('getUserMCPAuthMap', () => {
expect(result).toEqual({});
});
it('propagates lookup failures when throwOnError is set, and asks the map to throw too', async () => {
const toolInstances = [createMockTool('test_mcp_Server1', 'Server1')];
const dbError = new Error('Database connection failed');
mockGetPluginAuthMap.mockRejectedValue(dbError);
await expect(
getUserMCPAuthMap({
userId: 'user123',
toolInstances,
findPluginAuthsByKeys: mockFindPluginAuthsByKeys,
throwOnError: true,
}),
).rejects.toThrow('Database connection failed');
expect(mockGetPluginAuthMap).toHaveBeenCalledWith(
expect.objectContaining({ throwError: true }),
);
});
it('should handle non-Error exceptions gracefully', async () => {
const toolInstances = [createMockTool('test_mcp_Server1', 'Server1')];

View file

@ -20,6 +20,7 @@ export async function getUserMCPAuthMap({
toolInstances,
serverNames,
findPluginAuthsByKeys,
throwOnError = false,
}: {
userId: string;
tools?: (string | undefined)[];
@ -33,6 +34,12 @@ export async function getUserMCPAuthMap({
*/
serverNames?: readonly string[];
findPluginAuthsByKeys: PluginAuthMethods['findPluginAuthsByKeys'];
/**
* Propagate lookup/decryption failures instead of degrading to an empty map. Callers that must
* distinguish "this user has no vars" from "we could not read them" (and fail closed rather than
* proceed with unresolved credentials) opt in; every other caller keeps the swallowing default.
*/
throwOnError?: boolean;
}): Promise<Record<string, Record<string, string>>> {
let allMcpCustomUserVars: Record<string, Record<string, string>> = {};
let mcpPluginKeysToFetch: string[] = [];
@ -86,7 +93,7 @@ export async function getUserMCPAuthMap({
allMcpCustomUserVars = await getPluginAuthMap({
userId,
pluginKeys: mcpPluginKeysToFetch,
throwError: false,
throwError: throwOnError,
findPluginAuthsByKeys,
});
} catch (err) {
@ -96,6 +103,9 @@ export async function getUserMCPAuthMap({
)}), user ${userId}: ${err instanceof Error ? err.message : 'Unknown error'}`,
err,
);
if (throwOnError) {
throw err;
}
}
return allMcpCustomUserVars;