fix(security): escape dashboard scan-summary HTML sinks - #1091
fix(security): escape dashboard scan-summary HTML sinks#1091seonghobae wants to merge 6 commits into
Conversation
- Sanitized dynamic properties `s.id`, `s.total`, and stats metrics using `esc()` before interpolating them into HTML strings for the `console.html` dashboard. - Prevents script execution or HTML breakout from malicious JSON payloads.
|
👋 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: Organization UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthrough대시보드의 동적 통계, 스캔 정보, 배지 값에 Changes대시보드 출력 보안
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to The dashboard now escapes untrusted scan-summary values before rendering, preventing injected markup and attribute breakout while preserving the existing display behavior. No concrete current-head merge risk remains. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
seonghobae
added
area: ui-ux
ChatGPT Codex Connector
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
- Sanitized dynamic properties `s.id`, `s.total`, and stats metrics using `esc()` before interpolating them into HTML strings for the `console.html` dashboard. - Prevents script execution or HTML breakout from malicious JSON payloads.
- Sanitized dynamic properties `s.id`, `s.total`, and stats metrics using `esc()` before interpolating them into HTML strings for the `console.html` dashboard. - Prevents script execution or HTML breakout from malicious JSON payloads. - Added `tests/test_console_dashboard_xss_behavior.py` to test the script mitigation.
|
Readiness correction after exact diff review: this PR has valid hostile summary/text-field XSS deltas, so it is not being closed. Canonical #1117 now carries the production count-boundary repair and stronger real-Chromium hostile id/count/text oracle at |
Understood. Acknowledging that this work is now obsolete as the canonical #1117 carries the production repair, and stopping work on this task. |
- Updated `tests/test_console_dashboard_xss_behavior.py` to use `re.IGNORECASE` when matching `<script>` tags, fixing a CodeQL alert.
seonghobae
removed
the
status: needs-review
seonghobae
added
the
status: draft
ChatGPT Codex Connector
seonghobae
changed the title
|
Re-fetched concurrent exact head |
Understood. The changes only affect |
2 participants
Security defect
The standalone control-plane console inserts scan summary fields into
innerHTML. Protecteddevelop@e71d37e7c58118e6764c96ab7c4492fe33eed6f8rendereds.id,s.total, deploy/new-blocking counts and critical summary counts without consistently enforcing an HTML/attribute-context boundary. Hostile producer data could therefore break out of the intended markup context.Repair retained on this lineage
The production repair applies the existing
esc()encoder to the affected text/attribute interpolations and preserves detail routing.tests/test_console_dashboard_xss_behavior.pyexecutes the shippedload()renderer under Node with hostile<img onerror>,<svg onload>and quote-breakingdata-idvalues, requiring raw values not to reach summary/trend/history HTML and requiring the encoded attribute boundary to remain intact.Intervening commits that fixed the script-extraction regression were preserved. The unrelated feature-branch addition to
.jules/sentinel.mdhas been restored byte-for-byte to protecteddevelopauthority rather than turning one PR finding into repository-wide doctrine.Current authority
develop@e71d37e7c58118e6764c96ab7c4492fe33eed6f8ec9dcfb7ec6a93d5acbb093a8caa3c95b7be2b21scanner/dashboard/console.html,tests/test_console_dashboard_xss_behavior.py34068940005, Security Process34068939947, Pinned HTTPS Coverage34068939874, Retention Audit Coverage34068939886, Scan path context coverage34068939946, OpenSSF Evidence Coverage3406893991734068939926, SAST34068939887, CodeQL PR34068939933No predecessor result is counted as current GREEN and the remaining security/code-scanning lanes are not bypassable wait states.
Canonical successor relationship
#1117 is the broader candidate: it retains scan-id escaping, adds numeric coercion for summary/history counts, and uses a real Chromium regression with DOM-element and dialog oracles. #1091 remains open because complete succession is not established until one unchanged #1117 exact head proves every #1091 hostile text/count/id contract plus its own additional contracts with terminal hosted/browser evidence and qualifying current-head review.
If #1117 reaches that state, #1091 may be closed only as verified complete succession. Until then this PR remains a preserved predecessor, not a competing merge candidate.
Delivery gate
The causal encoding fix and executable regression are present. Exact-head Tests and several evidence lanes are GREEN, but Security Scan/SAST/CodeQL and qualifying current-head review are still outstanding. Do not merge, close as duplicate, suppress scanners, self-approve, force-push, destructively rebase, or use dummy commits to retrigger review/checks.