From 37f3a4c90a3b04273b83db9539450eb0643d3b2b Mon Sep 17 00:00:00 2001 From: Coco Sheng Date: Tue, 19 May 2026 13:56:59 -0400 Subject: [PATCH] fix(core): respect NO_PROXY in global fetch dispatcher (#27216) --- .prettierignore | 1 + .../core/src/ide/ide-connection-utils.test.ts | 34 +++++++- packages/core/src/tools/mcp-client.test.ts | 29 +++++++ packages/core/src/utils/fetch.test.ts | 81 +++++++++++++++++-- packages/core/src/utils/fetch.ts | 34 ++++++-- 5 files changed, 162 insertions(+), 17 deletions(-) diff --git a/.prettierignore b/.prettierignore index 78066b7e78..6d6781ce1f 100644 --- a/.prettierignore +++ b/.prettierignore @@ -23,3 +23,4 @@ Thumbs.db **/SKILL.md packages/sdk/test-data/*.json *.mdx +packages/vscode-ide-companion/NOTICES.txt diff --git a/packages/core/src/ide/ide-connection-utils.test.ts b/packages/core/src/ide/ide-connection-utils.test.ts index a2d554b7a6..f0e719e992 100644 --- a/packages/core/src/ide/ide-connection-utils.test.ts +++ b/packages/core/src/ide/ide-connection-utils.test.ts @@ -37,6 +37,12 @@ vi.mock('node:fs', async (importOriginal) => { }; }); vi.mock('node:os'); +vi.mock('undici', () => ({ + EnvHttpProxyAgent: vi.fn(), + fetch: vi.fn(), + setGlobalDispatcher: vi.fn(), + Agent: vi.fn(), +})); describe('ide-connection-utils', () => { beforeEach(() => { @@ -698,12 +704,36 @@ describe('ide-connection-utils', () => { }); describe('createProxyAwareFetch', () => { - it('should return a proxy-aware fetcher function', async () => { + it('should return a proxy-aware fetcher function that respects NO_PROXY and includes ideServerHost', async () => { const { createProxyAwareFetch } = await import( './ide-connection-utils.js' ); - const fetcher = await createProxyAwareFetch('127.0.0.1'); + const { EnvHttpProxyAgent } = await import('undici'); + const ideServerHost = '127.0.0.1'; + const existingNoProxy = 'google.com,example.com'; + vi.stubEnv('NO_PROXY', existingNoProxy); + + const fetcher = await createProxyAwareFetch(ideServerHost); expect(typeof fetcher).toBe('function'); + + expect(EnvHttpProxyAgent).toHaveBeenCalledWith({ + noProxy: `${existingNoProxy},${ideServerHost}`, + }); + }); + + it('should handle missing NO_PROXY when creating proxy-aware fetcher', async () => { + const { createProxyAwareFetch } = await import( + './ide-connection-utils.js' + ); + const { EnvHttpProxyAgent } = await import('undici'); + const ideServerHost = 'host.docker.internal'; + vi.stubEnv('NO_PROXY', ''); + + await createProxyAwareFetch(ideServerHost); + + expect(EnvHttpProxyAgent).toHaveBeenCalledWith({ + noProxy: ideServerHost, + }); }); }); }); diff --git a/packages/core/src/tools/mcp-client.test.ts b/packages/core/src/tools/mcp-client.test.ts index 7f0b837cba..1838032e95 100644 --- a/packages/core/src/tools/mcp-client.test.ts +++ b/packages/core/src/tools/mcp-client.test.ts @@ -74,6 +74,12 @@ let MOCK_CONTEXT: McpContext = MOCK_CONTEXT_DEFAULT; vi.mock('@modelcontextprotocol/sdk/client/stdio.js'); vi.mock('@modelcontextprotocol/sdk/client/index.js'); vi.mock('@google/genai'); +vi.mock('undici', () => ({ + EnvHttpProxyAgent: vi.fn(), + fetch: vi.fn(), + setGlobalDispatcher: vi.fn(), + Agent: vi.fn(), +})); vi.mock('../mcp/oauth-provider.js'); vi.mock('../mcp/oauth-token-storage.js'); vi.mock('../mcp/oauth-utils.js'); @@ -1779,6 +1785,29 @@ describe('mcp-client', () => { }); describe('createTransport', () => { + it('should create an HTTP transport that respects NO_PROXY', async () => { + const { createTransport } = await import('./mcp-client.js'); + const { EnvHttpProxyAgent } = await import('undici'); + const noProxyValue = 'localhost,127.0.0.1'; + vi.stubEnv('NO_PROXY', noProxyValue); + + await createTransport( + 'test-server', + { + url: 'http://test-server', + type: 'http', + }, + false, + MOCK_CONTEXT, + ); + + expect(EnvHttpProxyAgent).toHaveBeenCalledWith( + expect.objectContaining({ + noProxy: noProxyValue, + }), + ); + }); + describe('should connect via httpUrl', () => { it('uses MCP SDK authProvider token() path for oauth-enabled servers', async () => { const mockGetValidTokenWithMetadata = vi.fn().mockResolvedValue({ diff --git a/packages/core/src/utils/fetch.test.ts b/packages/core/src/utils/fetch.test.ts index f14f75eea3..ff438910c6 100644 --- a/packages/core/src/utils/fetch.test.ts +++ b/packages/core/src/utils/fetch.test.ts @@ -10,16 +10,16 @@ import * as dnsPromises from 'node:dns/promises'; import type { LookupAddress, LookupAllOptions } from 'node:dns'; import ipaddr from 'ipaddr.js'; -const { setGlobalDispatcher, Agent, ProxyAgent } = vi.hoisted(() => ({ +const { setGlobalDispatcher, Agent, EnvHttpProxyAgent } = vi.hoisted(() => ({ setGlobalDispatcher: vi.fn(), Agent: vi.fn(), - ProxyAgent: vi.fn(), + EnvHttpProxyAgent: vi.fn(), })); vi.mock('undici', () => ({ setGlobalDispatcher, Agent, - ProxyAgent, + EnvHttpProxyAgent, })); vi.mock('node:dns/promises', () => ({ @@ -33,6 +33,7 @@ const { isAddressPrivate, fetchWithTimeout, setGlobalProxy, + createSafeProxyAgent, } = await import('./fetch.js'); interface ErrorWithCode extends Error { code?: string; @@ -54,6 +55,8 @@ describe('fetch utils', () => { } return [{ address: '93.184.216.34', family: 4 }]; }); + vi.unstubAllEnvs(); + updateGlobalFetchTimeouts(60000); }); afterEach(() => { @@ -237,17 +240,81 @@ describe('fetch utils', () => { }); describe('setGlobalProxy', () => { - it('should configure ProxyAgent with experiment flag timeout', () => { - const proxyUrl = 'http://proxy.example.com'; + it('should configure EnvHttpProxyAgent with experiment flag timeout and noProxy', () => { + const proxyUrl = ' http://proxy.example.com '; + const noProxyValue = ' localhost,127.0.0.1 '; + vi.stubEnv('NO_PROXY', noProxyValue); + updateGlobalFetchTimeouts(45773134); setGlobalProxy(proxyUrl); - expect(ProxyAgent).toHaveBeenCalledWith({ - uri: proxyUrl, + expect(EnvHttpProxyAgent).toHaveBeenCalledWith({ + httpProxy: 'http://proxy.example.com', + httpsProxy: 'http://proxy.example.com', + noProxy: 'localhost,127.0.0.1', headersTimeout: 45773134, bodyTimeout: 300000, }); expect(setGlobalDispatcher).toHaveBeenCalled(); }); + + it('should fall back to no_proxy if NO_PROXY is not set', () => { + const proxyUrl = 'http://proxy.example.com'; + const noProxyValue = 'localhost,127.0.0.1'; + vi.stubEnv('no_proxy', noProxyValue); + + setGlobalProxy(proxyUrl); + + expect(EnvHttpProxyAgent).toHaveBeenCalledWith( + expect.objectContaining({ + noProxy: noProxyValue, + }), + ); + }); + + it('should handle empty NO_PROXY', () => { + const proxyUrl = 'http://proxy.example.com'; + vi.stubEnv('NO_PROXY', ''); + + setGlobalProxy(proxyUrl); + + expect(EnvHttpProxyAgent).toHaveBeenCalledWith( + expect.objectContaining({ + noProxy: '', + }), + ); + }); + + it('should handle multi-entry NO_PROXY with trimming', () => { + const proxyUrl = 'http://proxy.example.com'; + const noProxyValue = ' google.com, 127.0.0.1 , localhost '; + vi.stubEnv('NO_PROXY', noProxyValue); + + setGlobalProxy(proxyUrl); + + expect(EnvHttpProxyAgent).toHaveBeenCalledWith( + expect.objectContaining({ + noProxy: 'google.com, 127.0.0.1 , localhost', + }), + ); + }); + }); + + describe('createSafeProxyAgent', () => { + it('should create an EnvHttpProxyAgent with trimmed values and default timeouts', () => { + const proxyUrl = ' http://proxy.example.com '; + const noProxyValue = ' localhost,127.0.0.1 '; + vi.stubEnv('NO_PROXY', noProxyValue); + + createSafeProxyAgent(proxyUrl); + + expect(EnvHttpProxyAgent).toHaveBeenCalledWith({ + httpProxy: 'http://proxy.example.com', + httpsProxy: 'http://proxy.example.com', + noProxy: 'localhost,127.0.0.1', + headersTimeout: 60000, + bodyTimeout: 300000, + }); + }); }); }); diff --git a/packages/core/src/utils/fetch.ts b/packages/core/src/utils/fetch.ts index ff22df0a34..e0c34cf4f6 100644 --- a/packages/core/src/utils/fetch.ts +++ b/packages/core/src/utils/fetch.ts @@ -6,7 +6,7 @@ import { getErrorMessage, isAbortError } from './errors.js'; import { URL } from 'node:url'; -import { Agent, ProxyAgent, setGlobalDispatcher } from 'undici'; +import { Agent, EnvHttpProxyAgent, setGlobalDispatcher } from 'undici'; import ipaddr from 'ipaddr.js'; import { lookup } from 'node:dns/promises'; @@ -169,11 +169,21 @@ export async function isPrivateIpAsync(url: string): Promise { } /** - * Creates an undici ProxyAgent that incorporates safe DNS lookup. + * Creates an undici EnvHttpProxyAgent that incorporates safe DNS lookup. */ -export function createSafeProxyAgent(proxyUrl: string): ProxyAgent { - return new ProxyAgent({ - uri: proxyUrl, +export function createSafeProxyAgent(proxyUrl: string): EnvHttpProxyAgent { + const trimmedProxy = proxyUrl.trim(); + const noProxy = ( + process.env['NO_PROXY'] ?? + process.env['no_proxy'] ?? + '' + )?.trim(); + return new EnvHttpProxyAgent({ + httpProxy: trimmedProxy, + httpsProxy: trimmedProxy, + noProxy, + headersTimeout: defaultHeadersTimeout, + bodyTimeout: defaultBodyTimeout, }); } @@ -220,10 +230,18 @@ export async function fetchWithTimeout( } export function setGlobalProxy(proxy: string) { - currentProxy = proxy; + const trimmedProxy = proxy.trim(); + currentProxy = trimmedProxy; + const noProxy = ( + process.env['NO_PROXY'] ?? + process.env['no_proxy'] ?? + '' + )?.trim(); setGlobalDispatcher( - new ProxyAgent({ - uri: proxy, + new EnvHttpProxyAgent({ + httpProxy: trimmedProxy, + httpsProxy: trimmedProxy, + noProxy, headersTimeout: defaultHeadersTimeout, bodyTimeout: defaultBodyTimeout, }),