Claim a "lexxy" Trusted Types policy instead of racing the host for "dompurify" - #1232
Claim a "lexxy" Trusted Types policy instead of racing the host for "dompurify"#1232jeremy wants to merge 3 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a dedicated lexxy Trusted Types policy to prevent DOMPurify policy-name collisions and documents the required CSP configuration.
Changes:
- Creates and supplies the
lexxyTrusted Types policy. - Adds policy-name unit coverage.
- Documents CSP setup for consumers.
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 2 comments.
| File | Description |
|---|---|
src/config/dom_purify.js |
Creates and configures the Trusted Types policy. |
test/javascript/unit/helpers/sanitization_helper.test.js |
Tests dedicated policy creation. |
home/docs/configuration.md |
Documents CSP requirements and Rails configuration. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
jeremy
changed the title
The browser check changed this PRSummarising what is already in the body and the last commit, because it is the reason this PR was retitled. Original title: "Require a It doesn't. Chromium, real
Both fail identically: That sink is Lexxy's own, not DOMPurify's, so no policy handed to DOMPurify covers it — and it is one of about ten unwrapped raw-HTML sinks in Long-standing and untouched by this series: Retitled to "Claim a Where the claim came fromWorth recording, because it is the same failure mode as the over-scoping this whole campaign is unwinding. Two commits in #1227 (
The later commit The caveat is restored, in the source comment and in a "Lexxy does not yet work under enforced Trusted Types" section in Follow-up for a humanMaking Lexxy usable under Trusted Types is real, separate work: wrap every sink in CI: green. |
|
Handoff note for the whole campaign, including QA results and what still needs a human decision: #1234 |
a3f4eb0 to
22f5d01
Compare
Action required if — and only if — your app enforces `require-trusted-types-for 'script'`. Add `lexxy` to your `trusted-types` directive. Nothing else changes. Now that Lexxy sanitizes with its own DOMPurify instance, there are two instances on a page whose host app also uses DOMPurify. Every instance asks Trusted Types for a policy named `dompurify` on its first sanitize, and TT refuses a duplicate name, so one of them gets none. That is not a degraded mode: DOMPurify parses through DOMParser.parseFromString, which is itself a TT sink, so an unsigned instance throws. Which sanitizer breaks would depend on which one happened to run first. So Lexxy asks under its own name and hands the policy to DOMPurify rather than letting DOMPurify try. Creating a policy the CSP hasn't allowlisted throws, and that throw is caught: an app that doesn't list `lexxy` falls back to no policy, which is exactly where this stood before this series. Browsers without Trusted Types take the same path. There is no new public API and no new option. Documented in home/docs/configuration.md, with the Rails initializer, because until now this requirement existed only in a source comment. Release notes here are generated from PR titles, so the title carries it too. Verified against a stub in all three shapes: name allowlisted, name refused, no restriction at all.
…enforced TT Manual browser check in Chromium under `require-trusted-types-for 'script'`, which is the step that should have run before #1227 was merged. With `lexxy` allowlisted and without it, the editor throws while connecting: TypeError: Failed to execute 'parseFromString' on 'DOMParser': This document requires 'TrustedHTML' assignment. at parseHtml (src/helpers/html_helper.js:20) at #parseHtmlIntoLexicalNodes → #setEditorHtml → #loadInitialValue That sink is Lexxy's own, not DOMPurify's, so no policy handed to DOMPurify can cover it — and it is far from the only one: the attachment content insertAdjacentHTML and the innerHTML writes behind the toolbar and dropdowns are all in the same position. None of this is new, and none of it is changed by this series; html_helper.js is untouched by it. What the policy does buy stands, and is worth having on its own: our instance no longer claims the `dompurify` name, so it cannot be the reason the *host app's* sanitizer loses the race and throws. The claim is what was wrong. The comment this series replaced carried the caveat — "the insertAdjacentHTML sink ... needs the same attention, independently of this" — and the rewrite dropped it while reading as though the problem were solved. Restored here, in the source comment and in the docs, with the sinks named so the next person doesn't have to rediscover them in a browser. Verified in the guarded-refusal direction too: with `lexxy` absent from the directive the browser reports `trusted-types trusted-types-policy lexxy`, the throw is caught, and nothing else changes.
Two review catches. STYLE.md:60 asks for a plain conditional over a ternary, and the conditional spread was one. Building the config and assigning the policy after reads better anyway, and it makes the reason for the shape explicit: the key has to be *absent* rather than present-and-undefined, because DOMPurify reads cfg.TRUSTED_TYPES_POLICY and validates it. The tests only covered the happy path, while the PR claimed verification "in all three shapes: name allowlisted, name refused, no restriction at all". Two of the three were only ever checked by hand in a browser. Both are now unit tests, and they assert the absent key rather than merely that nothing threw — mutation checked by removing the catch, which fails the refusal case.
22f5d01 to
f0b3488
Compare
Consumer action
If your app enforces
require-trusted-types-for 'script', addlexxyto yourtrusted-typesdirective.Read the limitation below before assuming this makes Lexxy work under Trusted
Types. It does not.
Why
#1231 gives Lexxy its own DOMPurify instance, so a page whose host app also uses
DOMPurify now has two. Every instance asks Trusted Types for a policy named
dompurifyon its first sanitize, and TT refuses a duplicate name — so one ofthem gets none.
That is not a degraded mode. DOMPurify parses through
DOMParser.parseFromString, which is itself a TT sink, so an unsigned instancethrows. Which sanitizer breaks would come down to which one happened to run
first — and it could just as easily be the host's as ours.
So Lexxy asks under its own name and hands the policy to DOMPurify rather than
letting DOMPurify try. Creating a policy the CSP hasn't allowlisted throws, and
that throw is caught: an app that doesn't list
lexxyfalls back to no policy,which is exactly where this stood before the series. Browsers without Trusted
Types take the same path. No new public API, no new option.
The original version of this change (and its PR description) read as though
allowlisting
lexxywere enough. It isn't, and the manual browser check thatshould have run before #1227 was merged is what caught it.
Chromium,
require-trusted-types-for 'script', both withlexxyallowlisted andwithout — the editor throws while connecting:
That sink is Lexxy's own, not DOMPurify's, so no policy handed to DOMPurify
can cover it. It is not the only one either — the attachment content
insertAdjacentHTMLand theinnerHTMLwrites behind the toolbar and dropdownsare all unwrapped.
None of this is new and none of it is changed by this series:
html_helper.jsisuntouched by it. Lexxy has never run under enforced Trusted Types. What was wrong
was the claim, not the code.
Worth noting how the claim got lost: the comment this series replaced did carry
the caveat — "the
insertAdjacentHTMLsink innodes/custom_action_text_attachment_node.jsreceives a plain string and needsthe same attention, independently of this" — and the rewrite deleted it while
reading as though the problem were solved. It is restored here, in the source
comment and in the docs, with the sinks named.
Making Lexxy usable under Trusted Types is separate work. This PR is a
prerequisite for it, and is worth having on its own regardless: it stops us being
the reason a host app's sanitizer breaks.
Docs
Adds a Content Security Policy section to
home/docs/configuration.md,including a "Lexxy does not yet work under enforced Trusted Types" subsection.
Until now this requirement lived only in a source comment, and this project has
no CHANGELOG — release notes are generated from PR titles.
Verification
restriction at all.
as
trusted-types trusted-types-policy lexxyand caught when not; editor failsidentically either way, for the reason above.
Part of a series re-filing #1227 at reviewable scope, after #1227 was reverted
from
mainin 8c64aa4. Merge order: #1228 → #1229 → #1230 → #1231 → this →#1226. Nothing here is released.
Draft: needs human review and a soak period before merging.