fix: prevent XSS vulnerability in context menu labels#8887
Merged
Conversation
Replace innerHTML with textContent when setting context menu item labels to prevent XSS attacks via malicious filenames. This fixes a security vulnerability where filenames like "<img src=x onerror=alert()>" could execute arbitrary JavaScript when displayed in dropdowns. https://claude.ai/code/session_01LALt1HEgGvpWD7hhqcp2Gu
dosubot
bot
added
the
size:XS
Contributor
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughAdds a DOM-based HTML sanitization pipeline (DOMPurify hook, allowed tags/styles) and safer rendering in the context menu: uses Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
🎨 Storybook Build Status✅ Build completed successfully! ⏰ Completed at: 02/16/2026, 08:29:09 PM UTC 🔗 Links🎉 Your Storybook is ready for review! |
|
Playwright: ✅ 518 passed, 0 failed · 4 flaky 📊 Browser Reports
|
DrJKL
added
needs-backport
Bundle Size ReportSummary
Category Glance Per-category breakdownApp Entry Points — 21.7 kB (baseline 21.7 kB) • 🔴 +12 BMain entry bundles and manifests
Status: 1 added / 1 removed Graph Workspace — 886 kB (baseline 886 kB) • ⚪ 0 BGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed Views & Navigation — 69 kB (baseline 68.9 kB) • 🔴 +120 BTop-level views, pages, and routed surfaces
Status: 11 added / 11 removed Panels & Settings — 427 kB (baseline 427 kB) • 🔴 +80 BConfiguration panels, inspectors, and settings screens
Status: 10 added / 10 removed User & Accounts — 16.1 kB (baseline 16.1 kB) • 🔴 +40 BAuthentication, profile, and account management bundles
Status: 6 added / 6 removed Editors & Dialogs — 785 B (baseline 785 B) • ⚪ 0 BModals, dialogs, drawers, and in-app editors
Status: 1 added / 1 removed UI Components — 36.6 kB (baseline 36.6 kB) • ⚪ 0 BReusable component library chunks
Status: 9 added / 9 removed Data & Services — 2.16 MB (baseline 2.16 MB) • 🔴 +846 BStores, services, APIs, and repositories
Status: 14 added / 14 removed Utilities & Hooks — 237 kB (baseline 237 kB) • ⚪ 0 BHelpers, composables, and utility bundles
Status: 19 added / 19 removed Vendor & Third-Party — 8.69 MB (baseline 8.69 MB) • ⚪ 0 BExternal libraries and shared vendor chunks
Status: 6 added / 6 removed Other — 7.34 MB (baseline 7.34 MB) • ⚪ 0 BBundles that do not match a named category
Status: 76 added / 76 removed |
AustinMroz
previously approved these changes
Feb 15, 2026
christian-byrne
previously approved these changes
Feb 15, 2026
snomiao
previously approved these changes
Feb 15, 2026
Member
|
While this fix solves bug but not support html input anymore, Maybe a markdown render is better? As they sanitize all the input by default, and support safe htmls.
|
DrJKL
added
the
New Browser Test Expectations
|
Updating Playwright Expectations |
comfy-pr-bot
dismissed stale reviews from snomiao, christian-byrne, and AustinMroz
via
b7b3138
github-actions
bot
removed
the
New Browser Test Expectations
The previous XSS fix replaced innerHTML with textContent, which broke the color picker submenu that uses HTML spans for color swatches. Use a sanitizer that allows only safe tags and style properties. https://claude.ai/code/session_01LALt1HEgGvpWD7hhqcp2Gu
dosubot
bot
removed
the
size:XS
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/lib/litegraph/src/ContextMenu.ts (1)
8-62: Use DOMPurify for sanitizing context menu HTML.Custom HTML sanitizers are error-prone and vulnerable to bypass attacks (mutation XSS, namespace attacks, DOM clobbering). DOMPurify is available in the project (
"dompurify": "^3.2.5") and already used throughout the codebase. Per coding guidelines: "Sanitize HTML with DOMPurify".Replace the custom
sanitizeMenuHTML,sanitizeNode, andsanitizeStylefunctions with DOMPurify configuration:♻️ Proposed refactor using DOMPurify
+import DOMPurify from 'dompurify' + import type { ContextMenuDivElement, IContextMenuOptions, IContextMenuValue } from './interfaces' import { LiteGraph } from './litegraph' -const ALLOWED_TAGS = new Set(['span', 'b', 'i', 'em', 'strong']) -const ALLOWED_STYLE_PROPS = new Set([ - 'display', - 'color', - 'background-color', - 'padding-left', - 'border-left' -]) - -function sanitizeMenuHTML(html: string): string { - const doc = new DOMParser().parseFromString(html, 'text/html') - sanitizeNode(doc.body) - return doc.body.innerHTML -} - -function sanitizeNode(node: Node): void { - const toRemove: Node[] = [] - for (const child of node.childNodes) { - if (child.nodeType === Node.ELEMENT_NODE) { - const el = child as Element - if (!ALLOWED_TAGS.has(el.tagName.toLowerCase())) { - toRemove.push(child) - continue - } - // Strip all attributes except safe style properties - const attrs = [...el.attributes] - for (const attr of attrs) { - if (attr.name === 'style') { - el.setAttribute('style', sanitizeStyle(attr.value)) - } else { - el.removeAttribute(attr.name) - } - } - sanitizeNode(child) - } else if (child.nodeType === Node.TEXT_NODE) { - // Text nodes are always safe - } else { - toRemove.push(child) - } - } - for (const child of toRemove) node.removeChild(child) -} - -function sanitizeStyle(style: string): string { - return style - .split(';') - .map((s) => s.trim()) - .filter((s) => { - const colonIdx = s.indexOf(':') - if (colonIdx === -1) return false - const prop = s.slice(0, colonIdx).trim().toLowerCase() - return ALLOWED_STYLE_PROPS.has(prop) - }) - .join('; ') -} +function sanitizeMenuHTML(html: string): string { + return DOMPurify.sanitize(html, { + ALLOWED_TAGS: ['span', 'b', 'i', 'em', 'strong'], + ALLOWED_ATTR: ['style'] + }) +}
The previous fix checked if content !== text, but since the content is passed as the name parameter, they're always equal. Use a regex to detect HTML tags instead. https://claude.ai/code/session_01LALt1HEgGvpWD7hhqcp2Gu
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/lib/litegraph/src/ContextMenu.ts`:
- Around line 8-62: Replace the custom sanitizer (sanitizeMenuHTML,
sanitizeNode, sanitizeStyle and the ALLOWED_TAGS / ALLOWED_STYLE_PROPS logic)
with DOMPurify: import DOMPurify, call DOMPurify.sanitize(html, {ALLOWED_TAGS:
[...], ALLOWED_ATTR: ['style'], FORBID_TAGS: [], WHOLE_DOCUMENT: false,
ALLOWED_URI_REGEXP: /./}) and configure its hooks/options to only allow the same
tag list and the allowed style properties (map ALLOWED_STYLE_PROPS into a style
sanitizer hook or use DOMPurify's hooks to filter style declarations), then
update usages to call the new sanitize function; remove the old functions and
sets so the module relies solely on DOMPurify for HTML sanitization.
Replace custom sanitizer with DOMPurify library for more robust and well-tested HTML sanitization. Uses DOMPurify's hooks to filter style properties to the allowed set. https://claude.ai/code/session_01LALt1HEgGvpWD7hhqcp2Gu
DrJKL
added
the
New Browser Test Expectations
github-actions
bot
removed
the
New Browser Test Expectations
github-actions bot
pushed a commit
that referenced
this pull request
Feb 16, 2026
Replace innerHTML with textContent when setting context menu item labels to prevent XSS attacks via malicious filenames. This fixes a security vulnerability where filenames like "<img src=x onerror=alert()>" could execute arbitrary JavaScript when displayed in dropdowns. https://claude.ai/code/session_01LALt1HEgGvpWD7hhqcp2Gu ## Summary <!-- One sentence describing what changed and why. --> ## Changes - **What**: <!-- Core functionality added/modified --> - **Breaking**: <!-- Any breaking changes (if none, remove this line) --> - **Dependencies**: <!-- New dependencies (if none, remove this line) --> ## Review Focus <!-- Critical design decisions or edge cases that need attention --> <!-- If this PR fixes an issue, uncomment and update the line below --> <!-- Fixes #ISSUE_NUMBER --> ## Screenshots (if applicable) <!-- Add screenshots or video recording to help explain your changes --> ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8887-fix-prevent-XSS-vulnerability-in-context-menu-labels-3086d73d365081ccbe3cdb35cd7e5cb1) by [Unito](https://www.unito.io) --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: github-actions <github-actions@github.com>
github-actions bot
pushed a commit
that referenced
this pull request
Feb 16, 2026
Replace innerHTML with textContent when setting context menu item labels to prevent XSS attacks via malicious filenames. This fixes a security vulnerability where filenames like "<img src=x onerror=alert()>" could execute arbitrary JavaScript when displayed in dropdowns. https://claude.ai/code/session_01LALt1HEgGvpWD7hhqcp2Gu ## Summary <!-- One sentence describing what changed and why. --> ## Changes - **What**: <!-- Core functionality added/modified --> - **Breaking**: <!-- Any breaking changes (if none, remove this line) --> - **Dependencies**: <!-- New dependencies (if none, remove this line) --> ## Review Focus <!-- Critical design decisions or edge cases that need attention --> <!-- If this PR fixes an issue, uncomment and update the line below --> <!-- Fixes #ISSUE_NUMBER --> ## Screenshots (if applicable) <!-- Add screenshots or video recording to help explain your changes --> ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8887-fix-prevent-XSS-vulnerability-in-context-menu-labels-3086d73d365081ccbe3cdb35cd7e5cb1) by [Unito](https://www.unito.io) --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: github-actions <github-actions@github.com>
github-actions bot
pushed a commit
that referenced
this pull request
Feb 16, 2026
Replace innerHTML with textContent when setting context menu item labels to prevent XSS attacks via malicious filenames. This fixes a security vulnerability where filenames like "<img src=x onerror=alert()>" could execute arbitrary JavaScript when displayed in dropdowns. https://claude.ai/code/session_01LALt1HEgGvpWD7hhqcp2Gu ## Summary <!-- One sentence describing what changed and why. --> ## Changes - **What**: <!-- Core functionality added/modified --> - **Breaking**: <!-- Any breaking changes (if none, remove this line) --> - **Dependencies**: <!-- New dependencies (if none, remove this line) --> ## Review Focus <!-- Critical design decisions or edge cases that need attention --> <!-- If this PR fixes an issue, uncomment and update the line below --> <!-- Fixes #ISSUE_NUMBER --> ## Screenshots (if applicable) <!-- Add screenshots or video recording to help explain your changes --> ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8887-fix-prevent-XSS-vulnerability-in-context-menu-labels-3086d73d365081ccbe3cdb35cd7e5cb1) by [Unito](https://www.unito.io) --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: github-actions <github-actions@github.com>
github-actions bot
pushed a commit
that referenced
this pull request
Feb 16, 2026
Replace innerHTML with textContent when setting context menu item labels to prevent XSS attacks via malicious filenames. This fixes a security vulnerability where filenames like "<img src=x onerror=alert()>" could execute arbitrary JavaScript when displayed in dropdowns. https://claude.ai/code/session_01LALt1HEgGvpWD7hhqcp2Gu ## Summary <!-- One sentence describing what changed and why. --> ## Changes - **What**: <!-- Core functionality added/modified --> - **Breaking**: <!-- Any breaking changes (if none, remove this line) --> - **Dependencies**: <!-- New dependencies (if none, remove this line) --> ## Review Focus <!-- Critical design decisions or edge cases that need attention --> <!-- If this PR fixes an issue, uncomment and update the line below --> <!-- Fixes #ISSUE_NUMBER --> ## Screenshots (if applicable) <!-- Add screenshots or video recording to help explain your changes --> ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8887-fix-prevent-XSS-vulnerability-in-context-menu-labels-3086d73d365081ccbe3cdb35cd7e5cb1) by [Unito](https://www.unito.io) --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: github-actions <github-actions@github.com>
Member
This was referenced Feb 16, 2026
Member
Member
Member
github-actions
bot
removed
the
needs-backport
pythongosssss
pushed a commit
that referenced
this pull request
Feb 17, 2026
…labels (#8925) Backport of #8887 to `cloud/1.39` Automatically created by backport workflow. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8925-backport-cloud-1-39-fix-prevent-XSS-vulnerability-in-context-menu-labels-3096d73d3650813bae82c37f3349dfbd) by [Unito](https://www.unito.io) Co-authored-by: Alexander Brown <drjkl@comfy.org> Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: github-actions <github-actions@github.com>
pythongosssss
pushed a commit
that referenced
this pull request
Feb 17, 2026
…abels (#8924) Backport of #8887 to `core/1.39` Automatically created by backport workflow. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8924-backport-core-1-39-fix-prevent-XSS-vulnerability-in-context-menu-labels-3096d73d365081a59774ca63128b6b20) by [Unito](https://www.unito.io) Co-authored-by: Alexander Brown <drjkl@comfy.org> Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: github-actions <github-actions@github.com>
pythongosssss
pushed a commit
that referenced
this pull request
Feb 17, 2026
…abels (#8922) Backport of #8887 to `core/1.38` Automatically created by backport workflow. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8922-backport-core-1-38-fix-prevent-XSS-vulnerability-in-context-menu-labels-3096d73d3650811a9448fc9b2344a88b) by [Unito](https://www.unito.io) Co-authored-by: Alexander Brown <drjkl@comfy.org> Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: github-actions <github-actions@github.com>
pythongosssss
pushed a commit
that referenced
this pull request
Feb 17, 2026
…labels (#8923) Backport of #8887 to `cloud/1.38` Automatically created by backport workflow. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8923-backport-cloud-1-38-fix-prevent-XSS-vulnerability-in-context-menu-labels-3096d73d365081baa2dce3b6c8027736) by [Unito](https://www.unito.io) Co-authored-by: Alexander Brown <drjkl@comfy.org> Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: github-actions <github-actions@github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Replace innerHTML with textContent when setting context menu item labels to prevent XSS attacks via malicious filenames. This fixes a security vulnerability where filenames like "
" could execute arbitrary JavaScript when displayed in dropdowns.
https://claude.ai/code/session_01LALt1HEgGvpWD7hhqcp2Gu
Summary
Changes
Review Focus
Screenshots (if applicable)
┆Issue is synchronized with this Notion page by Unito