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

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",

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"

packages/core/src/utils/richTextSanitize.test.ts

@@ -0,0 +1,193 @@
+import { describe, expect, it } from "vitest";
+import { parseHTML } from "linkedom";
+import { isRichTextFormattingTag, sanitizeRichTextChildren } from "./richTextSanitize";
+
+// Both parsers, every case. The browser runs this against a live element and
+// the server runs it against linkedom, and the whole point of one shared module
+// is that the two cannot disagree about what may be written to a file.
+const PARSERS: Array<[string, (html: string) => Element]> = [
+  [
+    "jsdom",
+    (html) => {
+      const host = document.createElement("div");
+      host.innerHTML = html;
+      return host;
+    },
+  ],
+  [
+    "linkedom",
+    (html) => {
+      const { document: doc } = parseHTML(`<!DOCTYPE html><html><body></body></html>`);
+      const host = doc.createElement("div");
+      host.innerHTML = html;
+      return host as unknown as Element;
+    },
+  ],
+];
+
+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('<span style="color: red">hi</span>', parse)).toBe(
+      '<span style="color: red">hi</span>',
+    );
+  });
+
+  it("keeps plain text untouched", () => {
+    expect(clean("just words", parse)).toBe("just words");
+  });
+
+  it("keeps nested formatting and its nesting", () => {
+    expect(clean('<b><span style="color: red">x</span></b>', parse)).toBe(
+      '<b><span style="color: red">x</span></b>',
+    );
+  });
+
+  it("keeps a line break", () => {
+    expect(clean("a<br>b", parse)).toContain("<br>");
+  });
+
+  it("removes a script and does not leave its source as visible text", () => {
+    const out = clean("<script>alert(1)</script>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('<span onclick="steal()" style="color: red">x</span>', 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('<span id="a" class="b" data-x="c" style="color: red">x</span>', 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(
+      '<span data-hf-text-key="child:1" data-hf-id="hf-abc" style="color: red">x</span>',
+      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(`<span data-hf-text-key='a" onload="alert(1)'>x</span>`, 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(
+      '<span style="font-family: Inter; font-size: 48px; letter-spacing: -1px; line-height: 1.2">x</span>',
+      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(`<span style="font-family: url(http://x/f.woff)">x</span>`, parse);
+    expect(out).not.toContain("url(");
+  });
+
+  it("unwraps a tag that is not formatting, keeping its words in place", () => {
+    expect(clean("before<div>middle</div>after", parse)).toBe("beforemiddleafter");
+  });
+
+  it("unwraps deeply and keeps the formatting found inside", () => {
+    const out = clean('<div><p><span style="color: red">deep</span></p></div>', parse);
+    expect(out).toBe('<span style="color: red">deep</span>');
+  });
+
+  it("keeps only the allowlisted style properties", () => {
+    const out = clean('<span style="color: red; position: fixed; z-index: 99">x</span>', 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(`<span style="${style}">x</span>`, 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(
+      '<span style="background-color: url(javascript:alert(1)); color: red">x</span>',
+      parse,
+    );
+    expect(out).not.toContain("javascript");
+    expect(out).not.toContain("url(");
+    expect(out).toContain("color: red");
+  });
+
+  it("drops the style attribute entirely when nothing in it survives", () => {
+    expect(clean('<span style="position: fixed">x</span>', parse)).toBe("<span>x</span>");
+  });
+
+  it("keeps a value carrying a function with its own separators", () => {
+    const out = clean('<span style="color: rgb(1, 2, 3); font-style: italic">x</span>', parse);
+    expect(out).toContain("rgb(1, 2, 3)");
+    expect(out).toContain("font-style: italic");
+  });
+
+  it("removes a comment, which is neither text nor formatting", () => {
+    expect(clean("a<!-- note -->b", 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('<span style="color: red">open', parse);
+    expect(out).toBe('<span style="color: red">open</span>');
+  });
+});
+
+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);
+    }
+  });
+});