CRW-10794: Fix CVE-2026-41240 by updating DOMPurify to patched version #705
CRW-10794: Fix CVE-2026-41240 by updating DOMPurify to patched version #705sbouchet wants to merge 4 commits into
Conversation
|
@sbouchet: This pull request references CRW-10794 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the vulnerability to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
Click here to review and test in web IDE: |
&message=Dev%20cluster%20(for%20maintainers)&logo=eclipseche&color=525C86&labelColor=FDB940)
sbouchet
requested review from
RomanNikitenko,
azatsarynnyy,
rgrunber and
vitaliy-guliy
as code owners
|
@sbouchet: This pull request references CRW-10794 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the vulnerability to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
📝 WalkthroughWalkthroughThis PR upgrades DOMPurify from version 3.2.7 to 3.4.2 across all Che Code extension dependencies, type definitions, and vendored implementation. The upgrade introduces expanded configuration options allowing predicate functions for dynamic tag/attribute validation (ADD_TAGS, ADD_ATTR), a new ADD_FORBID_CONTENTS option, and substantial internal security enhancements including recursive shadow DOM sanitization, stricter attribute validation with clobbering protection, reserved custom element names, and improved input type handling. Build metadata and rebase conflict handlers are updated to align with the new version. Sequence Diagram(s)sequenceDiagram
participant Caller
participant stringifyValue
participant _parseConfig
participant sanitizePipeline
Caller->>stringifyValue: provide dirty input
stringifyValue-->>_parseConfig: normalized string
_parseConfig->>sanitizePipeline: normalized config + EXTRA_ELEMENT_HANDLING
sanitizePipeline->>sanitizePipeline: element checks, attribute checks, shadow DOM recursion
sanitizePipeline-->>Caller: sanitized output or thrown error
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@rebase.sh`:
- Around line 375-395: The rebase helper currently auto-resolves DomPurify by
unconditionally running git checkout --ours and git add in
apply_code_vs_base_browser_dompurify_changes() (and the similar block for
dompurify.d.ts), which can silently drop upstream security fixes; replace the
unconditional checkout/add with logic that detects a merge conflict on
code/src/vs/base/browser/dompurify/dompurify.js and dompurify.d.ts and aborts
the script with a non-zero exit and a clear message instructing a manual review
(or run a version/sha compare against the upstream file and abort if upstream is
newer) instead of staging changes automatically so maintainers must inspect and
resolve DomPurify merges manually.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 09ee6db4-3491-4ba7-a3bb-4db80f3bb403
⛔ Files ignored due to path filters (2)
code/extensions/markdown-language-features/package-lock.jsonis excluded by!**/package-lock.jsoncode/extensions/mermaid-chat-features/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (10)
.rebase/CHANGELOG.md.rebase/override/code/extensions/markdown-language-features/package.json.rebase/override/code/extensions/mermaid-chat-features/package.json.rebase/replace/code/src/vs/base/browser/dompurify/cgmanifest.json.jsoncode/extensions/markdown-language-features/package.jsoncode/extensions/mermaid-chat-features/package.jsoncode/src/vs/base/browser/dompurify/cgmanifest.jsoncode/src/vs/base/browser/dompurify/dompurify.d.tscode/src/vs/base/browser/dompurify/dompurify.jsrebase.sh
| git checkout --ours code/src/vs/base/browser/dompurify/dompurify.d.ts > /dev/null 2>&1 | ||
|
|
||
| # don't apply changes, keep ours version totally | ||
|
|
||
| # resolve the change | ||
| git add code/src/vs/base/browser/dompurify/dompurify.d.ts > /dev/null 2>&1 | ||
| } | ||
|
|
||
| # Apply changes on code/src/vs/base/browser/dompurify/dompurify.js file | ||
| apply_code_vs_base_browser_dompurify_changes() { | ||
|
|
||
| echo " ⚙️ reworking code/src/vs/base/browser/dompurify/dompurify.js..." | ||
|
|
||
| # reset the file from what is upstream | ||
| git checkout --ours code/src/vs/base/browser/dompurify/dompurify.js > /dev/null 2>&1 | ||
|
|
||
| # don't apply changes, keep ours version totally | ||
|
|
||
| # resolve the change | ||
| git add code/src/vs/base/browser/dompurify/dompurify.js > /dev/null 2>&1 | ||
| } |
There was a problem hiding this comment.
Avoid auto-resolving DomPurify conflicts with unconditional --ours.
This path can silently suppress upstream DomPurify security fixes on future rebases. For security-sensitive vendored files, fail fast and require manual review (or compare versions and abort when upstream is newer) instead of auto-staging --ours.
Suggested safer approach
- elif [[ "$conflictingFile" == "code/src/vs/base/browser/dompurify/dompurify.d.ts" ]]; then
- apply_code_vs_base_browser_dompurify_d_changes
- elif [[ "$conflictingFile" == "code/src/vs/base/browser/dompurify/dompurify.js" ]]; then
- apply_code_vs_base_browser_dompurify_changes
+ elif [[ "$conflictingFile" == "code/src/vs/base/browser/dompurify/dompurify.d.ts" ]] || \
+ [[ "$conflictingFile" == "code/src/vs/base/browser/dompurify/dompurify.js" ]]; then
+ echo "DomPurify conflict detected in $conflictingFile. Manual security review required."
+ exit 1Also applies to: 560-563
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@rebase.sh` around lines 375 - 395, The rebase helper currently auto-resolves
DomPurify by unconditionally running git checkout --ours and git add in
apply_code_vs_base_browser_dompurify_changes() (and the similar block for
dompurify.d.ts), which can silently drop upstream security fixes; replace the
unconditional checkout/add with logic that detects a merge conflict on
code/src/vs/base/browser/dompurify/dompurify.js and dompurify.d.ts and aborts
the script with a non-zero exit and a clear message instructing a manual review
(or run a version/sha compare against the upstream file and abort if upstream is
newer) instead of staging changes automatically so maintainers must inspect and
resolve DomPurify merges manually.
|
Pull Request images published ✨ Editor amd64: quay.io/che-incubator-pull-requests/che-code:pr-705-amd64 |
1 similar comment
|
Pull Request images published ✨ Editor amd64: quay.io/che-incubator-pull-requests/che-code:pr-705-amd64 |
Signed-off-by: Stephane Bouchet <sbouchet@redhat.com>
Protect the DOMPurify ^3.2.7 → ^3.4.2 bump across upstream rebases: - Override rules for markdown-language-features and mermaid-chat-features package.json to set dompurify dependency to ^3.4.2 - Replace rules for dompurify/cgmanifest.json to update commitHash, tag, and version fields - Routing in rebase.sh for dompurify.d.ts and dompurify.js using git checkout --ours (vendored library full version bump) - New routing for mermaid-chat-features/package.json Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Stephane Bouchet <sbouchet@redhat.com>
Signed-off-by: Stephane Bouchet <sbouchet@redhat.com>
|
@sbouchet: This pull request references CRW-10794 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the vulnerability to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@code/src/vs/base/browser/dompurify/dompurify.js`:
- Line 1: The vendored DOMPurify file was updated to 3.4.2 but the root
package-lock.json still pins DOMPurify at "3.2.7"; update the lockfile so the
two occurrences of version "3.2.7" become "3.4.2" (or simply regenerate the root
lockfile) to match the bundled build — e.g., run a fresh install (npm install /
npm ci) or regenerate the lockfile and commit the updated package-lock.json so
the root lockfile and the vendored DOMPurify (3.4.2) are consistent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 8e2a072d-3d39-4e5c-84ce-5c4c731241ec
⛔ Files ignored due to path filters (2)
code/extensions/markdown-language-features/package-lock.jsonis excluded by!**/package-lock.jsoncode/extensions/mermaid-chat-features/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (10)
.rebase/CHANGELOG.md.rebase/override/code/extensions/markdown-language-features/package.json.rebase/override/code/extensions/mermaid-chat-features/package.json.rebase/replace/code/src/vs/base/browser/dompurify/cgmanifest.json.jsoncode/extensions/markdown-language-features/package.jsoncode/extensions/mermaid-chat-features/package.jsoncode/src/vs/base/browser/dompurify/cgmanifest.jsoncode/src/vs/base/browser/dompurify/dompurify.d.tscode/src/vs/base/browser/dompurify/dompurify.jsrebase.sh
✅ Files skipped from review due to trivial changes (3)
- code/extensions/mermaid-chat-features/package.json
- .rebase/replace/code/src/vs/base/browser/dompurify/cgmanifest.json.json
- code/src/vs/base/browser/dompurify/cgmanifest.json
🚧 Files skipped from review as they are similar to previous changes (5)
- code/extensions/markdown-language-features/package.json
- .rebase/override/code/extensions/mermaid-chat-features/package.json
- .rebase/override/code/extensions/markdown-language-features/package.json
- rebase.sh
- code/src/vs/base/browser/dompurify/dompurify.d.ts
|
Pull Request images published ✨ Editor amd64: quay.io/che-incubator-pull-requests/che-code:pr-705-amd64 |
|
Pull Request images published ✨ Editor amd64: quay.io/che-incubator-pull-requests/che-code:pr-705-amd64 |
2 participants
What does this PR do?
This PR fixes CVE-2026-41240
dompurifyversion is updated to3.4.2dompurify sources files are also updated to use the upstream ones from that version.
What issues does this PR fix?
https://redhat.atlassian.net/browse/CRW-10794
How to test this PR?
Does this PR contain changes that override default upstream Code-OSS behavior?
git rebasewere added to the .rebase folderSummary by CodeRabbit
New Features
Chores
Documentation