fix(apps): gate the capability observer on the bridge's own initialize schema
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 <cliff@futurescale.com>
This commit is contained in:
parent
b86db5c7a2
commit
98d1fe752d
2 changed files with 83 additions and 36 deletions
|
|
@ -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({});
|
||||
|
|
|
|||
|
|
@ -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<TMessage = unknown> {
|
|||
}
|
||||
|
||||
/**
|
||||
* 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<string, 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;
|
||||
): Record<string, unknown> | 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<string, unknown>)
|
||||
: 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<string, unknown>
|
||||
| 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);
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue