Adopt marked + DOMPurify for markdown rendering (D-WEB-7) #174

Closed
opened 2026-09-06 16:21:20 +00:00 by crueber · 3 comments
Owner

Adopt marked + DOMPurify for markdown rendering (D-WEB-7)

Approved direction (user, 2026-09-06; investigation in /tmp/opencode/markdown-options.md): replace the hand-rolled markdown-lite renderer (web/src/lib/markdown.js) and allowlist sanitizer (web/src/lib/sanitize.js) with marked@18.0.11 (MIT) + dompurify@3.4.15 (MPL-2.0-or-Apache-2.0), both zero-dependency (+24.0 KB gzip measured with the repo's esbuild).

Amendment (lands in DEVIATIONS.md + AGENTS.md law-1 line in the same change)

D-WEB-7 — Third-party markdown renderer + sanitizer (2026-09-06, EXPLICIT USER REQUEST). Frontend runtime budget amended from exactly solid-js + @solidjs/router to exactly solid-js + @solidjs/router + marked + dompurify (pinned: marked@18.0.11 MIT, dompurify@3.4.15 MPL-2.0-or-Apache-2.0; both declare zero dependencies — verified zero transitive packages in the bundled output). Rationale: markdown-lite covers CommonMark-subset only; marked is the only candidate that is zero-dep, fully GFM out of the box, multi-maintainer, and HTML-string (drop-in for the innerHTML pipeline); DOMPurify (Cure53, 0 open issues) replaces the regex allowlist. Integration is one wrapper module (web/src/lib/render-md.js, DOMPurify config pinned to explicit allowlists) consumed by the five existing call sites; no other runtime addition without a new amendment.

Migration (per investigation §8)

  1. New web/src/lib/render-md.js (marked GFM + pinned DOMPurify config incl. del,s,input,checked/disabled/type/class); export renderBody(src).
  2. Five call sites to innerHTML={renderBody(...)} (Blob, Tree, Release, IssueNew, ThreadTimeline). No shape change.
  3. DELETE markdown.js + sanitize.js in the same change (pre-1.0 rule).
  4. Tests: rewrite markdown.test.js against render-md.js — marked layer in node --test (option (a): DOMPurify path covered by the real-Chromium pass, not Node); preserve all current assertions + new: strikethrough, task checkboxes (disabled), javascript: dropped, <script> dropped, bare-URL autolink. Conscious choice required: marked emits <code class="language-js"> vs current <code data-lang> — map back or update CSS/tests.
  5. Verify: make test-web, make web, bundle delta ≤ ~25 KB gzip note in commit, real-browser pass (/, task-list/table comment, /setup, console clean).
  6. Docs: D-WEB-7 + implementing doc's Decisions section same commit (law 12).

Acceptance criteria

  • D-WEB-7 written; budget is exactly the four packages (verify package.json + bundle metafile).
  • All five surfaces render GFM (tables/strike/tasks/autolink) with sanitizer gate; no raw-HTML execution.
  • node --test green; make web clean; browser pass dark + light, zero console errors.
# Adopt marked + DOMPurify for markdown rendering (D-WEB-7) Approved direction (user, 2026-09-06; investigation in `/tmp/opencode/markdown-options.md`): replace the hand-rolled markdown-lite renderer (`web/src/lib/markdown.js`) and allowlist sanitizer (`web/src/lib/sanitize.js`) with `marked@18.0.11` (MIT) + `dompurify@3.4.15` (MPL-2.0-or-Apache-2.0), both zero-dependency (+24.0 KB gzip measured with the repo's esbuild). ## Amendment (lands in DEVIATIONS.md + AGENTS.md law-1 line in the same change) > **D-WEB-7 — Third-party markdown renderer + sanitizer (2026-09-06, EXPLICIT USER REQUEST).** Frontend runtime budget amended from exactly `solid-js` + `@solidjs/router` to exactly **`solid-js` + `@solidjs/router` + `marked` + `dompurify`** (pinned: `marked@18.0.11` MIT, `dompurify@3.4.15` MPL-2.0-or-Apache-2.0; both declare zero dependencies — verified zero transitive packages in the bundled output). Rationale: markdown-lite covers CommonMark-subset only; `marked` is the only candidate that is zero-dep, fully GFM out of the box, multi-maintainer, and HTML-string (drop-in for the `innerHTML` pipeline); `DOMPurify` (Cure53, 0 open issues) replaces the regex allowlist. Integration is one wrapper module (`web/src/lib/render-md.js`, DOMPurify config pinned to explicit allowlists) consumed by the five existing call sites; no other runtime addition without a new amendment. ## Migration (per investigation §8) 1. New `web/src/lib/render-md.js` (`marked` GFM + pinned DOMPurify config incl. `del,s,input,checked/disabled/type/class`); export `renderBody(src)`. 2. Five call sites to `innerHTML={renderBody(...)}` (Blob, Tree, Release, IssueNew, ThreadTimeline). No shape change. 3. DELETE `markdown.js` + `sanitize.js` in the same change (pre-1.0 rule). 4. Tests: rewrite `markdown.test.js` against `render-md.js` — marked layer in `node --test` (option (a): DOMPurify path covered by the real-Chromium pass, not Node); preserve all current assertions + new: strikethrough, task checkboxes (disabled), `javascript:` dropped, `<script>` dropped, bare-URL autolink. Conscious choice required: marked emits `<code class="language-js">` vs current `<code data-lang>` — map back or update CSS/tests. 5. Verify: `make test-web`, `make web`, bundle delta ≤ ~25 KB gzip note in commit, real-browser pass (`/`, task-list/table comment, `/setup`, console clean). 6. Docs: D-WEB-7 + implementing doc's Decisions section same commit (law 12). ## Acceptance criteria - [ ] D-WEB-7 written; budget is exactly the four packages (verify `package.json` + bundle metafile). - [ ] All five surfaces render GFM (tables/strike/tasks/autolink) with sanitizer gate; no raw-HTML execution. - [ ] `node --test` green; `make web` clean; browser pass dark + light, zero console errors.
Author
Owner

PR #175 ready for review: #175 — marked@18.0.11 + dompurify@3.4.15 behind render-md.js; 363/363 node green, make web clean, Chromium 14/14 + zero console errors, bundle +22.2 KB gzip. Not merging per instructions.

PR #175 ready for review: https://git.packden.us/crueber/walhub/pulls/175 — marked@18.0.11 + dompurify@3.4.15 behind render-md.js; 363/363 node green, make web clean, Chromium 14/14 + zero console errors, bundle +22.2 KB gzip. Not merging per instructions.
Author
Owner

REVIEW PR #175 (feat/issue-174, commit 4f270f5) — verified in scratch worktrees (since removed); main worktree untouched (still clean on main apart from pre-existing untracked .opencode/).

AMENDMENT / DOCS

  • D-WEB-7 in DEVIATIONS.md matches the issue amendment text verbatim plus an appended measured bundle-delta sentence and Sources line. AGENTS.md law-1 now names exactly the four runtime packages with pins (marked@18.0.11 MIT, dompurify@3.4.15 MPL-2.0-or-Apache-2.0, both zero-dep). 12_web_ui.md carries the §2.2 budget, the render-md pipeline, breaks:true + language-* mapping decisions, and the Decisions-section entry in the same commit (law 12 satisfied).
  • Diff is exactly the 16 expected files: package.json + lock, pnpm-workspace line, render-md.js (new), 5 call sites (Blob, Tree, Release, IssueNew, ThreadTimeline), markdown.js + sanitize.js deleted, 2 rewritten tests, 3 doc files. No Go/backend change.

SANITIZER AUDIT (web/src/lib/render-md.js:31-42) — PASS

  • ALLOWED_TAGS: old core + del/s/input only. No svg, math, form, button, video/audio, link, meta, base, details. on*/style/target absent from ALLOWED_ATTR; style attr stripped (probed). svg/math tags are stripped (probed, incl. onload/onmouseover variants).
  • FORBID_TAGS + FORBID_CONTENTS both pin the old DROP_CONTENT six (script/style/iframe/object/embed/noscript) — content dropped with tags (probed).
  • marked raw-HTML/URI passthrough confirmed gated entirely by DOMPurify (test pins passthrough at the marked layer; renderBody throws without a DOM — fail closed, never raw).
  • Residuals (non-blocking, noted): data: URIs pass on (DOMPurify DATA_URI_TAGS default — old sanitizer would have dropped them; inert in context, scripting disabled for image-document SVG); data-/aria- attrs ride DOMPurify defaults (no JS/CSS consumer — inert); class is unrestricted but Tailwind is build-time-compiled so injected classes are inert; bare survives but is form-less/inert. All probed live with zero execution.

OTHER CHECKS

  • Pins exact (no ^ on the two new; solid-js/router keep pre-existing ^). pnpm install --frozen-lockfile clean, so the lockfile is consistent. minimumReleaseAgeExclude for dompurify@3.4.15 is inert (no minimumReleaseAge set anywhere in-repo) but harmless forward-guard; only adds marked + dompurify + optional @types/trusted-types (type-only) — masks nothing.
  • All 5 call sites use innerHTML={renderBody(...)}; zero markdown.js/sanitize.js/renderMarkdown-legacy references in web/src (only pre-existing comments: Tree.jsx:157 gate comment still true; CommentComposer.jsx:119 'markdown+sanitize' wording predates this PR — trivial nit, left alone).
  • breaks:true pinned by test (paragraph
    contract); data-lang has zero CSS/JS consumers (grep) — language-* mapping sound.
  • Bundle delta RECOMPUTED, not trusted: built main (356.55 kB / 101.34 kB gzip) and PR (422.71 kB / 123.52 kB gzip, byte-identical to the claimed figures) in scratch worktrees → +22.18 KB gzip ≤ 25 KB budget. PASS.

TESTS

  • node --test web/test/unit/*.test.js in scratch: 363 pass / 0 fail (after pnpm install; node_modules absent initially).
  • Browser (hub CDP :9222, HeadlessChrome/151, real renderBody module over innerHTML): 20-assertion XSS suite + 10-assertion obfuscation suite — all pass, window.__pwned* never set (script/style/iframe/object/embed/noscript w/ content, svg/math, img onerror, javascript:/data: URIs incl. tab/newline-obfuscated, nested/conditional/mXSS shapes, form/button/video/base/meta/link stripped; task checkboxes disabled+checked; table/strike/language-*/breaks GFM shapes kept). This covers the DOMPurify enforcement layer per test plan (a); dark/light + console-clean SPA pass remains the author's claim.

MERGE RECOMMENDATION: ready to merge (no fixes pushed — nothing blocking found).

REVIEW PR #175 (feat/issue-174, commit 4f270f5) — verified in scratch worktrees (since removed); main worktree untouched (still clean on main apart from pre-existing untracked .opencode/). AMENDMENT / DOCS - D-WEB-7 in DEVIATIONS.md matches the issue amendment text verbatim plus an appended measured bundle-delta sentence and Sources line. AGENTS.md law-1 now names exactly the four runtime packages with pins (marked@18.0.11 MIT, dompurify@3.4.15 MPL-2.0-or-Apache-2.0, both zero-dep). 12_web_ui.md carries the §2.2 budget, the render-md pipeline, breaks:true + language-* mapping decisions, and the Decisions-section entry in the same commit (law 12 satisfied). - Diff is exactly the 16 expected files: package.json + lock, pnpm-workspace line, render-md.js (new), 5 call sites (Blob, Tree, Release, IssueNew, ThreadTimeline), markdown.js + sanitize.js deleted, 2 rewritten tests, 3 doc files. No Go/backend change. SANITIZER AUDIT (web/src/lib/render-md.js:31-42) — PASS - ALLOWED_TAGS: old core + del/s/input only. No svg, math, form, button, video/audio, link, meta, base, details. on*/style/target absent from ALLOWED_ATTR; style attr stripped (probed). svg/math tags are stripped (probed, incl. onload/onmouseover variants). - FORBID_TAGS + FORBID_CONTENTS both pin the old DROP_CONTENT six (script/style/iframe/object/embed/noscript) — content dropped with tags (probed). - marked raw-HTML/URI passthrough confirmed gated entirely by DOMPurify (test pins passthrough at the marked layer; renderBody throws without a DOM — fail closed, never raw). - Residuals (non-blocking, noted): data: URIs pass on <img> (DOMPurify DATA_URI_TAGS default — old sanitizer would have dropped them; inert in <img> context, scripting disabled for image-document SVG); data-*/aria-* attrs ride DOMPurify defaults (no JS/CSS consumer — inert); class is unrestricted but Tailwind is build-time-compiled so injected classes are inert; bare <input type=text> survives but is form-less/inert. All probed live with zero execution. OTHER CHECKS - Pins exact (no ^ on the two new; solid-js/router keep pre-existing ^). pnpm install --frozen-lockfile clean, so the lockfile is consistent. minimumReleaseAgeExclude for dompurify@3.4.15 is inert (no minimumReleaseAge set anywhere in-repo) but harmless forward-guard; only adds marked + dompurify + optional @types/trusted-types (type-only) — masks nothing. - All 5 call sites use innerHTML={renderBody(...)}; zero markdown.js/sanitize.js/renderMarkdown-legacy references in web/src (only pre-existing comments: Tree.jsx:157 gate comment still true; CommentComposer.jsx:119 'markdown+sanitize' wording predates this PR — trivial nit, left alone). - breaks:true pinned by test (paragraph <br> contract); data-lang has zero CSS/JS consumers (grep) — language-* mapping sound. - Bundle delta RECOMPUTED, not trusted: built main (356.55 kB / 101.34 kB gzip) and PR (422.71 kB / 123.52 kB gzip, byte-identical to the claimed figures) in scratch worktrees → +22.18 KB gzip ≤ 25 KB budget. PASS. TESTS - node --test web/test/unit/*.test.js in scratch: 363 pass / 0 fail (after pnpm install; node_modules absent initially). - Browser (hub CDP :9222, HeadlessChrome/151, real renderBody module over innerHTML): 20-assertion XSS suite + 10-assertion obfuscation suite — all pass, window.__pwned* never set (script/style/iframe/object/embed/noscript w/ content, svg/math, img onerror, javascript:/data: URIs incl. tab/newline-obfuscated, nested/conditional/mXSS shapes, form/button/video/base/meta/link stripped; task checkboxes disabled+checked; table/strike/language-*/breaks GFM shapes kept). This covers the DOMPurify enforcement layer per test plan (a); dark/light + console-clean SPA pass remains the author's claim. MERGE RECOMMENDATION: ready to merge (no fixes pushed — nothing blocking found).
Author
Owner

Implemented in PR #175 (review: sanitizer audited line-by-line + 30-probe browser XSS drive, all stripped; +22.18 KB gzip; 363/363), merged. Closing.

Implemented in PR #175 (review: sanitizer audited line-by-line + 30-probe browser XSS drive, all stripped; +22.18 KB gzip; 363/363), merged. Closing.
crueber added this to the v1 milestone 2026-09-10 22:27:16 +00:00
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
crueber/walhub#174
No description provided.