diff --git a/CHANGELOG.md b/CHANGELOG.md index aeaaf8b9..4cfd009c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,16 @@ ## Unreleased +### Added + +- Support for VS Code's built-in proxy settings: `http.noProxy` (as fallback when `coder.proxyBypass` + is not set), `http.proxyAuthorization`, and `http.proxyStrictSSL`. + +### Fixed + +- Fixed proxy scheme handling where URLs with schemes got duplicated and URLs without schemes + were not normalized. + ### Changed - WebSocket connections are now more robust and reconnect less frequently, only when truly diff --git a/src/api/coderApi.ts b/src/api/coderApi.ts index a04bfeeb..70a0c5cc 100644 --- a/src/api/coderApi.ts +++ b/src/api/coderApi.ts @@ -70,6 +70,9 @@ const webSocketConfigSettings = [ "coder.tlsAltHost", "http.proxy", "coder.proxyBypass", + "http.noProxy", + "http.proxyAuthorization", + "http.proxyStrictSSL", ] as const; /** diff --git a/src/api/proxy.ts b/src/api/proxy.ts index 04052ebc..d71dc2f6 100644 --- a/src/api/proxy.ts +++ b/src/api/proxy.ts @@ -14,6 +14,9 @@ const DEFAULT_PORTS: Record = { /** * @param {string|object} url - The URL, or the result from url.parse. + * @param {string} httpProxy - The proxy URL to use. + * @param {string} noProxy - Primary no-proxy list (e.g., coder.proxyBypass). + * @param {string} noProxyFallback - Fallback no-proxy list (e.g., http.noProxy). * @return {string} The URL of the proxy that should handle the request to the * given URL. If no proxy is set, this will be an empty string. */ @@ -21,6 +24,7 @@ export function getProxyForUrl( url: string, httpProxy: string | null | undefined, noProxy: string | null | undefined, + noProxyFallback?: string | null, ): string { const parsedUrl = typeof url === "string" ? parseUrl(url) : url || {}; let proto = parsedUrl.protocol; @@ -36,7 +40,7 @@ export function getProxyForUrl( hostname = hostname.replace(/:\d*$/, ""); const port = (portRaw && Number.parseInt(portRaw)) || DEFAULT_PORTS[proto] || 0; - if (!shouldProxy(hostname, port, noProxy)) { + if (!shouldProxy(hostname, port, noProxy, noProxyFallback)) { return ""; // Don't proxy URLs that match NO_PROXY. } @@ -46,7 +50,7 @@ export function getProxyForUrl( getEnv(proto + "_proxy") || getEnv("npm_config_proxy") || getEnv("all_proxy"); - if (proxy?.includes("://")) { + if (proxy && !proxy.includes("://")) { // Missing scheme in proxy, default to the requested URL's scheme. proxy = proto + "://" + proxy; } @@ -58,6 +62,8 @@ export function getProxyForUrl( * * @param {string} hostname - The host name of the URL. * @param {number} port - The effective port of the URL. + * @param {string} noProxy - Primary no-proxy list (e.g., coder.proxyBypass). + * @param {string} noProxyFallback - Fallback no-proxy list (e.g., http.noProxy). * @returns {boolean} Whether the given URL should be proxied. * @private */ @@ -65,9 +71,11 @@ function shouldProxy( hostname: string, port: number, noProxy: string | null | undefined, + noProxyFallback?: string | null, ): boolean { const NO_PROXY = ( noProxy || + noProxyFallback || getEnv("npm_config_no_proxy") || getEnv("no_proxy") ).toLowerCase(); diff --git a/src/api/utils.ts b/src/api/utils.ts index 86604e3e..bc4a6f7a 100644 --- a/src/api/utils.ts +++ b/src/api/utils.ts @@ -11,7 +11,7 @@ import { getProxyForUrl } from "./proxy"; * If mTLS is in use (as specified by the cert or key files being set) then * token authorization is disabled. Otherwise, it is enabled. */ -export function needToken(cfg: WorkspaceConfiguration): boolean { +export function needToken(cfg: Pick): boolean { const certFile = expandPath( String(cfg.get("coder.tlsCertFile") ?? "").trim(), ); @@ -24,9 +24,13 @@ export function needToken(cfg: WorkspaceConfiguration): boolean { * Configures proxy, TLS certificates, and security options. */ export async function createHttpAgent( - cfg: WorkspaceConfiguration, + cfg: Pick, ): Promise { - const insecure = Boolean(cfg.get("coder.insecure")); + 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"); + const certFile = expandPath( String(cfg.get("coder.tlsCertFile") ?? "").trim(), ); @@ -40,6 +44,11 @@ export async function createHttpAgent( caFile === "" ? Promise.resolve(undefined) : fs.readFile(caFile), ]); + // Build proxy authorization header if configured. + const headers: Record | undefined = proxyAuthorization + ? { "Proxy-Authorization": proxyAuthorization } + : undefined; + return new ProxyAgent({ // Called each time a request is made. getProxyForUrl: (url: string) => { @@ -47,14 +56,17 @@ export async function createHttpAgent( url, cfg.get("http.proxy"), cfg.get("coder.proxyBypass"), + httpNoProxy, ); }, + headers, cert, key, ca, servername: altHost === "" ? undefined : altHost, - // rejectUnauthorized defaults to true, so we need to explicitly set it to - // false if we want to allow self-signed certificates. - rejectUnauthorized: !insecure, + // TLS verification is disabled if either: + // - http.proxyStrictSSL is false (VS Code's proxy SSL setting) + // - coder.insecure is true (backward compatible override for Coder server) + rejectUnauthorized: proxyStrictSSL && !insecure, }); } diff --git a/test/unit/api/proxy.test.ts b/test/unit/api/proxy.test.ts new file mode 100644 index 00000000..1816c516 --- /dev/null +++ b/test/unit/api/proxy.test.ts @@ -0,0 +1,257 @@ +import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; + +import { getProxyForUrl } from "@/api/proxy"; + +describe("proxy", () => { + const proxy = "http://proxy.example.com:8080"; + + beforeEach(() => { + vi.unstubAllEnvs(); + }); + + afterEach(() => { + vi.unstubAllEnvs(); + }); + + describe("getProxyForUrl", () => { + describe("proxy resolution", () => { + it("returns httpProxy when provided", () => { + expect(getProxyForUrl("https://example.com", proxy, null)).toBe(proxy); + }); + + it("falls back to environment variables when httpProxy is null", () => { + vi.stubEnv("HTTPS_PROXY", "http://env-proxy.example.com:8080"); + expect(getProxyForUrl("https://example.com", null, null)).toBe( + "http://env-proxy.example.com:8080", + ); + }); + + it("returns empty string when no proxy is configured", () => { + const proxyEnvVars = [ + "HTTPS_PROXY", + "https_proxy", + "HTTP_PROXY", + "http_proxy", + "ALL_PROXY", + "all_proxy", + "npm_config_https_proxy", + "npm_config_proxy", + ]; + proxyEnvVars.forEach((v) => vi.stubEnv(v, "")); + + expect(getProxyForUrl("https://example.com", null, null)).toBe(""); + }); + + it("returns empty string for invalid URLs", () => { + expect(getProxyForUrl("invalid", proxy, null)).toBe(""); + }); + }); + + describe("noProxy handling", () => { + interface NoProxyBypassCase { + name: string; + noProxy: string; + url: string; + } + + it.each([ + { + name: "exact match", + noProxy: "internal.example.com", + url: "https://internal.example.com", + }, + { + name: "wildcard", + noProxy: "*.internal.example.com", + url: "https://api.internal.example.com", + }, + { + name: "suffix", + noProxy: ".example.com", + url: "https://api.example.com", + }, + { + name: "wildcard *", + noProxy: "*", + url: "https://any.domain.com", + }, + { + name: "port-specific", + noProxy: "example.com:8443", + url: "https://example.com:8443", + }, + ])( + "bypasses proxy when hostname matches noProxy ($name)", + ({ noProxy, url }) => { + expect(getProxyForUrl(url, proxy, noProxy)).toBe(""); + }, + ); + + it("proxies when hostname does not match noProxy", () => { + expect( + getProxyForUrl("https://external.com", proxy, "internal.example.com"), + ).toBe(proxy); + }); + + it("proxies when port does not match noProxy port", () => { + expect( + getProxyForUrl("https://example.com:443", proxy, "example.com:8443"), + ).toBe(proxy); + }); + + it("handles multiple entries in noProxy (comma-separated)", () => { + const noProxy = "localhost,127.0.0.1,.internal.com"; + + expect(getProxyForUrl("https://localhost", proxy, noProxy)).toBe(""); + expect(getProxyForUrl("https://127.0.0.1", proxy, noProxy)).toBe(""); + expect(getProxyForUrl("https://api.internal.com", proxy, noProxy)).toBe( + "", + ); + expect(getProxyForUrl("https://external.com", proxy, noProxy)).toBe( + proxy, + ); + }); + }); + + describe("noProxy fallback chain", () => { + const targetUrl = "https://internal.example.com"; + const targetHost = "internal.example.com"; + + interface NoProxyFallbackCase { + name: string; + noProxy: string | null; + noProxyFallback: string; + } + + it.each([ + { + name: "noProxy (coder.proxyBypass)", + noProxy: targetHost, + noProxyFallback: "other.example.com", + }, + { + name: "noProxyFallback when noProxy is null", + noProxy: null, + noProxyFallback: targetHost, + }, + { + name: "noProxyFallback when noProxy is empty", + noProxy: "", + noProxyFallback: targetHost, + }, + ])("uses $name", ({ noProxy, noProxyFallback }) => { + expect(getProxyForUrl(targetUrl, proxy, noProxy, noProxyFallback)).toBe( + "", + ); + }); + + interface EnvVarFallbackCase { + name: string; + envVar: string; + } + + it.each([ + { name: "npm_config_no_proxy", envVar: "npm_config_no_proxy" }, + { name: "NO_PROXY", envVar: "NO_PROXY" }, + { name: "no_proxy (lowercase)", envVar: "no_proxy" }, + ])("falls back to $name env var", ({ envVar }) => { + // Clear all no_proxy env vars first + vi.stubEnv("npm_config_no_proxy", ""); + vi.stubEnv("NO_PROXY", ""); + vi.stubEnv("no_proxy", ""); + + vi.stubEnv(envVar, targetHost); + expect(getProxyForUrl(targetUrl, proxy, null, null)).toBe(""); + }); + + it("prioritizes noProxy over noProxyFallback over env vars", () => { + vi.stubEnv("NO_PROXY", "env.example.com"); + + // noProxy takes precedence + expect( + getProxyForUrl( + "https://primary.example.com", + proxy, + "primary.example.com", + "fallback.example.com", + ), + ).toBe(""); + + // noProxyFallback is used when noProxy is null + expect( + getProxyForUrl( + "https://fallback.example.com", + proxy, + null, + "fallback.example.com", + ), + ).toBe(""); + + // env var is used when both are null + expect( + getProxyForUrl("https://env.example.com", proxy, null, null), + ).toBe(""); + }); + }); + + describe("protocol and port handling", () => { + interface ProtocolCase { + protocol: string; + url: string; + } + + it.each([ + { protocol: "http://", url: "http://example.com" }, + { protocol: "https://", url: "https://example.com" }, + { protocol: "ws://", url: "ws://example.com" }, + { protocol: "wss://", url: "wss://example.com" }, + ])("handles $protocol URLs", ({ url }) => { + expect(getProxyForUrl(url, proxy, null)).toBe(proxy); + }); + + interface DefaultPortCase { + protocol: string; + url: string; + defaultPort: number; + } + + it.each([ + { protocol: "HTTP", url: "http://example.com", defaultPort: 80 }, + { protocol: "HTTPS", url: "https://example.com", defaultPort: 443 }, + { protocol: "WS", url: "ws://example.com", defaultPort: 80 }, + { protocol: "WSS", url: "wss://example.com", defaultPort: 443 }, + ])( + "uses default port $defaultPort for $protocol", + ({ url, defaultPort }) => { + expect(getProxyForUrl(url, proxy, `example.com:${defaultPort}`)).toBe( + "", + ); + }, + ); + }); + + describe("proxy scheme handling", () => { + it("adds scheme to proxy URL when missing", () => { + expect( + getProxyForUrl("https://example.com", "proxy.example.com:8080", null), + ).toBe("https://proxy.example.com:8080"); + }); + + it("uses request scheme when proxy has no scheme", () => { + expect( + getProxyForUrl("http://example.com", "proxy.example.com:8080", null), + ).toBe("http://proxy.example.com:8080"); + }); + + it("preserves existing scheme in proxy URL", () => { + expect( + getProxyForUrl( + "https://example.com", + "http://proxy.example.com:8080", + null, + ), + ).toBe("http://proxy.example.com:8080"); + }); + }); + }); +}); diff --git a/test/unit/api/utils.test.ts b/test/unit/api/utils.test.ts new file mode 100644 index 00000000..1c46d122 --- /dev/null +++ b/test/unit/api/utils.test.ts @@ -0,0 +1,229 @@ +import { vol } from "memfs"; +import { describe, it, expect, vi, beforeEach } from "vitest"; + +import { createHttpAgent, needToken } from "@/api/utils"; + +import { MockConfigurationProvider } from "../../mocks/testHelpers"; + +import type * as http from "node:http"; +import type { ConnectionOptions } from "node:tls"; +import type { ProxyAgentOptions } from "proxy-agent"; + +vi.mock("node:fs/promises", async () => { + const memfs = await import("memfs"); + return { default: memfs.fs.promises, ...memfs.fs.promises }; +}); + +// ProxyAgentOptions extends TLS options but TypeScript doesn't resolve the intersection. +type AgentOpts = ProxyAgentOptions & ConnectionOptions; + +describe("needToken", () => { + interface NeedTokenCase { + name: string; + config: Record; + expected: boolean; + } + + it.each([ + { name: "no mTLS certificates", config: {}, expected: true }, + { + name: "cert file configured", + config: { "coder.tlsCertFile": "/cert.pem" }, + expected: false, + }, + { + name: "key file configured", + config: { "coder.tlsKeyFile": "/key.pem" }, + expected: false, + }, + { + name: "both cert and key configured", + config: { + "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); + }); +}); + +describe("createHttpAgent", () => { + beforeEach(() => { + vol.reset(); + }); + + describe("TLS certificates", () => { + it("reads certificate files from disk", async () => { + vol.fromJSON({ + "/cert.pem": "cert-content", + "/key.pem": "key-content", + "/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; + + expect(Buffer.isBuffer(opts.cert) && opts.cert.toString()).toBe( + "cert-content", + ); + expect(Buffer.isBuffer(opts.key) && opts.key.toString()).toBe( + "key-content", + ); + expect(Buffer.isBuffer(opts.ca) && opts.ca.toString()).toBe("ca-content"); + }); + + 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; + + expect(opts.cert).toBeUndefined(); + expect(opts.key).toBeUndefined(); + expect(opts.ca).toBeUndefined(); + }); + + 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; + + expect(opts.servername).toBe("alt.example.com"); + }); + }); + + describe("TLS verification", () => { + interface TlsVerificationCase { + name: string; + config: Record; + expected: boolean; + } + + it.each([ + { name: "enabled by default", config: {}, expected: true }, + { + name: "disabled when proxyStrictSSL=false", + config: { "http.proxyStrictSSL": false }, + expected: false, + }, + { + name: "disabled when insecure=true", + config: { "coder.insecure": true }, + expected: false, + }, + { + name: "disabled when both proxyStrictSSL=false and insecure=false", + config: { "http.proxyStrictSSL": false, "coder.insecure": false }, + expected: false, + }, + { + name: "disabled when insecure overrides proxyStrictSSL", + config: { "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; + + 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(); + + const agent = await createHttpAgent(cfg); + + expect(agent.connectOpts?.headers).toBeUndefined(); + }); + }); + + 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(""); + }); + + 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"); + + const agent = await createHttpAgent(cfg); + + expect( + await agent.getProxyForUrl("https://primary.example.com", mockRequest), + ).toBe(""); + expect( + await agent.getProxyForUrl("https://fallback.example.com", mockRequest), + ).toBe(proxy); + }); + }); +});