fix: show blocksy theme styles in the design library #3630
Conversation
WalkthroughAggregates Blocksy dynamic and static CSS into Stackable’s global-theme styles with sanitization, exposes those styles via a new design-library filter-backed method, removes direct global stylesheet localization from editor init, and adds a Blocksy SCSS custom property for button background. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Editor as Block Editor
participant WP as WordPress Filters
participant DL as Stackable_Design_Library
participant Blocksy as Blocksy Integration
Editor->>WP: apply_filters('stackable_localize_script', args)
WP->>DL: invoke add_wp_theme_global_styles(args)
activate DL
DL->>WP: apply_filters('stackable.design-library.global-theme-styles', styles)
WP->>Blocksy: stackable_blocksy_theme_global_styles(styles)
activate Blocksy
Note right of Blocksy #D6EAF8: collect dynamic CSS\n(via blocksy_manager)\nand gather allowed static CSS files
Blocksy->>Blocksy: validate file paths & extension\nread .css contents
Blocksy->>Blocksy: sanitize aggregated CSS\n(stackable_sanitize_css_string)
Blocksy-->>WP: return aggregated sanitized styles
deactivate Blocksy
DL->>DL: styles += wp_get_global_stylesheet()
DL-->>WP: return args with wpGlobalStylesInlineCss
deactivate DL
WP-->>Editor: localized args updated
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🤖 Pull request artifacts
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/compatibility/blocksy/index.php (1)
93-125: Consider caching the aggregated styles.This function aggregates CSS from multiple sources on every filter invocation. Since these styles are relatively static (changing only when theme options or files are updated), consider implementing transient-based caching to improve performance.
Example caching approach:
function stackable_blocksy_theme_global_styles( $styles ) { $cache_key = 'stackable_blocksy_global_styles_' . get_stylesheet(); $cached_styles = get_transient( $cache_key ); if ( false !== $cached_styles ) { return $styles . $cached_styles; } $blocksy_styles = ''; // ... existing logic to build $blocksy_styles ... // Cache for 1 day set_transient( $cache_key, $blocksy_styles, DAY_IN_SECONDS ); return $styles . $blocksy_styles; }Note: You'll need to clear this transient when Blocksy theme options are saved or theme files are updated.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/compatibility/blocksy/index.php(1 hunks)src/compatibility/blocksy/style.scss(1 hunks)src/compatibility/index.php(1 hunks)src/design-library/init.php(2 hunks)src/init.php(0 hunks)
💤 Files with no reviewable changes (1)
- src/init.php
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: build
- GitHub Check: PHP 7.3 and WP latest
- GitHub Check: PHP 8.2 and WP 6.6.2
- GitHub Check: PHP 8.2 and WP 6.7.2
- GitHub Check: PHP 8.2 and WP 6.5.5
- GitHub Check: PHP 8.2 and WP latest
- GitHub Check: PHP 7.3 and WP 6.5.5
🔇 Additional comments (6)
src/compatibility/index.php (1)
11-11: LGTM!The Blocksy compatibility include follows the established pattern and correctly loads the integration module.
src/compatibility/blocksy/style.scss (1)
6-6: LGTM!The CSS custom property correctly maps Stackable's button background color to Blocksy's theme variable, maintaining consistency with the existing link and heading color fallbacks.
src/design-library/init.php (2)
34-34: LGTM!The filter registration correctly wires the global styles aggregation into the script localization pipeline.
305-313: LGTM!The method provides a clean extension point via the
stackable.design-library.global-theme-stylesfilter, allowing theme-specific integrations (like Blocksy) to contribute styles before appending the core WordPress global stylesheet.src/compatibility/blocksy/index.php (2)
95-101: LGTM!The dynamic CSS retrieval correctly checks for function availability and safely appends the Blocksy backend CSS.
127-127: LGTM!The filter registration correctly hooks into the design library's global theme styles pipeline.
src/compatibility/blocksy/index.php
Outdated
| $mime = mime_content_type( $file_path ); | ||
| $is_valid_mime = $mime === 'text/css' || $mime === 'text/plain'; | ||
| if ( file_exists( $file_path ) && is_readable( $file_path ) && $is_valid_mime ) { | ||
| $styles .= file_get_contents( $file_path ); |
There was a problem hiding this comment.
since this contents come from a file, we would need to add some sanitization here
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/compatibility/blocksy/index.php(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/compatibility/blocksy/index.php (3)
src/plugins/global-settings/color-schemes/editor-loader.js (2)
css(33-34)styles(139-139)src/plugins/global-settings/buttons-and-icons/index.js (1)
styles(387-387)src/plugins/global-settings/spacing-and-borders/index.js (1)
styles(419-419)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: PHP 8.2 and WP 6.7.2
- GitHub Check: PHP 8.2 and WP 6.6.2
- GitHub Check: build
- GitHub Check: PHP 8.2 and WP latest
- GitHub Check: PHP 8.2 and WP 6.5.5
- GitHub Check: PHP 7.3 and WP 6.5.5
- GitHub Check: PHP 7.3 and WP latest
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/compatibility/blocksy/index.php (2)
103-114: Consider allowing data: URIs and relative paths in the URL allowlist.The current URL sanitization only permits HTTP/HTTPS URLs from the theme directory. This may break legitimate CSS features:
data:URIs (commonly used for embedded SVGs, fonts, or small images)- Relative paths (e.g.,
url('../fonts/...')orurl('./images/...'))These are safe CSS features that don't pose security risks when used in stylesheets.
Apply this diff to expand the allowlist:
// Only allow URLs from the theme directory $theme_uri = preg_quote( get_template_directory_uri(), '/' ); $css = preg_replace_callback( '/url\(\s*[\'"]?\s*(https?:\/\/[^\'")]+)\s*[\'"]?\s*\)/i', function( $matches ) use ( $theme_uri ) { if ( preg_match( "/^{$theme_uri}/i", $matches[1] ) ) { return $matches[0]; // Keep theme URLs } return 'url("")'; // Remove others }, $css ); + + // Additionally preserve data: URIs and relative paths which are safe + // (The above regex only matches http/https, so data: and relative paths pass through)Alternatively, to be more explicit about what's allowed, modify the regex pattern:
$theme_uri = preg_quote( get_template_directory_uri(), '/' ); $css = preg_replace_callback( - '/url\(\s*[\'"]?\s*(https?:\/\/[^\'")]+)\s*[\'"]?\s*\)/i', + '/url\(\s*[\'"]?\s*([^\'")]+)\s*[\'"]?\s*\)/i', function( $matches ) use ( $theme_uri ) { + $url = $matches[1]; + // Allow theme URLs, data: URIs, and relative paths + if ( preg_match( "/^{$theme_uri}/i", $url ) || + preg_match( '/^data:/i', $url ) || + ! preg_match( '/^https?:/i', $url ) ) { - if ( preg_match( "/^{$theme_uri}/i", $matches[1] ) ) { return $matches[0]; } return 'url("")'; }, $css );
157-161: Remove redundant line.Line 157 assigns
$file_pathbut it's immediately overwritten in line 161, making it dead code.Apply this diff to clean up:
foreach ( $blocksy_static_files as $file ) { if ( isset( $file['url'] ) ) { - $file_path = get_template_directory() . $file['url']; - // Normalize and validate the path to prevent traversal $file_url = ltrim( $file['url'], '/' ); $file_path = get_template_directory() . '/' . $file_url;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/compatibility/blocksy/index.php(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/compatibility/blocksy/index.php (3)
src/plugins/global-settings/color-schemes/editor-loader.js (2)
css(33-34)styles(139-139)src/plugins/global-settings/buttons-and-icons/index.js (1)
styles(387-387)src/plugins/global-settings/spacing-and-borders/index.js (1)
styles(419-419)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: PHP 8.2 and WP 6.7.2
- GitHub Check: PHP 8.2 and WP latest
- GitHub Check: PHP 7.3 and WP 6.5.5
- GitHub Check: PHP 8.2 and WP 6.6.2
- GitHub Check: PHP 8.2 and WP 6.5.5
- GitHub Check: PHP 7.3 and WP latest
- GitHub Check: build
🔇 Additional comments (1)
src/compatibility/blocksy/index.php (1)
135-188: Good implementation of security measures.The function properly addresses the security concerns from previous reviews:
- ✅ Path traversal protection using
realpath()and directory validation (lines 159-168)- ✅ Extension-based validation instead of unreliable MIME type checking (lines 171-174)
- ✅ Error handling for
file_get_contents()(lines 175-178)- ✅ Comprehensive CSS sanitization applied to all collected styles (line 186)
The integration with Blocksy's dynamic and static CSS systems is well-structured.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/compatibility/blocksy/index.php (3)
93-133: Comprehensive CSS sanitization with room for child theme support.The sanitization function implements strong security measures including:
- Tag stripping and expression removal
- URL allowlist restricted to theme directory
- Blocking unsafe tokens (eval, mocha, behavior)
- Normalization of spacing and empty rules
All past security concerns have been properly addressed.
Optional: Consider extending URL allowlist to support child themes by also checking
get_stylesheet_directory_uri():// Only allow URLs from the theme directory - $theme_uri = preg_quote( get_template_directory_uri(), '/' ); + $parent_theme_uri = preg_quote( get_template_directory_uri(), '/' ); + $child_theme_uri = preg_quote( get_stylesheet_directory_uri(), '/' ); $css = preg_replace_callback( '/url\(\s*[\'"]?\s*(https?:\/\/[^\'")]+)\s*[\'"]?\s*\)/i', - function( $matches ) use ( $theme_uri ) { - if ( preg_match( "/^{$theme_uri}/i", $matches[1] ) ) { + function( $matches ) use ( $parent_theme_uri, $child_theme_uri ) { + if ( preg_match( "/^{$parent_theme_uri}/i", $matches[1] ) || preg_match( "/^{$child_theme_uri}/i", $matches[1] ) ) { return $matches[0]; // Keep theme URLs } return 'url("")'; // Remove others }, $css );
137-143: Sanitize dynamic CSS for consistency and add type safety.While Blocksy's dynamic CSS output is likely trusted, applying sanitization ensures defense in depth and consistency with the static CSS handling (lines 183-187). Additionally, the return value should be validated to ensure it's a string.
Apply this diff to add sanitization and type checking:
if ( function_exists( 'blocksy_manager' ) ) { $blocksy_css = blocksy_manager()->dynamic_css->load_backend_dynamic_css([ 'echo' => false ] ); - - $styles .= $blocksy_css; + + if ( is_string( $blocksy_css ) && ! empty( $blocksy_css ) ) { + $blocksy_css = stackable_sanitize_css_string( $blocksy_css ); + $styles .= $blocksy_css; + } }
145-188: Excellent security implementation with one type safety consideration.The static CSS handling properly addresses all past security concerns:
- Path traversal prevention using
realpath()validation- Extension-based validation instead of MIME type checking
- Error handling for file operations
- Sanitization before appending to styles
Type safety improvement: Add validation for
all_static_files()return value:if ( class_exists( 'Blocksy_Static_Css_Files' ) ) { $blocksy_static_files = ( new Blocksy_Static_Css_Files() )->all_static_files(); + + if ( ! is_array( $blocksy_static_files ) ) { + return $styles; + } $blocksy_static_files = array_filter(
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/compatibility/blocksy/index.php(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/compatibility/blocksy/index.php (3)
src/plugins/global-settings/color-schemes/editor-loader.js (2)
css(33-34)styles(139-139)src/plugins/global-settings/buttons-and-icons/index.js (1)
styles(387-387)src/plugins/global-settings/spacing-and-borders/index.js (1)
styles(419-419)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: PHP 7.3 and WP 6.5.5
- GitHub Check: PHP 7.3 and WP latest
- GitHub Check: PHP 8.2 and WP 6.7.2
🔇 Additional comments (1)
src/compatibility/blocksy/index.php (1)
193-193: LGTM!Filter registration is correct and properly integrates Blocksy theme styles into Stackable's design library global theme styles workflow.
2 participants
Summary by CodeRabbit
New Features
Style
Bug Fixes / Reliability
Chores