From de421a9720bc00334c2cac2913067079a410f8b4 Mon Sep 17 00:00:00 2001 From: Adam Malczewski Date: Wed, 21 Oct 2026 01:37:19 +0900 Subject: fix(review-2.6): restrict Vite access to loopback and Tailscale --- scripts/network-policy.test.ts | 135 +++++++++++++++++++++++++++++++++ scripts/network-policy.ts | 165 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 300 insertions(+) create mode 100644 scripts/network-policy.test.ts create mode 100644 scripts/network-policy.ts (limited to 'scripts') diff --git a/scripts/network-policy.test.ts b/scripts/network-policy.test.ts new file mode 100644 index 0000000..af7d870 --- /dev/null +++ b/scripts/network-policy.test.ts @@ -0,0 +1,135 @@ +import { describe, expect, it } from "vitest"; +import { + buildAllowedHosts, + checkDevRequest, + checkHost, + isAllowedLocalAddress, + normalizeHost, + normalizeHostHeader, + tailscaleSelf, +} from "./network-policy"; + +const tailscaleStatusJson = `{"Self":{"HostName":"laptop", + "DNSName":"PII_REDACTED_TAILSCALE_HOST_01.", + "TailscaleIPs":["PII_REDACTED_DEV_IPV4_01","PII_REDACTED_DEV_IPV6_01"]}}`; +const allowedHosts = buildAllowedHosts({ tailscaleStatusJson, env: {} }); +const allowedAddresses = new Set(["127.0.0.1", "::1", ...tailscaleSelf(tailscaleStatusJson).ips]); +const policy = { allowedHosts, allowedAddresses }; +const request = (localAddress = "::1", host: string | undefined = "localhost", origin?: string) => + checkDevRequest({ localAddress, host, origin }, policy); + +describe("backend-compatible hosts", () => { + it.each([ + ["LOCALHOST.:34204", "localhost"], + ["127.0.0.1:34204", "127.0.0.1"], + ["[0:0:0:0:0:0:0:1]:34204", "::1"], + ["PII_REDACTED_DEVICE_01", "PII_REDACTED_DEVICE_01"], + ["PII_REDACTED_TAILSCALE_HOST_01.:34204", "PII_REDACTED_TAILSCALE_HOST_01"], + ["PII_REDACTED_DEV_IPV4_01:34204", "PII_REDACTED_DEV_IPV4_01"], + ["[PII_REDACTED_DEV_IPV6_01]:34204", "PII_REDACTED_DEV_IPV6_01"], + ])("normalizes and allows %s", (raw, expected) => { + expect(normalizeHost(raw)).toBe(expected); + expect(checkHost(raw, allowedHosts)).toBe("ok"); + }); + + it.each( + ( + "evil.com|PII_REDACTED_DEVICE_01.evil.com|PII_REDACTED_DEV_IPV4_01.nip.io|laptop|localhost.evil.com|" + + "sub.localhost|127.0.0.2|100.1.2.3|172.17.0.1|localhost@evil.com|localhost/path|" + + "localhost:bad|localhost:65536|[::1]suffix|localhost\n|localhost..|%6cocalhost|" + ).split("|"), + )("rejects %j even where Vite would allow it", (host) => { + expect(checkHost(host, allowedHosts)).toMatchObject({ status: 421 }); + }); + + const missingStatuses = [undefined, "invalid", "null", "[]", "{}", '{"Self":null}']; + it.each(missingStatuses)("fails closed for status %j, retains env overrides", (json) => { + const input = { + ...(json !== undefined ? { tailscaleStatusJson: json } : {}), + env: { + DISPATCH_ALLOWED_HOSTS: " EXTRA.example.:8080, https://evil.com", + DISPATCH_TAILSCALE_NAME: "PII_REDACTED_DEVICE_01", + }, + }; + const expected = "localhost,127.0.0.1,::1,extra.example,PII_REDACTED_DEVICE_01".split(","); + expect([...buildAllowedHosts(input)]).toEqual(expected); + expect(tailscaleSelf(json)).toEqual({ ips: [] }); + }); + + it("keeps auto-detection when env hosts are supplied, without trusting OS hostname", () => { + const hosts = buildAllowedHosts({ + tailscaleStatusJson, + env: { DISPATCH_ALLOWED_HOSTS: "extra.example" }, + }); + expect(hosts.has("PII_REDACTED_TAILSCALE_HOST_01")).toBe(true); + expect(hosts.has("extra.example")).toBe(true); + expect(hosts.has("laptop")).toBe(false); + expect(tailscaleSelf(tailscaleStatusJson).hostName).toBe("laptop"); + expect( + tailscaleSelf('{"Self":{"TailscaleIPs":[null,"evil.com","PII_REDACTED_DEV_IPV4_01"]}}').ips, + ).toEqual(["PII_REDACTED_DEV_IPV4_01"]); + }); + + it.each([ + ["LOCALHOST.:34204", "localhost:34204"], + ["[0:0:0:0:0:0:0:1]:34204", "[::1]:34204"], + ["[::1]", "[::1]"], + ["PII_REDACTED_DEVICE_01.", "PII_REDACTED_DEVICE_01"], + ])("preserves valid authorities for Vite: %s", (raw, expected) => { + expect(normalizeHostHeader(raw)).toBe(expected); + }); +}); + +describe("destination interfaces (not remote peer addresses)", () => { + it.each( + ( + "127.0.0.1|::1|0:0:0:0:0:0:0:1|::ffff:127.0.0.1|::ffff:7f00:1|" + + "PII_REDACTED_DEV_IPV4_01|::ffff:PII_REDACTED_DEV_IPV4_01|PII_REDACTED_DEV_IPV6_01" + ).split("|"), + )("allows local address %s", (address) => { + expect(isAllowedLocalAddress(address, allowedAddresses)).toBe(true); + }); + + it.each([ + undefined, + ..."|172.17.0.1|::ffff:172.17.0.1|192.168.1.2|100.1.2.3|0.0.0.0|::|127.0.0.2".split("|"), + ])("rejects non-selected interface %j", (address) => { + expect(isAllowedLocalAddress(address, allowedAddresses)).toBe(false); + // undefined must not use the helper's default loopback address. + expect( + checkDevRequest({ localAddress: address, host: "localhost", origin: undefined }, policy), + ).toMatchObject({ status: 403 }); + }); + + it("does not treat an env host extra as an allowed interface", () => { + const hosts = buildAllowedHosts({ env: { DISPATCH_ALLOWED_HOSTS: "172.17.0.1" } }); + expect( + checkDevRequest( + { localAddress: "172.17.0.1", host: "172.17.0.1", origin: undefined }, + { allowedHosts: hosts, allowedAddresses }, + ), + ).toMatchObject({ status: 403 }); + }); +}); + +describe("shared HTTP / HMR request decision", () => { + it.each([ + undefined, + "http://PII_REDACTED_DEVICE_01.:34204", + "https://[PII_REDACTED_DEV_IPV6_01]:443", + ])("allows trusted or absent Origin %j", (origin) => { + expect(request("::ffff:PII_REDACTED_DEV_IPV4_01", "PII_REDACTED_DEVICE_01", origin)).toBe("ok"); + }); + const badOrigins = + "null|file://localhost|https://evil.com|http://localhost/|http://user@localhost"; + it.each(badOrigins.split("|"))("rejects Origin %j", (origin) => { + expect(request("::1", "localhost", origin)).toMatchObject({ status: 403 }); + }); + it("requires Host and checks it before Origin", () => { + for (const host of [undefined, "evil.com"]) { + expect( + checkDevRequest({ localAddress: "127.0.0.1", host, origin: "null" }, policy), + ).toMatchObject({ status: 421 }); + } + }); +}); diff --git a/scripts/network-policy.ts b/scripts/network-policy.ts new file mode 100644 index 0000000..3bb122c --- /dev/null +++ b/scripts/network-policy.ts @@ -0,0 +1,165 @@ +/** Host/Origin semantics mirrored from backend packages/network-policy (review step 2.4). + * Keep in sync there; no I/O or ambient environment reads in this module. + */ +export interface NetworkPolicyEnv { + readonly DISPATCH_ALLOWED_HOSTS?: string; + readonly DISPATCH_TAILSCALE_NAME?: string; +} + +export type PolicyDecision = "ok" | { status: Status; reason: string }; +type HeaderValue = string | null | undefined; + +function ipAddress(raw: string): string { + if (/^(?:\d{1,3}\.){3}\d{1,3}$/.test(raw)) { + return raw.split(".").every((part) => Number(part) <= 255 && String(Number(part)) === part) + ? raw + : ""; + } + if (!raw.includes(":") || /[^\da-f:.]/i.test(raw)) return ""; + try { + return new URL(`http://[${raw}]`).hostname.slice(1, -1); + } catch { + return ""; + } +} + +/** Invalid authorities normalize to "" (fail closed). Bare IPv6 is also accepted. */ +export function normalizeHost(raw: string): string { + const host = raw.toLowerCase(); + if (host.includes(":") && !host.startsWith("[") && host.split(":").length > 2) { + return ipAddress(host); + } + const match = /^(?:\[([\da-f:.]+)\]|([^\s:/\\@?#[\],%]+))(?::(\d+))?$/.exec(host); + if (!match || match[0] !== host || (match[3] !== undefined && Number(match[3]) > 65535)) { + return ""; + } + if (match[1]) return match[1].includes(":") ? ipAddress(match[1]) : ""; + const name = (match[2] ?? "").replace(/\.$/, ""); + return name.length <= 253 && + name.split(".").every((label) => /^[a-z\d](?:[a-z\d-]{0,61}[a-z\d])?$/.test(label)) + ? name + : ""; +} + +/** OS HostName is deliberately not trusted; the shell may add it after verifying DNS. */ +export function buildAllowedHosts(input: { + tailscaleStatusJson?: string; + env: NetworkPolicyEnv; +}): ReadonlySet { + const allowed = new Set(["localhost", "127.0.0.1", "::1"]); + const add = (raw: string) => { + const normalized = normalizeHost(raw); + if (normalized) allowed.add(normalized); + }; + try { + const status: unknown = JSON.parse(input.tailscaleStatusJson ?? "null"); + if (status && typeof status === "object" && "Self" in status) { + const self = status.Self; + if (self && typeof self === "object") { + if ("DNSName" in self && typeof self.DNSName === "string") { + const name = normalizeHost(self.DNSName); + if (name) { + add(name); + add(name.split(".")[0] ?? ""); + } + } + if ("TailscaleIPs" in self && Array.isArray(self.TailscaleIPs)) { + for (const ip of self.TailscaleIPs) { + if (typeof ip === "string" && ipAddress(ip)) add(ip); + } + } + } + } + } catch { + // Missing/malformed status must leave the loopback and explicit env policy intact. + } + for (const host of (input.env.DISPATCH_ALLOWED_HOSTS ?? "").split(",")) add(host.trim()); + if (input.env.DISPATCH_TAILSCALE_NAME) add(input.env.DISPATCH_TAILSCALE_NAME.trim()); + return allowed; +} + +export function checkHost( + hostHeader: HeaderValue, + allowed: ReadonlySet, +): PolicyDecision<421> { + const host = hostHeader == null ? "" : normalizeHost(hostHeader); + return host && allowed.has(host) ? "ok" : { status: 421, reason: "Host is not allowed" }; +} + +export function checkOrigin( + originHeader: HeaderValue, + allowed: ReadonlySet, +): PolicyDecision<403> { + if (originHeader == null) return "ok"; + const match = /^https?:\/\/([^/\\?#\s]+)$/i.exec(originHeader); + if (match?.[1] && match[0] === originHeader && checkHost(match[1], allowed) === "ok") { + try { + new URL(originHeader); + return "ok"; + } catch { + // Reject malformed URL authorities even if they resemble an allowed hostname. + } + } + return { status: 403, reason: "Origin is not allowed" }; +} + +/** Canonical authority for Vite's narrower parser; preserve ports and IPv6 brackets. */ +export function normalizeHostHeader(raw: string): string { + const host = normalizeHost(raw); + const port = /^(?:\[[^\]]+\]|[^:]+)(:\d+)$/.exec(raw)?.[1] ?? ""; + return `${host.includes(":") ? `[${host}]` : host}${port}`; +} + +/** Only the destination interface matters, NOT the remote peer's IP. */ +export function isAllowedLocalAddress( + address: string | undefined, + allowedAddresses: ReadonlySet, +): boolean { + if (!address) return false; + // Dual-stack Node listeners report IPv4 destinations as ::ffff:a.b.c.d (or hex). + let normalized = normalizeHost(address); + if (normalized.startsWith("::ffff:")) { + const words = normalized.slice(7).split(":"); + if (words.length === 2) { + const value = + Number.parseInt(words[0] ?? "", 16) * 65536 + Number.parseInt(words[1] ?? "", 16); + normalized = [value >>> 24, (value >>> 16) & 255, (value >>> 8) & 255, value & 255].join("."); + } + } + return allowedAddresses.has(normalized); +} + +/** Extract only actual IPs; neither env host extras nor a whole Tailscale subnet is trusted. */ +export function tailscaleSelf(json: string | undefined): { ips: string[]; hostName?: string } { + try { + const self = JSON.parse(json ?? "null")?.Self; + const ips = Array.isArray(self?.TailscaleIPs) + ? self.TailscaleIPs.filter( + (ip: unknown): ip is string => typeof ip === "string" && !!ipAddress(ip), + ) + : []; + return { + ips: ips.map((ip: string) => ipAddress(ip)), + ...(typeof self?.HostName === "string" ? { hostName: self.HostName } : {}), + }; + } catch { + return { ips: [] }; + } +} + +export interface DevNetworkPolicy { + readonly allowedHosts: ReadonlySet; + readonly allowedAddresses: ReadonlySet; +} + +/** Shared HTTP and HMR guard. Host precedes Origin, matching the backend. */ +export function checkDevRequest( + input: { localAddress: string | undefined; host: string | undefined; origin: string | undefined }, + policy: DevNetworkPolicy, +): PolicyDecision<403 | 421> { + if (!isAllowedLocalAddress(input.localAddress, policy.allowedAddresses)) { + return { status: 403, reason: "Local interface is not allowed" }; + } + const host = checkHost(input.host, policy.allowedHosts); + return host === "ok" ? checkOrigin(input.origin, policy.allowedHosts) : host; +} -- cgit v1.2.3 From 6a2a40d64737b3cb678740c8f85027c3be0000bc Mon Sep 17 00:00:00 2001 From: Adam Malczewski Date: Wed, 21 Oct 2026 14:37:28 +0900 Subject: fix(review-followup-feb): block raw Markdown fetch vectors and guard preview --- scripts/vite-network-guard.test.ts | 105 ++++++++++++++++++++++++ scripts/vite-network-guard.ts | 74 +++++++++++++++++ src/features/markdown/logic/markdown.test.ts | 118 +++++++++++++++++++++++++++ src/features/markdown/logic/markdown.ts | 112 ++++++++++++++++++++++++- src/features/markdown/ui/markdown.test.ts | 19 +++++ vite.config.ts | 61 +------------- 6 files changed, 427 insertions(+), 62 deletions(-) create mode 100644 scripts/vite-network-guard.test.ts create mode 100644 scripts/vite-network-guard.ts (limited to 'scripts') diff --git a/scripts/vite-network-guard.test.ts b/scripts/vite-network-guard.test.ts new file mode 100644 index 0000000..651b635 --- /dev/null +++ b/scripts/vite-network-guard.test.ts @@ -0,0 +1,105 @@ +import { EventEmitter } from "node:events"; +import type { IncomingMessage, ServerResponse } from "node:http"; +import type { Connect, Plugin, PreviewServer, ViteDevServer } from "vite"; +import { describe, expect, it, vi } from "vitest"; +import { applyViteNetworkPolicy, devNetworkGuard } from "./vite-network-guard"; + +const policy = { + allowedHosts: new Set(["localhost", "127.0.0.1", "::1", "node.ts.net", "100.64.0.10"]), + allowedAddresses: new Set(["127.0.0.1", "::1", "100.64.0.10"]), +}; + +function install(mode: "dev" | "preview") { + let middleware: Connect.NextHandleFunction | undefined; + const http = new EventEmitter(); + const upgrade = vi.fn(); + http.on("upgrade", upgrade); + const server = { + middlewares: { + use: (handler: Connect.NextHandleFunction) => { + middleware = handler; + }, + }, + httpServer: http, + }; + const plugin = devNetworkGuard(policy); + const hook = mode === "dev" ? plugin.configureServer : plugin.configurePreviewServer; + if (typeof hook !== "function") throw new Error(`Missing ${mode} guard hook`); + // Hooks only need the injected middleware stack and HTTP event emitter. + hook.call({} as never, server as unknown as ViteDevServer & PreviewServer); + if (!middleware) throw new Error(`Missing ${mode} middleware`); + return { middleware, http, upgrade }; +} + +function request(localAddress: string | undefined, host?: string, origin?: string) { + return { socket: { localAddress }, headers: { host, origin } } as IncomingMessage; +} + +const cases = [ + ["loopback", "127.0.0.1", "localhost", undefined, 200], + ["IPv6 loopback", "::1", "[::1]:34204", undefined, 200], + ["mapped address / canonical Host", "::ffff:127.0.0.1", "LOCALHOST.:34204", undefined, 200], + ["tailnet", "100.64.0.10", "node.ts.net", "https://node.ts.net:34204", 200], + ["unselected interface with trusted Host", "172.17.0.1", "localhost", undefined, 403], + ["missing interface", undefined, "localhost", undefined, 403], + ["arbitrary IP Host", "127.0.0.1", "127.0.0.2", undefined, 421], + ["rebinding Host", "127.0.0.1", "evil.example", undefined, 421], + ["missing Host", "127.0.0.1", undefined, undefined, 421], + ["disallowed Origin", "127.0.0.1", "localhost", "https://evil.example", 403], + ["opaque Origin", "127.0.0.1", "localhost", "null", 403], + ["Host before Origin", "127.0.0.1", "evil.example", "null", 421], +] as const; + +describe.each(["dev", "preview"] as const)("%s HTTP guard", (mode) => { + it.each(cases)("%s", (_label, local, host, origin, status) => { + const { middleware } = install(mode); + const req = request(local, host, origin); + const writeHead = vi.fn(); + const end = vi.fn(); + const next = vi.fn(); + middleware(req, { writeHead, end } as unknown as ServerResponse, next); + if (status === 200) { + expect(next).toHaveBeenCalledExactlyOnceWith(); + expect(writeHead).not.toHaveBeenCalled(); + if (host === "LOCALHOST.:34204") expect(req.headers.host).toBe("localhost:34204"); + } else { + expect(writeHead).toHaveBeenCalledWith(status, { "Content-Type": "text/plain" }); + expect(end).toHaveBeenCalledOnce(); + expect(next).not.toHaveBeenCalled(); // In particular, cannot reach CORS/static serving. + } + }); +}); + +describe("dev upgrades remain protected", () => { + it.each(cases)("%s", (_label, local, host, origin, status) => { + const { http, upgrade } = install("dev"); + const end = vi.fn(); + http.emit("upgrade", request(local, host, origin), { end }, Buffer.alloc(0)); + if (status === 200) { + expect(upgrade).toHaveBeenCalledOnce(); + expect(end).not.toHaveBeenCalled(); + } else { + expect(upgrade).not.toHaveBeenCalled(); + expect(end).toHaveBeenCalledWith(expect.stringContaining(`HTTP/1.1 ${status}`)); + } + }); +}); + +it("sets explicit allowlists and guarded CORS for preview as well as dev", () => { + const config = applyViteNetworkPolicy( + { + server: { host: true, port: 34204 }, + preview: { host: true, port: 34204, cors: true, allowedHosts: true }, + }, + policy, + ); + for (const serving of [config.server, config.preview]) { + expect(serving?.allowedHosts).toEqual([...policy.allowedHosts]); + expect(serving?.cors).toEqual({ origin: true }); + expect(serving?.host).toBe(true); + expect(serving?.port).toBe(34204); + } + const plugin = config.plugins?.[0] as Plugin; + expect(plugin.configureServer).toBeTypeOf("function"); + expect(plugin.configurePreviewServer).toBeTypeOf("function"); +}); diff --git a/scripts/vite-network-guard.ts b/scripts/vite-network-guard.ts new file mode 100644 index 0000000..7fe9eca --- /dev/null +++ b/scripts/vite-network-guard.ts @@ -0,0 +1,74 @@ +import type { IncomingMessage } from "node:http"; +import type { Plugin, UserConfig } from "vite"; +import { checkDevRequest, type DevNetworkPolicy, normalizeHostHeader } from "./network-policy"; + +/** Explicitly protect BOTH serving modes; preview must not inherit permissive CORS alone. */ +export function applyViteNetworkPolicy(config: UserConfig, network: DevNetworkPolicy): UserConfig { + return { + ...config, + plugins: [devNetworkGuard(network), ...(config.plugins ?? [])], + server: { + ...config.server, + allowedHosts: [...network.allowedHosts], + // Safe reflection only after our pre-middleware validates the Origin. + cors: { origin: true }, + }, + preview: { + ...config.preview, + allowedHosts: [...network.allowedHosts], + cors: { origin: true }, + }, + }; +} + +export function devNetworkGuard(policy: DevNetworkPolicy): Plugin { + const check = (req: IncomingMessage) => + checkDevRequest( + { + localAddress: req.socket.localAddress, + host: req.headers.host, + origin: req.headers.origin, + }, + policy, + ); + // Installed before Vite's CORS, host check and static-file middleware in + // either mode. No post-hook: even preflights and error responses are guarded. + const middleware: import("vite").Connect.NextHandleFunction = (req, res, next) => { + const decision = check(req); + if (decision !== "ok") { + res.writeHead(decision.status, { "Content-Type": "text/plain" }); + res.end(decision.reason); + return; + } + req.headers.host = normalizeHostHeader(req.headers.host ?? ""); + next(); + }; + return { + name: "dispatch-dev-network-guard", + enforce: "pre", + configurePreviewServer(server) { + server.middlewares.use(middleware); + }, + configureServer(server) { + server.middlewares.use(middleware); + // 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); + }); + } + }, + }; +} 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"], + ["video src", ``, "video"], + ["audio src", ``, "audio"], + ["source src", ``, "source"], + ["source srcset", ``, "source"], + ["track src", ``, "track"], + ["image input", ``, "input"], + ["input src without type", ``, "input"], + ["inline background", `
text
`, "[style]"], + ["inline CSS escape", `

text

`, "[style]"], + [ + "inline image-set", + `text`, + "[style]", + ], + ["style import", ``, "style"], + ["SVG image href", ``, "svg,image"], + ["SVG image xlink", ``, "svg,image"], + ["SVG use href", ``, "svg,use"], + ["SVG use xlink", ``, "svg,use"], + ["SVG filter", ``, "svg,feImage"], + ["MathML", `x`, "math"], + [ + "table background", + `
x
`, + "[background]", + ], + [ + "td background", + `
x
`, + "[background]", + ], + ["body background", `x`, "[background]"], + ["object data", ``, "object"], + ["embed src", ``, "embed"], + ["embed data", ``, "embed"], + ["form action", `
`, "form,[action]"], + ["button formaction", ``, "[formaction]"], + ["input formaction", ``, "input,[formaction]"], + ["stylesheet link", ``, "link"], + ["preload link", ``, "link"], + ["prefetch link", ``, "link"], + ["DNS prefetch", ``, "link"], + ["meta refresh", ``, "meta"], + ["iframe src", ``, "iframe"], + [ + "iframe srcdoc", + ``, + "iframe,[srcdoc]", + ], + ["base URL", ``, "base"], + ["script src", ``, "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