feat: disable codex credential sharing, enable proxied calls#854
Conversation
Co-authored-by: lpcox <[email protected]>
Claude
AI
changed the title
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
|
Chroot tests passed! Smoke Chroot - All security and functionality tests succeeded. |
|
✨ The prophecy is fulfilled... Smoke Codex has completed its mystical journey. The stars align. 🌟 |
|
🎬 THE END — Smoke Claude MISSION: ACCOMPLISHED! The hero saves the day! ✨ |
✅ Coverage Check PassedOverall Coverage
📁 Per-file Coverage Changes (1 files)
Coverage comparison generated by |
Bun Build Test Results ✅All Bun projects tested successfully!
Overall: PASS
|
Deno Build Test Results
Overall: ✅ PASS All Deno tests completed successfully.
|
.NET Build Test Results
Overall: PASS ✅ All .NET projects successfully restored, built, and ran.
|
Node.js Build Test Results
Overall: PASS ✅ All Node.js projects installed successfully and all tests passed.
|
|
Merged PRs: feat: proxy claude api calls to secure auth token; fix: use 0o666 mode for ~/.claude.json to fix permissions
|
Go Build Test Results
Overall: PASS ✅ All Go projects successfully downloaded dependencies and passed their tests.
|
Rust Build Test Results ✅
Overall: PASS All Rust projects built and tested successfully!
|
Smoke Test Results (Run #22024159945)Last 2 Merged PRs:
Test Results:
Status: PASS ✅ cc: @lpcox
|
Chroot Version Comparison Test Results
Overall Status: ❌ Tests did not pass The chroot environment successfully accesses host binaries, but version mismatches detected for Python and Node.js. Go versions match correctly.
|
Build Test: Java - Results
Overall: PASS All Java projects successfully compiled and tested through the AWF firewall with Maven proxy configuration.
|
Smoke Test ResultsLast 2 Merged PRs:
Test Results:
Overall Status: PASS
|
There was a problem hiding this comment.
Pull request overview
This PR tightens credential isolation for Codex by removing the CODEX_API_KEY pass-through to the agent container when --enable-api-proxy is used, and re-enables proxied OpenAI/Codex calls by setting OPENAI_BASE_URL to the api-proxy sidecar endpoint.
Changes:
- Exclude
CODEX_API_KEYfrom the agent environment when api-proxy is enabled. - Set
OPENAI_BASE_URLin the agent to route OpenAI/Codex traffic through the sidecar (http://{proxyIp}:10000/v1). - Update health-check enforcement and Jest expectations to reflect the new isolation/proxy behavior.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/docker-manager.ts |
Stops leaking CODEX_API_KEY to the agent under api-proxy and re-enables OPENAI_BASE_URL for proxied OpenAI/Codex calls. |
src/docker-manager.test.ts |
Updates tests to expect OPENAI_BASE_URL and to assert CODEX_API_KEY is not exposed to the agent when api-proxy is enabled. |
containers/agent/api-proxy-health-check.sh |
Extends credential isolation checks to fail if CODEX_API_KEY is present in the agent environment. |
Comments suppressed due to low confidence (1)
src/docker-manager.ts:336
- With
enableApiProxyon,CODEX_API_KEYis always added toEXCLUDED_ENV_VARS. If a user enables the proxy but only providesCODEX_API_KEY(noconfig.openaiApiKey/OPENAI_API_KEY), the agent will loseCODEX_API_KEYwhileOPENAI_BASE_URLis never set, which can break Codex runs without actually configuring the OpenAI proxy. Consider either (a) only excludingCODEX_API_KEYwhen the OpenAI proxy is configured, or (b) treatingCODEX_API_KEYas an OpenAI key for proxy configuration (pass it to the sidecar and setOPENAI_BASE_URL).
if (config.enableApiProxy) {
EXCLUDED_ENV_VARS.add('OPENAI_API_KEY');
EXCLUDED_ENV_VARS.add('OPENAI_KEY');
EXCLUDED_ENV_VARS.add('CODEX_API_KEY');
EXCLUDED_ENV_VARS.add('ANTHROPIC_API_KEY');
EXCLUDED_ENV_VARS.add('CLAUDE_API_KEY');
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (config.openaiApiKey) { | ||
| environment.OPENAI_BASE_URL = `http://${networkConfig.proxyIp}:10000/v1`; | ||
| logger.debug(`OpenAI API will be proxied through sidecar at http://${networkConfig.proxyIp}:10000/v1`); | ||
| } |
There was a problem hiding this comment.
OPENAI_BASE_URL is only set when config.openaiApiKey is present. In combination with excluding CODEX_API_KEY when enableApiProxy is true, this creates a failure mode where Codex credentials are removed but no proxy endpoint is configured (e.g., proxy enabled + CODEX_API_KEY present + openaiApiKey absent). Consider aligning the condition for setting OPENAI_BASE_URL with the condition for excluding Codex/OpenAI keys (or emit a hard error when enableApiProxy is enabled without an OpenAI key).
This issue also appears on line 331 of the same file.
| if (config.openaiApiKey) { | |
| environment.OPENAI_BASE_URL = `http://${networkConfig.proxyIp}:10000/v1`; | |
| logger.debug(`OpenAI API will be proxied through sidecar at http://${networkConfig.proxyIp}:10000/v1`); | |
| } | |
| environment.OPENAI_BASE_URL = `http://${networkConfig.proxyIp}:10000/v1`; | |
| logger.debug(`OpenAI API will be proxied through sidecar at http://${networkConfig.proxyIp}:10000/v1`); |
| it('should not leak CODEX_API_KEY to agent when api-proxy is enabled with envAll', () => { | ||
| // Simulate the key being in process.env AND envAll enabled | ||
| // CODEX_API_KEY is intentionally passed through (unlike other keys) for Codex agent compatibility | ||
| // CODEX_API_KEY is now excluded when api-proxy is enabled for credential isolation | ||
| const origKey = process.env.CODEX_API_KEY; | ||
| process.env.CODEX_API_KEY = 'sk-codex-secret'; | ||
| try { | ||
| const configWithProxy = { ...mockConfig, enableApiProxy: true, openaiApiKey: 'sk-test', envAll: true }; | ||
| const result = generateDockerCompose(configWithProxy, mockNetworkConfigWithProxy); | ||
| const agent = result.services.agent; | ||
| const env = agent.environment as Record<string, string>; | ||
| // CODEX_API_KEY is intentionally passed to agent for Codex compatibility | ||
| expect(env.CODEX_API_KEY).toBe('sk-codex-secret'); | ||
| // OPENAI_BASE_URL temporarily disabled for Codex - will be re-enabled in future | ||
| expect(env.OPENAI_BASE_URL).toBeUndefined(); | ||
| // CODEX_API_KEY should NOT be passed to agent when api-proxy is enabled | ||
| expect(env.CODEX_API_KEY).toBeUndefined(); | ||
| // OPENAI_BASE_URL should be set when api-proxy is enabled with openaiApiKey | ||
| expect(env.OPENAI_BASE_URL).toBe('http://172.30.0.30:10000/v1'); |
There was a problem hiding this comment.
Test coverage currently validates that CODEX_API_KEY is excluded when enableApiProxy is true and openaiApiKey is provided, but it doesn’t cover the important edge case where enableApiProxy is true and only process.env.CODEX_API_KEY is set (no openaiApiKey). Given the new exclusion logic, adding an explicit test for that scenario would prevent regressions and clarify intended behavior (pass-through vs proxy configuration vs error).
2 participants
Removes temporary exception that allowed CODEX_API_KEY to bypass credential isolation. Enables OPENAI_BASE_URL to route OpenAI/Codex API calls through the api-proxy sidecar.
Changes
Credential isolation (
src/docker-manager.ts)CODEX_API_KEYtoEXCLUDED_ENV_VARSwhen api-proxy enabled (line 334)OPENAI_BASE_URLconfiguration to proxy calls tohttp://{proxyIp}:10000/v1(lines 1011-1014)Health checks (
containers/agent/api-proxy-health-check.sh)CODEX_API_KEYnot present in agent environment (lines 67-75)Tests (
src/docker-manager.test.ts)CODEX_API_KEYexcluded,OPENAI_BASE_URLset when api-proxy enabledBehavior
Before:
After:
Codex agents now use the same credential isolation pattern as other LLM providers.