From 491f847bbc286588175695ea43fa4e13cd14a437 Mon Sep 17 00:00:00 2001 From: "Dr.Lt.Data" <128333288+ltdrdata@users.noreply.github.com> Date: Wed, 22 Apr 2026 05:04:07 +0900 Subject: [PATCH] fix(security): harden CSRF with Content-Type gate and OpenAPI sync (#2819) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Defense-in-depth over GET→POST alone: reject the three CORS-safelisted simple-form Content-Types (x-www-form-urlencoded, multipart/form-data, text/plain) on 5 no-body POST handlers (snapshot/save, manager/queue/{reset,start,update_comfyui}, manager/reboot) to block
CSRF that bypasses method-only gating. Convert 10 pure state-changing endpoints (fetch_updates, queue/{update_all,reset,start, update_comfyui}, snapshot/{remove,restore,save}, comfyui_switch_version, reboot) from GET to POST and split 5 config endpoints (db_mode/preview_method/channel_url_list/policy/{component,update}) into GET(read) + POST(write, JSON body). Emit the in_progress + done event pair from the /manager/queue/install sync-enable fast-path so client UI finalizes (previously only queue/start's empty worker done fired, leaving item.restart unset and the Enable button visible after a successful enable). Harden js/custom-nodes-manager.js completion path: await onQueueCompleted with try/catch (surfaces silent turbogrid stale-item throws), replace the {}.length == 0 no-op empty guard, set install_context before queue/install to avoid a sync-completion race, wrap classList/updateCell in try/catch. Resynchronize openapi.yaml with the converted routes (method → post, query params → requestBody JSON schema, sibling post on 5 split endpoints). Update 31 JS fetchApi call sites across 7 files; add tests/test_csrf_content_type_helper.py covering 5 Content-Type cases via aiohttp TestClient. 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) --- glob/manager_core.py | 2 +- glob/manager_server.py | 243 +++++++++++++++++-------- js/cm-api.js | 2 +- js/comfyui-manager.js | 28 +-- js/common.js | 2 +- js/custom-nodes-manager.js | 82 +++++++-- js/model-manager.js | 6 +- js/snapshot.js | 6 +- openapi.yaml | 217 +++++++++++++++++++--- pyproject.toml | 2 +- tests/test_csrf_content_type_helper.py | 121 ++++++++++++ 11 files changed, 567 insertions(+), 144 deletions(-) create mode 100644 tests/test_csrf_content_type_helper.py diff --git a/glob/manager_core.py b/glob/manager_core.py index e0b3a6fe..7c7e2fae 100644 --- a/glob/manager_core.py +++ b/glob/manager_core.py @@ -44,7 +44,7 @@ import manager_migration from node_package import InstalledNodePackage -version_code = [3, 39, 2] +version_code = [3, 40] version_str = f"V{version_code[0]}.{version_code[1]}" + (f'.{version_code[2]}' if len(version_code) > 2 else '') diff --git a/glob/manager_server.py b/glob/manager_server.py index eff7c032..8474bb9c 100644 --- a/glob/manager_server.py +++ b/glob/manager_server.py @@ -312,6 +312,57 @@ def security_403_response(): return web.json_response({"error": "security_level"}, status=403) +# CORS "simple request" Content-Type set per Fetch spec §3.2.3. Browsers send +# submissions with one of these three MIME types and do NOT +# trigger a CORS preflight, so a malicious cross-origin page can silently POST +# into state-changing endpoints if we only gate on HTTP method. Blocking these +# three Content-Types on no-body mutation endpoints forces any non-same-origin +# POST to use a non-simple Content-Type (e.g. application/json), which triggers +# a preflight that this server rejects by not advertising an Access-Control- +# Allow-Origin response. +_SIMPLE_FORM_CONTENT_TYPES = frozenset({ + 'application/x-www-form-urlencoded', + 'multipart/form-data', + 'text/plain', +}) + + +def _reject_simple_form_content_type(request): + """Reject Content-Types that enable preflight-less CSRF. + + Applied ONLY to POST handlers that do not consume a request body (e.g., + /snapshot/save, /manager/queue/{reset,start,update_comfyui}, + /manager/reboot). These are vulnerable to cross-origin + attacks because the handler accepts the request without parsing any body — + the attacker needs no ability to forge a valid payload, only to point a + hidden form at the URL. + + Handlers that already read a body via ``await request.json()`` are NOT + gated here: a cross-origin cannot forge a valid JSON + body because the browser refuses to send ``application/json`` without a + CORS preflight, which this server does not answer. + + DO NOT add this gate to body-reading handlers — redundant and UX-breaking. + DO NOT remove this gate from no-body handlers — this is the bypass vector. + + aiohttp's ``request.content_type`` normalizes the header (lower-cases, + strips parameters), so ``multipart/form-data; boundary=----X`` is compared + as ``multipart/form-data``. + + Returns: + web.Response(status=400) when the request has a simple-form + Content-Type that must be rejected. None when the request is allowed + to proceed (no Content-Type, application/json, or any non-simple + Content-Type). + """ + if request.content_type in _SIMPLE_FORM_CONTENT_TYPES: + return web.Response( + status=400, + text='Invalid Content-Type for this endpoint. Use application/json or omit body.', + ) + return None + + def get_model_dir(data, show_log=False): if 'download_model_base' in folder_paths.folder_names_and_paths: models_base = folder_paths.folder_names_and_paths['download_model_base'][0][0] @@ -737,16 +788,19 @@ async def fetch_customnode_mappings(request): return web.json_response(json_obj, content_type='application/json') -@routes.get("/customnode/fetch_updates") +@routes.post("/customnode/fetch_updates") async def fetch_updates(request): try: - if request.rel_url.query["mode"] == "local": + json_data = await request.json() + mode = json_data.get("mode", "default") + + if mode == "local": channel = 'local' else: channel = core.get_config()['channel_url'] - await core.unified_manager.reload(request.rel_url.query["mode"]) - await core.unified_manager.get_custom_nodes(channel, request.rel_url.query["mode"]) + await core.unified_manager.reload(mode) + await core.unified_manager.get_custom_nodes(channel, mode) res = core.unified_manager.fetch_or_pull_git_repo(is_pull=False) @@ -764,7 +818,7 @@ async def fetch_updates(request): return web.Response(status=400) -@routes.get("/manager/queue/update_all") +@routes.post("/manager/queue/update_all") async def update_all(request): if not is_allowed_security_level('middle'): logging.error(SECURITY_MESSAGE_MIDDLE_OR_BELOW) @@ -774,16 +828,19 @@ async def update_all(request): is_processing = task_worker_thread is not None and task_worker_thread.is_alive() if is_processing: return web.Response(status=401) - + await core.save_snapshot_with_postfix('autosave') - if request.rel_url.query["mode"] == "local": + json_data = await request.json() + mode = json_data.get("mode", "default") + + if mode == "local": channel = 'local' else: channel = core.get_config()['channel_url'] - await core.unified_manager.reload(request.rel_url.query["mode"]) - await core.unified_manager.get_custom_nodes(channel, request.rel_url.query["mode"]) + await core.unified_manager.reload(mode) + await core.unified_manager.get_custom_nodes(channel, mode) for k, v in core.unified_manager.active_nodes.items(): if k == 'comfyui-manager': @@ -1006,14 +1063,15 @@ def get_safe_snapshot_path(target): return os.path.join(core.manager_snapshot_path, f"{target}.json") -@routes.get("/snapshot/remove") +@routes.post("/snapshot/remove") async def remove_snapshot(request): if not is_allowed_security_level('middle'): logging.error(SECURITY_MESSAGE_MIDDLE_OR_BELOW) return security_403_response() try: - target = request.rel_url.query["target"] + json_data = await request.json() + target = json_data["target"] path = get_safe_snapshot_path(target) if path is None: @@ -1028,14 +1086,15 @@ async def remove_snapshot(request): return web.Response(status=400) -@routes.get("/snapshot/restore") +@routes.post("/snapshot/restore") async def restore_snapshot(request): if not is_allowed_security_level('middle'): logging.error(SECURITY_MESSAGE_MIDDLE_OR_BELOW) return security_403_response() try: - target = request.rel_url.query["target"] + json_data = await request.json() + target = json_data["target"] path = get_safe_snapshot_path(target) if path is None: @@ -1066,8 +1125,11 @@ async def get_current_snapshot_api(request): return web.Response(status=400) -@routes.get("/snapshot/save") +@routes.post("/snapshot/save") async def save_snapshot(request): + resp = _reject_simple_form_content_type(request) + if resp is not None: + return resp try: await core.save_snapshot_with_postfix('snapshot') return web.Response(status=200) @@ -1228,8 +1290,11 @@ async def reinstall_custom_node(request): await install_custom_node(request) -@routes.get("/manager/queue/reset") +@routes.post("/manager/queue/reset") async def reset_queue(request): + resp = _reject_simple_form_content_type(request) + if resp is not None: + return resp global task_queue task_queue = queue.Queue() return web.Response(status=200) @@ -1270,6 +1335,26 @@ async def install_custom_node(request): if skip_post_install: if cnr_id in core.unified_manager.nightly_inactive_nodes or cnr_id in core.unified_manager.cnr_inactive_nodes: core.unified_manager.unified_enable(cnr_id) + # Mirror the pair of events (in_progress then done) that the async + # task_worker normally emits for queued operations. The in_progress + # event is what sets item.restart=true on the client row so the + # action cell re-renders as "Restart Required"; without it, the + # "Enable" button remains visible after successful enable. The done + # event drives the completion UI (toast, restart indicator, button + # loading clear). + ui_id = json_data.get('ui_id', cnr_id) + PromptServer.instance.send_sync( + "cm-queue-status", + {'status': 'in_progress', + 'target': ui_id, + 'ui_target': 'nodepack_manager', + 'total_count': 1, 'done_count': 0}) + PromptServer.instance.send_sync( + "cm-queue-status", + {'status': 'done', + 'nodepack_result': {ui_id: 'success'}, + 'model_result': {}, + 'total_count': 1, 'done_count': 1}) return web.Response(status=200) elif selected_version is None: selected_version = 'latest' @@ -1311,8 +1396,11 @@ async def install_custom_node(request): task_worker_thread:threading.Thread = None -@routes.get("/manager/queue/start") +@routes.post("/manager/queue/start") async def queue_start(request): + resp = _reject_simple_form_content_type(request) + if resp is not None: + return resp global nodepack_result global model_result global task_worker_thread @@ -1427,8 +1515,11 @@ async def update_custom_node(request): return web.Response(status=200) -@routes.get("/manager/queue/update_comfyui") +@routes.post("/manager/queue/update_comfyui") async def update_comfyui(request): + resp = _reject_simple_form_content_type(request) + if resp is not None: + return resp is_stable = core.get_config()['update_policy'] != 'nightly-comfyui' task_queue.put(("update-comfyui", ('comfyui', is_stable))) return web.Response(status=200) @@ -1445,11 +1536,12 @@ async def comfyui_versions(request): return web.Response(status=400) -@routes.get("/comfyui_manager/comfyui_switch_version") +@routes.post("/comfyui_manager/comfyui_switch_version") async def comfyui_switch_version(request): try: - if "ver" in request.rel_url.query: - core.switch_comfyui(request.rel_url.query['ver']) + json_data = await request.json() + if "ver" in json_data: + core.switch_comfyui(json_data['ver']) return web.Response(status=200) except Exception as e: @@ -1526,83 +1618,87 @@ async def install_model(request): @routes.get("/manager/preview_method") -async def preview_method(request): - # Setting change request - if "value" in request.rel_url.query: - # Reject setting change if per-queue preview feature is available - if COMFYUI_HAS_PER_QUEUE_PREVIEW: - return web.Response(text="DISABLED", status=403) +async def get_preview_method(request): + if COMFYUI_HAS_PER_QUEUE_PREVIEW: + return web.Response(text="DISABLED", status=200) + return web.Response(text=core.manager_funcs.get_current_preview_method(), status=200) - # Process normally if not available - set_preview_method(request.rel_url.query['value']) - core.write_config() - return web.Response(status=200) - # Status query request - else: - # Return DISABLED if per-queue preview feature is available - if COMFYUI_HAS_PER_QUEUE_PREVIEW: - return web.Response(text="DISABLED", status=200) +@routes.post("/manager/preview_method") +async def set_preview_method_handler(request): + if COMFYUI_HAS_PER_QUEUE_PREVIEW: + return web.Response(text="DISABLED", status=403) - # Return current value if not available - return web.Response(text=core.manager_funcs.get_current_preview_method(), status=200) + json_data = await request.json() + set_preview_method(json_data['value']) + core.write_config() + return web.Response(status=200) @routes.get("/manager/db_mode") -async def db_mode(request): - if "value" in request.rel_url.query: - set_db_mode(request.rel_url.query['value']) - core.write_config() - else: - return web.Response(text=core.get_config()['db_mode'], status=200) +async def get_db_mode(request): + return web.Response(text=core.get_config()['db_mode'], status=200) + +@routes.post("/manager/db_mode") +async def set_db_mode_handler(request): + json_data = await request.json() + set_db_mode(json_data['value']) + core.write_config() return web.Response(status=200) @routes.get("/manager/policy/component") -async def component_policy(request): - if "value" in request.rel_url.query: - set_component_policy(request.rel_url.query['value']) - core.write_config() - else: - return web.Response(text=core.get_config()['component_policy'], status=200) +async def get_component_policy(request): + return web.Response(text=core.get_config()['component_policy'], status=200) + +@routes.post("/manager/policy/component") +async def set_component_policy_handler(request): + json_data = await request.json() + set_component_policy(json_data['value']) + core.write_config() return web.Response(status=200) @routes.get("/manager/policy/update") -async def update_policy(request): - if "value" in request.rel_url.query: - set_update_policy(request.rel_url.query['value']) - core.write_config() - else: - return web.Response(text=core.get_config()['update_policy'], status=200) +async def get_update_policy(request): + return web.Response(text=core.get_config()['update_policy'], status=200) + +@routes.post("/manager/policy/update") +async def set_update_policy_handler(request): + json_data = await request.json() + set_update_policy(json_data['value']) + core.write_config() return web.Response(status=200) @routes.get("/manager/channel_url_list") -async def channel_url_list(request): +async def get_channel_url_list(request): channels = core.get_channel_dict() - if "value" in request.rel_url.query: - channel_url = channels.get(request.rel_url.query['value']) - if channel_url is not None: - core.get_config()['channel_url'] = channel_url - core.write_config() - else: - selected = 'custom' - selected_url = core.get_config()['channel_url'] + selected = 'custom' + selected_url = core.get_config()['channel_url'] - for name, url in channels.items(): - if url == selected_url: - selected = name - break + for name, url in channels.items(): + if url == selected_url: + selected = name + break - res = {'selected': selected, - 'list': core.get_channel_list()} - return web.json_response(res, status=200) + res = {'selected': selected, + 'list': core.get_channel_list()} + return web.json_response(res, status=200) + +@routes.post("/manager/channel_url_list") +async def set_channel_url_list(request): + json_data = await request.json() + channels = core.get_channel_dict() + channel_url = channels.get(json_data['value']) + if channel_url is not None: + core.get_config()['channel_url'] = channel_url + core.write_config() return web.Response(status=200) @@ -1700,8 +1796,11 @@ async def get_startup_alerts(request): return web.json_response(alerts) -@routes.get("/manager/reboot") +@routes.post("/manager/reboot") def restart(self): + resp = _reject_simple_form_content_type(self) + if resp is not None: + return resp if not is_allowed_security_level('middle'): logging.error(SECURITY_MESSAGE_MIDDLE_OR_BELOW) return security_403_response() diff --git a/js/cm-api.js b/js/cm-api.js index c4d6da03..53752a9b 100644 --- a/js/cm-api.js +++ b/js/cm-api.js @@ -52,7 +52,7 @@ async function tryInstallCustomNode(event) { } } - let response = await api.fetchApi("/manager/reboot"); + let response = await api.fetchApi("/manager/reboot", { method: 'POST' }); if(response.status == 403) { await handle403Response(response); return false; diff --git a/js/comfyui-manager.js b/js/comfyui-manager.js index bcf7e9e5..53431657 100644 --- a/js/comfyui-manager.js +++ b/js/comfyui-manager.js @@ -470,12 +470,12 @@ async function updateComfyUI() { set_inprogress_mode(); - const response = await api.fetchApi('/manager/queue/update_comfyui'); + const response = await api.fetchApi('/manager/queue/update_comfyui', { method: 'POST' }); showTerminal(); is_updating = true; - await api.fetchApi('/manager/queue/start'); + await api.fetchApi('/manager/queue/start', { method: 'POST' }); } function showVersionSelectorDialog(versions, current, onSelect) { @@ -625,14 +625,14 @@ async function switchComfyUI() { showVersionSelectorDialog(versions, obj.current, async (selected_version) => { if(selected_version == 'nightly') { update_policy_combo.value = 'nightly-comfyui'; - api.fetchApi('/manager/policy/update?value=nightly-comfyui'); + api.fetchApi('/manager/policy/update', { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ value: 'nightly-comfyui' }) }); } else { update_policy_combo.value = 'stable-comfyui'; - api.fetchApi('/manager/policy/update?value=stable-comfyui'); + api.fetchApi('/manager/policy/update', { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ value: 'stable-comfyui' }) }); } - let response = await api.fetchApi(`/comfyui_manager/comfyui_switch_version?ver=${selected_version}`, { cache: "no-store" }); + let response = await api.fetchApi('/comfyui_manager/comfyui_switch_version', { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ ver: selected_version }), cache: "no-store" }); if (response.status == 200) { infoToast(`ComfyUI version is switched to ${selected_version}`); } @@ -769,10 +769,10 @@ async function updateAll(update_comfyui) { if(update_comfyui) { update_all_button.innerText = "Updating ComfyUI..."; - await api.fetchApi('/manager/queue/update_comfyui'); + await api.fetchApi('/manager/queue/update_comfyui', { method: 'POST' }); } - const response = await api.fetchApi(`/manager/queue/update_all?mode=${mode}`); + const response = await api.fetchApi('/manager/queue/update_all', { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ mode: mode }) }); if (response.status == 403) { await handle403Response(response); @@ -784,7 +784,7 @@ async function updateAll(update_comfyui) { } else if(response.status == 200) { is_updating = true; - await api.fetchApi('/manager/queue/start'); + await api.fetchApi('/manager/queue/start', { method: 'POST' }); } } @@ -813,7 +813,7 @@ function restartOrStop() { rebootAPI(); } else { - api.fetchApi('/manager/queue/reset'); + api.fetchApi('/manager/queue/reset', { method: 'POST' }); infoToast('Cancel', 'Remaining tasks will stop after completing the current task.'); } } @@ -967,7 +967,7 @@ class ManagerMenuDialog extends ComfyDialog { .then(data => { this.datasrc_combo.value = data; }); this.datasrc_combo.addEventListener('change', function (event) { - api.fetchApi(`/manager/db_mode?value=${event.target.value}`); + api.fetchApi('/manager/db_mode', { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ value: event.target.value }) }); }); const dbRetrievalSetttingItem = createSettingsCombo("DB", this.datasrc_combo); @@ -1043,7 +1043,7 @@ class ManagerMenuDialog extends ComfyDialog { } // Normal operation - api.fetchApi(`/manager/preview_method?value=${event.target.value}`) + api.fetchApi('/manager/preview_method', { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ value: event.target.value }) }) .then(response => { if (response.status === 403) { // Feature transitioned to native @@ -1087,7 +1087,7 @@ class ManagerMenuDialog extends ComfyDialog { } channel_combo.addEventListener('change', function (event) { - api.fetchApi(`/manager/channel_url_list?value=${event.target.value}`); + api.fetchApi('/manager/channel_url_list', { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ value: event.target.value }) }); }); channel_combo.value = data.selected; @@ -1152,7 +1152,7 @@ class ManagerMenuDialog extends ComfyDialog { }); component_policy_combo.addEventListener('change', function (event) { - api.fetchApi(`/manager/policy/component?value=${event.target.value}`); + api.fetchApi('/manager/policy/component', { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ value: event.target.value }) }); set_component_policy(event.target.value); }); @@ -1171,7 +1171,7 @@ class ManagerMenuDialog extends ComfyDialog { }); update_policy_combo.addEventListener('change', function (event) { - api.fetchApi(`/manager/policy/update?value=${event.target.value}`); + api.fetchApi('/manager/policy/update', { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ value: event.target.value }) }); }); const updateSetttingItem = createSettingsCombo("Update", update_policy_combo); diff --git a/js/common.js b/js/common.js index b8193055..d51485da 100644 --- a/js/common.js +++ b/js/common.js @@ -185,7 +185,7 @@ export async function rebootAPI() { const isConfirmed = await customConfirm("Are you sure you'd like to reboot the server?"); if (isConfirmed) { try { - const response = await api.fetchApi("/manager/reboot"); + const response = await api.fetchApi("/manager/reboot", { method: 'POST' }); if (response.status == 403) { await handle403Response(response); return false; diff --git a/js/custom-nodes-manager.js b/js/custom-nodes-manager.js index b290df61..1fdb2e20 100644 --- a/js/custom-nodes-manager.js +++ b/js/custom-nodes-manager.js @@ -462,7 +462,7 @@ export class CustomNodesManager { ".cn-manager-stop": { click: () => { - api.fetchApi('/manager/queue/reset'); + api.fetchApi('/manager/queue/reset', { method: 'POST' }); infoToast('Cancel', 'Remaining tasks will stop after completing the current task.'); } }, @@ -1476,9 +1476,15 @@ export class CustomNodesManager { let needRestart = false; let errorMsg = ""; - await api.fetchApi('/manager/queue/reset'); + await api.fetchApi('/manager/queue/reset', { method: 'POST' }); + // Set install_context BEFORE per-item queue enqueue calls so that any + // server-side synchronous completion (e.g., sync enable of an inactive + // node) that emits cm-queue-status before we return here still finds + // install_context populated in onQueueCompleted. target_items is shared + // by reference so further pushes below remain visible. let target_items = []; + this.install_context = {btn: btn, targets: target_items}; for (const hash of list) { const item = this.grid.getRowItemBy("hash", hash); @@ -1550,8 +1556,6 @@ export class CustomNodesManager { } } - this.install_context = {btn: btn, targets: target_items}; - if(errorMsg) { this.showError(errorMsg); show_message("[Installation Errors]\n"+errorMsg); @@ -1563,7 +1567,7 @@ export class CustomNodesManager { } } else { - await api.fetchApi('/manager/queue/start'); + await api.fetchApi('/manager/queue/start', { method: 'POST' }); this.showStop(); showTerminal(); } @@ -1576,6 +1580,10 @@ export class CustomNodesManager { const item = self.grid.getRowItemBy("hash", hash); + if (!item) { + return; + } + item.restart = true; self.restartMap[item.hash] = true; self.grid.updateCell(item, "action"); @@ -1583,45 +1591,81 @@ export class CustomNodesManager { } else if(event.detail.status == 'done') { self.hideStop(); - self.onQueueCompleted(event.detail); + // Await + error logging so any unhandled rejection surfaces to the + // console instead of silently swallowing completion finalization + // (root cause of disable/enable button staying loading with no toast). + try { + await self.onQueueCompleted(event.detail); + } catch (e) { + console.error("[ComfyUI-Manager] onQueueCompleted failed:", e); + } } } async onQueueCompleted(info) { + // `nodepack_result` is a dict serialized from a Python dict, not an array. + // `dict.length` is `undefined` and `undefined == 0` is `false`, so the + // previous `result.length == 0` guard was a no-op; switch to a correct + // empty-check that also tolerates null/undefined. let result = info.nodepack_result; - if(result.length == 0) { + if (!result || Object.keys(result).length === 0) { return; } let self = CustomNodesManager.instance; - if(!self.install_context) { + if (!self || !self.install_context) { return; } const { target, label, mode } = self.install_context.btn; - target.classList.remove("cn-btn-loading"); + const targets = self.install_context.targets || []; + // Compute errorMsg upfront so the downstream user-visible finalization + // (showRestart / showMessage / infoToast) fires regardless of whether + // any DOM-touching step below throws. let errorMsg = ""; - - for(let hash in result){ + for (let hash in result) { let v = result[hash]; - - if(v != 'success' && v != 'skip') - errorMsg += v+'\n'; + if (v != 'success' && v != 'skip') { + errorMsg += v + '\n'; + } } - for(let k in self.install_context.targets) { - let item = self.install_context.targets[k]; - self.grid.updateCell(item, "action"); + // Defensive: `target` may be a detached DOM node (the in_progress + // handler's updateCell can re-render the row and replace the button + // element). classList.remove on a detached node is a no-op, but we + // still guard in case target was torn down entirely. + try { + if (target && target.classList) { + target.classList.remove("cn-btn-loading"); + } + } catch (e) { + console.warn("[ComfyUI-Manager] Failed to clear button loading state:", e); + } + + // Defensive: grid.updateCell can throw if the item was removed or the + // grid was re-rendered between in_progress and done. Do NOT let this + // loop abort the completion finalization below — that was the observed + // failure mode for disable/enable (no toast, no "restart required" + // message). + try { + for (let k in targets) { + let item = targets[k]; + if (item) { + self.grid.updateCell(item, "action"); + } + } + } catch (e) { + console.warn("[ComfyUI-Manager] Failed to refresh target cells after queue completion:", e); } if (errorMsg) { self.showError(errorMsg); - show_message("Installation Error:\n"+errorMsg); + show_message("Installation Error:\n" + errorMsg); } else { - self.showStatus(`${label} ${result.length} custom node(s) successfully`); + self.showStatus(`${label} ${Object.keys(result).length} custom node(s) successfully`); } self.showRestart(); diff --git a/js/model-manager.js b/js/model-manager.js index 0f0de552..d01c5399 100644 --- a/js/model-manager.js +++ b/js/model-manager.js @@ -170,7 +170,7 @@ export class ModelManager { ".cmm-manager-stop": { click: () => { - api.fetchApi('/manager/queue/reset'); + api.fetchApi('/manager/queue/reset', { method: 'POST' }); infoToast('Cancel', 'Remaining tasks will stop after completing the current task.'); } }, @@ -444,7 +444,7 @@ export class ModelManager { let needRefresh = false; let errorMsg = ""; - await api.fetchApi('/manager/queue/reset'); + await api.fetchApi('/manager/queue/reset', { method: 'POST' }); let target_items = []; @@ -503,7 +503,7 @@ export class ModelManager { } } else { - await api.fetchApi('/manager/queue/start'); + await api.fetchApi('/manager/queue/start', { method: 'POST' }); this.showStop(); showTerminal(); } diff --git a/js/snapshot.js b/js/snapshot.js index b5dc0817..ea6bad90 100644 --- a/js/snapshot.js +++ b/js/snapshot.js @@ -9,7 +9,7 @@ loadCss("./snapshot.css"); async function restore_snapshot(target) { if(SnapshotManager.instance) { try { - const response = await api.fetchApi(`/snapshot/restore?target=${target}`, { cache: "no-store" }); + const response = await api.fetchApi('/snapshot/restore', { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ target: target }), cache: "no-store" }); if(response.status == 403) { await handle403Response(response); @@ -37,7 +37,7 @@ async function restore_snapshot(target) { async function remove_snapshot(target) { if(SnapshotManager.instance) { try { - const response = await api.fetchApi(`/snapshot/remove?target=${target}`, { cache: "no-store" }); + const response = await api.fetchApi('/snapshot/remove', { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ target: target }), cache: "no-store" }); if(response.status == 403) { await handle403Response(response); @@ -63,7 +63,7 @@ async function remove_snapshot(target) { async function save_current_snapshot() { try { - const response = await api.fetchApi('/snapshot/save', { cache: "no-store" }); + const response = await api.fetchApi('/snapshot/save', { method: 'POST', cache: "no-store" }); app.ui.dialog.close(); return true; } diff --git a/openapi.yaml b/openapi.yaml index 0446259e..4a828a8d 100644 --- a/openapi.yaml +++ b/openapi.yaml @@ -191,11 +191,20 @@ paths: description: Mapping of node packages to node classes /customnode/fetch_updates: - get: + post: summary: Check for updates description: Fetches updates for custom nodes - parameters: - - $ref: '#/components/parameters/modeParam' + requestBody: + required: false + content: + application/json: + schema: + type: object + properties: + mode: + type: string + enum: [local, remote, default] + description: Source mode (e.g., "local", "remote") responses: '200': description: No updates available @@ -423,13 +432,22 @@ paths: # Queue Management Endpoints /manager/queue/update_all: - get: + post: summary: Update all custom nodes description: Queues update operations for all installed custom nodes security: - securityLevel: [] - parameters: - - $ref: '#/components/parameters/modeParam' + requestBody: + required: false + content: + application/json: + schema: + type: object + properties: + mode: + type: string + enum: [local, remote, default] + description: Source mode (e.g., "local", "remote") responses: '200': description: Update queued successfully @@ -439,7 +457,7 @@ paths: description: Security policy violation /manager/queue/reset: - get: + post: summary: Reset queue description: Resets the operation queue responses: @@ -479,7 +497,7 @@ paths: description: Target node not found or security issue /manager/queue/start: - get: + post: summary: Start queue processing description: Starts processing the operation queue responses: @@ -575,7 +593,7 @@ paths: description: Disable operation queued successfully /manager/queue/update_comfyui: - get: + post: summary: Update ComfyUI description: Queues an update operation for ComfyUI itself responses: @@ -621,13 +639,22 @@ paths: $ref: '#/components/schemas/SnapshotItem' /snapshot/remove: - get: + post: summary: Remove snapshot description: Removes a specified snapshot security: - securityLevel: [] - parameters: - - $ref: '#/components/parameters/targetParam' + requestBody: + required: true + content: + application/json: + schema: + type: object + required: [target] + properties: + target: + type: string + description: Target identifier responses: '200': description: Snapshot removed successfully @@ -637,13 +664,22 @@ paths: description: Security policy violation /snapshot/restore: - get: + post: summary: Restore snapshot description: Restores a specified snapshot security: - securityLevel: [] - parameters: - - $ref: '#/components/parameters/targetParam' + requestBody: + required: true + content: + application/json: + schema: + type: object + required: [target] + properties: + target: + type: string + description: Target identifier responses: '200': description: Snapshot restoration scheduled @@ -667,7 +703,7 @@ paths: description: Error creating snapshot /snapshot/save: - get: + post: summary: Save snapshot description: Saves the current system state as a new snapshot responses: @@ -699,15 +735,19 @@ paths: description: Error retrieving versions /comfyui_manager/comfyui_switch_version: - get: + post: summary: Switch ComfyUI version description: Switches to a specified ComfyUI version - parameters: - - name: ver - in: query - description: Target version - schema: - type: string + requestBody: + required: false + content: + application/json: + schema: + type: object + properties: + ver: + type: string + description: Target version responses: '200': description: Version switch successful @@ -715,7 +755,7 @@ paths: description: Error switching version /manager/reboot: - get: + post: summary: Reboot ComfyUI description: Restarts the ComfyUI server security: @@ -746,7 +786,32 @@ paths: text/plain: schema: type: string - + post: + summary: Set preview method + description: Sets the latent preview method (write-only; use GET to read) + requestBody: + required: true + content: + application/json: + schema: + type: object + required: [value] + properties: + value: + type: string + enum: [auto, latent2rgb, taesd, none] + description: New preview method + responses: + '200': + description: Setting updated + content: + text/plain: + schema: + type: string + '400': + description: Invalid value + + /manager/db_mode: get: summary: Get or set database mode @@ -766,7 +831,32 @@ paths: text/plain: schema: type: string - + post: + summary: Set database mode + description: Sets the database mode (write-only; use GET to read) + requestBody: + required: true + content: + application/json: + schema: + type: object + required: [value] + properties: + value: + type: string + enum: [channel, local, remote] + description: New database mode + responses: + '200': + description: Setting updated + content: + text/plain: + schema: + type: string + '400': + description: Invalid value + + /manager/policy/component: get: summary: Get or set component policy @@ -785,7 +875,29 @@ paths: text/plain: schema: type: string - + post: + summary: Set component policy + description: Sets the component policy (write-only; use GET to read) + requestBody: + required: true + content: + application/json: + schema: + type: object + required: [value] + properties: + value: + type: string + description: New component policy + responses: + '200': + description: Setting updated + content: + text/plain: + schema: + type: string + + /manager/policy/update: get: summary: Get or set update policy @@ -805,7 +917,32 @@ paths: text/plain: schema: type: string - + post: + summary: Set update policy + description: Sets the update policy (write-only; use GET to read) + requestBody: + required: true + content: + application/json: + schema: + type: object + required: [value] + properties: + value: + type: string + enum: [stable, nightly, nightly-comfyui] + description: New update policy + responses: + '200': + description: Setting updated + content: + text/plain: + schema: + type: string + '400': + description: Invalid value + + /manager/channel_url_list: get: summary: Get or set channel URL @@ -836,7 +973,29 @@ paths: type: string url: type: string - + post: + summary: Set channel URL + description: Sets the channel URL for custom node sources (write-only; use GET to read current + list) + requestBody: + required: true + content: + application/json: + schema: + type: object + required: [value] + properties: + value: + type: string + description: New channel name + responses: + '200': + description: Setting updated + content: + text/plain: + schema: + type: string + + # Component Management Endpoints /manager/component/save: post: diff --git a/pyproject.toml b/pyproject.toml index b4c1d244..8a154cef 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,7 +1,7 @@ [project] name = "comfyui-manager" description = "ComfyUI-Manager provides features to install and manage custom nodes for ComfyUI, as well as various functionalities to assist with ComfyUI." -version = "3.39.3" +version = "3.40" license = { file = "LICENSE.txt" } dependencies = ["GitPython", "PyGithub", "matrix-nio", "transformers", "huggingface-hub>0.20", "typer", "rich", "typing-extensions", "toml", "uv", "chardet"] diff --git a/tests/test_csrf_content_type_helper.py b/tests/test_csrf_content_type_helper.py new file mode 100644 index 00000000..e1467475 --- /dev/null +++ b/tests/test_csrf_content_type_helper.py @@ -0,0 +1,121 @@ +"""AC verification for wi-001 (B2): Content-Type rejection helper. + +Validates _reject_simple_form_content_type against the 5-item curl matrix +from the WI's acceptance criteria using aiohttp test client. The test +builds a minimal aiohttp app that mirrors the helper's wiring into a +no-body POST handler, so we exercise the real request.content_type +parsing path rather than a mock. + +AC matrix: + form-url → 400 + multipart → 400 + text/plain → 400 + no-CT → 200 + application/json → 200 +""" +import asyncio +import unittest +from pathlib import Path + +from aiohttp import web +from aiohttp.test_utils import TestClient, TestServer + + +# Parse the helper from manager_server.py without importing it, to avoid +# pulling in the full ComfyUI/PromptServer stack. Note: we intentionally do +# NOT add the `glob/` directory to sys.path — the dir name would shadow +# Python's stdlib `glob` module and break pytest collection. +REPO_ROOT = Path(__file__).resolve().parent.parent + + +def _load_helper(): + """Parse manager_server.py and execute only the helper definition.""" + import ast + + source = (REPO_ROOT / "glob" / "manager_server.py").read_text() + tree = ast.parse(source) + wanted = {"_SIMPLE_FORM_CONTENT_TYPES", "_reject_simple_form_content_type"} + nodes = [] + for node in tree.body: + if isinstance(node, ast.Assign): + for target in node.targets: + if isinstance(target, ast.Name) and target.id in wanted: + nodes.append(node) + elif isinstance(node, ast.FunctionDef) and node.name in wanted: + nodes.append(node) + module = ast.Module(body=nodes, type_ignores=[]) + ns = {"web": web, "frozenset": frozenset} + exec(compile(module, "manager_server_helpers", "exec"), ns) + return ns["_reject_simple_form_content_type"] + + +_reject_simple_form_content_type = _load_helper() + + +async def _handler(request): + resp = _reject_simple_form_content_type(request) + if resp is not None: + return resp + return web.Response(status=200) + + +class ContentTypeRejectionTest(unittest.TestCase): + @classmethod + def setUpClass(cls): + cls.loop = asyncio.new_event_loop() + asyncio.set_event_loop(cls.loop) + app = web.Application() + app.router.add_post("/noop", _handler) + cls.server = TestServer(app, loop=cls.loop) + cls.client = TestClient(cls.server, loop=cls.loop) + cls.loop.run_until_complete(cls.client.start_server()) + + @classmethod + def tearDownClass(cls): + cls.loop.run_until_complete(cls.client.close()) + cls.loop.close() + + def _post(self, headers): + async def go(): + return await self.client.post("/noop", headers=headers, data=b"") + + return self.loop.run_until_complete(go()) + + def test_form_urlencoded_rejected(self): + r = self._post({"Content-Type": "application/x-www-form-urlencoded"}) + self.assertEqual(r.status, 400) + + def test_multipart_form_data_rejected(self): + # aiohttp requires a boundary for multipart; helper should still reject + # based on the primary mimetype. + r = self._post({"Content-Type": "multipart/form-data; boundary=xyz"}) + self.assertEqual(r.status, 400) + + def test_text_plain_rejected(self): + r = self._post({"Content-Type": "text/plain"}) + self.assertEqual(r.status, 400) + + def test_no_content_type_allowed(self): + # Explicitly strip Content-Type: aiohttp client may add a default, + # so we use a raw request to ensure absence is tested. + async def go(): + import aiohttp + + async with aiohttp.ClientSession() as session: + async with session.post( + self.client.make_url("/noop"), + data=None, + skip_auto_headers=["Content-Type"], + ) as resp: + return resp.status + + status = self.loop.run_until_complete(go()) + self.assertEqual(status, 200) + + def test_application_json_allowed(self): + r = self._post({"Content-Type": "application/json"}) + self.assertEqual(r.status, 200) + + +if __name__ == "__main__": + unittest.main(verbosity=2)