Block Editor: Strip per-block custom CSS on save for users without edit_css#76650
Block Editor: Strip per-block custom CSS on save for users without edit_css#76650
Conversation
glendaviesnz
requested review from
spacedmonkey and
tellthemachines
as code owners
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
@aaronrobertshaw It was decided here that for users without edit-css permissions we should strip any block level CSS when they save. |
|
Size Change: +130 B (0%) Total Size: 7.75 MB 📦 View Changed
ℹ️ View Unchanged
|
|
It is not quite working yet. Warning banner shows but CSS is not stripped - looking at it |
|
Flaky tests detected in f9947cc. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/24547321719
|
|
Seems to be working now @aaronrobertshaw - was just an issue with the stripping not accounting for escaped content. |
aaronrobertshaw
left a comment
aaronrobertshaw
left a comment
There was a problem hiding this comment.
Thanks for the quick work on this fix @glendaviesnz 🚀
This is mostly testing well for me however I think I found one small issue.
✅ Editing as Author strips custom CSS on save and it's no longer output on frontend
✅ Opening a post as Admin does not show the warning banner
✅ Saving a post with custom CSS in blocks as Admin works as expected
✅ Could not add block with custom CSS via code editor as Author
✅ Could not add custom CSS in block content via REST API
✅ Existing tests are passing locally
❓ Warning banner does not show until selecting a block with custom CSS when opening post as an Author
Overall, the approach looks good to me. Mirroring core's KSES pattern with the init and set_current_user hooks seems legit and covers the REST API saves too.
Good catch on the slashing bug as well 👍
It might be worth noting in the PR description that existing posts with the custom CSS already saved won't be cleaned up, and that saved custom CSS will still render on the frontend. Both seem intentional and reasonable, but probably worth being explicit about.
I think this is pretty close. My understanding is that there should be an immediate warning banner for Author users etc and that's not what I was seeing:
Screen.Recording.2026-03-19.at.1.44.12.am.mp4
It should show as soon as post opened now.
Done |
ramonjd
left a comment
ramonjd
left a comment
There was a problem hiding this comment.
Thanks for the continued efforts here @glendaviesnz
Sorry for all the questions. I'm primarily thinking about how this translates to Core. And I'm overthinking things :D
| add_action( 'init', 'gutenberg_custom_css_kses_init', 20 ); | ||
| add_action( 'set_current_user', 'gutenberg_custom_css_kses_init' ); |
There was a problem hiding this comment.
Just so I understand, these checks remove/add the hooks for different scenarios, e.g., page requests and REST?
There was a problem hiding this comment.
Yes exactly. This mirrors the pattern Core uses for KSES in kses_init() / kses_init_filters(). The init hook covers the normal page-load/REST request path, and set_current_user covers cases where the current user changes after init (e.g. application passwords on REST, or XML-RPC authentication). The remove-then-conditionally-add pattern in gutenberg_custom_css_kses_init() ensures the filters are always in sync with whoever the current user actually is.
| * | ||
| * @param array $blocks Blocks to process. | ||
| */ | ||
| $strip = static function ( &$blocks ) use ( &$strip, &$changed ) { |
There was a problem hiding this comment.
Any reason not to use a helper that returns a boolean?
I guess it's "another function" in the global namespace, but you wouldn't need to capture the references here. And $changed = false; could go.
No action required, just a thought.
There was a problem hiding this comment.
No strong reason — the closure keeps it self-contained and avoids polluting the global namespace as you noted. A named helper would be slightly cleaner to read but adds another gutenberg_ prefixed function for something that's only used in one place. Happy to refactor if there's a preference either way, but leaving as-is for now.
| * @param string $content Post content to filter. | ||
| * @return string Filtered post content with block custom CSS removed. | ||
| */ | ||
| function gutenberg_strip_custom_css_from_blocks( $content ) { |
There was a problem hiding this comment.
What are your thoughts on the migration path to Core here? I hesitate to say it out loud but filter_block_kses_value seems like candidate. Anyway, if and when it becomes clear, it might be worth noting them for the backport.
There was a problem hiding this comment.
Good question. filter_block_kses_value is an interesting candidate — it already walks block attributes during KSES and could potentially strip style.css inline rather than needing the separate content filter approach. The tricky part is that filter_block_kses_value operates at the attribute level during KSES processing, while this approach operates on the full serialized content, and probably more importantly it was already noted on the abandoned fix in core that this is not a KSES issue - mixing the CSS fix back into KSES at this point seems to muddy that water again.
There was a problem hiding this comment.
Yeah, it's tricky 🤔
The question is going to come up at some point though: "Where would this all live in Core?" As a set of filters perhaps?
| */ | ||
| function gutenberg_custom_css_force_filtered_html_on_import_filter( $arg ) { | ||
| if ( $arg ) { | ||
| gutenberg_custom_css_kses_init_filters(); |
There was a problem hiding this comment.
What is this doing exactly? Would it strip CSS for admins during import as well?
There was a problem hiding this comment.
Yes, this is the import safety net — again mirroring what Core does for KSES. When force_filtered_html_on_import is true (e.g. during a WXR import), it unconditionally enables the CSS stripping filters regardless of the current user's capabilities. So even an admin importing content would have custom CSS stripped. The rationale is the same as KSES during import: you can't trust the content source, so filter it regardless of who is doing the import.
There was a problem hiding this comment.
Got it, thanks for the explainer
| add_filter( 'content_save_pre', 'gutenberg_strip_custom_css_from_blocks', 8 ); | ||
| add_filter( 'content_filtered_save_pre', 'gutenberg_strip_custom_css_from_blocks', 8 ); |
There was a problem hiding this comment.
wp_insert_post_data looks like it comes bundled with post_content and post_content_filtered in the $data arg.
It runs after KSES, but I guess that doesn't matter if we're stripping it.
And I think you wouldn't need the init/set_current_user dance either because wp_insert_post_data fires inside wp_insert_post() at the moment of save, and current_user_can('edit_css')` is evaluated in the hook callback against whoever is actually saving.
I could be missing something obvious.
There was a problem hiding this comment.
That's a fair point — wp_insert_post_data would simplify things since the capability check happens at save time against the actual user. The main reason for mirroring the KSES pattern (content_save_pre + init/set_current_user) was to stay consistent with how Core handles similar content filtering, which makes the eventual backport more straightforward. The content_save_pre / content_filtered_save_pre hooks are the same ones KSES uses, so the stripping runs at the same stage in the save pipeline. wp_insert_post_data runs later and bundles both fields in $data, which could work but would diverge from the established pattern. Open to changing it if you feel the simplicity outweighs the consistency argument though.
There was a problem hiding this comment.
I don't have a strong opinion, and thanks for confirming.
As noted above I'm just thinking about how this would look in Core, and where the processing would take place.
I guess it's contained to the custom-css.php file for now, and, as you say, it could follow the footnotes precedent:
https://github.com/wordpress/wordpress-develop/blob/trunk/src/wp-includes/blocks.php#L3107
|
Just kicked off the E2E tests. They don't look related. I think this is a good change for the plugin. What's the Core plan do you think? Did we miss the WordPress boat? There are a couple of RCs https://make.wordpress.org/core/2026/02/12/wordpress-7-0-release-party-schedule/ It's be good to get a Core backport up so folks with bigger brains than I have can discuss it there too. Thanks, Glen! |
ramonjd
force-pushed
the
strip-css-for-non-edit-css-users
branch
from
2845a81 to
8aa2961
Compare
|
Rebased |
| $strip( $block['innerBlocks'] ); | ||
| } | ||
| } | ||
| }; |
There was a problem hiding this comment.
particularly given that we only changing the JSON attributes and uninterested in the order of traversal, a simpler stack-based approach could be more efficient than recursion due to the continued high cost of PHP function calls.
$blocks = parse_blocks( $unslashed );
$changed = false;
while ( null !== ( $block = array_pop( $blocks ) ) {
…
foreach ( $block['innerBlocks'] as $inner_block ) {
$blocks[] = $inner_block;
}
}that might need an extra & somewhere.
I recently also proposed use of the WP_Block_Processor for this kind of operation, as it allows us to avoid a lot of work. it doesn’t yet have support for modifying the JSON attributes, but we can handle that in a couple of different ways currently.
$processor = new WP_Block_Processor( $unslashed );
$without_css = '';
$was_at = 0;
while ( $processor->next_block() ) {
$span = $processor->get_span();
$text = substr( $unslashed, $span->start, $span->length );
// the text cannot contain a `style.css` attribute.
if ( false === strpos( $text, '"css"' ) ) {
continue;
}
$attrs = $processor->allocate_and_return_parsed_attributes();
if ( ! empty( $attrs['style']['css'] ) ) {
continue;
}
unset( $attrs['styling']['css'] );
$attrs = empty( $attrs ) ? '' : wp_json_serialize( … );
$slash = 'void' === $processor->get_delimiter_type() ? '/' : '';
$without_css .= substr( $unslashed, $was_at, $span->start - $was_at );
$without_css .= "<!-- wp:{$processor->get_block_type()} {$attrs} {$slash}-->";
$was_at = $span->start + $span->length;
}
if ( 0 === $was_at ) {
return $content;
}
if ( $was_at < strlen( $unslashed ) ) {
$without_css .= substr( $unslashed, $was_at );
}
return wp_slash( $without_css );There was a problem hiding this comment.
Thanks for the suggestion! Implemented the stack-based approach in 461e443.
Re WP_Block_Processor — I considered it, but it seems like it is WP 6.9+ and GB currently also supports 6.8, so might be worth revisting once GB support is bumped to 6.9.
For the stack-based loop itself: the key detail is that every entry on the stack is a by-reference alias back into $blocks. When we do $block = &$stack[count($stack) - 1] we get a mutable reference to the actual block in the tree, so unset($block['attrs']['style']['css']) mutates the original $blocks array in-place — no need to rebuild anything. Inner blocks are pushed by reference too (foreach ($block['innerBlocks'] as &$inner_block)), so nested mutations propagate all the way up. The unset($block) at the end of each iteration breaks the reference variable to prevent cross-iteration contamination. Net result: same behavior, no closure overhead, flat loop. If this is not what you were suggesting/expecting, let me know.
There was a problem hiding this comment.
For a fully-compatible way to do this going back in time you can see how @hbhalodia did it in https://github.com/WordPress/wordpress-importer/pull/254/changes#diff-6b3894e370d6014710dd9fca37bfd757f3d241b7dc710d2c44611aeeed7cc94fR1169
Should we preserve CSS when it hasn't changed?Currently An alternative: compare the incoming content against the already-saved content and only strip CSS that actually changed. The problem with
|
| Scenario | Behavior |
|---|---|
| User reorders blocks, CSS unchanged | Sorted arrays match → CSS preserved |
| User duplicates a block with CSS | Array length differs → strip all (conservative but safe) |
| User deletes a block with CSS | Missing value → strip all (conservative but safe) |
| Copy-paste block with CSS from another post | New value in set → strip all (correct) |
| New post, no saved content | Strip all (no comparison possible) |
| Import path | Always strip unconditionally (already handled) |
Hook change implications
- Moving from
content_save_pretowp_insert_post_databreaks the kses-mirror pattern but is the only way to get the post ID wp_insert_post_datacovers all save paths (REST API, classic editor, XML-RPC)content_filtered_save_prewould need separate handling or an unconditional fallback- Slashing behavior is the same (
$data['post_content']arrives slashed) - One extra
get_post()call per save, but WP likely has the post cached already
Verdict
Feasible and the comparison logic is simple, but the hook change adds complexity. The main benefit is preventing accidental CSS loss when contributors save posts where an admin previously added custom CSS. Worth doing if that's a real pain point, but the current unconditional strip is simpler and more obviously correct from a security perspective.
Thoughts on whether this is worth pursuing @aaronrobertshaw, @ramonjd, @dmsnell ?
|
|
||
| while ( ! empty( $stack ) ) { | ||
| $block = &$stack[ count( $stack ) - 1 ]; | ||
| array_pop( $stack ); |
There was a problem hiding this comment.
I get confused on this because objects are returned as refs and I thought the array would be too. I’m glad you figured out the right conventions.
| $unslashed = wp_unslash( $content ); | ||
|
|
||
| // Fast check: if no "css" key anywhere, skip parsing entirely. | ||
| if ( false === strpos( $unslashed, '"css"' ) ) { |
There was a problem hiding this comment.
we have str_contains() and WPCS will flag this as poor quality, even though it’s fine.
|
|
||
| if ( false === $json_rel_end || $json_rel_start >= $json_rel_end ) { | ||
| continue; | ||
| } |
There was a problem hiding this comment.
I left a note in the other PR about this check. at this point we know the conditions have to be true or we would have bailed already. we can provide a comment or leave the check in, but its awkward that we’re not examining $json_rel_start
it definitely seems worth pursuing, but I think it also requires a few more steps of care to ensure we don’t allow something nefarious. for instance, relying on sorted captures of the styles does ensure that no new styles are introduced, but they could have still be moved from one block to another, and I can imagine there could be downsides to that. we should be able to combine efforts with what is happening with real-time collaboration and identify which portions of the document are changed, and then ask if the CSS is changing or not. when are you trying to get this in? is it still for WordPress 7.0? if transplanting existing CSS onto another block is not considered a risk then the sorting approach isn’t bad. it could be better than wiping out everything. one thing remaining which seems the most important thing here is communicating to an editor that the CSS is being removed. I don’t even feel like this would be a problem if we communicated that in the editor before saving. that would take additional code and wouldn’t be in the REST API calls (though we could also return an error and provide a message like “would wipe out CSS, retry with I would just really hope we don’t surprise people by corrupting a post on save, especially since this doesn’t typically show up until after leaving the editor and re-opening the post. within the editor we have |
I'm still catching up on the discussion, but at a glance, if we can tick all the security boxes well enough I'd say optimising for the end user experience here would be the best bet. |
@dmsnell this PR includes a warning notice in the editor if a user without 
I think the only problem is that a user can manipulate it all, and save, in code view, and so bypass any checks we might be doing in the editor. @aaronrobertshaw, @ramonjd, @dmsnell I am thinking if we want any chance of getting this into a 7.0 RC (and it seems like it is important enough a security risk to try for that - but happy to take your more current core release experience on that) then I think we need to keep it simple and just stick with the warning to the user and stripping all block CSS if they save. It seems that adding in trying to determine which CSS may or may not have changed adds another level of complexity and opens us up to missing some combination of manual innocent versus nefarious combination of CSS changes. It seems like this will be an edge case for 7.0. I suspect the number of people adding block level CSS to posts that are then edited by author level users and below is going to be very small, and 🤞 a more robust approach can be put in place for 7.1. |
sorry for missing that @glendaviesnz — very good
in that case I vote for scope-cutting it and leaving it as-is. @sirreal’s input would also be nice. we will be really rushing if we try to peel open the conservative approach in the timespan of a couple days. |
+1 to leaving scope as is. Iterating on the approach could be considered an enhancement for 7.1. |
|
@dmsnell I had a quick look at the possibility of also adding a warning in REST as you suggested, but this also adds a bit of complexity, are you happy for this to be a follow up as well? I imagine an author or less user updating via REST only, and not the editor is probably also fairly uncommon. |
|
@ramonjd, @aaronrobertshaw, @dmsnell, @sirreal - i guess this needs to be answered before we merge this - will leave it in your capable hands to debate this - feel free to go ahead and merge this once that is decided. |
|
Do you mean this comment?
I replied and asked for alternatives: https://core.trac.wordpress.org/ticket/64771#comment:44 Another idea is to punt and leave this in GB for a cycle. |
Yes that was it. I personally think @sirreal's latest comment there makes sense. This matches other approaches to non |
|
Can we get this in @glendaviesnz and prep WordPress/wordpress-develop#11347 Looks like it plugs a hole |
Cover save-time stripping of style.css from block attributes: single blocks, nested inner blocks, non-block content passthrough, preservation of other style properties, and empty style cleanup.
content_save_pre receives slashed content (escaped quotes), which breaks parse_blocks JSON parsing. Unslash before parsing and re-slash after serializing. Add test for slashed content handling.
The edit hook only renders when a block is selected, so the warning notice was not shown until the user selected a block with custom CSS. Move the notice dispatch to useBlockProps which runs for all blocks on mount via createBlockListBlockFilter, ensuring the banner appears as soon as the post is opened.
Gutenberg plugin doesn't use @SInCE tags — they're added during Core backport.
Rephrase for clarity: "If you save this post, the custom CSS will be removed."
Trim the customCSS value before checking truthiness so that whitespace-only strings (e.g. ' ') don't trigger the "custom CSS will be removed" warning notice.
Replace parse_blocks() + stack traversal + serialize_blocks() with WP_Block_Parser::next_token() to scan tokens and splice only the changed attribute JSON into the original string. This avoids the parse/serialize round-trip and any formatting drift it may cause. Pattern follows the WordPress importer approach endorsed by dmsnell.
Use str_contains() instead of strpos() for the fast-check and remove the unnecessary $json_rel_start/$json_rel_end validation since the parser guarantees valid JSON braces at that point.
…fast-path
- Replace wp_unslash/wp_slash with stripslashes/addslashes (content is always a string)
- Remove str_contains('"css"') short-circuit that could miss alternate JSON encodings
- Document slashed input expectation in @param docstring
- Fix PHPCS alignment warnings
ramonjd
force-pushed
the
strip-css-for-non-edit-css-users
branch
from
a8942ce to
f9947cc
Compare
|
I'll merge this, and rebase WordPress/wordpress-develop#11347, and see if we can some 👀 on it. |
ramonjd
added
the
Backport to WP 7.0 Beta/RC
github-actions
Bot
removed
the
Backport to WP 7.0 Beta/RC
…it_css (#76650) Co-authored-by: glendaviesnz <glendaviesnz@git.wordpress.org> Co-authored-by: aaronrobertshaw <aaronrobertshaw@git.wordpress.org> Co-authored-by: ramonjd <ramonopoly@git.wordpress.org> Co-authored-by: dmsnell <dmsnell@git.wordpress.org> Co-authored-by: sirreal <jonsurrell@git.wordpress.org>
|
I just cherry-picked this PR to the wp/7.0 branch to get it included in the next release: 02b2545 |
github-actions
Bot
added
the
Backported to WP Core
5 participants
What?
Adds server-side enforcement of the
edit_csscapability for per-block custom CSS (style.cssblock attribute). Also adds an editor warning notice for users who lack the capability when editing a post that contains blocks with custom CSS.Why?
PR #73959 added per-block custom CSS support, but the
edit_csscapability check is only performed client-side (hiding the CSS panel in the editor). Any user withedit_postscan bypass this by using Code Editor or the REST API to injectstyle.cssinto block attributes, which gets rendered as arbitrary inline CSS on the frontend. This enables UI redress attacks, phishing overlays, and content defacement.Trac ticket for backport: https://core.trac.wordpress.org/ticket/64771
How?
PHP (
lib/block-supports/custom-css.php)Adds a
content_save_prefilter following the established kses pattern used for footnotes inlib/blocks.php:gutenberg_strip_custom_css_from_blocks()— parses blocks, recursively walks all blocks (includinginnerBlocks), removesattrs.style.css, and re-serializes. Short-circuits if content has no blocks or nothing changed.gutenberg_custom_css_kses_init()— hooked oninit(priority 20) andset_current_user. Only activates the filter when! current_user_can( 'edit_css' ).gutenberg_custom_css_kses_init_filters()/gutenberg_custom_css_remove_filters()— add/remove thecontent_save_preandcontent_filtered_save_prehooks at priority 8 (before kses at 9).gutenberg_custom_css_force_filtered_html_on_import_filter()— forces the filter on during imports (priority 999).JS (
packages/block-editor/src/hooks/custom-css.js)In the
CustomCSSEditcomponent, whencanEditCSSis false and the block hasstyle.cssset, dispatches a one-time warning notice (deduplicated via fixed notice ID) informing the user that custom CSS will be removed on save.Not covered
Existing posts with the custom CSS already saved won't be cleaned up, and that saved custom CSS will still render on the frontend. As this feature has only been out for a short time and only in the experimental plugin, there is not enough risk from this to warrant trying to clean up any existing content that may have been added by users without edit-css permissions.
Testing Instructions
1. Test as Author user (has
edit_posts, noedit_css):style.cssis stripped from all blocks in the saved content2. Test as Admin user (has
edit_css):3. Code Editor bypass test:
<!-- wp:paragraph {"style":{"css":"body{background:red}"}} -->style.cssattribute is stripped from saved content4. REST API test:
PUT /wp-json/wp/v2/posts/<id>with a content payload containing CSS in block attributes5. Existing tests:
phpunit/block-supports/custom-css-test.phpshould still pass (they test render behaviour, not save filtering)