docs(quick-261012-s3g): decision note naming DOMPurify and Vitest browser mode for the admin
This commit is contained in:
@@ -0,0 +1,61 @@
|
||||
# Decision: DOMPurify sanitizes the admin richeditor read-only display
|
||||
|
||||
**Date:** 2026-10-12
|
||||
**Status:** Accepted (quick task 261012-s3g, closes T-10-16 / T-17-10)
|
||||
**Relates to:** `.planning/notes/2026-10-11-framework-html-sanitizer-bluemonday.md` (the server-side gate on every richeditor save), `.planning/phases/10-admin-vue-spa/10-SECURITY.md`
|
||||
|
||||
## Decision
|
||||
|
||||
The admin SPA (`admin/package.json`) adds three npm packages, each with an exact pin and the committed `package-lock.json`:
|
||||
|
||||
| Package | Version | Kind | Use |
|
||||
|---------|---------|------|-----|
|
||||
| `dompurify` | 3.4.16 | runtime dependency | sanitizes the richeditor read-only display (`displayHtml` in `admin/src/components/form/richeditor/html.ts`) |
|
||||
| `@vitest/browser` | 3.2.7 | devDependency | Vitest browser mode, matching the installed vitest 3.2.7 exactly (its peerDependency) |
|
||||
| `playwright` | 1.60.0 | devDependency | the browser-mode provider (Chromium) |
|
||||
|
||||
The browser suite runs as `npm --prefix admin run test:browser` from `admin/vitest.browser.config.ts` and covers `admin/tests/browser/**/*.test.ts`. The happy-dom suite (`npm --prefix admin test`) excludes `tests/browser`.
|
||||
|
||||
`displayHtml` uses a dedicated DOMPurify instance (`DOMPurify(window)`, so the hook is not global) with one frozen config:
|
||||
|
||||
- `ALLOWED_TAGS`: the editor's elements, `EDITOR_TAGS` from `lossy.ts`;
|
||||
- `ALLOWED_ATTR`: `['href']`; `ALLOW_DATA_ATTR` and `ALLOW_ARIA_ATTR` false;
|
||||
- an `uponSanitizeAttribute` hook that keeps `href` only on `<a>` and only when `isAllowedLink` accepts it (http, https, mailto, tel or a relative URL);
|
||||
- `ADD_FORBID_CONTENTS`: the former `DROPPED` list (script, style, template, noscript, iframe, frame, frameset, object, embed, applet, svg, math, title, head, meta, link, base), so those elements go with their content;
|
||||
- `KEEP_CONTENT` true: any other element is unwrapped and keeps its text;
|
||||
- string input and string output (`RETURN_TRUSTED_TYPE` false); never `RETURN_DOM`, `RETURN_DOM_FRAGMENT` or `IN_PLACE`. DOMPurify parses the string with `DOMParser` into an inert document, so nothing is adopted into the live document and no image request starts while sanitizing.
|
||||
|
||||
`displayHtml` fails closed: it returns the empty string unless DOMPurify reports `isSupported` and a fixed canary (`<p onclick="x"><b>a</b><script>b</script><a href="javascript:c">d</a></p>`) sanitizes to exactly `<p><b>a</b><a>d</a></p>`. The stored value is never bound.
|
||||
|
||||
## Why
|
||||
|
||||
- **The hand-rolled walker was bypassable.** `clean()` read `nodeType`, `localName`, `childNodes` and `attributes` from each element. A `<form>` holding `<input name="nodeType">` (or `name="localName"`, or an `img`/`button` named that way) clobbers those properties, so the walker skipped the form and everything inside it, including an `<img onerror>`, and the result was bound with a raw-HTML directive at `RichEditorField.vue:305`.
|
||||
- **Maintained library for security code.** The project prefers a maintained sanitizer over a hand-built one (bluemonday on the server, the Journal note's rejection of a hand-built allowlist). DOMPurify is the de-facto browser sanitizer, has explicit DOM-clobbering checks (`_isClobbered`, removal of clobbered forms before attributes are visited) and reads node properties through prototype getters captured at load, not through the element itself.
|
||||
- **happy-dom cannot test it.** happy-dom does not reproduce form-control clobbering, and under happy-dom 20.11.6 DOMPurify reports `isSupported` true yet visits only top-level nodes: `<section><p class="x">a</p></section><script>b</script>` comes back as `<p class="x">a</p><script>b</script>`. Hence the canary (fail closed where the walker is not faithful) and a regression suite in a real engine.
|
||||
|
||||
## Scope vs Phase 10 D-23
|
||||
|
||||
Playwright is used only as the Vitest browser-mode provider, for unit-level tests that need a real engine. It does not implement the deferred Playwright end-to-end follow-up recorded in `10-CONTEXT.md` (D-23); that stays deferred.
|
||||
|
||||
## Legitimacy
|
||||
|
||||
Re-checked on 2026-10-12 before install, matching the planning-time table:
|
||||
|
||||
| Package | Version | Check | Result |
|
||||
|---------|---------|-------|--------|
|
||||
| dompurify | 3.4.16 | `npm view dompurify@3.4.16 dist.integrity maintainers repository.url` | integrity `sha512-sqo+pNp3qRhCIpbgRi1y8Tgk27Bo2Ry7w0dC1NBeNTdZChWjz9Xb/KOoZbRP/R6pQZ80Qw8YhXw13hWWBbMRnQ==`; maintainer `cure53 <mario@cure53.de>` (sole maintainer since the start); repo `github.com/cure53/DOMPurify`; license `(MPL-2.0 OR Apache-2.0)`; one dependency `@types/trusted-types ^2.0.7`; released 2026-09-23 |
|
||||
| @vitest/browser | 3.2.7 | `npm view @vitest/browser@3.2.7 maintainers peerDependencies` | maintainers ariperkkio, antfu, hiogawa, oreanno, yyx990803 (vitest-dev); repo `github.com/vitest-dev/vitest`; peer `vitest: 3.2.7`, `playwright: *`; released 2026-07-06 |
|
||||
| playwright | 1.60.0 | `npm view playwright@1.60.0 maintainers dependencies` | maintainers pavelfeldman, yurys, dgozman-ms, playwright-bot (Microsoft); repo `github.com/microsoft/playwright`; one dependency `playwright-core 1.60.0`; released 2026-05-11 |
|
||||
|
||||
The lockfile's dompurify integrity must equal the value above; `npm ci` installs from the committed lockfile.
|
||||
|
||||
## Version choice
|
||||
|
||||
3.4.17 is the latest release, but it was published on 2026-10-12, hours before this decision. Its release notes list `IN_PLACE` and hook fixes and edge-case hardening, with no security advisory. 3.4.16 is pinned under a 7-day release cooldown. Bump to 3.4.17 or later after 2026-10-19.
|
||||
|
||||
## Consequences
|
||||
|
||||
- The read-only display drops the content of DOMPurify's default content-forbidden elements (thead, colgroup, audio, video and others) on top of the `DROPPED` list. The editor itself never produces them.
|
||||
- happy-dom tests can no longer assert exact display output: `displayHtml` returns the empty string there. Exact assertions live in `admin/tests/browser/richeditor-display.test.ts`.
|
||||
- The browser suite needs the Playwright Chromium build on the machine (`npx --prefix admin playwright install chromium` once).
|
||||
- The committed admin dist grows by the DOMPurify bundle; the only URL literal it adds is `http://www.w3.org`, which the dist-origin rule already allows.
|
||||
Reference in New Issue
Block a user