fix: pass proxy environment through to ssh

Author: EhabY

src/api/proxy.ts

@@ -12,6 +12,18 @@ const DEFAULT_PORTS: Record<string, number> = {
 	wss: 443,
 };
 
+/** Join a no-proxy list into a comma string, dropping blanks. */
+export function joinNoProxy(
+	entries: string[] | null | undefined,
+): string | undefined {
+	return (
+		entries
+			?.map((entry) => entry.trim())
+			.filter(Boolean)
+			.join(",") || undefined
+	);
+}
+
 /**
  * @param {string|object} url - The URL, or the result from url.parse.
  * @param {string} httpProxy - The proxy URL to use.

src/api/utils.ts

@@ -4,7 +4,7 @@ import { type WorkspaceConfiguration } from "vscode";
 
 import { expandPath } from "../util";
 
-import { getProxyForUrl } from "./proxy";
+import { getProxyForUrl, joinNoProxy } from "./proxy";
 
 /**
  * Return whether the API will need a token for authorization.
@@ -56,7 +56,7 @@ export async function createHttpAgent(
 				url,
 				cfg.get("http.proxy"),
 				cfg.get("coder.proxyBypass"),
-				httpNoProxy?.map((noProxy) => noProxy.trim())?.join(","),
+				joinNoProxy(httpNoProxy),
 			);
 		},
 		headers,

src/core/dismissibleNotifier.ts

@@ -5,12 +5,8 @@ import type { Memento } from "vscode";
 const DISMISS = "Don't Show Again";
 
 /** globalState keys under which "Don't Show Again" dismissals are stored. */
-export const DISMISSIBLE_NOTIFICATION_KEYS = [
-	"coder.proxyUseLocalServerWarningDismissed",
-] as const;
-
 export type DismissibleNotificationKey =
-	(typeof DISMISSIBLE_NOTIFICATION_KEYS)[number];
+	"coder.proxyUseLocalServerWarningDismissed";
 
 export class DismissibleNotifier {
 	public constructor(private readonly globalState: Memento) {}

src/remote/environment.ts

@@ -1,4 +1,4 @@
-import { getProxyForUrl } from "../api/proxy";
+import { joinNoProxy } from "../api/proxy";
 
 import type { WorkspaceConfiguration } from "vscode";
 
@@ -30,34 +30,31 @@ export const SSH_PROXY_SETTINGS: ReadonlyArray<{
  * to disk and multiple windows onto the same workspace stay independent.
  *
  * Best-effort: only processes spawned afterwards inherit the change. MS VS Code
- * with `remote.SSH.useLocalServer=false` spawns ssh off a path that does not
- * inherit, so propagation there needs `useLocalServer=true`. Returns a disposable
- * that restores the previous values.
+ * with `remote.SSH.useLocalServer=false` spawns ssh through a code path that
+ * does not inherit, so propagation there needs `useLocalServer=true`. Returns a
+ * disposable that restores the previous values.
  */
 export function applySshEnvironment(
-	baseUrl: string,
 	cfg: Pick<WorkspaceConfiguration, "get">,
 	env: Environment = process.env,
 ): { dispose(): void } {
-	return applyEnvironment(getSshProxyEnvironment(baseUrl, cfg), env);
+	return applyEnvironment(getSshProxyEnvironment(cfg), env);
 }
 
 /**
  * The proxy portion of the SSH environment. Exposed so callers can check whether
- * a proxy actually applies to a deployment via `.HTTP_PROXY`.
+ * proxy settings are configured via `.HTTP_PROXY`.
  */
 export function getSshProxyEnvironment(
-	baseUrl: string,
 	cfg: Pick<WorkspaceConfiguration, "get">,
 ): SshEnvironment {
-	const httpProxy = getSetting(cfg, "http.proxy");
-	const noProxy = getSetting(cfg, "coder.proxyBypass") ?? getHttpNoProxy(cfg);
-	const proxy = httpProxy
-		? getProxyForUrl(baseUrl, httpProxy, noProxy, undefined)
-		: "";
+	const httpProxy = trimmed(cfg.get<string | null>("http.proxy"));
+	const noProxy =
+		trimmed(cfg.get<string | null>("coder.proxyBypass")) ??
+		joinNoProxy(cfg.get<string[]>("http.noProxy"));
 
 	return {
-		...(proxy ? { HTTP_PROXY: proxy, HTTPS_PROXY: proxy } : {}),
+		...(httpProxy ? { HTTP_PROXY: httpProxy, HTTPS_PROXY: httpProxy } : {}),
 		...(noProxy ? { NO_PROXY: noProxy } : {}),
 	};
 }
@@ -71,10 +68,9 @@ function applyEnvironment(
 		if (value === undefined) {
 			continue;
 		}
-		for (const envKey of getEnvKeys(env, key)) {
-			previous.push([envKey, Object.hasOwn(env, envKey), env[envKey]]);
-			env[envKey] = value;
-		}
+		const previousValue = env[key];
+		previous.push([key, previousValue !== undefined, previousValue]);
+		env[key] = value;
 	}
 
 	let disposed = false;
@@ -84,8 +80,7 @@ function applyEnvironment(
 				return;
 			}
 			disposed = true;
-			for (let i = previous.length - 1; i >= 0; i--) {
-				const [key, existed, value] = previous[i];
+			for (const [key, existed, value] of previous) {
 				if (existed) {
 					env[key] = value;
 				} else {
@@ -96,29 +91,6 @@ function applyEnvironment(
 	};
 }
 
-function getEnvKeys(env: Environment, key: string): string[] {
-	const keys = Object.keys(env).filter(
-		(envKey) => envKey.toLowerCase() === key.toLowerCase(),
-	);
-	return keys.length > 0 ? keys : [key];
-}
-
-function getSetting(
-	cfg: Pick<WorkspaceConfiguration, "get">,
-	setting: string,
-): string | undefined {
-	const value = cfg.get<string | null>(setting);
+function trimmed(value: string | null | undefined): string | undefined {
 	return typeof value === "string" ? value.trim() || undefined : undefined;
 }
-
-function getHttpNoProxy(
-	cfg: Pick<WorkspaceConfiguration, "get">,
-): string | undefined {
-	return (
-		cfg
-			.get<string[]>("http.noProxy", [])
-			.map((value) => value.trim())
-			.filter(Boolean)
-			.join(",") || undefined
-	);
-}

src/remote/remote.ts

@@ -213,9 +213,9 @@ export class Remote {
 
 		try {
 			disposables.push(
-				applySshEnvironment(baseUrl, vscode.workspace.getConfiguration()),
+				applySshEnvironment(vscode.workspace.getConfiguration()),
 			);
-			await this.warnIfProxyEnvNotInherited(baseUrl);
+			await this.warnIfProxyEnvNotInherited();
 			// Create OAuth session manager for this remote deployment
 			const remoteOAuthManager = OAuthSessionManager.create(
 				{ url: baseUrl, safeHostname: parts.safeHostname },
@@ -840,17 +840,17 @@ export class Remote {
 	/**
 	 * MS VS Code with `remote.SSH.useLocalServer=false` spawns ssh without
 	 * inheriting process.env, so the proxy variables never reach it. Warn once and
-	 * offer to enable the local server when a proxy applies to this deployment.
+	 * offer to enable the local server when proxy settings are configured.
 	 *
 	 * Blocks setup with a modal: the write must land before ssh spawns (which
 	 * happens after setup returns) for it to apply without a reload. Catches
 	 * internally so a failure here never aborts the connection.
 	 */
-	private async warnIfProxyEnvNotInherited(baseUrl: string): Promise<void> {
+	private async warnIfProxyEnvNotInherited(): Promise<void> {
 		try {
 			const cfg = vscode.workspace.getConfiguration();
-			// HTTP_PROXY is only set when a proxy actually applies to this deployment.
-			if (!getSshProxyEnvironment(baseUrl, cfg).HTTP_PROXY) {
+			// HTTP_PROXY is only set when http.proxy is configured.
+			if (!getSshProxyEnvironment(cfg).HTTP_PROXY) {
 				return;
 			}
 			if (cfg.get<boolean>("remote.SSH.useLocalServer") !== false) {

test/unit/remote/environment.test.ts

@@ -8,7 +8,6 @@ import {
 
 import { MockConfigurationProvider } from "../../mocks/testHelpers";
 
-const URL = "https://coder.example.com";
 const proxy = "http://proxy.example.com:8080";
 
 function setup() {
@@ -32,12 +31,16 @@ describe("getSshProxyEnvironment", () => {
 			expected: { HTTP_PROXY: proxy, HTTPS_PROXY: proxy },
 		},
 		{
-			name: "drops the proxy when the deployment is bypassed",
+			name: "passes through the proxy when the deployment is bypassed",
 			settings: {
 				"http.proxy": proxy,
 				"coder.proxyBypass": "coder.example.com",
 			},
-			expected: { NO_PROXY: "coder.example.com" },
+			expected: {
+				HTTP_PROXY: proxy,
+				HTTPS_PROXY: proxy,
+				NO_PROXY: "coder.example.com",
+			},
 		},
 		{
 			name: "falls back to http.noProxy when coder.proxyBypass is unset",
@@ -64,17 +67,35 @@ describe("getSshProxyEnvironment", () => {
 				NO_PROXY: "primary.example.com",
 			},
 		},
+		{
+			name: "ignores a whitespace-only http.proxy",
+			settings: { "http.proxy": "   " },
+			expected: {},
+		},
+		{
+			name: "falls back to http.noProxy when coder.proxyBypass is whitespace",
+			settings: {
+				"http.proxy": proxy,
+				"coder.proxyBypass": "   ",
+				"http.noProxy": ["fallback.example.com"],
+			},
+			expected: {
+				HTTP_PROXY: proxy,
+				HTTPS_PROXY: proxy,
+				NO_PROXY: "fallback.example.com",
+			},
+		},
 	])("$name", ({ settings, expected }) => {
 		const { config } = setup();
 
-		expect(getSshProxyEnvironment(URL, config(settings))).toEqual(expected);
+		expect(getSshProxyEnvironment(config(settings))).toEqual(expected);
 	});
 
 	it("ignores an existing env proxy when http.proxy is unset", () => {
 		const { config } = setup();
 		vi.stubEnv("HTTPS_PROXY", "http://env-proxy.example.com:8080");
 
-		expect(getSshProxyEnvironment(URL, config())).toEqual({});
+		expect(getSshProxyEnvironment(config())).toEqual({});
 	});
 });
 
@@ -84,7 +105,6 @@ describe("applySshEnvironment", () => {
 		const env: Record<string, string | undefined> = {};
 
 		const applied = applySshEnvironment(
-			URL,
 			config({
 				"http.proxy": proxy,
 				"coder.proxyBypass": "internal.example.com",
@@ -101,28 +121,42 @@ describe("applySshEnvironment", () => {
 		expect(env).toEqual({});
 	});
 
-	it("overwrites and restores existing lowercase variables", () => {
+	it("does not overwrite existing lowercase variables", () => {
 		const { config } = setup();
 		const original = {
 			http_proxy: "http://old-http-proxy.example.com:8080",
 			https_proxy: "http://old-https-proxy.example.com:8080",
 		};
 		const env: Record<string, string | undefined> = { ...original };
 
-		const applied = applySshEnvironment(
-			URL,
-			config({ "http.proxy": proxy }),
-			env,
-		);
-		expect(env).toEqual({ http_proxy: proxy, https_proxy: proxy });
+		const applied = applySshEnvironment(config({ "http.proxy": proxy }), env);
+		expect(env).toEqual({
+			...original,
+			HTTP_PROXY: proxy,
+			HTTPS_PROXY: proxy,
+		});
 
 		applied.dispose();
 		expect(env).toEqual(original);
 	});
 
+	it("restores existing case-insensitive variables", () => {
+		const { config } = setup();
+		const original = "http://old-http-proxy.example.com:8080";
+		const env = caseInsensitiveEnvironment({ http_proxy: original });
+
+		const applied = applySshEnvironment(config({ "http.proxy": proxy }), env);
+		expect(env.HTTP_PROXY).toBe(proxy);
+		expect(env.http_proxy).toBe(proxy);
+
+		applied.dispose();
+		expect(env.HTTP_PROXY).toBe(original);
+		expect(env.http_proxy).toBe(original);
+	});
+
 	it("propagates proxy variables to newly spawned child processes", () => {
 		const { config } = setup();
-		const applied = applySshEnvironment(URL, config({ "http.proxy": proxy }));
+		const applied = applySshEnvironment(config({ "http.proxy": proxy }));
 
 		try {
 			expect(getProxyEnvFromChild()).toEqual({ http: proxy, https: proxy });
@@ -132,6 +166,41 @@ describe("applySshEnvironment", () => {
 	});
 });
 
+function caseInsensitiveEnvironment(
+	values: Record<string, string>,
+): Record<string, string | undefined> {
+	return new Proxy(values, {
+		get(target, property) {
+			if (typeof property !== "string") {
+				return undefined;
+			}
+			return target[getCaseInsensitiveKey(target, property) ?? property];
+		},
+		set(target, property, value) {
+			if (typeof property !== "string") {
+				return false;
+			}
+			target[getCaseInsensitiveKey(target, property) ?? property] = value;
+			return true;
+		},
+		deleteProperty(target, property) {
+			if (typeof property !== "string") {
+				return false;
+			}
+			return delete target[getCaseInsensitiveKey(target, property) ?? property];
+		},
+	});
+}
+
+function getCaseInsensitiveKey(
+	values: Record<string, string | undefined>,
+	key: string,
+): string | undefined {
+	return Object.keys(values).find(
+		(valueKey) => valueKey.toLowerCase() === key.toLowerCase(),
+	);
+}
+
 function getProxyEnvFromChild(): { http: string; https: string } {
 	const result = spawnSync(
 		process.execPath,