feat(web): add sandbox CSP builder library (closes #1558) by cliffhall · Pull Request #1588 · modelcontextprotocol/inspector

cliffhall · 2026-07-01T22:23:33Z

Wave 1 of the PR #1510 decomposition (tracking issue #1579). Adds a pure, DOM-free CSP-builder library at clients/web/src/lib/sandbox-csp.ts that the MCP Apps sandbox will use to enforce app-supplied _meta.ui.csp requests. Wiring into the app bridge is a follow-up issue — this PR is the standalone, unit-testable library only.

What it does

SAFE_CSP_SOURCE — strict regex accepting only common source forms (scheme://host[:port][/path], scheme-only like data:/blob:, *, wildcard hosts). Rejects anything that could inject a directive (;) or break out of the meta attribute (", <, >, whitespace).
approveCspSources() — filters an untrusted McpUiResourceCsp down to enforceable entries, dropping unsafe values with a console.warn and omitting keys with no accepted source. Returns what the host will echo back to the view (granted, not requested).
buildSandboxCspPolicy() — translates the approved CSP into the enforced policy string: default-src 'none' catch-all, form-action/object-src/worker-src 'none', per-directive source allowlists, 'unsafe-inline' for the app's own inline script/style.
escapeHtmlAttr() / wrapSandboxedHtml() — defense-in-depth wrapper so the CSP <meta> is always the literal first <head> child and untrusted bytes never precede the applied policy.

Testing

Full unit test suite (sandbox-csp.test.ts) covering defaults, source filtering, escaping, and the wrapper ordering guarantees.
100% lines / statements / functions / branches for sandbox-csp.ts.
The file is added to the web coverage include list (by filename, so the untested legacy src/lib siblings are not pulled under the gate) so it is genuinely gated at ≥90.
npm run validate (web) and npm run test:coverage (web) both pass.

🤖 Generated with Claude Code

https://claude.ai/code/session_01S3fTN8H3R8YV4yUGvZjYnX

cliffhall · 2026-07-01T22:52:26Z

@claude review

claude · 2026-07-01T22:52:43Z

Claude finished @cliffhall's task in 2m 31s —— View job

Review: sandbox CSP builder library

Nice, self-contained PR. The library is pure, well-documented, and the tests genuinely exercise the security-relevant behavior (injection rejection, escaping, wrapper ordering, the window.parent invariant). I verified the wrapper claims against static/sandbox_proxy.html:154-167 — the output really is written via document.write into the inner iframe, so the "no host-authored <iframe>" test is protecting a real invariant. Below are findings, roughly by importance.

1. `resourceDomains` is granted `script-src` — confirm this is intended

buildSandboxCspPolicy routes resourceDomains into script-src and style-src (via inlineResource) as well as img-src/font-src/media-src (sandbox-csp.ts:63-82). That means an app that requests a domain only to load images or fonts from a CDN also gets script execution from that origin — the most powerful grant CSP hands out. If the app-bridge spec intends resourceDomains to cover scripts, this is correct and worth a one-line comment saying so explicitly; if resourceDomains is meant to be passive subresources only, scripts should be gated behind a narrower key (or script-src should stay 'unsafe-inline'-only with no host allowlist). Worth confirming against the McpUiResourceCsp contract before wiring.

Relatedly, SAFE_CSP_SOURCE accepts a bare *, so resourceDomains: ["*"] → script-src 'unsafe-inline' * (scripts from anywhere) and connectDomains: ["*"] → connect-src * (open exfil channel). approveCspSources only screens for injection safety, not for breadth — which may be fine given the opaque-origin sandbox, but it's a policy decision worth stating in the doc comment so a future reader doesn't assume approve implies "restrictive."

2. `CSP_KEYS` isn't exhaustive over `McpUiResourceCsp`

const CSP_KEYS = [ ... ] as const satisfies readonly (keyof McpUiResourceCsp)[];

satisfies readonly (keyof …)[] guarantees every listed key is valid, but not that every key of McpUiResourceCsp is listed. If the ext-apps type later gains a domain key (say mediaDomains), it will be silently dropped by approveCspSources with no compile error — a quiet security-relevant gap (a requested restriction just vanishes). Consider a compile-time exhaustiveness assertion so adding a key to the upstream type forces a change here. Fix this →

3. Minor: scheme matching is lowercase-only

The scheme alternative is [a-z][a-z0-9+.-]*:, so a syntactically valid but upper/mixed-case scheme (HTTPS://a.com) is rejected. Schemes are case-insensitive per the URL spec; this only over-rejects (no security impact), but it's a small correctness wart if a server sends a capitalized scheme. Low priority.

4. Minor: CSP `<meta>` precedes `<meta charset>`

In wrapSandboxedHtml the CSP meta is emitted before <meta charset="utf-8"> (sandbox-csp.ts:109-110). Putting the policy first is the right security call. Just be aware that if an approved policy ever grows long enough to push the charset declaration past the first 1024 bytes, the browser ignores the charset. Policies here are short ASCII, so this is theoretical — noting for completeness, no change needed.

Things that are correct and worth calling out

Injection surface is properly closed: ;, whitespace, ", <, > are all outside every character class, and keyword sources ('unsafe-eval', etc.) can't be smuggled because no alternative starts with '. The reject test cases cover the important ones.
The regex has no nested-quantifier ReDoS risk on the short inputs it sees.
base-uri 'self' default resolves to the sandbox's opaque origin, effectively blocking <base> — a safe default.
Coverage include-by-filename (rather than a src/lib/** glob) is the right call to avoid dragging untested siblings under the gate; the comment explains it well.

Nothing here is a blocker — items 1 and 2 are the two I'd resolve (or explicitly document) before this gets wired into the bridge.
· branch v2/1558-sandbox-csp

@claude

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

cliffhall · 2026-07-01T23:17:41Z

Thanks for the review — addressed in cb54de5.

1. resourceDomains → script-src / bare * breadth — Confirmed intended. The McpUiResourceCsp contract defines resourceDomains as a single "static resources" allowlist that "Maps to CSP img-src, script-src, style-src, font-src, media-src directives," so granting script execution from a listed origin is by design (there is no narrower per-directive key). Documented that explicitly on buildSandboxCspPolicy. Also documented on approveCspSources that it screens injection safety only, not breadth — a bare * is an approved source, which is acceptable under the opaque-origin, credential-free sandbox; "approved" means "cannot break out of the meta attribute," not "restrictive."

2. CSP_KEYS exhaustiveness — Fixed. Replaced the satisfies readonly (keyof McpUiResourceCsp)[] (which only proves each listed key is valid) with an exhaustiveCspKeys() identity helper whose parameter type collapses to never if keyof McpUiResourceCsp ever stops extending the listed union. A new upstream domain key now fails to compile here instead of being silently dropped.

3. Case-sensitive scheme — Fixed. SAFE_CSP_SOURCE now accepts case-insensitive schemes ([a-zA-Z][a-zA-Z0-9+.-]*); previously HTTPS://… was over-rejected. Added accept-tests for capitalized/mixed-case schemes.

4. CSP <meta> before <meta charset> — No change (as you noted): policy-first is the right security ordering and policies here are short ASCII, well under the 1024-byte charset window.

No behavior change to the enforced policy string. validate + test:coverage pass; sandbox-csp.ts remains at 100% lines/statements/functions/branches.

cliffhall · 2026-07-01T23:17:48Z

@claude review

claude · 2026-07-01T23:18:07Z

Claude finished @cliffhall's task in 1m 25s —— View job

Re-review: fixes in `cb54de50`

Went back through the diff against the four points from the last pass. All landed, and correctly:

1. resourceDomains → script-src / bare-* breadth — Resolved as documentation. The doc comment on buildSandboxCspPolicy (sandbox-csp.ts:89-96) now states the resourceDomains → script-src/style-src mapping is by design per the McpUiResourceCsp contract, and approveCspSources (sandbox-csp.ts:47-55) spells out that it screens injection-safety only, not breadth, and why that's acceptable under the opaque-origin sandbox. That's exactly the "state the policy decision so a future reader doesn't misread approve" ask. ✅

2. CSP_KEYS exhaustiveness — Fixed properly (sandbox-csp.ts:26-37). The exhaustiveCspKeys identity helper's parameter type T & (keyof McpUiResourceCsp extends T[number] ? unknown : never) collapses to never when a key is missing, so a new upstream domain key now fails to compile at the call site instead of being silently dropped. This is a stronger guarantee than the old satisfies and reads clearly with the comment. ✅

3. Case-insensitive schemes — Fixed (sandbox-csp.ts:13); the scheme class is now [a-zA-Z][a-zA-Z0-9+.-]* and the accept-tests cover HTTPS://…, WSS://…, Https://*…. ✅

4. CSP <meta> before <meta charset> — Correctly left as-is (policy-first ordering, short ASCII policies). ✅

Injection surface is unchanged and still closed (;, whitespace, ", <, > excluded from every class), and no behavior change to the enforced policy string.

One informational nit (not a blocker)

With the scheme now case-insensitive, SAFE_CSP_SOURCE accepts any bare scheme: scheme-only form, including javascript: — so resourceDomains: ["javascript:"] is "approved" and yields script-src 'unsafe-inline' javascript:. This is the same breadth-not-restrictiveness class you already documented for *, and 'unsafe-inline' is present regardless, so it changes little in practice under the opaque-origin sandbox. If you ever want to narrow breadth (a follow-up, not this PR), an allowlist of scheme-only forms (data:, blob:, https:, wss:) would be the natural knob — but as scoped here (injection-safety only), this is consistent and fine to ship. Noting for the record only.

Nothing outstanding. LGTM. 👍

Read sandbox-csp.ts + tests
Verify the four fixes landed correctly
Check for any remaining issues
Post review
· branch v2/1558-sandbox-csp

cliffhall · 2026-07-02T02:55:15Z

📋 Smoke test & AGENTS.md audit for this PR: #1558 (comment) (posted on the linked issue #1558).

…ase change; tree unchanged)

cliffhall added the v2 Issues and PRs for v2 label Jul 1, 2026

cliffhall linked an issue Jul 1, 2026 that may be closed by this pull request

web: sandbox CSP builder library (lib/sandbox-csp.ts) #1558

Open

cliffhall mentioned this pull request Jul 2, 2026

Wave 1 rollup: integrated 1579-wave-1 (#1600) #1601

Open

chore: retarget #1588 onto 1579-wave-1 rollup (empty commit enables b…

060b64a

…ase change; tree unchanged)

cliffhall changed the base branch from v2/main to 1579-wave-1 July 2, 2026 16:44

cliffhall merged commit 3de93b9 into 1579-wave-1 Jul 2, 2026
1 check passed

cliffhall deleted the v2/1558-sandbox-csp branch July 2, 2026 16:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat(web): add sandbox CSP builder library (closes #1558)#1588

feat(web): add sandbox CSP builder library (closes #1558)#1588
cliffhall merged 1 commit into
1579-wave-1from
v2/1558-sandbox-csp

cliffhall commented Jul 1, 2026

Uh oh!

cliffhall commented Jul 1, 2026

Uh oh!

claude Bot commented Jul 1, 2026 •

edited

Loading

Uh oh!

cliffhall commented Jul 1, 2026

Uh oh!

cliffhall commented Jul 1, 2026

Uh oh!

claude Bot commented Jul 1, 2026 •

edited

Loading

Uh oh!

cliffhall commented Jul 2, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

cliffhall commented Jul 1, 2026

What it does

Testing

Uh oh!

cliffhall commented Jul 1, 2026

Uh oh!

claude Bot commented Jul 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review: sandbox CSP builder library

1. resourceDomains is granted script-src — confirm this is intended

2. CSP_KEYS isn't exhaustive over McpUiResourceCsp

3. Minor: scheme matching is lowercase-only

4. Minor: CSP <meta> precedes <meta charset>

Things that are correct and worth calling out

Uh oh!

cliffhall commented Jul 1, 2026

Uh oh!

cliffhall commented Jul 1, 2026

Uh oh!

claude Bot commented Jul 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Re-review: fixes in cb54de50

One informational nit (not a blocker)

Uh oh!

cliffhall commented Jul 2, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

claude Bot commented Jul 1, 2026 •

edited

Loading

1. `resourceDomains` is granted `script-src` — confirm this is intended

2. `CSP_KEYS` isn't exhaustive over `McpUiResourceCsp`

4. Minor: CSP `<meta>` precedes `<meta charset>`

claude Bot commented Jul 1, 2026 •

edited

Loading

Re-review: fixes in `cb54de50`