diff --git a/containers/api-proxy/server.js b/containers/api-proxy/server.js index 76a83b47d..e409bdebb 100644 --- a/containers/api-proxy/server.js +++ b/containers/api-proxy/server.js @@ -64,10 +64,48 @@ const ANTHROPIC_API_KEY = (process.env.ANTHROPIC_API_KEY || '').trim() || undefi const COPILOT_GITHUB_TOKEN = (process.env.COPILOT_GITHUB_TOKEN || '').trim() || undefined; const GEMINI_API_KEY = (process.env.GEMINI_API_KEY || '').trim() || undefined; +/** + * Normalizes an API target value to a bare hostname. + * Accepts either a hostname or a full URL and extracts only the hostname, + * discarding any scheme, path, query, fragment, credentials, or port. + * Path configuration must be provided separately via the existing + * *_API_BASE_PATH environment variables. + * + * @param {string|undefined} value - Raw env var value + * @returns {string|undefined} Bare hostname, or undefined if input is falsy + */ +function normalizeApiTarget(value) { + if (!value) return value; + + const trimmed = value.trim(); + if (!trimmed) return undefined; + + const candidate = /^[a-zA-Z][a-zA-Z\d+\-.]*:\/\//.test(trimmed) + ? trimmed + : `https://${trimmed}`; + + try { + const parsed = new URL(candidate); + + if (parsed.pathname !== '/' || parsed.search || parsed.hash || parsed.username || parsed.password || parsed.port) { + console.warn( + `Ignoring unsupported API target URL components in ${sanitizeForLog(trimmed)}; ` + + 'configure path prefixes via the corresponding *_API_BASE_PATH environment variable.' + ); + } + + return parsed.hostname || undefined; + } catch (err) { + console.warn(`Invalid API target ${sanitizeForLog(trimmed)}; expected a hostname (e.g. 'api.example.com') or URL`); + return undefined; + } +} + // Configurable API target hosts (supports custom endpoints / internal LLM routers) -const OPENAI_API_TARGET = process.env.OPENAI_API_TARGET || 'api.openai.com'; -const ANTHROPIC_API_TARGET = process.env.ANTHROPIC_API_TARGET || 'api.anthropic.com'; -const GEMINI_API_TARGET = process.env.GEMINI_API_TARGET || 'generativelanguage.googleapis.com'; +// Values are normalized to bare hostnames — buildUpstreamPath() prepends https:// +const OPENAI_API_TARGET = normalizeApiTarget(process.env.OPENAI_API_TARGET) || 'api.openai.com'; +const ANTHROPIC_API_TARGET = normalizeApiTarget(process.env.ANTHROPIC_API_TARGET) || 'api.anthropic.com'; +const GEMINI_API_TARGET = normalizeApiTarget(process.env.GEMINI_API_TARGET) || 'generativelanguage.googleapis.com'; /** * Normalizes a base path for use as a URL path prefix. @@ -123,7 +161,7 @@ const GEMINI_API_BASE_PATH = normalizeBasePath(process.env.GEMINI_API_BASE_PATH) // Priority: COPILOT_API_TARGET env var > auto-derive from GITHUB_SERVER_URL > default function deriveCopilotApiTarget() { if (process.env.COPILOT_API_TARGET) { - return process.env.COPILOT_API_TARGET; + return normalizeApiTarget(process.env.COPILOT_API_TARGET); } // Auto-derive from GITHUB_SERVER_URL: // - GitHub Enterprise Cloud (*.ghe.com): Copilot inference/models/MCP are served at @@ -932,4 +970,4 @@ if (require.main === module) { } // Export for testing -module.exports = { deriveCopilotApiTarget, normalizeBasePath, buildUpstreamPath, proxyWebSocket }; +module.exports = { normalizeApiTarget, deriveCopilotApiTarget, normalizeBasePath, buildUpstreamPath, proxyWebSocket }; diff --git a/containers/api-proxy/server.test.js b/containers/api-proxy/server.test.js index cd0c8ebe0..657d404c9 100644 --- a/containers/api-proxy/server.test.js +++ b/containers/api-proxy/server.test.js @@ -5,7 +5,46 @@ const http = require('http'); const tls = require('tls'); const { EventEmitter } = require('events'); -const { deriveCopilotApiTarget, normalizeBasePath, buildUpstreamPath, proxyWebSocket } = require('./server'); +const { normalizeApiTarget, deriveCopilotApiTarget, normalizeBasePath, buildUpstreamPath, proxyWebSocket } = require('./server'); + +describe('normalizeApiTarget', () => { + it('should strip https:// prefix', () => { + expect(normalizeApiTarget('https://my-gateway.example.com')).toBe('my-gateway.example.com'); + }); + + it('should strip http:// prefix', () => { + expect(normalizeApiTarget('http://my-gateway.example.com')).toBe('my-gateway.example.com'); + }); + + it('should preserve bare hostname', () => { + expect(normalizeApiTarget('api.openai.com')).toBe('api.openai.com'); + }); + + it('should normalize a URL with a path to just the hostname', () => { + expect(normalizeApiTarget('https://my-gateway.example.com/some-path')).toBe('my-gateway.example.com'); + }); + + it('should trim whitespace', () => { + expect(normalizeApiTarget(' https://api.openai.com ')).toBe('api.openai.com'); + }); + + it('should return undefined for falsy input', () => { + expect(normalizeApiTarget(undefined)).toBeUndefined(); + expect(normalizeApiTarget('')).toBe(''); + }); + + it('should not strip scheme-like substrings in the middle', () => { + expect(normalizeApiTarget('api.https.example.com')).toBe('api.https.example.com'); + }); + + it('should discard port from URL', () => { + expect(normalizeApiTarget('https://my-gateway.example.com:8443')).toBe('my-gateway.example.com'); + }); + + it('should discard query and fragment from URL', () => { + expect(normalizeApiTarget('https://my-gateway.example.com/path?key=val#frag')).toBe('my-gateway.example.com'); + }); +}); describe('deriveCopilotApiTarget', () => { let originalEnv; @@ -268,6 +307,25 @@ describe('buildUpstreamPath', () => { .toBe('/serving-endpoints/v1/chat/completions'); }); }); + + describe('with normalized API target (gh-aw#25137 regression)', () => { + it('should produce correct path when target was already normalized', () => { + // normalizeApiTarget('https://my-gateway.example.com/some-path') + // returns 'my-gateway.example.com' (hostname only) + const target = 'my-gateway.example.com'; + expect(buildUpstreamPath('/v1/messages', target, '')) + .toBe('/v1/messages'); + }); + + it('should produce wrong hostname if scheme is NOT stripped (demonstrating the bug)', () => { + // Without normalizeApiTarget, the scheme-prefixed value causes + // new URL() to parse 'https' as the hostname instead of the real host + const badTarget = 'https://my-gateway.example.com'; + const targetUrl = new URL('/v1/messages', `https://${badTarget}`); + // Node parses this as hostname='https', not 'my-gateway.example.com' + expect(targetUrl.hostname).not.toBe('my-gateway.example.com'); + }); + }); }); // ── Helpers for proxyWebSocket tests ────────────────────────────────────────── diff --git a/src/docker-manager.test.ts b/src/docker-manager.test.ts index 29b7284e6..08fdbdabd 100644 --- a/src/docker-manager.test.ts +++ b/src/docker-manager.test.ts @@ -1,4 +1,4 @@ -import { generateDockerCompose, subnetsOverlap, writeConfigs, startContainers, stopContainers, fastKillAgentContainer, isAgentExternallyKilled, resetAgentExternallyKilled, AGENT_CONTAINER_NAME, cleanup, runAgentCommand, validateIdNotInSystemRange, getSafeHostUid, getSafeHostGid, getRealUserHome, extractGhHostFromServerUrl, readGitHubPathEntries, mergeGitHubPathEntries, readEnvFile, MIN_REGULAR_UID, ACT_PRESET_BASE_IMAGE } from './docker-manager'; +import { generateDockerCompose, subnetsOverlap, writeConfigs, startContainers, stopContainers, fastKillAgentContainer, isAgentExternallyKilled, resetAgentExternallyKilled, AGENT_CONTAINER_NAME, cleanup, runAgentCommand, validateIdNotInSystemRange, getSafeHostUid, getSafeHostGid, getRealUserHome, extractGhHostFromServerUrl, readGitHubPathEntries, mergeGitHubPathEntries, readEnvFile, MIN_REGULAR_UID, ACT_PRESET_BASE_IMAGE, stripScheme } from './docker-manager'; import { WrapperConfig } from './types'; import * as fs from 'fs'; import * as path from 'path'; @@ -295,6 +295,28 @@ describe('docker-manager', () => { }); }); + describe('stripScheme', () => { + it('should strip https:// prefix', () => { + expect(stripScheme('https://my-gateway.example.com')).toBe('my-gateway.example.com'); + }); + + it('should strip http:// prefix', () => { + expect(stripScheme('http://my-gateway.example.com')).toBe('my-gateway.example.com'); + }); + + it('should preserve bare hostname', () => { + expect(stripScheme('api.openai.com')).toBe('api.openai.com'); + }); + + it('should normalize URL with path to hostname only', () => { + expect(stripScheme('https://my-gateway.example.com/some-path')).toBe('my-gateway.example.com'); + }); + + it('should not strip scheme-like substrings in the middle', () => { + expect(stripScheme('api.https.example.com')).toBe('api.https.example.com'); + }); + }); + describe('generateDockerCompose', () => { const mockConfig: WrapperConfig = { allowedDomains: ['github.com', 'npmjs.org'], @@ -2380,6 +2402,22 @@ describe('docker-manager', () => { expect(env.ANTHROPIC_API_TARGET).toBe('custom.anthropic-router.internal'); }); + it('should strip https:// scheme from API target values (gh-aw#25137)', () => { + const configWithProxy = { + ...mockConfig, + enableApiProxy: true, + anthropicApiKey: 'sk-ant-test-key', + anthropicApiTarget: 'https://my-gateway.example.com', + openaiApiKey: 'sk-openai-test', + openaiApiTarget: 'https://openai-router.internal', + }; + const result = generateDockerCompose(configWithProxy, mockNetworkConfigWithProxy); + const proxy = result.services['api-proxy']; + const env = proxy.environment as Record; + expect(env.ANTHROPIC_API_TARGET).toBe('my-gateway.example.com'); + expect(env.OPENAI_API_TARGET).toBe('openai-router.internal'); + }); + it('should not set ANTHROPIC_API_TARGET in api-proxy when anthropicApiTarget is not provided', () => { const configWithProxy = { ...mockConfig, enableApiProxy: true, anthropicApiKey: 'sk-ant-test-key' }; const result = generateDockerCompose(configWithProxy, mockNetworkConfigWithProxy); diff --git a/src/docker-manager.ts b/src/docker-manager.ts index 837e71909..f4c3f7050 100644 --- a/src/docker-manager.ts +++ b/src/docker-manager.ts @@ -372,6 +372,29 @@ export interface SslConfig { sslDbPath: string; } +/** + * Normalizes an API target value to a bare hostname. + * API target values should be bare hostnames (e.g., "api.openai.com"), but + * may arrive with a scheme or path when set via GitHub Actions expressions + * that are resolved at runtime (see github/gh-aw#25137). + * Discards any scheme, path, query, fragment, credentials, or port — + * path prefixes must use the separate *_API_BASE_PATH settings. + */ +export function stripScheme(value: string): string { + const trimmed = value.trim(); + if (!trimmed) return trimmed; + + const candidate = /^[a-zA-Z][a-zA-Z\d+\-.]*:\/\//.test(trimmed) + ? trimmed + : `https://${trimmed}`; + + try { + return new URL(candidate).hostname || trimmed; + } catch { + return trimmed; + } +} + /** * Generates Docker Compose configuration * Note: Uses external network 'awf-net' created by host-iptables setup @@ -1456,12 +1479,15 @@ export function generateDockerCompose( ...(config.copilotGithubToken && { COPILOT_GITHUB_TOKEN: config.copilotGithubToken }), ...(config.geminiApiKey && { GEMINI_API_KEY: config.geminiApiKey }), // Configurable API targets (for GHES/GHEC / custom endpoints) - ...(config.copilotApiTarget && { COPILOT_API_TARGET: config.copilotApiTarget }), - ...(config.openaiApiTarget && { OPENAI_API_TARGET: config.openaiApiTarget }), + // Strip any scheme prefix — server.js also normalizes defensively, but + // stripping here prevents a scheme-prefixed hostname from reaching the + // container at all (belt-and-suspenders for gh-aw#25137). + ...(config.copilotApiTarget && { COPILOT_API_TARGET: stripScheme(config.copilotApiTarget) }), + ...(config.openaiApiTarget && { OPENAI_API_TARGET: stripScheme(config.openaiApiTarget) }), ...(config.openaiApiBasePath && { OPENAI_API_BASE_PATH: config.openaiApiBasePath }), - ...(config.anthropicApiTarget && { ANTHROPIC_API_TARGET: config.anthropicApiTarget }), + ...(config.anthropicApiTarget && { ANTHROPIC_API_TARGET: stripScheme(config.anthropicApiTarget) }), ...(config.anthropicApiBasePath && { ANTHROPIC_API_BASE_PATH: config.anthropicApiBasePath }), - ...(config.geminiApiTarget && { GEMINI_API_TARGET: config.geminiApiTarget }), + ...(config.geminiApiTarget && { GEMINI_API_TARGET: stripScheme(config.geminiApiTarget) }), ...(config.geminiApiBasePath && { GEMINI_API_BASE_PATH: config.geminiApiBasePath }), // Forward GITHUB_SERVER_URL so api-proxy can auto-derive enterprise endpoints ...(process.env.GITHUB_SERVER_URL && { GITHUB_SERVER_URL: process.env.GITHUB_SERVER_URL }),