From 215d7d4cb07e147e6f623682c89fba0fa1b5ec0b Mon Sep 17 00:00:00 2001 From: "Dr.Lt.Data" <128333288+ltdrdata@users.noreply.github.com> Date: Wed, 11 Nov 2026 23:26:09 +0900 Subject: [PATCH] v4: Allow selected flagged CNR nodepacks through a whitelist (#3487) --- .github/workflows/ci.yml | 5 +- CHANGELOG.md | 8 + README.md | 65 +++++-- comfyui_manager/common/manager_security.py | 69 +++++++- comfyui_manager/glob/manager_core.py | 13 +- comfyui_manager/glob/manager_server.py | 6 +- comfyui_manager/legacy/manager_core.py | 13 +- comfyui_manager/legacy/manager_server.py | 6 +- comfyui_manager/prestartup_script.py | 11 +- tests/e2e/test_e2e_flagged_nodepacks.py | 167 +++++++++++++++++- tests/e2e/verify_live_flagged_whitelist.py | 125 +++++++++++++ tests/test_flagged_nodepack_policy.py | 194 ++++++++++++++++++--- 12 files changed, 619 insertions(+), 63 deletions(-) create mode 100644 tests/e2e/verify_live_flagged_whitelist.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ec06b27a4..47b1d0077 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -37,7 +37,8 @@ jobs: code-quality: name: Code Quality Checks - runs-on: ubuntu-latest + # Python 3.9 builds are unavailable for Ubuntu 26.04. + runs-on: ubuntu-24.04 steps: - uses: actions/checkout@v4 with: @@ -67,4 +68,4 @@ jobs: if: steps.changed-py-files.outputs.any_changed == 'true' run: | echo "Changed files: ${{ steps.changed-py-files.outputs.all_changed_files }}" - echo "${{ steps.changed-py-files.outputs.all_changed_files }}" | xargs -r ruff check \ No newline at end of file + echo "${{ steps.changed-py-files.outputs.all_changed_files }}" | xargs -r ruff check diff --git a/CHANGELOG.md b/CHANGELOG.md index 7161e0063..4fdfbae22 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,14 @@ and this project adheres to [Semantic Versioning 2.0.0](https://semver.org/spec/ ## [Unreleased] +### Security + +- Allow administrators to exempt selected Registry IDs from the flagged CNR + install restriction using `nodepack-whitelist.list` beside Manager's `config.ini`. + Entries are case-insensitive, apply to all versions, and take effect without a + restart. Server startup creates a commented template with no default exceptions; + other installation security checks still apply. + ## [4.3] - 2026-09-19 ### Security diff --git a/README.md b/README.md index 4a3f88005..97dac1471 100644 --- a/README.md +++ b/README.md @@ -100,6 +100,7 @@ In `ComfyUI-Manager` V4.0.3b4 and later, configuration files and dynamically gen * Configurable pip overrides: `/__manager/pip_overrides.json` * Configurable pip blacklist: `/__manager/pip_blacklist.list` * Configurable pip auto fix: `/__manager/pip_auto_fix.list` +* Configurable flagged nodepack whitelist: `/__manager/nodepack-whitelist.list` * Saved snapshot files: `/__manager/snapshots` * Startup script files: `/__manager/startup-scripts` * Component files: `/__manager/components` @@ -370,16 +371,44 @@ 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. +`--listen` examples, the private network override setting, the pack whitelist, +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. +* Active versions continue through the normal install flow. New HTTP requests + to install CNR releases use the `middle` security level, so they are available + on non-local listeners with `security_level = normal`, `normal-`, or `weak`. + `strong` denies these new requests, including requests for whitelisted packs. * 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. + override below is enabled or the pack ID is whitelisted. +* To allow specific packs on any listener without enabling all flagged installs, + edit `nodepack-whitelist.list` beside Manager's `config.ini` + (normally `/__manager/nodepack-whitelist.list`). + At server startup, Manager creates a commented template if the file is missing; + existing files are preserved. The template grants no exceptions by default. + Enter one Registry pack ID per line: + + ```text + # Nodepack whitelist + # Add one Registry ID per line; each entry applies to all CNR versions. + # Registry IDs are case-insensitive. + + comfyui-impact-pack + comfyui-inspire-pack # Inline comments are supported. + ``` + + Entries are case-insensitive and apply to all CNR versions of that pack. Use + Registry IDs, not display names, Git URLs, or `id@version`; wildcards are not + expanded. Blank lines and `#` comments are ignored. Delete an entry or comment + out its line with `#` to revoke that exception. Manager reads the file + for each installation check, so edits and removals apply without restarting. + A missing, unreadable, or NUL-containing file grants no whitelist exceptions. + If mixed encodings introduce NUL characters, Manager logs a warning to save + the file as UTF-8. Whitelist-based permission is logged at WARNING level with + the Registry ID, including in snapshot restore children. This records the policy + decision, not successful installation. Packs absent from the list continue to + use the listener and global option rules. * To allow flagged versions on a non-local listener in a **trusted private network**, an administrator can explicitly opt in: @@ -395,17 +424,27 @@ and directs users to the terminal. Only the terminal provides loopback-only 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. + installation. Neither the option nor the whitelist bypasses the `strong` + restriction on new HTTP install requests 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. +* Older deferred switches without a stored status require loopback-only listeners, + the override, or a whitelisted pack ID. 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. + and check it against the current listen address, option, and whitelist 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. + server's current listener and setting, and read the whitelist from the server's + Manager configuration directory. +* Startup execution of deferred CNR switches and server-scheduled snapshot + restores does not recheck `security_level`. Changing it to `strong` does not + cancel previously scheduled work. For example, a flagged version switch + scheduled under `normal` can still run after restarting with `strong` if the + pack remains whitelisted. The startup flagged-version checks described above + still apply. This is existing behavior of scheduled execution. * Standalone `cm-cli` commands remain local administrator operations and do not use the server listener policy. diff --git a/comfyui_manager/common/manager_security.py b/comfyui_manager/common/manager_security.py index a6e49420d..cf4df63e3 100644 --- a/comfyui_manager/common/manager_security.py +++ b/comfyui_manager/common/manager_security.py @@ -18,6 +18,7 @@ DO NOT add the gate to body-reading handlers (redundant + UX-breaking). DO NOT remove the gate from no-body handlers (this is the bypass vector). """ +import logging import os from enum import Enum from typing import Optional @@ -27,6 +28,8 @@ from aiohttp import web is_personal_cloud_mode = False handler_policy = {} +FLAGGED_NODEPACK_WHITELIST_FILE = 'nodepack-whitelist.list' + FLAGGED_NODEPACK_INSTALL_ERROR = ( 'This action is not allowed by the current security configuration. ' 'See the terminal for details.' @@ -39,7 +42,10 @@ FLAGGED_NODEPACK_INSTALL_GUIDANCE = ( ' 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. ' + '3. To allow specific packs, add their Registry pack IDs (case-insensitive), one per line, ' + f'to {FLAGGED_NODEPACK_WHITELIST_FILE} beside ComfyUI-Manager\'s config.ini. ' + 'Whitelist changes apply to subsequent installations without a restart.\n' + 'Restart ComfyUI after changing the listener or config.ini. ' 'All other installation security checks still apply.' ) @@ -99,7 +105,7 @@ def is_loopback(address): return False -def is_loopback_listener(listen_address: str) -> bool: +def is_loopback_listener(listen_address: Optional[str]) -> bool: """All addresses bound by --listen must resolve exclusively to loopback.""" import socket @@ -120,9 +126,62 @@ def is_loopback_listener(listen_address: str) -> bool: 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 ensure_nodepack_whitelist(manager_files_path): + """Create a commented template without replacing an existing whitelist.""" + path = os.path.join(manager_files_path, FLAGGED_NODEPACK_WHITELIST_FILE) + try: + os.makedirs(manager_files_path, exist_ok=True) + with open(path, 'x', encoding='utf-8') as file: + file.write( + '# Allow selected flagged CNR nodepacks on non-local listeners.\n' + '# One Registry ID per line; IDs are case-insensitive and apply to all versions.\n' + '# Blank lines and # comments are ignored. Changes apply without restarting.\n' + '# Other installation security checks still apply.\n' + '#\n' + '# Example entries (commented out; uncomment to allow):\n' + '# comfyui-impact-pack\n' + '# comfyui-inspire-pack # Optional inline comment\n' + ) + except OSError as exc: + if isinstance(exc, FileExistsError) and os.path.isfile(path): + return + logging.warning('[ComfyUI-Manager] Cannot create nodepack whitelist %s: %s', path, exc) + + +def is_flagged_nodepack_whitelisted(node_id: str, manager_files_path) -> bool: + """Match a whole Registry ID, ignoring case, in the current Manager whitelist.""" + if not manager_files_path: + return False + path = os.path.join(manager_files_path, FLAGGED_NODEPACK_WHITELIST_FILE) + try: + with open(path, encoding='utf-8-sig') as file: + contents = file.read() + if '\x00' in contents: + raise UnicodeError('NUL characters found; save the file as UTF-8') + entries = (line.split('#', 1)[0].strip() for line in contents.split('\n')) + if node_id.lower() in {entry.lower() for entry in entries if entry}: + logging.warning('[ComfyUI-Manager] CNR installation of %r permitted by nodepack whitelist (%s).', node_id, path) + return True + return False + except FileNotFoundError: + return False + except (OSError, UnicodeError) as exc: + logging.warning('[ComfyUI-Manager] Cannot read flagged nodepack whitelist %s: %s', path, exc) + return False + + +def is_cnr_install_allowed( + status: str, allow_flagged: bool, listen_address: Optional[str], *, + node_id: Optional[str] = None, manager_files_path=None, +) -> bool: + """Flagged versions require loopback, explicit permission, or a whitelisted ID. + + Restore children pass their parent's resolved permission and no listener. + """ + return (status != 'NodeVersionStatusFlagged' + or bool(allow_flagged) + or is_loopback_listener(listen_address) + or (node_id is not None and is_flagged_nodepack_whitelisted(node_id, manager_files_path))) def is_dedicated_install_allowed(flag_value: bool, listen_address: str, network_mode: str) -> bool: diff --git a/comfyui_manager/glob/manager_core.py b/comfyui_manager/glob/manager_core.py index 630963344..289d1b763 100644 --- a/comfyui_manager/glob/manager_core.py +++ b/comfyui_manager/glob/manager_core.py @@ -913,7 +913,8 @@ class UnifiedManager: 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(): + if (node_info is not None and node_info.status == 'NodeVersionStatusFlagged' + and not manager_funcs.is_flagged_install_allowed(node_id)): logging.error(manager_security.FLAGGED_NODEPACK_INSTALL_GUIDANCE) raise PermissionError(manager_security.FLAGGED_NODEPACK_INSTALL_ERROR) return node_info @@ -1696,9 +1697,13 @@ 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 is_flagged_install_allowed(self, node_id): + # Restore children use the parent's permission and whitelist directory, + # even if the CLI resolves a different user directory. + return manager_security.is_cnr_install_allowed( + 'NodeVersionStatusFlagged', os.environ.get('_COMFYUI_MANAGER_CNR_ALLOW_FLAGGED', 'true') == 'true', + listen_address=None, node_id=node_id, + manager_files_path=os.environ.get('_COMFYUI_MANAGER_CNR_WHITELIST_DIR')) def run_script(self, cmd, cwd='.'): if len(cmd) > 0 and cmd[0].startswith("#"): diff --git a/comfyui_manager/glob/manager_server.py b/comfyui_manager/glob/manager_server.py index d6bda32ca..8c003fc4c 100644 --- a/comfyui_manager/glob/manager_server.py +++ b/comfyui_manager/glob/manager_server.py @@ -133,8 +133,10 @@ 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 is_flagged_install_allowed(self, node_id): + return manager_security.is_cnr_install_allowed( + 'NodeVersionStatusFlagged', core.get_config()['allow_flagged_nodepack_install'], args.listen, + node_id=node_id, manager_files_path=context.manager_files_path) def run_script(self, cmd, cwd="."): if len(cmd) > 0 and cmd[0].startswith("#"): diff --git a/comfyui_manager/legacy/manager_core.py b/comfyui_manager/legacy/manager_core.py index 05ffdc7ea..cd0f220cc 100644 --- a/comfyui_manager/legacy/manager_core.py +++ b/comfyui_manager/legacy/manager_core.py @@ -899,7 +899,8 @@ class UnifiedManager: 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(): + if (node_info is not None and node_info.status == 'NodeVersionStatusFlagged' + and not manager_funcs.is_flagged_install_allowed(node_id)): logging.error(manager_security.FLAGGED_NODEPACK_INSTALL_GUIDANCE) raise PermissionError(manager_security.FLAGGED_NODEPACK_INSTALL_ERROR) return node_info @@ -1712,9 +1713,13 @@ 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 is_flagged_install_allowed(self, node_id): + # Restore children use the parent's permission and whitelist directory, + # even if the CLI resolves a different user directory. + return manager_security.is_cnr_install_allowed( + 'NodeVersionStatusFlagged', os.environ.get('_COMFYUI_MANAGER_CNR_ALLOW_FLAGGED', 'true') == 'true', + listen_address=None, node_id=node_id, + manager_files_path=os.environ.get('_COMFYUI_MANAGER_CNR_WHITELIST_DIR')) def run_script(self, cmd, cwd='.'): if len(cmd) > 0 and cmd[0].startswith("#"): diff --git a/comfyui_manager/legacy/manager_server.py b/comfyui_manager/legacy/manager_server.py index 07f84b8e0..1bf7df620 100644 --- a/comfyui_manager/legacy/manager_server.py +++ b/comfyui_manager/legacy/manager_server.py @@ -162,8 +162,10 @@ 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 is_flagged_install_allowed(self, node_id): + return manager_security.is_cnr_install_allowed( + 'NodeVersionStatusFlagged', core.get_config()['allow_flagged_nodepack_install'], args.listen, + node_id=node_id, manager_files_path=context.manager_files_path) def run_script(self, cmd, cwd='.'): if len(cmd) > 0 and cmd[0].startswith("#"): diff --git a/comfyui_manager/prestartup_script.py b/comfyui_manager/prestartup_script.py index d63b12abb..7fe519ef9 100644 --- a/comfyui_manager/prestartup_script.py +++ b/comfyui_manager/prestartup_script.py @@ -16,6 +16,7 @@ from .common import security_check from .common import manager_util from .common import cm_global from .common import manager_downloader +from .common import manager_security from .common.timestamp_utils import current_timestamp import folder_paths @@ -67,6 +68,7 @@ comfyui_manager_path = os.path.abspath(os.path.dirname(__file__)) custom_nodes_base_path = folder_paths.get_folder_paths('custom_nodes')[0] manager_files_path = folder_paths.get_system_user_directory("manager") +manager_security.ensure_nodepack_whitelist(manager_files_path) manager_pip_overrides_path = os.path.join(manager_files_path, "pip_overrides.json") manager_pip_blacklist_path = os.path.join(manager_files_path, "pip_blacklist.list") restore_snapshot_path = os.path.join(manager_files_path, "startup-scripts", "restore-snapshot.json") @@ -571,12 +573,12 @@ if os.path.exists(restore_snapshot_path): 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() + new_env['_COMFYUI_MANAGER_CNR_WHITELIST_DIR'] = manager_files_path 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) @@ -630,14 +632,15 @@ def execute_lazy_cnr_switch(target, zip_url, from_path, to_path, no_deps, custom 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 manager_security.is_cnr_install_allowed( + status or 'NodeVersionStatusFlagged', allow_flagged, args.listen, + node_id=target, manager_files_path=manager_files_path): 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) + logging.error(manager_security.FLAGGED_NODEPACK_INSTALL_GUIDANCE) return False # 1. download diff --git a/tests/e2e/test_e2e_flagged_nodepacks.py b/tests/e2e/test_e2e_flagged_nodepacks.py index 3bf99127d..bed4e46a3 100644 --- a/tests/e2e/test_e2e_flagged_nodepacks.py +++ b/tests/e2e/test_e2e_flagged_nodepacks.py @@ -99,7 +99,7 @@ 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: +with urllib.request.urlopen(os.environ['CM_E2E_CATALOG_URL']) as response: manager_util.save_to_cache(cnr_utils.base_url + '/nodes', json.load(response)) ''' BOOTSTRAP = ''' @@ -135,7 +135,8 @@ def prepare_environment(root, override, security, registry): 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) + CM_E2E_MAIN=str(E2E_ROOT / 'comfyui/main.py'), CM_E2E_REGISTRY=registry.url, + CM_E2E_CATALOG_URL=getattr(registry, 'catalog_url', registry.url + '/nodes')) return env @@ -197,11 +198,11 @@ def comfy_server(request, tmp_path_factory, registry): yield server -def install(server, node, version, operation='install'): +def install(server, node, version, operation='install', *, repository='https://example.invalid/fixture'): 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'} + 'repository': repository} if server.tree == 'legacy': response = requests.post(server.base + '/v2/manager/queue/batch', json={'batch_id': ui_id, operation: [params]}, timeout=10) @@ -229,9 +230,47 @@ def install(server, node, version, operation='install'): pytest.fail('Install did not complete:\n' + (server.root / 'server.log').read_text()[-6000:]) +@pytest.mark.parametrize('command', ['save-snapshot', 'restore-snapshot']) +def test_cli_does_not_create_server_whitelist(tmp_path, command): + base = tmp_path / 'cli-base' + (base / 'custom_nodes').mkdir(parents=True) + server_manager = tmp_path / 'server-user/__manager' + server_manager.mkdir(parents=True) + whitelist = server_manager / 'nodepack-whitelist.list' + whitelist.write_text('comfyui-impact-pack\n') + requested_user = tmp_path / 'requested-user' + # Stop after CLI initialization, before any snapshot work or network access. + if command == 'save-snapshot': + arguments = [command, '--user-directory', str(requested_user), + '--output', str(tmp_path / 'invalid-extension.txt')] + expected_error = 'output path should be either' + else: + arguments = [command, str(tmp_path / 'missing-snapshot.json')] + expected_error = 'is not exists.' + bootstrap = ''' +import runpy, sys +from comfy.cli_args import args +args.base_directory = sys.argv[1] +sys.argv = ['cm-cli', *sys.argv[2:]] +runpy.run_module('cm_cli', run_name='__main__') +''' + env = dict(os.environ, + PYTHONPATH=os.pathsep.join([str(REPO_ROOT), str(E2E_ROOT / 'comfyui')]), + COMFYUI_PATH=str(E2E_ROOT / 'comfyui'), COMFYUI_FOLDERS_BASE_PATH=str(base), + _COMFYUI_MANAGER_CNR_ALLOW_FLAGGED='false', + _COMFYUI_MANAGER_CNR_WHITELIST_DIR=str(server_manager)) + result = subprocess.run([str(E2E_ROOT / 'venv/bin/python'), '-c', bootstrap, + str(base), *arguments], env=env, cwd=tmp_path, + capture_output=True, text=True, timeout=30) + output = result.stdout + result.stderr + assert result.returncode == 1 and expected_error in output, output + assert whitelist.read_text() == 'comfyui-impact-pack\n' + assert sorted(tmp_path.rglob('nodepack-whitelist.list')) == [whitelist] + + 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', + 'allow_flagged_nodepack_install = true', 'config.ini', 'nodepack-whitelist.list', 'trusted private network', 'Restart ComfyUI'): assert detail in log @@ -274,6 +313,124 @@ def test_install_policy(comfy_server, node, version): assert_flagged_denial(result, (server.root / 'server.log').read_text()[log_before:]) +@pytest.mark.parametrize('tree', ['legacy', 'glob']) +def test_whitelist_install_policy(tmp_path, registry, tree): + with running_server(tmp_path, tree, '0.0.0.0', False, 'normal', registry) as server: + log_path = tmp_path / 'server.log' + whitelist = tmp_path / 'user/__manager/nodepack-whitelist.list' + template = whitelist.read_text() + assert template.strip() and all(line.lstrip().startswith('#') for line in template.splitlines() if line.strip()) + # Edited after startup: no restart is required to pick up this policy. + whitelist.write_text('Fixture-Flagged\nfixture-banned\nfixture-switch\n') + for node, version, expected_error in [ + ('fixture-flagged', '2.0.0', None), + ('fixture-latest', 'latest', 'This action is not allowed by the current security configuration.'), + ('fixture-banned', '1.0.0', 'not available node: fixture-banned@1.0.0'), + ]: + log_before = len(log_path.read_text()) + before = len(registry.calls) + result = install(server, node, version) + allowed = expected_error is None + marker = tmp_path / 'custom_nodes' / node / 'installed.txt' + assert marker.exists() is allowed, (result, log_path.read_text()[log_before:]) + calls = registry.calls[before:] + assert any(call.startswith('/zip/') for call in calls) is allowed + assert sum('/install' in call for call in calls) == 1, calls + assert ('permitted by nodepack whitelist' in log_path.read_text()[log_before:]) is allowed + assert 'permitted by nodepack whitelist' not in json.dumps(result) + if allowed: + assert marker.read_text() == version + if tree == 'legacy': + assert list(result['nodepack_result'].values()) == ['success'], result + else: + assert result['history']['result'] == 'success', result + else: + assert expected_error in json.dumps(result), result + if node == 'fixture-latest': + assert_flagged_denial(result, log_path.read_text()[log_before:]) + + # Removing the file revokes the exception for another listed pack. + whitelist.unlink() + log_before = len(log_path.read_text()) + before = len(registry.calls) + result = install(server, 'fixture-switch', '2.0.0') + assert not (tmp_path / 'custom_nodes/fixture-switch').exists(), result + calls = registry.calls[before:] + assert sum('/install' in call for call in calls) == 1, calls + assert not any(call.startswith('/zip/') for call in calls), calls + assert_flagged_denial(result, log_path.read_text()[log_before:]) + + +@pytest.mark.parametrize('tree', ['legacy', 'glob']) +def test_whitelist_does_not_override_strong_security(tmp_path, registry, tree): + with running_server(tmp_path, tree, '0.0.0.0', False, 'strong', registry) as server: + (tmp_path / 'user/__manager/nodepack-whitelist.list').write_text('Fixture-Flagged\n') + log_path = tmp_path / 'server.log' + log_before = len(log_path.read_text()) + before = len(registry.calls) + result = install(server, 'fixture-flagged', '2.0.0') + if tree == 'legacy': + assert result['failed'] == ['fixture-flagged'], result + else: + assert result['history']['result'] == 'failed', result + assert 'normal or below' in log_path.read_text()[log_before:] + assert not (tmp_path / 'custom_nodes/fixture-flagged').exists() + assert not any('/install' in call or call.startswith('/zip/') for call in registry.calls[before:]) + + +@pytest.mark.parametrize('tree', ['legacy', 'glob']) +@pytest.mark.parametrize('keep_listed', [False, True]) +def test_restart_rechecks_whitelist(tmp_path, registry, tree, keep_listed): + whitelist = tmp_path / 'user/__manager/nodepack-whitelist.list' + marker = tmp_path / 'custom_nodes/fixture-switch/installed.txt' + with running_server(tmp_path, tree, '0.0.0.0', False, 'normal', registry) as server: + whitelist.write_text('Fixture-Switch\n') + install(server, 'fixture-switch', '1.0.0') + install(server, 'fixture-switch', '2.0.0') + reserved = tmp_path / 'user/__manager/startup-scripts/install-scripts.txt' + assert 'NodeVersionStatusFlagged' in reserved.read_text() + assert marker.read_text() == '1.0.0' + + if not keep_listed: + whitelist.write_text('fixture-other\n') + before = len(registry.calls) + with running_server(tmp_path, tree, '0.0.0.0', False, 'normal', registry): + assert whitelist.read_text() == ('Fixture-Switch\n' if keep_listed else 'fixture-other\n') + calls = registry.calls[before:] + assert not any('/install' in call for call in calls), calls + assert any(call.startswith('/zip/') for call in calls) is keep_listed, calls + assert marker.read_text() == ('2.0.0' if keep_listed else '1.0.0') + + +@pytest.mark.parametrize('tree', ['legacy', 'glob']) +def test_scheduled_snapshot_uses_whitelist(tmp_path, registry, tree): + config_dir = tmp_path / 'user/__manager' + startup_dir = config_dir / 'startup-scripts' + startup_dir.mkdir(parents=True) + (config_dir / 'nodepack-whitelist.list').write_text('Fixture-Flagged\n') + reservation = startup_dir / 'restore-snapshot.json' + reservation.write_text(json.dumps({ + 'cnr_custom_nodes': {'fixture-flagged': '2.0.0', 'fixture-latest': '2.0.0'}, + 'git_custom_nodes': {}, 'file_custom_nodes': [], + })) + + before = len(registry.calls) + with running_server(tmp_path, tree, '0.0.0.0', False, 'normal', registry): + log = (tmp_path / 'server.log').read_text() + assert 'Restore snapshot done.' in log, log[-6000:] + assert 'Restore snapshot failed.' not in log, log[-6000:] + assert not reservation.exists() + installed = tmp_path / 'custom_nodes/fixture-flagged' + assert 'version = "2.0.0"' in (installed / 'pyproject.toml').read_text() + assert not (tmp_path / 'custom_nodes/fixture-latest').exists() + assert "CNR installation of 'fixture-flagged' permitted by nodepack whitelist" in log, log[-6000:] + assert "CNR installation of 'fixture-latest' permitted by nodepack whitelist" not in log + calls = registry.calls[before:] + assert sum('/install' in call for call in calls) == 2, calls + # torchvision may probe redirects before downloading the same archive. + assert {call for call in calls if call.startswith('/zip/')} == {'/zip/fixture-flagged/2.0.0'} + + @pytest.mark.parametrize('operation', ['install', 'update']) def test_flagged_transition_preserves_active_version(comfy_server, operation): server = comfy_server diff --git a/tests/e2e/verify_live_flagged_whitelist.py b/tests/e2e/verify_live_flagged_whitelist.py new file mode 100644 index 000000000..04495b109 --- /dev/null +++ b/tests/e2e/verify_live_flagged_whitelist.py @@ -0,0 +1,125 @@ +"""Opt-in production Registry/CDN verification; never collected by pytest. + +Requires E2E_ROOT with comfyui/ and venv/bin/python, as for the local E2E suite. +Pass two explicitly chosen ID@version values via --listed and --unlisted, and a +new --output directory. Both versions must currently be Flagged. The listed +archive must have no install.py or requirements.txt. Servers use isolated data +directories, the normal installation path, and do not reload installed nodes. +""" + +import argparse +import hashlib +import json +import subprocess +import zipfile +from datetime import datetime, timezone +from pathlib import Path +from types import SimpleNamespace +from urllib.parse import quote + +import requests +import test_e2e_flagged_nodepacks as harness + +REGISTRY = 'https://api.comfy.org' + + +def get_json(path, **params): + response = requests.get(REGISTRY + path, params=params, timeout=30) + response.raise_for_status() + return response.json() + + +def main(): + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument('--listed', required=True, metavar='ID@VERSION') + parser.add_argument('--unlisted', required=True, metavar='ID@VERSION') + parser.add_argument('--output', required=True, type=Path) + args = parser.parse_args() + if not (harness.E2E_ROOT / 'venv/bin/python').is_file(): + parser.error('E2E_ROOT must point to the ComfyUI + venv fixture') + packs = [] + for value in (args.listed, args.unlisted): + node, separator, version = value.rpartition('@') + if not node or not separator or not version: + parser.error('Specify each pack as ID@VERSION') + catalog = get_json('/nodes/' + quote(node, safe='')) + assert catalog['id'] == node, catalog + packs.append((catalog, version)) + assert packs[0][0]['id'].lower() != packs[1][0]['id'].lower(), 'Choose two different packs' + output = args.output.resolve() + output.mkdir(parents=True) + catalog = {'nodes': [pack for pack, _ in packs], 'totalPages': 1} + catalog_path = output / 'catalog.json' + catalog_path.write_text(json.dumps(catalog, indent=2), encoding='utf-8') + # Seed only the catalog cache from live /nodes/{id} responses. Installation + # metadata and ZIP downloads still go directly to the production services. + registry = SimpleNamespace(url=REGISTRY, catalog_url=catalog_path.as_uri()) + results = [] + + def check(server, label, pack, version, allowed): + node = pack['id'] + metadata = get_json('/nodes/' + quote(node, safe='') + '/install', version=version) + assert metadata['status'] == 'NodeVersionStatusFlagged', metadata + assert metadata['node_id'] == node and metadata['version'] == version, metadata + expected = {} + if allowed: + response = requests.get(metadata['downloadUrl'], timeout=60) + response.raise_for_status() + archive_path = server.root / 'cdn.zip' + archive_path.write_bytes(response.content) + with zipfile.ZipFile(archive_path) as archive: + assert 'install.py' not in archive.namelist(), 'Choose a pack without an install script' + assert 'requirements.txt' not in archive.namelist(), 'Choose a pack without pip requirements' + for member in archive.infolist(): + if not member.is_dir(): + expected[member.filename] = hashlib.sha256(archive.read(member)).hexdigest() + assert expected, 'CDN archive is empty' + log_path = server.root / 'server.log' + offset = len(log_path.read_text()) + queue = harness.install(server, node, version, repository=pack['repository']) + installed = server.root / 'custom_nodes' / node + assert installed.exists() is allowed, queue + if allowed: + if server.tree == 'legacy': + assert list(queue['nodepack_result'].values()) == ['success'], queue + else: + assert queue['history']['result'] == 'success', queue + for name, digest in expected.items(): + assert hashlib.sha256((installed / name).read_bytes()).hexdigest() == digest, name + assert (installed / '.tracking').is_file() + else: + harness.assert_flagged_denial(queue, log_path.read_text()[offset:]) + assert not any(installed.parent.glob('CNR_temp_*')) + results.append({'tree': server.tree, 'label': label, + 'observed_at': datetime.now(timezone.utc).isoformat(), + 'registry_response': metadata, 'queue_response': queue, + 'installed': installed.exists(), 'matched_archive_files': expected}) + (output / 'results.json').write_text(json.dumps(results, indent=2), encoding='utf-8') + print(f'{server.tree} {label}: installed={installed.exists()}, verified_files={len(expected)}', flush=True) + + for tree in ('glob', 'legacy'): + root = output / tree + root.mkdir() + with harness.running_server(root, tree, '0.0.0.0', False, 'normal', registry) as server: + whitelist = root / 'user/__manager/nodepack-whitelist.list' + template = whitelist.read_text() + assert template.strip() and all(line.lstrip().startswith('#') for line in template.splitlines() if line.strip()) + listed, version = packs[0] + check(server, 'default-template', listed, version, False) + whitelist.write_text(listed['id'].swapcase() + '\n', encoding='utf-8') + check(server, 'listed-different-case', listed, version, True) + check(server, 'unlisted-control', *packs[1], False) + + manifest = { + 'tested_commit': subprocess.check_output( + ['git', 'rev-parse', 'HEAD'], cwd=harness.REPO_ROOT, text=True).strip(), + 'completed_cases': len(results), 'registry': REGISTRY, + 'archive_sha256': {str(path.relative_to(output)): hashlib.sha256(path.read_bytes()).hexdigest() + for path in output.glob('*/cdn.zip')}, + } + (output / 'manifest.json').write_text(json.dumps(manifest, indent=2), encoding='utf-8') + print(f'PASS: {len(results)} live Registry cases; evidence: {output}', flush=True) + + +if __name__ == '__main__': + main() diff --git a/tests/test_flagged_nodepack_policy.py b/tests/test_flagged_nodepack_policy.py index 576a36502..b784264cc 100644 --- a/tests/test_flagged_nodepack_policy.py +++ b/tests/test_flagged_nodepack_policy.py @@ -16,6 +16,14 @@ import pytest from _install_flags_testutil import import_context, import_reader +@pytest.fixture +def security(): + import_context() + from comfyui_manager.common import manager_security + + return manager_security + + @pytest.fixture def cnr_client(monkeypatch): import_context() @@ -29,7 +37,7 @@ def cnr_client(monkeypatch): @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.manager_funcs, 'is_flagged_install_allowed', lambda node_id: 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 @@ -42,13 +50,29 @@ def api_response(status='NodeVersionStatusFlagged', http=200): }) -@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): +@pytest.mark.parametrize('permission,listed_id,allowed,whitelist_logged', [ + (None, None, True, False), ('true', 'FiXtUrE', True, False), ('false', None, False, False), + ('false', 'FiXtUrE', True, True), ('false', 'other', False, False), +]) +def test_cli_install_uses_parent_policy( + core, monkeypatch, caplog, permission, listed_id, allowed, whitelist_logged, tmp_path, +): from comfy.cli_args import args + # Restore children use Python's default WARNING threshold. + caplog.set_level(logging.WARNING) monkeypatch.delenv('_COMFYUI_MANAGER_CNR_ALLOW_FLAGGED', raising=False) if permission is not None: monkeypatch.setenv('_COMFYUI_MANAGER_CNR_ALLOW_FLAGGED', permission) + monkeypatch.delenv('_COMFYUI_MANAGER_CNR_WHITELIST_DIR', raising=False) + if listed_id is not None: + parent_dir = tmp_path / 'parent-manager' + parent_dir.mkdir() + (parent_dir / 'nodepack-whitelist.list').write_text(listed_id + '\n') + monkeypatch.setenv('_COMFYUI_MANAGER_CNR_WHITELIST_DIR', str(parent_dir)) + # The child's local config cannot expand the server's permission. + monkeypatch.setattr(core.context, 'manager_files_path', str(tmp_path)) + (tmp_path / 'nodepack-whitelist.list').write_text('fixture\n') 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'}) @@ -65,6 +89,12 @@ def test_cli_install_uses_only_parent_permission(core, monkeypatch, permission, assert result.result is allowed, result.msg assert get.call_count == 1 assert download.call_count == int(allowed) + records = [record for record in caplog.records if 'permitted by nodepack whitelist' in record.getMessage()] + assert len(records) == int(whitelist_logged) + if whitelist_logged: + assert records[0].levelno == logging.WARNING + assert "'fixture'" in records[0].getMessage() + assert str(parent_dir / 'nodepack-whitelist.list') in records[0].getMessage() @pytest.mark.parametrize('listen,allowed', [ @@ -73,25 +103,131 @@ def test_cli_install_uses_only_parent_permission(core, monkeypatch, permission, ('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_listener_policy(security, listen, allowed): + assert security.is_cnr_install_allowed('NodeVersionStatusFlagged', False, listen) is allowed + assert security.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 test_hostname_requires_all_resolved_addresses_to_be_loopback(security, monkeypatch): 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') + assert security.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') + assert not security.is_loopback_listener('localhost') monkeypatch.setattr(socket, 'getaddrinfo', Mock(side_effect=socket.gaierror)) - assert not is_loopback_listener('invalid.invalid') + assert not security.is_loopback_listener('invalid.invalid') + + +@pytest.mark.parametrize('contents,node_id,allowed', [ + (None, 'fixture', False), ('', 'fixture', False), ('# fixture\n', 'fixture', False), + ('fixture\n', 'fixture', True), ('\ufefffixture\n', 'fixture', True), + (' \tfixture \r\n', 'fixture', True), ('fixture # approved\n', 'fixture', True), + ('Fixture\n', 'fixture', True), ('fixture\n', 'FIXTURE', True), + ('FiXtUrE\n', 'fIxTuRe', True), ('fixture-other\n', 'fixture', False), + ('fixture@2.0.0\n', 'fixture', False), ('*\nfixture-*\n', 'fixture', False), + ('https://example.invalid/fixture\n', 'fixture', False), +]) +def test_whitelist_matches_pack_ids(security, tmp_path, contents, node_id, allowed): + path = tmp_path / 'nodepack-whitelist.list' + if contents is not None: + path.write_text(contents, encoding='utf-8') + assert security.is_flagged_nodepack_whitelisted(node_id, tmp_path) is allowed + + +def test_whitelist_template_grants_no_exceptions(security, tmp_path): + manager_dir = tmp_path / 'user/__manager' + security.ensure_nodepack_whitelist(manager_dir) + contents = (manager_dir / 'nodepack-whitelist.list').read_text() + assert contents.strip() and all(line.lstrip().startswith('#') for line in contents.splitlines() if line.strip()) + assert not security.is_flagged_nodepack_whitelisted('comfyui-impact-pack', manager_dir) + assert not security.is_flagged_nodepack_whitelisted('comfyui-inspire-pack', manager_dir) + + +@pytest.mark.parametrize('contents', [b'', b'# Keep this comment\nFiXtUrE\n']) +def test_whitelist_template_preserves_existing_file(security, tmp_path, caplog, contents): + path = tmp_path / 'nodepack-whitelist.list' + path.write_bytes(contents) + security.ensure_nodepack_whitelist(tmp_path) + assert path.read_bytes() == contents + assert 'Cannot create nodepack whitelist' not in caplog.text + + +def test_whitelist_template_write_failure_is_logged(security, tmp_path, monkeypatch, caplog): + monkeypatch.setattr(security, 'open', Mock(side_effect=PermissionError('read-only')), raising=False) + security.ensure_nodepack_whitelist(tmp_path) + assert not (tmp_path / 'nodepack-whitelist.list').exists() + assert 'Cannot create nodepack whitelist' in caplog.text + + +@pytest.mark.parametrize('failure', ['manager-file', 'dangling-symlink', 'whitelist-directory']) +def test_whitelist_template_path_failure_is_logged(security, tmp_path, caplog, failure): + manager_dir = tmp_path / 'manager' + path = manager_dir / 'nodepack-whitelist.list' + if failure == 'manager-file': + manager_dir.write_text('existing file') + else: + manager_dir.mkdir() + if failure == 'dangling-symlink': + try: + path.symlink_to(manager_dir / 'missing-target') + except (OSError, NotImplementedError) as exc: + pytest.skip(f'Symlinks unavailable: {exc}') + else: + path.mkdir() + + security.ensure_nodepack_whitelist(manager_dir) + + assert 'Cannot create nodepack whitelist' in caplog.text + assert str(path) in caplog.text + if failure == 'manager-file': + assert manager_dir.read_text() == 'existing file' + elif failure == 'dangling-symlink': + assert path.is_symlink() and not path.exists() + else: + assert path.is_dir() + + +@pytest.mark.parametrize('prefix,suffix', [ + (b'', b''), + (b'', b'comfyui-impact-pack\n'), + (b'comfyui-impact-pack\n' + b'# comment\n' * 2048, b''), +], ids=['utf16-appended', 'utf8-after-utf16', 'match-before-invalid-encoding']) +def test_mixed_encoding_whitelist_is_rejected(security, tmp_path, caplog, prefix, suffix): + security.ensure_nodepack_whitelist(tmp_path) + path = tmp_path / 'nodepack-whitelist.list' + with path.open('ab') as file: + file.write(prefix + 'comfyui-impact-pack\r\n'.encode('utf-16-le') + suffix) + + assert not security.is_flagged_nodepack_whitelisted('comfyui-impact-pack', tmp_path) + assert 'NUL' in caplog.text and 'UTF-8' in caplog.text + assert str(path) in caplog.text + assert 'permitted by nodepack whitelist' not in caplog.text + + +def test_whitelist_reloads_edits_and_removal(security, tmp_path): + path = tmp_path / 'nodepack-whitelist.list' + path.write_text('fixture\n', encoding='utf-8') + assert security.is_flagged_nodepack_whitelisted('fixture', tmp_path) + path.write_text('other\n', encoding='utf-8') + assert not security.is_flagged_nodepack_whitelisted('fixture', tmp_path) + assert security.is_flagged_nodepack_whitelisted('other', tmp_path) + path.unlink() + assert not security.is_flagged_nodepack_whitelisted('other', tmp_path) + + +@pytest.mark.parametrize('failure', ['encoding', 'directory']) +def test_unreadable_whitelist_grants_no_exceptions(security, tmp_path, caplog, failure): + path = tmp_path / 'nodepack-whitelist.list' + if failure == 'encoding': + # The invalid byte is beyond the first read buffer. A matching first + # line must not hide a decoding failure later in the file. + path.write_bytes(b'fixture\n' + b'# comment\n' * 2048 + b'\xff') + else: + path.mkdir() + assert not security.is_flagged_nodepack_whitelisted('fixture', tmp_path) + assert 'Cannot read flagged nodepack whitelist' in caplog.text @pytest.mark.parametrize('status,permission,allowed', [ @@ -101,7 +237,7 @@ def test_hostname_requires_all_resolved_addresses_to_be_loopback(cnr_client, mon 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) + monkeypatch.setattr(core.manager_funcs, 'is_flagged_install_allowed', lambda node_id: 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']) @@ -126,7 +262,7 @@ def test_install_uses_status_from_one_response(core, monkeypatch, tmp_path, capl @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) + monkeypatch.setattr(core.manager_funcs, 'is_flagged_install_allowed', lambda node_id: 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')) @@ -383,14 +519,27 @@ def test_git_reinstall_replaces_existing_pack(core, monkeypatch, tmp_path, versi 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): +@pytest.mark.parametrize('status,override,listen,whitelist,allowed', [ + ('NodeVersionStatusActive', False, '0.0.0.0', '', True), + ('NodeVersionStatusFlagged', False, '127.0.0.1', '', True), + ('NodeVersionStatusFlagged', True, '0.0.0.0', '', True), + ('NodeVersionStatusFlagged', False, '0.0.0.0', '', False), + ('NodeVersionStatusFlagged', False, '0.0.0.0', 'FiXtUrE\n', True), + ('NodeVersionStatusFlagged', False, '0.0.0.0', 'fixture-other\n', False), + ('', False, '127.0.0.1', '', True), + ('', True, '0.0.0.0', '', True), + ('', False, '0.0.0.0', '', False), + ('', False, '0.0.0.0', 'FiXtUrE\n', True), + ('', False, '0.0.0.0', 'fixture-other\n', False), +]) +def test_deferred_switch_reuses_status_with_current_policy( + cnr_client, monkeypatch, tmp_path, caplog, status, override, listen, whitelist, allowed, +): from comfy.cli_args import args as server_args - from comfyui_manager.common import manager_downloader, manager_util + from comfyui_manager.common import manager_downloader, manager_security, manager_util monkeypatch.setattr(server_args, 'listen', listen) + (tmp_path / 'nodepack-whitelist.list').write_text(whitelist) # 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()) @@ -398,6 +547,8 @@ def test_deferred_switch_reuses_status_with_current_policy(cnr_client, monkeypat and node.name == 'execute_lazy_cnr_switch') scope = {'__package__': 'comfyui_manager', 'os': os, 'logging': logging, 'manager_downloader': manager_downloader, 'manager_util': manager_util, + 'manager_security': manager_security, + 'manager_files_path': str(tmp_path), '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' @@ -415,7 +566,6 @@ def test_deferred_switch_reuses_status_with_current_policy(cnr_client, monkeypat 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