Fix: security for unfiltered html cap #6
Conversation
WalkthroughVersion bumped to 1.3.2 across configuration files. Comprehensive sanitization framework added to action type classes with type-specific data validation. Security enhancements include permission-based attribute restrictions and dangerous attribute detection. Editor UI updated to surface permission restrictions. Build script modified for premium version handling. Documentation expanded significantly. Changes
Sequence DiagramsequenceDiagram
participant User
participant Editor as Editor UI
participant Interaction as Interaction Class
participant ActionType as Action Type Handler
participant Security as Permission Check
participant Database as Post Storage
User->>Editor: Submit action with data
Editor->>Interaction: Call update with action data
Interaction->>Interaction: Call secure_interaction_data()
Interaction->>ActionType: Call sanitize_data_for_saving()
alt Dangerous Attribute Detected
ActionType->>Security: Check if user has unfiltered_html
alt User Lacks Permission
Security-->>ActionType: Return WP_Error
ActionType-->>Interaction: Return WP_Error
Interaction-->>Editor: Display error
else User Has Permission
Security-->>ActionType: Proceed
ActionType->>ActionType: Apply sanitization
ActionType-->>Interaction: Return sanitized value
end
else Safe Action Type
ActionType->>ActionType: Apply type-specific sanitization
ActionType-->>Interaction: Return sanitized value
end
Interaction->>Interaction: Validate sanitized data
Interaction->>Database: Save sanitized action
Interaction->>Interaction: Clear cache
Interaction-->>Editor: Return success
Editor-->>User: Display confirmation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45–60 minutes Areas requiring extra attention:
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
♻️ Duplicate comments (1)
src/action-types/class-action-type-scale.php (1)
50-66: LGTM! Follows consistent pattern.The implementation is correct and consistent with other numeric field validations (e.g., skew, rotate).
🧹 Nitpick comments (6)
readme.txt (1)
1-8: Docs and metadata match the release; consider filling the Upgrade NoticeThe updated header (requires/tested/stable), expanded description, feature lists, and the
1.3.2changelog entries (“restrictions for users without unfiltered_html capabilities” and “additional input sanitization”) all line up well with the code changes in this PR.The only minor nit is that
== Upgrade Notice ==is present but empty; if you want that section to show something in the plugin directory, consider adding a brief note for 1.3.2, otherwise you could also omit the section entirely.Also applies to: 15-28, 72-75, 104-108, 144-148, 152-156, 159-161, 169-170
src/action-types/class-action-type-skew.php (1)
50-66: LGTM! Consider extracting numeric validation helper.The sanitization logic is correct and includes proper null checks. The numeric validation pattern (lines 52-56 and 59-63) is identical across multiple action types (scale, rotate, move, opacity).
Consider extracting this pattern to a helper method in the abstract base class:
protected function sanitize_numeric_field( $value, $key ) { if ( is_array( $value ) && isset( $value[ $key ] ) ) { if ( is_numeric( $value[ $key ] ) ) { $value[ $key ] = $value[ $key ] + 0; } else { $value[ $key ] = null; } } return $value; }Then use it as:
$value = $this->sanitize_numeric_field( $value, 'x' );src/action-types/class-action-type-toggle-class.php (1)
54-57:sanitize_key()may be too restrictive for CSS class names.CSS class names are case-sensitive and can contain uppercase letters (e.g.,
myButton,Card__header).sanitize_key()converts to lowercase and removes characters beyond[a-z0-9_-], which may break BEM-style or component-based class naming conventions.Consider using a more permissive sanitization that preserves case:
if ( is_array( $value ) && isset( $value['class'] ) ) { - $value['class'] = sanitize_key( $value['class'] ); + // Sanitize class name: allow alphanumeric, hyphens, underscores, preserve case + $value['class'] = preg_replace( '/[^a-zA-Z0-9_\-]/', '', $value['class'] ); }src/action-types/class-action-type-move.php (1)
62-75: LGTM! Clean loop-based sanitization.The loop approach is more maintainable than repetitive conditionals used in sibling files (e.g.,
class-action-type-scale.php,class-action-type-skew.php). The intentional preservation of empty strings aligns with the property defaults.For consistency with other action types that check
is_array($value)first, you could optionally add that guard:public function sanitize_data_for_saving( $value ) { + if ( ! is_array( $value ) ) { + return $value; + } // Ensure x, y, z are sanitized as numeric... foreach ( [ 'x', 'y', 'z' ] as $key ) {src/action-types/abstract-action-type.php (1)
279-290: Good foundational CSS sanitization, consider extending coverage.The implementation correctly handles the main CSS injection vectors (
expression()andjavascript:URIs). The recursive regex for nested parentheses is well-crafted.For comprehensive protection, consider also sanitizing:
url()values containingjavascript:ordata:URIs- Legacy IE vectors like
-moz-bindingandbehavior:public function sanitize_style_value( $string ) { if ( ! is_string( $string ) ) { return $string; } // Remove all expression(...) (case-insensitive). $string = preg_replace( '/expression\s*\((?:[^\(\)]|(?R))*\)/i', '', $string ); // Remove all javascript: URIs (case-insensitive). $string = preg_replace( '/javascript\s*:/i', '', $string ); + // Remove data: URIs which can execute scripts. + $string = preg_replace( '/data\s*:/i', '', $string ); + + // Remove legacy IE behavior/binding vectors. + $string = preg_replace( '/(behavior|(-moz-)?binding)\s*:/i', '', $string ); return $string; }src/action-types/class-action-type-update-attribute.php (1)
62-87: Consider addingsrcdocto the dangerous attributes list.The
srcdocattribute (used with iframes) can contain arbitrary HTML and JavaScript, making it as dangerous ashrefwithjavascript:URIs. It should be included in the dangerous attributes list.Additionally, you might consider adding
posteranddata(object tag) for completeness, though they are lower risk.$dangerous_attributes = [ 'href', 'src', + 'srcdoc', 'action', 'formaction', // 'style', // Can contain CSS with expression() or javascript: URIs 'form', 'formmethod', 'formtarget', + 'poster', + 'data', ];
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (25)
.vscode/settings.json(1 hunks)interactions.php(3 hunks)package.json(1 hunks)readme.txt(4 hunks)scripts/package.js(1 hunks)src/action-types/abstract-action-type.php(1 hunks)src/action-types/class-action-type-background-color.php(1 hunks)src/action-types/class-action-type-background-image.php(1 hunks)src/action-types/class-action-type-css-rule.php(1 hunks)src/action-types/class-action-type-display.php(1 hunks)src/action-types/class-action-type-move.php(1 hunks)src/action-types/class-action-type-opacity.php(1 hunks)src/action-types/class-action-type-redirect-to-url.php(1 hunks)src/action-types/class-action-type-rotate.php(1 hunks)src/action-types/class-action-type-scale.php(1 hunks)src/action-types/class-action-type-skew.php(1 hunks)src/action-types/class-action-type-text-color.php(1 hunks)src/action-types/class-action-type-toggle-class.php(1 hunks)src/action-types/class-action-type-toggle-video.php(1 hunks)src/action-types/class-action-type-update-attribute.php(2 hunks)src/action-types/class-action-type-visibility.php(1 hunks)src/class-interaction.php(3 hunks)src/editor/components/timeline/property-control.js(2 hunks)src/editor/editor.php(1 hunks)src/freemius.php(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (12)
src/action-types/class-action-type-text-color.php (3)
src/action-types/abstract-action-type.php (2)
sanitize_data_for_saving(268-271)sanitize_style_value(279-290)src/action-types/class-action-type-background-color.php (1)
sanitize_data_for_saving(56-59)src/action-types/class-action-type-toggle-class.php (1)
sanitize_data_for_saving(54-67)
src/action-types/class-action-type-rotate.php (8)
src/action-types/abstract-action-type.php (2)
sanitize_data_for_saving(268-271)sanitize_style_value(279-290)src/action-types/class-action-type-background-color.php (1)
sanitize_data_for_saving(56-59)src/action-types/class-action-type-background-image.php (1)
sanitize_data_for_saving(37-40)src/action-types/class-action-type-css-rule.php (1)
sanitize_data_for_saving(44-47)src/action-types/class-action-type-opacity.php (1)
sanitize_data_for_saving(40-49)src/action-types/class-action-type-text-color.php (1)
sanitize_data_for_saving(38-43)src/action-types/class-action-type-update-attribute.php (1)
sanitize_data_for_saving(89-133)src/action-types/class-action-type-visibility.php (1)
sanitize_data_for_saving(44-52)
src/action-types/class-action-type-visibility.php (10)
src/action-types/abstract-action-type.php (1)
sanitize_data_for_saving(268-271)src/action-types/class-action-type-background-color.php (1)
sanitize_data_for_saving(56-59)src/action-types/class-action-type-background-image.php (1)
sanitize_data_for_saving(37-40)src/action-types/class-action-type-css-rule.php (1)
sanitize_data_for_saving(44-47)src/action-types/class-action-type-display.php (1)
sanitize_data_for_saving(59-80)src/action-types/class-action-type-opacity.php (1)
sanitize_data_for_saving(40-49)src/action-types/class-action-type-redirect-to-url.php (1)
sanitize_data_for_saving(41-50)src/action-types/class-action-type-text-color.php (1)
sanitize_data_for_saving(38-43)src/action-types/class-action-type-toggle-class.php (1)
sanitize_data_for_saving(54-67)src/action-types/class-action-type-toggle-video.php (1)
sanitize_data_for_saving(58-73)
src/action-types/abstract-action-type.php (15)
src/action-types/class-action-type-background-color.php (1)
sanitize_data_for_saving(56-59)src/action-types/class-action-type-background-image.php (1)
sanitize_data_for_saving(37-40)src/action-types/class-action-type-css-rule.php (1)
sanitize_data_for_saving(44-47)src/action-types/class-action-type-display.php (1)
sanitize_data_for_saving(59-80)src/action-types/class-action-type-move.php (1)
sanitize_data_for_saving(62-75)src/action-types/class-action-type-opacity.php (1)
sanitize_data_for_saving(40-49)src/action-types/class-action-type-redirect-to-url.php (1)
sanitize_data_for_saving(41-50)src/action-types/class-action-type-rotate.php (1)
sanitize_data_for_saving(67-96)src/action-types/class-action-type-scale.php (1)
sanitize_data_for_saving(50-66)src/action-types/class-action-type-skew.php (1)
sanitize_data_for_saving(50-66)src/action-types/class-action-type-text-color.php (1)
sanitize_data_for_saving(38-43)src/action-types/class-action-type-toggle-class.php (1)
sanitize_data_for_saving(54-67)src/action-types/class-action-type-toggle-video.php (1)
sanitize_data_for_saving(58-73)src/action-types/class-action-type-update-attribute.php (1)
sanitize_data_for_saving(89-133)src/action-types/class-action-type-visibility.php (1)
sanitize_data_for_saving(44-52)
src/action-types/class-action-type-css-rule.php (11)
src/action-types/abstract-action-type.php (2)
sanitize_data_for_saving(268-271)sanitize_style_value(279-290)src/action-types/class-action-type-background-color.php (1)
sanitize_data_for_saving(56-59)src/action-types/class-action-type-background-image.php (1)
sanitize_data_for_saving(37-40)src/action-types/class-action-type-display.php (1)
sanitize_data_for_saving(59-80)src/action-types/class-action-type-opacity.php (1)
sanitize_data_for_saving(40-49)src/action-types/class-action-type-redirect-to-url.php (1)
sanitize_data_for_saving(41-50)src/action-types/class-action-type-scale.php (1)
sanitize_data_for_saving(50-66)src/action-types/class-action-type-skew.php (1)
sanitize_data_for_saving(50-66)src/action-types/class-action-type-text-color.php (1)
sanitize_data_for_saving(38-43)src/action-types/class-action-type-toggle-class.php (1)
sanitize_data_for_saving(54-67)src/action-types/class-action-type-visibility.php (1)
sanitize_data_for_saving(44-52)
src/class-interaction.php (16)
src/action-types/abstract-action-type.php (1)
sanitize_data_for_saving(268-271)src/action-types/class-action-type-background-color.php (1)
sanitize_data_for_saving(56-59)src/action-types/class-action-type-background-image.php (1)
sanitize_data_for_saving(37-40)src/action-types/class-action-type-css-rule.php (1)
sanitize_data_for_saving(44-47)src/action-types/class-action-type-display.php (1)
sanitize_data_for_saving(59-80)src/action-types/class-action-type-move.php (1)
sanitize_data_for_saving(62-75)src/action-types/class-action-type-opacity.php (1)
sanitize_data_for_saving(40-49)src/action-types/class-action-type-redirect-to-url.php (1)
sanitize_data_for_saving(41-50)src/action-types/class-action-type-rotate.php (1)
sanitize_data_for_saving(67-96)src/action-types/class-action-type-scale.php (1)
sanitize_data_for_saving(50-66)src/action-types/class-action-type-skew.php (1)
sanitize_data_for_saving(50-66)src/action-types/class-action-type-text-color.php (1)
sanitize_data_for_saving(38-43)src/action-types/class-action-type-toggle-class.php (1)
sanitize_data_for_saving(54-67)src/action-types/class-action-type-toggle-video.php (1)
sanitize_data_for_saving(58-73)src/action-types/class-action-type-update-attribute.php (1)
sanitize_data_for_saving(89-133)src/action-types/class-action-type-visibility.php (1)
sanitize_data_for_saving(44-52)
src/action-types/class-action-type-toggle-class.php (4)
src/action-types/abstract-action-type.php (1)
sanitize_data_for_saving(268-271)src/action-types/class-action-type-toggle-video.php (1)
sanitize_data_for_saving(58-73)src/action-types/class-action-type-update-attribute.php (1)
sanitize_data_for_saving(89-133)src/action-types/class-action-type-visibility.php (1)
sanitize_data_for_saving(44-52)
src/action-types/class-action-type-background-image.php (3)
src/action-types/abstract-action-type.php (2)
sanitize_data_for_saving(268-271)sanitize_style_value(279-290)src/action-types/class-action-type-background-color.php (1)
sanitize_data_for_saving(56-59)src/action-types/class-action-type-toggle-class.php (1)
sanitize_data_for_saving(54-67)
src/action-types/class-action-type-redirect-to-url.php (8)
src/action-types/abstract-action-type.php (1)
sanitize_data_for_saving(268-271)src/action-types/class-action-type-background-color.php (1)
sanitize_data_for_saving(56-59)src/action-types/class-action-type-background-image.php (1)
sanitize_data_for_saving(37-40)src/action-types/class-action-type-css-rule.php (1)
sanitize_data_for_saving(44-47)src/action-types/class-action-type-opacity.php (1)
sanitize_data_for_saving(40-49)src/action-types/class-action-type-text-color.php (1)
sanitize_data_for_saving(38-43)src/action-types/class-action-type-toggle-class.php (1)
sanitize_data_for_saving(54-67)src/action-types/class-action-type-visibility.php (1)
sanitize_data_for_saving(44-52)
src/action-types/class-action-type-toggle-video.php (15)
src/action-types/abstract-action-type.php (1)
sanitize_data_for_saving(268-271)src/action-types/class-action-type-background-color.php (1)
sanitize_data_for_saving(56-59)src/action-types/class-action-type-background-image.php (1)
sanitize_data_for_saving(37-40)src/action-types/class-action-type-css-rule.php (1)
sanitize_data_for_saving(44-47)src/action-types/class-action-type-display.php (1)
sanitize_data_for_saving(59-80)src/action-types/class-action-type-move.php (1)
sanitize_data_for_saving(62-75)src/action-types/class-action-type-opacity.php (1)
sanitize_data_for_saving(40-49)src/action-types/class-action-type-redirect-to-url.php (1)
sanitize_data_for_saving(41-50)src/action-types/class-action-type-rotate.php (1)
sanitize_data_for_saving(67-96)src/action-types/class-action-type-scale.php (1)
sanitize_data_for_saving(50-66)src/action-types/class-action-type-skew.php (1)
sanitize_data_for_saving(50-66)src/action-types/class-action-type-text-color.php (1)
sanitize_data_for_saving(38-43)src/action-types/class-action-type-toggle-class.php (1)
sanitize_data_for_saving(54-67)src/action-types/class-action-type-update-attribute.php (1)
sanitize_data_for_saving(89-133)src/action-types/class-action-type-visibility.php (1)
sanitize_data_for_saving(44-52)
src/action-types/class-action-type-update-attribute.php (3)
src/action-types/abstract-action-type.php (2)
sanitize_data_for_saving(268-271)sanitize_style_value(279-290)src/action-types/class-action-type-background-color.php (1)
sanitize_data_for_saving(56-59)src/action-types/class-action-type-toggle-video.php (1)
sanitize_data_for_saving(58-73)
src/action-types/class-action-type-opacity.php (15)
src/action-types/abstract-action-type.php (1)
sanitize_data_for_saving(268-271)src/action-types/class-action-type-background-color.php (1)
sanitize_data_for_saving(56-59)src/action-types/class-action-type-background-image.php (1)
sanitize_data_for_saving(37-40)src/action-types/class-action-type-css-rule.php (1)
sanitize_data_for_saving(44-47)src/action-types/class-action-type-display.php (1)
sanitize_data_for_saving(59-80)src/action-types/class-action-type-move.php (1)
sanitize_data_for_saving(62-75)src/action-types/class-action-type-redirect-to-url.php (1)
sanitize_data_for_saving(41-50)src/action-types/class-action-type-rotate.php (1)
sanitize_data_for_saving(67-96)src/action-types/class-action-type-scale.php (1)
sanitize_data_for_saving(50-66)src/action-types/class-action-type-skew.php (1)
sanitize_data_for_saving(50-66)src/action-types/class-action-type-text-color.php (1)
sanitize_data_for_saving(38-43)src/action-types/class-action-type-toggle-class.php (1)
sanitize_data_for_saving(54-67)src/action-types/class-action-type-toggle-video.php (1)
sanitize_data_for_saving(58-73)src/action-types/class-action-type-update-attribute.php (1)
sanitize_data_for_saving(89-133)src/action-types/class-action-type-visibility.php (1)
sanitize_data_for_saving(44-52)
🔇 Additional comments (20)
src/freemius.php (2)
604-604: LGTM!Good refactor removing the inline
onClickhandler in favor of a class-based approach with an event listener. This is more maintainable and aligns with the encapsulated event handling pattern in the IIFE below.
613-622: LGTM!Good encapsulation using an IIFE. The click-outside-to-close handler properly checks the busy state to prevent accidental dismissal during async operations. Event handlers are well-scoped.
package.json (1)
3-3: Version bump consistency looks goodVersion updated to
1.3.2here matches the plugin header and readme stable tag, keeping build tooling and PHP code in sync..vscode/settings.json (1)
43-46: Explicitly includingpro__premium_onlyin search is reasonableSetting
"**/pro__premium_only": falseundersearch.excludeensures that folder stays searchable even if excluded elsewhere (e.g., user/workspace defaults). If that’s the intention (to always see premium code in search), this is a good, low-impact editor tweak.interactions.php (1)
10-10: Versioning and activation option update are consistentUpdating the header
Version,INTERACT_VERSIONconstant, and callingupdate_option( 'interact_plugin_version', INTERACT_VERSION )insideinteract_on_activation()keeps the stored version aligned with the running code and maintains the existing migration flow viainteract/on_plugin_update. No issues from what’s visible here.Also applies to: 21-21, 35-36
src/editor/editor.php (1)
66-80: UsingcurrentUserCanUnfilteredHtmlin localized data is appropriateAdding
currentUserCanUnfilteredHtml => current_user_can( 'unfiltered_html' )cleanly exposes this capability to the editor script so the UI can show restrictions for non‑privileged users. This is safe to expose and will serialize as a proper boolean.Verified that the flag is used exclusively for UI gating—specifically to display a
restrictedNoticein the property control component when users lack the capability. Server-side enforcement is robustly implemented: unprivileged users triggerwp_kses_post()sanitization on string values, and dangerous HTML attributes are blocked via validation logic that returnsWP_Errorfor violations. The client flag drives UX only; security decisions remain fully enforced on the server.src/action-types/class-action-type-display.php (1)
59-80: LGTM! Proper whitelist validation.The sanitization logic correctly validates the display value against allowed CSS display values and provides a safe fallback. The strict comparison in
in_array()is appropriate for security.src/action-types/class-action-type-toggle-video.php (1)
58-73: LGTM! Comprehensive validation.The sanitization correctly validates both the mode enum and numeric startTime field with appropriate fallbacks and null safety.
src/action-types/class-action-type-redirect-to-url.php (1)
41-50: LGTM! Appropriate URL sanitization.Using
esc_url()is the correct approach for sanitizing URLs in WordPress, with proper type checking and null fallback.src/action-types/class-action-type-visibility.php (1)
44-52: LGTM! Secure whitelist validation.The sanitization correctly validates visibility values against an allowed list with a safe fallback, using strict comparison for security.
src/action-types/class-action-type-text-color.php (1)
38-43: LGTM! Robust defensive check before sanitization.The implementation correctly guards against non-array or missing 'color' key before sanitizing, which is safer than some sibling implementations (e.g.,
class-action-type-background-color.phpat lines 55-58 which accesses$value['color']without checking). This pattern should ideally be applied consistently across all action types.src/action-types/class-action-type-toggle-class.php (1)
59-64: LGTM! Action validation is correct.Allowlist validation with strict comparison and safe default fallback is the correct approach for enum-like values.
src/editor/components/timeline/property-control.js (1)
53-74: LGTM! Security restriction notice correctly implemented.The conditional rendering logic is correct—showing the restriction notice only when both
restrictedNoticeexists and the user lacksunfiltered_htmlcapability. The HTML tag stripping for the tooltip text is a reasonable approach.src/action-types/abstract-action-type.php (1)
268-271: LGTM! Clean extension point for subclass sanitization.The default no-op implementation with clear documentation provides a solid hook for action-type-specific sanitization logic.
src/action-types/class-action-type-opacity.php (1)
40-49: LGTM! Consistent numeric sanitization.The implementation correctly follows the established pattern for numeric value sanitization with proper type checking and null fallback for invalid input.
src/class-interaction.php (3)
62-74: LGTM - Sanitization workflow properly integrated.The changes correctly integrate the sanitization workflow: sanitizing data before persistence, returning early on errors, and using the sanitized data for
post_content. The intentional use of$interaction_datafor metadata fields (key, title, status) while using$sanitized_datafor the serialized content is appropriate.
86-108: Verify the intended behavior for nested mixed-type arrays.The sanitization logic correctly bypasses for
unfiltered_htmlusers and recursively handles arrays. However, note that after recursively processing an array, the subsequentis_stringcheck on line 103 won't re-sanitize the array itself (which is correct), but intermediate non-string/non-array types (integers, booleans, objects) pass through unchanged. This is likely intentional, but verify this is the expected behavior for all action value types.
125-135: LGTM - Action value sanitization order is correct.The sanitization applies general HTML sanitization first via
sanitize_interaction_value, then type-specific sanitization viasanitize_data_for_saving. The WP_Error handling correctly propagates failures from type-specific sanitizers (like the dangerous attribute check in update-attribute). The sanitized value is properly written back and used for HMAC signing.src/action-types/class-action-type-update-attribute.php (2)
33-33: LGTM - User-facing security notice added.The
restrictedNoticeprovides clear guidance to users about the permission requirements for certain attributes.
89-133: Verify intentional sanitization for users withunfiltered_htmlcapability.Sanitization via
sanitize_key(line 100) andsanitize_style_value(line 105) is applied to all users before theunfiltered_htmlcheck at line 116. This means even administrators with unfiltered HTML capability will have:
- Attribute names lowercased and special characters stripped (e.g.,
data:customAttr→datacustomattr)- Values processed through
sanitize_style_valueIf this is intentional (defense-in-depth), consider adding a brief comment. If admins should have full control, move the capability check earlier:
public function sanitize_data_for_saving( $value ) { // Sanitize action value: ensure $value is an array and attribute/value are strings. if ( ! is_array( $value ) ) { return new WP_Error( 'invalid_structure', __( 'Value must be an array containing attribute and value keys.', 'interactions' ) ); } + // Admins with unfiltered_html bypass attribute/value sanitization but still get structure validation. + if ( current_user_can( 'unfiltered_html' ) ) { + // Still validate action field for consistency. + if ( isset( $value['action'] ) ) { + $allowed_actions = [ 'update', 'remove', 'toggle' ]; + if ( ! in_array( $value['action'], $allowed_actions, true ) ) { + $value['action'] = 'update'; + } + } + return $value; + } + // Sanitize attribute name if ( isset( $value['attribute'] ) && is_string( $value['attribute'] ) ) {
| // Rename interactions.php to plugin.php for premium builds only | ||
| if ( IS_PREMIUM_BUILD ) { | ||
| const oldPath = path.join( BUILD_DIR, 'interactions.php' ) | ||
| const newPath = path.join( BUILD_DIR, 'plugin.php' ) | ||
| if ( fs.existsSync( oldPath ) ) { | ||
| fs.renameSync( oldPath, newPath ) | ||
| console.log( '📝 Renamed interactions.php to plugin.php for premium build' ) | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for premium build references and related configuration
echo "=== Searching for premium build references ==="
rg -i "premium" scripts/package.js | head -20
echo ""
echo "=== Searching for Freemius or updater references ==="
rg -i "freemius|updater|basename" . | head -30
echo ""
echo "=== Searching for interactions.php references ==="
rg "interactions\.php" . --type js --type json --type php | head -30
echo ""
echo "=== Searching for plugin.php references ==="
rg "plugin\.php" . --type js --type json --type php | head -30
echo ""
echo "=== Checking for build output/distribution config ==="
rg -i "build|dist|package" . --type json -l | head -10Repository: gambitph/Interactions
Length of output: 5119
🏁 Script executed:
#!/bin/bash
# Check package.json for build scripts and context
echo "=== package.json build scripts ==="
cat package.json | head -50
echo ""
echo "=== Looking for migration or update handling ==="
rg -i "migrate|upgrade|basename.*change|plugin.*main.*file" . --type php --type js | head -20
echo ""
echo "=== Check the full context around the rename in package.js ==="
sed -n '300,330p' scripts/package.jsRepository: gambitph/Interactions
Length of output: 5171
Rename from interactions.php to plugin.php breaks existing premium upgrade path and Freemius tracking
The rename creates a breaking change for existing premium installations. Previous premium releases have the plugin activated as interactions/interactions.php, but after this change, the main file becomes interactions/plugin.php.
Freemius tracks the plugin by plugin_basename($this->plugin_main_file), which will now return interactions/plugin.php instead of interactions/interactions.php. On upgrade, WordPress will have both files present—the old active file and the new main file—but Freemius and the plugin action links hook will reference the new basename, causing:
- License verification to fail or become confused
- Plugin action links not to appear correctly
- Update checks to not recognize the existing installation
Either add a migration strategy (deactivate and reactivate the plugin during upgrade), document this as a breaking change requiring manual re-activation, or reconsider if renaming to plugin.php is necessary for the premium build.
🤖 Prompt for AI Agents
In scripts/package.js around lines 315-323, renaming interactions.php to
plugin.php breaks existing premium upgrade paths and Freemius tracking; instead
of renaming, change the build step to preserve interactions.php for backward
compatibility and also add the new plugin.php entrypoint: copy interactions.php
to plugin.php (do not delete the original) or create a small wrapper
interactions.php that loads/require_once plugin.php so
plugin_basename($this->plugin_main_file) still returns
interactions/interactions.php and existing installs continue to be recognized;
ensure both files are included in the BUILD_DIR and update the log message
accordingly.
| public function sanitize_data_for_saving( $value ) { | ||
| $value['color'] = $this->sanitize_style_value( $value['color'] ); | ||
| return $value; | ||
| } |
There was a problem hiding this comment.
Guard $value['color'] access to avoid notices on malformed data
As with the CSS rule action, the use of sanitize_style_value is appropriate, but directly indexing $value['color'] assumes structure that might not always hold (filters, bad data, etc.), leading to PHP warnings.
A small defensive tweak keeps behavior while hardening against bad input:
- public function sanitize_data_for_saving( $value ) {
- $value['color'] = $this->sanitize_style_value( $value['color'] );
- return $value;
- }
+ public function sanitize_data_for_saving( $value ) {
+ if ( is_array( $value ) && isset( $value['color'] ) ) {
+ $value['color'] = $this->sanitize_style_value( $value['color'] );
+ }
+
+ return $value;
+ }This matches the defensive style used in other action types while preserving the new sanitization.
🤖 Prompt for AI Agents
In src/action-types/class-action-type-background-color.php around lines 56 to
59, the code directly indexes $value['color'] which can trigger PHP notices if
$value is not an array or lacks the 'color' key; update the method to
defensively check that $value is an array and that 'color' exists (e.g. isset or
array_key_exists) before passing it to sanitize_style_value, and ensure the
function returns a sensible default (unchanged $value or null) when the key is
absent to preserve behavior without emitting warnings.
| public function sanitize_data_for_saving( $value ) { | ||
| $value['image'] = $this->sanitize_style_value( $value['image'] ); | ||
| return $value; | ||
| } |
There was a problem hiding this comment.
Add null checks before accessing array keys.
Lines 38 accesses $value['image'] without verifying that $value is an array or that the key exists. This will generate PHP notices if the key is not present.
Apply this diff for consistency with other sanitization methods:
public function sanitize_data_for_saving( $value ) {
- $value['image'] = $this->sanitize_style_value( $value['image'] );
+ if ( is_array( $value ) && isset( $value['image'] ) ) {
+ $value['image'] = $this->sanitize_style_value( $value['image'] );
+ }
return $value;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public function sanitize_data_for_saving( $value ) { | |
| $value['image'] = $this->sanitize_style_value( $value['image'] ); | |
| return $value; | |
| } | |
| public function sanitize_data_for_saving( $value ) { | |
| if ( is_array( $value ) && isset( $value['image'] ) ) { | |
| $value['image'] = $this->sanitize_style_value( $value['image'] ); | |
| } | |
| return $value; | |
| } |
🤖 Prompt for AI Agents
In src/action-types/class-action-type-background-image.php around lines 37 to
40, add null/structure checks before accessing $value['image']; currently the
code accesses the key directly which can raise notices if $value is not an array
or the key is missing. Update the method to first verify that $value is an array
(or cast to one) and that the 'image' key exists (or use isset) before calling
$this->sanitize_style_value, e.g. return early if $value is not an array or
lacks 'image', otherwise sanitize $value['image'] and return the array; follow
the same pattern used by other sanitize methods for consistency.
| public function sanitize_data_for_saving( $value ) { | ||
| $value['value'] = $this->sanitize_style_value( $value['value'] ); | ||
| return $value; | ||
| } |
There was a problem hiding this comment.
Add defensive checks before accessing $value['value']
The sanitization logic is good, but accessing $value['value'] without checking its shape can trigger PHP notices if $value isn’t an array or the key is missing (e.g., corrupted data, custom filters).
To match the more defensive pattern used in other action types (e.g., text color, redirect), you can guard the access:
- public function sanitize_data_for_saving( $value ) {
- $value['value'] = $this->sanitize_style_value( $value['value'] );
- return $value;
- }
+ public function sanitize_data_for_saving( $value ) {
+ if ( is_array( $value ) && isset( $value['value'] ) ) {
+ $value['value'] = $this->sanitize_style_value( $value['value'] );
+ }
+
+ return $value;
+ }This keeps the sanitization behavior while avoiding runtime warnings on unexpected input shapes.
🤖 Prompt for AI Agents
In src/action-types/class-action-type-css-rule.php around lines 44 to 47, the
code directly accesses $value['value'] which can trigger PHP notices if $value
is not an array or the key is missing; add defensive checks before accessing the
key (e.g., if not is_array($value) return $value as-is; if the 'value' key is
not set, either set it to a safe default or return $value unchanged), then only
call $this->sanitize_style_value($value['value']) when the key exists; finally
return the (possibly modified) $value.
| public function sanitize_data_for_saving( $value ) { | ||
| if ( is_array( $value ) && isset( $value['rotate'] ) ) { | ||
| if ( is_numeric( $value['rotate'] ) ) { | ||
| $value['rotate'] = $value['rotate'] + 0; | ||
| } else { | ||
| $value['rotate'] = null; | ||
| } | ||
| } | ||
|
|
||
| if ( is_array( $value ) && isset( $value['transformOrigin'] ) ) { | ||
| $allowed_transform_origins = [ | ||
| 'center', | ||
| 'top', | ||
| 'right', | ||
| 'bottom', | ||
| 'left', | ||
| 'top left', | ||
| 'top right', | ||
| 'bottom left', | ||
| 'bottom right', | ||
| 'custom', | ||
| ]; | ||
| if ( ! in_array( $value['transformOrigin'], $allowed_transform_origins, true ) ) { | ||
| $value['transformOrigin'] = 'center'; | ||
| } | ||
| } | ||
|
|
||
| $value['customTransformOrigin'] = $this->sanitize_style_value( $value['customTransformOrigin'] ); | ||
| return $value; | ||
| } |
There was a problem hiding this comment.
Add null check before accessing array key.
Line 94 accesses $value['customTransformOrigin'] without verifying the key exists. This will generate a PHP notice if the key is not present.
Apply this diff to add the check:
if ( is_array( $value ) && isset( $value['transformOrigin'] ) ) {
$allowed_transform_origins = [
'center',
'top',
'right',
'bottom',
'left',
'top left',
'top right',
'bottom left',
'bottom right',
'custom',
];
if ( ! in_array( $value['transformOrigin'], $allowed_transform_origins, true ) ) {
$value['transformOrigin'] = 'center';
}
}
- $value['customTransformOrigin'] = $this->sanitize_style_value( $value['customTransformOrigin'] );
+ if ( isset( $value['customTransformOrigin'] ) ) {
+ $value['customTransformOrigin'] = $this->sanitize_style_value( $value['customTransformOrigin'] );
+ }
return $value;🤖 Prompt for AI Agents
In src/action-types/class-action-type-rotate.php around lines 67 to 96, the code
accesses $value['customTransformOrigin'] on line 94 without verifying the key
exists; update the logic to check that $value is an array and that the
'customTransformOrigin' key exists (use isset or array_key_exists) before
calling $this->sanitize_style_value on it, and if the key is missing set
$value['customTransformOrigin'] to null (or a safe default) so no PHP notice is
raised.
| wp.ajax.post( 'interactfa_activate', { license_key: btoa( licenseKey ), nonce: "{$esc_attr( $nonce )}" } ).done( function( success ) { | ||
| result.classList.remove( 'err' ) | ||
| result.classList.add( 'success' ) | ||
| result.textContent = success; | ||
| console.log('result', result); | ||
| dialog.classList.remove( 'busy' ) | ||
| setTimeout( function() { window.location.reload() }, 1000 ) | ||
| }).fail( function ( error ) { | ||
| result.classList.remove( 'success' ) | ||
| result.classList.add( 'err' ) | ||
| result.textContent = error; | ||
| dialog.classList.remove( 'busy' ) | ||
| dialog.querySelector( 'button.close' ).disabled = false | ||
| dialog.querySelector( 'button.activate' ).disabled = false | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Remove debug console.log statements.
Debug statements left in production code. These should be removed.
wp.ajax.post( 'interactfa_activate', { license_key: btoa( licenseKey ), nonce: "{$esc_attr( $nonce )}" } ).done( function( success ) {
result.classList.remove( 'err' )
result.classList.add( 'success' )
result.textContent = success;
- console.log('result', result);
dialog.classList.remove( 'busy' )
setTimeout( function() { window.location.reload() }, 1000 )📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| wp.ajax.post( 'interactfa_activate', { license_key: btoa( licenseKey ), nonce: "{$esc_attr( $nonce )}" } ).done( function( success ) { | |
| result.classList.remove( 'err' ) | |
| result.classList.add( 'success' ) | |
| result.textContent = success; | |
| console.log('result', result); | |
| dialog.classList.remove( 'busy' ) | |
| setTimeout( function() { window.location.reload() }, 1000 ) | |
| }).fail( function ( error ) { | |
| result.classList.remove( 'success' ) | |
| result.classList.add( 'err' ) | |
| result.textContent = error; | |
| dialog.classList.remove( 'busy' ) | |
| dialog.querySelector( 'button.close' ).disabled = false | |
| dialog.querySelector( 'button.activate' ).disabled = false | |
| }); | |
| wp.ajax.post( 'interactfa_activate', { license_key: btoa( licenseKey ), nonce: "{$esc_attr( $nonce )}" } ).done( function( success ) { | |
| result.classList.remove( 'err' ) | |
| result.classList.add( 'success' ) | |
| result.textContent = success; | |
| dialog.classList.remove( 'busy' ) | |
| setTimeout( function() { window.location.reload() }, 1000 ) | |
| }).fail( function ( error ) { | |
| result.classList.remove( 'success' ) | |
| result.classList.add( 'err' ) | |
| result.textContent = error; | |
| dialog.classList.remove( 'busy' ) | |
| dialog.querySelector( 'button.close' ).disabled = false | |
| dialog.querySelector( 'button.activate' ).disabled = false | |
| }); |
🤖 Prompt for AI Agents
In src/freemius.php around lines 637 to 651, there is a leftover debug
console.log('result', result); in the AJAX success handler; remove that
console.log statement (and any other stray debug console.log calls in this
block) so the production JS contains no debug logging, keeping the rest of the
success handler intact.
| wp.ajax.post( 'interactfa_deactivate', { nonce: "{$esc_attr( $nonce )}" } ).done( function( success ) { | ||
| result.classList.remove( 'err' ) | ||
| result.classList.add( 'success' ) | ||
| result.textContent = success; | ||
| console.log('result', result); | ||
| dialog.classList.remove( 'busy' ) | ||
| setTimeout( function() { window.location.reload() }, 1000 ) |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Remove debug console.log statement.
Same issue as in the activate handler - debug statement should be removed.
wp.ajax.post( 'interactfa_deactivate', { nonce: "{$esc_attr( $nonce )}" } ).done( function( success ) {
result.classList.remove( 'err' )
result.classList.add( 'success' )
result.textContent = success;
- console.log('result', result);
dialog.classList.remove( 'busy' )
setTimeout( function() { window.location.reload() }, 1000 )📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| wp.ajax.post( 'interactfa_deactivate', { nonce: "{$esc_attr( $nonce )}" } ).done( function( success ) { | |
| result.classList.remove( 'err' ) | |
| result.classList.add( 'success' ) | |
| result.textContent = success; | |
| console.log('result', result); | |
| dialog.classList.remove( 'busy' ) | |
| setTimeout( function() { window.location.reload() }, 1000 ) | |
| wp.ajax.post( 'interactfa_deactivate', { nonce: "{$esc_attr( $nonce )}" } ).done( function( success ) { | |
| result.classList.remove( 'err' ) | |
| result.classList.add( 'success' ) | |
| result.textContent = success; | |
| dialog.classList.remove( 'busy' ) | |
| setTimeout( function() { window.location.reload() }, 1000 ) |
🤖 Prompt for AI Agents
In src/freemius.php around lines 660 to 666, there's a leftover debug
console.log('result', result); in the deactivate AJAX done handler; remove that
console.log line so no debugging output is emitted to the browser console,
leaving the remaining DOM updates and reload logic intact.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.