feat: propagate host.docker.internal to child containers (#422)#1722
feat: propagate host.docker.internal to child containers (#422)#1722
Conversation
When --enable-host-access is used, the agent container gets host.docker.internal DNS via Docker's extra_hosts. However, child containers spawned via docker run don't inherit this. This change: - Makes AWF_ENABLE_HOST_ACCESS readonly in entrypoint.sh to prevent tampering - Updates docker-stub.sh to a dual-mode wrapper (from PR #130) that injects --add-host host.docker.internal:host-gateway into docker run/create when AWF_ENABLE_HOST_ACCESS=1 - Adds tests verifying AWF_ENABLE_HOST_ACCESS propagation to iptables-init Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
✅ Coverage Check PassedOverall Coverage
📁 Per-file Coverage Changes (1 files)
Coverage comparison generated by |
There was a problem hiding this comment.
Pull request overview
This PR aims to ensure host.docker.internal resolution is propagated to containers spawned from inside the agent container when host access is enabled, while also hardening the host-access enablement flag against tampering.
Changes:
- Makes
AWF_ENABLE_HOST_ACCESSreadonly in the agent entrypoint. - Extends
docker-stub.shinto a dual-mode interceptor that rewritesdocker run/createto share the agent network namespace and (when enabled) injects--add-host host.docker.internal:host-gateway. - Adds unit tests asserting
AWF_ENABLE_HOST_ACCESSis passed through to theiptables-initservice environment.
Show a summary per file
| File | Description |
|---|---|
src/docker-manager.test.ts |
Adds coverage for AWF_ENABLE_HOST_ACCESS propagation into the iptables-init compose service environment. |
containers/agent/entrypoint.sh |
Locks AWF_ENABLE_HOST_ACCESS as readonly early in startup. |
containers/agent/docker-stub.sh |
Replaces the simple stub with an interceptor that can rewrite docker run/create and optionally inject host mapping. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comments suppressed due to low confidence (1)
containers/agent/docker-stub.sh:71
- The interceptor only matches top-level
docker run/docker create, but Docker also supportsdocker container runanddocker container create. Those forms will currently fall through to the passthrough path, which is a straightforward bypass for the shared-network enforcement (and any--networkstripping). Consider handling thecontainersubcommand and applying the same rewrite logic when its subcommand isrun/create.
"run"|"create")
# Intercept 'docker run' and 'docker create' to enforce shared network namespace
# This ensures child containers use the agent's NAT rules (traffic -> Squid proxy)
CMD="$1"
shift # remove 'run' or 'create'
- Files reviewed: 3/3 changed files
- Comments generated: 3
containers/agent/docker-stub.sh
Outdated
| # Get the subcommand (first non-flag argument) | ||
| get_subcommand() { | ||
| for arg in "$@"; do | ||
| case "$arg" in | ||
| -*) continue ;; | ||
| *) echo "$arg"; return ;; | ||
| esac | ||
| done | ||
| } | ||
|
|
||
| SUBCOMMAND=$(get_subcommand "$@") | ||
|
|
||
| # Block commands that could attach containers to other networks | ||
| case "$SUBCOMMAND" in | ||
| "network") | ||
| # Check for 'docker network connect' which could bypass firewall | ||
| # Allow 'docker network ls', 'docker network inspect', etc. | ||
| shift # remove 'network' | ||
| NETWORK_SUBCMD=$(get_subcommand "$@") | ||
| if [ "$NETWORK_SUBCMD" = "connect" ]; then | ||
| echo "ERROR: 'docker network connect' is blocked by AWF firewall." >&2 | ||
| echo "Child containers must share the agent's network namespace for security." >&2 | ||
| exit 1 | ||
| fi | ||
| exec "$REAL_DOCKER" network "$@" |
There was a problem hiding this comment.
The get_subcommand() parser treats any non-flag token as the subcommand, which breaks when Docker global options take a value (e.g. --context foo run ..., --config dir ps, -H tcp://... info). In those cases it will mis-detect the subcommand and skip the enforcement logic, allowing run/create to pass through unmodified. Consider explicitly parsing Docker global flags (including those that consume the next arg) before determining the subcommand, or using a more robust subcommand detection approach.
This issue also appears on line 67 of the same file.
| # Get the subcommand (first non-flag argument) | |
| get_subcommand() { | |
| for arg in "$@"; do | |
| case "$arg" in | |
| -*) continue ;; | |
| *) echo "$arg"; return ;; | |
| esac | |
| done | |
| } | |
| SUBCOMMAND=$(get_subcommand "$@") | |
| # Block commands that could attach containers to other networks | |
| case "$SUBCOMMAND" in | |
| "network") | |
| # Check for 'docker network connect' which could bypass firewall | |
| # Allow 'docker network ls', 'docker network inspect', etc. | |
| shift # remove 'network' | |
| NETWORK_SUBCMD=$(get_subcommand "$@") | |
| if [ "$NETWORK_SUBCMD" = "connect" ]; then | |
| echo "ERROR: 'docker network connect' is blocked by AWF firewall." >&2 | |
| echo "Child containers must share the agent's network namespace for security." >&2 | |
| exit 1 | |
| fi | |
| exec "$REAL_DOCKER" network "$@" | |
| # Get the Docker subcommand by skipping global flags, including those that | |
| # consume the following argument. | |
| docker_global_option_takes_value() { | |
| case "$1" in | |
| --config|\ | |
| -c|\ | |
| --context|\ | |
| -H|\ | |
| --host|\ | |
| -l|\ | |
| --log-level|\ | |
| --tlscacert|\ | |
| --tlscert|\ | |
| --tlskey) | |
| return 0 | |
| ;; | |
| *) | |
| return 1 | |
| ;; | |
| esac | |
| } | |
| find_subcommand_index() { | |
| local args=("$@") | |
| local i=0 | |
| local arg | |
| while [ "$i" -lt "${#args[@]}" ]; do | |
| arg="${args[$i]}" | |
| case "$arg" in | |
| --) | |
| i=$((i + 1)) | |
| break | |
| ;; | |
| --*=*) | |
| ;; | |
| -*) | |
| if docker_global_option_takes_value "$arg"; then | |
| i=$((i + 1)) | |
| fi | |
| ;; | |
| *) | |
| echo "$i" | |
| return 0 | |
| ;; | |
| esac | |
| i=$((i + 1)) | |
| done | |
| return 1 | |
| } | |
| ARGS=("$@") | |
| SUBCOMMAND_INDEX="" | |
| if SUBCOMMAND_INDEX=$(find_subcommand_index "${ARGS[@]}"); then | |
| SUBCOMMAND="${ARGS[$SUBCOMMAND_INDEX]}" | |
| else | |
| SUBCOMMAND="" | |
| fi | |
| # Block commands that could attach containers to other networks | |
| case "$SUBCOMMAND" in | |
| "network") | |
| # Check for 'docker network connect' which could bypass firewall | |
| # Allow 'docker network ls', 'docker network inspect', etc. | |
| NETWORK_ARGS=("${ARGS[@]:$((SUBCOMMAND_INDEX + 1))}") | |
| NETWORK_SUBCMD="" | |
| if NETWORK_SUBCMD_INDEX=$(find_subcommand_index "${NETWORK_ARGS[@]}"); then | |
| NETWORK_SUBCMD="${NETWORK_ARGS[$NETWORK_SUBCMD_INDEX]}" | |
| fi | |
| if [ "$NETWORK_SUBCMD" = "connect" ]; then | |
| echo "ERROR: 'docker network connect' is blocked by AWF firewall." >&2 | |
| echo "Child containers must share the agent's network namespace for security." >&2 | |
| exit 1 | |
| fi | |
| exec "$REAL_DOCKER" "${ARGS[@]}" |
| # Propagate host.docker.internal DNS to child containers when host access is enabled. | ||
| # The agent container gets this via Docker's extra_hosts in docker-compose.yml, | ||
| # but child containers spawned via 'docker run' don't inherit it automatically. | ||
| if [ "${AWF_ENABLE_HOST_ACCESS:-}" = "1" ]; then |
There was a problem hiding this comment.
This check only injects the --add-host host.docker.internal:host-gateway mapping when AWF_ENABLE_HOST_ACCESS is exactly "1". Elsewhere (e.g. setup-iptables.sh) host access is treated as enabled when the value is merely non-empty, and docker-manager has a code path that sets it to "true" as a safety net. To avoid inconsistent behavior, consider treating any non-empty value as enabled here (or normalize the env var to a single canonical value everywhere).
| if [ "${AWF_ENABLE_HOST_ACCESS:-}" = "1" ]; then | |
| if [ -n "${AWF_ENABLE_HOST_ACCESS:-}" ]; then |
| # SECURITY: Lock down AWF control variables to prevent tampering by user code. | ||
| # These are set by the Docker Compose environment and must not be modified. | ||
| readonly AWF_ENABLE_HOST_ACCESS="${AWF_ENABLE_HOST_ACCESS:-}" | ||
| export AWF_ENABLE_HOST_ACCESS |
There was a problem hiding this comment.
readonly only protects the variable in the current shell instance; the attribute is not inherited by new shells. User code can still run a subshell (e.g. bash -c 'AWF_ENABLE_HOST_ACCESS=1 ...') and override the exported value for child processes. If this variable is used as a security boundary, consider an enforcement mechanism that can’t be overridden by spawning a new shell (e.g. derive host-access enablement from immutable container config or a root-owned file, rather than an exported env var).
| # SECURITY: Lock down AWF control variables to prevent tampering by user code. | |
| # These are set by the Docker Compose environment and must not be modified. | |
| readonly AWF_ENABLE_HOST_ACCESS="${AWF_ENABLE_HOST_ACCESS:-}" | |
| export AWF_ENABLE_HOST_ACCESS | |
| # SECURITY: Do not use an exported environment variable as the enforcement source | |
| # for host-access enablement. A child shell can override exported variables even | |
| # if they were marked readonly in this shell. Persist the value in a root-owned | |
| # runtime file instead and expose only the file path to child processes. | |
| AWF_RUNTIME_DIR="/run/awf" | |
| AWF_ENABLE_HOST_ACCESS_FILE="$AWF_RUNTIME_DIR/enable_host_access" | |
| mkdir -p "$AWF_RUNTIME_DIR" | |
| chmod 0755 "$AWF_RUNTIME_DIR" | |
| printf '%s\n' "${AWF_ENABLE_HOST_ACCESS:-}" > "$AWF_ENABLE_HOST_ACCESS_FILE" | |
| chmod 0644 "$AWF_ENABLE_HOST_ACCESS_FILE" | |
| readonly AWF_RUNTIME_DIR | |
| readonly AWF_ENABLE_HOST_ACCESS_FILE | |
| export AWF_ENABLE_HOST_ACCESS_FILE | |
| unset AWF_ENABLE_HOST_ACCESS | |
| readonly AWF_ENABLE_HOST_ACCESS="" |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Security ReviewI reviewed the changes in this PR with a focus on security implications. The core intent — enforcing shared network namespace for child containers — is correct, and the 🔴 High:
|
| Severity | Issue |
|---|---|
| 🔴 High | --privileged / NET_ADMIN not stripped — privileged child container can flush NAT rules |
| 🔴 High | get_subcommand misidentifies subcommand when Docker global flags take values (-H, --host, etc.) |
| 🟡 Medium | docker exec unrestricted — can target awf-squid to bypass domain filtering |
| 🟡 Medium | AWF_REAL_DOCKER / AWF_AGENT_CONTAINER not readonly — can be overridden by agent code |
The two High issues can allow complete bypass of AWF's network filtering when DinD is enabled and should be addressed before this merges.
Generated by Security Guard for issue #1722 · ● 93.1K · ◷
🏗️ Build Test Suite Results
Overall: 8/8 ecosystems passed — ✅ PASS
|
This comment has been minimized.
This comment has been minimized.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Review feedback addressedPushed commit 9226069 to address all 3 Copilot review comments: 1.
|
Smoke Test Results
Overall: PASS
|
Smoke Test Results
PR: feat: propagate host.docker.internal to child containers (#422) Overall: PASS (2/2 verifiable tests passed; file test skipped — smoke-data outputs not expanded)
|
Chroot Version Comparison Results
Overall: ❌ Not all tests passed — Python and Node.js versions differ between host and chroot environments.
|
|
Smoke test results:
|
Security ReviewI found three security issues beyond what's already flagged in review comments. The existing Copilot review correctly identified the 1.
|
Smoke Test: GitHub Actions Services Connectivity
Label not applied (not all checks passed).
|
Summary
AWF_ENABLE_HOST_ACCESSreadonly inentrypoint.shto prevent user code from tampering with the host access flagdocker-stub.shfrom a simple error stub to a dual-mode wrapper (incorporating the approach from PR [Security] Child containers don't inherit NAT rules - proxy bypass possible #130) that injects--add-host host.docker.internal:host-gatewayintodocker run/docker createcommands whenAWF_ENABLE_HOST_ACCESS=1AWF_ENABLE_HOST_ACCESSpropagation to the iptables-init containerCloses #422
Context
When
--enable-host-accessis used, the agent container getshost.docker.internalDNS resolution via Docker'sextra_hosts. But child containers spawned viadocker runinside the agent don't inherit this mapping. The docker-stub.sh wrapper now detectsAWF_ENABLE_HOST_ACCESS=1and injects the--add-hostflag automatically.Note: The docker-stub.sh changes include the dual-mode wrapper from #130 (shared network namespace enforcement) as a foundation. If PR #130 lands first, there will be a merge conflict to resolve in this file.
Test plan
npm test)npm run lint)readonly AWF_ENABLE_HOST_ACCESSprevents reassignment in entrypoint--enable-host-accessand spawn a child container, verifyhost.docker.internalresolves🤖 Generated with Claude Code