[PR #2818] [MERGED] fix(security): harden CSRF with Content-Type gate and expand E2E coverage #2800

New issue

Closed

opened 2026-10-28 21:12:46 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-28 21:12:46 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/Comfy-Org/ComfyUI-Manager/pull/2818
Author: @ltdrdata
Created: 4/21/2026
Status: ✅ Merged
Merged: 4/21/2026
Merged by: @ltdrdata

Base: manager-v4 ← Head: fix/csrf-post-conversion

📝 Commits (1)

4eb5727 fix(security): harden CSRF with Content-Type gate and expand E2E coverage

📊 Changes

70 files changed (+13632 additions, -428 deletions)

View changed files

📝 .github/workflows/e2e.yml (+1 -1)
📝 .gitignore (+2 -0)
➕ CHANGELOG.md (+123 -0)
📝 README.md (+2 -2)
📝 comfyui_manager/__init__.py (+9 -1)
📝 comfyui_manager/common/manager_security.py (+64 -0)
📝 comfyui_manager/data_models/__init__.py (+2 -2)
📝 comfyui_manager/data_models/generated_models.py (+19 -3)
📝 comfyui_manager/glob/constants.py (+1 -0)
📝 comfyui_manager/glob/manager_core.py (+11 -2)
📝 comfyui_manager/glob/manager_server.py (+151 -53)
📝 comfyui_manager/glob/utils/environment_utils.py (+12 -0)
📝 comfyui_manager/glob/utils/security_utils.py (+9 -1)
📝 comfyui_manager/js/cm-api.js (+1 -1)
📝 comfyui_manager/js/comfyui-manager.js (+16 -7)
📝 comfyui_manager/js/common.js (+1 -1)
📝 comfyui_manager/js/custom-nodes-manager.js (+1 -1)
📝 comfyui_manager/js/model-manager.js (+1 -1)
📝 comfyui_manager/js/snapshot.js (+3 -3)
📝 comfyui_manager/legacy/manager_core.py (+7 -2)

...and 50 more files

📄 Description

Summary

Defense-in-depth CSRF hardening: Content-Type gate on 16 no-body POST handlers (8 glob + 8 legacy) to block <form method=POST> that bypasses method-only gating. The three CORS-safelisted simple-form Content-Types (x-www-form-urlencoded, multipart/form-data, text/plain) are rejected with 400; any other Content-Type forces a CORS preflight the server does not answer.
comfyui_switch_version migrated to JSON body ({ver, client_id, ui_id}). db_mode/policy/update/channel_url_list split into GET(read) + POST(write, JSON body).
Security level tightening: do_fix 'high' → 'high+' (align with SECURITY_MESSAGE_HIGH_P); comfyui_switch_version newly gated at 'high+'; three previously-ungated config setters (db_mode, policy/update, channel_url_list) now gated at 'middle'.
openapi.yaml fully resynchronized (27 paths, 30 operations, 3 dual-method splits). ComfyUISwitchVersionParams consolidated as a shared $ref component.
E2E infrastructure: 3 harness variants (start_comfyui_{legacy,permissive,strict}.sh), Playwright config + 6 legacy-UI spec files, pytest CSRF/secgate suites, 39-endpoint coverage matrix.
CHANGELOG.md (Keep a Changelog 1.1.0) added.

Breaking changes (migration notes in `CHANGELOG.md`)

Legacy per-op POST routes removed; callers already use POST /v2/manager/queue/batch:
queue/install, queue/uninstall, queue/fix, queue/disable, queue/update, queue/reinstall, queue/abort_current.
GET /manager/notice (v1) removed; /v2/manager/notice retained.

Parallel PR on main branch (v1 /manager/* routes) handling the same CVE: #2819
Both PRs share subject prefix (fix(security): harden CSRF with Content-Type gate and ...) and identical Reported-by / CVSS trailer for audit cross-reference (git log --grep='XlabAI' --all).

Test plan

ruff check comfyui_manager/ → All checks passed
python -m py_compile on all touched files → OK
pytest tests/ --ignore=tests/e2e --ignore=tests/playwright → 206 passed, 12 skipped (baseline preserved)
pytest --collect-only tests/e2e/test_e2e_csrf.py tests/e2e/test_e2e_csrf_legacy.py → 110 tests
In-process CSRF attack reproduction (fresh-eye review): 7/7 adversarial scenarios PASS
CORS Content-Type normalization probe (19 cases): no bypass path
python scripts/verify_audit_counts.py → PASS (122,0,0,1,123)
CI: Code Quality / Ruff / OpenAPI spec / E2E (ubuntu/macOS/Windows) — all green

Review summary

Pre-PR security review (3 agents — reviewer / consumer-impact / doc-sync): all CRITICAL/MAJOR items addressed
Fresh-eye adversarial review: PASS (peer consensus)
Coordinated with #2819 (parallel PR) and cmfront-team (ComfyUI_frontend type regeneration)

References

Reported-by: XlabAI Team of Tencent Xuanwu Lab
CVSS: 8.1 (AV:N/AC:L/PR:N/UI:R/S:U/C:N/I:H/A:H)

_{🔄 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/2818 **Author:** [@ltdrdata](https://github.com/ltdrdata) **Created:** 4/21/2026 **Status:** ✅ Merged **Merged:** 4/21/2026 **Merged by:** [@ltdrdata](https://github.com/ltdrdata) **Base:** `manager-v4` ← **Head:** `fix/csrf-post-conversion` --- ### 📝 Commits (1) - [`4eb5727`](https://github.com/Comfy-Org/ComfyUI-Manager/commit/4eb572776af813c6637fbf175e04224855719589) fix(security): harden CSRF with Content-Type gate and expand E2E coverage ### 📊 Changes **70 files changed** (+13632 additions, -428 deletions) <details> <summary>View changed files</summary> 📝 `.github/workflows/e2e.yml` (+1 -1) 📝 `.gitignore` (+2 -0) ➕ `CHANGELOG.md` (+123 -0) 📝 `README.md` (+2 -2) 📝 `comfyui_manager/__init__.py` (+9 -1) 📝 `comfyui_manager/common/manager_security.py` (+64 -0) 📝 `comfyui_manager/data_models/__init__.py` (+2 -2) 📝 `comfyui_manager/data_models/generated_models.py` (+19 -3) 📝 `comfyui_manager/glob/constants.py` (+1 -0) 📝 `comfyui_manager/glob/manager_core.py` (+11 -2) 📝 `comfyui_manager/glob/manager_server.py` (+151 -53) 📝 `comfyui_manager/glob/utils/environment_utils.py` (+12 -0) 📝 `comfyui_manager/glob/utils/security_utils.py` (+9 -1) 📝 `comfyui_manager/js/cm-api.js` (+1 -1) 📝 `comfyui_manager/js/comfyui-manager.js` (+16 -7) 📝 `comfyui_manager/js/common.js` (+1 -1) 📝 `comfyui_manager/js/custom-nodes-manager.js` (+1 -1) 📝 `comfyui_manager/js/model-manager.js` (+1 -1) 📝 `comfyui_manager/js/snapshot.js` (+3 -3) 📝 `comfyui_manager/legacy/manager_core.py` (+7 -2) _...and 50 more files_ </details> ### 📄 Description ## Summary - Defense-in-depth CSRF hardening: Content-Type gate on 16 no-body POST handlers (8 glob + 8 legacy) to block `<form method=POST>` that bypasses method-only gating. The three CORS-safelisted simple-form Content-Types (`x-www-form-urlencoded`, `multipart/form-data`, `text/plain`) are rejected with 400; any other Content-Type forces a CORS preflight the server does not answer. - `comfyui_switch_version` migrated to JSON body (`{ver, client_id, ui_id}`). `db_mode`/`policy/update`/`channel_url_list` split into GET(read) + POST(write, JSON body). - Security level tightening: `do_fix` `'high'` → `'high+'` (align with `SECURITY_MESSAGE_HIGH_P`); `comfyui_switch_version` newly gated at `'high+'`; three previously-ungated config setters (`db_mode`, `policy/update`, `channel_url_list`) now gated at `'middle'`. - `openapi.yaml` fully resynchronized (27 paths, 30 operations, 3 dual-method splits). `ComfyUISwitchVersionParams` consolidated as a shared `$ref` component. - E2E infrastructure: 3 harness variants (`start_comfyui_{legacy,permissive,strict}.sh`), Playwright config + 6 legacy-UI spec files, pytest CSRF/secgate suites, 39-endpoint coverage matrix. - `CHANGELOG.md` (Keep a Changelog 1.1.0) added. ### Breaking changes (migration notes in `CHANGELOG.md`) - Legacy per-op POST routes removed; callers already use `POST /v2/manager/queue/batch`: `queue/install`, `queue/uninstall`, `queue/fix`, `queue/disable`, `queue/update`, `queue/reinstall`, `queue/abort_current`. - `GET /manager/notice` (v1) removed; `/v2/manager/notice` retained. ## Related - Parallel PR on `main` branch (v1 `/manager/*` routes) handling the same CVE: #2819 - Both PRs share subject prefix (`fix(security): harden CSRF with Content-Type gate and ...`) and identical `Reported-by` / `CVSS` trailer for audit cross-reference (`git log --grep='XlabAI' --all`). ## Test plan - [x] `ruff check comfyui_manager/` → All checks passed - [x] `python -m py_compile` on all touched files → OK - [x] `pytest tests/ --ignore=tests/e2e --ignore=tests/playwright` → 206 passed, 12 skipped (baseline preserved) - [x] `pytest --collect-only tests/e2e/test_e2e_csrf.py tests/e2e/test_e2e_csrf_legacy.py` → 110 tests - [x] In-process CSRF attack reproduction (fresh-eye review): 7/7 adversarial scenarios PASS - [x] CORS Content-Type normalization probe (19 cases): no bypass path - [x] `python scripts/verify_audit_counts.py` → PASS (122,0,0,1,123) - [x] CI: Code Quality / Ruff / OpenAPI spec / E2E (ubuntu/macOS/Windows) — all green ## Review summary - Pre-PR security review (3 agents — reviewer / consumer-impact / doc-sync): all CRITICAL/MAJOR items addressed - Fresh-eye adversarial review: PASS (peer consensus) - Coordinated with #2819 (parallel PR) and cmfront-team (ComfyUI_frontend type regeneration) ## References - Reported-by: XlabAI Team of Tencent Xuanwu Lab - CVSS: 8.1 (AV:N/AC:L/PR:N/UI:R/S:U/C:N/I:H/A:H) --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-28 21:12:46 +00:00