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.
|
Node.js Build Test Results
Overall: PASS ✅ All Node.js projects installed successfully and all tests passed.
|
Deno Build Test Results
Overall: ✅ PASS All Deno tests completed successfully.
|
|
Smoke Test Results ✅ GitHub MCP: #557, #556 Status: PASS ✅ cc: @Mossaka
|
C++ Build Test Results
Overall: PASS ✅ All C++ projects built successfully.
|
Build Test: Bun - Results
Overall: PASS ✅ All Bun build tests completed successfully.
|
|
Smoke Test: Claude Engine ✅ GitHub MCP - Last 2 merged PRs:
✅ Playwright - Page title verified: "GitHub · Change is constant. GitHub keeps you ahead. · GitHub" ✅ File Writing - Created ✅ Bash Tool - File read successful Status: PASS
|
Chroot Version Comparison Test Results
Overall Status: Tests did not pass - only Go versions matched. The chroot environment successfully accessed host binaries but version mismatches were detected for Python and Node.js, likely due to different installation paths or environment configurations being used.
|
|
✨ The prophecy is fulfilled... Smoke Codex has completed its mystical journey. The stars align. 🌟 |
Move Maven proxy configuration to the workflow markdown (settings.xml created at runtime using SQUID_PROXY_HOST/SQUID_PROXY_PORT env vars) rather than generating it in docker-manager.ts. Add Java/Maven/Gradle troubleshooting section to docs and JAVA_TOOL_OPTIONS documentation to CLAUDE.md. Recompile build-test-java workflow. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…-restriction-merge # Conflicts: # containers/agent/setup-iptables.sh
|
📰 DEVELOPING STORY: Smoke Copilot reports failed. Our correspondents are investigating the incident... |
|
💫 TO BE CONTINUED... Smoke Claude failed! Our hero faces unexpected challenges... |
|
Chroot tests failed Smoke Chroot failed - See logs for details. |
|
🌑 The shadows whisper... Smoke Codex failed. The oracle requires further meditation... |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| try { | ||
| fs.copyFileSync('/etc/hosts', chrootHostsPath); | ||
| } catch { | ||
| fs.writeFileSync(chrootHostsPath, '127.0.0.1 localhost\n'); |
Check failure
Code scanning / CodeQL
Insecure temporary file High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 9 hours ago
In general, to fix insecure temporary file issues, you must avoid constructing file paths in shared/writable directories and then blindly creating them. Instead, either (a) use a secure temp‑file/dir library that atomically creates private files/directories with correct permissions, or (b) ensure that the directory is not world‑writable and is created by your process with safe permissions.
Here, the problem is not the specific file name chroot-hosts, but that it lives under a user‑controlled config.workDir, which in tests is /tmp/awf-test and might be a temp or shared directory in real use. The minimal, non‑breaking fix is to ensure that config.workDir exists with safe permissions before using it, and to treat the “chroot-hosts” file as a normal config file in that controlled directory. We can call fs.mkdirSync(config.workDir, { recursive: true }) and then explicitly set the directory mode to 0o700 (owner‑only), which mitigates the temp‑dir attack: even if config.workDir is under /tmp, the subdirectory will have restrictive permissions owned by the current user. We then proceed to create chroot-hosts as before.
Concretely in src/docker-manager.ts, inside generateDockerCompose where chrootHostsPath is created, insert code just before const chrootHostsPath = path.join(config.workDir, 'chroot-hosts'); to create config.workDir securely and tighten its permissions if possible. We do not need new imports; fs and path are already imported. The tests already create/remove /tmp/awf-test, but that is fine: our code will now also ensure the directory exists with safe mode, which remains compatible with tests.
| @@ -500,6 +500,17 @@ | ||
| // 1. Pre-resolve allowed domains using the host's DNS stack (supports Tailscale MagicDNS, | ||
| // split DNS, and other custom resolvers not available inside the container) | ||
| // 2. Inject host.docker.internal when --enable-host-access is set | ||
| // Ensure workDir exists with restrictive permissions so that chroot-hosts is not created | ||
| // in a world-writable/shared directory without protection. | ||
| try { | ||
| fs.mkdirSync(config.workDir, { recursive: true }); | ||
| // Best-effort: tighten permissions to 0700 (owner-only) if possible. | ||
| fs.chmodSync(config.workDir, 0o700); | ||
| } catch { | ||
| // If we cannot safely create the directory, we still attempt to proceed; | ||
| // subsequent filesystem operations may fail and be handled by callers. | ||
| } | ||
|
|
||
| const chrootHostsPath = path.join(config.workDir, 'chroot-hosts'); | ||
| try { | ||
| fs.copyFileSync('/etc/hosts', chrootHostsPath); |
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
📰 DEVELOPING STORY: Smoke Copilot reports failed. Our correspondents are investigating the incident... |
|
💫 TO BE CONTINUED... Smoke Claude failed! Our hero faces unexpected challenges... |
|
Chroot tests failed Smoke Chroot failed - See logs for details. |
|
🌑 The shadows whisper... Smoke Codex failed. The oracle requires further meditation... |
1 participant
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