Skip to content

#5569 Add compliance with Trusted Types - #5575

Merged
sosnovsky merged 5 commits into
masterfrom
5569-trusted-types-compliance-live-test
Jan 24, 2024
Merged

#5569 Add compliance with Trusted Types#5575
sosnovsky merged 5 commits into
masterfrom
5569-trusted-types-compliance-live-test

Conversation

@sosnovsky

Copy link
Copy Markdown
Collaborator

This PR adds Trusted Types policy for compatibility with upcoming Gmail policy update

close #5569


Tests (delete all except exactly one):

  • Does not need tests (refactor only, docs or internal changes)

To be filled by reviewers

I have reviewed that this PR... (tick whichever items you personally focused on during this review):

  • addresses the issue it closes (if any)
  • code is readable and understandable
  • is accompanied with tests, or tests are not needed
  • is free of vulnerabilities
  • is documented clearly and usefully, or doesn't need documentation

@sosnovsky
sosnovsky requested a review from martgil January 22, 2024 13:02
@sosnovsky

Copy link
Copy Markdown
Collaborator Author

@martgil this PR is ready for review, thanks!

@martgil martgil left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is also a good idea to update some of the few options for the DOMPurify configuration option to return trusted type by adding the RETURN_TRUSTED_TYPE property set to true when DOMPurify.sanitize() is used:

export declare function sanitize(source: string | Node, config: Config & { RETURN_DOM: true; }): HTMLElement;

Reference:
https://github.com/cure53/DOMPurify?tab=readme-ov-file#what-about-dompurify-and-trusted-types
https://web.dev/articles/trusted-types#use_a_library

@sosnovsky

Copy link
Copy Markdown
Collaborator Author

I think it is also a good idea to update some of the few options for the DOMPurify configuration option to return trusted type by adding the RETURN_TRUSTED_TYPE property set to true when DOMPurify.sanitize() is used:

Nice suggestion, I'll check this property and also standard dompurify types from https://github.com/DefinitelyTyped/DefinitelyTyped/tree/master/types/dompurify as we currently use outdated types for v2.

@sosnovsky

Copy link
Copy Markdown
Collaborator Author

@martgil I tried to add RETURN_TRUSTED_TYPE property to DOMPurify, but it'll require a lot of changes in our code - at least all usages of Xss.htmlSanitize, as currently it returns string, but with Trusted Types it's return type will be string | TrustedHTML. And also adding @types/trusted-types leads to build error, will need investigation too.

So let's plan this change to the next milestone (I created separate issue for it - #5576), as we need to finish current release in the beginning of February, before Gmail policy update.
In this PR I've also updated purify.d.ts to the latest version from https://github.com/DefinitelyTyped/DefinitelyTyped/tree/master/types/dompurify

@sosnovsky
sosnovsky enabled auto-merge (squash) January 23, 2024 21:40
@martgil

martgil commented Jan 24, 2024

Copy link
Copy Markdown
Collaborator

@martgil I tried to add RETURN_TRUSTED_TYPE property to DOMPurify, but it'll require a lot of changes in our code - at least all usages of Xss.htmlSanitize, as currently it returns string, but with Trusted Types it's return type will be string | TrustedHTML. And also adding @types/trusted-types leads to build error, will need investigation too.

I understood completely. This PR also looks good. thanks!

So let's plan this change to the next milestone (I created separate issue for it - #5576), as we need to finish current release in the beginning of February, before Gmail policy update. In this PR I've also updated purify.d.ts to the latest version from https://github.com/DefinitelyTyped/DefinitelyTyped/tree/master/types/dompurify

Okay, thanks! I'll assigned it to myself to work on it.

@martgil martgil left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! thank you.

@sosnovsky
sosnovsky merged commit 7dbae7d into master Jan 24, 2024
@sosnovsky
sosnovsky deleted the 5569-trusted-types-compliance-live-test branch January 24, 2024 00:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Check compliance with Trusted Types

2 participants