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.

Sign up for GitHub

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

Jump to bottom

Update CSS sanitization safelist to support variables #574

Open
ghost opened this issue Jan 28, 2019 · 6 comments
Open

Update CSS sanitization safelist to support variables #574

ghost opened this issue Jan 28, 2019 · 6 comments
Labels
[Component] Jetpack Customizations to Jetpack [Component] Themes & Customization Custom WC Themes, compatibility with core themes, and theme-adjacent customizations to sites Migrated from Trac

Comments

@ghost
Copy link

ghost commented Jan 28, 2019

Imported from https://meta.trac.wordpress.org/ticket/4108
Created by @iandunn:

Most browsers support CSS variables now, but they're stripped out by the Jetpack validation process, or the Remote CSS sanitization process.

https://wordpress.slack.com/archives/C08M59V3P/p1548543160179600

Either way, it's probably just because the syntax is new, and the safelist needs to be updated to support it.

  1. Determine which code needs to be updated (Jetpack's Custom CSS module, WordCamp.org's mu-plugins/jetpack-tweaks/css-sanitization.php, or both)
  2. If Jetpack, open an issue on their GitHub and add a link to this report
  3. If Remote CSS, add unit tests, and create patch to make them pass. If there are any ways to inject JavaScript, expressions, etc through the new syntax, then tests should be written for that as well. If the problem turns out to be in sanitize_urls_in_css_properties(), let me know before writing a patch since I have some notes about a potential bug there.
@ghost
Copy link
Author

ghost commented Mar 21, 2019

Comment by slackbot:

This ticket was mentioned in Slack in #meta-wordcamp by coreymckrill. View the logs.

@ghost
Copy link
Author

ghost commented Mar 21, 2019

Comment by @iandunn:

  • Status set to assigned

@ghost
Copy link
Author

ghost commented Jun 11, 2020

Comment by slackbot:

This ticket was mentioned in Slack in #meta-wordcamp by ryelle. View the logs.

@ghost ghost added Migrated from Trac [Type] Good First Issue Straightforward, self-contained, doesn't require deep knowledge of codebase labels Sep 24, 2020
@megmorsie megmorsie mentioned this issue Mar 17, 2021
Closed
@ryelle ryelle self-assigned this Aug 29, 2021
@ryelle
Copy link
Contributor

ryelle commented Aug 29, 2021

Possible Jetpack fix: Automattic/jetpack#20129

@ryelle ryelle added [Component] Jetpack Customizations to Jetpack [Component] Themes & Customization Custom WC Themes, compatibility with core themes, and theme-adjacent customizations to sites and removed [Type] Good First Issue Straightforward, self-contained, doesn't require deep knowledge of codebase labels Feb 28, 2022
@ryelle
Copy link
Contributor

ryelle commented Sep 23, 2022

The Jetpack issue says the problem was fixed, so this is worth looking into again.

@iandunn
Copy link
Member

iandunn commented Dec 2, 2022

It doesn't look like that fixed it, so I think we need to track Automattic/jetpack#19669.

iandunn added a commit that referenced this issue Dec 2, 2022
@iandunn
Remote CSS: Test that custom CSS property values are sanitized
5c6322c 
When support for custom properties is added (by us or upstream in Jetpack), we need to make sure that the values are still subject to sanitization.

At that time we'll probably need to change the expected value to something like `--foo: ;`, but having some kind of test in place now will at least make it obvious if the JS makes it into the sanitized output.

See #574
@renintw renintw moved this to ⚠ On Hold/Blocked in Community Events Active Tasks Apr 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
[Component] Jetpack Customizations to Jetpack [Component] Themes & Customization Custom WC Themes, compatibility with core themes, and theme-adjacent customizations to sites Migrated from Trac
Projects
Community Events Active Tasks
Status: ⚠ On Hold/Blocked
Milestone
No milestone
Development

No branches or pull requests
3 participants
@iandunn @ryelle @renintw