Skip to content

Commit 73ed2df

Browse files
committed
refactor: simplify proxy env watching and document the approach
Derive the watched proxy settings from a single SSH_PROXY_SETTINGS source, collapse the redundant useLocalServer/useExecServer test matrix, and document why the proxy variables are propagated via process.env.
1 parent acd86f6 commit 73ed2df

3 files changed

Lines changed: 41 additions & 73 deletions

File tree

src/remote/environment.ts

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,13 @@
11
/**
2-
* Helpers for local process environment inherited by Remote-SSH children.
2+
* Sets HTTP_PROXY/HTTPS_PROXY/NO_PROXY on this extension host's process.env so
3+
* the spawned `coder ssh` ProxyCommand inherits them (the coder CLI reads them
4+
* like any Go HTTP client and has no proxy flag). We mutate process.env rather
5+
* than baking them into the SSH config so no credentialed proxy URL is written
6+
* to disk and multiple windows onto the same workspace stay independent.
37
*
4-
* This is best-effort: it affects processes spawned after these values are
5-
* applied. Modes such as `remote.SSH.useLocalServer` can reuse existing local
6-
* SSH processes, and `remote.SSH.useExecServer` changes the bootstrap path, so
7-
* already-running processes may need a reload/reconnect to inherit changes.
8+
* Best-effort: only processes spawned after it runs inherit the change. Notably
9+
* MS VS Code with `remote.SSH.useLocalServer=false` spawns ssh off a path that
10+
* does not inherit, so propagation there needs `useLocalServer=true`.
811
*/
912

1013
import { getProxyForUrl } from "../api/proxy";
@@ -15,6 +18,19 @@ export type SshProxyEnvironment = Partial<
1518
Record<"HTTP_PROXY" | "HTTPS_PROXY" | "NO_PROXY", string>
1619
>;
1720

21+
/**
22+
* The settings {@link getSshProxyEnvironment} reads, paired with display titles.
23+
* Watch these to prompt for a reload when the SSH proxy environment changes.
24+
*/
25+
export const SSH_PROXY_SETTINGS: ReadonlyArray<{
26+
setting: string;
27+
title: string;
28+
}> = [
29+
{ setting: "http.proxy", title: "HTTP Proxy" },
30+
{ setting: "http.noProxy", title: "HTTP No Proxy" },
31+
{ setting: "coder.proxyBypass", title: "Proxy Bypass" },
32+
];
33+
1834
type Environment = Record<string, string | undefined>;
1935
type PreviousValue = [key: string, existed: boolean, value: string | undefined];
2036

src/remote/remote.ts

Lines changed: 6 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ import {
6161
import { vscodeProposed } from "../vscodeProposed";
6262
import { WorkspaceMonitor } from "../workspace/workspaceMonitor";
6363

64-
import { applySshProxyEnvironment } from "./environment";
64+
import { applySshProxyEnvironment, SSH_PROXY_SETTINGS } from "./environment";
6565
import {
6666
SshConfig,
6767
type SshValues,
@@ -458,24 +458,11 @@ export class Remote {
458458
title: "SSH Flags",
459459
getValue: () => getSshFlags(vscode.workspace.getConfiguration()),
460460
},
461-
{
462-
setting: "http.proxy",
463-
title: "HTTP Proxy",
464-
getValue: () =>
465-
vscode.workspace.getConfiguration().get("http.proxy") ?? "",
466-
},
467-
{
468-
setting: "http.noProxy",
469-
title: "HTTP No Proxy",
470-
getValue: () =>
471-
vscode.workspace.getConfiguration().get("http.noProxy") ?? [],
472-
},
473-
{
474-
setting: "coder.proxyBypass",
475-
title: "Proxy Bypass",
476-
getValue: () =>
477-
vscode.workspace.getConfiguration().get("coder.proxyBypass") ?? "",
478-
},
461+
...SSH_PROXY_SETTINGS.map(({ setting, title }) => ({
462+
setting,
463+
title,
464+
getValue: () => vscode.workspace.getConfiguration().get(setting),
465+
})),
479466
];
480467
if (featureSet.proxyLogDirectory) {
481468
settingsToWatch.push({

test/unit/remote/environment.test.ts

Lines changed: 14 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -127,55 +127,20 @@ describe("applySshProxyEnvironment", () => {
127127
});
128128
});
129129

130-
interface RemoteSshModeCase {
131-
name: string;
132-
useExecServer: boolean;
133-
useLocalServer: boolean;
134-
}
135-
136-
it.each<RemoteSshModeCase>([
137-
{
138-
name: "local server and exec server",
139-
useLocalServer: true,
140-
useExecServer: true,
141-
},
142-
{
143-
name: "local server without exec server",
144-
useLocalServer: true,
145-
useExecServer: false,
146-
},
147-
{
148-
name: "terminal server and exec server",
149-
useLocalServer: false,
150-
useExecServer: true,
151-
},
152-
{
153-
name: "terminal server without exec server",
154-
useLocalServer: false,
155-
useExecServer: false,
156-
},
157-
])(
158-
"propagates proxy variables to new child processes with $name",
159-
({ useExecServer, useLocalServer }) => {
160-
const cfg = new MockConfigurationProvider();
161-
cfg.set("http.proxy", proxy);
162-
cfg.set("remote.SSH.useExecServer", useExecServer);
163-
cfg.set("remote.SSH.useLocalServer", useLocalServer);
164-
const applied = applySshProxyEnvironment(
165-
"https://coder.example.com",
166-
cfg,
167-
);
168-
169-
try {
170-
expect(getProxyEnvFromChild()).toEqual({
171-
http: proxy,
172-
https: proxy,
173-
});
174-
} finally {
175-
applied.dispose();
176-
}
177-
},
178-
);
130+
it("propagates proxy variables to newly spawned child processes", () => {
131+
const cfg = new MockConfigurationProvider();
132+
cfg.set("http.proxy", proxy);
133+
const applied = applySshProxyEnvironment("https://coder.example.com", cfg);
134+
135+
try {
136+
expect(getProxyEnvFromChild()).toEqual({
137+
http: proxy,
138+
https: proxy,
139+
});
140+
} finally {
141+
applied.dispose();
142+
}
143+
});
179144
});
180145

181146
interface ChildProxyEnvironment {

0 commit comments

Comments
 (0)