From 85bcf6d031386c5dcf3435fe1a1f6b0abf4f9a61 Mon Sep 17 00:00:00 2001
From: Adam Malczewski <github@tradam.dev>
Date: Wed, 21 Oct 2026 01:10:55 +0900
Subject: fix(review-2.7): require clicks to load remote Markdown images

---
 src/features/markdown/ui/Markdown.svelte  | 27 ++++++++++++++++++---
 src/features/markdown/ui/markdown.test.ts | 39 +++++++++++++++++++++++++++++++
 2 files changed, 63 insertions(+), 3 deletions(-)

(limited to 'src/features/markdown/ui')

diff --git a/src/features/markdown/ui/Markdown.svelte b/src/features/markdown/ui/Markdown.svelte
index 0adbbea..a0fd276 100644
--- a/src/features/markdown/ui/Markdown.svelte
+++ b/src/features/markdown/ui/Markdown.svelte
@@ -14,7 +14,8 @@
      * Per-conversation local-image context: the backend HTTP base and the
      * conversation cwd. Markdown image srcs that are local paths are rewritten
      * through `${apiBase}/local-image?path=…` (relative paths resolve against
-     * `cwd`); remote/data URLs are untouched. Omit it to render srcs as-is.
+     * `cwd`). Remote HTTP(S) images require a click; data, same-origin and
+     * backend images load inline. Omit it to skip local-path rewriting.
      */
     image?: MarkdownImageOptions;
   } = $props();
@@ -41,8 +42,8 @@
 
   let container: HTMLElement;
 
-  // One delegated listener on the stable container handles every code block's
-  // copy button — including blocks re-created when the tail html changes
+  // One delegated listener on the stable container handles image consent and
+  // every code block's copy button — including blocks re-created when the tail html changes
   // (streaming), since the listener lives on the container, not the buttons.
   // Clipboard is the edge effect; absent (insecure context) → no-op.
   $effect(() => {
@@ -52,6 +53,26 @@
     const onClick = (event: Event): void => {
       const target = event.target;
       if (!(target instanceof Element)) return;
+      const remote = target.closest<HTMLButtonElement>("button.remote-image[data-src]");
+      if (remote !== null && el.contains(remote)) {
+        event.preventDefault(); // A Markdown image may be wrapped in a link.
+        event.stopPropagation();
+        // data-* is authorable in raw HTML: validate again at the DOM edge.
+        let url: URL;
+        try {
+          url = new URL(remote.dataset.src ?? "");
+        } catch {
+          return;
+        }
+        if (url.protocol !== "http:" && url.protocol !== "https:") return;
+        const img = el.ownerDocument.createElement("img");
+        img.alt = remote.dataset.alt ?? "";
+        if (remote.dataset.title !== undefined) img.title = remote.dataset.title;
+        img.setAttribute("referrerpolicy", "no-referrer");
+        img.src = url.href;
+        remote.replaceWith(img);
+        return;
+      }
       const button = target.closest<HTMLButtonElement>("[data-copy]");
       if (button === null) return;
 
diff --git a/src/features/markdown/ui/markdown.test.ts b/src/features/markdown/ui/markdown.test.ts
index 721dddb..149bff5 100644
--- a/src/features/markdown/ui/markdown.test.ts
+++ b/src/features/markdown/ui/markdown.test.ts
@@ -39,6 +39,45 @@ describe("Markdown", () => {
     expect(writeText.mock.calls[0]?.[0]).toContain("const x = 1;");
   });
 
+  it.each([
+    false,
+    true,
+  ])("loads only the clicked remote image (streaming=%s)", async (streaming) => {
+    const { container } = render(Markdown, {
+      props: {
+        text: '![first](https://evil.example/a.png "A title")\n\n![second](https://other.example/b.png)',
+        streaming,
+      },
+    });
+    expect(container.querySelector("img")).toBeNull();
+    const button = screen.getByRole("button", { name: "Load image from evil.example" });
+    await fireEvent.click(button);
+    const img = container.querySelector("img");
+    expect(img?.getAttribute("src")).toBe("https://evil.example/a.png");
+    expect(img?.getAttribute("alt")).toBe("first");
+    expect(img?.getAttribute("title")).toBe("A title");
+    expect(img?.getAttribute("referrerpolicy")).toBe("no-referrer");
+    expect(
+      screen.getByRole("button", { name: "Load image from other.example" }),
+    ).toBeInTheDocument();
+  });
+
+  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>' },
+    });
+    await fireEvent.click(screen.getByRole("button", { name: "Load" }));
+    expect(container.querySelector("img")).toBeNull();
+  });
+
+  it("delegates clicks on newly rendered images and prevents linked-image navigation", async () => {
+    const { container, rerender } = render(Markdown, { props: { text: "start" } });
+    await rerender({ text: "[![pic](https://evil.example/a.png)](https://other.example)" });
+    const button = screen.getByRole("button", { name: "Load image from evil.example" });
+    expect(await fireEvent.click(button)).toBe(false);
+    expect(container.querySelector("img")?.getAttribute("src")).toBe("https://evil.example/a.png");
+  });
+
   it("non-streaming renders a single container (today's full-render path)", () => {
     const { container } = render(Markdown, { props: { text: "# Hello" } });
     expect(container.querySelector(".markdown-stream-prefix")).toBeNull();
-- 
cgit v1.2.3