diff --git a/package-lock.json b/package-lock.json index 7faf9bf..d34046f 100644 --- a/package-lock.json +++ b/package-lock.json @@ -17,6 +17,7 @@ "keytar": "^7.9.0", "ora": "^9.0.0", "proper-lockfile": "^4.1.2", + "undici": "^7.19.2", "uuid": "^13.0.0" }, "bin": { @@ -11127,10 +11128,9 @@ "license": "MIT" }, "node_modules/undici": { - "version": "7.16.0", - "resolved": "https://registry.npmjs.org/undici/-/undici-7.16.0.tgz", - "integrity": "sha512-QEg3HPMll0o3t2ourKwOeUAZ159Kn9mx5pnzHRQO8+Wixmh88YdZRiIwat0iNzNNXn0yoEtXJqFpyW7eM8BV7g==", - "dev": true, + "version": "7.19.2", + "resolved": "https://npm.artifacts.indeed.tech/npm/undici/-/undici-7.19.2.tgz", + "integrity": "sha512-4VQSpGEGsWzk0VYxyB/wVX/Q7qf9t5znLRgs0dzszr9w9Fej/8RVNQ+S20vdXSAyra/bJ7ZQfGv6ZMj7UEbzSg==", "license": "MIT", "engines": { "node": ">=20.18.1" diff --git a/package.json b/package.json index 55f38d5..71c2d26 100644 --- a/package.json +++ b/package.json @@ -59,6 +59,7 @@ "keytar": "^7.9.0", "ora": "^9.0.0", "proper-lockfile": "^4.1.2", + "undici": "^7.19.2", "uuid": "^13.0.0" }, "devDependencies": { diff --git a/src/core/factory.ts b/src/core/factory.ts index e377b9b..beb2284 100644 --- a/src/core/factory.ts +++ b/src/core/factory.ts @@ -127,7 +127,7 @@ export async function createMcpClient(options: CreateMcpClientOptions): Promise< if (options.mcpSessionId) { transportOptions.mcpSessionId = options.mcpSessionId; } - const transport = createTransportFromConfig(options.serverConfig, transportOptions); + const transport = await createTransportFromConfig(options.serverConfig, transportOptions); await client.connect(transport); } diff --git a/src/core/transports.ts b/src/core/transports.ts index ffc1ace..e055b8f 100644 --- a/src/core/transports.ts +++ b/src/core/transports.ts @@ -27,6 +27,7 @@ export { export type { OAuthClientProvider } from '@modelcontextprotocol/sdk/client/auth.js'; import type { Transport } from '@modelcontextprotocol/sdk/shared/transport.js'; +import type { FetchLike } from '@modelcontextprotocol/sdk/shared/transport.js'; import type { OAuthClientProvider } from '@modelcontextprotocol/sdk/client/auth.js'; import { StdioClientTransport, @@ -40,6 +41,45 @@ import { createLogger, getVerbose } from '../lib/logger.js'; import type { ServerConfig } from '../lib/types.js'; import { ClientError } from '../lib/errors.js'; +/** + * Create a proxy-aware fetch function if HTTPS_PROXY or https_proxy is set. + * This allows mcpc to work in environments where network access is routed through + * a proxy (e.g., Claude Code's sandbox, corporate proxies). + * + * Node.js native fetch (undici) does not respect proxy environment variables, + * so we need to explicitly configure a ProxyAgent dispatcher. + */ +async function createProxyAwareFetch(): Promise { + const proxyUrl = process.env.https_proxy || process.env.HTTPS_PROXY; + if (!proxyUrl) { + return undefined; + } + + const logger = createLogger('ProxyFetch'); + logger.debug(`Configuring proxy-aware fetch with proxy: ${proxyUrl}`); + + try { + // Dynamically import undici to create a ProxyAgent + // undici is the HTTP client that powers Node.js native fetch + const { ProxyAgent, fetch: undiciFetch } = await import('undici'); + const proxyAgent = new ProxyAgent(proxyUrl); + + // Return a fetch function that uses the proxy dispatcher + const proxyFetch: FetchLike = (input, init) => { + return undiciFetch(input, { + ...init, + dispatcher: proxyAgent, + }) as Promise; + }; + + logger.debug('Proxy-aware fetch configured successfully'); + return proxyFetch; + } catch (error) { + logger.debug(`Failed to configure proxy-aware fetch: ${error}`); + return undefined; + } +} + /** * Create a stdio transport for a local MCP server */ @@ -60,10 +100,10 @@ export function createStdioTransport(config: StdioServerParameters): Transport { /** * Create a Streamable HTTP transport for a remote MCP server */ -export function createStreamableHttpTransport( +export async function createStreamableHttpTransport( url: string, options: Omit = {} -): Transport { +): Promise { const logger = createLogger('StreamableHttpTransport'); logger.debug('Creating Streamable HTTP transport', { url }); logger.debug('Transport options:', { @@ -79,10 +119,21 @@ export function createStreamableHttpTransport( maxRetries: 10, // Max 10 reconnection attempts }; - const transport = new StreamableHTTPClientTransport(new URL(url), { + // Create proxy-aware fetch if proxy environment variables are set + const proxyFetch = await createProxyAwareFetch(); + + const transportOptions: StreamableHTTPClientTransportOptions = { reconnectionOptions: defaultReconnectionOptions, ...options, - }); + }; + + // Use proxy-aware fetch if available + if (proxyFetch) { + transportOptions.fetch = proxyFetch; + logger.debug('Using proxy-aware fetch for HTTP transport'); + } + + const transport = new StreamableHTTPClientTransport(new URL(url), transportOptions); // Verify authProvider is correctly attached // @ts-expect-error accessing private property for debugging @@ -124,10 +175,10 @@ export interface CreateTransportOptions { /** * Create a transport from a generic transport configuration */ -export function createTransportFromConfig( +export async function createTransportFromConfig( config: ServerConfig, options: CreateTransportOptions = {} -): Transport { +): Promise { // Stdio transport if (config.command) { const stdioConfig: StdioServerParameters = { @@ -178,7 +229,7 @@ export function createTransportFromConfig( }; } - return createStreamableHttpTransport(config.url, transportOptions); + return await createStreamableHttpTransport(config.url, transportOptions); } throw new ClientError('Invalid ServerConfig: must have either url or command'); diff --git a/test/unit/core/factory.test.ts b/test/unit/core/factory.test.ts index b35c96a..78ed407 100644 --- a/test/unit/core/factory.test.ts +++ b/test/unit/core/factory.test.ts @@ -5,9 +5,9 @@ import { McpClient } from '../../../src/core/mcp-client.js'; import { createMcpClient } from '../../../src/core/factory.js'; -// Mock the transports +// Mock the transports (now async) jest.mock('../../../src/core/transports', () => ({ - createTransportFromConfig: jest.fn().mockReturnValue({ + createTransportFromConfig: jest.fn().mockResolvedValue({ start: jest.fn().mockResolvedValue(undefined), send: jest.fn().mockResolvedValue(undefined), close: jest.fn().mockResolvedValue(undefined), diff --git a/test/unit/core/transports.test.ts b/test/unit/core/transports.test.ts index b8dd4a3..90b5758 100644 --- a/test/unit/core/transports.test.ts +++ b/test/unit/core/transports.test.ts @@ -4,6 +4,7 @@ import { createTransportFromConfig } from '../../../src/core/transports'; import { ClientError } from '../../../src/lib/errors'; +import { StreamableHTTPClientTransport } from '@modelcontextprotocol/sdk/client/streamableHttp.js'; // Mock the SDK transports jest.mock('@modelcontextprotocol/sdk/client/stdio.js', () => ({ @@ -24,9 +25,29 @@ jest.mock('@modelcontextprotocol/sdk/client/streamableHttp.js', () => ({ StreamableHTTPError: class StreamableHTTPError extends Error {}, })); +// Mock undici for proxy tests +jest.mock('undici', () => ({ + ProxyAgent: jest.fn().mockImplementation((url: string) => ({ proxyUrl: url })), + fetch: jest.fn().mockResolvedValue({ ok: true }), +})); + describe('createTransportFromConfig', () => { - it('should create stdio transport from config', () => { - const transport = createTransportFromConfig({ + const originalEnv = process.env; + + beforeEach(() => { + jest.clearAllMocks(); + // Reset environment variables before each test + process.env = { ...originalEnv }; + delete process.env.https_proxy; + delete process.env.HTTPS_PROXY; + }); + + afterAll(() => { + process.env = originalEnv; + }); + + it('should create stdio transport from config', async () => { + const transport = await createTransportFromConfig({ command: 'node', args: ['server.js'], }); @@ -34,22 +55,22 @@ describe('createTransportFromConfig', () => { expect(transport).toBeDefined(); }); - it('should create http transport from config', () => { - const transport = createTransportFromConfig({ + it('should create http transport from config', async () => { + const transport = await createTransportFromConfig({ url: 'https://mcp.example.com', }); expect(transport).toBeDefined(); }); - it('should throw error for config without url or command', () => { - expect(() => + it('should throw error for config without url or command', async () => { + await expect( createTransportFromConfig({} as any) - ).toThrow(ClientError); + ).rejects.toThrow(ClientError); }); - it('should pass headers to http transport', () => { - const transport = createTransportFromConfig({ + it('should pass headers to http transport', async () => { + const transport = await createTransportFromConfig({ url: 'https://mcp.example.com', headers: { Authorization: 'Bearer token', @@ -59,8 +80,8 @@ describe('createTransportFromConfig', () => { expect(transport).toBeDefined(); }); - it('should pass environment variables to stdio transport', () => { - const transport = createTransportFromConfig({ + it('should pass environment variables to stdio transport', async () => { + const transport = await createTransportFromConfig({ command: 'node', args: ['server.js'], env: { @@ -71,3 +92,87 @@ describe('createTransportFromConfig', () => { expect(transport).toBeDefined(); }); }); + +describe('proxy-aware fetch', () => { + const originalEnv = process.env; + const MockedStreamableHTTPClientTransport = StreamableHTTPClientTransport as jest.MockedClass; + + beforeEach(() => { + jest.clearAllMocks(); + process.env = { ...originalEnv }; + delete process.env.https_proxy; + delete process.env.HTTPS_PROXY; + }); + + afterAll(() => { + process.env = originalEnv; + }); + + it('should not use proxy fetch when no proxy env var is set', async () => { + await createTransportFromConfig({ + url: 'https://mcp.example.com', + }); + + // Check that StreamableHTTPClientTransport was called without a custom fetch + expect(MockedStreamableHTTPClientTransport).toHaveBeenCalledTimes(1); + const callArgs = MockedStreamableHTTPClientTransport.mock.calls[0]; + expect(callArgs[1]).not.toHaveProperty('fetch'); + }); + + it('should use proxy fetch when https_proxy is set', async () => { + process.env.https_proxy = 'http://localhost:8080'; + + await createTransportFromConfig({ + url: 'https://mcp.example.com', + }); + + // Check that StreamableHTTPClientTransport was called with a custom fetch + expect(MockedStreamableHTTPClientTransport).toHaveBeenCalledTimes(1); + const callArgs = MockedStreamableHTTPClientTransport.mock.calls[0]; + expect(callArgs[1]).toHaveProperty('fetch'); + expect(typeof callArgs[1]?.fetch).toBe('function'); + }); + + it('should use proxy fetch when HTTPS_PROXY is set', async () => { + process.env.HTTPS_PROXY = 'http://localhost:8080'; + + await createTransportFromConfig({ + url: 'https://mcp.example.com', + }); + + // Check that StreamableHTTPClientTransport was called with a custom fetch + expect(MockedStreamableHTTPClientTransport).toHaveBeenCalledTimes(1); + const callArgs = MockedStreamableHTTPClientTransport.mock.calls[0]; + expect(callArgs[1]).toHaveProperty('fetch'); + expect(typeof callArgs[1]?.fetch).toBe('function'); + }); + + it('should prefer https_proxy over HTTPS_PROXY when both are set', async () => { + process.env.https_proxy = 'http://localhost:8080'; + process.env.HTTPS_PROXY = 'http://localhost:9090'; + + await createTransportFromConfig({ + url: 'https://mcp.example.com', + }); + + // The proxy fetch should be configured (we can't easily verify which URL was used + // without more complex mocking, but we can verify a fetch was provided) + expect(MockedStreamableHTTPClientTransport).toHaveBeenCalledTimes(1); + const callArgs = MockedStreamableHTTPClientTransport.mock.calls[0]; + expect(callArgs[1]).toHaveProperty('fetch'); + }); + + it('should not affect stdio transport when proxy is set', async () => { + process.env.https_proxy = 'http://localhost:8080'; + + const transport = await createTransportFromConfig({ + command: 'node', + args: ['server.js'], + }); + + // Stdio transport should still work normally + expect(transport).toBeDefined(); + // StreamableHTTPClientTransport should not have been called + expect(MockedStreamableHTTPClientTransport).not.toHaveBeenCalled(); + }); +});