site-builder/docs/superpowers/specs/2026-07-12-site-builder-security-hardening-design.md

Site Builder — security & data-loss hardening (2026-07-12)

Fixes the Critical (stored-XSS / HTML-breakout) and High (state-corruption and save data-loss) findings from the 2026-07-12 Fable audit of the Craft.js site builder (/workspace/site-builder/craft/). Medium/Low findings are deferred to the audit backlog (in memory), not this spec. Sequenced before the asset-picker work per user decision. Every claim below was verified against the code before writing this spec.

1 — Escaping cluster (Critical, do first: closes the most with one change)

Verified root cause: escapeHtml exists once (utils/html-export.ts:201) but is not exported and no component uses it; there are 27 local esc/escapeHtml definitions across src/, in three incompatible variants (some omit &, some omit quotes entirely, none escape '), and several land in attribute or JS-string contexts where HTML-entity escaping is wrong anyway.

1a — Shared escaping/URL util

New craft/src/utils/escape.ts (exported, single source of truth):

export function escapeHtml(s: string): string;   // & < > "  — text/content context
export function escapeAttr(s: string): string;   // escapeHtml + '  — attribute context
export function safeUrl(s: string): string;      // returns '' (or '#') for javascript:/data:text/html/vbscript: schemes

• Move the correct body from html-export.ts into escape.ts; html-export.ts imports it. Delete all 27 local copies and import from escape.ts.
• safeUrl wraps every exported href/src/action/url() value: ButtonLink, Icon, SocialLinks, Logo, Menu, Navbar, _cta-helpers, ContentSlider, FeaturesGrid, PricingTable, ImageBlock, VideoBlock, Gallery, background images. Attribute values use escapeAttr; text nodes use escapeHtml.

1b — JS-string / raw-HTML contexts (entity escaping is NOT enough here)

• HtmlBlock (basic/HtmlBlock.tsx:144): toHtml returns props.code raw while the render sanitizes via the file's own purifyHtml. Fix: return { html: purifyHtml(props.code || '') }. (DOMPurify already a dep.)
• Countdown (sections/Countdown.tsx:298): new Date("${targetDate}") inside an inline <script>. Fix: validate /^\d{4}-\d{2}-\d{2}(T…)?$/ and/or emit via JSON.stringify(targetDate); reject/blank otherwise.
• Gallery (sections/Gallery.tsx:296): onclick="${galleryId}_open('${esc(img.src)}')" — esc doesn't escape '. Fix: emit data-src="${escapeAttr(img.src)}" and read it via getAttribute in the lightbox handler (no inline string interpolation).

1c — cssPropsToString (utils/style-helpers.ts)

Does zero escaping, so free-text style props (e.g. Section "Background Gradient", background-image url()) can break out of style="…". Escape values destined for style (strip "/; appropriately, safeUrl for url() contents).

2 — Copy / Paste / Duplicate reuse node IDs (High, state corruption)

Verified: ContextMenu.duplicate (:65) computes a fresh clone (parseSerializedNode(...).toNode(), :75) then discards it and calls actions.addNodeTree(freshTree, parentId) with the original tree — reusing original ids. ContextMenu.pasteNode (:97) and useKeyboardShortcuts duplicate (:76) both toNodeTree() → addNodeTree with original ids. Duplicating/pasting while the source still exists yields duplicate node ids → corrupted editor state.

Fix: one shared helper that regenerates ids for a whole tree before insertion — use Craft's query.parseFreshNode per node, or clone the NodeTree remapping every id (and nodes/parent/linkedNodes refs) to fresh ids. Apply in all three call sites. Also: pasteNode targets the right-clicked node as parent (:97) — pasting onto a leaf throws; insert as a sibling (use the target's parent, fall back to ROOT).

3 — Header/Footer save corruption (High, silent data loss)

Verified: useWhpApi.save (hooks/useWhpApi.ts:12) serializes the live canvas (query.serialize()) into craft_state and into whichever page has id === activePageId, while header_craft_state/footer_craft_state and header_html/footer_html are exported from the stale headerPage.craftState/ footerPage.craftState (:32,:44,:102-103). When editing the header/footer, activePageId is __header__/__footer__ — matching no page — so live header/footer edits are written into top-level craft_state/html (mislabeled) and the real header/footer slots stay stale. Auto-save fires every 30s (TopBar.tsx) regardless of edit mode → silent loss.

Fix: before serializing, commit the live canvas into the active zone's state (PageContext's existing save-current-state path — confirm exact name during implementation), then branch in save() on isEditingHeader/isEditingFooter: route the live serialization to header_craft_state/footer_craft_state (and header_html/footer_html) rather than the top-level page slots, and skip mis-assigning it to a non-existent active page.

4 — AI-boundary validation (High)

utils/apply-ai-response.ts deserializes AI output into craft state with no guards: buildNodeTree reads node.props.node_id/node.type.resolvedName unchecked (:32,:37,:75,:81); update_props does blind Object.assign(p, op.props) (:247) — can overwrite node_id/style; AI-supplied node_id is used directly as a map key (:32) with no ROOT/uniqueness check; an unknown resolvedName throws at addNodeTree. The live PageContext.treeToState already guards (:378,:380) — this path doesn't.

Fix: validate every node's resolvedName against componentResolver before insertion; allowlist update_props keys per component (never node_id); ensure generated/patched ids are unique and non-ROOT; fail soft (skip the op + log) rather than throwing into addNodeTree. Reuse the treeToState guard logic (ties into backlog item "consolidate the three tree-flatteners", but only the validation is in scope here).

Testing

Currently no tests cover toHtml escaping or the state layer. Add:

• craft/src/utils/escape.test.ts — escapeHtml/escapeAttr/safeUrl incl. javascript:/data:text/html rejection and ' handling.
• toHtml escaping regression tests for the fixed components (HtmlBlock purifies; Countdown rejects a malicious date; Gallery uses data-src; a "-containing href/src stays inside its attribute; javascript: href is neutralized).
• Node-id regeneration test: duplicate/paste yields all-fresh ids (no collision with the source subtree).
• Header/footer save test: editing the header then saving writes live content to header_craft_state/header_html, not the top-level page slot.
• AI-boundary: unknown resolvedName and a node_id-overwriting update_props are rejected without throwing.
• Full cd craft && npx vitest run + npm run build green.

Sequencing / shippability

Independent, each shippable alone; recommended order = the section order above (1 first — highest risk, closes the most, and the escaping tests lock in the rest). Ships via the standard WHP release pipeline (whp-deploy-release skill) after implementation + review. The asset-picker spec (2026-07-12-site-builder-asset-picker-design.md) follows this work.

Explicitly out of scope (→ audit backlog in memory)

All Medium/Low findings: dead related.settings/ui/ code, HeaderZone/FooterZone resolver gap, slug dedupe, VideoBlock URL parsing, a11y pass, any-typing, tree-flattener consolidation (beyond the AI-guard reuse), stale dev proxy, etc.