-
Notifications
You must be signed in to change notification settings - Fork 83
Use safeHTML consistently #2653
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Apply safeHTML() from @wordpress/dom to all dangerouslySetInnerHTML usages to ensure frontend sanitization regardless of backend state.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR enhances security by consistently applying the safeHTML() sanitization function from @wordpress/dom to all HTML content rendered via dangerouslySetInnerHTML across the codebase. This ensures that user-generated or external HTML content is properly sanitized before being rendered in the browser, reducing the risk of XSS (Cross-Site Scripting) vulnerabilities.
Key Changes:
- Added
safeHTMLimport from@wordpress/domto three source files - Wrapped all
dangerouslySetInnerHTMLcontent withsafeHTML()function calls - Updated corresponding build artifacts to reflect the source changes
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
src/follow-me/edit.js |
Added safeHTML import and wrapped profile summary HTML content with sanitization |
src/extra-fields/edit.js |
Added safeHTML import and wrapped extra field values with sanitization |
src/app/routes/feed/inspector.tsx |
Added safeHTML import, updated RenderHTML helper to sanitize decoded HTML, and improved documentation |
build/follow-me/index.js |
Minified build artifact reflecting source changes |
build/follow-me/index.asset.php |
Updated build version hash |
build/extra-fields/index.js |
Minified build artifact with wp-dom dependency added |
build/extra-fields/index.asset.php |
Updated dependencies to include 'wp-dom' and new version hash |
build/app/index.js |
Minified build artifact reflecting source changes |
build/app/index.asset.php |
Updated build version hash |
build/app/feed-content.6f565d4e.js |
New build artifact with sanitization logic |
The changes are well-implemented and consistent. All uses of dangerouslySetInnerHTML in the modified files now properly sanitize their content through safeHTML(). The PR successfully improves the security posture of the application without introducing any issues.
Comments suppressed due to low confidence (2)
build/app/feed-content.6f565d4e.js:4
- Variable 't' is used before its declaration.
build/app/feed-content.6f565d4e.js:4 - This statement is unreachable.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
pfefferle
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Proposed changes:
safeHTML()consistently across alldangerouslySetInnerHTMLusagesOther information:
N/A
Testing instructions: