refactor(web): document + guard sandbox CSP builder per review
Address round-1 @claude review of #1588 (no behavior change to the enforced policy): - approveCspSources / buildSandboxCspPolicy: document that resourceDomains intentionally feeds script-src/style-src per the McpUiResourceCsp contract, and that approve screens injection-safety only (not breadth — a bare `*` is accepted; safe under the opaque-origin sandbox). - CSP_KEYS: replace the `satisfies` (proves listed keys valid) with an exhaustiveCspKeys() helper that ALSO fails to compile if the upstream ext-apps type gains a domain key CSP_KEYS omits, so a requested restriction can never be silently dropped. - SAFE_CSP_SOURCE: accept case-insensitive schemes (URL schemes are case-insensitive); previously over-rejected `HTTPS://…`. Adds tests. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S3fTN8H3R8YV4yUGvZjYnX
This commit is contained in:
parent
77e57e0d25
commit
cb54de50d8
2 changed files with 44 additions and 3 deletions
|
|
@ -19,6 +19,12 @@ describe("SAFE_CSP_SOURCE", () => {
|
|||
"data:",
|
||||
"blob:",
|
||||
"example.com",
|
||||
// Schemes are case-insensitive per the URL spec, so a capitalized or
|
||||
// mixed-case scheme is still a valid source (over-rejecting it would
|
||||
// silently drop a restriction a server asked for).
|
||||
"HTTPS://api.example.com",
|
||||
"WSS://realtime.service.com",
|
||||
"Https://*.cloudflare.com",
|
||||
])("accepts %s", (s) => {
|
||||
expect(SAFE_CSP_SOURCE.test(s)).toBe(true);
|
||||
});
|
||||
|
|
|
|||
|
|
@ -10,14 +10,31 @@ import type { McpUiResourceCsp } from "@modelcontextprotocol/ext-apps/app-bridge
|
|||
* dropped by {@link approveCspSources}.
|
||||
*/
|
||||
export const SAFE_CSP_SOURCE =
|
||||
/^(?:\*|[a-z][a-z0-9+.-]*:(?:\/\/(?:\*\.)?[A-Za-z0-9._~%!$&'()*+,=@:-]+(?::\d+)?(?:\/[A-Za-z0-9._~%!$&'()*+,=@:/-]*)?)?|(?:\*\.)?[A-Za-z0-9](?:[A-Za-z0-9.-]*[A-Za-z0-9])?)$/;
|
||||
/^(?:\*|[a-zA-Z][a-zA-Z0-9+.-]*:(?:\/\/(?:\*\.)?[A-Za-z0-9._~%!$&'()*+,=@:-]+(?::\d+)?(?:\/[A-Za-z0-9._~%!$&'()*+,=@:/-]*)?)?|(?:\*\.)?[A-Za-z0-9](?:[A-Za-z0-9.-]*[A-Za-z0-9])?)$/;
|
||||
|
||||
const CSP_KEYS = [
|
||||
/**
|
||||
* Identity helper that pins `CSP_KEYS` to an exhaustive, valid key list at
|
||||
* compile time. `satisfies readonly (keyof McpUiResourceCsp)[]` alone only
|
||||
* proves every *listed* key is valid; it does NOT prove the list is *complete*.
|
||||
* The intersected conditional adds the missing half: when `CSP_KEYS` covers
|
||||
* every key, `keyof McpUiResourceCsp extends T[number]` holds and the parameter
|
||||
* type is just `T`; if the upstream ext-apps type ever gains a new domain key,
|
||||
* the conditional collapses the parameter type to `never` and the call fails to
|
||||
* compile — forcing the key to be added here rather than being silently dropped
|
||||
* (a vanished restriction) by {@link approveCspSources}.
|
||||
*/
|
||||
function exhaustiveCspKeys<const T extends readonly (keyof McpUiResourceCsp)[]>(
|
||||
keys: T & (keyof McpUiResourceCsp extends T[number] ? unknown : never),
|
||||
): T {
|
||||
return keys;
|
||||
}
|
||||
|
||||
const CSP_KEYS = exhaustiveCspKeys([
|
||||
"connectDomains",
|
||||
"resourceDomains",
|
||||
"frameDomains",
|
||||
"baseUriDomains",
|
||||
] as const satisfies readonly (keyof McpUiResourceCsp)[];
|
||||
]);
|
||||
|
||||
/**
|
||||
* Filter an app-supplied {@link McpUiResourceCsp} down to the entries the host
|
||||
|
|
@ -26,6 +43,16 @@ const CSP_KEYS = [
|
|||
* 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
|
||||
* syntactically safe source, so `resourceDomains: ["*"]` is "approved" and maps
|
||||
* to `script-src 'unsafe-inline' *` (scripts from anywhere), just as
|
||||
* `connectDomains: ["*"]` maps to `connect-src *`. That breadth is acceptable
|
||||
* here because the enforced document runs in an opaque-origin sandbox with no
|
||||
* ambient credentials and nothing to exfiltrate beyond what the app already
|
||||
* received; "approved" therefore means "cannot break out of the meta
|
||||
* attribute," not "restrictive."
|
||||
*/
|
||||
export function approveCspSources(
|
||||
csp: McpUiResourceCsp | undefined,
|
||||
|
|
@ -59,6 +86,14 @@ function joinSources(list: string[] | undefined, fallback: string): string {
|
|||
* `style-src` carry `'unsafe-inline'` because the app's own inline code ships
|
||||
* with the inline-delivered HTML and has no origin to allowlist; external loads
|
||||
* stay restricted to `resourceDomains`.
|
||||
*
|
||||
* `resourceDomains` intentionally feeds `script-src` (and `style-src`) in
|
||||
* addition to `img-src`/`font-src`/`media-src`: the `McpUiResourceCsp` contract
|
||||
* defines it as a single "static resources" allowlist that "Maps to CSP
|
||||
* `img-src`, `script-src`, `style-src`, `font-src`, `media-src` directives," so
|
||||
* an app that lists a CDN there is granted script execution from that origin by
|
||||
* design. There is no narrower per-directive key in the contract; if the spec
|
||||
* ever splits scripts out, update the mapping here accordingly.
|
||||
*/
|
||||
export function buildSandboxCspPolicy(approved: McpUiResourceCsp): string {
|
||||
const resourceSrc = joinSources(approved.resourceDomains, "'none'");
|
||||
|
|
|
|||
Loading…
Reference in a new issue