diff --git a/src/commands.ts b/src/commands.ts index bddd564f..a7ad0332 100644 --- a/src/commands.ts +++ b/src/commands.ts @@ -26,7 +26,7 @@ import { maybeAskAgent, maybeAskUrl } from "./promptUtils"; import { RECOMMENDED_SSH_SETTINGS, applySettingOverrides, -} from "./remote/userSettings"; +} from "./remote/sshOverrides"; import { getGlobalFlags, resolveCliAuth } from "./settings/cli"; import { escapeCommandArg, toRemoteAuthority, toSafeHost } from "./util"; import { vscodeProposed } from "./vscodeProposed"; diff --git a/src/remote/remote.ts b/src/remote/remote.ts index 526579a8..e482725a 100644 --- a/src/remote/remote.ts +++ b/src/remote/remote.ts @@ -45,6 +45,7 @@ import { getHeaderCommand } from "../settings/headers"; import { AuthorityPrefix, escapeCommandArg, + expandPath, parseRemoteAuthority, } from "../util"; import { vscodeProposed } from "../vscodeProposed"; @@ -57,9 +58,9 @@ import { parseCoderSshOptions, parseSshConfig, } from "./sshConfig"; +import { applySettingOverrides, buildSshOverrides } from "./sshOverrides"; import { SshProcessMonitor } from "./sshProcess"; import { computeSshProperties, sshSupportsSetEnv } from "./sshSupport"; -import { applySettingOverrides, buildSshOverrides } from "./userSettings"; import { WorkspaceStateMachine } from "./workspaceStateMachine"; export interface RemoteDetails extends vscode.Disposable { @@ -459,33 +460,12 @@ export class Remote { const inbox = await Inbox.create(workspace, workspaceClient, this.logger); disposables.push(inbox); - this.logger.info("Modifying settings..."); - const overrides = buildSshOverrides( - vscodeProposed.workspace.getConfiguration(), - parts.sshHost, - agent.operating_system, - ); - if (overrides.length > 0) { - const ok = await applySettingOverrides( - this.pathResolver.getUserSettingsPath(), - overrides, - this.logger, - ); - if (ok) { - this.logger.info("Settings modified successfully"); - } - } - const logDir = this.getLogDir(featureSet); - // This ensures the Remote SSH extension resolves the host to execute the - // Coder binary properly. - // - // If we didn't write to the SSH config file, connecting would fail with - // "Host not found". + let computedSshProperties: Record = {}; try { this.logger.info("Updating SSH config..."); - await this.updateSSHConfig( + computedSshProperties = await this.updateSSHConfig( workspaceClient, parts.safeHostname, parts.sshHost, @@ -499,6 +479,27 @@ export class Remote { throw error; } + const remoteCommand = computedSshProperties.RemoteCommand; + + this.logger.info("Modifying settings..."); + const overrides = buildSshOverrides( + vscodeProposed.workspace.getConfiguration(), + parts.sshHost, + agent.operating_system, + remoteCommand, + this.logger, + ); + if (overrides.length > 0) { + const ok = await applySettingOverrides( + this.pathResolver.getUserSettingsPath(), + overrides, + this.logger, + ); + if (ok) { + this.logger.info("Settings modified successfully"); + } + } + // Monitor SSH process and display network status const sshMonitor = SshProcessMonitor.start({ sshHost: parts.sshHost, @@ -731,6 +732,13 @@ export class Remote { return ["--log-dir", escapeCommandArg(logDir), "-v"]; } + private getSshConfigPath(): string { + const configured = vscode.workspace + .getConfiguration() + .get("remote.SSH.configFile"); + return expandPath(configured || path.join("~", ".ssh", "config")); + } + // updateSSHConfig updates the SSH configuration with a wildcard that handles // all Coder entries. private async updateSSHConfig( @@ -741,7 +749,7 @@ export class Remote { logDir: string, featureSet: FeatureSet, cliAuth: CliAuth, - ) { + ): Promise> { let deploymentSSHConfig = {}; try { const deploymentConfig = await restClient.getDeploymentSSHConfig(); @@ -761,17 +769,7 @@ export class Remote { } } - let sshConfigFile = vscode.workspace - .getConfiguration() - .get("remote.SSH.configFile"); - if (!sshConfigFile) { - sshConfigFile = path.join(os.homedir(), ".ssh", "config"); - } - // VS Code Remote resolves ~ to the home directory. - // This is required for the tilde to work on Windows. - if (sshConfigFile.startsWith("~")) { - sshConfigFile = path.join(os.homedir(), sshConfigFile.slice(1)); - } + const sshConfigFile = this.getSshConfigPath(); const sshConfig = new SshConfig(sshConfigFile, this.logger); await sshConfig.load(); @@ -852,7 +850,7 @@ export class Remote { throw new Error("SSH config mismatch, closing remote"); } - return sshConfig.getRaw(); + return computedProperties; } private watchSettings( diff --git a/src/remote/userSettings.ts b/src/remote/sshOverrides.ts similarity index 73% rename from src/remote/userSettings.ts rename to src/remote/sshOverrides.ts index 25f11202..65b13345 100644 --- a/src/remote/userSettings.ts +++ b/src/remote/sshOverrides.ts @@ -59,6 +59,14 @@ const AUTO_SETUP_DEFAULTS = { "remote.SSH.maxReconnectionAttempts": null, // max allowed } as const satisfies Partial>; +/** + * Whether the given RemoteCommand value represents an active command + * (i.e. present, non-empty, and not the SSH default "none"). + */ +function isActiveRemoteCommand(cmd: string | undefined): boolean { + return !!cmd && cmd.toLowerCase() !== "none"; +} + /** * Build the list of VS Code setting overrides needed for a remote SSH * connection to a Coder workspace. @@ -67,19 +75,45 @@ export function buildSshOverrides( config: Pick, sshHost: string, agentOS: string, + remoteCommand: string | undefined, + logger: Logger, ): SettingOverride[] { const overrides: SettingOverride[] = []; - // Set the remote platform for this host to bypass the platform prompt. + // When enableRemoteCommand is true and the host has an active + // RemoteCommand, we must not set remotePlatform: it causes VS Code + // to append 'bash', which conflicts with RemoteCommand. We gate on + // enableRemoteCommand so users who haven't opted in don't get an + // unexpected platform prompt. + const enableRemoteCommand = config.get( + "remote.SSH.enableRemoteCommand", + false, + ); + const skipRemotePlatform = + enableRemoteCommand && isActiveRemoteCommand(remoteCommand); + const remotePlatforms = config.get>( "remote.SSH.remotePlatform", {}, ); - if (remotePlatforms[sshHost] !== agentOS) { - overrides.push({ - key: "remote.SSH.remotePlatform", - value: { ...remotePlatforms, [sshHost]: agentOS }, - }); + if (skipRemotePlatform) { + logger.info("RemoteCommand detected, skipping remotePlatform override"); + // Remove any stale entry so it doesn't block RemoteCommand. + if (sshHost in remotePlatforms) { + const { [sshHost]: _removed, ...rest } = remotePlatforms; + overrides.push({ + key: "remote.SSH.remotePlatform", + value: rest, + }); + } + } else { + // Set the remote platform to bypass the platform prompt. + if (remotePlatforms[sshHost] !== agentOS) { + overrides.push({ + key: "remote.SSH.remotePlatform", + value: { ...remotePlatforms, [sshHost]: agentOS }, + }); + } } // Default 15s is too short for startup scripts; enforce a minimum. diff --git a/test/unit/remote/userSettings.test.ts b/test/unit/remote/sshOverrides.test.ts similarity index 71% rename from test/unit/remote/userSettings.test.ts rename to test/unit/remote/sshOverrides.test.ts index 60761c3d..0c60117d 100644 --- a/test/unit/remote/userSettings.test.ts +++ b/test/unit/remote/sshOverrides.test.ts @@ -5,7 +5,7 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import { applySettingOverrides, buildSshOverrides, -} from "@/remote/userSettings"; +} from "@/remote/sshOverrides"; import { MockConfigurationProvider, @@ -33,6 +33,7 @@ interface TimeoutCase { } describe("buildSshOverrides", () => { + const buildLogger = createMockLogger(); describe("remote platform", () => { it("adds host when missing or OS differs", () => { const config = new MockConfigurationProvider(); @@ -41,7 +42,13 @@ describe("buildSshOverrides", () => { config.set("remote.SSH.remotePlatform", { "other-host": "darwin" }); expect( findOverride( - buildSshOverrides(config, "new-host", "linux"), + buildSshOverrides( + config, + "new-host", + "linux", + undefined, + buildLogger, + ), "remote.SSH.remotePlatform", ), ).toEqual({ "other-host": "darwin", "new-host": "linux" }); @@ -50,7 +57,7 @@ describe("buildSshOverrides", () => { config.set("remote.SSH.remotePlatform", { "my-host": "windows" }); expect( findOverride( - buildSshOverrides(config, "my-host", "linux"), + buildSshOverrides(config, "my-host", "linux", undefined, buildLogger), "remote.SSH.remotePlatform", ), ).toEqual({ "my-host": "linux" }); @@ -61,11 +68,85 @@ describe("buildSshOverrides", () => { config.set("remote.SSH.remotePlatform", { "my-host": "linux" }); expect( findOverride( - buildSshOverrides(config, "my-host", "linux"), + buildSshOverrides(config, "my-host", "linux", undefined, buildLogger), "remote.SSH.remotePlatform", ), ).toBeUndefined(); }); + + describe("RemoteCommand compatibility", () => { + it("removes host from remotePlatform when enableRemoteCommand is true", () => { + const config = new MockConfigurationProvider(); + config.set("remote.SSH.enableRemoteCommand", true); + config.set("remote.SSH.remotePlatform", { + "my-host": "linux", + "other-host": "darwin", + }); + expect( + findOverride( + buildSshOverrides( + config, + "my-host", + "linux", + "exec bash -l", + buildLogger, + ), + "remote.SSH.remotePlatform", + ), + ).toEqual({ "other-host": "darwin" }); + }); + + it("produces no override when host has no stale remotePlatform entry", () => { + const config = new MockConfigurationProvider(); + config.set("remote.SSH.enableRemoteCommand", true); + config.set("remote.SSH.remotePlatform", {}); + expect( + findOverride( + buildSshOverrides( + config, + "my-host", + "linux", + "exec bash -l", + buildLogger, + ), + "remote.SSH.remotePlatform", + ), + ).toBeUndefined(); + }); + + it("sets platform normally when enableRemoteCommand is false", () => { + const config = new MockConfigurationProvider(); + config.set("remote.SSH.enableRemoteCommand", false); + config.set("remote.SSH.remotePlatform", {}); + expect( + findOverride( + buildSshOverrides( + config, + "my-host", + "linux", + "exec bash -l", + buildLogger, + ), + "remote.SSH.remotePlatform", + ), + ).toEqual({ "my-host": "linux" }); + }); + + it.each(["none", "None", "NONE", "", undefined])( + "sets platform normally when remoteCommand is %j", + (cmd) => { + const config = new MockConfigurationProvider(); + config.set("remote.SSH.enableRemoteCommand", true); + config.set("remote.SSH.remotePlatform", {}); + expect( + findOverride( + buildSshOverrides(config, "my-host", "linux", cmd, buildLogger), + "remote.SSH.remotePlatform", + ), + ).toEqual({ "my-host": "linux" }); + }, + ); + }); }); describe("connect timeout", () => { @@ -81,7 +162,7 @@ describe("buildSshOverrides", () => { } expect( findOverride( - buildSshOverrides(config, "host", "linux"), + buildSshOverrides(config, "host", "linux", undefined, buildLogger), "remote.SSH.connectTimeout", ), ).toBe(1800); @@ -95,7 +176,7 @@ describe("buildSshOverrides", () => { config.set("remote.SSH.connectTimeout", timeout); expect( findOverride( - buildSshOverrides(config, "host", "linux"), + buildSshOverrides(config, "host", "linux", undefined, buildLogger), "remote.SSH.connectTimeout", ), ).toBeUndefined(); @@ -106,7 +187,13 @@ describe("buildSshOverrides", () => { it("defaults to 8 hours when not configured", () => { expect( findOverride( - buildSshOverrides(new MockConfigurationProvider(), "host", "linux"), + buildSshOverrides( + new MockConfigurationProvider(), + "host", + "linux", + undefined, + buildLogger, + ), "remote.SSH.reconnectionGraceTime", ), ).toBe(28800); @@ -117,7 +204,7 @@ describe("buildSshOverrides", () => { config.set("remote.SSH.reconnectionGraceTime", 3600); expect( findOverride( - buildSshOverrides(config, "host", "linux"), + buildSshOverrides(config, "host", "linux", undefined, buildLogger), "remote.SSH.reconnectionGraceTime", ), ).toBeUndefined(); @@ -132,6 +219,8 @@ describe("buildSshOverrides", () => { new MockConfigurationProvider(), "host", "linux", + undefined, + buildLogger, ); expect(findOverride(overrides, key)).toBe(expected); }); @@ -143,7 +232,9 @@ describe("buildSshOverrides", () => { config.set("remote.SSH.reconnectionGraceTime", 7200); config.set("remote.SSH.serverShutdownTimeout", 600); config.set("remote.SSH.maxReconnectionAttempts", 4); - expect(buildSshOverrides(config, "my-host", "linux")).toHaveLength(0); + expect( + buildSshOverrides(config, "my-host", "linux", undefined, buildLogger), + ).toHaveLength(0); }); }); diff --git a/test/unit/remote/sshSupport.test.ts b/test/unit/remote/sshSupport.test.ts index 8da0eb6f..9bd47289 100644 --- a/test/unit/remote/sshSupport.test.ts +++ b/test/unit/remote/sshSupport.test.ts @@ -1,4 +1,4 @@ -import { it, expect } from "vitest"; +import { describe, it, expect } from "vitest"; import { computeSshProperties, @@ -26,10 +26,11 @@ it("current shell supports ssh", () => { expect(sshSupportsSetEnv()).toBeTruthy(); }); -it("computes the config for a host", () => { - const properties = computeSshProperties( - "coder-vscode--testing", - `Host * +describe("computeSshProperties", () => { + it("computes the config for a host", () => { + const properties = computeSshProperties( + "coder-vscode--testing", + `Host * StrictHostKeyChecking yes # --- START CODER VSCODE --- @@ -39,19 +40,19 @@ Host coder-vscode--* ProxyCommand=/tmp/coder --header="X-FOO=bar" coder.dev # --- END CODER VSCODE --- `, - ); + ); - expect(properties).toEqual({ - Another: "true", - StrictHostKeyChecking: "yes", - ProxyCommand: '/tmp/coder --header="X-FOO=bar" coder.dev', + expect(properties).toEqual({ + Another: "true", + StrictHostKeyChecking: "yes", + ProxyCommand: '/tmp/coder --header="X-FOO=bar" coder.dev', + }); }); -}); -it("handles ? wildcards", () => { - const properties = computeSshProperties( - "coder-vscode--testing", - `Host * + it("handles ? wildcards", () => { + const properties = computeSshProperties( + "coder-vscode--testing", + `Host * StrictHostKeyChecking yes Host i-???????? i-????????????????? @@ -67,19 +68,70 @@ Host coder-v?code--* ProxyCommand=/tmp/coder --header="X-BAR=foo" coder.dev # --- END CODER VSCODE --- `, - ); + ); - expect(properties).toEqual({ - Another: "true", - StrictHostKeyChecking: "yes", - ProxyCommand: '/tmp/coder --header="X-BAR=foo" coder.dev', + expect(properties).toEqual({ + Another: "true", + StrictHostKeyChecking: "yes", + ProxyCommand: '/tmp/coder --header="X-BAR=foo" coder.dev', + }); + }); + + it("picks up RemoteCommand from a user Host block alongside a Coder block", () => { + const props = computeSshProperties( + "coder-vscode.example.com--user--ws", + `# --- START CODER VSCODE example.com --- +Host coder-vscode.example.com--* + ProxyCommand /path/to/coder ssh --stdio %h + StrictHostKeyChecking no +# --- END CODER VSCODE example.com --- + +Host coder-vscode.example.com--* + RequestTTY yes + RemoteCommand exec /bin/bash -l +`, + ); + expect(props.RemoteCommand).toBe("exec /bin/bash -l"); + expect(props.ProxyCommand).toBe("/path/to/coder ssh --stdio %h"); + }); + + it("returns RemoteCommand none literally", () => { + const props = computeSshProperties( + "coder-vscode.example.com--user--ws", + `Host coder-vscode.example.com--* + RemoteCommand none +`, + ); + expect(props.RemoteCommand).toBe("none"); + }); + + it("inherits RemoteCommand from a Host * block", () => { + const props = computeSshProperties( + "coder-vscode.example.com--user--ws", + `Host * + RemoteCommand exec /bin/zsh -l + +Host coder-vscode.example.com--* + ProxyCommand /path/to/coder ssh --stdio %h +`, + ); + expect(props.RemoteCommand).toBe("exec /bin/zsh -l"); }); -}); -it("properly escapes meaningful regex characters", () => { - const properties = computeSshProperties( - "coder-vscode.dev.coder.com--matalfi--dogfood", - `Host * + it("handles RemoteCommand with = delimiter", () => { + const props = computeSshProperties( + "coder-vscode.example.com--user--ws", + `Host coder-vscode.example.com--* + RemoteCommand=exec /bin/bash -l +`, + ); + expect(props.RemoteCommand).toBe("exec /bin/bash -l"); + }); + + it("properly escapes meaningful regex characters", () => { + const properties = computeSshProperties( + "coder-vscode.dev.coder.com--matalfi--dogfood", + `Host * StrictHostKeyChecking yes # ------------START-CODER----------- @@ -102,12 +154,13 @@ Host coder-vscode.dev.coder.com--* # --- END CODER VSCODE dev.coder.com ---% `, - ); - - expect(properties).toEqual({ - StrictHostKeyChecking: "yes", - ProxyCommand: - '"/Users/matifali/Library/Application Support/Code/User/globalStorage/coder.coder-remote/dev.coder.com/bin/coder-darwin-arm64" vscodessh --network-info-dir "/Users/matifali/Library/Application Support/Code/User/globalStorage/coder.coder-remote/net" --session-token-file "/Users/matifali/Library/Application Support/Code/User/globalStorage/coder.coder-remote/dev.coder.com/session" --url-file "/Users/matifali/Library/Application Support/Code/User/globalStorage/coder.coder-remote/dev.coder.com/url" %h', - UserKnownHostsFile: "/dev/null", + ); + + expect(properties).toEqual({ + StrictHostKeyChecking: "yes", + ProxyCommand: + '"/Users/matifali/Library/Application Support/Code/User/globalStorage/coder.coder-remote/dev.coder.com/bin/coder-darwin-arm64" vscodessh --network-info-dir "/Users/matifali/Library/Application Support/Code/User/globalStorage/coder.coder-remote/net" --session-token-file "/Users/matifali/Library/Application Support/Code/User/globalStorage/coder.coder-remote/dev.coder.com/session" --url-file "/Users/matifali/Library/Application Support/Code/User/globalStorage/coder.coder-remote/dev.coder.com/url" %h', + UserKnownHostsFile: "/dev/null", + }); }); });