diff --git a/src/cli.test.ts b/src/cli.test.ts index 6424e4cca..39c65444d 100644 --- a/src/cli.test.ts +++ b/src/cli.test.ts @@ -1,5 +1,5 @@ import { Command } from 'commander'; -import { parseEnvironmentVariables, parseDomains, parseDomainsFile, escapeShellArg, joinShellArgs, parseVolumeMounts, isValidIPv4, isValidIPv6, parseDnsServers, parseDnsOverHttps, validateAgentImage, isAgentImagePreset, AGENT_IMAGE_PRESETS, processAgentImageOption, processLocalhostKeyword, validateSkipPullWithBuildLocal, validateAllowHostPorts, parseMemoryLimit, validateFormat, validateApiProxyConfig, buildRateLimitConfig, validateRateLimitFlags, hasRateLimitOptions, collectRulesetFile, validateApiTargetInAllowedDomains, DEFAULT_OPENAI_API_TARGET, DEFAULT_ANTHROPIC_API_TARGET, DEFAULT_COPILOT_API_TARGET, emitApiProxyTargetWarnings, formatItem, program, parseAgentTimeout, applyAgentTimeout, handlePredownloadAction } from './cli'; +import { parseEnvironmentVariables, parseDomains, parseDomainsFile, escapeShellArg, joinShellArgs, parseVolumeMounts, isValidIPv4, isValidIPv6, parseDnsServers, parseDnsOverHttps, validateAgentImage, isAgentImagePreset, AGENT_IMAGE_PRESETS, processAgentImageOption, processLocalhostKeyword, validateSkipPullWithBuildLocal, validateAllowHostPorts, parseMemoryLimit, validateFormat, validateApiProxyConfig, buildRateLimitConfig, validateRateLimitFlags, hasRateLimitOptions, collectRulesetFile, validateApiTargetInAllowedDomains, DEFAULT_OPENAI_API_TARGET, DEFAULT_ANTHROPIC_API_TARGET, DEFAULT_COPILOT_API_TARGET, emitApiProxyTargetWarnings, formatItem, program, parseAgentTimeout, applyAgentTimeout, handlePredownloadAction, resolveApiTargetsToAllowedDomains } from './cli'; import { redactSecrets } from './redact-secrets'; import * as fs from 'fs'; import * as path from 'path'; @@ -1852,6 +1852,135 @@ describe('cli', () => { }); }); + describe('resolveApiTargetsToAllowedDomains', () => { + it('should add copilot-api-target option to allowed domains', () => { + const domains: string[] = ['github.com']; + resolveApiTargetsToAllowedDomains({ copilotApiTarget: 'custom.copilot.com' }, domains); + expect(domains).toContain('custom.copilot.com'); + expect(domains).toContain('https://custom.copilot.com'); + }); + + it('should add openai-api-target option to allowed domains', () => { + const domains: string[] = ['github.com']; + resolveApiTargetsToAllowedDomains({ openaiApiTarget: 'custom.openai.com' }, domains); + expect(domains).toContain('custom.openai.com'); + expect(domains).toContain('https://custom.openai.com'); + }); + + it('should add anthropic-api-target option to allowed domains', () => { + const domains: string[] = ['github.com']; + resolveApiTargetsToAllowedDomains({ anthropicApiTarget: 'custom.anthropic.com' }, domains); + expect(domains).toContain('custom.anthropic.com'); + expect(domains).toContain('https://custom.anthropic.com'); + }); + + it('should prefer option flag over env var', () => { + const domains: string[] = []; + const env = { COPILOT_API_TARGET: 'env.copilot.com' }; + resolveApiTargetsToAllowedDomains({ copilotApiTarget: 'flag.copilot.com' }, domains, env); + expect(domains).toContain('flag.copilot.com'); + expect(domains).not.toContain('env.copilot.com'); + }); + + it('should fall back to env var when option flag is not set', () => { + const domains: string[] = []; + const env = { COPILOT_API_TARGET: 'env.copilot.com' }; + resolveApiTargetsToAllowedDomains({}, domains, env); + expect(domains).toContain('env.copilot.com'); + expect(domains).toContain('https://env.copilot.com'); + }); + + it('should read OPENAI_API_TARGET from env when flag not set', () => { + const domains: string[] = []; + const env = { OPENAI_API_TARGET: 'env.openai.com' }; + resolveApiTargetsToAllowedDomains({}, domains, env); + expect(domains).toContain('env.openai.com'); + }); + + it('should read ANTHROPIC_API_TARGET from env when flag not set', () => { + const domains: string[] = []; + const env = { ANTHROPIC_API_TARGET: 'env.anthropic.com' }; + resolveApiTargetsToAllowedDomains({}, domains, env); + expect(domains).toContain('env.anthropic.com'); + }); + + it('should not duplicate a domain already in the list', () => { + const domains: string[] = ['custom.copilot.com']; + resolveApiTargetsToAllowedDomains({ copilotApiTarget: 'custom.copilot.com' }, domains); + const count = domains.filter(d => d === 'custom.copilot.com').length; + expect(count).toBe(1); + }); + + it('should not duplicate the https:// form if already in the list', () => { + const domains: string[] = ['github.com', 'https://custom.copilot.com']; + resolveApiTargetsToAllowedDomains({ copilotApiTarget: 'custom.copilot.com' }, domains); + const count = domains.filter(d => d === 'https://custom.copilot.com').length; + expect(count).toBe(1); + }); + + it('should preserve an existing https:// prefix without doubling it', () => { + const domains: string[] = []; + resolveApiTargetsToAllowedDomains({ copilotApiTarget: 'https://custom.copilot.com' }, domains); + expect(domains).toContain('https://custom.copilot.com'); + const count = domains.filter(d => d === 'https://custom.copilot.com').length; + expect(count).toBe(1); + }); + + it('should handle http:// prefix without adding another https://', () => { + const domains: string[] = []; + resolveApiTargetsToAllowedDomains({ openaiApiTarget: 'http://internal.openai.com' }, domains); + expect(domains).toContain('http://internal.openai.com'); + }); + + it('should add all three targets when all are specified', () => { + const domains: string[] = []; + resolveApiTargetsToAllowedDomains( + { + copilotApiTarget: 'copilot.internal', + openaiApiTarget: 'openai.internal', + anthropicApiTarget: 'anthropic.internal', + }, + domains + ); + expect(domains).toContain('copilot.internal'); + expect(domains).toContain('openai.internal'); + expect(domains).toContain('anthropic.internal'); + }); + + it('should call debug with auto-added domains', () => { + const domains: string[] = []; + const debugMessages: string[] = []; + resolveApiTargetsToAllowedDomains( + { copilotApiTarget: 'copilot.internal' }, + domains, + {}, + (msg) => debugMessages.push(msg) + ); + expect(debugMessages.some(m => m.includes('copilot.internal'))).toBe(true); + }); + + it('should not call debug when no api targets are set', () => { + const domains: string[] = []; + const debugMessages: string[] = []; + resolveApiTargetsToAllowedDomains({}, domains, {}, (msg) => debugMessages.push(msg)); + expect(debugMessages).toHaveLength(0); + }); + + it('should return the same allowedDomains array reference', () => { + const domains: string[] = []; + const returned = resolveApiTargetsToAllowedDomains({ copilotApiTarget: 'x.com' }, domains); + expect(returned).toBe(domains); + }); + + it('should ignore empty env var values', () => { + const domains: string[] = []; + const env = { COPILOT_API_TARGET: ' ', OPENAI_API_TARGET: '' }; + resolveApiTargetsToAllowedDomains({}, domains, env); + // Whitespace-only and empty values are filtered out + expect(domains).toHaveLength(0); + }); + }); + describe('formatItem', () => { it('should format item with description on same line when term fits', () => { const result = formatItem('-v', 'verbose output', 20, 2, 2, 80); diff --git a/src/cli.ts b/src/cli.ts index 9cd05d642..f4eea2a4c 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -378,6 +378,72 @@ export function emitApiProxyTargetWarnings( } } +/** + * Resolves API target values from CLI options and environment variables, and merges them + * into the allowed domains list. Also ensures each target is present as an https:// URL. + * @param options - Partial options with API target flag values + * @param allowedDomains - The current list of allowed domains (mutated in place) + * @param env - Environment variables (defaults to process.env) + * @param debug - Optional debug logging function + * @returns The updated allowedDomains array (same reference, mutated) + */ +export function resolveApiTargetsToAllowedDomains( + options: { + copilotApiTarget?: string; + openaiApiTarget?: string; + anthropicApiTarget?: string; + }, + allowedDomains: string[], + env: Record = process.env, + debug: (msg: string) => void = () => {} +): string[] { + const apiTargets: string[] = []; + + if (options.copilotApiTarget) { + apiTargets.push(options.copilotApiTarget); + } else if (env['COPILOT_API_TARGET']) { + apiTargets.push(env['COPILOT_API_TARGET']); + } + + if (options.openaiApiTarget) { + apiTargets.push(options.openaiApiTarget); + } else if (env['OPENAI_API_TARGET']) { + apiTargets.push(env['OPENAI_API_TARGET']); + } + + if (options.anthropicApiTarget) { + apiTargets.push(options.anthropicApiTarget); + } else if (env['ANTHROPIC_API_TARGET']) { + apiTargets.push(env['ANTHROPIC_API_TARGET']); + } + + // Merge raw target values into the allowedDomains list so that later + // checks/logs about "no allowed domains" see the final, expanded allowlist. + const normalizedApiTargets = apiTargets.filter((t) => typeof t === 'string' && t.trim().length > 0); + if (normalizedApiTargets.length > 0) { + for (const target of normalizedApiTargets) { + if (!allowedDomains.includes(target)) { + allowedDomains.push(target); + } + } + debug(`Auto-added API target values to allowed domains: ${normalizedApiTargets.join(', ')}`); + } + + // Also ensure each target is present as an explicit https:// URL + for (const target of normalizedApiTargets) { + + // Ensure auto-added API targets are explicitly HTTPS to avoid over-broad HTTP+HTTPS allowlisting + const normalizedTarget = /^https?:\/\//.test(target) ? target : `https://${target}`; + + if (!allowedDomains.includes(normalizedTarget)) { + allowedDomains.push(normalizedTarget); + debug(`Automatically added API target to allowlist: ${normalizedTarget}`); + } + } + + return allowedDomains; +} + /** * Builds a RateLimitConfig from parsed CLI options. */ @@ -1239,52 +1305,7 @@ program // Automatically add API target values to allowlist when specified // This ensures that when engine.api-target is set in GitHub Agentic Workflows, // the target domain is automatically accessible through the firewall - const apiTargets: string[] = []; - - if (options.copilotApiTarget) { - apiTargets.push(options.copilotApiTarget); - } else if (process.env.COPILOT_API_TARGET) { - apiTargets.push(process.env.COPILOT_API_TARGET); - } - - if (options.openaiApiTarget) { - apiTargets.push(options.openaiApiTarget); - } else if (process.env.OPENAI_API_TARGET) { - apiTargets.push(process.env.OPENAI_API_TARGET); - } - - if (options.anthropicApiTarget) { - apiTargets.push(options.anthropicApiTarget); - } else if (process.env.ANTHROPIC_API_TARGET) { - apiTargets.push(process.env.ANTHROPIC_API_TARGET); - } - - // Merge any API target values into the allowedDomains list so that later - // checks/logs about "no allowed domains" see the final, expanded allowlist. - const normalizedApiTargets = apiTargets.filter((t) => typeof t === 'string' && t.trim().length > 0); - if (normalizedApiTargets.length > 0) { - for (const target of normalizedApiTargets) { - if (!allowedDomains.includes(target)) { - allowedDomains.push(target); - } - } - logger.debug(`Auto-added API target values to allowed domains: ${normalizedApiTargets.join(', ')}`); - } - - // Add API targets to allowedDomains if not already present - for (const target of apiTargets) { - if (!target) { - continue; - } - - // Ensure auto-added API targets are explicitly HTTPS to avoid over-broad HTTP+HTTPS allowlisting - const normalizedTarget = /^https?:\/\//.test(target) ? target : `https://${target}`; - - if (!allowedDomains.includes(normalizedTarget)) { - allowedDomains.push(normalizedTarget); - logger.debug(`Automatically added API target to allowlist: ${normalizedTarget}`); - } - } + resolveApiTargetsToAllowedDomains(options, allowedDomains, process.env, logger.debug.bind(logger)); // Validate all domains and patterns for (const domain of allowedDomains) {