Skip to content
Merged
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
43 changes: 25 additions & 18 deletions src/api-proxy-config-domains.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,58 +6,58 @@ 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).not.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).not.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).not.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');
expect(domains).toContain('https://flag.copilot.com');
expect(domains).not.toContain('https://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).not.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');
expect(domains).toContain('https://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');
expect(domains).toContain('https://env.anthropic.com');
});

it('should not duplicate a domain already in the list', () => {
const domains: string[] = ['custom.copilot.com'];
const domains: string[] = ['https://custom.copilot.com'];
resolveApiTargetsToAllowedDomains({ copilotApiTarget: 'custom.copilot.com' }, domains);
const count = domains.filter(d => d === 'custom.copilot.com').length;
const count = domains.filter(d => d === 'https://custom.copilot.com').length;
expect(count).toBe(1);
});

Expand Down Expand Up @@ -92,9 +92,9 @@ describe('resolveApiTargetsToAllowedDomains', () => {
},
domains
);
expect(domains).toContain('copilot.internal');
expect(domains).toContain('openai.internal');
expect(domains).toContain('anthropic.internal');
expect(domains).toContain('https://copilot.internal');
expect(domains).toContain('https://openai.internal');
expect(domains).toContain('https://anthropic.internal');
});

it('should call debug with auto-added domains', () => {
Expand Down Expand Up @@ -129,26 +129,33 @@ describe('resolveApiTargetsToAllowedDomains', () => {
expect(domains).toHaveLength(0);
});

it('should trim whitespace from api targets', () => {
const domains: string[] = [];
resolveApiTargetsToAllowedDomains({ openaiApiTarget: ' custom.openai.com ' }, domains);
expect(domains).toContain('https://custom.openai.com');
expect(domains).not.toContain('https:// custom.openai.com ');
});

it('should add gemini-api-target option to allowed domains', () => {
const domains: string[] = ['github.com'];
resolveApiTargetsToAllowedDomains({ geminiApiTarget: 'custom.gemini.internal' }, domains);
expect(domains).toContain('custom.gemini.internal');
expect(domains).not.toContain('custom.gemini.internal');
expect(domains).toContain('https://custom.gemini.internal');
});

it('should read GEMINI_API_TARGET from env when flag not set', () => {
const domains: string[] = [];
const env = { GEMINI_API_TARGET: 'env.gemini.internal' };
resolveApiTargetsToAllowedDomains({}, domains, env);
expect(domains).toContain('env.gemini.internal');
expect(domains).toContain('https://env.gemini.internal');
});

it('should prefer geminiApiTarget option over GEMINI_API_TARGET env var', () => {
const domains: string[] = [];
const env = { GEMINI_API_TARGET: 'env.gemini.internal' };
resolveApiTargetsToAllowedDomains({ geminiApiTarget: 'flag.gemini.internal' }, domains, env);
expect(domains).toContain('flag.gemini.internal');
expect(domains).not.toContain('env.gemini.internal');
expect(domains).toContain('https://flag.gemini.internal');
expect(domains).not.toContain('https://env.gemini.internal');
});
});

Expand Down Expand Up @@ -368,7 +375,7 @@ describe('resolveApiTargetsToAllowedDomains with GHES', () => {
expect(domains).toContain('api.githubcopilot.com');
expect(domains).toContain('api.enterprise.githubcopilot.com');
expect(domains).toContain('telemetry.enterprise.githubcopilot.com');
expect(domains).toContain('custom.copilot.com');
expect(domains).not.toContain('custom.copilot.com');
expect(domains).toContain('https://custom.copilot.com');
});
});
20 changes: 20 additions & 0 deletions src/api-proxy-config-validation.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,26 @@ describe('validateApiTargetInAllowedDomains (via emitApiProxyTargetWarnings)', (
expect(warnings.filter(w => w.includes('openai-api-target'))).toEqual([]);
});

it('should not warn when custom host is in allowed domains as https:// entry (auto-added form)', () => {
const warnings: string[] = [];
emitApiProxyTargetWarnings(
{ enableApiProxy: true, openaiApiTarget: 'custom.example.com' },
['https://custom.example.com'],
(msg) => warnings.push(msg)
);
expect(warnings.filter(w => w.includes('openai-api-target'))).toEqual([]);
});

it('should not warn when custom host is covered by https:// parent domain entry', () => {
const warnings: string[] = [];
emitApiProxyTargetWarnings(
{ enableApiProxy: true, openaiApiTarget: 'api.example.com' },
['https://example.com'],
(msg) => warnings.push(msg)
);
expect(warnings.filter(w => w.includes('openai-api-target'))).toEqual([]);
});

it('should not warn when custom host matches a parent domain in allowed list', () => {
const warnings: string[] = [];
emitApiProxyTargetWarnings(
Expand Down
31 changes: 12 additions & 19 deletions src/api-proxy-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,9 +96,13 @@ function validateApiTargetInAllowedDomains(
// No warning needed if using the default host
if (targetHost === defaultHost) return null;

// Check if the hostname or any of its parent domains is explicitly allowed
// Check if the hostname or any of its parent domains is explicitly allowed.
// Strip any https?:// prefix from allowedDomains entries so that auto-added
// protocol-scoped entries (e.g. "https://custom.example.com") are matched
// correctly against the bare targetHost.
const isDomainAllowed = allowedDomains.some(d => {
const domain = d.startsWith('.') ? d.slice(1) : d;
const withoutProtocol = d.replace(/^https?:\/\//, '');
const domain = withoutProtocol.startsWith('.') ? withoutProtocol.slice(1) : withoutProtocol;
return targetHost === domain || targetHost.endsWith('.' + domain);
});

Expand Down Expand Up @@ -375,27 +379,16 @@ export function resolveApiTargetsToAllowedDomains(
debug(`Auto-added GHES domains from engine.api-target: ${ghesDomains.join(', ')}`);
}

// Merge raw target values into the allowedDomains list so that later
// checks/logs about "no allowed domains" see the final, expanded allowlist.
// Auto-add API targets to allowed domains. Bare hostnames are promoted to https:// only
// to avoid over-broad HTTP+HTTPS allowlisting; explicit http:// or https:// prefixes
// are preserved as-is so deployments that intentionally use HTTP continue to work.
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}`;

const trimmedTarget = target.trim();
const normalizedTarget = /^https?:\/\//.test(trimmedTarget) ? trimmedTarget : `https://${trimmedTarget}`;
if (!allowedDomains.includes(normalizedTarget)) {
Comment on lines 386 to 389
allowedDomains.push(normalizedTarget);
debug(`Automatically added API target to allowlist: ${normalizedTarget}`);
debug(`Auto-added API target to allowed domains: ${normalizedTarget}`);
Comment on lines 386 to +391
}
}

Expand Down
2 changes: 1 addition & 1 deletion tests/integration/api-target-allowlist.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,6 @@ describe('API Target Allowlist', () => {
expect(result.stderr).toContain('api.anthropic.custom.com');

// Should see debug log messages for each auto-added domain
expect(result.stderr).toContain('Automatically added API target to allowlist');
expect(result.stderr).toContain('Auto-added API target to allowed domains');
}, 120000);
});
Loading