From cdf59fd9158af0934d08d93c43111ab3daec6b68 Mon Sep 17 00:00:00 2001 From: Ehab Younes Date: Mon, 23 Mar 2026 15:59:57 +0300 Subject: [PATCH 1/3] fix: skip remotePlatform when RemoteCommand is configured (#549) When a user configures RemoteCommand in their SSH config, VS Code's Remote SSH extension conflicts with it by appending 'bash' to the SSH command whenever remotePlatform is set for the host. This extension unconditionally set remotePlatform, making RemoteCommand unusable. Now buildSshOverrides checks remote.SSH.enableRemoteCommand and the host's effective RemoteCommand (via computeSshProperties). When both indicate active RemoteCommand usage, the remotePlatform entry is skipped (or removed if stale) so VS Code does not append a shell. --- src/remote/remote.ts | 76 ++++++++++++++------------- src/remote/userSettings.ts | 44 +++++++++++++--- test/unit/remote/sshSupport.test.ts | 51 ++++++++++++++++++ test/unit/remote/userSettings.test.ts | 69 ++++++++++++++++++++++++ 4 files changed, 198 insertions(+), 42 deletions(-) diff --git a/src/remote/remote.ts b/src/remote/remote.ts index 526579a8..7c5242e6 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"; @@ -59,7 +60,11 @@ import { } from "./sshConfig"; import { SshProcessMonitor } from "./sshProcess"; import { computeSshProperties, sshSupportsSetEnv } from "./sshSupport"; -import { applySettingOverrides, buildSshOverrides } from "./userSettings"; +import { + applySettingOverrides, + buildSshOverrides, + isActiveRemoteCommand, +} from "./userSettings"; import { WorkspaceStateMachine } from "./workspaceStateMachine"; export interface RemoteDetails extends vscode.Disposable { @@ -459,33 +464,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 +483,29 @@ export class Remote { throw error; } + const remoteCommand = computedSshProperties.RemoteCommand; + if (isActiveRemoteCommand(remoteCommand)) { + this.logger.info("RemoteCommand detected in SSH config"); + } + + this.logger.info("Modifying settings..."); + const overrides = buildSshOverrides( + vscodeProposed.workspace.getConfiguration(), + parts.sshHost, + agent.operating_system, + remoteCommand, + ); + 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 +738,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( @@ -761,17 +775,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 +856,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/userSettings.ts index 25f11202..7d82a3ed 100644 --- a/src/remote/userSettings.ts +++ b/src/remote/userSettings.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"). + */ +export 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,43 @@ export function buildSshOverrides( config: Pick, sshHost: string, agentOS: string, + remoteCommand?: string, ): 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) { + // 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/sshSupport.test.ts b/test/unit/remote/sshSupport.test.ts index 8da0eb6f..9e7b5059 100644 --- a/test/unit/remote/sshSupport.test.ts +++ b/test/unit/remote/sshSupport.test.ts @@ -76,6 +76,57 @@ Host coder-v?code--* }); }); +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("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", diff --git a/test/unit/remote/userSettings.test.ts b/test/unit/remote/userSettings.test.ts index 60761c3d..f38b3df7 100644 --- a/test/unit/remote/userSettings.test.ts +++ b/test/unit/remote/userSettings.test.ts @@ -5,6 +5,7 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import { applySettingOverrides, buildSshOverrides, + isActiveRemoteCommand, } from "@/remote/userSettings"; import { @@ -32,6 +33,18 @@ interface TimeoutCase { label: string; } +describe("isActiveRemoteCommand", () => { + it.each(["exec bash -l", "exec /bin/zsh", "/usr/bin/tmux"])( + "returns true for %j", + (cmd) => expect(isActiveRemoteCommand(cmd)).toBe(true), + ); + + it.each([undefined, "", "none", "None", "NONE"])( + "returns false for %j", + (cmd) => expect(isActiveRemoteCommand(cmd)).toBe(false), + ); +}); + describe("buildSshOverrides", () => { describe("remote platform", () => { it("adds host when missing or OS differs", () => { @@ -66,6 +79,62 @@ describe("buildSshOverrides", () => { ), ).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"), + "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"), + "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"), + "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), + "remote.SSH.remotePlatform", + ), + ).toEqual({ "my-host": "linux" }); + }, + ); + }); }); describe("connect timeout", () => { From f2b04e7da6f7d6fc54878c4d04c14172efe6d053 Mon Sep 17 00:00:00 2001 From: Ehab Younes Date: Wed, 25 Mar 2026 13:53:30 +0300 Subject: [PATCH 2/3] Self-review --- src/commands.ts | 2 +- src/remote/remote.ts | 12 +- .../{userSettings.ts => sshOverrides.ts} | 2 +- ...rSettings.test.ts => sshOverrides.test.ts} | 26 ++-- test/unit/remote/sshSupport.test.ts | 124 +++++++++--------- 5 files changed, 89 insertions(+), 77 deletions(-) rename src/remote/{userSettings.ts => sshOverrides.ts} (99%) rename test/unit/remote/{userSettings.test.ts => sshOverrides.test.ts} (93%) 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 7c5242e6..e46fc837 100644 --- a/src/remote/remote.ts +++ b/src/remote/remote.ts @@ -58,13 +58,13 @@ import { parseCoderSshOptions, parseSshConfig, } from "./sshConfig"; -import { SshProcessMonitor } from "./sshProcess"; -import { computeSshProperties, sshSupportsSetEnv } from "./sshSupport"; import { applySettingOverrides, buildSshOverrides, isActiveRemoteCommand, -} from "./userSettings"; +} from "./sshOverrides"; +import { SshProcessMonitor } from "./sshProcess"; +import { computeSshProperties, sshSupportsSetEnv } from "./sshSupport"; import { WorkspaceStateMachine } from "./workspaceStateMachine"; export interface RemoteDetails extends vscode.Disposable { @@ -485,7 +485,9 @@ export class Remote { const remoteCommand = computedSshProperties.RemoteCommand; if (isActiveRemoteCommand(remoteCommand)) { - this.logger.info("RemoteCommand detected in SSH config"); + this.logger.info( + "RemoteCommand detected, skipping remotePlatform override", + ); } this.logger.info("Modifying settings..."); @@ -755,7 +757,7 @@ export class Remote { logDir: string, featureSet: FeatureSet, cliAuth: CliAuth, - ) { + ): Promise> { let deploymentSSHConfig = {}; try { const deploymentConfig = await restClient.getDeploymentSSHConfig(); diff --git a/src/remote/userSettings.ts b/src/remote/sshOverrides.ts similarity index 99% rename from src/remote/userSettings.ts rename to src/remote/sshOverrides.ts index 7d82a3ed..55ccf018 100644 --- a/src/remote/userSettings.ts +++ b/src/remote/sshOverrides.ts @@ -75,7 +75,7 @@ export function buildSshOverrides( config: Pick, sshHost: string, agentOS: string, - remoteCommand?: string, + remoteCommand: string | undefined, ): SettingOverride[] { const overrides: SettingOverride[] = []; diff --git a/test/unit/remote/userSettings.test.ts b/test/unit/remote/sshOverrides.test.ts similarity index 93% rename from test/unit/remote/userSettings.test.ts rename to test/unit/remote/sshOverrides.test.ts index f38b3df7..102fb599 100644 --- a/test/unit/remote/userSettings.test.ts +++ b/test/unit/remote/sshOverrides.test.ts @@ -6,7 +6,7 @@ import { applySettingOverrides, buildSshOverrides, isActiveRemoteCommand, -} from "@/remote/userSettings"; +} from "@/remote/sshOverrides"; import { MockConfigurationProvider, @@ -54,7 +54,7 @@ describe("buildSshOverrides", () => { config.set("remote.SSH.remotePlatform", { "other-host": "darwin" }); expect( findOverride( - buildSshOverrides(config, "new-host", "linux"), + buildSshOverrides(config, "new-host", "linux", undefined), "remote.SSH.remotePlatform", ), ).toEqual({ "other-host": "darwin", "new-host": "linux" }); @@ -63,7 +63,7 @@ describe("buildSshOverrides", () => { config.set("remote.SSH.remotePlatform", { "my-host": "windows" }); expect( findOverride( - buildSshOverrides(config, "my-host", "linux"), + buildSshOverrides(config, "my-host", "linux", undefined), "remote.SSH.remotePlatform", ), ).toEqual({ "my-host": "linux" }); @@ -74,7 +74,7 @@ describe("buildSshOverrides", () => { config.set("remote.SSH.remotePlatform", { "my-host": "linux" }); expect( findOverride( - buildSshOverrides(config, "my-host", "linux"), + buildSshOverrides(config, "my-host", "linux", undefined), "remote.SSH.remotePlatform", ), ).toBeUndefined(); @@ -150,7 +150,7 @@ describe("buildSshOverrides", () => { } expect( findOverride( - buildSshOverrides(config, "host", "linux"), + buildSshOverrides(config, "host", "linux", undefined), "remote.SSH.connectTimeout", ), ).toBe(1800); @@ -164,7 +164,7 @@ describe("buildSshOverrides", () => { config.set("remote.SSH.connectTimeout", timeout); expect( findOverride( - buildSshOverrides(config, "host", "linux"), + buildSshOverrides(config, "host", "linux", undefined), "remote.SSH.connectTimeout", ), ).toBeUndefined(); @@ -175,7 +175,12 @@ describe("buildSshOverrides", () => { it("defaults to 8 hours when not configured", () => { expect( findOverride( - buildSshOverrides(new MockConfigurationProvider(), "host", "linux"), + buildSshOverrides( + new MockConfigurationProvider(), + "host", + "linux", + undefined, + ), "remote.SSH.reconnectionGraceTime", ), ).toBe(28800); @@ -186,7 +191,7 @@ describe("buildSshOverrides", () => { config.set("remote.SSH.reconnectionGraceTime", 3600); expect( findOverride( - buildSshOverrides(config, "host", "linux"), + buildSshOverrides(config, "host", "linux", undefined), "remote.SSH.reconnectionGraceTime", ), ).toBeUndefined(); @@ -201,6 +206,7 @@ describe("buildSshOverrides", () => { new MockConfigurationProvider(), "host", "linux", + undefined, ); expect(findOverride(overrides, key)).toBe(expected); }); @@ -212,7 +218,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), + ).toHaveLength(0); }); }); diff --git a/test/unit/remote/sshSupport.test.ts b/test/unit/remote/sshSupport.test.ts index 9e7b5059..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,19 @@ 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 --- + 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 @@ -89,48 +90,48 @@ 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"); -}); + ); + 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--* + 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"); -}); + ); + expect(props.RemoteCommand).toBe("none"); + }); -it("inherits RemoteCommand from a Host * block", () => { - const props = computeSshProperties( - "coder-vscode.example.com--user--ws", - `Host * + 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"); -}); + ); + expect(props.RemoteCommand).toBe("exec /bin/zsh -l"); + }); -it("handles RemoteCommand with = delimiter", () => { - const props = computeSshProperties( - "coder-vscode.example.com--user--ws", - `Host coder-vscode.example.com--* + 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"); -}); + ); + 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 * + it("properly escapes meaningful regex characters", () => { + const properties = computeSshProperties( + "coder-vscode.dev.coder.com--matalfi--dogfood", + `Host * StrictHostKeyChecking yes # ------------START-CODER----------- @@ -153,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", + }); }); }); From d2b46144ce114fe9f84a2c9474f937261d64cfcf Mon Sep 17 00:00:00 2001 From: Ehab Younes Date: Thu, 26 Mar 2026 16:02:55 +0300 Subject: [PATCH 3/3] Move RemoteCommand logging into buildSshOverrides Pass the logger into buildSshOverrides and log there, where the enableRemoteCommand setting is actually checked. This avoids exposing isActiveRemoteCommand, which alone does not account for the setting. --- src/remote/remote.ts | 12 +----- src/remote/sshOverrides.ts | 4 +- test/unit/remote/sshOverrides.test.ts | 62 ++++++++++++++++----------- 3 files changed, 43 insertions(+), 35 deletions(-) diff --git a/src/remote/remote.ts b/src/remote/remote.ts index e46fc837..e482725a 100644 --- a/src/remote/remote.ts +++ b/src/remote/remote.ts @@ -58,11 +58,7 @@ import { parseCoderSshOptions, parseSshConfig, } from "./sshConfig"; -import { - applySettingOverrides, - buildSshOverrides, - isActiveRemoteCommand, -} from "./sshOverrides"; +import { applySettingOverrides, buildSshOverrides } from "./sshOverrides"; import { SshProcessMonitor } from "./sshProcess"; import { computeSshProperties, sshSupportsSetEnv } from "./sshSupport"; import { WorkspaceStateMachine } from "./workspaceStateMachine"; @@ -484,11 +480,6 @@ export class Remote { } const remoteCommand = computedSshProperties.RemoteCommand; - if (isActiveRemoteCommand(remoteCommand)) { - this.logger.info( - "RemoteCommand detected, skipping remotePlatform override", - ); - } this.logger.info("Modifying settings..."); const overrides = buildSshOverrides( @@ -496,6 +487,7 @@ export class Remote { parts.sshHost, agent.operating_system, remoteCommand, + this.logger, ); if (overrides.length > 0) { const ok = await applySettingOverrides( diff --git a/src/remote/sshOverrides.ts b/src/remote/sshOverrides.ts index 55ccf018..65b13345 100644 --- a/src/remote/sshOverrides.ts +++ b/src/remote/sshOverrides.ts @@ -63,7 +63,7 @@ const AUTO_SETUP_DEFAULTS = { * Whether the given RemoteCommand value represents an active command * (i.e. present, non-empty, and not the SSH default "none"). */ -export function isActiveRemoteCommand(cmd: string | undefined): boolean { +function isActiveRemoteCommand(cmd: string | undefined): boolean { return !!cmd && cmd.toLowerCase() !== "none"; } @@ -76,6 +76,7 @@ export function buildSshOverrides( sshHost: string, agentOS: string, remoteCommand: string | undefined, + logger: Logger, ): SettingOverride[] { const overrides: SettingOverride[] = []; @@ -96,6 +97,7 @@ export function buildSshOverrides( {}, ); 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; diff --git a/test/unit/remote/sshOverrides.test.ts b/test/unit/remote/sshOverrides.test.ts index 102fb599..0c60117d 100644 --- a/test/unit/remote/sshOverrides.test.ts +++ b/test/unit/remote/sshOverrides.test.ts @@ -5,7 +5,6 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import { applySettingOverrides, buildSshOverrides, - isActiveRemoteCommand, } from "@/remote/sshOverrides"; import { @@ -33,19 +32,8 @@ interface TimeoutCase { label: string; } -describe("isActiveRemoteCommand", () => { - it.each(["exec bash -l", "exec /bin/zsh", "/usr/bin/tmux"])( - "returns true for %j", - (cmd) => expect(isActiveRemoteCommand(cmd)).toBe(true), - ); - - it.each([undefined, "", "none", "None", "NONE"])( - "returns false for %j", - (cmd) => expect(isActiveRemoteCommand(cmd)).toBe(false), - ); -}); - describe("buildSshOverrides", () => { + const buildLogger = createMockLogger(); describe("remote platform", () => { it("adds host when missing or OS differs", () => { const config = new MockConfigurationProvider(); @@ -54,7 +42,13 @@ describe("buildSshOverrides", () => { config.set("remote.SSH.remotePlatform", { "other-host": "darwin" }); expect( findOverride( - buildSshOverrides(config, "new-host", "linux", undefined), + buildSshOverrides( + config, + "new-host", + "linux", + undefined, + buildLogger, + ), "remote.SSH.remotePlatform", ), ).toEqual({ "other-host": "darwin", "new-host": "linux" }); @@ -63,7 +57,7 @@ describe("buildSshOverrides", () => { config.set("remote.SSH.remotePlatform", { "my-host": "windows" }); expect( findOverride( - buildSshOverrides(config, "my-host", "linux", undefined), + buildSshOverrides(config, "my-host", "linux", undefined, buildLogger), "remote.SSH.remotePlatform", ), ).toEqual({ "my-host": "linux" }); @@ -74,7 +68,7 @@ describe("buildSshOverrides", () => { config.set("remote.SSH.remotePlatform", { "my-host": "linux" }); expect( findOverride( - buildSshOverrides(config, "my-host", "linux", undefined), + buildSshOverrides(config, "my-host", "linux", undefined, buildLogger), "remote.SSH.remotePlatform", ), ).toBeUndefined(); @@ -90,7 +84,13 @@ describe("buildSshOverrides", () => { }); expect( findOverride( - buildSshOverrides(config, "my-host", "linux", "exec bash -l"), + buildSshOverrides( + config, + "my-host", + "linux", + "exec bash -l", + buildLogger, + ), "remote.SSH.remotePlatform", ), ).toEqual({ "other-host": "darwin" }); @@ -102,7 +102,13 @@ describe("buildSshOverrides", () => { config.set("remote.SSH.remotePlatform", {}); expect( findOverride( - buildSshOverrides(config, "my-host", "linux", "exec bash -l"), + buildSshOverrides( + config, + "my-host", + "linux", + "exec bash -l", + buildLogger, + ), "remote.SSH.remotePlatform", ), ).toBeUndefined(); @@ -114,7 +120,13 @@ describe("buildSshOverrides", () => { config.set("remote.SSH.remotePlatform", {}); expect( findOverride( - buildSshOverrides(config, "my-host", "linux", "exec bash -l"), + buildSshOverrides( + config, + "my-host", + "linux", + "exec bash -l", + buildLogger, + ), "remote.SSH.remotePlatform", ), ).toEqual({ "my-host": "linux" }); @@ -128,7 +140,7 @@ describe("buildSshOverrides", () => { config.set("remote.SSH.remotePlatform", {}); expect( findOverride( - buildSshOverrides(config, "my-host", "linux", cmd), + buildSshOverrides(config, "my-host", "linux", cmd, buildLogger), "remote.SSH.remotePlatform", ), ).toEqual({ "my-host": "linux" }); @@ -150,7 +162,7 @@ describe("buildSshOverrides", () => { } expect( findOverride( - buildSshOverrides(config, "host", "linux", undefined), + buildSshOverrides(config, "host", "linux", undefined, buildLogger), "remote.SSH.connectTimeout", ), ).toBe(1800); @@ -164,7 +176,7 @@ describe("buildSshOverrides", () => { config.set("remote.SSH.connectTimeout", timeout); expect( findOverride( - buildSshOverrides(config, "host", "linux", undefined), + buildSshOverrides(config, "host", "linux", undefined, buildLogger), "remote.SSH.connectTimeout", ), ).toBeUndefined(); @@ -180,6 +192,7 @@ describe("buildSshOverrides", () => { "host", "linux", undefined, + buildLogger, ), "remote.SSH.reconnectionGraceTime", ), @@ -191,7 +204,7 @@ describe("buildSshOverrides", () => { config.set("remote.SSH.reconnectionGraceTime", 3600); expect( findOverride( - buildSshOverrides(config, "host", "linux", undefined), + buildSshOverrides(config, "host", "linux", undefined, buildLogger), "remote.SSH.reconnectionGraceTime", ), ).toBeUndefined(); @@ -207,6 +220,7 @@ describe("buildSshOverrides", () => { "host", "linux", undefined, + buildLogger, ); expect(findOverride(overrides, key)).toBe(expected); }); @@ -219,7 +233,7 @@ describe("buildSshOverrides", () => { config.set("remote.SSH.serverShutdownTimeout", 600); config.set("remote.SSH.maxReconnectionAttempts", 4); expect( - buildSshOverrides(config, "my-host", "linux", undefined), + buildSshOverrides(config, "my-host", "linux", undefined, buildLogger), ).toHaveLength(0); }); });