fix: add api-proxy IP to squid allowlist

[Claude]

Squid was blocking traffic to the api-proxy sidecar because only the hostname 'api-proxy' was in the allowlist, not its IP address 172.30.0.30. Some HTTP clients bypass NO_PROXY settings or use the IP directly when BASE_URL environment variables contain IPs.

Changes

• src/docker-manager.ts: Add both hostname and IP to Squid allowlist when api-proxy is enabled

  const domainsForSquid = config.enableApiProxy && networkConfig.proxyIp && (config.openaiApiKey || config.anthropicApiKey)
    ? [...config.allowedDomains, 'api-proxy', networkConfig.proxyIp]  // Now includes IP
    : config.allowedDomains;

• src/docker-manager.test.ts: Verify IP appears in generated Squid config

• src/host-iptables.test.ts: Remove invalid test calling setupHostIptables with incorrect signature (4 params instead of 3)

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

• https://api.github.com/repos/github/gh-aw-firewall/actions/jobs/63495506647
  • Triggering command: /usr/bin/gh gh run view 21978551088 --job 63495506647 --log (http block)

If you need me to access, download, or install something from one of these locations, you can either:

• Configure Actions setup steps to set up my environment, which run before the firewall is enabled
• Add the appropriate URLs or hosts to the custom allowlist in this repository's Copilot coding agent settings (admins only)

[Copilot]

Pull request overview

This PR attempts to fix Squid blocking traffic to the api-proxy sidecar by adding the api-proxy IP address (172.30.0.30) to the Squid allowlist, addressing cases where HTTP clients bypass NO_PROXY settings or use the IP directly from BASE_URL environment variables.

Changes:

• Added api-proxy IP address (172.30.0.30) to Squid allowlist alongside the hostname
• Added test to verify IP appears in Squid config
• Removed invalid test that called setupHostIptables with incorrect 4-parameter signature

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
src/docker-manager.ts	Adds api-proxy IP to domains array for Squid allowlist (lines 1150-1153)
src/docker-manager.test.ts	Adds test assertion to verify IP appears in squid.conf (line 1915)
src/host-iptables.test.ts	Removes invalid test calling setupHostIptables with 4 params instead of 3 (lines 229-259 removed)

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

This test verifies that the IP address '172.30.0.30' appears in the Squid config, but it doesn't verify that the IP is correctly formatted for Squid ACLs. Squid's dstdomain ACL type does not match IP addresses - only domain names. IP addresses require the 'dst' ACL type. The test should verify that the IP appears in a 'dst' ACL, not a 'dstdomain' ACL, or alternatively verify that the IP is NOT added if the design is to rely solely on NO_PROXY for direct connections.

Suggested change

	// Verify squid.conf includes api-proxy hostname and IP in allowed domains
	const squidConfPath = path.join(testDir, 'squid.conf');
	if (fs.existsSync(squidConfPath)) {
	const content = fs.readFileSync(squidConfPath, 'utf-8');
	expect(content).toContain('github.com');
	expect(content).toContain('api-proxy');
	expect(content).toContain('172.30.0.30'); // api-proxy IP address
	// Verify squid.conf includes api-proxy hostname and IP in allowed ACLs
	const squidConfPath = path.join(testDir, 'squid.conf');
	if (fs.existsSync(squidConfPath)) {
	const content = fs.readFileSync(squidConfPath, 'utf-8');
	expect(content).toContain('github.com');
	expect(content).toContain('api-proxy');
	expect(content).toMatch(/^acl\s+\S+\s+dst\s+172\.30\.0\.30/m); // api-proxy IP address in dst ACL

[Copilot]

IP addresses cannot be properly handled by Squid's dstdomain ACL type. The current implementation adds the IP address to the domains array, which will be processed by formatDomainForSquid() and used with the 'dstdomain' ACL. However, Squid's dstdomain ACL is designed for domain names and will not match IP addresses. For IP addresses, Squid requires the 'dst' ACL type instead.

Additionally, formatDomainForSquid() will incorrectly add a leading dot to the IP address (e.g., ".172.30.0.30") since it contains dots, which is semantically incorrect for an IP.

To fix this, IP addresses should either be:

1. Handled separately and added to Squid config using 'acl allowed_ips dst' instead of 'acl allowed_domains dstdomain', or
2. Not added to Squid allowlist at all if NO_PROXY is sufficient (which is the intended design based on comments at lines 966-975)

See below for a potential fix:

  // When API proxy is enabled and has API keys, add api-proxy hostname to allowed domains so the agent can communicate with it
  const domainsForSquid = config.enableApiProxy && networkConfig.proxyIp && (config.openaiApiKey || config.anthropicApiKey)
    ? [...config.allowedDomains, 'api-proxy']