From de28930ddf2c9800dd79d05702c881fb51a50366 Mon Sep 17 00:00:00 2001 From: Dustin Healy <54083382+dustinhealy@users.noreply.github.com> Date: Tue, 23 Jun 2026 23:29:16 -0700 Subject: [PATCH] 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. --- api/server/controllers/mcpApps.js | 20 ++++++++++++++++--- client/src/hooks/MCP/useAppBridge.ts | 11 +++++++++- client/src/utils/mcpApps.ts | 12 +++++++++-- .../api/src/mcp/__tests__/parsers.test.ts | 19 ++++++++++++++++++ packages/api/src/mcp/parsers.ts | 7 +++++-- 5 files changed, 61 insertions(+), 8 deletions(-) diff --git a/api/server/controllers/mcpApps.js b/api/server/controllers/mcpApps.js index 9d7fb2cbd5..ca9738fe78 100644 --- a/api/server/controllers/mcpApps.js +++ b/api/server/controllers/mcpApps.js @@ -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, diff --git a/client/src/hooks/MCP/useAppBridge.ts b/client/src/hooks/MCP/useAppBridge.ts index ba98be4c80..43b8ca695c 100644 --- a/client/src/hooks/MCP/useAppBridge.ts +++ b/client/src/hooks/MCP/useAppBridge.ts @@ -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 {}; }; diff --git a/client/src/utils/mcpApps.ts b/client/src/utils/mcpApps.ts index 78baeafefa..1efe5f166c 100644 --- a/client/src/utils/mcpApps.ts +++ b/client/src/utils/mcpApps.ts @@ -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, }; diff --git a/packages/api/src/mcp/__tests__/parsers.test.ts b/packages/api/src/mcp/__tests__/parsers.test.ts index 6bb6b19c17..7ca4fccce1 100644 --- a/packages/api/src/mcp/__tests__/parsers.test.ts +++ b/packages/api/src/mcp/__tests__/parsers.test.ts @@ -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) => + formatToolContent( + { + content: [ + { + type: 'resource', + resource: { uri: 'ui://app', mimeType: 'text/html', text: '

same

' }, + }, + ], + 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: [ diff --git a/packages/api/src/mcp/parsers.ts b/packages/api/src/mcp/parsers.ts index 0df5c19c1b..942cb096ac 100644 --- a/packages/api/src/mcp/parsers.ts +++ b/packages/api/src/mcp/parsers.ts @@ -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,