fix(http): pin the remaining credential-carrying urllib openers to http(s) handlers only (BE-3298)

comfy_cli/cloud/oauth.py

@@ -52,7 +52,7 @@
     CLIENT_NAME,
     get_base_url,
 )
-from comfy_cli.http import NoRedirectHandler
+from comfy_cli.http import NoRedirectHandler, build_http_only_opener
 
 # ---------------------------------------------------------------------------
 # Error types — caller maps these to renderer.error(code=...) codes.
@@ -882,7 +882,7 @@ def _assert_https_or_loopback(url: str) -> None:
     raise _HTTPFail(0, f"refusing plaintext HTTP for OAuth endpoint: {url}")
 
 
-_OAUTH_OPENER = urllib.request.build_opener(NoRedirectHandler())
+_OAUTH_OPENER = build_http_only_opener(NoRedirectHandler())
 
 
 def _send_and_parse(req: urllib.request.Request) -> dict:

comfy_cli/comfy_client.py

@@ -23,7 +23,7 @@
 from dataclasses import dataclass
 from typing import Any
 
-from comfy_cli.http import NoRedirectHandler
+from comfy_cli.http import NoRedirectHandler, build_http_only_opener
 from comfy_cli.target import Target
 
 _LOOPBACK_HOSTS = {"localhost", "127.0.0.1", "::1", "[::1]"}
@@ -95,7 +95,7 @@ class Unauthenticated(Exception):
     """Target needs auth but no valid session is present."""
 
 
-_OPENER = urllib.request.build_opener(NoRedirectHandler())
+_OPENER = build_http_only_opener(NoRedirectHandler())
 
 
 def _assert_safe_url(url: str) -> None:

comfy_cli/command/jobs.py

@@ -36,7 +36,7 @@
 from comfy_cli import cancellation, execution_errors, tracking
 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.http import authed_urlopen
+from comfy_cli.http import authed_urlopen, plain_urlopen
 from comfy_cli.output import get_renderer
 from comfy_cli.where import cloud_preflight_or_exit
 
@@ -84,7 +84,7 @@ def _server_or_error(host: str, port: int, *, raise_on_missing: bool = True) ->
 def _http_get_json(url: str, *, timeout: float = 10.0) -> Any:
     req = urllib.request.Request(url)
     try:
-        with urllib.request.urlopen(req, timeout=timeout) as resp:
+        with plain_urlopen(req, timeout=timeout) as resp:
             return json.loads(resp.read())
     except urllib.error.URLError as e:
         raise RuntimeError(f"failed to GET {url}: {e}") from e
@@ -992,7 +992,7 @@ def _local_cancel(prompt_id: str, host: str, port: int) -> None:
     )
     queue_ok = True
     try:
-        with urllib.request.urlopen(queue_req, timeout=10) as resp:
+        with plain_urlopen(queue_req, timeout=10) as resp:
             _ = resp.read()
     except (urllib.error.HTTPError, urllib.error.URLError, OSError):
         # Server refused the delete; common when the prompt isn't in queue.
@@ -1016,7 +1016,7 @@ def _local_cancel(prompt_id: str, host: str, port: int) -> None:
     if prompt_id in running_ids:
         interrupt_req = urllib.request.Request(f"{base}/interrupt", method="POST")
         try:
-            with urllib.request.urlopen(interrupt_req, timeout=10) as resp:
+            with plain_urlopen(interrupt_req, timeout=10) as resp:
                 _ = resp.read()
         except (urllib.error.HTTPError, urllib.error.URLError, OSError):
             interrupt_ok = False

comfy_cli/command/run/execution.py

@@ -38,6 +38,7 @@
 
 from comfy_cli import execution_errors
 from comfy_cli.command.run.loader import _MAX_BODY_PREVIEW, _node_errors_to_list
+from comfy_cli.http import no_redirect_urlopen
 from comfy_cli.output import get_renderer
 from comfy_cli.output import rprint as pprint
 from comfy_cli.workspace_manager import WorkspaceManager
@@ -167,8 +168,12 @@ def queue(self):
         req = request.Request(f"http://{self.host}:{self.port}/prompt", json.dumps(data).encode("utf-8"))
         req.add_header("Comfy-Usage-Source", "comfy-cli")
         try:
-            resp = request.urlopen(req, timeout=self.timeout)
-            raw_body = resp.read()
+            # No-redirect, not ``plain_urlopen``: ``extra_data`` can carry a
+            # Comfy Org credential, so this submit gets the same refuse-a-30x
+            # policy as every other credentialed call rather than leaning on
+            # urllib happening to drop the body when it follows a redirect.
+            with no_redirect_urlopen(req, timeout=self.timeout) as resp:
+                raw_body = resp.read()
         except urllib.error.HTTPError as e:
             body_bytes = e.read()
             body_text = body_bytes.decode("utf-8", errors="replace").strip() if body_bytes else ""

comfy_cli/command/run/preflight.py

@@ -11,11 +11,11 @@
 
 import json
 import urllib.error
-from urllib import request
 
 import typer
 
 from comfy_cli.command.run.loader import _MAX_BODY_PREVIEW
+from comfy_cli.http import plain_urlopen
 from comfy_cli.output import get_renderer
 from comfy_cli.output import rprint as pprint
 
@@ -24,6 +24,11 @@
 # the authoritative signal is the `api_node: true` flag.
 PARTNER_NODE_CATEGORY_PREFIXES = ("partner/",)
 
+# Cap on what we'll pull off the wire for /object_info, success or error. A
+# real schema dump is a few MiB at most; the bound is there so a wedged or
+# hostile server can't stream us out of memory.
+_MAX_OBJECT_INFO_BYTES = 64 * 1024 * 1024
+
 
 def fetch_object_info(host, port, timeout):
     """GET ``/object_info`` from the running ComfyUI server.
@@ -37,10 +42,10 @@ def fetch_object_info(host, port, timeout):
     renderer = get_renderer()
     url = f"http://{host}:{port}/object_info"
     try:
-        with request.urlopen(url, timeout=timeout) as resp:
-            body = resp.read(64 * 1024 * 1024)
+        with plain_urlopen(url, timeout=timeout) as resp:
+            body = resp.read(_MAX_OBJECT_INFO_BYTES)
     except urllib.error.HTTPError as e:
-        body_text = e.read().decode("utf-8", errors="replace").strip()
+        body_text = e.read(_MAX_OBJECT_INFO_BYTES).decode("utf-8", errors="replace").strip()
         renderer.error(
             code="object_info_unavailable",
             message=f"Failed to fetch /object_info (HTTP {e.code})",

comfy_cli/command/templates.py

@@ -28,6 +28,7 @@
 import typer
 
 from comfy_cli import tracking
+from comfy_cli.http import plain_urlopen
 from comfy_cli.output import get_renderer, rprint
 
 app = typer.Typer(no_args_is_help=True, help="Browse the Comfy workflow-template gallery.")
@@ -48,7 +49,7 @@ def _cache_path() -> Path:
 
 def _fetch_gallery(url: str = GALLERY_URL, timeout: float = 15.0) -> bytes:
     req = urllib.request.Request(url, headers={"User-Agent": "comfy-cli"})
-    with urllib.request.urlopen(req, timeout=timeout) as resp:
+    with plain_urlopen(req, timeout=timeout) as resp:
         if resp.status != 200:
             raise RuntimeError(f"gallery fetch failed: HTTP {resp.status}")
         return resp.read()
@@ -410,7 +411,7 @@ def _fetch_template_workflow(name: str, *, timeout: float = 15.0) -> bytes:
     """Pull a single template's workflow JSON from the canonical GitHub raw URL."""
     url = _TEMPLATE_WORKFLOW_URL.format(name=urllib.parse.quote(name, safe=""))
     req = urllib.request.Request(url, headers={"User-Agent": "comfy-cli"})
-    with urllib.request.urlopen(req, timeout=timeout) as resp:
+    with plain_urlopen(req, timeout=timeout) as resp:
         if resp.status != 200:
             raise RuntimeError(f"template workflow fetch failed: HTTP {resp.status}")
         return resp.read()

comfy_cli/command/transfer.py

@@ -27,7 +27,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.http import NoRedirectHandler, build_http_only_opener
 from comfy_cli.output import get_renderer
 from comfy_cli.output import rprint as pprint
 from comfy_cli.target import resolve_target
@@ -105,8 +105,8 @@ def redirect_request(self, req, fp, code, msg, headers, newurl):
         return new_req
 
 
-_TRANSFER_OPENER = urllib.request.build_opener(NoRedirectHandler("redirect refused (auth leak prevention)"))
-_DOWNLOAD_OPENER = urllib.request.build_opener(_DownloadRedirectHandler())
+_TRANSFER_OPENER = build_http_only_opener(NoRedirectHandler("redirect refused (auth leak prevention)"))
+_DOWNLOAD_OPENER = build_http_only_opener(_DownloadRedirectHandler())
 
 # Per-output safety cap, shared by the HTTP download stream and local-output copies.
 _MAX_DOWNLOAD_BYTES = 10 * 1024 * 1024 * 1024  # 10 GB

comfy_cli/cql/engine.py

@@ -22,7 +22,7 @@
 from typing import Any
 
 from comfy_cli.cql._net import is_loopback_host
-from comfy_cli.http import NoRedirectHandler
+from comfy_cli.http import NoRedirectHandler, build_http_only_opener
 
 # ---------------------------------------------------------------------------
 # Types — mirrors nodegraph/types.go
@@ -1194,7 +1194,7 @@ def _check_autogrow_required(
 _MAX_OBJECT_INFO_BYTES = 64 * 1024 * 1024
 
 
-_opener = urllib.request.build_opener(NoRedirectHandler())
+_opener = build_http_only_opener(NoRedirectHandler())
 
 
 class LoadError(Exception):

comfy_cli/cql/loader.py

@@ -30,15 +30,15 @@
 
 from comfy_cli.cql._net import is_loopback_host
 from comfy_cli.cql.errors import CQLRuntimeError
-from comfy_cli.http import NoRedirectHandler
+from comfy_cli.http import NoRedirectHandler, build_http_only_opener
 
 # 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
 # server and would just OOM the CLI before json.loads even fails.
 MAX_INPUT_BYTES = 256 * 1024 * 1024
 
 
-_LOADER_OPENER = urllib.request.build_opener(NoRedirectHandler())
+_LOADER_OPENER = build_http_only_opener(NoRedirectHandler())
 
 
 def load_graph(

comfy_cli/http.py

@@ -26,31 +26,94 @@ def http_error_301(self, req, fp, code, msg, headers):
     http_error_302 = http_error_303 = http_error_307 = http_error_308 = http_error_301
 
 
-def _build_authed_opener() -> urllib.request.OpenerDirector:
-    """Build the credential-carrying opener with http/https handlers only.
-
-    ``build_opener()`` would also install ``FileHandler``/``FTPHandler``. Every
-    call site builds its URL from a trusted ``target.base_url``, so that isn't
-    reachable today, but this is the opener that attaches credentials — pinning
-    it to http(s) means a future caller can't be steered into a ``file://`` or
-    ``ftp://`` fetch. Unknown schemes fall to ``UnknownHandler``, which raises
+def _http_only_proxy_handler() -> urllib.request.ProxyHandler:
+    """A ProxyHandler that can only proxy http(s).
+
+    ``ProxyHandler()`` defaults to ``getproxies()`` and registers a
+    ``<scheme>_open`` method for *every* entry it finds, so an ``ftp_proxy`` in
+    the environment would give the opener an ``ftp_open`` — servicing
+    ``ftp://`` through the proxy and stepping straight past the
+    ``UnknownHandler`` that ``build_http_only_opener`` relies on. Filtering the
+    map to http(s) keeps real proxy support (``proxy_bypass``/``no_proxy`` read
+    the environment directly, not this dict) while leaving non-http schemes
+    with nowhere to go.
+    """
+    proxies = {scheme: url for scheme, url in urllib.request.getproxies().items() if scheme in ("http", "https")}
+    return urllib.request.ProxyHandler(proxies)
+
+
+def build_http_only_opener(*handlers: urllib.request.BaseHandler) -> urllib.request.OpenerDirector:
+    """Build an opener that speaks http(s) and nothing else.
+
+    ``build_opener()`` would also install ``FileHandler``/``FTPHandler``/
+    ``DataHandler``. Our call sites build their URLs from a trusted
+    ``target.base_url``, so that isn't reachable today, but these openers
+    attach credentials — pinning them to http(s) means a future caller can't
+    be steered into a ``file://``, ``ftp://`` or ``data:`` fetch. Unknown
+    schemes fall to ``UnknownHandler``, which raises
     ``URLError("unknown url type")``.
+
+    ``handlers`` are the caller's own additions (e.g. a redirect policy). As in
+    ``build_opener``, a caller-supplied handler replaces the default it
+    subclasses rather than being appended behind it. Note that no redirect
+    handler is installed unless the caller passes one, so a bare opener
+    surfaces a 30x as an ``HTTPError`` rather than following it.
     """
+    defaults = [
+        (urllib.request.ProxyHandler, _http_only_proxy_handler),
+        (urllib.request.HTTPHandler, urllib.request.HTTPHandler),
+        (urllib.request.HTTPDefaultErrorHandler, urllib.request.HTTPDefaultErrorHandler),
+        (urllib.request.HTTPErrorProcessor, urllib.request.HTTPErrorProcessor),
+        (urllib.request.UnknownHandler, urllib.request.UnknownHandler),
+    ]
+    # urllib.request only defines HTTPSHandler on an SSL-capable build; naming it
+    # unconditionally would blow up at import time on one without.
+    if hasattr(urllib.request, "HTTPSHandler"):
+        defaults.append((urllib.request.HTTPSHandler, urllib.request.HTTPSHandler))
+
     opener = urllib.request.OpenerDirector()
-    for handler in (
-        urllib.request.ProxyHandler(),
-        urllib.request.HTTPHandler(),
-        urllib.request.HTTPSHandler(),
-        urllib.request.HTTPDefaultErrorHandler(),
-        urllib.request.HTTPErrorProcessor(),
-        NoRedirectHandler(),
-        urllib.request.UnknownHandler(),
-    ):
+    for klass, factory in defaults:
+        if not any(isinstance(handler, klass) for handler in handlers):
+            opener.add_handler(factory())
+    for handler in handlers:
         opener.add_handler(handler)
     return opener
 
 
-_AUTHED_OPENER = _build_authed_opener()
+_AUTHED_OPENER = build_http_only_opener(NoRedirectHandler())
+
+# The uncredentialed fetches — the template gallery on raw.githubusercontent.com
+# and the REST calls against a local ``http://{host}:{port}`` ComfyUI server.
+# These reached for ``urllib.request.urlopen()``, i.e. the global default
+# opener, which also speaks ``file://``, ``ftp://`` and ``data:``. Nothing here
+# attaches a credential header, so unlike ``_AUTHED_OPENER`` there is no
+# redirect-replay exposure and ``HTTPRedirectHandler`` is installed explicitly
+# to keep the redirect-following those call sites have always had. What the
+# pinning buys is that a URL which stops being trusted — a gallery URL that
+# becomes configurable, say — still can't be steered into a local-file read.
+_PLAIN_OPENER = build_http_only_opener(urllib.request.HTTPRedirectHandler())
+
+
+def plain_urlopen(url, *, timeout: float = 30.0):
+    """Open an uncredentialed request via the http(s)-only shared opener.
+
+    ``url`` is a full URL or a prepared ``Request``. Redirects are followed, as
+    they were when these call sites used the global default opener.
+    """
+    return _PLAIN_OPENER.open(url, timeout=timeout)
+
+
+def no_redirect_urlopen(url, *, timeout: float = 30.0):
+    """Open a prepared credential-bearing ``Request`` without following redirects.
+
+    ``authed_urlopen`` covers the common case where the credential rides a
+    header we attach ourselves. This is the escape hatch for a request whose
+    credential the caller has already placed somewhere we can't build — the
+    ``/prompt`` submit carries ``api_key_comfy_org`` inside its JSON body — and
+    which therefore wants the same no-redirect policy without the header
+    mechanics.
+    """
+    return _AUTHED_OPENER.open(url, timeout=timeout)
 
 
 def build_authed_request(

tests/comfy_cli/command/github/test_pr.py

@@ -179,7 +179,7 @@ def test_find_pr_by_branch_error(self, mock_get):
 
     @patch("requests.get")
     def test_find_pr_by_branch_rate_limit(self, mock_get):
-        """A rate-limited 403 surfaces as GitHubRateLimitError, not a silent "no PR found\""""
+        """A rate-limited 403 surfaces as GitHubRateLimitError, not a silent "no PR found\" """
         mock_response = Mock()
         mock_response.status_code = 403
         mock_response.headers = {"x-ratelimit-remaining": "0", "x-ratelimit-reset": "1777415867"}