fix(comfy): allow private service hostnames

[jacobtomlinson]

Summary

• What problem this solves: Comfy local workflows could not use Docker Compose service hostnames such as http://comfyui:8188 because the provider only built its private-network SSRF policy when the hostname already looked private before DNS.
• Why this change was made: the actual SSRF block happens after DNS resolution, so service-discovery names need the configured Comfy host to receive the private-network policy when local/private access is intended.
• User impact: users can configure the bundled Comfy plugin with stable service names instead of brittle bridge or gateway IP literals.
• Intended outcome: Comfy local mode allows loopback/private literals and single-label service hostnames; single-label names use exact-host trust, and public-looking FQDNs remain strict unless allowPrivateNetwork: true is explicit.
• Out of scope: no config migration, no broader SSRF policy changes, and no live ComfyUI workflow changes.
• Reviewer focus: the split between local default trust and explicit operator trust in resolveComfyNetworkPolicy().

Linked context

No GitHub issue found in the handoff.

Related local handoff: OpenClaw ComfyUI SSRF guard fix summary.

Requested by a maintainer/user through the handoff for the myagents Comfy provider setup.

Real behavior proof (required for external PRs)

• Behavior or issue addressed: Comfy provider SSRF policy construction for service-discovery hostnames.
• Real environment tested: local source checkout on Node 24, using the Comfy provider tests and the real changed-lane type/lint guard. The Comfy HTTP responses are test-controlled; no secrets or live ComfyUI instance were used.
• Exact steps or command run after this patch:
  • node scripts/run-vitest.mjs extensions/comfy/image-generation-provider.test.ts
  • node scripts/run-vitest.mjs extensions/comfy
  • env OPENCLAW_CHECK_CHANGED_REMOTE_CHILD=1 OPENCLAW_CHANGED_LANES_RAW_SYNC=1 node scripts/check-changed.mjs --base upstream/main --head HEAD --timed
  • .agents/skills/autoreview/scripts/autoreview --mode local --base origin/main
  • git diff --check
• Evidence after fix: targeted image-provider tests passed with 16 tests; full Comfy extension tests passed with 5 files and 30 tests; changed gate passed for extensions and extensionTests; final autoreview against upstream/main reported no accepted/actionable findings.
• Observed result after fix: http://comfyui:8188 in local mode receives exact-host trust without the dangerous private-network flag, local public-looking FQDNs stay strict by default, cloud mode stays strict unless explicitly opted in, explicit cloud opt-in receives allowPrivateNetwork: true, and cloud output redirects do not inherit the Comfy private-network policy.
• What was not tested: the original Docker Compose/myagents stack and a live ComfyUI service were not run from this checkout.
• Proof limitations or environment constraints: Blacksmith Testbox proof could not run locally because the blacksmith CLI is unavailable; Azure Crabbox fallback also could not run because Azure CLI/login is unavailable. The local changed gate was run with an explicit upstream base instead.
• Before evidence: the handoff included the prior runtime log line Blocked: resolves to private/internal/special-use IP address for targetOrigin=http://comfyui:8188.

Tests and validation

Commands run:

• git diff --check
• node scripts/run-vitest.mjs extensions/comfy/image-generation-provider.test.ts
• node scripts/run-vitest.mjs extensions/comfy
• node scripts/run-oxlint.mjs extensions/comfy/workflow-runtime.ts extensions/comfy/image-generation-provider.test.ts
• env OPENCLAW_CHECK_CHANGED_REMOTE_CHILD=1 OPENCLAW_CHANGED_LANES_RAW_SYNC=1 node scripts/check-changed.mjs --base upstream/main --head HEAD --timed
• .agents/skills/autoreview/scripts/autoreview --mode local --base origin/main

Regression coverage added:

• Local single-label service hostnames get exact-host trust without the dangerous private-network flag.
• Local public-looking FQDNs remain strict without explicit private-network opt-in.
• Cloud service hostnames remain strict by default.
• Cloud service hostnames honor explicit allowPrivateNetwork: true.

Known failure before this fix:

• http://comfyui:8188 did not get an SSRF policy because comfyui is not an IP literal or localhost, so the shared fetch guard blocked it after DNS resolved it to the Compose private network.

Risk checklist

Did user-visible behavior change? Yes

Did config, environment, or migration behavior change? No

Did security, auth, secrets, network, or tool execution behavior change? Yes

Highest-risk area: SSRF policy construction for Comfy provider network calls.

Risk mitigation: implicit service-discovery hostnames use exact-host trust so the shared SSRF guard still blocks link-local/metadata DNS answers; the dangerous private-network flag is reserved for private/loopback literals or explicit operator opt-in; public-looking FQDNs require explicit allowPrivateNetwork: true; cloud redirect downloads do not inherit the Comfy private-network policy.

Current review state

Next action: maintainer review and CI.

Still waiting on: PR CI and optional live Docker Compose/ComfyUI proof if reviewers want product-level confirmation.

Bot/reviewer comments addressed: local autoreview initially flagged an overly broad trust change; the patch was tightened and autoreview reran clean.

[clawsweeper]

Codex review: needs real behavior proof before merge. Reviewed July 2, 2026, 8:05 AM ET / 12:05 UTC.

Summary
The PR changes the bundled Comfy workflow runtime to build scoped SSRF policies for local single-label service hostnames and explicit private-network opt-in, with image-provider regression tests.

PR surface: Source +26, Tests +169. Total +195 across 2 files.

Reproducibility: yes. at source level: current main sets local Comfy private-network intent but drops non-private-looking hostnames before building the SSRF policy, so the shared guard blocks private DNS answers. I did not run the Docker Compose or live ComfyUI scenario in this read-only review.

Review metrics: 1 noteworthy metric.

• Comfy SSRF trust path: 1 local single-label exact-host branch added. This is the maintainer-relevant behavior change because it decides which configured Comfy hostnames may reach private DNS results.

Stored data model
Persistent data-model change detected: serialized state: extensions/comfy/image-generation-provider.test.ts. Confirm migration or upgrade compatibility proof before merge.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #77922
Summary: The current PR is a candidate fix for the open Comfy private-DNS SSRF bug, while an older direct fix PR for the same issue is closed unmerged.

Members:

• canonical: [Bug]: Comfy plugin: allowPrivateNetwork: true in local mode does not bypass SSRF guard when baseUrl is a DNS hostname resolving to a private IP #77922 - The issue is open and reports the same Comfy local/private DNS hostname SSRF block that this PR addresses.
• superseded: fix(comfy): bypass SSRF guard for DNS hostnames when allowPrivateNetwork is set in local mode #77944 - The older direct PR used closing syntax for the same issue but was closed unmerged after only mocked proof.

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🦪 silver shellfish
Proof: 🦪 silver shellfish
Patch quality: 🐚 platinum hermit
Result: blocked until real behavior proof from a real setup is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

• [P1] Add redacted terminal output, logs, or a recording from a real Docker Compose or live ComfyUI setup using the service hostname after the fix.
• State whether this PR is meant to close [Bug]: Comfy plugin: allowPrivateNetwork: true in local mode does not bypass SSRF guard when baseUrl is a DNS hostname resolving to a private IP #77922 or only the single-label service-host subset.

Proof guidance:

• [P1] Needs real behavior proof before merge: The PR body reports targeted tests and local checks, but it explicitly does not include a live Docker Compose or real ComfyUI run after the fix. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Risk before merge

• [P1] No live Docker Compose or ComfyUI proof is in the PR yet; the posted validation is mocked/unit/gate output, so the actual service-host DNS path still needs real behavior proof before merge.
• [P1] The PR changes Comfy SSRF trust for local hostnames, so maintainers need to accept the exact-host single-label policy split and decide whether it fully covers [Bug]: Comfy plugin: allowPrivateNetwork: true in local mode does not bypass SSRF guard when baseUrl is a DNS hostname resolving to a private IP #77922 or only the service-host subset.

Maintainer options:

1. Require live service-host proof (recommended)
   Ask for redacted Docker Compose or live ComfyUI output showing the configured service hostname succeeds after the fix and that the scoped SSRF policy is the path being exercised.
2. Accept source-level proof only
   Maintainers may intentionally merge with unit/CI proof only if they explicitly accept the service-host SSRF policy split as adequately reviewed without live runtime proof.
3. Pause for canonical issue scope
   If this PR is expected to close the broader private DNS FQDN issue, pause until the PR states and proves that explicit-opt-in FQDN scope.

Next step before merge

• [P1] Human review is needed because the PR has a protected maintainer label, changes an SSRF trust boundary, and lacks contributor-provided real behavior proof; there is no narrow automation defect to repair.

Security
Cleared: No concrete security defect was found in the diff; the new local service-host path uses exact-host trust and the shared guard still blocks metadata/link-local rebinding for that trust mode.

Review details

Best possible solution:

Land a scoped Comfy plugin SSRF fix only after redacted real Comfy/Docker service-host proof and maintainer/security acceptance; link the PR to #77922 if it is intended to close that broader DNS-hostname bug.

Do we have a high-confidence way to reproduce the issue?

Yes at source level: current main sets local Comfy private-network intent but drops non-private-looking hostnames before building the SSRF policy, so the shared guard blocks private DNS answers. I did not run the Docker Compose or live ComfyUI scenario in this read-only review.

Is this the best way to solve the issue?

Yes as a bounded PR direction, with a remaining maintainer decision: the patch is more conservative than the older closed PR by exact-trusting local single-label hostnames and requiring explicit opt-in for public-looking FQDNs. It should not merge until live proof and the intended relationship to #77922 are clear.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 4f933ccf7b62.

Label changes

Label justifications:

• P2: This is a bounded bundled-plugin bug fix for local Comfy private-network use with limited blast radius.
• merge-risk: 🚨 security-boundary: The diff changes which Comfy hostnames can bypass private-network DNS blocking under the shared SSRF guard.
• rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🐚 platinum hermit.
• status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body reports targeted tests and local checks, but it explicitly does not include a live Docker Compose or real ComfyUI run after the fix. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Evidence reviewed

PR surface:

Source +26, Tests +169. Total +195 across 2 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	1	29	3	+26
Tests	1	169	0	+169
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	2	198	3	+195

What I checked:

• Current main still has the hostname gate: Current main only builds a Comfy private-network SSRF policy when the configured hostname already passes isPrivateOrLoopbackHost, while local mode sets allowPrivateNetwork before that gate; ordinary service or DNS names therefore still fall through without policy. (extensions/comfy/workflow-runtime.ts:291, 4f933ccf7b62)
• PR adds the service-host policy split: The PR head adds explicitAllowPrivateNetwork, exact-host trust for non-explicit single-label service names, and keeps the broader private-network policy for explicit opt-in. (extensions/comfy/workflow-runtime.ts:307, b5fc32ea8d1b)
• Shared SSRF contract checked: The shared guard treats allowedHostnames as exact-host trust that skips general private-network checks, while still blocking link-local and cloud metadata resolved addresses for trusted hostnames without allowPrivateNetwork. (src/infra/net/ssrf.ts:229, 4f933ccf7b62)
• Comfy local LAN behavior is documented: The Comfy provider docs describe local mode as running a ComfyUI instance on the user's machine or LAN, which matches the user-facing bug boundary. Public docs: docs/providers/comfy.md. (docs/providers/comfy.md:35, 4f933ccf7b62)
• Related canonical issue remains open: Live GitHub context shows [Bug]: Comfy plugin: allowPrivateNetwork: true in local mode does not bypass SSRF guard when baseUrl is a DNS hostname resolving to a private IP #77922 is open and reports the same Comfy private-DNS SSRF block, while the older direct PR fix(comfy): bypass SSRF guard for DNS hostnames when allowPrivateNetwork is set in local mode #77944 is closed unmerged.
• Real behavior proof is mock-only: The PR body lists Vitest, lint/check gates, and explicitly says the original Docker Compose/myagents stack and a live ComfyUI service were not run from the checkout. (b5fc32ea8d1b)

Likely related people:

• joshavant: Current checkout blame attributes resolveComfyNetworkPolicy and the relevant shared SSRF helper lines to a recent full-file add, though the commit topic is broader than Comfy. (role: recent line owner; confidence: medium; commits: 7fa26e088d28; files: extensions/comfy/workflow-runtime.ts, src/infra/net/ssrf.ts, src/plugin-sdk/ssrf-policy.ts)
• steipete: Git history shows the bundled Comfy workflow media runtime was introduced in the Comfy media provider work, with substantial adjacent SSRF and plugin-runtime history in the same area. (role: introduced behavior and adjacent owner; confidence: high; commits: aeb9ad52fa37, 1b9ec88d9cc5, 57334cd7d851; files: extensions/comfy/workflow-runtime.ts, extensions/comfy/image-generation-provider.test.ts, src/infra/net/ssrf.ts)
• jacobtomlinson: Beyond authoring this PR, prior merged history includes routing extension fetch calls through fetchWithSsrFGuard and other security/network changes, making this person relevant to the affected boundary. (role: adjacent SSRF contributor; confidence: medium; commits: f92c92515bd4, 20c7cbbf7835, d61f8e56723e; files: extensions/bluebubbles/src/send.ts, extensions/mattermost/src/mattermost/client.ts, extensions/thread-ownership/index.ts)

What the crustacean ranks mean

• 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
• 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
• 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
• 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
• 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
• 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
• 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works

• ClawSweeper keeps one durable marker-backed review comment per issue or PR.
• Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
• A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
• PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
• Maintainers can also comment @clawsweeper review to request a fresh review only.
• Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
• Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
• Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.