Skip to content

Commit 01c736d

Browse files
yashranawayyashranaway
committed
fix(ssh): retain known-host ports when connecting
1 parent 52e4b44 commit 01c736d

4 files changed

Lines changed: 82 additions & 28 deletions

File tree

‎packages/ssh/src/command.test.ts‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import { ChildProcessSpawner } from "effect/unstable/process";
1212

1313
import {
1414
baseSshArgs,
15+
buildSshHostSpecEffect,
1516
getLastNonEmptyOutputLine,
1617
parseSshResolveOutput,
1718
runSshCommand,
@@ -81,6 +82,22 @@ describe("ssh command", () => {
8182
}),
8283
);
8384

85+
it.effect("keeps a discovered SSH URI valid after resolving its user", () =>
86+
Effect.gen(function* () {
87+
for (const [alias, hostname, expected] of [
88+
["ssh://host.example:2222", "host.example", "ssh://[email protected]:2222"],
89+
["ssh://[::1]:2222", "::1", "ssh://alice@[::1]:2222"],
90+
]) {
91+
const resolved = parseSshResolveOutput(
92+
alias!,
93+
`hostname ${hostname}\nuser alice\nport 2222\n`,
94+
);
95+
assert.equal(yield* buildSshHostSpecEffect(resolved), expected);
96+
assert.include(baseSshArgs(resolved), "2222");
97+
}
98+
}),
99+
);
100+
84101
it.effect("builds interactive ssh args without forcing batch mode", () =>
85102
Effect.sync(() => {
86103
assert.deepEqual(

‎packages/ssh/src/command.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,11 @@ function buildSshHostSpec(target: DesktopSshEnvironmentTarget): string {
8484
if (destination.length === 0) {
8585
throw new Error("SSH target is missing its alias/hostname.");
8686
}
87+
if (target.username && destination.startsWith("ssh://")) {
88+
const uri = new URL(destination);
89+
uri.username = target.username;
90+
return uri.toString();
91+
}
8792
return target.username ? `${target.username}@${destination}` : destination;
8893
}
8994

‎packages/ssh/src/config.test.ts‎

Lines changed: 38 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -58,13 +58,6 @@ describe("ssh config", () => {
5858

5959
const hosts = yield* discoverSshHosts({ homeDir });
6060
assert.deepEqual(hosts, [
61-
{
62-
alias: "bastion.example.com",
63-
hostname: "bastion.example.com",
64-
username: null,
65-
port: null,
66-
source: "known-hosts",
67-
},
6861
{
6962
alias: "devbox",
7063
hostname: "devbox",
@@ -86,6 +79,13 @@ describe("ssh config", () => {
8679
port: null,
8780
source: "known-hosts",
8881
},
82+
{
83+
alias: "ssh://bastion.example.com:2222",
84+
hostname: "bastion.example.com",
85+
username: null,
86+
port: 2222,
87+
source: "known-hosts",
88+
},
8989
{
9090
alias: "staging",
9191
hostname: "staging",
@@ -97,6 +97,37 @@ describe("ssh config", () => {
9797
}).pipe(Effect.provide(NodeServices.layer), Effect.scoped),
9898
);
9999

100+
it.effect("preserves distinct known-host ports and IPv6 destinations", () =>
101+
Effect.gen(function* () {
102+
const fs = yield* FileSystem.FileSystem;
103+
const path = yield* Path.Path;
104+
const homeDir = yield* makeTempHomeDir();
105+
const sshDir = path.join(homeDir, ".ssh");
106+
yield* fs.makeDirectory(sshDir);
107+
yield* fs.writeFileString(
108+
path.join(sshDir, "known_hosts"),
109+
[
110+
"host.example.com ssh-ed25519 AAAA",
111+
"[host.example.com]:2222 ssh-ed25519 AAAA",
112+
"[host.example.com]:2223 ssh-ed25519 AAAA",
113+
"[host.example.com]:2222 ssh-rsa BBBB",
114+
"[2001:db8::1]:2200 ssh-ed25519 AAAA",
115+
"[invalid.example.com]:65536 ssh-ed25519 AAAA",
116+
].join("\n"),
117+
);
118+
const hosts = yield* discoverSshHosts({ homeDir });
119+
assert.deepEqual(
120+
hosts.map(({ alias, hostname, port }) => ({ alias, hostname, port })),
121+
[
122+
{ alias: "host.example.com", hostname: "host.example.com", port: null },
123+
{ alias: "ssh://[2001:db8::1]:2200", hostname: "2001:db8::1", port: 2200 },
124+
{ alias: "ssh://host.example.com:2222", hostname: "host.example.com", port: 2222 },
125+
{ alias: "ssh://host.example.com:2223", hostname: "host.example.com", port: 2223 },
126+
],
127+
);
128+
}).pipe(Effect.provide(NodeServices.layer), Effect.scoped),
129+
);
130+
100131
it.effect("parses known_hosts entries without returning hashed hosts", () =>
101132
Effect.sync(() => {
102133
assert.deepEqual(

‎packages/ssh/src/config.ts‎

Lines changed: 22 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -170,8 +170,8 @@ function normalizeKnownHostsHostname(rawHost: string): string {
170170
return firstColonIndex === lastColonIndex ? rawHost.slice(0, lastColonIndex) : rawHost;
171171
}
172172

173-
export function parseKnownHostsHostnames(raw: string): ReadonlyArray<string> {
174-
const hostnames = new Set<string>();
173+
function parseKnownHostsTargets(raw: string): ReadonlyArray<DesktopDiscoveredSshHost> {
174+
const targets = new Map<string, DesktopDiscoveredSshHost>();
175175

176176
for (const line of raw.split(/\r?\n/u)) {
177177
const trimmed = line.trim();
@@ -192,19 +192,29 @@ export function parseKnownHostsHostnames(raw: string): ReadonlyArray<string> {
192192
if (host.length === 0 || hasSshPattern(host)) {
193193
continue;
194194
}
195-
hostnames.add(host);
195+
const portText =
196+
/^\[[^\]]+\]:(\d+)$/.exec(rawHost)?.[1] ?? /^[^:]+:(\d+)$/.exec(rawHost)?.[1];
197+
const port = portText === undefined ? null : Number(portText);
198+
if (port !== null && (!Number.isSafeInteger(port) || port < 1 || port > 65535)) continue;
199+
const alias =
200+
port === null ? host : `ssh://${host.includes(":") ? `[${host}]` : host}:${port}`;
201+
targets.set(alias, { alias, hostname: host, username: null, port, source: "known-hosts" });
196202
}
197203
}
198204

199-
return [...hostnames].toSorted((left, right) => left.localeCompare(right));
205+
return [...targets.values()].toSorted((left, right) => left.alias.localeCompare(right.alias));
206+
}
207+
208+
export function parseKnownHostsHostnames(raw: string): ReadonlyArray<string> {
209+
return [...new Set(parseKnownHostsTargets(raw).map((target) => target.hostname))].toSorted(
210+
(left, right) => left.localeCompare(right),
211+
);
200212
}
201213

202-
const readKnownHostsHostnames = Effect.fnUntraced(function* (filePath: string) {
214+
const readKnownHostsTargets = Effect.fnUntraced(function* (filePath: string) {
203215
const fs = yield* FileSystem.FileSystem;
204-
if (!(yield* fs.exists(filePath))) {
205-
return NO_HOSTS;
206-
}
207-
return parseKnownHostsHostnames(yield* fs.readFileString(filePath));
216+
if (!(yield* fs.exists(filePath))) return [];
217+
return parseKnownHostsTargets(yield* fs.readFileString(filePath));
208218
});
209219

210220
export const discoverSshHosts = Effect.fnUntraced(
@@ -229,7 +239,7 @@ export const discoverSshHosts = Effect.fnUntraced(
229239
new Set<string>(),
230240
homeDir,
231241
);
232-
const knownHosts = yield* readKnownHostsHostnames(path.join(sshDirectory, "known_hosts"));
242+
const knownHosts = yield* readKnownHostsTargets(path.join(sshDirectory, "known_hosts"));
233243
const discovered = new Map<string, DesktopDiscoveredSshHost>();
234244

235245
for (const alias of configAliases) {
@@ -242,17 +252,8 @@ export const discoverSshHosts = Effect.fnUntraced(
242252
});
243253
}
244254

245-
for (const hostname of knownHosts) {
246-
if (discovered.has(hostname)) {
247-
continue;
248-
}
249-
discovered.set(hostname, {
250-
alias: hostname,
251-
hostname,
252-
username: null,
253-
port: null,
254-
source: "known-hosts",
255-
});
255+
for (const target of knownHosts) {
256+
if (!discovered.has(target.alias)) discovered.set(target.alias, target);
256257
}
257258

258259
return [...discovered.values()].toSorted((left, right) =>

0 commit comments

Comments
 (0)