diff --git a/CHANGELOG.md b/CHANGELOG.md index 06be9fd3e..9f35e5f82 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,23 @@ from published versions since it shows up in the VS Code extension changelog tab and is confusing to users. Add it back between releases if needed. --> +## Unreleased + +> **Breaking:** API requests now respect `http.proxySupport: off`. Previously +> the extension applied VS Code's proxy settings (`http.proxy`, `http.noProxy`, +> `coder.proxyBypass`) to API requests even when `http.proxySupport` was `off`. +> Now `off` ignores those settings and only proxy environment variables are +> used. If you set those settings and relied on them while proxy support was +> `off`, either set `http.proxySupport` to `on` to keep using them, or move the +> values into the `HTTP_PROXY`/`HTTPS_PROXY` (from `http.proxy`) and `NO_PROXY` +> (from `http.noProxy`/`coder.proxyBypass`) environment variables. + +### Fixed + +- Honor `http.proxySupport: off` when deriving proxy settings for SSH and API + connections, so VS Code's proxy settings are ignored while inherited proxy + environment variables still apply. + ## [v1.15.1](https://github.com/coder/vscode-coder/releases/tag/v1.15.1) 2026-06-26 ### Added diff --git a/package.json b/package.json index 08cfcf66d..da3524852 100644 --- a/package.json +++ b/package.json @@ -123,7 +123,8 @@ "scope": "machine" }, "coder.proxyBypass": { - "markdownDescription": "If not set, will inherit from the `no_proxy` or `NO_PROXY` environment variables. `http.proxySupport` must be set to `on` or `off`, otherwise VS Code will override the proxy agent set by the plugin.", + "markdownDescription": "If not set, will inherit from the `no_proxy` or `NO_PROXY` environment variables. Has no effect when `http.proxySupport` is set to `off`. With values other than `on`, VS Code will override the proxy agent set by the plugin.", + "markdownDeprecationMessage": "Deprecated: prefer `http.noProxy`.", "type": "string", "default": "", "scope": "machine" diff --git a/src/api/coderApi.ts b/src/api/coderApi.ts index 643152685..8f8ef8a6f 100644 --- a/src/api/coderApi.ts +++ b/src/api/coderApi.ts @@ -82,6 +82,7 @@ const webSocketConfigSettings = [ "coder.tlsCaFile", "coder.tlsAltHost", "http.proxy", + "http.proxySupport", "coder.proxyBypass", "http.noProxy", "http.proxyAuthorization", diff --git a/src/api/utils.ts b/src/api/utils.ts index 98b4c61fe..fb58a74da 100644 --- a/src/api/utils.ts +++ b/src/api/utils.ts @@ -28,8 +28,16 @@ export async function createHttpAgent( ): Promise { const insecure = cfg.get("coder.insecure", false); const proxyStrictSSL = cfg.get("http.proxyStrictSSL", true); - const proxyAuthorization = cfg.get("http.proxyAuthorization"); - const httpNoProxy = cfg.get("http.noProxy"); + // "off" ignores VS Code proxy config; inherited env proxies still apply. + const proxyEnabled = cfg.get("http.proxySupport") !== "off"; + const proxySetting = (key: string) => + proxyEnabled ? cfg.get(key) : undefined; + const proxyAuthorization = proxySetting( + "http.proxyAuthorization", + ); + const httpProxy = proxySetting("http.proxy"); + const coderProxyBypass = proxySetting("coder.proxyBypass"); + const httpNoProxy = proxySetting("http.noProxy"); const certFile = expandPath( String(cfg.get("coder.tlsCertFile") ?? "").trim(), @@ -54,8 +62,8 @@ export async function createHttpAgent( getProxyForUrl: (url: string) => { return getProxyForUrl( url, - cfg.get("http.proxy"), - cfg.get("coder.proxyBypass"), + httpProxy, + coderProxyBypass, joinNoProxy(httpNoProxy), ); }, diff --git a/src/remote/environment.ts b/src/remote/environment.ts index 62ca17e0d..ab9e35c84 100644 --- a/src/remote/environment.ts +++ b/src/remote/environment.ts @@ -19,6 +19,7 @@ export const SSH_PROXY_SETTINGS: ReadonlyArray<{ title: string; }> = [ { setting: "http.proxy", title: "HTTP Proxy" }, + { setting: "http.proxySupport", title: "HTTP Proxy Support" }, { setting: "http.noProxy", title: "HTTP No Proxy" }, { setting: "coder.proxyBypass", title: "Proxy Bypass" }, ]; @@ -65,6 +66,10 @@ export function applySshEnvironment( export function getSshProxyEnvironment( cfg: Pick, ): SshEnvironment { + if (cfg.get("http.proxySupport") === "off") { + return {}; + } + const httpProxy = trimmed(cfg.get("http.proxy")); const noProxy = trimmed(cfg.get("coder.proxyBypass")) ?? diff --git a/test/mocks/testHelpers.ts b/test/mocks/testHelpers.ts index 52905e351..d3a24e459 100644 --- a/test/mocks/testHelpers.ts +++ b/test/mocks/testHelpers.ts @@ -149,6 +149,25 @@ export class MockConfigurationProvider { } } +export type Settings = Record; + +/** Proxy URL for proxy-config tests. */ +export const PROXY_URL = "http://proxy.example.com:8080"; + +/** A MockConfigurationProvider seeded with the given settings. */ +export function config(settings: Settings = {}): MockConfigurationProvider { + const cfg = new MockConfigurationProvider(); + for (const [key, value] of Object.entries(settings)) { + cfg.set(key, value); + } + return cfg; +} + +/** Settings with http.proxy set. */ +export function withProxy(settings: Settings = {}): Settings { + return { "http.proxy": PROXY_URL, ...settings }; +} + /** * Mock progress reporter that integrates with vscode.window.withProgress. * Use this to control progress reporting behavior and cancellation in tests. diff --git a/test/unit/api/coderApi.test.ts b/test/unit/api/coderApi.test.ts index d79c878b4..78f43044b 100644 --- a/test/unit/api/coderApi.test.ts +++ b/test/unit/api/coderApi.test.ts @@ -923,25 +923,31 @@ describe("CoderApi", () => { expect(sockets).toHaveLength(1); }); - it("reconnects sockets in DISCONNECTED state when config changes", async () => { - mockConfig.set("coder.insecure", false); - const { sockets, handlers } = setupAutoOpeningWebSocket(); - api = createApi(CODER_URL, AXIOS_TOKEN); - await api.watchAgentMetadata(AGENT_ID); - await tick(); - - // Trigger close with unrecoverable code to put socket in DISCONNECTED - handlers["close"]?.({ code: 1002, reason: "Protocol error" }); - await tick(); - - mockConfig.set("coder.insecure", true); - await new Promise((resolve) => - setTimeout(resolve, CONFIG_CHANGE_DEBOUNCE_MS + 50), - ); + it.each([ + ["coder.insecure", false, true], + ["http.proxySupport", "on", "off"], + ])( + "reconnects sockets in DISCONNECTED state when %s changes", + async (setting, before, after) => { + mockConfig.set(setting, before); + const { sockets, handlers } = setupAutoOpeningWebSocket(); + api = createApi(CODER_URL, AXIOS_TOKEN); + await api.watchAgentMetadata(AGENT_ID); + await tick(); + + // Trigger close with unrecoverable code to put socket in DISCONNECTED + handlers["close"]?.({ code: 1002, reason: "Protocol error" }); + await tick(); + + mockConfig.set(setting, after); + await new Promise((resolve) => + setTimeout(resolve, CONFIG_CHANGE_DEBOUNCE_MS + 50), + ); - // Only DISCONNECTED sockets get reconnected by config changes - expect(sockets).toHaveLength(2); - }); + // Only DISCONNECTED sockets get reconnected by config changes + expect(sockets).toHaveLength(2); + }, + ); }); }); diff --git a/test/unit/api/utils.test.ts b/test/unit/api/utils.test.ts index 3bc138907..18298a02f 100644 --- a/test/unit/api/utils.test.ts +++ b/test/unit/api/utils.test.ts @@ -1,9 +1,14 @@ import { vol } from "memfs"; -import { describe, it, expect, vi, beforeEach } from "vitest"; +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; import { createHttpAgent, needToken } from "@/api/utils"; -import { MockConfigurationProvider } from "../../mocks/testHelpers"; +import { + config, + PROXY_URL as proxy, + withProxy, + type Settings, +} from "../../mocks/testHelpers"; import type * as http from "node:http"; import type { ConnectionOptions } from "node:tls"; @@ -18,44 +23,96 @@ vi.mock("node:fs/promises", async () => { // resolve to `never` and drops TLS fields. Re-add them for test assertions. type AgentOpts = ProxyAgentOptions & ConnectionOptions; +const proxyAuthorization = "Basic dXNlcjpwYXNz"; +// Our getProxyForUrl wrapper only uses the URL, not the request object. +// The request parameter exists to match proxy-agent's callback signature. +const mockRequest = {} as http.ClientRequest; + +const proxyEnvVars = [ + "HTTP_PROXY", + "http_proxy", + "HTTPS_PROXY", + "https_proxy", + "ALL_PROXY", + "all_proxy", + "npm_config_http_proxy", + "NPM_CONFIG_HTTP_PROXY", + "npm_config_https_proxy", + "NPM_CONFIG_HTTPS_PROXY", + "npm_config_proxy", + "NPM_CONFIG_PROXY", + "NO_PROXY", + "no_proxy", + "npm_config_no_proxy", + "NPM_CONFIG_NO_PROXY", +]; + +async function createAgentOptions(settings: Settings = {}) { + const agent = await createHttpAgent(config(settings)); + return agent.connectOpts as AgentOpts | undefined; +} + +async function createProxyResolver(settings: Settings = {}) { + const agent = await createHttpAgent(config(settings)); + return async (url = "https://example.com"): Promise => + await agent.getProxyForUrl(url, mockRequest); +} + +async function proxyForUrl( + settings: Settings, + url = "https://example.com", +): Promise { + const getProxy = await createProxyResolver(settings); + return await getProxy(url); +} + +function clearProxyEnv(): void { + for (const envVar of proxyEnvVars) { + vi.stubEnv(envVar, ""); + } +} + describe("needToken", () => { interface NeedTokenCase { name: string; - config: Record; + settings: Settings; expected: boolean; } it.each([ - { name: "no mTLS certificates", config: {}, expected: true }, + { name: "no mTLS certificates", settings: {}, expected: true }, { name: "cert file configured", - config: { "coder.tlsCertFile": "/cert.pem" }, + settings: { "coder.tlsCertFile": "/cert.pem" }, expected: false, }, { name: "key file configured", - config: { "coder.tlsKeyFile": "/key.pem" }, + settings: { "coder.tlsKeyFile": "/key.pem" }, expected: false, }, { name: "both cert and key configured", - config: { + settings: { "coder.tlsCertFile": "/cert.pem", "coder.tlsKeyFile": "/key.pem", }, expected: false, }, - ])("returns $expected when $name", ({ config, expected }) => { - const cfg = new MockConfigurationProvider(); - Object.entries(config).forEach(([k, v]) => cfg.set(k, v)); - - expect(needToken(cfg)).toBe(expected); + ])("returns $expected when $name", ({ settings, expected }) => { + expect(needToken(config(settings))).toBe(expected); }); }); describe("createHttpAgent", () => { beforeEach(() => { vol.reset(); + vi.unstubAllEnvs(); + clearProxyEnv(); + }); + + afterEach(() => { + vi.unstubAllEnvs(); }); describe("TLS certificates", () => { @@ -66,13 +123,11 @@ describe("createHttpAgent", () => { "/ca.pem": "ca-content", }); - const cfg = new MockConfigurationProvider(); - cfg.set("coder.tlsCertFile", "/cert.pem"); - cfg.set("coder.tlsKeyFile", "/key.pem"); - cfg.set("coder.tlsCaFile", "/ca.pem"); - - const agent = await createHttpAgent(cfg); - const opts = agent.connectOpts as AgentOpts | undefined; + const opts = await createAgentOptions({ + "coder.tlsCertFile": "/cert.pem", + "coder.tlsKeyFile": "/key.pem", + "coder.tlsCaFile": "/ca.pem", + }); expect(Buffer.isBuffer(opts?.cert) && opts.cert.toString()).toBe( "cert-content", @@ -86,10 +141,7 @@ describe("createHttpAgent", () => { }); it("leaves cert options undefined when files not configured", async () => { - const cfg = new MockConfigurationProvider(); - - const agent = await createHttpAgent(cfg); - const opts = agent.connectOpts as AgentOpts | undefined; + const opts = await createAgentOptions(); expect(opts?.cert).toBeUndefined(); expect(opts?.key).toBeUndefined(); @@ -97,11 +149,9 @@ describe("createHttpAgent", () => { }); it("sets servername from tlsAltHost", async () => { - const cfg = new MockConfigurationProvider(); - cfg.set("coder.tlsAltHost", "alt.example.com"); - - const agent = await createHttpAgent(cfg); - const opts = agent.connectOpts as AgentOpts | undefined; + const opts = await createAgentOptions({ + "coder.tlsAltHost": "alt.example.com", + }); expect(opts?.servername).toBe("alt.example.com"); }); @@ -110,160 +160,190 @@ describe("createHttpAgent", () => { describe("TLS verification", () => { interface TlsVerificationCase { name: string; - config: Record; + settings: Settings; expected: boolean; } it.each([ - { name: "enabled by default", config: {}, expected: true }, + { name: "enabled by default", settings: {}, expected: true }, { name: "disabled when proxyStrictSSL=false", - config: { "http.proxyStrictSSL": false }, + settings: { "http.proxyStrictSSL": false }, expected: false, }, { name: "disabled when insecure=true", - config: { "coder.insecure": true }, + settings: { "coder.insecure": true }, expected: false, }, { name: "disabled when both proxyStrictSSL=false and insecure=false", - config: { "http.proxyStrictSSL": false, "coder.insecure": false }, + settings: { "http.proxyStrictSSL": false, "coder.insecure": false }, expected: false, }, { name: "disabled when insecure overrides proxyStrictSSL", - config: { "http.proxyStrictSSL": true, "coder.insecure": true }, + settings: { "http.proxyStrictSSL": true, "coder.insecure": true }, expected: false, }, - ])("rejectUnauthorized=$expected ($name)", async ({ config, expected }) => { - const cfg = new MockConfigurationProvider(); - Object.entries(config).forEach(([k, v]) => cfg.set(k, v)); - - const agent = await createHttpAgent(cfg); - const opts = agent.connectOpts as AgentOpts | undefined; + ])( + "rejectUnauthorized=$expected ($name)", + async ({ settings, expected }) => { + const opts = await createAgentOptions(settings); - expect(opts?.rejectUnauthorized).toBe(expected); - }); + expect(opts?.rejectUnauthorized).toBe(expected); + }, + ); }); describe("proxy authorization", () => { - it("sets Proxy-Authorization header when configured", async () => { - const cfg = new MockConfigurationProvider(); - // VS Code's http.proxyAuthorization is the complete header value - cfg.set("http.proxyAuthorization", "Basic dXNlcjpwYXNz"); - - const agent = await createHttpAgent(cfg); - - expect(agent.connectOpts?.headers).toEqual({ - "Proxy-Authorization": "Basic dXNlcjpwYXNz", - }); - }); - - it("omits headers when proxyAuthorization is not set", async () => { - const cfg = new MockConfigurationProvider(); + interface ProxyAuthorizationCase { + name: string; + settings: Settings; + expected: Record | undefined; + } - const agent = await createHttpAgent(cfg); + it.each([ + { + name: "sets Proxy-Authorization header when configured", + settings: { "http.proxyAuthorization": proxyAuthorization }, + expected: { "Proxy-Authorization": proxyAuthorization }, + }, + { + name: "omits headers when proxyAuthorization is not set", + settings: {}, + expected: undefined, + }, + { + name: "ignores proxyAuthorization when proxy support is off", + settings: { + "http.proxySupport": "off", + "http.proxyAuthorization": proxyAuthorization, + }, + expected: undefined, + }, + ])("$name", async ({ settings, expected }) => { + const opts = await createAgentOptions(settings); - expect(agent.connectOpts?.headers).toBeUndefined(); + expect(opts?.headers).toEqual(expected); }); }); describe("proxy resolution", () => { - // Our getProxyForUrl wrapper only uses the URL, not the request object. - // The request parameter exists to match proxy-agent's callback signature. - const mockRequest = {} as http.ClientRequest; - const proxy = "http://proxy.example.com:8080"; - - it("uses http.proxy setting", async () => { - const cfg = new MockConfigurationProvider(); - cfg.set("http.proxy", proxy); - - const agent = await createHttpAgent(cfg); - - expect( - await agent.getProxyForUrl("https://example.com", mockRequest), - ).toBe(proxy); - }); - - it("bypasses proxy for hosts in coder.proxyBypass", async () => { - const cfg = new MockConfigurationProvider(); - cfg.set("http.proxy", proxy); - cfg.set("coder.proxyBypass", "internal.example.com"); - - const agent = await createHttpAgent(cfg); - - expect( - await agent.getProxyForUrl("https://internal.example.com", mockRequest), - ).toBe(""); - expect( - await agent.getProxyForUrl("https://external.example.com", mockRequest), - ).toBe(proxy); - }); - - it("uses http.noProxy as fallback when coder.proxyBypass is not set", async () => { - const cfg = new MockConfigurationProvider(); - cfg.set("http.proxy", proxy); - cfg.set("http.noProxy", ["internal.example.com"]); - - const agent = await createHttpAgent(cfg); - - expect( - await agent.getProxyForUrl("https://internal.example.com", mockRequest), - ).toBe(""); - }); + interface ProxySupportCase { + name: string; + settings: Settings; + } - it("prefers coder.proxyBypass over http.noProxy", async () => { - const cfg = new MockConfigurationProvider(); - cfg.set("http.proxy", proxy); - cfg.set("coder.proxyBypass", "primary.example.com"); - cfg.set("http.noProxy", ["fallback.example.com"]); + it.each([ + { name: "unset", settings: withProxy() }, + { + name: "on", + settings: withProxy({ "http.proxySupport": "on" }), + }, + { + name: "fallback", + settings: withProxy({ "http.proxySupport": "fallback" }), + }, + { + name: "override", + settings: withProxy({ "http.proxySupport": "override" }), + }, + ])( + "uses http.proxy setting when proxy support is $name", + async ({ settings }) => { + expect(await proxyForUrl(settings)).toBe(proxy); + }, + ); - const agent = await createHttpAgent(cfg); + it("preserves inherited proxy env when proxy support is off", async () => { + vi.stubEnv("HTTPS_PROXY", "http://env-proxy.example.com:8080"); expect( - await agent.getProxyForUrl("https://primary.example.com", mockRequest), - ).toBe(""); - expect( - await agent.getProxyForUrl("https://fallback.example.com", mockRequest), - ).toBe(proxy); + await proxyForUrl( + withProxy({ + "http.proxySupport": "off", + "coder.proxyBypass": "example.com", + "http.noProxy": ["example.com"], + }), + ), + ).toBe("http://env-proxy.example.com:8080"); }); - it("trims and joins multiple http.noProxy entries", async () => { - const cfg = new MockConfigurationProvider(); - cfg.set("http.proxy", proxy); - cfg.set("http.noProxy", [" first.example.com ", "second.example.com "]); - - const agent = await createHttpAgent(cfg); + interface ProxyResolutionCase { + name: string; + settings: Settings; + expectedByUrl: Record; + } - expect( - await agent.getProxyForUrl("https://first.example.com", mockRequest), - ).toBe(""); - expect( - await agent.getProxyForUrl("https://second.example.com", mockRequest), - ).toBe(""); - expect( - await agent.getProxyForUrl("https://other.example.com", mockRequest), - ).toBe(proxy); + it.each([ + { + name: "ignores VS Code proxy settings when proxy support is off", + settings: withProxy({ + "http.proxySupport": "off", + "coder.proxyBypass": "example.com", + "http.noProxy": ["example.com"], + }), + expectedByUrl: { "https://example.com": "" }, + }, + { + name: "bypasses proxy for hosts in coder.proxyBypass", + settings: withProxy({ + "coder.proxyBypass": "internal.example.com", + }), + expectedByUrl: { + "https://internal.example.com": "", + "https://external.example.com": proxy, + }, + }, + { + name: "uses http.noProxy as fallback when coder.proxyBypass is not set", + settings: withProxy({ + "http.noProxy": ["internal.example.com"], + }), + expectedByUrl: { "https://internal.example.com": "" }, + }, + { + name: "prefers coder.proxyBypass over http.noProxy", + settings: withProxy({ + "coder.proxyBypass": "primary.example.com", + "http.noProxy": ["fallback.example.com"], + }), + expectedByUrl: { + "https://primary.example.com": "", + "https://fallback.example.com": proxy, + }, + }, + { + name: "trims and joins multiple http.noProxy entries", + settings: withProxy({ + "http.noProxy": [" first.example.com ", "second.example.com "], + }), + expectedByUrl: { + "https://first.example.com": "", + "https://second.example.com": "", + "https://other.example.com": proxy, + }, + }, + ])("$name", async ({ settings, expectedByUrl }) => { + const getProxy = await createProxyResolver(settings); + for (const [url, expected] of Object.entries(expectedByUrl)) { + expect(await getProxy(url)).toBe(expected); + } }); interface NoProxyTestCase { name: string; noProxy: string[] | undefined; } + it.each([ { name: "undefined", noProxy: undefined }, { name: "empty array", noProxy: [] }, ])("uses proxy when http.noProxy is $name", async ({ noProxy }) => { - const cfg = new MockConfigurationProvider(); - cfg.set("http.proxy", proxy); - cfg.set("http.noProxy", noProxy); - - const agent = await createHttpAgent(cfg); - - expect( - await agent.getProxyForUrl("https://example.com", mockRequest), - ).toBe(proxy); + expect(await proxyForUrl(withProxy({ "http.noProxy": noProxy }))).toBe( + proxy, + ); }); }); }); diff --git a/test/unit/remote/environment.test.ts b/test/unit/remote/environment.test.ts index 630ea49e7..8476a8332 100644 --- a/test/unit/remote/environment.test.ts +++ b/test/unit/remote/environment.test.ts @@ -1,71 +1,84 @@ import { spawnSync } from "node:child_process"; -import { describe, expect, it, vi } from "vitest"; +import { beforeEach, describe, expect, it, vi } from "vitest"; import { applySshEnvironment, getSshProxyEnvironment, } from "@/remote/environment"; -import { MockConfigurationProvider } from "../../mocks/testHelpers"; +import { + config, + PROXY_URL as proxy, + withProxy, + type Settings, +} from "../../mocks/testHelpers"; -const proxy = "http://proxy.example.com:8080"; +const proxyEnv = { HTTP_PROXY: proxy, HTTPS_PROXY: proxy }; +type Environment = Record; -function setup() { +beforeEach(() => { vi.unstubAllEnvs(); - return { - config(settings: Record = {}): MockConfigurationProvider { - const cfg = new MockConfigurationProvider(); - for (const [key, value] of Object.entries(settings)) { - cfg.set(key, value); - } - return cfg; - }, - }; -} +}); describe("getSshProxyEnvironment", () => { - it.each([ + interface ProxyEnvironmentCase { + name: string; + settings: Settings; + expected: Record; + } + + it.each([ { name: "sets both proxy variables from http.proxy", - settings: { "http.proxy": proxy }, - expected: { HTTP_PROXY: proxy, HTTPS_PROXY: proxy }, + settings: withProxy(), + expected: proxyEnv, }, { - name: "passes through the proxy when the deployment is bypassed", - settings: { - "http.proxy": proxy, + name: "sets both proxy variables when proxy support is on", + settings: withProxy({ "http.proxySupport": "on" }), + expected: proxyEnv, + }, + { + name: "sets both proxy variables when proxy support is fallback", + settings: withProxy({ "http.proxySupport": "fallback" }), + expected: proxyEnv, + }, + { + name: "sets both proxy variables when proxy support is override", + settings: withProxy({ "http.proxySupport": "override" }), + expected: proxyEnv, + }, + { + name: "ignores VS Code proxy settings when proxy support is off", + settings: withProxy({ + "http.proxySupport": "off", "coder.proxyBypass": "coder.example.com", - }, - expected: { - HTTP_PROXY: proxy, - HTTPS_PROXY: proxy, - NO_PROXY: "coder.example.com", - }, + "http.noProxy": ["fallback.example.com"], + }), + expected: {}, + }, + { + name: "passes through the proxy when the deployment is bypassed", + settings: withProxy({ "coder.proxyBypass": "coder.example.com" }), + expected: { ...proxyEnv, NO_PROXY: "coder.example.com" }, }, { name: "falls back to http.noProxy when coder.proxyBypass is unset", - settings: { - "http.proxy": proxy, + settings: withProxy({ "http.noProxy": [" first.example.com ", "second.example.com "], - }, + }), expected: { - HTTP_PROXY: proxy, - HTTPS_PROXY: proxy, + ...proxyEnv, NO_PROXY: "first.example.com,second.example.com", }, }, { name: "prefers coder.proxyBypass over http.noProxy", - settings: { - "http.proxy": proxy, + settings: withProxy({ "coder.proxyBypass": "primary.example.com", "http.noProxy": ["fallback.example.com"], - }, - expected: { - HTTP_PROXY: proxy, - HTTPS_PROXY: proxy, - NO_PROXY: "primary.example.com", - }, + }), + expected: { ...proxyEnv, NO_PROXY: "primary.example.com" }, }, { name: "ignores a whitespace-only http.proxy", @@ -74,25 +87,17 @@ describe("getSshProxyEnvironment", () => { }, { name: "falls back to http.noProxy when coder.proxyBypass is whitespace", - settings: { - "http.proxy": proxy, + settings: withProxy({ "coder.proxyBypass": " ", "http.noProxy": ["fallback.example.com"], - }, - expected: { - HTTP_PROXY: proxy, - HTTPS_PROXY: proxy, - NO_PROXY: "fallback.example.com", - }, + }), + expected: { ...proxyEnv, NO_PROXY: "fallback.example.com" }, }, ])("$name", ({ settings, expected }) => { - const { config } = setup(); - 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(config())).toEqual({}); @@ -101,23 +106,16 @@ describe("getSshProxyEnvironment", () => { describe("applySshEnvironment", () => { it("applies proxy variables to process.env and the collection, and restores on dispose", () => { - const { config } = setup(); - const env: Record = {}; + const env: Environment = {}; const collection = fakeEnvCollection(); + const expected = { ...proxyEnv, NO_PROXY: "internal.example.com" }; const applied = applySshEnvironment( - config({ - "http.proxy": proxy, - "coder.proxyBypass": "internal.example.com", - }), + config(withProxy({ "coder.proxyBypass": "internal.example.com" })), collection, env, ); - const expected = { - HTTP_PROXY: proxy, - HTTPS_PROXY: proxy, - NO_PROXY: "internal.example.com", - }; + expect(env).toEqual(expected); expect(collection.vars).toEqual(expected); expect(collection.persistent).toBe(false); @@ -128,8 +126,7 @@ describe("applySshEnvironment", () => { }); it("sets nothing when no proxy is configured", () => { - const { config } = setup(); - const env: Record = {}; + const env: Environment = {}; const collection = fakeEnvCollection(); applySshEnvironment(config(), collection, env); @@ -138,36 +135,49 @@ describe("applySshEnvironment", () => { expect(collection.vars).toEqual({}); }); + it("does not clear existing env proxy variables when proxy support is off", () => { + const original = { + HTTP_PROXY: "http://old-http-proxy.example.com:8080", + HTTPS_PROXY: "http://old-https-proxy.example.com:8080", + }; + const env: Environment = { ...original }; + const collection = fakeEnvCollection(); + + applySshEnvironment( + config(withProxy({ "http.proxySupport": "off" })), + collection, + env, + ); + + expect(env).toEqual(original); + expect(collection.vars).toEqual({}); + }); + 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 = { ...original }; + const env: Environment = { ...original }; const applied = applySshEnvironment( - config({ "http.proxy": proxy }), + config(withProxy()), fakeEnvCollection(), env, ); - expect(env).toEqual({ - ...original, - HTTP_PROXY: proxy, - HTTPS_PROXY: proxy, - }); + + expect(env).toEqual({ ...original, ...proxyEnv }); 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 }), + config(withProxy()), fakeEnvCollection(), env, ); @@ -180,9 +190,8 @@ describe("applySshEnvironment", () => { }); it("propagates proxy variables to newly spawned child processes", () => { - const { config } = setup(); const applied = applySshEnvironment( - config({ "http.proxy": proxy }), + config(withProxy()), fakeEnvCollection(), ); @@ -212,7 +221,7 @@ function fakeEnvCollection() { function caseInsensitiveEnvironment( values: Record, -): Record { +): Environment { return new Proxy(values, { get(target, property) { if (typeof property !== "string") {