fix: exempt DIFC/cli-proxy host from proxy routing in isolation mode#6438
Conversation
buildProxyEnvironment only added topologyAttach peers to NO_PROXY, so a difcProxyHost set independently of topologyAttach was routed through Squid and denied. Add the DIFC/cli-proxy host (stripping any ":port" suffix) when in network-isolation mode with a compose agent, fixing the failing proxy-environment test. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 14b695ec-1224-416b-999f-d3845138ea2f
There was a problem hiding this comment.
Pull request overview
Adds the DIFC/CLI proxy host to NO_PROXY during compose-based network isolation.
Changes:
- Exempts
difcProxyHostfrom Squid routing. - Attempts to remove its port suffix.
Show a summary per file
| File | Description |
|---|---|
src/services/agent-environment/proxy-environment.ts |
Adds the DIFC proxy host to NO_PROXY. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Medium
| // must bypass Squid even when it isn't listed in topologyAttach. Strip any | ||
| // ":port" suffix since undici matches NO_PROXY against the hostname only. | ||
| if (config.difcProxyHost) { | ||
| noProxyHosts.push(config.difcProxyHost.split(':')[0]); |
|
@copilot address review feedback |
✅ Coverage Check PassedOverall Coverage
📁 Per-file Coverage Changes (1 files)
Coverage comparison generated by |
Replace `split(':')[0]` with `parseDifcProxyHost().host` so that
scheme-prefixed values (e.g. https://proxy.internal:443) and bracketed
IPv6 addresses (e.g. [::1]:18443) are handled correctly.
Add test cases covering port-only, scheme-prefixed, and IPv6 forms.
Done in the latest commit. Replaced |
|
✅ Copilot review passed with no inline comments. @lpcox Add the |
|
🛡️ Smoke Copilot Network Isolation confirmed the egress allowlist is enforced. ✅ |
|
✅ Smoke Claude passed |
|
📡 Smoke OTel Tracing completed. All tracing scenarios validated. ✅ |
|
🔌 Smoke Services — All services reachable! ✅ |
|
Chroot tests passed! Smoke Chroot - All security and functionality tests succeeded. |
|
✅ Smoke Copilot BYOK AOAI (Entra) completed. Copilot AOAI BYOK (Entra) mode operational. 🔓 |
|
🔑 Smoke Copilot PAT PAT auth validated. All systems operational. ✅ |
|
✅ Smoke Copilot BYOK completed. Copilot BYOK mode operational. 🔓 |
|
✅ Smoke Gemini completed. All facets verified. 💎 |
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
|
🚀 Security Guard has started processing this pull request |
|
✅ Build Test Suite completed successfully! |
|
✅ Smoke Copilot BYOK AOAI (api-key) completed. Copilot AOAI BYOK (api-key) mode operational. 🔓 |
|
❌ Contribution Check failed. Please review the logs for details. |
|
✨ The prophecy is fulfilled... Smoke Codex has completed its mystical journey. The stars align. 🌟 |
|
📰 VERDICT: Smoke Docker Sbx has concluded. All systems operational. This is a developing story. 🎤 |
|
Smoke test: Copilot network isolation egress enforcement — @lpcox EGRESS_RESULT allow=pass deny=pass
Overall: PASS Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "example.com"See Network Configuration for more information.
|
Smoke Test: Copilot PAT Auth — PASS ✅
Auth mode: PAT (COPILOT_GITHUB_TOKEN)
|
Smoke Test Results — Services Connectivity
Overall: FAIL —
|
Chroot Version Comparison Results ✅
All runtime versions match between host and chroot environment.
|
Smoke Test: API Proxy OpenTelemetry Tracing
All scenarios pass. OTEL tracing integration is fully implemented.
|
|
|
✅ 6429 Internalize dead Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "registry.npmjs.org"See Network Configuration for more information.
|
🏗️ Build Test Suite Results
Overall: 8/8 ecosystems passed — ✅ PASS
|
|
GitHub MCP Test: ❌ Running in direct BYOK mode (COPILOT_PROVIDER_API_KEY + COPILOT_PROVIDER_BASE_URL) via api-proxy → Azure OpenAI (Foundry, o4-mini-aw)
|
🧪 Smoke Test: Docker Sbx Validation
Overall:
|
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.
|
Address review feedback on auto-allowing topology peers in the Squid ACL: - Gate the ACL addition on runtimeUsesComposeAgent() in addition to networkIsolation, mirroring buildProxyEnvironment() exactly. microVM backends (Docker sbx) run the agent off awf-net and reach peers via their own proxy-chaining path, so topology hostnames are skipped there too, keeping the ACL and NO_PROXY code paths consistent. - Also auto-allow the DIFC/cli-proxy host (difcProxyHost) through Squid, mirroring the NO_PROXY entry added in #6438, so NO_PROXY-ignoring proxy clients can reach it. The scheme/port are stripped via parseDifcProxyHost. - Document Squid dstdomain wildcard semantics (formatDomainForSquid prepends a leading dot, matching the host and its subdomains). - Add tests: non-compose runtime skip, explicit compose runtime add, and difcProxyHost auto-allow. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 5f6caa3b-9079-4d7f-a752-524b0f990e0c
* Initial plan * Auto-allow topology-attached container hostnames in Squid ACL In network-isolation mode, topology-attached containers (e.g. awmg-mcpg) are already added to NO_PROXY so proxy-aware clients connect to them directly. However, tools that honour HTTP(S)_PROXY but ignore NO_PROXY route those requests through Squid, which blocks them because the container hostname is not in the allowed-domain ACL. Fix: in resolveAllowedDomains(), when networkIsolation is true and topologyAttach container names are present, automatically add each name to allowedDomains so Squid permits those requests. * fix: align topology ACL gating with proxy env and mirror difcProxyHost Address review feedback on auto-allowing topology peers in the Squid ACL: - Gate the ACL addition on runtimeUsesComposeAgent() in addition to networkIsolation, mirroring buildProxyEnvironment() exactly. microVM backends (Docker sbx) run the agent off awf-net and reach peers via their own proxy-chaining path, so topology hostnames are skipped there too, keeping the ACL and NO_PROXY code paths consistent. - Also auto-allow the DIFC/cli-proxy host (difcProxyHost) through Squid, mirroring the NO_PROXY entry added in #6438, so NO_PROXY-ignoring proxy clients can reach it. The scheme/port are stripped via parseDifcProxyHost. - Document Squid dstdomain wildcard semantics (formatDomainForSquid prepends a leading dot, matching the host and its subdomains). - Add tests: non-compose runtime skip, explicit compose runtime add, and difcProxyHost auto-allow. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 5f6caa3b-9079-4d7f-a752-524b0f990e0c * fix: make Squid actually serve topology peers (DNS + non-standard ports) Address reviewer feedback that adding topology peer hostnames to the domain allowlist alone does not make them reachable through Squid: 1. DNS: Squid resolves proxied destinations via external `dns_nameservers`, which cannot resolve Docker-only peer names (e.g. `awmg-mcpg`). `patchComposeWithTopologyHosts` now also patches the `squid-proxy` service `extra_hosts` (Squid reads /etc/hosts before DNS). The phased startup applies this before the full `docker compose up`, which recreates squid-proxy to pick up the entries. 2. Ports: `http_access deny !Safe_ports` (80/443) fires before the domain allowlist, so `http://awmg-mcpg:8080` was denied on the port. A new `SquidConfig.topologyPeers` emits a per-peer `http_access allow` (dstdomain) *before* the Safe_ports and raw-IP deny rules, permitting any port to these operator-attached, trusted peers. This mirrors the existing apiProxyIp/apiProxyPorts precedent. Also extract `resolveTopologyPeerHosts()` (topology-peers.ts) shared by preflight (domain ACL) and config-writer (Squid allow), so the peer set and compose-runtime gating stay in sync with buildProxyEnvironment(). Tests: topology-peers helper, Squid topology-peer section + rule ordering, and squid-proxy extra_hosts patching. Full suite: 3986 passing. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 5f6caa3b-9079-4d7f-a752-524b0f990e0c --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Landon Cox <landon.cox@microsoft.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Summary
npm testwas failing onmainwith one broken test insrc/services/agent-environment/proxy-environment.test.ts:buildProxyEnvironmentonly addedconfig.topologyAttachpeers toNO_PROXY. AdifcProxyHostconfigured independently oftopologyAttachwas therefore routed through Squid and denied instead of being reached directly.Fix
In network-isolation mode with a compose agent, also add
config.difcProxyHosttoNO_PROXY, stripping any:portsuffix (undici matchesNO_PROXYagainst the hostname only).Testing
npm test— all 3959 tests pass (was 1 failing).