diff --git a/packages/core/package-subpaths.json b/packages/core/package-subpaths.json index 2a61955dc0..9ca6bebbe9 100644 --- a/packages/core/package-subpaths.json +++ b/packages/core/package-subpaths.json @@ -38,6 +38,12 @@ "types": "./dist/utils/htmlAttrSafety.d.ts", "environments": ["browser", "bun", "node"] }, + "./rich-text-sanitize": { + "source": "./src/utils/richTextSanitize.ts", + "runtime": "./dist/utils/richTextSanitize.js", + "types": "./dist/utils/richTextSanitize.d.ts", + "environments": ["browser", "bun", "node"] + }, "./composition-contract": { "source": "./src/compositionContract.ts", "runtime": "./dist/compositionContract.js", diff --git a/packages/core/package.json b/packages/core/package.json index 789dd05934..16cf4fa8ad 100644 --- a/packages/core/package.json +++ b/packages/core/package.json @@ -52,6 +52,12 @@ "import": "./src/utils/htmlAttrSafety.ts", "types": "./src/utils/htmlAttrSafety.ts" }, + "./rich-text-sanitize": { + "bun": "./src/utils/richTextSanitize.ts", + "node": "./dist/utils/richTextSanitize.js", + "import": "./src/utils/richTextSanitize.ts", + "types": "./src/utils/richTextSanitize.ts" + }, "./composition-contract": { "bun": "./src/compositionContract.ts", "node": "./dist/compositionContract.js", @@ -326,6 +332,10 @@ "import": "./dist/utils/htmlAttrSafety.js", "types": "./dist/utils/htmlAttrSafety.d.ts" }, + "./rich-text-sanitize": { + "import": "./dist/utils/richTextSanitize.js", + "types": "./dist/utils/richTextSanitize.d.ts" + }, "./composition-contract": { "import": "./dist/compositionContract.js", "types": "./dist/compositionContract.d.ts" diff --git a/packages/core/src/utils/richTextSanitize.test.ts b/packages/core/src/utils/richTextSanitize.test.ts new file mode 100644 index 0000000000..7c0efba874 --- /dev/null +++ b/packages/core/src/utils/richTextSanitize.test.ts @@ -0,0 +1,237 @@ +import { describe, expect, it } from "vitest"; +import { parseHTML } from "linkedom"; +import { isRichTextFormattingTag, sanitizeRichTextChildren } from "./richTextSanitize"; + +function parseWithLinkedom(html: string): Element { + const { document: doc } = parseHTML( + `
${html}
`, + ); + const host = doc.getElementById("test-host"); + if (!host) throw new Error("test host was not parsed"); + return host as unknown as Element; +} + +// Both DOM implementations, every case. The contract must not depend on which +// parser constructs the inert tree at a trust boundary. +const PARSERS: Array<[string, (html: string) => Element]> = [ + [ + "jsdom", + (html) => { + const host = document.createElement("div"); + host.innerHTML = html; + return host; + }, + ], + ["linkedom", parseWithLinkedom], +]; + +function clean(html: string, parse: (html: string) => Element): string { + const host = parse(html); + sanitizeRichTextChildren(host); + return host.innerHTML; +} + +describe.each(PARSERS)("sanitizeRichTextChildren (%s)", (_name, parse) => { + it("keeps a styled span, which is the whole point", () => { + expect(clean('hi', parse)).toBe( + 'hi', + ); + }); + + it("keeps plain text untouched", () => { + expect(clean("just words", parse)).toBe("just words"); + }); + + it("keeps nested formatting and its nesting", () => { + expect(clean('x', parse)).toBe( + 'x', + ); + }); + + it("keeps a line break", () => { + expect(clean("a
b", parse)).toContain("
"); + }); + + it("removes a script and does not leave its source as visible text", () => { + const out = clean("keep", parse); + expect(out).not.toContain("script"); + expect(out).not.toContain("alert"); + expect(out).toContain("keep"); + }); + + it("strips an event handler from a tag it otherwise keeps", () => { + const out = clean('x', parse); + expect(out).not.toContain("onclick"); + expect(out).toContain("color: red"); + }); + + it("strips every attribute that is neither style nor an identity", () => { + const out = clean('x', parse); + expect(out).not.toContain("id="); + expect(out).not.toContain("class="); + expect(out).not.toContain("data-x"); + expect(out).toContain("color: red"); + }); + + // The design panel tracks each text layer by this. Stripping it left the + // panel unable to match a layer to its source after any inline style edit. + it("keeps the attributes a text layer is tracked by", () => { + const out = clean( + 'x', + parse, + ); + expect(out).toContain('data-hf-text-key="child:1"'); + expect(out).toContain('data-hf-id="hf-abc"'); + }); + + it("drops an identity attribute whose value is not a bare token", () => { + const out = clean(`x`, parse); + expect(out).not.toContain("onload"); + expect(out).not.toContain("data-hf-text-key"); + }); + + // These are what the design panel writes onto those same spans. Sanitizing + // them away did not stop a text edit changing layout, it deleted the layout + // the user had already set: colouring one word dropped a sibling's size. + it("keeps the typography the design panel authors on a text layer", () => { + const out = clean( + 'x', + parse, + ); + expect(out).toContain("font-family: Inter"); + expect(out).toContain("font-size: 48px"); + expect(out).toContain("letter-spacing: -1px"); + expect(out).toContain("line-height: 1.2"); + }); + + it("still refuses a value that reaches outside the stylesheet", () => { + const out = clean(`x`, parse); + expect(out).not.toContain("url("); + }); + + it("unwraps a tag that is not formatting, keeping its words in place", () => { + expect(clean("before
middle
after", parse)).toBe("beforemiddleafter"); + }); + + it("unwraps deeply and keeps the formatting found inside", () => { + const out = clean('

deep

', parse); + expect(out).toBe('deep'); + }); + + it("keeps only the allowlisted style properties", () => { + const out = clean('x', parse); + expect(out).toContain("color: red"); + expect(out).not.toContain("position"); + expect(out).not.toContain("z-index"); + }); + + it("keeps every property the allowlist names", () => { + const style = + "color: red; background-color: blue; font-weight: 700; font-style: italic; text-decoration-line: underline"; + const out = clean(`x`, parse); + for (const property of [ + "color", + "background-color", + "font-weight", + "font-style", + "text-decoration-line", + ]) { + expect(out).toContain(property); + } + }); + + it("rejects a value that smuggles a url or a script in", () => { + const out = clean( + 'x', + parse, + ); + expect(out).not.toContain("javascript"); + expect(out).not.toContain("url("); + expect(out).toContain("color: red"); + }); + + it.each([ + ["an image event handler", "safe", "safe"], + ["a script URL", 'safe', "safe"], + ["an SVG script", "safe", "safe"], + [ + "a legacy CSS expression", + 'safe', + "safe", + ], + [ + "an entity-encoded script scheme", + 'safe', + "safe", + ], + [ + "a case-folded script scheme", + 'safe', + "safe", + ], + ])("rejects %s", (_case, html, expected) => { + expect(clean(html, parse)).toBe(expected); + }); + + it("drops the style attribute entirely when nothing in it survives", () => { + expect(clean('x', parse)).toBe("x"); + }); + + it("keeps a value carrying a function with its own separators", () => { + const out = clean('x', parse); + expect(out).toContain("rgb(1, 2, 3)"); + expect(out).toContain("font-style: italic"); + }); + + it("keeps a quoted semicolon inside a style value", () => { + const out = clean(`x`, parse); + expect(out).toContain("Roboto Mono; a"); + expect(out).toContain("color: red"); + }); + + it("removes a comment, which is neither text nor formatting", () => { + expect(clean("ab", parse)).toBe("ab"); + }); + + it("leaves an empty element alone", () => { + expect(clean("", parse)).toBe(""); + }); + + it("does not produce unbalanced markup from an unclosed tag", () => { + const out = clean('open', parse); + expect(out).toBe('open'); + }); + + it("is a fixed point", () => { + const host = parse('
x
'); + sanitizeRichTextChildren(host); + const once = host.innerHTML; + sanitizeRichTextChildren(host); + expect(host.innerHTML).toBe(once); + }); +}); + +it("sanitizes adversarially deep markup without recursive stack growth", () => { + const depth = 15_000; + const host = parseWithLinkedom(`${"".repeat(depth)}x${"".repeat(depth)}`); + + expect(() => sanitizeRichTextChildren(host)).not.toThrow(); +}); + +describe("isRichTextFormattingTag", () => { + it("names the tags an inline edit may contain", () => { + for (const tag of ["SPAN", "B", "STRONG", "I", "EM", "U", "BR"]) { + expect(isRichTextFormattingTag(tag)).toBe(true); + } + }); + + it("is case-insensitive, since the two parsers disagree about case", () => { + expect(isRichTextFormattingTag("span")).toBe(true); + }); + + it("says no to anything structural", () => { + for (const tag of ["DIV", "P", "H1", "IMG", "SCRIPT", "A"]) { + expect(isRichTextFormattingTag(tag)).toBe(false); + } + }); +}); diff --git a/packages/core/src/utils/richTextSanitize.ts b/packages/core/src/utils/richTextSanitize.ts new file mode 100644 index 0000000000..1b5a6476ca --- /dev/null +++ b/packages/core/src/utils/richTextSanitize.ts @@ -0,0 +1,231 @@ +/** + * What inline formatting a composition file is allowed to receive. + * + * Editing text in the Studio preview can style a run of characters, which means + * markup now travels from a contenteditable element into a file on disk. This + * module is the only thing deciding what may make that trip. The server write + * boundary applies it unconditionally before returning composition bytes. + * + * One module rather than two implementations. Two would drift, and the drift + * would be a security bug rather than an inconsistency. + * + * It works on an element's subtree in place. Untrusted markup must be parsed in + * an inert document before this function receives it. + */ + +/** Tags an inline text edit may contain. Everything else is not text styling. */ +const FORMATTING_TAGS = new Set(["SPAN", "B", "STRONG", "I", "EM", "U", "BR"]); + +/** + * Style properties a formatting tag may carry. + * + * This was paint-only, on the reasoning that a property which moves or resizes + * text would let an edit inside one element change the composition's layout, + * and layout is the design panel's job. The reasoning was wrong about who was + * being restricted: the design panel writes exactly these typography + * properties onto exactly these spans, as its text layers. Sanitizing them + * away did not stop text from changing layout, it deleted the layout the user + * had already set — colouring one word silently dropped a sibling layer's font + * size. The line that matters is the one below, values that reach outside the + * stylesheet, not which of its own properties the editor is allowed to keep. + */ +const FORMATTING_STYLE_PROPS = new Set([ + "color", + "background-color", + "font-weight", + "font-style", + "text-decoration-line", + "font-family", + "font-size", + "letter-spacing", + "line-height", + // Paints the glyph fill and inherits, so an ancestor that sets it wins over + // any `color` below. The editor mirrors a run's colour into it when that is + // happening, and stripping it here would put the colour back to invisible. + "-webkit-text-fill-color", +]); + +// Keep this list limited to properties whose grammar cannot fetch a resource. +// Adding a URL-consuming property also requires decoding CSS escapes before +// UNSAFE_VALUE can be a sufficient guard. + +/** + * Attributes a formatting tag may carry. + * + * The identity a text layer is tracked by. Everything else is dropped: a + * contenteditable is a paste target, and an event handler or an id that + * shadows a composition's own is not formatting. + */ +const FORMATTING_ATTRS = new Set(["data-hf-text-key", "data-hf-id"]); + +/** + * What those attributes are allowed to look like: a bare token, nothing else. + * `:` is deliberate because text keys use selector-like tokens such as + * `child:1`; neither allowed attribute is interpreted as a URL. + */ +const SAFE_ATTR_VALUE = /^[A-Za-z0-9_:-]+$/; + +/** + * Tags dropped whole rather than unwrapped. + * + * Everything else is unwrapped, so an unexpected tag costs the user its + * formatting and not their words. These are the ones whose contents are not + * words: unwrapping a script would turn its source into visible text. + */ +const OPAQUE_TAGS = new Set([ + "SCRIPT", + "STYLE", + "TEMPLATE", + "NOSCRIPT", + "IFRAME", + "OBJECT", + "EMBED", + "SVG", + "MATH", +]); + +/** Anything that reaches out of the stylesheet, in a property that should not. */ +const UNSAFE_VALUE = /url\(|expression\(|javascript:|vbscript:|@import|<\//i; + +const ELEMENT_NODE = 1; +const TEXT_NODE = 3; + +type SanitizerFrame = + | { phase: "visit"; node: Node } + | { phase: "sanitize"; element: Element; tag: string }; + +export function isRichTextFormattingTag(tagName: string): boolean { + return FORMATTING_TAGS.has(tagName.toUpperCase()); +} + +function isElementNode(node: Node): node is Element { + return node.nodeType === ELEMENT_NODE; +} + +/** + * Strip everything but allowed formatting from an element's contents, in place. + * + * The element itself is never touched, only what is inside it. Callers own the + * element, and it is the composition's, not the editor's, to rewrite. + * + * When the children came from untrusted markup, callers must parse that markup + * into an inert document (for example linkedom or a detached DOMParser document) + * first. Never assign untrusted HTML to a live DOM element and then call this + * function: active content can run before sanitization begins. + */ +export function sanitizeRichTextChildren(parent: Element): void { + const pending: SanitizerFrame[] = Array.from( + parent.childNodes, + (node): SanitizerFrame => ({ phase: "visit", node }), + ).reverse(); + + // Post-order without recursion: adversarially deep pasted markup must not + // exhaust either the server or browser call stack. + for (let frame = pending.pop(); frame; frame = pending.pop()) { + if (frame.phase === "sanitize") { + if (!FORMATTING_TAGS.has(frame.tag)) unwrap(frame.element); + else stripAttributes(frame.element); + continue; + } + + const child = frame.node; + if (child.nodeType === TEXT_NODE) continue; + + if (!isElementNode(child)) { + // Comments and processing instructions are neither words nor formatting. + child.parentNode?.removeChild(child); + continue; + } + + const element = child; + const tag = element.tagName.toUpperCase(); + + if (OPAQUE_TAGS.has(tag)) { + element.parentNode?.removeChild(element); + continue; + } + + pending.push({ phase: "sanitize", element, tag }); + for (const descendant of Array.from(element.childNodes).reverse()) { + pending.push({ phase: "visit", node: descendant }); + } + } +} + +/** Replace an element with its own children, keeping their order and place. */ +function unwrap(element: Element): void { + const parent = element.parentNode; + if (!parent) return; + while (element.firstChild) parent.insertBefore(element.firstChild, element); + parent.removeChild(element); +} + +/** Leave a kept tag with a filtered style attribute and its identity, no more. */ +function stripAttributes(element: Element): void { + const style = element.getAttribute("style"); + for (const name of Array.from(element.getAttributeNames())) { + const value = element.getAttribute(name) ?? ""; + if (FORMATTING_ATTRS.has(name.toLowerCase()) && SAFE_ATTR_VALUE.test(value)) continue; + element.removeAttribute(name); + } + if (style === null) return; + const safe = filterStyle(style); + if (safe) element.setAttribute("style", safe); + else element.removeAttribute("style"); +} + +/** Keep only the allowlisted declarations, and only if their values are inert. */ +function filterStyle(style: string): string { + return splitDeclarations(style) + .map((declaration) => { + const colon = declaration.indexOf(":"); + if (colon === -1) return null; + const property = declaration.slice(0, colon).trim().toLowerCase(); + const value = declaration.slice(colon + 1).trim(); + if (!FORMATTING_STYLE_PROPS.has(property)) return null; + if (!value || UNSAFE_VALUE.test(value)) return null; + return `${property}: ${value}`; + }) + .filter((declaration): declaration is string => declaration !== null) + .join("; "); +} + +function isQuoteDelimiter(char: string): char is "'" | '"' { + return char === "'" || char === '"'; +} + +function nextParenthesisDepth(depth: number, char: string): number { + if (char === "(") return depth + 1; + if (char === ")") return Math.max(0, depth - 1); + return depth; +} + +function isDeclarationSeparator(char: string, depth: number, quote: "'" | '"' | null): boolean { + return char === ";" && depth === 0 && quote === null; +} + +/** + * Split on the semicolons that separate declarations, not the ones inside a + * value. `color: rgb(1, 2, 3)` is one declaration however many separators its + * value contains. + */ +function splitDeclarations(style: string): string[] { + const declarations: string[] = []; + let current = ""; + let depth = 0; + let quote: "'" | '"' | null = null; + for (const char of style) { + if (char === quote) quote = null; + else if (quote === null && isQuoteDelimiter(char)) quote = char; + else if (isDeclarationSeparator(char, depth, quote)) { + declarations.push(current); + current = ""; + continue; + } else if (quote === null) { + depth = nextParenthesisDepth(depth, char); + } + current += char; + } + if (current.trim()) declarations.push(current); + return declarations; +} diff --git a/packages/studio-server/src/helpers/sourceMutation.richText.test.ts b/packages/studio-server/src/helpers/sourceMutation.richText.test.ts new file mode 100644 index 0000000000..0fb0a04e9e --- /dev/null +++ b/packages/studio-server/src/helpers/sourceMutation.richText.test.ts @@ -0,0 +1,150 @@ +import { describe, expect, it } from "vitest"; +import { patchElementInHtml } from "./sourceMutation.js"; + +/** + * The `rich-text` operation is the only one that can write markup into a + * composition, so it is also the only place a patch payload can carry + * something dangerous all the way to a file. These are the tests for that + * boundary, and for the promise that the older text operation did not quietly + * become a markup sink alongside it. + */ + +const DOC = (inner: string) => + `

${inner}

`; + +function patchTitle(inner: string, value: string, type: "rich-text" | "text-content") { + return patchElementInHtml(DOC(inner), { id: "title" }, [{ type, property: "", value }]); +} + +describe("rich-text patch operation", () => { + it("writes allowed formatting into the source", () => { + const { html, matched } = patchTitle( + "hello world", + 'hello world', + "rich-text", + ); + + expect(matched).toBe(true); + // The id is minted here so the bytes Studio records match the bytes on + // disk — see stampNewChildIds. + expect(html).toMatch(/o<\/span>/); + }); + + it("keeps the words and drops the script when the payload is hostile", () => { + const { html } = patchTitle("safe", "stillhere", "rich-text"); + + expect(html).not.toContain("script"); + expect(html).not.toContain("alert"); + expect(html).toContain("still"); + expect(html).toMatch(/here<\/b>/); + }); + + it("strips an event handler smuggled onto an allowed tag", () => { + const { html } = patchTitle("safe", 'x', "rich-text"); + + expect(html).not.toContain("onclick"); + expect(html).toContain("x"); + }); + + it("keeps only the allowlisted style properties", () => { + const { html } = patchTitle( + "safe", + 'x', + "rich-text", + ); + + expect(html).toContain("color: red"); + expect(html).not.toContain("position: fixed"); + }); + + it("unwraps a structural tag rather than losing the text inside it", () => { + const { html } = patchTitle("safe", "
kept
", "rich-text"); + + expect(html).toContain("kept"); + expect(html).not.toContain("
kept"); + }); + + it("replaces the previous contents rather than appending to them", () => { + const { html } = patchTitle("old words", "new words", "rich-text"); + + expect(html).toContain("new words"); + expect(html).not.toContain("old words"); + }); + + it("reports unmatched for an element that is not there", () => { + const result = patchElementInHtml(DOC("x"), { id: "absent" }, [ + { type: "rich-text", property: "", value: "y" }, + ]); + + expect(result.matched).toBe(false); + }); + + it("leaves the source alone when the value is null", () => { + const before = DOC("keep me"); + const { html } = patchElementInHtml(before, { id: "title" }, [ + { type: "rich-text", property: "", value: null }, + ]); + + expect(html).toContain("keep me"); + }); +}); + +describe("text-content is still not a markup sink", () => { + it("escapes markup handed to the older operation, exactly as before", () => { + const { html } = patchTitle("safe", 'x', "text-content"); + + expect(html).not.toContain(''); + expect(html).toContain("<span"); + }); +}); + +describe("rich-text round trips what a real composition contains", () => { + it("keeps text that looks like markup as text", () => { + const { html } = patchTitle("safe", "a <b> & c", "rich-text"); + + expect(html).toContain("<b>"); + expect(html).not.toContain(""); + }); + + it("keeps non-ASCII text intact", () => { + const { html } = patchTitle("safe", "héllo 👍 世界", "rich-text"); + + expect(html).toContain("héllo"); + expect(html).toContain("👍"); + expect(html).toContain("世界"); + }); + + it("keeps a line break", () => { + const { html } = patchTitle("safe", "a
b", "rich-text"); + + expect(html).toMatch(/
/); + }); + + it("keeps the wrapper span a flex element needs", () => { + const { html } = patchTitle( + "safe", + 'a b c', + "rich-text", + ); + + expect(html).toMatch( + /a b<\/span> c<\/span>/, + ); + }); + + it("empties the element when every character was deleted", () => { + const { html } = patchTitle("gone", "", "rich-text"); + + expect(html).toContain('id="title">'); + }); + + it("does not accumulate markup when the same value is written twice", () => { + const value = 'x'; + const once = patchTitle("safe", value, "rich-text").html; + const twice = patchElementInHtml(once, { id: "title" }, [ + { type: "rich-text", property: "", value }, + ]).html; + + expect(twice).toBe(once); + }); +}); diff --git a/packages/studio-server/src/helpers/sourceMutation.test.ts b/packages/studio-server/src/helpers/sourceMutation.test.ts index 476fceac26..0fe4e3900f 100644 --- a/packages/studio-server/src/helpers/sourceMutation.test.ts +++ b/packages/studio-server/src/helpers/sourceMutation.test.ts @@ -542,3 +542,46 @@ describe("T7 — data-hf-id targeting (spec for R1)", () => { expect(html).toContain('data-hf-id="hf-a1b2"'); }); }); + +/** + * A rich-text operation adds elements, so it has to give them their stable ids + * here, in the bytes it writes and returns. + * + * Otherwise the next preview request mints them and writes the file a second + * time, after Studio has recorded the edit. The recorded "after" stops matching + * disk, the content check refuses, and undo reports the file as changed outside + * Studio — for every colour applied to a run of characters. + */ +describe("patchElementInHtml stamps the ids a rich-text patch introduces", () => { + it("gives each new span its id in the same write", () => { + const source = '
plain
'; + const { html, matched } = patchElementInHtml(source, { id: "t" }, [ + { type: "rich-text", property: "", value: 'abc' }, + ]); + + expect(matched).toBe(true); + expect(html).toContain("color: red"); + expect((html.match(/data-hf-id=/g) ?? []).length).toBe(2); + }); + + it("leaves an id a rich-text patch carried in alone", () => { + const source = '
plain
'; + const { html } = patchElementInHtml(source, { id: "t" }, [ + { type: "rich-text", property: "", value: 'b' }, + ]); + + expect(html).toContain('data-hf-id="hf-keep"'); + }); + + it("does not collide with an id inside a composition template", () => { + const source = `

plain

`; + const { html } = patchElementInHtml(source, { id: "title" }, [ + { type: "rich-text", property: "", value: 'b' }, + ]); + + expect(html.match(/data-hf-id="hf-3x72"/g)).toHaveLength(1); + const introducedId = /]*data-hf-id="([^"]+)"/.exec(html)?.[1]; + expect(introducedId).toBeDefined(); + expect(introducedId).not.toBe("hf-3x72"); + }); +}); diff --git a/packages/studio-server/src/helpers/sourceMutation.ts b/packages/studio-server/src/helpers/sourceMutation.ts index 1d406ddd34..a60a7fd4fa 100644 --- a/packages/studio-server/src/helpers/sourceMutation.ts +++ b/packages/studio-server/src/helpers/sourceMutation.ts @@ -2,7 +2,13 @@ import { parseHTML } from "linkedom"; import postcss from "postcss"; import selectorParser from "postcss-selector-parser"; import { isAllowedHtmlAttribute, isSafeAttributeValue } from "@hyperframes/core/html-attr-safety"; -import { ensureHfIds } from "@hyperframes/parsers/hf-ids"; +import { sanitizeRichTextChildren } from "@hyperframes/core/rich-text-sanitize"; +import { + EXCLUDED_TAGS, + ensureHfIds, + mintHfId, + walkCompositionDescendants, +} from "@hyperframes/parsers/hf-ids"; import { readClipTiming, writeClipTiming } from "@hyperframes/core/composition-contract"; import { parseStyleDecls, patchStyleAttrString } from "./sourceStyleMutation.js"; @@ -136,7 +142,7 @@ export function isHTMLElement(el: Node): el is HTMLElement { } export interface PatchOperation { - type: "inline-style" | "attribute" | "html-attribute" | "text-content"; + type: "inline-style" | "attribute" | "html-attribute" | "text-content" | "rich-text"; property: string; value: string | null; childSelector?: string; @@ -158,6 +164,36 @@ function resolveOperationTarget(parent: HTMLElement, op: PatchOperation): HTMLEl } } +/** + * Give the elements a rich-text patch just introduced their stable ids, here, + * in the bytes about to be written and handed back. + * + * Otherwise the next preview request mints them and writes the file a second + * time, after Studio has already recorded the edit in its history. The recorded + * "after" stops matching disk, the content check refuses, and undo reports the + * file as changed outside Studio — for every colour applied to a run of + * characters and every text layer added. The clip split stamps its own clone + * for exactly this reason. + * + * Minted one element at a time with the same function `ensureHfIds` uses, so + * these ids are the ones the next pass would have assigned. Not `ensureHfIds` + * itself: it takes a whole document, and handing it this element's markup would + * put the markup back as one. + */ +function stampNewChildIds(parent: Element): void { + const assigned = new Set(); + const root = parent.ownerDocument?.body ?? parent; + walkCompositionDescendants(root, (el) => { + const id = el.getAttribute("data-hf-id"); + if (id) assigned.add(id); + }); + for (const el of parent.querySelectorAll("*")) { + if (el.getAttribute("data-hf-id")) continue; + if (EXCLUDED_TAGS.has(el.tagName.toLowerCase())) continue; + el.setAttribute("data-hf-id", mintHfId(el, assigned)); + } +} + // fallow-ignore-next-line complexity export function patchElementInHtml( source: string, @@ -215,6 +251,17 @@ export function patchElementInHtml( textTarget.textContent = op.value; } break; + // The one operation that can write markup, so the one that has to check + // it. Assigned first and sanitised after, rather than sanitising a + // string: parsing is what turns a payload into the tree the allowlist + // can actually judge, and linkedom never runs anything it parses. + case "rich-text": + if (op.value != null) { + opTarget.innerHTML = op.value; + sanitizeRichTextChildren(opTarget); + stampNewChildIds(opTarget); + } + break; } } diff --git a/packages/studio/src/utils/sourcePatcher.ts b/packages/studio/src/utils/sourcePatcher.ts index c020ff7ab2..b2b9c97fdf 100644 --- a/packages/studio/src/utils/sourcePatcher.ts +++ b/packages/studio/src/utils/sourcePatcher.ts @@ -87,7 +87,11 @@ function splitInlineStyleDeclarations(style: string): string[] { } export interface PatchOperation { - type: "inline-style" | "attribute" | "text-content" | "html-attribute"; + // `rich-text` is the only member that carries markup. It is deliberately + // separate from `text-content`, whose contract is "this value is text": the + // design panel and every other caller rely on that, and widening it would + // have turned all of them into markup sinks at once. + type: "inline-style" | "attribute" | "text-content" | "html-attribute" | "rich-text"; property: string; value: string | null; childSelector?: string;