Skip to content

Commit 96f9fee

Browse files
authored
fix: parse remote authority from right (#944)
Coder SSH host names follow the format `coder-vscode.<safeHostname>--<username>--<workspace>(.<agent?>)`, but the previous left-to-right `--` split rejected or misparsed safe hostnames that contain `--` (Punycode, multi-dash domains). * Parse from the right: last two segments are username and workspace/agent; the rest is rejoined as the safe hostname. * Require the safe hostname, username, and workspace/agent to be non-empty; intermediate empty segments are allowed. * Remove empty-hostname fallbacks in `pathResolver`, `secretsManager`, and `sshConfig`; skip session lookup when no deployment is selected. * Add regression tests for double-dash, Punycode, and consecutive-dash safe hostnames. Closes #937
1 parent 2b3b3fe commit 96f9fee

9 files changed

Lines changed: 148 additions & 279 deletions

File tree

CHANGELOG.md

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,10 @@
2626

2727
### Fixed
2828

29-
- Workspaces hosted on internationalized (IDN) domains can now be opened from
30-
recent connections. The SSH authority parser was splitting Punycode (`xn--`)
31-
domain labels across the field separator and rejecting the host as invalid.
29+
- Workspaces on hostnames containing `--`, such as internationalized (IDN)
30+
domains with Punycode (`xn--`) labels, can now be opened from recent
31+
connections. The SSH authority parser was splitting these names across the
32+
field separator and rejecting the host as invalid.
3233

3334
## [v1.14.5](https://github.com/coder/vscode-coder/releases/tag/v1.14.5) 2026-04-30
3435

src/core/cliManager.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -113,9 +113,8 @@ export class CliManager {
113113
}
114114

115115
/**
116-
* Download and return the path to a working binary for the deployment with
117-
* the provided hostname using the provided client. If the hostname is empty,
118-
* use the old deployment-unaware path instead.
116+
* Download and return the path to a working binary for the deployment using
117+
* the provided client.
119118
*
120119
* If there is already a working binary and it matches the server version,
121120
* return that, skipping the download. If it does not match but downloads are

src/core/pathResolver.ts

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -13,22 +13,16 @@ export class PathResolver {
1313
* Return the directory for the deployment with the provided hostname to
1414
* where the global Coder configs are stored.
1515
*
16-
* If the hostname is empty, read the old deployment-unaware config instead.
17-
*
1816
* The caller must ensure this directory exists before use.
1917
*/
2018
public getGlobalConfigDir(safeHostname: string): string {
21-
return safeHostname
22-
? path.join(this.basePath, safeHostname)
23-
: this.basePath;
19+
return path.join(this.basePath, safeHostname);
2420
}
2521

2622
/**
2723
* Return the directory for a deployment with the provided hostname to where
2824
* its binary is cached.
2925
*
30-
* If the hostname is empty, read the old deployment-unaware config instead.
31-
*
3226
* The caller must ensure this directory exists before use.
3327
*/
3428
public getBinaryCachePath(safeHostname: string): string {
@@ -85,8 +79,6 @@ export class PathResolver {
8579
* Return the directory for the deployment with the provided hostname to
8680
* where its session token is stored.
8781
*
88-
* If the hostname is empty, read the old deployment-unaware config instead.
89-
*
9082
* The caller must ensure this directory exists before use.
9183
*/
9284
public getSessionTokenPath(safeHostname: string): string {
@@ -97,8 +89,6 @@ export class PathResolver {
9789
* Return the directory for the deployment with the provided hostname to
9890
* where its session token was stored by older code.
9991
*
100-
* If the hostname is empty, read the old deployment-unaware config instead.
101-
*
10292
* The caller must ensure this directory exists before use.
10393
*/
10494
public getLegacySessionTokenPath(safeHostname: string): string {
@@ -109,8 +99,6 @@ export class PathResolver {
10999
* Return the directory for the deployment with the provided hostname to
110100
* where its url is stored.
111101
*
112-
* If the hostname is empty, read the old deployment-unaware config instead.
113-
*
114102
* The caller must ensure this directory exists before use.
115103
*/
116104
public getUrlPath(safeHostname: string): string {

src/core/secretsManager.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ export class SecretsManager {
5858
) {}
5959

6060
private buildKey(prefix: SecretKeyPrefix, safeHostname: string): string {
61-
return `${prefix}${safeHostname || "<legacy>"}`;
61+
return `${prefix}${safeHostname}`;
6262
}
6363

6464
private async getSecret<T>(

src/remote/sshConfig.ts

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -183,14 +183,10 @@ export class SshConfig {
183183
private raw: string | undefined;
184184

185185
private startBlockComment(safeHostname: string): string {
186-
return safeHostname
187-
? `# --- START CODER VSCODE ${safeHostname} ---`
188-
: `# --- START CODER VSCODE ---`;
186+
return `# --- START CODER VSCODE ${safeHostname} ---`;
189187
}
190188
private endBlockComment(safeHostname: string): string {
191-
return safeHostname
192-
? `# --- END CODER VSCODE ${safeHostname} ---`
193-
: `# --- END CODER VSCODE ---`;
189+
return `# --- END CODER VSCODE ${safeHostname} ---`;
194190
}
195191

196192
constructor(
@@ -248,13 +244,13 @@ export class SshConfig {
248244
const endBlockCount = countSubstring(endBlock, raw);
249245
if (startBlockCount !== endBlockCount) {
250246
throw new SshConfigBadFormat(
251-
`Malformed config: ${this.filePath} has an unterminated START CODER VSCODE ${safeHostname ? safeHostname + " " : ""}block. Each START block must have an END block.`,
247+
`Malformed config: ${this.filePath} has an unterminated START CODER VSCODE ${safeHostname} block. Each START block must have an END block.`,
252248
);
253249
}
254250

255251
if (startBlockCount > 1 || endBlockCount > 1) {
256252
throw new SshConfigBadFormat(
257-
`Malformed config: ${this.filePath} has ${startBlockCount} START CODER VSCODE ${safeHostname ? safeHostname + " " : ""}sections. Please remove all but one.`,
253+
`Malformed config: ${this.filePath} has ${startBlockCount} START CODER VSCODE ${safeHostname} sections. Please remove all but one.`,
258254
);
259255
}
260256

src/util.ts

Lines changed: 36 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,10 @@ export interface AuthorityParts {
1313
// they should be handled by this extension.
1414
export const AuthorityPrefix = "coder-vscode";
1515

16+
const authorityHostPrefix = `${AuthorityPrefix}.`;
17+
const invalidAuthorityMessage =
18+
"Invalid Coder SSH authority. Must be: <hostname>--<username>--<workspace>(.<agent?>)";
19+
1620
// Regex patterns to find the SSH port from Remote SSH extension logs.
1721
// `ms-vscode-remote.remote-ssh`: `-> socksPort <port> ->` or `between local port <port>`
1822
// `codeium.windsurf-remote-openssh`, `jeanp413.open-remote-ssh`, `google.antigravity-remote-openssh`: `=> <port>(socks) =>`
@@ -49,60 +53,57 @@ export function findPort(text: string): number | null {
4953
/**
5054
* Given an authority, parse into the expected parts.
5155
*
56+
* The authority looks like `<scheme>://ssh-remote+<ssh host name>`, where the
57+
* SSH host names created by this extension match the format:
58+
* coder-vscode.<safeHostname>--<username>--<workspace>(.<agent?>)
59+
*
5260
* If this is not a Coder host, return null.
5361
*
5462
* Throw an error if the host is invalid.
5563
*/
5664
export function parseRemoteAuthority(authority: string): AuthorityParts | null {
57-
// The authority looks like: vscode://ssh-remote+<ssh host name>
5865
const authorityParts = authority.split("+");
66+
const sshHost = authorityParts[1];
67+
if (!sshHost) {
68+
return null;
69+
}
5970

60-
// We create SSH host names in a format matching:
61-
// coder-vscode(--|.)<username>--<workspace>(--|.)<agent?>
62-
// The agent can be omitted; the user will be prompted for it instead.
63-
// Anything else is unrelated to Coder and can be ignored.
64-
const parts = authorityParts[1].split("--");
65-
if (
66-
parts.length <= 1 ||
67-
(parts[0] !== AuthorityPrefix &&
68-
!parts[0].startsWith(`${AuthorityPrefix}.`))
69-
) {
71+
const parts = sshHost.split("--");
72+
if (!parts[0].startsWith(authorityHostPrefix)) {
7073
return null;
7174
}
7275

73-
// Reassemble Punycode labels (xn--...) the split broke apart: when the
74-
// prefix ends in ".xn", the cut landed inside an "xn--..." label.
75-
while (parts.length >= 2 && parts[0].endsWith(".xn")) {
76-
parts.splice(0, 2, `${parts[0]}--${parts[1]}`);
76+
if (parts.length < 3) {
77+
throw new Error(invalidAuthorityMessage);
7778
}
7879

79-
// It has the proper prefix, so this is probably a Coder host name.
80-
// Validate the SSH host name. Including the prefix, we expect at least
81-
// three parts, or four if including the agent.
82-
if ((parts.length !== 3 && parts.length !== 4) || parts.some((p) => !p)) {
83-
throw new Error(
84-
`Invalid Coder SSH authority. Must be: <username>--<workspace>(--|.)<agent?>`,
85-
);
80+
// Parse from the right because safe hostnames can contain "--".
81+
const hostPrefix = parts.slice(0, -2).join("--");
82+
const safeHostname = hostPrefix.slice(authorityHostPrefix.length);
83+
const username = parts[parts.length - 2];
84+
const workspaceAndAgent = parts[parts.length - 1];
85+
if (!safeHostname || !username || !workspaceAndAgent) {
86+
throw new Error(invalidAuthorityMessage);
8687
}
8788

88-
let workspace = parts[2];
89+
let workspace = workspaceAndAgent;
8990
let agent = "";
90-
if (parts.length === 4) {
91-
agent = parts[3];
92-
} else if (parts.length === 3) {
93-
const workspaceParts = parts[2].split(".");
94-
if (workspaceParts.length === 2) {
95-
workspace = workspaceParts[0];
96-
agent = workspaceParts[1];
91+
const workspaceParts = workspaceAndAgent.split(".");
92+
// Multiple dots are ambiguous because workspace and agent share this separator.
93+
if (workspaceParts.length === 2) {
94+
workspace = workspaceParts[0];
95+
agent = workspaceParts[1];
96+
if (!workspace || !agent) {
97+
throw new Error(invalidAuthorityMessage);
9798
}
9899
}
99100

100101
return {
101-
agent: agent,
102-
sshHost: authorityParts[1],
103-
safeHostname: parts[0].replace(/^coder-vscode\.?/, ""),
104-
username: parts[1],
105-
workspace: workspace,
102+
agent,
103+
sshHost,
104+
safeHostname,
105+
username,
106+
workspace,
106107
};
107108
}
108109

test/unit/core/pathResolver.test.ts

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -19,15 +19,6 @@ describe("PathResolver", () => {
1919
mockConfig = new MockConfigurationProvider();
2020
});
2121

22-
it("should use base path for empty labels", () => {
23-
expectPathsEqual(pathResolver.getGlobalConfigDir(""), basePath);
24-
expectPathsEqual(
25-
pathResolver.getSessionTokenPath(""),
26-
path.join(basePath, "session"),
27-
);
28-
expectPathsEqual(pathResolver.getUrlPath(""), path.join(basePath, "url"));
29-
});
30-
3122
describe("getProxyLogPath", () => {
3223
const defaultLogPath = path.join(basePath, "log");
3324

test/unit/remote/sshConfig.test.ts

Lines changed: 0 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -45,44 +45,6 @@ afterEach(() => {
4545
vi.clearAllMocks();
4646
});
4747

48-
it("creates a new file and adds config with empty label", async () => {
49-
mockFileSystem.readFile.mockRejectedValueOnce("No file found");
50-
mockFileSystem.stat.mockRejectedValueOnce({ code: "ENOENT" });
51-
52-
const sshConfig = new SshConfig(sshFilePath, mockLogger, mockFileSystem);
53-
await sshConfig.load();
54-
await sshConfig.update("", { ...BASE_SSH_VALUES, Host: "coder-vscode--*" });
55-
56-
const expectedOutput = `# --- START CODER VSCODE ---
57-
${managedHeader}
58-
Host coder-vscode--*
59-
ConnectTimeout 0
60-
LogLevel ERROR
61-
ProxyCommand some-command-here
62-
ServerAliveCountMax 3
63-
ServerAliveInterval 10
64-
StrictHostKeyChecking no
65-
UserKnownHostsFile /dev/null
66-
# --- END CODER VSCODE ---`;
67-
68-
expect(mockFileSystem.readFile).toHaveBeenCalledWith(
69-
sshFilePath,
70-
expect.anything(),
71-
);
72-
expect(mockFileSystem.writeFile).toHaveBeenCalledWith(
73-
expect.stringContaining(sshTempFilePrefix),
74-
expectedOutput,
75-
expect.objectContaining({
76-
encoding: "utf-8",
77-
mode: 0o600, // Default mode for new files.
78-
}),
79-
);
80-
expect(mockFileSystem.rename).toHaveBeenCalledWith(
81-
expect.stringContaining(sshTempFilePrefix),
82-
sshFilePath,
83-
);
84-
});
85-
8648
it("creates a new file and adds the config", async () => {
8749
mockFileSystem.readFile.mockRejectedValueOnce("No file found");
8850
mockFileSystem.stat.mockRejectedValueOnce({ code: "ENOENT" });
@@ -407,48 +369,6 @@ Host afterconfig
407369
);
408370
});
409371

410-
it("throws an error if there is a mismatched start and end block count (without label)", async () => {
411-
// As above, but without a label.
412-
const existentSSHConfig = `Host beforeconfig
413-
HostName before.config.tld
414-
User before
415-
416-
# --- START CODER VSCODE ---
417-
Host coder-vscode.dev.coder.com--*
418-
ConnectTimeout 0
419-
LogLevel ERROR
420-
ProxyCommand some-command-here
421-
StrictHostKeyChecking no
422-
UserKnownHostsFile /dev/null
423-
# missing END CODER VSCODE ---
424-
425-
Host donotdelete
426-
HostName dont.delete.me
427-
User please
428-
429-
# --- START CODER VSCODE ---
430-
Host coder-vscode.dev.coder.com--*
431-
ConnectTimeout 0
432-
LogLevel ERROR
433-
ProxyCommand some-command-here
434-
StrictHostKeyChecking no
435-
UserKnownHostsFile /dev/null
436-
# --- END CODER VSCODE ---
437-
438-
Host afterconfig
439-
HostName after.config.tld
440-
User after`;
441-
442-
const sshConfig = new SshConfig(sshFilePath, mockLogger, mockFileSystem);
443-
mockFileSystem.readFile.mockResolvedValueOnce(existentSSHConfig);
444-
await sshConfig.load();
445-
446-
// When we try to update the config, it should throw an error.
447-
await expect(sshConfig.update("", BASE_SSH_VALUES)).rejects.toThrow(
448-
`Malformed config: ${sshFilePath} has an unterminated START CODER VSCODE block. Each START block must have an END block.`,
449-
);
450-
});
451-
452372
it("throws an error if there are more than one sections with the same label", async () => {
453373
const existentSSHConfig = `Host beforeconfig
454374
HostName before.config.tld

0 commit comments

Comments
 (0)