fix: route CLI proxy through isolated egress relay - #7338
Conversation
There was a problem hiding this comment.
Pull request overview
Adds conditional awf-ext connectivity for cli-proxy when targeting external DIFC proxies in network-isolation mode.
Changes:
- Adds DIFC host classification and conditional dual-homing.
- Adds classifier and Compose-network tests.
- Documents the revised network topology.
Show a summary per file
| File | Description |
|---|---|
src/services/cli-proxy-service.ts |
Classifies DIFC hosts and conditionally attaches awf-ext. |
src/services/cli-proxy-service.test.ts |
Tests host classification. |
src/compose-generator.test.ts |
Tests conditional dual-homing. |
docs/network-isolation-design.md |
Documents cli-proxy network behavior. |
Review details
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
- Files reviewed: 4/4 changed files
- Comments generated: 3
- Review effort level: Balanced
| if (isValidIPv4(normalized) || isValidIPv6(normalized)) return true; | ||
| if (normalized === 'localhost') return true; |
| // Dotted names (host.docker.internal, difc.example.com) resolve outside the | ||
| // Compose project; bare labels are Docker service/container names. | ||
| return normalized.includes('.'); |
| - The **cli-proxy is dual-homed too, but only when it targets an *external* DIFC proxy** | ||
| (`host.docker.internal`, a bare IP, or a dotted DNS name). Without a route off the | ||
| internal network its tcp-tunnel fails with `ENETUNREACH` and the container never | ||
| becomes healthy. When the DIFC proxy is an attached sibling container (a bare Docker | ||
| service name), the cli-proxy stays on `awf-net` only. |
|
@copilot address review feedback |
Addressed in 28791eb:
|
|
✅ Copilot review passed with no inline comments. @copilot Add the |
|
| Metric | Base | PR | Delta |
|---|---|---|---|
| Lines | 94.11% | 93.79% | 📉 -0.32% |
| Statements | 93.00% | 92.63% | 📉 -0.37% |
| Functions | 93.27% | 92.74% | 📉 -0.53% |
| Branches | 86.33% | 85.77% | 📉 -0.56% |
📁 Per-file Coverage Changes (7 files)
| File | Lines (Before → After) | Statements (Before → After) |
|---|---|---|
src/cloud-hypervisor/manager.ts |
89.2% → 88.3% (-0.86%) | 88.3% → 87.3% (-1.01%) |
src/cloud-hypervisor-runtime-backend.ts |
97.9% → 97.2% (-0.69%) | 95.1% → 94.8% (-0.27%) |
src/cloud-hypervisor/launcher.ts |
93.9% → 93.8% (-0.18%) | 92.8% → 92.5% (-0.22%) |
src/cloud-hypervisor/preflight.ts |
95.4% → 95.8% (+0.45%) | 94.8% → 95.3% (+0.50%) |
src/microvm/workspace.ts |
89.5% → 90.1% (+0.59%) | 88.4% → 88.8% (+0.48%) |
src/services/cli-proxy-service.ts |
95.0% → 97.8% (+2.77%) | 95.0% → 94.2% (-0.77%) |
src/log-directory-setup.ts |
96.2% → 100.0% (+3.78%) | 96.3% → 100.0% (+3.71%) |
✨ New Files (3 files)
src/cloud-hypervisor/exports.ts: 87.9% linessrc/cloud-hypervisor/virtiofsd.ts: 76.2% linessrc/microvm/rootfs.ts: 72.0% lines
Coverage comparison generated by scripts/ci/compare-coverage.ts
Keep the credential-bearing CLI proxy on the internal network. Route external DIFC traffic through a credential-free fixed-target relay. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0ee67ebb-e8fc-45a1-a3a1-3d0cdcbc022e
lpcox
changed the title
|
| Metric | Base | PR | Delta |
|---|---|---|---|
| Lines | 94.11% | 93.86% | 📉 -0.25% |
| Statements | 93.00% | 92.69% | 📉 -0.31% |
| Functions | 93.27% | 92.63% | 📉 -0.64% |
| Branches | 86.31% | 85.85% | 📉 -0.46% |
📁 Per-file Coverage Changes (10 files)
| File | Lines (Before → After) | Statements (Before → After) |
|---|---|---|
src/cloud-hypervisor-runtime-backend.ts |
97.9% → 97.1% (-0.83%) | 95.1% → 94.6% (-0.52%) |
src/cloud-hypervisor/launcher.ts |
93.9% → 93.8% (-0.18%) | 92.8% → 92.5% (-0.22%) |
src/firecracker-runtime-backend.ts |
98.1% → 98.0% (-0.16%) | 94.7% → 94.3% (-0.44%) |
src/services/optional-services.ts |
100.0% → 100.0% (+0.00%) | 96.6% → 96.7% (+0.04%) |
src/cloud-hypervisor/preflight.ts |
95.4% → 95.8% (+0.45%) | 94.8% → 95.3% (+0.50%) |
src/microvm/workspace.ts |
89.5% → 90.1% (+0.59%) | 88.4% → 88.8% (+0.48%) |
src/cloud-hypervisor/manager.ts |
89.2% → 90.7% (+1.45%) | 88.3% → 89.3% (+1.02%) |
src/services/cli-proxy-service.ts |
95.0% → 98.0% (+3.03%) | 95.0% → 94.8% (-0.18%) |
src/log-directory-setup.ts |
96.2% → 100.0% (+3.78%) | 96.3% → 100.0% (+3.71%) |
src/microvm/network.ts |
96.0% → 100.0% (+3.95%) | 95.3% → 100.0% (+4.69%) |
✨ New Files (11 files)
src/cloud-hypervisor/diagnostics.ts: 84.7% linessrc/cloud-hypervisor/exports.ts: 87.9% linessrc/cloud-hypervisor/guest-execution.ts: 100.0% linessrc/cloud-hypervisor/manager-types.ts: 100.0% linessrc/cloud-hypervisor/virtiofsd.ts: 76.2% linessrc/cloud-hypervisor/vm-config-builder.ts: 89.5% linessrc/microvm/guest-environment.ts: 100.0% linessrc/microvm/network-commands.ts: 97.8% linessrc/microvm/network-manager.ts: 98.4% linessrc/microvm/network-plan.ts: 95.2% linessrc/microvm/rootfs.ts: 72.0% lines
Coverage comparison generated by scripts/ci/compare-coverage.ts
|
🛡️ Smoke Copilot Network Isolation confirmed the egress allowlist is enforced. ✅ Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "example.com"See Network Configuration for more information.
|
|
❌ Smoke Copilot BYOK AOAI (Entra) reports failed. AOAI BYOK (Entra) mode investigation needed...
|
|
✨ The prophecy is fulfilled... Smoke Codex has completed its mystical journey. The stars align. 🌟 Warning Firewall blocked 5 domainsThe following domains were blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "msfeed12.pkgs.visualstudio.com"
- "msfeed17.pkgs.visualstudio.com"
- "msfeed2.pkgs.visualstudio.com"
- "msfeed25.pkgs.visualstudio.com"
- "registry.npmjs.org"See Network Configuration for more information.
|
|
✅ Security Guard completed successfully! Security review of PR #7338: No security issues found. Changes to cli-proxy network isolation are properly gated feature flags with secure defaults (loopback-only binding unless explicitly enabled). No firewall, capability, ACL, or validation weaknesses introduced.
|
|
📡 Smoke OTel Tracing completed. All tracing scenarios validated. ✅
|
|
🔌 Smoke Services — All services reachable! ✅
|
|
❌ Smoke Gemini reports failed. Facets need polishing...
|
|
📰 VERDICT: Smoke Docker Sbx has concluded. All systems operational. This is a developing story. 🎤
|
|
✅ Build Test Suite completed successfully!
|
|
Chroot tests passed! Smoke Chroot - All security and functionality tests succeeded.
|
|
❌ Smoke Copilot BYOK AOAI (api-key) reports failed. AOAI BYOK (api-key) mode investigation needed...
|
|
Smoke Test Results
Overall: PASS
|
Smoke Test: Claude Engine Validation
Overall result: PASS
|
|
Copilot BYOK (Direct) Smoke Test Recent merged PRs: #7370 docs: include Cloud Hypervisor in runtime guides, #7362 Remove Firecracker support
Running in direct BYOK mode (COPILOT_PROVIDER_API_KEY) via api-proxy → api.githubcopilot.com Overall: PASS cc
|
|
Smoke Test: Copilot Network Isolation Egress Enforcement EGRESS_RESULT allow=pass deny=pass ✅ Allowed domain (github.com) reachable — HTTP 200 Overall: PASS cc Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "example.com"See Network Configuration for more information.
|
Smoke Test: GitHub Actions Services Connectivity
Overall: FAIL — sandbox cannot resolve/reach
|
github-actions
Bot
added
the
smoke-copilot-network-isolation
|
Smoke Test: API Proxy OTel Tracing — results
Overall: all applicable scenarios pass; Scenario 5 is expected-pending since this smoke test doesn't invoke the api-proxy sidecar directly.
|
|
Smoke Test: Docker Sbx — PR #7338: fix: route CLI proxy through isolated egress relay
Overall: PASS
|
|
Smoke test: FAIL
Warning Firewall blocked 5 domainsThe following domains were blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "msfeed12.pkgs.visualstudio.com"
- "msfeed17.pkgs.visualstudio.com"
- "msfeed2.pkgs.visualstudio.com"
- "msfeed25.pkgs.visualstudio.com"
- "registry.npmjs.org"See Network Configuration for more information.
|
Chroot Version Comparison
Overall: FAILED — Node.js version mismatch between host and chroot environments.
|
🏗️ Build Test Suite Results
Overall: 7/8 ecosystems passed — FAIL Failure detailsJava (gson, caffeine): Root cause:
|
In network-isolation mode,
awf-netis internal and cannot reach a host or external DIFC proxy. The previous implementation fixed that reachability issue by dual-homing the credential-bearingcli-proxy, which gave agent-controlledghand Git subprocesses an unrestricted route throughawf-ext.This revision keeps
cli-proxyonawf-netonly. For an external DIFC target, AWF now creates a separatecli-proxy-egressservice that:awf-ext;The credential-bearing proxy connects to the relay over
awf-net. Attached sibling DIFC proxies continue to work directly and do not create the relay. Loopback DIFC addresses are normalized tohost.docker.internalso the relay reaches the runner-host proxy rather than itself.Tests cover external relay creation, credential and mount isolation, attached sibling behavior, loopback normalization, network membership, and dependency ordering.
Fixes #7335