Skip to content
This repository was archived by the owner on Aug 6, 2026. It is now read-only.

Commit 983fcb0

Browse files
charlesviencharlesvien
committed
enforce CSP on sandboxed MCP app HTML
1 parent cdf2fb6 commit 983fcb0

7 files changed

Lines changed: 89 additions & 44 deletions

File tree

‎apps/mobile/src/features/mcp/sandbox/sandboxProxyHtml.ts‎

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ export const sandboxProxyHtml = `<!DOCTYPE html>
4747
}
4848
4949
var inner = document.createElement("iframe");
50-
inner.setAttribute("sandbox", "allow-scripts allow-same-origin allow-forms");
50+
inner.setAttribute("sandbox", "allow-scripts allow-forms");
5151
document.body.appendChild(inner);
5252
5353
function buildAllowAttribute(permissions) {
@@ -77,19 +77,17 @@ export const sandboxProxyHtml = `<!DOCTYPE html>
7777
var allowValue = buildAllowAttribute(params.permissions);
7878
if (allowValue) inner.setAttribute("allow", allowValue);
7979
80-
var doc = inner.contentDocument;
81-
if (doc) {
82-
doc.open();
83-
doc.write(params.html);
84-
doc.close();
85-
}
80+
// srcdoc, not document.write: the inner frame is sandboxed without
81+
// allow-same-origin, so contentDocument is cross-origin (VERIA-354).
82+
inner.setAttribute("srcdoc", params.html);
8683
}
8784
return;
8885
}
8986
9087
// All other host messages get relayed into the inner iframe untouched.
9188
if (inner.contentWindow) {
92-
inner.contentWindow.postMessage(data, location.origin || "*");
89+
// An opaque origin serializes to "null", which postMessage rejects.
90+
inner.contentWindow.postMessage(data, "*");
9391
}
9492
};
9593

‎packages/shared/src/mcp-sandbox-proxy.test.ts‎

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,20 +20,23 @@ describe("sandboxProxyHtml", () => {
2020
);
2121
});
2222

23-
it("creates inner iframe with allow-scripts, allow-same-origin, and allow-forms sandbox", () => {
23+
it("creates inner iframe without allow-same-origin", () => {
2424
expect(sandboxProxyHtml).toContain(
25-
"allow-scripts allow-same-origin allow-forms",
25+
'inner.setAttribute("sandbox", "allow-scripts allow-forms")',
2626
);
2727
});
2828

29-
it("uses document.write to inject HTML instead of srcdoc", () => {
30-
expect(sandboxProxyHtml).toContain("doc.open()");
31-
expect(sandboxProxyHtml).toContain("doc.write(params.html)");
32-
expect(sandboxProxyHtml).toContain("doc.close()");
29+
it("uses srcdoc to inject HTML, never document.write", () => {
30+
expect(sandboxProxyHtml).toContain('inner.setAttribute("srcdoc"');
31+
expect(sandboxProxyHtml).not.toContain("doc.write(");
3332
});
3433

35-
it("uses location.origin for forwarding messages to inner iframe", () => {
36-
expect(sandboxProxyHtml).toContain("postMessage(data, location.origin)");
34+
it("forwards to the inner iframe with a wildcard target origin", () => {
35+
// An opaque origin serializes to "null", which postMessage rejects.
36+
expect(sandboxProxyHtml).toContain('postMessage(data, "*")');
37+
expect(sandboxProxyHtml).not.toContain(
38+
"postMessage(data, location.origin)",
39+
);
3740
});
3841

3942
it("builds permission policy allow attribute with cross-origin delegation", () => {

‎packages/shared/src/mcp-sandbox-proxy.ts‎

Lines changed: 19 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -5,14 +5,14 @@
55
*
66
* Host (renderer) → Outer iframe (sandbox proxy) → Inner iframe (MCP App)
77
*
8-
* The outer iframe is served from the host's custom protocol, giving it an
9-
* isolated origin separate from the renderer. The inner iframe uses
10-
* allow-same-origin so the proxy can write HTML via document.write() — srcdoc
11-
* creates an opaque origin that breaks WebGL canvas operations (toDataURL) and
12-
* cross-origin resource access.
8+
* The host sandboxes the outer iframe without allow-same-origin, so both frames
9+
* get opaque origins and the app cannot traverse `window.parent.parent` to
10+
* reach the host's DOM, storage or cookies, nor another app's frame. That
11+
* isolation no longer depends on which URL the proxy happens to be served from
12+
* (a blob: URL inherits the creator's origin, so the web host had none).
1313
*
14-
* Because the proxy's origin differs from the renderer's origin, the app cannot
15-
* traverse `window.parent.parent` to access the host's DOM, storage, or cookies.
14+
* The app HTML is handed to the inner frame via srcdoc: under an opaque origin
15+
* the proxy cannot reach `inner.contentDocument` to document.write() into it.
1616
*
1717
* The HTML string itself is portable browser JavaScript with no host APIs; the
1818
* protocol that serves it is the host-specific seam.
@@ -56,7 +56,7 @@ export const sandboxProxyHtml: string = `<!DOCTYPE html>
5656
5757
var inner = document.createElement("iframe");
5858
inner.style.cssText = "width:100%; height:100%; border:none;";
59-
inner.setAttribute("sandbox", "allow-scripts allow-same-origin allow-forms");
59+
inner.setAttribute("sandbox", "allow-scripts allow-forms");
6060
document.body.appendChild(inner);
6161
6262
// Build Permission Policy allow attribute from permissions object.
@@ -98,32 +98,29 @@ export const sandboxProxyHtml: string = `<!DOCTYPE html>
9898
inner.setAttribute("allow", allowValue);
9999
}
100100
101-
// Use document.write() instead of srcdoc to preserve origin.
102-
// srcdoc creates an opaque origin that breaks WebGL canvas operations
103-
// like toDataURL() and cross-origin resource access.
104-
var doc = inner.contentDocument;
105-
log("Writing HTML to inner iframe", {
106-
htmlLength: params.html.length,
107-
hasContentDocument: !!doc
101+
// srcdoc, not document.write: this frame is sandboxed without
102+
// allow-same-origin, so inner.contentDocument is cross-origin.
103+
log("Setting inner iframe srcdoc", {
104+
htmlLength: params.html.length
108105
});
109106
110-
doc.open();
111-
doc.write(params.html);
112-
doc.close();
107+
inner.setAttribute("srcdoc", params.html);
113108
114-
log("HTML written to inner iframe");
109+
log("HTML handed to inner iframe");
115110
}
116111
} else {
117112
// Forward all other messages to inner iframe
118113
log("Forwarding host -> inner", {
119114
method: data.method,
120115
id: data.id,
121-
hasInner: !!(inner && inner.contentWindow),
122-
targetOrigin: location.origin
116+
hasInner: !!(inner && inner.contentWindow)
123117
});
124118
119+
// targetOrigin "*" because an opaque origin serializes to "null",
120+
// which postMessage rejects. The message still only reaches the
121+
// inner frame, since it is posted to that window reference.
125122
if (inner && inner.contentWindow) {
126-
inner.contentWindow.postMessage(data, location.origin);
123+
inner.contentWindow.postMessage(data, "*");
127124
}
128125
}
129126
}

‎packages/ui/src/features/mcp-apps/components/McpAppHost.tsx‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -239,10 +239,14 @@ export function McpAppHost({
239239
<iframe
240240
ref={setIframeEl}
241241
src={sandboxProxyUrl}
242-
// No allow-popups: app JS is same-origin with the proxy realm, so popup
243-
// permission here would let it window.open() past the sandbox. Apps
244-
// open links via ui/open-link, which the host scheme-validates.
245-
sandbox="allow-scripts allow-same-origin allow-forms allow-presentation"
242+
// No allow-same-origin: it would let the proxy realm resolve to the real
243+
// origin behind sandboxProxyUrl, which on web is a blob: URL inheriting
244+
// this app's own origin (host DOM, storage, tokens) and on desktop is a
245+
// scheme every MCP app shares, so one app could reach another's frame.
246+
// Omitting it forces a per-frame opaque origin (VERIA-354).
247+
// No allow-popups either: it would let app JS window.open() past the
248+
// sandbox. Apps open links via ui/open-link, which the host validates.
249+
sandbox="allow-scripts allow-forms allow-presentation"
246250
style={{
247251
height: displayMode === "fullscreen" ? "100%" : `${iframeHeight}px`,
248252
}}

‎packages/ui/src/features/mcp-apps/hooks/useAppBridge.ts‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import { useCallback, useEffect, useRef } from "react";
1717
import { logger } from "../../../shell/logger";
1818
import { useDraftStore } from "../../message-editor/draftStore";
1919
import type { ToolCall } from "../../sessions/types";
20+
import { applyCspToHtml } from "../utils/mcp-app-csp";
2021
import {
2122
computeContainerDimensions,
2223
INLINE_MAX_HEIGHT,
@@ -315,9 +316,13 @@ export function useAppBridge(args: UseAppBridgeArgs): UseAppBridgeReturn {
315316
await bridge.connect(transport);
316317
bridgeRef.current = bridge;
317318

318-
// Send resource to proxy
319+
// Prepend the CSP as a leading <meta> so it constrains the untrusted
320+
// MCP HTML before any script runs; the computed CSP was previously
321+
// threaded to the proxy and dropped, leaving the app unrestricted
322+
// (VERIA-354). Note: a server sets its own csp, so this caps an honest
323+
// or poisoned app, not a fully malicious server declaring a lax policy.
319324
await bridge.sendSandboxResourceReady({
320-
html: resource.html,
325+
html: applyCspToHtml(resource.html, resource.csp),
321326
csp: resource.csp,
322327
permissions: resource.permissions,
323328
});

‎packages/ui/src/features/mcp-apps/utils/mcp-app-csp.test.ts‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { describe, expect, it } from "vitest";
22
import {
3+
applyCspToHtml,
34
buildCspMetaTag,
45
buildCspString,
56
escapeAttr,
@@ -162,3 +163,26 @@ describe("buildCspMetaTag", () => {
162163
expect(tag).toMatch(/content="[^"]+"/);
163164
});
164165
});
166+
167+
describe("applyCspToHtml", () => {
168+
it("prepends the CSP meta when there is no doctype", () => {
169+
const out = applyCspToHtml("<html><body>hi</body></html>");
170+
expect(out.startsWith(buildCspMetaTag())).toBe(true);
171+
});
172+
173+
it("inserts the CSP meta after a leading doctype (no quirks mode)", () => {
174+
const out = applyCspToHtml("<!doctype html><html><head></head></html>");
175+
expect(out.startsWith("<!doctype html>")).toBe(true);
176+
expect(out).toBe(
177+
`<!doctype html>${buildCspMetaTag()}<html><head></head></html>`,
178+
);
179+
});
180+
181+
it("handles a doctype with leading whitespace and mixed case", () => {
182+
const out = applyCspToHtml(" <!DOCTYPE html>\n<html></html>");
183+
expect(out.startsWith(" <!DOCTYPE html>")).toBe(true);
184+
expect(out.indexOf("<!DOCTYPE html>")).toBeLessThan(
185+
out.indexOf(buildCspMetaTag()),
186+
);
187+
});
188+
});

‎packages/ui/src/features/mcp-apps/utils/mcp-app-csp.ts‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,3 +83,17 @@ export function buildCspMetaTag(csp?: McpUiResourceCsp): string {
8383
const cspString = buildCspString(csp);
8484
return `<meta http-equiv="Content-Security-Policy" content="${escapeAttr(cspString)}">`;
8585
}
86+
87+
// Insert the CSP meta after a leading doctype, if present, so it doesn't push
88+
// the doctype out of first position and trip the iframe into quirks mode. The
89+
// meta still lands in <head> before any script, so the policy stays in force.
90+
export function applyCspToHtml(html: string, csp?: McpUiResourceCsp): string {
91+
const meta = buildCspMetaTag(csp);
92+
const doctype = html.match(/^\s*<!doctype[^>]*>/i);
93+
if (doctype) {
94+
return (
95+
html.slice(0, doctype[0].length) + meta + html.slice(doctype[0].length)
96+
);
97+
}
98+
return meta + html;
99+
}

0 commit comments

Comments
 (0)