feat(core): sanitize rich text on the way into a composition (#3141)

* feat(core): sanitize rich text on the way into a composition Studio's patch vocabulary was inline-style, attribute, html-attribute and text-content. text-content assigns textContent, and the text-field model escapes markup on the way out and refuses a change in child structure, so a styled span had no route into a composition file. Adds a rich-text operation with one, guarded by a single sanitizer called on both ends of the trip: in the browser so the preview shows what will be saved, and on the server because that is where the file is written. Tags and style properties are a small allowlist, and an unexpected tag loses its formatting rather than its words. Spans an edit adds get their ids in the same write, so a follow-up write cannot race it. No UI yet — this is the persistence contract the editor is built on. * fix(core): document and test the sanitizer boundary * fix(core): harden rich text sanitizer traversal
2026-09-10 22:20:14 +00:00 · 2026-08-11 02:03:47 -04:00
parent fceb376551
commit 636dc042a7
8 changed files with 731 additions and 3 deletions
@@ -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) =>
+  `<!doctype html><html><body><div data-composition-id="main"><h1 id="title">${inner}</h1></div></body></html>`;
+
+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",
+      'hell<span style="color: red">o</span> 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(/<span data-hf-id="hf-[^"]+" style="color: red">o<\/span>/);
+  });
+
+  it("keeps the words and drops the script when the payload is hostile", () => {
+    const { html } = patchTitle("safe", "<script>alert(1)</script>still<b>here</b>", "rich-text");
+
+    expect(html).not.toContain("script");
+    expect(html).not.toContain("alert");
+    expect(html).toContain("still");
+    expect(html).toMatch(/<b data-hf-id="hf-[^"]+">here<\/b>/);
+  });
+
+  it("strips an event handler smuggled onto an allowed tag", () => {
+    const { html } = patchTitle("safe", '<span onclick="steal()">x</span>', "rich-text");
+
+    expect(html).not.toContain("onclick");
+    expect(html).toContain("x");
+  });
+
+  it("keeps only the allowlisted style properties", () => {
+    const { html } = patchTitle(
+      "safe",
+      '<span style="color: red; position: fixed">x</span>',
+      "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", "<div>kept</div>", "rich-text");
+
+    expect(html).toContain("kept");
+    expect(html).not.toContain("<div>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: "<b>y</b>" },
+    ]);
+
+    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", '<span style="color: red">x</span>', "text-content");
+
+    expect(html).not.toContain('<span style="color: red">');
+    expect(html).toContain("&lt;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 &lt;b&gt; &amp; c", "rich-text");
+
+    expect(html).toContain("&lt;b&gt;");
+    expect(html).not.toContain("<b>");
+  });
+
+  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<br>b", "rich-text");
+
+    expect(html).toMatch(/<br data-hf-id="hf-[^"]+">/);
+  });
+
+  it("keeps the wrapper span a flex element needs", () => {
+    const { html } = patchTitle(
+      "safe",
+      '<span>a <span style="color: red">b</span> c</span>',
+      "rich-text",
+    );
+
+    expect(html).toMatch(
+      /<span data-hf-id="hf-[^"]+">a <span data-hf-id="hf-[^"]+" style="color: red">b<\/span> c<\/span>/,
+    );
+  });
+
+  it("empties the element when every character was deleted", () => {
+    const { html } = patchTitle("gone", "", "rich-text");
+
+    expect(html).toContain('id="title"></h1>');
+  });
+
+  it("does not accumulate markup when the same value is written twice", () => {
+    const value = '<span style="color: red">x</span>';
+    const once = patchTitle("safe", value, "rich-text").html;
+    const twice = patchElementInHtml(once, { id: "title" }, [
+      { type: "rich-text", property: "", value },
+    ]).html;
+
+    expect(twice).toBe(once);
+  });
+});
@@ -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 = '<div data-hf-id="hf-a" id="t">plain</div>';
+    const { html, matched } = patchElementInHtml(source, { id: "t" }, [
+      { type: "rich-text", property: "", value: 'a<span style="color: red">b</span>c' },
+    ]);
+
+    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 = '<div data-hf-id="hf-a" id="t">plain</div>';
+    const { html } = patchElementInHtml(source, { id: "t" }, [
+      { type: "rich-text", property: "", value: '<span data-hf-id="hf-keep">b</span>' },
+    ]);
+
+    expect(html).toContain('data-hf-id="hf-keep"');
+  });
+
+  it("does not collide with an id inside a composition template", () => {
+    const source = `<!doctype html><html><body><template data-composition-id="nested"><p data-hf-id="hf-3x72">nested</p></template><h1 id="title">plain</h1></body></html>`;
+    const { html } = patchElementInHtml(source, { id: "title" }, [
+      { type: "rich-text", property: "", value: '<span style="color: red">b</span>' },
+    ]);
+
+    expect(html.match(/data-hf-id="hf-3x72"/g)).toHaveLength(1);
+    const introducedId = /<span[^>]*data-hf-id="([^"]+)"/.exec(html)?.[1];
+    expect(introducedId).toBeDefined();
+    expect(introducedId).not.toBe("hf-3x72");
+  });
+});
@@ -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<string>();
+  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;
    }
  }