fix: enforce shared net namespace for child containers#1721
fix: enforce shared net namespace for child containers#1721
Conversation
When --enable-dind is active, child containers spawned by the agent get their own network namespace and don't inherit NAT rules, allowing proxy bypass. This fix converts docker-stub.sh into a dual-mode wrapper that intercepts docker run/create commands and injects --network container:awf-agent, forcing child containers to share the agent's network namespace where all iptables rules apply. 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 addresses a firewall bypass when Docker-in-Docker (--enable-dind) is enabled by attempting to force all agent-spawned child containers to share the agent container’s network namespace so the agent’s NAT-to-Squid rules apply uniformly.
Changes:
- Add DinD-specific environment variables to the agent service so the Docker stub can switch into “wrapper” mode.
- Install and activate a Docker command wrapper in chroot mode that rewrites
docker run/createto use--network container:<agent>and blocks certain network escape commands. - Add unit tests asserting DinD env var presence/absence in the generated compose config.
Show a summary per file
| File | Description |
|---|---|
src/docker-manager.ts |
Sets AWF_DIND_ENABLED / AWF_AGENT_CONTAINER when DinD is enabled. |
src/docker-manager.test.ts |
Adds tests for the new DinD environment variables. |
containers/agent/entrypoint.sh |
Copies wrapper into chroot-accessible path and prepends it to PATH in the generated chroot script. |
containers/agent/docker-stub.sh |
Converts Docker stub into a dual-mode wrapper that rewrites docker run/create and blocks some commands when DinD is enabled. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 4/4 changed files
- Comments generated: 5
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 "$@" | ||
| ;; | ||
|
|
||
| "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' | ||
|
|
There was a problem hiding this comment.
get_subcommand() treats the first non-dash token as the Docker subcommand, but Docker global options often take values (e.g., --context foo, -H unix:///...). In those cases the value (e.g. foo) will be mis-identified as the subcommand, and the later shift/CMD="$1" logic will rewrite or forward arguments incorrectly, breaking docker run/create/network handling and potentially skipping the network enforcement. Parse global options (including options-with-values) to find the actual COMMAND position, preserve global opts separately, and only then apply the run/create/network-specific rewriting.
containers/agent/docker-stub.sh
Outdated
| exit 127 | ||
| fi | ||
|
|
||
| AGENT_CONTAINER="${AWF_AGENT_CONTAINER:-awf-agent}" |
There was a problem hiding this comment.
AWF_AGENT_CONTAINER is taken directly from the environment, which means user code can set it to any existing container name on the host and then run docker run to join that container’s network namespace (potentially one without AWF NAT/proxy rules). To prevent this bypass, don’t accept an arbitrary container name from the user environment—hardcode the expected agent container name or validate that the value matches the known agent container (and fail closed otherwise).
| AGENT_CONTAINER="${AWF_AGENT_CONTAINER:-awf-agent}" | |
| EXPECTED_AGENT_CONTAINER="awf-agent" | |
| if [ -n "${AWF_AGENT_CONTAINER:-}" ] && [ "${AWF_AGENT_CONTAINER}" != "${EXPECTED_AGENT_CONTAINER}" ]; then | |
| echo "ERROR: AWF_AGENT_CONTAINER must be '${EXPECTED_AGENT_CONTAINER}' when DinD is enabled." >&2 | |
| exit 1 | |
| fi | |
| AGENT_CONTAINER="${EXPECTED_AGENT_CONTAINER}" |
containers/agent/entrypoint.sh
Outdated
| # SECURITY: When DinD is enabled, prepend /tmp/awf-lib to PATH so the Docker wrapper | ||
| # is found before the real docker binary. Also export AWF env vars needed by the wrapper. | ||
| if [ "$AWF_DOCKER_WRAPPER_INSTALLED" = "true" ]; then | ||
| echo "# AWF Docker wrapper: enforce shared network namespace for child containers" >> "/host${SCRIPT_FILE}" | ||
| echo "export PATH=\"/tmp/awf-lib:\$PATH\"" >> "/host${SCRIPT_FILE}" | ||
| echo "export AWF_REAL_DOCKER=\"${AWF_REAL_DOCKER}\"" >> "/host${SCRIPT_FILE}" |
There was a problem hiding this comment.
Only prepending /tmp/awf-lib to PATH doesn’t prevent bypass: user code can invoke the real Docker binary by absolute path, and exporting AWF_REAL_DOCKER into the user environment makes that path trivial to discover. If the goal is to enforce shared netns, consider bind-mounting the wrapper over the real docker path inside /host (so /usr/bin/docker in the chroot is the wrapper), and keep the real binary at a separate non-user-controlled path used only by the wrapper.
| # SECURITY: When DinD is enabled, prepend /tmp/awf-lib to PATH so the Docker wrapper | |
| # is found before the real docker binary. Also export AWF env vars needed by the wrapper. | |
| if [ "$AWF_DOCKER_WRAPPER_INSTALLED" = "true" ]; then | |
| echo "# AWF Docker wrapper: enforce shared network namespace for child containers" >> "/host${SCRIPT_FILE}" | |
| echo "export PATH=\"/tmp/awf-lib:\$PATH\"" >> "/host${SCRIPT_FILE}" | |
| echo "export AWF_REAL_DOCKER=\"${AWF_REAL_DOCKER}\"" >> "/host${SCRIPT_FILE}" | |
| # SECURITY: When DinD is enabled, enforce the Docker wrapper by bind-mounting it | |
| # over the real Docker binary path inside /host. This prevents bypass via absolute | |
| # paths and avoids exposing the real Docker path in the user-visible environment. | |
| if [ "$AWF_DOCKER_WRAPPER_INSTALLED" = "true" ]; then | |
| AWF_DOCKER_WRAPPER_PATH="/tmp/awf-lib/docker" | |
| AWF_HOST_DOCKER_PATH="/host${AWF_REAL_DOCKER}" | |
| if [ ! -x "$AWF_DOCKER_WRAPPER_PATH" ]; then | |
| echo "[entrypoint][ERROR] Docker wrapper not found or not executable: $AWF_DOCKER_WRAPPER_PATH" | |
| exit 1 | |
| fi | |
| if [ ! -e "$AWF_HOST_DOCKER_PATH" ]; then | |
| echo "[entrypoint][ERROR] Host Docker binary not found: $AWF_HOST_DOCKER_PATH" | |
| exit 1 | |
| fi | |
| mount --bind "$AWF_DOCKER_WRAPPER_PATH" "$AWF_HOST_DOCKER_PATH" | |
| echo "# AWF Docker wrapper: enforce shared network namespace for child containers" >> "/host${SCRIPT_FILE}" |
containers/agent/entrypoint.sh
Outdated
| # Make AWF_DIND_ENABLED readonly to prevent tampering by user code | ||
| readonly AWF_DIND_ENABLED |
There was a problem hiding this comment.
readonly AWF_DIND_ENABLED here only affects the entrypoint shell; it doesn’t make the variable immutable for user code running later inside the chroot (which can still export AWF_DIND_ENABLED=...). If you intend tamper resistance, enforce it in the docker wrapper itself (e.g., ignore env overrides) or emit readonly in the generated chroot script (though even that can be bypassed by running a different shell).
| # Make AWF_DIND_ENABLED readonly to prevent tampering by user code | |
| readonly AWF_DIND_ENABLED | |
| # Do not rely on AWF_DIND_ENABLED for tamper resistance here: a readonly variable | |
| # only affects this shell. DinD/network enforcement is provided by the installed | |
| # Docker wrapper itself, not by making this environment variable readonly. |
src/docker-manager.ts
Outdated
| // Set DinD environment variables so the docker-stub wrapper enforces | ||
| // shared network namespace for child containers (prevents proxy bypass) | ||
| environment.AWF_DIND_ENABLED = '1'; | ||
| environment.AWF_AGENT_CONTAINER = 'awf-agent'; |
There was a problem hiding this comment.
AWF_AGENT_CONTAINER is set to the hard-coded string 'awf-agent' even though this file already defines AGENT_CONTAINER_NAME = 'awf-agent'. Using the constant here avoids drift if the container name ever changes and keeps compose generation/tests in sync.
| environment.AWF_AGENT_CONTAINER = 'awf-agent'; | |
| environment.AWF_AGENT_CONTAINER = AGENT_CONTAINER_NAME; |
|
Smoke test results — run 24051295776 ✅ GitHub MCP: "⚡ Token optimization for pelis-agent-factory-advisor", "Optimize security-review workflow" Overall: PASS
|
|
Smoke test results:
|
🔥 Smoke Test Results
PR: "fix: enforce shared net namespace for child containers" — @Mossaka Overall: FAIL — smoke-data step outputs not injected into prompt (template variables unresolved).
|
Chroot Version Comparison Results
Overall: ❌ Not all versions match — Python and Node.js versions differ between host and chroot environments.
|
Smoke Test: GitHub Actions Services Connectivity
All 3 checks failed.
|
Security Review — PR #1721The direction here is correct (child containers must share the agent's network namespace to inherit NAT rules). However, the implementation exposes two high-severity bypass paths that should be fixed before merge. 🔴 HIGH —
|
| # | Severity | Issue | Needs fix before merge? |
|---|---|---|---|
| 1 | 🔴 HIGH | AWF_REAL_DOCKER in user env = wrapper bypass |
Yes |
| 2 | 🔴 HIGH | AWF_AGENT_CONTAINER mutable → squid namespace takeover |
Yes |
| 3 | 🟡 MEDIUM | docker build bypasses namespace enforcement |
Yes (or documented) |
The underlying approach — shared network namespace via --network container:awf-agent — is sound. The fixes needed are: bake AWF_REAL_DOCKER and AWF_AGENT_CONTAINER into the wrapper script at install time rather than passing them as runtime env vars, and block docker build.
Generated by Security Guard for issue #1721 · ● 108.7K · ◷
- Fix get_subcommand() parsing: match against known Docker subcommands instead of assuming first non-flag token is the subcommand, preventing misidentification of global option values (e.g., --context foo) - Hardcode agent container name 'awf-agent' in docker-stub.sh instead of reading from AWF_AGENT_CONTAINER env var to prevent namespace hijacking - Stop exporting AWF_REAL_DOCKER to user environment; write the real Docker path to /tmp/awf-lib/.docker-path file that only the wrapper reads - Add comment acknowledging that readonly AWF_DIND_ENABLED only protects the entrypoint shell, not subshells — real enforcement is the wrapper - Use AGENT_CONTAINER_NAME constant instead of string literal in docker-manager.ts - Block docker build/buildx commands to prevent BuildKit containers from bypassing NAT rules with unrestricted network access Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Review feedback addressed in f18e2a0All 5 Copilot review comments have been addressed, plus one additional security fix: 1.
|
Security Review (Post-Fix)Reviewed all 4 changed files in the context of the existing entrypoint, docker-manager, and container architecture. This is a well-structured fix for a real proxy bypass vulnerability. Below are findings ranked by severity. [MEDIUM] Real Docker binary path readable by user code via
|
Security hardening for the Docker wrapper in DinD mode: 1. Bind-mount wrapper over real Docker binary: The wrapper is now bind-mounted over the original Docker binary path (e.g., /usr/bin/docker) inside /host, so even absolute-path invocations go through the wrapper. Previously, only PATH precedence was used, which could be bypassed. 2. Hide real Docker binary: The real Docker binary is copied to a hidden location (/tmp/awf-lib/.docker-real) and the path is hardcoded in the wrapper script. No environment variables or world-readable config files expose the real binary location. 3. Remove AWF_AGENT_CONTAINER env var: The agent container name is now hardcoded in the wrapper script only. Previously it was passed as an env var that user code could override to join arbitrary container namespaces (e.g., awf-squid which has unrestricted outbound access). 4. Fix global options parsing: Added split_at_subcommand() to properly separate Docker global options from subcommand arguments, fixing cases like `docker --context foo run` where global option values were misidentified. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Additional security hardening in e8162c8The previous fix (f18e2a0) addressed the three security findings by storing the real Docker path in a file instead of an env var, hardcoding the agent container name, and blocking 1. Bind-mount wrapper over real Docker binary (strongest fix)The wrapper is now bind-mounted over the original Docker binary path inside 2. Hidden real Docker binary with hardcoded pathThe real Docker binary is copied to 3. Removed
|
|
we should |
Summary
Closes #130
--enable-dindis enabled, child containers spawned by the agent get their own network namespace onawf-netbut do NOT inherit NAT rules, allowing proxy bypassdocker-stub.shinto a dual-mode wrapper: blocks Docker when DinD is disabled (existing behavior), interceptsdocker run/docker createwhen DinD is enabled--network/--netflags and injects--network container:awf-agentto force child containers to share the agent's network namespacedocker network connectanddocker composeto prevent network escapesentrypoint.shinstalls the wrapper at/tmp/awf-lib/dockerand prepends it to PATH in the chroot script, so it intercepts docker commands before the real binaryFiles changed
containers/agent/docker-stub.sh— converted from error-only stub to network-enforcing wrappersrc/docker-manager.ts— setsAWF_DIND_ENABLED=1andAWF_AGENT_CONTAINER=awf-agentwhen DinD enabledcontainers/agent/entrypoint.sh— installs wrapper in chroot, exports env vars, adds to PATHsrc/docker-manager.test.ts— 2 new tests for DinD env var presence/absenceTest plan
npm run lintpasses (0 errors)npm testpasses (1313 tests, +2 new)sudo awf --enable-dind --allow-domains github.com 'docker run --rm alpine wget -q -O- https://evil.com'should failsudo awf --enable-dind --allow-domains github.com 'docker run --network host --rm alpine wget -q -O- https://evil.com'should strip--network hostand use agent namespacesudo awf --allow-domains github.com 'docker ps'should still show DinD-disabled error🤖 Generated with Claude Code