Skip to content

Commit 1d8e441

Browse files
committed
Fix SetEnv SSH config parsing and accumulation
- Add parseSshConfig function to correctly parse "SetEnv MY_VAR=value" (previously split on first "=" producing incorrect key/value) - Modify mergeSshConfigValues to concatenate SetEnv values instead of replacing, preserving the default CODER_SSH_SESSION_TYPE - Ignore empty SetEnv values to prevent accidentally clearing defaults
1 parent e7c06ec commit 1d8e441

File tree

5 files changed

+214
-31
lines changed

5 files changed

+214
-31
lines changed

CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,10 @@
22

33
## Unreleased
44

5+
### Fixed
6+
7+
- Fixed `SetEnv` SSH config parsing and accumulation with user-defined values.
8+
59
## [v1.11.6](https://github.com/coder/vscode-coder/releases/tag/v1.11.6) 2025-12-15
610

711
### Added

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@
3636
"title": "Coder",
3737
"properties": {
3838
"coder.sshConfig": {
39-
"markdownDescription": "These values will be included in the ssh config file. Eg: `'ConnectTimeout=10'` will set the timeout to 10 seconds. Any values included here will override anything provided by default or by the deployment. To unset a value that is written by default, set the value to the empty string, Eg: `'ConnectTimeout='` will unset it.",
39+
"markdownDescription": "These values will be included in the ssh config file. Eg: `'ConnectTimeout=10'` will set the timeout to 10 seconds. Any values included here will override anything provided by default or by the deployment. To unset a value that is written by default, set the value to the empty string, Eg: `'ConnectTimeout='` will unset it.\n\nNote: `SetEnv` values are accumulated across multiple entries and cannot be unset.",
4040
"type": "array",
4141
"items": {
4242
"title": "SSH Config Value",

src/remote/remote.ts

Lines changed: 9 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,12 @@ import {
4040
} from "../util";
4141
import { WorkspaceMonitor } from "../workspace/workspaceMonitor";
4242

43-
import { SSHConfig, type SSHValues, mergeSSHConfigValues } from "./sshConfig";
43+
import {
44+
SSHConfig,
45+
type SSHValues,
46+
mergeSshConfigValues,
47+
parseSshConfig as parseSshConfig,
48+
} from "./sshConfig";
4449
import { SshProcessMonitor } from "./sshProcess";
4550
import { computeSSHProperties, sshSupportsSetEnv } from "./sshSupport";
4651
import { WorkspaceStateMachine } from "./workspaceStateMachine";
@@ -717,29 +722,11 @@ export class Remote {
717722

718723
// deploymentConfig is now set from the remote coderd deployment.
719724
// Now override with the user's config.
720-
const userConfigSSH =
725+
const userConfigSsh =
721726
vscode.workspace.getConfiguration("coder").get<string[]>("sshConfig") ||
722727
[];
723-
// Parse the user's config into a Record<string, string>.
724-
const userConfig = userConfigSSH.reduce(
725-
(acc, line) => {
726-
let i = line.indexOf("=");
727-
if (i === -1) {
728-
i = line.indexOf(" ");
729-
if (i === -1) {
730-
// This line is malformed. The setting is incorrect, and does not match
731-
// the pattern regex in the settings schema.
732-
return acc;
733-
}
734-
}
735-
const key = line.slice(0, i);
736-
const value = line.slice(i + 1);
737-
acc[key] = value;
738-
return acc;
739-
},
740-
{} as Record<string, string>,
741-
);
742-
const sshConfigOverrides = mergeSSHConfigValues(
728+
const userConfig = parseSshConfig(userConfigSsh);
729+
const sshConfigOverrides = mergeSshConfigValues(
743730
deploymentSSHConfig,
744731
userConfig,
745732
);

src/remote/sshConfig.ts

Lines changed: 57 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -36,10 +36,48 @@ const defaultFileSystem: FileSystem = {
3636
writeFile,
3737
};
3838

39+
/**
40+
* Parse an array of SSH config lines into a Record.
41+
* Handles both "Key value" and "Key=value" formats.
42+
* Accumulates SetEnv values since SSH allows multiple environment variables.
43+
*/
44+
export function parseSshConfig(lines: string[]): Record<string, string> {
45+
return lines.reduce(
46+
(acc, line) => {
47+
// Match key pattern (same as VS Code settings: ^[a-zA-Z0-9-]+)
48+
const keyMatch = line.match(/^[a-zA-Z0-9-]+/);
49+
if (!keyMatch) {
50+
return acc; // Malformed line
51+
}
52+
53+
const key = keyMatch[0];
54+
const separator = line[key.length];
55+
if (separator !== "=" && separator !== " ") {
56+
return acc; // Malformed line
57+
}
58+
59+
const value = line.slice(key.length + 1);
60+
61+
// Accumulate SetEnv values since there can be multiple.
62+
if (key.toLowerCase() === "setenv") {
63+
// Ignore empty SetEnv values
64+
if (value !== "") {
65+
const existing = acc["SetEnv"];
66+
acc["SetEnv"] = existing ? `${existing} ${value}` : ` ${value}`;
67+
}
68+
} else {
69+
acc[key] = value;
70+
}
71+
return acc;
72+
},
73+
{} as Record<string, string>,
74+
);
75+
}
76+
3977
// mergeSSHConfigValues will take a given ssh config and merge it with the overrides
4078
// provided. The merge handles key case insensitivity, so casing in the "key" does
4179
// not matter.
42-
export function mergeSSHConfigValues(
80+
export function mergeSshConfigValues(
4381
config: Record<string, string>,
4482
overrides: Record<string, string>,
4583
): Record<string, string> {
@@ -62,11 +100,17 @@ export function mergeSSHConfigValues(
62100
const value = overrides[correctCaseKey];
63101
delete caseInsensitiveOverrides[lower];
64102

65-
// If the value is empty, do not add the key. It is being removed.
66-
if (value === "") {
103+
// Special handling for SetEnv - concatenate values instead of replacing.
104+
if (lower === "setenv") {
105+
merged["SetEnv"] = `${config[key]}${value}`;
67106
return;
68107
}
69-
merged[correctCaseKey] = value;
108+
109+
// If the value is empty, do not add the key. It is being removed.
110+
if (value !== "") {
111+
merged[correctCaseKey] = value;
112+
}
113+
70114
return;
71115
}
72116
// If no override, take the original value.
@@ -78,7 +122,14 @@ export function mergeSSHConfigValues(
78122
// Add remaining overrides.
79123
Object.keys(caseInsensitiveOverrides).forEach((lower) => {
80124
const correctCaseKey = caseInsensitiveOverrides[lower];
81-
merged[correctCaseKey] = overrides[correctCaseKey];
125+
const value = overrides[correctCaseKey];
126+
127+
// Special handling for SetEnv - concatenate if already exists
128+
if (lower === "setenv" && merged["SetEnv"]) {
129+
merged["SetEnv"] = `${merged["SetEnv"]}${value}`;
130+
} else {
131+
merged[correctCaseKey] = value;
132+
}
82133
});
83134

84135
return merged;
@@ -203,7 +254,7 @@ export class SSHConfig {
203254
const lines = [this.startBlockComment(label), `Host ${Host}`];
204255

205256
// configValues is the merged values of the defaults and the overrides.
206-
const configValues = mergeSSHConfigValues(otherValues, overrides || {});
257+
const configValues = mergeSshConfigValues(otherValues, overrides || {});
207258

208259
// keys is the sorted keys of the merged values.
209260
const keys = (

test/unit/remote/sshConfig.test.ts

Lines changed: 143 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
1-
import { it, afterEach, vi, expect } from "vitest";
1+
import { it, afterEach, vi, expect, describe } from "vitest";
22

3-
import { SSHConfig } from "@/remote/sshConfig";
3+
import {
4+
SSHConfig,
5+
parseSshConfig,
6+
mergeSshConfigValues,
7+
} from "@/remote/sshConfig";
48

59
// This is not the usual path to ~/.ssh/config, but
610
// setting it to a different path makes it easier to test
@@ -712,3 +716,140 @@ it("fails if we are unable to rename the temporary file", async () => {
712716
}),
713717
).rejects.toThrow(/Failed to rename temporary SSH config file.*EACCES/);
714718
});
719+
720+
describe("parseSshConfig", () => {
721+
type ParseTest = {
722+
name: string;
723+
input: string[];
724+
expected: Record<string, string>;
725+
};
726+
727+
it.each<ParseTest>([
728+
{
729+
name: "space separator",
730+
input: ["Key value"],
731+
expected: { Key: "value" },
732+
},
733+
{
734+
name: "equals separator",
735+
input: ["Key=value"],
736+
expected: { Key: "value" },
737+
},
738+
{
739+
name: "SetEnv with space",
740+
input: ["SetEnv MY_VAR=value OTHER_VAR=othervalue"],
741+
expected: { SetEnv: " MY_VAR=value OTHER_VAR=othervalue" },
742+
},
743+
{
744+
name: "SetEnv with equals",
745+
input: ["SetEnv=MY_VAR=value OTHER_VAR=othervalue"],
746+
expected: { SetEnv: " MY_VAR=value OTHER_VAR=othervalue" },
747+
},
748+
{
749+
name: "accumulates SetEnv entries",
750+
input: ["SetEnv A=1", "setenv B=2 C=3"],
751+
expected: { SetEnv: " A=1 B=2 C=3" },
752+
},
753+
{
754+
name: "skips malformed lines",
755+
input: ["malformed", "# comment", "key=value", " indented"],
756+
expected: { key: "value" },
757+
},
758+
{
759+
name: "value with spaces",
760+
input: ["ProxyCommand ssh -W %h:%p proxy"],
761+
expected: { ProxyCommand: "ssh -W %h:%p proxy" },
762+
},
763+
{
764+
name: "quoted value with spaces",
765+
input: ['SetEnv key="Hello world"'],
766+
expected: { SetEnv: ' key="Hello world"' },
767+
},
768+
{
769+
name: "multiple keys",
770+
input: ["ConnectTimeout 10", "LogLevel=DEBUG", "SetEnv VAR=1"],
771+
expected: { ConnectTimeout: "10", LogLevel: "DEBUG", SetEnv: " VAR=1" },
772+
},
773+
{
774+
name: "ignores empty SetEnv",
775+
input: ["SetEnv=", "SetEnv "],
776+
expected: {},
777+
},
778+
])("$name", ({ input, expected }) => {
779+
expect(parseSshConfig(input)).toEqual(expected);
780+
});
781+
});
782+
783+
describe("mergeSshConfigValues", () => {
784+
type MergeTest = {
785+
name: string;
786+
config: Record<string, string>;
787+
overrides: Record<string, string>;
788+
expected: Record<string, string>;
789+
};
790+
791+
it.each<MergeTest>([
792+
{
793+
name: "overrides case-insensitively",
794+
config: { LogLevel: "ERROR" },
795+
overrides: { loglevel: "DEBUG" },
796+
expected: { loglevel: "DEBUG" },
797+
},
798+
{
799+
name: "removes keys with empty string",
800+
config: { LogLevel: "ERROR", Foo: "bar" },
801+
overrides: { LogLevel: "" },
802+
expected: { Foo: "bar" },
803+
},
804+
{
805+
name: "adds new keys from overrides",
806+
config: { LogLevel: "ERROR" },
807+
overrides: { NewKey: "value" },
808+
expected: { LogLevel: "ERROR", NewKey: "value" },
809+
},
810+
{
811+
name: "preserves keys not in overrides",
812+
config: { A: "1", B: "2" },
813+
overrides: { B: "3" },
814+
expected: { A: "1", B: "3" },
815+
},
816+
{
817+
name: "concatenates SetEnv values",
818+
config: { SetEnv: " A=1" },
819+
overrides: { SetEnv: " B=2" },
820+
expected: { SetEnv: " A=1 B=2" },
821+
},
822+
{
823+
name: "concatenates SetEnv case-insensitively",
824+
config: { SetEnv: " A=1" },
825+
overrides: { setenv: " B=2" },
826+
expected: { SetEnv: " A=1 B=2" },
827+
},
828+
{
829+
name: "SetEnv only in override",
830+
config: {},
831+
overrides: { SetEnv: " B=2" },
832+
expected: { SetEnv: " B=2" },
833+
},
834+
{
835+
name: "SetEnv only in config",
836+
config: { SetEnv: " A=1" },
837+
overrides: {},
838+
expected: { SetEnv: " A=1" },
839+
},
840+
{
841+
name: "SetEnv with other values",
842+
config: { SetEnv: " A=1", LogLevel: "ERROR" },
843+
overrides: { SetEnv: " B=2", Timeout: "10" },
844+
expected: { SetEnv: " A=1 B=2", LogLevel: "ERROR", Timeout: "10" },
845+
},
846+
{
847+
name: "ignores empty SetEnv override",
848+
config: { SetEnv: " A=1 B=2" },
849+
overrides: { SetEnv: "" },
850+
expected: { SetEnv: " A=1 B=2" },
851+
},
852+
])("$name", ({ config, overrides, expected }) => {
853+
expect(mergeSshConfigValues(config, overrides)).toEqual(expected);
854+
});
855+
});

0 commit comments

Comments
 (0)