refactor: extract shared buildNoProxyEnv helper to eliminate duplicate NO_PROXY assembly#6371
Conversation
There was a problem hiding this comment.
Pull request overview
Centralizes proxy-bypass environment construction to prevent drift across container services.
Changes:
- Adds shared
NO_PROXY/no_proxyhelpers. - Refactors agent, API proxy, and CLI proxy environments without changing values.
Show a summary per file
| File | Description |
|---|---|
src/services/no-proxy-utils.ts |
Adds shared bypass helpers. |
src/services/api-proxy-env-config.ts |
Uses shared baseline environment. |
src/services/agent-environment/proxy-environment.ts |
Collects bypass hosts before assembly. |
src/services/cli-proxy-service.ts |
Uses shared environment with host access. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 4/4 changed files
- Comments generated: 0
- Review effort level: Medium
|
@copilot merge main |
…-proxy-bypass-code
Done — merged |
|
✅ Smoke Copilot BYOK completed. Copilot BYOK mode operational. 🔓 |
|
✅ Smoke Copilot BYOK AOAI (api-key) completed. Copilot AOAI BYOK (api-key) mode operational. 🔓 |
|
✨ The prophecy is fulfilled... Smoke Codex has completed its mystical journey. The stars align. 🌟 |
|
Chroot tests passed! Smoke Chroot - All security and functionality tests succeeded. |
|
✅ Build Test Suite completed successfully! |
|
✅ Smoke Claude passed |
|
✅ Contribution Check completed successfully! |
|
📰 VERDICT: Smoke Docker Sbx has concluded. All systems operational. This is a developing story. 🎤 |
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
|
✅ Smoke Copilot BYOK AOAI (Entra) completed. Copilot AOAI BYOK (Entra) mode operational. 🔓 |
|
🔌 Smoke Services — All services reachable! ✅ |
|
🚀 Security Guard has started processing this pull request |
|
| Metric | Base | PR | Delta |
|---|---|---|---|
| Lines | 98.64% | 98.68% | 📈 +0.04% |
| Statements | 98.46% | 98.50% | 📈 +0.04% |
| Functions | 99.24% | 99.24% | ➡️ +0.00% |
| Branches | 94.55% | 94.53% | 📉 -0.02% |
📁 Per-file Coverage Changes (2 files)
| File | Lines (Before → After) | Statements (Before → After) |
|---|---|---|
src/services/cli-proxy-service.ts |
94.7% → 95.0% (+0.27%) | 94.7% → 95.0% (+0.27%) |
src/log-directory-setup.ts |
96.2% → 100.0% (+3.78%) | 96.3% → 100.0% (+3.71%) |
✨ New Files (1 files)
src/services/no-proxy-utils.ts: 100.0% lines
Coverage comparison generated by scripts/ci/compare-coverage.ts
|
Please add tests for the new proxy-bypass helper and the updated environment builders. CONTRIBUTING.md asks contributors to "write tests for new functionality" and the PR process section says to include tests for new functionality. The PR description is clear and references #6357, so this looks good otherwise. 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 ✅
Status: PASS — 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: Copilot network isolation egress enforcement EGRESS_RESULT allow=pass deny=pass
Overall: PASS — Warning Firewall blocked 2 domainsThe following domains were blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "awmgmcpg"
- "example.com"See Network Configuration for more information.
|
Smoke Test: Copilot PAT Auth
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 ValidationOverall 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 Results
Overall: 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
Warning Firewall blocked 2 domainsThe following domains were blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "awmgmcpg"
- "registry.npmjs.org"See Network Configuration for more information.
|
|
Smoke Test: Copilot BYOK (Direct) Mode (Azure OpenAI via Entra)
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: Gemini Engine
Overall Status: PASS 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: GitHub Actions Services Connectivity
Overall: FAIL The AWF agent container runs in an isolated network ( 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: API Proxy OpenTelemetry Tracing
All scenarios pass. OTEL tracing integration is functioning correctly. 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.
|
Chroot Version Comparison
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.
|
|
$(sed s///g /tmp/gh-aw/agent/comment_body.txt) 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.
|
🔍 Smoke Test: Docker Sbx Validation
cc
|
NO_PROXY/no_proxywas assembled independently in three security-critical paths, each hardcodinglocalhost,127.0.0.1,::1before appending service-specific bypasses — making it easy for the three to silently drift apart.Changes
src/services/no-proxy-utils.ts(new) — shared helpers:buildNoProxyValue(additionalHosts?)→ comma-joined bypass stringbuildNoProxyEnv(additionalHosts?)→{ NO_PROXY, no_proxy }object (both identical)api-proxy-env-config.ts— replace inline literals with...buildNoProxyEnv()agent-environment/proxy-environment.ts— replace incremental string mutations with a collectednoProxyHosts[]array passed once tobuildNoProxyValue()cli-proxy-service.ts— replace inline literals with...buildNoProxyEnv(['host.docker.internal'])Example
Before (three divergent sites):
After (single source of truth):