From 587dee4e227e56df34b9469078d58b391bf5f86f Mon Sep 17 00:00:00 2001 From: Landon Cox Date: Wed, 8 Apr 2026 08:35:31 -0700 Subject: [PATCH 1/3] fix: normalize API target env vars to strip scheme prefix When --*-api-target values originate from GitHub Actions expressions (e.g., ${{ vars.ANTHROPIC_BASE_URL }}), the scheme is not stripped at compile time. At runtime the full URL (https://host) reaches the API proxy, which prepends https:// again, producing double-scheme URLs like https://https://host that Squid rejects. Fix at two layers: 1. containers/api-proxy/server.js: add normalizeApiTarget() that strips any http(s):// prefix. Applied to all four API targets (OpenAI, Anthropic, Gemini, Copilot) on startup. 2. src/docker-manager.ts: add stripScheme() and apply it when setting *_API_TARGET env vars in the api-proxy container. This prevents the scheme from ever reaching the container. Fixes #1795 Upstream: github/gh-aw#25137 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- containers/api-proxy/server.js | 26 ++++++++++++--- containers/api-proxy/server.test.js | 52 ++++++++++++++++++++++++++++- src/docker-manager.test.ts | 40 +++++++++++++++++++++- src/docker-manager.ts | 21 +++++++++--- 4 files changed, 128 insertions(+), 11 deletions(-) diff --git a/containers/api-proxy/server.js b/containers/api-proxy/server.js index 76a83b47d..764b14951 100644 --- a/containers/api-proxy/server.js +++ b/containers/api-proxy/server.js @@ -64,10 +64,26 @@ 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 (+ optional path). + * Strips any https?:// scheme prefix that may arrive when the value + * originates from a GitHub Actions expression resolved at runtime + * (e.g., ${{ vars.ANTHROPIC_BASE_URL }} → "https://my-gateway.example.com"). + * Also trims whitespace for safety. + * + * @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; + return value.trim().replace(/^https?:\/\//, ''); +} + // 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 strip any scheme prefix — 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 +139,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 +948,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..5e10a9b37 100644 --- a/containers/api-proxy/server.test.js +++ b/containers/api-proxy/server.test.js @@ -5,7 +5,38 @@ 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 preserve hostname with path', () => { + expect(normalizeApiTarget('https://my-gateway.example.com/some-path')).toBe('my-gateway.example.com/some-path'); + }); + + 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'); + }); +}); describe('deriveCopilotApiTarget', () => { let originalEnv; @@ -268,6 +299,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 scheme-stripped', () => { + // normalizeApiTarget('https://my-gateway.example.com/some-path') + // returns 'my-gateway.example.com/some-path' + 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..8e03d88f4 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 preserve hostname with path after scheme strip', () => { + expect(stripScheme('https://my-gateway.example.com/some-path')).toBe('my-gateway.example.com/some-path'); + }); + + 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..1d12ba48c 100644 --- a/src/docker-manager.ts +++ b/src/docker-manager.ts @@ -372,6 +372,16 @@ export interface SslConfig { sslDbPath: string; } +/** + * Strips any http:// or https:// scheme prefix from an API target hostname. + * API target values should be bare hostnames (e.g., "api.openai.com"), but + * may arrive with a scheme when set via GitHub Actions expressions that are + * resolved at runtime (see github/gh-aw#25137). + */ +export function stripScheme(value: string): string { + return value.replace(/^https?:\/\//, ''); +} + /** * Generates Docker Compose configuration * Note: Uses external network 'awf-net' created by host-iptables setup @@ -1456,12 +1466,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 }), From c514330ade8195df3319e501ad12cb3a023241db Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 8 Apr 2026 15:49:43 +0000 Subject: [PATCH 2/3] fix: normalize API targets to bare hostnames only Use URL parsing in both normalizeApiTarget() and stripScheme() to extract only the hostname, discarding path, query, fragment, port, and credentials. Paths must use the separate *_API_BASE_PATH settings. Addresses review feedback that target values are used as TLS SNI / hostname in https.request(), so including paths would break requests. Agent-Logs-Url: https://github.com/github/gh-aw-firewall/sessions/ab65b061-c924-41d8-a573-dd954497def2 --- containers/api-proxy/server.js | 36 +++++++++++++++++++++++------ containers/api-proxy/server.test.js | 16 +++++++++---- src/docker-manager.test.ts | 4 ++-- src/docker-manager.ts | 21 +++++++++++++---- 4 files changed, 60 insertions(+), 17 deletions(-) diff --git a/containers/api-proxy/server.js b/containers/api-proxy/server.js index 764b14951..a71b42979 100644 --- a/containers/api-proxy/server.js +++ b/containers/api-proxy/server.js @@ -65,22 +65,44 @@ const COPILOT_GITHUB_TOKEN = (process.env.COPILOT_GITHUB_TOKEN || '').trim() || const GEMINI_API_KEY = (process.env.GEMINI_API_KEY || '').trim() || undefined; /** - * Normalizes an API target value to a bare hostname (+ optional path). - * Strips any https?:// scheme prefix that may arrive when the value - * originates from a GitHub Actions expression resolved at runtime - * (e.g., ${{ vars.ANTHROPIC_BASE_URL }} → "https://my-gateway.example.com"). - * Also trims whitespace for safety. + * 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; - return value.trim().replace(/^https?:\/\//, ''); + + 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 or URL`); + return undefined; + } } // Configurable API target hosts (supports custom endpoints / internal LLM routers) -// Values are normalized to strip any scheme prefix — buildUpstreamPath() prepends https:// +// 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'; diff --git a/containers/api-proxy/server.test.js b/containers/api-proxy/server.test.js index 5e10a9b37..657d404c9 100644 --- a/containers/api-proxy/server.test.js +++ b/containers/api-proxy/server.test.js @@ -20,8 +20,8 @@ describe('normalizeApiTarget', () => { expect(normalizeApiTarget('api.openai.com')).toBe('api.openai.com'); }); - it('should preserve hostname with path', () => { - expect(normalizeApiTarget('https://my-gateway.example.com/some-path')).toBe('my-gateway.example.com/some-path'); + 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', () => { @@ -36,6 +36,14 @@ describe('normalizeApiTarget', () => { 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', () => { @@ -301,9 +309,9 @@ describe('buildUpstreamPath', () => { }); describe('with normalized API target (gh-aw#25137 regression)', () => { - it('should produce correct path when target was already scheme-stripped', () => { + it('should produce correct path when target was already normalized', () => { // normalizeApiTarget('https://my-gateway.example.com/some-path') - // returns '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'); diff --git a/src/docker-manager.test.ts b/src/docker-manager.test.ts index 8e03d88f4..08fdbdabd 100644 --- a/src/docker-manager.test.ts +++ b/src/docker-manager.test.ts @@ -308,8 +308,8 @@ describe('docker-manager', () => { expect(stripScheme('api.openai.com')).toBe('api.openai.com'); }); - it('should preserve hostname with path after scheme strip', () => { - expect(stripScheme('https://my-gateway.example.com/some-path')).toBe('my-gateway.example.com/some-path'); + 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', () => { diff --git a/src/docker-manager.ts b/src/docker-manager.ts index 1d12ba48c..f4c3f7050 100644 --- a/src/docker-manager.ts +++ b/src/docker-manager.ts @@ -373,13 +373,26 @@ export interface SslConfig { } /** - * Strips any http:// or https:// scheme prefix from an API target hostname. + * 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 when set via GitHub Actions expressions that are - * resolved at runtime (see github/gh-aw#25137). + * 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 { - return value.replace(/^https?:\/\//, ''); + 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; + } } /** From a22c06925ba68531b9dc0640a7c44e00e1324dab Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 8 Apr 2026 15:51:14 +0000 Subject: [PATCH 3/3] fix: improve error message in normalizeApiTarget Agent-Logs-Url: https://github.com/github/gh-aw-firewall/sessions/ab65b061-c924-41d8-a573-dd954497def2 --- containers/api-proxy/server.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/containers/api-proxy/server.js b/containers/api-proxy/server.js index a71b42979..e409bdebb 100644 --- a/containers/api-proxy/server.js +++ b/containers/api-proxy/server.js @@ -96,7 +96,7 @@ function normalizeApiTarget(value) { return parsed.hostname || undefined; } catch (err) { - console.warn(`Invalid API target ${sanitizeForLog(trimmed)}; expected a hostname or URL`); + console.warn(`Invalid API target ${sanitizeForLog(trimmed)}; expected a hostname (e.g. 'api.example.com') or URL`); return undefined; } }