Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
23 changes: 23 additions & 0 deletions clients/web/src/utils/sandbox-csp.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,29 @@ describe("approveCspSources", () => {
warn.mockRestore();
});

it("names the offending field and states the expected shape, without calling the value unsafe", () => {
// #2064: the filter screens for injection safety only, so the message must
// not read as a verdict on the value. These four fields are origin lists,
// so the real cause is almost always "not an origin" — a CSP keyword has
// no field in the contract to be requested through. Naming the key also
// tells an app declaring several lists which one the bad entry was in.
const warn = vi.spyOn(console, "warn").mockImplementation(() => {});
approveCspSources({
connectDomains: ["'unsafe-eval'"],
frameDomains: ["<script>"],
});
const messages = warn.mock.calls.map(([message]) => String(message));
expect(messages).toHaveLength(2);
expect(messages[0]).toContain('"connectDomains"');
expect(messages[1]).toContain('"frameDomains"');
for (const message of messages) {
expect(message).toContain("expected an origin");
expect(message).not.toContain("unsafe");
}
expect(warn.mock.calls[0]?.[1]).toBe("'unsafe-eval'");
warn.mockRestore();
});

it("omits a key when every entry is rejected", () => {
const warn = vi.spyOn(console, "warn").mockImplementation(() => {});
expect(approveCspSources({ connectDomains: ['"; x'] })).toEqual({});
Expand Down
25 changes: 19 additions & 6 deletions clients/web/src/utils/sandbox-csp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,11 @@ const CSP_KEYS = exhaustiveCspKeys([

/**
* Filter an app-supplied {@link McpUiResourceCsp} down to the entries the host
* will actually enforce. Unsafe values are dropped (and warned), and the
* resulting object contains only keys with at least one accepted source. The
* return value is what the host echoes back to the view via
* `hostCapabilities.sandbox.csp` so the app sees what was granted, not what it
* asked for.
* will actually enforce. Entries that don't parse as a source expression are
* dropped (and warned), and the resulting object contains only keys with at
Comment thread
cliffhall marked this conversation as resolved.
Outdated
* least one accepted source. The return value is what the host echoes back to
* the view via `hostCapabilities.sandbox.csp` so the app sees what was granted,
* not what it asked for.
*
* NOTE: this screens each source for *injection safety* only — it does NOT
* bound the *breadth* of a grant. A bare `*` (and a scheme-wildcard host) is a
Expand All @@ -56,6 +56,14 @@ const CSP_KEYS = exhaustiveCspKeys([
* ambient credentials and nothing to exfiltrate beyond what the app already
* received; "approved" therefore means "cannot break out of the meta
* attribute," not "restrictive."
*
* The drop warning is worded accordingly: it names the field the entry came
* from and states the shape expected, and makes no claim about the *safety* of
* the value. These four fields are origin lists, so the usual cause of a drop
* is an entry that simply isn't an origin — a CSP keyword such as
* `'unsafe-eval'` has no field in the contract to be requested through. Saying
* "unsafe" instead read as a security verdict this check never makes, and sent
* #2012 chasing an XSS regression that wasn't one (#2064).
*/
export function approveCspSources(
csp: McpUiResourceCsp | undefined,
Expand All @@ -70,7 +78,12 @@ export function approveCspSources(
if (typeof entry === "string" && SAFE_CSP_SOURCE.test(entry)) {
accepted.push(entry);
} else {
console.warn("[mcp-app sandbox] dropping unsafe CSP source:", entry);
console.warn(
`[mcp-app sandbox] dropping "${key}" entry (expected an origin such ` +
`as https://example.com or https://*.example.com, a scheme such as ` +
`data:, or *):`,
entry,
);
}
}
if (accepted.length > 0) approved[key] = accepted;
Expand Down