Skip to content

Commit 1133a0f

Browse files
EhabYEhabY
authored
Support VS Code's built-in proxy settings (#756)
Add support for VS Code's proxy settings that were not previously respected: - http.noProxy: Falls back when coder.proxyBypass is not set - http.proxyAuthorization: Passed directly as Proxy-Authorization header - http.proxyStrictSSL: Combined with coder.insecure for TLS verification Also fixes a bug in proxy.ts where the scheme condition was inverted, causing schemes to be added when they already existed. Closes #755
1 parent 31a55b9 commit 1133a0f

File tree

6 files changed

+527
-8
lines changed

6 files changed

+527
-8
lines changed

‎CHANGELOG.md‎

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

33
## Unreleased
44

5+
### Added
6+
7+
- Support for VS Code's built-in proxy settings: `http.noProxy` (as fallback when `coder.proxyBypass`
8+
is not set), `http.proxyAuthorization`, and `http.proxyStrictSSL`.
9+
10+
### Fixed
11+
12+
- Fixed proxy scheme handling where URLs with schemes got duplicated and URLs without schemes
13+
were not normalized.
14+
515
### Changed
616

717
- WebSocket connections are now more robust and reconnect less frequently, only when truly

‎src/api/coderApi.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,9 @@ const webSocketConfigSettings = [
7070
"coder.tlsAltHost",
7171
"http.proxy",
7272
"coder.proxyBypass",
73+
"http.noProxy",
74+
"http.proxyAuthorization",
75+
"http.proxyStrictSSL",
7376
] as const;
7477

7578
/**

‎src/api/proxy.ts‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,13 +14,17 @@ const DEFAULT_PORTS: Record<string, number> = {
1414

1515
/**
1616
* @param {string|object} url - The URL, or the result from url.parse.
17+
* @param {string} httpProxy - The proxy URL to use.
18+
* @param {string} noProxy - Primary no-proxy list (e.g., coder.proxyBypass).
19+
* @param {string} noProxyFallback - Fallback no-proxy list (e.g., http.noProxy).
1720
* @return {string} The URL of the proxy that should handle the request to the
1821
* given URL. If no proxy is set, this will be an empty string.
1922
*/
2023
export function getProxyForUrl(
2124
url: string,
2225
httpProxy: string | null | undefined,
2326
noProxy: string | null | undefined,
27+
noProxyFallback?: string | null,
2428
): string {
2529
const parsedUrl = typeof url === "string" ? parseUrl(url) : url || {};
2630
let proto = parsedUrl.protocol;
@@ -36,7 +40,7 @@ export function getProxyForUrl(
3640
hostname = hostname.replace(/:\d*$/, "");
3741
const port =
3842
(portRaw && Number.parseInt(portRaw)) || DEFAULT_PORTS[proto] || 0;
39-
if (!shouldProxy(hostname, port, noProxy)) {
43+
if (!shouldProxy(hostname, port, noProxy, noProxyFallback)) {
4044
return ""; // Don't proxy URLs that match NO_PROXY.
4145
}
4246

@@ -46,7 +50,7 @@ export function getProxyForUrl(
4650
getEnv(proto + "_proxy") ||
4751
getEnv("npm_config_proxy") ||
4852
getEnv("all_proxy");
49-
if (proxy?.includes("://")) {
53+
if (proxy && !proxy.includes("://")) {
5054
// Missing scheme in proxy, default to the requested URL's scheme.
5155
proxy = proto + "://" + proxy;
5256
}
@@ -58,16 +62,20 @@ export function getProxyForUrl(
5862
*
5963
* @param {string} hostname - The host name of the URL.
6064
* @param {number} port - The effective port of the URL.
65+
* @param {string} noProxy - Primary no-proxy list (e.g., coder.proxyBypass).
66+
* @param {string} noProxyFallback - Fallback no-proxy list (e.g., http.noProxy).
6167
* @returns {boolean} Whether the given URL should be proxied.
6268
* @private
6369
*/
6470
function shouldProxy(
6571
hostname: string,
6672
port: number,
6773
noProxy: string | null | undefined,
74+
noProxyFallback?: string | null,
6875
): boolean {
6976
const NO_PROXY = (
7077
noProxy ||
78+
noProxyFallback ||
7179
getEnv("npm_config_no_proxy") ||
7280
getEnv("no_proxy")
7381
).toLowerCase();

‎src/api/utils.ts‎

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import { getProxyForUrl } from "./proxy";
1111
* If mTLS is in use (as specified by the cert or key files being set) then
1212
* token authorization is disabled. Otherwise, it is enabled.
1313
*/
14-
export function needToken(cfg: WorkspaceConfiguration): boolean {
14+
export function needToken(cfg: Pick<WorkspaceConfiguration, "get">): boolean {
1515
const certFile = expandPath(
1616
String(cfg.get("coder.tlsCertFile") ?? "").trim(),
1717
);
@@ -24,9 +24,13 @@ export function needToken(cfg: WorkspaceConfiguration): boolean {
2424
* Configures proxy, TLS certificates, and security options.
2525
*/
2626
export async function createHttpAgent(
27-
cfg: WorkspaceConfiguration,
27+
cfg: Pick<WorkspaceConfiguration, "get">,
2828
): Promise<ProxyAgent> {
29-
const insecure = Boolean(cfg.get("coder.insecure"));
29+
const insecure = cfg.get<boolean>("coder.insecure", false);
30+
const proxyStrictSSL = cfg.get<boolean>("http.proxyStrictSSL", true);
31+
const proxyAuthorization = cfg.get<string>("http.proxyAuthorization");
32+
const httpNoProxy = cfg.get<string>("http.noProxy");
33+
3034
const certFile = expandPath(
3135
String(cfg.get("coder.tlsCertFile") ?? "").trim(),
3236
);
@@ -40,21 +44,29 @@ export async function createHttpAgent(
4044
caFile === "" ? Promise.resolve(undefined) : fs.readFile(caFile),
4145
]);
4246

47+
// Build proxy authorization header if configured.
48+
const headers: Record<string, string> | undefined = proxyAuthorization
49+
? { "Proxy-Authorization": proxyAuthorization }
50+
: undefined;
51+
4352
return new ProxyAgent({
4453
// Called each time a request is made.
4554
getProxyForUrl: (url: string) => {
4655
return getProxyForUrl(
4756
url,
4857
cfg.get("http.proxy"),
4958
cfg.get("coder.proxyBypass"),
59+
httpNoProxy,
5060
);
5161
},
62+
headers,
5263
cert,
5364
key,
5465
ca,
5566
servername: altHost === "" ? undefined : altHost,
56-
// rejectUnauthorized defaults to true, so we need to explicitly set it to
57-
// false if we want to allow self-signed certificates.
58-
rejectUnauthorized: !insecure,
67+
// TLS verification is disabled if either:
68+
// - http.proxyStrictSSL is false (VS Code's proxy SSL setting)
69+
// - coder.insecure is true (backward compatible override for Coder server)
70+
rejectUnauthorized: proxyStrictSSL && !insecure,
5971
});
6072
}

‎test/unit/api/proxy.test.ts‎

Lines changed: 257 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,257 @@
1+
import { describe, it, expect, beforeEach, afterEach, vi } from "vitest";
2+
3+
import { getProxyForUrl } from "@/api/proxy";
4+
5+
describe("proxy", () => {
6+
const proxy = "http://proxy.example.com:8080";
7+
8+
beforeEach(() => {
9+
vi.unstubAllEnvs();
10+
});
11+
12+
afterEach(() => {
13+
vi.unstubAllEnvs();
14+
});
15+
16+
describe("getProxyForUrl", () => {
17+
describe("proxy resolution", () => {
18+
it("returns httpProxy when provided", () => {
19+
expect(getProxyForUrl("https://example.com", proxy, null)).toBe(proxy);
20+
});
21+
22+
it("falls back to environment variables when httpProxy is null", () => {
23+
vi.stubEnv("HTTPS_PROXY", "http://env-proxy.example.com:8080");
24+
expect(getProxyForUrl("https://example.com", null, null)).toBe(
25+
"http://env-proxy.example.com:8080",
26+
);
27+
});
28+
29+
it("returns empty string when no proxy is configured", () => {
30+
const proxyEnvVars = [
31+
"HTTPS_PROXY",
32+
"https_proxy",
33+
"HTTP_PROXY",
34+
"http_proxy",
35+
"ALL_PROXY",
36+
"all_proxy",
37+
"npm_config_https_proxy",
38+
"npm_config_proxy",
39+
];
40+
proxyEnvVars.forEach((v) => vi.stubEnv(v, ""));
41+
42+
expect(getProxyForUrl("https://example.com", null, null)).toBe("");
43+
});
44+
45+
it("returns empty string for invalid URLs", () => {
46+
expect(getProxyForUrl("invalid", proxy, null)).toBe("");
47+
});
48+
});
49+
50+
describe("noProxy handling", () => {
51+
interface NoProxyBypassCase {
52+
name: string;
53+
noProxy: string;
54+
url: string;
55+
}
56+
57+
it.each<NoProxyBypassCase>([
58+
{
59+
name: "exact match",
60+
noProxy: "internal.example.com",
61+
url: "https://internal.example.com",
62+
},
63+
{
64+
name: "wildcard",
65+
noProxy: "*.internal.example.com",
66+
url: "https://api.internal.example.com",
67+
},
68+
{
69+
name: "suffix",
70+
noProxy: ".example.com",
71+
url: "https://api.example.com",
72+
},
73+
{
74+
name: "wildcard *",
75+
noProxy: "*",
76+
url: "https://any.domain.com",
77+
},
78+
{
79+
name: "port-specific",
80+
noProxy: "example.com:8443",
81+
url: "https://example.com:8443",
82+
},
83+
])(
84+
"bypasses proxy when hostname matches noProxy ($name)",
85+
({ noProxy, url }) => {
86+
expect(getProxyForUrl(url, proxy, noProxy)).toBe("");
87+
},
88+
);
89+
90+
it("proxies when hostname does not match noProxy", () => {
91+
expect(
92+
getProxyForUrl("https://external.com", proxy, "internal.example.com"),
93+
).toBe(proxy);
94+
});
95+
96+
it("proxies when port does not match noProxy port", () => {
97+
expect(
98+
getProxyForUrl("https://example.com:443", proxy, "example.com:8443"),
99+
).toBe(proxy);
100+
});
101+
102+
it("handles multiple entries in noProxy (comma-separated)", () => {
103+
const noProxy = "localhost,127.0.0.1,.internal.com";
104+
105+
expect(getProxyForUrl("https://localhost", proxy, noProxy)).toBe("");
106+
expect(getProxyForUrl("https://127.0.0.1", proxy, noProxy)).toBe("");
107+
expect(getProxyForUrl("https://api.internal.com", proxy, noProxy)).toBe(
108+
"",
109+
);
110+
expect(getProxyForUrl("https://external.com", proxy, noProxy)).toBe(
111+
proxy,
112+
);
113+
});
114+
});
115+
116+
describe("noProxy fallback chain", () => {
117+
const targetUrl = "https://internal.example.com";
118+
const targetHost = "internal.example.com";
119+
120+
interface NoProxyFallbackCase {
121+
name: string;
122+
noProxy: string | null;
123+
noProxyFallback: string;
124+
}
125+
126+
it.each<NoProxyFallbackCase>([
127+
{
128+
name: "noProxy (coder.proxyBypass)",
129+
noProxy: targetHost,
130+
noProxyFallback: "other.example.com",
131+
},
132+
{
133+
name: "noProxyFallback when noProxy is null",
134+
noProxy: null,
135+
noProxyFallback: targetHost,
136+
},
137+
{
138+
name: "noProxyFallback when noProxy is empty",
139+
noProxy: "",
140+
noProxyFallback: targetHost,
141+
},
142+
])("uses $name", ({ noProxy, noProxyFallback }) => {
143+
expect(getProxyForUrl(targetUrl, proxy, noProxy, noProxyFallback)).toBe(
144+
"",
145+
);
146+
});
147+
148+
interface EnvVarFallbackCase {
149+
name: string;
150+
envVar: string;
151+
}
152+
153+
it.each<EnvVarFallbackCase>([
154+
{ name: "npm_config_no_proxy", envVar: "npm_config_no_proxy" },
155+
{ name: "NO_PROXY", envVar: "NO_PROXY" },
156+
{ name: "no_proxy (lowercase)", envVar: "no_proxy" },
157+
])("falls back to $name env var", ({ envVar }) => {
158+
// Clear all no_proxy env vars first
159+
vi.stubEnv("npm_config_no_proxy", "");
160+
vi.stubEnv("NO_PROXY", "");
161+
vi.stubEnv("no_proxy", "");
162+
163+
vi.stubEnv(envVar, targetHost);
164+
expect(getProxyForUrl(targetUrl, proxy, null, null)).toBe("");
165+
});
166+
167+
it("prioritizes noProxy over noProxyFallback over env vars", () => {
168+
vi.stubEnv("NO_PROXY", "env.example.com");
169+
170+
// noProxy takes precedence
171+
expect(
172+
getProxyForUrl(
173+
"https://primary.example.com",
174+
proxy,
175+
"primary.example.com",
176+
"fallback.example.com",
177+
),
178+
).toBe("");
179+
180+
// noProxyFallback is used when noProxy is null
181+
expect(
182+
getProxyForUrl(
183+
"https://fallback.example.com",
184+
proxy,
185+
null,
186+
"fallback.example.com",
187+
),
188+
).toBe("");
189+
190+
// env var is used when both are null
191+
expect(
192+
getProxyForUrl("https://env.example.com", proxy, null, null),
193+
).toBe("");
194+
});
195+
});
196+
197+
describe("protocol and port handling", () => {
198+
interface ProtocolCase {
199+
protocol: string;
200+
url: string;
201+
}
202+
203+
it.each<ProtocolCase>([
204+
{ protocol: "http://", url: "http://example.com" },
205+
{ protocol: "https://", url: "https://example.com" },
206+
{ protocol: "ws://", url: "ws://example.com" },
207+
{ protocol: "wss://", url: "wss://example.com" },
208+
])("handles $protocol URLs", ({ url }) => {
209+
expect(getProxyForUrl(url, proxy, null)).toBe(proxy);
210+
});
211+
212+
interface DefaultPortCase {
213+
protocol: string;
214+
url: string;
215+
defaultPort: number;
216+
}
217+
218+
it.each<DefaultPortCase>([
219+
{ protocol: "HTTP", url: "http://example.com", defaultPort: 80 },
220+
{ protocol: "HTTPS", url: "https://example.com", defaultPort: 443 },
221+
{ protocol: "WS", url: "ws://example.com", defaultPort: 80 },
222+
{ protocol: "WSS", url: "wss://example.com", defaultPort: 443 },
223+
])(
224+
"uses default port $defaultPort for $protocol",
225+
({ url, defaultPort }) => {
226+
expect(getProxyForUrl(url, proxy, `example.com:${defaultPort}`)).toBe(
227+
"",
228+
);
229+
},
230+
);
231+
});
232+
233+
describe("proxy scheme handling", () => {
234+
it("adds scheme to proxy URL when missing", () => {
235+
expect(
236+
getProxyForUrl("https://example.com", "proxy.example.com:8080", null),
237+
).toBe("https://proxy.example.com:8080");
238+
});
239+
240+
it("uses request scheme when proxy has no scheme", () => {
241+
expect(
242+
getProxyForUrl("http://example.com", "proxy.example.com:8080", null),
243+
).toBe("http://proxy.example.com:8080");
244+
});
245+
246+
it("preserves existing scheme in proxy URL", () => {
247+
expect(
248+
getProxyForUrl(
249+
"https://example.com",
250+
"http://proxy.example.com:8080",
251+
null,
252+
),
253+
).toBe("http://proxy.example.com:8080");
254+
});
255+
});
256+
});
257+
});

0 commit comments

Comments
 (0)