Merge pull request #2098 from modelcontextprotocol/v2/fix/2055-ui-resource-meta-ui
fix(web): read MCP App UI resource metadata from _meta.ui
This commit is contained in:
commit
7682263321
7 changed files with 595 additions and 24 deletions
|
|
@ -368,6 +368,29 @@ vi.mock("@inspector/core/react/useSettingsDraft.js", () => ({
|
|||
})),
|
||||
}));
|
||||
|
||||
// --- App bridge factory spy (#2055) -----------------------------------------
|
||||
// Passes through to the real factory but records the deps App hands it, so a
|
||||
// test can drive `getListedResourceMeta` — the wiring that carries a
|
||||
// `resources/list` entry's `_meta.ui` into the sandbox CSP. Without this the
|
||||
// bridge-factory unit tests would still pass while App stopped supplying it.
|
||||
const appBridgeFactoryDeps: AppBridgeFactoryDeps[] = [];
|
||||
vi.mock(
|
||||
"./components/elements/AppRenderer/createAppBridgeFactory",
|
||||
async (importOriginal) => {
|
||||
const actual =
|
||||
await importOriginal<
|
||||
typeof import("./components/elements/AppRenderer/createAppBridgeFactory")
|
||||
>();
|
||||
return {
|
||||
...actual,
|
||||
createAppBridgeFactory: (deps: AppBridgeFactoryDeps) => {
|
||||
appBridgeFactoryDeps.push(deps);
|
||||
return actual.createAppBridgeFactory(deps);
|
||||
},
|
||||
};
|
||||
},
|
||||
);
|
||||
|
||||
// --- InspectorView double ---------------------------------------------------
|
||||
// Surfaces each piece of session-scoped state under test and exposes buttons
|
||||
// that invoke the App's connect / call-tool / get-prompt / read-resource /
|
||||
|
|
@ -617,6 +640,10 @@ import type {
|
|||
MessageEntry,
|
||||
ServerEntry,
|
||||
} from "@inspector/core/mcp/types.js";
|
||||
import type { AppBridgeFactoryDeps } from "./components/elements/AppRenderer/createAppBridgeFactory";
|
||||
import { useManagedResources } from "@inspector/core/react/useManagedResources.js";
|
||||
import type { UseManagedResourcesResult } from "@inspector/core/react/useManagedResources.js";
|
||||
import type { Resource } from "@modelcontextprotocol/client";
|
||||
|
||||
// Default useInspectorClient return — capabilities empty (no task tool calls).
|
||||
// Individual tests override via vi.mocked(...).mockReturnValue(...).
|
||||
|
|
@ -3374,3 +3401,66 @@ describe("App background command rejections (#2049)", () => {
|
|||
}
|
||||
});
|
||||
});
|
||||
|
||||
/** A complete `useManagedResources` return, so the mock stays type-checked against the hook's contract. */
|
||||
function managedResourcesResult(
|
||||
resources: Resource[],
|
||||
): UseManagedResourcesResult {
|
||||
return {
|
||||
error: null,
|
||||
resources,
|
||||
listChanged: false,
|
||||
refresh: vi.fn().mockResolvedValue(resources),
|
||||
clearListChanged: vi.fn(),
|
||||
};
|
||||
}
|
||||
|
||||
describe("App MCP App listed-resource metadata wiring (#2055)", () => {
|
||||
beforeEach(() => {
|
||||
appBridgeFactoryDeps.length = 0;
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
// The hook mock is module-level and shared, so put the empty-list default
|
||||
// back rather than leaving a populated list for whatever runs next.
|
||||
vi.mocked(useManagedResources).mockReturnValue(managedResourcesResult([]));
|
||||
});
|
||||
|
||||
it("hands the bridge factory a getListedResourceMeta reading the resources/list entries", async () => {
|
||||
// ext-apps treats a listing entry's `_meta.ui` as the static default for a
|
||||
// UI resource, so the App has to surface the listing to the bridge — the
|
||||
// factory's own tests inject the dep and cannot see this wiring break.
|
||||
const listedMeta = {
|
||||
ui: { csp: { connectDomains: ["https://api.example.com"] } },
|
||||
};
|
||||
vi.mocked(useManagedResources).mockReturnValue(
|
||||
managedResourcesResult([
|
||||
{ uri: "ui://weather/app.html", name: "app", _meta: listedMeta },
|
||||
]),
|
||||
);
|
||||
|
||||
renderWithMantine(<App />);
|
||||
|
||||
// App builds two factories, differing only in `advertiseElicitation`. Wait
|
||||
// for BOTH by that flag rather than for a count: waiting on "at least one"
|
||||
// would still pass with the wiring stripped from either call site, since
|
||||
// the other's deps sit in the same array.
|
||||
const appsFactory = () =>
|
||||
appBridgeFactoryDeps.find((d) => !d.advertiseElicitation);
|
||||
const elicitationFactory = () =>
|
||||
appBridgeFactoryDeps.find((d) => d.advertiseElicitation === true);
|
||||
await waitFor(() => {
|
||||
expect(appsFactory()).toBeDefined();
|
||||
expect(elicitationFactory()).toBeDefined();
|
||||
});
|
||||
|
||||
for (const deps of [appsFactory(), elicitationFactory()]) {
|
||||
expect(deps?.getListedResourceMeta?.("ui://weather/app.html")).toEqual(
|
||||
listedMeta,
|
||||
);
|
||||
expect(
|
||||
deps?.getListedResourceMeta?.("ui://other/app.html"),
|
||||
).toBeUndefined();
|
||||
}
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -25,6 +25,7 @@ import type {
|
|||
LoggingMessageNotification,
|
||||
Progress,
|
||||
ProgressToken,
|
||||
Resource,
|
||||
Task,
|
||||
Tool,
|
||||
} from "@modelcontextprotocol/client";
|
||||
|
|
@ -787,10 +788,31 @@ 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[]>([]);
|
||||
// Best-effort by construction: this reads the list as it stands when the app
|
||||
// opens, and does not distinguish "no entry for this URI" from "the list
|
||||
// hasn't arrived yet". Both yield no hints, which is the same outcome as
|
||||
// having no listing carrier at all. Blocking the render on list readiness
|
||||
// instead would mean waiting on a request that, for a server advertising no
|
||||
// `resources` capability or whose list errored, never resolves — trading a
|
||||
// missing default for an app that never renders.
|
||||
const getListedResourceMeta = useCallback(
|
||||
(uri: string) =>
|
||||
listedResourcesRef.current.find((r) => r.uri === uri)?._meta,
|
||||
[],
|
||||
);
|
||||
|
||||
const sandboxBridgeFactory = useMemo(
|
||||
() =>
|
||||
createAppBridgeFactory({
|
||||
getClient: () => inspectorClient?.getAppRendererClient() ?? null,
|
||||
getListedResourceMeta,
|
||||
readResource: async (uri) => {
|
||||
if (!inspectorClient) throw new Error("No MCP client connected.");
|
||||
const invocation = await inspectorClient.readResource(uri);
|
||||
|
|
@ -810,7 +832,7 @@ function App() {
|
|||
});
|
||||
},
|
||||
}),
|
||||
[inspectorClient],
|
||||
[inspectorClient, getListedResourceMeta],
|
||||
);
|
||||
|
||||
// App-rendered form elicitations (#1854). The controller is created once and
|
||||
|
|
@ -864,6 +886,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);
|
||||
|
|
@ -880,7 +903,7 @@ function App() {
|
|||
});
|
||||
},
|
||||
}),
|
||||
[inspectorClient],
|
||||
[inspectorClient, getListedResourceMeta],
|
||||
);
|
||||
/**
|
||||
* Close the previous client's session and open one for the client being
|
||||
|
|
@ -1241,6 +1264,14 @@ 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,
|
||||
refresh: refreshTasks,
|
||||
|
|
|
|||
|
|
@ -83,21 +83,37 @@ 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.at` places the bag on the result
|
||||
* envelope (`McpUiReadResourceResult`) instead of the content item, and
|
||||
* `opts.flat` writes it unnested (the non-conforming shape the host ignores).
|
||||
*/
|
||||
function uiResource(
|
||||
text: string | undefined,
|
||||
meta?: Record<string, unknown>,
|
||||
opts: { at?: "content" | "result"; flat?: boolean } = {},
|
||||
): ReadResourceResult {
|
||||
const bag = meta ? { _meta: opts.flat ? meta : { ui: meta } } : {};
|
||||
const onResult = opts.at === "result";
|
||||
return {
|
||||
...(onResult ? bag : {}),
|
||||
contents: [
|
||||
{
|
||||
uri: "ui://weather/app.html",
|
||||
...(text === undefined ? {} : { text }),
|
||||
...(meta ? { _meta: meta } : {}),
|
||||
...(onResult ? {} : bag),
|
||||
},
|
||||
],
|
||||
} 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();
|
||||
|
|
@ -230,6 +246,243 @@ describe("createAppBridgeFactory", () => {
|
|||
});
|
||||
});
|
||||
|
||||
it("falls back to the read result's _meta.ui when the content item carries none", async () => {
|
||||
// `McpUiReadResourceResult` — what a registerAppResource callback returns —
|
||||
// types `_meta.ui` at the envelope level, so a server stamping it there
|
||||
// must have it honored.
|
||||
const readResource = vi.fn().mockResolvedValue(
|
||||
uiResource(
|
||||
"<h1>x</h1>",
|
||||
{
|
||||
permissions: { microphone: {} },
|
||||
csp: { connectDomains: ["https://envelope.example.com"] },
|
||||
},
|
||||
{ at: "result" },
|
||||
),
|
||||
);
|
||||
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;
|
||||
permissions: unknown;
|
||||
};
|
||||
expect(call.permissions).toEqual({ microphone: {} });
|
||||
expect(call.html).toContain("connect-src https://envelope.example.com");
|
||||
});
|
||||
|
||||
it("prefers the read result's _meta.ui over the listing entry's", async () => {
|
||||
// Precedence runs most-specific first: content item, then the result
|
||||
// envelope, then the listing default.
|
||||
const readResource = vi
|
||||
.fn()
|
||||
.mockResolvedValue(
|
||||
uiResource(
|
||||
"<h1>x</h1>",
|
||||
{ csp: { connectDomains: ["https://envelope.example.com"] } },
|
||||
{ at: "result" },
|
||||
),
|
||||
);
|
||||
const factory = createAppBridgeFactory({
|
||||
getClient: () => fakeClient,
|
||||
readResource,
|
||||
getListedResourceMeta: () =>
|
||||
listedMeta({ csp: { connectDomains: ["https://listed.example.com"] } }),
|
||||
});
|
||||
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).toContain("connect-src https://envelope.example.com");
|
||||
expect(call.html).not.toContain("listed.example.com");
|
||||
});
|
||||
|
||||
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 'none'");
|
||||
});
|
||||
|
||||
it("prefers the content item's _meta.ui over both broader carriers", async () => {
|
||||
// Pins the full three-way order — content item > result envelope > listing
|
||||
// — which no single-fallback case can: swapping the first two operands
|
||||
// would still satisfy them.
|
||||
const result = uiResource("<h1>x</h1>", {
|
||||
csp: { connectDomains: ["https://content.example.com"] },
|
||||
});
|
||||
(result as { _meta?: unknown })._meta = {
|
||||
ui: { csp: { connectDomains: ["https://envelope.example.com"] } },
|
||||
};
|
||||
const factory = createAppBridgeFactory({
|
||||
getClient: () => fakeClient,
|
||||
readResource: vi.fn().mockResolvedValue(result),
|
||||
getListedResourceMeta: () =>
|
||||
listedMeta({ csp: { connectDomains: ["https://listed.example.com"] } }),
|
||||
});
|
||||
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).toContain("connect-src https://content.example.com");
|
||||
expect(call.html).not.toContain("envelope.example.com");
|
||||
expect(call.html).not.toContain("listed.example.com");
|
||||
});
|
||||
|
||||
it("stops at a carrier whose _meta.ui is malformed rather than falling through", async () => {
|
||||
// A declared-but-unusable `ui` is that carrier's answer. Falling through
|
||||
// would grant whatever a BROADER carrier asked for — wrong precedence, and
|
||||
// the wrong direction to fail in.
|
||||
const readResource = vi
|
||||
.fn()
|
||||
.mockResolvedValue(
|
||||
uiResource("<h1>x</h1>", { ui: null }, { flat: true }),
|
||||
);
|
||||
const factory = createAppBridgeFactory({
|
||||
getClient: () => fakeClient,
|
||||
readResource,
|
||||
getListedResourceMeta: () =>
|
||||
listedMeta({
|
||||
permissions: { camera: {} },
|
||||
csp: { connectDomains: ["https://listed.example.com"] },
|
||||
}),
|
||||
});
|
||||
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).not.toContain("listed.example.com");
|
||||
expect(call.html).toContain("connect-src 'none'");
|
||||
});
|
||||
|
||||
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 'none'");
|
||||
});
|
||||
|
||||
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
|
||||
|
|
|
|||
|
|
@ -17,6 +17,7 @@ import type {
|
|||
} from "@modelcontextprotocol/client";
|
||||
import {
|
||||
approveCspSources,
|
||||
approveSandboxPermissions,
|
||||
buildSandboxCspPolicy,
|
||||
wrapSandboxedHtml,
|
||||
} from "../../../utils/sandbox-csp";
|
||||
|
|
@ -88,6 +89,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
|
||||
|
|
@ -100,18 +110,65 @@ 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`, …) a single `_meta` bag
|
||||
* declares under its `ui` key, or undefined when that bag declares none.
|
||||
*
|
||||
* The two "no hints" cases are deliberately distinguished by the return type:
|
||||
* a bag with no `ui` key at all returns undefined (this carrier is silent —
|
||||
* keep looking), while a bag whose `ui` is malformed returns `{ meta:
|
||||
* undefined }` (this carrier spoke, and what it said is unusable — stop). Were
|
||||
* they conflated, a content item declaring `ui: null` would fall through to a
|
||||
* broader carrier and be granted whatever *that* one asked for, which is both
|
||||
* the wrong precedence and the wrong direction to fail in.
|
||||
*
|
||||
* `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(
|
||||
bag: object | undefined,
|
||||
): { meta: McpUiResourceMeta | undefined } | undefined {
|
||||
if (bag === undefined || !("ui" in bag)) return undefined;
|
||||
const ui: unknown = (bag as { ui: unknown }).ui;
|
||||
const wellFormed =
|
||||
ui !== null && typeof ui === "object" && !Array.isArray(ui);
|
||||
return { meta: wellFormed ? (ui as McpUiResourceMeta) : undefined };
|
||||
}
|
||||
|
||||
/**
|
||||
* First text content block of a UI resource, plus its `_meta.ui` sandbox hints.
|
||||
*
|
||||
* ext-apps exposes three carriers, and they are consulted most-specific first:
|
||||
*
|
||||
* 1. the **read content item**'s own `_meta.ui`;
|
||||
* 2. the **read result envelope**'s — `McpUiReadResourceResult` (what a
|
||||
* `registerAppResource` callback returns) types `_meta.ui` at that level;
|
||||
* 3. the matching **`resources/list` entry**'s, which `McpUiAppResourceConfig`
|
||||
* documents as the static default a host reviews at connection time, and
|
||||
* which a read content item explicitly takes precedence over.
|
||||
*
|
||||
* The first carrier that *declares* a `ui` key decides, and its value is used
|
||||
* outright — the levels are not merged, since a server that restates only
|
||||
* `csp` at the more specific level means that to be the whole grant, not a
|
||||
* patch over the broader one. A carrier declaring a malformed `ui` decides
|
||||
* too, for "no hints"; see {@link uiMeta}.
|
||||
*/
|
||||
function extractHtmlAndMeta(
|
||||
result: ReadResourceResult,
|
||||
listedMeta?: object,
|
||||
): {
|
||||
html: string;
|
||||
meta: McpUiResourceMeta | undefined;
|
||||
} {
|
||||
for (const content of result.contents) {
|
||||
const text = (content as { text?: unknown }).text;
|
||||
if (typeof text === "string") {
|
||||
return {
|
||||
html: text,
|
||||
meta: content._meta as McpUiResourceMeta | undefined,
|
||||
};
|
||||
const carried =
|
||||
uiMeta(content._meta) ?? uiMeta(result._meta) ?? uiMeta(listedMeta);
|
||||
return { html: text, meta: carried?.meta };
|
||||
}
|
||||
}
|
||||
throw new Error("UI resource has no text (HTML) content");
|
||||
|
|
@ -271,7 +328,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
|
||||
|
|
@ -283,23 +343,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 <meta> 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 <meta> 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));
|
||||
|
|
|
|||
|
|
@ -1307,7 +1307,7 @@ describe("useServers", () => {
|
|||
result.current.updateServerSettings(existing, {
|
||||
headers: [],
|
||||
env: [],
|
||||
metadata: [],
|
||||
metadata: {},
|
||||
roots: [],
|
||||
connectionTimeout: 5000,
|
||||
requestTimeout: 30000,
|
||||
|
|
|
|||
|
|
@ -2,6 +2,7 @@ import { describe, expect, it, vi } from "vitest";
|
|||
import {
|
||||
SAFE_CSP_SOURCE,
|
||||
approveCspSources,
|
||||
approveSandboxPermissions,
|
||||
buildSandboxCspPolicy,
|
||||
escapeHtmlAttr,
|
||||
wrapSandboxedHtml,
|
||||
|
|
@ -81,6 +82,62 @@ describe("approveCspSources", () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe("approveSandboxPermissions", () => {
|
||||
it("keeps the spec's empty-object marker for each known key", () => {
|
||||
expect(
|
||||
approveSandboxPermissions({ camera: {}, clipboardWrite: {} }),
|
||||
).toEqual({ camera: {}, clipboardWrite: {} });
|
||||
});
|
||||
|
||||
it("drops a truthy non-marker the proxy would have honored", () => {
|
||||
// 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. `true` is rejected too: it is not a marker the
|
||||
// spec defines, and guessing in the granting direction is the wrong bet.
|
||||
const warn = vi.spyOn(console, "warn").mockImplementation(() => {});
|
||||
expect(
|
||||
approveSandboxPermissions({
|
||||
camera: "false",
|
||||
microphone: true,
|
||||
geolocation: 1,
|
||||
clipboardWrite: [],
|
||||
}),
|
||||
).toBeUndefined();
|
||||
expect(warn).toHaveBeenCalledTimes(4);
|
||||
warn.mockRestore();
|
||||
});
|
||||
|
||||
it("accepts a non-empty object, since `{}` is the spec's extension point", () => {
|
||||
// The key's presence is the request; a field the host doesn't know can't
|
||||
// narrow it, and rejecting it would drop grants from servers written
|
||||
// against a newer spec. The value is normalized to `{}` on the way out.
|
||||
expect(approveSandboxPermissions({ camera: { enabled: false } })).toEqual({
|
||||
camera: {},
|
||||
});
|
||||
});
|
||||
|
||||
it("drops `false` without warning, and ignores unknown keys", () => {
|
||||
const warn = vi.spyOn(console, "warn").mockImplementation(() => {});
|
||||
expect(
|
||||
approveSandboxPermissions({
|
||||
camera: false,
|
||||
midi: {},
|
||||
clipboardWrite: {},
|
||||
}),
|
||||
).toEqual({ clipboardWrite: {} });
|
||||
// `false` is an explicit refusal, not a malformed value.
|
||||
expect(warn).not.toHaveBeenCalled();
|
||||
warn.mockRestore();
|
||||
});
|
||||
|
||||
it("returns undefined when there is nothing to grant", () => {
|
||||
expect(approveSandboxPermissions(undefined)).toBeUndefined();
|
||||
expect(approveSandboxPermissions(null)).toBeUndefined();
|
||||
expect(approveSandboxPermissions("camera")).toBeUndefined();
|
||||
expect(approveSandboxPermissions({})).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
describe("buildSandboxCspPolicy", () => {
|
||||
it("emits locked-down defaults for an empty approved csp", () => {
|
||||
const policy = buildSandboxCspPolicy({});
|
||||
|
|
|
|||
|
|
@ -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,81 @@ 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 permissions bag 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-marker 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 of no declared type, hence
|
||||
* the `unknown` parameter: this is the boundary that decides what the shape is,
|
||||
* not a consumer of an already-validated one.
|
||||
*
|
||||
* A key counts as requested only when its value is an **object** — the marker
|
||||
* {@link McpUiResourcePermissions} defines for every permission is `{}`, and
|
||||
* presence of that object is the request. Everything else fails closed:
|
||||
* `true`, `false`, any string, `null`, a number, an array, and any key outside
|
||||
* the four the proxy knows. `true` in particular looks like a harmless
|
||||
* shorthand, but honoring an undocumented one means a server typo turns an
|
||||
* iframe permission ON, which is the wrong direction to guess in.
|
||||
*
|
||||
* The object is not required to be *empty*. `{}` is a type with no fields yet,
|
||||
* not a promise there will never be any — it is precisely the spec's extension
|
||||
* point, so rejecting `{ camera: { … } }` would fail closed the day a
|
||||
* permission gains an option, silently dropping grants from servers written
|
||||
* against the newer spec. The value is normalized to `{}` on the way out
|
||||
* either way, so an unrecognized field can't reach the proxy; what it cannot
|
||||
* do is *narrow* a grant the key's presence already asked for.
|
||||
*
|
||||
* 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: unknown,
|
||||
): McpUiResourcePermissions | undefined {
|
||||
if (typeof permissions !== "object" || permissions === null) return undefined;
|
||||
const bag: Record<string, unknown> = { ...permissions };
|
||||
const approved: McpUiResourcePermissions = {};
|
||||
let granted = false;
|
||||
for (const key of PERMISSION_KEYS) {
|
||||
const requested = bag[key];
|
||||
if (requested === undefined || requested === false) continue;
|
||||
if (
|
||||
typeof requested === "object" &&
|
||||
requested !== null &&
|
||||
!Array.isArray(requested)
|
||||
) {
|
||||
approved[key] = {};
|
||||
granted = true;
|
||||
} else {
|
||||
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;
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue