Skip to content

Commit 770009c

Browse files
lpcoxCopilot
andcommitted
fix: address review feedback on BYOK support
- Reword docker-manager comment to clarify placeholder behavior when api-proxy is enabled - Update github-copilot example to accept either COPILOT_GITHUB_TOKEN or COPILOT_API_KEY instead of hard-requiring COPILOT_API_KEY - Extract resolveCopilotAuthToken() as a testable function with JSDoc and export it from server.js - Add 8 unit tests covering precedence, fallback, empty/whitespace handling, and trimming behavior Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 9691e45 commit 770009c

4 files changed

Lines changed: 67 additions & 12 deletions

File tree

containers/api-proxy/server.js

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -63,8 +63,20 @@ const OPENAI_API_KEY = (process.env.OPENAI_API_KEY || '').trim() || undefined;
6363
const ANTHROPIC_API_KEY = (process.env.ANTHROPIC_API_KEY || '').trim() || undefined;
6464
const COPILOT_GITHUB_TOKEN = (process.env.COPILOT_GITHUB_TOKEN || '').trim() || undefined;
6565
const COPILOT_API_KEY = (process.env.COPILOT_API_KEY || '').trim() || undefined;
66-
// BYOK: use COPILOT_GITHUB_TOKEN (GitHub OAuth) or COPILOT_API_KEY (direct key), GitHub token takes precedence
67-
const COPILOT_AUTH_TOKEN = COPILOT_GITHUB_TOKEN || COPILOT_API_KEY;
66+
67+
/**
68+
* Resolves the Copilot auth token from environment variables.
69+
* COPILOT_GITHUB_TOKEN (GitHub OAuth) takes precedence over COPILOT_API_KEY (direct key).
70+
* @param {Record<string, string|undefined>} env - Environment variables to inspect
71+
* @returns {string|undefined} The resolved auth token, or undefined if neither is set
72+
*/
73+
function resolveCopilotAuthToken(env = process.env) {
74+
const githubToken = (env.COPILOT_GITHUB_TOKEN || '').trim() || undefined;
75+
const apiKey = (env.COPILOT_API_KEY || '').trim() || undefined;
76+
return githubToken || apiKey;
77+
}
78+
79+
const COPILOT_AUTH_TOKEN = resolveCopilotAuthToken(process.env);
6880
const GEMINI_API_KEY = (process.env.GEMINI_API_KEY || '').trim() || undefined;
6981

7082
/**
@@ -999,4 +1011,4 @@ if (require.main === module) {
9991011
}
10001012

10011013
// Export for testing
1002-
module.exports = { normalizeApiTarget, deriveCopilotApiTarget, normalizeBasePath, buildUpstreamPath, proxyWebSocket };
1014+
module.exports = { normalizeApiTarget, deriveCopilotApiTarget, normalizeBasePath, buildUpstreamPath, proxyWebSocket, resolveCopilotAuthToken };

containers/api-proxy/server.test.js

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
const http = require('http');
66
const tls = require('tls');
77
const { EventEmitter } = require('events');
8-
const { normalizeApiTarget, deriveCopilotApiTarget, normalizeBasePath, buildUpstreamPath, proxyWebSocket } = require('./server');
8+
const { normalizeApiTarget, deriveCopilotApiTarget, normalizeBasePath, buildUpstreamPath, proxyWebSocket, resolveCopilotAuthToken } = require('./server');
99

1010
describe('normalizeApiTarget', () => {
1111
it('should strip https:// prefix', () => {
@@ -620,3 +620,43 @@ describe('proxyWebSocket', () => {
620620
});
621621
});
622622

623+
describe('resolveCopilotAuthToken', () => {
624+
it('should return COPILOT_GITHUB_TOKEN when only it is set', () => {
625+
expect(resolveCopilotAuthToken({ COPILOT_GITHUB_TOKEN: 'gho_abc123' })).toBe('gho_abc123');
626+
});
627+
628+
it('should return COPILOT_API_KEY when only it is set', () => {
629+
expect(resolveCopilotAuthToken({ COPILOT_API_KEY: 'sk-byok-key' })).toBe('sk-byok-key');
630+
});
631+
632+
it('should prefer COPILOT_GITHUB_TOKEN over COPILOT_API_KEY when both are set', () => {
633+
expect(resolveCopilotAuthToken({
634+
COPILOT_GITHUB_TOKEN: 'gho_abc123',
635+
COPILOT_API_KEY: 'sk-byok-key',
636+
})).toBe('gho_abc123');
637+
});
638+
639+
it('should return undefined when neither is set', () => {
640+
expect(resolveCopilotAuthToken({})).toBeUndefined();
641+
});
642+
643+
it('should return undefined for empty strings', () => {
644+
expect(resolveCopilotAuthToken({ COPILOT_GITHUB_TOKEN: '', COPILOT_API_KEY: '' })).toBeUndefined();
645+
});
646+
647+
it('should return undefined for whitespace-only values', () => {
648+
expect(resolveCopilotAuthToken({ COPILOT_GITHUB_TOKEN: ' ', COPILOT_API_KEY: ' \n' })).toBeUndefined();
649+
});
650+
651+
it('should trim whitespace from token values', () => {
652+
expect(resolveCopilotAuthToken({ COPILOT_API_KEY: ' sk-byok-key ' })).toBe('sk-byok-key');
653+
});
654+
655+
it('should fall back to COPILOT_API_KEY when COPILOT_GITHUB_TOKEN is whitespace-only', () => {
656+
expect(resolveCopilotAuthToken({
657+
COPILOT_GITHUB_TOKEN: ' ',
658+
COPILOT_API_KEY: 'sk-byok-key',
659+
})).toBe('sk-byok-key');
660+
});
661+
});
662+

examples/github-copilot.sh

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
#
77
# Prerequisites:
88
# - GitHub Copilot CLI installed: npm install -g @github/copilot
9-
# - COPILOT_API_KEY environment variable set (for API proxy)
9+
# - COPILOT_GITHUB_TOKEN or COPILOT_API_KEY environment variable set (for API proxy)
1010
# - GITHUB_TOKEN environment variable set (for GitHub API access)
1111
#
1212
# Usage: sudo -E ./examples/github-copilot.sh
@@ -16,10 +16,12 @@ set -e
1616
echo "=== AWF GitHub Copilot CLI Example (with API Proxy) ==="
1717
echo ""
1818

19-
# Check for COPILOT_API_KEY
20-
if [ -z "$COPILOT_API_KEY" ]; then
21-
echo "Error: COPILOT_API_KEY environment variable is not set"
22-
echo "Set it with: export COPILOT_API_KEY='your_copilot_api_key'"
19+
# Check for Copilot credential (COPILOT_GITHUB_TOKEN or COPILOT_API_KEY)
20+
if [ -z "$COPILOT_GITHUB_TOKEN" ] && [ -z "$COPILOT_API_KEY" ]; then
21+
echo "Error: No Copilot credential set"
22+
echo "Set one of:"
23+
echo " export COPILOT_GITHUB_TOKEN='your_github_token'"
24+
echo " export COPILOT_API_KEY='your_copilot_api_key'"
2325
exit 1
2426
fi
2527

@@ -37,8 +39,8 @@ echo "Running GitHub Copilot CLI with API proxy and debug logging enabled..."
3739
echo ""
3840

3941
# Run Copilot CLI with API proxy enabled
40-
# Use sudo -E to preserve environment variables (COPILOT_API_KEY, GITHUB_TOKEN, AWF_ONE_SHOT_TOKEN_DEBUG)
41-
# The api-proxy sidecar holds the real COPILOT_API_KEY and injects it into requests.
42+
# Use sudo -E to preserve environment variables (COPILOT_GITHUB_TOKEN, COPILOT_API_KEY, GITHUB_TOKEN, AWF_ONE_SHOT_TOKEN_DEBUG)
43+
# The api-proxy sidecar holds the real Copilot credential and injects it into requests.
4244
# Required domains:
4345
# - api.githubcopilot.com: Copilot API endpoint (proxied via api-proxy)
4446
# - github.com: GitHub API access

src/docker-manager.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -747,7 +747,8 @@ export function generateDockerCompose(
747747
// it gets a placeholder value set earlier (line ~362) for credential isolation
748748
if (process.env.COPILOT_GITHUB_TOKEN && !config.enableApiProxy) environment.COPILOT_GITHUB_TOKEN = process.env.COPILOT_GITHUB_TOKEN;
749749
// COPILOT_API_KEY (BYOK) — forward when api-proxy is NOT enabled; when api-proxy IS enabled,
750-
// it is excluded from agent env (held securely in api-proxy sidecar)
750+
// the real key is not forwarded to the agent env and a placeholder may still be present
751+
// for credential isolation while the real key is held securely in the api-proxy sidecar
751752
if (process.env.COPILOT_API_KEY && !config.enableApiProxy) environment.COPILOT_API_KEY = process.env.COPILOT_API_KEY;
752753
if (process.env.USER) environment.USER = process.env.USER;
753754
// When --tty is set, we use TERM=xterm-256color (set above); otherwise inherit host TERM

0 commit comments

Comments
 (0)