diff --git a/clients/web/src/App.tsx b/clients/web/src/App.tsx index 613ad758..59201890 100644 --- a/clients/web/src/App.tsx +++ b/clients/web/src/App.tsx @@ -1254,6 +1254,13 @@ function App() { const tools = toolsPagination.items; const prompts = promptsPagination.items; const resources = resourcesPagination.items; + // Whatever the resource list currently holds — every page in the default + // aggregate mode, only the pages fetched so far under `paginatedLists`. The + // listing is a documented *default* that a read content item overrides, and + // an app whose entry sits on an unfetched page simply falls back to no hints + // (`connect-src 'none'`), exactly as before this wiring existed. Walking the + // whole list to close that would issue the very requests the user opted out + // of by turning pagination on, so the setting wins. listedResourcesRef.current = resources; const { tasks, diff --git a/clients/web/src/components/elements/AppRenderer/createAppBridgeFactory.ts b/clients/web/src/components/elements/AppRenderer/createAppBridgeFactory.ts index 6363d14e..2480458d 100644 --- a/clients/web/src/components/elements/AppRenderer/createAppBridgeFactory.ts +++ b/clients/web/src/components/elements/AppRenderer/createAppBridgeFactory.ts @@ -17,6 +17,7 @@ import type { } from "@modelcontextprotocol/client"; import { approveCspSources, + approveSandboxPermissions, buildSandboxCspPolicy, wrapSandboxedHtml, } from "../../../utils/sandbox-csp"; @@ -324,23 +325,25 @@ export function createAppBridgeFactory( // 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. + // Both halves of the app-supplied sandbox config are screened, for + // different reasons. `csp` is injection-filtered by approveCspSources + // because its sources are interpolated into the CSP content + // string. `permissions` can't reach the `sandbox`/`allow` attribute as + // raw text — the proxy maps each known key to a fixed Permissions-Policy + // token — but it tests those keys for TRUTHINESS, so a non-boolean + // (`camera: "false"`) would read as a grant; approveSandboxPermissions + // reduces the bag to the keys actually requested. The `allow-same-origin` + // strip and the rest of that layer stay owned by #1565. + const approvedPermissions = approveSandboxPermissions( + meta?.permissions, + ); hostCapabilities.sandbox = { - permissions: meta?.permissions, + permissions: approvedPermissions, csp: approvedCsp, }; await bridge.sendSandboxResourceReady({ html: wrapSandboxedHtml(html, buildSandboxCspPolicy(approvedCsp)), - permissions: meta?.permissions, + permissions: approvedPermissions, }); } catch (err) { const error = err instanceof Error ? err : new Error(String(err)); diff --git a/clients/web/src/utils/sandbox-csp.test.ts b/clients/web/src/utils/sandbox-csp.test.ts index de7d6352..899ebd08 100644 --- a/clients/web/src/utils/sandbox-csp.test.ts +++ b/clients/web/src/utils/sandbox-csp.test.ts @@ -2,10 +2,12 @@ import { describe, expect, it, vi } from "vitest"; import { SAFE_CSP_SOURCE, approveCspSources, + approveSandboxPermissions, buildSandboxCspPolicy, escapeHtmlAttr, wrapSandboxedHtml, } from "./sandbox-csp"; +import type { McpUiResourcePermissions } from "@modelcontextprotocol/ext-apps/app-bridge"; describe("SAFE_CSP_SOURCE", () => { it.each([ @@ -81,6 +83,52 @@ describe("approveCspSources", () => { }); }); +describe("approveSandboxPermissions", () => { + it("keeps the spec's empty-object marker and `true` as shorthand", () => { + expect( + approveSandboxPermissions({ + camera: {}, + microphone: true as unknown as Record, + }), + ).toEqual({ camera: {}, microphone: {} }); + }); + + it("drops a truthy non-marker value the proxy would have honored", () => { + // The proxy's buildAllowAttribute() tests each key for truthiness, so the + // string "false" — a plausible way for a server to mean "off" — would + // otherwise switch the camera on. + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + expect( + approveSandboxPermissions({ + camera: "false", + geolocation: 1, + microphone: null, + } as unknown as McpUiResourcePermissions), + ).toBeUndefined(); + expect(warn).toHaveBeenCalledTimes(3); + warn.mockRestore(); + }); + + it("drops `false` without warning, and ignores unknown keys", () => { + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + expect( + approveSandboxPermissions({ + camera: false, + midi: {}, + clipboardWrite: {}, + } as unknown as McpUiResourcePermissions), + ).toEqual({ clipboardWrite: {} }); + // `false` is an explicit refusal, not a malformed value. + expect(warn).not.toHaveBeenCalled(); + warn.mockRestore(); + }); + + it("returns undefined for no permissions at all", () => { + expect(approveSandboxPermissions(undefined)).toBeUndefined(); + expect(approveSandboxPermissions({})).toBeUndefined(); + }); +}); + describe("buildSandboxCspPolicy", () => { it("emits locked-down defaults for an empty approved csp", () => { const policy = buildSandboxCspPolicy({}); diff --git a/clients/web/src/utils/sandbox-csp.ts b/clients/web/src/utils/sandbox-csp.ts index b3d44004..b3283335 100644 --- a/clients/web/src/utils/sandbox-csp.ts +++ b/clients/web/src/utils/sandbox-csp.ts @@ -1,4 +1,7 @@ -import type { McpUiResourceCsp } from "@modelcontextprotocol/ext-apps/app-bridge"; +import type { + McpUiResourceCsp, + McpUiResourcePermissions, +} from "@modelcontextprotocol/ext-apps/app-bridge"; /** * Allowed shapes for a CSP source-expression supplied by an app's @@ -75,6 +78,65 @@ export function approveCspSources( return approved; } +/** The permission-key analog of {@link exhaustiveCspKeys}; same compile-time completeness proof. */ +function exhaustivePermissionKeys< + const T extends readonly (keyof McpUiResourcePermissions)[], +>( + keys: T & + (keyof McpUiResourcePermissions extends T[number] ? unknown : never), +): T { + return keys; +} + +const PERMISSION_KEYS = exhaustivePermissionKeys([ + "camera", + "microphone", + "geolocation", + "clipboardWrite", +]); + +/** + * Filter an app-supplied {@link McpUiResourcePermissions} down to the keys the + * host will forward to the sandbox proxy, keeping only those the app actually + * *requested*. + * + * The proxy's `buildAllowAttribute()` tests each key for **truthiness**, so an + * unvalidated bag lets a non-boolean stand in for a grant: `{ camera: "false" }` + * — a plausible way for a server to mean "off" — is truthy and would switch the + * `allow` attribute on. `_meta` is untrusted input, so the value is screened + * here rather than at the point of use. A key counts as requested when it + * carries the spec's empty-object marker (`camera: {}`), or `true` as the + * obvious shorthand; everything else — `false`, any string, `null`, a number — + * is dropped, as is any key outside the four the proxy knows. + * + * Returns undefined when nothing survives, so the notification and the + * `hostCapabilities.sandbox` echo carry no permissions at all rather than an + * empty object. + */ +export function approveSandboxPermissions( + permissions: McpUiResourcePermissions | undefined, +): McpUiResourcePermissions | undefined { + if (!permissions) return undefined; + const approved: McpUiResourcePermissions = {}; + let granted = false; + for (const key of PERMISSION_KEYS) { + const requested: unknown = permissions[key]; + if ( + requested === true || + (typeof requested === "object" && requested !== null) + ) { + approved[key] = {}; + granted = true; + } else if (requested !== undefined && requested !== false) { + console.warn( + `[mcp-app sandbox] dropping unrecognized "${key}" permission value:`, + requested, + ); + } + } + return granted ? approved : undefined; +} + function joinSources(list: string[] | undefined, fallback: string): string { return list && list.length > 0 ? list.join(" ") : fallback; }