[PR #3297] [MERGED] v4: Secure legacy UI rendering and preserve administrator settings #3174

New issue

Closed

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

bsakaguchi commented

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

Owner

📋 Pull Request Information

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

Base: manager-v4 ← Head: fix/v4-ui-escaping

📝 Commits (1)

68fcd74 Harden legacy UI escaping and preserve administrator settings

📊 Changes

32 files changed (+3492 additions, -331 deletions)

View changed files

➕ comfyui_manager/common/config_writer.py (+76 -0)
➕ comfyui_manager/common/security_messages.py (+7 -0)
📝 comfyui_manager/glob/constants.py (+0 -9)
📝 comfyui_manager/glob/manager_core.py (+27 -34)
📝 comfyui_manager/glob/manager_server.py (+4 -2)
📝 comfyui_manager/glob/utils/formatting_utils.py (+0 -39)
📝 comfyui_manager/js/comfyui-manager.js (+10 -9)
📝 comfyui_manager/js/comfyui-share-common.js (+3 -2)
📝 comfyui_manager/js/comfyui-share-copus.js (+2 -2)
📝 comfyui_manager/js/comfyui-share-openart.js (+2 -2)
📝 comfyui_manager/js/comfyui-share-youml.js (+2 -2)
📝 comfyui_manager/js/common.js (+53 -3)
📝 comfyui_manager/js/custom-nodes-manager.js (+93 -58)
➕ comfyui_manager/js/manager-grid.js (+67 -0)
📝 comfyui_manager/js/model-manager.js (+90 -92)
📝 comfyui_manager/js/snapshot.js (+3 -3)
➕ comfyui_manager/legacy/html_utils.py (+82 -0)
📝 comfyui_manager/legacy/manager_core.py (+25 -32)
📝 comfyui_manager/legacy/manager_server.py (+52 -38)
📝 pyproject.toml (+3 -1)

...and 12 more files

📄 Description

Purpose and scope

External node/model metadata and error responses can execute HTML in the legacy Manager UI. Make these inputs inert while preserving names, formatted descriptions, valid links and search behavior. Handle rejected and malformed batch requests so errors are visible and pending controls reflect the work actually accepted.

The connected configuration fix preserves manually edited security settings during unrelated UI saves, porting behavior already in v3 main through #3157. Rendering changes cover legacy responses and the shipped UI; configuration persistence and shared denial messages cover both glob and legacy servers.

Key changes

Apply text, attribute and URL handling across node/model views, missing-node messages, sharing, snapshots and task results. Keep Markdown URLs outside formatting passes and sanitize notices with nh3.
Make ManagerGrid search match displayed text for both server-escaped fields and raw metadata, including literal entities. Highlighting stays inert; unknown pack states retain readable labels without invalid actions. Remove the unused glob Markdown renderer.
Map rejected batch IDs to readable names, clear pending controls on complete rejection, request failure or malformed responses, and keep waiting for accepted work after partial rejection.
Merge changed settings onto current disk contents through one writer used by both cores. Preserve unrelated keys/sections and manual edits; refuse unreadable existing files and replace completed writes atomically.
Share server denial guidance, handle malformed legacy Git/pip denial responses, and declare nh3>=0.3.7 and packaging in both manifests.

Settings writes preserve parsed values, not INI comments/formatting. Malformed-file recovery warns and rewrites current settings, preserving the existing fallback. Installation-flag edits require restart.

Validation approach

Python tests execute production rendering, sanitization and both configuration cores using temporary files. Chromium checks exercise actual grid setup and search: encoded names match their displayed form, while raw authors/filenames retain literal entities. The search defect fails before correction and passes afterward.

Playwright checks use served UI assets with controlled HTTP responses/events to verify inert rendering, safe links, unknown states, malformed errors, complete batch rejection, partial acceptance and network/HTTP/JSON/response-shape failures. All eight request-failure cases fail before correction and pass afterward. Separate live scenarios use real legacy routes, queue, installation and WebSocket completion; a local Registry provides metadata and harmless archives. They verify rendering/search, Active installation, Flagged denial and preservation of manual settings edits.

Validation results

Current source is 68fcd746; the UI + Flagged integration is 6df4aa14. The final change only strengthens the denial-response E2E assertion; production code and the suites recorded below are unchanged. Both real legacy denial endpoints passed the revised exact-body assertion. A controlled extra JSON field was accepted before the change and rejected afterward.

Scope	Result
Standalone 744def89, non-E2E regression	464 passed; 12 skipped; 3 inherited failures
UI + Flagged integration 1a04122a, non-E2E regression	569 passed; 12 skipped; the same 3 failures
Served-assets browser suite on 1a04122a	49 passed
Live browser E2E on 1a04122a	7 scenarios passed; no uncaught browser exceptions

The three local failures are Git backup fixtures reproduced on unchanged manager-v4. The twelve skips require a separate CLI environment.

GitHub Ruff, OpenAPI, code-quality and Socket checks passed. CLI E2E CI passed on Ubuntu, Windows and macOS after rebasing on manager-v4, which includes #3300.

Live checks use frontend 1.45.15 and single-tab sessions. The legacy menu opens through its registered command because of an inherited toolbar issue; toolbar visibility is not established by these tests. External sharing uploads and the public Registry were not exercised. The existing disabled TLS verification in notice retrieval is tracked in #3302, and cross-batch model completion handling in #3303. Both are separate follow-ups with acceptance criteria; HTML sanitization does not authenticate upstream notice content.

_{🔄 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/3297 **Author:** [@ltdrdata](https://github.com/ltdrdata) **Created:** 9/18/2026 **Status:** ✅ Merged **Merged:** 9/18/2026 **Merged by:** [@ltdrdata](https://github.com/ltdrdata) **Base:** `manager-v4` ← **Head:** `fix/v4-ui-escaping` --- ### 📝 Commits (1) - [`68fcd74`](https://github.com/Comfy-Org/ComfyUI-Manager/commit/68fcd7463dd567eda181892a20446f519aec393b) Harden legacy UI escaping and preserve administrator settings ### 📊 Changes **32 files changed** (+3492 additions, -331 deletions) <details> <summary>View changed files</summary> ➕ `comfyui_manager/common/config_writer.py` (+76 -0) ➕ `comfyui_manager/common/security_messages.py` (+7 -0) 📝 `comfyui_manager/glob/constants.py` (+0 -9) 📝 `comfyui_manager/glob/manager_core.py` (+27 -34) 📝 `comfyui_manager/glob/manager_server.py` (+4 -2) 📝 `comfyui_manager/glob/utils/formatting_utils.py` (+0 -39) 📝 `comfyui_manager/js/comfyui-manager.js` (+10 -9) 📝 `comfyui_manager/js/comfyui-share-common.js` (+3 -2) 📝 `comfyui_manager/js/comfyui-share-copus.js` (+2 -2) 📝 `comfyui_manager/js/comfyui-share-openart.js` (+2 -2) 📝 `comfyui_manager/js/comfyui-share-youml.js` (+2 -2) 📝 `comfyui_manager/js/common.js` (+53 -3) 📝 `comfyui_manager/js/custom-nodes-manager.js` (+93 -58) ➕ `comfyui_manager/js/manager-grid.js` (+67 -0) 📝 `comfyui_manager/js/model-manager.js` (+90 -92) 📝 `comfyui_manager/js/snapshot.js` (+3 -3) ➕ `comfyui_manager/legacy/html_utils.py` (+82 -0) 📝 `comfyui_manager/legacy/manager_core.py` (+25 -32) 📝 `comfyui_manager/legacy/manager_server.py` (+52 -38) 📝 `pyproject.toml` (+3 -1) _...and 12 more files_ </details> ### 📄 Description ## Purpose and scope External node/model metadata and error responses can execute HTML in the legacy Manager UI. Make these inputs inert while preserving names, formatted descriptions, valid links and search behavior. Handle rejected and malformed batch requests so errors are visible and pending controls reflect the work actually accepted. The connected configuration fix preserves manually edited security settings during unrelated UI saves, porting behavior already in v3 main through #3157. Rendering changes cover legacy responses and the shipped UI; configuration persistence and shared denial messages cover both glob and legacy servers. ## Key changes - Apply text, attribute and URL handling across node/model views, missing-node messages, sharing, snapshots and task results. Keep Markdown URLs outside formatting passes and sanitize notices with nh3. - Make ManagerGrid search match displayed text for both server-escaped fields and raw metadata, including literal entities. Highlighting stays inert; unknown pack states retain readable labels without invalid actions. Remove the unused glob Markdown renderer. - Map rejected batch IDs to readable names, clear pending controls on complete rejection, request failure or malformed responses, and keep waiting for accepted work after partial rejection. - Merge changed settings onto current disk contents through one writer used by both cores. Preserve unrelated keys/sections and manual edits; refuse unreadable existing files and replace completed writes atomically. - Share server denial guidance, handle malformed legacy Git/pip denial responses, and declare nh3>=0.3.7 and packaging in both manifests. Settings writes preserve parsed values, not INI comments/formatting. Malformed-file recovery warns and rewrites current settings, preserving the existing fallback. Installation-flag edits require restart. ## Validation approach Python tests execute production rendering, sanitization and both configuration cores using temporary files. Chromium checks exercise actual grid setup and search: encoded names match their displayed form, while raw authors/filenames retain literal entities. The search defect fails before correction and passes afterward. Playwright checks use served UI assets with controlled HTTP responses/events to verify inert rendering, safe links, unknown states, malformed errors, complete batch rejection, partial acceptance and network/HTTP/JSON/response-shape failures. All eight request-failure cases fail before correction and pass afterward. Separate live scenarios use real legacy routes, queue, installation and WebSocket completion; a local Registry provides metadata and harmless archives. They verify rendering/search, Active installation, Flagged denial and preservation of manual settings edits. ## Validation results Current source is 68fcd746; the UI + Flagged integration is 6df4aa14. The final change only strengthens the denial-response E2E assertion; production code and the suites recorded below are unchanged. Both real legacy denial endpoints passed the revised exact-body assertion. A controlled extra JSON field was accepted before the change and rejected afterward. | Scope | Result | |---|---| | Standalone 744def89, non-E2E regression | 464 passed; 12 skipped; 3 inherited failures | | UI + Flagged integration 1a04122a, non-E2E regression | 569 passed; 12 skipped; the same 3 failures | | Served-assets browser suite on 1a04122a | 49 passed | | Live browser E2E on 1a04122a | 7 scenarios passed; no uncaught browser exceptions | The three local failures are Git backup fixtures reproduced on unchanged manager-v4. The twelve skips require a separate CLI environment. GitHub Ruff, OpenAPI, code-quality and Socket checks passed. [CLI E2E CI](https://github.com/Comfy-Org/ComfyUI-Manager/actions/runs/35402176395) passed on Ubuntu, Windows and macOS after rebasing on manager-v4, which includes #3300. Live checks use frontend 1.45.15 and single-tab sessions. The legacy menu opens through its registered command because of an inherited toolbar issue; toolbar visibility is not established by these tests. External sharing uploads and the public Registry were not exercised. The existing disabled TLS verification in notice retrieval is tracked in [#3302](https://github.com/Comfy-Org/ComfyUI-Manager/issues/3302), and cross-batch model completion handling in [#3303](https://github.com/Comfy-Org/ComfyUI-Manager/issues/3303). Both are separate follow-ups with acceptance criteria; HTML sanitization does not authenticate upstream notice content. --- <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:50 +00:00