mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-08-03 22:32:42 +00:00
fix(mcp): resolve Codex review on the app-bridge follow-ups
Validates open-link schemes before opening. A sandboxed app could send ui/open-link with any string; onmessage now opens only http and https URLs and ignores other schemes and malformed URLs, so apps cannot launch javascript: or data: targets from the host page. Decodes blob-backed app resources. resources/read may return HTML as a base64 blob rather than text per the MCP Apps spec, so fetchMCPResourceHtml decodes the blob when text is absent instead of rendering a blank iframe. Disambiguates embedded ui:// resource ids by payload. The embedded resource id was hashed from only the template text or URI, so the same template returned by multiple calls with different structuredContent collided and the conversation resource map overwrote earlier entries. The id now mixes in the structured content and tool arguments, matching the synthetic-resource path. Allows a dedicated sandbox origin to be framed by the host. The MCP Apps spec requires the host and sandbox to have different origins for web hosts, but the sandbox route hardcoded same-origin framing. Framing stays same-origin by default and an operator can list allowed host origins via MCP_SANDBOX_FRAME_ANCESTORS for a cross-origin sandbox deployment.
This commit is contained in:
parent
b664cce8ca
commit
de28930ddf
5 changed files with 61 additions and 8 deletions
|
|
@ -75,9 +75,23 @@ const serveMCPSandbox = async (_req, res) => {
|
|||
res.setHeader('Cache-Control', 'no-store, no-cache, must-revalidate');
|
||||
res.setHeader('X-Content-Type-Options', 'nosniff');
|
||||
res.setHeader('Referrer-Policy', 'same-origin');
|
||||
res.setHeader('Cross-Origin-Resource-Policy', 'same-origin');
|
||||
res.setHeader('Content-Security-Policy', "frame-ancestors 'self'");
|
||||
res.setHeader('X-Frame-Options', 'SAMEORIGIN');
|
||||
|
||||
// The MCP Apps spec requires the Host and Sandbox to have different origins for web hosts.
|
||||
// Default to same-origin framing; when a dedicated sandbox origin is deployed, the operator
|
||||
// lists the allowed host origin(s) so the host page can frame this sandbox cross-origin.
|
||||
const allowedParents = (process.env.MCP_SANDBOX_FRAME_ANCESTORS || '').trim();
|
||||
if (allowedParents) {
|
||||
const ancestors = allowedParents
|
||||
.split(/[\s,]+/)
|
||||
.filter(Boolean)
|
||||
.join(' ');
|
||||
res.setHeader('Content-Security-Policy', `frame-ancestors 'self' ${ancestors}`);
|
||||
res.setHeader('Cross-Origin-Resource-Policy', 'cross-origin');
|
||||
} else {
|
||||
res.setHeader('Content-Security-Policy', "frame-ancestors 'self'");
|
||||
res.setHeader('Cross-Origin-Resource-Policy', 'same-origin');
|
||||
res.setHeader('X-Frame-Options', 'SAMEORIGIN');
|
||||
}
|
||||
|
||||
const sandboxPath = path.resolve(
|
||||
__dirname,
|
||||
|
|
|
|||
|
|
@ -85,7 +85,16 @@ export function useAppBridge(
|
|||
) as never;
|
||||
|
||||
bridge.onopenlink = async ({ url }) => {
|
||||
window.open(url, '_blank', 'noopener,noreferrer');
|
||||
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');
|
||||
}
|
||||
return {};
|
||||
};
|
||||
|
||||
|
|
|
|||
|
|
@ -77,12 +77,20 @@ export async function fetchMCPResourceHtml(
|
|||
permissions?: ResourceUiMeta['permissions'];
|
||||
}> {
|
||||
const result = (await readMCPResource(serverName, uri, userId)) as {
|
||||
contents?: Array<{ text?: string; _meta?: { ui?: ResourceUiMeta } }>;
|
||||
contents?: Array<{ text?: string; blob?: string; _meta?: { ui?: ResourceUiMeta } }>;
|
||||
};
|
||||
const item = result?.contents?.[0];
|
||||
const uiMeta = item?._meta?.ui;
|
||||
let html = item?.text ?? '';
|
||||
if (!html && typeof item?.blob === 'string' && item.blob) {
|
||||
try {
|
||||
html = atob(item.blob);
|
||||
} catch {
|
||||
html = '';
|
||||
}
|
||||
}
|
||||
return {
|
||||
html: item?.text ?? '',
|
||||
html,
|
||||
csp: uiMeta?.csp,
|
||||
permissions: uiMeta?.permissions,
|
||||
};
|
||||
|
|
|
|||
|
|
@ -307,6 +307,25 @@ describe('formatToolContent', () => {
|
|||
expect(uiResourceArtifact?.content).toEqual(result.content);
|
||||
});
|
||||
|
||||
it('gives embedded ui:// resources distinct ids per tool result payload', () => {
|
||||
const resourceIdFor = (sc: Record<string, unknown>) =>
|
||||
formatToolContent(
|
||||
{
|
||||
content: [
|
||||
{
|
||||
type: 'resource',
|
||||
resource: { uri: 'ui://app', mimeType: 'text/html', text: '<p>same</p>' },
|
||||
},
|
||||
],
|
||||
structuredContent: sc,
|
||||
} as t.MCPToolCallResponse,
|
||||
'openai',
|
||||
{ serverName: 'srv', toolName: 'do_thing' },
|
||||
)[1]?.ui_resources?.data?.[0]?.resourceId;
|
||||
|
||||
expect(resourceIdFor({ a: 1 })).not.toEqual(resourceIdFor({ a: 2 }));
|
||||
});
|
||||
|
||||
it('should handle regular resources', () => {
|
||||
const result: t.MCPToolCallResponse = {
|
||||
content: [
|
||||
|
|
|
|||
|
|
@ -194,11 +194,14 @@ export function formatToolContent(
|
|||
const resourceText: string[] = [];
|
||||
|
||||
if (isUiResource) {
|
||||
const contentToHash =
|
||||
const baseHash =
|
||||
'text' in item.resource && item.resource.text && typeof item.resource.text === 'string'
|
||||
? item.resource.text
|
||||
: item.resource.uri;
|
||||
const resourceId = generateResourceId(contentToHash);
|
||||
const scKey =
|
||||
result?.structuredContent != null ? JSON.stringify(result.structuredContent) : '';
|
||||
const argsKey = metadata?.toolArgs != null ? JSON.stringify(metadata.toolArgs) : '';
|
||||
const resourceId = generateResourceId(baseHash + '\x00' + scKey + '\x00' + argsKey);
|
||||
const uiResource: UIResource = {
|
||||
...item.resource,
|
||||
resourceId,
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue