Skip to content

Commit 583829a

Browse files
wangwllusteipete
wangwllu
and
steipete
authored
fix(ssh): scope tunnel port preflight to loopback (#94603) (#94607)
Merged via squash. Prepared head SHA: 6798b71 Co-authored-by: wangwllu <[email protected]> Co-authored-by: steipete <[email protected]> Reviewed-by: @steipete
1 parent 7b94ae9 commit 583829a

2 files changed

Lines changed: 130 additions & 7 deletions

File tree

‎src/infra/ssh-tunnel.test.ts‎

Lines changed: 126 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,25 @@
1-
// Covers SSH target parsing.
2-
import { describe, expect, it } from "vitest";
3-
import { parseSshTarget } from "./ssh-tunnel.js";
1+
// Covers SSH target parsing and tunnel startup preflight behavior.
2+
import { EventEmitter } from "node:events";
3+
import net from "node:net";
4+
import { afterEach, describe, expect, it, vi } from "vitest";
5+
6+
const mocks = vi.hoisted(() => ({
7+
ensurePortAvailable: vi.fn<(port: number, host?: string) => Promise<void>>(),
8+
spawn: vi.fn(),
9+
}));
10+
11+
vi.mock("./ports.js", async (importOriginal) => ({
12+
...(await importOriginal<typeof import("./ports.js")>()),
13+
ensurePortAvailable: mocks.ensurePortAvailable,
14+
}));
15+
16+
vi.mock("node:child_process", async (importOriginal) => ({
17+
...(await importOriginal<typeof import("node:child_process")>()),
18+
spawn: mocks.spawn,
19+
}));
20+
21+
import { PortInUseError } from "./ports.js";
22+
import { parseSshTarget, startSshPortForward } from "./ssh-tunnel.js";
423

524
describe("parseSshTarget", () => {
625
it("parses user@host:port targets", () => {
@@ -30,3 +49,107 @@ describe("parseSshTarget", () => {
3049
expect(parseSshTarget("-oProxyCommand=echo")).toBeNull();
3150
});
3251
});
52+
53+
describe("startSshPortForward", () => {
54+
const openServers: net.Server[] = [];
55+
56+
afterEach(async () => {
57+
while (openServers.length > 0) {
58+
const server = openServers.pop();
59+
await new Promise<void>((resolve) => {
60+
server?.close(() => resolve());
61+
});
62+
}
63+
mocks.ensurePortAvailable.mockReset();
64+
mocks.spawn.mockReset();
65+
});
66+
67+
// Fake ssh child that, when spawned, parses the -L forward spec and starts a
68+
// real IPv4-loopback listener on the chosen local port so waitForLocalListener
69+
// resolves without launching a real ssh process.
70+
function spawnFakeSshListening() {
71+
mocks.spawn.mockImplementation((_cmd: string, args: string[]) => {
72+
const forwardSpec = args[args.indexOf("-L") + 1] ?? "";
73+
const localPort = Number(forwardSpec.split(":")[1]);
74+
const server = net.createServer();
75+
server.on("error", () => {});
76+
openServers.push(server);
77+
server.listen(localPort, "127.0.0.1");
78+
79+
const child = new EventEmitter() as EventEmitter & {
80+
killed: boolean;
81+
pid: number;
82+
stderr: EventEmitter & { setEncoding: (enc: string) => void };
83+
kill: (signal?: string) => boolean;
84+
};
85+
child.killed = false;
86+
child.pid = 4242;
87+
const stderr = new EventEmitter() as EventEmitter & { setEncoding: (enc: string) => void };
88+
stderr.setEncoding = () => {};
89+
child.stderr = stderr;
90+
child.kill = (signal?: string) => {
91+
child.killed = true;
92+
queueMicrotask(() => child.emit("exit", 0, signal ?? null));
93+
return true;
94+
};
95+
return child;
96+
});
97+
}
98+
99+
it("scopes the preferred-port preflight to the IPv4 loopback interface", async () => {
100+
const sentinel = new Error("stop before spawning ssh");
101+
mocks.ensurePortAvailable.mockRejectedValueOnce(sentinel);
102+
103+
await expect(
104+
startSshPortForward({
105+
target: "[email protected]:2222",
106+
localPortPreferred: 43210,
107+
remotePort: 18789,
108+
timeoutMs: 250,
109+
}),
110+
).rejects.toBe(sentinel);
111+
112+
expect(mocks.ensurePortAvailable).toHaveBeenCalledWith(43210, "127.0.0.1");
113+
});
114+
115+
it("falls back to an ephemeral port when the preferred port is in use", async () => {
116+
// ensurePortAvailable raises the domain PortInUseError (no errno `code`),
117+
// which the catch must treat as "busy" and route to pickEphemeralPort.
118+
// Reserve a real port so pickEphemeralPort (listen(0)) cannot hand the same
119+
// number back and make the assertion flaky.
120+
const occupied = net.createServer();
121+
await new Promise<void>((resolve, reject) => {
122+
occupied.once("error", reject);
123+
occupied.listen(0, "127.0.0.1", () => {
124+
occupied.off("error", reject);
125+
resolve();
126+
});
127+
});
128+
openServers.push(occupied);
129+
const addr = occupied.address();
130+
if (!addr || typeof addr === "string") {
131+
throw new Error("failed to reserve preferred port");
132+
}
133+
const preferredPort = addr.port;
134+
135+
mocks.ensurePortAvailable.mockRejectedValueOnce(new PortInUseError(preferredPort));
136+
spawnFakeSshListening();
137+
138+
const tunnel = await startSshPortForward({
139+
target: "[email protected]:2222",
140+
localPortPreferred: preferredPort,
141+
remotePort: 18789,
142+
timeoutMs: 1000,
143+
});
144+
145+
expect(tunnel.localPort).not.toBe(preferredPort);
146+
expect(tunnel.localPort).toBeGreaterThan(0);
147+
expect(mocks.spawn).toHaveBeenCalledWith(
148+
"/usr/bin/ssh",
149+
expect.arrayContaining(["-L", `127.0.0.1:${tunnel.localPort}:127.0.0.1:18789`]),
150+
expect.anything(),
151+
);
152+
153+
await tunnel.stop();
154+
});
155+
});

‎src/infra/ssh-tunnel.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import net from "node:net";
44
import { normalizeStringEntries } from "@openclaw/normalization-core/string-normalization";
55
import { formatErrorMessage, isErrno } from "./errors.js";
66
import { parseStrictPositiveInteger } from "./parse-finite-number.js";
7-
import { ensurePortAvailable } from "./ports.js";
7+
import { ensurePortAvailable, PortInUseError } from "./ports.js";
88

99
export type SshParsedTarget = {
1010
user?: string;
@@ -119,9 +119,9 @@ export async function startSshPortForward(opts: {
119119

120120
let localPort = opts.localPortPreferred;
121121
try {
122-
await ensurePortAvailable(localPort);
122+
await ensurePortAvailable(localPort, "127.0.0.1");
123123
} catch (err) {
124-
if (isErrno(err) && err.code === "EADDRINUSE") {
124+
if (err instanceof PortInUseError || (isErrno(err) && err.code === "EADDRINUSE")) {
125125
localPort = await pickEphemeralPort();
126126
} else {
127127
throw err;
@@ -132,7 +132,7 @@ export async function startSshPortForward(opts: {
132132
const args = [
133133
"-N",
134134
"-L",
135-
`${localPort}:127.0.0.1:${opts.remotePort}`,
135+
`127.0.0.1:${localPort}:127.0.0.1:${opts.remotePort}`,
136136
"-p",
137137
String(parsed.port),
138138
"-o",

0 commit comments

Comments
 (0)