From 98d1fe752da582a2199c197c461079e127bc30a4 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Sat, 22 Aug 2026 21:29:55 -0400 Subject: [PATCH] fix(apps): gate the capability observer on the bridge's own initialize schema MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The hand-rolled accept check was shallower than the bridge's, so frames it rejects could still set `elicitation` here — `appInfo: {}` passed, though `McpUiInitializeRequestSchema` requires an implementation name and version. Any approximation drifts; the schema itself does not, and ext-apps exports it. Capabilities are still read from the ORIGINAL frame rather than the parse output, since that schema is precisely what strips `elicitation` — validating with it and reading through it would defeat the module. The one check the schema cannot make is that the frame is a request at all (a notification carries the same method and params), so the id check stays. Also stops treating a truthy non-object as an advertisement: the draft declares `elicitation` as an object, so `elicitation: true` is a value it does not define and the bridge's schema will reject it the moment it carries the key. Signed-off-by: cliffhall --- .../AppRenderer/appCapabilities.test.ts | 38 +++++++++ .../elements/AppRenderer/appCapabilities.ts | 81 ++++++++++--------- 2 files changed, 83 insertions(+), 36 deletions(-) diff --git a/clients/web/src/components/elements/AppRenderer/appCapabilities.test.ts b/clients/web/src/components/elements/AppRenderer/appCapabilities.test.ts index 8217a4ff..8a111e8f 100644 --- a/clients/web/src/components/elements/AppRenderer/appCapabilities.test.ts +++ b/clients/web/src/components/elements/AppRenderer/appCapabilities.test.ts @@ -57,6 +57,10 @@ describe("appCapabilities (#1854)", () => { expect(appAdvertisesElicitation(bridge)).toBe(true); }); + it("is false when the bridge advertises nothing at all", () => { + expect(appAdvertisesElicitation(makeBridge(undefined))).toBe(false); + }); + it("is false for an app that advertised no elicitation", () => { const bridge = makeBridge({ availableDisplayModes: ["inline"] }); const { transport } = makeTransport(); @@ -176,6 +180,40 @@ describe("appCapabilities (#1854)", () => { expect(appAdvertisesElicitation(bridge)).toBe(true); }); + it("rejects a frame the bridge's own schema rejects", () => { + // `appInfo: {}` is missing the implementation name/version that + // `McpUiInitializeRequestSchema` requires, so the bridge answers an error + // and stores nothing — this gate must not be the laxer of the two. + const bridge = makeBridge({}); + const { transport } = makeTransport(); + observeAppCapabilities(bridge, transport); + transport.onmessage?.({ + jsonrpc: "2.0", + id: 1, + method: "ui/initialize", + params: { + protocolVersion: "2026-01-26", + appInfo: {}, + appCapabilities: { elicitation: {} }, + }, + } as unknown as JSONRPCMessage); + expect(appAdvertisesElicitation(bridge)).toBe(false); + }); + + it("does not treat a non-object elicitation value as an advertisement", () => { + // The draft declares `elicitation` as an object; `true` is a value it does + // not define, and truthiness would accept it. + const bridge = makeBridge({}); + const { transport } = makeTransport(); + observeAppCapabilities(bridge, transport); + transport.onmessage?.(initializeFrame({ elicitation: true })); + expect(appAdvertisesElicitation(bridge)).toBe(false); + transport.onmessage?.(initializeFrame({ elicitation: [] })); + expect(appAdvertisesElicitation(bridge)).toBe(false); + transport.onmessage?.(initializeFrame({ elicitation: {} })); + expect(appAdvertisesElicitation(bridge)).toBe(true); + }); + it("keeps bridges independent", () => { const a = makeBridge({}); const b = makeBridge({}); diff --git a/clients/web/src/components/elements/AppRenderer/appCapabilities.ts b/clients/web/src/components/elements/AppRenderer/appCapabilities.ts index f1f3f1ef..9dbac057 100644 --- a/clients/web/src/components/elements/AppRenderer/appCapabilities.ts +++ b/clients/web/src/components/elements/AppRenderer/appCapabilities.ts @@ -1,3 +1,4 @@ +import { McpUiInitializeRequestSchema } from "@modelcontextprotocol/ext-apps/app-bridge"; import type { AppBridge } from "@modelcontextprotocol/ext-apps/app-bridge"; /** @@ -39,40 +40,36 @@ export interface MessageObservable { } /** - * Whether `message` is a well-formed `ui/initialize` REQUEST — a JSON-RPC - * request id, plus the three params the handshake requires - * (`protocolVersion`, `appInfo`, `appCapabilities`). + * The capabilities a view advertised in an `ui/initialize` the bridge will + * ACCEPT, or `undefined` for any other frame. * - * This is a gate, not a second copy of the bridge's schema: the bridge stays - * the authority on everything else in the frame. What it has to stop is a - * *malformed* initialize the bridge will reject, since accepting one here - * would let a view flip `elicitation` on through a frame that never negotiated - * anything. Narrows from `unknown` throughout — the frame is whatever the - * sandboxed view posted. + * Acceptance is decided by the bridge's own `McpUiInitializeRequestSchema` + * rather than a hand-rolled approximation of it — an approximation drifts, and + * anything it waves through that the bridge rejects becomes a second, laxer + * route to setting `elicitation` on a frame that negotiated nothing. The one + * check the schema cannot make is that this is a request at all: a JSON-RPC + * notification carries the same `method`/`params`, so the id is checked here. + * + * The capabilities are then read from the ORIGINAL frame, not the parse output, + * because that schema is exactly what strips `elicitation` (see the WeakMap's + * doc comment) — validating with it and reading through it would defeat the + * purpose of this module. */ -function isInitializeRequest( +function acceptedInitializeCapabilities( message: unknown, -): message is { params: { appCapabilities: Record } } { - if (typeof message !== "object" || message === null) return false; - const frame = message as { id?: unknown; method?: unknown; params?: unknown }; - if (frame.method !== "ui/initialize") return false; +): Record | undefined { + if (typeof message !== "object" || message === null) return undefined; + const frame = message as { id?: unknown; params?: unknown }; // A notification (no id) is not the handshake request. - if (frame.id === undefined || frame.id === null) return false; - if (typeof frame.params !== "object" || frame.params === null) return false; - const params = frame.params as { - protocolVersion?: unknown; - appInfo?: unknown; - appCapabilities?: unknown; - }; - // All three are required by the bridge's own initialize schema, so a frame - // missing any of them is one the bridge rejects. - return ( - typeof params.protocolVersion === "string" && - typeof params.appInfo === "object" && - params.appInfo !== null && - typeof params.appCapabilities === "object" && - params.appCapabilities !== null - ); + if (frame.id === undefined || frame.id === null) return undefined; + if (!McpUiInitializeRequestSchema.safeParse(message).success) { + return undefined; + } + const advertised = (frame.params as { appCapabilities?: unknown }) + .appCapabilities; + return typeof advertised === "object" && advertised !== null + ? (advertised as Record) + : undefined; } /** @@ -85,15 +82,14 @@ function isInitializeRequest( * capabilities the bridge no longer holds, in both directions. * * A frame the bridge would *reject* records nothing and leaves the previous - * value alone, which is what keeps the gate fail-closed: a malformed initialize - * must not be a second, laxer route to setting `elicitation`. + * value alone: it is a route to changing this gate in neither direction. */ function recordAdvertisedCapabilities( bridge: AppBridge, message: unknown, ): void { - if (!isInitializeRequest(message)) return; - rawAppCapabilities.set(bridge, message.params.appCapabilities); + const advertised = acceptedInitializeCapabilities(message); + if (advertised) rawAppCapabilities.set(bridge, advertised); } /** @@ -124,6 +120,19 @@ export function appAdvertisesElicitation(bridge: AppBridge): boolean { const parsed = bridge.getAppCapabilities() as | Record | undefined; - if (parsed?.elicitation) return true; - return Boolean(rawAppCapabilities.get(bridge)?.elicitation); + return ( + isCapabilityObject(parsed?.elicitation) || + isCapabilityObject(rawAppCapabilities.get(bridge)?.elicitation) + ); +} + +/** + * A declared capability is an OBJECT (`{}` today, room for sub-options later), + * which is what ext-apps#733 specifies. Truthiness is not the same test: an + * `elicitation: true` is a value the draft does not define, and treating it as + * an advertisement would accept something the bridge's own schema will reject + * the moment it carries the key. + */ +function isCapabilityObject(value: unknown): boolean { + return typeof value === "object" && value !== null && !Array.isArray(value); }