Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make task-lists-elements CSP Trusted Types compatible #35

Merged
merged 1 commit into from
Jan 9, 2023

Conversation

KyFaSt
Copy link
Contributor

@KyFaSt KyFaSt commented Dec 21, 2022

This change makes task-lists-elements compatible with the CSP directive Trusted Types. This CSP directive allows developers to mark a value as a Trusted Type, usually this would be done in conjunction with running some type of sanitizer like DOMPurify to ensure the value doesn't contain any unsafe elements. Fortunately, task-lists-elements doesn't have major violations, just this one. Unfortunately the change in this PR does not buy any security benefits, it just adheres to the Trusted Types API -- not passing bare strings directly to potentially dangerous injection sinks. Currently this implementation is the best way to make this library compatible with trusted types.

@KyFaSt KyFaSt requested a review from a team as a code owner December 21, 2022 14:27
@KyFaSt KyFaSt force-pushed the pse-paved-paths/trusted-types-compatibility branch from da82936 to 8fc70a8 Compare December 21, 2022 14:28
* this change doesn't actually add any security to task-lists-elements, it effectively
  just adheres to the trusted types API by not passing bare strings directly
  to
@KyFaSt KyFaSt force-pushed the pse-paved-paths/trusted-types-compatibility branch from 8fc70a8 to 98a446c Compare December 21, 2022 14:30
@primer-css
Copy link

👋 Hello and thanks for pinging us! This issue or PR has been added to our inbox and a Design Infrastructure first responder will review it soon.

  • 🎨 If this is a PR that includes a visual change, please make sure to add screenshots in the description or deploy this code to a lab machine with instructions for how to test.
  • If this is a PR that includes changes to an interaction, please include a video recording in the description.
  • ⚠️ If this is urgent, please visit us in #primer on Slack and tag the first responders listed in the channel topic.

Copy link
Member

@fletchto99 fletchto99 left a comment

Choose a reason for hiding this comment

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

Looks good from a pse-architecture perspective! Its unfortunate we need to be so verbose for something like this but unfortunately we have no way to indicate that an innerHTML is coming from a hardcoded string 😓

@lgarron lgarron changed the title Make task-lists-elements Trusted Types compatible Make task-lists-elements CSP Trusted Types compatible Jan 9, 2023
@lgarron lgarron merged commit 99c72d4 into main Jan 9, 2023
@lgarron lgarron deleted the pse-paved-paths/trusted-types-compatibility branch January 9, 2023 19:57
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.

6 participants