fix: restrict host gateway iptables bypass to allowed ports only #558
Conversation
|
Chroot tests passed! Smoke Chroot - All security and functionality tests succeeded. |
|
🎬 THE END — Smoke Claude MISSION: ACCOMPLISHED! The hero saves the day! ✨ |
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
✅ Coverage Check PassedOverall Coverage
📁 Per-file Coverage Changes (1 files)
Coverage comparison generated by |
C++ Build Test Results
Overall: PASS ✅ All C++ projects built successfully.
|
Node.js Build Test Results
Overall: PASS All Node.js projects built and tested successfully.
|
|
Smoke Test Results for Copilot
Status: PASS ✅ cc @Mossaka
|
Go Build Test Results
Overall: PASS ✅ All Go projects successfully downloaded dependencies and passed tests.
|
Deno Build Test Results
Overall: ✅ PASS All Deno tests completed successfully.
|
Build Test: Bun - FAILED ❌Status: Test execution failed due to environment limitations Test Results
Overall: FAIL Error DetailsRoot Cause: Bun requires a functional Environment Check:
Impact: Unable to run This test cannot proceed without /proc access. Consider running in an environment with full /proc filesystem support.
|
Smoke Test: Claude EngineLast 2 merged PRs:
Results:
Status: FAIL (Playwright timeout)
|
Chroot Version Comparison Test Results
Overall Result: ❌ FAILED (not all runtimes matched) The chroot environment successfully uses host-installed Go, but Python and Node.js versions differ between host and chroot environments.
|
There was a problem hiding this comment.
Pull request overview
Tightens host-access networking controls by narrowing the iptables host/network gateway bypass to approved ports, adding basic IP validation before rule insertion, and hardening the chroot /etc/hosts mount mode.
Changes:
- Restrict host/network gateway OUTPUT ACCEPT bypass to TCP ports 80/443 plus
--allow-host-ports. - Add IPv4-format validation for dynamically resolved gateway IPs before using them in iptables.
- Mount the generated
chroot-hostsfile as read-only and update the associated unit test.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
src/docker-manager.ts |
Changes chroot /etc/hosts mount to read-only when using generated chroot-hosts. |
src/docker-manager.test.ts |
Updates test expectation to match the new :ro mount. |
containers/agent/setup-iptables.sh |
Adds IPv4 validation and restricts gateway bypass ACCEPT rules to allowed ports only. |
Comments suppressed due to low confidence (1)
containers/agent/setup-iptables.sh:151
- AWF_ALLOW_HOST_PORTS supports range syntax like "3000-3010" (per CLI/docs), but these ACCEPT rules pass port_spec directly to iptables --dport. iptables’ TCP/UDP --dport range syntax is typically START:END, so a hyphenated range may error and abort setup-iptables.sh (set -e). Consider normalizing "START-END" to "START:END" (and handling single ports separately) before invoking iptables.
port_spec=$(echo "$port_spec" | xargs)
echo "[iptables] Allow host gateway port $port_spec"
iptables -A OUTPUT -p tcp -d "$HOST_GATEWAY_IP" --dport "$port_spec" -j ACCEPT
done
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/docker-manager.ts
Outdated
| } | ||
| fs.chmodSync(chrootHostsPath, 0o644); | ||
| agentVolumes.push(`${chrootHostsPath}:/host/etc/hosts`); | ||
| agentVolumes.push(`${chrootHostsPath}:/host/etc/hosts:ro`); |
There was a problem hiding this comment.
Mounting chroot-hosts as read-only can break host.docker.internal injection fallback: entrypoint.sh appends the container’s host.docker.internal line into /host/etc/hosts when AWF_ENABLE_HOST_ACCESS=1 and the entry is missing. With a :ro mount, that append will fail if docker-manager couldn’t resolve/append the gateway IP (the try/catch allows continuing), potentially leaving chroot without host.docker.internal resolution. Consider keeping this mount writable, or make docker-manager injection guaranteed and update entrypoint logic accordingly.
| agentVolumes.push(`${chrootHostsPath}:/host/etc/hosts:ro`); | |
| agentVolumes.push(`${chrootHostsPath}:/host/etc/hosts:rw`); |
| # Function to validate an IPv4 address format (e.g., 172.17.0.1) | ||
| is_valid_ipv4() { | ||
| local ip="$1" | ||
| echo "$ip" | grep -qE '^([0-9]{1,3}\.){3}[0-9]{1,3}$' |
There was a problem hiding this comment.
is_valid_ipv4 only checks a dotted-quad format, so values like 999.999.999.999 will pass and then cause iptables to error (and the script runs with set -e). It should validate each octet is 0–255 (and ideally reject 0.0.0.0/broadcast) to avoid failing open/DoS when name resolution returns unexpected output.
| echo "$ip" | grep -qE '^([0-9]{1,3}\.){3}[0-9]{1,3}$' | |
| # Split into octets | |
| local IFS='.' | |
| read -r -a octets <<< "$ip" | |
| # Must have exactly 4 octets | |
| if [ "${#octets[@]}" -ne 4 ]; then | |
| return 1 | |
| fi | |
| # Validate each octet is numeric and in range 0–255 | |
| local octet | |
| for octet in "${octets[@]}"; do | |
| # Numeric only | |
| if ! [[ "$octet" =~ ^[0-9]+$ ]]; then | |
| return 1 | |
| fi | |
| # Range check; use 10#$octet to avoid octal interpretation | |
| if (( 10#$octet < 0 || 10#$octet > 255 )); then | |
| return 1 | |
| fi | |
| done | |
| # Reject unspecified and all-hosts broadcast addresses | |
| if [[ "$ip" == "0.0.0.0" || "$ip" == "255.255.255.255" ]]; then | |
| return 1 | |
| fi | |
| return 0 |
| if [ -n "$AWF_ALLOW_HOST_PORTS" ]; then | ||
| IFS=',' read -ra HOST_PORTS <<< "$AWF_ALLOW_HOST_PORTS" | ||
| for port_spec in "${HOST_PORTS[@]}"; do | ||
| port_spec=$(echo "$port_spec" | xargs) | ||
| echo "[iptables] Allow host gateway port $port_spec" | ||
| iptables -A OUTPUT -p tcp -d "$HOST_GATEWAY_IP" --dport "$port_spec" -j ACCEPT | ||
| done |
There was a problem hiding this comment.
The gateway-bypass ACCEPT rules use AWF_ALLOW_HOST_PORTS values without validating range/format or excluding DANGEROUS_PORTS. Because AWF_ALLOW_HOST_PORTS can be user-controlled (e.g., via env override) this can re-open blocked ports on the host gateway/network gateway (e.g., 5432/6379) and bypass Squid + the dangerous-ports protection. Recommend validating/sanitizing port specs here (numeric/range within 1–65535) and explicitly rejecting any port(s) in the DANGEROUS_PORTS list before adding ACCEPT rules.
This issue also appears on line 148 of the same file.
Build Test: Java - Environment Issue ❌Status: FAILED - Environment Configuration Error ProblemThe test could not run because Java/Maven are not functional in the current AWF chroot environment. Technical Details
Root Cause: The AWF chroot environment has Environment detected:
Required ActionsOne of the following solutions is needed:
Test Projects Status
Overall: BLOCKED - Environment issue prevents testing
|
Build Test: Rust - Results
Overall: PASS ✅ All Rust projects built and tested successfully.
|
The --enable-host-access flag added an iptables ACCEPT rule for host.docker.internal with no port restriction, allowing agent code to reach ANY service on the host (databases, admin panels, etc.) and bypassing the dangerous-ports blocklist entirely. Changes: - Restrict host gateway FILTER ACCEPT to ports 80, 443, and any ports from --allow-host-ports (was: all ports) - Apply same port restriction to network gateway bypass - Add IPv4 format validation for dynamically resolved IPs before using them in iptables rules - Mount chroot-hosts as read-only (:ro) since host.docker.internal is pre-injected by docker-manager.ts before mounting The NAT RETURN rule (which prevents DNAT to Squid) is unchanged, so MCP traffic still bypasses Squid correctly. Non-allowed port traffic hits the final DROP rule in the FILTER chain. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Mossaka
force-pushed
the
fix/host-gateway-port-restriction
branch
from
d0fea8c to
12683ac
Compare
|
Chroot tests passed! Smoke Chroot - All security and functionality tests succeeded. |
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
|
🎬 THE END — Smoke Claude MISSION: ACCOMPLISHED! The hero saves the day! ✨ |
Build Test: Java - Results
Overall: FAIL Error DetailsBoth projects failed during Maven dependency resolution with SSL/network errors: Root Cause: Maven Central repository ( Action Required: Add
|
Go Build Test Results
Overall: PASS ✅ All Go projects successfully downloaded dependencies and passed tests.
|
C++ Build Test Results
Overall: PASS ✅ All C++ projects built successfully.
|
Rust Build Test Results
Overall: PASS ✅ All Rust projects built successfully and tests passed.
|
|
fix: ensure .copilot/logs exists before docker mount
|
Chroot Version Comparison Test Results ❌The chroot mode tests have completed. Here are the runtime version comparisons between the host system and chroot environment:
Test Status: FAILED The chroot environment is using different versions of Python and Node.js compared to the host system. Only Go versions match between host and chroot.
|
✅ Java Build Test ResultsAll Java projects compiled and tested successfully through the AWF firewall!
Overall: PASS 🎉 All Maven dependencies were successfully downloaded through the Squid proxy (172.30.0.10:3128) and all tests passed.
|
Build complete chroot-hosts content in memory, then write atomically to a securely-created temp directory (fs.mkdtempSync). This satisfies CodeQL's js/insecure-temporary-file rule by using the recognized sanitizer for temp file creation. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Mossaka
force-pushed
the
fix/host-gateway-port-restriction
branch
from
45cde65 to
c375743
Compare
…-restriction-v2 # Conflicts: # .github/workflows/build-test-java.md
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
|
🎬 THE END — Smoke Claude MISSION: ACCOMPLISHED! The hero saves the day! ✨ |
|
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. 🌟 |
Go Build Test ResultsAll Go projects tested successfully! ✅
Overall: PASS
|
Build Test: Node.js - ResultsAll Node.js projects tested successfully!
Overall: PASS ✅
|
Smoke Test Results - ClaudeRecent merged PRs:
Test Results:
Status: PASS
|
🦀 Rust Build Test Results
Overall: PASS ✅ All Rust projects built and tested successfully.
|
C++ Build Test Results
Overall: PASS ✅ All C++ projects built successfully.
|
Deno Build Test Results
Overall: ✅ PASS All Deno tests completed successfully.
|
Smoke Test Results✅ GitHub MCP: #572, #643 Status: PASS | cc: @Mossaka
|
|
GitHub MCP merged PRs: ✅ fix: configure Java proxy via JAVA_TOOL_OPTIONS for Maven builds | fix: ensure .copilot/logs exists before docker mount
|
Chroot Version Comparison Test Results
Overall: ❌ Not all versions match (1/3 passed) The chroot mode successfully accessed the host Go installation, but Python and Node.js versions differ between the host and chroot environments.
|
✅ Build Test: Java - PASSEDAll Java projects compiled and tested successfully through the AWF firewall.
Overall: PASS Configuration
|
.NET Build Test Results
Overall: PASS ✅ All .NET projects built and ran successfully.
|
Summary
--allow-host-portsonly (was: all ports):ro) since entry is pre-injected before mountSecurity context
When
--enable-host-accessis set,setup-iptables.shadds aniptables -A OUTPUT -d $HOST_GATEWAY_IP -j ACCEPTrule with no--dportrestriction. This allows agent code to reach any service on the host machine — including databases (port 5432/3306/6379), admin panels, and other MCP servers — completely bypassing domain filtering and the dangerous-ports blocklist, with zero Squid logging.Before (all ports open)
After (restricted to allowed ports)
The NAT
RETURNrule is unchanged — MCP traffic still bypasses Squid (which crashes on Streamable HTTP/SSE). Non-allowed port traffic now hits the finalDROPrule in the FILTER chain.Smoke test compatibility
--allow-domains localhostwhich auto-configures--allow-host-ports 3000,3001,...,9090— all gateway ports are in the ACCEPT list--allow-host-portswith common dev ports, so no change neededLocal verification
Test plan
npm test— 735/735 pass (updated test expects:romount)npm run lint— pass🤖 Generated with Claude Code