From db53135bb729cbedd3d61603b4375bf491245d5d Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Mon, 1 Jun 2026 13:23:54 -0700 Subject: [PATCH] fix(copilot-byok): prefer COPILOT_PROVIDER_API_KEY over COPILOT_API_KEY COPILOT_API_KEY is the Copilot CLI's session-token env var, not a BYOK upstream credential. In gh-aw's byok-copilot flow it is intentionally set to a placeholder sentinel (dummy-byok-key-for-offline-mode), while the real BYOK key is delivered via COPILOT_PROVIDER_API_KEY + COPILOT_PROVIDER_BASE_URL. Before this change, resolveCopilotApiKey preferred COPILOT_API_KEY, so AWF forwarded the sentinel into the api-proxy sidecar and the upstream BYOK endpoint rejected the resulting requests with 503/401. Flipping the precedence so COPILOT_PROVIDER_API_KEY wins fixes the symptom without adding any literal-string coupling to gh-aw's sentinel constant. COPILOT_API_KEY remains honored as a fallback for callers that only set it. Refs: github/gh-aw#35575, github/gh-aw-firewall#4040 --- src/cli.test.ts | 8 ++++---- src/copilot-api-resolver.test.ts | 8 ++++---- src/copilot-api-resolver.ts | 17 +++++++++++++++-- 3 files changed, 23 insertions(+), 10 deletions(-) diff --git a/src/cli.test.ts b/src/cli.test.ts index 99263afdf..69739f1a3 100644 --- a/src/cli.test.ts +++ b/src/cli.test.ts @@ -349,14 +349,14 @@ describe('cli', () => { }); describe('Copilot BYOK env resolution', () => { - it('prefers COPILOT_API_KEY and falls back to COPILOT_PROVIDER_API_KEY', () => { + it('prefers COPILOT_PROVIDER_API_KEY and falls back to COPILOT_API_KEY', () => { expect(resolveCopilotApiKey({ - COPILOT_API_KEY: 'primary-key', - COPILOT_PROVIDER_API_KEY: 'fallback-key', + COPILOT_API_KEY: 'fallback-key', + COPILOT_PROVIDER_API_KEY: 'primary-key', })).toBe('primary-key'); expect(resolveCopilotApiKey({ - COPILOT_PROVIDER_API_KEY: 'fallback-key', + COPILOT_API_KEY: 'fallback-key', })).toBe('fallback-key'); }); diff --git a/src/copilot-api-resolver.test.ts b/src/copilot-api-resolver.test.ts index 234b4f685..4d776d030 100644 --- a/src/copilot-api-resolver.test.ts +++ b/src/copilot-api-resolver.test.ts @@ -15,22 +15,22 @@ describe('resolveCopilotApiKey', () => { expect(copilotApiResolverModule).not.toHaveProperty('copilotApiResolverTestHelpers'); }); - it('should return COPILOT_API_KEY when set', () => { + it('should return COPILOT_API_KEY when only it is set', () => { const env = { COPILOT_API_KEY: 'key123' }; expect(resolveCopilotApiKey(env)).toBe('key123'); }); - it('should return COPILOT_PROVIDER_API_KEY when COPILOT_API_KEY is not set', () => { + it('should return COPILOT_PROVIDER_API_KEY when only it is set', () => { const env = { COPILOT_PROVIDER_API_KEY: 'provider_key456' }; expect(resolveCopilotApiKey(env)).toBe('provider_key456'); }); - it('should prefer COPILOT_API_KEY over COPILOT_PROVIDER_API_KEY', () => { + it('should prefer COPILOT_PROVIDER_API_KEY over COPILOT_API_KEY', () => { const env = { COPILOT_API_KEY: 'key123', COPILOT_PROVIDER_API_KEY: 'provider_key456', }; - expect(resolveCopilotApiKey(env)).toBe('key123'); + expect(resolveCopilotApiKey(env)).toBe('provider_key456'); }); it('should return undefined when neither key is set', () => { diff --git a/src/copilot-api-resolver.ts b/src/copilot-api-resolver.ts index 52ffba427..4cdcf3549 100644 --- a/src/copilot-api-resolver.ts +++ b/src/copilot-api-resolver.ts @@ -5,12 +5,25 @@ import { /** * Resolve the Copilot BYOK key from supported environment variables. - * COPILOT_API_KEY takes precedence over COPILOT_PROVIDER_API_KEY. + * + * `COPILOT_PROVIDER_API_KEY` takes precedence over `COPILOT_API_KEY`. + * + * Rationale: `COPILOT_API_KEY` is the Copilot CLI's *session token* env var, + * not a BYOK upstream credential. In integrations such as gh-aw's + * `byok-copilot: true` flow, `COPILOT_API_KEY` is intentionally set to a + * placeholder sentinel (e.g. `dummy-byok-key-for-offline-mode`) to satisfy + * the CLI's startup check, while the real upstream BYOK credential is passed + * via `COPILOT_PROVIDER_API_KEY` (paired with `COPILOT_PROVIDER_BASE_URL`). + * Preferring `COPILOT_PROVIDER_API_KEY` here prevents AWF from forwarding the + * sentinel to the sidecar (github/gh-aw#35575, github/gh-aw-firewall#4040). + * + * `COPILOT_API_KEY` is still honored as a fallback for callers that have + * only ever set that variable. */ export function resolveCopilotApiKey( env: Record = process.env ): string | undefined { - return env.COPILOT_API_KEY || env.COPILOT_PROVIDER_API_KEY; + return env.COPILOT_PROVIDER_API_KEY || env.COPILOT_API_KEY; } /**