diff --git a/README.md b/README.md index 3f5795b32..4a3f88005 100644 --- a/README.md +++ b/README.md @@ -216,6 +216,7 @@ The following settings are applied based on the section marked as `is_default`. security_level = strong|normal|normal-|weak> allow_git_url_install = allow_pip_install = + allow_flagged_nodepack_install = always_lazy_install = network_mode = public|private|offline|personal_cloud> ``` @@ -328,8 +329,8 @@ The security settings are applied based on whether the ComfyUI server's listener |-------------|---------------------------------------------------------------------------------------------------------------------------------------| | high+ | * **Switch ComfyUI version**
* **Fix nodepack** | | high | _(no features at this tier — `Fix nodepack` promoted to `high+` to align the enforcement gate with the `SECURITY_MESSAGE_HIGH_P` log text)_ | -| middle+ | * Uninstall/Update
* Installation of nodepack registered in the `default channel`.
* Restore/Remove Snapshot
* Install model | -| middle | * Restart | +| middle+ | * Uninstall/Update
* Installation of Git/nightly nodepacks registered in the `default channel`.
* Restore/Remove Snapshot
* Install model | +| middle | * Restart
* Installation of CNR release versions (subject to the flagged-version policy below) | | low | * Update ComfyUI | * **Note**: `Install via git url` and `pip install` are no longer gated by `security_level` — they moved to the dedicated flags `allow_git_url_install` / `allow_pip_install`. Installation of a nodepack registered not in the `default channel` likewise requires `allow_git_url_install` (in addition to the `middle+` level preconditions) instead of a `high+` security level. See the [Dedicated install flags](#dedicated-install-flags-allow_git_url_install--allow_pip_install) subsection below. @@ -359,6 +360,55 @@ The `Install via git url` and `pip install` features are governed by two dedicat * Changes to these flags require a restart of ComfyUI to take effect. * Migration note: if you previously relied on `security_level = weak` or `normal-` to use these features, you must now opt in explicitly by setting the flags in the `[default]` section of `config.ini`. The flags are not auto-seeded from your `security_level`. +### Flagged CNR nodepacks + +Before downloading a CNR release, Manager checks the selected version's `status` +in the existing `/nodes/{id}/install` response. No additional Registry request is +needed. The check applies to both the default and legacy Manager implementations, +including CNR version changes, updates, and snapshot restores. Legacy batch +reinstallation checks the target before removing the existing nodepack, and a +policy refusal preserves the current installation. For flagged-policy denials, +the UI reports that the current security configuration does not allow the action +and directs users to the terminal. Only the terminal provides loopback-only +`--listen` examples, the private network override setting, and restart instructions. + +* Active versions continue through the normal install flow. CNR release installs + use the `middle` security level, so they are available on non-local listeners + with `security_level = normal`, `normal-`, or `weak`. `strong` still denies + these install requests. +* Flagged versions (`NodeVersionStatusFlagged`) must satisfy the same installation + conditions as Active versions. In addition, **all** addresses in `--listen` + must resolve to loopback, such as `127.0.0.1` and `::1`, unless the explicit + override below is enabled. +* To allow flagged versions on a non-local listener in a **trusted private + network**, an administrator can explicitly opt in: + + ```ini + [default] + allow_flagged_nodepack_install = true + ``` + +* The option defaults to `false`. Only `true` (case-insensitive) enables it; + missing or invalid values are treated as `false`. Restart ComfyUI after editing it. +* With this option enabled, the flagged-version check allows installation + regardless of the listen address. Manager does not automatically detect a + private network. This option does not require `network_mode = private`, and + `network_mode = personal_cloud` alone does not allow flagged versions. +* CNR API refusals, including banned or unavailable packages, still prevent + installation. This option does not override `security_level = strong` or + the separate Git/nightly and pip installation policies. +* Enabling an already installed version does not query the Registry or reinstall it. +* Older deferred switches without a stored status require loopback-only listeners + or the override. Otherwise they are rejected without changing the installed pack; + request the installation again so Manager can check its current Registry status. +* Newly scheduled deferred version switches retain the returned version status + and check it against the current listen address and option again at restart. + This uses the saved status; it does not query CNR again for later status changes. + Server-scheduled snapshot restores inherit the permission computed from the + server's current listener and setting. +* Standalone `cm-cli` commands remain local administrator operations and do not + use the server listener policy. + # Disclaimer diff --git a/comfyui_manager/common/cnr_utils.py b/comfyui_manager/common/cnr_utils.py index 4112e60e0..7e62fcfb7 100644 --- a/comfyui_manager/common/cnr_utils.py +++ b/comfyui_manager/common/cnr_utils.py @@ -134,6 +134,7 @@ class NodeVersion: id: str version: str download_url: str + status: str = '' def map_node_version(api_node_version): @@ -165,6 +166,7 @@ def map_node_version(api_node_version): download_url=api_node_version.get( "downloadUrl", "" ), # Provide a default value if 'downloadUrl' is missing + status=api_node_version.get('status', ''), ) @@ -257,4 +259,3 @@ def read_cnr_id(fullpath): pass return None - diff --git a/comfyui_manager/common/manager_security.py b/comfyui_manager/common/manager_security.py index a0c1b1d4a..a6e49420d 100644 --- a/comfyui_manager/common/manager_security.py +++ b/comfyui_manager/common/manager_security.py @@ -27,6 +27,22 @@ from aiohttp import web is_personal_cloud_mode = False handler_policy = {} +FLAGGED_NODEPACK_INSTALL_ERROR = ( + 'This action is not allowed by the current security configuration. ' + 'See the terminal for details.' +) +FLAGGED_NODEPACK_INSTALL_GUIDANCE = ( + 'Installation of this flagged CNR version is blocked. To satisfy the additional ' + 'flagged-version requirement, choose one of the following:\n' + '1. Run ComfyUI with loopback-only listeners, for example --listen 127.0.0.1 ' + 'or --listen ::1. All configured listen addresses must be loopback.\n' + ' Do not use bare --listen or any non-loopback address, such as 0.0.0.0 or ::.\n' + '2. For a trusted private network, set allow_flagged_nodepack_install = true ' + 'in the [default] section of ComfyUI-Manager\'s config.ini.\n' + 'Restart ComfyUI after changing the listener or configuration. ' + 'All other installation security checks still apply.' +) + # 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 @@ -83,6 +99,32 @@ def is_loopback(address): return False +def is_loopback_listener(listen_address: str) -> bool: + """All addresses bound by --listen must resolve exclusively to loopback.""" + import socket + + if not isinstance(listen_address, str) or not listen_address: + return False + for address in listen_address.split(','): + address = address.strip() + if not address: + return False + if is_loopback(address): + continue + try: + resolved = socket.getaddrinfo(address, None, type=socket.SOCK_STREAM) + except OSError: + return False + if not resolved or any(not is_loopback(item[4][0]) for item in resolved): + return False + return True + + +def is_cnr_install_allowed(status: str, allow_flagged: bool, listen_address: str) -> bool: + """Only flagged versions require loopback or the private-network opt-in.""" + return status != 'NodeVersionStatusFlagged' or bool(allow_flagged) or is_loopback_listener(listen_address) + + def is_dedicated_install_allowed(flag_value: bool, listen_address: str, network_mode: str) -> bool: """P-direct predicate for the dedicated install flags (goal265-spec.md §1.2). diff --git a/comfyui_manager/glob/manager_core.py b/comfyui_manager/glob/manager_core.py index 8f1b84d93..630963344 100644 --- a/comfyui_manager/glob/manager_core.py +++ b/comfyui_manager/glob/manager_core.py @@ -38,6 +38,7 @@ from ..common import cm_global from ..common.config_writer import DirtyTrackingConfig, write_config_merged from ..common import cnr_utils from ..common import manager_util +from ..common import manager_security from ..common import git_utils from ..common import manager_downloader from ..common.node_package import InstalledNodePackage @@ -881,10 +882,10 @@ class UnifiedManager: return res - def reserve_cnr_switch(self, target, zip_url, from_path, to_path, no_deps): + def reserve_cnr_switch(self, target, zip_url, from_path, to_path, no_deps, status=''): script_path = os.path.join(context.manager_startup_script_path, "install-scripts.txt") with open(script_path, "a") as file: - obj = [target, "#LAZY-CNR-SWITCH-SCRIPT", zip_url, from_path, to_path, no_deps, get_default_custom_nodes_path(), sys.executable] + obj = [target, "#LAZY-CNR-SWITCH-SCRIPT", zip_url, from_path, to_path, no_deps, get_default_custom_nodes_path(), sys.executable, status] file.write(f"{obj}\n") print(f"Installation reserved: {target}") @@ -910,35 +911,45 @@ class UnifiedManager: return result + def _get_cnr_install_info(self, node_id, version_spec): + node_info = cnr_utils.install_node(node_id, version_spec) + if node_info is not None and node_info.status == 'NodeVersionStatusFlagged' and not manager_funcs.is_flagged_install_allowed(): + logging.error(manager_security.FLAGGED_NODEPACK_INSTALL_GUIDANCE) + raise PermissionError(manager_security.FLAGGED_NODEPACK_INSTALL_ERROR) + return node_info + def cnr_switch_version(self, node_id, version_spec=None, instant_execution=False, no_deps=False, return_postinstall=False): - if instant_execution: - return self.cnr_switch_version_instant(node_id, version_spec, instant_execution, no_deps, return_postinstall) - else: - return self.cnr_switch_version_lazy(node_id, version_spec, no_deps, return_postinstall) - - def cnr_switch_version_lazy(self, node_id, version_spec=None, no_deps=False, return_postinstall=False): - """ - switch between cnr version (lazy mode) - """ - result = ManagedResult('switch-cnr') - node_info = cnr_utils.install_node(node_id, version_spec) + try: + node_info = self._get_cnr_install_info(node_id, version_spec) + except PermissionError as exc: + return result.fail(str(exc)) if node_info is None or not node_info.download_url: return result.fail(f'not available node: {node_id}@{version_spec}') - version_spec = node_info.version + return self._cnr_switch_version(node_id, node_info, instant_execution, no_deps, return_postinstall) - if self.active_nodes[node_id][0] == version_spec: + def _cnr_switch_version(self, node_id, node_info, instant_execution=False, no_deps=False, return_postinstall=False): + """Execute a switch using the checked CNR response.""" + if self.active_nodes[node_id][0] == node_info.version: return ManagedResult('skip').with_msg("Up to date") + if instant_execution: + return self._cnr_switch_version_instant(node_id, node_info, no_deps, return_postinstall) + return self._cnr_switch_version_lazy(node_id, node_info, no_deps, return_postinstall) + + def _cnr_switch_version_lazy(self, node_id, node_info, no_deps=False, return_postinstall=False): + """Reserve a switch using the validated install response.""" + result = ManagedResult('switch-cnr') + version_spec = node_info.version zip_url = node_info.download_url from_path = self.active_nodes[node_id][1] target = node_id to_path = os.path.join(get_default_custom_nodes_path(), target) def postinstall(): - return self.reserve_cnr_switch(target, zip_url, from_path, to_path, no_deps) + return self.reserve_cnr_switch(target, zip_url, from_path, to_path, no_deps, node_info.status) if return_postinstall: return result.with_postinstall(postinstall) @@ -948,23 +959,12 @@ class UnifiedManager: return result - def cnr_switch_version_instant(self, node_id, version_spec=None, instant_execution=True, no_deps=False, return_postinstall=False): - """ - switch between cnr version - """ - - # 1. download + def _cnr_switch_version_instant(self, node_id, node_info, no_deps=False, return_postinstall=False): + """Apply a switch using the validated install response.""" result = ManagedResult('switch-cnr') - - node_info = cnr_utils.install_node(node_id, version_spec) - if node_info is None or not node_info.download_url: - return result.fail(f'not available node: {node_id}@{version_spec}') - version_spec = node_info.version - if self.active_nodes[node_id][0] == version_spec: - return ManagedResult('skip').with_msg("Up to date") - + # 1. download archive_name = f"CNR_temp_{str(uuid.uuid4())}.zip" # should be unpredictable name - security precaution download_path = os.path.join(get_default_custom_nodes_path(), archive_name) manager_downloader.basic_download_url(node_info.download_url, get_default_custom_nodes_path(), archive_name) @@ -1009,7 +1009,7 @@ class UnifiedManager: result.target = version_spec def postinstall(): - res = self.execute_install_script(f"{node_id}@{version_spec}", install_path, instant_execution=instant_execution, no_deps=no_deps) + res = self.execute_install_script(f"{node_id}@{version_spec}", install_path, instant_execution=True, no_deps=no_deps) return res if return_postinstall: @@ -1290,10 +1290,24 @@ class UnifiedManager: if 'comfyui-manager' in node_id.lower(): return result.fail(f"ignored: installing '{node_id}'") - node_info = cnr_utils.install_node(node_id, version_spec) + try: + node_info = self._get_cnr_install_info(node_id, version_spec) + except PermissionError as exc: + return result.fail(str(exc)) if node_info is None or not node_info.download_url: return result.fail(f'not available node: {node_id}@{version_spec}') + result = self._cnr_install(node_id, node_info, instant_execution, no_deps, return_postinstall) + # Keep the requested version in the public result. + if result.target is not None: + result.target = version_spec + return result + + def _cnr_install(self, node_id, node_info, instant_execution=False, no_deps=False, return_postinstall=False): + """Install using the checked CNR response.""" + result = ManagedResult('install-cnr') + version_spec = node_info.version + archive_name = f"CNR_temp_{str(uuid.uuid4())}.zip" # should be unpredictable name - security precaution download_path = os.path.join(get_default_custom_nodes_path(), archive_name) @@ -1539,6 +1553,14 @@ class UnifiedManager: return res.with_target(version_spec) + # Check before changing the existing pack, then reuse this response. + try: + node_info = self._get_cnr_install_info(node_id, version_spec) + except PermissionError as exc: + return ManagedResult('install-cnr').fail(str(exc)) + if node_info is None or not node_info.download_url: + return ManagedResult('install-cnr').fail(f'not available node: {node_id}@{version_spec}') + if self.is_enabled(node_id, 'nightly'): # disable nightly nodes self.unified_disable(node_id, False) # NOTE: don't return from here @@ -1550,12 +1572,12 @@ class UnifiedManager: if self.is_disabled(node_id, "cnr"): # enable and switch version if cnr is disabled (not specified version) self.unified_enable(node_id, "cnr") - return self.cnr_switch_version(node_id, version_spec, no_deps=no_deps, return_postinstall=return_postinstall) + return self._cnr_switch_version(node_id, node_info, instant_execution, no_deps, return_postinstall) if self.is_enabled(node_id, "cnr"): - return self.cnr_switch_version(node_id, version_spec, no_deps=no_deps, return_postinstall=return_postinstall) + return self._cnr_switch_version(node_id, node_info, instant_execution, no_deps, return_postinstall) - res = self.cnr_install(node_id, version_spec, instant_execution=instant_execution, no_deps=no_deps, return_postinstall=return_postinstall) + res = self._cnr_install(node_id, node_info, instant_execution, no_deps, return_postinstall) if res.result: self.active_nodes[node_id] = version_spec, res.to_path @@ -1674,6 +1696,10 @@ class ManagerFuncs: def __init__(self): pass + def is_flagged_install_allowed(self): + # Server-scheduled restore children inherit only the resolved permission. + return os.environ.get('_COMFYUI_MANAGER_CNR_ALLOW_FLAGGED', 'true') == 'true' + def run_script(self, cmd, cwd='.'): if len(cmd) > 0 and cmd[0].startswith("#"): print(f"[ComfyUI-Manager] Unexpected behavior: `{cmd}`") @@ -1707,6 +1733,7 @@ WRITTEN_CONFIG_KEYS = ( 'verbose', 'allow_git_url_install', 'allow_pip_install', + 'allow_flagged_nodepack_install', ) @@ -1751,6 +1778,7 @@ def read_config(): 'verbose': get_bool('verbose', False), 'allow_git_url_install': get_bool('allow_git_url_install', False), 'allow_pip_install': get_bool('allow_pip_install', False), + 'allow_flagged_nodepack_install': get_bool('allow_flagged_nodepack_install', False), } except Exception: @@ -1781,6 +1809,7 @@ def read_config(): 'verbose': False, 'allow_git_url_install': False, 'allow_pip_install': False, + 'allow_flagged_nodepack_install': False, } @@ -3209,15 +3238,20 @@ async def restore_snapshot(snapshot_path, git_helper_extras=None): skip_node_packs.append(f"{x[0]}@{x[1]}") elif not ps.result: failed.append(f"{x[0]}@{x[1]}") + logging.error(ps.msg) - # install listed cnr nodes + # Do not retry processed switches as fresh installs. + checked_cnr = {node_id for node_id, _ in todo_checkout} for k, v in cnr_info.items(): - if 'comfyui-manager' in k: + if 'comfyui-manager' in k or k in checked_cnr: continue ps = await unified_manager.install_by_id(k, version_spec=v, instant_execution=True, return_postinstall=True) if ps.action == 'install-cnr' and ps.result: installed_node_packs.append(f"{k}@{v}") + elif not ps.result: + failed.append(f"{k}@{v}") + logging.error(ps.msg) if ps is not None and ps.result: if hasattr(ps, 'postinstall'): diff --git a/comfyui_manager/glob/manager_server.py b/comfyui_manager/glob/manager_server.py index 93121901c..d6bda32ca 100644 --- a/comfyui_manager/glob/manager_server.py +++ b/comfyui_manager/glob/manager_server.py @@ -44,7 +44,7 @@ from comfyui_manager.glob.utils import ( from server import PromptServer from . import manager_core as core -from ..common import manager_util +from ..common import manager_util, manager_security from ..common import cm_global from ..common import manager_downloader from ..common import context @@ -133,6 +133,9 @@ def error_response( class ManagerFuncsInComfyUI(core.ManagerFuncs): + def is_flagged_install_allowed(self): + return core.get_config()['allow_flagged_nodepack_install'] or manager_security.is_loopback_listener(args.listen) + def run_script(self, cmd, cwd="."): if len(cmd) > 0 and cmd[0].startswith("#"): logging.error(f"[ComfyUI-Manager] Unexpected behavior: `{cmd}`") @@ -815,10 +818,6 @@ async def task_worker(): await core.unified_manager.reload(ManagerDatabaseSource.cache.value) async def do_install(params: InstallPackParams) -> str: - if not security_utils.is_allowed_security_level('middle+'): - logging.error(SECURITY_MESSAGE_MIDDLE_P) - return OperationResult.failed.value - node_id = params.id node_version = params.selected_version channel = params.channel @@ -844,6 +843,11 @@ async def task_worker(): return f"Cannot resolve install target: '{node_id}@{node_version}'" node_name, version_spec, is_specified = node_spec + # CNR versions have their own flagged-status check before download. + level = 'middle+' if version_spec in ('nightly', 'unknown') else 'middle' + if not security_utils.is_allowed_security_level(level): + logging.error(SECURITY_MESSAGE_MIDDLE_P if level == 'middle+' else SECURITY_MESSAGE_MIDDLE) + return OperationResult.failed.value res = await core.unified_manager.install_by_id( node_name, version_spec, @@ -915,7 +919,7 @@ async def task_worker(): base_res["msg"] = OperationResult.success.value return base_res - base_res["msg"] = f"An error occurred while updating '{node_name}'." + base_res["msg"] = res.msg or f"An error occurred while updating '{node_name}'." logging.error( f"\nERROR: An error occurred while updating '{node_name}'. (res.result={res.result}, res.action={res.action})" ) @@ -2176,4 +2180,3 @@ threading.Thread(target=lambda: asyncio.run(default_cache_update())).start() if not os.path.exists(context.manager_config_path): core.get_config() core.write_config() - diff --git a/comfyui_manager/legacy/manager_core.py b/comfyui_manager/legacy/manager_core.py index 17938bf45..05ffdc7ea 100644 --- a/comfyui_manager/legacy/manager_core.py +++ b/comfyui_manager/legacy/manager_core.py @@ -36,6 +36,7 @@ from ..common import cm_global from ..common.config_writer import DirtyTrackingConfig, write_config_merged from ..common import cnr_utils from ..common import manager_util +from ..common import manager_security from ..common import git_utils from ..common import manager_downloader from ..common.node_package import InstalledNodePackage @@ -867,10 +868,10 @@ class UnifiedManager: return res - def reserve_cnr_switch(self, target, zip_url, from_path, to_path, no_deps): + def reserve_cnr_switch(self, target, zip_url, from_path, to_path, no_deps, status=''): script_path = os.path.join(context.manager_startup_script_path, "install-scripts.txt") with open(script_path, "a") as file: - obj = [target, "#LAZY-CNR-SWITCH-SCRIPT", zip_url, from_path, to_path, no_deps, get_default_custom_nodes_path(), sys.executable] + obj = [target, "#LAZY-CNR-SWITCH-SCRIPT", zip_url, from_path, to_path, no_deps, get_default_custom_nodes_path(), sys.executable, status] file.write(f"{obj}\n") print(f"Installation reserved: {target}") @@ -896,35 +897,45 @@ class UnifiedManager: return result + def _get_cnr_install_info(self, node_id, version_spec): + node_info = cnr_utils.install_node(node_id, version_spec) + if node_info is not None and node_info.status == 'NodeVersionStatusFlagged' and not manager_funcs.is_flagged_install_allowed(): + logging.error(manager_security.FLAGGED_NODEPACK_INSTALL_GUIDANCE) + raise PermissionError(manager_security.FLAGGED_NODEPACK_INSTALL_ERROR) + return node_info + def cnr_switch_version(self, node_id, version_spec=None, instant_execution=False, no_deps=False, return_postinstall=False): - if instant_execution: - return self.cnr_switch_version_instant(node_id, version_spec, instant_execution, no_deps, return_postinstall) - else: - return self.cnr_switch_version_lazy(node_id, version_spec, no_deps, return_postinstall) - - def cnr_switch_version_lazy(self, node_id, version_spec=None, no_deps=False, return_postinstall=False): - """ - switch between cnr version (lazy mode) - """ - result = ManagedResult('switch-cnr') - node_info = cnr_utils.install_node(node_id, version_spec) + try: + node_info = self._get_cnr_install_info(node_id, version_spec) + except PermissionError as exc: + return result.fail(str(exc)) if node_info is None or not node_info.download_url: return result.fail(f'not available node: {node_id}@{version_spec}') - version_spec = node_info.version + return self._cnr_switch_version(node_id, node_info, instant_execution, no_deps, return_postinstall) - if self.active_nodes[node_id][0] == version_spec: + def _cnr_switch_version(self, node_id, node_info, instant_execution=False, no_deps=False, return_postinstall=False): + """Execute a switch using the checked CNR response.""" + if self.active_nodes[node_id][0] == node_info.version: return ManagedResult('skip').with_msg("Up to date") + if instant_execution: + return self._cnr_switch_version_instant(node_id, node_info, no_deps, return_postinstall) + return self._cnr_switch_version_lazy(node_id, node_info, no_deps, return_postinstall) + + def _cnr_switch_version_lazy(self, node_id, node_info, no_deps=False, return_postinstall=False): + """Reserve a switch using the validated install response.""" + result = ManagedResult('switch-cnr') + version_spec = node_info.version zip_url = node_info.download_url from_path = self.active_nodes[node_id][1] target = node_id to_path = os.path.join(get_default_custom_nodes_path(), target) def postinstall(): - return self.reserve_cnr_switch(target, zip_url, from_path, to_path, no_deps) + return self.reserve_cnr_switch(target, zip_url, from_path, to_path, no_deps, node_info.status) if return_postinstall: return result.with_postinstall(postinstall) @@ -934,23 +945,12 @@ class UnifiedManager: return result - def cnr_switch_version_instant(self, node_id, version_spec=None, instant_execution=True, no_deps=False, return_postinstall=False): - """ - switch between cnr version - """ - - # 1. download + def _cnr_switch_version_instant(self, node_id, node_info, no_deps=False, return_postinstall=False): + """Apply a switch using the validated install response.""" result = ManagedResult('switch-cnr') - - node_info = cnr_utils.install_node(node_id, version_spec) - if node_info is None or not node_info.download_url: - return result.fail(f'not available node: {node_id}@{version_spec}') - version_spec = node_info.version - if self.active_nodes[node_id][0] == version_spec: - return ManagedResult('skip').with_msg("Up to date") - + # 1. download archive_name = f"CNR_temp_{str(uuid.uuid4())}.zip" # should be unpredictable name - security precaution download_path = os.path.join(get_default_custom_nodes_path(), archive_name) manager_downloader.basic_download_url(node_info.download_url, get_default_custom_nodes_path(), archive_name) @@ -995,7 +995,7 @@ class UnifiedManager: result.target = version_spec def postinstall(): - res = self.execute_install_script(f"{node_id}@{version_spec}", install_path, instant_execution=instant_execution, no_deps=no_deps) + res = self.execute_install_script(f"{node_id}@{version_spec}", install_path, instant_execution=True, no_deps=no_deps) return res if return_postinstall: @@ -1277,10 +1277,24 @@ class UnifiedManager: if 'comfyui-manager' in node_id.lower(): return result.fail(f"ignored: installing '{node_id}'") - node_info = cnr_utils.install_node(node_id, version_spec) + try: + node_info = self._get_cnr_install_info(node_id, version_spec) + except PermissionError as exc: + return result.fail(str(exc)) if node_info is None or not node_info.download_url: return result.fail(f'not available node: {node_id}@{version_spec}') + result = self._cnr_install(node_id, node_info, instant_execution, no_deps, return_postinstall) + # Keep the requested version in the public result. + if result.target is not None: + result.target = version_spec + return result + + def _cnr_install(self, node_id, node_info, instant_execution=False, no_deps=False, return_postinstall=False): + """Install using the checked CNR response.""" + result = ManagedResult('install-cnr') + version_spec = node_info.version + archive_name = f"CNR_temp_{str(uuid.uuid4())}.zip" # should be unpredictable name - security precaution download_path = os.path.join(get_default_custom_nodes_path(), archive_name) @@ -1469,31 +1483,13 @@ class UnifiedManager: else: version_spec = self.resolve_unspecified_version(node_id) - if version_spec == 'unknown' or version_spec == 'nightly': + if version_spec in ('unknown', 'nightly'): try: - custom_nodes = await self.get_custom_nodes(channel, mode) - except InvalidChannel as e: - return ManagedResult('fail').fail(f'Invalid channel is used: {e.channel}') - - the_node = custom_nodes.get(node_id) - if the_node is not None: - if version_spec == 'unknown': - repo_url = the_node['files'][0] - else: # nightly - repo_url = the_node['repository'] - else: - # Fallback for nightly only: use repository URL from CNR map - # when node is registered in CNR but absent from nightly manifest - if version_spec == 'nightly': - cnr_fallback = self.cnr_map.get(node_id) - if cnr_fallback is not None and cnr_fallback.get('repository'): - repo_url = cnr_fallback['repository'] - else: - result = ManagedResult('install') - return result.fail(f"Node '{node_id}@{version_spec}' not found in [{channel}, {mode}]") - else: - result = ManagedResult('install') - return result.fail(f"Node '{node_id}@{version_spec}' not found in [{channel}, {mode}]") + repo_url = await self._get_git_install_url(node_id, version_spec, channel, mode) + except InvalidChannel as exc: + return ManagedResult('fail').fail(f'Invalid channel is used: {exc.channel}') + except LookupError as exc: + return ManagedResult('install').fail(str(exc)) if self.is_enabled(node_id, version_spec): return ManagedResult('skip').with_target(f"{node_id}@{version_spec}") @@ -1501,32 +1497,16 @@ class UnifiedManager: elif self.is_disabled(node_id, version_spec): return self.unified_enable(node_id, version_spec) - elif version_spec == 'unknown' or version_spec == 'nightly': - to_path = os.path.abspath(os.path.join(get_default_custom_nodes_path(), node_id)) + elif version_spec in ('unknown', 'nightly'): + return self._install_git(node_id, version_spec, repo_url, instant_execution, no_deps, return_postinstall) - if version_spec == 'nightly': - # disable cnr nodes - if self.is_enabled(node_id, 'cnr'): - self.unified_disable(node_id, False) - - # use `repo name` as a dir name instead of `cnr id` if system added nodepack (i.e. publisher is null) - cnr = self.cnr_map.get(node_id) - - if cnr is not None and cnr.get('publisher') is None: - repo_name = os.path.basename(git_utils.normalize_url(repo_url)) - to_path = os.path.abspath(os.path.join(get_default_custom_nodes_path(), repo_name)) - - res = self.repo_install(repo_url, to_path, instant_execution=instant_execution, no_deps=no_deps, return_postinstall=return_postinstall) - if res.result: - if version_spec == 'unknown': - self.unknown_active_nodes[node_id] = repo_url, to_path - elif version_spec == 'nightly': - cnr_utils.generate_cnr_id(to_path, node_id) - self.active_nodes[node_id] = 'nightly', to_path - else: - return res - - return res.with_target(version_spec) + # Check before changing the existing pack, then reuse this response. + try: + node_info = self._get_cnr_install_info(node_id, version_spec) + except PermissionError as exc: + return ManagedResult('install-cnr').fail(str(exc)) + if node_info is None or not node_info.download_url: + return ManagedResult('install-cnr').fail(f'not available node: {node_id}@{version_spec}') if self.is_enabled(node_id, 'nightly'): # disable nightly nodes @@ -1539,17 +1519,89 @@ class UnifiedManager: if self.is_disabled(node_id, "cnr"): # enable and switch version if cnr is disabled (not specified version) self.unified_enable(node_id, "cnr") - return self.cnr_switch_version(node_id, version_spec, no_deps=no_deps, return_postinstall=return_postinstall) + return self._cnr_switch_version(node_id, node_info, instant_execution, no_deps, return_postinstall) if self.is_enabled(node_id, "cnr"): - return self.cnr_switch_version(node_id, version_spec, no_deps=no_deps, return_postinstall=return_postinstall) + return self._cnr_switch_version(node_id, node_info, instant_execution, no_deps, return_postinstall) - res = self.cnr_install(node_id, version_spec, instant_execution=instant_execution, no_deps=no_deps, return_postinstall=return_postinstall) + res = self._cnr_install(node_id, node_info, instant_execution, no_deps, return_postinstall) if res.result: self.active_nodes[node_id] = version_spec, res.to_path return res + async def _get_git_install_url(self, node_id, version_spec, channel, mode): + custom_nodes = await self.get_custom_nodes(channel, mode) + node = custom_nodes.get(node_id) + if node is not None: + return node['files'][0] if version_spec == 'unknown' else node['repository'] + if version_spec == 'nightly': + cnr = self.cnr_map.get(node_id) + if cnr is not None and cnr.get('repository'): + return cnr['repository'] + raise LookupError(f"Node '{node_id}@{version_spec}' not found in [{channel}, {mode}]") + + def _install_git(self, node_id, version_spec, repo_url, instant_execution=False, no_deps=False, return_postinstall=False): + to_path = os.path.abspath(os.path.join(get_default_custom_nodes_path(), node_id)) + + if version_spec == 'nightly': + # disable cnr nodes + if self.is_enabled(node_id, 'cnr'): + self.unified_disable(node_id, False) + + # use `repo name` as a dir name instead of `cnr id` if system added nodepack (i.e. publisher is null) + cnr = self.cnr_map.get(node_id) + + if cnr is not None and cnr.get('publisher') is None: + repo_name = os.path.basename(git_utils.normalize_url(repo_url)) + to_path = os.path.abspath(os.path.join(get_default_custom_nodes_path(), repo_name)) + + res = self.repo_install(repo_url, to_path, instant_execution=instant_execution, no_deps=no_deps, return_postinstall=return_postinstall) + if res.result: + if version_spec == 'unknown': + self.unknown_active_nodes[node_id] = repo_url, to_path + elif version_spec == 'nightly': + cnr_utils.generate_cnr_id(to_path, node_id) + self.active_nodes[node_id] = 'nightly', to_path + else: + return res + + return res.with_target(version_spec) + + async def reinstall_by_id(self, node_id, version_spec, channel=None, mode=None): + if 'comfyui-manager' in node_id.lower(): + return ManagedResult('skip').fail(f"ignored: installing '{node_id}'") + + if version_spec in ('unknown', 'nightly'): + try: + repo_url = await self._get_git_install_url(node_id, version_spec, channel, mode) + except InvalidChannel as exc: + return ManagedResult('fail').fail(f'Invalid channel is used: {exc.channel}') + except LookupError as exc: + return ManagedResult('install').fail(str(exc)) + else: + try: + node_info = self._get_cnr_install_info(node_id, version_spec) + except PermissionError as exc: + return ManagedResult('install-cnr').fail(str(exc)) + if node_info is None or not node_info.download_url: + return ManagedResult('install-cnr').fail(f'not available node: {node_id}@{version_spec}') + + removed = self.unified_uninstall(node_id, version_spec == 'unknown') + if not removed.result: + return removed + + for item in removed.items: + path = item if version_spec == 'unknown' else item[1] + self.processed_install.discard(os.path.join(path, 'install.py')) + if version_spec in ('unknown', 'nightly'): + return self._install_git(node_id, version_spec, repo_url) + + result = self._cnr_install(node_id, node_info) + if result.result: + self.active_nodes[node_id] = node_info.version, result.to_path + return result + unified_manager = UnifiedManager() @@ -1660,6 +1712,10 @@ class ManagerFuncs: def __init__(self): pass + def is_flagged_install_allowed(self): + # Server-scheduled restore children inherit only the resolved permission. + return os.environ.get('_COMFYUI_MANAGER_CNR_ALLOW_FLAGGED', 'true') == 'true' + def run_script(self, cmd, cwd='.'): if len(cmd) > 0 and cmd[0].startswith("#"): print(f"[ComfyUI-Manager] Unexpected behavior: `{cmd}`") @@ -1691,6 +1747,7 @@ WRITTEN_CONFIG_KEYS = ( 'db_mode', 'allow_git_url_install', 'allow_pip_install', + 'allow_flagged_nodepack_install', ) @@ -1730,6 +1787,7 @@ def read_config(): 'db_mode': default_conf.get('db_mode', DBMode.CACHE.value).lower(), 'allow_git_url_install': get_bool('allow_git_url_install', False), 'allow_pip_install': get_bool('allow_pip_install', False), + 'allow_flagged_nodepack_install': get_bool('allow_flagged_nodepack_install', False), } except Exception: @@ -1755,6 +1813,7 @@ def read_config(): 'db_mode': DBMode.CACHE.value, 'allow_git_url_install': False, 'allow_pip_install': False, + 'allow_flagged_nodepack_install': False, } @@ -3192,15 +3251,20 @@ async def restore_snapshot(snapshot_path, git_helper_extras=None): skip_node_packs.append(f"{x[0]}@{x[1]}") elif not ps.result: failed.append(f"{x[0]}@{x[1]}") + logging.error(ps.msg) - # install listed cnr nodes + # Do not retry processed switches as fresh installs. + checked_cnr = {node_id for node_id, _ in todo_checkout} for k, v in cnr_info.items(): - if 'comfyui-manager' in k: + if 'comfyui-manager' in k or k in checked_cnr: continue ps = await unified_manager.install_by_id(k, version_spec=v, instant_execution=True, return_postinstall=True) if ps.action == 'install-cnr' and ps.result: installed_node_packs.append(f"{k}@{v}") + elif not ps.result: + failed.append(f"{k}@{v}") + logging.error(ps.msg) if ps is not None and ps.result: if hasattr(ps, 'postinstall'): diff --git a/comfyui_manager/legacy/manager_server.py b/comfyui_manager/legacy/manager_server.py index ee3a27b3f..07f84b8e0 100644 --- a/comfyui_manager/legacy/manager_server.py +++ b/comfyui_manager/legacy/manager_server.py @@ -162,6 +162,9 @@ async def get_risky_level(files, pip_packages): class ManagerFuncsInComfyUI(core.ManagerFuncs): + def is_flagged_install_allowed(self): + return core.get_config()['allow_flagged_nodepack_install'] or manager_security.is_loopback_listener(args.listen) + def run_script(self, cmd, cwd='.'): if len(cmd) > 0 and cmd[0].startswith("#"): logging.error(f"[ComfyUI-Manager] Unexpected behavior: `{cmd}`") @@ -468,7 +471,7 @@ async def task_worker(): await core.unified_manager.reload('cache') - async def do_install(item) -> str: + async def do_install(item, operation) -> str: ui_id, node_spec_str, channel, mode, skip_post_install = item try: @@ -478,7 +481,15 @@ async def task_worker(): return f"Cannot resolve install target: '{node_spec_str}'" node_name, version_spec, is_specified = node_spec - res = await core.unified_manager.install_by_id(node_name, version_spec, channel, mode, return_postinstall=skip_post_install) # discard post install if skip_post_install mode + # Check the resolved target, including requests with misleading metadata. + level = 'middle+' if version_spec in ('nightly', 'unknown') else 'middle' + if not is_allowed_security_level(level): + logging.error(SECURITY_MESSAGE_MIDDLE_P if level == 'middle+' else SECURITY_MESSAGE_MIDDLE) + return 'Installation blocked by security policy' + if operation == 'reinstall': + res = await core.unified_manager.reinstall_by_id(node_name, version_spec, channel, mode) + else: + res = await core.unified_manager.install_by_id(node_name, version_spec, channel, mode, return_postinstall=skip_post_install) if res.action not in ['skip', 'enable', 'install-git', 'install-cnr', 'switch-cnr']: logging.error(f"[ComfyUI-Manager] Installation failed:\n{res.msg}") @@ -530,7 +541,7 @@ async def task_worker(): base_res['msg'] = 'success' return base_res - base_res['msg'] = f"An error occurred while updating '{node_name}'." + base_res['msg'] = res.msg or f"An error occurred while updating '{node_name}'." logging.error(f"\nERROR: An error occurred while updating '{node_name}'. (res.result={res.result}, res.action={res.action})") return base_res except Exception: @@ -705,8 +716,8 @@ async def task_worker(): tasks_in_progress.add((kind, item[0])) try: - if kind == 'install': - msg = await do_install(item) + if kind in ('install', 'reinstall'): + msg = await do_install(item, kind) elif kind == 'enable': msg = await do_enable(item) elif kind == 'install-model': @@ -770,19 +781,9 @@ async def queue_batch(request): if k == 'update_all': await _update_all({'mode': v}) - elif k == 'reinstall': + elif k in ('install', 'reinstall'): for x in v: - res = await _uninstall_custom_node(x) - if res.status != 200: - failed.add(x['id']) - else: - res = await _install_custom_node(x) - if res.status != 200: - failed.add(x['id']) - - elif k == 'install': - for x in v: - res = await _install_custom_node(x) + res = await _queue_node_install(x, k) if res.status != 200: failed.add(x['id']) @@ -1459,15 +1460,15 @@ async def queue_count(request): 'is_processing': is_processing}) -async def _install_custom_node(json_data): - if not is_allowed_security_level('middle+'): - logging.error(SECURITY_MESSAGE_MIDDLE_P) +async def _queue_node_install(json_data, operation): + if not is_allowed_security_level('middle'): + logging.error(SECURITY_MESSAGE_MIDDLE) return web.Response(status=403, text="A security error has occurred. Please check the terminal logs") - # non-nightly cnr is safe + # CNR versions are checked for flagged status by the install worker. risky_level = None cnr_id = json_data.get('id') - skip_post_install = json_data.get('skip_post_install') + skip_post_install = False if operation == 'reinstall' else json_data.get('skip_post_install') git_url = None @@ -1509,10 +1510,11 @@ async def _install_custom_node(json_data): else: return web.Response(status=404, text=f"Following node pack doesn't provide `nightly` version: ${git_url}") - # goal265 S-C (middle+ entry gate above UNCHANGED): unknown git URL ('high+') - # -> dedicated-flag full predicate replaces the security_level check (spec §1.2); - # unknown pip ('block') -> unconditional deny via is_allowed_security_level (Q1). - # Flag-deny PRESERVES today's 404 response shape at this position (R1). + if risky_level != 'low' and not is_allowed_security_level('middle+'): + logging.error(SECURITY_MESSAGE_MIDDLE_P) + return web.Response(status=403, text="A security error has occurred. Please check the terminal logs") + + # Unknown Git installs also require the dedicated flag; preserve the 404 response. if risky_level == 'high+': if not _dedicated_install_allowed('allow_git_url_install'): logging.error(SECURITY_MESSAGE_FLAG_GIT_URL.format( @@ -1523,7 +1525,7 @@ async def _install_custom_node(json_data): return web.Response(status=404, text="A security error has occurred. Please check the terminal logs") install_item = json_data.get('ui_id'), node_spec_str, json_data['channel'], json_data['mode'], skip_post_install - temp_queue_batch.append(("install", install_item)) + temp_queue_batch.append((operation, install_item)) return web.Response(status=200) diff --git a/comfyui_manager/prestartup_script.py b/comfyui_manager/prestartup_script.py index 121a51940..d63b12abb 100644 --- a/comfyui_manager/prestartup_script.py +++ b/comfyui_manager/prestartup_script.py @@ -570,6 +570,13 @@ if os.path.exists(restore_snapshot_path): if 'COMFYUI_PATH' not in new_env: new_env['COMFYUI_PATH'] = os.path.dirname(folder_paths.__file__) + from comfy.cli_args import args + from .common import manager_security + + allow_flagged = default_conf.get('allow_flagged_nodepack_install', '').lower() == 'true' + new_env['_COMFYUI_MANAGER_CNR_ALLOW_FLAGGED'] = str( + allow_flagged or manager_security.is_loopback_listener(args.listen) + ).lower() cmd_str = [sys.executable, '-m', 'cm_cli', 'restore-snapshot', restore_snapshot_path] exit_code = process_wrap(cmd_str, custom_nodes_base_path, handler=msg_capture, env=new_env) @@ -619,9 +626,19 @@ def execute_lazy_install_script(repo_path, executable): process_wrap(install_cmd, repo_path, env=new_env) -def execute_lazy_cnr_switch(target, zip_url, from_path, to_path, no_deps, custom_nodes_path): +def execute_lazy_cnr_switch(target, zip_url, from_path, to_path, no_deps, custom_nodes_path, status=''): import uuid import shutil + from comfy.cli_args import args + from .common.manager_security import FLAGGED_NODEPACK_INSTALL_GUIDANCE, is_cnr_install_allowed + + allow_flagged = default_conf.get('allow_flagged_nodepack_install', '').lower() == 'true' + if not is_cnr_install_allowed(status or 'NodeVersionStatusFlagged', allow_flagged, args.listen): + if not status: + logging.error("Cannot execute reserved CNR switch for '%s': stored Registry status is missing. Request the installation again so its current status can be checked.", target) + else: + logging.error(FLAGGED_NODEPACK_INSTALL_GUIDANCE) + return False # 1. download archive_name = f"CNR_temp_{str(uuid.uuid4())}.zip" # should be unpredictable name - security precaution @@ -669,6 +686,8 @@ def execute_lazy_cnr_switch(target, zip_url, from_path, to_path, no_deps, custom with open(tracking_info_file, "w", encoding='utf-8') as file: file.write('\n'.join(list(extracted))) + return True + script_executed = False @@ -724,8 +743,9 @@ def execute_startup_script(): execute_lazy_install_script(script[0], script[2]) elif script[1] == "#LAZY-CNR-SWITCH-SCRIPT": - execute_lazy_cnr_switch(script[0], script[2], script[3], script[4], script[5], script[6]) - execute_lazy_install_script(script[3], script[7]) + status = script[8] if len(script) > 8 else '' + if execute_lazy_cnr_switch(script[0], script[2], script[3], script[4], script[5], script[6], status): + execute_lazy_install_script(script[3], script[7]) elif script[1] == "#LAZY-DELETE-NODEPACK": execute_lazy_delete(script[2]) diff --git a/tests/e2e/test_e2e_flagged_nodepacks.py b/tests/e2e/test_e2e_flagged_nodepacks.py new file mode 100644 index 000000000..3bf99127d --- /dev/null +++ b/tests/e2e/test_e2e_flagged_nodepacks.py @@ -0,0 +1,509 @@ +"""Real ComfyUI HTTP/queue/install tests with a local CNR service and harmless ZIPs. + +Run with E2E_ROOT pointing to the existing ComfyUI + venv fixture. Each server +uses an isolated base directory; no installed nodes or user config are changed. +""" + +import ast +import configparser +import io +import json +import os +import socket +import subprocess +import threading +import time +import uuid +import zipfile +from contextlib import contextmanager +from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer +from pathlib import Path +from types import SimpleNamespace +from urllib.parse import parse_qs, urlparse + +import pytest +import requests + + +E2E_ROOT = Path(os.environ.get('E2E_ROOT', '/nonexistent')) +REPO_ROOT = Path(__file__).resolve().parents[2] +pytestmark = pytest.mark.skipif( + not (E2E_ROOT / 'venv/bin/python').is_file(), reason='E2E_ROOT is required', +) +PACKS = ['fixture-active', 'fixture-flagged', 'fixture-pending', 'fixture-banned', + 'fixture-latest', 'fixture-switch', 'fixture-update', 'fixture-reinstall', 'fixture-snapshot'] + + +@pytest.fixture(scope='module') +def registry(): + calls = [] + + class Handler(BaseHTTPRequestHandler): + def log_message(self, *args): + pass + + def do_GET(self): + parsed = urlparse(self.path) + calls.append(parsed.path + ('?' + parsed.query if parsed.query else '')) + parts = parsed.path.strip('/').split('/') + version = parse_qs(parsed.query).get('version', ['2.0.0'])[0] + node = parts[1] if len(parts) > 1 else '' + base = f'http://127.0.0.1:{self.server.server_port}' + + def metadata(node, version): + status = {'1.0.0': 'Active', '1.1.0': 'Active', '2.0.0': 'Flagged', '3.0.0': 'Pending'}[version] + return {'id': f'{node}-{version}', 'node_id': node, 'version': version, + 'status': 'NodeVersionStatus' + status, 'dependencies': [], + 'downloadUrl': f'{base}/zip/{node}/{version}'} + + status = 200 + if parts[0] == 'zip': + node, version = parts[1:] + archive = io.BytesIO() + with zipfile.ZipFile(archive, 'w') as z: + z.writestr('__init__.py', 'NODE_CLASS_MAPPINGS = {}\n') + z.writestr('pyproject.toml', f'[project]\nname = "{node}"\nversion = "{version}"\n') + z.writestr('install.py', 'from pathlib import Path\n' + f'Path("installed.txt").write_text("{version}")\n') + body = archive.getvalue() + elif parsed.path == '/nodes': + body = json.dumps({'nodes': [ + {'id': name, 'name': name, 'description': '', 'status': 'NodeStatusActive', + 'repository': f'https://example.invalid/{name}', + 'publisher': {'id': 'fixture', 'name': 'Fixture'}, + 'latest_version': metadata(name, '2.0.0')} + for name in PACKS], 'totalPages': 1}).encode() + elif len(parts) == 3 and parts[0] == 'nodes' and parts[2] == 'install': + if node == 'fixture-banned': + status, body = 404, b'{"message":"Not found"}' + else: + body = json.dumps(metadata(node, version)).encode() + else: + status, body = 404, b'{}' + self.send_response(status) + self.send_header('Content-Length', str(len(body))) + self.send_header('Content-Type', 'application/zip' if parts[0] == 'zip' else 'application/json') + self.end_headers() + self.wfile.write(body) + + server = ThreadingHTTPServer(('127.0.0.1', 0), Handler) + thread = threading.Thread(target=server.serve_forever, daemon=True) + thread.start() + yield SimpleNamespace(url=f'http://127.0.0.1:{server.server_port}', calls=calls) + server.shutdown() + server.server_close() + thread.join() + + +REGISTRY_BOOTSTRAP = ''' +import json, os, urllib.request +from comfyui_manager.common import cnr_utils, manager_util +cnr_utils.base_url = os.environ['CM_E2E_REGISTRY'] +with urllib.request.urlopen(cnr_utils.base_url + '/nodes') as response: + manager_util.save_to_cache(cnr_utils.base_url + '/nodes', json.load(response)) +''' +BOOTSTRAP = ''' +import os, runpy, sys +sys.argv = [os.environ['CM_E2E_MAIN'], *sys.argv[1:]] +import comfy.options +comfy.options.enable_args_parsing() +''' + REGISTRY_BOOTSTRAP + "\nrunpy.run_path(sys.argv[0], run_name='__main__')\n" +RESTORE_BOOTSTRAP = ''' +import os +from comfy.cli_args import args +args.base_directory = os.environ['CM_E2E_BASE'] +''' + REGISTRY_BOOTSTRAP + ''' +from comfyui_manager.legacy import manager_core as core +core.unified_manager.custom_node_map_cache[(core.normalize_channel('default'), 'cache')] = core.NormalizedKeyDict() +''' + + +def prepare_environment(root, override, security, registry): + (root / 'custom_nodes').mkdir(exist_ok=True) + config_dir = root / 'user/__manager' + config_dir.mkdir(parents=True, exist_ok=True) + (config_dir / 'config.ini').write_text( + '[default]\nnetwork_mode = offline\nuse_uv = false\n' + f'security_level = {security}\nallow_flagged_nodepack_install = {override}\n' + ) + # The real restore subprocess inherits the same isolated Registry and paths. + (root / 'sitecustomize.py').write_text( + "import sys\nif sys.argv[0] == '-m' and 'cm_cli' in sys.orig_argv:\n" + + '\n'.join(' ' + line for line in RESTORE_BOOTSTRAP.splitlines()) + ) + env = dict(os.environ, PYTHONUNBUFFERED='1', + PYTHONPATH=os.pathsep.join([str(root), str(REPO_ROOT), str(E2E_ROOT / 'comfyui')]), + COMFYUI_PATH=str(E2E_ROOT / 'comfyui'), COMFYUI_FOLDERS_BASE_PATH=str(root), + CM_E2E_BASE=str(root), + CM_E2E_MAIN=str(E2E_ROOT / 'comfyui/main.py'), CM_E2E_REGISTRY=registry.url) + return env + + +@contextmanager +def running_server(root, tree, listen, override, security, registry): + env = prepare_environment(root, override, security, registry) + with socket.socket() as sock: + sock.bind(('127.0.0.1', 0)) + port = sock.getsockname()[1] + base = f'http://127.0.0.1:{port}' + command = [str(E2E_ROOT / 'venv/bin/python'), '-c', BOOTSTRAP, + '--cpu', '--enable-manager', '--base-directory', str(root), + '--listen', listen, '--port', str(port), + '--database-url', f'sqlite:///{root / "comfyui.db"}'] + if tree == 'legacy': + command.append('--enable-manager-legacy-ui') + log_path = root / 'server.log' + with log_path.open('w') as log: + process = subprocess.Popen(command, env=env, cwd=E2E_ROOT / 'comfyui', stdout=log, stderr=subprocess.STDOUT) + try: + deadline = time.monotonic() + 90 + while time.monotonic() < deadline: + assert process.poll() is None, log_path.read_text() + try: + if requests.get(base + '/system_stats', timeout=1).status_code == 200: + break + except requests.RequestException: + pass + time.sleep(.2) + else: + pytest.fail('ComfyUI did not start:\n' + log_path.read_text()) + yield SimpleNamespace(root=root, tree=tree, base=base, listen=listen, + override=override, security=security, registry=registry, + process=process, command=command, env=env) + finally: + process.terminate() + try: + process.wait(timeout=10) + except subprocess.TimeoutExpired: + process.kill() + process.wait(timeout=5) + + +@pytest.fixture(scope='module', params=[ + (tree, listen, override, security) + for tree in ('legacy', 'glob') + for listen, override, security in [ + ('127.0.0.1', False, 'normal'), + ('0.0.0.0', False, 'normal'), + ('0.0.0.0', True, 'normal'), + ('0.0.0.0', True, 'strong'), + ('127.0.0.1', False, 'strong'), + ] +], ids=lambda row: '-'.join(map(str, row))) +def comfy_server(request, tmp_path_factory, registry): + tree, listen, override, security = request.param + root = tmp_path_factory.mktemp('flagged-' + tree) + with running_server(root, tree, listen, override, security, registry) as server: + yield server + + +def install(server, node, version, operation='install'): + ui_id = uuid.uuid4().hex + params = {'id': node, 'version': '1.0.0', 'selected_version': version, + 'mode': 'cache', 'channel': 'default', 'ui_id': ui_id, + 'repository': 'https://example.invalid/fixture'} + if server.tree == 'legacy': + response = requests.post(server.base + '/v2/manager/queue/batch', + json={'batch_id': ui_id, operation: [params]}, timeout=10) + response.raise_for_status() + if response.json()['failed']: + return response.json() + query = {'id': ui_id} + else: + if operation == 'update': + params = {'node_name': node, 'node_ver': '1.0.0'} + response = requests.post(server.base + '/v2/manager/queue/task', json={ + 'ui_id': ui_id, 'client_id': 'flagged-e2e', 'kind': operation, 'params': params, + }, timeout=10) + response.raise_for_status() + requests.post(server.base + '/v2/manager/queue/start', timeout=10).raise_for_status() + query = {'ui_id': ui_id} + deadline = time.monotonic() + 30 + while time.monotonic() < deadline: + response = requests.get(server.base + '/v2/manager/queue/history', params=query, timeout=5) + if response.status_code == 200: + data = response.json() + if server.tree == 'legacy' or data.get('history'): + return data + time.sleep(.1) + pytest.fail('Install did not complete:\n' + (server.root / 'server.log').read_text()[-6000:]) + + +def assert_terminal_guidance(log): + for detail in ('--listen 127.0.0.1', '--listen ::1', '0.0.0.0', '[default]', + 'allow_flagged_nodepack_install = true', 'config.ini', + 'trusted private network', 'Restart ComfyUI'): + assert detail in log + + +def assert_flagged_denial(result, log): + message = json.dumps(result) + assert ('This action is not allowed by the current security configuration. ' + 'See the terminal for details.') in message, result + assert 'allow_flagged_nodepack_install' not in message, result + assert '--listen' not in message, result + assert_terminal_guidance(log) + + +@pytest.mark.parametrize('node,version', [ + ('fixture-active', '1.0.0'), ('fixture-flagged', '2.0.0'), + ('fixture-pending', '3.0.0'), ('fixture-banned', '1.0.0'), + ('fixture-latest', 'latest'), +]) +def test_install_policy(comfy_server, node, version): + server = comfy_server + log_before = len((server.root / 'server.log').read_text()) + before = len(server.registry.calls) + result = install(server, node, version) + calls = server.registry.calls[before:] + strong = server.security == 'strong' + flagged = version in ('2.0.0', 'latest') + allowed = not strong and node != 'fixture-banned' and ( + not flagged or server.override or server.listen == '127.0.0.1') + marker = server.root / 'custom_nodes' / node / 'installed.txt' + assert marker.exists() is allowed, (result, (server.root / 'server.log').read_text()[-6000:]) + assert sum('/install' in call for call in calls) == (0 if strong else 1), calls + assert any(call.startswith('/zip/') for call in calls) is allowed, calls + if allowed: + assert marker.read_text() == ('2.0.0' if version == 'latest' else version) + if server.tree == 'legacy': + assert list(result['nodepack_result'].values()) == ['success'], result + else: + assert result['history']['result'] == 'success', result + elif flagged and not strong: + assert_flagged_denial(result, (server.root / 'server.log').read_text()[log_before:]) + + +@pytest.mark.parametrize('operation', ['install', 'update']) +def test_flagged_transition_preserves_active_version(comfy_server, operation): + server = comfy_server + if server.security == 'strong': + pytest.skip('No installed version in strong mode') + # Keep this state separate from test_install_policy, regardless of test order. + node = 'fixture-switch' if operation == 'install' else 'fixture-update' + marker = server.root / 'custom_nodes' / node / 'installed.txt' + install(server, node, '1.0.0') + log_before = len((server.root / 'server.log').read_text()) + before = len(server.registry.calls) + result = install(server, node, '2.0.0', operation) + calls = server.registry.calls[before:] + assert sum('/install' in call for call in calls) == 1 + assert not any(call.startswith('/zip/') for call in calls) + assert marker.read_text() == '1.0.0' + reserved = server.root / 'user/__manager/startup-scripts/install-scripts.txt' + if server.override or server.listen == '127.0.0.1': + assert '#LAZY-CNR-SWITCH-SCRIPT' in reserved.read_text() + if server.tree == 'legacy': + assert list(result['nodepack_result'].values()) == ['success'], result + else: + assert result['history']['result'] == 'success', result + else: + assert not reserved.exists() + assert_flagged_denial(result, (server.root / 'server.log').read_text()[log_before:]) + + +@pytest.mark.parametrize('listen,override', [ + ('127.0.0.1', False), ('0.0.0.0', False), ('0.0.0.0', True), +]) +@pytest.mark.parametrize('version', ['1.0.0', '2.0.0']) +def test_legacy_reinstall_preserves_denied_pack_and_runs_allowed_script( + tmp_path, registry, listen, override, version): + with running_server(tmp_path, 'legacy', listen, override, 'normal', registry) as server: + node = 'fixture-reinstall' + install(server, node, '1.0.0') + marker = tmp_path / 'custom_nodes' / node / 'installed.txt' + assert marker.read_text() == '1.0.0' + original = (marker.read_bytes(), marker.stat().st_mtime_ns) + stale = marker.parent / 'stale.txt' + stale.write_text('removed only by an allowed reinstall') + before = len(registry.calls) + log_before = len((server.root / 'server.log').read_text()) + + result = install(server, node, version, 'reinstall') + + calls = registry.calls[before:] + allowed = version == '1.0.0' or override or listen == '127.0.0.1' + assert sum('/install' in call for call in calls) == 1, calls + assert any(call.startswith('/zip/') for call in calls) is allowed, calls + if allowed: + assert list(result['nodepack_result'].values()) == ['success'], result + assert marker.read_text() == version, result + assert not stale.exists() + else: + assert_flagged_denial(result, (server.root / 'server.log').read_text()[log_before:]) + assert marker.exists(), 'Rejected reinstall removed the existing pack' + assert (marker.read_bytes(), marker.stat().st_mtime_ns) == original + assert stale.exists() + + +def test_nonlocal_override_does_not_allow_nightly(comfy_server): + server = comfy_server + if server.listen == '127.0.0.1': + pytest.skip('This regression checks the non-local Git gate') + log_path = server.root / 'server.log' + log_before = len(log_path.read_text()) + before = len(server.registry.calls) + result = install(server, 'fixture-nightly', 'nightly') + # A missing node or a failed clone must not count as a security rejection. + if server.tree == 'legacy': + assert result['failed'] == ['fixture-nightly'], result + else: + assert result['history']['result'] == 'failed', result + expected_reason = ('normal or below' if server.security == 'strong' and server.tree == 'legacy' + else 'network_mode must be set to `personal_cloud`') + assert expected_reason in log_path.read_text()[log_before:], result + assert not (server.root / 'custom_nodes/fixture-nightly').exists() + assert not any('/install' in call or call.startswith('/zip/') + for call in server.registry.calls[before:]) + + +@pytest.mark.parametrize('tree', ['legacy', 'glob']) +@pytest.mark.parametrize('listen,override,restart_listen,restart_override,allowed', [ + ('127.0.0.1', False, '0.0.0.0', False, False), + ('0.0.0.0', True, '0.0.0.0', False, False), + ('127.0.0.1', False, '127.0.0.1', False, True), + ('0.0.0.0', True, '0.0.0.0', True, True), +]) +@pytest.mark.parametrize('stored_status', [True, False]) +def test_restart_applies_current_policy_without_registry_query( + tree, listen, override, restart_listen, restart_override, allowed, stored_status, tmp_path, registry): + with running_server(tmp_path, tree, listen, override, 'normal', registry) as server: + marker = server.root / 'custom_nodes/fixture-active/installed.txt' + install(server, 'fixture-active', '1.0.0') + reserved = server.root / 'user/__manager/startup-scripts/install-scripts.txt' + install(server, 'fixture-active', '2.0.0') + assert 'NodeVersionStatusFlagged' in reserved.read_text() + original_marker = (marker.read_bytes(), marker.stat().st_mtime_ns) + server.process.terminate() + server.process.wait(timeout=10) + if not stored_status: + record = ast.literal_eval(reserved.read_text().strip()) + assert record[1] == '#LAZY-CNR-SWITCH-SCRIPT' + assert len(record) == 9 + reserved.write_text(repr(record[:8]) + '\n') + + config_path = server.root / 'user/__manager/config.ini' + config = configparser.ConfigParser() + config.read(config_path) + config['default']['allow_flagged_nodepack_install'] = str(restart_override) + with config_path.open('w') as config_file: + config.write(config_file) + command = list(server.command) + command[command.index('--listen') + 1] = restart_listen + before = len(server.registry.calls) + log_path = server.root / 'restart.log' + with log_path.open('w') as log: + process = subprocess.Popen(command, env=server.env, cwd=E2E_ROOT / 'comfyui', + stdout=log, stderr=subprocess.STDOUT) + try: + deadline = time.monotonic() + 90 + while time.monotonic() < deadline: + assert process.poll() is None, log_path.read_text()[-6000:] + try: + if requests.get(server.base + '/system_stats', timeout=1).status_code == 200: + break + except requests.RequestException: + pass + time.sleep(.2) + else: + pytest.fail(log_path.read_text()[-6000:]) + calls = server.registry.calls[before:] + assert not any('/install' in call for call in calls), calls + assert any(call.startswith('/zip/') for call in calls) is allowed, calls + if allowed: + assert marker.read_text() == '2.0.0', log_path.read_text()[-6000:] + else: + if stored_status: + assert_terminal_guidance(log_path.read_text()) + else: + assert 'stored Registry status is missing' in log_path.read_text() + assert 'Request the installation again' in log_path.read_text() + assert (marker.read_bytes(), marker.stat().st_mtime_ns) == original_marker + finally: + process.terminate() + try: + process.wait(timeout=10) + except subprocess.TimeoutExpired: + process.kill() + process.wait(timeout=5) + + +@pytest.mark.parametrize('operation', ['install', 'restore-snapshot']) +def test_direct_cli_installs_flagged_without_server_options(tmp_path, registry, operation): + env = prepare_environment(tmp_path, False, 'normal', registry) + command = [str(E2E_ROOT / 'venv/bin/python'), '-m', 'cm_cli', operation] + if operation == 'install': + command += ['fixture-flagged@2.0.0', '--mode', 'cache'] + else: + snapshot = tmp_path / 'direct-snapshot.json' + snapshot.write_text(json.dumps({'cnr_custom_nodes': {'fixture-flagged': '2.0.0'}, + 'git_custom_nodes': {}, 'file_custom_nodes': []})) + command.append(str(snapshot)) + command += ['--user-directory', str(tmp_path / 'user')] + before = len(registry.calls) + + result = subprocess.run(command, env=env, cwd=E2E_ROOT / 'comfyui', + capture_output=True, text=True, timeout=60) + + output = result.stdout + result.stderr + assert result.returncode == 0, output + installed = tmp_path / 'custom_nodes/fixture-flagged' + assert 'version = "2.0.0"' in (installed / 'pyproject.toml').read_text(), output + if operation == 'install': + assert (installed / 'installed.txt').read_text() == '2.0.0', output + assert sum('/install' in call for call in registry.calls[before:]) == 1 + assert 'Installation of this flagged CNR version is blocked' not in output + + +@pytest.mark.parametrize('tree', ['legacy', 'glob']) +@pytest.mark.parametrize('existing', [False, True]) +@pytest.mark.parametrize('version,listen,override,allowed', [ + ('1.1.0', '0.0.0.0', False, True), + ('2.0.0', '0.0.0.0', False, False), + ('2.0.0', '0.0.0.0', True, True), + ('2.0.0', '127.0.0.1', False, True), +]) +def test_scheduled_snapshot_restores_through_server_restart( + tmp_path, registry, tree, existing, version, listen, override, allowed): + installed = tmp_path / 'custom_nodes/fixture-snapshot' + marker = installed / 'installed.txt' + reservation = tmp_path / 'user/__manager/startup-scripts/restore-snapshot.json' + with running_server(tmp_path, tree, '127.0.0.1', False, 'normal', registry) as server: + if existing: + install(server, 'fixture-snapshot', '1.0.0') + assert marker.read_text() == '1.0.0' + original = (marker.read_bytes(), marker.stat().st_mtime_ns) + snapshot = tmp_path / 'user/__manager/snapshots/flagged-policy.json' + snapshot.write_text(json.dumps({'cnr_custom_nodes': {'fixture-snapshot': version}, + 'git_custom_nodes': {}, 'file_custom_nodes': []})) + before = len(registry.calls) + response = requests.post(server.base + '/v2/snapshot/restore', + params={'target': snapshot.stem}, json={}, timeout=10) + assert response.status_code == 200, response.text + assert reservation.read_bytes() == snapshot.read_bytes() + assert registry.calls[before:] == [] + if existing: + assert (marker.read_bytes(), marker.stat().st_mtime_ns) == original + else: + assert not installed.exists() + + before = len(registry.calls) + with running_server(tmp_path, tree, listen, override, 'normal', registry): + log = (tmp_path / 'server.log').read_text() + calls = registry.calls[before:] + assert 'Restore snapshot done.' in log, log[-6000:] + assert 'Restore snapshot failed.' not in log, log[-6000:] + assert 'Error in sitecustomize' not in log, log[-6000:] + assert not reservation.exists() + assert sum('/install' in call for call in calls) == 1, (calls, log[-6000:]) + assert any(call.startswith('/zip/') for call in calls) is allowed, calls + if allowed: + assert f'version = "{version}"' in (installed / 'pyproject.toml').read_text(), log[-6000:] + assert 'allow_flagged_nodepack_install' not in log + else: + assert_terminal_guidance(log) + if existing: + assert (marker.read_bytes(), marker.stat().st_mtime_ns) == original + assert '1.0.0' in (installed / 'pyproject.toml').read_text() + else: + assert not installed.exists() diff --git a/tests/test_flagged_nodepack_policy.py b/tests/test_flagged_nodepack_policy.py new file mode 100644 index 000000000..576a36502 --- /dev/null +++ b/tests/test_flagged_nodepack_policy.py @@ -0,0 +1,431 @@ +"""Exercise the real CNR client and both install implementations.""" + +import asyncio +import ast +import json +import logging +import os +import socket +import zipfile +from pathlib import Path +from types import SimpleNamespace +from unittest.mock import AsyncMock, Mock + +import pytest + +from _install_flags_testutil import import_context, import_reader + + +@pytest.fixture +def cnr_client(monkeypatch): + import_context() + from comfy.cli_args import args + from comfyui_manager.common import cnr_utils + + monkeypatch.setattr(args, 'listen', '0.0.0.0') + return cnr_utils + + +@pytest.fixture(params=['glob', 'legacy']) +def core(request, cnr_client, monkeypatch, tmp_path): + core = import_reader(request.param) + monkeypatch.setattr(core.manager_funcs, 'is_flagged_install_allowed', lambda: False) + monkeypatch.setattr(core, 'get_config', lambda: {'allow_flagged_nodepack_install': False}) + monkeypatch.setattr(core, 'get_default_custom_nodes_path', lambda: str(tmp_path)) + return core + + +def api_response(status='NodeVersionStatusFlagged', http=200): + return SimpleNamespace(status_code=http, json=lambda: { + 'id': 'version-id', 'node_id': 'fixture', 'version': '2.0.0', + 'status': status, 'downloadUrl': 'https://example.invalid/fixture.zip', + }) + + +@pytest.mark.parametrize('permission,allowed', [(None, True), ('true', True), ('false', False)]) +def test_cli_install_uses_only_parent_permission(core, monkeypatch, permission, allowed, tmp_path): + from comfy.cli_args import args + + monkeypatch.delenv('_COMFYUI_MANAGER_CNR_ALLOW_FLAGGED', raising=False) + if permission is not None: + monkeypatch.setenv('_COMFYUI_MANAGER_CNR_ALLOW_FLAGGED', permission) + monkeypatch.setattr(core, 'manager_funcs', core.ManagerFuncs()) + monkeypatch.setattr(args, 'listen', '127.0.0.1' if permission == 'false' else '0.0.0.0') + monkeypatch.setattr(core, 'get_config', lambda: {'allow_flagged_nodepack_install': permission == 'false'}) + get = Mock(return_value=api_response()) + monkeypatch.setattr(core.cnr_utils.requests, 'get', get) + download = Mock(side_effect=lambda url, directory, name: (tmp_path / name).write_bytes(b'fixture')) + monkeypatch.setattr(core.manager_downloader, 'download_url', download) + monkeypatch.setattr(core.manager_util, 'extract_package_as_zip', lambda *a: ['__init__.py']) + manager = core.UnifiedManager() + manager.execute_install_script = Mock(return_value=True) + + result = asyncio.run(manager.install_by_id('fixture', '2.0.0')) + + assert result.result is allowed, result.msg + assert get.call_count == 1 + assert download.call_count == int(allowed) + + +@pytest.mark.parametrize('listen,allowed', [ + ('127.0.0.1', True), ('127.2.3.4', True), ('::1', True), + ('127.0.0.1,::1', True), ('0.0.0.0', False), ('::', False), + ('192.168.1.2', False), ('127.0.0.1,0.0.0.0', False), + ('0.0.0.0,::', False), ('', False), ('127.0.0.1,', False), +]) +def test_listener_policy(cnr_client, listen, allowed): + from comfyui_manager.common.manager_security import is_cnr_install_allowed + + assert is_cnr_install_allowed('NodeVersionStatusFlagged', False, listen) is allowed + assert is_cnr_install_allowed('NodeVersionStatusFlagged', True, listen) is True + + +def test_hostname_requires_all_resolved_addresses_to_be_loopback(cnr_client, monkeypatch): + from comfyui_manager.common.manager_security import is_loopback_listener + + def addresses(*hosts): + return [(socket.AF_INET, socket.SOCK_STREAM, 6, '', (host, 0)) for host in hosts] + + monkeypatch.setattr(socket, 'getaddrinfo', lambda *a, **k: addresses('127.0.0.1', '::1')) + assert is_loopback_listener('localhost') + monkeypatch.setattr(socket, 'getaddrinfo', lambda *a, **k: addresses('127.0.0.1', '192.168.1.2')) + assert not is_loopback_listener('localhost') + monkeypatch.setattr(socket, 'getaddrinfo', Mock(side_effect=socket.gaierror)) + assert not is_loopback_listener('invalid.invalid') + + +@pytest.mark.parametrize('status,permission,allowed', [ + ('NodeVersionStatusActive', False, True), ('NodeVersionStatusPending', False, True), + ('NodeVersionStatusFlagged', False, False), ('NodeVersionStatusFlagged', True, True), +]) +def test_install_uses_status_from_one_response(core, monkeypatch, tmp_path, caplog, status, permission, allowed): + get = Mock(return_value=api_response(status)) + monkeypatch.setattr(core.cnr_utils.requests, 'get', get) + monkeypatch.setattr(core.manager_funcs, 'is_flagged_install_allowed', lambda: permission) + download = Mock(side_effect=lambda url, directory, name: (tmp_path / name).write_bytes(b'fixture')) + monkeypatch.setattr(core.manager_downloader, 'download_url', download) + monkeypatch.setattr(core.manager_util, 'extract_package_as_zip', lambda *a: ['__init__.py']) + manager = core.UnifiedManager() + manager.execute_install_script = Mock(return_value=True) + + result = manager.cnr_install('fixture', '2.0.0') + + assert result.result is allowed, result.msg + if not allowed: + assert result.msg == ('This action is not allowed by the current security configuration. ' + 'See the terminal for details.') + for detail in ('--listen 127.0.0.1', '--listen ::1', '0.0.0.0', '[default]', + 'allow_flagged_nodepack_install = true', 'config.ini', + 'trusted private network', 'Restart ComfyUI'): + assert detail in caplog.text + else: + assert 'allow_flagged_nodepack_install' not in caplog.text + assert get.call_count == 1 + assert download.call_count == int(allowed) + + +@pytest.mark.parametrize('http', [403, 404, 500]) +def test_override_does_not_bypass_registry_refusal(core, monkeypatch, tmp_path, http): + monkeypatch.setattr(core.manager_funcs, 'is_flagged_install_allowed', lambda: True) + get = Mock(return_value=api_response(http=http)) + monkeypatch.setattr(core.cnr_utils.requests, 'get', get) + download = Mock(side_effect=AssertionError('Registry refusal must prevent downloading')) + monkeypatch.setattr(core.manager_downloader, 'download_url', download) + manager = core.UnifiedManager() + + result = asyncio.run(manager.install_by_id('fixture', '2.0.0')) + + assert result.result is False + assert result.msg == 'not available node: fixture@2.0.0' + assert get.call_count == 1 + assert download.call_count == 0 + assert not (tmp_path / 'fixture').exists() + + +@pytest.mark.parametrize('operation', ['install', 'lazy', 'instant']) +def test_direct_install_and_switch_reject_before_side_effects(core, monkeypatch, operation): + get = Mock(return_value=api_response()) + monkeypatch.setattr(core.cnr_utils.requests, 'get', get) + download = Mock(side_effect=AssertionError('download must not happen')) + monkeypatch.setattr(core.manager_downloader, 'download_url', download) + monkeypatch.setattr(core.manager_downloader, 'basic_download_url', download) + manager = core.UnifiedManager() + manager.reserve_cnr_switch = Mock(side_effect=AssertionError('reservation must not happen')) + if operation == 'install': + result = manager.cnr_install('fixture', '2.0.0') + else: + result = manager.cnr_switch_version('fixture', '2.0.0', instant_execution=operation == 'instant') + assert result.result is False + assert result.msg == ('This action is not allowed by the current security configuration. ' + 'See the terminal for details.') + assert get.call_count == 1 + + +@pytest.mark.parametrize('operation', ['install', 'lazy', 'instant']) +@pytest.mark.parametrize('return_postinstall', [False, True]) +def test_direct_cnr_execution_and_postinstall(core, monkeypatch, tmp_path, operation, return_postinstall): + get = Mock(return_value=api_response('NodeVersionStatusActive')) + monkeypatch.setattr(core.cnr_utils.requests, 'get', get) + + def download(url, directory, name): + with zipfile.ZipFile(Path(directory) / name, 'w') as archive: + archive.writestr('__init__.py', 'VERSION = "2.0.0"') + + downloader = Mock(side_effect=download) + monkeypatch.setattr(core.manager_downloader, 'download_url', downloader) + monkeypatch.setattr(core.manager_downloader, 'basic_download_url', downloader) + manager = core.UnifiedManager() + installed = tmp_path / 'fixture' + if operation != 'install': + installed.mkdir() + (installed / '__init__.py').write_text('original') + (installed / '.tracking').write_text('__init__.py') + manager.active_nodes['fixture'] = ('1.0.0', str(installed)) + manager.reserve_cnr_switch = Mock(return_value=True) + manager.execute_install_script = Mock(return_value=True) + + if operation == 'install': + result = manager.cnr_install('fixture', instant_execution=True, + no_deps=True, return_postinstall=return_postinstall) + else: + result = manager.cnr_switch_version('fixture', instant_execution=operation == 'instant', + no_deps=True, return_postinstall=return_postinstall) + + assert result.result is True, result.msg + assert result.target == ('2.0.0' if operation == 'instant' else None) + effect = manager.reserve_cnr_switch if operation == 'lazy' else manager.execute_install_script + assert effect.call_count == int(not return_postinstall) + if return_postinstall: + assert result.postinstall() + assert effect.call_count == 1 + assert get.call_count == 1 + assert downloader.call_count == int(operation != 'lazy') + assert (installed / '__init__.py').read_text() == ( + 'original' if operation == 'lazy' else 'VERSION = "2.0.0"') + if operation == 'lazy': + assert effect.call_args.args[1] == 'https://example.invalid/fixture.zip' + assert effect.call_args.args[-2:] == (True, 'NodeVersionStatusActive') + else: + assert effect.call_args.kwargs == {'instant_execution': True, 'no_deps': True} + + +@pytest.mark.parametrize('installed', ['nightly', 'disabled-cnr', 'new']) +def test_rejection_preserves_existing_install_state(core, monkeypatch, installed): + get = Mock(return_value=api_response()) + monkeypatch.setattr(core.cnr_utils.requests, 'get', get) + manager = core.UnifiedManager() + manager.is_enabled = lambda node, version=None: installed == 'nightly' and version == 'nightly' + manager.is_disabled = lambda node, version=None: installed == 'disabled-cnr' and version == 'cnr' + manager.unified_disable = Mock(side_effect=AssertionError('must not disable')) + manager.unified_enable = Mock(side_effect=AssertionError('must not enable')) + result = asyncio.run(manager.install_by_id('fixture', '2.0.0')) + assert result.result is False + assert result.msg == ('This action is not allowed by the current security configuration. ' + 'See the terminal for details.') + assert get.call_count == 1 + + +@pytest.mark.parametrize('installed', ['new', 'cnr', 'disabled-cnr']) +@pytest.mark.parametrize('instant', [False, True]) +def test_preflight_response_is_reused(core, monkeypatch, installed, instant, tmp_path): + get = Mock(return_value=api_response('NodeVersionStatusActive')) + monkeypatch.setattr(core.cnr_utils.requests, 'get', get) + manager = core.UnifiedManager() + manager.is_enabled = lambda node, version=None: installed == 'cnr' and version == 'cnr' + manager.is_disabled = lambda node, version=None: installed == 'disabled-cnr' and version == 'cnr' + manager.unified_enable = Mock() + manager.active_nodes['fixture'] = ('1.0.0', str(tmp_path / 'fixture')) + manager.reserve_cnr_switch = Mock(return_value=True) + manager.execute_install_script = Mock(return_value=True) + marker = tmp_path / 'fixture/__init__.py' + if installed != 'new': + marker.parent.mkdir() + marker.write_text('original') + (marker.parent / '.tracking').write_text('__init__.py') + + def download(url, directory, name): + (tmp_path / name).write_bytes(b'fixture') + + def extract(archive, directory): + (Path(directory) / '__init__.py').write_text('replacement') + return ['__init__.py'] + + monkeypatch.setattr(core.manager_downloader, 'download_url', download) + monkeypatch.setattr(core.manager_downloader, 'basic_download_url', download) + monkeypatch.setattr(core.manager_util, 'extract_package_as_zip', extract) + result = asyncio.run(manager.install_by_id('fixture', '2.0.0', instant_execution=instant)) + assert result.result is True + assert get.call_count == 1 + if installed == 'new' or instant: + assert marker.read_text() == 'replacement' + manager.reserve_cnr_switch.assert_not_called() + manager.execute_install_script.assert_called_once() + assert manager.execute_install_script.call_args.kwargs['instant_execution'] is instant + else: + assert marker.read_text() == 'original' + manager.reserve_cnr_switch.assert_called_once() + manager.execute_install_script.assert_not_called() + + +def test_exact_installed_version_enables_without_registry_lookup(core, monkeypatch, tmp_path): + manager = core.UnifiedManager() + installed = tmp_path / '.disabled/fixture@2_0_0' + installed.mkdir(parents=True) + (installed / '__init__.py').write_text('already installed') + manager.cnr_inactive_nodes['fixture'] = {'2.0.0': str(installed)} + query = Mock(side_effect=AssertionError('activation must not query the Registry')) + monkeypatch.setattr(core.cnr_utils.requests, 'get', query) + manager.execute_install_script = Mock(side_effect=AssertionError('activation must not install')) + + result = asyncio.run(manager.install_by_id('fixture', '2.0.0')) + + assert result.result is True + assert (tmp_path / 'fixture/__init__.py').read_text() == 'already installed' + assert not installed.exists() + assert manager.active_nodes['fixture'] == ('2.0.0', str(tmp_path / 'fixture')) + assert asyncio.run(manager.install_by_id('fixture', '2.0.0')).action == 'skip' + query.assert_not_called() + manager.execute_install_script.assert_not_called() + + +def test_snapshot_denial_preserves_existing_pack_without_retry(core, monkeypatch, tmp_path): + manager = core.UnifiedManager() + monkeypatch.setattr(core, 'unified_manager', manager) + installed = tmp_path / 'fixture' + installed.mkdir() + marker = installed / '__init__.py' + marker.write_text('original') + (installed / '.tracking').write_text('__init__.py') + manager.active_nodes['fixture'] = ('1.0.0', str(installed)) + manager.reload = AsyncMock() + manager.get_custom_nodes = AsyncMock(return_value={}) + monkeypatch.setattr(core.manager_util, 'restore_pip_snapshot', Mock()) + get = Mock(return_value=api_response()) + monkeypatch.setattr(core.cnr_utils.requests, 'get', get) + download = Mock(side_effect=AssertionError('denied target must not download')) + monkeypatch.setattr(core.manager_downloader, 'download_url', download) + monkeypatch.setattr(core.manager_downloader, 'basic_download_url', download) + snapshot = tmp_path / 'snapshot.json' + snapshot.write_text(json.dumps({'cnr_custom_nodes': {'fixture': '2.0.0'}, + 'git_custom_nodes': {}, 'file_custom_nodes': []})) + + asyncio.run(core.restore_snapshot(str(snapshot))) + + assert marker.read_text() == 'original' + assert manager.active_nodes['fixture'] == ('1.0.0', str(installed)) + assert download.call_count == 0 + assert get.call_count == 1 + + +@pytest.mark.parametrize('core', ['legacy'], indirect=True) +def test_reinstall_stops_when_removal_fails(core, monkeypatch): + get = Mock(return_value=api_response('NodeVersionStatusActive')) + monkeypatch.setattr(core.cnr_utils.requests, 'get', get) + download = Mock(side_effect=AssertionError('failed removal must prevent downloading')) + monkeypatch.setattr(core.manager_downloader, 'download_url', download) + manager = core.UnifiedManager() + manager.unified_uninstall = Mock(return_value=core.ManagedResult('uninstall').fail('cannot remove')) + + result = asyncio.run(manager.reinstall_by_id('fixture', '2.0.0')) + + assert result.result is False + assert result.msg == 'cannot remove' + assert get.call_count == 1 + assert download.call_count == 0 + + +@pytest.mark.parametrize('core', ['legacy'], indirect=True) +@pytest.mark.parametrize('version,source', [ + ('nightly', 'manifest'), ('unknown', 'manifest'), + ('nightly', 'cnr'), ('nightly', 'system-repository'), +]) +def test_git_reinstall_replaces_existing_pack(core, monkeypatch, tmp_path, version, source): + manager = core.UnifiedManager() + directory = 'repository-name' if source == 'system-repository' else 'fixture' + installed = tmp_path / directory + installed.mkdir() + marker = installed / 'original.txt' + marker.write_text('original') + repo_url = f'https://example.invalid/{directory}' + if version == 'nightly': + manager.active_nodes['fixture'] = ('nightly', str(installed)) + else: + manager.unknown_active_nodes['fixture'] = (repo_url, str(installed)) + manager.get_custom_nodes = AsyncMock(return_value={ + 'fixture': {'repository': repo_url, 'files': [repo_url]}, + } if source == 'manifest' else {}) + if source != 'manifest': + manager.cnr_map['fixture'] = { + 'repository': repo_url, 'publisher': None if source == 'system-repository' else {'id': 'fixture'}, + } + manager.processed_install.add(str(installed / 'install.py')) + execute = Mock(return_value=True) + monkeypatch.setattr(core, 'try_install_script', execute) + monkeypatch.setattr(core.cnr_utils.requests, 'get', Mock(side_effect=AssertionError('Git needs no CNR install query'))) + + def clone(url, path, **kwargs): + assert url == repo_url + assert not Path(path).exists() + Path(path).mkdir() + (Path(path) / 'replacement.txt').write_text('installed') + (Path(path) / 'install.py').write_text('') + assert manager.execute_install_script(url, path) + return core.ManagedResult('install-git') + + manager.repo_install = Mock(side_effect=clone) + result = asyncio.run(manager.reinstall_by_id('fixture', version)) + + assert result.result is True + assert not marker.exists() + assert (installed / 'replacement.txt').read_text() == 'installed' + assert manager.repo_install.call_count == 1 + assert execute.call_count == 1 + assert execute.call_args.args[1] == str(installed) + + +@pytest.mark.parametrize('status', ['', 'NodeVersionStatusActive', 'NodeVersionStatusFlagged']) +@pytest.mark.parametrize('override', [False, True]) +@pytest.mark.parametrize('listen', ['127.0.0.1', '0.0.0.0']) +def test_deferred_switch_reuses_status_with_current_policy(cnr_client, monkeypatch, tmp_path, caplog, status, override, listen): + from comfy.cli_args import args as server_args + from comfyui_manager.common import manager_downloader, manager_util + + monkeypatch.setattr(server_args, 'listen', listen) + # Execute the real function without running prestartup's module-level startup. + source = Path(cnr_client.__file__).parents[1] / 'prestartup_script.py' + tree = ast.parse(source.read_text()) + function = next(node for node in tree.body if isinstance(node, ast.FunctionDef) + and node.name == 'execute_lazy_cnr_switch') + scope = {'__package__': 'comfyui_manager', 'os': os, 'logging': logging, + 'manager_downloader': manager_downloader, 'manager_util': manager_util, + 'default_conf': {'allow_flagged_nodepack_install': str(override)}} + exec(compile(ast.Module(body=[function], type_ignores=[]), str(source), 'exec'), scope) + from_path, to_path = tmp_path / 'fixture@1.0.0', tmp_path / 'fixture' + from_path.mkdir() + (from_path / '.tracking').write_text('__init__.py') + (from_path / '__init__.py').write_text('') + + def download(url, directory, name): + (Path(directory) / name).write_bytes(b'fixture') + + downloader = Mock(side_effect=download) + monkeypatch.setattr(manager_downloader, 'download_url', downloader) + monkeypatch.setattr(manager_util, 'extract_package_as_zip', lambda *a: {'__init__.py'}) + monkeypatch.setattr(cnr_client.requests, 'get', Mock(side_effect=AssertionError('no CNR query'))) + args = ('fixture', 'https://example.invalid/archive.zip', str(from_path), str(to_path), False, str(tmp_path)) + # Omit the added argument for the old on-disk reservation format. + result = scope['execute_lazy_cnr_switch'](*args, *([status] if status else [])) + allowed = status == 'NodeVersionStatusActive' or override or listen == '127.0.0.1' + assert result is allowed + assert downloader.call_count == int(allowed) + assert to_path.exists() is allowed + assert from_path.exists() is not allowed + if not allowed and not status: + assert 'stored Registry status is missing' in caplog.text + assert 'Request the installation again' in caplog.text + elif not allowed: + for detail in ('--listen 127.0.0.1', '--listen ::1', '[default]', + 'allow_flagged_nodepack_install = true', 'Restart ComfyUI'): + assert detail in caplog.text + else: + assert 'allow_flagged_nodepack_install' not in caplog.text diff --git a/tests/test_install_flags_config.py b/tests/test_install_flags_config.py index 96a040281..bac14f160 100644 --- a/tests/test_install_flags_config.py +++ b/tests/test_install_flags_config.py @@ -43,7 +43,7 @@ import pytest sys.path.insert(0, os.path.dirname(os.path.abspath(__file__))) from _install_flags_testutil import import_context, import_reader # noqa: E402 -FLAG_KEYS = ("allow_git_url_install", "allow_pip_install") +FLAG_KEYS = ("allow_git_url_install", "allow_pip_install", "allow_flagged_nodepack_install") READERS = ("glob", "legacy") @@ -169,7 +169,8 @@ def test_sc21_write_config_round_trips_both_flags( ini = _write_ini( tmp_path, "[default]\nsecurity_level = normal\n" - "allow_git_url_install = true\nallow_pip_install = true\n", + "allow_git_url_install = true\nallow_pip_install = true\n" + "allow_flagged_nodepack_install = true\n", ) point_config(ini) diff --git a/tests/test_legacy_secgate_other_paths.py b/tests/test_legacy_secgate_other_paths.py index ae997c689..bd8bf04d7 100644 --- a/tests/test_legacy_secgate_other_paths.py +++ b/tests/test_legacy_secgate_other_paths.py @@ -240,7 +240,7 @@ def test_p1_install_custom_node_denies_at_strong_403(ms, gate): """P1 deny: at sl=strong the middle+ entry gate (first statement of _install_custom_node) returns 403 before any further processing.""" gate("strong", is_local_mode=True) - resp = _run(ms._install_custom_node({})) + resp = _run(ms._queue_node_install({}, "install")) assert resp.status == 403 @@ -263,7 +263,7 @@ def test_p1_install_custom_node_passes_entry_gate_at_normal(ms, gate, monkeypatc "channel": "default", "mode": "cache", } - resp = _run(ms._install_custom_node(dict(json_data))) + resp = _run(ms._queue_node_install(dict(json_data), "install")) assert resp.status == 200 @@ -286,7 +286,7 @@ def test_p2_unknown_pip_block_denies_404_even_at_weak(ms, gate, monkeypatch): "channel": "default", "mode": "cache", } - resp = _run(ms._install_custom_node(dict(json_data))) + resp = _run(ms._queue_node_install(dict(json_data), "install")) assert resp.status == 404