Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 34 additions & 0 deletions packages/cli/src/config/config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -438,6 +438,40 @@ describe('loadCliConfig', () => {
const config = await loadCliConfig(settings, [], 'test-session', argv);
expect(config.getProxy()).toBe('http://localhost:7890');
});

it('should disable proxy when --proxy="" is explicitly provided', async () => {
// Set environment variable that would normally be used
vi.stubEnv('HTTPS_PROXY', 'http://localhost:7891');
process.argv = ['node', 'script.js', '--proxy', ''];
const argv = await parseArguments({} as Settings);
const settings: Settings = {};
const config = await loadCliConfig(settings, [], 'test-session', argv);
// Empty string should disable proxy (return undefined)
expect(config.getProxy()).toBeUndefined();
});

it('should fall back to environment variables when --proxy is not provided', async () => {
vi.stubEnv('HTTPS_PROXY', 'http://env-proxy:8080');
process.argv = ['node', 'script.js']; // No --proxy argument
const argv = await parseArguments({} as Settings);
const settings: Settings = {};
const config = await loadCliConfig(settings, [], 'test-session', argv);
expect(config.getProxy()).toBe('http://env-proxy:8080');
});

it('should respect proxy precedence: HTTPS_PROXY > https_proxy > HTTP_PROXY > http_proxy', async () => {
// Test HTTPS_PROXY takes precedence
vi.stubEnv('HTTPS_PROXY', 'http://https-upper:8080');
vi.stubEnv('https_proxy', 'http://https-lower:8080');
vi.stubEnv('HTTP_PROXY', 'http://http-upper:8080');
vi.stubEnv('http_proxy', 'http://http-lower:8080');

process.argv = ['node', 'script.js'];
const argv = await parseArguments({} as Settings);
const settings: Settings = {};
const config = await loadCliConfig(settings, [], 'test-session', argv);
expect(config.getProxy()).toBe('http://https-upper:8080');
});
});
});

Expand Down
29 changes: 23 additions & 6 deletions packages/cli/src/config/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -614,12 +614,7 @@ export async function loadCliConfig(
},
checkpointing:
argv.checkpointing || settings.general?.checkpointing?.enabled,
proxy:
argv.proxy ||
process.env['HTTPS_PROXY'] ||
process.env['https_proxy'] ||
process.env['HTTP_PROXY'] ||
process.env['http_proxy'],
proxy: getProxyConfiguration(argv.proxy),
cwd,
fileDiscoveryService: fileService,
bugCommand: settings.advanced?.bugCommand,
Expand Down Expand Up @@ -738,3 +733,25 @@ function mergeExcludeTools(
}
return [...allExcludeTools];
}

/**
* Determines the proxy configuration with proper handling of explicit empty string overrides.
*
* @param argvProxy - The proxy value from command line arguments
* @returns The proxy URL string or undefined if no proxy should be used
*/
function getProxyConfiguration(argvProxy: string | undefined): string | undefined {
// If --proxy was explicitly provided (even as empty string), respect that choice
if (argvProxy !== undefined) {
// Empty string means explicitly disable proxy
return argvProxy === '' ? undefined : argvProxy;
}

// If --proxy was not provided, fall back to environment variables
return (
process.env['HTTPS_PROXY'] ||
process.env['https_proxy'] ||
process.env['HTTP_PROXY'] ||
process.env['http_proxy']
);
}
176 changes: 176 additions & 0 deletions packages/core/src/core/client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2670,4 +2670,180 @@ ${JSON.stringify(
await expect(client.reinitialize()).resolves.not.toThrow();
});
});

describe('Proxy Configuration', () => {
let originalEnvVars: Record<string, string | undefined>;

beforeEach(() => {
// Store original environment variables
originalEnvVars = {
HTTP_PROXY: process.env['HTTP_PROXY'],
http_proxy: process.env['http_proxy'],
HTTPS_PROXY: process.env['HTTPS_PROXY'],
https_proxy: process.env['https_proxy'],
NO_PROXY: process.env['NO_PROXY'],
no_proxy: process.env['no_proxy'],
};

// Clear all proxy environment variables for clean test state
delete process.env['HTTP_PROXY'];
delete process.env['http_proxy'];
delete process.env['HTTPS_PROXY'];
delete process.env['https_proxy'];
delete process.env['NO_PROXY'];
delete process.env['no_proxy'];
});

afterEach(() => {
// Restore original environment variables
Object.entries(originalEnvVars).forEach(([key, value]) => {
if (value === undefined) {
delete process.env[key];
} else {
process.env[key] = value;
}
});
});

it('should not setup proxy when no proxy is configured', () => {
// Arrange: No proxy in config, no environment variables
mockConfigObject.getProxy.mockReturnValue(undefined);

// Act: Create a new client (which calls setupProxyConfiguration)
const testClient = new GeminiClient(
new Config({ sessionId: 'test-proxy-none' } as never),
);

// Assert: No proxy should be set up (we can't easily test setGlobalDispatcher,
// but we can verify the logic by checking environment variables weren't modified)
expect(process.env['HTTP_PROXY']).toBeUndefined();
expect(process.env['HTTPS_PROXY']).toBeUndefined();
});

it('should setup proxy when --proxy parameter is provided', () => {
// Arrange: Proxy from --proxy parameter, no environment variables
const proxyUrl = 'http://cli-proxy:8080';
mockConfigObject.getProxy.mockReturnValue(proxyUrl);

// Act: Create a new client
const testClient = new GeminiClient(
new Config({ sessionId: 'test-proxy-cli' } as never),
);

// Assert: Environment variables should be set for EnvHttpProxyAgent
expect(process.env['HTTP_PROXY']).toBe(proxyUrl);
expect(process.env['HTTPS_PROXY']).toBe(proxyUrl);
});

it('should not override existing environment variables when --proxy is provided', () => {
// Arrange: Both --proxy parameter and existing environment variables
const cliProxyUrl = 'http://cli-proxy:8080';
const envProxyUrl = 'https://env-proxy:3128';

process.env['HTTPS_PROXY'] = envProxyUrl;
mockConfigObject.getProxy.mockReturnValue(cliProxyUrl);

// Act: Create a new client
const testClient = new GeminiClient(
new Config({ sessionId: 'test-proxy-both' } as never),
);

// Assert: Existing environment variables should not be overridden
expect(process.env['HTTPS_PROXY']).toBe(envProxyUrl);
// HTTP_PROXY should not be set because HTTPS_PROXY is already set
expect(process.env['HTTP_PROXY']).toBeUndefined();
});

it('should setup proxy when only environment variables are set', () => {
// Arrange: No --proxy parameter, but environment variables are set
const envProxyUrl = 'https://corporate-proxy:8080';
process.env['HTTPS_PROXY'] = envProxyUrl;
mockConfigObject.getProxy.mockReturnValue(undefined);

// Act: Create a new client
const testClient = new GeminiClient(
new Config({ sessionId: 'test-proxy-env' } as never),
);

// Assert: Environment variables should remain unchanged
expect(process.env['HTTPS_PROXY']).toBe(envProxyUrl);
});

it('should include NO_PROXY values in proxy configuration', () => {
// Arrange: Set up proxy and NO_PROXY environment variables
const proxyUrl = 'http://proxy:8080';
const noProxyHosts = 'internal-llm.company.com,localhost';

process.env['HTTPS_PROXY'] = proxyUrl;
process.env['NO_PROXY'] = noProxyHosts;
mockConfigObject.getProxy.mockReturnValue(undefined);

// Act: Create a new client (this calls setupProxyConfiguration)
const testClient = new GeminiClient(
new Config({ sessionId: 'test-no-proxy' } as never),
);

// Assert: NO_PROXY should still be set
expect(process.env['NO_PROXY']).toBe(noProxyHosts);

// Note: We can't easily test the EnvHttpProxyAgent configuration directly,
// but the setupProxyConfiguration method should build a noProxy list that includes:
// - The existing NO_PROXY value
// - localhost, 127.0.0.1, ::1 (automatically added)
});

it('should handle mixed case environment variables', () => {
// Arrange: Use lowercase environment variables
const proxyUrl = 'http://lowercase-proxy:3128';
process.env['http_proxy'] = proxyUrl;
process.env['no_proxy'] = 'example.com';
mockConfigObject.getProxy.mockReturnValue(undefined);

// Act: Create a new client
const testClient = new GeminiClient(
new Config({ sessionId: 'test-proxy-lowercase' } as never),
);

// Assert: Lowercase variables should be recognized
expect(process.env['http_proxy']).toBe(proxyUrl);
expect(process.env['no_proxy']).toBe('example.com');
});

it('should work with --proxy=undefined (explicit disable)', () => {
// Arrange: --proxy explicitly set to undefined, but environment variables exist
const envProxyUrl = 'https://env-proxy:8080';
process.env['HTTPS_PROXY'] = envProxyUrl;
mockConfigObject.getProxy.mockReturnValue(undefined); // This simulates --proxy=""

// Act: Create a new client
const testClient = new GeminiClient(
new Config({ sessionId: 'test-proxy-disable' } as never),
);

// Assert: Environment variables should still be used
// (because --proxy="" disables config proxy but doesn't clear env vars)
expect(process.env['HTTPS_PROXY']).toBe(envProxyUrl);
});

it('should automatically add localhost to NO_PROXY list', () => {
// This test verifies the logic that automatically includes localhost variants
// in the NO_PROXY list to prevent issues with local services

// Arrange: Set up proxy without any NO_PROXY
const proxyUrl = 'http://proxy:8080';
mockConfigObject.getProxy.mockReturnValue(proxyUrl);

// Act: Create a new client
const testClient = new GeminiClient(
new Config({ sessionId: 'test-auto-localhost' } as never),
);

// Assert: The setupProxyConfiguration method should have been called
// and would have built a noProxy list including localhost variants
// We can't directly test the EnvHttpProxyAgent configuration,
// but we can verify the environment was set up correctly
expect(process.env['HTTP_PROXY']).toBe(proxyUrl);
expect(process.env['HTTPS_PROXY']).toBe(proxyUrl);
});
});
});
50 changes: 46 additions & 4 deletions packages/core/src/core/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import type {
Schema,
Tool,
} from '@google/genai';
import { ProxyAgent, setGlobalDispatcher } from 'undici';
import { EnvHttpProxyAgent, setGlobalDispatcher } from 'undici';
import type { UserTierId } from '../code_assist/types.js';
import type { Config } from '../config/config.js';
import { ApprovalMode } from '../config/config.js';
Expand Down Expand Up @@ -130,15 +130,57 @@ export class GeminiClient {
private hasFailedCompressionAttempt = false;

constructor(private readonly config: Config) {
if (config.getProxy()) {
setGlobalDispatcher(new ProxyAgent(config.getProxy() as string));
}
this.setupProxyConfiguration();

this.embeddingModel = config.getEmbeddingModel();
this.loopDetector = new LoopDetectionService(config);
this.lastPromptId = this.config.getSessionId();
}

/**
* Sets up proxy configuration with NO_PROXY support for LLM requests.
* Supports both --proxy parameter and standard proxy environment variables.
* Uses EnvHttpProxyAgent which respects NO_PROXY, unlike the basic ProxyAgent.
*/
private setupProxyConfiguration(): void {
const configProxy = this.config.getProxy(); // This includes --proxy parameter handling

// Check if any proxy environment variables are already set
const hasProxyEnvVars = !!(
process.env['HTTP_PROXY'] ||
process.env['http_proxy'] ||
process.env['HTTPS_PROXY'] ||
process.env['https_proxy']
);

// If no proxy is configured via --proxy or environment variables, nothing to do
if (!configProxy && !hasProxyEnvVars) {
return;
}

// If --proxy was specified but no environment variables are set,
// temporarily set environment variables for EnvHttpProxyAgent to use
if (configProxy && !hasProxyEnvVars) {
// Set both HTTP and HTTPS proxy environment variables
process.env['HTTPS_PROXY'] = configProxy;
process.env['HTTP_PROXY'] = configProxy;
}

// Build NO_PROXY list including localhost and any existing NO_PROXY values
const existingNoProxy = process.env['NO_PROXY'] || process.env['no_proxy'] || '';
const noProxyList = [existingNoProxy, 'localhost', '127.0.0.1', '::1']
.filter(Boolean)
.join(',');

// Create EnvHttpProxyAgent that respects NO_PROXY and reads proxy URLs from environment
const proxyAgent = new EnvHttpProxyAgent({
// Respect NO_PROXY environment variable
noProxy: noProxyList,
});

setGlobalDispatcher(proxyAgent);
}

async initialize(
contentGeneratorConfig: ContentGeneratorConfig,
extraHistory?: Content[],
Expand Down
Loading