From 3939835401eceaa0fd7e502aef564c1f1e702803 Mon Sep 17 00:00:00 2001 From: "Dr.Lt.Data" Date: Wed, 11 Nov 2026 12:59:47 +0900 Subject: [PATCH] feat(v4): allow selected flagged CNR nodepacks Read nodepack-whitelist.list from the Manager configuration directory to permit listed Registry IDs case-insensitively without enabling all flagged installs. Create a commented template when missing and preserve existing files. Apply the list to both server implementations, deferred version switches, and scheduled snapshot restores. Document the format and cover admission, revocation, template initialization, and existing security restrictions. Pin the code-quality runner to Ubuntu 24.04, which provides Python 3.9 builds. --- .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