[PR #3161] [MERGED] fix(ui): show the server's reason for a batch 404, not a false channel message (#3128) #3063

New issue

Closed

opened 2026-10-28 21:14:53 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-28 21:14:53 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/Comfy-Org/ComfyUI-Manager/pull/3161
Author: @ltdrdata
Created: 8/13/2026
Status: ✅ Merged
Merged: 8/15/2026
Merged by: @ltdrdata

Base: main ← Head: fix/issue-3128-c1

📝 Commits (2)

4fe65b0 fix(ui): surface the server's reason for a batch 404 (#3128)
98be899 harden(ui): escape untrusted text before the batch error dialog (#3128)

📊 Changes

2 files changed (+4 additions, -8 deletions)

View changed files

📝 js/common.js (+2 -4)
📝 js/custom-nodes-manager.js (+2 -4)

📄 Description

Summary

Fixes the primary defect of #3128 (section 1): on the batch install and uninstall paths the client discarded the server's 404 body and substituted a false lead —

With the current security level configuration, only custom nodes from the "default channel" can be installed/uninstalled.

That lead is wrong. The install 404 comes from either the allow_git_url_install gate (the risky_level == 'high' arm, which routes through is_dedicated_install_allowed() and never reads security_level) or from a node pack that has no nightly version — neither of which a channel or security-level change can resolve. The uninstall endpoint does not return 404 at all, so there the message described a gate the request never reached.

The fix drops the 404 branch at both sites (js/custom-nodes-manager.js, js/common.js) so it falls through to the existing server-text else path. The security 404 then points the user at the terminal logs — where the denial message corrected in #3157 is printed — and a non-security 404 states its own reason. No new message is invented and no server code is touched.

Relationship to #3157

#3157 fixed #3128 section 2 (the dedicated-install denial messages omitted the loopback half). This PR fixes section 1 — the issue's title defect — which #3157 left unaddressed. With both merged, #3128 is fully resolved.

Verification

Drove the real shipped modules from both trees under node --preserve-symlinks, stubbing only the network (api.fetchApi) and DOM, capturing app.ui.dialog.show() — the surface the user actually reads:

Before (base): both paths render the false default channel line.
After (this PR): both render A security error has occurred. Please check the terminal logs; the install path's other 404 body (doesn't provide nightly version) also surfaces correctly.

Scope

Client-only, 2 files, +13 −4. No server change.

Reported in #3128 (section 1) by @plz12345, with this fix suggested there.

_{🔄 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/3161 **Author:** [@ltdrdata](https://github.com/ltdrdata) **Created:** 8/13/2026 **Status:** ✅ Merged **Merged:** 8/15/2026 **Merged by:** [@ltdrdata](https://github.com/ltdrdata) **Base:** `main` ← **Head:** `fix/issue-3128-c1` --- ### 📝 Commits (2) - [`4fe65b0`](https://github.com/Comfy-Org/ComfyUI-Manager/commit/4fe65b088e8c0042924a10a3a3afd289eed27a2e) fix(ui): surface the server's reason for a batch 404 (#3128) - [`98be899`](https://github.com/Comfy-Org/ComfyUI-Manager/commit/98be899011f0c6b9626a8f2bdb16122b7bdc9748) harden(ui): escape untrusted text before the batch error dialog (#3128) ### 📊 Changes **2 files changed** (+4 additions, -8 deletions) <details> <summary>View changed files</summary> 📝 `js/common.js` (+2 -4) 📝 `js/custom-nodes-manager.js` (+2 -4) </details> ### 📄 Description ## Summary Fixes the primary defect of #3128 (section 1): on the **batch install and uninstall** paths the client discarded the server's `404` body and substituted a false lead — > With the current security level configuration, only custom nodes from the **"default channel"** can be installed/uninstalled. That lead is wrong. The install `404` comes from either the `allow_git_url_install` gate (the `risky_level == 'high'` arm, which routes through `is_dedicated_install_allowed()` and never reads `security_level`) or from a node pack that has no `nightly` version — neither of which a channel or security-level change can resolve. The uninstall endpoint does not return `404` at all, so there the message described a gate the request never reached. The fix drops the `404` branch at both sites (`js/custom-nodes-manager.js`, `js/common.js`) so it falls through to the **existing** server-text `else` path. The security `404` then points the user at the terminal logs — where the denial message corrected in #3157 is printed — and a non-security `404` states its own reason. No new message is invented and no server code is touched. ## Relationship to #3157 #3157 fixed #3128 **section 2** (the dedicated-install denial messages omitted the loopback half). This PR fixes **section 1** — the issue's title defect — which #3157 left unaddressed. With both merged, #3128 is fully resolved. ## Verification Drove the real shipped modules from both trees under `node --preserve-symlinks`, stubbing only the network (`api.fetchApi`) and DOM, capturing `app.ui.dialog.show()` — the surface the user actually reads: - **Before (base):** both paths render the false `default channel` line. - **After (this PR):** both render `A security error has occurred. Please check the terminal logs`; the install path's other `404` body (`doesn't provide nightly version`) also surfaces correctly. ## Scope Client-only, 2 files, `+13 −4`. No server change. Reported in #3128 (section 1) by @plz12345, with this fix suggested there. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-28 21:14:53 +00:00