From ccc5e631e28b6ad82796b4e3fe17e7c4cb64552a Mon Sep 17 00:00:00 2001 From: iris Date: Fri, 10 Jul 2026 02:46:51 +0200 Subject: [PATCH] web-ui: sanitize markdown HTML with DOMPurify to fix XSS MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both mdNode implementations (agent UI app.js, dashboard common.js) assigned marked.parse() output straight to innerHTML with no sanitizer. marked v5+ dropped its built-in sanitize option, and there was no DOMPurify anywhere in frontend/, so markdown containing raw HTML/script tags rendered live in the browser. Both sinks receive untrusted input in practice: the agent UI's mdNode renders recv tool_result bodies, assistant prose, and send/ask/answer payloads sourced from peer agents and matrix-relayed messages (the documented prompt-injection adversary); the dashboard's mdNode renders agent-authored .md files served verbatim by GET /api/state-file (the endpoint validates path, not content). Since the per-agent UI and dashboard are same-origin behind the gateway with operator-authority endpoints (approve/spawn/rebuild/destroy/answer-question), injected script would run with the operator's session. Fix: DOMPurify.sanitize() the marked.parse() output at both sinks before assigning to innerHTML. Added dompurify as a dependency to both the agent and dashboard npm workspaces, recomputed npmDepsHash in nix/frontend.nix for the updated lockfile. Also corrected docs/web-ui/shape.md, which claimed the markdown-rendering path was XSS-safe by construction the same way the text-node-based linkify path is — it isn't; it's safe because it's sanitized. CSP hardening for the dashboard (no unsafe-inline) is a separate, larger backend change (response headers in hive-c0re) and is left as a fast-follow rather than folded into this fix. --- docs/web-ui/shape.md | 19 +++++++++++++------ frontend/package-lock.json | 18 ++++++++++++++++++ frontend/packages/agent/package.json | 5 +++-- frontend/packages/agent/src/app.js | 8 +++++++- frontend/packages/dashboard/package.json | 1 + frontend/packages/dashboard/src/common.js | 9 +++++++-- nix/frontend.nix | 2 +- 7 files changed, 50 insertions(+), 12 deletions(-) diff --git a/docs/web-ui/shape.md b/docs/web-ui/shape.md index 84c2d36d..bd657f15 100644 --- a/docs/web-ui/shape.md +++ b/docs/web-ui/shape.md @@ -27,10 +27,14 @@ registers a kind→renderer map; unknown kinds fall through to a JSON-dump note row. Bare `http(s)://` URLs in row text are turned into clickable new-tab links by `linkify` (text-node - based, no `innerHTML` — XSS-safe); markdown bodies get the - same treatment via `marked`'s autolink (npm dep, replacing the - vendored UMD bundle), with the rendered ``s rewritten to - `target="_blank"`. + based, no `innerHTML` — XSS-safe); markdown bodies go through + `marked` (npm dep, replacing the vendored UMD bundle) and then + `DOMPurify.sanitize()` before hitting `innerHTML` — untrusted row + text (peer-agent / matrix-relayed message bodies, agent-authored + state files) can carry arbitrary HTML/script via markdown, so the + markdown path is sanitized rather than XSS-safe by construction the + way the text-node `linkify` path is. Rendered ``s are rewritten + to `target="_blank"`. - `GET /api/state` → JSON snapshot the JS app renders into the DOM. Includes a top-level `seq` (the dashboard event channel's high-water mark at the moment the snapshot was assembled); @@ -177,8 +181,11 @@ text get wrapped in `` inside a fresh text node, so the autolinker never touches `innerHTML` and untrusted row content can't smuggle markup. The trailing-punctuation strip keeps `.,;:` outside the link surface. -Markdown bodies go through `marked` separately and get the same -target rewrite. +Markdown bodies go through `marked` separately, then +`DOMPurify.sanitize()` on the resulting HTML before it's assigned to +`innerHTML` (see `mdNode` in `app.js` / `common.js`) — this path is +sanitized, not text-node-safe like `linkify`, since markdown can +carry raw HTML. Rendered ``s get the same target rewrite. The JS app handles all `form[data-async]` submissions via a delegated listener: read `data-confirm`, swap the button to a spinner, POST diff --git a/frontend/package-lock.json b/frontend/package-lock.json index c5509af5..7a66f63b 100644 --- a/frontend/package-lock.json +++ b/frontend/package-lock.json @@ -476,6 +476,13 @@ "integrity": "sha512-M5UknZPHRu3DEDWoipU6sE8PdkZ6Z/S+v4dD+Ke8IaNlpdSQah50lz1KtcFBa2vsdOnwbbnxJwVM4wty6udA5w==", "license": "MIT" }, + "node_modules/@types/trusted-types": { + "version": "2.0.7", + "resolved": "https://registry.npmjs.org/@types/trusted-types/-/trusted-types-2.0.7.tgz", + "integrity": "sha512-ScaPdn1dQczgbl0QFTeTOmVHFULt394XJgOQNoyVhZ6r2vLnMLJfBPd53SB52T/3G36VI1/g2MZaX0cwDuXsfw==", + "license": "MIT", + "optional": true + }, "node_modules/chart.js": { "version": "4.5.1", "resolved": "https://registry.npmjs.org/chart.js/-/chart.js-4.5.1.tgz", @@ -488,6 +495,15 @@ "pnpm": ">=8" } }, + "node_modules/dompurify": { + "version": "3.2.4", + "resolved": "https://registry.npmjs.org/dompurify/-/dompurify-3.2.4.tgz", + "integrity": "sha512-ysFSFEDVduQpyhzAob/kkuJjf5zWkZD8/A9ywSp1byueyuCfHamrCBa14/Oc2iiB0e51B+NpxSl5gmzn+Ms/mg==", + "license": "(MPL-2.0 OR Apache-2.0)", + "optionalDependencies": { + "@types/trusted-types": "^2.0.7" + } + }, "node_modules/esbuild": { "version": "0.28.0", "resolved": "https://registry.npmjs.org/esbuild/-/esbuild-0.28.0.tgz", @@ -548,6 +564,7 @@ "dependencies": { "@hive/shared": "*", "chart.js": "4.5.1", + "dompurify": "^3.2.4", "marked": "18.0.4" } }, @@ -556,6 +573,7 @@ "version": "0.0.0", "dependencies": { "@hive/shared": "*", + "dompurify": "^3.2.4", "marked": "18.0.4" } }, diff --git a/frontend/packages/agent/package.json b/frontend/packages/agent/package.json index 0b0776a9..a7e0d5b6 100644 --- a/frontend/packages/agent/package.json +++ b/frontend/packages/agent/package.json @@ -9,7 +9,8 @@ }, "dependencies": { "@hive/shared": "*", - "marked": "18.0.4", - "chart.js": "4.5.1" + "chart.js": "4.5.1", + "dompurify": "^3.2.4", + "marked": "18.0.4" } } diff --git a/frontend/packages/agent/src/app.js b/frontend/packages/agent/src/app.js index e4b46379..9c6bb8f1 100644 --- a/frontend/packages/agent/src/app.js +++ b/frontend/packages/agent/src/app.js @@ -4,6 +4,7 @@ import { create as termCreate, linkify as termLinkify } from '@hive/shared/terminal.js'; import { marked } from 'marked'; +import DOMPurify from 'dompurify'; // Expose the previously-script-tag-provided globals so the IIFE below // keeps working unchanged. Pre-split these were attached by @@ -1429,6 +1430,11 @@ window.marked = marked; // `.md` class (CSS in TERMINAL_CSS scopes paragraph/code/list // styles to it). Falls back to a plain text node if marked isn't // loaded (network glitch, asset 404) so the body still renders. + // `text` is untrusted (peer-agent / matrix-relayed message bodies, + // agent-authored files) — the parsed HTML is run through DOMPurify + // before it ever touches innerHTML, since markdown can carry raw + // HTML/script tags that `marked` itself no longer strips (v5+ + // dropped the built-in sanitizer). function mdNode(text) { const div = document.createElement('div'); div.className = 'md'; @@ -1436,7 +1442,7 @@ window.marked = marked; if (window.marked && typeof window.marked.parse === 'function') { try { marked.setOptions({ breaks: true, gfm: true }); - div.innerHTML = marked.parse(src); + div.innerHTML = DOMPurify.sanitize(marked.parse(src)); // marked autolinks URLs but leaves them same-tab — open them // externally so a click never unloads the terminal. div.querySelectorAll('a[href]').forEach((a) => { diff --git a/frontend/packages/dashboard/package.json b/frontend/packages/dashboard/package.json index de2d7751..05296c8f 100644 --- a/frontend/packages/dashboard/package.json +++ b/frontend/packages/dashboard/package.json @@ -9,6 +9,7 @@ }, "dependencies": { "@hive/shared": "*", + "dompurify": "^3.2.4", "marked": "18.0.4" } } diff --git a/frontend/packages/dashboard/src/common.js b/frontend/packages/dashboard/src/common.js index 2a66ffdf..d1ef70e8 100644 --- a/frontend/packages/dashboard/src/common.js +++ b/frontend/packages/dashboard/src/common.js @@ -4,6 +4,7 @@ // infrastructure for the side panel. import { linkify as termLinkify } from '@hive/shared/terminal.js'; +import DOMPurify from 'dompurify'; // Themed dialog/toast helpers (modal.js imports `el` back from here — a safe // deferred cycle: neither side uses the other at module-init time, only inside // runtime handlers). @@ -543,12 +544,16 @@ function svgImage(text) { return img; } // Marked-rendered markdown node (raw text fallback if `marked` -// failed to load). +// failed to load). `text` is untrusted (agent-authored state files served +// verbatim by /api/state-file) — the parsed HTML is run through DOMPurify +// before it touches innerHTML, since markdown can carry raw HTML/script +// tags that `marked` itself no longer strips (v5+ dropped the built-in +// sanitizer). function mdNode(text) { const div = el('div', { class: 'md' }); if (window.marked && typeof window.marked.parse === 'function') { window.marked.setOptions({ breaks: true, gfm: true }); - div.innerHTML = window.marked.parse(text); + div.innerHTML = DOMPurify.sanitize(window.marked.parse(text)); // marked autolinks URLs but leaves them same-tab — open externally // so a click never navigates away from the dashboard. div.querySelectorAll('a[href]').forEach((a) => { diff --git a/nix/frontend.nix b/nix/frontend.nix index 87d43200..94cd38a3 100644 --- a/nix/frontend.nix +++ b/nix/frontend.nix @@ -40,7 +40,7 @@ buildNpmPackage { # Update whenever the lockfile changes. Recompute locally with the # same command (`pkgs.prefetch-npm-deps`), or let the build fail # and copy the actual hash from the error message. - npmDepsHash = "sha256-3pBR4YDE/ZQ9w7wzIwdGL/n6VSKRtM0m+7ftu1uUOdE="; + npmDepsHash = "sha256-/aklU+l5HqSs4l68gf9J3mgyKM30reBgTkaEjtx2MNY="; # `npm run build` recurses into all workspaces (`--workspaces # --if-present`). The workspaces' build scripts each run their own