diff --git a/clients/web/src/components/elements/AppRenderer/createAppBridgeFactory.test.ts b/clients/web/src/components/elements/AppRenderer/createAppBridgeFactory.test.ts index 54dd6020a..ab8ad7273 100644 --- a/clients/web/src/components/elements/AppRenderer/createAppBridgeFactory.test.ts +++ b/clients/web/src/components/elements/AppRenderer/createAppBridgeFactory.test.ts @@ -61,7 +61,10 @@ vi.mock("@modelcontextprotocol/ext-apps/app-bridge", () => { }; }); -import { createAppBridgeFactory } from "./createAppBridgeFactory"; +import { + createAppBridgeFactory, + HOST_CAPABILITIES, +} from "./createAppBridgeFactory"; const tool: Tool = { name: "weather_app", @@ -141,11 +144,11 @@ describe("createAppBridgeFactory", () => { } }); - it("on sandboxready, reads the UI resource and pushes html + meta to the sandbox", async () => { + it("on sandboxready, reads the UI resource, wraps the html with the per-app CSP, and echoes the approved sandbox config", async () => { const readResource = vi.fn().mockResolvedValue( uiResource("

weather

", { permissions: { geolocation: {} }, - csp: { connectSrc: ["https://api.example.com"] }, + csp: { connectDomains: ["https://api.example.com"] }, }), ); const factory = createAppBridgeFactory({ @@ -159,11 +162,79 @@ describe("createAppBridgeFactory", () => { await flush(); expect(readResource).toHaveBeenCalledWith("ui://weather/app.html"); - expect(bridge.sendSandboxResourceReady).toHaveBeenCalledWith({ - html: "

weather

", + // The html is wrapped in a host-authored shell whose first child is + // the CSP ; the untrusted markup lands inside . The per-app + // connect-src the app requested is baked into the policy. + const call = bridge.sendSandboxResourceReady.mock.calls[0][0] as { + html: string; + permissions: unknown; + csp?: unknown; + }; + expect(call.permissions).toEqual({ geolocation: {} }); + // csp is NOT sent inline — it is enforced via the wrapped and echoed + // through hostCapabilities.sandbox instead. + expect(call.csp).toBeUndefined(); + expect(call.html).toContain('http-equiv="Content-Security-Policy"'); + expect(call.html).toContain("connect-src https://api.example.com"); + expect(call.html).toContain("

weather

"); + + // The approved (post-filter) csp + permissions are echoed on the bridge's + // hostCapabilities so the view sees what was granted. + const caps = bridge.ctorArgs[2] as { + sandbox?: { permissions?: unknown; csp?: unknown }; + }; + expect(caps.sandbox).toEqual({ permissions: { geolocation: {} }, - csp: { connectSrc: ["https://api.example.com"] }, + csp: { connectDomains: ["https://api.example.com"] }, + }); + }); + + it("does not mutate the shared HOST_CAPABILITIES when echoing the approved sandbox", async () => { + // The factory builds a per-app copy ({ ...HOST_CAPABILITIES }) so the + // sandbox echo never leaks across apps/renders. Lock that in: after a + // sandboxready run that sets hostCapabilities.sandbox, the shared constant + // must stay untouched. + const readResource = vi.fn().mockResolvedValue( + uiResource("

x

", { + csp: { connectDomains: ["https://api.example.com"] }, + }), + ); + const factory = createAppBridgeFactory({ + getClient: () => fakeClient, + readResource, + }); + await factory(makeIframe(), tool); + bridgeInstances[0].emit("sandboxready"); + await flush(); + expect(HOST_CAPABILITIES.sandbox).toBeUndefined(); + }); + + it("drops an unsafe app-supplied CSP source before wrapping", async () => { + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + const readResource = vi.fn().mockResolvedValue( + uiResource("

x

", { + // The second source injects a directive terminator — it must be dropped. + csp: { + connectDomains: ["https://ok.example.com", "evil; script-src *"], + }, + }), + ); + const factory = createAppBridgeFactory({ + getClient: () => fakeClient, + readResource, }); + await factory(makeIframe(), tool); + const bridge = bridgeInstances[0]; + bridge.emit("sandboxready"); + await flush(); + + const call = bridge.sendSandboxResourceReady.mock.calls[0][0] as { + html: string; + }; + expect(call.html).toContain("connect-src https://ok.example.com;"); + expect(call.html).not.toContain("evil"); + expect(warn).toHaveBeenCalled(); + warn.mockRestore(); }); it("does not push when the tool has no UI resource uri", async () => { @@ -182,21 +253,72 @@ describe("createAppBridgeFactory", () => { expect(bridgeInstances[0].sendSandboxResourceReady).not.toHaveBeenCalled(); }); - it("swallows a resources/read failure without rejecting", async () => { + it("reports a resources/read failure via onResourceError and console.error without rejecting", async () => { + const err = vi.spyOn(console, "error").mockImplementation(() => {}); + const onResourceError = vi.fn(); const readResource = vi.fn().mockRejectedValue(new Error("read boom")); const factory = createAppBridgeFactory({ getClient: () => fakeClient, readResource, + onResourceError, }); await factory(makeIframe(), tool); const bridge = bridgeInstances[0]; bridge.emit("sandboxready"); await flush(); expect(bridge.sendSandboxResourceReady).not.toHaveBeenCalled(); + expect(onResourceError).toHaveBeenCalledWith( + expect.objectContaining({ message: "read boom" }), + ); + expect(err).toHaveBeenCalled(); + err.mockRestore(); }); - it("swallows a UI resource that has no text content", async () => { + it("reports a UI resource that has no text content via onResourceError", async () => { + const err = vi.spyOn(console, "error").mockImplementation(() => {}); + const onResourceError = vi.fn(); const readResource = vi.fn().mockResolvedValue(uiResource(undefined)); + const factory = createAppBridgeFactory({ + getClient: () => fakeClient, + readResource, + onResourceError, + }); + await factory(makeIframe(), tool); + const bridge = bridgeInstances[0]; + bridge.emit("sandboxready"); + await flush(); + expect(bridge.sendSandboxResourceReady).not.toHaveBeenCalled(); + expect(onResourceError).toHaveBeenCalledWith( + expect.objectContaining({ + message: expect.stringContaining("no text"), + }), + ); + err.mockRestore(); + }); + + it("wraps a non-Error rejection into an Error before reporting", async () => { + const err = vi.spyOn(console, "error").mockImplementation(() => {}); + const onResourceError = vi.fn(); + const readResource = vi.fn().mockRejectedValue("plain string boom"); + const factory = createAppBridgeFactory({ + getClient: () => fakeClient, + readResource, + onResourceError, + }); + await factory(makeIframe(), tool); + const bridge = bridgeInstances[0]; + bridge.emit("sandboxready"); + await flush(); + expect(onResourceError).toHaveBeenCalledWith( + expect.objectContaining({ message: "plain string boom" }), + ); + expect(onResourceError.mock.calls[0][0]).toBeInstanceOf(Error); + err.mockRestore(); + }); + + it("does not throw on read failure when onResourceError is omitted", async () => { + const err = vi.spyOn(console, "error").mockImplementation(() => {}); + const readResource = vi.fn().mockRejectedValue(new Error("read boom")); const factory = createAppBridgeFactory({ getClient: () => fakeClient, readResource, @@ -206,6 +328,8 @@ describe("createAppBridgeFactory", () => { bridge.emit("sandboxready"); await flush(); expect(bridge.sendSandboxResourceReady).not.toHaveBeenCalled(); + expect(err).toHaveBeenCalled(); + err.mockRestore(); }); it("opens http(s) links in a new tab and reports non-http as error", async () => { diff --git a/clients/web/src/components/elements/AppRenderer/createAppBridgeFactory.ts b/clients/web/src/components/elements/AppRenderer/createAppBridgeFactory.ts index 079d26650..2a3dbc07a 100644 --- a/clients/web/src/components/elements/AppRenderer/createAppBridgeFactory.ts +++ b/clients/web/src/components/elements/AppRenderer/createAppBridgeFactory.ts @@ -12,6 +12,11 @@ import type { Implementation, ReadResourceResult, } from "@modelcontextprotocol/sdk/types.js"; +import { + approveCspSources, + buildSandboxCspPolicy, + wrapSandboxedHtml, +} from "../../../lib/sandbox-csp"; import type { BridgeFactory } from "./AppRenderer"; /** @@ -42,6 +47,12 @@ export interface AppBridgeFactoryDeps { getClient: () => Client | null; /** Reads a UI resource (resources/read) and returns the SDK result. */ readResource: (uri: string) => Promise; + /** + * Called when reading or posting the UI resource fails after the sandbox + * proxy is ready. Without this the user is left staring at a blank-but-live + * frame; the error is also always console.error'd. + */ + onResourceError?: (err: Error) => void; } /** @@ -99,7 +110,8 @@ function extractHtmlAndMeta(result: ReadResourceResult): { * 1. constructs a host-side {@link AppBridge} over the SDK client (so the view * can call tools/resources/prompts directly), * 2. on the sandbox proxy's `sandboxready` signal, reads the tool's UI - * resource and pushes its HTML + sandbox/permissions into the inner iframe, + * resource and pushes its HTML + sandbox/permissions/CSP into the inner + * iframe, echoing the applied sandbox config back via hostCapabilities, * 3. handles `openLinks` by opening http(s) URLs in a new tab, * 4. connects a {@link PostMessageTransport} to the iframe and returns the * live bridge. @@ -123,15 +135,21 @@ export function createAppBridgeFactory( throw new Error("Cannot render MCP App: sandbox iframe has no window."); } - const bridge = new AppBridge(client, HOST_INFO, HOST_CAPABILITIES, { + // Per-app copy so the approved-sandbox echo (set on sandboxready below) + // never mutates the shared HOST_CAPABILITIES constant — each app may + // declare its own csp/permissions. + const hostCapabilities: McpUiHostCapabilities = { ...HOST_CAPABILITIES }; + const bridge = new AppBridge(client, HOST_INFO, hostCapabilities, { hostContext: { theme: currentTheme() }, }); // The double-iframe proxy posts `sandboxready` once it can receive content. // Read the tool's UI resource and hand its HTML (plus any sandbox/permission - // hints from the resource _meta) to the inner sandboxed iframe. Swallow - // failures: a read error leaves an empty (but live) app frame rather than - // tearing down the bridge mid-handshake. + // hints from the resource _meta) to the inner sandboxed iframe. A failure + // here is the case a developer most needs surfaced (their app's resource is + // erroring or malformed) — log it and report it via deps.onResourceError so + // the host can show something better than a blank frame. The bridge stays + // live so a retry path remains possible. bridge.addEventListener("sandboxready", () => { void (async () => { try { @@ -139,13 +157,42 @@ export function createAppBridgeFactory( if (!uri) return; const result = await deps.readResource(uri); const { html, meta } = extractHtmlAndMeta(result); + // Build the per-app CSP host-side: filter the requested sources to + // ones the host accepts, render the policy string, and wrap the + // app's HTML in a fixed shell whose first child is the CSP + // . The proxy assigns that document to srcdoc verbatim — it + // never parses the untrusted bytes — so the policy is guaranteed to + // apply before any app content loads. The approved (post-filter) csp + // is what we echo back via hostCapabilities.sandbox so the view sees + // what was granted, not what it asked for. Set before + // sendSandboxResourceReady: the view only sends ui/initialize once it + // has the HTML, so the bridge reflects this in the initialize result. + const approvedCsp = approveCspSources(meta?.csp); + // NOTE on the CSP-vs-permissions asymmetry: `csp` is injection-filtered + // by approveCspSources because its sources are interpolated into the + // CSP content string. `permissions` is NOT filtered here — it is + // a structured object (camera/microphone/geolocation/clipboardWrite + // booleans), and its only consumer is the sandbox proxy's + // buildAllowAttribute(), which maps each known key to a fixed + // Permissions-Policy token and ignores anything else. Untrusted values + // therefore can't reach the iframe `sandbox`/`allow` attribute as raw + // text (that layer, and the allow-same-origin strip, is owned by the + // sandbox-hardening work in #1565), so no source-style allowlist applies. + hostCapabilities.sandbox = { + permissions: meta?.permissions, + csp: approvedCsp, + }; await bridge.sendSandboxResourceReady({ - html, + html: wrapSandboxedHtml(html, buildSandboxCspPolicy(approvedCsp)), permissions: meta?.permissions, - csp: meta?.csp, }); - } catch { - /* read/post failed (or bridge closed) — leave the frame empty */ + } catch (err) { + const error = err instanceof Error ? err : new Error(String(err)); + console.error( + "[mcp-app] failed to load UI resource into sandbox:", + error, + ); + deps.onResourceError?.(error); } })(); });