From ac4fb9e844b286f83cf2512fd2c26d240a6d6035 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 4 May 2026 22:59:55 +0000 Subject: [PATCH 1/3] Initial plan From ddda0e28dac5d05405c3b5658416771c4aac1754 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 4 May 2026 23:14:56 +0000 Subject: [PATCH 2/3] fix: prevent double Bearer prefix in BYOK copilot auth header - Strip accidental "Bearer " prefix from COPILOT_API_KEY/COPILOT_GITHUB_TOKEN in resolveCopilotAuthToken() and createCopilotAdapter() to prevent double-prefixed "Authorization: Bearer Bearer " rejected by external providers (e.g. OpenRouter) with 400 "badly formatted" error - Extend upstream_auth_error log to also fire on HTTP 400 with BYOK hint - Add unit tests: Bearer-prefix stripping, BYOK getAuthHeaders format, 400/401 upstream_auth_error log events Agent-Logs-Url: https://github.com/github/gh-aw-firewall/sessions/f4fac661-5248-46ac-919a-4f77d41f89a0 --- containers/api-proxy/providers/copilot.js | 18 +- containers/api-proxy/server.js | 7 +- containers/api-proxy/server.test.js | 217 +++++++++++++++++++++- 3 files changed, 234 insertions(+), 8 deletions(-) diff --git a/containers/api-proxy/providers/copilot.js b/containers/api-proxy/providers/copilot.js index e6590285..110193a8 100644 --- a/containers/api-proxy/providers/copilot.js +++ b/containers/api-proxy/providers/copilot.js @@ -21,13 +21,20 @@ const { URL } = require('url'); * Resolves the Copilot auth token from environment variables. * COPILOT_GITHUB_TOKEN (GitHub OAuth) takes precedence over COPILOT_API_KEY (direct key). * + * Any accidental "Bearer " prefix is stripped so that the injected + * Authorization header is exactly "Bearer " rather than the + * double-prefixed "Bearer Bearer " that would be rejected by + * external providers in BYOK mode. + * * @param {Record} env - Environment variables to inspect * @returns {string|undefined} The resolved auth token, or undefined if neither is set */ function resolveCopilotAuthToken(env = process.env) { - const githubToken = (env.COPILOT_GITHUB_TOKEN || '').trim() || undefined; - const apiKey = (env.COPILOT_API_KEY || '').trim() || undefined; - return githubToken || apiKey; + // Strip any accidental "Bearer " prefix (with optional leading whitespace) before + // trimming, so that a value like "Bearer " (prefix only, no actual token) + // correctly reduces to undefined rather than "Bearer". + const strip = (v) => ((v || '').replace(/^\s*Bearer\s+/i, '').trim()) || undefined; + return strip(env.COPILOT_GITHUB_TOKEN) || strip(env.COPILOT_API_KEY); } /** @@ -127,8 +134,9 @@ function deriveGitHubApiBasePath(env = process.env) { * @returns {import('./index').ProviderAdapter} */ function createCopilotAdapter(env, deps = {}) { - const githubToken = (env.COPILOT_GITHUB_TOKEN || '').trim() || undefined; - const apiKey = (env.COPILOT_API_KEY || '').trim() || undefined; + const strip = (v) => ((v || '').replace(/^\s*Bearer\s+/i, '').trim()) || undefined; + const githubToken = strip(env.COPILOT_GITHUB_TOKEN); + const apiKey = strip(env.COPILOT_API_KEY); const authToken = resolveCopilotAuthToken(env); const integrationId = env.COPILOT_INTEGRATION_ID || 'copilot-developer-cli'; const rawTarget = deriveCopilotApiTarget(env); diff --git a/containers/api-proxy/server.js b/containers/api-proxy/server.js index 1f8b6fce..12fddf3f 100644 --- a/containers/api-proxy/server.js +++ b/containers/api-proxy/server.js @@ -379,11 +379,14 @@ function proxyRequest(req, res, targetHost, injectHeaders, provider, basePath = const resHeaders = { ...proxyRes.headers, 'x-request-id': requestId }; - if (proxyRes.statusCode === 401 || proxyRes.statusCode === 403) { + if (proxyRes.statusCode === 400 || proxyRes.statusCode === 401 || proxyRes.statusCode === 403) { + const message = proxyRes.statusCode === 400 + ? `Upstream returned 400 — possible malformed Authorization header; check that the API key does not include a "Bearer " prefix (BYOK mode)` + : `Upstream returned ${proxyRes.statusCode} — check that the API key is valid and has not expired`; logRequest('warn', 'upstream_auth_error', { request_id: requestId, provider, status: proxyRes.statusCode, upstream_host: targetHost, path: sanitizeForLog(req.url), - message: `Upstream returned ${proxyRes.statusCode} — check that the API key is valid and has not expired`, + message, }); } diff --git a/containers/api-proxy/server.test.js b/containers/api-proxy/server.test.js index 605e9738..450bc78b 100644 --- a/containers/api-proxy/server.test.js +++ b/containers/api-proxy/server.test.js @@ -11,7 +11,7 @@ const { EventEmitter } = require('events'); const { normalizeApiTarget, normalizeBasePath, buildUpstreamPath, shouldStripHeader, stripGeminiKeyParam, composeBodyTransforms } = require('./proxy-utils'); // Provider-specific functions that live in their respective adapter modules -const { deriveCopilotApiTarget, deriveGitHubApiTarget, deriveGitHubApiBasePath, resolveCopilotAuthToken } = require('./providers/copilot'); +const { deriveCopilotApiTarget, deriveGitHubApiTarget, deriveGitHubApiBasePath, resolveCopilotAuthToken, createCopilotAdapter } = require('./providers/copilot'); const { resolveOpenCodeRoute } = require('./providers/opencode'); // Core proxy functions that remain in server.js @@ -921,6 +921,113 @@ describe('resolveCopilotAuthToken', () => { COPILOT_API_KEY: 'sk-byok-key', })).toBe('sk-byok-key'); }); + + // ── BYOK: Bearer-prefix stripping ──────────────────────────────────────── + // If an API key is accidentally provided with a "Bearer " prefix + // (e.g. COPILOT_API_KEY="Bearer sk-or-v1-xxx"), the sidecar must strip it + // to prevent the injected Authorization header from being double-prefixed + // ("Authorization: Bearer Bearer sk-or-v1-xxx"), which external providers + // like OpenRouter reject with 400 "Authorization header is badly formatted". + + it('strips "Bearer " prefix from COPILOT_API_KEY (BYOK mode)', () => { + expect(resolveCopilotAuthToken({ COPILOT_API_KEY: 'Bearer sk-or-v1-abc' })).toBe('sk-or-v1-abc'); + }); + + it('strips "Bearer " prefix (case-insensitive) from COPILOT_API_KEY', () => { + expect(resolveCopilotAuthToken({ COPILOT_API_KEY: 'bearer sk-or-v1-abc' })).toBe('sk-or-v1-abc'); + expect(resolveCopilotAuthToken({ COPILOT_API_KEY: 'BEARER sk-or-v1-abc' })).toBe('sk-or-v1-abc'); + }); + + it('strips "Bearer " prefix from COPILOT_GITHUB_TOKEN', () => { + expect(resolveCopilotAuthToken({ COPILOT_GITHUB_TOKEN: 'Bearer gho_abc123' })).toBe('gho_abc123'); + }); + + it('prefers stripped COPILOT_GITHUB_TOKEN over stripped COPILOT_API_KEY', () => { + expect(resolveCopilotAuthToken({ + COPILOT_GITHUB_TOKEN: 'Bearer gho_abc123', + COPILOT_API_KEY: 'Bearer sk-byok-key', + })).toBe('gho_abc123'); + }); + + it('does not strip a key that starts with "Bearer" but lacks a space after it', () => { + // "BearerToken" has no space — should not be stripped + expect(resolveCopilotAuthToken({ COPILOT_API_KEY: 'BearerToken123' })).toBe('BearerToken123'); + }); + + it('returns undefined when token is only "Bearer " with nothing after the prefix', () => { + expect(resolveCopilotAuthToken({ COPILOT_API_KEY: 'Bearer ' })).toBeUndefined(); + expect(resolveCopilotAuthToken({ COPILOT_API_KEY: 'Bearer ' })).toBeUndefined(); + }); +}); + +// ── createCopilotAdapter — BYOK auth header format ─────────────────────────── +// +// These tests guard against the "badly formatted Authorization header" bug in +// BYOK mode (COPILOT_PROVIDER_API_KEY + COPILOT_PROVIDER_BASE_URL) where the +// sidecar could produce "Authorization: Bearer Bearer " if the key value +// already contained the "Bearer " prefix. They also verify that the header +// injected for inference requests is exactly "Bearer " and that the +// Copilot-Integration-Id header is present. + +describe('createCopilotAdapter — BYOK getAuthHeaders', () => { + const fakeReq = { url: '/v1/chat/completions', method: 'POST', headers: {} }; + const fakeModelsReq = { url: '/models', method: 'GET', headers: {} }; + + it('injects Authorization: Bearer for BYOK inference request', () => { + const adapter = createCopilotAdapter({ COPILOT_API_KEY: 'sk-or-v1-abc123' }); + const headers = adapter.getAuthHeaders(fakeReq); + expect(headers['Authorization']).toBe('Bearer sk-or-v1-abc123'); + }); + + it('injects Copilot-Integration-Id header for BYOK inference request', () => { + const adapter = createCopilotAdapter({ COPILOT_API_KEY: 'sk-or-v1-abc123' }); + const headers = adapter.getAuthHeaders(fakeReq); + expect(headers['Copilot-Integration-Id']).toBe('copilot-developer-cli'); + }); + + it('prevents double "Bearer " prefix when API key already contains "Bearer " prefix (BYOK bug fix)', () => { + const adapter = createCopilotAdapter({ COPILOT_API_KEY: 'Bearer sk-or-v1-abc123' }); + const headers = adapter.getAuthHeaders(fakeReq); + // Must NOT be "Bearer Bearer sk-or-v1-abc123" + expect(headers['Authorization']).toBe('Bearer sk-or-v1-abc123'); + expect(headers['Authorization']).not.toContain('Bearer Bearer'); + }); + + it('strips "Bearer " prefix case-insensitively from API key', () => { + const adapter = createCopilotAdapter({ COPILOT_API_KEY: 'BEARER sk-or-v1-abc123' }); + const headers = adapter.getAuthHeaders(fakeReq); + expect(headers['Authorization']).toBe('Bearer sk-or-v1-abc123'); + }); + + it('uses COPILOT_GITHUB_TOKEN (not COPILOT_API_KEY) for /models GET in BYOK+token mode', () => { + const adapter = createCopilotAdapter({ + COPILOT_GITHUB_TOKEN: 'gho_oauth_token', + COPILOT_API_KEY: 'sk-or-v1-abc123', + }); + const headers = adapter.getAuthHeaders(fakeModelsReq); + expect(headers['Authorization']).toBe('Bearer gho_oauth_token'); + }); + + it('uses API key for /models GET when no GITHUB_TOKEN is set (BYOK-only mode)', () => { + const adapter = createCopilotAdapter({ COPILOT_API_KEY: 'sk-or-v1-abc123' }); + const headers = adapter.getAuthHeaders(fakeModelsReq); + // In BYOK-only mode, githubToken is undefined so falls through to authToken (apiKey) + expect(headers['Authorization']).toBe('Bearer sk-or-v1-abc123'); + }); + + it('is enabled when only COPILOT_API_KEY is set', () => { + const adapter = createCopilotAdapter({ COPILOT_API_KEY: 'sk-or-v1-abc123' }); + expect(adapter.isEnabled()).toBe(true); + }); + + it('uses custom COPILOT_INTEGRATION_ID when set', () => { + const adapter = createCopilotAdapter({ + COPILOT_API_KEY: 'sk-or-v1-abc123', + COPILOT_INTEGRATION_ID: 'my-custom-integration', + }); + const headers = adapter.getAuthHeaders(fakeReq); + expect(headers['Copilot-Integration-Id']).toBe('my-custom-integration'); + }); }); describe('resolveOpenCodeRoute', () => { @@ -2362,4 +2469,112 @@ describe('createProviderServer', () => { await new Promise((r) => setTimeout(r, 100)); expect(callCount).toBe(1); }); + + // ── 400/401/403 upstream response → upstream_auth_error log ────────────── + // + // When the upstream provider returns an auth-related error status, the proxy + // must emit an 'upstream_auth_error' log event so operators can diagnose + // credential problems quickly. A 400 specifically indicates a possible + // malformed Authorization header (e.g. double "Bearer " prefix in BYOK mode). + + /** + * Build a minimal mock for https.request that immediately calls back with a + * response of the given status code. The mock proxyRes emits 'end' after + * the callback so request_complete is also logged. + */ + function mockHttpsWithStatus(statusCode) { + return jest.spyOn(https, 'request').mockImplementation((options, callback) => { + const proxyReq = new EventEmitter(); + proxyReq.write = jest.fn(); + proxyReq.end = jest.fn(() => { + setImmediate(() => { + const proxyRes = new EventEmitter(); + proxyRes.statusCode = statusCode; + proxyRes.headers = { 'content-type': 'application/json' }; + proxyRes.pipe = jest.fn((destRes) => { destRes.end('{}'); }); + callback(proxyRes); + setImmediate(() => proxyRes.emit('end')); + }); + }); + proxyReq.destroy = jest.fn(); + return proxyReq; + }); + } + + it('emits upstream_auth_error with 400-specific message when upstream returns 400 (BYOK malformed header)', async () => { + const { lines, spy } = collectLogOutput(); + mockHttpsWithStatus(400); + + const adapter = { + name: 'copilot', port: 0, isManagementPort: false, alwaysBind: false, + participatesInValidation: false, + isEnabled: () => true, + getTargetHost: () => 'openrouter.ai', + getBasePath: () => '', + getAuthHeaders: () => ({ 'Authorization': 'Bearer sk-or-key' }), + getBodyTransform: () => null, + }; + const port = await startAdapter(adapter); + + await fetch(port, '/v1/chat/completions', { method: 'POST', body: '{}' }); + + jest.restoreAllMocks(); + spy.mockRestore(); + + const authErrLog = lines.find(l => l.event === 'upstream_auth_error' && l.status === 400); + expect(authErrLog).toBeDefined(); + expect(authErrLog.provider).toBe('copilot'); + expect(authErrLog.message).toContain('Bearer'); + expect(authErrLog.message).toContain('400'); + }); + + it('emits upstream_auth_error when upstream returns 401', async () => { + const { lines, spy } = collectLogOutput(); + mockHttpsWithStatus(401); + + const adapter = { + name: 'copilot', port: 0, isManagementPort: false, alwaysBind: false, + participatesInValidation: false, + isEnabled: () => true, + getTargetHost: () => 'api.githubcopilot.com', + getBasePath: () => '', + getAuthHeaders: () => ({ 'Authorization': 'Bearer gho_token' }), + getBodyTransform: () => null, + }; + const port = await startAdapter(adapter); + + await fetch(port, '/v1/chat/completions', { method: 'POST', body: '{}' }); + + jest.restoreAllMocks(); + spy.mockRestore(); + + const authErrLog = lines.find(l => l.event === 'upstream_auth_error' && l.status === 401); + expect(authErrLog).toBeDefined(); + expect(authErrLog.provider).toBe('copilot'); + expect(authErrLog.message).toContain('401'); + }); + + it('does NOT emit upstream_auth_error for a successful 200 response', async () => { + const { lines, spy } = collectLogOutput(); + mockHttpsWithStatus(200); + + const adapter = { + name: 'copilot', port: 0, isManagementPort: false, alwaysBind: false, + participatesInValidation: false, + isEnabled: () => true, + getTargetHost: () => 'api.githubcopilot.com', + getBasePath: () => '', + getAuthHeaders: () => ({ 'Authorization': 'Bearer gho_token' }), + getBodyTransform: () => null, + }; + const port = await startAdapter(adapter); + + await fetch(port, '/v1/chat/completions', { method: 'POST', body: '{}' }); + + jest.restoreAllMocks(); + spy.mockRestore(); + + const authErrLog = lines.find(l => l.event === 'upstream_auth_error'); + expect(authErrLog).toBeUndefined(); + }); }); From 8a5568540c5ccbce4901d627f6289bdf4f0bd9ec Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 4 May 2026 23:37:07 +0000 Subject: [PATCH 3/3] refactor: address review feedback on BYOK auth header fix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Extract stripBearerPrefix() as a named module-level helper in copilot.js, reused by both resolveCopilotAuthToken() and createCopilotAdapter() — eliminates the duplicated inline `strip` function; export for direct unit testing - Replace Copilot/BYOK-specific HTTP 400 message in proxyRequest with a generic "check that the API key is valid and correctly formatted" message that is accurate for all providers - Fix server.test.js BYOK mode comment to reference COPILOT_API_KEY (the sidecar's actual input) rather than COPILOT_PROVIDER_API_KEY (the agent-side placeholder) - Add dedicated describe('stripBearerPrefix') unit tests for the new helper; simplify resolveCopilotAuthToken integration tests Agent-Logs-Url: https://github.com/github/gh-aw-firewall/sessions/fcc4060e-94ff-474f-8254-d8b81acd6204 Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> --- containers/api-proxy/providers/copilot.js | 33 ++++++--- containers/api-proxy/server.js | 5 +- containers/api-proxy/server.test.js | 81 ++++++++++++++--------- 3 files changed, 73 insertions(+), 46 deletions(-) diff --git a/containers/api-proxy/providers/copilot.js b/containers/api-proxy/providers/copilot.js index 110193a8..a25ed1ad 100644 --- a/containers/api-proxy/providers/copilot.js +++ b/containers/api-proxy/providers/copilot.js @@ -17,24 +17,35 @@ const { normalizeApiTarget } = require('../proxy-utils'); const { URL } = require('url'); +/** + * Strip any accidental "Bearer " prefix from a raw credential value and trim + * surrounding whitespace. Returns undefined when the result is empty so that + * callers can use `|| undefined` fall-through cleanly. + * + * A value like "Bearer " (prefix with nothing after it) reduces to undefined + * rather than "Bearer", which is why the prefix is removed before trimming. + * + * @param {string|undefined} value - Raw credential string + * @returns {string|undefined} + */ +function stripBearerPrefix(value) { + return ((value || '').replace(/^\s*Bearer\s+/i, '').trim()) || undefined; +} + /** * Resolves the Copilot auth token from environment variables. * COPILOT_GITHUB_TOKEN (GitHub OAuth) takes precedence over COPILOT_API_KEY (direct key). * - * Any accidental "Bearer " prefix is stripped so that the injected - * Authorization header is exactly "Bearer " rather than the - * double-prefixed "Bearer Bearer " that would be rejected by + * Any accidental "Bearer " prefix is stripped via stripBearerPrefix so that + * the injected Authorization header is exactly "Bearer " rather than + * the double-prefixed "Bearer Bearer " that would be rejected by * external providers in BYOK mode. * * @param {Record} env - Environment variables to inspect * @returns {string|undefined} The resolved auth token, or undefined if neither is set */ function resolveCopilotAuthToken(env = process.env) { - // Strip any accidental "Bearer " prefix (with optional leading whitespace) before - // trimming, so that a value like "Bearer " (prefix only, no actual token) - // correctly reduces to undefined rather than "Bearer". - const strip = (v) => ((v || '').replace(/^\s*Bearer\s+/i, '').trim()) || undefined; - return strip(env.COPILOT_GITHUB_TOKEN) || strip(env.COPILOT_API_KEY); + return stripBearerPrefix(env.COPILOT_GITHUB_TOKEN) || stripBearerPrefix(env.COPILOT_API_KEY); } /** @@ -134,9 +145,8 @@ function deriveGitHubApiBasePath(env = process.env) { * @returns {import('./index').ProviderAdapter} */ function createCopilotAdapter(env, deps = {}) { - const strip = (v) => ((v || '').replace(/^\s*Bearer\s+/i, '').trim()) || undefined; - const githubToken = strip(env.COPILOT_GITHUB_TOKEN); - const apiKey = strip(env.COPILOT_API_KEY); + const githubToken = stripBearerPrefix(env.COPILOT_GITHUB_TOKEN); + const apiKey = stripBearerPrefix(env.COPILOT_API_KEY); const authToken = resolveCopilotAuthToken(env); const integrationId = env.COPILOT_INTEGRATION_ID || 'copilot-developer-cli'; const rawTarget = deriveCopilotApiTarget(env); @@ -253,6 +263,7 @@ function createCopilotAdapter(env, deps = {}) { module.exports = { createCopilotAdapter, resolveCopilotAuthToken, + stripBearerPrefix, deriveCopilotApiTarget, deriveGitHubApiTarget, deriveGitHubApiBasePath, diff --git a/containers/api-proxy/server.js b/containers/api-proxy/server.js index 12fddf3f..d7a3e169 100644 --- a/containers/api-proxy/server.js +++ b/containers/api-proxy/server.js @@ -380,13 +380,10 @@ function proxyRequest(req, res, targetHost, injectHeaders, provider, basePath = const resHeaders = { ...proxyRes.headers, 'x-request-id': requestId }; if (proxyRes.statusCode === 400 || proxyRes.statusCode === 401 || proxyRes.statusCode === 403) { - const message = proxyRes.statusCode === 400 - ? `Upstream returned 400 — possible malformed Authorization header; check that the API key does not include a "Bearer " prefix (BYOK mode)` - : `Upstream returned ${proxyRes.statusCode} — check that the API key is valid and has not expired`; logRequest('warn', 'upstream_auth_error', { request_id: requestId, provider, status: proxyRes.statusCode, upstream_host: targetHost, path: sanitizeForLog(req.url), - message, + message: `Upstream returned ${proxyRes.statusCode} — check that the API key is valid and correctly formatted`, }); } diff --git a/containers/api-proxy/server.test.js b/containers/api-proxy/server.test.js index 450bc78b..69eb1f5e 100644 --- a/containers/api-proxy/server.test.js +++ b/containers/api-proxy/server.test.js @@ -11,7 +11,7 @@ const { EventEmitter } = require('events'); const { normalizeApiTarget, normalizeBasePath, buildUpstreamPath, shouldStripHeader, stripGeminiKeyParam, composeBodyTransforms } = require('./proxy-utils'); // Provider-specific functions that live in their respective adapter modules -const { deriveCopilotApiTarget, deriveGitHubApiTarget, deriveGitHubApiBasePath, resolveCopilotAuthToken, createCopilotAdapter } = require('./providers/copilot'); +const { deriveCopilotApiTarget, deriveGitHubApiTarget, deriveGitHubApiBasePath, resolveCopilotAuthToken, stripBearerPrefix, createCopilotAdapter } = require('./providers/copilot'); const { resolveOpenCodeRoute } = require('./providers/opencode'); // Core proxy functions that remain in server.js @@ -883,6 +883,45 @@ describe('proxyWebSocket', () => { }); }); +describe('stripBearerPrefix', () => { + it('strips "Bearer " prefix from a token value', () => { + expect(stripBearerPrefix('Bearer sk-or-v1-abc')).toBe('sk-or-v1-abc'); + }); + + it('strips "Bearer " prefix case-insensitively', () => { + expect(stripBearerPrefix('bearer sk-or-v1-abc')).toBe('sk-or-v1-abc'); + expect(stripBearerPrefix('BEARER sk-or-v1-abc')).toBe('sk-or-v1-abc'); + }); + + it('strips leading whitespace before "Bearer "', () => { + expect(stripBearerPrefix(' Bearer sk-or-v1-abc')).toBe('sk-or-v1-abc'); + }); + + it('returns value unchanged when no "Bearer " prefix is present', () => { + expect(stripBearerPrefix('sk-or-v1-abc')).toBe('sk-or-v1-abc'); + expect(stripBearerPrefix('gho_abc123')).toBe('gho_abc123'); + }); + + it('does not strip "Bearer" without a following space', () => { + expect(stripBearerPrefix('BearerToken123')).toBe('BearerToken123'); + }); + + it('returns undefined when value is only "Bearer " (nothing after prefix)', () => { + expect(stripBearerPrefix('Bearer ')).toBeUndefined(); + expect(stripBearerPrefix('Bearer ')).toBeUndefined(); + }); + + it('returns undefined for empty or whitespace-only input', () => { + expect(stripBearerPrefix('')).toBeUndefined(); + expect(stripBearerPrefix(' ')).toBeUndefined(); + expect(stripBearerPrefix(undefined)).toBeUndefined(); + }); + + it('trims surrounding whitespace from the token', () => { + expect(stripBearerPrefix(' sk-or-v1-abc ')).toBe('sk-or-v1-abc'); + }); +}); + describe('resolveCopilotAuthToken', () => { it('should return COPILOT_GITHUB_TOKEN when only it is set', () => { expect(resolveCopilotAuthToken({ COPILOT_GITHUB_TOKEN: 'gho_abc123' })).toBe('gho_abc123'); @@ -922,23 +961,14 @@ describe('resolveCopilotAuthToken', () => { })).toBe('sk-byok-key'); }); - // ── BYOK: Bearer-prefix stripping ──────────────────────────────────────── - // If an API key is accidentally provided with a "Bearer " prefix - // (e.g. COPILOT_API_KEY="Bearer sk-or-v1-xxx"), the sidecar must strip it - // to prevent the injected Authorization header from being double-prefixed - // ("Authorization: Bearer Bearer sk-or-v1-xxx"), which external providers - // like OpenRouter reject with 400 "Authorization header is badly formatted". + // Integration: verify that Bearer-prefix stripping (via stripBearerPrefix) is + // applied to both token sources when resolving. - it('strips "Bearer " prefix from COPILOT_API_KEY (BYOK mode)', () => { + it('strips "Bearer " prefix from COPILOT_API_KEY when resolving', () => { expect(resolveCopilotAuthToken({ COPILOT_API_KEY: 'Bearer sk-or-v1-abc' })).toBe('sk-or-v1-abc'); }); - it('strips "Bearer " prefix (case-insensitive) from COPILOT_API_KEY', () => { - expect(resolveCopilotAuthToken({ COPILOT_API_KEY: 'bearer sk-or-v1-abc' })).toBe('sk-or-v1-abc'); - expect(resolveCopilotAuthToken({ COPILOT_API_KEY: 'BEARER sk-or-v1-abc' })).toBe('sk-or-v1-abc'); - }); - - it('strips "Bearer " prefix from COPILOT_GITHUB_TOKEN', () => { + it('strips "Bearer " prefix from COPILOT_GITHUB_TOKEN when resolving', () => { expect(resolveCopilotAuthToken({ COPILOT_GITHUB_TOKEN: 'Bearer gho_abc123' })).toBe('gho_abc123'); }); @@ -948,26 +978,16 @@ describe('resolveCopilotAuthToken', () => { COPILOT_API_KEY: 'Bearer sk-byok-key', })).toBe('gho_abc123'); }); - - it('does not strip a key that starts with "Bearer" but lacks a space after it', () => { - // "BearerToken" has no space — should not be stripped - expect(resolveCopilotAuthToken({ COPILOT_API_KEY: 'BearerToken123' })).toBe('BearerToken123'); - }); - - it('returns undefined when token is only "Bearer " with nothing after the prefix', () => { - expect(resolveCopilotAuthToken({ COPILOT_API_KEY: 'Bearer ' })).toBeUndefined(); - expect(resolveCopilotAuthToken({ COPILOT_API_KEY: 'Bearer ' })).toBeUndefined(); - }); }); // ── createCopilotAdapter — BYOK auth header format ─────────────────────────── // // These tests guard against the "badly formatted Authorization header" bug in -// BYOK mode (COPILOT_PROVIDER_API_KEY + COPILOT_PROVIDER_BASE_URL) where the -// sidecar could produce "Authorization: Bearer Bearer " if the key value -// already contained the "Bearer " prefix. They also verify that the header -// injected for inference requests is exactly "Bearer " and that the -// Copilot-Integration-Id header is present. +// BYOK mode where the sidecar is configured with COPILOT_API_KEY (the real key +// held by the sidecar) and could produce "Authorization: Bearer Bearer " +// if the COPILOT_API_KEY value already contained the "Bearer " prefix. +// They also verify that the header injected for inference requests is exactly +// "Bearer " and that the Copilot-Integration-Id header is present. describe('createCopilotAdapter — BYOK getAuthHeaders', () => { const fakeReq = { url: '/v1/chat/completions', method: 'POST', headers: {} }; @@ -2501,7 +2521,7 @@ describe('createProviderServer', () => { }); } - it('emits upstream_auth_error with 400-specific message when upstream returns 400 (BYOK malformed header)', async () => { + it('emits upstream_auth_error when upstream returns 400', async () => { const { lines, spy } = collectLogOutput(); mockHttpsWithStatus(400); @@ -2524,7 +2544,6 @@ describe('createProviderServer', () => { const authErrLog = lines.find(l => l.event === 'upstream_auth_error' && l.status === 400); expect(authErrLog).toBeDefined(); expect(authErrLog.provider).toBe('copilot'); - expect(authErrLog.message).toContain('Bearer'); expect(authErrLog.message).toContain('400'); });