Skip to content

Commit b798aca

Browse files
CopilotlpcoxCopilot
authored
Auto-allow topology-attached container hostnames in Squid ACL (#6473)
* 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>
1 parent 2144cdc commit b798aca

11 files changed

Lines changed: 391 additions & 2 deletions

src/commands/preflight.test.ts

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -296,6 +296,79 @@ describe('resolveAllowedDomains', () => {
296296
expect(mockedRules.loadAndMergeDomains).not.toHaveBeenCalled();
297297
expect(result.allowedDomains).toEqual([]);
298298
});
299+
300+
it('auto-adds topology-attached container names to allowedDomains in network-isolation mode', () => {
301+
const result = resolveAllowedDomains({
302+
networkIsolation: true,
303+
topologyAttach: ['awmg-mcpg', 'awmg-cli-proxy'],
304+
});
305+
expect(result.allowedDomains).toContain('awmg-mcpg');
306+
expect(result.allowedDomains).toContain('awmg-cli-proxy');
307+
expect(mockedLogger.debug).toHaveBeenCalledWith(
308+
expect.stringContaining('auto-allowing topology peer "awmg-mcpg"')
309+
);
310+
expect(mockedLogger.debug).toHaveBeenCalledWith(
311+
expect.stringContaining('auto-allowing topology peer "awmg-cli-proxy"')
312+
);
313+
});
314+
315+
it('does not add topology-attached container names when networkIsolation is false', () => {
316+
const result = resolveAllowedDomains({
317+
networkIsolation: false,
318+
topologyAttach: ['awmg-mcpg'],
319+
});
320+
expect(result.allowedDomains).not.toContain('awmg-mcpg');
321+
});
322+
323+
it('does not duplicate topology container names already in allowedDomains', () => {
324+
mockedDomainUtils.parseDomains.mockReturnValue(['awmg-mcpg']);
325+
mockedOptionParsers.processLocalhostKeyword.mockReturnValue({
326+
allowedDomains: ['awmg-mcpg'],
327+
localhostDetected: false,
328+
shouldEnableHostAccess: false,
329+
});
330+
331+
const result = resolveAllowedDomains({
332+
allowDomains: 'awmg-mcpg',
333+
networkIsolation: true,
334+
topologyAttach: ['awmg-mcpg'],
335+
});
336+
expect(result.allowedDomains.filter(d => d === 'awmg-mcpg')).toHaveLength(1);
337+
});
338+
339+
it('does not add topology containers when topologyAttach is empty', () => {
340+
const result = resolveAllowedDomains({
341+
networkIsolation: true,
342+
topologyAttach: [],
343+
});
344+
expect(result.allowedDomains).toEqual([]);
345+
});
346+
347+
it('does not add topology container names for a non-compose runtime (e.g. sbx)', () => {
348+
const result = resolveAllowedDomains({
349+
networkIsolation: true,
350+
containerRuntime: 'sbx',
351+
topologyAttach: ['awmg-mcpg'],
352+
});
353+
expect(result.allowedDomains).not.toContain('awmg-mcpg');
354+
});
355+
356+
it('adds topology container names for an explicit compose runtime', () => {
357+
const result = resolveAllowedDomains({
358+
networkIsolation: true,
359+
containerRuntime: 'runc',
360+
topologyAttach: ['awmg-mcpg'],
361+
});
362+
expect(result.allowedDomains).toContain('awmg-mcpg');
363+
});
364+
365+
it('auto-allows the DIFC/cli-proxy host even when not listed in topologyAttach', () => {
366+
const result = resolveAllowedDomains({
367+
networkIsolation: true,
368+
difcProxyHost: 'https://awmg-cli-proxy:18443',
369+
});
370+
expect(result.allowedDomains).toContain('awmg-cli-proxy');
371+
});
299372
});
300373

301374
describe('parseDomainOptions', () => {

src/commands/preflight.ts

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import { parseDomains, parseDomainsFile } from '../domain-utils';
88
import { processLocalhostKeyword } from '../option-parsers';
99
import { resolveCopilotApiRouting } from '../copilot-api-resolver';
1010
import { resolveApiTargetsToAllowedDomains } from '../api-proxy-config';
11+
import { resolveTopologyPeerHosts } from '../topology-peers';
1112

1213
/**
1314
* Resolves the Commander option-value source for a given option name.
@@ -176,6 +177,30 @@ export function resolveAllowedDomains(options: Record<string, unknown>): Allowed
176177
logger.debug.bind(logger)
177178
);
178179

180+
// In network-isolation (topology) mode, automatically add trusted topology
181+
// peer hostnames to the Squid allowed-domain ACL. NO_PROXY is also set for
182+
// these peers (in proxy-environment.ts) so that proxy-aware clients
183+
// (undici/rmcp) connect directly; adding them here ensures Squid does not
184+
// block requests from tools that honour HTTP(S)_PROXY but ignore NO_PROXY.
185+
//
186+
// This covers the standard-port (80/443) path. Non-standard MCP ports (e.g.
187+
// http://awmg-mcpg:8080) and Squid's DNS resolution of these Docker-only
188+
// hostnames are handled separately via SquidConfig.topologyPeers and the
189+
// squid-proxy extra_hosts patch (see config-writer.ts / topology.ts).
190+
//
191+
// NOTE ON SQUID SEMANTICS: these names are emitted as dstdomain ACL entries
192+
// via formatDomainForSquid, which prepends a leading dot (e.g. "awmg-mcpg" ->
193+
// ".awmg-mcpg"). Squid therefore matches the host itself *and* any subdomain
194+
// (*.awmg-mcpg). This is safe for internal Docker hostnames (a bare label
195+
// like "github" matches host "github", not "github.com"), but operators
196+
// should avoid topology names that collide with trusted public labels.
197+
for (const peer of resolveTopologyPeerHosts(options)) {
198+
if (!allowedDomains.includes(peer)) {
199+
allowedDomains.push(peer);
200+
logger.debug(`Network-isolation: auto-allowing topology peer "${peer}" in Squid ACL`);
201+
}
202+
}
203+
179204
validateAllowedDomains(allowedDomains);
180205

181206
return { allowedDomains, localhostResult, resolvedCopilotApiTarget, resolvedCopilotApiBasePath };

src/config-writer.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import * as yaml from 'js-yaml';
44
import { WrapperConfig, API_PROXY_PORTS, DockerComposeConfig } from './types';
55
import { logger } from './logger';
66
import { generatePolicyManifest, generateSquidConfig } from './squid-config';
7+
import { resolveTopologyPeerHosts } from './topology-peers';
78
import { generateSessionCa, initSslDb, isOpenSslAvailable } from './ssl-bump';
89
import { parseUrlPatterns } from './domain-matchers';
910
import { SslConfig, SQUID_PORT } from './host-env';
@@ -331,6 +332,11 @@ export async function writeConfigs(config: WrapperConfig): Promise<void> {
331332
apiProxyIp: networkConfig.proxyIp,
332333
apiProxyPorts: Object.values(API_PROXY_PORTS),
333334
} : {}),
335+
// Allow trusted topology peers (MCP gateway, DIFC/cli-proxy) on any port in
336+
// network-isolation mode, for proxy clients that ignore NO_PROXY. DNS for
337+
// these Docker-only names is provided via the squid-proxy extra_hosts patch
338+
// (see patchComposeWithTopologyHosts in topology.ts).
339+
topologyPeers: resolveTopologyPeerHosts(config),
334340
});
335341
const squidConfigPath = path.join(config.workDir, 'squid.conf');
336342
fs.writeFileSync(squidConfigPath, squidConfig, { mode: 0o644 });
Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
import { generateSquidConfig } from './squid-config';
2+
3+
describe('generateSquidConfig: topology peers (network-isolation)', () => {
4+
const port = 3128;
5+
6+
it('emits a dstdomain allow rule for each topology peer', () => {
7+
const config = generateSquidConfig({
8+
domains: ['github.com'],
9+
port,
10+
topologyPeers: ['awmg-mcpg', 'awmg-cli-proxy'],
11+
});
12+
expect(config).toContain('acl topology_peer_awmg_mcpg dstdomain .awmg-mcpg');
13+
expect(config).toContain('http_access allow topology_peer_awmg_mcpg');
14+
expect(config).toContain('acl topology_peer_awmg_cli_proxy dstdomain .awmg-cli-proxy');
15+
expect(config).toContain('http_access allow topology_peer_awmg_cli_proxy');
16+
});
17+
18+
it('places the peer allow before the Safe_ports deny (so any port is reachable)', () => {
19+
const config = generateSquidConfig({
20+
domains: ['github.com'],
21+
port,
22+
topologyPeers: ['awmg-mcpg'],
23+
});
24+
const allowPos = config.indexOf('http_access allow topology_peer_awmg_mcpg');
25+
const safePortsDenyPos = config.indexOf('http_access deny !Safe_ports');
26+
expect(allowPos).toBeGreaterThan(-1);
27+
expect(safePortsDenyPos).toBeGreaterThan(-1);
28+
expect(allowPos).toBeLessThan(safePortsDenyPos);
29+
});
30+
31+
it('places the peer allow before the raw-IP deny rules', () => {
32+
const config = generateSquidConfig({
33+
domains: ['github.com'],
34+
port,
35+
topologyPeers: ['awmg-mcpg'],
36+
});
37+
const allowPos = config.indexOf('http_access allow topology_peer_awmg_mcpg');
38+
const rawIpDenyPos = config.indexOf('http_access deny dst_ipv4');
39+
expect(allowPos).toBeLessThan(rawIpDenyPos);
40+
});
41+
42+
it('emits no topology peer rules when the list is empty or omitted', () => {
43+
const withEmpty = generateSquidConfig({ domains: ['github.com'], port, topologyPeers: [] });
44+
const withUndefined = generateSquidConfig({ domains: ['github.com'], port });
45+
expect(withEmpty).not.toContain('topology_peer_');
46+
expect(withUndefined).not.toContain('topology_peer_');
47+
});
48+
49+
it('sanitizes peer names into safe ACL identifiers', () => {
50+
const config = generateSquidConfig({
51+
domains: ['github.com'],
52+
port,
53+
topologyPeers: ['mcp.gateway-01'],
54+
});
55+
expect(config).toContain('acl topology_peer_mcp_gateway_01 dstdomain .mcp.gateway-01');
56+
});
57+
58+
it('rejects a peer name containing squid-config-breaking characters', () => {
59+
expect(() =>
60+
generateSquidConfig({
61+
domains: ['github.com'],
62+
port,
63+
topologyPeers: ['evil\nhttp_access allow all'],
64+
})
65+
).toThrow(/unsafe for Squid config/i);
66+
});
67+
});

src/squid/config-generator.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ const { version: AWF_VERSION } = require('../../package.json') as { version: str
3232
* // Blocked: internal.example.com -> acl blocked_domains dstdomain .internal.example.com
3333
*/
3434
export function generateSquidConfig(config: SquidConfig): string {
35-
const { domains, blockedDomains, port, sslBump, caFiles, sslDbPath, urlPatterns, enableHostAccess, allowHostPorts, enableDlp, dnsServers, upstreamProxy, apiProxyIp, apiProxyPorts } = config;
35+
const { domains, blockedDomains, port, sslBump, caFiles, sslDbPath, urlPatterns, enableHostAccess, allowHostPorts, enableDlp, dnsServers, upstreamProxy, apiProxyIp, apiProxyPorts, topologyPeers } = config;
3636

3737
validateApiProxyIp(apiProxyIp);
3838

@@ -61,6 +61,7 @@ export function generateSquidConfig(config: SquidConfig): string {
6161
apiProxySection,
6262
allowedIpSection,
6363
dnsSection,
64+
topologyPeersSection,
6465
} = buildConfigSections({
6566
enableDlp,
6667
port,
@@ -76,6 +77,7 @@ export function generateSquidConfig(config: SquidConfig): string {
7677
apiProxyPorts,
7778
apiProxyIp,
7879
dnsServers,
80+
topologyPeers,
7981
});
8082

8183
return `# Squid configuration for egress traffic control
@@ -119,7 +121,7 @@ acl localnet src 192.168.0.0/16
119121
acl localnet src fc00::/7
120122
acl localnet src fe80::/10
121123
122-
${portAclsAndRules}
124+
${topologyPeersSection}${portAclsAndRules}
123125
${apiProxySection}
124126
${allowedIpSection}# Deny CONNECT to raw IP addresses (IPv4 and IPv6)
125127
# Prevents bypassing domain-based filtering via direct IP connections

src/squid/config-sections.ts

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { generateDlpSquidConfig } from '../dlp';
33
import { DEFAULT_DNS_SERVERS } from '../dns-resolver';
44
import { generateSslBumpSection } from './ssl-bump';
55
import { validateAndSanitizeHostAccessPort, validateApiProxyPort } from './validation';
6+
import { formatDomainForSquid } from './domain-acl';
67

78
type DomainsByProto = ReturnType<typeof import('./domain-acl').parseDomainConfig>['domainsByProto'];
89
type PatternsByProto = ReturnType<typeof import('./domain-acl').parseDomainConfig>['patternsByProto'];
@@ -141,6 +142,37 @@ http_access allow from_api_proxy
141142

142143
const IPV4_REGEX = /^[0-9]+\.[0-9]+\.[0-9]+\.[0-9]+$/;
143144

145+
/**
146+
* Generate allow rules for trusted topology-peer hostnames in network-isolation
147+
* (topology) mode.
148+
*
149+
* Docker-internal peers such as an MCP gateway (`awmg-mcpg`) are typically
150+
* served on non-standard ports (e.g. 8080). The normal domain allowlist cannot
151+
* reach them because `http_access deny !Safe_ports` (80/443) fires first. These
152+
* peers are operator-attached and trusted (the same trust level as the direct
153+
* NO_PROXY connection), so we emit an explicit `http_access allow` per peer
154+
* *before* the Safe_ports and raw-IP deny rules, permitting any port.
155+
*
156+
* Scoped by dstdomain to the exact peer hostnames, so this does not widen access
157+
* to arbitrary internet destinations.
158+
*/
159+
function generateTopologyPeersSection(topologyPeers?: string[]): string {
160+
if (!topologyPeers || topologyPeers.length === 0) return '';
161+
162+
const lines = [
163+
'# Allow trusted topology peers (network-isolation mode) on any port before',
164+
'# the Safe_ports and raw-IP deny rules. These are operator-attached',
165+
'# containers (e.g. an MCP gateway) served on non-standard ports; clients',
166+
'# that honour HTTP(S)_PROXY but ignore NO_PROXY reach them through Squid.',
167+
];
168+
for (const peer of topologyPeers) {
169+
const aclName = `topology_peer_${peer.replace(/[^a-zA-Z0-9]/g, '_')}`;
170+
lines.push(`acl ${aclName} dstdomain ${formatDomainForSquid(peer)}`);
171+
lines.push(`http_access allow ${aclName}`);
172+
}
173+
return lines.join('\n') + '\n\n';
174+
}
175+
144176
/**
145177
* Generate allow rules for raw IP addresses that appear in the allowed domains list.
146178
*
@@ -185,6 +217,7 @@ function generateConfigSections(options: {
185217
apiProxyPorts?: number[];
186218
apiProxyIp?: string;
187219
dnsServers?: string[];
220+
topologyPeers?: string[];
188221
}): {
189222
dlpAclSection: string;
190223
dlpAccessSection: string;
@@ -195,6 +228,7 @@ function generateConfigSections(options: {
195228
apiProxySection: string;
196229
allowedIpSection: string;
197230
dnsSection: string;
231+
topologyPeersSection: string;
198232
} {
199233
const {
200234
enableDlp,
@@ -211,6 +245,7 @@ function generateConfigSections(options: {
211245
apiProxyPorts,
212246
apiProxyIp,
213247
dnsServers,
248+
topologyPeers,
214249
} = options;
215250

216251
const { aclSection: dlpAclSection, accessSection: dlpAccessSection } = generateDlpSections(enableDlp);
@@ -226,6 +261,7 @@ function generateConfigSections(options: {
226261
const portAclsAndRules = generatePortAclsAndRules(enableHostAccess, allowHostPorts, apiProxyPorts);
227262
const apiProxySection = generateApiProxySection(apiProxyIp);
228263
const allowedIpSection = generateAllowedIpSection(domains ?? []);
264+
const topologyPeersSection = generateTopologyPeersSection(topologyPeers);
229265
const dnsSection = generateDnsSection(dnsServers);
230266

231267
return {
@@ -238,6 +274,7 @@ function generateConfigSections(options: {
238274
apiProxySection,
239275
allowedIpSection,
240276
dnsSection,
277+
topologyPeersSection,
241278
};
242279
}
243280

src/topology-peers.test.ts

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
import { resolveTopologyPeerHosts } from './topology-peers';
2+
3+
describe('resolveTopologyPeerHosts', () => {
4+
it('returns topology-attach names in network-isolation + compose runtime', () => {
5+
expect(
6+
resolveTopologyPeerHosts({
7+
networkIsolation: true,
8+
topologyAttach: ['awmg-mcpg', 'awmg-cli-proxy'],
9+
})
10+
).toEqual(['awmg-mcpg', 'awmg-cli-proxy']);
11+
});
12+
13+
it('returns [] when network isolation is off', () => {
14+
expect(
15+
resolveTopologyPeerHosts({
16+
networkIsolation: false,
17+
topologyAttach: ['awmg-mcpg'],
18+
})
19+
).toEqual([]);
20+
});
21+
22+
it('returns [] for a non-compose runtime (e.g. sbx)', () => {
23+
expect(
24+
resolveTopologyPeerHosts({
25+
networkIsolation: true,
26+
containerRuntime: 'sbx',
27+
topologyAttach: ['awmg-mcpg'],
28+
})
29+
).toEqual([]);
30+
});
31+
32+
it('includes the DIFC/cli-proxy host with scheme and port stripped', () => {
33+
expect(
34+
resolveTopologyPeerHosts({
35+
networkIsolation: true,
36+
difcProxyHost: 'https://awmg-cli-proxy:18443',
37+
})
38+
).toEqual(['awmg-cli-proxy']);
39+
});
40+
41+
it('deduplicates when the DIFC host is also in topologyAttach', () => {
42+
expect(
43+
resolveTopologyPeerHosts({
44+
networkIsolation: true,
45+
topologyAttach: ['awmg-cli-proxy'],
46+
difcProxyHost: 'awmg-cli-proxy:18443',
47+
})
48+
).toEqual(['awmg-cli-proxy']);
49+
});
50+
51+
it('ignores empty / non-string topologyAttach entries', () => {
52+
expect(
53+
resolveTopologyPeerHosts({
54+
networkIsolation: true,
55+
topologyAttach: ['awmg-mcpg', '', ' ', 42 as unknown as string],
56+
})
57+
).toEqual(['awmg-mcpg']);
58+
});
59+
60+
it('returns [] when nothing is attached', () => {
61+
expect(resolveTopologyPeerHosts({ networkIsolation: true, topologyAttach: [] })).toEqual([]);
62+
});
63+
});

0 commit comments

Comments
 (0)