Apply mXSS-safe SAFE_FOR_XML on untrusted re-inflation paths

[jeremy]

What

Stored Trix HTML is re-inflated back into the editor on load (editor.loadHTML) and on reparse (composition.replaceHTML). Both parsed through DOMPurify with the default config, which leaves SAFE_FOR_XML off — so the untrusted storage round-trip is not defended against mutation-XSS. composition.insertHTML already opts into { purifyOptions: { SAFE_FOR_XML: true } } per call; this extends the same idiom to the two remaining re-inflation entry points.

Why this is scoped per-call, not a global flip

A global SAFE_FOR_XML: true in config/dompurify.js would reintroduce the regression fixed in #1213. Attachment content is serialized into the data-trix-attachment attribute, and with config.action_view.annotate_rendered_view_with_filenames on, that content carries Rails view-annotation comments like <!-- BEGIN app/views/users/_user.html.erb -->. DOMPurify's SAFE_FOR_XML attribute-value guard drops any attribute whose value contains a comment terminator (-->, --!>, ]>, or a raw </style-style close), so the whole data-trix-attachment attribute is stripped and the attachment silently disappears on the round-trip. That guard runs before DOMPurify consults forceKeepAttr, so the existing data-trix keep-hook alone does not save it.

The preservation hook

The uponSanitizeAttribute hook in html_sanitizer.js now, for data-trix-* attributes, neutralizes only the copy DOMPurify inspects for its XML-safety guard and then sets forceKeepAttr, which keeps the original value verbatim (the neutralized copy is never written to the DOM). These are data attributes — always entity-escaped on serialization, never re-parsed as markup — so retaining them is mXSS-safe. The neutralization is scoped to data-trix-*; XML-unsafe values on ordinary attributes are still stripped by SAFE_FOR_XML.

Changes

• models/editor.js — loadHTML parses with SAFE_FOR_XML: true
• models/composition.js — replaceHTML parses with SAFE_FOR_XML: true
• models/html_sanitizer.js — preservation hook for serialized data-trix-* attribute values

Tests

• html_sanitizer_test.js — data-trix-* comment markers preserved under SAFE_FOR_XML; XML-unsafe values on non-data-trix attributes still stripped (scoping proof)
• serialization_test.js — round-trip: mXSS payload neutralized; attachment comment survives
• attachment_test.js — re-inflation keeps a comment-bearing attachment and leaves its content byte-identical

yarn test (web-test-runner / Playwright Chromium): 506 tests, 477 passed, 0 failed, 29 pre-existing skips.

Follow-up (deliberately held)

npm publish + the bc3 consumer bump are gated follow-ups and are not part of this PR.

[Copilot]

Pull request overview

Hardens stored HTML re-inflation with DOMPurify’s XML-safe mode while attempting to preserve serialized attachment comments.

Tip

If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.

Changes:

• Enables SAFE_FOR_XML for editor loading and composition reparsing.
• Adds preservation logic for data-trix-* attributes.
• Adds sanitizer, serialization, and attachment regression tests.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
src/trix/models/html_sanitizer.js	Preserves serialized Trix attributes under XML-safe sanitization.
src/trix/models/editor.js	Enables XML-safe parsing when loading HTML.
src/trix/models/composition.js	Enables XML-safe parsing during DOM reparsing.
src/test/unit/serialization_test.js	Tests safe parsing and serialization round-trips.
src/test/unit/html_sanitizer_test.js	Tests XML-unsafe attribute handling.
src/test/unit/attachment_test.js	Tests preservation of comment-bearing attachments.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 51d9b84c8c

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

Suppressed comments (2)

src/trix/models/html_sanitizer.js:34

• This bypasses the XML-safety check for the entire data-trix-attachment JSON value, but attachment content is not terminal data: html_parser.js:198 parses it into an attachment and attachment_view.js:38 later reparses it as HTML with the repository default SAFE_FOR_XML: false (config/dompurify.js:3). An attacker can therefore place an mXSS sequence in serialized attachment content, have this hook preserve it past the protected outer parse, and reach a second unprotected parse. Apply SAFE_FOR_XML: true when rendering attachment content (and test a nested mXSS payload through the actual editor render), or narrowly preserve only data that cannot be reparsed as markup.

    data.attrValue = data.attrValue.replace(XML_UNSAFE_ATTRIBUTE_VALUE, "")
    data.forceKeepAttr = true

src/test/unit/serialization_test.js:20

• This helper manually supplies the new option instead of exercising either changed production entry point. Because Editor.loadHTML and Composition.replaceHTML only add this wiring, these tests still pass if either production change is removed. Add system coverage that sends the payload through editor.loadHTML and through a DOM mutation that invokes replaceHTML, then verifies the rendered/serialized result.

  const reinflate = (html) => {
    const document = HTMLParser.parse(html, { purifyOptions: { SAFE_FOR_XML: true } }).getDocument()
    return serializeToContentType(document, "text/html")

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/trix/models/html_sanitizer.js:34

• This bypass leaves nested attachment HTML outside the new mXSS protection. data-trix-attachment is JSON-decoded by html_parser.js:41-44, then its content is re-parsed by AttachmentView through HTMLSanitizer.setHTML without SAFE_FOR_XML (attachment_view.js:37-38), while the default remains false. An mXSS payload inside content therefore skips the outer guard here and reaches another unsafe sanitize/serialize/reparse cycle; the new system test covers only an ordinary onerror payload. Apply SAFE_FOR_XML to that second parse and add a nested mXSS regression test before exempting this attribute.

    data.attrValue = data.attrValue.replace(XML_UNSAFE_ATTRIBUTE_VALUE, "")
    data.forceKeepAttr = true

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-08-30T05:02:00.789277Z	4292ffa	New commits

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.