From cc0770babd267d95cae1e2a256b10b175ab39eca Mon Sep 17 00:00:00 2001 From: cliffhall Date: Sat, 22 Aug 2026 20:45:28 -0400 Subject: [PATCH] fix(apps): fail closed on a malformed app ui/initialize MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The capability observer recorded `params.appCapabilities` from ANY frame whose method was `ui/initialize`, before the bridge validated it. A view could therefore send a second, malformed initialize — one the bridge rejects, keeping the capabilities it had already accepted — and flip `elicitation` on in this gate, after which the host would forward an elicitation the bridge never negotiated. Now only a well-formed initialize REQUEST is recorded (a JSON-RPC id, plus the handshake's required `protocolVersion` and `appInfo`), and only the first one: the bridge keeps what it accepted and ignores re-initialization, so this gate agrees rather than offering a second, laxer path to the same flag. A malformed first frame records nothing. This is a gate, not a second copy of the bridge's schema — the bridge stays the authority on the rest of the frame. Signed-off-by: cliffhall --- .../AppRenderer/appCapabilities.test.ts | 62 ++++++++++++++++++- .../elements/AppRenderer/appCapabilities.ts | 51 ++++++++++++--- 2 files changed, 102 insertions(+), 11 deletions(-) diff --git a/clients/web/src/components/elements/AppRenderer/appCapabilities.test.ts b/clients/web/src/components/elements/AppRenderer/appCapabilities.test.ts index 6979d170..c7f8e374 100644 --- a/clients/web/src/components/elements/AppRenderer/appCapabilities.test.ts +++ b/clients/web/src/components/elements/AppRenderer/appCapabilities.test.ts @@ -19,12 +19,20 @@ function makeTransport(): { return { transport, inner }; } -function initializeFrame(appCapabilities: unknown): JSONRPCMessage { +function initializeFrame( + appCapabilities: unknown, + overrides: Record = {}, +): JSONRPCMessage { return { jsonrpc: "2.0", id: 1, method: "ui/initialize", - params: { appCapabilities }, + params: { + protocolVersion: "2026-01-26", + appInfo: { name: "test-app", version: "1.0.0" }, + appCapabilities, + ...overrides, + }, } as unknown as JSONRPCMessage; } @@ -94,6 +102,56 @@ describe("appCapabilities (#1854)", () => { expect(inner).toHaveBeenCalledTimes(4); }); + it("ignores a malformed initialize the bridge would reject", () => { + // Fail-closed: accepting one would let a view flip `elicitation` on with a + // frame that never negotiated anything, and the host would then forward a + // request the bridge does not consider negotiated. + const bridge = makeBridge({}); + const { transport } = makeTransport(); + observeAppCapabilities(bridge, transport); + + // No protocolVersion. + transport.onmessage?.({ + jsonrpc: "2.0", + id: 1, + method: "ui/initialize", + params: { appInfo: { name: "a" }, appCapabilities: { elicitation: {} } }, + } as unknown as JSONRPCMessage); + // No appInfo. + transport.onmessage?.({ + jsonrpc: "2.0", + id: 2, + method: "ui/initialize", + params: { + protocolVersion: "2026-01-26", + appCapabilities: { elicitation: {} }, + }, + } as unknown as JSONRPCMessage); + // A notification, not the handshake request. + transport.onmessage?.({ + jsonrpc: "2.0", + method: "ui/initialize", + params: { + protocolVersion: "2026-01-26", + appInfo: { name: "a" }, + appCapabilities: { elicitation: {} }, + }, + } as unknown as JSONRPCMessage); + + expect(appAdvertisesElicitation(bridge)).toBe(false); + }); + + it("keeps the capabilities from the first handshake, not a later frame", () => { + // The bridge keeps what it accepted and ignores re-initialization; this + // gate has to agree, or a second frame could change what it reports. + const bridge = makeBridge({}); + const { transport } = makeTransport(); + observeAppCapabilities(bridge, transport); + transport.onmessage?.(initializeFrame({ availableDisplayModes: [] })); + transport.onmessage?.(initializeFrame({ elicitation: {} })); + expect(appAdvertisesElicitation(bridge)).toBe(false); + }); + 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 a676fe98..a32f7db8 100644 --- a/clients/web/src/components/elements/AppRenderer/appCapabilities.ts +++ b/clients/web/src/components/elements/AppRenderer/appCapabilities.ts @@ -39,20 +39,53 @@ export interface MessageObservable { } /** - * Record `params.appCapabilities` if `message` is a `ui/initialize` frame. - * Narrows from `unknown` rather than asserting a shape: the frame is whatever - * the sandboxed view posted. + * Whether `message` is a well-formed `ui/initialize` REQUEST — a JSON-RPC + * request id, plus the two params the handshake requires (`protocolVersion`, + * `appInfo`). + * + * 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. + */ +function isInitializeRequest( + message: unknown, +): message is { params: { appCapabilities?: unknown } } { + 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; + // 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; + }; + return ( + typeof params.protocolVersion === "string" && + typeof params.appInfo === "object" && + params.appInfo !== null + ); +} + +/** + * Record `params.appCapabilities` from the view's handshake. + * + * Only the FIRST well-formed `ui/initialize` is recorded, mirroring the bridge: + * it keeps the capabilities it accepted and ignores re-initialization, so a + * later frame — malformed or not — must not be able to change what this gate + * reports either. Fail-closed in both directions: a malformed first frame + * records nothing. */ function recordAdvertisedCapabilities( bridge: AppBridge, message: unknown, ): void { - if (typeof message !== "object" || message === null) return; - const frame = message as { method?: unknown; params?: unknown }; - if (frame.method !== "ui/initialize") return; - if (typeof frame.params !== "object" || frame.params === null) return; - const advertised = (frame.params as { appCapabilities?: unknown }) - .appCapabilities; + if (rawAppCapabilities.has(bridge)) return; + if (!isInitializeRequest(message)) return; + const advertised = message.params.appCapabilities; if (typeof advertised === "object" && advertised !== null) { rawAppCapabilities.set(bridge, advertised as Record); }