History

fix(ssh): disable tunnel multiplexing (#3533)

Issue #3533 requires the supervised port-forward process to own its lifecycle even when the user's SSH config enables ControlMaster or ControlPersist.

Julius Marminge committed 3 months ago

Commit d2de1251 parent 5d34f9f2 files · +94 −15Browse files

Unified · Split

2 files
  1. packages/ssh/src/tunnel.test.ts +56 −0
  2. packages/ssh/src/tunnel.ts +38 −15
packages/ssh/src/tunnel.test.ts+56−0
@@ -14,6 +14,7 @@ import { ChildProcess, ChildProcessSpawner } from "effect/unstable/process"; · 13 unmodified lines
14 14
15import { SshPasswordPrompt } from "./auth.ts";15import { SshPasswordPrompt } from "./auth.ts";
16import {16import {
17 buildSupervisedSshTunnelArgs,
17 buildRemoteLaunchScript,18 buildRemoteLaunchScript,
18 buildRemotePairingScript,19 buildRemotePairingScript,
19 buildRemoteStopScript,20 buildRemoteStopScript,
@@ -89,6 +90,50 @@ function commandArgs(command: ChildProcess.Command): ReadonlyArray<string> { · 69 unmodified lines
89}90}
90 91
91describe("ssh tunnel scripts", () => {92describe("ssh tunnel scripts", () => {
93 it("disables connection multiplexing for the supervised port-forward command", () => {
94 const target = {
95 alias: "devbox",
96 hostname: "devbox.example.com",
97 username: "julius",
98 port: 2222,
99 } as const;
100
101 assert.deepEqual(
102 buildSupervisedSshTunnelArgs({
103 target,
104 batchMode: "no",
105 localPort: 41_773,
106 remotePort: 3_773,
107 hostSpec: "julius@devbox",
108 }),
109 [
110 "-o",
111 "BatchMode=no",
112 "-o",
113 "ConnectTimeout=10",
114 "-p",
115 "2222",
116 "-o",
117 "ControlMaster=no",
118 "-o",
119 "ControlPath=none",
120 "-o",
121 "ControlPersist=no",
122 "-o",
123 "ExitOnForwardFailure=yes",
124 "-o",
125 "ServerAliveInterval=15",
126 "-o",
127 "ServerAliveCountMax=3",
128 "-n",
129 "-N",
130 "-L",
131 "41773:127.0.0.1:3773",
132 "julius@devbox",
133 ],
134 );
135 });
136
92 it("builds the remote t3 runner with npx and npm fallbacks", () => {137 it("builds the remote t3 runner with npx and npm fallbacks", () => {
93 const script = buildRemoteT3RunnerScript({ nodeEngineRange: TEST_NODE_ENGINE_RANGE });138 const script = buildRemoteT3RunnerScript({ nodeEngineRange: TEST_NODE_ENGINE_RANGE });
94 139
@@ -382,6 +427,17 @@ describe("ssh tunnel scripts", () => { · 287 unmodified lines
382 const first = yield* manager.ensureEnvironment(target);427 const first = yield* manager.ensureEnvironment(target);
383 assert.equal(first.httpBaseUrl, "http://127.0.0.1:41773/");428 assert.equal(first.httpBaseUrl, "http://127.0.0.1:41773/");
384 429
430 const firstTunnelArgs = spawnedCommands.find((args) => args.includes("-N"));
431 assert.isDefined(firstTunnelArgs);
432 assert.include(firstTunnelArgs, "ControlMaster=no");
433 assert.include(firstTunnelArgs, "ControlPath=none");
434 assert.include(firstTunnelArgs, "ControlPersist=no");
435 for (const args of spawnedCommands.filter((entry) => !entry.includes("-N"))) {
436 assert.notInclude(args, "ControlMaster=no");
437 assert.notInclude(args, "ControlPath=none");
438 assert.notInclude(args, "ControlPersist=no");
439 }
440
385 yield* manager.disconnectEnvironment(target);441 yield* manager.disconnectEnvironment(target);
386 assert.equal(tunnelKillCount, 1);442 assert.equal(tunnelKillCount, 1);
387 assert.equal(stopCommandCount, 1);443 assert.equal(stopCommandCount, 1);
packages/ssh/src/tunnel.ts+38−15
@@ -917,6 +917,38 @@ const reserveLocalTunnelPort = Effect.fn("ssh/tunnel.reserveLocalTunnelPort")(fu · 916 unmodified lines
917 return yield* net.reserveLoopbackPort();917 return yield* net.reserveLoopbackPort();
918});918});
919 919
920export function buildSupervisedSshTunnelArgs(input: {
921 readonly target: DesktopSshEnvironmentTarget;
922 readonly batchMode: "yes" | "no";
923 readonly localPort: number;
924 readonly remotePort: number;
925 readonly hostSpec: string;
926}): ReadonlyArray<string> {
927 return [
928 ...baseSshArgs(input.target, { batchMode: input.batchMode }),
929 // This process owns the tunnel lifecycle. Inherited ControlMaster settings
930 // can otherwise hand the forward to an existing master and let `ssh -N`
931 // exit while the manager believes it still supervises the connection.
932 "-o",
933 "ControlMaster=no",
934 "-o",
935 "ControlPath=none",
936 "-o",
937 "ControlPersist=no",
938 "-o",
939 "ExitOnForwardFailure=yes",
940 "-o",
941 "ServerAliveInterval=15",
942 "-o",
943 "ServerAliveCountMax=3",
944 "-n",
945 "-N",
946 "-L",
947 `${input.localPort}:127.0.0.1:${input.remotePort}`,
948 input.hostSpec,
949 ];
950}
951
920const startSshTunnel = Effect.fn("ssh/tunnel.startSshTunnel")(function* (input: {952const startSshTunnel = Effect.fn("ssh/tunnel.startSshTunnel")(function* (input: {
921 readonly key: string;953 readonly key: string;
922 readonly resolvedTarget: DesktopSshEnvironmentTarget;954 readonly resolvedTarget: DesktopSshEnvironmentTarget;
@@ -956,22 +988,13 @@ const startSshTunnel = Effect.fn("ssh/tunnel.startSshTunnel")(function* (input: · 33 unmodified lines
956 }),988 }),
957 ),989 ),
958 );990 );
959 const args = [991 const args = buildSupervisedSshTunnelArgs({
960 ...baseSshArgs(input.resolvedTarget, {992 target: input.resolvedTarget,
961 batchMode: input.authOptions.batchMode ?? "no",993 batchMode: input.authOptions.batchMode ?? "no",
962 }),994 localPort: input.localPort,
963 "-o",995 remotePort: input.remotePort,
964 "ExitOnForwardFailure=yes",
965 "-o",
966 "ServerAliveInterval=15",
967 "-o",
968 "ServerAliveCountMax=3",
969 "-n",
970 "-N",
971 "-L",
972 `${input.localPort}:127.0.0.1:${input.remotePort}`,
973 hostSpec,996 hostSpec,
974 ];997 });
975 const sshCommand = yield* resolveSshCommand;998 const sshCommand = yield* resolveSshCommand;
976 const tunnelCommand = [sshCommand, ...args];999 const tunnelCommand = [sshCommand, ...args];
977 const spawner = yield* ChildProcessSpawner.ChildProcessSpawner;1000 const spawner = yield* ChildProcessSpawner.ChildProcessSpawner;