From aa63d6b9f4cb14535fd0191ab358c27f0ee62ce8 Mon Sep 17 00:00:00 2001 From: Adam Malczewski Date: Mon, 19 Oct 2026 23:48:50 +0900 Subject: feat(markdown): render local-path images inline via backend image route --- src/features/markdown/ui/Markdown.svelte | 19 ++++++++++++++++--- 1 file changed, 16 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 da2c9ba..0adbbea 100644 --- a/src/features/markdown/ui/Markdown.svelte +++ b/src/features/markdown/ui/Markdown.svelte @@ -1,16 +1,29 @@ `, "script"], + ["anchor ping", `link`, "[ping]"], + ["legacy command", ``, "command"], + ["legacy applet", ``, "applet"], + ["non-image src", `
text
`, "[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(''), + "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(''))).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, ""); } +// 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 / 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: '
', + 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: '' }, diff --git a/vite.config.ts b/vite.config.ts index 4a74b9b..4a45c51 100644 --- a/vite.config.ts +++ b/vite.config.ts @@ -11,27 +11,17 @@ import { svelteTesting } from "@testing-library/svelte/vite"; import { defineConfig, type Plugin, type UserConfig } from "vitest/config"; import { buildAllowedHosts, - checkDevRequest, type DevNetworkPolicy, normalizeHost, - normalizeHostHeader, tailscaleSelf, } from "./scripts/network-policy"; +import { applyViteNetworkPolicy } from "./scripts/vite-network-guard"; async function secureDevConfig(config: UserConfig, serving: boolean): Promise { // Build/test have no listener and need no subprocess or DNS effects. if (!serving) return config; const network = await detectDevNetworkPolicy(); - return { - ...config, - plugins: [devNetworkGuard(network), ...(config.plugins ?? [])], - server: { - ...config.server, - allowedHosts: [...network.allowedHosts], - // Safe reflection: our guard rejects disallowed Origins before Vite's CORS. - cors: { origin: true }, - }, - }; + return applyViteNetworkPolicy(config, network); } async function detectDevNetworkPolicy(): Promise { @@ -75,53 +65,6 @@ async function detectDevNetworkPolicy(): Promise { return { allowedHosts, allowedAddresses: new Set(["127.0.0.1", "::1", ...self.ips]) }; } -function devNetworkGuard(policy: DevNetworkPolicy): Plugin { - return { - name: "dispatch-dev-network-guard", - enforce: "pre", - configureServer(server) { - const check = (req: import("node:http").IncomingMessage) => - checkDevRequest( - { - localAddress: req.socket.localAddress, - host: req.headers.host, - origin: req.headers.origin, - }, - policy, - ); - server.middlewares.use((req, res, next) => { - const decision = check(req); - if (decision !== "ok") { - res.writeHead(decision.status, { "Content-Type": "text/plain" }); - res.end(decision.reason); - return; - } - // Vite's own check is case-sensitive and doesn't strip trailing dots. - req.headers.host = normalizeHostHeader(req.headers.host ?? ""); - next(); - }); - // Vite registers HMR before configureServer. Wrap (rather than prepend to) - // its listeners: EventEmitter otherwise continues into HMR after a denial. - const http = server.httpServer; - if (http) { - const upgrades = http.listeners("upgrade"); - http.removeAllListeners("upgrade"); - http.on("upgrade", (req, socket, head) => { - const decision = check(req); - if (decision !== "ok") { - socket.end( - `HTTP/1.1 ${decision.status} ${decision.status === 421 ? "Misdirected Request" : "Forbidden"}\r\nConnection: close\r\nContent-Length: 0\r\n\r\n`, - ); - return; - } - req.headers.host = normalizeHostHeader(req.headers.host ?? ""); - for (const upgrade of upgrades) upgrade.call(http, req, socket, head); - }); - } - }, - }; -} - /** * SPIKE (plan 03) — the Svelte-runtime import map. * -- cgit v1.2.3