🛡️ Sentinel: Fix potential XSS in Health Check page#38
🛡️ Sentinel: Fix potential XSS in Health Check page#38AGI-Corporation wants to merge 2 commits intomainfrom
Conversation
Refactored the Health Check page to remove the use of `dangerouslySetInnerHTML`. System metadata for version and websocket checks is now rendered using safe React JSX fragments instead of raw HTML strings. Added rel="noreferrer" to external links for improved security. Severity: MEDIUM Vulnerability: Potential XSS via dangerouslySetInnerHTML. Impact: Possible script execution if metadata is compromised. Fix: Use React nodes for dynamic content rendering. Verification: ESLint and tsc passed; manual verification of the file content. Co-authored-by: AGI-Corporation <186229839+AGI-Corporation@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe changes implement XSS mitigation by replacing HTML string assembly and Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
There was a problem hiding this comment.
Pull request overview
This PR hardens the Platform → Infra → Health page against XSS by eliminating HTML-string rendering and switching to safe JSX/React node rendering for the “details” column.
Changes:
- Replaced
dangerouslySetInnerHTMLusage with JSX fragments/React nodes for version and WebSocket health details. - Added
rel="noreferrer"on external links opened in a new tab. - Added a security learning entry documenting the XSS mitigation.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/react-ui/src/app/routes/platform/infra/health/index.tsx | Renders health check “details” as React nodes instead of HTML strings to remove an XSS vector; updates external link attributes. |
| .jules/sentinel.md | Documents the XSS lesson/prevention guidance for future reference. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <a | ||
| className="font-medium text-blue-600 dark:text-blue-500 hover:underline" | ||
| href="https://github.com/activepieces/activepieces/releases" | ||
| target="_blank" | ||
| rel="noreferrer" | ||
| > |
There was a problem hiding this comment.
For external links opened with target="_blank", consider using rel="noopener noreferrer" (not just noreferrer) to prevent reverse-tabnabbing in older/edge browser implementations and to match the existing pattern used elsewhere in the UI (e.g. help-and-feedback.tsx).
| <a | ||
| className="font-medium text-blue-600 dark:text-blue-500 hover:underline" | ||
| href="https://www.activepieces.com/docs/install/configuration/troubleshooting" | ||
| target="_blank" | ||
| rel="noreferrer" | ||
| > |
There was a problem hiding this comment.
Same as above: when using target="_blank", it’s safer and more consistent to set rel="noopener noreferrer" rather than only noreferrer.
Refactored the Health Check page to remove the use of `dangerouslySetInnerHTML`. System metadata for version and websocket checks is now rendered using safe React JSX fragments instead of raw HTML strings. Added rel="noreferrer" to external links for improved security. Fixes #1 Severity: MEDIUM Vulnerability: Potential XSS via dangerouslySetInnerHTML. Impact: Possible script execution if metadata is compromised. Fix: Use React nodes for dynamic content rendering. Verification: ESLint and tsc passed; manual verification of the file content. Co-authored-by: AGI-Corporation <186229839+AGI-Corporation@users.noreply.github.com>
Refactored the Health Check page to remove the use of
dangerouslySetInnerHTML. System metadata for version and websocket checks is now rendered using safe React JSX fragments instead of raw HTML strings. Addedrel="noreferrer"to external links for improved security. Also added a security learning entry in.jules/sentinel.md.PR created automatically by Jules for task 4190290139442982079 started by @AGI-Corporation
Summary by CodeRabbit