fix(site-builder): address Task 25 review findings on <style> scoping

Four issues from adversarial review of the block-scoped <style> feature:

1. (Critical) transformBlock() recursed once per @media/@supports/@container
   nesting level with no cap -- ~7000 nested rules blew the call stack, and
   nothing between a Custom HTML block's toHtml() and the publish pipeline
   catches exceptions, so this took down the whole page's publish and
   crashed the live editor on every keystroke. Added MAX_NESTING_DEPTH=20
   (pass the body through unscoped beyond it) and wrapped scopeCss() so it
   never throws on any input, matching repairOrphanNodes's existing
   contract. Caught and fixed a variable-shadowing bug in my own first pass
   at this: the new depth parameter was silently shadowed by a pre-existing
   `let depth` used for brace-matching in the same block, which would have
   defeated the cap with no type error.

2. (Important) FORCE_BODY: true was unconditional, but it isn't a no-op for
   style-free input: it also changes how the parser preserves whitespace
   after a LEADING html comment, which this repo's own fixture starts with.
   Verified via a raw byte-diff against HtmlBlock.tsx@6a9b227 (extracted
   verbatim, run standalone against real dompurify+jsdom) that the fixture
   gained bytes. Fixed by applying FORCE_BODY only when the input has a
   real (non-comment) <style> tag to rescue -- confirmed empirically that
   this is a true no-op for every other input. Pinned the old output as a
   checked-in regression fixture and added a raw toBe() diff test.

3. (Important) scopeStyleBlocks() wasn't idempotent -- pasting previously
   published/exported output into a fresh block nested a second wrapper
   and re-prefixed every selector. Added isAlreadyScoped(), which detects
   a lone root wrapper whose <style> content is already a no-op under
   scopeCss for that wrapper's own class (reusing scopeCss's own
   idempotency guarantee) and leaves it untouched.

4. (Minor) Documented, not fixed: the 32-bit scope-id hash is
   brute-forceable (CSS-only impact, same trust tier as other accepted
   risks here), and DOMPurify's SAFE_FOR_XML silently drops an entire
   <style> block when its content merely looks tag-like (e.g.
   content: "<Read More>").

1155/1155 tests passing (was 1141), tsc clean.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

craft/src/components/basic/HtmlBlock.tsx

@@ -78,6 +78,21 @@ const PURIFY_CONFIG = {
     // DOMPurify runs, so it can only match inside this block's own wrapper
     // element. See the FORCE_BODY comment below and scopeStyleBlocks() for
     // why allowing the tag alone is not sufficient.
+    //
+    // Review note (Task 25 follow-up, documented not fixed): DOMPurify's
+    // SAFE_FOR_XML default (on unless a caller explicitly disables it,
+    // which PURIFY_CONFIG does not) silently drops an ENTIRE <style>
+    // element -- not just the offending part -- if its text content
+    // contains anything that merely LOOKS tag-like (a `<` followed by a
+    // word character, `/`, or `!`), as an mXSS-namespace-confusion defense
+    // that isn't specific to <style>. So `.x::after{content:"<Read
+    // More>"}` -- a plausible, entirely benign real-world CSS content
+    // string -- makes the whole style block vanish with no error, the same
+    // way a `<script>` would. This is a GOOD security property (better
+    // paranoid than exploitable), but it's an undocumented interaction
+    // with this newly-widened surface that will otherwise confuse whoever
+    // debugs the inevitable "my CSS just disappeared" report -- confirmed
+    // empirically against dompurify+jsdom directly, not guessed at.
     'style',
   ],
   // NOTE: supplying ALLOWED_ATTR replaces DOMPurify's own default attribute
@@ -137,25 +152,58 @@ const PURIFY_CONFIG = {
   // scopeStyleBlocks() doesn't care about namespace/nesting depth).
   FORBID_TAGS: ['script','object','embed','link','meta'],
   FORBID_ATTR: [/^on/i],
-  // Task 25: without this, DOMPurify parses `input` as a full (mini) HTML
-  // document via DOMParser and only serializes <body>'s contents. Per the
-  // HTML5 parsing algorithm, a tag that can only legally appear in <head>
-  // -- and now that <style> is allowed, that includes <style> -- gets
-  // implicitly placed in <head> when it appears before any other content,
-  // and is silently lost (DOMPurify never looks at <head>). A block whose
-  // entire `code` is `<style>h1{color:red}</style>` -- a very plausible
-  // paste, style-before-markup is a common snippet shape -- would vanish
-  // with no error anywhere, despite <style> sitting right there in
-  // ALLOWED_TAGS. FORCE_BODY prepends an internal element before parsing so
-  // the parser is already in body-insertion-mode by the time it reaches the
-  // customer's first tag, keeping a leading <style> (or anything else) in
-  // <body> where DOMPurify's body-only serialization actually looks.
-  // Confirmed empirically against dompurify+jsdom directly (not just this
-  // app's behavior) -- see the "leading <style> with nothing before it"
-  // test in HtmlBlock.test.ts.
-  FORCE_BODY: true,
+  // NOTE: FORCE_BODY is deliberately NOT set here -- see
+  // needsForceBody()/purifyHtml() below. It's applied conditionally, per
+  // call, only when the input actually has a real <style> tag to rescue.
 };
 
+// Task 25: without FORCE_BODY, DOMPurify parses `input` as a full (mini)
+// HTML document via DOMParser and only serializes <body>'s contents. Per
+// the HTML5 parsing algorithm, a tag that can only legally appear in
+// <head> -- and now that <style> is allowed, that includes <style> --
+// gets implicitly placed in <head> when it appears before any other real
+// content, and is silently lost (DOMPurify never looks at <head>). A block
+// whose entire `code` is `<style>h1{color:red}</style>` -- a very
+// plausible paste, style-before-markup is a common snippet shape -- would
+// vanish with no error anywhere, despite <style> sitting right there in
+// ALLOWED_TAGS. FORCE_BODY prepends an internal element before parsing so
+// the parser is already in body-insertion-mode by the time it reaches the
+// customer's first tag, keeping a leading <style> (or anything else) in
+// <body> where DOMPurify's body-only serialization actually looks.
+// Confirmed empirically against dompurify+jsdom directly (not just this
+// app's behavior) -- see the "leading <style> with nothing before it" test
+// in HtmlBlock.test.ts.
+//
+// Review finding (Task 25 follow-up): FORCE_BODY is NOT a no-op for input
+// that has no <style> tag at all. It also changes how the HTML parser
+// treats character content sitting between a LEADING comment and the next
+// real tag -- normal parsing (before <body> is established) silently drops
+// pure-whitespace text runs there per the HTML5 "before head" insertion
+// mode rules, while FORCE_BODY (already in body-insertion-mode from the
+// first token) preserves that whitespace as a real text node. Concretely:
+// a block starting with a multi-line HTML comment -- this repo's own
+// ~16KB fixture does exactly that -- gained 2 extra leading bytes (a
+// preserved newline) once FORCE_BODY was unconditionally on, which
+// silently broke the "blocks without <style> are byte-identical to
+// pre-Task-25 output" guarantee (confirmed with a raw diff against
+// HtmlBlock.tsx@6a9b227 -- the commit immediately before this task -- over
+// the fixture and a comment-led block; see HtmlBlock.test.ts). Fix: only
+// ever set FORCE_BODY when the input has a real <style> tag to rescue --
+// the one and only case that needs it -- so every other input takes
+// exactly the pre-Task-25 code path, unchanged.
+//
+// "Real" deliberately excludes a `<style` substring that only appears
+// inside an HTML comment (e.g. a customer's own code-sample text
+// mentioning `<style>`) -- that text can never become an actual <style>
+// element, but naively substring-matching it would still flip FORCE_BODY
+// on and reintroduce the exact same whitespace-preservation side effect
+// for a block that never had, and never needed, real style scoping.
+const STYLE_TAG_RE = /<style[\s>/]/i;
+const HTML_COMMENT_RE = /<!--[\s\S]*?-->/g;
+function needsForceBody(input: string): boolean {
+  return STYLE_TAG_RE.test(input.replace(HTML_COMMENT_RE, ''));
+}
+
 // M-6: `<iframe>` is allowed (maps/video embeds are a legitimate use case)
 // but an iframe with a `src` and NO `sandbox` attribute is a clickjacking/
 // phishing vector (DOMPurify already strips <script>/on*=, but an
@@ -205,15 +253,80 @@ const IFRAME_SANDBOX_HOOK = (node: Element): void => {
  * on its own -- same code in, byte-identical output out, every time, in
  * both places it's called.
  */
+const SCOPE_CLASS_RE = /^whp-html-[0-9a-z]+$/;
+
+/**
+ * Idempotency (review finding, Task 25 follow-up): `purifyHtml()` is not
+ * reachable-with-its-own-output through any CURRENT code path, but nothing
+ * stops a customer from pasting previously-published or exported HTML from
+ * this exact feature into a fresh Custom HTML block -- at which point
+ * `code` already contains our own `<div class="whp-html-OLD">...<style>
+ * .whp-html-OLD h1{...}</style>...</div>` wrapper. Without this check,
+ * `scopeStyleBlocks` would hash the NEW `code` to a NEW scope class, fail
+ * to recognise the embedded selectors as already scoped (they're prefixed
+ * for the OLD class, not the new one `scopeCss`'s own idempotency guard
+ * checks against), and nest a second wrapper div around the first while
+ * re-prefixing every selector under the new class on top of the old one.
+ *
+ * Detects "the sanitized content IS ALREADY exactly one of our own scoped
+ * wrappers": a single root element, a <div>, whose class matches our own
+ * naming convention, and whose `<style>` descendant(s) are each already a
+ * no-op under `scopeCss` for that div's own class -- i.e. re-scoping would
+ * change nothing. That last check reuses `scopeCss`'s own idempotency
+ * guarantee (`scopeCss(scopeCss(x, S), S) === scopeCss(x, S)`, proved in
+ * scope-css.test.ts) rather than re-implementing "is this CSS already
+ * scoped" as a second parser: if scoping again under the div's own class
+ * is a no-op, the CSS is already confined to that div, regardless of
+ * whether this app was the one that put it there -- which is the actual
+ * safety property this function exists to guarantee, not merely a proxy
+ * for it.
+ */
+function isAlreadyScoped(container: HTMLElement): boolean {
+  if (container.children.length !== 1) return false;
+  const root = container.children[0];
+  if (root.tagName !== 'DIV') return false;
+  const cls = root.getAttribute('class') || '';
+  if (!SCOPE_CLASS_RE.test(cls)) return false;
+
+  const scopeSelector = `.${cls}`;
+  const styleEls = Array.from(root.querySelectorAll('style'));
+  if (styleEls.length === 0) return false; // matches our naming by coincidence but scopes nothing -- not ours to protect
+
+  return styleEls.every((el) => {
+    const text = el.textContent || '';
+    if (text.trim() === '') return true;
+    return scopeCss(text, scopeSelector) === text;
+  });
+}
+
 function scopeStyleBlocks(sanitized: string, rawCode: string): string {
   if (!sanitized.includes('<style')) return sanitized;
 
   const container = document.createElement('div');
   container.innerHTML = sanitized;
+
+  if (isAlreadyScoped(container)) return sanitized;
+
   const styleEls = Array.from(container.querySelectorAll('style'));
   const nonEmpty = styleEls.filter((el) => (el.textContent || '').trim() !== '');
   if (nonEmpty.length === 0) return sanitized;
 
+  // Review note (Task 25 follow-up, documented not fixed): `stableHash` is
+  // a 32-bit djb2 hash, so it's brute-forceable in principle -- a customer
+  // could deliberately craft a second block's `code` to collide onto the
+  // same `whp-html-<hash>` class as an existing block on the same page, at
+  // which point the two blocks' <style> rules apply to (and override) each
+  // other, since they'd share one wrapper class. Impact is CSS-only --
+  // visual breakage, never script execution or data exposure -- the same
+  // trust tier as other accepted risks in this file (e.g. remote url() in
+  // style content, or the pre-existing DATA_URI_TAGS mimetype-blindness
+  // documented in HtmlBlock.security.test.ts). Not fixed here: closing it
+  // would mean either a wider hash (cheap, but every existing scope class
+  // set with THIS Task 25 code would silently reshuffle -- a similar
+  // "changing the hash function reshuffles stored HTML" cost the pinned
+  // hash test above already guards against happening BY ACCIDENT) or a
+  // collision-checked/salted scheme, either of which is a bigger design
+  // decision than a follow-up-review fix.
   const scopeClass = `whp-html-${stableHash(rawCode)}`;
   for (const el of nonEmpty) {
     el.textContent = scopeCss(el.textContent || '', `.${scopeClass}`);
@@ -230,8 +343,13 @@ export function purifyHtml(input: string): string {
   // multiple copies of the same hook.
   DOMPurify.addHook('afterSanitizeAttributes', IFRAME_SANDBOX_HOOK);
   try {
-    const sanitized = DOMPurify.sanitize(input || '', PURIFY_CONFIG as any) as unknown as string;
-    return scopeStyleBlocks(sanitized, input || '');
+    const raw = input || '';
+    // See needsForceBody()/the FORCE_BODY comment above PURIFY_CONFIG:
+    // applied only when there's a real <style> tag to rescue, so every
+    // other input takes the exact pre-Task-25 sanitize() call, unchanged.
+    const config = needsForceBody(raw) ? { ...PURIFY_CONFIG, FORCE_BODY: true } : PURIFY_CONFIG;
+    const sanitized = DOMPurify.sanitize(raw, config as any) as unknown as string;
+    return scopeStyleBlocks(sanitized, raw);
   } finally {
     DOMPurify.removeHook('afterSanitizeAttributes', IFRAME_SANDBOX_HOOK as any);
   }