refactor: extract shared credential-isolation scaffold for API proxy providers#5870
Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors the API-proxy “credential isolation” environment construction by extracting a shared helper (buildProviderCredentialIsolationEnv) and updating each provider-specific env builder to use the common scaffold. This reduces duplication in a security-sensitive area where drift could otherwise cause real credentials to leak into the agent container.
Changes:
- Added
buildProviderCredentialIsolationEnv(opts)to centralize: enabled guard, sidecar URL construction, base-url env injection, debug logging, and placeholder/extra-env merges. - Refactored OpenAI, Anthropic, and Gemini credential env builders into declarative calls to the shared helper.
- Refactored Copilot to use the helper for the common scaffold while preserving BYOK-specific conditional masking and wire-API selection logic.
Show a summary per file
| File | Description |
|---|---|
| src/services/credentials/provider-credential-isolation.ts | New shared helper that builds sidecar-routing env + credential placeholders. |
| src/services/credentials/openai-credential-env.ts | Switched to helper-based scaffold for OpenAI proxy routing + placeholders. |
| src/services/credentials/anthropic-credential-env.ts | Switched to helper-based scaffold for Anthropic proxy routing + helper-script env. |
| src/services/credentials/gemini-credential-env.ts | Switched to helper-based scaffold for Gemini dual base-url vars + placeholder key. |
| src/services/credentials/copilot-credential-env.ts | Uses helper for common proxy routing; keeps BYOK masking + wire-API env logic as post-processing. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 5/5 changed files
- Comments generated: 1
- Review effort level: Low
| const hasCopilotProviderApiKey = !!config.copilotProviderApiKey; | ||
| const hasCopilotProviderBaseUrl = !!config.copilotProviderBaseUrl || !!getConfigEnvValue(config, 'COPILOT_PROVIDER_BASE_URL'); | ||
| if (!config.copilotGithubToken && !hasCopilotProviderApiKey && !hasCopilotProviderBaseUrl) { | ||
| return {}; | ||
| } | ||
| const enabled = !!(config.copilotGithubToken || hasCopilotProviderApiKey || hasCopilotProviderBaseUrl); |
|
@copilot address review feedback |
Fixed in 9a41b25. |
|
✅ Copilot review passed with no inline comments. @copilot Add the |
|
✅ Smoke Copilot BYOK AOAI (Entra) completed. Copilot AOAI BYOK (Entra) mode operational. 🔓 |
|
🔑 Smoke Copilot PAT PAT auth validated. All systems operational. ✅ |
|
Chroot tests failed Smoke Chroot failed - See logs for details. |
|
🌑 The shadows whisper... Smoke Codex failed. The oracle requires further meditation... |
|
🔌 Smoke Services — All services reachable! ✅ |
|
✅ Smoke Gemini completed. All facets verified. 💎 |
|
✅ Smoke Copilot BYOK completed. Copilot BYOK mode operational. 🔓 |
|
✅ Smoke Copilot BYOK AOAI (api-key) completed. Copilot AOAI BYOK (api-key) mode operational. 🔓 |
|
✅ Smoke Claude passed |
|
📡 Smoke OTel Tracing completed. All tracing scenarios validated. ✅ |
|
✅ Contribution Check completed successfully! PR #5870 follows the applicable CONTRIBUTING.md guidelines; no contribution-guidelines comment needed. |
|
🚀 Security Guard has started processing this pull request |
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
|
✅ Build Test Suite completed successfully! |
|
| Metric | Base | PR | Delta |
|---|---|---|---|
| Lines | 98.59% | 98.62% | 📈 +0.03% |
| Statements | 98.52% | 98.55% | 📈 +0.03% |
| Functions | 99.43% | 99.43% | ➡️ +0.00% |
| Branches | 94.33% | 94.31% | 📉 -0.02% |
📁 Per-file Coverage Changes (1 files)
| File | Lines (Before → After) | Statements (Before → After) |
|---|---|---|
src/workdir-setup.ts |
93.0% → 94.8% (+1.74%) | 93.0% → 94.8% (+1.74%) |
✨ New Files (1 files)
src/services/credentials/provider-credential-isolation.ts: 100.0% lines
Coverage comparison generated by scripts/ci/compare-coverage.ts
🧪 Smoke Test: PAT Auth — PASS
Overall: PASS — Auth mode: PAT (COPILOT_GITHUB_TOKEN) cc Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "awmgmcpg"See Network Configuration for more information.
|
Smoke Test: Claude Engine Validation
Overall Result: PASS Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "awmgmcpg"See Network Configuration for more information.
|
|
Smoke Test: Copilot BYOK (Direct) — Azure OpenAI (Foundry)
Running in direct BYOK mode via API proxy → Azure OpenAI (Foundry, o4-mini-aw) authenticated via Microsoft Entra Overall: PASS cc Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "awmgmcpg"See Network Configuration for more information.
|
Smoke Test: Copilot BYOK (Direct) Mode✅ GitHub MCP testing Status: PASS Running in direct BYOK mode (COPILOT_PROVIDER_API_KEY) via api-proxy → api.githubcopilot.com Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "awmgmcpg"See Network Configuration for more information.
|
🔥 Smoke Test Results
Overall: PASS (core connectivity verified) Author: Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "awmgmcpg"See Network Configuration for more information.
|
Smoke Test Results
Overall: FAIL — Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "awmgmcpg"See Network Configuration for more information.
|
Smoke Test Results
Overall status: FAIL Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "localhost"See Network Configuration for more information.
|
📡 Smoke Test: API Proxy OpenTelemetry Tracing
All scenarios pass or are expected-pending during development. ✅ Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "awmgmcpg"See Network Configuration for more information.
|
🏗️ Build Test Suite Results
Overall: 8/8 ecosystems passed — ✅ PASS Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "awmgmcpg"See Network Configuration for more information.
|
|
${{ steps.smoke-data.outputs.SMOKE_PR_DATA }}
Running in direct BYOK mode (COPILOT_PROVIDER_API_KEY + COPILOT_PROVIDER_BASE_URL) via api-proxy → Azure OpenAI (Foundry, o4-mini-aw) Overall status: PASS Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "awmgmcpg"See Network Configuration for more information.
|
The four per-provider credential-env builders (
openai,anthropic,copilot,gemini) each repeated the same security-critical flow: enabled guard → proxy URL construction → base-URL env injection → debug logging → placeholder credential merge. Drift here risks real API keys leaking into the agent environment or bypassing the sidecar.Changes
New
provider-credential-isolation.ts— exportsbuildProviderCredentialIsolationEnv(opts), a typed helper that owns the shared scaffold:{}when provider should not be proxied)http://<proxyIp>:<port>)GOOGLE_GEMINI_BASE_URL+GEMINI_API_BASE_URL)openai-credential-env.ts/anthropic-credential-env.ts/gemini-credential-env.ts— reduced to a single declarativebuildProviderCredentialIsolationEnvcall each; provider-specific comments preserved in placecopilot-credential-env.ts— calls the helper for the common scaffold; conditional BYOK placeholders (COPILOT_GITHUB_TOKEN,COPILOT_PROVIDER_API_KEY) andCOPILOT_PROVIDER_WIRE_APIremain as explicit post-processing on the returned objectExample
Before (repeated in each provider):
After: