* refactor(web): sharpen src/lib vs src/utils rule and move misfiled modules (#1776) Codify the empirical rule the web client already followed loosely — utils = pure functions that compute; lib = things that instantiate, adapt, or touch the environment — in AGENTS.md and clients/web/README.md, and move the modules that sat on the wrong side of it. Moves (source + co-located test): - lib/sandbox-csp -> utils (pure CSP-string construction; type-only import) - utils/browserTabVisibility -> lib (registers visibilitychange listeners) - utils/oauthResume -> lib (reads/writes sessionStorage) - utils/clearServerOAuthState -> lib (drives live InspectorClient / OAuthStorage) Borderline audits (kept in place): pendingReauth (pure type declarations), deepLink / correlateTransportErrors / mcpNetworkHeaders / oauthUx / oauthFlow / oauthNetworkPhase (pure transforms / type-only core imports) stay in utils; downloadFile stays whole in lib (dominant side is DOM side effects). Also relocate the lone src/test/lib/downloadFile.test.ts straggler beside its source at src/lib/downloadFile.test.ts, and document the test-placement rule (side-by-side by default; src/test/ only for core/ tests, the integration project, and shared scaffolding). Pure move — no behavior change; no build/coverage config keys off the boundary. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * refactor(web): address review — fix stale paths, move customHeaders, one-way arrows (#1776) Responds to the @claude review on PR #1784: 1. Stale path references (fixed): update the four in-repo pointers left at pre-move paths — specification/v2_auth_mid_session.md (oauthResume, browserTabVisibility → lib/), and the two load-bearing sandbox security comments (sandbox_proxy.html, sandbox-controller.ts → src/utils/sandbox-csp). 2. lib/types/customHeaders (moved): it's pure (types + pure transforms), the rule's clearest counterexample. Moved lib/types/customHeaders.{ts,test.ts} → utils/ (no external importers; only its own test) and removed the undocumented lib/types/ subdir. Documented that utils/ is the home for pure shared domain types + their constructors — no separate types/ bucket. 3. One-way arrows (item 3): declare OAuthResumeAuthKind in utils/pendingReauth and re-export it from lib/oauthResume, so the only cross-directory arrow now points lib → utils. Codified that invariant in AGENTS.md. Wording nit: AGENTS.md + README now state diagnostic logging (console.warn/error) doesn't count as a side effect for the utils rule. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * docs: address review round 2 — stale customHeaders refs, rule wording (#1776) Doc-only follow-up to the round-2 @claude review on PR #1784 (no code changed): 1. Stale paths (same class as round-1 item 1): the customHeaders move left three references to the deleted lib/types/customHeaders.ts path in specification/ (v2_ux_interfaces_phase_0_4_inventory.md, v2_ux_interfaces_plan.md ×2) — updated to utils/customHeaders.ts. 2. customHeaders anchor: note in AGENTS.md that its shape is staged for ServerSettingsForm (per v2_ux_interfaces_plan.md), so its zero-importer state reads as intentional rather than rot; kept as the no-types/-bucket illustration and led the anchor list with load-bearing modules. 3. Type-only-import clause: split the conflated clause — a type-only import (pendingReauth) and re-exporting core's pure functions/constants (oauthUx/oauthFlow) are distinct reasons to stay in utils; what makes a module lib is wrapping core's stateful runtime. Fixed in both AGENTS.md and the web README. Cosmetic: re-padded the v2_auth_mid_session.md table row to column width. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * docs: address review round 3 — scope the "no types/ bucket" claim (#1776) Doc-only follow-up to the round-3 @claude review on PR #1784 (no code changed): 1. "no separate types/ bucket" overreached: clients/web/src/types/ does exist (navigation.ts + react-syntax-highlighter .d.ts ambient stubs wired via tsconfig.app.json paths). Scoped the claim in both AGENTS.md and the web README to "no types/ sub-bucket INSIDE lib/utils", and noted src/types/ is for ambient .d.ts stubs, not domain types. 2. README example list drifted from AGENTS.md's round-2 load-bearing anchors — realigned it (inspectorTabs/mcpNetworkHeaders, dropped the zero-importer customHeaders from the headline list). 3. Clarified v2_ux_interfaces_plan.md:185 so the v2 path reads as the copy's destination, not as the v1.5 source location. Follow-up filed as #1785 (board Todo): src/types/navigation.ts InspectorTab is dead and duplicates utils/inspectorTabs.ts — kept out of this pure-move PR. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * docs: address review round 4 — reconcile src/types/ claim with #1785 (#1776) Doc-only follow-up to the round-4 @claude review on PR #1784 (no code changed): The round-3 scope fix overshot: it asserted src/types/ holds "not domain types at all", but the dead navigation.ts InspectorTab is a plain-.ts domain type still living there, and specification/v2_ux_interfaces_plan.md:181-182 actively prescribes placing InspectorTab there — so AGENTS.md and that spec doc gave opposite guidance about the same directory. - AGENTS.md + web README: softened to "not the place for new domain types" and added a parenthetical that the one lingering domain type (dead navigation.ts) is tracked for removal in #1785; until then a pure domain type belongs in utils/, not src/types/. - v2_ux_interfaces_plan.md:181: marked the tab-enum placement guidance as superseded by #1785 (live type is InspectorTabId in utils/inspectorTabs.ts). Self-heals on merge of #1785. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * docs: address review round 5 — fix garbled #1785 clause, restructure utils rule (#1776) Doc-only follow-up to the round-5 @claude review on PR #1784 (no code changed): 1. Grammar (item 1): the round-4 #1785 clause read "superseding [#1785] will remove it" — a participle with no subject. Reworded to "the dead navigation.ts InspectorTab, which #1785 will remove". 2. Readability (item 2): the src/utils/ bullet had accreted into a single ~1,400-char blob across five review rounds. Promoted the carve-outs to scannable sub-bullets (Domain types / Diagnostic logging / Importing from core) and lifted the src/types/ note out to its own line, since it's a statement about a sibling directory, not about utils. AGENTS.md only; the README's parallel sentence was already clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * docs+config: address review round 6 — correct the coverage-glob claim (#1776) Responds to the round-6 @claude review on PR #1784: 1. "no build/coverage glob" was factually wrong (should-fix): the web coverage include in clients/web/vite.config.ts DOES enumerate src/lib/** and src/utils/**. The conclusion (a move between them is coverage-neutral) survives, but the reason was wrong. Reworded both AGENTS.md and the web README to say nothing *enforces* the boundary — no path alias, and the include lists both dirs so a move is coverage-neutral — plus the important whitelist caveat: a module outside components/lib/utils/server falls out of the ≥90 gate entirely. 2. Removed a pre-existing duplicate glob in vite.config.ts (src/lib/**/*.{ts,tsx} appeared twice, lines 161 and 163). Inert (vitest de-dupes) but it sat three lines from the config the doc now describes. Nits: gave the README bullet the same round-5 sub-bullet restructure as AGENTS.md, and made its #1785 reference a real markdown link (bare #N doesn't autolink in rendered repo markdown). npm run ci green (config change re-verified through coverage + smokes + Storybook). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
146 lines
6.4 KiB
TypeScript
146 lines
6.4 KiB
TypeScript
import type { McpUiResourceCsp } from "@modelcontextprotocol/ext-apps/app-bridge";
|
|
|
|
/**
|
|
* Allowed shapes for a CSP source-expression supplied by an app's
|
|
* `_meta.ui.csp`. Each entry is server-supplied and untrusted: it MUST NOT
|
|
* inject extra directives (`;`) or break out of the meta attribute
|
|
* (`"`, `<`, `>`). Only common source forms are accepted —
|
|
* `scheme://host[:port][/path]`, scheme-only (`data:`, `blob:`), `*`, and
|
|
* wildcard hosts (`*.example.com`, `https://*.example.com`); anything else is
|
|
* dropped by {@link approveCspSources}.
|
|
*/
|
|
export const SAFE_CSP_SOURCE =
|
|
/^(?:\*|[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])?)$/;
|
|
|
|
/**
|
|
* 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",
|
|
]);
|
|
|
|
/**
|
|
* 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.
|
|
*
|
|
* 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,
|
|
): McpUiResourceCsp {
|
|
const approved: McpUiResourceCsp = {};
|
|
if (!csp) return approved;
|
|
for (const key of CSP_KEYS) {
|
|
const requested = csp[key];
|
|
if (!Array.isArray(requested)) continue;
|
|
const accepted: string[] = [];
|
|
for (const entry of requested) {
|
|
if (typeof entry === "string" && SAFE_CSP_SOURCE.test(entry)) {
|
|
accepted.push(entry);
|
|
} else {
|
|
console.warn("[mcp-app sandbox] dropping unsafe CSP source:", entry);
|
|
}
|
|
}
|
|
if (accepted.length > 0) approved[key] = accepted;
|
|
}
|
|
return approved;
|
|
}
|
|
|
|
function joinSources(list: string[] | undefined, fallback: string): string {
|
|
return list && list.length > 0 ? list.join(" ") : fallback;
|
|
}
|
|
|
|
/**
|
|
* Translate an approved {@link McpUiResourceCsp} into the Content-Security-Policy
|
|
* string enforced on the inner sandboxed document. `default-src 'none'` is the
|
|
* catch-all so any fetch type not explicitly mapped is denied. `script-src` /
|
|
* `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'");
|
|
const inlineResource =
|
|
resourceSrc === "'none'"
|
|
? "'unsafe-inline'"
|
|
: `'unsafe-inline' ${resourceSrc}`;
|
|
return [
|
|
"default-src 'none'",
|
|
`connect-src ${joinSources(approved.connectDomains, "'none'")}`,
|
|
`script-src ${inlineResource}`,
|
|
`style-src ${inlineResource}`,
|
|
`img-src ${resourceSrc}`,
|
|
`font-src ${resourceSrc}`,
|
|
`media-src ${resourceSrc}`,
|
|
`frame-src ${joinSources(approved.frameDomains, "'none'")}`,
|
|
`base-uri ${joinSources(approved.baseUriDomains, "'self'")}`,
|
|
"form-action 'none'",
|
|
"object-src 'none'",
|
|
"worker-src 'none'",
|
|
].join("; ");
|
|
}
|
|
|
|
/** HTML-attribute-encode a string (defense-in-depth for the CSP meta value). */
|
|
export function escapeHtmlAttr(s: string): string {
|
|
return s
|
|
.replace(/&/g, "&")
|
|
.replace(/"/g, """)
|
|
.replace(/'/g, "'")
|
|
.replace(/</g, "<")
|
|
.replace(/>/g, ">");
|
|
}
|
|
|
|
/**
|
|
* Wrap an app's untrusted HTML in a host-authored document whose first
|
|
* `<head>` child is the CSP `<meta>`. The wrapper bytes are fixed — the
|
|
* untrusted content lands inside `<body>` and never precedes the policy, so a
|
|
* `<head>`/`<!-- -->` token in the app's HTML cannot push the meta inert or
|
|
* load resources before the policy applies. If the app's HTML is itself a full
|
|
* document, the second `<!doctype>`/`<html>`/`<head>` are parsed inside
|
|
* `<body>` (the HTML parser ignores duplicate document-structure tags) while
|
|
* its scripts and styles still run — governed by the already-applied policy.
|
|
*/
|
|
export function wrapSandboxedHtml(
|
|
untrustedHtml: string,
|
|
policy: string,
|
|
): string {
|
|
const meta = `<meta http-equiv="Content-Security-Policy" content="${escapeHtmlAttr(policy)}">`;
|
|
return `<!DOCTYPE html><html><head>${meta}<meta charset="utf-8"></head><body>${untrustedHtml}</body></html>`;
|
|
}
|