Cancel the default paste when Trix handles a paste as a file paste - #1342
Cancel the default paste when Trix handles a paste as a file paste#1342jeremy wants to merge 1 commit into
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
Pull request overview
Prevents browser-default HTML insertion when Trix handles mixed file/HTML pastes.
Changes:
- Cancels default paste behavior for processable file pastes.
- Updates the bundled Action Text asset.
- Adds regression coverage for mixed file/HTML clipboard data.
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.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
src/trix/controllers/level_2_input_controller.js |
Cancels the browser’s default file-paste action. |
src/test/system/level_2_input_test.js |
Tests cancellation and safe attachment insertion. |
action_text-trix/app/assets/javascripts/trix.js |
Synchronizes the bundled implementation. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
8ce06c2 to
c7e067f
Compare
When a paste carries a file alongside HTML, Trix inserts the file but left the browser's default paste uncanceled, so the browser also inserted the accompanying clipboard HTML directly into the editor — unsanitized — until the next redraw replaced it. If that redraw is disrupted, the browser-inserted markup survives in the live DOM and is later serialized back into the editor value, which can turn attacker markup into executable content. Every other insertFromPaste branch already calls preventDefault. The file branch now does too, so Trix fully owns the paste and untrusted clipboard HTML never lands in the editor DOM. This also matches the documented intent to prioritize files over HTML on paste, rather than inserting both.
c7e067f to
369c832
Compare
What
When a paste carries a file alongside HTML (a common mixed-clipboard shape — e.g. an image plus its
text/htmlrepresentation),insertFromPaste'sprocessableFilePastebranch inserted the file but did not callpreventDefault(). Every other branch ininsertFromPaste(the URL, plain-text, and HTML branches) cancels the browser's default paste; this one didn't.The result: Trix inserted the file and the browser's default action inserted the accompanying clipboard HTML directly into the contenteditable — unsanitized — until the next editor redraw replaced it with sanitized DOM.
Why it matters
The safety of that markup depends entirely on the redraw running and replacing it. If the redraw is disrupted, the browser-inserted markup survives in the live DOM and is later serialized back into the editor's value. Trix's serializer re-inflates
data-trix-serialized-attributesinto real element attributes, so surviving attacker markup on that path can be turned into executable content. Relying on a post-hoc redraw to clean up unsanitized DOM that was never supposed to be inserted is the wrong invariant.The fix
Add
this.event.preventDefault()to the file-paste branch so Trix fully owns the paste — the browser never inserts the accompanying clipboard HTML, and no untrusted markup lands in the editor DOM in the first place. This also matches the documented intent to prioritize files over HTML on paste (#1148), rather than inserting both.Test
src/test/system/level_2_input_test.jsadds a case dispatching a mixed file +text/htmlpaste and asserting thebeforeinput/insertFromPastedefault is canceled, the file is inserted as an attachment, and none of the clipboard HTML's attributes survive into the editor value. Red before the one-line change, green after. Full suite green (473 passed).Note for maintainers
This closes the paste entry vector. As defense in depth, the serializer's re-inflation of
data-trix-serialized-attributesinto arbitrary attributes (src/trix/core/serialization.js) remains a sharp edge that trusts sanitized DOM; it's worth a hard look at that sink independently, since its only legitimate producer emits a singlesrcoverride.