Merge branch 'v2/main' into v2/chore/2003-pack-verify-app-render
This commit is contained in:
commit
5e43263b09
2 changed files with 44 additions and 6 deletions
|
|
@ -67,6 +67,29 @@ describe("approveCspSources", () => {
|
||||||
warn.mockRestore();
|
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", () => {
|
it("omits a key when every entry is rejected", () => {
|
||||||
const warn = vi.spyOn(console, "warn").mockImplementation(() => {});
|
const warn = vi.spyOn(console, "warn").mockImplementation(() => {});
|
||||||
expect(approveCspSources({ connectDomains: ['"; x'] })).toEqual({});
|
expect(approveCspSources({ connectDomains: ['"; x'] })).toEqual({});
|
||||||
|
|
|
||||||
|
|
@ -41,11 +41,15 @@ const CSP_KEYS = exhaustiveCspKeys([
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Filter an app-supplied {@link McpUiResourceCsp} down to the entries the host
|
* Filter an app-supplied {@link McpUiResourceCsp} down to the entries the host
|
||||||
* will actually enforce. Unsafe values are dropped (and warned), and the
|
* will actually enforce. The accepted subset is the origin forms
|
||||||
* resulting object contains only keys with at least one accepted source. The
|
* {@link SAFE_CSP_SOURCE} matches — `scheme://host[:port][/path]`, a bare or
|
||||||
* return value is what the host echoes back to the view via
|
* wildcard host, a scheme such as `data:`, and `*`. Anything outside it is
|
||||||
* `hostCapabilities.sandbox.csp` so the app sees what was granted, not what it
|
* dropped (and warned) — including a CSP keyword like `'unsafe-eval'`, which
|
||||||
* asked for.
|
* is a perfectly valid source expression in CSP but not something these
|
||||||
|
* origin-list fields can express. The resulting object contains only keys with
|
||||||
|
* at least one accepted source, and 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
|
* 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
|
* bound the *breadth* of a grant. A bare `*` (and a scheme-wildcard host) is a
|
||||||
|
|
@ -56,6 +60,12 @@ const CSP_KEYS = exhaustiveCspKeys([
|
||||||
* ambient credentials and nothing to exfiltrate beyond what the app already
|
* ambient credentials and nothing to exfiltrate beyond what the app already
|
||||||
* received; "approved" therefore means "cannot break out of the meta
|
* received; "approved" therefore means "cannot break out of the meta
|
||||||
* attribute," not "restrictive."
|
* 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. Saying "unsafe" instead read as a security verdict this check
|
||||||
|
* never makes, and sent #2012 chasing an XSS regression that wasn't one, when
|
||||||
|
* the entry had merely been the wrong kind of thing for the field (#2064).
|
||||||
*/
|
*/
|
||||||
export function approveCspSources(
|
export function approveCspSources(
|
||||||
csp: McpUiResourceCsp | undefined,
|
csp: McpUiResourceCsp | undefined,
|
||||||
|
|
@ -70,7 +80,12 @@ export function approveCspSources(
|
||||||
if (typeof entry === "string" && SAFE_CSP_SOURCE.test(entry)) {
|
if (typeof entry === "string" && SAFE_CSP_SOURCE.test(entry)) {
|
||||||
accepted.push(entry);
|
accepted.push(entry);
|
||||||
} else {
|
} 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;
|
if (accepted.length > 0) approved[key] = accepted;
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue