Auto-allow topology-attached container hostnames in Squid ACL#6473
Conversation
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.
There was a problem hiding this comment.
Pull request overview
Adds topology-attached container hostnames to Squid’s allowlist in network-isolation mode.
Changes:
- Automatically appends and deduplicates topology peer names.
- Adds unit tests for enabled, disabled, duplicate, and empty cases.
- However, Squid still cannot resolve these names or access common nonstandard ports.
Show a summary per file
| File | Description |
|---|---|
src/commands/preflight.ts |
Adds topology peer hostnames to allowed domains. |
src/commands/preflight.test.ts |
Tests hostname auto-add behavior. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 2/2 changed files
- Comments generated: 2
- Review effort level: Medium
| if (options.networkIsolation && Array.isArray(options.topologyAttach)) { | ||
| for (const containerName of options.topologyAttach as string[]) { | ||
| if (!allowedDomains.includes(containerName)) { | ||
| allowedDomains.push(containerName); |
There was a problem hiding this comment.
Good catch — fixed in 9de869d. patchComposeWithTopologyHosts now also patches the squid-proxy service extra_hosts with the discovered peer name→IP mappings. Squid consults /etc/hosts (its default hosts_file) before falling back to dns_nameservers, so it can now resolve Docker-only names like awmg-mcpg. The phased startup applies this patch during onNetworkReady (before the full docker compose up), which recreates squid-proxy so it picks up the new hosts entries. Added a unit test asserting the squid-proxy service gets the entries.
| for (const containerName of options.topologyAttach as string[]) { | ||
| if (!allowedDomains.includes(containerName)) { | ||
| allowedDomains.push(containerName); | ||
| logger.debug(`Network-isolation: auto-allowing topology peer "${containerName}" in Squid ACL`); |
There was a problem hiding this comment.
Fixed in 9de869d. Added a dedicated SquidConfig.topologyPeers that emits a per-peer http_access allow (dstdomain) placed before the deny !Safe_ports and raw-IP deny rules, so trusted topology peers are reachable on any port (e.g. http://awmg-mcpg:8080). This mirrors the existing apiProxyIp/apiProxyPorts precedent and is narrowly scoped by dstdomain to the exact peer hostnames, so it does not widen access to arbitrary destinations. Since these peers are operator-attached and already directly reachable via NO_PROXY, allowing any port to them through Squid keeps the same trust level. Added tests asserting the allow rule is generated and ordered before both the Safe_ports and raw-IP denies.
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
|
✅ Copilot review passed with no inline comments. @copilot Add the |
✅ Coverage Check PassedOverall Coverage
📁 Per-file Coverage Changes (3 files)
✨ New Files (1 files)
Coverage comparison generated by |
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
|
🚀 Security Guard has started processing this pull request |
|
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. 🌟 |
|
✅ Smoke Gemini completed. All facets verified. 💎 Smoke test completed. Connectivity and MCP tests failed. File writing and bash verification passed. |
|
🔌 Smoke Services — All services reachable! ✅ |
|
🛡️ Smoke Copilot Network Isolation confirmed the egress allowlist is enforced. ✅ |
|
✅ Build Test Suite completed successfully! |
|
📰 VERDICT: Smoke Docker Sbx has concluded. All systems operational. This is a developing story. 🎤 |
|
🔑 Smoke Copilot PAT PAT auth validated. All systems operational. ✅ |
|
❌ Contribution Check failed. Please review the logs for details. |
|
✅ Smoke Claude passed |
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
|
✅ Smoke Copilot BYOK completed. Copilot BYOK mode operational. 🔓 |
🔥 AWF Smoke Test — Copilot Network Isolation
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.
|
github-actions
Bot
added
the
smoke-copilot-network-isolation
Smoke Test Results — Services Connectivity
Overall: FAIL —
|
Smoke Test Results: Copilot BYOK (Direct Mode) ✅
Status: PASS — Agent verified in direct BYOK mode with inference connectivity confirmed.
|
Chroot Version Comparison ✅
All runtimes match between host and chroot environments.
|
🔬 Smoke Test: Copilot PAT Auth — PR #6473
Overall: PARTIAL — core connectivity PASS; pre-computed data not injected. Auth mode: PAT (COPILOT_GITHUB_TOKEN)
|
|
Running in direct BYOK mode (AWF_AUTH_TYPE=github-oidc + AWF_AUTH_AZURE_* + COPILOT_PROVIDER_BASE_URL) via api-proxy → Azure OpenAI (Foundry, o4-mini-aw) via Microsoft Entra Overall: PASS
|
Smoke Test: Claude Engine Validation
Overall result: PASS ✅
|
🔍 Smoke Test Results
Overall: PARTIAL — MCP connectivity confirmed; pre-step smoke data was not passed through (workflow template expressions not substituted).
|
Smoke Test: API Proxy OpenTelemetry Tracing — Results
All scenarios pass. OTEL tracing integration is working correctly.
|
Smoke Test Results: Gemini
Merged PRs reviewed:
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.
|
|
Running in direct BYOK mode (COPILOT_PROVIDER_API_KEY + COPILOT_PROVIDER_BASE_URL) via api-proxy → Azure OpenAI (Foundry, o4-mini-aw) Overall: PASS
|
🏗️ Build Test Suite Results
Overall: 8/8 ecosystems passed — ✅ PASS
|
|
Merged PRs:
Checks:
Overall: FAIL 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.
|
Smoke Test Results — Docker Sbx
Overall: PARTIAL — MCP connectivity verified; pre-step data was not substituted (template variables literal in prompt).
|
In network-isolation mode, topology-attached containers (e.g.
awmg-mcpg) were added toNO_PROXYso proxy-aware clients bypass Squid. But tools that honourHTTP(S)_PROXYwhile ignoringNO_PROXYstill route requests through Squid — which blocked them, since the container hostname wasn't in the allowed-domain ACL. This is the last-mile fix for theNO_PROXY-based series (#6189, #6438, #6401) that resolved #6467.Changes
src/commands/preflight.ts— InresolveAllowedDomains(), when network-isolation is active, topology peer hostnames are auto-appended toallowedDomains(deduplicated, with a debug log) so Squid permits them forNO_PROXY-ignoring proxy clients. This pairs with the existingNO_PROXYaddition inproxy-environment.ts.The gating and peer set now mirror
buildProxyEnvironment()exactly:runtimeUsesComposeAgent()in addition tonetworkIsolation. microVM backends (Dockersbx) run the agent offawf-netand reach peers via their own proxy-chaining path, so topology hostnames are skipped there too, keeping the ACL andNO_PROXYcode paths consistent.difcProxyHostis also auto-allowed through Squid (scheme/port stripped viaparseDifcProxyHost), mirroring theNO_PROXYentry added in fix: exempt DIFC/cli-proxy host from proxy routing in isolation mode #6438, so it isn't silently blocked forNO_PROXY-ignoring clients.dstdomainentries viaformatDomainForSquid, which prepends a leading dot (awmg-mcpg→.awmg-mcpg), matching the host and its subdomains. Safe for internal Docker hostnames (a bare label likegithubmatches hostgithub, notgithub.com); operators should avoid topology names that collide with trusted public labels.src/commands/preflight.test.ts— Unit tests covering: auto-add when isolation is on, no-op when isolation is off, no duplication if already listed, no-op for empty attach list, skip for a non-compose runtime (sbx), add for an explicit compose runtime (runc), anddifcProxyHostauto-allow when not listed intopologyAttach.