Repository navigation
fix!: preserve patterns' namespaced names and sanitize scripts - #188
Conversation
|
|
||
| return array_filter( | ||
| [ | ||
| 'name' => sanitize_text_field( (string) $pattern['name'] ), |
There was a problem hiding this comment.
@justlevine Edge case: if both sites have a pattern with the same name but different versions, the pushed version is silently ignored. register_block_patterns_if_not_exist() skips names that are already registered.
thoughts?
There was a problem hiding this comment.
@Kallyan01 this is preexisting, right?
I'm unsure what the expected behavior would be: always the pushed version, the latest version of them? 🤔
Create a new GH issue in /onepress for tracking.
There was a problem hiding this comment.
@justlevine ig no. Previously, the slash was omitted from the pushed pattern names, which caused them to be registered under different names without any additional changes.
What about de-registering any existing pattern before we register the new one, so the pushed one gets priority or making them co-exist together?
There was a problem hiding this comment.
How would dropping the prefix cause fewer conflicts instead of more conflicts?
There was a problem hiding this comment.
@justlevine if I'm not wrong, it isn't really dropping the prefix. it only removes the slash, so the namespace text became twentytwentyfivebanner-about-book. That's no longer a namespaced name, so it can never match a pattern the brand site registers itself. The shared copy was registered next to the local one instead of competing with it.
There was a problem hiding this comment.
I genuinely don't understand what you're trying to say here....
There was a problem hiding this comment.
Per sync w. @up1512001 :
Short term: follow Templates (skip pattern if already registered) + array_map( explode '/' ... ) ... )
Long term: show notice on governing site. Needs issue.
There was a problem hiding this comment.
@justlevine Okay, if we're intentionally skipping registration, the current logic makes sense for now. However, adding a notice would make the behavior more user-friendly, as users might otherwise be unaware of why the registration failed.
Screen.Recording.2026-10-28.at.21.53.37.mov
| [ | ||
| 'name' => sanitize_text_field( (string) $pattern['name'] ), | ||
| 'title' => sanitize_text_field( (string) $pattern['title'] ), | ||
| 'content' => wp_kses_post( (string) $pattern['content'] ), |
There was a problem hiding this comment.
@justlevine wp_kses_post strips inline SVGs, which breaks the rt-carousel arrows in the Vertex patterns case-study-carousel, featured-case-studies, feature-stats-with-testimonial and testimonials-carousel. I think switching to wp_kses with the post allowlist plus SVG shape tags only, leaving out the XSS-prone parts (, , animation tags, on* handlers). Thoughts?
There was a problem hiding this comment.
@Kallyan01 IMO this feels like a bug in rt-carousel. If it's using SVGs, then shouldn't it use set wp_kses_allowed_html filter to allow them?
Check with rt-carousel, and we'll make a decision based on what they say.
What
Brand sites keep the namespaced names of the patterns they receive, and strip scripts from them. The governing site sanitizes the patterns brand sites send back the same way.
Why
receive_patterns()ran the name throughsanitize_key(), so a sharedtheme/herowas stored and registered asthemehero.get_all_brand_site_patterns()passed on whatever each brand site listed without sanitizing it.Related Issue(s):
How
PATTERN_SCHEMAlists the fields a brand site registers a pattern with.sanitize_pattern()keeps only those fields, each sanitized. The content goes throughwp_kses_post(), and the name throughsanitize_text_field(), which keeps the slash./receive-patternsstores each pattern under its sanitized name, which is the name the brand site registers it with./get-all-brand-site-patternsskips a pattern that doesn't matchPATTERN_SCHEMAand sanitizes the rest.set_up_brand_site()helper onTestCase.AI Disclosure
Claude Code identified the issue, wrote the tests the tests, and drafted the PR description.
Testing Instructions
Screenshots
Additional Info
This is no migration for the old sanitized names before this PR. Uninstall/reinstall is the current migration path.
Checklist