Skip to content

Commit 0287d4a

Browse files
nxmxbbdnxmxbbd
committed
fix(doctor): suppress env proxy warning when NO_PROXY=* bypasses every target
matchesNoProxy() treats a bare * NO_PROXY value as bypass-everything, so enabling tools.web.fetch.useTrustedEnvProxy in that environment would not route any web_fetch request through the proxy and the remediation was misleading. Gate the doctor/status/security-audit diagnostic on the same all-bypass contract (lower-case no_proxy shadows upper-case, * entries inside lists are still per-entry skipped and keep warning), and cover the negative and permutation cases across all three surfaces.
1 parent 6dafc2c commit 0287d4a

4 files changed

Lines changed: 116 additions & 1 deletion

File tree

‎src/commands/doctor-security.test.ts‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,8 @@ describe("noteSecurityWarnings gateway exposure", () => {
4444
let prevHttpsProxyLower: string | undefined;
4545
let prevAllProxy: string | undefined;
4646
let prevAllProxyLower: string | undefined;
47+
let prevNoProxy: string | undefined;
48+
let prevNoProxyLower: string | undefined;
4749

4850
beforeEach(() => {
4951
note.mockClear();
@@ -60,6 +62,8 @@ describe("noteSecurityWarnings gateway exposure", () => {
6062
prevHttpsProxyLower = process.env.https_proxy;
6163
prevAllProxy = process.env.ALL_PROXY;
6264
prevAllProxyLower = process.env.all_proxy;
65+
prevNoProxy = process.env.NO_PROXY;
66+
prevNoProxyLower = process.env.no_proxy;
6367
delete process.env.OPENCLAW_GATEWAY_TOKEN;
6468
delete process.env.OPENCLAW_GATEWAY_PASSWORD;
6569
delete process.env.OPENCLAW_SERVICE_KIND;
@@ -69,6 +73,8 @@ describe("noteSecurityWarnings gateway exposure", () => {
6973
delete process.env.https_proxy;
7074
delete process.env.ALL_PROXY;
7175
delete process.env.all_proxy;
76+
delete process.env.NO_PROXY;
77+
delete process.env.no_proxy;
7278
});
7379

7480
afterEach(() => {
@@ -122,6 +128,16 @@ describe("noteSecurityWarnings gateway exposure", () => {
122128
} else {
123129
process.env.all_proxy = prevAllProxyLower;
124130
}
131+
if (prevNoProxy === undefined) {
132+
delete process.env.NO_PROXY;
133+
} else {
134+
process.env.NO_PROXY = prevNoProxy;
135+
}
136+
if (prevNoProxyLower === undefined) {
137+
delete process.env.no_proxy;
138+
} else {
139+
process.env.no_proxy = prevNoProxyLower;
140+
}
125141
});
126142

127143
const lastMessage = () => String(note.mock.calls[note.mock.calls.length - 1]?.[0] ?? "");
@@ -377,6 +393,56 @@ describe("noteSecurityWarnings gateway exposure", () => {
377393
expect(message).not.toContain("tools.web.fetch.useTrustedEnvProxy is not enabled");
378394
});
379395

396+
it("does not warn when NO_PROXY=* bypasses every target (#95560)", async () => {
397+
process.env.HTTP_PROXY = "http://127.0.0.1:7897";
398+
process.env.NO_PROXY = "*";
399+
const cfg = {} as OpenClawConfig;
400+
await noteSecurityWarnings(cfg);
401+
const message = lastMessage();
402+
expect(message).not.toContain("tools.web.fetch.useTrustedEnvProxy is not enabled");
403+
expect(message).not.toContain("web_fetch will use direct connections");
404+
});
405+
406+
it("does not warn when lowercase no_proxy=* bypasses every target (#95560)", async () => {
407+
process.env.HTTP_PROXY = "http://127.0.0.1:7897";
408+
process.env.no_proxy = "*";
409+
const cfg = {} as OpenClawConfig;
410+
await noteSecurityWarnings(cfg);
411+
const message = lastMessage();
412+
expect(message).not.toContain("tools.web.fetch.useTrustedEnvProxy is not enabled");
413+
});
414+
415+
it("still warns when a * entry sits inside a NO_PROXY list (matchesNoProxy skips it) (#95560)", async () => {
416+
process.env.HTTP_PROXY = "http://127.0.0.1:7897";
417+
process.env.NO_PROXY = "*,localhost";
418+
const cfg = {} as OpenClawConfig;
419+
await noteSecurityWarnings(cfg);
420+
const message = lastMessage();
421+
expect(message).toContain("HTTP_PROXY");
422+
expect(message).toContain("tools.web.fetch.useTrustedEnvProxy");
423+
});
424+
425+
it("still warns when NO_PROXY only excludes some hosts (#95560)", async () => {
426+
process.env.HTTP_PROXY = "http://127.0.0.1:7897";
427+
process.env.NO_PROXY = "internal.example.com";
428+
const cfg = {} as OpenClawConfig;
429+
await noteSecurityWarnings(cfg);
430+
const message = lastMessage();
431+
expect(message).toContain("HTTP_PROXY");
432+
expect(message).toContain("tools.web.fetch.useTrustedEnvProxy");
433+
});
434+
435+
it("still warns when blank lowercase no_proxy shadows uppercase NO_PROXY=* (#95560)", async () => {
436+
process.env.HTTP_PROXY = "http://127.0.0.1:7897";
437+
process.env.NO_PROXY = "*";
438+
process.env.no_proxy = "";
439+
const cfg = {} as OpenClawConfig;
440+
await noteSecurityWarnings(cfg);
441+
const message = lastMessage();
442+
expect(message).toContain("HTTP_PROXY");
443+
expect(message).toContain("tools.web.fetch.useTrustedEnvProxy");
444+
});
445+
380446
it("warns about HTTP_PROXY but omits ALL_PROXY from the env list when both are set (#95560)", async () => {
381447
process.env.HTTP_PROXY = "http://127.0.0.1:7897";
382448
process.env.ALL_PROXY = "socks5://127.0.0.1:7897";

‎src/commands/status.scan.config-shared.test.ts‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -325,6 +325,33 @@ describe("status.scan.config-shared", () => {
325325
expect(result.secretDiagnostics).toStrictEqual([]);
326326
});
327327

328+
it("does not add a proxy diagnostic when NO_PROXY=* bypasses every target (#95560)", async () => {
329+
const sourceConfig = {};
330+
const readConfigSnapshot = vi.fn(async () => ({
331+
config: sourceConfig,
332+
sourceConfig,
333+
}));
334+
const resolveConfig = vi.fn(async () => ({
335+
resolvedConfig: sourceConfig,
336+
diagnostics: [],
337+
}));
338+
339+
const result = await loadStatusScanCommandConfig({
340+
commandName: "status --json",
341+
readConfigSnapshot,
342+
resolveConfig,
343+
env: {
344+
VITEST: "true",
345+
HTTP_PROXY: "http://127.0.0.1:7897",
346+
HTTPS_PROXY: "http://127.0.0.1:7897",
347+
NO_PROXY: "*",
348+
},
349+
allowMissingConfigFastPath: true,
350+
});
351+
352+
expect(result.secretDiagnostics).toStrictEqual([]);
353+
});
354+
328355
it("adds a status diagnostic when lowercase http_proxy is set without useTrustedEnvProxy (#95560)", async () => {
329356
const sourceConfig = {};
330357
const readConfigSnapshot = vi.fn(async () => ({

‎src/gateway/web-fetch-proxy-source-conflict.ts‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ type WebFetchProxySourceConflict = {
3636

3737
/**
3838
* Returns a warning when HTTP/HTTPS proxy env vars are present, web_fetch is enabled,
39-
* and trusted-env proxy use is not opted in.
39+
* trusted-env proxy use is not opted in, and NO_PROXY does not bypass every target.
4040
*/
4141
export function resolveWebFetchProxySourceConflict(params: {
4242
cfg: OpenClawConfig;
@@ -52,6 +52,15 @@ export function resolveWebFetchProxySourceConflict(params: {
5252
return null;
5353
}
5454

55+
// Mirror matchesNoProxy()'s all-bypass contract: a bare `*` NO_PROXY value
56+
// (lower-case shadows upper-case, blanks included) bypasses the env proxy for
57+
// every target, so enabling useTrustedEnvProxy would not route anything and
58+
// the remediation would be misleading. `*` entries inside comma/whitespace
59+
// lists are skipped per-entry by that matcher, so partial lists still warn.
60+
if ((params.env.no_proxy ?? params.env.NO_PROXY) === "*") {
61+
return null;
62+
}
63+
5564
// Do not describe web_fetch connection behavior when the tool is explicitly disabled.
5665
if (params.cfg.tools?.web?.fetch?.enabled === false) {
5766
return null;

‎src/security/audit-gateway.test.ts‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -249,6 +249,19 @@ describe("security audit gateway config findings", () => {
249249
);
250250
});
251251

252+
it("does not warn about web_fetch proxy env when NO_PROXY=* bypasses every target (#95560)", () => {
253+
const cfg: OpenClawConfig = {};
254+
const findings = collectGatewayConfigFindings(cfg, cfg, {
255+
HTTP_PROXY: "http://127.0.0.1:7897",
256+
HTTPS_PROXY: "http://127.0.0.1:7897",
257+
NO_PROXY: "*",
258+
});
259+
260+
expect(hasFinding("tools.web.fetch.env_proxy_without_use_trusted_env_proxy", findings)).toBe(
261+
false,
262+
);
263+
});
264+
252265
it("warns when lowercase http_proxy is set but tools.web.fetch.useTrustedEnvProxy is not enabled (#95560)", () => {
253266
const cfg: OpenClawConfig = {};
254267
const findings = collectGatewayConfigFindings(cfg, cfg, {

0 commit comments

Comments
 (0)