diff --git a/CHANGELOG.md b/CHANGELOG.md index 6b7fb7ac..e9ab6790 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +### Fixed + +- Fixed `SetEnv` SSH config parsing and accumulation with user-defined values. + ## [v1.11.6](https://github.com/coder/vscode-coder/releases/tag/v1.11.6) 2025-12-15 ### Added diff --git a/package.json b/package.json index 2caac7bc..29b9d138 100644 --- a/package.json +++ b/package.json @@ -36,7 +36,7 @@ "title": "Coder", "properties": { "coder.sshConfig": { - "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.", + "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.", "type": "array", "items": { "title": "SSH Config Value", diff --git a/src/remote/remote.ts b/src/remote/remote.ts index 9aaea237..b5fedc51 100644 --- a/src/remote/remote.ts +++ b/src/remote/remote.ts @@ -40,7 +40,12 @@ import { } from "../util"; import { WorkspaceMonitor } from "../workspace/workspaceMonitor"; -import { SSHConfig, type SSHValues, mergeSSHConfigValues } from "./sshConfig"; +import { + SSHConfig, + type SSHValues, + mergeSshConfigValues, + parseSshConfig, +} from "./sshConfig"; import { SshProcessMonitor } from "./sshProcess"; import { computeSSHProperties, sshSupportsSetEnv } from "./sshSupport"; import { WorkspaceStateMachine } from "./workspaceStateMachine"; @@ -717,29 +722,11 @@ export class Remote { // deploymentConfig is now set from the remote coderd deployment. // Now override with the user's config. - const userConfigSSH = + const userConfigSsh = vscode.workspace.getConfiguration("coder").get("sshConfig") || []; - // Parse the user's config into a Record. - const userConfig = userConfigSSH.reduce( - (acc, line) => { - let i = line.indexOf("="); - if (i === -1) { - i = line.indexOf(" "); - if (i === -1) { - // This line is malformed. The setting is incorrect, and does not match - // the pattern regex in the settings schema. - return acc; - } - } - const key = line.slice(0, i); - const value = line.slice(i + 1); - acc[key] = value; - return acc; - }, - {} as Record, - ); - const sshConfigOverrides = mergeSSHConfigValues( + const userConfig = parseSshConfig(userConfigSsh); + const sshConfigOverrides = mergeSshConfigValues( deploymentSSHConfig, userConfig, ); @@ -782,7 +769,7 @@ export class Remote { if (sshSupportsSetEnv()) { // This allows for tracking the number of extension // users connected to workspaces! - sshValues.SetEnv = " CODER_SSH_SESSION_TYPE=vscode"; + sshValues.SetEnv = "CODER_SSH_SESSION_TYPE=vscode"; } await sshConfig.update(label, sshValues, sshConfigOverrides); diff --git a/src/remote/sshConfig.ts b/src/remote/sshConfig.ts index f5fea264..fc750925 100644 --- a/src/remote/sshConfig.ts +++ b/src/remote/sshConfig.ts @@ -36,10 +36,48 @@ const defaultFileSystem: FileSystem = { writeFile, }; +/** + * Parse an array of SSH config lines into a Record. + * Handles both "Key value" and "Key=value" formats. + * Accumulates SetEnv values since SSH allows multiple environment variables. + */ +export function parseSshConfig(lines: string[]): Record { + return lines.reduce( + (acc, line) => { + // Match key pattern (same as VS Code settings: ^[a-zA-Z0-9-]+) + const keyMatch = line.match(/^[a-zA-Z0-9-]+/); + if (!keyMatch) { + return acc; // Malformed line + } + + const key = keyMatch[0]; + const separator = line.at(key.length); + if (separator !== "=" && separator !== " ") { + return acc; // Malformed line + } + + const value = line.slice(key.length + 1); + + // Accumulate SetEnv values since there can be multiple. + if (key.toLowerCase() === "setenv") { + // Ignore empty SetEnv values + if (value !== "") { + const existing = acc["SetEnv"]; + acc["SetEnv"] = existing ? `${existing} ${value}` : value; + } + } else { + acc[key] = value; + } + return acc; + }, + {} as Record, + ); +} + // mergeSSHConfigValues will take a given ssh config and merge it with the overrides // provided. The merge handles key case insensitivity, so casing in the "key" does // not matter. -export function mergeSSHConfigValues( +export function mergeSshConfigValues( config: Record, overrides: Record, ): Record { @@ -62,11 +100,21 @@ export function mergeSSHConfigValues( const value = overrides[correctCaseKey]; delete caseInsensitiveOverrides[lower]; - // If the value is empty, do not add the key. It is being removed. - if (value === "") { + // Special handling for SetEnv - concatenate values instead of replacing. + if (lower === "setenv") { + if (value === "") { + merged["SetEnv"] = config[key]; + } else { + merged["SetEnv"] = `${config[key]} ${value}`; + } return; } - merged[correctCaseKey] = value; + + // If the value is empty, do not add the key. It is being removed. + if (value !== "") { + merged[correctCaseKey] = value; + } + return; } // If no override, take the original value. @@ -78,7 +126,14 @@ export function mergeSSHConfigValues( // Add remaining overrides. Object.keys(caseInsensitiveOverrides).forEach((lower) => { const correctCaseKey = caseInsensitiveOverrides[lower]; - merged[correctCaseKey] = overrides[correctCaseKey]; + const value = overrides[correctCaseKey]; + + // Special handling for SetEnv - concatenate if already exists + if (lower === "setenv" && merged["SetEnv"]) { + merged["SetEnv"] = `${merged["SetEnv"]} ${value}`; + } else { + merged[correctCaseKey] = value; + } }); return merged; @@ -203,7 +258,7 @@ export class SSHConfig { const lines = [this.startBlockComment(label), `Host ${Host}`]; // configValues is the merged values of the defaults and the overrides. - const configValues = mergeSSHConfigValues(otherValues, overrides || {}); + const configValues = mergeSshConfigValues(otherValues, overrides || {}); // keys is the sorted keys of the merged values. const keys = ( diff --git a/test/unit/remote/sshConfig.test.ts b/test/unit/remote/sshConfig.test.ts index cfc48c74..6b90c0b5 100644 --- a/test/unit/remote/sshConfig.test.ts +++ b/test/unit/remote/sshConfig.test.ts @@ -1,6 +1,10 @@ -import { it, afterEach, vi, expect } from "vitest"; +import { it, afterEach, vi, expect, describe } from "vitest"; -import { SSHConfig } from "@/remote/sshConfig"; +import { + SSHConfig, + parseSshConfig, + mergeSshConfigValues, +} from "@/remote/sshConfig"; // This is not the usual path to ~/.ssh/config, but // 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 () => { }), ).rejects.toThrow(/Failed to rename temporary SSH config file.*EACCES/); }); + +describe("parseSshConfig", () => { + type ParseTest = { + name: string; + input: string[]; + expected: Record; + }; + + it.each([ + { + name: "space separator", + input: ["Key value"], + expected: { Key: "value" }, + }, + { + name: "equals separator", + input: ["Key=value"], + expected: { Key: "value" }, + }, + { + name: "SetEnv with space", + input: ["SetEnv MY_VAR=value OTHER_VAR=othervalue"], + expected: { SetEnv: "MY_VAR=value OTHER_VAR=othervalue" }, + }, + { + name: "SetEnv with equals", + input: ["SetEnv=MY_VAR=value OTHER_VAR=othervalue"], + expected: { SetEnv: "MY_VAR=value OTHER_VAR=othervalue" }, + }, + { + name: "accumulates SetEnv entries", + input: ["SetEnv A=1", "setenv B=2 C=3"], + expected: { SetEnv: "A=1 B=2 C=3" }, + }, + { + name: "skips malformed lines", + input: ["malformed", "# comment", "key=value", " indented"], + expected: { key: "value" }, + }, + { + name: "value with spaces", + input: ["ProxyCommand ssh -W %h:%p proxy"], + expected: { ProxyCommand: "ssh -W %h:%p proxy" }, + }, + { + name: "quoted value with spaces", + input: ['SetEnv key="Hello world"'], + expected: { SetEnv: 'key="Hello world"' }, + }, + { + name: "multiple keys", + input: ["ConnectTimeout 10", "LogLevel=DEBUG", "SetEnv VAR=1"], + expected: { ConnectTimeout: "10", LogLevel: "DEBUG", SetEnv: "VAR=1" }, + }, + { + name: "ignores empty SetEnv", + input: ["SetEnv=", "SetEnv "], + expected: {}, + }, + ])("$name", ({ input, expected }) => { + expect(parseSshConfig(input)).toEqual(expected); + }); +}); + +describe("mergeSshConfigValues", () => { + type MergeTest = { + name: string; + config: Record; + overrides: Record; + expected: Record; + }; + + it.each([ + { + name: "overrides case-insensitively", + config: { LogLevel: "ERROR" }, + overrides: { loglevel: "DEBUG" }, + expected: { loglevel: "DEBUG" }, + }, + { + name: "removes keys with empty string", + config: { LogLevel: "ERROR", Foo: "bar" }, + overrides: { LogLevel: "" }, + expected: { Foo: "bar" }, + }, + { + name: "adds new keys from overrides", + config: { LogLevel: "ERROR" }, + overrides: { NewKey: "value" }, + expected: { LogLevel: "ERROR", NewKey: "value" }, + }, + { + name: "preserves keys not in overrides", + config: { A: "1", B: "2" }, + overrides: { B: "3" }, + expected: { A: "1", B: "3" }, + }, + { + name: "concatenates SetEnv values", + config: { SetEnv: "A=1" }, + overrides: { SetEnv: "B=2" }, + expected: { SetEnv: "A=1 B=2" }, + }, + { + name: "concatenates SetEnv case-insensitively", + config: { SetEnv: "A=1" }, + overrides: { setenv: "B=2" }, + expected: { SetEnv: "A=1 B=2" }, + }, + { + name: "SetEnv only in override", + config: {}, + overrides: { SetEnv: "B=2" }, + expected: { SetEnv: "B=2" }, + }, + { + name: "SetEnv only in config", + config: { SetEnv: "A=1" }, + overrides: {}, + expected: { SetEnv: "A=1" }, + }, + { + name: "SetEnv with other values", + config: { SetEnv: "A=1", LogLevel: "ERROR" }, + overrides: { SetEnv: "B=2", Timeout: "10" }, + expected: { SetEnv: "A=1 B=2", LogLevel: "ERROR", Timeout: "10" }, + }, + { + name: "ignores empty SetEnv override", + config: { SetEnv: "A=1 B=2" }, + overrides: { SetEnv: "" }, + expected: { SetEnv: "A=1 B=2" }, + }, + ])("$name", ({ config, overrides, expected }) => { + expect(mergeSshConfigValues(config, overrides)).toEqual(expected); + }); +});