[PR #3296] [MERGED] v3: Prevent HTML and URL injection in the Manager UI #3172

New issue

Closed

opened 2026-10-28 21:15:49 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-28 21:15:49 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/Comfy-Org/ComfyUI-Manager/pull/3296
Author: @ltdrdata
Created: 9/18/2026
Status: ✅ Merged
Merged: 9/18/2026
Merged by: @ltdrdata

Base: main ← Head: fix/v3-ui-escaping

📝 Commits (1)

2f7d16f Escape untrusted Manager UI content

📊 Changes

30 files changed (+3922 additions, -115 deletions)

View changed files

📝 .gitignore (+3 -1)
📝 glob/manager_server.py (+31 -24)
📝 glob/manager_util.py (+80 -0)
📝 js/comfyui-manager.js (+7 -7)
📝 js/comfyui-share-common.js (+2 -2)
📝 js/comfyui-share-copus.js (+2 -2)
📝 js/comfyui-share-openart.js (+2 -2)
📝 js/comfyui-share-youml.js (+2 -2)
📝 js/common.js (+42 -6)
📝 js/custom-nodes-manager.js (+42 -28)
➕ js/manager-grid.js (+67 -0)
📝 js/model-manager.js (+36 -23)
📝 js/node-usage-analyzer.js (+17 -9)
📝 js/snapshot.js (+3 -3)
📝 prestartup_script.py (+21 -5)
📝 pyproject.toml (+1 -1)
📝 requirements.txt (+2 -0)
➕ tests/cases/url_sanitize_cases.json (+153 -0)
➕ tests/js_lift.py (+159 -0)
➕ tests/manager_test_utils.py (+32 -0)

...and 10 more files

📄 Description

Purpose and scope

Node/model metadata, workflow names and error messages can become executable HTML in Manager dialogs. Render untrusted values safely while preserving readable text, formatted descriptions, valid links and search behavior.

Covers Custom Nodes, Model Manager, Node Usage Analyzer, snapshots, sharing links, update-result dialogs, Markdown descriptions and notices. Configuration-persistence fixes are already present in main through #3157.

Key changes

Escape text and attributes at rendering boundaries and validate links through shared utilities. Update All success/failure dialogs preserve titles, URLs and labels for entries without URLs.
Keep Markdown URLs separate from formatting passes to preserve valid links. Sanitize remote notice HTML with nh3.
Make ManagerGrid search match displayed text for both server-escaped fields and raw metadata, including literal entity text. Highlighting stays inert; unknown pack states retain readable labels without invalid actions.
Declare nh3>=0.3.7 and packaging in both manifests. Startup reads the declared requirement, accepts comments and pip option lines, and installs missing or outdated nh3 before loading it.

Validation approach

Python/Node tests execute production rendering and sanitization. Chromium tests load the actual grid modules and execute the update-completion handler against a DOM-backed dialog. Update-result checks cover both success and failure lists, quoted URLs, titles and missing-URL labels.

Search regressions exercise the actual node/model grid setup: encoded names remain searchable as displayed, while raw author/file values retain literal entities. Dependency checks simulate missing, outdated and supported versions with plain and commented manifests. Source inventories check complete selection/error arguments, occurrence counts and error-message assignments, and reject unparsed statically named sink calls such as quoted member access and returned-object calls. These checks are a bounded source inventory, not general JavaScript dataflow analysis. Mutation checks detect added raw values, including multiline selection markup and concatenations that retain an existing escape. Runtime tests separately verify rendering behavior.

Live Playwright scenarios use an isolated ComfyUI server, real routes, queue, installation and WebSocket completion. A local Registry provides metadata and harmless archives. They cover rendering, search/highlighting, Active installation, Flagged denial and preservation of manual settings edits. v3 runs without --enable-manager.

The new completion, search, commented-manifest and guard regressions fail before correction and pass afterward. Valid inputs remain covered alongside rejected inputs.

Validation results

Scope	Result
Standalone `2f7d16f3`, non-E2E regression	192 passed; 223 subtests passed
UI + Flagged integration 82eba56f, non-E2E regression	236 passed; 223 subtests passed
Live browser E2E on the final production files	7 scenarios passed; no uncaught browser exceptions
GitHub checks on `2f7d16f3`	Ruff and Socket passed

Live checks use frontend 1.45.15 and single-tab sessions. The public Registry and external sharing uploads were not exercised in this rerun. Update All rendering is tested with completion events in an isolated browser; a live backend Update All operation was not run.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/Comfy-Org/ComfyUI-Manager/pull/3296 **Author:** [@ltdrdata](https://github.com/ltdrdata) **Created:** 9/18/2026 **Status:** ✅ Merged **Merged:** 9/18/2026 **Merged by:** [@ltdrdata](https://github.com/ltdrdata) **Base:** `main` ← **Head:** `fix/v3-ui-escaping` --- ### 📝 Commits (1) - [`2f7d16f`](https://github.com/Comfy-Org/ComfyUI-Manager/commit/2f7d16f3820ca9ec73c94fadd483520a7e8aed6a) Escape untrusted Manager UI content ### 📊 Changes **30 files changed** (+3922 additions, -115 deletions) <details> <summary>View changed files</summary> 📝 `.gitignore` (+3 -1) 📝 `glob/manager_server.py` (+31 -24) 📝 `glob/manager_util.py` (+80 -0) 📝 `js/comfyui-manager.js` (+7 -7) 📝 `js/comfyui-share-common.js` (+2 -2) 📝 `js/comfyui-share-copus.js` (+2 -2) 📝 `js/comfyui-share-openart.js` (+2 -2) 📝 `js/comfyui-share-youml.js` (+2 -2) 📝 `js/common.js` (+42 -6) 📝 `js/custom-nodes-manager.js` (+42 -28) ➕ `js/manager-grid.js` (+67 -0) 📝 `js/model-manager.js` (+36 -23) 📝 `js/node-usage-analyzer.js` (+17 -9) 📝 `js/snapshot.js` (+3 -3) 📝 `prestartup_script.py` (+21 -5) 📝 `pyproject.toml` (+1 -1) 📝 `requirements.txt` (+2 -0) ➕ `tests/cases/url_sanitize_cases.json` (+153 -0) ➕ `tests/js_lift.py` (+159 -0) ➕ `tests/manager_test_utils.py` (+32 -0) _...and 10 more files_ </details> ### 📄 Description ## Purpose and scope Node/model metadata, workflow names and error messages can become executable HTML in Manager dialogs. Render untrusted values safely while preserving readable text, formatted descriptions, valid links and search behavior. Covers Custom Nodes, Model Manager, Node Usage Analyzer, snapshots, sharing links, update-result dialogs, Markdown descriptions and notices. Configuration-persistence fixes are already present in main through #3157. ## Key changes - Escape text and attributes at rendering boundaries and validate links through shared utilities. Update All success/failure dialogs preserve titles, URLs and labels for entries without URLs. - Keep Markdown URLs separate from formatting passes to preserve valid links. Sanitize remote notice HTML with nh3. - Make ManagerGrid search match displayed text for both server-escaped fields and raw metadata, including literal entity text. Highlighting stays inert; unknown pack states retain readable labels without invalid actions. - Declare nh3>=0.3.7 and packaging in both manifests. Startup reads the declared requirement, accepts comments and pip option lines, and installs missing or outdated nh3 before loading it. ## Validation approach Python/Node tests execute production rendering and sanitization. Chromium tests load the actual grid modules and execute the update-completion handler against a DOM-backed dialog. Update-result checks cover both success and failure lists, quoted URLs, titles and missing-URL labels. Search regressions exercise the actual node/model grid setup: encoded names remain searchable as displayed, while raw author/file values retain literal entities. Dependency checks simulate missing, outdated and supported versions with plain and commented manifests. Source inventories check complete selection/error arguments, occurrence counts and error-message assignments, and reject unparsed statically named sink calls such as quoted member access and returned-object calls. These checks are a bounded source inventory, not general JavaScript dataflow analysis. Mutation checks detect added raw values, including multiline selection markup and concatenations that retain an existing escape. Runtime tests separately verify rendering behavior. Live Playwright scenarios use an isolated ComfyUI server, real routes, queue, installation and WebSocket completion. A local Registry provides metadata and harmless archives. They cover rendering, search/highlighting, Active installation, Flagged denial and preservation of manual settings edits. v3 runs without --enable-manager. The new completion, search, commented-manifest and guard regressions fail before correction and pass afterward. Valid inputs remain covered alongside rejected inputs. ## Validation results | Scope | Result | |---|---| | Standalone 2f7d16f3, non-E2E regression | 192 passed; 223 subtests passed | | UI + Flagged integration 82eba56f, non-E2E regression | 236 passed; 223 subtests passed | | Live browser E2E on the final production files | 7 scenarios passed; no uncaught browser exceptions | | GitHub checks on 2f7d16f3 | Ruff and Socket passed | Live checks use frontend 1.45.15 and single-tab sessions. The public Registry and external sharing uploads were not exercised in this rerun. Update All rendering is tested with completion events in an isolated browser; a live backend Update All operation was not run. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-28 21:15:49 +00:00