Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 19 additions & 2 deletions clients/web/src/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import type {
LoggingMessageNotification,
Progress,
ProgressToken,
Resource,
Task,
Tool,
} from "@modelcontextprotocol/client";
Expand Down Expand Up @@ -784,10 +785,24 @@ function App() {
});
}, [configBaseUrl]);

// The `resources/list` entries, read lazily by the App bridge factories below.
// ext-apps treats a listing entry's `_meta.ui` as the static default for its
// UI resource (a read content item's own `_meta.ui` wins), so the sandbox CSP
// has to be able to see it. A ref rather than a dependency: `resources` is
// derived further down this component, and the factories only read it inside
// an async sandboxready handler, long after any render that produced it.
const listedResourcesRef = useRef<Resource[]>([]);
const getListedResourceMeta = useCallback(
(uri: string) =>
listedResourcesRef.current.find((r) => r.uri === uri)?._meta,
[],
);

const sandboxBridgeFactory = useMemo(
() =>
createAppBridgeFactory({
getClient: () => inspectorClient?.getAppRendererClient() ?? null,
getListedResourceMeta,
Comment thread
cliffhall marked this conversation as resolved.
readResource: async (uri) => {
if (!inspectorClient) throw new Error("No MCP client connected.");
const invocation = await inspectorClient.readResource(uri);
Expand All @@ -807,7 +822,7 @@ function App() {
});
},
}),
[inspectorClient],
[inspectorClient, getListedResourceMeta],
);

// App-rendered form elicitations (#1854). The controller is created once and
Expand Down Expand Up @@ -861,6 +876,7 @@ function App() {
createAppBridgeFactory({
advertiseElicitation: true,
getClient: () => inspectorClient?.getAppRendererClient() ?? null,
getListedResourceMeta,
readResource: async (uri) => {
if (!inspectorClient) throw new Error("No MCP client connected.");
const invocation = await inspectorClient.readResource(uri);
Expand All @@ -877,7 +893,7 @@ function App() {
});
},
}),
[inspectorClient],
[inspectorClient, getListedResourceMeta],
);
/**
* Close the previous client's session and open one for the client being
Expand Down Expand Up @@ -1238,6 +1254,7 @@ function App() {
const tools = toolsPagination.items;
const prompts = promptsPagination.items;
const resources = resourcesPagination.items;
listedResourcesRef.current = resources;
const {
tasks,
refresh: refreshTasks,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,21 +83,33 @@ function makeIframe(hasWindow = true): HTMLIFrameElement {

const fakeClient = { name: "sdk-client" } as unknown as Client;

/**
* A UI resource read result. `meta` is the MCP Apps sandbox metadata, which the
* spec nests under the `_meta` bag's `ui` key — the helper wraps it so every
* test drives the real wire shape. `opts.flat` writes it to `_meta` unnested
* instead (the non-conforming shape the host must ignore).
*/
function uiResource(
text: string | undefined,
meta?: Record<string, unknown>,
opts: { flat?: boolean } = {},
): ReadResourceResult {
return {
contents: [
{
uri: "ui://weather/app.html",
...(text === undefined ? {} : { text }),
...(meta ? { _meta: meta } : {}),
...(meta ? { _meta: opts.flat ? meta : { ui: meta } } : {}),
},
],
} as ReadResourceResult;
}

/** The `_meta` of a `resources/list` entry, as `getListedResourceMeta` returns it. */
function listedMeta(meta: Record<string, unknown>): { ui: unknown } {
return { ui: meta };
}

async function flush(): Promise<void> {
await Promise.resolve();
await Promise.resolve();
Expand Down Expand Up @@ -230,6 +242,121 @@ describe("createAppBridgeFactory", () => {
});
});

it("falls back to the resources/list entry's _meta.ui when the content block carries none", async () => {
// ext-apps documents the listing-level `_meta.ui` as the static default for
// a UI resource, so an app declaring its CSP only there must still have it
// honored.
const readResource = vi.fn().mockResolvedValue(uiResource("<h1>x</h1>"));
const getListedResourceMeta = vi.fn().mockReturnValue(
listedMeta({
permissions: { camera: {} },
csp: { connectDomains: ["https://api.example.com"] },
}),
);
const factory = createAppBridgeFactory({
getClient: () => fakeClient,
readResource,
getListedResourceMeta,
});
await factory(makeIframe(), { kind: "tool", tool });
const bridge = bridgeInstances[0];
bridge.emit("sandboxready");
await flush();

expect(getListedResourceMeta).toHaveBeenCalledWith("ui://weather/app.html");
const call = bridge.sendSandboxResourceReady.mock.calls[0][0] as {
html: string;
permissions: unknown;
};
expect(call.permissions).toEqual({ camera: {} });
expect(call.html).toContain("connect-src https://api.example.com");
});

it("prefers the read content item's _meta.ui over the listing entry's", async () => {
// The listing value is only a default: a content item that carries its own
// `_meta.ui` wins outright, rather than being merged with it.
const readResource = vi.fn().mockResolvedValue(
uiResource("<h1>x</h1>", {
csp: { connectDomains: ["https://read.example.com"] },
}),
);
const getListedResourceMeta = vi.fn().mockReturnValue(
listedMeta({
permissions: { camera: {} },
csp: { connectDomains: ["https://listed.example.com"] },
}),
);
const factory = createAppBridgeFactory({
getClient: () => fakeClient,
readResource,
getListedResourceMeta,
});
await factory(makeIframe(), { kind: "tool", tool });
const bridge = bridgeInstances[0];
bridge.emit("sandboxready");
await flush();

const call = bridge.sendSandboxResourceReady.mock.calls[0][0] as {
html: string;
permissions: unknown;
};
expect(call.html).toContain("connect-src https://read.example.com");
expect(call.html).not.toContain("listed.example.com");
expect(call.permissions).toBeUndefined();
});

it("renders with no sandbox hints when neither the content item nor the listing has _meta.ui", async () => {
// A host with no resource listing (or one not covering this URI) is
// unaffected: the optional dep is simply absent.
const readResource = vi.fn().mockResolvedValue(uiResource("<h1>x</h1>"));
const factory = createAppBridgeFactory({
getClient: () => fakeClient,
readResource,
getListedResourceMeta: () => undefined,
});
await factory(makeIframe(), { kind: "tool", tool });
const bridge = bridgeInstances[0];
bridge.emit("sandboxready");
await flush();

const call = bridge.sendSandboxResourceReady.mock.calls[0][0] as {
html: string;
permissions: unknown;
};
expect(call.permissions).toBeUndefined();
expect(call.html).toContain("connect-src &#39;none&#39;");
});

it("ignores sandbox metadata written unnested on _meta", async () => {
// `McpUiResourceMeta` describes the value of `_meta.ui`, not `_meta`. A bag
// whose keys sit at the top level is not the spec shape, so it must not be
// read as one — reading it there is what produced #2055 in reverse.
const readResource = vi
.fn()
.mockResolvedValue(
uiResource(
"<h1>x</h1>",
{ csp: { connectDomains: ["https://api.example.com"] } },
{ flat: true },
),
);
const factory = createAppBridgeFactory({
getClient: () => fakeClient,
readResource,
});
await factory(makeIframe(), { kind: "tool", tool });
const bridge = bridgeInstances[0];
bridge.emit("sandboxready");
await flush();

const call = bridge.sendSandboxResourceReady.mock.calls[0][0] as {
html: string;
};
expect(call.html).not.toContain("api.example.com");
// The policy is rendered into an HTML attribute, so quotes arrive escaped.
expect(call.html).toContain("connect-src &#39;none&#39;");
});

it("does not mutate the shared HOST_CAPABILITIES when echoing the approved sandbox", async () => {
// The factory builds a per-app copy ({ ...HOST_CAPABILITIES }) so the
// sandbox echo never leaks across apps/renders. Lock that in: after a
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,15 @@ export interface AppBridgeFactoryDeps {
* frame; the error is also always console.error'd.
*/
onResourceError?: (err: Error) => void;
/**
* The `_meta` of the `resources/list` entry for `uri`, when the host has that
* listing. ext-apps treats the listing-level `_meta.ui` as the static default
* for a UI resource, so an app that declares its CSP only there must still
* get it honored; a read content item's own `_meta.ui` wins over it. Omitted
* (or returning undefined) simply means no default — the host that has no
* resource listing, or whose listing does not include this URI, is unaffected.
*/
getListedResourceMeta?: (uri: string) => object | undefined;
/**
* Advertise `hostCapabilities.elicitation` — "this host can forward a
* form-mode `elicitation/create` to the app and return its result to the
Expand All @@ -100,8 +109,37 @@ export interface AppBridgeFactoryDeps {
advertiseElicitation?: boolean;
}

/** First text content block of a UI resource, plus its `_meta` (sandbox hints). */
function extractHtmlAndMeta(result: ReadResourceResult): {
/**
* The MCP Apps sandbox hints (`csp`, `permissions`, …) nested under a `_meta`
* bag's `ui` key. `McpUiResourceMeta` describes the value of `_meta.ui`, NOT
* `_meta` itself — reading `_meta` directly leaves every field `undefined`, so
* a spec-conforming app's declared `connectDomains` never reach the generated
* CSP and it renders under `connect-src 'none'` (#2055). This mirrors the tool
* side, where `getToolUiResourceUri` already reads `_meta.ui.resourceUri`.
*/
function uiMeta(meta: object | undefined): McpUiResourceMeta | undefined {
// `_meta` is typed with an index signature, which has no properties in common
// with a `{ ui?: unknown }` parameter — hence the cast rather than a narrower
// parameter type. Reading one key off an untyped bag is safe.
const ui = (meta as { ui?: unknown } | undefined)?.ui;
return ui !== null && typeof ui === "object"
? (ui as McpUiResourceMeta)
: undefined;
Comment thread
cliffhall marked this conversation as resolved.
Outdated
}

/**
* First text content block of a UI resource, plus its `_meta.ui` sandbox hints.
*
* `listedMeta` is the `_meta` of the matching `resources/list` entry, which
* ext-apps documents as the static default a host can review at connection
* time; a `resources/read` content item carrying its own `_meta.ui` takes
* precedence over it (`McpUiAppResourceConfig`). Nothing else is consulted —
* in particular the read RESULT's own `_meta` is not a documented carrier.
Comment thread
cliffhall marked this conversation as resolved.
Outdated
*/
function extractHtmlAndMeta(
result: ReadResourceResult,
listedMeta?: object,
): {
html: string;
meta: McpUiResourceMeta | undefined;
} {
Expand All @@ -110,7 +148,7 @@ function extractHtmlAndMeta(result: ReadResourceResult): {
if (typeof text === "string") {
return {
html: text,
meta: content._meta as McpUiResourceMeta | undefined,
meta: uiMeta(content._meta) ?? uiMeta(listedMeta),
};
}
}
Expand Down Expand Up @@ -271,7 +309,10 @@ export function createAppBridgeFactory(
const uri = resolveSourceUri(source);
if (!uri) return;
const result = await deps.readResource(uri);
const { html, meta } = extractHtmlAndMeta(result);
const { html, meta } = extractHtmlAndMeta(
result,
deps.getListedResourceMeta?.(uri),
);
// Build the per-app CSP host-side: filter the requested sources to
// ones the host accepts, render the policy string, and wrap the
// app's HTML in a fixed shell whose first <head> child is the CSP
Expand Down