fix: include our own SSH config from the top of the user's

SSH uses the first value it obtains for each option, so a catch-all
"Host *" in the user's config beat the block we appended to the end of
it, and connections aborted with "Unexpected SSH Config Option". Writing
the block higher up would not be enough: it still loses to hosts pulled
in by an Include above it, and a Host line moved over the options
someone wrote outside any block would capture them.

Write the blocks to ~/.ssh/coder/config instead and include that file
from the first line of the user's config, where nothing can be parsed
before it. Their config is written once to add the include, and the
deployment's old block moves out of it on the next connect.

The include path keeps its tilde: relative includes resolve against
~/.ssh no matter where the including file lives, and an absolute path
would not survive a config synced between machines.

Since placement now guarantees the options apply, the block that
recomputed them and aborted the connection on a mismatch is gone. What
remains of computeSshProperties reads RemoteCommand, which can only come
from the user's config.

Author: EhabY

CHANGELOG.md

@@ -17,6 +17,13 @@
 
 ### Changed
 
+- Write workspace SSH hosts to `~/.ssh/coder/config` and include that file
+  from the top of your own SSH config, rather than writing the block into your
+  config directly. SSH uses the first value it obtains for each option, so a
+  catch-all like `Host *` used to override the connection's `ProxyCommand` and
+  abort it with an "Unexpected SSH Config Option" error; now the extension's
+  options win and that error is gone. Your config is only written once, to add
+  the include, and the block for a deployment moves out of it on next connect.
 - Filter the Shared Workspaces view with the server-side `shared_with_user`
   query instead of filtering `shared:true` results on the client, so fewer
   workspaces are fetched and the view loads faster. Deployments too old to

src/remote/remote.ts

@@ -77,6 +77,13 @@ import type { SecretsManager } from "../core/secretsManager";
 import type { Logger } from "../logging/logger";
 import type { LoginCoordinator } from "../login/loginCoordinator";
 
+/**
+ * Our own config, included from the user's. Keep the tilde: relative includes
+ * always resolve against ~/.ssh, and an absolute path would not survive a
+ * config synced between machines.
+ */
+const CODER_SSH_CONFIG_PATH = "~/.ssh/coder/config";
+
 export interface RemoteDetails extends vscode.Disposable {
 	safeHostname: string;
 	url: string;
@@ -953,15 +960,20 @@ export class Remote {
 			}
 		}
 
-		const sshConfigFile = this.getSshConfigPath();
-
-		const sshConfig = new SshConfig(sshConfigFile, this.logger);
+		// Our blocks live in our own file; the user's only gains the include.
+		const sshConfig = new SshConfig(this.getSshConfigPath(), this.logger);
 		await sshConfig.load();
+		const coderConfig = new SshConfig(
+			expandPath(CODER_SSH_CONFIG_PATH),
+			this.logger,
+		);
+		await coderConfig.load();
 
 		// Merge SSH config from three sources (highest to lowest priority):
 		// 1. User's VS Code coder.sshConfig setting
 		// 2. coder config-ssh --ssh-option flags from the CLI block
 		// 3. Deployment SSH config from the coderd API
+		// The CLI writes its block to the user's config, so read it from there.
 		const configSshOptions = parseCoderSshOptions(sshConfig.getRaw());
 		const userConfigSsh = vscode.workspace
 			.getConfiguration("coder")
@@ -1001,42 +1013,15 @@ export class Remote {
 			sshValues.SetEnv = "CODER_SSH_SESSION_TYPE=vscode";
 		}
 
-		await sshConfig.update(safeHostname, sshValues, sshConfigOverrides);
+		// Write our file before including it, so the include never dangles.
+		await coderConfig.update(safeHostname, sshValues, sshConfigOverrides);
+		await sshConfig.updateInclude(CODER_SSH_CONFIG_PATH, safeHostname);
 
-		// A user can provide a "Host *" entry in their SSH config to add options
-		// to all hosts. We need to ensure that the options we set are not
-		// overridden by the user's config.
-		const computedProperties = computeSshProperties(
+		// Mirror SSH's parse order; RemoteCommand can come from the user's config.
+		return computeSshProperties(
 			hostName,
-			sshConfig.getRaw(),
+			`${coderConfig.getRaw()}\n${sshConfig.getRaw()}`,
 		);
-		const keysToMatch: Array<keyof SshValues> = [
-			"ProxyCommand",
-			"UserKnownHostsFile",
-			"StrictHostKeyChecking",
-		];
-		for (const key of keysToMatch) {
-			if (computedProperties[key] === sshValues[key]) {
-				continue;
-			}
-
-			const result = await vscodeProposed.window.showErrorMessage(
-				"Unexpected SSH Config Option",
-				{
-					useCustom: true,
-					modal: true,
-					detail: `Your SSH config is overriding the "${key}" property to "${computedProperties[key]}" when it expected "${sshValues[key]}" for the "${hostName}" host. Please fix this and try again!`,
-				},
-				"Reload Window",
-			);
-			if (result === "Reload Window") {
-				await this.reloadWindow();
-			}
-			await this.closeRemote();
-			throw new Error("SSH config mismatch, closing remote");
-		}
-
-		return computedProperties;
 	}
 
 	private watchSettings(

src/remote/sshConfig.ts

@@ -53,6 +53,18 @@ const defaultFileSystem: FileSystem = {
 // Matches an SSH config key at the start of a line (e.g. "ConnectTimeout", "LogLevel").
 const sshKeyRegex = /^[a-zA-Z0-9-]+/;
 
+const START_BLOCK_PREFIX = "# --- START CODER VSCODE";
+const END_BLOCK_PREFIX = "# --- END CODER VSCODE";
+
+const INCLUDE_START = `${START_BLOCK_PREFIX} INCLUDE ---`;
+const INCLUDE_END = `${END_BLOCK_PREFIX} INCLUDE ---`;
+
+/** Matches our include block wherever it currently sits. */
+const INCLUDE_BLOCK_REGEX = new RegExp(
+	`^${INCLUDE_START}$.*?^${INCLUDE_END}$`,
+	"ms",
+);
+
 // Matches the Coder CLI's START-CODER / END-CODER block, flexible on dash count.
 const coderBlockRegex = /^# -+START-CODER-+$(.*?)^# -+END-CODER-+$/ms;
 
@@ -184,10 +196,10 @@ export class SshConfig {
 	private raw: string | undefined;
 
 	private startBlockComment(safeHostname: string): string {
-		return `# --- START CODER VSCODE ${safeHostname} ---`;
+		return `${START_BLOCK_PREFIX} ${safeHostname} ---`;
 	}
 	private endBlockComment(safeHostname: string): string {
-		return `# --- END CODER VSCODE ${safeHostname} ---`;
+		return `${END_BLOCK_PREFIX} ${safeHostname} ---`;
 	}
 
 	constructor(
@@ -233,6 +245,41 @@ export class SshConfig {
 		await this.save();
 	}
 
+	/**
+	 * Include `includePath` from the first line, so the options it holds win:
+	 * SSH uses the first value it obtains for each one. Also drops the
+	 * deployment's own block, which the included file supersedes.
+	 */
+	async updateInclude(includePath: string, safeHostname: string) {
+		const original = this.getRaw();
+		const block = this.getBlock(safeHostname);
+		if (block) {
+			this.logger.debug("Removing superseded SSH config block", safeHostname);
+			this.removeBlock(block);
+		}
+		const include = [
+			INCLUDE_START,
+			"# Your Coder workspaces, managed by the Coder VS Code extension. Keep first:",
+			"# SSH uses the first value found, so anything above this block overrides them.",
+			`Include ${includePath}`,
+			INCLUDE_END,
+		].join("\n");
+		const rest = this.getRaw().replace(INCLUDE_BLOCK_REGEX, "").trim();
+		this.raw = rest ? `${include}\n\n${rest}` : include;
+		if (this.getRaw() !== original) {
+			this.logger.debug("Including SSH config", includePath);
+			await this.save();
+		}
+	}
+
+	private removeBlock(block: Block) {
+		const raw = this.getRaw();
+		const start = raw.indexOf(block.raw);
+		const before = raw.slice(0, start).trimEnd();
+		const after = raw.slice(start + block.raw.length).trimStart();
+		this.raw = [before, after].filter(Boolean).join("\n\n");
+	}
+
 	/**
 	 * Get the block for the deployment with the provided hostname.
 	 */
@@ -262,14 +309,6 @@ export class SshConfig {
 			return;
 		}
 
-		if (startBlockIndex === -1) {
-			throw new SshConfigBadFormat("Start block not found");
-		}
-
-		if (startBlockIndex === -1) {
-			throw new SshConfigBadFormat("End block not found");
-		}
-
 		if (endBlockIndex < startBlockIndex) {
 			throw new SshConfigBadFormat(
 				"Malformed config, end block is before start block",
@@ -303,8 +342,8 @@ export class SshConfig {
 		const { Host, ...otherValues } = values;
 		const lines = [
 			this.startBlockComment(safeHostname),
-			"# This section is managed by the Coder VS Code extension.",
-			"# Changes will be overwritten on the next workspace connection.",
+			"# Rewritten by the Coder VS Code extension on every connection.",
+			'# To change these options, use the "coder.sshConfig" setting instead.',
 			`Host ${Host}`,
 		];
 

test/unit/remote/sshConfig.test.ts

@@ -16,8 +16,8 @@ import { createMockLogger } from "../../mocks/testHelpers";
 const sshFilePath = "/Path/To/UserHomeDir/.sshConfigDir/sshConfigFile";
 const sshTempFilePrefix =
 	"/Path/To/UserHomeDir/.sshConfigDir/.sshConfigFile.vscode-coder-tmp-";
-const managedHeader = `# This section is managed by the Coder VS Code extension.
-# Changes will be overwritten on the next workspace connection.`;
+const managedHeader = `# Rewritten by the Coder VS Code extension on every connection.
+# To change these options, use the "coder.sshConfig" setting instead.`;
 
 const mockFileSystem = {
 	mkdir: vi.fn(),
@@ -858,3 +858,70 @@ Host work-server
 		});
 	});
 });
+
+describe("updateInclude", () => {
+	const include = `# --- START CODER VSCODE INCLUDE ---
+# Your Coder workspaces, managed by the Coder VS Code extension. Keep first:
+# SSH uses the first value found, so anything above this block overrides them.
+Include ~/.ssh/coder/config
+# --- END CODER VSCODE INCLUDE ---`;
+
+	const managedBlock = `# --- START CODER VSCODE dev.coder.com ---
+Host coder-vscode.dev.coder.com--*
+  ProxyCommand some-command-here
+# --- END CODER VSCODE dev.coder.com ---`;
+
+	/** Include our config in `existing`, returning what was written, if anything. */
+	async function updateInclude(existing: string): Promise<string | undefined> {
+		mockFileSystem.readFile.mockResolvedValueOnce(existing);
+		mockFileSystem.stat.mockResolvedValueOnce({ mode: 0o644 });
+		const sshConfig = new SshConfig(sshFilePath, mockLogger, mockFileSystem);
+		await sshConfig.load();
+		await sshConfig.updateInclude("~/.ssh/coder/config", "dev.coder.com");
+		return mockFileSystem.writeFile.mock.calls.at(-1)?.[1] as
+			string | undefined;
+	}
+
+	it("goes above everything the user wrote", async () => {
+		const config =
+			"AddKeysToAgent yes\n\nInclude ~/.ssh/work\n\nHost *\n  ConnectTimeout 5";
+
+		await expect(updateInclude(config)).resolves.toBe(
+			`${include}\n\n${config}`,
+		);
+	});
+
+	it("creates the include in an empty config", async () => {
+		await expect(updateInclude("")).resolves.toBe(include);
+	});
+
+	it("leaves the file alone when the include is already first", async () => {
+		await expect(
+			updateInclude(`${include}\n\nHost *`),
+		).resolves.toBeUndefined();
+	});
+
+	it("moves an include that is no longer first", async () => {
+		const config = "Host *\n  ConnectTimeout 5";
+
+		await expect(updateInclude(`${config}\n\n${include}`)).resolves.toBe(
+			`${include}\n\n${config}`,
+		);
+	});
+
+	it("drops the block the included file supersedes", async () => {
+		const config = "Host *\n  ConnectTimeout 5";
+
+		await expect(updateInclude(`${config}\n\n${managedBlock}`)).resolves.toBe(
+			`${include}\n\n${config}`,
+		);
+	});
+
+	it("keeps blocks belonging to other deployments", async () => {
+		const other = managedBlock.replaceAll("dev.coder.com", "dev2.coder.com");
+
+		await expect(updateInclude(`${other}\n\n${managedBlock}`)).resolves.toBe(
+			`${include}\n\n${other}`,
+		);
+	});
+});