diff --git a/src/api-proxy-config-domains.test.ts b/src/api-proxy-config-domains.test.ts index 8dd105cf6..81853395a 100644 --- a/src/api-proxy-config-domains.test.ts +++ b/src/api-proxy-config-domains.test.ts @@ -1,7 +1,5 @@ import { resolveApiTargetsToAllowedDomains, - extractGhesDomainsFromEngineApiTarget, - extractGhecDomainsFromServerUrl, } from './api-proxy-config'; describe('resolveApiTargetsToAllowedDomains', () => { @@ -154,23 +152,17 @@ describe('resolveApiTargetsToAllowedDomains', () => { }); }); -describe('extractGhesDomainsFromEngineApiTarget', () => { - it('should return empty array when ENGINE_API_TARGET is not set', () => { - const domains = extractGhesDomainsFromEngineApiTarget({}); - expect(domains).toEqual([]); - }); - - it('should use process.env by default when no env argument is provided', () => { - const saved = process.env.ENGINE_API_TARGET; - delete process.env.ENGINE_API_TARGET; - const domains = extractGhesDomainsFromEngineApiTarget(); - expect(domains).toEqual([]); - if (saved !== undefined) process.env.ENGINE_API_TARGET = saved; +describe('extractGhesDomainsFromEngineApiTarget (via resolveApiTargetsToAllowedDomains)', () => { + it('should return no GHES domains when ENGINE_API_TARGET is not set', () => { + const domains: string[] = []; + resolveApiTargetsToAllowedDomains({}, domains, {}); + expect(domains).toHaveLength(0); }); it('should extract GHES domains from api.github.* format', () => { + const domains: string[] = []; const env = { ENGINE_API_TARGET: 'https://api.github.mycompany.com' }; - const domains = extractGhesDomainsFromEngineApiTarget(env); + resolveApiTargetsToAllowedDomains({}, domains, env); expect(domains).toContain('github.mycompany.com'); expect(domains).toContain('api.github.mycompany.com'); expect(domains).toContain('api.githubcopilot.com'); @@ -179,8 +171,9 @@ describe('extractGhesDomainsFromEngineApiTarget', () => { }); it('should handle non-api.* hostnames', () => { + const domains: string[] = []; const env = { ENGINE_API_TARGET: 'https://github.mycompany.com' }; - const domains = extractGhesDomainsFromEngineApiTarget(env); + resolveApiTargetsToAllowedDomains({}, domains, env); expect(domains).toContain('github.mycompany.com'); expect(domains).toContain('api.githubcopilot.com'); expect(domains).toContain('api.enterprise.githubcopilot.com'); @@ -188,47 +181,53 @@ describe('extractGhesDomainsFromEngineApiTarget', () => { }); it('should handle invalid URL gracefully', () => { + const domains: string[] = []; const env = { ENGINE_API_TARGET: 'not-a-valid-url' }; - const domains = extractGhesDomainsFromEngineApiTarget(env); - expect(domains).toEqual([]); + resolveApiTargetsToAllowedDomains({}, domains, env); + expect(domains).toHaveLength(0); }); it('should always include Copilot API domains for GHES', () => { + const domains: string[] = []; const env = { ENGINE_API_TARGET: 'https://api.github.enterprise.local' }; - const domains = extractGhesDomainsFromEngineApiTarget(env); + resolveApiTargetsToAllowedDomains({}, domains, env); expect(domains).toContain('api.githubcopilot.com'); expect(domains).toContain('api.enterprise.githubcopilot.com'); expect(domains).toContain('telemetry.enterprise.githubcopilot.com'); }); }); -describe('extractGhecDomainsFromServerUrl', () => { - it('should return empty array when no env vars are set', () => { - const domains = extractGhecDomainsFromServerUrl({}); - expect(domains).toEqual([]); +describe('extractGhecDomainsFromServerUrl (via resolveApiTargetsToAllowedDomains)', () => { + it('should return no GHEC domains when no env vars are set', () => { + const domains: string[] = []; + resolveApiTargetsToAllowedDomains({}, domains, {}); + expect(domains).toHaveLength(0); }); - it('should return empty array when GITHUB_SERVER_URL is github.com', () => { - const domains = extractGhecDomainsFromServerUrl({ GITHUB_SERVER_URL: 'https://github.com' }); - expect(domains).toEqual([]); + it('should return no GHEC domains when GITHUB_SERVER_URL is github.com', () => { + const domains: string[] = []; + resolveApiTargetsToAllowedDomains({}, domains, { GITHUB_SERVER_URL: 'https://github.com' }); + expect(domains).toHaveLength(0); }); - it('should return empty array for GHES (non-ghe.com) server URL', () => { - const domains = extractGhecDomainsFromServerUrl({ GITHUB_SERVER_URL: 'https://github.mycompany.com' }); - expect(domains).toEqual([]); + it('should return no GHEC domains for GHES (non-ghe.com) server URL', () => { + const domains: string[] = []; + resolveApiTargetsToAllowedDomains({}, domains, { GITHUB_SERVER_URL: 'https://github.mycompany.com' }); + expect(domains).toHaveLength(0); }); it('should extract GHEC tenant, API, Copilot API, and telemetry subdomains from GITHUB_SERVER_URL', () => { - const domains = extractGhecDomainsFromServerUrl({ GITHUB_SERVER_URL: 'https://myorg.ghe.com' }); + const domains: string[] = []; + resolveApiTargetsToAllowedDomains({}, domains, { GITHUB_SERVER_URL: 'https://myorg.ghe.com' }); expect(domains).toContain('myorg.ghe.com'); expect(domains).toContain('api.myorg.ghe.com'); expect(domains).toContain('copilot-api.myorg.ghe.com'); expect(domains).toContain('copilot-telemetry-service.myorg.ghe.com'); - expect(domains).toHaveLength(4); }); it('should handle GITHUB_SERVER_URL with trailing slash', () => { - const domains = extractGhecDomainsFromServerUrl({ GITHUB_SERVER_URL: 'https://myorg.ghe.com/' }); + const domains: string[] = []; + resolveApiTargetsToAllowedDomains({}, domains, { GITHUB_SERVER_URL: 'https://myorg.ghe.com/' }); expect(domains).toContain('myorg.ghe.com'); expect(domains).toContain('api.myorg.ghe.com'); expect(domains).toContain('copilot-api.myorg.ghe.com'); @@ -236,7 +235,8 @@ describe('extractGhecDomainsFromServerUrl', () => { }); it('should handle GITHUB_SERVER_URL with path components', () => { - const domains = extractGhecDomainsFromServerUrl({ GITHUB_SERVER_URL: 'https://acme.ghe.com/some/path' }); + const domains: string[] = []; + resolveApiTargetsToAllowedDomains({}, domains, { GITHUB_SERVER_URL: 'https://acme.ghe.com/some/path' }); expect(domains).toContain('acme.ghe.com'); expect(domains).toContain('api.acme.ghe.com'); expect(domains).toContain('copilot-api.acme.ghe.com'); @@ -244,12 +244,14 @@ describe('extractGhecDomainsFromServerUrl', () => { }); it('should extract from GITHUB_API_URL for GHEC', () => { - const domains = extractGhecDomainsFromServerUrl({ GITHUB_API_URL: 'https://api.myorg.ghe.com' }); + const domains: string[] = []; + resolveApiTargetsToAllowedDomains({}, domains, { GITHUB_API_URL: 'https://api.myorg.ghe.com' }); expect(domains).toContain('api.myorg.ghe.com'); }); it('should not add GITHUB_API_URL domain if already present from GITHUB_SERVER_URL', () => { - const domains = extractGhecDomainsFromServerUrl({ + const domains: string[] = []; + resolveApiTargetsToAllowedDomains({}, domains, { GITHUB_SERVER_URL: 'https://myorg.ghe.com', GITHUB_API_URL: 'https://api.myorg.ghe.com', }); @@ -259,35 +261,28 @@ describe('extractGhecDomainsFromServerUrl', () => { expect(apiCount).toBe(1); }); - it('should return empty array when GITHUB_API_URL is api.github.com', () => { - const domains = extractGhecDomainsFromServerUrl({ GITHUB_API_URL: 'https://api.github.com' }); - expect(domains).toEqual([]); + it('should return no GHEC domains when GITHUB_API_URL is api.github.com', () => { + const domains: string[] = []; + resolveApiTargetsToAllowedDomains({}, domains, { GITHUB_API_URL: 'https://api.github.com' }); + expect(domains).toHaveLength(0); }); it('should ignore non-ghe.com GITHUB_API_URL', () => { - const domains = extractGhecDomainsFromServerUrl({ GITHUB_API_URL: 'https://api.github.mycompany.com' }); - expect(domains).toEqual([]); + const domains: string[] = []; + resolveApiTargetsToAllowedDomains({}, domains, { GITHUB_API_URL: 'https://api.github.mycompany.com' }); + expect(domains).toHaveLength(0); }); it('should handle invalid GITHUB_SERVER_URL gracefully', () => { - const domains = extractGhecDomainsFromServerUrl({ GITHUB_SERVER_URL: 'not-a-valid-url' }); - expect(domains).toEqual([]); + const domains: string[] = []; + resolveApiTargetsToAllowedDomains({}, domains, { GITHUB_SERVER_URL: 'not-a-valid-url' }); + expect(domains).toHaveLength(0); }); it('should handle invalid GITHUB_API_URL gracefully', () => { - const domains = extractGhecDomainsFromServerUrl({ GITHUB_API_URL: 'not-a-valid-url' }); - expect(domains).toEqual([]); - }); - - it('should use process.env by default when no env argument is provided', () => { - const savedServerUrl = process.env.GITHUB_SERVER_URL; - const savedApiUrl = process.env.GITHUB_API_URL; - delete process.env.GITHUB_SERVER_URL; - delete process.env.GITHUB_API_URL; - const domains = extractGhecDomainsFromServerUrl(); - expect(domains).toEqual([]); - if (savedServerUrl !== undefined) process.env.GITHUB_SERVER_URL = savedServerUrl; - if (savedApiUrl !== undefined) process.env.GITHUB_API_URL = savedApiUrl; + const domains: string[] = []; + resolveApiTargetsToAllowedDomains({}, domains, { GITHUB_API_URL: 'not-a-valid-url' }); + expect(domains).toHaveLength(0); }); }); diff --git a/src/api-proxy-config-validation.test.ts b/src/api-proxy-config-validation.test.ts index 409fc827a..3bb18a50c 100644 --- a/src/api-proxy-config-validation.test.ts +++ b/src/api-proxy-config-validation.test.ts @@ -1,7 +1,7 @@ import { validateApiProxyConfig, validateAnthropicCacheTailTtl, - validateApiTargetInAllowedDomains, + emitApiProxyTargetWarnings, } from './api-proxy-config'; describe('validateApiProxyConfig', () => { @@ -91,78 +91,81 @@ describe('validateApiProxyConfig', () => { }); }); -describe('validateApiTargetInAllowedDomains', () => { - it('should return null when using the default host', () => { - const result = validateApiTargetInAllowedDomains( - 'api.openai.com', - 'api.openai.com', - '--openai-api-target', - ['example.com'] +describe('validateApiTargetInAllowedDomains (via emitApiProxyTargetWarnings)', () => { + it('should not warn when using the default host', () => { + const warnings: string[] = []; + emitApiProxyTargetWarnings( + { enableApiProxy: true, openaiApiTarget: 'api.openai.com' }, + ['example.com'], + (msg) => warnings.push(msg) ); - expect(result).toBeNull(); + // No warning for openai since it's the default target + expect(warnings.filter(w => w.includes('openai-api-target'))).toEqual([]); }); - it('should return null when custom host is in allowed domains', () => { - const result = validateApiTargetInAllowedDomains( - 'custom.example.com', - 'api.openai.com', - '--openai-api-target', - ['custom.example.com', 'other.com'] + it('should not warn when custom host is in allowed domains', () => { + const warnings: string[] = []; + emitApiProxyTargetWarnings( + { enableApiProxy: true, openaiApiTarget: 'custom.example.com' }, + ['custom.example.com', 'other.com'], + (msg) => warnings.push(msg) ); - expect(result).toBeNull(); + expect(warnings.filter(w => w.includes('openai-api-target'))).toEqual([]); }); - it('should return null when custom host matches a parent domain in allowed list', () => { - const result = validateApiTargetInAllowedDomains( - 'llm-router.internal.example.com', - 'api.openai.com', - '--openai-api-target', - ['example.com'] + it('should not warn when custom host matches a parent domain in allowed list', () => { + const warnings: string[] = []; + emitApiProxyTargetWarnings( + { enableApiProxy: true, openaiApiTarget: 'llm-router.internal.example.com' }, + ['example.com'], + (msg) => warnings.push(msg) ); - expect(result).toBeNull(); + expect(warnings.filter(w => w.includes('openai-api-target'))).toEqual([]); }); - it('should return null when custom host matches a dotted parent domain in allowed list', () => { - const result = validateApiTargetInAllowedDomains( - 'api.example.com', - 'api.openai.com', - '--openai-api-target', - ['.example.com'] + it('should not warn when custom host matches a dotted parent domain in allowed list', () => { + const warnings: string[] = []; + emitApiProxyTargetWarnings( + { enableApiProxy: true, openaiApiTarget: 'api.example.com' }, + ['.example.com'], + (msg) => warnings.push(msg) ); - expect(result).toBeNull(); + expect(warnings.filter(w => w.includes('openai-api-target'))).toEqual([]); }); - it('should return a warning when custom host is not in allowed domains', () => { - const result = validateApiTargetInAllowedDomains( - 'custom.llm-router.internal', - 'api.openai.com', - '--openai-api-target', - ['github.com', 'api.openai.com'] + it('should warn when custom host is not in allowed domains', () => { + const warnings: string[] = []; + emitApiProxyTargetWarnings( + { enableApiProxy: true, openaiApiTarget: 'custom.llm-router.internal' }, + ['github.com', 'api.openai.com'], + (msg) => warnings.push(msg) ); - expect(result).not.toBeNull(); - expect(result).toContain('--openai-api-target=custom.llm-router.internal'); - expect(result).toContain('--allow-domains'); - }); - - it('should return a warning with the correct flag name and host', () => { - const result = validateApiTargetInAllowedDomains( - 'custom.anthropic-router.com', - 'api.anthropic.com', - '--anthropic-api-target', - [] + const openaiWarnings = warnings.filter(w => w.includes('openai-api-target')); + expect(openaiWarnings).toHaveLength(1); + expect(openaiWarnings[0]).toContain('custom.llm-router.internal'); + expect(openaiWarnings[0]).toContain('--allow-domains'); + }); + + it('should warn with the correct flag name and host for anthropic', () => { + const warnings: string[] = []; + emitApiProxyTargetWarnings( + { enableApiProxy: true, anthropicApiTarget: 'custom.anthropic-router.com' }, + [], + (msg) => warnings.push(msg) ); - expect(result).not.toBeNull(); - expect(result).toContain('--anthropic-api-target=custom.anthropic-router.com'); + const anthropicWarnings = warnings.filter(w => w.includes('anthropic-api-target')); + expect(anthropicWarnings).toHaveLength(1); + expect(anthropicWarnings[0]).toContain('custom.anthropic-router.com'); }); - it('should return null when allowed domains list is empty and using default host', () => { - const result = validateApiTargetInAllowedDomains( - 'api.anthropic.com', - 'api.anthropic.com', - '--anthropic-api-target', - [] + it('should not warn when allowed domains list is empty and using default host', () => { + const warnings: string[] = []; + emitApiProxyTargetWarnings( + { enableApiProxy: true, anthropicApiTarget: 'api.anthropic.com' }, + [], + (msg) => warnings.push(msg) ); - expect(result).toBeNull(); + expect(warnings.filter(w => w.includes('anthropic-api-target'))).toEqual([]); }); }); diff --git a/src/api-proxy-config.ts b/src/api-proxy-config.ts index 9bd7550d8..528e91da6 100644 --- a/src/api-proxy-config.ts +++ b/src/api-proxy-config.ts @@ -82,7 +82,7 @@ export function validateAnthropicCacheTailTtl(value: string | undefined): void { * @param flagName - The CLI flag name for use in the warning message (e.g. "--openai-api-target") * @param allowedDomains - The list of domains allowed through the firewall */ -export function validateApiTargetInAllowedDomains( +function validateApiTargetInAllowedDomains( targetHost: string, defaultHost: string, flagName: string, @@ -213,7 +213,7 @@ export function warnClassicPATWithCopilotModel( * @returns Array of GHEC-related domains (tenant, api.*, copilot-api.*, copilot-telemetry-service.*) * to auto-add to the allowlist, or an empty array if not GHEC */ -export function extractGhecDomainsFromServerUrl( +function extractGhecDomainsFromServerUrl( env: Record = process.env ): string[] { const domains: string[] = []; @@ -264,7 +264,7 @@ export function extractGhecDomainsFromServerUrl( * @param env - Environment variables (defaults to process.env) * @returns Array of domains to auto-add to allowlist, or empty array if not GHES */ -export function extractGhesDomainsFromEngineApiTarget( +function extractGhesDomainsFromEngineApiTarget( env: Record = process.env ): string[] { const engineApiTarget = env['ENGINE_API_TARGET']; diff --git a/src/cli-workflow.test.ts b/src/cli-workflow.test.ts index e5ef790cd..4411bcab5 100644 --- a/src/cli-workflow.test.ts +++ b/src/cli-workflow.test.ts @@ -1,4 +1,4 @@ -import { runMainWorkflow, WorkflowDependencies } from './cli-workflow'; +import { runMainWorkflow } from './cli-workflow'; import { WrapperConfig } from './types'; import { HostAccessConfig } from './host-iptables'; @@ -22,7 +22,7 @@ const createLogger = () => ({ describe('runMainWorkflow', () => { it('executes workflow steps in order and logs success for zero exit code', async () => { const callOrder: string[] = []; - const dependencies: WorkflowDependencies = { + const dependencies = { ensureFirewallNetwork: jest.fn().mockImplementation(async () => { callOrder.push('ensureFirewallNetwork'); return { squidIp: '172.30.0.10' }; @@ -69,7 +69,7 @@ describe('runMainWorkflow', () => { ...baseConfig, agentTimeout: 30, }; - const dependencies: WorkflowDependencies = { + const dependencies = { ensureFirewallNetwork: jest.fn().mockResolvedValue({ squidIp: '172.30.0.10' }), setupHostIptables: jest.fn().mockResolvedValue(undefined), writeConfigs: jest.fn().mockResolvedValue(undefined), @@ -90,7 +90,7 @@ describe('runMainWorkflow', () => { }); it('passes undefined agentTimeout when not set', async () => { - const dependencies: WorkflowDependencies = { + const dependencies = { ensureFirewallNetwork: jest.fn().mockResolvedValue({ squidIp: '172.30.0.10' }), setupHostIptables: jest.fn().mockResolvedValue(undefined), writeConfigs: jest.fn().mockResolvedValue(undefined), @@ -116,7 +116,7 @@ describe('runMainWorkflow', () => { enableHostAccess: true, allowHostPorts: '3000,8080', }; - const dependencies: WorkflowDependencies = { + const dependencies = { ensureFirewallNetwork: jest.fn().mockResolvedValue({ squidIp: '172.30.0.10', proxyIp: '172.30.0.30' }), setupHostIptables: jest.fn().mockResolvedValue(undefined), writeConfigs: jest.fn().mockResolvedValue(undefined), @@ -141,7 +141,7 @@ describe('runMainWorkflow', () => { allowHostPorts: '3000', allowHostServicePorts: '5432,6379', }; - const dependencies: WorkflowDependencies = { + const dependencies = { ensureFirewallNetwork: jest.fn().mockResolvedValue({ squidIp: '172.30.0.10', proxyIp: '172.30.0.30' }), setupHostIptables: jest.fn().mockResolvedValue(undefined), writeConfigs: jest.fn().mockResolvedValue(undefined), @@ -164,7 +164,7 @@ describe('runMainWorkflow', () => { }); it('passes undefined hostAccess when enableHostAccess is not set', async () => { - const dependencies: WorkflowDependencies = { + const dependencies = { ensureFirewallNetwork: jest.fn().mockResolvedValue({ squidIp: '172.30.0.10', proxyIp: '172.30.0.30' }), setupHostIptables: jest.fn().mockResolvedValue(undefined), writeConfigs: jest.fn().mockResolvedValue(undefined), @@ -183,7 +183,7 @@ describe('runMainWorkflow', () => { it('logs warning with exit code when command fails', async () => { const callOrder: string[] = []; - const dependencies: WorkflowDependencies = { + const dependencies = { ensureFirewallNetwork: jest.fn().mockImplementation(async () => { callOrder.push('ensureFirewallNetwork'); return { squidIp: '172.30.0.10' }; @@ -231,7 +231,7 @@ describe('runMainWorkflow', () => { ...baseConfig, diagnosticLogs: true, }; - const dependencies: WorkflowDependencies = { + const dependencies = { ensureFirewallNetwork: jest.fn().mockResolvedValue({ squidIp: '172.30.0.10' }), setupHostIptables: jest.fn().mockResolvedValue(undefined), writeConfigs: jest.fn().mockResolvedValue(undefined), @@ -257,7 +257,7 @@ describe('runMainWorkflow', () => { it('does not call collectDiagnosticLogs when diagnosticLogs is disabled', async () => { const collectDiagnosticLogs = jest.fn().mockResolvedValue(undefined); - const dependencies: WorkflowDependencies = { + const dependencies = { ensureFirewallNetwork: jest.fn().mockResolvedValue({ squidIp: '172.30.0.10' }), setupHostIptables: jest.fn().mockResolvedValue(undefined), writeConfigs: jest.fn().mockResolvedValue(undefined), @@ -278,7 +278,7 @@ describe('runMainWorkflow', () => { ...baseConfig, diagnosticLogs: true, }; - const dependencies: WorkflowDependencies = { + const dependencies = { ensureFirewallNetwork: jest.fn().mockResolvedValue({ squidIp: '172.30.0.10' }), setupHostIptables: jest.fn().mockResolvedValue(undefined), writeConfigs: jest.fn().mockResolvedValue(undefined), @@ -298,7 +298,7 @@ describe('runMainWorkflow', () => { ...baseConfig, diagnosticLogs: true, }; - const dependencies: WorkflowDependencies = { + const dependencies = { ensureFirewallNetwork: jest.fn().mockResolvedValue({ squidIp: '172.30.0.10' }), setupHostIptables: jest.fn().mockResolvedValue(undefined), writeConfigs: jest.fn().mockResolvedValue(undefined), @@ -318,7 +318,7 @@ describe('runMainWorkflow', () => { ...baseConfig, diagnosticLogs: true, }; - const dependencies: WorkflowDependencies = { + const dependencies = { ensureFirewallNetwork: jest.fn().mockResolvedValue({ squidIp: '172.30.0.10' }), setupHostIptables: jest.fn().mockResolvedValue(undefined), writeConfigs: jest.fn().mockResolvedValue(undefined), @@ -337,7 +337,7 @@ describe('runMainWorkflow', () => { it('does not call collectDiagnosticLogs on startContainers failure when diagnosticLogs is disabled', async () => { const startError = new Error('Squid container is unhealthy'); const collectDiagnosticLogs = jest.fn().mockResolvedValue(undefined); - const dependencies: WorkflowDependencies = { + const dependencies = { ensureFirewallNetwork: jest.fn().mockResolvedValue({ squidIp: '172.30.0.10' }), setupHostIptables: jest.fn().mockResolvedValue(undefined), writeConfigs: jest.fn().mockResolvedValue(undefined), @@ -359,7 +359,7 @@ describe('runMainWorkflow', () => { ...baseConfig, diagnosticLogs: true, }; - const dependencies: WorkflowDependencies = { + const dependencies = { ensureFirewallNetwork: jest.fn().mockResolvedValue({ squidIp: '172.30.0.10' }), setupHostIptables: jest.fn().mockResolvedValue(undefined), writeConfigs: jest.fn().mockResolvedValue(undefined), @@ -380,7 +380,7 @@ describe('runMainWorkflow', () => { ...baseConfig, diagnosticLogs: true, }; - const dependencies: WorkflowDependencies = { + const dependencies = { ensureFirewallNetwork: jest.fn().mockResolvedValue({ squidIp: '172.30.0.10' }), setupHostIptables: jest.fn().mockResolvedValue(undefined), writeConfigs: jest.fn().mockResolvedValue(undefined), @@ -403,7 +403,7 @@ describe('runMainWorkflow', () => { ...baseConfig, enableApiProxy: true, }; - const dependencies: WorkflowDependencies = { + const dependencies = { ensureFirewallNetwork: jest.fn().mockResolvedValue({ squidIp: '172.30.0.10', proxyIp: '172.30.0.30', agentIp: '172.30.0.20', subnet: '172.30.0.0/24' }), setupHostIptables: jest.fn().mockResolvedValue(undefined), writeConfigs: jest.fn().mockResolvedValue(undefined), @@ -420,7 +420,7 @@ describe('runMainWorkflow', () => { }); it('passes undefined apiProxyIp when enableApiProxy is false', async () => { - const dependencies: WorkflowDependencies = { + const dependencies = { ensureFirewallNetwork: jest.fn().mockResolvedValue({ squidIp: '172.30.0.10', proxyIp: '172.30.0.30', agentIp: '172.30.0.20', subnet: '172.30.0.0/24' }), setupHostIptables: jest.fn().mockResolvedValue(undefined), writeConfigs: jest.fn().mockResolvedValue(undefined), @@ -441,7 +441,7 @@ describe('runMainWorkflow', () => { ...baseConfig, dnsOverHttps: 'https://dns.google/dns-query', }; - const dependencies: WorkflowDependencies = { + const dependencies = { ensureFirewallNetwork: jest.fn().mockResolvedValue({ squidIp: '172.30.0.10', proxyIp: '172.30.0.30', agentIp: '172.30.0.20', subnet: '172.30.0.0/24' }), setupHostIptables: jest.fn().mockResolvedValue(undefined), writeConfigs: jest.fn().mockResolvedValue(undefined), @@ -458,7 +458,7 @@ describe('runMainWorkflow', () => { }); it('passes undefined dohProxyIp when dnsOverHttps is not set', async () => { - const dependencies: WorkflowDependencies = { + const dependencies = { ensureFirewallNetwork: jest.fn().mockResolvedValue({ squidIp: '172.30.0.10' }), setupHostIptables: jest.fn().mockResolvedValue(undefined), writeConfigs: jest.fn().mockResolvedValue(undefined), @@ -479,7 +479,7 @@ describe('runMainWorkflow', () => { ...baseConfig, difcProxyHost: 'proxy.corp.com:18443', }; - const dependencies: WorkflowDependencies = { + const dependencies = { ensureFirewallNetwork: jest.fn().mockResolvedValue({ squidIp: '172.30.0.10', proxyIp: '172.30.0.30', agentIp: '172.30.0.20', subnet: '172.30.0.0/24' }), setupHostIptables: jest.fn().mockResolvedValue(undefined), writeConfigs: jest.fn().mockResolvedValue(undefined), @@ -497,7 +497,7 @@ describe('runMainWorkflow', () => { }); it('passes undefined cliProxyConfig when difcProxyHost is not set', async () => { - const dependencies: WorkflowDependencies = { + const dependencies = { ensureFirewallNetwork: jest.fn().mockResolvedValue({ squidIp: '172.30.0.10' }), setupHostIptables: jest.fn().mockResolvedValue(undefined), writeConfigs: jest.fn().mockResolvedValue(undefined), @@ -520,7 +520,7 @@ describe('runMainWorkflow', () => { diagnosticLogs: true, }; const performCleanup = jest.fn().mockResolvedValue(undefined); - const dependencies: WorkflowDependencies = { + const dependencies = { ensureFirewallNetwork: jest.fn().mockResolvedValue({ squidIp: '172.30.0.10' }), setupHostIptables: jest.fn().mockResolvedValue(undefined), writeConfigs: jest.fn().mockResolvedValue(undefined), diff --git a/src/cli-workflow.ts b/src/cli-workflow.ts index 94b12eefb..6325f7b98 100644 --- a/src/cli-workflow.ts +++ b/src/cli-workflow.ts @@ -3,7 +3,7 @@ import { HostAccessConfig, CliProxyHostConfig } from './host-iptables'; import { DEFAULT_DNS_SERVERS } from './dns-resolver'; import { parseDifcProxyHost } from './docker-manager'; -export interface WorkflowDependencies { +interface WorkflowDependencies { ensureFirewallNetwork: () => Promise<{ squidIp: string; agentIp: string; proxyIp: string; subnet: string }>; setupHostIptables: (squidIp: string, port: number, dnsServers: string[], apiProxyIp?: string, dohProxyIp?: string, hostAccess?: HostAccessConfig, cliProxyConfig?: CliProxyHostConfig) => Promise; writeConfigs: (config: WrapperConfig) => Promise; diff --git a/src/dns-resolver.test.ts b/src/dns-resolver.test.ts index c6822b384..c9ead9bfe 100644 --- a/src/dns-resolver.test.ts +++ b/src/dns-resolver.test.ts @@ -1,4 +1,4 @@ -import { parseResolvConf, detectHostDnsServers, DEFAULT_DNS_SERVERS } from './dns-resolver'; +import { detectHostDnsServers, DEFAULT_DNS_SERVERS } from './dns-resolver'; import * as fs from 'fs'; jest.mock('fs'); @@ -17,18 +17,18 @@ beforeEach(() => { jest.clearAllMocks(); }); -describe('parseResolvConf', () => { +describe('parseResolvConf (via detectHostDnsServers)', () => { it('extracts nameservers from standard content', () => { - const content = `# Generated by systemd-resolved + mockedFs.readFileSync.mockReturnValue(`# Generated by systemd-resolved nameserver 1.1.1.1 nameserver 9.9.9.9 search example.com -`; - expect(parseResolvConf(content)).toEqual(['1.1.1.1', '9.9.9.9']); +`); + expect(detectHostDnsServers(mockLogger as any)).toEqual(['1.1.1.1', '9.9.9.9']); }); it('ignores comments and empty lines', () => { - const content = ` + mockedFs.readFileSync.mockReturnValue(` # This is a comment ; Another comment style @@ -36,24 +36,25 @@ nameserver 1.1.1.1 # nameserver 2.2.2.2 nameserver 8.8.8.8 -`; - expect(parseResolvConf(content)).toEqual(['1.1.1.1', '8.8.8.8']); +`); + expect(detectHostDnsServers(mockLogger as any)).toEqual(['1.1.1.1', '8.8.8.8']); }); it('skips invalid IPs', () => { - const content = `nameserver 1.1.1.1 + mockedFs.readFileSync.mockReturnValue(`nameserver 1.1.1.1 nameserver not-an-ip nameserver 8.8.8.8 -`; - expect(parseResolvConf(content)).toEqual(['1.1.1.1', '8.8.8.8']); +`); + expect(detectHostDnsServers(mockLogger as any)).toEqual(['1.1.1.1', '8.8.8.8']); }); - it('handles IPv6 nameservers', () => { - const content = `nameserver 2001:4860:4860::8888 + it('handles IPv6 nameservers (loopback filtered by detectHostDnsServers)', () => { + mockedFs.readFileSync.mockReturnValue(`nameserver 2001:4860:4860::8888 nameserver 1.1.1.1 nameserver ::1 -`; - expect(parseResolvConf(content)).toEqual(['2001:4860:4860::8888', '1.1.1.1', '::1']); +`); + // ::1 is loopback so detectHostDnsServers filters it out + expect(detectHostDnsServers(mockLogger as any)).toEqual(['2001:4860:4860::8888', '1.1.1.1']); }); }); diff --git a/src/dns-resolver.ts b/src/dns-resolver.ts index 7f352ab96..ab384984d 100644 --- a/src/dns-resolver.ts +++ b/src/dns-resolver.ts @@ -30,7 +30,7 @@ function isLoopback(ip: string): boolean { * Parse nameserver entries from resolv.conf content. * Pure function — no I/O. */ -export function parseResolvConf(content: string): string[] { +function parseResolvConf(content: string): string[] { const servers: string[] = []; for (const line of content.split('\n')) { const match = line.match(/^\s*nameserver\s+(\S+)/); diff --git a/src/docker-manager-utils.test.ts b/src/docker-manager-utils.test.ts index 899807dd2..b463641e4 100644 --- a/src/docker-manager-utils.test.ts +++ b/src/docker-manager-utils.test.ts @@ -7,8 +7,6 @@ import { } from './host-env'; import { hostEnvTestHelpers } from './host-env.test-utils'; import { - validateIdNotInSystemRange, - MIN_REGULAR_UID, ACT_PRESET_BASE_IMAGE, } from './host-identity'; import { @@ -16,7 +14,6 @@ import { readGitHubPathEntries, mergeGitHubPathEntries, readGitHubEnvEntries, - parseGitHubEnvFile, readEnvFile, } from './github-env'; import * as fs from 'fs'; @@ -71,18 +68,40 @@ describe('docker-manager utilities', () => { }); }); - describe('validateIdNotInSystemRange', () => { + describe('validateIdNotInSystemRange (via getSafeHostUid)', () => { + const originalGetuid = process.getuid; + const originalSudoUid = process.env.SUDO_UID; + + afterEach(() => { + process.getuid = originalGetuid; + if (originalSudoUid !== undefined) { + process.env.SUDO_UID = originalSudoUid; + } else { + delete process.env.SUDO_UID; + } + }); + it('should return 1000 for system UIDs (0-999)', () => { - expect(validateIdNotInSystemRange(0)).toBe('1000'); - expect(validateIdNotInSystemRange(1)).toBe('1000'); - expect(validateIdNotInSystemRange(13)).toBe('1000'); // proxy user - expect(validateIdNotInSystemRange(999)).toBe('1000'); + // Test via SUDO_UID path which calls validateIdNotInSystemRange + process.getuid = () => 0; + process.env.SUDO_UID = '0'; + expect(getSafeHostUid()).toBe('1000'); + process.env.SUDO_UID = '1'; + expect(getSafeHostUid()).toBe('1000'); + process.env.SUDO_UID = '13'; // proxy user + expect(getSafeHostUid()).toBe('1000'); + process.env.SUDO_UID = '999'; + expect(getSafeHostUid()).toBe('1000'); }); it('should return the UID as-is for regular users (>= 1000)', () => { - expect(validateIdNotInSystemRange(1000)).toBe('1000'); - expect(validateIdNotInSystemRange(1001)).toBe('1001'); - expect(validateIdNotInSystemRange(65534)).toBe('65534'); // nobody user on some systems + process.getuid = () => 0; + process.env.SUDO_UID = '1000'; + expect(getSafeHostUid()).toBe('1000'); + process.env.SUDO_UID = '1001'; + expect(getSafeHostUid()).toBe('1001'); + process.env.SUDO_UID = '65534'; // nobody user on some systems + expect(getSafeHostUid()).toBe('65534'); }); }); @@ -311,9 +330,27 @@ describe('docker-manager utilities', () => { }); }); - describe('MIN_REGULAR_UID constant', () => { - it('should be 1000 (standard Linux regular user UID threshold)', () => { - expect(MIN_REGULAR_UID).toBe(1000); + describe('MIN_REGULAR_UID threshold (via getSafeHostUid)', () => { + const originalGetuid = process.getuid; + const originalSudoUid = process.env.SUDO_UID; + + afterEach(() => { + process.getuid = originalGetuid; + if (originalSudoUid !== undefined) { + process.env.SUDO_UID = originalSudoUid; + } else { + delete process.env.SUDO_UID; + } + }); + + it('should treat 1000 as the minimum regular UID threshold', () => { + process.getuid = () => 0; + // UID 999 is in system range → returns 1000 + process.env.SUDO_UID = '999'; + expect(getSafeHostUid()).toBe('1000'); + // UID 1000 is the first regular UID → returns 1000 + process.env.SUDO_UID = '1000'; + expect(getSafeHostUid()).toBe('1000'); }); }); @@ -444,9 +481,33 @@ describe('docker-manager utilities', () => { }); }); - describe('parseGitHubEnvFile', () => { + describe('parseGitHubEnvFile (via readGitHubEnvEntries)', () => { + let tmpDir: string; + let originalGithubEnv: string | undefined; + + beforeEach(() => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'awf-parse-env-')); + originalGithubEnv = process.env.GITHUB_ENV; + }); + + afterEach(() => { + if (originalGithubEnv !== undefined) { + process.env.GITHUB_ENV = originalGithubEnv; + } else { + delete process.env.GITHUB_ENV; + } + fs.rmSync(tmpDir, { recursive: true, force: true }); + }); + + function parseViaPublicApi(content: string): Record { + const envFile = path.join(tmpDir, 'env'); + fs.writeFileSync(envFile, content); + process.env.GITHUB_ENV = envFile; + return readGitHubEnvEntries(); + } + it('should parse simple KEY=VALUE entries', () => { - const result = parseGitHubEnvFile('GOROOT=/usr/local/go\nJAVA_HOME=/usr/lib/jvm/java-17\n'); + const result = parseViaPublicApi('GOROOT=/usr/local/go\nJAVA_HOME=/usr/lib/jvm/java-17\n'); expect(result).toEqual({ GOROOT: '/usr/local/go', JAVA_HOME: '/usr/lib/jvm/java-17', @@ -454,18 +515,18 @@ describe('docker-manager utilities', () => { }); it('should handle values containing = characters', () => { - const result = parseGitHubEnvFile('MY_VAR=key=value=extra\n'); + const result = parseViaPublicApi('MY_VAR=key=value=extra\n'); expect(result).toEqual({ MY_VAR: 'key=value=extra' }); }); it('should handle heredoc multiline values', () => { const content = 'MULTI_LINE< { - const result = parseGitHubEnvFile('GOROOT=/usr/local/go\r\nJAVA_HOME=/usr/lib/jvm\r\n'); + const result = parseViaPublicApi('GOROOT=/usr/local/go\r\nJAVA_HOME=/usr/lib/jvm\r\n'); expect(result).toEqual({ GOROOT: '/usr/local/go', JAVA_HOME: '/usr/lib/jvm', @@ -474,7 +535,7 @@ describe('docker-manager utilities', () => { it('should handle mixed simple and heredoc entries', () => { const content = 'SIMPLE=value\nHEREDOC< { }); it('should skip empty lines', () => { - const result = parseGitHubEnvFile('\n\nGOROOT=/go\n\n'); + const result = parseViaPublicApi('\n\nGOROOT=/go\n\n'); expect(result).toEqual({ GOROOT: '/go' }); }); it('should return empty object for empty content', () => { - expect(parseGitHubEnvFile('')).toEqual({}); + expect(parseViaPublicApi('')).toEqual({}); }); it('should handle unterminated heredoc gracefully', () => { const content = 'BROKEN< { }); }); -describe('parseGitHubEnvFile', () => { +describe('parseGitHubEnvFile (via readGitHubEnvEntries)', () => { + let testDir: string; + let originalGithubEnv: string | undefined; + + beforeEach(() => { + testDir = fs.mkdtempSync(path.join(os.tmpdir(), 'awf-parse-env-test-')); + originalGithubEnv = process.env.GITHUB_ENV; + }); + + afterEach(() => { + if (originalGithubEnv !== undefined) { + process.env.GITHUB_ENV = originalGithubEnv; + } else { + delete process.env.GITHUB_ENV; + } + if (fs.existsSync(testDir)) { + fs.rmSync(testDir, { recursive: true, force: true }); + } + }); + + function parseViaPublicApi(content: string): Record { + const envFile = path.join(testDir, 'env'); + fs.writeFileSync(envFile, content); + process.env.GITHUB_ENV = envFile; + return readGitHubEnvEntries(); + } + describe('simple format', () => { it('should parse single KEY=VALUE line', () => { - expect(parseGitHubEnvFile('FOO=bar\n')).toEqual({ FOO: 'bar' }); + expect(parseViaPublicApi('FOO=bar\n')).toEqual({ FOO: 'bar' }); }); it('should parse multiple KEY=VALUE lines', () => { - expect(parseGitHubEnvFile('FOO=bar\nBAZ=qux\n')).toEqual({ + expect(parseViaPublicApi('FOO=bar\nBAZ=qux\n')).toEqual({ FOO: 'bar', BAZ: 'qux', }); }); it('should handle values with equals signs', () => { - expect(parseGitHubEnvFile('URL=https://example.com?key=value\n')).toEqual({ + expect(parseViaPublicApi('URL=https://example.com?key=value\n')).toEqual({ URL: 'https://example.com?key=value', }); }); it('should handle empty values', () => { - expect(parseGitHubEnvFile('EMPTY=\n')).toEqual({ EMPTY: '' }); + expect(parseViaPublicApi('EMPTY=\n')).toEqual({ EMPTY: '' }); }); it('should ignore empty lines', () => { - expect(parseGitHubEnvFile('FOO=bar\n\nBAZ=qux\n')).toEqual({ + expect(parseViaPublicApi('FOO=bar\n\nBAZ=qux\n')).toEqual({ FOO: 'bar', BAZ: 'qux', }); }); it('should ignore whitespace-only lines', () => { - expect(parseGitHubEnvFile('FOO=bar\n \t\nBAZ=qux\n')).toEqual({ + expect(parseViaPublicApi('FOO=bar\n \t\nBAZ=qux\n')).toEqual({ FOO: 'bar', BAZ: 'qux', }); }); it('should handle values with spaces', () => { - expect(parseGitHubEnvFile('MESSAGE=hello world\n')).toEqual({ + expect(parseViaPublicApi('MESSAGE=hello world\n')).toEqual({ MESSAGE: 'hello world', }); }); @@ -182,24 +207,24 @@ describe('parseGitHubEnvFile', () => { describe('heredoc format', () => { it('should parse single-line heredoc', () => { const input = 'FOO< { const input = 'FOO< { const input = 'FOO< { const input = 'FOO< { it('should handle mixed simple and heredoc format', () => { const input = 'SIMPLE=value\nHEREDOC< { it('should handle heredoc with empty content', () => { const input = 'FOO< { const input = 'URL< { describe('CRLF handling', () => { it('should normalize CRLF to LF in simple format', () => { - expect(parseGitHubEnvFile('FOO=bar\r\n')).toEqual({ FOO: 'bar' }); + expect(parseViaPublicApi('FOO=bar\r\n')).toEqual({ FOO: 'bar' }); }); it('should normalize CRLF to LF in heredoc', () => { const input = 'FOO< { describe('edge cases', () => { it('should handle empty input', () => { - expect(parseGitHubEnvFile('')).toEqual({}); + expect(parseViaPublicApi('')).toEqual({}); }); it('should ignore lines without equals sign (simple format)', () => { - expect(parseGitHubEnvFile('FOO=bar\nINVALID\nBAZ=qux\n')).toEqual({ + expect(parseViaPublicApi('FOO=bar\nINVALID\nBAZ=qux\n')).toEqual({ FOO: 'bar', BAZ: 'qux', }); @@ -255,7 +280,7 @@ describe('parseGitHubEnvFile', () => { it('should handle unclosed heredoc gracefully', () => { // Missing closing delimiter - all remaining lines become the value const input = 'FOO< { it('should handle heredoc with delimiter appearing in content', () => { // Only exact match on its own line closes heredoc const input = 'FOO< { /** * Parses the content of a $GITHUB_ENV file into key-value pairs. - * @internal Exported for testing */ -export function parseGitHubEnvFile(content: string): Record { +function parseGitHubEnvFile(content: string): Record { const result: Record = {}; // Normalize CRLF to LF const lines = content.replace(/\r\n/g, '\n').split('\n'); diff --git a/src/host-identity.ts b/src/host-identity.ts index 4ec0abf6f..e81a51e49 100644 --- a/src/host-identity.ts +++ b/src/host-identity.ts @@ -10,14 +10,13 @@ export const ACT_PRESET_BASE_IMAGE = 'ghcr.io/catthehacker/ubuntu:act-24.04'; * Minimum UID/GID value for regular users. * UIDs 0-999 are reserved for system users on most Linux distributions. */ -export const MIN_REGULAR_UID = 1000; +const MIN_REGULAR_UID = 1000; /** * Validates that a UID/GID value is safe for use (not in system range). * Returns the value if valid, or the default (1000) if in system range. - * @internal Exported for testing */ -export function validateIdNotInSystemRange(id: number): string { +function validateIdNotInSystemRange(id: number): string { // Reject system UIDs/GIDs (0-999) - use default unprivileged user instead if (id < MIN_REGULAR_UID) { return MIN_REGULAR_UID.toString();