fix(http): route the remaining bare urlopen() call sites through an http(s)-only opener (BE-3313)

[mattmillerai]

ELI-5

When Python opens a URL the "normal" way (urllib.request.urlopen()), it hands you an opener that knows how to speak http:// and also file://, ftp:// and data:. So if a URL ever came from somewhere untrustworthy, "fetch this" could quietly turn into "read a file off my disk."

#542 fixed that for every opener that carries a password. This PR does the same for the last few call sites that had no password attached — they still shouldn't be able to read local files. Nothing about how they behave changes; they just can't speak those extra languages anymore.

What changed

Adds a shared _PLAIN_OPENER + plain_urlopen() to comfy_cli/http.py for the uncredentialed case (the credentialed counterpart of _AUTHED_OPENER / authed_urlopen), and routes every remaining bare-urlopen site through it:

Site	URL
templates._fetch_gallery	GALLERY_URL
templates._fetch_template_workflow	_TEMPLATE_WORKFLOW_URL
jobs._http_get_json	http://{host}:{port}
jobs._local_cancel → POST /queue	http://{host}:{port}
jobs._local_cancel → POST /interrupt	http://{host}:{port}
run/execution.queue → POST /prompt	http://{host}:{port}
run/preflight.fetch_object_info → GET /object_info	http://{host}:{port}

grep -rn "urlopen(" comfy_cli/ now returns zero call sites on the global default opener.

Severity: low — same framing as #530 / #542

None of these attach Authorization / X-API-Key, so there is no credential-replay exposure. The URLs are also trusted today (a module-level constant; loopback host/port). This is defense-in-depth against a future caller passing an attacker-influenced URL, not a live exploit.

Judgment calls

1. Two extra call sites, beyond the five the ticket enumerated. run/execution.py and run/preflight.py import from urllib import request and call request.urlopen(...), so a grep for urllib.request.urlopen misses them. They are the same class of gap (global default opener, no credentials, loopback URL), so leaving them would have shipped a fix that only looked complete. Flagging as scope the ticket didn't ask for.

2. One shared opener rather than a module-level opener per site. The ticket sketched _TEMPLATES_OPENER, _JOBS_OPENER, etc. I went with a single _PLAIN_OPENER because all seven sites want identical policy (http(s)-only, follows redirects, no credentials), and because the existing tests patch one urlopen seam that covers both the preflight and execution paths — four separate openers would have fragmented that seam and turned a mechanical test update into a rewrite. Happy to split them if you'd rather have per-site openers.

3. Redirects are still followed — deliberately. build_http_only_opener() installs no redirect handler unless you pass one, so a bare opener would have converted these into no-redirect openers and started raising HTTPError on a 30x that used to be followed transparently. All seven sites follow redirects today, so HTTPRedirectHandler() is passed explicitly to preserve that. Not silently changed.

4. Follow-up worth a separate look (not done here). run/execution.queue's POST /prompt body can carry api_key_comfy_org inside extra_data — a credential in the body rather than a header. Following a redirect would replay it. That is pre-existing behavior (the default opener did exactly this), so changing it is out of scope for a defense-in-depth PR that promises to preserve behavior — but a NoRedirectHandler there may be warranted. Say the word and I'll file it.

Tests

tests/comfy_cli/test_http_only_openers.py — _PLAIN_OPENER appended to the table-driven OPENERS list (so it inherits the scheme-refusal + handler-set guards), plus test_uncredentialed_openers_still_follow_redirects, mirroring test_download_opener_still_follows_redirects, to lock in that pinning the scheme did not make it a no-redirect opener.

Test patch targets in test_run.py / test_run_json.py / test_jobs.py moved from urllib.request.urlopen to the _PLAIN_OPENER.open seam — mechanical, and test_queue_passes_timeout_to_urlopen still proves the seam is live rather than vacuously green.

Verification

• uv run python -m pytest tests/comfy_cli — 2528 passed, 13 skipped
• uv run ruff format --check comfy_cli tests — clean (236 files)
• uv run ruff check on changed files — clean (14 pre-existing repo-wide errors are unrelated and present on the base branch)
• Behavior verified live against a local HTTP server, not just by handler introspection: a 302 is followed transparently (200, final URL /final), while file:///etc/passwd, ftp://example.com/x and data:text/plain,hi each raise URLError: unknown url type.

Note: this is a capability-preserving change — no deny/dead-end path is added, so there is no negative claim to falsify.

Stacked on #542

Base is matt/be-3298-http-only-openers — build_http_only_opener() isn't on main yet. GitHub will retarget this to main once #542 merges. Review only the top commit.

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 3bb23995-173a-470b-adc4-a81a880f6ae3

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch matt/be-3313-bare-urlopen-openers

✨ Simplify code

• Create PR with simplified code
• Commit simplified code in branch matt/be-3313-bare-urlopen-openers

---

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

🔍 Cursor Review — Consolidated panel

Triggered by @mattmillerai.

Found 2 finding(s).

Severity	Count
🟢 Low	2

Panel: 6/8 reviewers contributed findings.

Reviewers that did not contribute: kimi-k2.5:adversarial (empty), kimi-k2.5:edge-case (empty)