fix: address review feedback on child container NAT enforcement

Mossaka · claude · Mossaka · commit f18e2a097547 · 2026-04-06T21:33:42.000Z
- 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) &lt;noreply@anthropic.com&gt;
diff --git a/containers/agent/docker-stub.sh b/containers/agent/docker-stub.sh
@@ -29,20 +29,46 @@ fi
 
 # --- DinD enabled: enforce shared network namespace ---
 
-REAL_DOCKER="${AWF_REAL_DOCKER:-}"
+# SECURITY: Read the real Docker path from a private file rather than an environment
+# variable. Environment variables are visible to (and modifiable by) user code, which
+# could invoke the real Docker binary directly to bypass the wrapper.
+AWF_DOCKER_CONFIG="/tmp/awf-lib/.docker-path"
+if [ ! -f "$AWF_DOCKER_CONFIG" ]; then
+  echo "ERROR: Docker wrapper config not found at $AWF_DOCKER_CONFIG" >&2
+  exit 127
+fi
+REAL_DOCKER=$(cat "$AWF_DOCKER_CONFIG")
 if [ -z "$REAL_DOCKER" ] || [ ! -x "$REAL_DOCKER" ]; then
-  echo "ERROR: AWF_REAL_DOCKER is not set or not executable: '$REAL_DOCKER'" >&2
+  echo "ERROR: Real Docker binary not found or not executable: '$REAL_DOCKER'" >&2
   exit 127
 fi
 
-AGENT_CONTAINER="${AWF_AGENT_CONTAINER:-awf-agent}"
+# SECURITY: Hardcode the agent container name to prevent user code from tampering
+# via the AWF_AGENT_CONTAINER environment variable to join arbitrary container namespaces.
+AGENT_CONTAINER="awf-agent"
+
+# Known Docker subcommands we intercept or pass through.
+# We match against this list rather than assuming the first non-flag token is
+# a subcommand, because Docker global options can take values (e.g., --context foo,
+# -H unix:///...) which would be misidentified as subcommands.
+KNOWN_SUBCOMMANDS="run create exec build buildx pull push network compose images ps logs inspect rm rmi tag stop start restart kill pause unpause wait top stats attach commit cp diff export history import load save port rename update volume system info version events"
 
-# Get the subcommand (first non-flag argument)
 get_subcommand() {
   for arg in "$@"; do
     case "$arg" in
       -*) continue ;;
-      *) echo "$arg"; return ;;
+      *)
+        # Check if this token is a known Docker subcommand
+        for cmd in $KNOWN_SUBCOMMANDS; do
+          if [ "$arg" = "$cmd" ]; then
+            echo "$arg"
+            return
+          fi
+        done
+        # Unknown token — could be a value for a global option (e.g., --context foo),
+        # so skip it and keep looking.
+        continue
+        ;;
     esac
   done
 }
@@ -101,6 +127,25 @@ case "$SUBCOMMAND" in
     exec "$REAL_DOCKER" "$CMD" --network "container:${AGENT_CONTAINER}" "${FILTERED_ARGS[@]}"
     ;;
 
+  "build")
+    # SECURITY: Block 'docker build' because BuildKit intermediate containers get their
+    # own network namespace and are not subject to the agent's NAT/iptables rules.
+    # This means build steps (e.g., RUN curl) could make unrestricted outbound requests,
+    # bypassing the Squid proxy entirely.
+    echo "ERROR: 'docker build' is blocked by AWF firewall." >&2
+    echo "BuildKit containers bypass NAT rules and could make unrestricted network requests." >&2
+    echo "Build images outside the AWF wrapper before invoking awf." >&2
+    exit 1
+    ;;
+
+  "buildx")
+    # SECURITY: Block 'docker buildx' for the same reason as 'docker build' above.
+    echo "ERROR: 'docker buildx' is blocked by AWF firewall." >&2
+    echo "BuildKit containers bypass NAT rules and could make unrestricted network requests." >&2
+    echo "Build images outside the AWF wrapper before invoking awf." >&2
+    exit 1
+    ;;
+
   "compose")
     # For docker compose, we cannot easily rewrite compose files.
     # Block it to prevent spawning services on separate networks.
diff --git a/containers/agent/entrypoint.sh b/containers/agent/entrypoint.sh
@@ -520,7 +520,12 @@ if [ "${AWF_CHROOT_ENABLED}" = "true" ]; then
         # Copy our wrapper to /tmp/awf-lib/docker (writable in chroot)
         if cp /usr/bin/docker /host/tmp/awf-lib/docker 2>/dev/null && \
            chmod +x /host/tmp/awf-lib/docker 2>/dev/null; then
-          export AWF_REAL_DOCKER="$REAL_DOCKER_PATH"
+          # SECURITY: Write the real Docker path to a private file instead of an env var.
+          # This prevents user code from reading AWF_REAL_DOCKER to find and invoke the
+          # real Docker binary directly, bypassing the wrapper.
+          echo "$REAL_DOCKER_PATH" > /host/tmp/awf-lib/.docker-path
+          chmod 444 /host/tmp/awf-lib/.docker-path
+          AWF_REAL_DOCKER="$REAL_DOCKER_PATH"  # local use only, not exported
           AWF_DOCKER_WRAPPER_INSTALLED=true
           echo "[entrypoint] Docker wrapper installed at /tmp/awf-lib/docker"
           echo "[entrypoint] Real docker binary: $REAL_DOCKER_PATH"
@@ -533,7 +538,10 @@ if [ "${AWF_CHROOT_ENABLED}" = "true" ]; then
     else
       echo "[entrypoint][WARN] Could not create /tmp/awf-lib for Docker wrapper"
     fi
-    # Make AWF_DIND_ENABLED readonly to prevent tampering by user code
+    # Make AWF_DIND_ENABLED readonly in this shell. Note: this does NOT propagate to
+    # subshells or user code — shell readonly is per-process. The real security enforcement
+    # is the Docker wrapper (docker-stub.sh) intercepting all docker commands via PATH
+    # precedence, not this environment variable.
     readonly AWF_DIND_ENABLED
   fi
 
@@ -719,9 +727,11 @@ AWFEOF
   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: AWF_REAL_DOCKER is NOT exported — the wrapper reads it from
+    # /tmp/awf-lib/.docker-path instead, preventing user code from discovering
+    # the real Docker binary path via the environment.
+    # AWF_AGENT_CONTAINER is NOT exported — the wrapper hardcodes 'awf-agent'.
     echo "export AWF_DIND_ENABLED=\"1\"" >> "/host${SCRIPT_FILE}"
-    echo "export AWF_AGENT_CONTAINER=\"${AWF_AGENT_CONTAINER:-awf-agent}\"" >> "/host${SCRIPT_FILE}"
   fi
 
   # Append the actual command arguments
diff --git a/src/docker-manager.ts b/src/docker-manager.ts
@@ -1007,7 +1007,7 @@ export function generateDockerCompose(
       // 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';
+      environment.AWF_AGENT_CONTAINER = AGENT_CONTAINER_NAME;
       logger.debug('Selective mounts configured: system paths (ro), home (rw), Docker socket exposed');
     } else {
       // Hide Docker socket to prevent firewall bypass via 'docker run'
