Skip to content

Commit e595b54

Browse files
EhabYEhabY
authored
fix: skip remotePlatform when RemoteCommand is configured (#854)
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. Fixes #549
1 parent f49adcf commit e595b54

File tree

5 files changed

+261
-85
lines changed

5 files changed

+261
-85
lines changed

‎src/commands.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ import { maybeAskAgent, maybeAskUrl } from "./promptUtils";
2626
import {
2727
RECOMMENDED_SSH_SETTINGS,
2828
applySettingOverrides,
29-
} from "./remote/userSettings";
29+
} from "./remote/sshOverrides";
3030
import { getGlobalFlags, resolveCliAuth } from "./settings/cli";
3131
import { escapeCommandArg, toRemoteAuthority, toSafeHost } from "./util";
3232
import { vscodeProposed } from "./vscodeProposed";

‎src/remote/remote.ts‎

Lines changed: 35 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ import { getHeaderCommand } from "../settings/headers";
4545
import {
4646
AuthorityPrefix,
4747
escapeCommandArg,
48+
expandPath,
4849
parseRemoteAuthority,
4950
} from "../util";
5051
import { vscodeProposed } from "../vscodeProposed";
@@ -57,9 +58,9 @@ import {
5758
parseCoderSshOptions,
5859
parseSshConfig,
5960
} from "./sshConfig";
61+
import { applySettingOverrides, buildSshOverrides } from "./sshOverrides";
6062
import { SshProcessMonitor } from "./sshProcess";
6163
import { computeSshProperties, sshSupportsSetEnv } from "./sshSupport";
62-
import { applySettingOverrides, buildSshOverrides } from "./userSettings";
6364
import { WorkspaceStateMachine } from "./workspaceStateMachine";
6465

6566
export interface RemoteDetails extends vscode.Disposable {
@@ -459,33 +460,12 @@ export class Remote {
459460
const inbox = await Inbox.create(workspace, workspaceClient, this.logger);
460461
disposables.push(inbox);
461462

462-
this.logger.info("Modifying settings...");
463-
const overrides = buildSshOverrides(
464-
vscodeProposed.workspace.getConfiguration(),
465-
parts.sshHost,
466-
agent.operating_system,
467-
);
468-
if (overrides.length > 0) {
469-
const ok = await applySettingOverrides(
470-
this.pathResolver.getUserSettingsPath(),
471-
overrides,
472-
this.logger,
473-
);
474-
if (ok) {
475-
this.logger.info("Settings modified successfully");
476-
}
477-
}
478-
479463
const logDir = this.getLogDir(featureSet);
480464

481-
// This ensures the Remote SSH extension resolves the host to execute the
482-
// Coder binary properly.
483-
//
484-
// If we didn't write to the SSH config file, connecting would fail with
485-
// "Host not found".
465+
let computedSshProperties: Record<string, string> = {};
486466
try {
487467
this.logger.info("Updating SSH config...");
488-
await this.updateSSHConfig(
468+
computedSshProperties = await this.updateSSHConfig(
489469
workspaceClient,
490470
parts.safeHostname,
491471
parts.sshHost,
@@ -499,6 +479,27 @@ export class Remote {
499479
throw error;
500480
}
501481

482+
const remoteCommand = computedSshProperties.RemoteCommand;
483+
484+
this.logger.info("Modifying settings...");
485+
const overrides = buildSshOverrides(
486+
vscodeProposed.workspace.getConfiguration(),
487+
parts.sshHost,
488+
agent.operating_system,
489+
remoteCommand,
490+
this.logger,
491+
);
492+
if (overrides.length > 0) {
493+
const ok = await applySettingOverrides(
494+
this.pathResolver.getUserSettingsPath(),
495+
overrides,
496+
this.logger,
497+
);
498+
if (ok) {
499+
this.logger.info("Settings modified successfully");
500+
}
501+
}
502+
502503
// Monitor SSH process and display network status
503504
const sshMonitor = SshProcessMonitor.start({
504505
sshHost: parts.sshHost,
@@ -731,6 +732,13 @@ export class Remote {
731732
return ["--log-dir", escapeCommandArg(logDir), "-v"];
732733
}
733734

735+
private getSshConfigPath(): string {
736+
const configured = vscode.workspace
737+
.getConfiguration()
738+
.get<string>("remote.SSH.configFile");
739+
return expandPath(configured || path.join("~", ".ssh", "config"));
740+
}
741+
734742
// updateSSHConfig updates the SSH configuration with a wildcard that handles
735743
// all Coder entries.
736744
private async updateSSHConfig(
@@ -741,7 +749,7 @@ export class Remote {
741749
logDir: string,
742750
featureSet: FeatureSet,
743751
cliAuth: CliAuth,
744-
) {
752+
): Promise<Record<string, string>> {
745753
let deploymentSSHConfig = {};
746754
try {
747755
const deploymentConfig = await restClient.getDeploymentSSHConfig();
@@ -761,17 +769,7 @@ export class Remote {
761769
}
762770
}
763771

764-
let sshConfigFile = vscode.workspace
765-
.getConfiguration()
766-
.get<string>("remote.SSH.configFile");
767-
if (!sshConfigFile) {
768-
sshConfigFile = path.join(os.homedir(), ".ssh", "config");
769-
}
770-
// VS Code Remote resolves ~ to the home directory.
771-
// This is required for the tilde to work on Windows.
772-
if (sshConfigFile.startsWith("~")) {
773-
sshConfigFile = path.join(os.homedir(), sshConfigFile.slice(1));
774-
}
772+
const sshConfigFile = this.getSshConfigPath();
775773

776774
const sshConfig = new SshConfig(sshConfigFile, this.logger);
777775
await sshConfig.load();
@@ -852,7 +850,7 @@ export class Remote {
852850
throw new Error("SSH config mismatch, closing remote");
853851
}
854852

855-
return sshConfig.getRaw();
853+
return computedProperties;
856854
}
857855

858856
private watchSettings(

src/remote/userSettings.ts renamed to src/remote/sshOverrides.ts

Lines changed: 40 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,14 @@ const AUTO_SETUP_DEFAULTS = {
5959
"remote.SSH.maxReconnectionAttempts": null, // max allowed
6060
} as const satisfies Partial<Record<SshSettingKey, number | null>>;
6161

62+
/**
63+
* Whether the given RemoteCommand value represents an active command
64+
* (i.e. present, non-empty, and not the SSH default "none").
65+
*/
66+
function isActiveRemoteCommand(cmd: string | undefined): boolean {
67+
return !!cmd && cmd.toLowerCase() !== "none";
68+
}
69+
6270
/**
6371
* Build the list of VS Code setting overrides needed for a remote SSH
6472
* connection to a Coder workspace.
@@ -67,19 +75,45 @@ export function buildSshOverrides(
6775
config: Pick<WorkspaceConfiguration, "get">,
6876
sshHost: string,
6977
agentOS: string,
78+
remoteCommand: string | undefined,
79+
logger: Logger,
7080
): SettingOverride[] {
7181
const overrides: SettingOverride[] = [];
7282

73-
// Set the remote platform for this host to bypass the platform prompt.
83+
// When enableRemoteCommand is true and the host has an active
84+
// RemoteCommand, we must not set remotePlatform: it causes VS Code
85+
// to append 'bash', which conflicts with RemoteCommand. We gate on
86+
// enableRemoteCommand so users who haven't opted in don't get an
87+
// unexpected platform prompt.
88+
const enableRemoteCommand = config.get<boolean>(
89+
"remote.SSH.enableRemoteCommand",
90+
false,
91+
);
92+
const skipRemotePlatform =
93+
enableRemoteCommand && isActiveRemoteCommand(remoteCommand);
94+
7495
const remotePlatforms = config.get<Record<string, string>>(
7596
"remote.SSH.remotePlatform",
7697
{},
7798
);
78-
if (remotePlatforms[sshHost] !== agentOS) {
79-
overrides.push({
80-
key: "remote.SSH.remotePlatform",
81-
value: { ...remotePlatforms, [sshHost]: agentOS },
82-
});
99+
if (skipRemotePlatform) {
100+
logger.info("RemoteCommand detected, skipping remotePlatform override");
101+
// Remove any stale entry so it doesn't block RemoteCommand.
102+
if (sshHost in remotePlatforms) {
103+
const { [sshHost]: _removed, ...rest } = remotePlatforms;
104+
overrides.push({
105+
key: "remote.SSH.remotePlatform",
106+
value: rest,
107+
});
108+
}
109+
} else {
110+
// Set the remote platform to bypass the platform prompt.
111+
if (remotePlatforms[sshHost] !== agentOS) {
112+
overrides.push({
113+
key: "remote.SSH.remotePlatform",
114+
value: { ...remotePlatforms, [sshHost]: agentOS },
115+
});
116+
}
83117
}
84118

85119
// Default 15s is too short for startup scripts; enforce a minimum.

test/unit/remote/userSettings.test.ts renamed to test/unit/remote/sshOverrides.test.ts

Lines changed: 100 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import { beforeEach, describe, expect, it, vi } from "vitest";
55
import {
66
applySettingOverrides,
77
buildSshOverrides,
8-
} from "@/remote/userSettings";
8+
} from "@/remote/sshOverrides";
99

1010
import {
1111
MockConfigurationProvider,
@@ -33,6 +33,7 @@ interface TimeoutCase {
3333
}
3434

3535
describe("buildSshOverrides", () => {
36+
const buildLogger = createMockLogger();
3637
describe("remote platform", () => {
3738
it("adds host when missing or OS differs", () => {
3839
const config = new MockConfigurationProvider();
@@ -41,7 +42,13 @@ describe("buildSshOverrides", () => {
4142
config.set("remote.SSH.remotePlatform", { "other-host": "darwin" });
4243
expect(
4344
findOverride(
44-
buildSshOverrides(config, "new-host", "linux"),
45+
buildSshOverrides(
46+
config,
47+
"new-host",
48+
"linux",
49+
undefined,
50+
buildLogger,
51+
),
4552
"remote.SSH.remotePlatform",
4653
),
4754
).toEqual({ "other-host": "darwin", "new-host": "linux" });
@@ -50,7 +57,7 @@ describe("buildSshOverrides", () => {
5057
config.set("remote.SSH.remotePlatform", { "my-host": "windows" });
5158
expect(
5259
findOverride(
53-
buildSshOverrides(config, "my-host", "linux"),
60+
buildSshOverrides(config, "my-host", "linux", undefined, buildLogger),
5461
"remote.SSH.remotePlatform",
5562
),
5663
).toEqual({ "my-host": "linux" });
@@ -61,11 +68,85 @@ describe("buildSshOverrides", () => {
6168
config.set("remote.SSH.remotePlatform", { "my-host": "linux" });
6269
expect(
6370
findOverride(
64-
buildSshOverrides(config, "my-host", "linux"),
71+
buildSshOverrides(config, "my-host", "linux", undefined, buildLogger),
6572
"remote.SSH.remotePlatform",
6673
),
6774
).toBeUndefined();
6875
});
76+
77+
describe("RemoteCommand compatibility", () => {
78+
it("removes host from remotePlatform when enableRemoteCommand is true", () => {
79+
const config = new MockConfigurationProvider();
80+
config.set("remote.SSH.enableRemoteCommand", true);
81+
config.set("remote.SSH.remotePlatform", {
82+
"my-host": "linux",
83+
"other-host": "darwin",
84+
});
85+
expect(
86+
findOverride(
87+
buildSshOverrides(
88+
config,
89+
"my-host",
90+
"linux",
91+
"exec bash -l",
92+
buildLogger,
93+
),
94+
"remote.SSH.remotePlatform",
95+
),
96+
).toEqual({ "other-host": "darwin" });
97+
});
98+
99+
it("produces no override when host has no stale remotePlatform entry", () => {
100+
const config = new MockConfigurationProvider();
101+
config.set("remote.SSH.enableRemoteCommand", true);
102+
config.set("remote.SSH.remotePlatform", {});
103+
expect(
104+
findOverride(
105+
buildSshOverrides(
106+
config,
107+
"my-host",
108+
"linux",
109+
"exec bash -l",
110+
buildLogger,
111+
),
112+
"remote.SSH.remotePlatform",
113+
),
114+
).toBeUndefined();
115+
});
116+
117+
it("sets platform normally when enableRemoteCommand is false", () => {
118+
const config = new MockConfigurationProvider();
119+
config.set("remote.SSH.enableRemoteCommand", false);
120+
config.set("remote.SSH.remotePlatform", {});
121+
expect(
122+
findOverride(
123+
buildSshOverrides(
124+
config,
125+
"my-host",
126+
"linux",
127+
"exec bash -l",
128+
buildLogger,
129+
),
130+
"remote.SSH.remotePlatform",
131+
),
132+
).toEqual({ "my-host": "linux" });
133+
});
134+
135+
it.each(["none", "None", "NONE", "", undefined])(
136+
"sets platform normally when remoteCommand is %j",
137+
(cmd) => {
138+
const config = new MockConfigurationProvider();
139+
config.set("remote.SSH.enableRemoteCommand", true);
140+
config.set("remote.SSH.remotePlatform", {});
141+
expect(
142+
findOverride(
143+
buildSshOverrides(config, "my-host", "linux", cmd, buildLogger),
144+
"remote.SSH.remotePlatform",
145+
),
146+
).toEqual({ "my-host": "linux" });
147+
},
148+
);
149+
});
69150
});
70151

71152
describe("connect timeout", () => {
@@ -81,7 +162,7 @@ describe("buildSshOverrides", () => {
81162
}
82163
expect(
83164
findOverride(
84-
buildSshOverrides(config, "host", "linux"),
165+
buildSshOverrides(config, "host", "linux", undefined, buildLogger),
85166
"remote.SSH.connectTimeout",
86167
),
87168
).toBe(1800);
@@ -95,7 +176,7 @@ describe("buildSshOverrides", () => {
95176
config.set("remote.SSH.connectTimeout", timeout);
96177
expect(
97178
findOverride(
98-
buildSshOverrides(config, "host", "linux"),
179+
buildSshOverrides(config, "host", "linux", undefined, buildLogger),
99180
"remote.SSH.connectTimeout",
100181
),
101182
).toBeUndefined();
@@ -106,7 +187,13 @@ describe("buildSshOverrides", () => {
106187
it("defaults to 8 hours when not configured", () => {
107188
expect(
108189
findOverride(
109-
buildSshOverrides(new MockConfigurationProvider(), "host", "linux"),
190+
buildSshOverrides(
191+
new MockConfigurationProvider(),
192+
"host",
193+
"linux",
194+
undefined,
195+
buildLogger,
196+
),
110197
"remote.SSH.reconnectionGraceTime",
111198
),
112199
).toBe(28800);
@@ -117,7 +204,7 @@ describe("buildSshOverrides", () => {
117204
config.set("remote.SSH.reconnectionGraceTime", 3600);
118205
expect(
119206
findOverride(
120-
buildSshOverrides(config, "host", "linux"),
207+
buildSshOverrides(config, "host", "linux", undefined, buildLogger),
121208
"remote.SSH.reconnectionGraceTime",
122209
),
123210
).toBeUndefined();
@@ -132,6 +219,8 @@ describe("buildSshOverrides", () => {
132219
new MockConfigurationProvider(),
133220
"host",
134221
"linux",
222+
undefined,
223+
buildLogger,
135224
);
136225
expect(findOverride(overrides, key)).toBe(expected);
137226
});
@@ -143,7 +232,9 @@ describe("buildSshOverrides", () => {
143232
config.set("remote.SSH.reconnectionGraceTime", 7200);
144233
config.set("remote.SSH.serverShutdownTimeout", 600);
145234
config.set("remote.SSH.maxReconnectionAttempts", 4);
146-
expect(buildSshOverrides(config, "my-host", "linux")).toHaveLength(0);
235+
expect(
236+
buildSshOverrides(config, "my-host", "linux", undefined, buildLogger),
237+
).toHaveLength(0);
147238
});
148239
});
149240

0 commit comments

Comments
 (0)