From 7748f664f18a3218339526e38ba842b6d67dede7 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 15 Apr 2026 18:49:52 +0000 Subject: [PATCH 1/4] Initial plan From 4ad506e4f7d3c02ff0a5621c20d10b893e5ed9a3 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 15 Apr 2026 18:55:45 +0000 Subject: [PATCH 2/4] fix(api-proxy): fix Gemini API_KEY_INVALID errors - Add x-goog-api-key to STRIPPED_HEADERS to ensure placeholder is always stripped before the real key is injected - Add stripGeminiKeyParam() to remove ?key= query params from URLs (the @google/genai SDK may append key= in addition to the header) - Apply stripGeminiKeyParam() in both HTTP and WebSocket Gemini handlers - Extend auth_inject debug logging to cover x-goog-api-key - Export shouldStripHeader and stripGeminiKeyParam for unit testing - Add tests for shouldStripHeader and stripGeminiKeyParam Agent-Logs-Url: https://github.com/github/gh-aw-firewall/sessions/e48bf273-4302-49fe-acde-42cbd46c679c --- containers/api-proxy/server.js | 29 +++++++++++- containers/api-proxy/server.test.js | 68 ++++++++++++++++++++++++++++- 2 files changed, 94 insertions(+), 3 deletions(-) diff --git a/containers/api-proxy/server.js b/containers/api-proxy/server.js index e4a2edc0..bb9c6661 100644 --- a/containers/api-proxy/server.js +++ b/containers/api-proxy/server.js @@ -46,6 +46,7 @@ const STRIPPED_HEADERS = new Set([ 'authorization', 'proxy-authorization', 'x-api-key', + 'x-goog-api-key', 'forwarded', 'via', ]); @@ -167,6 +168,24 @@ function buildUpstreamPath(reqUrl, targetHost, basePath) { return prefix + targetUrl.pathname + targetUrl.search; } +/** + * Strip the `key` query parameter from a Gemini request URL. + * + * The @google/genai SDK (and older Gemini SDK versions) may append `?key=` + * to every request URL in addition to setting the `x-goog-api-key` header. + * The proxy injects the real key via the header, so the placeholder `key=` + * value must be removed before forwarding to Google to prevent + * API_KEY_INVALID errors. + * + * @param {string} reqUrl - The incoming request URL (must start with '/') + * @returns {string} URL with the `key` query parameter removed + */ +function stripGeminiKeyParam(reqUrl) { + const parsed = new URL(reqUrl, 'http://localhost'); + parsed.searchParams.delete('key'); + return parsed.pathname + parsed.search; +} + // Optional base path prefixes for API targets (e.g. /serving-endpoints for Databricks) const OPENAI_API_BASE_PATH = normalizeBasePath(process.env.OPENAI_API_BASE_PATH); const ANTHROPIC_API_BASE_PATH = normalizeBasePath(process.env.ANTHROPIC_API_BASE_PATH); @@ -485,7 +504,7 @@ function proxyRequest(req, res, targetHost, injectHeaders, provider, basePath = Object.assign(headers, injectHeaders); // Log auth header injection for debugging credential-isolation issues - const injectedKey = injectHeaders['x-api-key'] || injectHeaders['authorization']; + const injectedKey = injectHeaders['x-api-key'] || injectHeaders['authorization'] || injectHeaders['x-goog-api-key']; if (injectedKey) { const keyPreview = injectedKey.length > 8 ? `${injectedKey.substring(0, 8)}...${injectedKey.substring(injectedKey.length - 4)}` @@ -1016,12 +1035,18 @@ if (require.main === module) { const contentLength = parseInt(req.headers['content-length'], 10) || 0; if (checkRateLimit(req, res, 'gemini', contentLength)) return; + // Strip any ?key= query parameter — the @google/genai SDK may append it to the URL. + // The proxy injects the real key via x-goog-api-key header instead. + req.url = stripGeminiKeyParam(req.url); + proxyRequest(req, res, GEMINI_API_TARGET, { 'x-goog-api-key': GEMINI_API_KEY, }, 'gemini', GEMINI_API_BASE_PATH); }); geminiServer.on('upgrade', (req, socket, head) => { + // Strip any ?key= query parameter — the @google/genai SDK may append it to the URL. + req.url = stripGeminiKeyParam(req.url); proxyWebSocket(req, socket, head, GEMINI_API_TARGET, { 'x-goog-api-key': GEMINI_API_KEY, }, 'gemini', GEMINI_API_BASE_PATH); @@ -1155,4 +1180,4 @@ if (require.main === module) { } // Export for testing -module.exports = { normalizeApiTarget, deriveCopilotApiTarget, deriveGitHubApiTarget, deriveGitHubApiBasePath, normalizeBasePath, buildUpstreamPath, proxyWebSocket, resolveCopilotAuthToken, resolveOpenCodeRoute }; +module.exports = { normalizeApiTarget, deriveCopilotApiTarget, deriveGitHubApiTarget, deriveGitHubApiBasePath, normalizeBasePath, buildUpstreamPath, proxyWebSocket, resolveCopilotAuthToken, resolveOpenCodeRoute, shouldStripHeader, stripGeminiKeyParam }; diff --git a/containers/api-proxy/server.test.js b/containers/api-proxy/server.test.js index c1ba1a46..a83617a4 100644 --- a/containers/api-proxy/server.test.js +++ b/containers/api-proxy/server.test.js @@ -5,7 +5,7 @@ const http = require('http'); const tls = require('tls'); const { EventEmitter } = require('events'); -const { normalizeApiTarget, deriveCopilotApiTarget, deriveGitHubApiTarget, deriveGitHubApiBasePath, normalizeBasePath, buildUpstreamPath, proxyWebSocket, resolveCopilotAuthToken, resolveOpenCodeRoute } = require('./server'); +const { normalizeApiTarget, deriveCopilotApiTarget, deriveGitHubApiTarget, deriveGitHubApiBasePath, normalizeBasePath, buildUpstreamPath, proxyWebSocket, resolveCopilotAuthToken, resolveOpenCodeRoute, shouldStripHeader, stripGeminiKeyParam } = require('./server'); describe('normalizeApiTarget', () => { it('should strip https:// prefix', () => { @@ -449,6 +449,72 @@ describe('buildUpstreamPath', () => { }); }); +describe('shouldStripHeader', () => { + it('should strip authorization header', () => { + expect(shouldStripHeader('authorization')).toBe(true); + expect(shouldStripHeader('Authorization')).toBe(true); + }); + + it('should strip x-api-key header', () => { + expect(shouldStripHeader('x-api-key')).toBe(true); + expect(shouldStripHeader('X-Api-Key')).toBe(true); + }); + + it('should strip x-goog-api-key header (Gemini placeholder must be stripped)', () => { + expect(shouldStripHeader('x-goog-api-key')).toBe(true); + expect(shouldStripHeader('X-Goog-Api-Key')).toBe(true); + }); + + it('should strip proxy-authorization header', () => { + expect(shouldStripHeader('proxy-authorization')).toBe(true); + }); + + it('should strip x-forwarded-* headers', () => { + expect(shouldStripHeader('x-forwarded-for')).toBe(true); + expect(shouldStripHeader('x-forwarded-host')).toBe(true); + }); + + it('should not strip content-type header', () => { + expect(shouldStripHeader('content-type')).toBe(false); + }); + + it('should not strip anthropic-version header', () => { + expect(shouldStripHeader('anthropic-version')).toBe(false); + }); +}); + +describe('stripGeminiKeyParam', () => { + it('should remove the key= query parameter', () => { + expect(stripGeminiKeyParam('/v1/models/gemini-pro:generateContent?key=placeholder')) + .toBe('/v1/models/gemini-pro:generateContent'); + }); + + it('should remove key= while preserving other query parameters', () => { + expect(stripGeminiKeyParam('/v1/models/gemini-pro:generateContent?key=placeholder&alt=json')) + .toBe('/v1/models/gemini-pro:generateContent?alt=json'); + }); + + it('should return path unchanged when no key= parameter is present', () => { + expect(stripGeminiKeyParam('/v1/models/gemini-pro:generateContent')) + .toBe('/v1/models/gemini-pro:generateContent'); + }); + + it('should return path unchanged when only unrelated query parameters exist', () => { + expect(stripGeminiKeyParam('/v1/models/gemini-pro:generateContent?alt=json&stream=true')) + .toBe('/v1/models/gemini-pro:generateContent?alt=json&stream=true'); + }); + + it('should handle root path without key param', () => { + expect(stripGeminiKeyParam('/')).toBe('/'); + }); + + it('should handle path with only key= param, leaving no trailing ?', () => { + // URL.search returns '' when no params remain after deletion + const result = stripGeminiKeyParam('/v1/generateContent?key=abc'); + expect(result).toBe('/v1/generateContent'); + }); +}); + // ── Helpers for proxyWebSocket tests ────────────────────────────────────────── /** Create a minimal mock socket with write/destroy spies. */ From 542503205c51428d33194f9cdf8bd0465a46a097 Mon Sep 17 00:00:00 2001 From: Landon Cox Date: Wed, 15 Apr 2026 13:28:05 -0700 Subject: [PATCH 3/4] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- containers/api-proxy/server.js | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/containers/api-proxy/server.js b/containers/api-proxy/server.js index bb9c6661..bd33a817 100644 --- a/containers/api-proxy/server.js +++ b/containers/api-proxy/server.js @@ -177,10 +177,15 @@ function buildUpstreamPath(reqUrl, targetHost, basePath) { * value must be removed before forwarding to Google to prevent * API_KEY_INVALID errors. * - * @param {string} reqUrl - The incoming request URL (must start with '/') + * @param {string} reqUrl - The incoming request URL (must start with exactly one '/') * @returns {string} URL with the `key` query parameter removed */ function stripGeminiKeyParam(reqUrl) { + // Only operate on relative request paths that begin with exactly one slash. + // Returning other inputs unchanged preserves downstream validation behavior. + if (typeof reqUrl !== 'string' || !reqUrl.startsWith('/') || reqUrl.startsWith('//')) { + return reqUrl; + } const parsed = new URL(reqUrl, 'http://localhost'); parsed.searchParams.delete('key'); return parsed.pathname + parsed.search; From 0d79261dbc3f350e98a0f20b7f06568c05902c4f Mon Sep 17 00:00:00 2001 From: Landon Cox Date: Wed, 15 Apr 2026 13:36:21 -0700 Subject: [PATCH 4/4] fix: case-insensitive auth header lookup for debug logging Address PR review feedback: - Make injected-key detection case-insensitive so auth_inject debug logs fire for OpenAI/Copilot (which use capital-A 'Authorization') in addition to Anthropic and Gemini. - Clarify stripGeminiKeyParam guard comments explaining why absolute/protocol-relative URLs are rejected before parsing. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- containers/api-proxy/server.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/containers/api-proxy/server.js b/containers/api-proxy/server.js index bd33a817..84889129 100644 --- a/containers/api-proxy/server.js +++ b/containers/api-proxy/server.js @@ -182,12 +182,15 @@ function buildUpstreamPath(reqUrl, targetHost, basePath) { */ function stripGeminiKeyParam(reqUrl) { // Only operate on relative request paths that begin with exactly one slash. - // Returning other inputs unchanged preserves downstream validation behavior. + // Returning other inputs unchanged lets proxyRequest's relative-URL check reject them. + // The guard prevents absolute URLs (e.g. 'http://evil.com/path?key=…') and + // protocol-relative URLs ('//host/path') from being normalized into a relative path. if (typeof reqUrl !== 'string' || !reqUrl.startsWith('/') || reqUrl.startsWith('//')) { return reqUrl; } const parsed = new URL(reqUrl, 'http://localhost'); parsed.searchParams.delete('key'); + // Reconstruct relative path only — never emit the scheme/host from the dummy base. return parsed.pathname + parsed.search; } @@ -509,7 +512,8 @@ function proxyRequest(req, res, targetHost, injectHeaders, provider, basePath = Object.assign(headers, injectHeaders); // Log auth header injection for debugging credential-isolation issues - const injectedKey = injectHeaders['x-api-key'] || injectHeaders['authorization'] || injectHeaders['x-goog-api-key']; + // Use case-insensitive lookup since providers use mixed casing (e.g. 'Authorization' vs 'authorization') + const injectedKey = Object.entries(injectHeaders).find(([k]) => ['x-api-key', 'authorization', 'x-goog-api-key'].includes(k.toLowerCase()))?.[1]; if (injectedKey) { const keyPreview = injectedKey.length > 8 ? `${injectedKey.substring(0, 8)}...${injectedKey.substring(injectedKey.length - 4)}`