Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion src/cli.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -349,10 +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',
})).toBe('fallback-key');

expect(resolveCopilotApiKey({
COPILOT_API_KEY: 'primary-key',
})).toBe('primary-key');

expect(resolveCopilotApiKey({
Expand Down
26 changes: 24 additions & 2 deletions src/copilot-api-resolver.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,32 @@ describe('resolveCopilotApiKey', () => {
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', () => {
// COPILOT_PROVIDER_API_KEY is an explicit BYOK opt-in signal, so it
// takes precedence over an inherited COPILOT_API_KEY. This also covers
// the common case where gh-aw injects the BYOK dummy sentinel into
// COPILOT_API_KEY alongside a real COPILOT_PROVIDER_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 treat the BYOK dummy sentinel in COPILOT_API_KEY as unset', () => {
// gh-aw's compiled lock injects this sentinel into COPILOT_API_KEY to
// satisfy the Copilot CLI's "must be set" check, but it must not be
// forwarded upstream. See github/gh-aw#33116 / github/gh-aw#35575.
const env = {
COPILOT_API_KEY: 'dummy-byok-key-for-offline-mode',
COPILOT_PROVIDER_API_KEY: 'real_provider_key',
};
expect(resolveCopilotApiKey(env)).toBe('real_provider_key');
});

it('should return undefined when only the BYOK dummy sentinel is set', () => {
const env = { COPILOT_API_KEY: 'dummy-byok-key-for-offline-mode' };
expect(resolveCopilotApiKey(env)).toBeUndefined();
});

it('should return undefined when neither key is set', () => {
Expand All @@ -39,6 +59,8 @@ describe('resolveCopilotApiKey', () => {
});

it('should return empty string when keys are empty strings', () => {
// Empty string is preserved (distinct from "not set") so callers can
// distinguish an explicit clear from an absent variable.
const env = {
COPILOT_API_KEY: '',
COPILOT_PROVIDER_API_KEY: '',
Expand Down
43 changes: 41 additions & 2 deletions src/copilot-api-resolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,53 @@ import {
deriveCopilotApiTargetFromProviderBaseUrl,
} from './copilot-api-resolver.internal';

/**
* Sentinel value injected by gh-aw's compiled lock-file `pathSetup` step as
* `COPILOT_API_KEY` whenever the user is operating in BYOK mode. The Copilot
* CLI requires *some* value in `COPILOT_API_KEY` to start up, but in BYOK
* mode the real upstream credential is `COPILOT_PROVIDER_API_KEY`, which is
* what the api-proxy sidecar must actually forward upstream.
*
* Treating this sentinel as "unset" lets the resolver fall through to
* `COPILOT_PROVIDER_API_KEY` instead of forwarding the placeholder, which
* would otherwise produce upstream 401/503 responses.
*
* See: github/gh-aw#33116 (introduces the sentinel) and
* github/gh-aw#35575 / github/gh-aw-firewall#4040 (the resulting BYOK
* regression this constant guards against).
*/
const COPILOT_BYOK_DUMMY_API_KEY = 'dummy-byok-key-for-offline-mode';

function cleanApiKey(value: string | undefined): string | undefined {
if (value === undefined) return undefined;
if (value === COPILOT_BYOK_DUMMY_API_KEY) return undefined;
return value;
}

/**
* Resolve the Copilot BYOK key from supported environment variables.
* COPILOT_API_KEY takes precedence over COPILOT_PROVIDER_API_KEY.
*
* Precedence:
* 1. `COPILOT_PROVIDER_API_KEY` (explicit BYOK signal; user opted in to a
* non-Copilot upstream provider)
* 2. `COPILOT_API_KEY` (real Copilot token in non-BYOK mode)
*
* The BYOK dummy sentinel injected into `COPILOT_API_KEY` by gh-aw is treated
* as unset, so it never shadows a real `COPILOT_PROVIDER_API_KEY`.
*
* Empty string is preserved (not coerced to undefined) for callers that
* distinguish "explicitly set to empty" from "not set".
*/
export function resolveCopilotApiKey(
env: Record<string, string | undefined> = process.env
): string | undefined {
return env.COPILOT_API_KEY || env.COPILOT_PROVIDER_API_KEY;
const providerKey = cleanApiKey(env.COPILOT_PROVIDER_API_KEY);
if (providerKey !== undefined) return providerKey;

const apiKey = cleanApiKey(env.COPILOT_API_KEY);
if (apiKey !== undefined) return apiKey;

return undefined;
}

/**
Expand Down
Loading