Global Styles: unable to save block some variation styles when KSES is active

[BogdanUngureanu]

The KSES filters prevent applying some block variation styles via the Global Styles sidebar in the site editor.

These filters are only registered when a user doesn't have the unfiltered_html capability (which is the case for multisite regular admins).

When these filters are active, some changes to the block variation property are reverted back to their previous value.

Looking at the HTTP request to the global-styles endpoint, the PUT request includes the block variation changes but the response doesn't.

After doing some debugging, I noticed this happens because of this line. After removing the validation, the save worked as expected. This change was introduced in the context of this PR, which seems to fix a similar issue.

Comparing the input and output of that line it seems that it removes properties when it shouldn't:

Given input

array (
  'elements' =>
  array (
    'heading' =>
    array (
      'color' =>
      array (
        'text' => 'var(--wp--preset--color--theme-2)',
      ),
    ),
    'button' =>
    array (
      ':hover' =>
      array (
        'color' =>
        array (
          'background' => 'color-mix(in srgb, var(--wp--preset--color--theme-4) 85%, #FFF)',
          'text' => 'var(--wp--preset--color--theme-3)',
        ),
      ),
      'color' =>
      array (
        'background' => 'var(--wp--preset--color--theme-4)',
        'text' => 'var(--wp--preset--color--theme-3)',
      ),
    ),
  ),
  'color' =>
  array (
    'background' => 'var(--wp--preset--color--theme-3)',
    'text' => 'var(--wp--preset--color--theme-4)',
  ),
  'blocks' =>
  array (
    'core/separator' =>
    array (
      'color' =>
      array (
        'text' => 'color-mix(in srgb, currentColor 25%, transparent)',
      ),
    ),
  ),
)

Output generated

array (
  0 =>
  array (
    'name' => 'background-color',
    'value' => 'var(--wp--preset--color--theme-3)',
  ),
  1 =>
  array (
    'name' => 'color',
    'value' => 'var(--wp--preset--color--theme-4)',
  ),
)

Step-by-step reproduction instructions

• Activate latest Gutenberg on a multisite install, or test with a user not having the unfiltered_html capability, or enable the KSES filters with add_action( 'init', 'kses_init_filters' );
• Install the Assembler theme
• Go to Appearance > Editor
• Apply this pattern on your site (just to be sure you follow the same steps): https://gist.github.com/richtabor/e25d9e316e51f39495425c86b425a49e
• Open the Global Styles panel
• Select a style variation (e.g. "Noir")
• Save it
• Notice the style variation card is not highlighted
• Click on it again
• Notice that the Save button is enabled
• Click Save and notice that some elements are not saved
• Repeat the save and notice that the same elements are not saved

[aaronrobertshaw]

Thanks for writing up this issue @BogdanUngureanu 👍

I had some trouble wrapping my head around it and following the replication steps initially.

Comparing the input and output of that line it seems that it removes properties when it shouldn't:

The input and output noted in the description don't appear to match in terms of format. Can you confirm exactly where you pulled that data from? When I dumped the theme.json data at the beginning and end of the remove_insecure_properties function, the format for both was consistent with the input in this issue's description.

The KSES filters prevent applying some block variation styles via the Global Styles sidebar in the site editor.

This doesn't appear to be related to block style variations, block type styles etc. I believe it's the CSS value being assigned that's the problem. More on that in a second.

When these filters are active, some changes to the block variation property are reverted back to their previous value.

For others coming to this issue, as far as I can tell, there's only one problematic style in the above data. It is:
'background' => 'color-mix(in srgb, var(--wp--preset--color--theme-4) 85%, #FFF)',

That color value fails the is_safe_css_declaration check which ultimately calls safecss_filter_attr().

You can confirm this if you temporarily force this check to pass. After doing so, you'll see the expected behaviour and the color-mix value in the output.

I'm not sure of the security implications of allowing color-mix that would likely require its own dedicated issue and core patch.

In the short term though there is a safecss_filter_attr_allow_css filter that could be leveraged by the theme to ensure color-mix is allowed. Before going down that path, you might need to double check with some security folks whether it opens up much of an attack vector.

I hope that helps some! 🙏

[aaronrobertshaw]

Quick follow up:

A simplistic search for themes using the color-mix CSS function only revealed a few. There are probably a good number of others out there though. One prominent example is the Assembler theme which appears to use that function a huge amount.

cc/ @richtabor

[BogdanUngureanu]

Hi @aaronrobertshaw!

I think there's a bit of confusion here. 😄 There are two issues here:

1. The issue about color-mix that was reported here and by me afterward in a Core ticket.
2. This issue - which is unrelated to KSES (that still reproduces after applying that KSES workaround).

In hindsight, we can add that KSES workaround in Gutenberg too, until WP Core gets a proper fix.

Going back to this issue. The output I've mentioned above was obtained by adding an error_log after this line.

So my code was something like

$variation_output = static::remove_insecure_styles( $variation_input );
error_log(var_export( $variation_input, true) );
error_log(var_export($variation_output, true) );

I did some debugging and the KSES filter doesn't block any CSS rule (after applying my KSES workaround), so the problem is from somewhere else; my best guess is that \WP_Theme_JSON_Gutenberg::compute_style_properties skips some properties from the $variation_input.

[aaronrobertshaw]

Thanks for the extra info @BogdanUngureanu 👍

I think there's a bit of confusion here. 😄 There are two issues here:

Agreed. I think we need to update this issue's title, description, etc., to focus solely on the issue that needs addressing here.

As you note, the problem with KSES has been or will be dealt with separately. So this issue now only relates to users without unfiltered HTML caps not being able to correctly save variation styles. In other words, the references to KSES are a bit of a red herring that might further confuse others coming to this issue down the road.

Going back to this issue. The output I've mentioned above was obtained by adding an error_log after this line.

That makes sense now. Where you're debugging the output, ignores the block of code after it that processes the element styles. Moving the output logging to after the output is finished being processed paints a clearer picture.

Once I add color-mix to the regex here in core, all the color mix styles come through with one exception, that being the blocks node within a variation.

The root cause of the problem is that variations' inner blocks and blocks.blockType.elements styles don't get processed in remove_insecure_properties.

I have a fix that needs some more refining and additional unit tests before I can throw up a PR for it. It's very late here, so I'll probably have to finish it off on Monday morning.