|
|
|
|
@@ -0,0 +1,523 @@
|
|
|
|
|
---
|
|
|
|
|
phase: quick-261012-s3g
|
|
|
|
|
plan: 01
|
|
|
|
|
type: execute
|
|
|
|
|
wave: 1
|
|
|
|
|
depends_on: []
|
|
|
|
|
files_modified:
|
|
|
|
|
- .planning/notes/2026-10-12-admin-display-sanitizer-dompurify.md
|
|
|
|
|
- .planning/research/STACK.md
|
|
|
|
|
- admin/package.json
|
|
|
|
|
- admin/package-lock.json
|
|
|
|
|
- admin/vitest.config.ts
|
|
|
|
|
- admin/vitest.browser.config.ts
|
|
|
|
|
- admin/src/components/form/richeditor/html.ts
|
|
|
|
|
- admin/tests/browser/richeditor-display.test.ts
|
|
|
|
|
- admin/tests/form/RichEditorField.test.ts
|
|
|
|
|
- admin/tests/views/ReorderView.test.ts
|
|
|
|
|
- README.md
|
|
|
|
|
- CLAUDE.md
|
|
|
|
|
- scripts/check-phase10.sh
|
|
|
|
|
- scripts/check-phase17.sh
|
|
|
|
|
- modules/boardwalk/boardwalk.go
|
|
|
|
|
- modules/boardwalk/boardwalk_test.go
|
|
|
|
|
- modules/boardwalk/README.md
|
|
|
|
|
- docs/backend/admin-spa.md
|
|
|
|
|
- modules/boardwalk/dist
|
|
|
|
|
- .planning/phases/10-admin-vue-spa/10-SECURITY-REVIEW.md
|
|
|
|
|
autonomous: true
|
|
|
|
|
requirements: [QUICK-261012-s3g]
|
|
|
|
|
|
|
|
|
|
estimate:
|
|
|
|
|
tokens: 125000
|
|
|
|
|
raw_tokens: 125000
|
|
|
|
|
tasks: 3
|
|
|
|
|
confidence: low
|
|
|
|
|
|
|
|
|
|
must_haves:
|
|
|
|
|
truths:
|
|
|
|
|
- "displayHtml (admin/src/components/form/richeditor/html.ts) sanitizes through a dedicated DOMPurify 3.4.16 instance: ALLOWED_TAGS = EDITOR_TAGS (lossy.ts), ALLOWED_ATTR = ['href'], ALLOW_DATA_ATTR false, ALLOW_ARIA_ATTR false, an uponSanitizeAttribute hook that keeps href only on <a> and only when isAllowedLink accepts it, ADD_FORBID_CONTENTS = the DROPPED list, KEEP_CONTENT true, string output. The hand-rolled clean() is gone (T-10-16, T-17-10)."
|
|
|
|
|
- "displayHtml parses into DOMPurify's inert DOMParser document and returns a string. No node is adopted into the live document before Vue binds the value, and no image request starts inside displayHtml (the html.ts:102 adoption is removed)."
|
|
|
|
|
- "displayHtml fails closed. Where DOMPurify is unsupported, or a fixed canary does not sanitize to its known answer (as under happy-dom, where DOMPurify leaves nested nodes unsanitized), it returns the empty string and never the stored value."
|
|
|
|
|
- "`npm --prefix admin run test:browser` runs admin/tests/browser/*.test.ts in Playwright Chromium (Vitest browser mode) and passes. It covers form/input name=nodeType with img onerror, img name=nodeType, button name=nodeType, the same nested in a div, input name=localName (no throw), an off-origin form action with a password input, iframe, meta refresh, javascript: href, an href outside the link allowlist (ftp:, sms:), href on a non-<a> element, the inert-parse (no request) check and the read-only RichEditorField mount. Run against the pre-fix html.ts, the suite fails (RED recorded in the SUMMARY)."
|
|
|
|
|
- "`npm --prefix admin test` (happy-dom) and `npm --prefix admin run typecheck` pass. The happy-dom suite excludes tests/browser and keeps only environment-independent 'never as stored' assertions for the read-only view."
|
|
|
|
|
- "The admin CSP (modules/boardwalk/boardwalk.go contentSecurityPolicy) is `frame-ancestors 'none'; base-uri 'none'; object-src 'none'; script-src 'self'; form-action 'self'; frame-src 'none'`. The boardwalk header tests, modules/boardwalk/README.md and docs/backend/admin-spa.md say so."
|
|
|
|
|
- "`bash scripts/check-phase10.sh --hygiene`, `--self-test` and `--all` each exit 0. The raw-HTML rule exempts exactly two more lines, both line-exact and commented with T-17-10: the RichEditorField.vue read-only binding and the lossy.ts inert-template parse. The dist-origin rule additionally allows exactly `https://prosemirror.net` and `https://admin.invalid`. context.ts, ReorderBranch.vue and ReorderView.vue are imported by path in a test."
|
|
|
|
|
- "`bash scripts/check-phase17.sh` runs `check-phase10.sh --hygiene` (docs stage) and the browser suite with required test names (admin stage), and `--all` passes (postgres per Docker availability)."
|
|
|
|
|
- "modules/boardwalk/dist is rebuilt from the new sources and committed. `scripts/check-admin-dist.sh`, `go vet ./...` and `go test ./...` pass."
|
|
|
|
|
- "Decision note .planning/notes/2026-10-12-admin-display-sanitizer-dompurify.md and the STACK.md rows name dompurify 3.4.16, @vitest/browser 3.2.7 and playwright 1.60.0 with legitimacy evidence. 10-SECURITY-REVIEW.md is refreshed (reviewed 2026-10-12) with a removal check for the clobbering vector."
|
|
|
|
|
artifacts:
|
|
|
|
|
- path: admin/src/components/form/richeditor/html.ts
|
|
|
|
|
provides: "DOMPurify-backed displayHtml with the isAllowedLink hook and a fail-closed canary"
|
|
|
|
|
contains: "dompurify"
|
|
|
|
|
- path: admin/vitest.browser.config.ts
|
|
|
|
|
provides: "Vitest browser mode, playwright provider, chromium instance, include tests/browser/**/*.test.ts"
|
|
|
|
|
contains: "playwright"
|
|
|
|
|
- path: admin/tests/browser/richeditor-display.test.ts
|
|
|
|
|
provides: "real-engine regression suite for the clobbering vectors and the read-only RichEditorField"
|
|
|
|
|
contains: "src/components/form/richeditor/html'"
|
|
|
|
|
- path: .planning/notes/2026-10-12-admin-display-sanitizer-dompurify.md
|
|
|
|
|
provides: "decision note for dompurify, @vitest/browser and playwright"
|
|
|
|
|
contains: "3.4.16"
|
|
|
|
|
- path: scripts/check-phase10.sh
|
|
|
|
|
provides: "T-17-10 line-exact raw-HTML exceptions, two allow-listed dist literals, the browser suite in --spa, new self-test plants"
|
|
|
|
|
contains: "T-17-10"
|
|
|
|
|
- path: scripts/check-phase17.sh
|
|
|
|
|
provides: "cross-phase check-phase10 --hygiene run and the browser-suite requirement list"
|
|
|
|
|
contains: "check-phase10.sh"
|
|
|
|
|
- path: modules/boardwalk/boardwalk.go
|
|
|
|
|
provides: "hardened admin CSP"
|
|
|
|
|
contains: "form-action 'self'; frame-src 'none'"
|
|
|
|
|
key_links:
|
|
|
|
|
- from: admin/src/components/form/fields/RichEditorField.vue
|
|
|
|
|
to: admin/src/components/form/richeditor/html.ts
|
|
|
|
|
via: "display = computed(() => readOnly ? displayHtml(stored) : '') bound at RichEditorField.vue:305"
|
|
|
|
|
pattern: "displayHtml\\(stored.value\\)"
|
|
|
|
|
- from: admin/src/components/form/richeditor/html.ts
|
|
|
|
|
to: dompurify
|
|
|
|
|
via: "dedicated instance (DOMPurify(window)) with the uponSanitizeAttribute hook calling isAllowedLink"
|
|
|
|
|
pattern: "uponSanitizeAttribute"
|
|
|
|
|
- from: scripts/check-phase10.sh run_spa
|
|
|
|
|
to: admin/vitest.browser.config.ts
|
|
|
|
|
via: "npm --prefix admin run test:browser, fail-closed on FAIL/skipped/No test files"
|
|
|
|
|
pattern: "test:browser"
|
|
|
|
|
- from: scripts/check-phase17.sh run_docs
|
|
|
|
|
to: scripts/check-phase10.sh
|
|
|
|
|
via: "bash \"$ROOT/scripts/check-phase10.sh\" --hygiene"
|
|
|
|
|
pattern: "check-phase10.sh\" --hygiene"
|
|
|
|
|
---
|
|
|
|
|
|
|
|
|
|
<objective>
|
|
|
|
|
Close the Phase 10 security blockers from `.planning/phases/10-admin-vue-spa/10-SECURITY.md` (g15office u44mzmlf). The full task text is `temp/sec_phase_10.md`; every numbered item and sub-bullet there is in scope, including the optional CSP hardening.
|
|
|
|
|
|
|
|
|
|
- **T-10-16 / T-17-10 (high):** a DOM-clobbering bypass of the hand-rolled `clean()` in `admin/src/components/form/richeditor/html.ts`, bound through `v-html` at `RichEditorField.vue:305`. Fix it with DOMPurify plus a real-browser regression suite, and harden the admin CSP.
|
|
|
|
|
- **T-10-24 (high) / T-10-25 (low):** `scripts/check-phase10.sh --hygiene` and `--self-test` are red since Phase 17. Make them green and make `check-phase17.sh` run the Phase 10 hygiene so cross-phase drift is caught.
|
|
|
|
|
- Rebuild and commit the embedded admin dist, pass every gate, and refresh `10-SECURITY-REVIEW.md`.
|
|
|
|
|
|
|
|
|
|
Purpose: Phase 10 sign-off is blocked (`threats_open: 2`). Stored richeditor values are not sanitized on read, so the read-only/preview view is the only barrier against legacy or imported HTML in the admin origin.
|
|
|
|
|
|
|
|
|
|
Output: the DOMPurify display sanitizer, a Vitest browser-mode suite, the CSP change, gate fixes, the rebuilt dist, a decision note with STACK.md rows, and a refreshed security review.
|
|
|
|
|
|
|
|
|
|
Not in scope (the orchestrator does these after execution):
|
|
|
|
|
- the g15office comment and moving u44mzmlf to Testing;
|
|
|
|
|
- re-running `/msd-secure-phase 10` and Phase 17's security check;
|
|
|
|
|
- STATE.md updates;
|
|
|
|
|
- editing `10-SECURITY.md`.
|
|
|
|
|
</objective>
|
|
|
|
|
|
|
|
|
|
<execution_context>
|
|
|
|
|
@~/.claude/msd-core/workflows/execute-plan.md
|
|
|
|
|
@~/.claude/msd-core/templates/summary.md
|
|
|
|
|
</execution_context>
|
|
|
|
|
|
|
|
|
|
<context>
|
|
|
|
|
@.planning/STATE.md
|
|
|
|
|
@./CLAUDE.md
|
|
|
|
|
@temp/sec_phase_10.md
|
|
|
|
|
@.planning/phases/10-admin-vue-spa/10-SECURITY.md
|
|
|
|
|
@.planning/phases/10-admin-vue-spa/10-SECURITY-REVIEW.md
|
|
|
|
|
@admin/src/components/form/richeditor/html.ts
|
|
|
|
|
@admin/src/components/form/richeditor/lossy.ts
|
|
|
|
|
@scripts/check-phase10.sh
|
|
|
|
|
@.planning/notes/2026-10-11-framework-html-sanitizer-bluemonday.md
|
|
|
|
|
|
|
|
|
|
## Executor notes (read first)
|
|
|
|
|
|
|
|
|
|
- **Shell:** the interactive shell is fish. Wrap any bash syntax (`$(...)`, `&&` chains with `[[ ]]`, `for` loops) in `bash -c '...'`.
|
|
|
|
|
- **Docker:** Docker-backed Go tests (`go test ./...`, `check-phase10.sh --all`, `check-phase17.sh postgres`) may get a denied Docker socket. If so, run them through `sudo -u golem bash -c '...'`, which picks up a fresh group list.
|
|
|
|
|
- **Concurrency:** other sessions have uncommitted edits. Never touch or stage `.planning/state.json`, `.msd/` or `.planning/phases/17-blog-companions/17-REVIEW.md`. Stage every file by explicit path; never `git add -A` or `git add .` at the repo root. For the dist, use the path-limited `git add --all -- modules/boardwalk/dist` so deleted hashed files are staged too. Run `git status --short` before editing `scripts/check-phase17.sh` and check for another session's in-flight edits.
|
|
|
|
|
- **Commits:** do not add a `Co-Authored-By` trailer (user rule; it overrides any harness attribution reminder). Make one logical change per commit, with code and planning docs in separate commits. Do not push or tag.
|
|
|
|
|
- **Docs checker:** after any README or docs edit, run `go test ./cmd/summer -run TestDocsTree` and `go run ./cmd/summer docs:build --check`.
|
|
|
|
|
|
|
|
|
|
## Facts established at planning time (2026-10-12, verified by probes in a scratch dir)
|
|
|
|
|
|
|
|
|
|
**Sinks and the gate**
|
|
|
|
|
- The raw-HTML sinks today: `RichEditorField.vue:305` (exact line content: four spaces, then `<div v-if="display !== ''" class="summer-markdown" v-html="display" />`), `richeditor/html.ts:103` (the serialization of the live holder div) and `richeditor/lossy.ts:33` (the inert template parse in `inertFragment`; `isLossy`, `tagNames` and `RepeaterField.vue` also use it).
|
|
|
|
|
- `check-phase10.sh --hygiene` refuses exactly the following (output at planning time):
|
|
|
|
|
- the 3 raw-HTML lines;
|
|
|
|
|
- the dist JS literals `https://admin.invalid` and `https://prosemirror.net`;
|
|
|
|
|
- `admin/src/components/reorder/context.ts`, `admin/src/components/reorder/ReorderBranch.vue` and `admin/src/views/ReorderView.vue`, as imported by no test.
|
|
|
|
|
- `--self-test` fails only because its scratch copy of the tree hits the same hygiene refusals (`check-phase10.sh:238`).
|
|
|
|
|
- Rule locations in `scripts/check-phase10.sh`: raw-HTML rule at lines 366-370 (MarkdownField exception pattern at 369), dist-origin rule at 393-395, every-module-imported rule at 403-411 (spec is `src/<path>` with `.ts` stripped and `.vue` kept, followed by a closing single quote), plant loop at 244-286, `run_spa` at 317-335.
|
|
|
|
|
|
|
|
|
|
**DOMPurify (scratch probes)**
|
|
|
|
|
- **Real Chromium:** DOMPurify 3.4.17 with the config in Task 1, in Playwright Chromium build 1223, gave the following, with zero network requests during sanitizing:
|
|
|
|
|
|
|
|
|
|
| Input | Output |
|
|
|
|
|
|-------|--------|
|
|
|
|
|
| `<section><p class="x">a</p></section><script>b</script>` | `<p>a</p>` |
|
|
|
|
|
| the read-only test value at `RichEditorField.test.ts:159` | `<p>Hi <a>bad</a> <a href="/ok">ok</a></p>` |
|
|
|
|
|
| form/input name=nodeType + img onerror | empty string |
|
|
|
|
|
| img name=nodeType | empty string |
|
|
|
|
|
| button name=nodeType + iframe | empty string |
|
|
|
|
|
| `<div><form><input name="nodeType"><meta http-equiv=refresh …></form></div>` | `<div></div>` |
|
|
|
|
|
| `<form><input name="localName"><p>t</p></form>` | `<p>t</p>` |
|
|
|
|
|
| off-origin form + password input + button Go | `Go` |
|
|
|
|
|
| iframe + meta refresh + `javascript:` link + style | `<a>x</a>` |
|
|
|
|
|
| `<p onclick="x"><b>a</b><script>b</script><a href="javascript:c">d</a></p>` | `<p><b>a</b><a>d</a></p>` |
|
|
|
|
|
|
|
|
|
|
- **happy-dom 20.11.6:** DOMPurify does NOT sanitize. Inside a vitest happy-dom environment it reports `isSupported` true, but it visits only top-level nodes:
|
|
|
|
|
- the `<section>` input above returns `<p class="x">a</p><script>b</script>`;
|
|
|
|
|
- `<p>one</p>` returns `one`;
|
|
|
|
|
- the clobbering vectors come back nearly intact.
|
|
|
|
|
|
|
|
|
|
This is why Task 1 adds a canary guard, and why exact display assertions live only in the browser suite.
|
|
|
|
|
- **Vite build:** building DOMPurify with the admin's Vite adds only the literal `http://www.w3.org` to the JS, which the dist-origin rule already allows.
|
|
|
|
|
|
|
|
|
|
**Browser for the test suite**
|
|
|
|
|
- Playwright browsers already on disk: `~/.cache/ms-playwright/chromium-1223` and `chromium_headless_shell-1223`, installed by playwright-core 1.60.0. Pinning `playwright` 1.60.0 needs no browser download.
|
|
|
|
|
- If Playwright still reports a missing executable, run `npx --prefix admin playwright install chromium` once and note it in the SUMMARY.
|
|
|
|
|
|
|
|
|
|
**CSP probe (headless Chromium 1223)**
|
|
|
|
|
- Under the full proposed CSP, `frame-src 'none'` blocks an `<iframe src>`, but a `sandbox=""` srcdoc iframe still renders, and its same-origin images load.
|
|
|
|
|
- So the sm-journal-plugin Blog Editor preview pane (`assets/src/blogeditor/element.ts:325`, sandboxed srcdoc) keeps working.
|
|
|
|
|
- No iframe or native off-origin form post exists in `admin/src`.
|
|
|
|
|
|
|
|
|
|
**Package legitimacy (registry checks at planning time)**
|
|
|
|
|
|
|
|
|
|
| Package | Version | Publisher / repo | Released | Verdict |
|
|
|
|
|
|---------|---------|------------------|----------|---------|
|
|
|
|
|
| dompurify | 3.4.16 | cure53 (sole maintainer since the start), github.com/cure53/DOMPurify, license MPL-2.0 OR Apache-2.0, dep `@types/trusted-types` | 2026-09-23 | [VERIFIED]. The lockfile integrity must equal `sha512-sqo+pNp3qRhCIpbgRi1y8Tgk27Bo2Ry7w0dC1NBeNTdZChWjz9Xb/KOoZbRP/R6pQZ80Qw8YhXw13hWWBbMRnQ==`. |
|
|
|
|
|
| @vitest/browser | 3.2.7 | vitest-dev, github.com/vitest-dev/vitest | 2026-07-06 | [VERIFIED]. It must match the installed vitest 3.2.7 exactly (vitest's own peerDependency). |
|
|
|
|
|
| playwright | 1.60.0 | Microsoft, github.com/microsoft/playwright, dep playwright-core 1.60.0 | 2026-05-11 | [VERIFIED]. |
|
|
|
|
|
|
|
|
|
|
- **Why 3.4.16 and not 3.4.17:** 3.4.17 is the latest, but it was published 2026-10-12, hours before planning. Its release notes list `IN_PLACE`/hook fixes and edge-case hardening, with no security advisory. Pin 3.4.16 under a 7-day release cooldown and record 3.4.17 as the next bump in the decision note.
|
|
|
|
|
|
|
|
|
|
**Precedents**
|
|
|
|
|
- **Decision notes:** named `.planning/notes/YYYY-MM-DD-<slug>.md`, with H1 `# Decision: ...`, then Date, Status and Relates-to lines, then the sections Decision, Why, Consequences. See the bluemonday note.
|
|
|
|
|
- **STACK.md:** has no admin/npm section yet (sections at lines 11-58).
|
|
|
|
|
- **Dist rebuilds:** committed separately as `build(...)` commits, e.g. 01bcd3e.
|
|
|
|
|
</context>
|
|
|
|
|
|
|
|
|
|
<tasks>
|
|
|
|
|
|
|
|
|
|
<task type="tracer" tdd="true">
|
|
|
|
|
<name>Task 1: DOMPurify display sanitizer with a real-browser regression suite (T-10-16, T-17-10)</name>
|
|
|
|
|
<files>.planning/notes/2026-10-12-admin-display-sanitizer-dompurify.md, .planning/research/STACK.md, admin/package.json, admin/package-lock.json, admin/vitest.config.ts, admin/vitest.browser.config.ts, admin/src/components/form/richeditor/html.ts, admin/tests/browser/richeditor-display.test.ts, admin/tests/form/RichEditorField.test.ts, README.md, CLAUDE.md</files>
|
|
|
|
|
<behavior>
|
|
|
|
|
Name the suite `describe('richeditor display in a real browser engine')`. check-phase17 requires these names in Task 2, so keep them exact.
|
|
|
|
|
- 'runs where form controls clobber form properties': in a DOMParser document, a form holding an input named nodeType reports a non-number nodeType. This proves the engine reproduces clobbering. It must fail if the file is ever run under happy-dom.
|
|
|
|
|
- 'drops clobbered forms with everything inside them': the vectors are form/input name=nodeType + img src onerror, img name=nodeType, button name=nodeType, and each of these nested in a div. Every output passes the policy check and contains no form, input, img, button, iframe or meta element and no on* attribute.
|
|
|
|
|
- 'keeps a form control named localName from breaking the view': displayHtml of `<form><input name="localName"><p>t</p></form>` does not throw and returns `<p>t</p>`.
|
|
|
|
|
- 'drops an off-origin form, iframes, meta refresh and disallowed links': the vectors are an off-origin form action with a password input, an iframe, a meta refresh and a javascript: href. The output keeps no such element, and every <a> loses a javascript:, ftp: or sms: href. An href on a <p> is dropped, while `<a href="docs/page">`, `<a href="mailto:a@example.test">` and `<a href="/ok">` keep theirs.
|
|
|
|
|
- 'parses into an inert document, so nothing loads before the value is bound': displayHtml runs on input with images pointing at a unique probe URL. A live control img must produce a resource-timing entry, which proves the observation works; the probe URL must produce none.
|
|
|
|
|
- The policy check helper parses an output with `inertFragment` from lossy.ts and requires two things. Every element's lower-cased localName must be in EDITOR_TAGS. The only attribute anywhere must be href on an a element, with isAllowedLink true.
|
|
|
|
|
- `describe('richeditor policy helpers')` 'unwraps unknown elements and drops scripts for display' moves here verbatim from RichEditorField.test.ts:186-189, with the exact `<p>a</p>` result.
|
|
|
|
|
- `describe('RichEditorField')` 'renders read-only values exactly through the display policy': mount RichEditorField read-only (field readOnly true, controlId f-body) with the RichEditorField.test.ts:159 value. The [data-richeditor-readonly] box has no script, no onclick, and a hrefs equal to [undefined, '/ok']. A second mount with the form/input name=nodeType + img onerror vector has no form, input or img in the box.
|
|
|
|
|
</behavior>
|
|
|
|
|
<action>
|
|
|
|
|
**A. Planning-docs commit first.** This project requires a decision note to name a new dependency before the dependency lands.
|
|
|
|
|
|
|
|
|
|
1. Re-run the legitimacy checks and paste the results into the note: `npm view dompurify@3.4.16 dist.integrity maintainers repository.url`, `npm view @vitest/browser@3.2.7 maintainers peerDependencies` and `npm view playwright@1.60.0 maintainers dependencies`. Stop and report if a maintainer, repository or integrity differs from the planning-time table in `<context>`.
|
|
|
|
|
2. Write `.planning/notes/2026-10-12-admin-display-sanitizer-dompurify.md` in the format of the bluemonday note. It must cover:
|
|
|
|
|
- **Decision:**
|
|
|
|
|
- the admin SPA adds dompurify 3.4.16 (exact pin, runtime dependency) for the richeditor read-only display (T-10-16, T-17-10);
|
|
|
|
|
- it adds @vitest/browser 3.2.7 and playwright 1.60.0 (exact pins, devDependencies) for a Vitest browser-mode suite run as `npm --prefix admin run test:browser` from `admin/vitest.browser.config.ts`;
|
|
|
|
|
- list the sanitizer config from step C.
|
|
|
|
|
- **Why:**
|
|
|
|
|
- the clobbering bypass in the hand-rolled walker;
|
|
|
|
|
- the project's preference for a maintained library in security code (the bluemonday precedent);
|
|
|
|
|
- DOMPurify's clobbering checks and prototype-getter traversal;
|
|
|
|
|
- happy-dom cannot reproduce clobbering, and DOMPurify does not sanitize under happy-dom 20.11.6, hence the canary and the browser suite.
|
|
|
|
|
- **Scope vs Phase 10 D-23:** Playwright is used only as the Vitest browser provider for unit-level tests in a real engine. It does not implement the deferred Playwright e2e follow-up from 10-CONTEXT.md.
|
|
|
|
|
- **Legitimacy:** the evidence table from step 1.
|
|
|
|
|
- **Version choice:** 3.4.16 over 3.4.17 (7-day cooldown; 3.4.17 has no security advisory). Bump to 3.4.17 or later after 2026-10-19.
|
|
|
|
|
- **Consequences:**
|
|
|
|
|
- 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;
|
|
|
|
|
- happy-dom tests can no longer assert exact display output;
|
|
|
|
|
- the browser suite needs the Playwright chromium build on the machine.
|
|
|
|
|
3. In `.planning/research/STACK.md`, add a subsection `### Admin SPA (npm) additions` after `### Development Tools`. Use the table shape Library | Version | Purpose | Notes, with rows for dompurify 3.4.16, @vitest/browser 3.2.7 (dev) and playwright 1.60.0 (dev), each pointing to the decision note.
|
|
|
|
|
4. Commit only these two files: `docs(quick-261012-s3g): decision note naming DOMPurify and Vitest browser mode for the admin`.
|
|
|
|
|
|
|
|
|
|
**B. Install with exact pins.**
|
|
|
|
|
|
|
|
|
|
1. Run `npm --prefix admin install --save-exact dompurify@3.4.16`, then `npm --prefix admin install --save-exact --save-dev @vitest/browser@3.2.7 playwright@1.60.0`, then `npm --prefix admin ci`.
|
|
|
|
|
2. Confirm all of the following:
|
|
|
|
|
- `git diff admin/package.json` adds exactly these three entries, with no caret;
|
|
|
|
|
- the dompurify integrity in `admin/package-lock.json` equals the value in `<context>`;
|
|
|
|
|
- `npm --prefix admin audit signatures` passes. If the registry cannot be reached, record that in the SUMMARY.
|
|
|
|
|
|
|
|
|
|
**C. Browser suite, then RED.**
|
|
|
|
|
|
|
|
|
|
1. In `admin/vitest.config.ts`, add an `exclude` of `configDefaults.exclude` plus `tests/browser/**`; import `configDefaults` from `vitest/config`.
|
|
|
|
|
2. Create `admin/vitest.browser.config.ts`:
|
|
|
|
|
- `defineConfig` from `vitest/config`, with the `vue()` plugin;
|
|
|
|
|
- `test.include` set to `['tests/browser/**/*.test.ts']`;
|
|
|
|
|
- `test.browser` set to `enabled: true`, `provider: 'playwright'`, `headless: true`, `screenshotFailures: false`, `instances: [{ browser: 'chromium' }]`;
|
|
|
|
|
- no setupFiles, since `tests/setup.ts` is a happy-dom shim.
|
|
|
|
|
|
|
|
|
|
Confirm these field names against the Vitest 3.2 browser-mode docs (Context7 or v3.vitest.dev/guide/browser). If the first run reloads on dependency optimization, add `optimizeDeps.include` for vue, @vue/test-utils and dompurify.
|
|
|
|
|
3. Add the script `"test:browser": "vitest run --config vitest.browser.config.ts"` to `admin/package.json`.
|
|
|
|
|
4. Write `admin/tests/browser/richeditor-display.test.ts` per `<behavior>`:
|
|
|
|
|
- import displayHtml and isAllowedLink by the path `../../src/components/form/richeditor/html`, EDITOR_TAGS and inertFragment from `../../src/components/form/richeditor/lossy`, and RichEditorField from `../../src/components/form/fields/RichEditorField.vue`;
|
|
|
|
|
- mount with @vue/test-utils;
|
|
|
|
|
- for the resource-timing check, use a token-unique path under the current origin, `performance.getEntriesByType('resource')`, and a control img created in the live document whose load/error event you await;
|
|
|
|
|
- do not use Polish catalogue words or application names, because the hygiene app-name rule scans admin/tests.
|
|
|
|
|
5. Run `npm --prefix admin run test:browser` against the CURRENT html.ts. It must fail at least on the clobbering and inert-document tests. Record the failing test names in the SUMMARY (RED).
|
|
|
|
|
|
|
|
|
|
**D. GREEN: rewrite html.ts.**
|
|
|
|
|
|
|
|
|
|
1. Keep:
|
|
|
|
|
- `LINK_SCHEMES`, `URL_IGNORED`, `URL_SCHEME` and `isAllowedLink` unchanged, including the `https://admin.invalid/` parse base (Task 2 allow-lists the literal);
|
|
|
|
|
- the DROPPED set, as the source of ADD_FORBID_CONTENTS;
|
|
|
|
|
- the `displayHtml(html: string): string` signature and its empty-string short-circuit.
|
|
|
|
|
2. Delete the `clean()` walker, the holder div and the `inertFragment` import.
|
|
|
|
|
3. Create a module-level dedicated instance by calling DOMPurify's default export with `window`, so the hook is not global. Register one `uponSanitizeAttribute` hook: when `data.attrName` is href and either the node's nodeName is not `A` or `isAllowedLink(data.attrValue)` is false, set `data.keepAttr = false`. Reading nodeName is safe here because DOMPurify removes clobbered forms before attributes are visited, and only form elements expose named controls.
|
|
|
|
|
4. Define one frozen config object, typed with DOMPurify's `Config`:
|
|
|
|
|
- `ALLOWED_TAGS` is a spread of `EDITOR_TAGS`;
|
|
|
|
|
- `ALLOWED_ATTR` is `['href']`;
|
|
|
|
|
- `ALLOW_DATA_ATTR` and `ALLOW_ARIA_ATTR` are false;
|
|
|
|
|
- `ADD_FORBID_CONTENTS` is a spread of DROPPED;
|
|
|
|
|
- `KEEP_CONTENT` is true;
|
|
|
|
|
- `RETURN_TRUSTED_TYPE` is false;
|
|
|
|
|
- never `RETURN_DOM`, `RETURN_DOM_FRAGMENT` or `IN_PLACE`, and never pass a Node. The string path parses with DOMParser into an inert document, so nothing is adopted into the live document.
|
|
|
|
|
5. Add a lazily computed, cached `sanitizerUsable()`. It is true only when the instance's `isSupported` is true AND sanitizing the canary `<p onclick="x"><b>a</b><script>b</script><a href="javascript:c">d</a></p>` with the config returns exactly `<p><b>a</b><a>d</a></p>`. displayHtml returns '' when it is false, because DOMPurify returns its input unchanged where it cannot run.
|
|
|
|
|
6. Update the header comment to cite T-10-16, T-17-10 and the decision note. html.ts must not contain any of the four raw-HTML tokens the hygiene rule at `scripts/check-phase10.sh:368` greps for, not even inside a comment: the rule counts comment lines.
|
|
|
|
|
7. Re-run `npm --prefix admin run test:browser` until it is GREEN.
|
|
|
|
|
|
|
|
|
|
**E. Happy-dom suite (`admin/tests/form/RichEditorField.test.ts`).**
|
|
|
|
|
|
|
|
|
|
1. In 'renders read-only values through the display policy, never as stored' (lines 154-168), keep the name and make the assertions environment-independent:
|
|
|
|
|
- the box contains no script and no element with an onclick attribute;
|
|
|
|
|
- the box HTML has no `javascript:`;
|
|
|
|
|
- every remaining a href passes isAllowedLink;
|
|
|
|
|
- `window.__re` stays undefined.
|
|
|
|
|
|
|
|
|
|
Drop the `find('p').attributes` and exact href-list assertions; the browser suite owns them.
|
|
|
|
|
2. Delete 'unwraps unknown elements and drops scripts for display' (lines 186-189); it now lives in the browser suite.
|
|
|
|
|
3. Add a short comment saying happy-dom cannot run DOMPurify, so displayHtml binds nothing there (fail closed), and that exact display assertions live in `tests/browser/richeditor-display.test.ts`.
|
|
|
|
|
4. Run the full `npm --prefix admin test` and fix any other happy-dom test that asserted sanitized read-only richeditor content (planning grep found none outside this file; RepeaterField's read-only test checks structure only).
|
|
|
|
|
|
|
|
|
|
**F. Docs.**
|
|
|
|
|
|
|
|
|
|
1. In `README.md`, in the admin scripts block at lines 150-157, add `npm --prefix admin run test:browser # Vitest browser mode in Playwright Chromium (needs the chromium build; npx --prefix admin playwright install chromium once)`.
|
|
|
|
|
2. In `CLAUDE.md`, change the commands line `npm --prefix admin run test | typecheck | build | dev` to include `test:browser`.
|
|
|
|
|
3. Run the docs checker.
|
|
|
|
|
|
|
|
|
|
**G. Commit.** Commit the code by path (package.json, package-lock.json, both vitest configs, html.ts, both test files, README.md, CLAUDE.md) as `fix(admin): sanitize the richeditor read-only display with DOMPurify (T-10-16)`.
|
|
|
|
|
</action>
|
|
|
|
|
<verify>
|
|
|
|
|
<automated>npm --prefix admin run test:browser && npm --prefix admin test && npm --prefix admin run typecheck && bash -c '! grep -nE "v-html|innerHTML|outerHTML|insertAdjacentHTML" admin/src/components/form/richeditor/html.ts && grep -q "\"dompurify\": \"3.4.16\"" admin/package.json && grep -q "\"@vitest/browser\": \"3.2.7\"" admin/package.json && grep -q "\"playwright\": \"1.60.0\"" admin/package.json && grep -q "sha512-sqo+pNp3qRhCIpbgRi1y8Tgk27Bo2Ry7w0dC1NBeNTdZChWjz9Xb/KOoZbRP/R6pQZ80Qw8YhXw13hWWBbMRnQ==" admin/package-lock.json'</automated>
|
|
|
|
|
</verify>
|
|
|
|
|
<done>
|
|
|
|
|
- The browser suite fails on the pre-fix html.ts (names recorded) and passes on the DOMPurify html.ts.
|
|
|
|
|
- The happy-dom suite and typecheck pass.
|
|
|
|
|
- html.ts has no clean() walker, no live-document holder and no raw-HTML token.
|
|
|
|
|
- The three exact pins and the dompurify integrity are in the committed package.json and lockfile.
|
|
|
|
|
- The decision note and STACK.md rows are committed in their own docs commit, before the code commit.
|
|
|
|
|
</done>
|
|
|
|
|
</task>
|
|
|
|
|
<!-- planner-discipline-allow: v-html -->
|
|
|
|
|
<!-- planner-discipline-allow: innerHTML -->
|
|
|
|
|
|
|
|
|
|
<task type="auto">
|
|
|
|
|
<name>Task 2: Green check-phase10 hygiene and self-test, cross-phase check in check-phase17, reorder imports, CSP hardening (T-10-24, T-10-25, T-10-04)</name>
|
|
|
|
|
<files>scripts/check-phase10.sh, scripts/check-phase17.sh, admin/tests/views/ReorderView.test.ts, modules/boardwalk/boardwalk.go, modules/boardwalk/boardwalk_test.go, modules/boardwalk/README.md, docs/backend/admin-spa.md</files>
|
|
|
|
|
<action>
|
|
|
|
|
**A. `scripts/check-phase10.sh` hygiene.**
|
|
|
|
|
|
|
|
|
|
1. **Raw-HTML rule (lines 366-370).** Add two exceptions next to the MarkdownField one, each line-exact: anchored at the start and end, naming the file path and the full trimmed line content. Each gets a comment line naming T-17-10 and what makes it safe.
|
|
|
|
|
- `admin/src/components/form/fields/RichEditorField.vue`: its read-only display div, whose exact content is given in `<context>`. Comment: binds only displayHtml output, DOMPurify, T-10-16.
|
|
|
|
|
- `admin/src/components/form/richeditor/lossy.ts`: the template parse in inertFragment. Comment: template contents are inert and are read for tag names only, never bound.
|
|
|
|
|
|
|
|
|
|
The RichEditorField line contains two adjacent single quotes, so write that pattern as a double-quoted shell string (or a `-e` argument built from one) rather than inside the existing single-quoted pattern. Do not widen any existing exception, and add none for html.ts: it must have no raw-HTML token.
|
|
|
|
|
2. **Dist-origin rule (line 394).** Extend the `grep -vxE` allow-list with exactly `https://prosemirror\.net` and `https://admin\.invalid`. Add a comment: the first is ProseMirror error text; the second is the reserved-TLD parse base in isAllowedLink. Both are literals and are never requested.
|
|
|
|
|
3. **Self-test plant loop (lines 244-286).** Add three plants, each refused for its own rule and never for "imported by no test":
|
|
|
|
|
- `vhtml-elsewhere`: the exact RichEditorField display line written into `admin/src/__plant/Plant.vue`, imported by the scratch test. Want: raw-HTML directive. This proves the exception is bound to its path.
|
|
|
|
|
- `template-parse`: a `.ts` plant in `admin/src/__plant` assigning markup to a template element the same way lossy.ts does. Want: raw-HTML directive.
|
|
|
|
|
- `origin`: a file `modules/boardwalk/dist/assets/__plant.js` in the scratch copy containing a string literal `https://evil.example/x`. Want: "JS names another origin". Remove that file at the top of each loop iteration, alongside the existing rm.
|
|
|
|
|
4. **`run_spa` (lines 317-335).** After the happy-dom run, run `npm --prefix "$ROOT/admin" run test:browser` the same fail-closed way: capture the log, and refuse on a non-zero exit or on any of `No test files found`, `Unhandled`, `FAIL `, ` skipped` or ` todo`. Print the Test Files/Tests summary lines.
|
|
|
|
|
5. Update the header comment and the comment at line 367 so they describe the new exceptions.
|
|
|
|
|
|
|
|
|
|
**B. `scripts/check-phase17.sh`.**
|
|
|
|
|
|
|
|
|
|
1. Make `vitest_json` pass any extra arguments after `dir` to vitest, so `--config vitest.browser.config.ts` can be added.
|
|
|
|
|
2. Remove `richeditor policy helpers unwraps unknown elements and drops scripts for display` from `ADMIN_REQUIRE`; Task 1 moved it to the browser suite.
|
|
|
|
|
3. Add `ADMIN_BROWSER_FILES=(tests/browser/richeditor-display.test.ts)` and `ADMIN_BROWSER_REQUIRE` with the full names (describe + it) of every test in `admin/tests/browser/richeditor-display.test.ts`. Copy them from a `--reporter=json` run; do not retype them.
|
|
|
|
|
4. In `run_admin`, run the browser suite through `vitest_json` with those lists.
|
|
|
|
|
5. In `run_docs`, after `check-admin-dist.sh`, run `bash "$ROOT/scripts/check-phase10.sh" --hygiene` so cross-phase admin drift fails Phase 17's gate.
|
|
|
|
|
6. In `run_self_test`, add one assertion that the script itself contains that check-phase10 hygiene call.
|
|
|
|
|
7. Update the stage header comments for `admin` and `docs`.
|
|
|
|
|
|
|
|
|
|
**C. Reorder imports (`admin/tests/views/ReorderView.test.ts`).** Add a `describe('reorder modules')` that imports by path:
|
|
|
|
|
- `../../src/views/ReorderView.vue`;
|
|
|
|
|
- `../../src/components/reorder/ReorderBranch.vue`;
|
|
|
|
|
- `REORDER_PAGE` from `../../src/components/reorder/context`.
|
|
|
|
|
|
|
|
|
|
Assert behavior, not a bare import:
|
|
|
|
|
1. On the existing mounted reorder route (reuse the file's helpers and the `tree()` reply), `findComponent(ReorderView)` exists and `findAllComponents(ReorderBranch)` is non-empty.
|
|
|
|
|
2. ReorderBranch mounted standalone, with `global.provide` keyed by `REORDER_PAGE` holding a stub page:
|
|
|
|
|
- refs for dragging, dropTarget and blocked set to null;
|
|
|
|
|
- `nameOf` returning the node name;
|
|
|
|
|
- `canMove` returning true;
|
|
|
|
|
- `vi.fn` handlers.
|
|
|
|
|
|
|
|
|
|
It renders `[data-reorder-node]` for each node with its name, and clicking a move button (`[data-action]`) calls `move` with that node id. Check the `TreeNode` shape in `src/components/reorder/tree.ts`.
|
|
|
|
|
|
|
|
|
|
**D. CSP hardening (optional item from the task file, included per the orchestrator).**
|
|
|
|
|
|
|
|
|
|
1. In `modules/boardwalk/boardwalk.go:32`, set `contentSecurityPolicy` to the old value plus `; form-action 'self'; frame-src 'none'`. Update the comment at lines 29-31 and the SetSecurityHeaders doc at 165-167: no native form posts off-origin, no framed content; sandboxed srcdoc previews stay allowed because they are not fetched.
|
|
|
|
|
2. Update both expected CSP strings in `modules/boardwalk/boardwalk_test.go` (lines 313 and 485).
|
|
|
|
|
3. Update the docs:
|
|
|
|
|
- `modules/boardwalk/README.md`: line 30 (features) names the directives; line 58 (API table) changes "the `script-src 'self'` CSP" to name `script-src 'self'`, `form-action 'self'` and `frame-src 'none'`;
|
|
|
|
|
- `docs/backend/admin-spa.md:18`: list the CSP directives.
|
|
|
|
|
4. Re-run the srcdoc probe from `<context>` with admin's own playwright 1.60.0 from a scratch script. Do not commit the script. Confirm a `sandbox=""` srcdoc iframe still renders under the new header and an `<iframe src>` is blocked, and record the result in the SUMMARY. If srcdoc is blocked, drop only `frame-src 'none'`, keep `form-action 'self'`, and record why.
|
|
|
|
|
|
|
|
|
|
**E. Commits, by path:**
|
|
|
|
|
- `fix(scripts): green check-phase10 hygiene after Phase 17 and run it from check-phase17 (T-10-24, T-10-25)` — the two scripts;
|
|
|
|
|
- `test(admin): import the reorder modules by path` — ReorderView.test.ts;
|
|
|
|
|
- `fix(boardwalk): add form-action and frame-src to the admin CSP` — boardwalk.go, boardwalk_test.go, README.md, docs/backend/admin-spa.md.
|
|
|
|
|
</action>
|
|
|
|
|
<verify>
|
|
|
|
|
<automated>bash scripts/check-phase10.sh --hygiene && bash scripts/check-phase10.sh --self-test && bash scripts/check-phase10.sh --spa && bash scripts/check-phase17.sh --self-test && bash scripts/check-phase17.sh admin && go test ./modules/boardwalk/... -count=1 && go test ./cmd/summer -run TestDocsTree -count=1 && go run ./cmd/summer docs:build --check</automated>
|
|
|
|
|
</verify>
|
|
|
|
|
<done>
|
|
|
|
|
- `--hygiene` prints `phase10 hygiene passed` and `--self-test` prints `phase10 self-test passed`, with the three new plants each refused for their own rule.
|
|
|
|
|
- `--spa` runs both the happy-dom suite and the browser suite.
|
|
|
|
|
- check-phase17's admin stage requires the browser-suite names, and its docs stage calls the Phase 10 hygiene.
|
|
|
|
|
- The boardwalk tests assert the new CSP, and the README and docs match.
|
|
|
|
|
- The srcdoc probe result is recorded.
|
|
|
|
|
</done>
|
|
|
|
|
</task>
|
|
|
|
|
|
|
|
|
|
<task type="auto">
|
|
|
|
|
<name>Task 3: Rebuild and commit the admin dist, run every gate, refresh 10-SECURITY-REVIEW.md with removal checks</name>
|
|
|
|
|
<files>modules/boardwalk/dist, .planning/phases/10-admin-vue-spa/10-SECURITY-REVIEW.md</files>
|
|
|
|
|
<action>
|
|
|
|
|
**A. Rebuild the dist.**
|
|
|
|
|
|
|
|
|
|
1. Run `npm --prefix admin run build`.
|
|
|
|
|
2. Stage with `git add --all -- modules/boardwalk/dist` and commit as `build(quick-261012-s3g): rebuild admin dist with the DOMPurify display sanitizer`.
|
|
|
|
|
3. Then run `bash scripts/check-admin-dist.sh` and `bash scripts/check-phase10.sh --hygiene`. If the dist-origin rule reports any literal other than the allow-listed ones, inspect where it occurs in the minified JS. Stop and report rather than allow-listing a URL the code actually loads.
|
|
|
|
|
|
|
|
|
|
**B. Gates, in this order.** Use `sudo -u golem bash -c '...'` for Docker-backed runs if the socket is denied.
|
|
|
|
|
|
|
|
|
|
1. `go vet ./...`
|
|
|
|
|
2. `go test ./...`
|
|
|
|
|
3. `npm --prefix admin run typecheck`
|
|
|
|
|
4. `npm --prefix admin test`
|
|
|
|
|
5. `npm --prefix admin run test:browser`
|
|
|
|
|
6. `bash scripts/check-admin-dist.sh`
|
|
|
|
|
7. `bash scripts/check-phase10.sh --all`
|
|
|
|
|
8. `bash scripts/check-phase17.sh` (default `--all`). If no Docker daemon answers, it prints NOT RUN postgres; then run `bash scripts/check-phase17.sh postgres` on the Docker host via sudo.
|
|
|
|
|
|
|
|
|
|
A failure caused by this plan's changes is fixed here. A failure that also fails on a clean checkout of HEAD before Task 1 (for example in a sibling repository) goes to `.planning/quick/261012-s3g-fix-phase-10-security-blockers-t-10-16-d/deferred-items.md` with the command and the output tail, and is named in the SUMMARY. Do not mark the review green while any gate is red.
|
|
|
|
|
|
|
|
|
|
**C. Removal checks.** For each mutation: apply it, run the command, record the exit code, restore the file, and confirm `git status --short` shows only the pre-existing foreign edits.
|
|
|
|
|
|
|
|
|
|
| Threat | Mutation | Command | Expected exit |
|
|
|
|
|
|--------|----------|---------|---------------|
|
|
|
|
|
| T-10-16 (a) | Restore the pre-fix html.ts from the commit before Task 1's code commit | `npm --prefix admin run test:browser` | 1 |
|
|
|
|
|
| T-10-16 (b) | Make sanitizerUsable return true unconditionally | `npx --prefix admin vitest run tests/form/RichEditorField.test.ts` (happy-dom) | 1, because DOMPurify keeps the script under happy-dom |
|
|
|
|
|
| T-10-16 (c) | Remove the uponSanitizeAttribute hook | `npm --prefix admin run test:browser` | 1, because the ftp: and sms: hrefs or the href on p survive |
|
|
|
|
|
| T-10-04 | Drop `form-action 'self'` from the CSP | `go test ./modules/boardwalk -run 'TestSecurityHeadersOnEveryResponse|TestPhase101BoardwalkExports' -count=1` | 1 |
|
|
|
|
|
| T-10-24 | — | `bash scripts/check-phase10.sh --self-test`, with the new plants | each plant refused |
|
|
|
|
|
| T-10-SC | Change the dompurify pin in a scratch copy of admin/package.json | `npm ci --dry-run --offline` against the committed lockfile | 1 |
|
|
|
|
|
|
|
|
|
|
**D. Refresh `.planning/phases/10-admin-vue-spa/10-SECURITY-REVIEW.md`.**
|
|
|
|
|
|
|
|
|
|
1. Set the frontmatter to `reviewed: "2026-10-12"`. Keep `threats_open: 0` only if every gate in B passed.
|
|
|
|
|
2. Add a short paragraph under the H1: the 2026-10-12 refresh re-reviews the admin surface after Phases 12.1-17 (17-02 added the read-only richeditor sink), and names the DOMPurify fix, the browser suite and the gate repair.
|
|
|
|
|
3. Rewrite these rows, keeping the columns, the `| T-10-NN ` row prefix and a named test, gate or `tests/` path in every high mitigate row (`run_evidence` checks this at check-phase10.sh:434-441):
|
|
|
|
|
- **T-10-04:** the new CSP directives.
|
|
|
|
|
- **T-10-16:** the production mitigation is now Vue text interpolation everywhere except two reviewed sinks: MarkdownField (server-rendered) and the RichEditorField read-only view through DOMPurify displayHtml with the canary. Tests: `tests/browser/richeditor-display.test.ts`, `tests/form/RichEditorField.test.ts`, `--hygiene` with line-exact T-17-10 exceptions, `--self-test` plants; stages `--spa`, `--hygiene`. Residual: the CSP still allows inline styles and off-origin images in rendered content.
|
|
|
|
|
- **T-10-22:** also names listState.ts (T-16.1-03).
|
|
|
|
|
- **T-10-24:** green on 2026-10-12, plus the check-phase17 cross-phase hygiene call.
|
|
|
|
|
- **T-10-25:** the two allow-listed literals and the reorder path imports.
|
|
|
|
|
- **T-10-SC:** dompurify 3.4.16, @vitest/browser 3.2.7 and playwright 1.60.0 added under the decision note, with exact pins, the committed lockfile and `npm ci`.
|
|
|
|
|
4. Add the removal-check rows from C to the Removal check table, keeping the existing rows.
|
|
|
|
|
5. Update Residual risk with:
|
|
|
|
|
- stored richeditor HTML is still not sanitized on read server-side;
|
|
|
|
|
- the display relies on DOMPurify plus the CSP;
|
|
|
|
|
- the 3.4.17 bump is pending after its cooldown.
|
|
|
|
|
6. Run `bash scripts/check-phase10.sh --evidence`. It must pass; it also re-runs the security, postgres and openapi stages.
|
|
|
|
|
7. Commit only this file as `docs(10): refresh security review evidence after the DOMPurify fix (T-10-16, T-10-24)`.
|
|
|
|
|
|
|
|
|
|
Do not edit 10-SECURITY.md, STATE.md or anything under `.planning/phases/17-blog-companions/`; the orchestrator owns them.
|
|
|
|
|
</action>
|
|
|
|
|
<verify>
|
|
|
|
|
<automated>bash scripts/check-admin-dist.sh && bash scripts/check-phase10.sh --all && bash scripts/check-phase17.sh && go vet ./... && go test ./... && npm --prefix admin test && npm --prefix admin run test:browser && npm --prefix admin run typecheck && bash -c 'grep -q "reviewed: \"2026-10-12\"" .planning/phases/10-admin-vue-spa/10-SECURITY-REVIEW.md && grep -q "tests/browser/richeditor-display.test.ts" .planning/phases/10-admin-vue-spa/10-SECURITY-REVIEW.md'</automated>
|
|
|
|
|
</verify>
|
|
|
|
|
<done>
|
|
|
|
|
- The dist is rebuilt and committed in its own build commit.
|
|
|
|
|
- Every listed gate exits 0 (or a pre-existing foreign failure is logged in deferred-items.md and named in the SUMMARY).
|
|
|
|
|
- The removal checks are recorded with their exit codes, and the tree is restored.
|
|
|
|
|
- 10-SECURITY-REVIEW.md is refreshed (2026-10-12), passes `--evidence`, and is committed as a planning-docs commit.
|
|
|
|
|
</done>
|
|
|
|
|
</task>
|
|
|
|
|
|
|
|
|
|
</tasks>
|
|
|
|
|
|
|
|
|
|
<threat_model>
|
|
|
|
|
## Trust Boundaries
|
|
|
|
|
|
|
|
|
|
| Boundary | Description |
|
|
|
|
|
|----------|-------------|
|
|
|
|
|
| Stored richeditor HTML → admin DOM | Legacy, imported or pre-gate values, never sanitized on read, rendered in the admin origin by the read-only/preview view |
|
|
|
|
|
| npm registry → admin build | New packages (dompurify at runtime; @vitest/browser and playwright for tests) enter the lockfile and the committed dist |
|
|
|
|
|
| Admin HTML responses → browser | CSP decides what injected markup can do: submit forms, frame content, run script |
|
|
|
|
|
| Gate scripts → phase acceptance | Hygiene exceptions and allow-lists decide what counts as clean |
|
|
|
|
|
|
|
|
|
|
## STRIDE Threat Register
|
|
|
|
|
|
|
|
|
|
| Threat ID | Category | Component | Severity | Disposition | Mitigation Plan |
|
|
|
|
|
|-----------|----------|-----------|----------|-------------|-----------------|
|
|
|
|
|
| T-s3g-01 | Tampering | `displayHtml` (DOM clobbering via form named controls) | high | mitigate | DOMPurify dedicated instance with the EDITOR_TAGS/href-only config. The canary fails closed. The real-Chromium suite covers every reported vector; removal check (a) proves it. |
|
|
|
|
|
| T-s3g-02 | Tampering | `displayHtml` in a DOM without a faithful walker (happy-dom, a future runtime) | medium | mitigate | `sanitizerUsable()` canary plus `isSupported`, returning '' otherwise. Removal check (b) proves the happy-dom test catches its removal. |
|
|
|
|
|
| T-s3g-03 | Information Disclosure | Requests started while sanitizing (img src, tracking) | medium | mitigate | String-mode DOMPurify parses with DOMParser into an inert document; the live-holder adoption is removed. The resource-timing test has a positive control. |
|
|
|
|
|
| T-s3g-04 | Tampering | Links outside the editor allowlist (javascript:, ftp:, sms:, href on non-a) | medium | mitigate | `uponSanitizeAttribute` hook with `isAllowedLink` plus DOMPurify's URI check. Removal check (c). |
|
|
|
|
|
| T-s3g-05 | Spoofing | Phishing form or frame overlay injected into the admin | medium | mitigate | CSP `form-action 'self'; frame-src 'none'` (boardwalk tests assert it). Probe-verified not to break sandboxed srcdoc previews. |
|
|
|
|
|
| T-s3g-06 | Repudiation | Gate exceptions too broad, hiding a new sink or origin | medium | mitigate | Line-exact, path-bound, commented exceptions, plus self-test plants: path-moved display line, template parse, foreign origin in dist. |
|
|
|
|
|
| T-s3g-SC | Tampering | npm installs (dompurify, @vitest/browser, playwright) | high | mitigate | All three were [VERIFIED] at planning (maintainers, repo, release date) and are re-checked by the executor before install. Exact pins, the dompurify integrity matched in the lockfile, `npm audit signatures`, `npm ci`, decision note, 7-day cooldown version choice. |
|
|
|
|
|
</threat_model>
|
|
|
|
|
|
|
|
|
|
<verification>
|
|
|
|
|
- `npm --prefix admin run test:browser` is RED on the pre-fix html.ts and GREEN after the fix.
|
|
|
|
|
- `npm --prefix admin test` and `npm --prefix admin run typecheck` pass.
|
|
|
|
|
- These all exit 0:
|
|
|
|
|
- `bash scripts/check-phase10.sh --hygiene`
|
|
|
|
|
- `bash scripts/check-phase10.sh --self-test`
|
|
|
|
|
- `bash scripts/check-phase10.sh --all`
|
|
|
|
|
- `bash scripts/check-phase17.sh` (postgres on a Docker host)
|
|
|
|
|
- `bash scripts/check-admin-dist.sh`
|
|
|
|
|
- `go vet ./...` and `go test ./...`
|
|
|
|
|
- `go test ./cmd/summer -run TestDocsTree` and `summer docs:build --check`
|
|
|
|
|
- The removal checks in Task 3 are recorded in 10-SECURITY-REVIEW.md.
|
|
|
|
|
- `git log` shows planning-docs commits (decision note + STACK.md; security review) separate from code commits (admin fix; scripts; reorder test; boardwalk CSP; dist build), none with a Co-Authored-By trailer.
|
|
|
|
|
</verification>
|
|
|
|
|
|
|
|
|
|
<success_criteria>
|
|
|
|
|
- T-10-16 / T-17-10: the clobbering bypass is closed by DOMPurify, proven in a real browser engine, with a fail-closed fallback and a CSP that blocks off-origin form posts and framed content.
|
|
|
|
|
- T-10-24: `check-phase10.sh --all` is green again, and check-phase17 runs the Phase 10 hygiene so the drift cannot recur silently.
|
|
|
|
|
- T-10-25: the two literal origins are allow-listed with reasons, and the reorder modules are imported by path in a behavior test.
|
|
|
|
|
- The embedded dist matches the sources; the decision note, STACK.md and 10-SECURITY-REVIEW.md record the change.
|
|
|
|
|
</success_criteria>
|
|
|
|
|
|
|
|
|
|
## Source coverage audit
|
|
|
|
|
|
|
|
|
|
| Source item (temp/sec_phase_10.md) | Covered by |
|
|
|
|
|
|------------------------------------|-----------|
|
|
|
|
|
| 1. Replace clean() with DOMPurify: ALLOWED_TAGS = EDITOR_TAGS, ALLOWED_ATTR ['href'], uponSanitizeAttribute + isAllowedLink, keep DROPPED-with-content | Task 1 C-D |
|
|
|
|
|
| 1. Decision note in .planning/notes/ | Task 1 A |
|
|
|
|
|
| 1. Add to .planning/research/STACK.md | Task 1 A |
|
|
|
|
|
| 1. Exact pin, legitimacy check, commit lockfile, npm ci | Task 1 A-B (planning-time [VERIFIED] table plus executor re-check) |
|
|
|
|
|
| 1. Sanitize into an inert document; no adoption into the live document (html.ts:102) | Task 1 D, inert-document test |
|
|
|
|
|
| 1. Real-browser regression test (Vitest browser mode + Playwright Chromium) covering every listed vector | Task 1 C (behavior list) |
|
|
|
|
|
| 1. Optional CSP `form-action 'self'; frame-src 'none'` + header tests, README, docs | Task 2 D |
|
|
|
|
|
| 2. Line-exact commented T-17-10 exceptions for the remaining raw-HTML lines | Task 2 A.1 |
|
|
|
|
|
| 2. Allow-list https://prosemirror.net and https://admin.invalid | Task 2 A.2 |
|
|
|
|
|
| 2. Import reorder context.ts, ReorderBranch.vue, ReorderView.vue by path in a test | Task 2 C |
|
|
|
|
|
| 2. check-phase17.sh runs check-phase10.sh --hygiene | Task 2 B |
|
|
|
|
|
| 3. Rebuild admin, commit dist, pass check-admin-dist, check-phase10 --all, check-phase17, go vet/test, npm test/typecheck | Task 3 A-B |
|
|
|
|
|
| 4. Refresh 10-SECURITY-REVIEW.md (new date, removal check for the clobbering vector) | Task 3 C-D |
|
|
|
|
|
| 4. g15office comment + move u44mzmlf to Testing; re-run /msd-secure-phase 10 and Phase 17's security check | EXCLUDED: the orchestrator does these after execution |
|
|
|
|
|
| Code and planning docs in separate commits; stage by path | Every task's commit steps |
|
|
|
|
|
|
|
|
|
|
<output>
|
|
|
|
|
Create `.planning/quick/261012-s3g-fix-phase-10-security-blockers-t-10-16-d/261012-s3g-SUMMARY.md` when done. It must include:
|
|
|
|
|
- the RED test names from Task 1;
|
|
|
|
|
- the srcdoc probe result;
|
|
|
|
|
- the gate results;
|
|
|
|
|
- the removal-check exit codes;
|
|
|
|
|
- any deferred items.
|
|
|
|
|
</output>
|
