Skip to content

refactor: demote test-only exports to module-private #4231

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
lpcox merged 3 commits into from
Jun 2, 2026
Merged

refactor: demote test-only exports to module-private #4231

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
402cf50
refactor: demote test-only exports to module-private
lpcox Jun 2, 2026
f7df8fa
Merge remote-tracking branch 'origin/main' into copilot/export-audit-…
Copilot Jun 2, 2026
6aca802
test: remove substring assertions in domain tests
Copilot Jun 2, 2026
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
105 changes: 50 additions & 55 deletions src/api-proxy-config-domains.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
import {
resolveApiTargetsToAllowedDomains,
extractGhesDomainsFromEngineApiTarget,
extractGhecDomainsFromServerUrl,
} from './api-proxy-config';

describe('resolveApiTargetsToAllowedDomains', () => {
Expand Down Expand Up @@ -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');
Expand All @@ -179,77 +171,87 @@ 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');
expect(domains).toContain('telemetry.enterprise.githubcopilot.com');
});

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');
expect(domains).toContain('copilot-telemetry-service.myorg.ghe.com');
});

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');
expect(domains).toContain('copilot-telemetry-service.acme.ghe.com');
});

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',
});
Expand All @@ -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);
});
});

Expand Down
115 changes: 59 additions & 56 deletions src/api-proxy-config-validation.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import {
validateApiProxyConfig,
validateAnthropicCacheTailTtl,
validateApiTargetInAllowedDomains,
emitApiProxyTargetWarnings,
} from './api-proxy-config';

describe('validateApiProxyConfig', () => {
Expand Down Expand Up @@ -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([]);
});
});

Expand Down
6 changes: 3 additions & 3 deletions src/api-proxy-config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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<string, string | undefined> = process.env
): string[] {
const domains: string[] = [];
Expand Down Expand Up @@ -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<string, string | undefined> = process.env
): string[] {
const engineApiTarget = env['ENGINE_API_TARGET'];
Expand Down
Loading
Loading