From 8f120dc90d1d164039a507cfbde489227463c7e7 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Sat, 11 Jul 2026 13:35:21 -0400 Subject: [PATCH 1/3] web: honor ui/download-file with confirmation and http(s) allowlist createAppBridgeFactory now advertises the `downloadFile` host capability and attaches an `ondownloadfile` handler: - Requires a host-mediated confirmation (window.confirm) listing the requested file(s); a declined prompt or an empty payload returns isError without acting. - Inline EmbeddedResources (text or base64 blob) are written to disk via the shared downloadBlob object-URL anchor; base64 is decoded via base64ToBytes. - ResourceLinks are opened in a new tab, restricted to http(s) by the shared isHttpUrl allowlist (javascript:/data:/file:/malformed are rejected). - Labels shown in the confirmation are sanitized (control + format chars stripped, length clamped) so a server-supplied filename/URI can't inject newlines or reflow the prompt. - Partial-batch success is surfaced (isError only when nothing downloaded), with skipped items warned. Adds factory tests for the full download surface (text/blob/link, decline, empty, throw, non-http rejection, label sanitization/clamping, partial batch). Closes #1569 Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01BrihGWcrM9JGRyu41nzZYw --- .../createAppBridgeFactory.test.ts | 305 +++++++++++++++++- .../AppRenderer/createAppBridgeFactory.ts | 113 ++++++- 2 files changed, 414 insertions(+), 4 deletions(-) diff --git a/clients/web/src/components/elements/AppRenderer/createAppBridgeFactory.test.ts b/clients/web/src/components/elements/AppRenderer/createAppBridgeFactory.test.ts index 99cd1ade0..76942aa56 100644 --- a/clients/web/src/components/elements/AppRenderer/createAppBridgeFactory.test.ts +++ b/clients/web/src/components/elements/AppRenderer/createAppBridgeFactory.test.ts @@ -6,7 +6,7 @@ * end-to-end iframe round-trip is covered by the AppsScreen Storybook play test. */ -import { describe, it, expect, vi, beforeEach } from "vitest"; +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; import type { Tool, ReadResourceResult, @@ -23,6 +23,12 @@ interface MockBridge { sendSandboxResourceReady: ReturnType; connect: ReturnType; onopenlink?: (params: { url: string }) => Promise<{ isError?: boolean }>; + ondownloadfile?: (params: { + contents: ( + | { type: "resource"; resource: Record } + | { type: "resource_link"; uri: string } + )[]; + }) => Promise<{ isError?: boolean }>; emit: (event: string, payload?: unknown) => void; } @@ -344,4 +350,301 @@ describe("createAppBridgeFactory", () => { open.mockRestore(); }); + + it("advertises the downloadFile host capability", async () => { + const factory = createAppBridgeFactory({ + getClient: () => fakeClient, + readResource: vi.fn().mockResolvedValue(uiResource("

x

")), + }); + await factory(makeIframe(), tool); + expect(bridgeInstances[0].ctorArgs[2]).toMatchObject({ downloadFile: {} }); + }); + + describe("ondownloadfile", () => { + // happy-dom does not implement window.confirm, so stub it (rather than + // spyOn an absent function). Returns the installed mock for assertions. + function stubConfirm(approved: boolean): ReturnType { + const confirm = vi.fn().mockReturnValue(approved); + vi.stubGlobal("confirm", confirm); + return confirm; + } + + async function buildBridge(): Promise { + const factory = createAppBridgeFactory({ + getClient: () => fakeClient, + readResource: vi.fn().mockResolvedValue(uiResource("

x

")), + }); + await factory(makeIframe(), tool); + return bridgeInstances[0]; + } + + afterEach(() => { + vi.unstubAllGlobals(); + vi.restoreAllMocks(); + }); + + it("downloads an inline text resource after confirmation", async () => { + vi.useFakeTimers(); + const confirm = stubConfirm(true); + const createUrl = vi + .spyOn(URL, "createObjectURL") + .mockReturnValue("blob:fake"); + const revokeUrl = vi + .spyOn(URL, "revokeObjectURL") + .mockImplementation(() => undefined); + const click = vi + .spyOn(HTMLAnchorElement.prototype, "click") + .mockImplementation(() => undefined); + + const bridge = await buildBridge(); + await expect( + bridge.ondownloadfile!({ + contents: [ + { + type: "resource", + resource: { + uri: "file:///report.csv", + mimeType: "text/csv", + text: "a,b\n1,2", + }, + }, + ], + }), + ).resolves.toEqual({ isError: false }); + + expect(confirm).toHaveBeenCalledTimes(1); + expect(createUrl).toHaveBeenCalledTimes(1); + expect(click).toHaveBeenCalledTimes(1); + const clickedAnchor = click.mock.instances[0] as HTMLAnchorElement; + expect(clickedAnchor.download).toBe("report.csv"); + // Revoke is deferred so the browser can read the blob before it's freed. + expect(revokeUrl).not.toHaveBeenCalled(); + vi.runAllTimers(); + expect(revokeUrl).toHaveBeenCalledWith("blob:fake"); + vi.useRealTimers(); + }); + + it("falls back to a 'download' filename when the URI has no usable tail", async () => { + vi.useFakeTimers(); + stubConfirm(true); + vi.spyOn(URL, "createObjectURL").mockReturnValue("blob:fake"); + vi.spyOn(URL, "revokeObjectURL").mockImplementation(() => undefined); + const click = vi + .spyOn(HTMLAnchorElement.prototype, "click") + .mockImplementation(() => undefined); + const bridge = await buildBridge(); + await bridge.ondownloadfile!({ + contents: [ + { + type: "resource", + resource: { + uri: "file:///path/", + mimeType: "text/plain", + text: "", + }, + }, + ], + }); + expect((click.mock.instances[0] as HTMLAnchorElement).download).toBe( + "download", + ); + vi.runAllTimers(); + vi.useRealTimers(); + }); + + it("warns and reports partial success when some items in a batch are skipped", async () => { + stubConfirm(true); + vi.spyOn(URL, "createObjectURL").mockReturnValue("blob:fake"); + vi.spyOn(URL, "revokeObjectURL").mockImplementation(() => undefined); + vi.spyOn(HTMLAnchorElement.prototype, "click").mockImplementation( + () => undefined, + ); + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + const bridge = await buildBridge(); + await expect( + bridge.ondownloadfile!({ + contents: [ + { + type: "resource", + resource: { uri: "file:///ok.txt", text: "ok" }, + }, + { type: "resource_link", uri: "javascript:alert(1)" }, + ], + }), + ).resolves.toEqual({ isError: false }); + expect(warn).toHaveBeenCalledWith( + expect.stringContaining("1 of 2 download item(s) skipped"), + expect.arrayContaining(["javascript:alert(1)"]), + ); + warn.mockRestore(); + }); + + it("decodes a base64 blob resource", async () => { + stubConfirm(true); + const createUrl = vi + .spyOn(URL, "createObjectURL") + .mockReturnValue("blob:fake"); + vi.spyOn(URL, "revokeObjectURL").mockImplementation(() => undefined); + vi.spyOn(HTMLAnchorElement.prototype, "click").mockImplementation( + () => undefined, + ); + + const bridge = await buildBridge(); + await expect( + bridge.ondownloadfile!({ + contents: [ + { + type: "resource", + resource: { + uri: "file:///logo.png", + mimeType: "image/png", + blob: btoa("PNGDATA"), + }, + }, + ], + }), + ).resolves.toEqual({ isError: false }); + + const blob = createUrl.mock.calls[0][0] as Blob; + expect(blob.type).toBe("image/png"); + expect(await blob.text()).toBe("PNGDATA"); + }); + + it("opens an http(s) resource link in a new tab", async () => { + stubConfirm(true); + const open = vi + .spyOn(window, "open") + .mockImplementation(() => null as unknown as Window); + + const bridge = await buildBridge(); + await expect( + bridge.ondownloadfile!({ + contents: [ + { type: "resource_link", uri: "https://example.com/a.pdf" }, + ], + }), + ).resolves.toEqual({ isError: false }); + + expect(open).toHaveBeenCalledWith( + "https://example.com/a.pdf", + "_blank", + "noopener,noreferrer", + ); + }); + + it("returns isError when the user declines the confirmation", async () => { + stubConfirm(false); + const createUrl = vi.spyOn(URL, "createObjectURL"); + + const bridge = await buildBridge(); + await expect( + bridge.ondownloadfile!({ + contents: [ + { type: "resource", resource: { uri: "file:///x.txt", text: "x" } }, + ], + }), + ).resolves.toEqual({ isError: true }); + expect(createUrl).not.toHaveBeenCalled(); + }); + + it("returns isError for an empty contents array without confirming", async () => { + const confirm = stubConfirm(true); + const bridge = await buildBridge(); + await expect(bridge.ondownloadfile!({ contents: [] })).resolves.toEqual({ + isError: true, + }); + expect(confirm).not.toHaveBeenCalled(); + }); + + it("returns isError when a download throws", async () => { + stubConfirm(true); + const err = vi.spyOn(console, "error").mockImplementation(() => {}); + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + vi.spyOn(URL, "createObjectURL").mockImplementation(() => { + throw new Error("boom"); + }); + + const bridge = await buildBridge(); + await expect( + bridge.ondownloadfile!({ + contents: [ + { type: "resource", resource: { uri: "file:///x.txt", text: "x" } }, + ], + }), + ).resolves.toEqual({ isError: true }); + err.mockRestore(); + warn.mockRestore(); + }); + + it("rejects non-http(s) resource_links without opening them", async () => { + stubConfirm(true); + vi.spyOn(console, "warn").mockImplementation(() => {}); + const open = vi + .spyOn(window, "open") + .mockImplementation(() => null as never); + const bridge = await buildBridge(); + for (const uri of [ + "javascript:alert(1)", + "data:text/html,", + "file:///etc/passwd", + "not a url", + ]) { + await expect( + bridge.ondownloadfile!({ + contents: [{ type: "resource_link", uri }], + }), + ).resolves.toEqual({ isError: true }); + } + expect(open).not.toHaveBeenCalled(); + }); + + it("sanitizes the confirmation summary so server-supplied labels cannot inject newlines", async () => { + const confirm = stubConfirm(false); + const bridge = await buildBridge(); + await bridge.ondownloadfile!({ + contents: [ + { + type: "resource_link", + uri: "https://example.com/a\n\nThis is safe, click OK", + }, + ], + }); + const prompt = confirm.mock.calls[0][0] as string; + expect(prompt).not.toContain("\n\nThis is safe"); + expect(prompt).toContain("This is safe, click OK"); + }); + + it("strips bidi-override and zero-width format characters from the confirmation summary", async () => { + const RLO = "\u{202E}"; + const ZWSP = "\u{200B}"; + const confirm = stubConfirm(false); + const bridge = await buildBridge(); + await bridge.ondownloadfile!({ + contents: [ + { + type: "resource_link", + uri: `https://example.com/${RLO}gpj.${ZWSP}exe`, + }, + ], + }); + const prompt = confirm.mock.calls[0][0] as string; + expect(prompt).not.toContain(RLO); + expect(prompt).not.toContain(ZWSP); + }); + + it("clamps an over-long label in the confirmation summary", async () => { + const confirm = stubConfirm(false); + const bridge = await buildBridge(); + const longName = "a".repeat(200); + await bridge.ondownloadfile!({ + contents: [ + { type: "resource_link", uri: `https://example.com/${longName}` }, + ], + }); + const prompt = confirm.mock.calls[0][0] as string; + expect(prompt).toContain("..."); + // The clamped label is 80 chars max (77 + "..."). + expect(prompt).not.toContain(longName); + }); + }); }); diff --git a/clients/web/src/components/elements/AppRenderer/createAppBridgeFactory.ts b/clients/web/src/components/elements/AppRenderer/createAppBridgeFactory.ts index fb785edb9..7893bdf98 100644 --- a/clients/web/src/components/elements/AppRenderer/createAppBridgeFactory.ts +++ b/clients/web/src/components/elements/AppRenderer/createAppBridgeFactory.ts @@ -10,14 +10,21 @@ import type { } from "@modelcontextprotocol/ext-apps/app-bridge"; import type { Client } from "@modelcontextprotocol/sdk/client/index.js"; import type { + EmbeddedResource, Implementation, ReadResourceResult, + ResourceLink, } from "@modelcontextprotocol/sdk/types.js"; import { approveCspSources, buildSandboxCspPolicy, wrapSandboxedHtml, } from "../../../lib/sandbox-csp"; +import { + downloadBlob, + fileNameFromUri, + isHttpUrl, +} from "../../../lib/downloadFile"; import { snapshotHostContext } from "./hostContext"; import type { BridgeFactory } from "./AppRenderer"; @@ -34,11 +41,12 @@ export const HOST_INFO: Implementation = { * Capabilities the inspector host offers a running MCP App. Constructed WITH an * MCP client (see {@link createAppBridgeFactory}), so the bridge auto-forwards * tools/resources/prompts to the view; we only declare the host-side features - * we actually back: external links (handled below), tool/resource list-change - * forwarding, and logging passthrough. + * we actually back: external links and file downloads (both handled below), + * tool/resource list-change forwarding, and logging passthrough. */ export const HOST_CAPABILITIES: McpUiHostCapabilities = { openLinks: {}, + downloadFile: {}, serverTools: { listChanged: true }, serverResources: { listChanged: true }, logging: {}, @@ -85,6 +93,64 @@ function extractHtmlAndMeta(result: ReadResourceResult): { throw new Error("UI resource has no text (HTML) content"); } +/** + * Decode a base64-encoded blob resource into bytes for download. Allocates the + * backing store explicitly so the return type is `Uint8Array` + * (Blob accepts `ArrayBufferView`, not the wider + * `ArrayBufferLike`). + */ +function base64ToBytes(b64: string): Uint8Array { + const binary = atob(b64); + const bytes = new Uint8Array(new ArrayBuffer(binary.length)); + for (let i = 0; i < binary.length; i++) bytes[i] = binary.charCodeAt(i); + return bytes; +} + +/** + * Strip control characters and clamp length so a server-supplied filename or + * URI cannot forge additional lines in the confirmation prompt or push the + * real summary off-screen. + */ +function sanitizeDownloadLabel(label: string): string { + // Cc = control chars (newlines, escape, etc.); Cf = format chars (bidi + // overrides, zero-width joiners, BOM) — both can spoof or reflow the prompt. + const cleaned = label.replace(/[\p{Cc}\p{Cf}]+/gu, " ").trim(); + return cleaned.length > 80 ? cleaned.slice(0, 77) + "..." : cleaned; +} + +/** Human-readable label for a download item, shown in the confirmation prompt. */ +function describeDownloadItem(item: EmbeddedResource | ResourceLink): string { + if (item.type === "resource_link") return item.uri; + return fileNameFromUri(item.resource.uri); +} + +/** + * Trigger a browser download for a single MCP resource item. Inline + * {@link EmbeddedResource}s (text or base64 blob) are written via + * {@link downloadBlob}. A {@link ResourceLink} is *opened* in a new tab — + * the inspector does not fetch the URL to disk itself, since the link may + * require auth or content negotiation the browser can supply but we cannot. + * Returns false when the item carries nothing downloadable or its URI is + * rejected by the http(s)-only allowlist. + */ +function downloadResourceItem(item: EmbeddedResource | ResourceLink): boolean { + if (item.type === "resource_link") { + const parsed = isHttpUrl(item.uri); + if (!parsed) return false; + window.open(parsed.href, "_blank", "noopener,noreferrer"); + return true; + } + const resource = item.resource; + const blob = + "blob" in resource + ? new Blob([base64ToBytes(resource.blob)], { + type: resource.mimeType ?? "application/octet-stream", + }) + : new Blob([resource.text], { type: resource.mimeType ?? "text/plain" }); + downloadBlob(fileNameFromUri(resource.uri), blob); + return true; +} + /** * Builds the {@link BridgeFactory} the AppRenderer uses to bring a sandbox * iframe to life. For each mounted iframe + tool it: @@ -95,7 +161,10 @@ function extractHtmlAndMeta(result: ReadResourceResult): { * 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 + * 4. handles `downloadFile` by confirming with the user, then writing each + * embedded resource to disk via an object-URL anchor (resource links are + * opened in a new tab), + * 5. connects a {@link PostMessageTransport} to the iframe and returns the * live bridge. * * Host-initiated tool input/result are pushed separately through the renderer's @@ -177,6 +246,44 @@ export function createAppBridgeFactory( return { isError: true }; }; + // The view asks the host to save MCP resource contents to disk (sandboxed + // iframes can't download directly). Confirm with the user first — the spec + // requires a host-mediated confirmation — then write each item out. A + // declined prompt, an empty payload, or a thrown error all return isError. + bridge.ondownloadfile = async ({ contents }) => { + if (!Array.isArray(contents) || contents.length === 0) { + return { isError: true }; + } + const summary = contents + .map((item) => sanitizeDownloadLabel(describeDownloadItem(item))) + .join("\n"); + const approved = window.confirm( + `This MCP App wants to download ${contents.length} file(s):\n\n${summary}`, + ); + if (!approved) return { isError: true }; + let succeeded = 0; + const skipped: string[] = []; + for (const item of contents) { + try { + if (downloadResourceItem(item)) { + succeeded++; + } else { + skipped.push(describeDownloadItem(item)); + } + } catch (err) { + skipped.push(describeDownloadItem(item)); + console.error("[mcp-app] download item failed:", err); + } + } + if (skipped.length > 0) { + console.warn( + `[mcp-app] ${skipped.length} of ${contents.length} download item(s) skipped:`, + skipped, + ); + } + return { isError: succeeded === 0 }; + }; + const transport = new PostMessageTransport(targetWindow, targetWindow); await bridge.connect(transport); return bridge; From a3a5bface5b17dfbbe87f7f3419218f216fa4d0b Mon Sep 17 00:00:00 2001 From: cliffhall Date: Sat, 11 Jul 2026 18:57:34 -0400 Subject: [PATCH 2/3] review: distinguish links, cap batch size, comment truncation (PR #1657) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address the claude[bot] review: - #1 (applied): the confirmation now reads "download or open N item(s)" and resource_links are prefixed "↗" in the summary, so a link that opens in a tab is distinguishable from an embedded file that saves to disk. - #2 (applied): cap a single ui/download-file batch at MAX_DOWNLOAD_ITEMS (20); an oversized batch is rejected (isError) before the prompt, with a warn. - #3 (declined): can't check window.open's return value to detect a blocked popup — we pass `noopener`, and per spec window.open returns null even on a successful noopener open, so null can't distinguish blocked from succeeded. - #4b (applied, comment): note the start-of-label truncation is intentional so a link's scheme+host stays visible for the trust decision. Adds tests: oversized batch rejected without confirming/acting; resource-link ↗ prefix in the summary. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01BrihGWcrM9JGRyu41nzZYw --- .../createAppBridgeFactory.test.ts | 33 +++++++++++++++ .../AppRenderer/createAppBridgeFactory.ts | 40 ++++++++++++++++--- 2 files changed, 67 insertions(+), 6 deletions(-) diff --git a/clients/web/src/components/elements/AppRenderer/createAppBridgeFactory.test.ts b/clients/web/src/components/elements/AppRenderer/createAppBridgeFactory.test.ts index 538a326b3..5cc740983 100644 --- a/clients/web/src/components/elements/AppRenderer/createAppBridgeFactory.test.ts +++ b/clients/web/src/components/elements/AppRenderer/createAppBridgeFactory.test.ts @@ -669,5 +669,38 @@ describe("createAppBridgeFactory", () => { // The clamped label is 80 chars max (77 + "..."). expect(prompt).not.toContain(longName); }); + + it("rejects an oversized batch without confirming or acting", async () => { + const confirm = stubConfirm(true); + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + const open = vi + .spyOn(window, "open") + .mockImplementation(() => null as never); + const bridge = await buildBridge(); + // 21 items exceeds the 20-item cap → rejected before the prompt. + const contents = Array.from({ length: 21 }, (_, i) => ({ + type: "resource_link" as const, + uri: `https://example.com/${i}.pdf`, + })); + await expect(bridge.ondownloadfile!({ contents })).resolves.toEqual({ + isError: true, + }); + expect(confirm).not.toHaveBeenCalled(); + expect(open).not.toHaveBeenCalled(); + expect(warn).toHaveBeenCalledWith( + expect.stringContaining("refusing download batch of 21 items"), + ); + warn.mockRestore(); + }); + + it("marks resource links with a ↗ prefix in the confirmation summary", async () => { + const confirm = stubConfirm(false); + const bridge = await buildBridge(); + await bridge.ondownloadfile!({ + contents: [{ type: "resource_link", uri: "https://example.com/a.pdf" }], + }); + const prompt = confirm.mock.calls[0][0] as string; + expect(prompt).toContain("↗ https://example.com/a.pdf"); + }); }); }); diff --git a/clients/web/src/components/elements/AppRenderer/createAppBridgeFactory.ts b/clients/web/src/components/elements/AppRenderer/createAppBridgeFactory.ts index d77f54f5e..ac3a18d72 100644 --- a/clients/web/src/components/elements/AppRenderer/createAppBridgeFactory.ts +++ b/clients/web/src/components/elements/AppRenderer/createAppBridgeFactory.ts @@ -106,6 +106,12 @@ function base64ToBytes(b64: string): Uint8Array { return bytes; } +/** + * Upper bound on items honored from a single `ui/download-file` request. One + * user approval must not fan out into an unbounded number of saves / new tabs. + */ +const MAX_DOWNLOAD_ITEMS = 20; + /** * Strip control characters and clamp length so a server-supplied filename or * URI cannot forge additional lines in the confirmation prompt or push the @@ -115,12 +121,24 @@ function sanitizeDownloadLabel(label: string): string { // Cc = control chars (newlines, escape, etc.); Cf = format chars (bidi // overrides, zero-width joiners, BOM) — both can spoof or reflow the prompt. const cleaned = label.replace(/[\p{Cc}\p{Cf}]+/gu, " ").trim(); + // Keep the START of an over-long label: for a link that preserves the + // scheme+host, which is what the user needs to make a trust decision. return cleaned.length > 80 ? cleaned.slice(0, 77) + "..." : cleaned; } -/** Human-readable label for a download item, shown in the confirmation prompt. */ -function describeDownloadItem(item: EmbeddedResource | ResourceLink): string { - if (item.type === "resource_link") return item.uri; +/** + * Human-readable label for a download item, shown in the confirmation prompt. + * `forPrompt` marks a resource_link with a leading "↗" so the user can tell a + * link that will *open in a tab* apart from an embedded file that will *save to + * disk* — the two item kinds share this "download" confirmation. + */ +function describeDownloadItem( + item: EmbeddedResource | ResourceLink, + forPrompt = false, +): string { + if (item.type === "resource_link") { + return forPrompt ? `↗ ${item.uri}` : item.uri; + } return fileNameFromUri(item.resource.uri); } @@ -259,16 +277,26 @@ export function createAppBridgeFactory( // The view asks the host to save MCP resource contents to disk (sandboxed // iframes can't download directly). Confirm with the user first — the spec // requires a host-mediated confirmation — then write each item out. A - // declined prompt, an empty payload, or a thrown error all return isError. + // declined prompt, an empty/oversized payload, or a thrown error all + // return isError. bridge.ondownloadfile = async ({ contents }) => { if (!Array.isArray(contents) || contents.length === 0) { return { isError: true }; } + // Sanity cap: one approval must not fan out into an unbounded number of + // downloads / new tabs. A buggy or hostile app requesting hundreds of + // items is rejected outright rather than acted on. + if (contents.length > MAX_DOWNLOAD_ITEMS) { + console.warn( + `[mcp-app] refusing download batch of ${contents.length} items (max ${MAX_DOWNLOAD_ITEMS})`, + ); + return { isError: true }; + } const summary = contents - .map((item) => sanitizeDownloadLabel(describeDownloadItem(item))) + .map((item) => sanitizeDownloadLabel(describeDownloadItem(item, true))) .join("\n"); const approved = window.confirm( - `This MCP App wants to download ${contents.length} file(s):\n\n${summary}`, + `This MCP App wants to download or open ${contents.length} item(s):\n\n${summary}`, ); if (!approved) return { isError: true }; let succeeded = 0; From ca21c59f45c59d0e7e6f1eb5c4e7f134b1dda3fe Mon Sep 17 00:00:00 2001 From: cliffhall Date: Sat, 11 Jul 2026 19:03:51 -0400 Subject: [PATCH 3/3] review: skip embedded resource with neither text nor blob (PR #1657 round 2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address the round-2 observation: an untrusted EmbeddedResource carrying neither `blob` nor a string `text` previously produced `new Blob([undefined])` — a file containing the literal text "undefined", counted as success. Now such an item is skipped (returns false, like a rejected link), so isError reflects it. Added a test asserting no object URL is created and the batch reports isError. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01BrihGWcrM9JGRyu41nzZYw --- .../AppRenderer/createAppBridgeFactory.test.ts | 16 ++++++++++++++++ .../AppRenderer/createAppBridgeFactory.ts | 4 ++++ 2 files changed, 20 insertions(+) diff --git a/clients/web/src/components/elements/AppRenderer/createAppBridgeFactory.test.ts b/clients/web/src/components/elements/AppRenderer/createAppBridgeFactory.test.ts index 5cc740983..45aa3cde9 100644 --- a/clients/web/src/components/elements/AppRenderer/createAppBridgeFactory.test.ts +++ b/clients/web/src/components/elements/AppRenderer/createAppBridgeFactory.test.ts @@ -702,5 +702,21 @@ describe("createAppBridgeFactory", () => { const prompt = confirm.mock.calls[0][0] as string; expect(prompt).toContain("↗ https://example.com/a.pdf"); }); + + it("skips an embedded resource with neither text nor blob (no 'undefined' file)", async () => { + stubConfirm(true); + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + const createUrl = vi.spyOn(URL, "createObjectURL"); + const bridge = await buildBridge(); + // Untrusted payload: a resource object carrying neither field. It must be + // skipped, not written as a file containing the literal text "undefined". + await expect( + bridge.ondownloadfile!({ + contents: [{ type: "resource", resource: { uri: "file:///x" } }], + }), + ).resolves.toEqual({ isError: true }); + expect(createUrl).not.toHaveBeenCalled(); + warn.mockRestore(); + }); }); }); diff --git a/clients/web/src/components/elements/AppRenderer/createAppBridgeFactory.ts b/clients/web/src/components/elements/AppRenderer/createAppBridgeFactory.ts index ac3a18d72..4b59b4f61 100644 --- a/clients/web/src/components/elements/AppRenderer/createAppBridgeFactory.ts +++ b/clients/web/src/components/elements/AppRenderer/createAppBridgeFactory.ts @@ -159,6 +159,10 @@ function downloadResourceItem(item: EmbeddedResource | ResourceLink): boolean { return true; } const resource = item.resource; + // The types forbid it, but the payload is untrusted: a resource with neither + // `blob` nor a string `text` has nothing to save. Skip it (like a rejected + // link) rather than writing a file containing the literal text "undefined". + if (!("blob" in resource) && typeof resource.text !== "string") return false; const blob = "blob" in resource ? new Blob([base64ToBytes(resource.blob)], {