Skip to content

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

Merged
lgarron merged 1 commit into
mainfrom
pse-paved-paths/trusted-types-compatibility
Jan 9, 2023
Merged

lgarron merged 1 commit into
mainfrom
pse-paved-paths/trusted-types-compatibility

Conversation

@KyFaSt

@KyFaSt KyFaSt commented Dec 21, 2022

Copy link
Copy Markdown
Contributor

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
Copy Markdown

👋 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.

@KyFaSt
KyFaSt requested a review from fletchto99 December 21, 2022 18:10

@fletchto99 fletchto99 left a comment

Copy link
Copy Markdown
Member

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