Add Content Security Policy compatibility (#14) #29
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Moves inline styles to external CSS file - Replaces onclick handler with addEventListener in external JS - Uses data-chat-url attribute instead of inline JS - Adds wp_enqueue_scripts hook for proper asset loading Sites with strict CSP headers will no longer block the widget. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
celticr
commented
Jan 12, 2026
Contributor
Author
celticr
left a comment
celticr
left a comment
There was a problem hiding this comment.
PR #29: Add Content Security Policy Compatibility - Review
Summary
Solid security improvement that moves inline styles and onclick handlers to external files for CSP compatibility. Well-structured approach with proper WordPress asset enqueueing.
Review Findings
Strengths:
- Properly uses
wp_enqueue_styleandwp_enqueue_scriptwith version cache-busting - Correctly uses
esc_url()for all URL output - Smart use of
data-chat-urlattribute for CSP-safe URL passing - Good popup fallback pattern if popup is blocked
- Focus styles added for accessibility (
:focusoutline)
Minor Issues (Low severity):
- Audit logging bundled with CSP changes - The PR includes audit logging functions (
sophia_chat_log_change, etc.) which is outside the CSP scope. Consider splitting to a separate PR for cleaner history, though not blocking.
Tests
- All 25 unit tests pass
- PHP syntax validates cleanly
Recommendation: Ready to merge
This was referenced Jan 12, 2026
Contributor
Author
UpdateThe audit logging is now in main (merged via PR #26's branch history). This PR now contains only CSP-specific changes:
The split recommended in review has been achieved through the merge process. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
<style>block to externalassets/css/sophia-chat.cssonclickhandler withaddEventListenerinassets/js/sophia-chat.jsdata-chat-urlattribute for CSP-safe URL passingwp_enqueue_scriptshookTest plan
How Verified
Manual testing: widget appears and functions correctly.
Reviewer Focus