Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 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
79 changes: 79 additions & 0 deletions clients/web/src/App.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -355,6 +355,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 /
Expand Down Expand Up @@ -590,6 +613,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(...).
Expand Down Expand Up @@ -3117,3 +3144,55 @@ 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 />);
await waitFor(() => expect(appBridgeFactoryDeps.length).toBeGreaterThan(0));

// Every factory App builds gets the wiring, not just the Apps-tab one.
for (const deps of appBridgeFactoryDeps) {
expect(deps.getListedResourceMeta?.("ui://weather/app.html")).toEqual(
listedMeta,
);
expect(
deps.getListedResourceMeta?.("ui://other/app.html"),
).toBeUndefined();
}
});
});
28 changes: 26 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,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,
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
Loading