From 49638f11c8cbb84bb1aed6b67943accbe984f894 Mon Sep 17 00:00:00 2001 From: John Brisbin Date: Wed, 1 Jul 2026 12:45:58 -0500 Subject: [PATCH 1/5] feat: add env var support for backend location in the comfy tool - COMFY_BACKEND=host:port (one variable, colon-separated) - COMFY_HOST and COMFY_PORT (separate variables) These provide fallback when --host/--port are not passed on the command line. Resolves the missing environment variable oversight for backend configuration. --- comfy_cli/cmdline.py | 38 ++++++++++++++++++++++++++++++++++++-- comfy_cli/command/jobs.py | 31 +++++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+), 2 deletions(-) diff --git a/comfy_cli/cmdline.py b/comfy_cli/cmdline.py index 3bae27dd..93e09f9c 100644 --- a/comfy_cli/cmdline.py +++ b/comfy_cli/cmdline.py @@ -44,6 +44,33 @@ from comfy_cli.command.models import models as models_command from comfy_cli.command.models import search as models_search_command from comfy_cli.config_manager import ConfigManager + +def _get_backend_from_env(): + """Resolve host/port from environment variables as requested. + - COMFY_BACKEND=host:port (combined, colon-separated) + - COMFY_HOST and COMFY_PORT (separate) + CLI flags take precedence; this is a fallback. + """ + import os + be = os.environ.get("COMFY_BACKEND") + if be: + b = be.replace("http://", "").replace("https://", "").strip("/") + if ":" in b: + h, p = b.rsplit(":", 1) + try: + return h, int(p) + except ValueError: + return h, None + return b, None + h = os.environ.get("COMFY_HOST") + p = os.environ.get("COMFY_PORT") + if h or p: + try: + return h, int(p) if p else None + except ValueError: + return h, None + return None, None + from comfy_cli.constants import GPU_OPTION, CUDAVersion, ROCmVersion from comfy_cli.cuda_detect import DEFAULT_CUDA_TAG, detect_cuda_driver_version, resolve_cuda_wheel from comfy_cli.discovery import build_discovery @@ -825,6 +852,13 @@ def run( if not port: port = bg_port + # env var support (COMFY_BACKEND or COMFY_HOST/COMFY_PORT) + env_h, env_p = _get_backend_from_env() + if not host and env_h: + host = env_h + if not port and env_p is not None: + port = env_p + if not host: host = "127.0.0.1" if not port: @@ -875,11 +909,11 @@ def validate( ] = None, host: Annotated[ str | None, - typer.Option(show_default=False, help="ComfyUI host (default 127.0.0.1)."), + typer.Option(show_default=False, help="ComfyUI host (default 127.0.0.1).", envvar="COMFY_HOST"), ] = None, port: Annotated[ int | None, - typer.Option(show_default=False, help="ComfyUI port (default 8188)."), + typer.Option(show_default=False, help="ComfyUI port (default 8188).", envvar="COMFY_PORT"), ] = None, input_path: Annotated[ str | None, diff --git a/comfy_cli/command/jobs.py b/comfy_cli/command/jobs.py index 1a78d8e8..afec769d 100644 --- a/comfy_cli/command/jobs.py +++ b/comfy_cli/command/jobs.py @@ -35,6 +35,29 @@ from comfy_cli import cancellation, execution_errors, tracking from comfy_cli.config_manager import ConfigManager + +def _get_backend_from_env(): + """Resolve host/port from environment variables (COMFY_BACKEND or COMFY_HOST/COMFY_PORT).""" + import os + be = os.environ.get("COMFY_BACKEND") + if be: + b = be.replace("http://", "").replace("https://", "").strip("/") + if ":" in b: + h, p = b.rsplit(":", 1) + try: + return h, int(p) + except ValueError: + return h, None + return b, None + h = os.environ.get("COMFY_HOST") + p = os.environ.get("COMFY_PORT") + if h or p: + try: + return h, int(p) if p else None + except ValueError: + return h, None + return None, None + from comfy_cli.env_checker import check_comfy_server_running from comfy_cli.output import get_renderer @@ -81,6 +104,14 @@ def _resolve_host_port(host: str | None, port: int | None) -> tuple[str, int]: host = bg[0] if not port and bg is not None: port = bg[1] + + # env var support (COMFY_BACKEND or COMFY_HOST/COMFY_PORT) + env_h, env_p = _get_backend_from_env() + if not host and env_h: + host = env_h + if not port and env_p is not None: + port = env_p + h = _validate_host(host or DEFAULT_HOST) # Bracket IPv6 literals so callers building "http://{host}:{port}" / # "ws://{host}:{port}" produce valid URLs (e.g. "::1" -> "[::1]"). From 77052aa07483c3c1ff3eb72b2c568b8158791e8f Mon Sep 17 00:00:00 2001 From: John Brisbin Date: Wed, 1 Jul 2026 15:25:36 -0500 Subject: [PATCH 2/5] feat: add full support for COMFY_HOST, COMFY_PORT, COMFY_HOST_PORT and COMFY_BACKEND env vars - Extend _get_backend_from_env to handle COMFY_HOST_PORT (and keep COMFY_BACKEND for combined form) - Update docstrings and usage in cmdline.py and jobs.py - CLI flags still take precedence as before - This matches the requested env var naming (tracking CLI params closely) --- comfy_cli/cmdline.py | 10 ++++++++++ comfy_cli/command/jobs.py | 16 +++++++++++++++- 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/comfy_cli/cmdline.py b/comfy_cli/cmdline.py index 93e09f9c..76e77ae7 100644 --- a/comfy_cli/cmdline.py +++ b/comfy_cli/cmdline.py @@ -48,6 +48,7 @@ def _get_backend_from_env(): """Resolve host/port from environment variables as requested. - COMFY_BACKEND=host:port (combined, colon-separated) + - COMFY_HOST_PORT=host:port (combined, colon-separated) - COMFY_HOST and COMFY_PORT (separate) CLI flags take precedence; this is a fallback. """ @@ -62,6 +63,15 @@ def _get_backend_from_env(): except ValueError: return h, None return b, None + hp = os.environ.get("COMFY_HOST_PORT") + if hp: + if ":" in hp: + h, p = hp.rsplit(":", 1) + try: + return h, int(p) + except ValueError: + return h, None + return hp, None h = os.environ.get("COMFY_HOST") p = os.environ.get("COMFY_PORT") if h or p: diff --git a/comfy_cli/command/jobs.py b/comfy_cli/command/jobs.py index afec769d..75fcef21 100644 --- a/comfy_cli/command/jobs.py +++ b/comfy_cli/command/jobs.py @@ -37,7 +37,12 @@ from comfy_cli.config_manager import ConfigManager def _get_backend_from_env(): - """Resolve host/port from environment variables (COMFY_BACKEND or COMFY_HOST/COMFY_PORT).""" + """Resolve host/port from environment variables as requested. + - COMFY_BACKEND=host:port (combined, colon-separated) + - COMFY_HOST_PORT=host:port (combined, colon-separated) + - COMFY_HOST and COMFY_PORT (separate) + CLI flags take precedence; this is a fallback. + """ import os be = os.environ.get("COMFY_BACKEND") if be: @@ -49,6 +54,15 @@ def _get_backend_from_env(): except ValueError: return h, None return b, None + hp = os.environ.get("COMFY_HOST_PORT") + if hp: + if ":" in hp: + h, p = hp.rsplit(":", 1) + try: + return h, int(p) + except ValueError: + return h, None + return hp, None h = os.environ.get("COMFY_HOST") p = os.environ.get("COMFY_PORT") if h or p: From f16e8021212a942c389a89025ccc4cf8d3b22c51 Mon Sep 17 00:00:00 2001 From: John Brisbin Date: Wed, 1 Jul 2026 15:56:09 -0500 Subject: [PATCH 3/5] fix: treat COMFY_HOST_PORT as equivalent to COMFY_BACKEND in resolver --- comfy_cli/cmdline.py | 14 ++------------ comfy_cli/command/jobs.py | 14 ++------------ 2 files changed, 4 insertions(+), 24 deletions(-) diff --git a/comfy_cli/cmdline.py b/comfy_cli/cmdline.py index 76e77ae7..df0c84d3 100644 --- a/comfy_cli/cmdline.py +++ b/comfy_cli/cmdline.py @@ -47,13 +47,12 @@ def _get_backend_from_env(): """Resolve host/port from environment variables as requested. - - COMFY_BACKEND=host:port (combined, colon-separated) - - COMFY_HOST_PORT=host:port (combined, colon-separated) + - COMFY_BACKEND=host:port or COMFY_HOST_PORT=host:port (combined, colon-separated) - COMFY_HOST and COMFY_PORT (separate) CLI flags take precedence; this is a fallback. """ import os - be = os.environ.get("COMFY_BACKEND") + be = os.environ.get("COMFY_BACKEND") or os.environ.get("COMFY_HOST_PORT") if be: b = be.replace("http://", "").replace("https://", "").strip("/") if ":" in b: @@ -63,15 +62,6 @@ def _get_backend_from_env(): except ValueError: return h, None return b, None - hp = os.environ.get("COMFY_HOST_PORT") - if hp: - if ":" in hp: - h, p = hp.rsplit(":", 1) - try: - return h, int(p) - except ValueError: - return h, None - return hp, None h = os.environ.get("COMFY_HOST") p = os.environ.get("COMFY_PORT") if h or p: diff --git a/comfy_cli/command/jobs.py b/comfy_cli/command/jobs.py index 75fcef21..bb244d85 100644 --- a/comfy_cli/command/jobs.py +++ b/comfy_cli/command/jobs.py @@ -38,13 +38,12 @@ def _get_backend_from_env(): """Resolve host/port from environment variables as requested. - - COMFY_BACKEND=host:port (combined, colon-separated) - - COMFY_HOST_PORT=host:port (combined, colon-separated) + - COMFY_BACKEND=host:port or COMFY_HOST_PORT=host:port (combined, colon-separated) - COMFY_HOST and COMFY_PORT (separate) CLI flags take precedence; this is a fallback. """ import os - be = os.environ.get("COMFY_BACKEND") + be = os.environ.get("COMFY_BACKEND") or os.environ.get("COMFY_HOST_PORT") if be: b = be.replace("http://", "").replace("https://", "").strip("/") if ":" in b: @@ -54,15 +53,6 @@ def _get_backend_from_env(): except ValueError: return h, None return b, None - hp = os.environ.get("COMFY_HOST_PORT") - if hp: - if ":" in hp: - h, p = hp.rsplit(":", 1) - try: - return h, int(p) - except ValueError: - return h, None - return hp, None h = os.environ.get("COMFY_HOST") p = os.environ.get("COMFY_PORT") if h or p: From f94aa731bd3435d7296033a0a1167842c92eb74d Mon Sep 17 00:00:00 2001 From: johnwbrisbin Date: Sun, 5 Jul 2026 03:18:31 -0500 Subject: [PATCH 4/5] refactor: extract _get_backend_from_env to shared env_backend module - Extract duplicate helper from cmdline.py and jobs.py into comfy_cli/env_backend.py - Add IPv6 bracketed host support ([::1]:8188) - Log warning on invalid port values instead of silent swallow - Remove local 'import os' (was Pylint W0621/W0404/C0415) - Route validate() through same env fallback as run() for feature parity - Remove Typer envvar= bindings from validate (was COMFY_HOST/COMFY_PORT only) Addresses CodeRabbit review feedback on PR #478. --- comfy_cli/cmdline.py | 38 ++++---------- comfy_cli/command/jobs.py | 27 +--------- comfy_cli/env_backend.py | 102 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 113 insertions(+), 54 deletions(-) create mode 100644 comfy_cli/env_backend.py diff --git a/comfy_cli/cmdline.py b/comfy_cli/cmdline.py index df0c84d3..20e4afcb 100644 --- a/comfy_cli/cmdline.py +++ b/comfy_cli/cmdline.py @@ -44,32 +44,7 @@ from comfy_cli.command.models import models as models_command from comfy_cli.command.models import search as models_search_command from comfy_cli.config_manager import ConfigManager - -def _get_backend_from_env(): - """Resolve host/port from environment variables as requested. - - COMFY_BACKEND=host:port or COMFY_HOST_PORT=host:port (combined, colon-separated) - - COMFY_HOST and COMFY_PORT (separate) - CLI flags take precedence; this is a fallback. - """ - import os - be = os.environ.get("COMFY_BACKEND") or os.environ.get("COMFY_HOST_PORT") - if be: - b = be.replace("http://", "").replace("https://", "").strip("/") - if ":" in b: - h, p = b.rsplit(":", 1) - try: - return h, int(p) - except ValueError: - return h, None - return b, None - h = os.environ.get("COMFY_HOST") - p = os.environ.get("COMFY_PORT") - if h or p: - try: - return h, int(p) if p else None - except ValueError: - return h, None - return None, None +from comfy_cli.env_backend import get_backend_from_env as _get_backend_from_env from comfy_cli.constants import GPU_OPTION, CUDAVersion, ROCmVersion from comfy_cli.cuda_detect import DEFAULT_CUDA_TAG, detect_cuda_driver_version, resolve_cuda_wheel @@ -909,11 +884,11 @@ def validate( ] = None, host: Annotated[ str | None, - typer.Option(show_default=False, help="ComfyUI host (default 127.0.0.1).", envvar="COMFY_HOST"), + typer.Option(show_default=False, help="ComfyUI host (default 127.0.0.1)."), ] = None, port: Annotated[ int | None, - typer.Option(show_default=False, help="ComfyUI port (default 8188).", envvar="COMFY_PORT"), + typer.Option(show_default=False, help="ComfyUI port (default 8188)."), ] = None, input_path: Annotated[ str | None, @@ -954,6 +929,13 @@ def validate( except Exception: pass + # env var fallback (same resolution as `comfy run`) + env_h, env_p = _get_backend_from_env() + if not host and env_h: + host = env_h + if not port and env_p is not None: + port = env_p + try: graph = Graph.load(mode=mode, input_path=input_path, host=host or "127.0.0.1", port=port or 8188) except LoadError as e: diff --git a/comfy_cli/command/jobs.py b/comfy_cli/command/jobs.py index bb244d85..d7f8a0de 100644 --- a/comfy_cli/command/jobs.py +++ b/comfy_cli/command/jobs.py @@ -35,32 +35,7 @@ from comfy_cli import cancellation, execution_errors, tracking from comfy_cli.config_manager import ConfigManager - -def _get_backend_from_env(): - """Resolve host/port from environment variables as requested. - - COMFY_BACKEND=host:port or COMFY_HOST_PORT=host:port (combined, colon-separated) - - COMFY_HOST and COMFY_PORT (separate) - CLI flags take precedence; this is a fallback. - """ - import os - be = os.environ.get("COMFY_BACKEND") or os.environ.get("COMFY_HOST_PORT") - if be: - b = be.replace("http://", "").replace("https://", "").strip("/") - if ":" in b: - h, p = b.rsplit(":", 1) - try: - return h, int(p) - except ValueError: - return h, None - return b, None - h = os.environ.get("COMFY_HOST") - p = os.environ.get("COMFY_PORT") - if h or p: - try: - return h, int(p) if p else None - except ValueError: - return h, None - return None, None +from comfy_cli.env_backend import get_backend_from_env as _get_backend_from_env from comfy_cli.env_checker import check_comfy_server_running from comfy_cli.output import get_renderer diff --git a/comfy_cli/env_backend.py b/comfy_cli/env_backend.py new file mode 100644 index 00000000..a1a356fa --- /dev/null +++ b/comfy_cli/env_backend.py @@ -0,0 +1,102 @@ +"""Shared environment-variable backend resolver for ComfyUI host/port. + +Supports four env vars (highest to lowest precedence within the env layer): + COMFY_BACKEND=host:port (combined, colon-separated) + COMFY_HOST_PORT=host:port (combined, same as COMFY_BACKEND) + COMFY_HOST + COMFY_PORT (separate) + +CLI flags always take precedence over env vars; this helper is a fallback. +""" +from __future__ import annotations + +import os +import re +from typing import Optional + +# IPv6 address detection: contains multiple colons or starts with [ +_IPV6_RE = re.compile(r"^\[.+\]$|^([0-9a-fA-F]{0,4}:){2,}") + + +def get_backend_from_env() -> tuple[Optional[str], Optional[int]]: + """Resolve host/port from environment variables. + + Returns (host, port) where either may be None if not set. + Handles IPv6 bracketed hosts (e.g. ``[::1]:8188``). + Logs a warning on invalid port values instead of silently swallowing. + """ + be = os.environ.get("COMFY_BACKEND") or os.environ.get("COMFY_HOST_PORT") + if be: + b = be.replace("http://", "").replace("https://", "").strip("/") + host, port = _split_host_port(b) + if port is not None and port is not None and not _valid_port(port): + import logging + logging.getLogger(__name__).warning( + "Invalid port %r in COMFY_BACKEND/COMFY_HOST_PORT, ignoring", port + ) + return host, None + return host, port + + h = os.environ.get("COMFY_HOST") + p = os.environ.get("COMFY_PORT") + if h or p: + port: Optional[int] = None + if p: + try: + port = int(p) + except ValueError: + import logging + logging.getLogger(__name__).warning( + "Invalid port %r in COMFY_PORT, ignoring", p + ) + return h, None + if not _valid_port(port): + logging.getLogger(__name__).warning( + "Port %d out of range in COMFY_PORT, ignoring", port + ) + return h, None + return h, port + + return None, None + + +def _split_host_port(s: str) -> tuple[Optional[str], Optional[int]]: + """Split a host:port string, handling IPv6 bracketed hosts. + + Examples: + "127.0.0.1:8188" -> ("127.0.0.1", 8188) + "[::1]:8188" -> ("[::1]", 8188) + "[::1]" -> ("[::1]", None) + "localhost:8188" -> ("localhost", 8188) + "::1" -> ("::1", None) (bare IPv6, no port) + """ + # Bracketed IPv6: [host]:port or [host] + if s.startswith("["): + end = s.find("]") + if end == -1: + return s, None # malformed, return as-is + host = s[:end + 1] # keep brackets + rest = s[end + 1:] + if rest.startswith(":"): + try: + return host, int(rest[1:]) + except ValueError: + return host, None + return host, None + + # Bare IPv6 (multiple colons, no brackets) — no port + if s.count(":") > 1: + return s, None + + # Normal host:port + if ":" in s: + h, p = s.rsplit(":", 1) + try: + return h, int(p) + except ValueError: + return h, None + return s, None + + +def _valid_port(port: int) -> bool: + """Check if a port number is in valid range.""" + return 1 <= port <= 65535 From 9e168c2f910ab36f941c5aec13574d81b7d14d98 Mon Sep 17 00:00:00 2001 From: johnwbrisbin Date: Sun, 5 Jul 2026 10:10:39 -0500 Subject: [PATCH 5/5] =?UTF-8?q?Merge=20upstream=20main=20=E2=80=94=20integ?= =?UTF-8?q?rate=20env=20var=20support=20with=20shared=20host=5Fport=20modu?= =?UTF-8?q?le?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Conflict resolution: - Upstream extracted host/port logic into comfy_cli/host_port.py - Our env var support now lives in env_backend.py (complementary module) - resolve_host_port() calls get_backend_from_env() internally — all 6 jobs.py call sites and cmdline.py run() get env var support automatically - validate() uses inline env fallback (doesn't go through resolve_host_port) - No duplicate code — single env_backend.py, single host_port.py - IPv6 handling from upstream's host_port.py preserved --- comfy_cli/cloud/oauth.py | 12 +- comfy_cli/cmdline.py | 37 +- comfy_cli/comfy_client.py | 18 +- comfy_cli/command/install.py | 4 +- comfy_cli/command/jobs.py | 54 +- comfy_cli/command/launch.py | 3 +- comfy_cli/command/models/search.py | 37 +- comfy_cli/command/nodes.py | 13 +- comfy_cli/command/transfer.py | 11 +- comfy_cli/command/workflow.py | 479 ++++++++++++++++-- comfy_cli/command/workflow_fragments.py | 5 +- comfy_cli/constants.py | 1 + comfy_cli/cql/_net.py | 28 + comfy_cli/cql/engine.py | 25 +- comfy_cli/cql/loader.py | 23 +- comfy_cli/error_codes.py | 31 +- comfy_cli/host_port.py | 113 +++++ comfy_cli/http.py | 26 + comfy_cli/uv.py | 4 +- comfy_cli/where.py | 24 + tests/comfy_cli/auth/test_where.py | 55 ++ tests/comfy_cli/command/models/test_search.py | 30 ++ .../command/test_launch_background.py | 73 +++ .../comfy_cli/command/test_workflow_saved.py | 264 +++++++++- tests/comfy_cli/cql/test_engine.py | 12 + tests/comfy_cli/cql/test_loader.py | 31 +- tests/comfy_cli/cql/test_net.py | 27 + tests/comfy_cli/test_host_port.py | 181 +++++++ tests/comfy_cli/test_http.py | 44 ++ .../test_install_python_resolution.py | 1 + tests/uv/test_uv.py | 2 +- 31 files changed, 1431 insertions(+), 237 deletions(-) create mode 100644 comfy_cli/cql/_net.py create mode 100644 comfy_cli/host_port.py create mode 100644 comfy_cli/http.py create mode 100644 tests/comfy_cli/command/test_launch_background.py create mode 100644 tests/comfy_cli/cql/test_net.py create mode 100644 tests/comfy_cli/test_host_port.py create mode 100644 tests/comfy_cli/test_http.py diff --git a/comfy_cli/cloud/oauth.py b/comfy_cli/cloud/oauth.py index 3c86ecf8..f6459fa8 100644 --- a/comfy_cli/cloud/oauth.py +++ b/comfy_cli/cloud/oauth.py @@ -52,6 +52,7 @@ CLIENT_NAME, get_base_url, ) +from comfy_cli.http import NoRedirectHandler # --------------------------------------------------------------------------- # Error types — caller maps these to renderer.error(code=...) codes. @@ -881,16 +882,7 @@ def _assert_https_or_loopback(url: str) -> None: raise _HTTPFail(0, f"refusing plaintext HTTP for OAuth endpoint: {url}") -# Refuse redirects: an evil 302 from the token endpoint to attacker.example -# would replay the verifier + code at the redirect target. -class _NoRedirect(urllib.request.HTTPRedirectHandler): - def http_error_301(self, req, fp, code, msg, headers): - raise HTTPError(req.full_url, code, "redirect refused", headers, fp) - - http_error_302 = http_error_303 = http_error_307 = http_error_308 = http_error_301 - - -_OAUTH_OPENER = urllib.request.build_opener(_NoRedirect()) +_OAUTH_OPENER = urllib.request.build_opener(NoRedirectHandler()) def _send_and_parse(req: urllib.request.Request) -> dict: diff --git a/comfy_cli/cmdline.py b/comfy_cli/cmdline.py index 20e4afcb..e1bbd2b1 100644 --- a/comfy_cli/cmdline.py +++ b/comfy_cli/cmdline.py @@ -44,8 +44,6 @@ from comfy_cli.command.models import models as models_command from comfy_cli.command.models import search as models_search_command from comfy_cli.config_manager import ConfigManager -from comfy_cli.env_backend import get_backend_from_env as _get_backend_from_env - from comfy_cli.constants import GPU_OPTION, CUDAVersion, ROCmVersion from comfy_cli.cuda_detect import DEFAULT_CUDA_TAG, detect_cuda_driver_version, resolve_cuda_wheel from comfy_cli.discovery import build_discovery @@ -452,7 +450,7 @@ def install( ), ] = None, cuda_version: Annotated[CUDAVersion | None, typer.Option(show_default=False)] = None, - rocm_version: Annotated[ROCmVersion, typer.Option(show_default=True)] = ROCmVersion.v6_3, + rocm_version: Annotated[ROCmVersion, typer.Option(show_default=True)] = ROCmVersion.v7_2, amd: Annotated[ bool | None, typer.Option( @@ -814,30 +812,14 @@ def run( ) return + from comfy_cli.host_port import parse_host_port_arg, resolve_host_port + if host: - s = host.split(":") - host = s[0] - if not port and len(s) == 2: - port = int(s[1]) - - if config.background: - bg_host, bg_port = config.background[0], config.background[1] - if not host: - host = bg_host - if not port: - port = bg_port - - # env var support (COMFY_BACKEND or COMFY_HOST/COMFY_PORT) - env_h, env_p = _get_backend_from_env() - if not host and env_h: - host = env_h - if not port and env_p is not None: - port = env_p - - if not host: - host = "127.0.0.1" - if not port: - port = 8188 + host, parsed_port = parse_host_port_arg(host) + if not port and parsed_port is not None: + port = parsed_port + + host, port = resolve_host_port(host, port) run_inner.execute( workflow, @@ -930,7 +912,8 @@ def validate( pass # env var fallback (same resolution as `comfy run`) - env_h, env_p = _get_backend_from_env() + from comfy_cli.env_backend import get_backend_from_env + env_h, env_p = get_backend_from_env() if not host and env_h: host = env_h if not port and env_p is not None: diff --git a/comfy_cli/comfy_client.py b/comfy_cli/comfy_client.py index bcee551e..4de1eb85 100644 --- a/comfy_cli/comfy_client.py +++ b/comfy_cli/comfy_client.py @@ -23,6 +23,7 @@ from dataclasses import dataclass from typing import Any +from comfy_cli.http import NoRedirectHandler from comfy_cli.target import Target _LOOPBACK_HOSTS = {"localhost", "127.0.0.1", "::1", "[::1]"} @@ -94,22 +95,7 @@ class Unauthenticated(Exception): """Target needs auth but no valid session is present.""" -class _NoRedirectHandler(urllib.request.HTTPRedirectHandler): - """Refuse to follow redirects. - - Following redirects with `Authorization: Bearer …` in flight risks - replaying the token at the redirect target. ComfyUI gateways don't issue - redirects under normal operation; if we ever see one it's almost certainly - a misconfiguration or attack. Surface as a clear HTTPError instead. - """ - - def http_error_301(self, req, fp, code, msg, headers): - raise urllib.error.HTTPError(req.full_url, code, "redirect refused", headers, fp) - - http_error_302 = http_error_303 = http_error_307 = http_error_308 = http_error_301 - - -_OPENER = urllib.request.build_opener(_NoRedirectHandler()) +_OPENER = urllib.request.build_opener(NoRedirectHandler()) def _assert_safe_url(url: str) -> None: diff --git a/comfy_cli/command/install.py b/comfy_cli/command/install.py index 1aa5723f..db89f2ed 100755 --- a/comfy_cli/command/install.py +++ b/comfy_cli/command/install.py @@ -51,7 +51,7 @@ def pip_install_comfyui_dependencies( skip_torch_or_directml: bool, skip_requirement: bool, python: str = sys.executable, - rocm_version: constants.ROCmVersion = constants.ROCmVersion.v6_3, + rocm_version: constants.ROCmVersion = constants.ROCmVersion.v7_2, cuda_tag: str | None = None, ): os.chdir(repo_dir) @@ -153,7 +153,7 @@ def execute( gpu: constants.GPU_OPTION | None = None, cuda_version: constants.CUDAVersion | None = None, cuda_tag: str | None = None, - rocm_version: constants.ROCmVersion = constants.ROCmVersion.v6_3, + rocm_version: constants.ROCmVersion = constants.ROCmVersion.v7_2, plat: constants.OS = None, skip_torch_or_directml: bool = False, skip_requirement: bool = False, diff --git a/comfy_cli/command/jobs.py b/comfy_cli/command/jobs.py index d7f8a0de..959bd5a7 100644 --- a/comfy_cli/command/jobs.py +++ b/comfy_cli/command/jobs.py @@ -34,19 +34,13 @@ from websocket import WebSocket, WebSocketException, WebSocketTimeoutException from comfy_cli import cancellation, execution_errors, tracking -from comfy_cli.config_manager import ConfigManager -from comfy_cli.env_backend import get_backend_from_env as _get_backend_from_env - from comfy_cli.env_checker import check_comfy_server_running +from comfy_cli.host_port import resolve_host_port as _resolve_host_port from comfy_cli.output import get_renderer app = typer.Typer(no_args_is_help=True, help="List, inspect, and live-watch ComfyUI prompts.") -DEFAULT_HOST = "127.0.0.1" -DEFAULT_PORT = 8188 - - def _is_pid_alive(pid: int) -> bool: """Check if a process with the given PID is still running.""" if pid <= 0: @@ -61,42 +55,9 @@ def _is_pid_alive(pid: int) -> bool: return True -# --------------------------------------------------------------------------- -# Host/port resolution (reuse the same precedence as `comfy run`) -# --------------------------------------------------------------------------- - - -_UNSAFE_HOST_CHARS = frozenset("/@?#") - - -def _validate_host(host: str) -> str: - """Reject host values that could cause URL injection.""" - if any(c in host for c in _UNSAFE_HOST_CHARS): - raise typer.BadParameter(f"invalid host: {host!r} (contains URL-special characters)") - return host - - -def _resolve_host_port(host: str | None, port: int | None) -> tuple[str, int]: - cfg = ConfigManager() - bg = cfg.background - if not host and bg is not None: - host = bg[0] - if not port and bg is not None: - port = bg[1] - - # env var support (COMFY_BACKEND or COMFY_HOST/COMFY_PORT) - env_h, env_p = _get_backend_from_env() - if not host and env_h: - host = env_h - if not port and env_p is not None: - port = env_p - - h = _validate_host(host or DEFAULT_HOST) - # Bracket IPv6 literals so callers building "http://{host}:{port}" / - # "ws://{host}:{port}" produce valid URLs (e.g. "::1" -> "[::1]"). - if ":" in h and not h.startswith("["): - h = f"[{h}]" - return (h, int(port or DEFAULT_PORT)) +# Host/port resolution (`resolve_host_port`) is shared with `comfy run` via +# `comfy_cli.host_port`; imported above as `_resolve_host_port` to preserve the +# call sites in this module unchanged. def _server_or_error(host: str, port: int, *, raise_on_missing: bool = True) -> bool: @@ -1380,14 +1341,9 @@ def _is_cloud(where: str | None) -> bool: for ``jobs ls/status/watch``. """ from comfy_cli import where as where_module - from comfy_cli.config_manager import ConfigManager try: - config_value = ConfigManager().get(where_module.CONFIG_KEY_WHERE_DEFAULT) - except Exception: # noqa: BLE001 — never let a bad config break routing - config_value = None - try: - decision = where_module.resolve(flag=where, config_value=config_value) + decision = where_module.resolve_default(flag=where) except ValueError: # Invalid value — fall back to local; the validating command # (cmdline.py top-level option) will surface ``where_invalid``. diff --git a/comfy_cli/command/launch.py b/comfy_cli/command/launch.py index 0ebebf14..90cba6ad 100644 --- a/comfy_cli/command/launch.py +++ b/comfy_cli/command/launch.py @@ -225,8 +225,7 @@ def background_launch(extra, frontend_pr=None): cmd.extend(extra) - loop = asyncio.get_event_loop() - log = loop.run_until_complete(launch_and_monitor(cmd, listen, port)) + log = asyncio.run(launch_and_monitor(cmd, listen, port)) if log is not None: print( diff --git a/comfy_cli/command/models/search.py b/comfy_cli/command/models/search.py index 5424f5ff..3f8565e9 100644 --- a/comfy_cli/command/models/search.py +++ b/comfy_cli/command/models/search.py @@ -29,7 +29,7 @@ import urllib.error import urllib.parse import urllib.request -from typing import Annotated, Any +from typing import Annotated, Any, NoReturn import typer @@ -124,6 +124,25 @@ def _http_get_json(url: str, target, timeout: float = 30.0) -> Any: return json.loads(body) +def _emit_http_error(e: urllib.error.HTTPError, *, renderer, target, message: str, hint: str) -> NoReturn: + """Emit a renderer error for an ``HTTPError`` and exit with code 1. + + Shared by the ``list-folders`` and ``search`` handlers, whose HTTPError + branches are identical: the error ``code`` is cloud/local-routed, and the + response body is truncated to 1 KiB and decoded (lossily) into ``details`` + for debugging. Only ``message`` and ``hint`` differ per caller. The + single-hint ``show`` handler and the 404-special-casing ``list-folder`` + handler deliberately do NOT use this — their shapes differ. + """ + renderer.error( + code="cloud_http_error" if target.is_cloud else "server_not_running", + message=message, + hint=hint, + details={"status": e.code, "body": (e.read() or b"")[:1000].decode("utf-8", "replace")}, + ) + raise typer.Exit(code=1) from e + + # --------------------------------------------------------------------------- # list-folders / list-folder — runtime introspection # --------------------------------------------------------------------------- @@ -149,15 +168,15 @@ def list_folders_cmd( try: data = _http_get_json(url, target) except urllib.error.HTTPError as e: - renderer.error( - code="cloud_http_error" if target.is_cloud else "server_not_running", + _emit_http_error( + e, + renderer=renderer, + target=target, message=f"HTTP {e.code} from {url}", hint="run `comfy auth whoami` to verify auth" if target.is_cloud else "run `comfy launch` to start a local server", - details={"status": e.code, "body": (e.read() or b"")[:1000].decode("utf-8", "replace")}, ) - raise typer.Exit(code=1) from e except (urllib.error.URLError, OSError, json.JSONDecodeError) as e: renderer.error( code="server_not_running" if not target.is_cloud else "cloud_http_error", @@ -459,13 +478,13 @@ def search_cmd( else: rows, total = _local_search(target, text=text, type_=type_, limit=limit) except urllib.error.HTTPError as e: - renderer.error( - code="cloud_http_error" if target.is_cloud else "server_not_running", + _emit_http_error( + e, + renderer=renderer, + target=target, message=f"HTTP {e.code} during models search", hint="check auth (`comfy auth whoami`) or network", - details={"status": e.code, "body": (e.read() or b"")[:1000].decode("utf-8", "replace")}, ) - raise typer.Exit(code=1) from e except (urllib.error.URLError, OSError, json.JSONDecodeError) as e: renderer.error( code="cloud_http_error" if target.is_cloud else "server_not_running", diff --git a/comfy_cli/command/nodes.py b/comfy_cli/command/nodes.py index d1565cea..27e48989 100644 --- a/comfy_cli/command/nodes.py +++ b/comfy_cli/command/nodes.py @@ -40,17 +40,10 @@ def _resolved_where(where: str | None) -> str: # Mirror comfy_cli.target.resolve_target()'s defensive fallback: a corrupt # config must not take the whole `comfy nodes *` surface down with a - # traceback before the structured renderer ever runs. - config_value: str | None = None + # traceback before the structured renderer ever runs (resolve_default reads + # the persisted where_default defensively for the same reason). try: - from comfy_cli.config_manager import ConfigManager - - config_value = ConfigManager().get(where_module.CONFIG_KEY_WHERE_DEFAULT) - except Exception: # noqa: BLE001 — never break routing on a bad config - config_value = None - - try: - decision = where_module.resolve(flag=where, config_value=config_value) + decision = where_module.resolve_default(flag=where) except ValueError: # An invalid persisted where_default shouldn't be fatal; fall back to # the flag (if valid) or auto-detect with the bad config value dropped. diff --git a/comfy_cli/command/transfer.py b/comfy_cli/command/transfer.py index ede794f2..b32043b5 100644 --- a/comfy_cli/command/transfer.py +++ b/comfy_cli/command/transfer.py @@ -24,6 +24,7 @@ from comfy_cli import jobs_state from comfy_cli.comfy_client import Client, Unauthenticated, extract_output_entries +from comfy_cli.http import NoRedirectHandler from comfy_cli.output import get_renderer from comfy_cli.output import rprint as pprint from comfy_cli.target import resolve_target @@ -74,14 +75,6 @@ def _auth_headers(target: Any) -> dict[str, str]: return headers -# Refuse redirects on upload — a 30x from an authenticated POST is suspicious. -class _NoRedirectHandler(urllib.request.HTTPRedirectHandler): - def http_error_301(self, req, fp, code, msg, headers): - raise urllib.error.HTTPError(req.full_url, code, "redirect refused (auth leak prevention)", headers, fp) - - http_error_302 = http_error_303 = http_error_307 = http_error_308 = http_error_301 - - # Stripped on every download redirect so auth never crosses origins. _AUTH_HEADERS_TO_STRIP = frozenset({"authorization", "x-api-key", "x-comfy-api-key", "cookie"}) _MAX_REDIRECTS = 5 @@ -109,7 +102,7 @@ def redirect_request(self, req, fp, code, msg, headers, newurl): return new_req -_TRANSFER_OPENER = urllib.request.build_opener(_NoRedirectHandler()) +_TRANSFER_OPENER = urllib.request.build_opener(NoRedirectHandler("redirect refused (auth leak prevention)")) _DOWNLOAD_OPENER = urllib.request.build_opener(_DownloadRedirectHandler()) diff --git a/comfy_cli/command/workflow.py b/comfy_cli/command/workflow.py index 99ed8cbe..d953c10e 100644 --- a/comfy_cli/command/workflow.py +++ b/comfy_cli/command/workflow.py @@ -92,12 +92,8 @@ def _get_graph(input_path: str | None, host: str | None, port: int | None, on_st return Graph.load(input_path=input_path, host=host or "127.0.0.1", port=port or 8188) # Live fetch: resolve mode from global routing chain, then use resilient loader. from comfy_cli import where as where_module - from comfy_cli.config_manager import ConfigManager - decision = where_module.resolve( - flag=None, - config_value=ConfigManager().get(where_module.CONFIG_KEY_WHERE_DEFAULT), - ) + decision = where_module.resolve_default() mode = "cloud" if decision.target is where_module.WhereTarget.CLOUD else "local" from comfy_cli.cql.loader import resilient_load_object_info @@ -411,29 +407,182 @@ def vary_cmd( # --------------------------------------------------------------------------- -# Cloud-saved workflows — list, get, save, delete via /api/workflows +# Saved workflows — list, get, save, delete. # --------------------------------------------------------------------------- # -# These four subcommands are cloud-only. ComfyUI's local server has no -# /api/workflows surface; users on local manage workflows as JSON files -# on disk via the slot-editing commands above. With ``--where local`` or -# no cloud session configured, we surface ``workflow_saved_local_unsupported`` -# with a hint pointing at the local file-based flow. +# These four subcommands route through ``--where``: +# +# cloud → Comfy Cloud's ``/api/workflows`` store (UUID-keyed, versioned). +# local → the running ComfyUI's ``/userdata`` file store, under the same +# ``workflows/`` dir the ComfyUI frontend uses. A workflow's id on +# the local path is its path *relative to* ``workflows/`` (e.g. +# ``flux.json`` or ``sub/dir/flux.json``) — that same string is what +# ``get``/``delete`` take and what ``save`` returns. +# +# The two paths share the ``--json`` envelope shape as far as feasible; the +# per-verb docstrings + PR note the deltas (local has no versioning, no +# server-side description, and reports raw file ``size``/``modified``/``created`` +# epoch-ms timestamps instead of cloud's ISO ``created_at``/``updated_at``). + + +# The userdata subdirectory the ComfyUI frontend stores saved workflows in. +_WORKFLOWS_DIR = "workflows" + +# Cap on a single ``/userdata`` response we buffer into memory. We read one byte +# past the cap so we can *detect* truncation and fail loudly, rather than +# silently writing a partial workflow and reporting success. +_USERDATA_MAX_BYTES = 64 * 1024 * 1024 -def _cloud_target_or_local_error(where: str | None, renderer): - """Resolve a cloud Target or emit ``workflow_saved_local_unsupported``.""" +class _ResponseTooLarge(Exception): + """A ``/userdata`` response exceeded ``_USERDATA_MAX_BYTES`` — refuse to truncate.""" + + +# Map the cloud ``--sort`` fields onto local FileInfo keys (client-side sort; +# ComfyUI's /userdata listing has no server-side sort/limit/filter). +_LOCAL_SORT_KEYS = {"create_time": "created", "update_time": "modified", "name": "path"} + + +def _resolve_where_target(where: str | None): + """Resolve the routing Target for a saved-workflow verb (cloud or local).""" from comfy_cli.target import resolve_target - target = resolve_target(where=where) - if not target.is_cloud: + return resolve_target(where=where) + + +def _strip_terminal_controls(text: str) -> str: + """Drop C0/C1 control chars (keeping tab / newline / carriage return) so + untrusted workflow content printed to a TTY can't emit ANSI/OSC escape + sequences that spoof output or manipulate the terminal.""" + return "".join(ch for ch in text if ch in "\t\n\r" or (0x20 <= ord(ch) < 0x7F) or ord(ch) >= 0xA0) + + +def _reject_unsafe_workflow_key(renderer, key: str) -> str: + """Validate a local workflow id/name as a safe relative path under ``workflows/``. + + Subdirectories are allowed (``sub/flux.json``), but traversal + (``..``), absolute paths, home refs, and backslashes are rejected so a + hostile id can't escape the userdata dir. Returns the cleaned key. + + Components are checked after stripping trailing dots and spaces, because a + Windows ComfyUI server strips those from filenames — so ``.. `` or ``...`` + would collapse to ``..`` and escape ``workflows/`` if we only matched the + literal ``..``. + """ + cleaned = key.strip() + parts = cleaned.split("/") + if ( + not cleaned + or cleaned.startswith("/") + or cleaned.startswith("~") + or "\\" in cleaned + # Catches "" (leading/trailing/double slash), ".", "..", "...", ".. ", etc. + or any(p.rstrip(" .") in ("", "..") for p in parts) + ): renderer.error( - code="workflow_saved_local_unsupported", - message="Saved-workflow management requires Comfy Cloud; local ComfyUI has no /api/workflows surface.", - hint="for local workflows, manage JSON files on disk via `comfy workflow slots/set-slot/vary`", + code="invalid_argument", + message=f"workflow id {key!r} is not a valid path under the local workflows/ dir", + hint="use a relative name like `flux.json` or `sub/flux.json` (no `..`, no leading `/`)", ) raise typer.Exit(code=1) - return target + return cleaned + + +def _userdata_request( + url: str, + target, + *, + method: str = "GET", + data: bytes | None = None, + content_type: str | None = None, + timeout: float = 30.0, +) -> tuple[int, bytes]: + """Authed HTTP call to a ComfyUI ``/userdata`` endpoint returning (status, raw_bytes). + + Raises urllib errors verbatim so callers can map them to envelope codes. + Local ComfyUI needs no auth; ``_authed_request`` is a no-op on the headers + when the Target carries no credential. + """ + import urllib.request + + req = _authed_request(url, target, method=method, data=data, content_type=content_type) + with urllib.request.urlopen(req, timeout=timeout) as resp: + status = resp.status + # Read one byte past the cap so we can tell a full body from a truncated one. + raw = resp.read(_USERDATA_MAX_BYTES + 1) + if len(raw) > _USERDATA_MAX_BYTES: + raise _ResponseTooLarge() + return status, raw + + +def _handle_local_http_error(renderer, e, *, operation: str, workflow_id: str | None = None) -> typer.Exit: + """Map local ``/userdata`` failures to envelope codes. Returns an Exit to ``raise from``. + + A *reachable* server that answers with an HTTP error or an unparseable body + gets a distinct code (``server_error`` / ``client_error`` / ``invalid_response``) + so the user isn't wrongly told to `comfy launch` — that hint is reserved for a + genuinely unreachable server (URLError / OSError). + """ + import urllib.error + + if isinstance(e, _ResponseTooLarge): + renderer.error( + code="workflow_too_large", + message=f"local ComfyUI /userdata response during {operation} exceeded the " + f"{_USERDATA_MAX_BYTES // (1024 * 1024)} MiB cap", + hint="the saved workflow is unexpectedly large; inspect it directly on the server", + details={"operation": operation, "limit_bytes": _USERDATA_MAX_BYTES}, + ) + elif isinstance(e, urllib.error.HTTPError) and e.code == 404: + renderer.error( + code="workflow_not_found", + message=f"no saved workflow with id {workflow_id!r}" + if workflow_id + else f"workflow not found ({operation})", + hint="list available workflows via `comfy --json --where local workflow list`", + details={"workflow_id": workflow_id, "operation": operation}, + ) + elif isinstance(e, urllib.error.HTTPError) and 500 <= e.code < 600: + renderer.error( + code="server_error", + message=f"HTTP {e.code} during {operation} against local ComfyUI /userdata", + hint="check the ComfyUI server logs", + details={"status": e.code, "operation": operation}, + ) + elif isinstance(e, urllib.error.HTTPError): + renderer.error( + code="client_error", + message=f"HTTP {e.code} during {operation} against local ComfyUI /userdata", + hint="the server rejected the request; check the workflow id and the server version", + details={"status": e.code, "operation": operation}, + ) + elif isinstance(e, json.JSONDecodeError): + renderer.error( + code="invalid_response", + message=f"local ComfyUI returned an unparseable body during {operation}", + hint="check that the host:port really is a ComfyUI server", + details={"operation": operation}, + ) + else: + renderer.error( + code="server_not_running", + message=f"could not reach local ComfyUI during {operation}: {e}", + hint="run `comfy launch` to start a local server", + ) + return typer.Exit(code=1) + + +def _userdata_file_url(target, key: str, query: dict | None = None) -> str: + """Build the ``/userdata/`` URL. The whole relative + path is percent-encoded into a single segment (``/`` → ``%2F``), exactly as + the ComfyUI frontend does, so subdir keys survive aiohttp's ``{file}`` route.""" + import urllib.parse + + encoded = urllib.parse.quote(f"{_WORKFLOWS_DIR}/{key}", safe="") + url = target.url("userdata", encoded) + if query: + url += "?" + urllib.parse.urlencode(query) + return url def _authed_request( @@ -512,7 +661,229 @@ def _handle_cloud_http_error(renderer, e, *, operation: str, workflow_id: str | return typer.Exit(code=1) -@app.command("list", help="List your saved workflows on Comfy Cloud.") +# --------------------------------------------------------------------------- +# Local ``/userdata`` implementations of the four saved-workflow verbs. +# --------------------------------------------------------------------------- + + +def _local_list(renderer, target, *, name: str | None, limit: int, sort: str, order: str) -> None: + import urllib.error + import urllib.parse + + params = {"dir": _WORKFLOWS_DIR, "recurse": "true", "split": "false", "full_info": "true"} + url = target.url("userdata") + "?" + urllib.parse.urlencode(params) + try: + _, raw = _userdata_request(url, target) + rows = json.loads(raw) if raw else [] + except urllib.error.HTTPError as e: + if e.code == 404: + rows = [] # the workflows/ dir doesn't exist yet → no saved workflows + else: + raise _handle_local_http_error(renderer, e, operation="list") from e + except (urllib.error.URLError, OSError, json.JSONDecodeError, _ResponseTooLarge) as e: + raise _handle_local_http_error(renderer, e, operation="list") from e + + rows = [r for r in rows if isinstance(r, dict) and isinstance(r.get("path"), str)] + if name: + needle = name.lower() + rows = [r for r in rows if needle in r["path"].lower()] + + sort_key = _LOCAL_SORT_KEYS.get(sort, "created") + reverse = order != "asc" + if sort_key == "path": + rows.sort(key=lambda r: r["path"].lower(), reverse=reverse) + else: + rows.sort(key=lambda r: r.get(sort_key) or 0, reverse=reverse) + rows = rows[: min(max(limit, 1), 100)] + + workflows = [ + { + "id": r["path"], + "name": r["path"], + "size": r.get("size"), + "modified": r.get("modified"), + "created": r.get("created"), + } + for r in rows + ] + payload = {"count": len(workflows), "workflows": workflows} + if renderer.is_pretty(): + from rich.table import Table + + tbl = Table(show_header=True, header_style="bold") + tbl.add_column("id") + tbl.add_column("size", justify="right", style="dim") + for r in workflows[:50]: + tbl.add_row(r["id"], str(r["size"]) if r["size"] is not None else "") + renderer.console().print(tbl) + rprint(f"[dim]{len(workflows)} workflow(s) (local)[/dim]") + renderer.emit(payload, command="workflow list", where="local") + + +def _local_get(renderer, target, workflow_id: str, out: str | None) -> None: + import urllib.error + + key = _reject_unsafe_workflow_key(renderer, workflow_id) + url = _userdata_file_url(target, key) + try: + _, raw = _userdata_request(url, target) + except (urllib.error.HTTPError, urllib.error.URLError, OSError, _ResponseTooLarge) as e: + raise _handle_local_http_error(renderer, e, operation="get", workflow_id=workflow_id) from e + + try: + # ``json.loads`` decodes bytes itself and raises ``UnicodeDecodeError`` (not a + # ``JSONDecodeError``) on non-UTF-8 input, so catch both. + data = json.loads(raw) + except (json.JSONDecodeError, UnicodeDecodeError): + data = None + if _is_frontend_format(data): + node_count = len(data["nodes"]) + elif isinstance(data, dict): + node_count = len(data) + else: + node_count = None + + # A valid-UTF-8-but-not-JSON body (e.g. an HTML proxy/error page returned 200) or + # non-UTF-8 bytes still get written verbatim; warn so a corrupt fetch isn't silent. + warnings: list[dict[str, str]] = [] + if data is None: + warnings.append( + { + "code": "workflow_content_not_json", + "message": "fetched content is not parseable JSON; wrote the raw bytes unchanged", + } + ) + + if out: + out_path = Path(out).expanduser() + try: + out_path.parent.mkdir(parents=True, exist_ok=True) + out_path.write_bytes(raw) + except OSError as e: + renderer.error( + code="workflow_write_error", + message=f"could not write workflow to {out_path}: {e}", + hint="check the --out path is writable and the disk has space", + ) + raise typer.Exit(code=1) from e + target_repr = str(out_path) + else: + if renderer.is_pretty(): + import sys + + # Strip control chars so untrusted content can't emit ANSI/OSC escapes + # that spoof or manipulate the terminal. + sys.stdout.write(_strip_terminal_controls(raw.decode("utf-8", "replace"))) + sys.stdout.write("\n") + target_repr = "stdout" + + payload: dict[str, Any] = { + "workflow_id": key, + "out": target_repr, + "bytes": len(raw), + "node_count": node_count, + } + if warnings: + payload["warnings"] = warnings + if renderer.is_pretty() and out: + rprint(f"[green]✓[/green] wrote {len(raw):,} bytes to {target_repr}") + renderer.emit(payload, command="workflow get", where="local") + + +def _local_save(renderer, target, workflow_file: str, name: str, description: str | None) -> None: + import urllib.error + + path = Path(workflow_file).expanduser() + if not path.is_file(): + renderer.error( + code="workflow_not_found", + message=f"local workflow file not found: {path}", + hint="check the path", + ) + raise typer.Exit(code=1) + try: + text = path.read_text(encoding="utf-8") + except (OSError, UnicodeDecodeError) as e: + renderer.error( + code="workflow_read_error", + message=f"could not read {path}: {e}", + hint="check file permissions and encoding", + ) + raise typer.Exit(code=1) from e + try: + workflow_json = json.loads(text) + except json.JSONDecodeError as e: + renderer.error( + code="workflow_invalid_json", + message=f"{path} is not valid JSON: {e}", + hint="re-export the workflow from ComfyUI", + ) + raise typer.Exit(code=1) from e + if not isinstance(workflow_json, dict): + renderer.error( + code="workflow_not_api_format", + message="workflow_json must be a JSON object", + hint="use ComfyUI's `File > Save` to export", + ) + raise typer.Exit(code=1) + + key = name if name.lower().endswith(".json") else f"{name}.json" + key = _reject_unsafe_workflow_key(renderer, key) + url = _userdata_file_url(target, key, query={"overwrite": "true", "full_info": "true"}) + try: + _, raw = _userdata_request( + url, target, method="POST", data=text.encode("utf-8"), content_type="application/json" + ) + except (urllib.error.HTTPError, urllib.error.URLError, OSError, _ResponseTooLarge) as e: + raise _handle_local_http_error(renderer, e, operation="save", workflow_id=key) from e + + info = None + try: + info = json.loads(raw) + except json.JSONDecodeError: + pass + # ComfyUI returns FileInfo.path as "workflows/"; strip the prefix back + # to the id the other verbs use. Fall back to the key we sent. + stored_id = key + if isinstance(info, dict) and isinstance(info.get("path"), str): + stored = info["path"] + prefix = f"{_WORKFLOWS_DIR}/" + stored_id = stored[len(prefix) :] if stored.startswith(prefix) else stored + + payload: dict[str, Any] = { + "workflow_id": stored_id, + "name": stored_id, + "source": str(path), + "size": info.get("size") if isinstance(info, dict) else None, + "modified": info.get("modified") if isinstance(info, dict) else None, + } + if description: + # Local file-backed userdata has no metadata store for a description. + payload["warnings"] = [ + {"code": "description_ignored", "message": "--description is ignored on the local path (no metadata store)"} + ] + if renderer.is_pretty(): + rprint(f"[green]✓[/green] saved [dim]{stored_id}[/dim]") + renderer.emit(payload, command="workflow save", where="local", changed=True) + + +def _local_delete(renderer, target, workflow_id: str) -> None: + import urllib.error + + key = _reject_unsafe_workflow_key(renderer, workflow_id) + url = _userdata_file_url(target, key) + try: + _userdata_request(url, target, method="DELETE") + except (urllib.error.HTTPError, urllib.error.URLError, OSError, _ResponseTooLarge) as e: + raise _handle_local_http_error(renderer, e, operation="delete", workflow_id=workflow_id) from e + + payload = {"workflow_id": key, "deleted": True} + if renderer.is_pretty(): + rprint(f"[green]✓[/green] deleted [dim]{key}[/dim]") + renderer.emit(payload, command="workflow delete", where="local", changed=True) + + +@app.command("list", help="List saved workflows (cloud store, or local ComfyUI /userdata with --where local).") @tracking.track_command("workflow") def list_cmd( name: Annotated[ @@ -534,9 +905,30 @@ def list_cmd( import urllib.parse renderer = get_renderer() - target = _cloud_target_or_local_error(where, renderer) - params: dict[str, Any] = {"limit": min(max(limit, 1), 100), "sort": sort, "order": order} + # Validate the free-form sort/order options up front (both routes) so a typo like + # `--order ASC` errors loudly instead of silently sorting the wrong way. + order_norm = order.lower() + if order_norm not in ("asc", "desc"): + renderer.error( + code="invalid_argument", + message=f"--order must be 'asc' or 'desc', got {order!r}", + hint="pass `--order asc` or `--order desc`", + ) + raise typer.Exit(code=1) + if sort not in _LOCAL_SORT_KEYS: + renderer.error( + code="invalid_argument", + message=f"--sort must be one of {', '.join(_LOCAL_SORT_KEYS)}, got {sort!r}", + hint="pass `--sort create_time|update_time|name`", + ) + raise typer.Exit(code=1) + + target = _resolve_where_target(where) + if not target.is_cloud: + return _local_list(renderer, target, name=name, limit=limit, sort=sort, order=order_norm) + + params: dict[str, Any] = {"limit": min(max(limit, 1), 100), "sort": sort, "order": order_norm} if name: params["name"] = name url = target.url("workflows") + "?" + urllib.parse.urlencode(params) @@ -583,10 +975,16 @@ def list_cmd( renderer.emit(payload, command="workflow list", where="cloud") -@app.command("get", help="Fetch a saved workflow's content from Comfy Cloud (writes JSON to --out or stdout).") +@app.command( + "get", + help="Fetch a saved workflow's content (cloud, or local with --where local); writes JSON to --out or stdout.", +) @tracking.track_command("workflow") def get_cmd( - workflow_id: Annotated[str, typer.Argument(help="The workflow UUID.")], + workflow_id: Annotated[ + str, + typer.Argument(help="Workflow id: cloud UUID, or local path under workflows/ (e.g. flux.json)."), + ], out: Annotated[ str | None, typer.Option("--out", "-o", show_default=False, help="Write JSON to this file instead of stdout."), @@ -596,7 +994,9 @@ def get_cmd( import urllib.error renderer = get_renderer() - target = _cloud_target_or_local_error(where, renderer) + target = _resolve_where_target(where) + if not target.is_cloud: + return _local_get(renderer, target, workflow_id, out) import urllib.parse as _up @@ -644,21 +1044,31 @@ def get_cmd( renderer.emit(payload, command="workflow get", where="cloud") -@app.command("save", help="Save a local workflow JSON to Comfy Cloud as a new saved workflow.") +@app.command( + "save", + help="Save a workflow JSON to the saved-workflow store (cloud, or local ComfyUI /userdata with --where local).", +) @tracking.track_command("workflow") def save_cmd( workflow_file: Annotated[str, typer.Argument(help="Path to a workflow JSON file.")], - name: Annotated[str, typer.Option("--name", help="Display name for the workflow.")], + name: Annotated[ + str, + typer.Option( + "--name", help="Cloud: display name. Local: filename under workflows/ ('.json' appended if absent)." + ), + ], description: Annotated[ str | None, - typer.Option("--description", show_default=False, help="Optional description."), + typer.Option("--description", show_default=False, help="Optional description (cloud only; ignored on local)."), ] = None, where: Annotated[str | None, typer.Option("--where", show_default=False)] = None, ): import urllib.error renderer = get_renderer() - target = _cloud_target_or_local_error(where, renderer) + target = _resolve_where_target(where) + if not target.is_cloud: + return _local_save(renderer, target, workflow_file, name, description) path = Path(workflow_file).expanduser() if not path.is_file(): @@ -706,16 +1116,21 @@ def save_cmd( renderer.emit(payload, command="workflow save", where="cloud", changed=True) -@app.command("delete", help="Delete a saved workflow from Comfy Cloud.") +@app.command("delete", help="Delete a saved workflow (cloud, or local ComfyUI /userdata with --where local).") @tracking.track_command("workflow") def delete_cmd( - workflow_id: Annotated[str, typer.Argument(help="The workflow UUID to delete.")], + workflow_id: Annotated[ + str, + typer.Argument(help="Workflow id to delete: cloud UUID, or local path under workflows/ (e.g. flux.json)."), + ], where: Annotated[str | None, typer.Option("--where", show_default=False)] = None, ): import urllib.error renderer = get_renderer() - target = _cloud_target_or_local_error(where, renderer) + target = _resolve_where_target(where) + if not target.is_cloud: + return _local_delete(renderer, target, workflow_id) import urllib.parse as _up diff --git a/comfy_cli/command/workflow_fragments.py b/comfy_cli/command/workflow_fragments.py index ca47f1dc..988be5f0 100644 --- a/comfy_cli/command/workflow_fragments.py +++ b/comfy_cli/command/workflow_fragments.py @@ -329,12 +329,9 @@ def _load_object_info(renderer, *, input_path: str | None, host: str | None, por p = Path(input_path).expanduser() return json.loads(p.read_text(encoding="utf-8")) from comfy_cli import where as where_module - from comfy_cli.config_manager import ConfigManager from comfy_cli.cql.loader import resilient_load_object_info - decision = where_module.resolve( - flag=None, config_value=ConfigManager().get(where_module.CONFIG_KEY_WHERE_DEFAULT) - ) + decision = where_module.resolve_default() mode = "cloud" if decision.target is where_module.WhereTarget.CLOUD else "local" return resilient_load_object_info(mode=mode, host=host or "127.0.0.1", port=port or 8188) except (OSError, json.JSONDecodeError) as e: diff --git a/comfy_cli/constants.py b/comfy_cli/constants.py index 0f7634bb..5f88be96 100644 --- a/comfy_cli/constants.py +++ b/comfy_cli/constants.py @@ -89,6 +89,7 @@ class CUDAVersion(str, Enum): class ROCmVersion(str, Enum): + v7_2 = "7.2" v7_1 = "7.1" v7_0 = "7.0" v6_3 = "6.3" diff --git a/comfy_cli/cql/_net.py b/comfy_cli/cql/_net.py new file mode 100644 index 00000000..47b021a6 --- /dev/null +++ b/comfy_cli/cql/_net.py @@ -0,0 +1,28 @@ +"""Shared network-target helpers for CQL loaders.""" + +from __future__ import annotations + +import ipaddress + +__all__ = ["is_loopback_host"] + + +def is_loopback_host(hostname: str) -> bool: + """True if *hostname* refers to a loopback interface. + + Expects an already-parsed hostname (e.g. ``urllib.parse.urlsplit(url).hostname``, + which strips IPv6 brackets). Matches the literal name ``localhost`` and any + address ``ipaddress`` classifies as loopback (127.0.0.0/8, ::1). Used to refuse + SSRF fetches to non-loopback targets in local mode. + + NOTE: this deliberately includes the ipaddress fallback, so it must NOT be used + for the require-HTTPS gates in comfy_client._assert_safe_url or + oauth._assert_https_or_loopback — those need exact-literal membership. + """ + h = (hostname or "").strip().lower() + if h == "localhost": + return True + try: + return ipaddress.ip_address(h).is_loopback + except ValueError: + return False diff --git a/comfy_cli/cql/engine.py b/comfy_cli/cql/engine.py index 52c3dd39..441e39a2 100644 --- a/comfy_cli/cql/engine.py +++ b/comfy_cli/cql/engine.py @@ -11,7 +11,6 @@ import copy import difflib -import ipaddress import json import logging import urllib.error @@ -22,6 +21,9 @@ from dataclasses import dataclass, field from typing import Any +from comfy_cli.cql._net import is_loopback_host +from comfy_cli.http import NoRedirectHandler + # --------------------------------------------------------------------------- # Types — mirrors nodegraph/types.go # --------------------------------------------------------------------------- @@ -1044,20 +1046,10 @@ def morphism_to_dict(self, m: Morphism) -> dict[str, Any]: _logger = logging.getLogger(__name__) -_LOOPBACK_HOSTS = frozenset({"127.0.0.1", "localhost", "::1", "[::1]"}) _MAX_OBJECT_INFO_BYTES = 64 * 1024 * 1024 -class _NoRedirectHandler(urllib.request.HTTPRedirectHandler): - """Refuse redirects — a 302 from /object_info is suspicious.""" - - def http_error_301(self, req, fp, code, msg, headers): - raise urllib.error.HTTPError(req.full_url, code, "redirect refused", headers, fp) - - http_error_302 = http_error_303 = http_error_307 = http_error_308 = http_error_301 - - -_opener = urllib.request.build_opener(_NoRedirectHandler()) +_opener = urllib.request.build_opener(NoRedirectHandler()) class LoadError(Exception): @@ -1102,14 +1094,7 @@ def _load_from_target(*, mode: str = "local", host: str = "127.0.0.1", port: int # Loopback guard for local targets if not target.is_cloud: parsed_host = urllib.parse.urlsplit(url).hostname or "" - host_l = parsed_host.lower() - is_loopback = host_l in _LOOPBACK_HOSTS - if not is_loopback: - try: - is_loopback = ipaddress.ip_address(host_l).is_loopback - except ValueError: - is_loopback = False - if not is_loopback: + if not is_loopback_host(parsed_host): raise LoadError( f"Refusing to fetch object_info from non-loopback host {parsed_host!r} " f"in local mode (potential SSRF). Use --where cloud for remote targets." diff --git a/comfy_cli/cql/loader.py b/comfy_cli/cql/loader.py index 5e78daef..c3d0e4b1 100644 --- a/comfy_cli/cql/loader.py +++ b/comfy_cli/cql/loader.py @@ -19,7 +19,6 @@ from __future__ import annotations import hashlib -import ipaddress import json import os import sys @@ -29,7 +28,9 @@ from pathlib import Path from typing import Any +from comfy_cli.cql._net import is_loopback_host from comfy_cli.cql.errors import CQLRuntimeError +from comfy_cli.http import NoRedirectHandler # Cap raw bytes read from disk or the network. Real `object_info` dumps are a # few MB; anything past 256 MiB is almost certainly a wrong path or a hostile @@ -37,17 +38,7 @@ MAX_INPUT_BYTES = 256 * 1024 * 1024 -class _NoRedirect(urllib.request.HTTPRedirectHandler): - """Refuse redirects — a 302 from /object_info to elsewhere is suspicious - and would expose us to a different server than the user asked to query.""" - - def http_error_301(self, req, fp, code, msg, headers): - raise urllib.error.HTTPError(req.full_url, code, "redirect refused", headers, fp) - - http_error_302 = http_error_303 = http_error_307 = http_error_308 = http_error_301 - - -_LOADER_OPENER = urllib.request.build_opener(_NoRedirect()) +_LOADER_OPENER = urllib.request.build_opener(NoRedirectHandler()) def load_graph( @@ -96,13 +87,7 @@ def _load_from_server(host: str, port: int, *, timeout: float) -> dict[str, Any] # own path; this loader is local-only by design.) parsed = urllib.parse.urlsplit(url) hostname = (parsed.hostname or "").strip().lower() - is_loopback = hostname == "localhost" - if not is_loopback: - try: - is_loopback = ipaddress.ip_address(hostname).is_loopback - except ValueError: - is_loopback = False - if not is_loopback: + if not is_loopback_host(hostname): raise CQLRuntimeError( f"refusing non-loopback CQL server target: {host}", details={"hint": "pass --input for remote object_info dumps"}, diff --git a/comfy_cli/error_codes.py b/comfy_cli/error_codes.py index b04f319b..a8a952f7 100644 --- a/comfy_cli/error_codes.py +++ b/comfy_cli/error_codes.py @@ -68,6 +68,25 @@ class ErrorCode: "Workflow file exists but isn't readable as UTF-8 text (OSError / UnicodeDecodeError).", "check file permissions and encoding", ), + ErrorCode( + "workflow_write_error", + "`workflow get --out` could not write the fetched workflow to disk (OSError: permissions, " + "missing parent dir, full disk, invalid path).", + "check the --out path is writable and the disk has space", + ), + ErrorCode( + "workflow_too_large", + "A local ComfyUI `/userdata` response exceeded the in-memory read cap, so the CLI refused to " + "truncate it into a corrupt/partial file. `details.limit_bytes` carries the cap.", + "the saved workflow is unexpectedly large; inspect it directly on the server", + ), + ErrorCode( + "workflow_content_not_json", + "`workflow get` fetched content that isn't parseable JSON (non-UTF-8 bytes or a non-JSON body such " + "as an HTML error page); the raw bytes were still written. Surfaced in `data.warnings[]`, not as an " + "error envelope, so the command still succeeds.", + "verify the id points at a real saved workflow, not a stray file, on the local server", + ), # --- local server / WebSocket -------------------------------------------- ErrorCode( "server_not_running", @@ -236,11 +255,6 @@ class ErrorCode: "`comfy jobs cancel` could not reach the local server to cancel the prompt.", "check the server is still running on the host/port", ), - ErrorCode( - "workflow_saved_local_unsupported", - "`comfy workflow {list,get,save,delete}` requires Comfy Cloud — local ComfyUI has no /api/workflows.", - "for local workflows, manage JSON files on disk via `workflow slots`/`set-slot`/`vary`", - ), # --- auth (provider keys + cloud session intertwined) -------------------- ErrorCode( "auth_invalid_key", @@ -458,6 +472,13 @@ class ErrorCode: "Surfaced in `data.warnings[]` (not as an error envelope) so the command still succeeds.", "re-run once the server/session is reachable to get a fresh schema", ), + ErrorCode( + "description_ignored", + "`comfy workflow save --where local --description` was given a description, but the local " + "file-backed `/userdata` store has nowhere to keep it. Surfaced in `data.warnings[]` " + "(not as an error envelope) so the save still succeeds.", + "descriptions are a Comfy Cloud feature; drop `--description` on the local path", + ), ErrorCode( "cql_query_invalid", "Grammar query failed to parse or evaluate.", diff --git a/comfy_cli/host_port.py b/comfy_cli/host_port.py new file mode 100644 index 00000000..01ffdf81 --- /dev/null +++ b/comfy_cli/host_port.py @@ -0,0 +1,113 @@ +"""Shared host/port parsing + resolution for local ComfyUI commands. + +Both ``comfy run`` and every ``comfy jobs`` subcommand accept a ``--host`` / +``--port`` pair, fall back to the persisted ``config.background`` server, then +to ``DEFAULT_HOST`` / ``DEFAULT_PORT``. In addition, ``comfy run`` accepts a +combined ``host[:port]`` string (parsed via ``parse_host_port_arg``); the +``comfy jobs`` subcommands only take the separate ``--host`` / ``--port`` +options. They all feed the resolved host straight into URLs like +``http://{host}:{port}/prompt`` / ``ws://{host}:{port}/ws``, so the value must +be validated (no URL-injection characters) and IPv6 literals bracketed. This +module is the single home for that logic; callers should not re-implement it. +""" + +from __future__ import annotations + +import typer + +from comfy_cli.config_manager import ConfigManager + +DEFAULT_HOST = "127.0.0.1" +DEFAULT_PORT = 8188 +_UNSAFE_HOST_CHARS = frozenset("/@?#") + + +def validate_host(host: str) -> str: + """Reject host values that could cause URL injection.""" + if any(c in host for c in _UNSAFE_HOST_CHARS): + raise typer.BadParameter(f"invalid host: {host!r} (contains URL-special characters)") + # Whitespace/control chars (notably CR/LF) never appear in a real host and + # are the canonical header/URL-injection vectors, so reject them too. + if any(c.isspace() or ord(c) < 0x20 or ord(c) == 0x7F for c in host): + raise typer.BadParameter(f"invalid host: {host!r} (contains whitespace or control characters)") + return host + + +def parse_host_port_arg(value: str) -> tuple[str, int | None]: + """Split a user-typed combined ``host[:port]`` string, IPv6-aware. + + Accepts: ``'host'``, ``'host:port'``, ``'[::1]'``, ``'[::1]:8188'``, bare + ``'::1'``. Returns ``(host, port_or_None)``. Raises ``typer.BadParameter`` + on a non-numeric port or an unterminated bracket. + """ + v = value.strip() + if v.startswith("["): + end = v.find("]") + if end == -1: + raise typer.BadParameter(f"invalid host: {value!r} (unterminated '[')") + host = v[1:end] + rest = v[end + 1 :] + if rest: + # Only a ``:port`` suffix is allowed after the bracket; anything + # else (e.g. ``[::1]8188``) is a typo we must not silently drop. + if not rest.startswith(":"): + raise typer.BadParameter(f"invalid host: {value!r} (unexpected text after ']')") + if rest[1:]: + return _require_host(host, value), _to_port(rest[1:], value) + return _require_host(host, value), None + if v.count(":") == 1: # exactly one colon -> host:port + h, p = v.split(":") + return _require_host(h, value), (_to_port(p, value) if p else None) + # zero colons -> hostname only; 2+ colons -> bare IPv6 literal (no port) + return _require_host(v, value), None + + +def _require_host(host: str, original: str) -> str: + """Reject an empty host so ``:8188`` / ``[]:8188`` error instead of + silently retargeting the request to the default/background server.""" + if not host: + raise typer.BadParameter(f"invalid host: {original!r} (empty host)") + return host + + +def _to_port(s: str, original: str) -> int: + try: + port = int(s) + except ValueError: + raise typer.BadParameter(f"invalid port in {original!r}: {s!r} is not a number") + if not (1 <= port <= 65535): + raise typer.BadParameter(f"invalid port in {original!r}: {port} is out of range (1-65535)") + return port + + +def resolve_host_port(host: str | None, port: int | None) -> tuple[str, int]: + """Fill host/port from environment variables, then ``config.background``, + then defaults; validate and bracket IPv6 literals so callers building + ``'http://{host}:{port}'`` get a well-formed URL (e.g. ``'::1'`` -> + ``'[::1]'``). + + Resolution order (highest to lowest): + 1. Explicit ``host`` / ``port`` arguments (from CLI flags). + 2. Environment variables: ``COMFY_BACKEND``, ``COMFY_HOST_PORT``, + ``COMFY_HOST`` / ``COMFY_PORT``. + 3. Persisted ``config.background`` server. + 4. Built-in defaults (``127.0.0.1:8188``). + """ + from comfy_cli.env_backend import get_backend_from_env + + env_h, env_p = get_backend_from_env() + if not host and env_h: + host = env_h + if not port and env_p is not None: + port = env_p + + cfg = ConfigManager() + bg = cfg.background + if not host and bg is not None: + host = bg[0] + if not port and bg is not None: + port = bg[1] + h = validate_host(host or DEFAULT_HOST) + if ":" in h and not h.startswith("["): + h = f"[{h}]" + return (h, int(port or DEFAULT_PORT)) diff --git a/comfy_cli/http.py b/comfy_cli/http.py new file mode 100644 index 00000000..793307b9 --- /dev/null +++ b/comfy_cli/http.py @@ -0,0 +1,26 @@ +"""Shared HTTP helpers with an auth-leak-safe redirect policy.""" + +import urllib.error +import urllib.request + + +class NoRedirectHandler(urllib.request.HTTPRedirectHandler): + """Refuse to follow HTTP redirects. + + Following a redirect with ``Authorization: Bearer …`` / ``X-API-Key`` in + flight risks replaying the credential at the redirect target (auth leak / + SSRF). None of our authenticated endpoints redirect under normal + operation; a 30x is almost certainly a misconfiguration or an attack, so + we surface it as a clear ``HTTPError`` instead of following it. + + ``message`` is parameterizable so call sites can keep their own wording. + """ + + def __init__(self, message: str = "redirect refused"): + super().__init__() + self._message = message + + def http_error_301(self, req, fp, code, msg, headers): + raise urllib.error.HTTPError(req.full_url, code, self._message, headers, fp) + + http_error_302 = http_error_303 = http_error_307 = http_error_308 = http_error_301 diff --git a/comfy_cli/uv.py b/comfy_cli/uv.py index 17bebd22..f1d89ae2 100644 --- a/comfy_cli/uv.py +++ b/comfy_cli/uv.py @@ -122,11 +122,11 @@ def parse_req_file(rf: PathLike, skips: list[str] | None = None): class DependencyCompiler: cpuPytorchUrl = "https://download.pytorch.org/whl/cpu" - rocmPytorchUrl = "https://download.pytorch.org/whl/rocm6.3" + rocmPytorchUrl = "https://download.pytorch.org/whl/rocm7.2" nvidiaPytorchUrl = "https://download.pytorch.org/whl/cu126" cpuTorchBackend = "cpu" - rocmTorchBackend = "rocm6.3" + rocmTorchBackend = "rocm7.2" nvidiaTorchBackend = "cu126" overrideGpu = dedent( diff --git a/comfy_cli/where.py b/comfy_cli/where.py index 24f7be07..6afd4caa 100644 --- a/comfy_cli/where.py +++ b/comfy_cli/where.py @@ -77,6 +77,30 @@ def resolve( return WhereResolution(target=WhereTarget.LOCAL, source="default") +def resolve_default( + flag: str | None = None, + *, + env: Mapping[str, str] | None = None, + project_value: str | None = _UNSET, +) -> WhereResolution: + """``resolve()`` with the persisted ``where_default`` config read for you. + + Convenience for the common call site that just wants routing resolved + against the config file: it looks up ``CONFIG_KEY_WHERE_DEFAULT`` internally + (defensively — a broken config never breaks routing, it just drops to the + next precedence source) and forwards everything to :func:`resolve`. Callers + keep their own error handling for the ``ValueError`` a bad ``flag``/env/ + project/config value raises, and their own shaping of the result. + """ + from comfy_cli.config_manager import ConfigManager + + try: + config_value = ConfigManager().get(CONFIG_KEY_WHERE_DEFAULT) + except Exception: # noqa: BLE001 — never let a bad config break routing + config_value = None + return resolve(flag=flag, env=env, config_value=config_value, project_value=project_value) + + def _project_where_default() -> str | None: """``defaults.where`` from the project/1 ``comfy.yaml`` governing cwd, if any. Discovery itself never raises (see :mod:`comfy_cli.project`); a diff --git a/tests/comfy_cli/auth/test_where.py b/tests/comfy_cli/auth/test_where.py index 863a9409..52a5b9a3 100644 --- a/tests/comfy_cli/auth/test_where.py +++ b/tests/comfy_cli/auth/test_where.py @@ -44,6 +44,61 @@ def test_resolve_invalid_raises(): assert "hybrid" in str(exc_info.value) +# --------------------------------------------------------------------------- +# resolve_default: reads the persisted where_default config key for the caller +# --------------------------------------------------------------------------- + + +class _FakeConfigManager: + """Stand-in for ConfigManager whose ``get`` behaviour is programmable.""" + + _value: str | None = None + _raise: bool = False + + def get(self, key): + assert key == where_module.CONFIG_KEY_WHERE_DEFAULT + if self._raise: + raise RuntimeError("corrupt config") + return self._value + + +@pytest.fixture +def fake_config(monkeypatch): + import comfy_cli.config_manager as cm + + monkeypatch.setattr(cm, "ConfigManager", _FakeConfigManager) + return _FakeConfigManager + + +def test_resolve_default_uses_config_value(fake_config, monkeypatch): + monkeypatch.setattr(fake_config, "_value", "cloud") + r = where_module.resolve_default(env={}, project_value=None) + assert r.target is where_module.WhereTarget.CLOUD + assert r.source == "config" + + +def test_resolve_default_flag_beats_config(fake_config, monkeypatch): + monkeypatch.setattr(fake_config, "_value", "cloud") + r = where_module.resolve_default(flag="local", env={}, project_value=None) + assert r.target is where_module.WhereTarget.LOCAL + assert r.source == "flag" + + +def test_resolve_default_broken_config_falls_through(fake_config, monkeypatch): + """A config read that raises never breaks routing — it drops to None.""" + monkeypatch.setattr(fake_config, "_raise", True) + r = where_module.resolve_default(env={}, project_value=None) + assert r.target is where_module.WhereTarget.LOCAL + assert r.source == "default" + + +def test_resolve_default_forwards_project_value(fake_config, monkeypatch): + monkeypatch.setattr(fake_config, "_value", "local") + r = where_module.resolve_default(env={}, project_value="cloud") + assert r.target is where_module.WhereTarget.CLOUD + assert r.source == "project" + + # --------------------------------------------------------------------------- # project/1 precedence: flag → env → project → config → auto # --------------------------------------------------------------------------- diff --git a/tests/comfy_cli/command/models/test_search.py b/tests/comfy_cli/command/models/test_search.py index 56cade1a..8316896b 100644 --- a/tests/comfy_cli/command/models/test_search.py +++ b/tests/comfy_cli/command/models/test_search.py @@ -219,6 +219,26 @@ def test_local_happy_path(self, local_target, monkeypatch, capsys): # The string-list shape normalizes to [{name, subfolders=[]}]. assert env["data"]["folders"][0] == {"name": "checkpoints", "subfolders": []} + def test_cloud_http_error_decodes_body(self, cloud_target, monkeypatch, capsys): + # Shared `_emit_http_error` path: cloud code + truncated/decoded body in details. + err = urllib.error.HTTPError("https://x", 502, "Bad Gateway", {}, io.BytesIO(b'{"error": "upstream"}')) + _patch_urlopen(monkeypatch, {"/api/experiment/models": err}) + env = _run(["list-folders", "--where", "cloud"], capsys) + assert env["ok"] is False + assert env["error"]["code"] == "cloud_http_error" + assert env["error"]["details"]["status"] == 502 + assert env["error"]["details"]["body"] == '{"error": "upstream"}' + + def test_local_http_error_uses_server_not_running(self, local_target, monkeypatch, capsys): + # Same shared path, local branch: code flips to server_not_running. + err = urllib.error.HTTPError("http://x", 500, "Server Error", {}, io.BytesIO(b"boom")) + _patch_urlopen(monkeypatch, {"127.0.0.1:8188/models": err}) + env = _run(["list-folders", "--where", "local"], capsys) + assert env["ok"] is False + assert env["error"]["code"] == "server_not_running" + assert env["error"]["details"]["status"] == 500 + assert env["error"]["details"]["body"] == "boom" + # --------------------------------------------------------------------------- # list-folder @@ -316,6 +336,16 @@ def test_local_text_filter_is_client_side(self, local_target, monkeypatch, capsy names = [r["name"] for r in env["data"]["rows"]] assert names == ["flux1-dev.safetensors"] + def test_cloud_http_error_decodes_body(self, cloud_target, monkeypatch, capsys): + # Shared `_emit_http_error` path via the search handler. + err = urllib.error.HTTPError("https://x", 400, "Bad Request", {}, io.BytesIO(b'{"detail": "bad tag"}')) + _patch_urlopen(monkeypatch, {"/api/assets": err}) + env = _run(["search", "--text", "flux", "--where", "cloud"], capsys) + assert env["ok"] is False + assert env["error"]["code"] == "cloud_http_error" + assert env["error"]["details"]["status"] == 400 + assert env["error"]["details"]["body"] == '{"detail": "bad tag"}' + # --------------------------------------------------------------------------- # show diff --git a/tests/comfy_cli/command/test_launch_background.py b/tests/comfy_cli/command/test_launch_background.py new file mode 100644 index 00000000..8972b743 --- /dev/null +++ b/tests/comfy_cli/command/test_launch_background.py @@ -0,0 +1,73 @@ +"""Regression tests for `comfy launch --background`. + +Guards against the Python 3.14 crash where `asyncio.get_event_loop()` raised +`RuntimeError: There is no current event loop in thread 'MainThread'` because +implicit main-thread loop creation was removed. `background_launch` now uses +`asyncio.run(...)`, which works identically on 3.10-3.14+. +""" + +from unittest.mock import AsyncMock, patch + +import pytest + +from comfy_cli.command import launch + + +@patch("comfy_cli.command.launch.os._exit") +@patch("comfy_cli.command.launch.launch_and_monitor", new_callable=AsyncMock) +@patch("comfy_cli.command.launch.check_comfy_server_running", return_value=False) +@patch("comfy_cli.command.launch.ConfigManager") +def test_background_launch_drives_coroutine_without_event_loop_error( + mock_config_manager, mock_check_running, mock_monitor, mock_exit +): + """The background monitor path must run the coroutine without touching + the removed `asyncio.get_event_loop()` implicit-loop behavior.""" + mock_config_manager.return_value.background = None + mock_monitor.return_value = None + + # No RuntimeError should escape here on Python 3.14 (or any 3.10+ version), + # and the monitor coroutine must actually be awaited. + launch.background_launch(extra=[]) + + mock_monitor.assert_awaited_once() + mock_exit.assert_called_once_with(1) + + +def test_background_launch_does_not_use_deprecated_get_event_loop(): + """`asyncio.run` is the forward-compatible idiom; `asyncio.get_event_loop` + (removed-behavior on 3.14) must not reappear in the launch module.""" + import inspect + + source = inspect.getsource(launch) + assert "asyncio.get_event_loop()" not in source + assert "asyncio.run(" in source + + +@patch("comfy_cli.command.launch.os._exit") +@patch("comfy_cli.command.launch.launch_and_monitor", new_callable=AsyncMock) +@patch("comfy_cli.command.launch.check_comfy_server_running", return_value=False) +@patch("comfy_cli.command.launch.ConfigManager") +def test_background_launch_surfaces_error_log(mock_config_manager, mock_check_running, mock_monitor, mock_exit): + """When the monitor returns a failure log, it is rendered and the process + exits non-zero.""" + mock_config_manager.return_value.background = None + mock_monitor.return_value = ["boom\n"] + + with patch("comfy_cli.command.launch.print") as mock_print: + launch.background_launch(extra=[]) + + mock_monitor.assert_awaited_once() + assert mock_print.called + mock_exit.assert_called_once_with(1) + + +@patch("comfy_cli.command.launch.utils.is_running", return_value=True) +@patch("comfy_cli.command.launch.ConfigManager") +def test_background_launch_refuses_when_already_running(mock_config_manager, mock_is_running): + """A second background launch is rejected before reaching the asyncio path.""" + import typer + + mock_config_manager.return_value.background = ("127.0.0.1", 8188, 4242) + + with pytest.raises(typer.Exit): + launch.background_launch(extra=[]) diff --git a/tests/comfy_cli/command/test_workflow_saved.py b/tests/comfy_cli/command/test_workflow_saved.py index 6a146b36..2328480b 100644 --- a/tests/comfy_cli/command/test_workflow_saved.py +++ b/tests/comfy_cli/command/test_workflow_saved.py @@ -1,8 +1,10 @@ -"""Tests for `comfy workflow list/get/save/delete` — cloud-saved workflows. +"""Tests for `comfy workflow list/get/save/delete` — saved workflows. -These commands wrap the `/api/workflows` surface documented at -docs.comfy.org/api-reference/cloud/workflow. All HTTP is mocked; the -live round-trip is verified manually. +The cloud path wraps the `/api/workflows` surface documented at +docs.comfy.org/api-reference/cloud/workflow. The local path (`--where local`) +wraps the running ComfyUI's `/userdata` file store under `workflows/`, mirroring +the ComfyUI frontend's layout. All HTTP is mocked; the live round-trip is +verified manually. """ from __future__ import annotations @@ -163,33 +165,257 @@ def _fake(req, timeout=None): } +# A frontend-format workflow (nodes[] list) — what the local /userdata store holds. +_LOCAL_WORKFLOW = { + "last_node_id": 2, + "nodes": [ + {"id": 1, "type": "KSampler"}, + {"id": 2, "type": "VAEDecode"}, + ], + "links": [], +} + +# ComfyUI's /userdata?dir=workflows&full_info=true response: FileInfo dicts with +# `path` relative to the workflows/ dir. +_USERDATA_LIST_RESPONSE = [ + {"path": "flux.json", "size": 120, "modified": 1700000002000, "created": 1700000000000}, + {"path": "sub/wan.json", "size": 340, "modified": 1700000001000, "created": 1700000001000}, +] + + +def _http_error(code: int): + return urllib.error.HTTPError("http://127.0.0.1:8188/userdata/x", code, "err", {}, io.BytesIO(b"")) + + # --------------------------------------------------------------------------- -# Local route — all four subcommands must reject with workflow_saved_local_unsupported +# Local route — /userdata-backed CRUD # --------------------------------------------------------------------------- -class TestLocalIsRejectedCleanly: - def test_list_local_unsupported(self, local_target, capsys): +class TestLocalList: + def test_lists_userdata_workflows(self, local_target, monkeypatch, capsys): + calls = _patch_urlopen(monkeypatch, {"/userdata": _USERDATA_LIST_RESPONSE}) + env = _run(["list", "--where", "local"], capsys) + assert env["ok"] is True + assert env["data"]["count"] == 2 + # id == name == the path relative to workflows/ (what get/delete take). + ids = {r["id"] for r in env["data"]["workflows"]} + assert ids == {"flux.json", "sub/wan.json"} + # Requests the recursive full-info listing of the workflows/ dir. + assert "dir=workflows" in calls[0]["url"] + assert "full_info=true" in calls[0]["url"] + + def test_name_filter_and_limit(self, local_target, monkeypatch, capsys): + _patch_urlopen(monkeypatch, {"/userdata": _USERDATA_LIST_RESPONSE}) + env = _run(["list", "--where", "local", "--name", "wan", "--limit", "10"], capsys) + assert env["data"]["count"] == 1 + assert env["data"]["workflows"][0]["id"] == "sub/wan.json" + + def test_missing_workflows_dir_is_empty_not_error(self, local_target, monkeypatch, capsys): + # ComfyUI 404s when the workflows/ dir doesn't exist yet — that's "none saved". + _patch_urlopen(monkeypatch, {"/userdata": _http_error(404)}) + env = _run(["list", "--where", "local"], capsys) + assert env["ok"] is True + assert env["data"]["count"] == 0 + + def test_server_not_running_envelope(self, local_target, monkeypatch, capsys): + _patch_urlopen(monkeypatch, {"/userdata": urllib.error.URLError("Connection refused")}) + env = _run(["list", "--where", "local"], capsys) + assert env["ok"] is False + assert env["error"]["code"] == "server_not_running" + + def test_reachable_server_error_is_not_server_not_running(self, local_target, monkeypatch, capsys): + # A reachable server that 500s must not be mislabeled "run comfy launch". + _patch_urlopen(monkeypatch, {"/userdata": _http_error(500)}) + env = _run(["list", "--where", "local"], capsys) + assert env["ok"] is False + assert env["error"]["code"] == "server_error" + + def test_unparseable_body_surfaces_invalid_response(self, local_target, monkeypatch, capsys): + _patch_urlopen(monkeypatch, {"/userdata": (b"not json", 200)}) env = _run(["list", "--where", "local"], capsys) assert env["ok"] is False - assert env["error"]["code"] == "workflow_saved_local_unsupported" + assert env["error"]["code"] == "invalid_response" - def test_get_local_unsupported(self, local_target, capsys): - env = _run(["get", "any-id", "--where", "local"], capsys) + def test_invalid_order_rejected(self, local_target, monkeypatch, capsys): + _patch_urlopen(monkeypatch, {}) # must fail before any HTTP + env = _run(["list", "--where", "local", "--order", "sideways"], capsys) assert env["ok"] is False - assert env["error"]["code"] == "workflow_saved_local_unsupported" + assert env["error"]["code"] == "invalid_argument" - def test_delete_local_unsupported(self, local_target, capsys): - env = _run(["delete", "any-id", "--where", "local"], capsys) + def test_order_normalized_case_insensitively(self, local_target, monkeypatch, capsys): + _patch_urlopen(monkeypatch, {"/userdata": _USERDATA_LIST_RESPONSE}) + env = _run(["list", "--where", "local", "--order", "ASC"], capsys) + assert env["ok"] is True + + def test_invalid_sort_rejected(self, local_target, monkeypatch, capsys): + _patch_urlopen(monkeypatch, {}) + env = _run(["list", "--where", "local", "--sort", "bogus"], capsys) + assert env["ok"] is False + assert env["error"]["code"] == "invalid_argument" + + +class TestLocalGet: + def test_writes_content_to_file(self, local_target, tmp_path, monkeypatch, capsys): + _patch_urlopen(monkeypatch, {"/userdata/": _LOCAL_WORKFLOW}) + out = tmp_path / "got.json" + env = _run(["get", "flux.json", "--out", str(out), "--where", "local"], capsys) + assert env["ok"] is True + assert env["data"]["workflow_id"] == "flux.json" + assert env["data"]["node_count"] == 2 # frontend format → len(nodes) + assert json.loads(out.read_text()) == _LOCAL_WORKFLOW + + def test_encodes_subdir_key_as_single_segment(self, local_target, tmp_path, monkeypatch, capsys): + calls = _patch_urlopen(monkeypatch, {"/userdata/": _LOCAL_WORKFLOW}) + _run(["get", "sub/wan.json", "--out", str(tmp_path / "o.json"), "--where", "local"], capsys) + # workflows/sub/wan.json is percent-encoded whole (slashes → %2F). + assert "workflows%2Fsub%2Fwan.json" in calls[0]["url"] + + def test_404_surfaces_workflow_not_found(self, local_target, monkeypatch, capsys): + _patch_urlopen(monkeypatch, {"/userdata/": _http_error(404)}) + env = _run(["get", "ghost.json", "--where", "local"], capsys) assert env["ok"] is False - assert env["error"]["code"] == "workflow_saved_local_unsupported" + assert env["error"]["code"] == "workflow_not_found" + + def test_rejects_path_traversal(self, local_target, monkeypatch, capsys): + _patch_urlopen(monkeypatch, {}) # must fail before any HTTP + env = _run(["get", "../../etc/passwd", "--where", "local"], capsys) + assert env["ok"] is False + assert env["error"]["code"] == "invalid_argument" + + def test_rejects_windows_trailing_dot_traversal(self, local_target, monkeypatch, capsys): + # A Windows server strips trailing dots/spaces, so "sub/.. /x" collapses to + # "sub/../x" and escapes workflows/ — reject it before any HTTP. + _patch_urlopen(monkeypatch, {}) + env = _run(["get", "sub/.. /secret", "--where", "local"], capsys) + assert env["ok"] is False + assert env["error"]["code"] == "invalid_argument" + + def test_non_json_body_warns_and_writes_raw(self, local_target, tmp_path, monkeypatch, capsys): + # A 200 with a non-JSON body (e.g. an HTML error page) still gets written, + # but a warning surfaces so the corrupt fetch isn't silent. + _patch_urlopen(monkeypatch, {"/userdata/": (b"nope", 200)}) + out = tmp_path / "got.json" + env = _run(["get", "flux.json", "--out", str(out), "--where", "local"], capsys) + assert env["ok"] is True + assert env["data"]["node_count"] is None + assert any(w["code"] == "workflow_content_not_json" for w in env["data"].get("warnings", [])) + assert out.read_bytes() == b"nope" + + def test_non_utf8_body_does_not_crash(self, local_target, tmp_path, monkeypatch, capsys): + # json.loads on non-UTF-8 bytes raises UnicodeDecodeError, not JSONDecodeError. + _patch_urlopen(monkeypatch, {"/userdata/": (b"\xff\xfe\x00bad", 200)}) + out = tmp_path / "got.json" + env = _run(["get", "flux.json", "--out", str(out), "--where", "local"], capsys) + assert env["ok"] is True + assert any(w["code"] == "workflow_content_not_json" for w in env["data"].get("warnings", [])) + assert out.read_bytes() == b"\xff\xfe\x00bad" - def test_save_local_unsupported(self, local_target, tmp_path, capsys): - wf = tmp_path / "wf.json" - wf.write_text(json.dumps({"1": {"class_type": "X"}})) - env = _run(["save", str(wf), "--name", "test", "--where", "local"], capsys) + def test_write_error_surfaces_envelope(self, local_target, tmp_path, monkeypatch, capsys): + _patch_urlopen(monkeypatch, {"/userdata/": _LOCAL_WORKFLOW}) + + def _boom(self, data): + raise OSError("disk full") + + monkeypatch.setattr("pathlib.Path.write_bytes", _boom) + env = _run(["get", "flux.json", "--out", str(tmp_path / "o.json"), "--where", "local"], capsys) + assert env["ok"] is False + assert env["error"]["code"] == "workflow_write_error" + + def test_response_over_cap_refuses_to_truncate(self, local_target, tmp_path, monkeypatch, capsys): + # Shrink the cap so the mocked body exceeds it; the CLI must fail loudly + # rather than silently writing a truncated file. + monkeypatch.setattr(workflow_cmd, "_USERDATA_MAX_BYTES", 4) + _patch_urlopen(monkeypatch, {"/userdata/": (b'{"nodes": []}', 200)}) + env = _run(["get", "flux.json", "--out", str(tmp_path / "o.json"), "--where", "local"], capsys) + assert env["ok"] is False + assert env["error"]["code"] == "workflow_too_large" + + +class TestLocalSave: + def test_posts_file_bytes_and_appends_json_ext(self, local_target, tmp_path, monkeypatch, capsys): + wf = tmp_path / "src.json" + wf.write_text(json.dumps(_LOCAL_WORKFLOW)) + stored = {"path": "flux.json", "size": 120, "modified": 1700000002000, "created": 1700000000000} + calls = _patch_urlopen(monkeypatch, {"/userdata/": stored}) + env = _run(["save", str(wf), "--name", "flux", "--where", "local"], capsys) + assert env["ok"] is True + assert env["data"]["workflow_id"] == "flux.json" + assert calls[0]["method"] == "POST" + # '.json' appended to the bare --name; encoded into the userdata path. + assert "workflows%2Fflux.json" in calls[0]["url"] + assert "overwrite=true" in calls[0]["url"] + assert json.loads(calls[0]["body"]) == _LOCAL_WORKFLOW + + def test_description_ignored_with_warning(self, local_target, tmp_path, monkeypatch, capsys): + wf = tmp_path / "src.json" + wf.write_text(json.dumps(_LOCAL_WORKFLOW)) + _patch_urlopen(monkeypatch, {"/userdata/": {"path": "flux.json"}}) + env = _run(["save", str(wf), "--name", "flux.json", "--description", "hi", "--where", "local"], capsys) + assert env["ok"] is True + assert any(w["code"] == "description_ignored" for w in env["data"].get("warnings", [])) + + def test_invalid_json_file(self, local_target, tmp_path, monkeypatch, capsys): + wf = tmp_path / "bad.json" + wf.write_text("not json") + _patch_urlopen(monkeypatch, {}) + env = _run(["save", str(wf), "--name", "x", "--where", "local"], capsys) + assert env["ok"] is False + assert env["error"]["code"] == "workflow_invalid_json" + + def test_non_utf8_file_surfaces_read_error(self, local_target, tmp_path, monkeypatch, capsys): + wf = tmp_path / "bin.json" + wf.write_bytes(b"\xff\xfe\x00") # not valid UTF-8 → UnicodeDecodeError on read + _patch_urlopen(monkeypatch, {}) + env = _run(["save", str(wf), "--name", "x", "--where", "local"], capsys) + assert env["ok"] is False + assert env["error"]["code"] == "workflow_read_error" + + def test_json_ext_appended_case_insensitively(self, local_target, tmp_path, monkeypatch, capsys): + # `--name flux.JSON` already ends in .json (case-insensitive) → don't double the ext. + wf = tmp_path / "src.json" + wf.write_text(json.dumps(_LOCAL_WORKFLOW)) + calls = _patch_urlopen(monkeypatch, {"/userdata/": {"path": "flux.JSON"}}) + env = _run(["save", str(wf), "--name", "flux.JSON", "--where", "local"], capsys) + assert env["ok"] is True + assert "workflows%2Fflux.JSON" in calls[0]["url"] + assert "flux.JSON.json" not in calls[0]["url"] + + def test_missing_file(self, local_target, tmp_path, monkeypatch, capsys): + _patch_urlopen(monkeypatch, {}) + env = _run(["save", str(tmp_path / "nope.json"), "--name", "x", "--where", "local"], capsys) + assert env["ok"] is False + assert env["error"]["code"] == "workflow_not_found" + + def test_server_not_running_envelope(self, local_target, tmp_path, monkeypatch, capsys): + wf = tmp_path / "src.json" + wf.write_text(json.dumps(_LOCAL_WORKFLOW)) + _patch_urlopen(monkeypatch, {"/userdata/": urllib.error.URLError("refused")}) + env = _run(["save", str(wf), "--name", "flux", "--where", "local"], capsys) + assert env["ok"] is False + assert env["error"]["code"] == "server_not_running" + + +class TestLocalDelete: + def test_sends_delete(self, local_target, monkeypatch, capsys): + calls = _patch_urlopen(monkeypatch, {"/userdata/": (b"", 204)}) + env = _run(["delete", "flux.json", "--where", "local"], capsys) + assert env["ok"] is True + assert env["data"]["deleted"] is True + assert calls[0]["method"] == "DELETE" + assert "workflows%2Fflux.json" in calls[0]["url"] + + def test_404_surfaces_workflow_not_found(self, local_target, monkeypatch, capsys): + _patch_urlopen(monkeypatch, {"/userdata/": _http_error(404)}) + env = _run(["delete", "ghost.json", "--where", "local"], capsys) + assert env["ok"] is False + assert env["error"]["code"] == "workflow_not_found" + + def test_rejects_absolute_path(self, local_target, monkeypatch, capsys): + _patch_urlopen(monkeypatch, {}) + env = _run(["delete", "/etc/passwd", "--where", "local"], capsys) assert env["ok"] is False - assert env["error"]["code"] == "workflow_saved_local_unsupported" + assert env["error"]["code"] == "invalid_argument" # --------------------------------------------------------------------------- diff --git a/tests/comfy_cli/cql/test_engine.py b/tests/comfy_cli/cql/test_engine.py index e9b6f378..dc26e677 100644 --- a/tests/comfy_cli/cql/test_engine.py +++ b/tests/comfy_cli/cql/test_engine.py @@ -1222,3 +1222,15 @@ class _FakeMeta: # Must NOT raise "interior node images not found"; value lands on node 9's dotted widget. engine._apply_one_slot(wf, "10/9.images.image0", "X", graph) assert wf["definitions"]["subgraphs"][0]["nodes"][0]["widgets_values"][0] == "X" + + +# =========================================================================== +# SSRF loopback guard on the local object_info fetch +# =========================================================================== + + +def test_load_from_target_refuses_non_loopback_local_host(): + from comfy_cli.cql.engine import LoadError, _load_from_target + + with pytest.raises(LoadError, match="non-loopback"): + _load_from_target(mode="local", host="example.com", port=8188) diff --git a/tests/comfy_cli/cql/test_loader.py b/tests/comfy_cli/cql/test_loader.py index 6aecd084..d5b9aa23 100644 --- a/tests/comfy_cli/cql/test_loader.py +++ b/tests/comfy_cli/cql/test_loader.py @@ -6,8 +6,9 @@ import pytest +from comfy_cli.cql import loader from comfy_cli.cql.errors import CQLRuntimeError -from comfy_cli.cql.loader import load_graph, normalize +from comfy_cli.cql.loader import _load_from_server, load_graph, normalize OBJECT_INFO = { "KSampler": { @@ -118,3 +119,31 @@ def test_load_graph_bad_json(tmp_path): def test_normalize_rejects_garbage(): with pytest.raises(CQLRuntimeError): normalize({"foo": 1, "bar": "baz"}) + + +class _FakeResp: + def __init__(self, payload: bytes): + self._payload = payload + + def __enter__(self): + return self + + def __exit__(self, *exc): + return False + + def read(self, _n=None): + return self._payload + + +def test_load_from_server_refuses_non_loopback_host(): + # SSRF guard: a public host must never be fetched by the local loader. + with pytest.raises(CQLRuntimeError, match="non-loopback"): + _load_from_server("example.com", 8188, timeout=0.1) + + +def test_load_from_server_accepts_loopback(monkeypatch): + # 127.0.0.1 passes the guard and proceeds to the fetch (mocked here). + payload = json.dumps(OBJECT_INFO).encode("utf-8") + monkeypatch.setattr(loader._LOADER_OPENER, "open", lambda *a, **k: _FakeResp(payload)) + g = _load_from_server("127.0.0.1", 8188, timeout=0.1) + assert {n["name"] for n in g["nodes"]} == {"KSampler", "CheckpointLoaderSimple"} diff --git a/tests/comfy_cli/cql/test_net.py b/tests/comfy_cli/cql/test_net.py new file mode 100644 index 00000000..48a7aafb --- /dev/null +++ b/tests/comfy_cli/cql/test_net.py @@ -0,0 +1,27 @@ +"""Unit tests for the shared loopback-host SSRF guard helper.""" + +from __future__ import annotations + +import pytest + +from comfy_cli.cql._net import is_loopback_host + + +@pytest.mark.parametrize( + "host,expected", + [ + ("localhost", True), + ("LOCALHOST", True), + ("127.0.0.1", True), + ("127.0.0.2", True), + ("127.5.5.5", True), + ("::1", True), + ("0.0.0.0", False), + ("10.0.0.5", False), + ("example.com", False), + ("", False), + ("not a host", False), + ], +) +def test_is_loopback_host(host, expected): + assert is_loopback_host(host) is expected diff --git a/tests/comfy_cli/test_host_port.py b/tests/comfy_cli/test_host_port.py new file mode 100644 index 00000000..536c3879 --- /dev/null +++ b/tests/comfy_cli/test_host_port.py @@ -0,0 +1,181 @@ +"""Tests for the shared host/port resolver (`comfy_cli.host_port`) and its use +by the `comfy run` command path.""" + +from __future__ import annotations + +from unittest.mock import MagicMock, patch + +import pytest +import typer + +from comfy_cli.host_port import ( + DEFAULT_HOST, + DEFAULT_PORT, + parse_host_port_arg, + resolve_host_port, + validate_host, +) + +# --------------------------------------------------------------------------- +# parse_host_port_arg +# --------------------------------------------------------------------------- + + +@pytest.mark.parametrize( + "value,expected", + [ + ("localhost", ("localhost", None)), + ("127.0.0.1:9000", ("127.0.0.1", 9000)), + ("[::1]:8188", ("::1", 8188)), + ("[::1]", ("::1", None)), + ("::1", ("::1", None)), # bare IPv6 literal (2+ colons) -> no port + ("127.0.0.1", ("127.0.0.1", None)), + (" localhost:8080 ", ("localhost", 8080)), # whitespace is stripped + ("localhost:", ("localhost", None)), # trailing colon, empty port + ], +) +def test_parse_host_port_arg(value, expected): + assert parse_host_port_arg(value) == expected + + +def test_parse_host_port_arg_non_numeric_port_raises(): + with pytest.raises(typer.BadParameter): + parse_host_port_arg("localhost:notaport") + + +def test_parse_host_port_arg_non_numeric_bracketed_port_raises(): + with pytest.raises(typer.BadParameter): + parse_host_port_arg("[::1]:notaport") + + +def test_parse_host_port_arg_unterminated_bracket_raises(): + with pytest.raises(typer.BadParameter): + parse_host_port_arg("[::1") + + +@pytest.mark.parametrize("value", ["[::1]8188", "[::1]junk"]) +def test_parse_host_port_arg_trailing_text_after_bracket_raises(value): + # A missing/garbled ':' after ']' must error, not silently drop the port. + with pytest.raises(typer.BadParameter): + parse_host_port_arg(value) + + +@pytest.mark.parametrize("value", ["host:0", "host:-1", "host:99999", "[::1]:0", "[::1]:70000"]) +def test_parse_host_port_arg_out_of_range_port_raises(value): + with pytest.raises(typer.BadParameter): + parse_host_port_arg(value) + + +@pytest.mark.parametrize("value", [":8188", "[]:8188"]) +def test_parse_host_port_arg_empty_host_raises(value): + with pytest.raises(typer.BadParameter): + parse_host_port_arg(value) + + +# --------------------------------------------------------------------------- +# validate_host +# --------------------------------------------------------------------------- + + +@pytest.mark.parametrize("host", ["127.0.0.1", "localhost", "::1", "example.com"]) +def test_validate_host_allows_safe(host): + assert validate_host(host) == host + + +@pytest.mark.parametrize("host", ["evil.com/@x", "a/b", "h@ost", "h?ost", "h#ost"]) +def test_validate_host_rejects_url_special_chars(host): + with pytest.raises(typer.BadParameter): + validate_host(host) + + +@pytest.mark.parametrize("host", ["host\r\nX", "ho st", "host\t", "host\x00"]) +def test_validate_host_rejects_whitespace_and_control_chars(host): + with pytest.raises(typer.BadParameter): + validate_host(host) + + +# --------------------------------------------------------------------------- +# resolve_host_port +# --------------------------------------------------------------------------- + + +def _patch_background(bg): + """Patch ConfigManager as seen by resolve_host_port so `.background` == bg.""" + cfg = MagicMock() + cfg.background = bg + return patch("comfy_cli.host_port.ConfigManager", return_value=cfg) + + +def test_resolve_host_port_brackets_ipv6(): + with _patch_background(None): + assert resolve_host_port("::1", None) == ("[::1]", DEFAULT_PORT) + + +def test_resolve_host_port_does_not_double_bracket(): + with _patch_background(None): + assert resolve_host_port("[::1]", 8188) == ("[::1]", 8188) + + +def test_resolve_host_port_rejects_unsafe_host(): + with _patch_background(None), pytest.raises(typer.BadParameter): + resolve_host_port("evil.com/@x", None) + + +def test_resolve_host_port_applies_defaults(): + with _patch_background(None): + assert resolve_host_port(None, None) == (DEFAULT_HOST, DEFAULT_PORT) + + +def test_resolve_host_port_falls_back_to_background(): + # config.background is a (host, port, pid) tuple. + with _patch_background(("10.0.0.5", 9001, 4242)): + assert resolve_host_port(None, None) == ("10.0.0.5", 9001) + + +def test_resolve_host_port_explicit_overrides_background(): + with _patch_background(("10.0.0.5", 9001, 4242)): + assert resolve_host_port("localhost", 7000) == ("localhost", 7000) + + +# --------------------------------------------------------------------------- +# `comfy run` integration — host flows through the resolver +# --------------------------------------------------------------------------- + + +def _write_workflow(tmp_path): + wf = tmp_path / "wf.json" + wf.write_text('{"1": {"class_type": "X", "inputs": {}}}') + return str(wf) + + +def test_run_ipv6_host_port_reaches_execute(tmp_path): + from typer.testing import CliRunner + + from comfy_cli.cmdline import app + + with patch("comfy_cli.command.run.execute") as mock_execute: + result = CliRunner().invoke( + app, + ["run", "--workflow", _write_workflow(tmp_path), "--host", "[::1]:8188"], + env={"COMFY_WHERE": "local"}, + ) + assert result.exit_code == 0, result.output + args, _ = mock_execute.call_args + # execute(workflow, host, port, ...) + assert args[1] == "[::1]" + assert args[2] == 8188 + + +def test_run_unsafe_host_exits_before_execute(tmp_path): + from typer.testing import CliRunner + + from comfy_cli.cmdline import app + + with patch("comfy_cli.command.run.execute") as mock_execute: + result = CliRunner().invoke( + app, + ["run", "--workflow", _write_workflow(tmp_path), "--host", "x/@y"], + env={"COMFY_WHERE": "local"}, + ) + assert result.exit_code != 0 + mock_execute.assert_not_called() diff --git a/tests/comfy_cli/test_http.py b/tests/comfy_cli/test_http.py new file mode 100644 index 00000000..7bac0220 --- /dev/null +++ b/tests/comfy_cli/test_http.py @@ -0,0 +1,44 @@ +import http.client +import urllib.error +import urllib.request + +import pytest + +from comfy_cli.http import NoRedirectHandler + + +def _call(handler, method_name, code=302): + req = urllib.request.Request("https://example.com/thing") + headers = http.client.HTTPMessage() + method = getattr(handler, method_name) + method(req, None, code, "Found", headers) + + +@pytest.mark.parametrize( + "method_name", + ["http_error_301", "http_error_302", "http_error_303", "http_error_307", "http_error_308"], +) +def test_refuses_every_redirect_status(method_name): + handler = NoRedirectHandler() + with pytest.raises(urllib.error.HTTPError) as exc_info: + _call(handler, method_name, code=308) + err = exc_info.value + assert err.code == 308 # status code is preserved + assert str(err.reason) == "redirect refused" # default message + assert err.url == "https://example.com/thing" + + +def test_default_message(): + handler = NoRedirectHandler() + with pytest.raises(urllib.error.HTTPError) as exc_info: + _call(handler, "http_error_301", code=301) + assert exc_info.value.code == 301 + assert str(exc_info.value.reason) == "redirect refused" + + +def test_custom_message_passthrough(): + handler = NoRedirectHandler("redirect refused (auth leak prevention)") + with pytest.raises(urllib.error.HTTPError) as exc_info: + _call(handler, "http_error_302", code=302) + assert str(exc_info.value.reason) == "redirect refused (auth leak prevention)" + assert exc_info.value.code == 302 diff --git a/tests/comfy_cli/test_install_python_resolution.py b/tests/comfy_cli/test_install_python_resolution.py index 8736fe56..f5d44137 100644 --- a/tests/comfy_cli/test_install_python_resolution.py +++ b/tests/comfy_cli/test_install_python_resolution.py @@ -283,6 +283,7 @@ class TestTorchInstallCommands: @pytest.mark.parametrize( "rocm_version,expected_url", [ + (constants.ROCmVersion.v7_2, "https://download.pytorch.org/whl/rocm7.2"), (constants.ROCmVersion.v7_1, "https://download.pytorch.org/whl/rocm7.1"), (constants.ROCmVersion.v7_0, "https://download.pytorch.org/whl/rocm7.0"), (constants.ROCmVersion.v6_3, "https://download.pytorch.org/whl/rocm6.3"), diff --git a/tests/uv/test_uv.py b/tests/uv/test_uv.py index bf2933c2..eefa1374 100644 --- a/tests/uv/test_uv.py +++ b/tests/uv/test_uv.py @@ -89,7 +89,7 @@ def test_torch_backend_nvidia(): def test_torch_backend_amd(): depComp = DependencyCompiler(cwd=temp, gpu=GPU_OPTION.AMD, outDir=temp, reqFilesCore=[], reqFilesExt=[]) - assert depComp.torchBackend == "rocm6.3" + assert depComp.torchBackend == "rocm7.2" assert depComp.gpuUrl == DependencyCompiler.rocmPytorchUrl