fix(ssh): scope tunnel port preflight to loopback (#94603) by wangwllu · Pull Request #94607 · openclaw/openclaw

wangwllu · 2026-06-18T14:38:14Z

Summary

Follow-up to #94379 / #94394. Two related fixes on the SSH-tunnel port preflight in src/infra/ssh-tunnel.ts:

1. Scope the preflight to IPv4 loopback. startSshPortForward called ensurePortAvailable(localPort) host-less. On a dual-stack host a bare listen binds the IPv6 wildcard :: and misses an IPv4-only occupant, so the preflight reports a busy port as free — the exact address-family flaw fixed for the Chrome CDP caller in #94394. Every other probe in this module is already pinned to IPv4 loopback:

pickEphemeralPort → listen(0, "127.0.0.1")
canConnectLocal → connect({ host: "127.0.0.1" })
the forward itself → -L ${localPort}:127.0.0.1:${remotePort}

await ensurePortAvailable(localPort, "127.0.0.1");

2. Route PortInUseError to the ephemeral-port fallback. Scoping the probe to 127.0.0.1 made a pre-existing bug reachable: ensurePortAvailable raises the domain PortInUseError (which has no errno code) on a busy port, but the catch only matched isErrno(err) && err.code === "EADDRINUSE". So a busy preferred port fell through to the rethrow branch and the intended ephemeral-port fallback never fired. The guard now also matches the domain error (consistent with handlePortError in ports.ts):

if (err instanceof PortInUseError || (isErrno(err) && err.code === "EADDRINUSE")) {
  localPort = await pickEphemeralPort();
} else {
  throw err;
}

Closes #94603

Real behavior proof

Behavior or issue addressed: (1) the host-less preflight binds the IPv6 wildcard on dual-stack hosts and fails to detect an IPv4-only occupant of the preferred port, reporting a busy port as free (same root cause as #94379); (2) once the probe actually detects a busy port, ensurePortAvailable throws PortInUseError, which the catch guard failed to recognize, so the ephemeral-port fallback was skipped and a hard error was thrown instead.

Real environment tested: macOS (darwin arm64, dual-stack), Node v26, run directly against node:net and the real guard logic to reproduce the OS-level bind behavior and the catch decision.

Exact steps or command run after this patch: (a) bound an occupant IPv4-only on 127.0.0.1:<port>, then compared the old host-less preflight against the new 127.0.0.1-scoped preflight via real net.createServer().listen() calls; (b) fed the PortInUseError that a busy port produces through the old vs new catch guard.

Evidence after fix:

# (a) address-family scoping
occupant bound IPv4-only on 127.0.0.1:51743
OLD host-less preflight (binds :: wildcard): SUCCEEDED -> reports FREE  [FALSE NEGATIVE]
NEW scoped preflight (127.0.0.1):            EADDRINUSE -> BUSY [CORRECT]

# (b) fallback guard
busy port -> ensurePortAvailable throws: PortInUseError | .code = undefined
OLD guard (isErrno && code===EADDRINUSE): false -> rethrows HARD ERROR, no fallback [BUG]
NEW guard (instanceof PortInUseError ||...): true  -> pickEphemeralPort fallback fires [FIXED]

Observed result after fix: the scoped preflight detects the IPv4-only occupant, and the busy-port PortInUseError is now routed to pickEphemeralPort() so the tunnel transparently selects a free port instead of failing. A regression test starts a real IPv4 listener (via a mocked ssh child) and asserts the tunnel falls back to a different port that is then used in the -L forward spec; it fails against the old broken guard.

What was not tested: a full live SSH connection (the spawn path is unchanged by this patch); the fix is confined to the preflight host argument and the catch guard.

Test plan

pnpm vitest run src/infra/ssh-tunnel.test.ts src/infra/ports.test.ts — 19/19 pass (ran 3x, stable)
regression guard proven: fallback test fails against the old broken guard, passes with the fix
pnpm exec oxlint src/infra/ssh-tunnel.ts src/infra/ssh-tunnel.test.ts — clean
pnpm tsgo:core / pnpm tsgo:core:test — exit 0
live node:net repro above (both scoping and fallback paths)

🤖 Generated with Claude Code

clawsweeper · 2026-06-18T14:39:55Z

Codex review: needs maintainer review before merge. Reviewed June 19, 2026, 1:55 PM ET / 17:55 UTC.

Summary
This PR scopes SSH tunnel preferred-port preflight and the local SSH -L listener to 127.0.0.1, treats PortInUseError as busy for fallback, and adds regression tests.

PR surface: Source 0, Tests +123. Total +123 across 2 files.

Reproducibility: yes. Current main source still has the hostless SSH tunnel preflight plus errno-only fallback, and the PR body supplies real node:net terminal output for the address-family and fallback behavior.

Review metrics: none identified.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #94596
Summary: This PR is a candidate fix for the canonical SSH tunnel hostless port-preflight issue; it also covers fallback and listener-binding details that several sibling one-line PRs do not.

Members:

canonical: ensurePortAvailable in startSshPortForward misses IPv4-only occupants (same address-family flaw as #94379) #94596 - It is the open canonical tracker for the same SSH tunnel ensurePortAvailable address-family bug.
duplicate: ensurePortAvailable in startSshPortForward misses IPv4-only occupants #94603 - It is a later duplicate by the same author and is already closed in favor of the canonical tracker, but this PR still names it in closing syntax.
partial_overlap: fix(infra): probe 127.0.0.1 in ensurePortAvailable to detect IPv4-only occupants #94394 - It merged the related Browser CDP helper/caller fix and introduced the host-scoped helper pattern, but did not update the SSH tunnel caller.
same_root_cause: fix(infra): pin ensurePortAvailable to 127.0.0.1 in startSshPortForward #94602 - It is another open candidate for the same canonical SSH tunnel issue, but the visible context shows a narrower one-line scope and a human-review label.
same_root_cause: fix(ssh-tunnel): pass host to ensurePortAvailable for correct address-family check #94897 - Live PR search shows it is another open candidate for the same startSshPortForward loopback probe issue.
1 more in the report.

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

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🐚 platinum hermit
Patch quality: 🦞 diamond lobster
Result: ready for maintainer review.

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

Rank-up moves:

Update the PR body to close or link ensurePortAvailable in startSshPortForward misses IPv4-only occupants (same address-family flaw as #94379) #94596 instead of only the closed duplicate.
[P2] Wait for the current queued required checks on head 6798b718de057ad5ededdfb4c659bfbe853aa934 to finish.

Risk before merge

[P1] The PR body still closes the already closed duplicate issue; maintainers should update the closing reference or close/link ensurePortAvailable in startSshPortForward misses IPv4-only occupants (same address-family flaw as #94379) #94596 when landing.
Current head still has queued CI checks, so merge should wait for normal required-check completion.
[P2] The proof does not include a full live SSH session; it does cover real node:net bind behavior, the fallback decision, mocked spawn arguments, and the OpenSSH -L contract.

Maintainer options:

Decide the mitigation before merge
Land this focused PR after maintainer review and green required checks, then close or link the canonical SSH-tunnel issue on merge.
Pause or close
Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

No automated repair job is needed; the branch is focused with no blocking code findings, leaving maintainer review, CI completion, and canonical issue linkage.

Security
Cleared: Cleared: the diff narrows the SSH tunnel listener/probe to loopback, preserves the -- target separator, and adds no dependency, workflow, secret, or package-resolution changes.

Review details

Best possible solution:

Land this focused PR after maintainer review and green required checks, then close or link the canonical SSH-tunnel issue on merge.

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

Yes. Current main source still has the hostless SSH tunnel preflight plus errno-only fallback, and the PR body supplies real node:net terminal output for the address-family and fallback behavior.

Is this the best way to solve the issue?

Yes. Fixing the SSH tunnel caller to probe and listen on 127.0.0.1 while recognizing PortInUseError is narrower than changing global helper semantics and matches the caller's loopback-only consumption path.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 1609365b3e74.

Label changes

Label changes:

add proof: sufficient: Contributor real behavior proof is sufficient. Sufficient: the PR body includes terminal-style after-fix evidence from real node:net bind behavior and the fallback guard; full live SSH was not run, but the changed path is covered by focused tests and the OpenSSH -L contract.

Label justifications:

P2: This is a bounded SSH tunnel port-selection bug fix with limited blast radius and clear source/test evidence.
rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🦞 diamond lobster.
status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): Sufficient: the PR body includes terminal-style after-fix evidence from real node:net bind behavior and the fallback guard; full live SSH was not run, but the changed path is covered by focused tests and the OpenSSH -L contract.
proof: sufficient: Contributor real behavior proof is sufficient. Sufficient: the PR body includes terminal-style after-fix evidence from real node:net bind behavior and the fallback guard; full live SSH was not run, but the changed path is covered by focused tests and the OpenSSH -L contract.

Evidence reviewed

PR surface:

Source 0, Tests +123. Total +123 across 2 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	1	4	4	0
Tests	1	126	3	+123
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	2	130	7	+123

What I checked:

Current main still has the reported gap: startSshPortForward on current main calls ensurePortAvailable(localPort) without a host and only falls back on errno EADDRINUSE, so a domain PortInUseError is rethrown instead of selecting an ephemeral port. (src/infra/ssh-tunnel.ts:122, 1609365b3e74)
PR head implements the caller-scoped probe and fallback: The PR head imports PortInUseError, probes ensurePortAvailable(localPort, "127.0.0.1"), and routes either PortInUseError or errno EADDRINUSE to pickEphemeralPort(). (src/infra/ssh-tunnel.ts:122, 6798b718de05)
PR head pins the SSH listener to loopback: The PR changes the -L argument to 127.0.0.1:${localPort}:127.0.0.1:${opts.remotePort}, matching the local gateway URL consumed by the caller. (src/infra/ssh-tunnel.ts:135, 6798b718de05)
Focused regression coverage is present: The PR test asserts the scoped preflight argument, simulates PortInUseError, verifies fallback to a different port, and checks the loopback-bound ssh -L spec. (src/infra/ssh-tunnel.test.ts:99, 6798b718de05)
Caller uses loopback for the tunnel target: gateway status starts startSshPortForward and then probes ws://127.0.0.1:${tunnel.localPort}, so a loopback-only listener matches the product path. (src/commands/gateway-status/probe-run.ts:70, 1609365b3e74)
Helper contract supports caller-owned interface scope: ensurePortAvailable(port, host?) passes the optional host through to tryListenOnPort and raises PortInUseError on EADDRINUSE. (src/infra/ports.ts:40, 1609365b3e74)

Likely related people:

Vincent Koc: Current blame and the shallow history for startSshPortForward and the gateway-status caller point to the commit that introduced the current SSH tunnel path in this checkout. (role: introduced current SSH tunnel implementation; confidence: medium; commits: e74a7d2f1435; files: src/infra/ssh-tunnel.ts, src/commands/gateway-status/probe-run.ts, src/commands/gateway-status.ts)
steipete: The PR is assigned to steipete, and the current head includes a steipete commit tightening the SSH listener bind address on the affected file. (role: recent reviewer and adjacent contributor; confidence: high; commits: 6798b718de05; files: src/infra/ssh-tunnel.ts, src/infra/ssh-tunnel.test.ts)
Pandah97: The merged related Browser CDP PR added the caller-scoped ensurePortAvailable host pattern reused by this SSH tunnel fix. (role: related helper author; confidence: high; commits: 4ec9d4a2b511; files: src/infra/ports.ts, src/infra/ports.test.ts, extensions/browser/src/browser/chrome.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.

wangwllu · 2026-06-19T02:47:16Z

Quick CI note: the two red checks appear unrelated to this diff, which only touches src/infra/ssh-tunnel.ts and its test.

checks-node-core-tooling-docker — looks like the flaky docker-build-helper signal-file race (docker.term not written), in code this PR doesn't touch.
security-fast — pnpm audit is flagging pre-existing undici HIGH advisories already present on main (e.g. GHSA-vmh5-mc38-953g); this PR changes no dependencies.

Core Real behavior proof is green. Happy to rebase or re-run if useful.

github-actions · 2026-06-19T16:51:17Z

Dependency graph guard cleared

This PR no longer has blocked dependency graph changes. A future dependency graph change requires a fresh /allow-dependencies-change comment after the guard blocks that new head SHA.

Current SHA: 6798b718de057ad5ededdfb4c659bfbe853aa934

startSshPortForward called ensurePortAvailable host-less, binding the IPv6 wildcard on dual-stack hosts and missing IPv4-only occupants — the same address-family flaw as openclaw#94379. Every other probe in this module is pinned to 127.0.0.1, so scope the preflight to match. Closes openclaw#94603 Co-Authored-By: Claude <[email protected]>

The preferred-port preflight calls ensurePortAvailable, which raises the domain PortInUseError (no errno `code`) on a busy port. The catch only matched `isErrno(err) && err.code === "EADDRINUSE"`, so PortInUseError fell through to the rethrow branch and the ephemeral-port fallback never fired. Scoping the probe to 127.0.0.1 made this reachable. Match on the domain error so a busy preferred port falls back as intended. Co-Authored-By: Claude <[email protected]>

steipete · 2026-06-19T18:00:29Z

Landed as 583829a.

Verification:

node scripts/run-vitest.mjs src/infra/ssh-tunnel.test.ts src/infra/ports.test.ts (2 files, 19 tests passed)
Fresh autoreview: no actionable findings; correctness confidence 0.98
OpenSSH contract checked with ssh(1) / ssh_config(5) and ssh -G: the explicit 127.0.0.1 bind prevents GatewayPorts yes from widening the listener
Exact-head CI release gate: https://github.com/openclaw/openclaw/actions/runs/27840761590
Exact-head Workflow Sanity: https://github.com/openclaw/openclaw/actions/runs/27840747526

The final patch scopes preferred-port detection to IPv4 loopback, preserves ephemeral fallback for both domain and errno busy-port errors, and pins the SSH forward listener to loopback. Thanks @wangwllu.

openclaw-barnacle Bot added size: XS proof: supplied External PR includes structured after-fix real behavior proof. labels Jun 18, 2026

openclaw-barnacle Bot added size: S and removed size: XS proof: sufficient ClawSweeper judged the real behavior proof convincing. labels Jun 18, 2026

clawsweeper Bot mentioned this pull request Jun 18, 2026

fix(infra): probe 127.0.0.1 in startSshPortForward port preflight #94615

Closed

2 tasks

clawsweeper Bot added the proof: sufficient ClawSweeper judged the real behavior proof convincing. label Jun 18, 2026

github-actions Bot mentioned this pull request Jun 18, 2026

🦞 OpenClaw 生态日报 2026-06-18 litang9/big_model_radar#85

Open

This was referenced Jun 18, 2026

fix(infra): pass 127.0.0.1 to ensurePortAvailable in SSH tunnel start #94635

Closed

fix(ssh): bind ensurePortAvailable to IPv4 loopback #94616

Closed

fix(infra): pin SSH tunnel port probe to IPv4 loopback (#94596) #94724

Closed

steipete self-assigned this Jun 19, 2026

github-actions Bot added the dependencies-changed PR changes dependency-related files label Jun 19, 2026

wanglu241 and others added 3 commits June 19, 2026 18:50

fix(ssh): pin tunnel listener to loopback

6798b71

steipete mentioned this pull request Jun 19, 2026

ensurePortAvailable in startSshPortForward misses IPv4-only occupants (same address-family flaw as #94379) #94596

Closed

This was referenced Jun 19, 2026

fix(ssh): reject hostnames with stray leading or trailing colons in parseSshTarget #93887

Merged

fix(ssh-tunnel): pass 127.0.0.1 host to ensurePortAvailable in startSshPortForward #94604

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(ssh): scope tunnel port preflight to loopback (#94603)#94607

fix(ssh): scope tunnel port preflight to loopback (#94603)#94607
steipete merged 3 commits into
openclaw:mainfrom
wangwllu:fix-94603-ssh-tunnel-port-preflight

wangwllu commented Jun 18, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented Jun 18, 2026 •

edited

Loading

Uh oh!

wangwllu commented Jun 19, 2026

Uh oh!

github-actions Bot commented Jun 19, 2026 •

edited

Loading

Uh oh!

steipete commented Jun 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

wangwllu commented Jun 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Real behavior proof

Test plan

Uh oh!

clawsweeper Bot commented Jun 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wangwllu commented Jun 19, 2026

Uh oh!

github-actions Bot commented Jun 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Dependency graph guard cleared

Uh oh!

steipete commented Jun 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

wangwllu commented Jun 18, 2026 •

edited

Loading

clawsweeper Bot commented Jun 18, 2026 •

edited

Loading

github-actions Bot commented Jun 19, 2026 •

edited

Loading