Files
site-builder/docs/superpowers/plans/2026-07-12-site-builder-hardening-and-asset-picker.md
2026-07-12 11:36:27 -07:00

22 KiB
Raw Permalink Blame History

Site Builder Hardening + Asset Picker Implementation Plan

For agentic workers: REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development to implement this plan task-by-task. Steps use checkbox (- [ ]) syntax for tracking.

Goal: Fix the full 2026-07-12 Fable audit — Critical stored-XSS + High data-loss bugs, the Medium/Low correctness/dead-code/a11y backlog — and ship the unified image/asset picker, across the Craft.js site builder.

Architecture: Work proceeds in dependency-ordered phases. Phase A builds a shared escaping/URL util that many later tasks consume, so it lands first. Phases that edit overlapping component files are serialized; genuinely disjoint tasks within a phase may run in parallel. Every task is TDD where a behavior is testable, and ends in a commit.

Tech Stack: Vite 6 + React 18 + TypeScript 5 (strict) + @craftjs/core 0.2.x + Vitest. DOMPurify already a dependency.

Global Constraints

  • Active source is craft/ only. Never touch the legacy top-level GrapesJS builder.
  • Verify with cd craft && npx vitest run and cd craft && npm run build (runs tsc — strict). Both must be green before a task is "done".
  • No native alert/confirm/prompt in any UI intended for the WHP panel (WHP UI ban). Use existing WHP/in-app patterns; do NOT add window.confirm.
  • toHtml output must stay deterministic (no Math.random()/Date.now() in export paths) so published HTML diffs/caches cleanly.
  • Inline styles only for user content; editor chrome uses editor.css. Follow existing component pattern (render + .craft + static toHtml).
  • Frequent commits: one per task minimum. Conventional-commit messages. End commit messages with the Co-Authored-By trailer used in this repo.
  • Path alias @/craft/src/.

Phase A — Escaping/URL foundation (Critical XSS). Blocks C, and precedes DG edits on shared files.

Task A1: Shared escape.ts util

Files:

  • Create: craft/src/utils/escape.ts
  • Test: craft/src/utils/escape.test.ts
  • Modify: craft/src/utils/html-export.ts (delete local escapeHtml at ~:201, import from escape.ts)

Produces (consumed by nearly every later task):

export function escapeHtml(s: string): string;   // & < > "   — text/content + double-quoted attrs
export function escapeAttr(s: string): string;   // escapeHtml + ' + ` — any attribute context
export function safeUrl(s: string): string;      // '' for javascript:/vbscript:/data:text/html schemes (case/space/entity-insensitive), else trimmed input
  • Step 1: Write failing tests covering: escapeHtml('a & b <c> "d"')a &amp; b &lt;c&gt; &quot;d&quot;; escapeAttr("' \ ")escapes single-quote and backtick;safeUrl('javascript:alert(1)')''; safeUrl(' JavaScript:alert(1)')''; safeUrl('data:text/html;base64,x')''; safeUrl('https://x.com/a?b=1&c=2')unchanged;safeUrl('/relative')unchanged;safeUrl('mailto:x@y.com')unchanged;safeUrl('#anchor')` unchanged; non-string / null-ish inputs coerce safely.
  • Step 2: Run npx vitest run src/utils/escape.test.ts → FAIL.
  • Step 3: Implement escape.ts. safeUrl normalizes by lowercasing, stripping whitespace and HTML entities/control chars, then rejecting a ^(javascript|vbscript|data:text/html) scheme; allow all other/relative URLs.
  • Step 4: Update html-export.ts to import escapeHtml from escape.ts; delete its private copy.
  • Step 5: Run npx vitest run src/utils/escape.test.ts + npm run build → PASS.
  • Step 6: Commit feat(builder): add shared escape/safeUrl util.

Task A2: Replace the 27 local esc/escapeHtml copies

Files (modify — import escapeHtml/escapeAttr from @/utils/escape, delete the local def): every file matching grep -rlE "const (esc|escapeHtml)\s*=|function (esc|escapeHtml)" craft/src. Includes (non-exhaustive) components/basic/{ButtonLink,Icon,SocialLinks,Logo,Menu,Navbar,Footer,StarRating}.tsx, components/sections/{HeroSimple,CTASection,CallToAction,FeaturesGrid,PricingTable,Gallery,ContentSlider,Testimonials,Countdown,Tabs,Accordion,SubscribeForm,MapEmbed,...}.tsx, components/layout/{Section,BackgroundSection,Container,ColumnLayout}.tsx, components/forms/*.tsx, components/basic/_cta-helpers.tsx.

Interfaces — Consumes: A1's escapeHtml/escapeAttr.

  • Step 1: For each file, replace the local escaper with the import. Use escapeAttr for values emitted inside attributes (title=, alt=, single/double-quoted attrs); escapeHtml for text-node content. This fixes the "omits &" and "omits quotes" variants at once.
  • Step 2: grep -rE "const (esc|escapeHtml)\s*=|function (esc|escapeHtml)" craft/src → only escape.ts remains (0 others).
  • Step 3: npm run build → PASS (tsc catches missed imports).
  • Step 4: Run existing toHtml tests npx vitest run → PASS.
  • Step 5: Commit refactor(builder): use shared escaper everywhere, drop 27 copies.

(May be split into 2-3 commits by directory for review sanity; behavior-preserving.)

Task A3: safeUrl on all exported URLs

Files (modify toHtml href/src/action/url(...) emission): basic/ButtonLink.tsx:230, basic/Icon.tsx:303,320, basic/SocialLinks.tsx:440, basic/Logo.tsx:417, basic/Menu.tsx:497, basic/Navbar.tsx:856-918, basic/_cta-helpers.tsx:86, sections/ContentSlider.tsx:484, sections/FeaturesGrid.tsx:292, sections/PricingTable.tsx:447, media/ImageBlock.tsx:479, media/VideoBlock.tsx:725,792, sections/Gallery.tsx img src, background-image emitters layout/BackgroundSection.tsx:191, sections/HeroSimple.tsx:421,441, sections/CallToAction.tsx:379, layout/Section.tsx bg.

Consumes: A1 safeUrl, escapeAttr.

  • Step 1: Wrap each exported URL: href="${escapeAttr(safeUrl(props.href||''))}"; for CSS url(...) use url('${escapeAttr(safeUrl(x))}').
  • Step 2: Write regression tests (in each component's .toHtml.test.ts, or a new toHtml-escaping.test.ts): a javascript: href renders empty; a "-containing href stays inside its attribute; bg url() with ") cannot break out.
  • Step 3: npx vitest run + npm run build → PASS.
  • Step 4: Commit fix(builder): neutralize javascript:/breakout URLs in HTML export.

Task A4: JS-string / raw-HTML context fixes

Files: basic/HtmlBlock.tsx:144, sections/Countdown.tsx:298, sections/Gallery.tsx:296.

  • Step 1: Tests — HtmlBlock toHtml({code:'<script>x</script>'}) output is DOMPurify-sanitized (no raw <script>); Countdown toHtml with targetDate:'2026-01-01");alert(1)//' produces no injected JS (date rejected/blanked or JSON-encoded); Gallery lightbox uses data-src (no onclick="..._open('...')" interpolation).
  • Step 2: Run → FAIL.
  • Step 3: Implement: HtmlBlock return { html: purifyHtml(props.code || '') }. Countdown: validate /^\d{4}-\d{2}-\d{2}([T ].*)?$/; emit var target=new Date(${JSON.stringify(validDate)}).getTime(). Gallery: emit data-src="${escapeAttr(img.src)}" + class="..._thumb", attach one delegated click listener in the gallery's inline script that reads getAttribute('data-src').
  • Step 4: Run → PASS; npm run build.
  • Step 5: Commit fix(builder): sanitize HtmlBlock export, JS-context in Countdown/Gallery.

Task A5: Escape values flowing into cssPropsToString

Files: craft/src/utils/style-helpers.ts (cssPropsToString).

  • Step 1: Test a style value containing ";color:red;background:url(x)" cannot terminate the declaration / inject extra props; url() contents run through safeUrl.
  • Step 2: Implement value sanitization (strip/encode ", ; where they'd break out; safeUrl inside url(...)). Keep valid multi-value styles intact.
  • Step 3: npx vitest run (all toHtml tests still pass) + npm run build.
  • Step 4: Commit fix(builder): sanitize style-string emission.

Phase B — Data-loss / state corruption (High). Independent of A; disjoint files, may run parallel to A.

Task B1: Node-id regeneration for copy/paste/duplicate

Files:

  • Create: craft/src/utils/craft-tree.tsregenerateTreeIds(query, tree): NodeTree
  • Modify: panels/context-menu/ContextMenu.tsx (duplicate, pasteNode), hooks/useKeyboardShortcuts.ts (duplicate)
  • Test: craft/src/utils/craft-tree.test.ts

Produces: regenerateTreeIds(query, tree) → a NodeTree with every rootNodeId, nodes key, .id, .data.parent, and linkedNodes value remapped to fresh ids (via Craft's id generator), preserving structure.

  • Step 1: Test — given a serialized 3-node subtree, regenerateTreeIds returns a tree whose id set is disjoint from the input's and internally consistent (parents/linkedNodes point to new ids).
  • Step 2: Run → FAIL.
  • Step 3: Implement helper. Update duplicate/pasteNode/keyboard-duplicate to addNodeTree(regenerateTreeIds(query, tree), parentId). Fix pasteNode parent: insert as sibling of the target (target's data.parent), fallback ROOT. Remove the dead clonedTree computation in duplicate.
  • Step 4: Run + npm run build → PASS.
  • Step 5: Commit fix(builder): regenerate node ids on duplicate/paste to prevent state corruption.

Task B2: Header/Footer save routing

Files: hooks/useWhpApi.ts (save), state/PageContext.tsx (confirm the commit-current-state fn name + isEditingHeader/Footer flags), test craft/src/hooks/useWhpApi.save.test.ts.

  • Step 1: Test — with activePageId==='__header__' and a live canvas, save()'s payload puts the live serialization in header_craft_state/header_html and does NOT overwrite a normal page's slot nor the top-level craft_state with header content. (Mock fetch, usePages, query.serialize.)
  • Step 2: Run → FAIL.
  • Step 3: Implement: before serializing, commit the live canvas to the active zone (call PageContext's existing save-current-state). Branch on editing-header/footer: assign live serialization to header/footer slots; otherwise current behavior. Ensure header/footer HTML is exported from the freshly-committed state, not stale headerPage.craftState.
  • Step 4: Run + npm run build → PASS.
  • Step 5: Commit fix(builder): route live header/footer edits correctly on save.

Task B3: AI-boundary validation

Files: craft/src/utils/apply-ai-response.ts, test apply-ai-response.test.ts (extend).

  • Step 1: Tests — a node with unknown resolvedName is skipped (no throw); update_props cannot overwrite node_id/style; AI node_id==='ROOT' or duplicate is rejected/regenerated.
  • Step 2: Run → FAIL.
  • Step 3: Implement: validate resolvedName against componentResolver keys; allowlist update_props keys per node (exclude node_id; merge style shallowly, never replace wholesale via blind assign); dedupe/validate ids; fail soft + console.warn. Reuse PageContext.treeToState guard logic.
  • Step 4: Run + npm run build → PASS.
  • Step 5: Commit fix(builder): validate AI response before deserializing into craft state.

Phase C — Unified asset picker. AFTER Phase A (shares component files: ImageBlock/Logo/Navbar/Gallery/Features/etc.).

Task C1: utils/assets.ts

Files: Create craft/src/utils/assets.ts, test assets.test.ts. Modify panels/right/styles/shared.tsx to re-export uploadToWhp as a thin wrapper over uploadAsset.

Produces:

export interface Asset { name: string; url: string; type: string; }
export async function uploadAsset(file: File): Promise<string | null>; // POST upload_asset; blob: fallback if no WHP_CONFIG
export async function listAssets(mediaType?: 'image'|'video'|'any'): Promise<Asset[]>; // [] if no WHP_CONFIG; filter by type prefix
  • Step 1: TestsuploadAsset w/o WHP_CONFIG returns a blob: URL; with config, POSTs to upload_asset and returns data.url; listAssets('image') returns only type startsWith image; listAssets() w/o config returns []. (Mock fetch, window.WHP_CONFIG, URL.createObjectURL.)
  • Step 2: Run → FAIL.
  • Step 3: Implement (bodies lifted from existing shared.tsx uploadToWhp + the list_assets fetch pattern). Update shared.tsx uploadToWhpexport const uploadToWhp = uploadAsset.
  • Step 4: Run + npm run build → PASS.
  • Step 5: Commit feat(builder): shared asset upload/list util.

Task C2: ui/AssetPicker.tsx

Files: Create craft/src/ui/AssetPicker.tsx, test AssetPicker.test.tsx.

Consumes: C1 uploadAsset/listAssets. Produces:

interface AssetPickerProps {
  value: string;
  onChange: (url: string) => void;
  mediaType?: 'image'|'video'|'any'; // default 'image'
  variant?: 'full'|'compact';        // default 'full'
  placeholder?: string;
}
export const AssetPicker: React.FC<AssetPickerProps>;
  • Step 1: Tests — renders value; grid select calls onChange(asset.url); URL "Apply" calls onChange(input); listAssets called with mediaType; Browse hidden when no WHP_CONFIG; variant="compact" renders the compact row.
  • Step 2: Run → FAIL.
  • Step 3: Implement both variants sharing all logic (upload/browse/url state); full mirrors current ImageStylePanel layout, compact the tight row. File-input accept and grid filter from mediaType.
  • Step 4: Run + npm run build → PASS.
  • Step 5: Commit feat(builder): reusable AssetPicker (full/compact, media-typed).

Task C3: Wire AssetPicker into all image fields

Files (replace ad-hoc image input with <AssetPicker>):

  • Full: panels/right/styles/ImageStylePanel.tsx, MediaStylePanel.tsx, BackgroundSectionStylePanel.tsx, HeroStylePanel.tsx (image), components/layout/Container.tsx (bg), components/basic/Logo.tsx, components/basic/Navbar.tsx (logo), components/media/ImageBlock.tsx.

  • Compact: panels/right/styles/FeaturesEditor.tsx, components/sections/FeaturesGrid.tsx, components/sections/Gallery.tsx, components/sections/ContentSlider.tsx.

  • Step 1: For each, render <AssetPicker value={x} onChange={v=>setProp(...x=v)} variant={full|compact}/>, writing to that consumer's existing prop. Remove now-dead local showBrowser/browserAssets/handleBrowse/handleUpload state.

  • Step 2: npm run build → PASS; npx vitest run → PASS (toHtml unchanged).

  • Step 3: Manual smoke via npm run dev: each field shows Upload+Browse+URL and selecting an asset updates the canvas.

  • Step 4: Commit (split by group) feat(builder): unify image fields on AssetPicker.

Task C4: Remove duplicated upload/browse code

  • Step 1: Delete leftover local uploadToWhp copies (FeaturesEditor etc.) + HeroStylePanel AssetBrowser now replaced. grep confirms no orphaned copies.
  • Step 2: npm run build + npx vitest run → PASS.
  • Step 3: Commit refactor(builder): drop duplicated asset browse/upload code.

Phase D — Correctness / parity (Medium). After A (shared files).

  • D1 — Emit collected props. Files: forms/ContactForm.tsx (render+export successMessage), layout/Container.tsx (apply cssId/cssClass in render + toHtml), sections/FeaturesGrid.tsx (make mediaType toggle actually select icon-vs-image instead of feat.image truthiness). Test each renders/exports. Commit fix(builder): emit ContactForm/Container/FeaturesGrid props.
  • D2 — Export parity. sections/Testimonials.tsx single-layout export matches editor (carousel markup or documented static + aria); layout/ColumnLayout.tsx width slider re-derives child widths in toHtml. Tests. Commit.
  • D3 — Slug dedupe. state/PageContext.tsx (addPage/renamePage/replaceAllPages) + PagesPanel.tsx: uniqueSlug(base, existing) appends -2, -3. Test collision. Commit.
  • D4 — VideoBlock URL parsing. media/VideoBlock.tsx:32-38: handle youtu.be, /shorts/ID, /live/ID, watch?...&v=ID, Vimeo vimeo.com/ID/HASH. Tests. Commit.
  • D5 — TemplateModal race. panels/topbar/TemplateModal.tsx: await the load sequence (promisified) then onClose(); show Loading state. Commit.
  • D6 — Pure state updaters. state/PageContext.tsx:248-262,318-323: move loadState/setActivePageId/ref mutations out of setPages/setHeaderPage updater callbacks into effects/post-set logic. Verify no double-deserialize under StrictMode. Commit.
  • D7 — Footer edit guard. basic/Footer.tsx: adopt the editedTextRef+commit-on-deselect pattern from Heading.tsx. Commit.
  • D8 — Preview head + $-safe replace. panels/topbar/TopBar.tsx:189-198: pass headCode to exportToHtml; replace html.replace(bodyMatch[1], composedBody) with a function replacer to avoid $-sequence corruption. Commit.
  • D9 — headCode consolidation. Remove dead per-page headCode (types/index.ts:19 + PageContext hardcoded ''); keep only SiteDesign.headCode. Commit.

Each D task: write/adjust a test, implement, npx vitest run + npm run build green, commit.


Phase E — Dead code / maintainability (Medium). After C (so picker isn't removed as "dead").

  • E1 — Remove dead settings UI. Confirm via grep that RightPanel.tsx renders only <GuidedStyles/> and that each *Settings component + all of ui/ (AdvancedTab/TypographyControl/BorderControl/SpacingInput/AnchorIdField/SettingsTabs) is imported ONLY by related.settings. Delete the dead related.settings panels and the ui/ files that become orphaned. Keep ui/ files still used by live StylePanels (verify each individually before deleting). npm run build proves no live import breaks. Commit chore(builder): remove dead related.settings panels + orphaned ui/.
  • E2 — HeaderZone/FooterZone. They're absent from resolver.ts and dead → delete the files, and add a guard in apply-ai-response/treeToState that skips unknown resolvedName (already added in B3 — just confirm). Update CLAUDE.md stale component list/count. Commit.
  • E3 — Consolidate tree-flatteners. Extract one shared flattener used by PageContext.treeToState and apply-ai-response.buildNodeTree; delete dead serializeTreeForCraft. Ensure the style:[]→{} normalization is in the shared path. Tests for both callers. Commit.
  • E4 — Panel/modal dedup. Extract useNodeProp(id) hook into shared.tsx (replace setProp/setPropStyle boilerplate in the 9 style panels); extract the shared array-item field editor used by SectionTypePanel + GenericPropsEditor; extract one <Modal> for TemplateModal/HeadCodeModal/SitesmithModal; extract the shared nav-link editor (Menu↔Navbar). Behavior-preserving; npx vitest run green. Commit per extraction.
  • E5 — Typing. Type StylePanelProps.nodeProps per-panel where feasible; add PatchOp/SerializedTreeNode types at the AI boundary. Reduce any in apply-ai-response.ts/treeToState. npm run build strict green. Commit.

Phase F — Accessibility (Medium). After C/E (touches widgets + panels).

  • F1 — Exported widget a11y. ContentSlider (prev/next/dots aria-label, aria-live, pause control), Tabs (role=tablist/tab/tabpanel, arrow-key nav), Gallery lightbox (role=dialog, Escape close, focus trap, keyboard-operable thumbnails). Model on Accordion's native <details>. Tests assert ARIA in toHtml. Commit per component.
  • F2 — Exported semantics. Form <label for>/input id association (InputField/TextareaField/ContactForm); StarRating role="img"+aria-label; Navbar hamburger accessible name + aria-expanded; iframe title (VideoBlock/MapEmbed); aria-hidden on decorative FA <i>. Tests. Commit.
  • F3 — Editor chrome a11y. Add role="button"/tabIndex/key handlers to clickable <div> rows (PagesPanel, LayersPanel, AssetsPanel, TemplateModal cards, ImageStylePanel/AssetPicker thumbnails); aria-label on topbar icon buttons. Commit.
  • F4 — Confirm + clipboard. AssetsPanel delete: add an in-app confirm consistent with the WHP no-native-dialog rule (reuse the app's existing confirm pattern, NOT window.confirm); also replace the existing window.confirm in SitesmithModal.tsx:85. copyUrl gets a .catch + fallback. Commit.
  • F5 — Menu deterministic id. basic/Menu.tsx:482: replace Math.random() id with a stable id (hash of content/props or a passed nodeId), matching Navbar. Test: two exports of unchanged Menu are byte-identical. Commit.

Phase G — Low. Last.

  • G1 — GuidedStyles cruft. panels/right/GuidedStyles.tsx: remove unused resolvedName/resolverMap; anchor /hero/i like siblings; drop isContainer match for the removed Header/Footer Zone. Commit.
  • G2 — Dev proxy. vite.config.ts:29: point /api proxy at whp-al10-test (192.168.1.148:8080). Commit.
  • G3 — Context-menu shortcuts. Either implement Ctrl+C/Ctrl+V in useKeyboardShortcuts.ts (using B1's regen helper for paste) or remove the advertised hints in ContextMenu.tsx:193,199. Prefer implementing to match the UI. Test. Commit.
  • G4 — Script nits. ContentSlider autoplay: pause on hover/visibility + clear interval; Countdown: stop at zero; iframe src: emit &amp; correctly. Commit.

Final verification (before report)

  • cd craft && npx vitest run — all green; note count.
  • cd craft && npm run build — tsc strict + vite succeed.
  • grep -rE "const (esc|escapeHtml)\s*=|function (esc|escapeHtml)" craft/src → only escape.ts.
  • Manual npm run dev smoke: XSS payload in a text/href/date field is neutralized in exported HTML; duplicate/paste produces fresh ids; header edit + save persists to header slot; every image field offers Upload+Browse+URL.
  • Build the site-builder bundle and stage per whp-deploy-release (deploy itself is a separate, user-gated step — the report will note readiness, not auto-deploy).
  • Update craft/CLAUDE.md (component count/list) and this plan's checkboxes.

Self-review notes

  • Spec coverage: security spec §1→A1-A5, §2→B1, §3→B2, §4→B3; asset-picker spec→C1-C4; Medium/Low backlog→D/E/F/G (each backlog bullet maps to a task).
  • Dependency ordering: A before C/D/E/F/G (escape util + shared-file edits); C before E (don't delete picker as dead); B independent.
  • Deploy is intentionally NOT automated here — user gates production per repo discipline.