fix: harden server.json import review, URL elicitation scheme, and preview CSP injection

clients/web/src/components/elements/ContentViewer/ContentViewer.stories.tsx

@@ -1,5 +1,7 @@
 import type { Meta, StoryObj } from "@storybook/react-vite";
+import { expect } from "storybook/test";
 import { ContentViewer } from "./ContentViewer";
+import { PREVIEW_HTML_CSP, wrapHtmlWithCsp } from "./contentViewerUtils";
 
 const meta: Meta<typeof ContentViewer> = {
   title: "Elements/ContentViewer",
@@ -142,6 +144,110 @@ export const HtmlSandboxed: Story = {
   },
 };
 
+// A full document whose first `<head>` is inside a comment — the shape that
+// once pulled the preview CSP into the comment. Parsing the wrapped document
+// in the play function is deliberate: the iframe is `sandbox=""` (an opaque
+// origin the story cannot inspect), and happy-dom's parser does not place a
+// leading `<meta>` the way the HTML spec does, so only a real browser can
+// assert where the policy actually lands.
+const COMMENTED_HEAD_HTML =
+  '<!-- <head> --><html lang="en"><head><title>Report</title></head><body><h1>Report</h1></body></html>';
+
+export const HtmlCommentedHead: Story = {
+  args: {
+    contents: {
+      uri: "file:///commented-head.html",
+      mimeType: "text/html",
+      text: COMMENTED_HEAD_HTML,
+    },
+  },
+  play: async () => {
+    const doc = new DOMParser().parseFromString(
+      wrapHtmlWithCsp(COMMENTED_HEAD_HTML),
+      "text/html",
+    );
+    const csp = doc.head.querySelector(
+      'meta[http-equiv="Content-Security-Policy"]',
+    );
+    await expect(csp).not.toBeNull();
+    // Nothing from the document precedes the policy in the head.
+    await expect(doc.head.firstElementChild).toBe(csp);
+    await expect(doc.title).toBe("Report");
+    await expect(doc.documentElement.getAttribute("lang")).toBe("en");
+    await expect(doc.body.textContent).toBe("Report");
+  },
+};
+
+// A doctype whose quoted SYSTEM / PUBLIC identifier contains `>`. Only the
+// simple `<!DOCTYPE html>` is kept ahead of the policy, so the meta goes first
+// here and must still be the head's first element, ahead of the image, however
+// the parser tokenizes the identifier. Parsed in the play function for the
+// same reason as `HtmlCommentedHead`.
+const QUOTED_SYSTEM_DOCTYPE_HTML =
+  '<!DOCTYPE html SYSTEM "x>"><img src="https://example.com/pixel">';
+const QUOTED_PUBLIC_DOCTYPE_HTML =
+  '<!DOCTYPE html PUBLIC "a>" "b>"><img src="https://example.com/pixel">';
+
+async function expectPolicyAheadOfImage(html: string) {
+  const doc = new DOMParser().parseFromString(
+    wrapHtmlWithCsp(html),
+    "text/html",
+  );
+  const csp = doc.head.querySelector(
+    'meta[http-equiv="Content-Security-Policy"]',
+  );
+  await expect(csp).not.toBeNull();
+  await expect(doc.head.firstElementChild).toBe(csp);
+  await expect(csp?.getAttribute("content")).toBe(PREVIEW_HTML_CSP);
+  // The image is a real element after the policy, not doctype text.
+  await expect(doc.body.querySelector("img")).not.toBeNull();
+}
+
+export const HtmlDoctypeQuotedSystem: Story = {
+  args: {
+    contents: {
+      uri: "file:///doctype-system.html",
+      mimeType: "text/html",
+      text: QUOTED_SYSTEM_DOCTYPE_HTML,
+    },
+  },
+  play: async () => {
+    await expectPolicyAheadOfImage(QUOTED_SYSTEM_DOCTYPE_HTML);
+  },
+};
+
+export const HtmlDoctypeQuotedPublic: Story = {
+  args: {
+    contents: {
+      uri: "file:///doctype-public.html",
+      mimeType: "text/html",
+      text: QUOTED_PUBLIC_DOCTYPE_HTML,
+    },
+  },
+  play: async () => {
+    await expectPolicyAheadOfImage(QUOTED_PUBLIC_DOCTYPE_HTML);
+  },
+};
+
+// U+00A0 ahead of a doctype is text, not HTML whitespace, so it opens the body:
+// a meta written after that doctype would land in the body, where a CSP meta
+// is not applied. Only HTML whitespace may precede a doctype kept first.
+const NBSP_DOCTYPE_HTML =
+  '\u00A0<!DOCTYPE html><img src="https://example.com/pixel">';
+
+export const HtmlDoctypeAfterNbsp: Story = {
+  args: {
+    contents: {
+      uri: "file:///doctype-nbsp.html",
+      mimeType: "text/html",
+      text: NBSP_DOCTYPE_HTML,
+    },
+  },
+  play: async () => {
+    await expectPolicyAheadOfImage(NBSP_DOCTYPE_HTML);
+  },
+};
+
 export const BinaryUnsupported: Story = {
   args: {
     contents: {

clients/web/src/components/elements/ContentViewer/contentViewerUtils.test.ts

@@ -197,32 +197,66 @@ describe("formatXml", () => {
 });
 
 describe("wrapHtmlWithCsp", () => {
-  it("injects the CSP meta at the top of an existing <head>", () => {
-    const out = wrapHtmlWithCsp(
-      "<html><head><title>t</title></head><body>x</body></html>",
-    );
-    expect(out).toContain(`<head><meta http-equiv="Content-Security-Policy"`);
-    expect(out.indexOf("Content-Security-Policy")).toBeLessThan(
-      out.indexOf("<title>"),
+  // String-shape assertions only: happy-dom's parser does not place a leading
+  // `<meta>` the way the HTML spec does, so where the policy lands once parsed
+  // is asserted in a real browser by the `HtmlCommentedHead` story.
+  const meta = `<meta http-equiv="Content-Security-Policy" content="${PREVIEW_HTML_CSP}">`;
+
+  it("writes the CSP meta as the first token of a full document", () => {
+    const html = "<html><head><title>t</title></head><body>x</body></html>";
+    expect(wrapHtmlWithCsp(html)).toBe(`${meta}${html}`);
+  });
+
+  it("keeps a leading doctype (and BOM/whitespace) ahead of the meta", () => {
+    expect(
+      wrapHtmlWithCsp("  <!DOCTYPE html><html><body>x</body></html>"),
+    ).toBe(`  <!DOCTYPE html>${meta}<html><body>x</body></html>`);
+    expect(wrapHtmlWithCsp("\uFEFF<!doctype html><p>x</p>")).toBe(
+      `\uFEFF<!doctype html>${meta}<p>x</p>`,
     );
   });
 
-  it("adds a <head> when the document has <html> but no head", () => {
-    const out = wrapHtmlWithCsp("<html><body>x</body></html>");
-    expect(out).toContain("<head><meta http-equiv");
-    expect(out).toContain("<body>x</body>");
+  it("puts the meta before a doctype carrying a PUBLIC or SYSTEM identifier", () => {
+    // Only the simple form is kept ahead of the meta; where the policy lands
+    // once parsed is asserted by the HtmlDoctypeQuoted* stories.
+    for (const html of [
+      '<!DOCTYPE html SYSTEM "x>"><img src="https://example.com/pixel">',
+      '<!DOCTYPE html PUBLIC "a>" "b>"><img src="https://example.com/pixel">',
+      '<!DOCTYPE html SYSTEM "about:legacy-compat"><p>x</p>',
+    ]) {
+      expect(wrapHtmlWithCsp(html)).toBe(`${meta}${html}`);
+    }
   });
 
-  it("wraps a bare fragment in a minimal document", () => {
-    const out = wrapHtmlWithCsp("<p>hi</p>");
-    expect(out).toBe(
-      `<html><head><meta http-equiv="Content-Security-Policy" content="${PREVIEW_HTML_CSP}"></head><body><p>hi</p></body></html>`,
+  it("keeps only HTML whitespace inside or before a kept doctype", () => {
+    expect(wrapHtmlWithCsp("<!DOCTYPE\thtml\n><p>x</p>")).toBe(
+      `<!DOCTYPE\thtml\n>${meta}<p>x</p>`,
     );
+    // U+00A0 is not HTML whitespace: it is text, so it must not precede
+    // (asserted in a browser by the HtmlDoctypeAfterNbsp story).
+    const nbsp = "\u00A0<!DOCTYPE html><p>x</p>";
+    expect(wrapHtmlWithCsp(nbsp)).toBe(`${meta}${nbsp}`);
+  });
+
+  it("is not fooled by a <head> inside a comment before the real head", () => {
+    const html =
+      "<!-- <head> --><html><head><title>t</title></head><body>x</body></html>";
+    // The meta precedes the comment rather than being spliced into it.
+    expect(wrapHtmlWithCsp(html)).toBe(`${meta}${html}`);
+  });
+
+  it("is not fooled by a <head> inside a script or attribute", () => {
+    const html = '<script>"<head>"</script><p title="<head>">x</p>';
+    expect(wrapHtmlWithCsp(html)).toBe(`${meta}${html}`);
+  });
+
+  it("does not treat a doctype after a comment as leading", () => {
+    const html = "<!-- c --><!DOCTYPE html><p>x</p>";
+    expect(wrapHtmlWithCsp(html)).toBe(`${meta}${html}`);
   });
 
-  it("preserves attributes on <head>/<html>", () => {
-    const out = wrapHtmlWithCsp('<head lang="en"></head>');
-    expect(out).toContain('<head lang="en"><meta http-equiv');
+  it("leaves a bare fragment unwrapped behind the meta", () => {
+    expect(wrapHtmlWithCsp("<p>hi</p>")).toBe(`${meta}<p>hi</p>`);
   });
 
   it("omits script-src so it falls through to default-src none", () => {

clients/web/src/components/elements/ContentViewer/contentViewerUtils.ts

@@ -258,23 +258,43 @@ export const PREVIEW_HTML_CSP =
 
 const CSP_META_TAG = `<meta http-equiv="Content-Security-Policy" content="${PREVIEW_HTML_CSP}">`;
 
+/**
+ * A leading simple doctype, the one token allowed ahead of the CSP meta. It
+ * must stay first: a doctype that follows any element is ignored by the HTML
+ * parser, which would drop a full document into quirks mode. Only the plain
+ * `<!DOCTYPE html>` form is matched, with nothing but HTML whitespace inside
+ * it and only HTML whitespace (and a BOM) before it. Anything cleverer — a
+ * PUBLIC or SYSTEM identifier, a leading comment — would mean re-implementing
+ * the tokenizer's rules for those (quoted identifiers, `--!>`, `<!-->`), and a
+ * disagreement there is exactly how the meta could be swallowed into a
+ * declaration or land behind markup. Leading text the parser does not treat as
+ * whitespace (U+00A0, say) opens the body, so a meta after the doctype would
+ * land there, unapplied. Any other document gets the meta at the very
+ * start, before every declaration, and renders in quirks mode, which is
+ * harmless in a sandboxed preview.
+ */
+const LEADING_DOCTYPE =
+  /^\uFEFF?[\t\n\f\r ]*<!doctype[\t\n\f\r ]+html[\t\n\f\r ]*>/i;
+
 /**
  * Inject the preview CSP `<meta>` into an HTML document before it's served to a
- * sandboxed iframe. Handles three shapes: a full document with a `<head>` (inject
- * at the top of head), a document with `<html>` but no `<head>` (add a head), and
- * a bare fragment (wrap in a minimal document).
+ * sandboxed iframe. The meta is written as the document's first token (after a
+ * leading simple `<!DOCTYPE html>`, if any), never located by searching the input for `<head>`:
+ * a search can be fooled by a `<head>` inside a comment, a `<script>`, or an
+ * attribute value, landing the policy where the parser never applies it.
+ *
+ * As the first token, the parser has no choice about where it goes. It opens
+ * the implied `<html>` and `<head>` and inserts the meta there, ahead of any
+ * markup in the document, so the policy is in effect before anything loads.
+ * The document's own tags still parse normally after it: `<html>` attributes
+ * merge onto the root element, a duplicate `<head>` is ignored and its children
+ * stay in the head, and a bare fragment lands in an implied `<body>`. That is
+ * why the input is not wrapped in a fixed `<body>` the way `wrapSandboxedHtml`
+ * wraps app HTML — a full document previewed here must keep its own head.
  */
 export function wrapHtmlWithCsp(html: string): string {
-  if (/<head[\s>]/i.test(html)) {
-    return html.replace(/<head([^>]*)>/i, `<head$1>${CSP_META_TAG}`);
-  }
-  if (/<html[\s>]/i.test(html)) {
-    return html.replace(
-      /<html([^>]*)>/i,
-      `<html$1><head>${CSP_META_TAG}</head>`,
-    );
-  }
-  return `<html><head>${CSP_META_TAG}</head><body>${html}</body></html>`;
+  const doctype = LEADING_DOCTYPE.exec(html)?.[0] ?? "";
+  return `${doctype}${CSP_META_TAG}${html.slice(doctype.length)}`;
 }
 
 /**

clients/web/src/components/groups/ElicitationUrlPanel/ElicitationUrlPanel.test.tsx

@@ -15,6 +15,28 @@ const baseProps = {
 };
 
 describe("ElicitationUrlPanel", () => {
+  it("disables Open in Browser and explains why when the URL is refused", () => {
+    renderWithMantine(
+      <ElicitationUrlPanel
+        {...baseProps}
+        url="javascript:alert(1)"
+        openRefusal="Refusing to open this URL."
+      />,
+    );
+    expect(
+      screen.getByRole("button", { name: "Open in Browser" }),
+    ).toBeDisabled();
+    expect(screen.getByText("URL refused")).toBeInTheDocument();
+    expect(screen.getByText("Refusing to open this URL.")).toBeInTheDocument();
+    // Copy stays available so the user can inspect the URL.
+    expect(screen.getByRole("button", { name: "Copy URL" })).toBeEnabled();
+  });
+
+  it("shows no refusal when the URL may be opened", () => {
+    renderWithMantine(<ElicitationUrlPanel {...baseProps} />);
+    expect(screen.queryByText("URL refused")).not.toBeInTheDocument();
+  });
+
   it("renders the message, url, and request id", () => {
     renderWithMantine(<ElicitationUrlPanel {...baseProps} />);
     expect(

clients/web/src/components/groups/ElicitationUrlPanel/ElicitationUrlPanel.tsx

@@ -38,6 +38,12 @@ export interface ElicitationUrlPanelProps {
    * again). Copy URL stays enabled — it doesn't resolve the request.
    */
   busy?: boolean;
+  /**
+   * Why the URL may not be opened (e.g. a non-http(s) scheme). When set, Open
+   * in Browser is disabled and the reason is shown, so a refused URL reads as
+   * refused rather than as a button that silently does nothing.
+   */
+  openRefusal?: string;
 }
 
 const ItalicMessage = Text.withProps({
@@ -65,6 +71,12 @@ const WarningAlert = Alert.withProps({
   title: "Warning",
 });
 
+const RefusalAlert = Alert.withProps({
+  color: "red",
+  variant: "light",
+  title: "URL refused",
+});
+
 function formatRequestId(id: string): string {
   return `Request ID: ${id}`;
 }
@@ -80,6 +92,7 @@ export function ElicitationUrlPanel({
   onCancel,
   origin = "server-request",
   busy = false,
+  openRefusal,
 }: ElicitationUrlPanelProps) {
   return (
     <Stack gap="md">
@@ -92,10 +105,15 @@ export function ElicitationUrlPanel({
         <Button variant="light" onClick={onCopyUrl}>
           Copy URL
         </Button>
-        <Button variant="light" onClick={onOpenInBrowser} disabled={busy}>
+        <Button
+          variant="light"
+          onClick={onOpenInBrowser}
+          disabled={busy || openRefusal !== undefined}
+        >
           {isWaiting ? "Reopen in Browser" : "Open in Browser"}
         </Button>
       </Group>
+      {openRefusal !== undefined && <RefusalAlert>{openRefusal}</RefusalAlert>}
       <Divider />
       {isWaiting && (
         <Group>

clients/web/src/components/groups/ImportServerJsonPanel/ImportServerJsonPanel.stories.tsx

@@ -56,6 +56,11 @@ export const ValidJson: Story = {
       { type: "success", message: "Package configuration is valid" },
       { type: "info", message: "1 package found" },
     ],
+    launchPreview: {
+      kind: "command",
+      value: "npx -y my-mcp-server",
+      envNames: [],
+    },
   },
 };
 
@@ -215,5 +220,10 @@ export const FullyConfigured: Story = {
         value: "true",
       },
     ],
+    launchPreview: {
+      kind: "command",
+      value: "npx -y @org/full-server",
+      envNames: ["TOKEN", "DEBUG"],
+    },
   },
 };