🔒 Fix XSS in Diff Checker - #87
Conversation
The Diff Checker was vulnerable to XSS due to unsafe fallback escaping logic and the possibility of malicious payload injection. Rather than using an unreliable HTML tokenizer or DOMPurify (which conceptually breaks diff views by stripping `<script>` tags instead of escaping them), the worker has been refactored to fully highlight the original text payloads before performing the diff. Because `highlight.js` safely entity-escapes raw string content internally, diffing the pre-processed HTML strings naturally mitigates the XSS vector while perfectly preserving the syntax-highlighted classes and spans. Co-authored-by: immineal <[email protected]>
|
👋 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. |
|
@jules resolve the merge conflicts in this pull request |
This commit removes the unreliable custom tokenizer in favor of a robust architectural fix: 1. `worker.js` now pre-processes the entire input strings through `highlight.js` (which natively performs secure HTML entity escaping). 2. If `highlight.js` detection falls back to plaintext, a secure `processCode` function correctly escapes all entities including quotes. 3. The diffing algorithm then safely compares the resulting pre-escaped HTML strings, eliminating the XSS vector while inherently preserving the syntax highlighting tags injected by `hljs`. Co-authored-by: immineal <[email protected]>
1 participant
🎯 What: An XSS vulnerability in the Diff Checker was mitigated by safely entity-escaping all input text.
⚠️ Risk: The previous implementation could allow unescaped payload strings to be concatenated directly into HTML cells via the
row.left,row.right, androw.codeHtmlvariables whenhighlight.jsfallback logic was utilized, leading to potentially critical XSS exploitation.🛡️ Solution: The
worker.jsscript was refactored to implement a "highlight-then-diff" architecture. By syntax highlighting the full raw strings before passing them to the diff generator, all user inputs are safely entity-escaped byhighlight.js(and the improved fallback logic) upfront. The diff logic then safely processes the HTML strings. This successfully mitigates the XSS vector while ensuring that syntax highlighting spans remain unbroken, avoiding the pitfalls of custom regex HTML tokenization or lossy DOM sanitization.PR created automatically by Jules for task 2566920800465889156 started by @immineal