CRW-10794: Fix CVE-2026-41240 by updating DOMPurify to patched version #705
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. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR upgrades DOMPurify from 3.2.7 to 3.4.2 across extension dependency declarations, vendored cgmanifest metadata, type definitions (.d.ts), and the core DOMPurify implementation. The update widens Config.ADD_ATTR/ADD_TAGS to accept predicate functions, adds ADD_FORBID_CONTENTS, introduces safer internal helpers (stringifyValue, guarded cloning, isRegex), tightens _parseConfig and attribute/tag decision paths (EXTRA_ELEMENT_HANDLING, FORBID_ATTR precedence), adds recursive shadow-DOM sanitization, inserts a changelog entry, and updates rebase conflict handlers to preserve local DOMPurify files. 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)
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 |
|
@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. |
|
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 |
|
@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. |
|
Pull Request images published ✨ Editor amd64: quay.io/che-incubator-pull-requests/che-code:pr-705-amd64 |
3 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
Security / Stability
Documentation