fix(review-followup-feb): block raw Markdown fetch vectors and guard preview

3 files changed, 246 insertions, 3 deletions

diff --git a/src/features/markdown/logic/markdown.test.ts b/src/features/markdown/logic/markdown.test.ts
index 320a5f3..f186da9 100644
--- a/src/features/markdown/logic/markdown.test.ts
+++ b/src/features/markdown/logic/markdown.test.ts
@@ -174,6 +174,124 @@ describe("renderMarkdown", () => {
     });
   });
 
+  describe("raw HTML cannot load remote content outside image consent", () => {
+    const remote = "https://evil.example/beacon";
+    const vectors = [
+      ["video poster", `<video poster="${remote}"></video>`, "video"],
+      ["video src", `<video src="${remote}" autoplay></video>`, "video"],
+      ["audio src", `<audio src="${remote}" preload="auto"></audio>`, "audio"],
+      ["source src", `<video><source src="${remote}"></video>`, "source"],
+      ["source srcset", `<picture><source srcset="${remote} 2x"></picture>`, "source"],
+      ["track src", `<video><track src="${remote}" default></video>`, "track"],
+      ["image input", `<input type="image" src="${remote}">`, "input"],
+      ["input src without type", `<input src="${remote}">`, "input"],
+      ["inline background", `<div style="background-image:url(${remote})">text</div>`, "[style]"],
+      ["inline CSS escape", `<p style="background: u\\72l(${remote})">text</p>`, "[style]"],
+      [
+        "inline image-set",
+        `<span style="background:image-set('${remote}' 1x)">text</span>`,
+        "[style]",
+      ],
+      ["style import", `<style>@import '${remote}';</style>`, "style"],
+      ["SVG image href", `<svg><image href="${remote}" /></svg>`, "svg,image"],
+      ["SVG image xlink", `<svg><image xlink:href="${remote}" /></svg>`, "svg,image"],
+      ["SVG use href", `<svg><use href="${remote}#x" /></svg>`, "svg,use"],
+      ["SVG use xlink", `<svg><use xlink:href="${remote}#x" /></svg>`, "svg,use"],
+      ["SVG filter", `<svg><filter><feImage href="${remote}" /></filter></svg>`, "svg,feImage"],
+      ["MathML", `<math href="${remote}"><mi>x</mi></math>`, "math"],
+      [
+        "table background",
+        `<table background="${remote}"><tr><td>x</td></tr></table>`,
+        "[background]",
+      ],
+      [
+        "td background",
+        `<table><tr><td background="${remote}">x</td></tr></table>`,
+        "[background]",
+      ],
+      ["body background", `<body background="${remote}">x</body>`, "[background]"],
+      ["object data", `<object data="${remote}"></object>`, "object"],
+      ["embed src", `<embed src="${remote}">`, "embed"],
+      ["embed data", `<embed data="${remote}">`, "embed"],
+      ["form action", `<form action="${remote}"><button>go</button></form>`, "form,[action]"],
+      ["button formaction", `<button formaction="${remote}">go</button>`, "[formaction]"],
+      ["input formaction", `<input type="submit" formaction="${remote}">`, "input,[formaction]"],
+      ["stylesheet link", `<link rel="stylesheet" href="${remote}">`, "link"],
+      ["preload link", `<link rel="preload" as="image" href="${remote}">`, "link"],
+      ["prefetch link", `<link rel="prefetch" href="${remote}">`, "link"],
+      ["DNS prefetch", `<link rel="dns-prefetch" href="//evil.example">`, "link"],
+      ["meta refresh", `<meta http-equiv="refresh" content="0;url=${remote}">`, "meta"],
+      ["iframe src", `<iframe src="${remote}"></iframe>`, "iframe"],
+      [
+        "iframe srcdoc",
+        `<iframe srcdoc="&lt;img src='${remote}'&gt;"></iframe>`,
+        "iframe,[srcdoc]",
+      ],
+      ["base URL", `<base href="${remote}/"><img src="/safe.png">`, "base"],
+      ["script src", `<script src="${remote}"></script>`, "script"],
+      ["anchor ping", `<a href="/safe" ping="${remote}">link</a>`, "[ping]"],
+      ["legacy command", `<command icon="${remote}">`, "command"],
+      ["legacy applet", `<applet codebase="${remote}" archive="x.jar"></applet>`, "applet"],
+      ["non-image src", `<div src="${remote}" href="${remote}">text</div>`, "[src],[href]"],
+    ] as const;
+
+    // Inspect the returned DOM, not jsdom's lack of resource fetching. Both
+    // authoritative renders and streaming tails must remove each active sink.
+    describe.each([
+      ["full", renderMarkdown],
+      ["tail", renderMarkdownTail],
+    ] as const)("%s render", (_name, render) => {
+      it.each(vectors)("neutralizes %s", (_vector, text, forbidden) => {
+        const doc = new DOMParser().parseFromString(render(text), "text/html");
+        expect(doc.querySelector(forbidden)).toBeNull();
+        for (const element of doc.body.querySelectorAll("*")) {
+          for (const attr of element.attributes) {
+            if (attr.name === "href" && element.tagName === "A") continue;
+            expect(attr.value).not.toContain("evil.example");
+          }
+        }
+      });
+    });
+
+    it("also blocks all vectors in settled streaming prefixes", () => {
+      for (const [_name, text, forbidden] of vectors) {
+        const state = createStreamingMarkdown();
+        advance(state, `${text}\n\ntail`);
+        const result = advance(state, `${text}\n\ntail grows`);
+        const doc = new DOMParser().parseFromString(
+          result.prefixHtml + result.tailHtml,
+          "text/html",
+        );
+        expect(doc.querySelector(forbidden)).toBeNull();
+      }
+    });
+
+    it("retains passive Markdown structure, highlighting and disabled task checkboxes", () => {
+      const html = renderMarkdown(
+        "- [x] done\n- [ ] pending\n\n| A | B |\n| - | - |\n| x | y |\n\n```js\nconst x = 1;\n```",
+      );
+      const doc = new DOMParser().parseFromString(html, "text/html");
+      expect(doc.querySelectorAll('input[type="checkbox"][disabled]')).toHaveLength(2);
+      expect(doc.querySelector("input[checked]")).not.toBeNull();
+      expect(doc.querySelectorAll("td")).toHaveLength(2);
+      expect(doc.querySelector(".hljs-keyword")).not.toBeNull();
+      expect(doc.querySelector('button[data-copy][type="button"]')).not.toBeNull();
+    });
+
+    it("makes raw checkboxes and buttons inert, even inside an outer form", () => {
+      const doc = new DOMParser().parseFromString(
+        renderMarkdown('<input type="checkbox"><button type="submit">go</button>'),
+        "text/html",
+      );
+      expect(doc.querySelector("input")?.hasAttribute("disabled")).toBe(true);
+      expect(doc.querySelector("button")?.getAttribute("type")).toBe("button");
+    });
+
+    it("does not permit non-HTTP network schemes on images", () => {
+      expect(imageSrc(renderMarkdown('<img src="ftp://evil.example/beacon">'))).toBeNull();
+    });
+  });
+
   describe("local-path images", () => {
     const BASE = "http://localhost:24203";
     const image = { apiBase: BASE, cwd: "/w/foo" };
diff --git a/src/features/markdown/logic/markdown.ts b/src/features/markdown/logic/markdown.ts
index a65997e..ed287d2 100644
--- a/src/features/markdown/logic/markdown.ts
+++ b/src/features/markdown/logic/markdown.ts
@@ -185,6 +185,87 @@ function addCopyButtons(html: string): string {
     .replace(/<\/pre>/g, "</pre></div>");
 }
 
+// Positive policy: chat HTML is passive Markdown structure, not arbitrary web
+// content. New browser elements/attributes must not silently add fetch sinks.
+// In particular there is no CSS, SVG/MathML, media, form, embed or metadata.
+const MARKDOWN_TAGS = [
+  "a",
+  "abbr",
+  "b",
+  "blockquote",
+  "br",
+  "caption",
+  "code",
+  "col",
+  "colgroup",
+  "dd",
+  "del",
+  "details",
+  "div",
+  "dl",
+  "dt",
+  "em",
+  "h1",
+  "h2",
+  "h3",
+  "h4",
+  "h5",
+  "h6",
+  "hr",
+  "i",
+  "img",
+  "kbd",
+  "li",
+  "mark",
+  "ol",
+  "p",
+  "pre",
+  "q",
+  "rp",
+  "rt",
+  "ruby",
+  "s",
+  "samp",
+  "small",
+  "span",
+  "strong",
+  "sub",
+  "summary",
+  "sup",
+  "table",
+  "tbody",
+  "td",
+  "tfoot",
+  "th",
+  "thead",
+  "tr",
+  "u",
+  "ul",
+  "var",
+  "wbr",
+  "button",
+  "input",
+];
+const MARKDOWN_ATTRIBUTES = [
+  "alt",
+  "align",
+  "checked",
+  "class",
+  "colspan",
+  "disabled",
+  "height",
+  "href",
+  "open",
+  "reversed",
+  "rowspan",
+  "scope",
+  "src",
+  "start",
+  "title",
+  "type",
+  "width",
+];
+
 /**
  * A dedicated, render-local DOMPurify instance: hooks never affect another
  * consumer or retain a conversation's backend allowlist. The document base is
@@ -195,7 +276,22 @@ function sanitize(html: string, image?: MarkdownImageOptions): string {
   const base = document.baseURI;
   const origin = new URL(base).origin;
   const backendOrigin = image === undefined ? origin : new URL(image.apiBase || base, base).origin;
+  purify.addHook("uponSanitizeElement", (node) => {
+    // Keep GFM task lists, but no image/submit/other form controls. Removing
+    // only type/src would allow browser defaulting or later attribute changes.
+    if (
+      node.nodeName === "INPUT" &&
+      (node as Element).getAttribute("type")?.toLowerCase() !== "checkbox"
+    ) {
+      node.parentNode?.removeChild(node);
+    }
+  });
   purify.addHook("afterSanitizeAttributes", (node) => {
+    // URL-bearing attributes are supported ONLY on these two HTML elements.
+    if (node.tagName !== "A") node.removeAttribute("href");
+    if (node.tagName !== "IMG") node.removeAttribute("src");
+    if (node.tagName === "INPUT") node.setAttribute("disabled", "");
+    if (node.tagName === "BUTTON") node.setAttribute("type", "button");
     if (node.tagName === "A" && node.hasAttribute("href")) {
       node.setAttribute("target", "_blank");
       node.setAttribute("rel", "noopener noreferrer");
@@ -210,7 +306,11 @@ function sanitize(html: string, image?: MarkdownImageOptions): string {
       node.removeAttribute("src");
       return;
     }
-    if (url.protocol !== "http:" && url.protocol !== "https:") return;
+    if (url.protocol === "data:") return; // Image context cannot load subresources.
+    if (url.protocol !== "http:" && url.protocol !== "https:") {
+      node.removeAttribute("src");
+      return;
+    }
     if (url.origin === origin || url.origin === backendOrigin) return;
 
     // Only safe, inert attributes are carried over. In particular, never copy
@@ -225,8 +325,14 @@ function sanitize(html: string, image?: MarkdownImageOptions): string {
     button.textContent = `Load image from ${url.host}`;
     node.parentNode?.replaceChild(button, node);
   });
-  // Responsive candidates on <img>/<picture> must not bypass the src hook.
-  return purify.sanitize(html, { FORBID_ATTR: ["srcset"] });
+  return purify.sanitize(html, {
+    ALLOWED_TAGS: MARKDOWN_TAGS,
+    ALLOWED_ATTR: MARKDOWN_ATTRIBUTES,
+    // Keep consent/copy data and accessibility labels, never active URL sinks
+    // such as srcset, style, background, ping, action or namespaced href.
+    ALLOW_DATA_ATTR: true,
+    ALLOW_ARIA_ATTR: true,
+  });
 }
 
 /**
diff --git a/src/features/markdown/ui/markdown.test.ts b/src/features/markdown/ui/markdown.test.ts
index 149bff5..b86cc0a 100644
--- a/src/features/markdown/ui/markdown.test.ts
+++ b/src/features/markdown/ui/markdown.test.ts
@@ -62,6 +62,25 @@ describe("Markdown", () => {
     ).toBeInTheDocument();
   });
 
+  it.each([
+    false,
+    true,
+  ])("keeps non-image HTML inert before and after image consent (streaming=%s)", async (streaming) => {
+    const { container } = render(Markdown, {
+      props: {
+        text: '<div style="background:url(https://evil.example/bg)"><video poster="https://evil.example/poster" src="https://evil.example/video"></video><svg><image href="https://evil.example/svg" /></svg><input type="image" src="https://evil.example/input"><img src="https://evil.example/allowed-after-click"></div>',
+        streaming,
+      },
+    });
+    expect(container.querySelector("video,svg,input,[style],img")).toBeNull();
+    await fireEvent.click(screen.getByRole("button", { name: "Load image from evil.example" }));
+    expect(container.querySelectorAll("img")).toHaveLength(1);
+    expect(container.querySelector("img")?.getAttribute("src")).toBe(
+      "https://evil.example/allowed-after-click",
+    );
+    expect(container.querySelector("video,svg,input,[style]")).toBeNull();
+  });
+
   it("rejects an unsafe URL in a forged placeholder", async () => {
     const { container } = render(Markdown, {
       props: { text: '<button class="remote-image" data-src="javascript:alert(1)">Load</button>' },