Skip to content

Commit ac73d00

Browse files
committed
feat(security): add dedicated install flags decoupled from security_level
Gate 'install via git URL' and 'install via pip' with dedicated opt-in boolean flags (allow_git_url_install / allow_pip_install) in config.ini [default], fully replacing the security_level term on those surfaces (REPLACE, not AND — a strict level no longer denies when the flag is on; a weak level no longer allows when the flag is off). - glob/manager_server.py: pure predicate is_dedicated_install_allowed (flag AND loopback, request-time args.listen); REPLACE gates at /customnode/install/git_url and /customnode/install/pip; batch unknown-URL arm routes through the same full predicate at the risky position (loopback term is load-bearing — the middle entry gate has no network-position term; the entry gate itself stays in force); unknown-pip in batch stays unconditionally blocked; new SECURITY_MESSAGE_FLAG_* denial constants name the responsible flag; security_403_response gains flag_token (comfyui_outdated keeps precedence) - glob/manager_core.py: register both keys (read via get_bool default-false, write list, exception fallback); "true"-only truthy; restart-only activation - js/common.js: 403 dialog copy names the responsible flag at the two install call sites - README.md: security-policy docs for both flags (per-surface scope incl. the batch entry-gate qualifier, REPLACE decoupling, loopback bound, opt-in config snippet, default-deny + migration note); stale tier lists corrected against the actual gates - CHANGELOG.md: opt-in migration note + accepted residual risk (flags bypass the forced-strong outdated-ComfyUI hardening on loopback, opt-in only), decoupling claim qualified for the batch entry gate Tests: unit suite (predicate truth table, REPLACE litmus both directions, AST binding-proofs against live handlers, subprocess-isolated config contract) plus a real-server E2E suite that mounts the Manager-under-test via git worktree (exact-SHA pin, detached) against a real ComfyUI and exercises both flag surfaces and both arms — deny arms (403 + flag-naming body/log + no install artifact), git-URL allow arm (real clone), pip allow arm as a two-phase reservation oracle — with zero-residual self-clean. Module skips without E2E_COMFYUI_ROOT; unit suite unaffected. The manager-v4 branch ships the identical policy (shared invariants + config contract); this tree uses the degraded predicate 'flag AND loopback' (no personal_cloud-equivalent mode here).
1 parent 88a7c52 commit ac73d00

15 files changed

Lines changed: 2385 additions & 14 deletions

CHANGELOG.md

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
# Changelog
2+
3+
## Unreleased
4+
5+
### Security policy: dedicated install flags (`allow_git_url_install` / `allow_pip_install`)
6+
7+
Two new boolean keys in `config.ini` (`[default]` section), both defaulting to
8+
`false`, now govern the arbitrary-install surfaces:
9+
10+
| Flag | Governs |
11+
|------|---------|
12+
| `allow_git_url_install` | `POST /customnode/install/git_url` and the unknown-git-URL arm of `POST /manager/queue/install` (incl. reinstall delegation) — the entire install transaction, transitive dependency pip installs included. On the batch queue path the flag applies **in addition to** the queue's `security_level` entry gate (see below) |
13+
| `allow_pip_install` | `POST /customnode/install/pip` only |
14+
15+
These surfaces additionally require a **loopback listener** (`--listen` on a
16+
local IP); the flags never open a non-loopback deployment. On the two
17+
**direct** endpoints (`POST /customnode/install/git_url` and
18+
`POST /customnode/install/pip`), the flags fully **decouple** the surface
19+
from `security_level`: it no longer has any effect in either direction — a
20+
strict level cannot deny them when the flag is `true`, and a weak level
21+
cannot allow them when the flag is `false`. On the **batch queue path**
22+
(`POST /manager/queue/install`), the flag is **necessary but not
23+
sufficient**: it gates the unknown-git-URL arm at the risky position, while
24+
the queue's normal `security_level` entry gate (`middle`) remains in force —
25+
at `security_level = strong`, batch unknown-URL installs stay denied even
26+
with the flag set to `true`. `security_level` continues to govern every
27+
other gated endpoint unchanged. Only the case-insensitive string `true`
28+
enables a flag; a missing or malformed key reads as `false`.
29+
30+
#### Migration note (no auto-seed)
31+
32+
There is **no automatic migration** from `security_level`. Users who
33+
previously relied on `security_level = weak` (or `normal-`) to use
34+
install-via-git-URL / install-pip must now **opt in explicitly** by adding to
35+
`config.ini`:
36+
37+
```ini
38+
[default]
39+
allow_git_url_install = true
40+
allow_pip_install = true
41+
```
42+
43+
Changes take effect after a **restart** (no hot reload).
44+
45+
#### Residual-risk note — outdated ComfyUI behavior change
46+
47+
On outdated ComfyUI versions (no system-user API), the manager previously
48+
forced `security_level = strong`, which unconditionally denied the
49+
git-URL/pip install surfaces. After this change those surfaces are governed
50+
by the new flags instead: an operator who explicitly sets a flag to `true`
51+
on a **loopback** listener can now perform installs on outdated ComfyUI
52+
where the forced-strong policy previously denied them. This is an accepted,
53+
deliberate trade-off: it requires explicit operator opt-in, remains bounded
54+
to loopback listeners, and the flag-deny path on outdated ComfyUI still
55+
surfaces the `comfyui_outdated` notice. If you operate an outdated ComfyUI
56+
deployment, leave both flags at their default `false` and update ComfyUI.

README.md

Lines changed: 63 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -384,19 +384,79 @@ When you run the `scan.sh` script:
384384
* all feature is available
385385
386386
* `high` level risky features
387-
* `Install via git url`, `pip install`
388-
* Installation of custom nodes registered not in the `default channel`.
389-
* Fix custom nodes
387+
* Downloading models that are not in `.safetensors` format and not
388+
registered in the `default channel` model list
389+
* NOTE: `Install via git url`, `pip install`, and installation of custom nodes
390+
not registered in the `default channel` are **no longer governed by
391+
`security_level`** — they are governed by the dedicated install flags
392+
described below.
390393
391394
* `middle` level risky features
392395
* Uninstall/Update
393396
* Installation of custom nodes registered in the `default channel`.
397+
* Fix custom nodes
394398
* Restore/Remove Snapshot
395399
* Restart
396400
397401
* `low` level risky features
398402
* Update ComfyUI
399403
404+
### Dedicated install flags: `allow_git_url_install` / `allow_pip_install`
405+
406+
The two arbitrary-install surfaces are governed by dedicated boolean keys in
407+
`config.ini` (`[default]` section), fully **decoupled** from `security_level`:
408+
409+
* `allow_git_url_install`
410+
* governs `Install via Git URL` (`POST /customnode/install/git_url`) **and**
411+
the unknown-git-URL arm of the batch install queue
412+
(`POST /manager/queue/install`, including reinstall delegation) — i.e.
413+
installing any custom node from a git URL that is not registered in the
414+
`default channel` catalog
415+
* on the **batch queue path**, the flag is **necessary but not
416+
sufficient**: the queue's normal `security_level` entry gate (`middle`)
417+
must ALSO pass — at `security_level = strong`, batch unknown-URL
418+
installs stay denied even with the flag set to `true` (only the direct
419+
`Install via Git URL` endpoint is fully independent of `security_level`)
420+
* covers the **entire install transaction** it starts, including the
421+
pack's transitive dependency pip installs
422+
* `allow_pip_install`
423+
* governs **only** the standalone `pip install` feature
424+
(`POST /customnode/install/pip`)
425+
426+
Key properties:
427+
428+
* **Decoupled from `security_level` (replace, not and)** — on the two
429+
**direct endpoints** (`Install via Git URL` and `pip install`),
430+
`security_level` no longer has any effect in either direction: a strict
431+
level cannot deny them when the flag is `true`, and a weak level cannot
432+
allow them when the flag is `false`. (The batch queue path keeps its
433+
`security_level` entry gate in ADDITION to the flag — see the scope bullet
434+
above.) Every other gated feature remains governed by `security_level` as
435+
described above.
436+
* **Loopback only** — the flags take effect **only** when the server listens
437+
on a loopback address (e.g. `--listen 127.0.0.1`). On a non-loopback
438+
listener these surfaces stay denied regardless of the flags; the flags
439+
never widen the exposure of a public deployment.
440+
* **Default deny / explicit opt-in** — both flags default to `false`. Only
441+
the case-insensitive string `true` enables a flag; a missing or malformed
442+
key reads as `false`.
443+
444+
To opt in, edit `config.ini`:
445+
446+
```ini
447+
[default]
448+
allow_git_url_install = true
449+
allow_pip_install = true
450+
```
451+
452+
Changes take effect after a **restart** (no hot reload).
453+
454+
> **Migration note**: there is no automatic migration from `security_level`.
455+
> If you previously relied on `security_level = weak` (or `normal-`) to use
456+
> install-via-git-URL / pip install, you must opt in explicitly with the flags
457+
> above. See `CHANGELOG.md` for details, including a behavior note for
458+
> outdated ComfyUI deployments.
459+
400460

401461
# Disclaimer
402462

glob/manager_core.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1699,6 +1699,8 @@ def write_config():
16991699
'always_lazy_install': get_config()['always_lazy_install'],
17001700
'network_mode': get_config()['network_mode'],
17011701
'db_mode': get_config()['db_mode'],
1702+
'allow_git_url_install': get_config()['allow_git_url_install'],
1703+
'allow_pip_install': get_config()['allow_pip_install'],
17021704
}
17031705

17041706
# Sanitize all string values to prevent CRLF injection attacks
@@ -1745,6 +1747,8 @@ def get_bool(key, default_value):
17451747
'network_mode': default_conf.get('network_mode', 'public').lower(),
17461748
'security_level': default_conf.get('security_level', 'normal').lower(),
17471749
'db_mode': default_conf.get('db_mode', 'cache').lower(),
1750+
'allow_git_url_install': get_bool('allow_git_url_install', False),
1751+
'allow_pip_install': get_bool('allow_pip_install', False),
17481752
}
17491753
manager_migration.force_security_level_if_needed(result)
17501754
return result
@@ -1774,6 +1778,8 @@ def get_bool(key, default_value):
17741778
'network_mode': 'public', # public | private | offline
17751779
'security_level': 'normal', # strong | normal | normal- | weak
17761780
'db_mode': 'cache', # local | cache | remote
1781+
'allow_git_url_install': False,
1782+
'allow_pip_install': False,
17771783
}
17781784
manager_migration.force_security_level_if_needed(result)
17791785
return result

glob/manager_server.py

Lines changed: 42 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,8 @@
3535
SECURITY_MESSAGE_NORMAL_MINUS = "ERROR: To use this feature, you must either set '--listen' to a local IP and set the security level to 'normal-' or lower, or set the security level to 'middle' or 'weak'. Please contact the administrator.\nReference: https://github.com/ltdrdata/ComfyUI-Manager#security-policy"
3636
SECURITY_MESSAGE_GENERAL = "ERROR: This installation is not allowed in this security_level. Please contact the administrator.\nReference: https://github.com/ltdrdata/ComfyUI-Manager#security-policy"
3737
SECURITY_MESSAGE_NORMAL_MINUS_MODEL = "ERROR: Downloading models that are not in '.safetensors' format is only allowed for models registered in the 'default' channel at this security level. If you want to download this model, set the security level to 'normal-' or lower."
38+
SECURITY_MESSAGE_FLAG_GIT_URL = "ERROR: This action requires 'allow_git_url_install = true' in config.ini ([default] section). This setting is independent of security_level. Reference: https://github.com/ltdrdata/ComfyUI-Manager#security-policy"
39+
SECURITY_MESSAGE_FLAG_PIP = "ERROR: This action requires 'allow_pip_install = true' in config.ini ([default] section). This setting is independent of security_level. Reference: https://github.com/ltdrdata/ComfyUI-Manager#security-policy"
3840

3941
routes = PromptServer.instance.routes
4042

@@ -82,6 +84,19 @@ def is_loopback(address):
8284
except ValueError:
8385
return False
8486

87+
88+
def is_dedicated_install_allowed(flag_value: bool, listen_address: str) -> bool:
89+
"""P-direct predicate (adopter-degraded form): flag AND loopback.
90+
91+
Pure helper for the dedicated install flags
92+
(allow_git_url_install / allow_pip_install) — callers pass the
93+
flag value from their own config read and the listener address
94+
from the CLI arguments (request-time evaluation; the import-time
95+
snapshot above is NOT consulted).
96+
"""
97+
return bool(flag_value) and is_loopback(listen_address)
98+
99+
85100
is_local_mode = is_loopback(args.listen)
86101

87102

@@ -305,10 +320,18 @@ def setup_environment():
305320
import urllib.request
306321

307322

308-
def security_403_response():
309-
"""Return appropriate 403 response based on ComfyUI version."""
323+
def security_403_response(flag_token=None):
324+
"""Return appropriate 403 response based on ComfyUI version.
325+
326+
When `flag_token` is given (dedicated install flag denials), the
327+
body names the responsible flag instead of "security_level". The
328+
`comfyui_outdated` branch stays the FIRST check regardless, and
329+
no-arg callers keep today's body byte-identical.
330+
"""
310331
if not manager_migration.has_system_user_api():
311332
return web.json_response({"error": "comfyui_outdated"}, status=403)
333+
if flag_token is not None:
334+
return web.json_response({"error": flag_token}, status=403)
312335
return web.json_response({"error": "security_level"}, status=403)
313336

314337

@@ -1384,7 +1407,17 @@ async def install_custom_node(request):
13841407
else:
13851408
return web.Response(status=404, text=f"Following node pack doesn't provide `nightly` version: ${git_url}")
13861409

1387-
if not is_allowed_security_level(risky_level):
1410+
if risky_level == 'high':
1411+
# unknown-URL arm: governed by the dedicated flag predicate
1412+
# (flag AND loopback, evaluated at request time). The loopback
1413+
# term is load-bearing here — the 'middle' entry gate above has
1414+
# no network-position term.
1415+
if not is_dedicated_install_allowed(core.get_config()['allow_git_url_install'], args.listen):
1416+
logging.error(SECURITY_MESSAGE_FLAG_GIT_URL)
1417+
return web.Response(status=404, text="A security error has occurred. Please check the terminal logs")
1418+
elif not is_allowed_security_level(risky_level):
1419+
# 'block' arm stays an unconditional deny (is_allowed_security_level
1420+
# returns False for 'block'); 'middle'/'low' arms unchanged.
13881421
logging.error(SECURITY_MESSAGE_GENERAL)
13891422
return web.Response(status=404, text="A security error has occurred. Please check the terminal logs")
13901423

@@ -1441,9 +1474,9 @@ async def fix_custom_node(request):
14411474

14421475
@routes.post("/customnode/install/git_url")
14431476
async def install_custom_node_git_url(request):
1444-
if not is_allowed_security_level('high'):
1445-
logging.error(SECURITY_MESSAGE_NORMAL_MINUS)
1446-
return security_403_response()
1477+
if not is_dedicated_install_allowed(core.get_config()['allow_git_url_install'], args.listen):
1478+
logging.error(SECURITY_MESSAGE_FLAG_GIT_URL)
1479+
return security_403_response(flag_token='allow_git_url_install')
14471480

14481481
url = await request.text()
14491482
res = await core.gitclone_install(url)
@@ -1461,9 +1494,9 @@ async def install_custom_node_git_url(request):
14611494

14621495
@routes.post("/customnode/install/pip")
14631496
async def install_custom_node_pip(request):
1464-
if not is_allowed_security_level('high'):
1465-
logging.error(SECURITY_MESSAGE_NORMAL_MINUS)
1466-
return security_403_response()
1497+
if not is_dedicated_install_allowed(core.get_config()['allow_pip_install'], args.listen):
1498+
logging.error(SECURITY_MESSAGE_FLAG_PIP)
1499+
return security_403_response(flag_token='allow_pip_install')
14671500

14681501
packages = await request.text()
14691502
core.pip_install(packages.split(' '))

js/common.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -232,7 +232,7 @@ export async function install_pip(packages) {
232232
});
233233

234234
if(res.status == 403) {
235-
await handle403Response(res);
235+
await handle403Response(res, "To use this feature, set 'allow_pip_install = true' in config.ini ([default] section). This setting is independent of security_level.");
236236
return;
237237
}
238238

@@ -267,7 +267,7 @@ export async function install_via_git_url(url, manager_dialog) {
267267
});
268268

269269
if(res.status == 403) {
270-
await handle403Response(res);
270+
await handle403Response(res, "To use this feature, set 'allow_git_url_install = true' in config.ini ([default] section). This setting is independent of security_level.");
271271
return;
272272
}
273273

tests/conftest.py

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
"""Test-runner guard for the GOAL #32 tests/ modules.
2+
3+
WHY THIS FILE EXISTS (collection hazard, not test logic):
4+
5+
The repo root contains ``__init__.py`` — the ComfyUI plugin entrypoint —
6+
which at import time appends ``glob/`` to sys.path and imports
7+
``manager_server`` (which needs ``folder_paths`` / ``comfy.cli_args`` /
8+
a constructed ``PromptServer``). pytest 8 collects any ancestor
9+
directory that carries an ``__init__.py`` as a ``Package`` node and
10+
IMPORTS that ``__init__.py`` during test setup (observed module name:
11+
``__init__``). Outside a live ComfyUI process that import can never
12+
succeed, so EVERY test under tests/ errors at setup — including the
13+
pre-existing tests/test_csrf_content_type_helper.py — whenever pytest's
14+
rootdir ends up at or above the repo root (e.g. running inside a git
15+
worktree nested under the parent checkout).
16+
17+
The guard below pre-seeds ``sys.modules`` with an inert stub whose
18+
``__file__`` matches the real path, so pytest's
19+
``import_path(<repo-root>/__init__.py)`` resolves to the stub without
20+
executing the plugin entrypoint. Conftest files load before the setup
21+
phase, so the stub is always in place in time. This does NOT touch
22+
production code and does NOT alter what the tests import themselves
23+
(they use AST-extraction / subprocess isolation per the
24+
tests/test_csrf_content_type_helper.py precedent — ``glob/`` is never
25+
added to the runner's sys.path).
26+
"""
27+
import sys
28+
import types
29+
from pathlib import Path
30+
31+
_REPO_ROOT = Path(__file__).resolve().parent.parent
32+
_ROOT_INIT = _REPO_ROOT / "__init__.py"
33+
34+
if _ROOT_INIT.exists() and "__init__" not in sys.modules:
35+
_stub = types.ModuleType("__init__")
36+
_stub.__file__ = str(_ROOT_INIT)
37+
_stub.__doc__ = (
38+
"Inert stand-in for the ComfyUI-Manager plugin entrypoint; "
39+
"see tests/conftest.py for rationale."
40+
)
41+
sys.modules["__init__"] = _stub

0 commit comments

Comments
 (0)