diff --git a/.github/workflows/test-integration-suite.yml b/.github/workflows/test-integration-suite.yml index c80f560d3..45b3706ce 100644 --- a/.github/workflows/test-integration-suite.yml +++ b/.github/workflows/test-integration-suite.yml @@ -274,7 +274,7 @@ jobs: run: | echo "=== Running container & ops tests ===" npm run test:integration -- \ - --testPathPatterns="(container-workdir|environment-variables|error-handling|exit-code-propagation|log-commands|no-docker|volume-mounts|skip-pull)" \ + --testPathPatterns="(container-workdir|environment-variables|error-handling|exit-code-propagation|filesystem-allowwrite|log-commands|no-docker|volume-mounts|skip-pull)" \ --verbose env: JEST_TIMEOUT: 180000 diff --git a/CLAUDE.md b/CLAUDE.md index d54dd517a..68fac033f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -156,7 +156,7 @@ The codebase follows a modular architecture with clear separation of concerns: - Selective bind mounts under `/host/`: system binaries `/usr`, `/bin`, `/sbin`, `/lib`, `/lib64`, `/opt`, `/sys`, `/dev` (ro); workspace and `/tmp` (rw); whitelisted `$HOME` subdirs (rw); select `/etc` files — NOT a blanket host FS mount; `/etc/shadow`, unwhitelisted home dirs, and most of `/etc` are excluded - `entrypoint.sh` handles: UID/GID remapping → DNS config → SSL CA import → chroot to `/host` → capability drop → run user command as host user - **procfs mount**: A container-scoped procfs is mounted at `/host/proc` with `hidepid=2` to support runtimes that read `/proc/self/exe` (Java, .NET) while preventing the agent from reading other processes' `/proc/[pid]/environ` (credential isolation) -- **iptables init container** (`awf-iptables-init`): separate container sharing agent's network namespace via `network_mode: service:agent`. Runs `setup-iptables.sh` to configure NAT rules before user command starts. Agent waits for `/tmp/awf-init/ready` signal file. +- **iptables init container** (`awf-iptables-init`): separate container sharing agent's network namespace via `network_mode: service:agent`. Runs `setup-iptables.sh` to configure NAT rules before user command starts. Agent waits for `/run/awf-init/ready` signal file (also accepting the legacy `/tmp/awf-init/ready` so an older pinned agent image still works). - Key iptables rules (in `setup-iptables.sh`): - Allow localhost (for stdio MCP servers) and DNS - Allow traffic to Squid proxy itself diff --git a/containers/agent/entrypoint.sh b/containers/agent/entrypoint.sh index 654a79bbc..bf8f256de 100644 --- a/containers/agent/entrypoint.sh +++ b/containers/agent/entrypoint.sh @@ -166,12 +166,17 @@ else echo "[entrypoint] Waiting for iptables initialization from init container..." INIT_TIMEOUT=300 # 300 * 0.1s = 30 seconds INIT_ELAPSED=0 - while [ ! -f /tmp/awf-init/ready ]; do + INIT_SIGNAL_DIR="${AWF_INIT_SIGNAL_DIR:-/run/awf-init}" + LEGACY_INIT_SIGNAL_DIR="/tmp/awf-init" + while [ ! -f "${INIT_SIGNAL_DIR}/ready" ] && [ ! -f "${LEGACY_INIT_SIGNAL_DIR}/ready" ]; do if [ "$INIT_ELAPSED" -ge "$INIT_TIMEOUT" ]; then echo "[entrypoint][ERROR] Timed out waiting for iptables init container after 30s" - if [ -f /tmp/awf-init/output.log ]; then + if [ -f "${INIT_SIGNAL_DIR}/output.log" ]; then echo "[entrypoint] Init container output:" - cat /tmp/awf-init/output.log + cat "${INIT_SIGNAL_DIR}/output.log" + elif [ -f "${LEGACY_INIT_SIGNAL_DIR}/output.log" ]; then + echo "[entrypoint] Init container output:" + cat "${LEGACY_INIT_SIGNAL_DIR}/output.log" else echo "[entrypoint] No init container output log found" fi @@ -597,36 +602,42 @@ mount_host_cgroupfs() { copy_preload_libs() { # Copy one-shot-token library to host filesystem for LD_PRELOAD in chroot # This prevents tokens from being read multiple times by malicious code - # Note: /tmp is always writable in chroot mode (mounted from host /tmp as rw) + # Staged under /run/awf-lib, which lives on the container's own writable + # rootfs. /tmp cannot be used: it is bind-mounted from the host and + # filesystem.allowWrite may narrow it to read-only, which would silently + # disable this protection. # Sets ONE_SHOT_TOKEN_LIB (empty string if unavailable or incompatible) ONE_SHOT_TOKEN_LIB="" if [ -f /usr/local/lib/one-shot-token.so ]; then - # Create the library directory in /tmp (always writable) - if mkdir -p /host/tmp/awf-lib 2>/dev/null; then + # Create the library directory on the container rootfs (always writable) + if mkdir -p /host/run/awf-lib 2>/dev/null; then # Copy the library and verify it exists after copying - if cp /usr/local/lib/one-shot-token.so /host/tmp/awf-lib/one-shot-token.so 2>/dev/null && \ - [ -f /host/tmp/awf-lib/one-shot-token.so ]; then + if cp /usr/local/lib/one-shot-token.so /host/run/awf-lib/one-shot-token.so 2>/dev/null && \ + [ -f /host/run/awf-lib/one-shot-token.so ]; then # Probe compatibility with the host's dynamic linker before committing to LD_PRELOAD. # Run the probe inside chroot /host so the ELF interpreter and all library paths # (e.g. /lib/ld-musl-*.so.1 on Alpine) resolve against the host filesystem, not # the container's. This avoids false negatives where the container's glibc # interpreter would accept the .so even though the host loader cannot. - if chroot /host /bin/sh -c 'LD_PRELOAD=/tmp/awf-lib/one-shot-token.so /bin/true' 2>/dev/null; then - ONE_SHOT_TOKEN_LIB="/tmp/awf-lib/one-shot-token.so" + if chroot /host /bin/sh -c 'LD_PRELOAD=/run/awf-lib/one-shot-token.so /bin/true' 2>/dev/null; then + ONE_SHOT_TOKEN_LIB="/run/awf-lib/one-shot-token.so" echo "[entrypoint] One-shot token library copied to chroot at ${ONE_SHOT_TOKEN_LIB}" else echo "[entrypoint][WARN] one-shot-token.so failed to load on host dynamic linker (host libc incompatibility, e.g. musl/Alpine)" echo "[entrypoint][WARN] Token protection will be disabled (tokens may be readable multiple times)" - rm -f /host/tmp/awf-lib/one-shot-token.so 2>/dev/null || true + rm -f /host/run/awf-lib/one-shot-token.so 2>/dev/null || true fi else - echo "[entrypoint][WARN] Could not copy one-shot-token library to /tmp/awf-lib" - echo "[entrypoint][WARN] Token protection will be disabled (tokens may be readable multiple times)" + # The library exists but could not be staged. Continuing would silently + # drop a security control, so fail closed instead. + echo "[entrypoint][ERROR] Could not copy one-shot-token library to /run/awf-lib" >&2 + echo "[entrypoint][ERROR] Refusing to start without one-shot token protection" >&2 + exit 1 fi else - echo "[entrypoint][ERROR] Could not create /tmp/awf-lib directory" - echo "[entrypoint][ERROR] This should not happen - /tmp is mounted read-write in chroot mode" - echo "[entrypoint][WARN] Token protection will be disabled (tokens may be readable multiple times)" + echo "[entrypoint][ERROR] Could not create /run/awf-lib directory" >&2 + echo "[entrypoint][ERROR] Refusing to start without one-shot token protection" >&2 + exit 1 fi fi } @@ -634,18 +645,19 @@ copy_preload_libs() { copy_agent_helper_scripts() { # Copy get-claude-key.sh and gh CLI proxy wrapper to chroot-accessible paths. # Both scripts are baked into the Docker image but shadowed by host bind mounts - # inside chroot. They are copied to /tmp/awf-lib/ (always writable) so they - # remain accessible after the chroot activates. + # inside chroot. They are copied to /run/awf-lib/ -- on the container's own + # writable rootfs, not the host-bound /tmp which filesystem.allowWrite may + # narrow to read-only -- so they remain accessible after the chroot activates. # Sets CHROOT_KEY_HELPER; may update AWF_HOST_PATH. # Copy get-claude-key.sh to chroot-accessible path # The script is baked into the Docker image at /usr/local/bin/, but the chroot # bind-mounts the host's /usr (read-only), shadowing the container's copy. - # We must copy it to /tmp/awf-lib/ (writable) before the chroot activates. + # We must copy it to /run/awf-lib/ (container rootfs) before the chroot activates. CHROOT_KEY_HELPER="" if [ -n "$CLAUDE_CODE_API_KEY_HELPER" ] && [ -f "$CLAUDE_CODE_API_KEY_HELPER" ]; then - if mkdir -p /host/tmp/awf-lib 2>/dev/null; then - CHROOT_KEY_HELPER="/tmp/awf-lib/$(basename "$CLAUDE_CODE_API_KEY_HELPER")" + if mkdir -p /host/run/awf-lib 2>/dev/null; then + CHROOT_KEY_HELPER="/run/awf-lib/$(basename "$CLAUDE_CODE_API_KEY_HELPER")" if cp "$CLAUDE_CODE_API_KEY_HELPER" "/host${CHROOT_KEY_HELPER}" 2>/dev/null && \ chmod +x "/host${CHROOT_KEY_HELPER}" 2>/dev/null; then echo "[entrypoint] Claude key helper copied to chroot at ${CHROOT_KEY_HELPER}" @@ -675,19 +687,22 @@ copy_agent_helper_scripts() { # Activate gh CLI proxy wrapper when CLI proxy sidecar is enabled. # The wrapper at /usr/local/bin/gh-cli-proxy-wrapper.sh (baked into the image) - # is copied to /tmp/awf-lib/gh so it is accessible inside the chroot at a + # is copied to /run/awf-lib/gh so it is accessible inside the chroot at a # location that takes precedence over the host's /usr/bin/gh mount. if [ -n "$AWF_CLI_PROXY_URL" ] && [ -f /usr/local/bin/gh-cli-proxy-wrapper.sh ]; then - if mkdir -p /host/tmp/awf-lib 2>/dev/null; then - if cp /usr/local/bin/gh-cli-proxy-wrapper.sh /host/tmp/awf-lib/gh 2>/dev/null && \ - chmod +x /host/tmp/awf-lib/gh 2>/dev/null; then - # The chroot will see this as /tmp/awf-lib/gh (the /host prefix is the bind mount) - echo "[entrypoint] gh CLI proxy wrapper installed at /tmp/awf-lib/gh (inside chroot)" - # Prepend /tmp/awf-lib to PATH so the wrapper takes precedence over host gh - export AWF_HOST_PATH="/tmp/awf-lib:${AWF_HOST_PATH:-$PATH}" - else - echo "[entrypoint][WARN] Could not install gh CLI proxy wrapper" - fi + if mkdir -p /host/run/awf-lib 2>/dev/null && \ + cp /usr/local/bin/gh-cli-proxy-wrapper.sh /host/run/awf-lib/gh 2>/dev/null && \ + chmod +x /host/run/awf-lib/gh 2>/dev/null; then + # The chroot will see this as /run/awf-lib/gh (the /host prefix is the bind mount) + echo "[entrypoint] gh CLI proxy wrapper installed at /run/awf-lib/gh (inside chroot)" + # Prepend /run/awf-lib to PATH so the wrapper takes precedence over host gh + export AWF_HOST_PATH="/run/awf-lib:${AWF_HOST_PATH:-$PATH}" + else + # The CLI proxy is enabled, so an unwrapped gh would bypass credential + # mediation entirely. Fail closed rather than silently downgrade. + echo "[entrypoint][ERROR] Could not install gh CLI proxy wrapper at /run/awf-lib/gh" >&2 + echo "[entrypoint][ERROR] Refusing to start with an unmediated gh CLI" >&2 + exit 1 fi fi @@ -696,7 +711,7 @@ copy_agent_helper_scripts() { copy_dind_runner_binary() { # In split-filesystem DinD setups with --docker-host-path-prefix pointing at # a shared /tmp root, docker-manager stages the invoking CLI binary under - # /tmp/awf-runner-bin/. Copy it into /tmp/awf-lib so the chrooted PATH + # /tmp/awf-runner-bin/. Copy it into /run/awf-lib so the chrooted PATH # can resolve the expected command name (copilot, claude, etc.) without # requiring manual bootstrap copies into the daemon's /usr/local/bin. # Sets STAGED_RUNNER_BINARY_CHROOT; may update AWF_HOST_PATH. @@ -705,13 +720,13 @@ copy_dind_runner_binary() { if [[ ! "${AWF_STAGED_RUNNER_BINARY_NAME}" =~ ^[A-Za-z0-9_][A-Za-z0-9_.-]*$ ]]; then echo "[entrypoint][WARN] Ignoring invalid AWF_STAGED_RUNNER_BINARY_NAME=${AWF_STAGED_RUNNER_BINARY_NAME}" elif [ -f "/tmp/awf-runner-bin/${AWF_STAGED_RUNNER_BINARY_NAME}" ]; then - if mkdir -p /host/tmp/awf-lib 2>/dev/null; then - if cp "/tmp/awf-runner-bin/${AWF_STAGED_RUNNER_BINARY_NAME}" "/host/tmp/awf-lib/${AWF_STAGED_RUNNER_BINARY_NAME}" 2>/dev/null && \ - chmod +x "/host/tmp/awf-lib/${AWF_STAGED_RUNNER_BINARY_NAME}" 2>/dev/null; then - STAGED_RUNNER_BINARY_CHROOT="/tmp/awf-lib/${AWF_STAGED_RUNNER_BINARY_NAME}" + if mkdir -p /host/run/awf-lib 2>/dev/null; then + if cp "/tmp/awf-runner-bin/${AWF_STAGED_RUNNER_BINARY_NAME}" "/host/run/awf-lib/${AWF_STAGED_RUNNER_BINARY_NAME}" 2>/dev/null && \ + chmod +x "/host/run/awf-lib/${AWF_STAGED_RUNNER_BINARY_NAME}" 2>/dev/null; then + STAGED_RUNNER_BINARY_CHROOT="/run/awf-lib/${AWF_STAGED_RUNNER_BINARY_NAME}" case ":${AWF_HOST_PATH:-$PATH}:" in - *":/tmp/awf-lib:"*) ;; - *) export AWF_HOST_PATH="/tmp/awf-lib:${AWF_HOST_PATH:-$PATH}" ;; + *":/run/awf-lib:"*) ;; + *) export AWF_HOST_PATH="/run/awf-lib:${AWF_HOST_PATH:-$PATH}" ;; esac echo "[entrypoint] Runner binary staged for chroot at ${STAGED_RUNNER_BINARY_CHROOT}" else @@ -756,7 +771,7 @@ resolve_chroot_binary_path() { if [ -n "${AWF_HOST_PATH:-}" ]; then search_path="${search_path}${AWF_HOST_PATH}:" fi - search_path="${search_path}/tmp/awf-runner-bin:/tmp/awf-lib:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin" + search_path="${search_path}/tmp/awf-runner-bin:/run/awf-lib:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin" local dir="" local IFS=':' @@ -944,15 +959,15 @@ copy_awf_ca_cert() { # Copy AWF CA certificate to chroot-accessible path for ssl-bump TLS trust. # NODE_EXTRA_CA_CERTS points to /usr/local/share/ca-certificates/awf-ca.crt which # is a Docker volume mount on the container's overlay filesystem. After chroot /host, - # this path is inaccessible. Copy to /tmp/awf-lib/ (always writable) and update the + # this path is inaccessible. Copy to /run/awf-lib/ (container rootfs) and update the # env var so Node.js (Claude Code), curl, git, Python, etc. trust the Squid CA. # Sets AWF_CA_CHROOT; exports NODE_EXTRA_CA_CERTS, SSL_CERT_FILE, REQUESTS_CA_BUNDLE. AWF_CA_CHROOT="" if [ "${AWF_SSL_BUMP_ENABLED}" = "true" ] && [ -f /usr/local/share/ca-certificates/awf-ca.crt ]; then - if mkdir -p /host/tmp/awf-lib 2>/dev/null; then - if cp /usr/local/share/ca-certificates/awf-ca.crt /host/tmp/awf-lib/awf-ca.crt 2>/dev/null && \ - [ -f /host/tmp/awf-lib/awf-ca.crt ]; then - AWF_CA_CHROOT="/tmp/awf-lib/awf-ca.crt" + if mkdir -p /host/run/awf-lib 2>/dev/null; then + if cp /usr/local/share/ca-certificates/awf-ca.crt /host/run/awf-lib/awf-ca.crt 2>/dev/null && \ + [ -f /host/run/awf-lib/awf-ca.crt ]; then + AWF_CA_CHROOT="/run/awf-lib/awf-ca.crt" export NODE_EXTRA_CA_CERTS="$AWF_CA_CHROOT" # SSL_CERT_FILE is respected by curl, git, Python requests, Ruby, and most # OpenSSL-based tools. This ensures non-Node.js tools also trust the AWF CA. @@ -965,7 +980,7 @@ copy_awf_ca_cert() { echo "[entrypoint][WARN] Could not copy AWF CA certificate to chroot — ssl-bump TLS may fail" fi else - echo "[entrypoint][WARN] Could not create /host/tmp/awf-lib for CA cert — ssl-bump TLS may fail in chroot" + echo "[entrypoint][WARN] Could not create /host/run/awf-lib for CA cert — ssl-bump TLS may fail in chroot" fi fi } @@ -974,7 +989,7 @@ copy_system_ca_bundle() { # Detect and copy the host system CA bundle to a chroot-accessible path. # On Amazon Linux / RHEL-family systems, the CA bundle often lives under # /etc/pki/. This function finds the system bundle and, when it is not already - # accessible in the chroot, copies it to /tmp/awf-lib/ so TLS works regardless + # accessible in the chroot, copies it to /run/awf-lib/ so TLS works regardless # of distro. # # In SSL Bump mode, the AWF CA must remain the active trust bundle for MITM @@ -1047,10 +1062,10 @@ copy_system_ca_bundle() { esac # Bundle is not accessible in chroot. Copy it. - if mkdir -p /host/tmp/awf-lib 2>/dev/null; then - if cp "$SYSTEM_BUNDLE" /host/tmp/awf-lib/system-ca-certificates.crt 2>/dev/null && \ - [ -s /host/tmp/awf-lib/system-ca-certificates.crt ]; then - local CA_PATH="/tmp/awf-lib/system-ca-certificates.crt" + if mkdir -p /host/run/awf-lib 2>/dev/null; then + if cp "$SYSTEM_BUNDLE" /host/run/awf-lib/system-ca-certificates.crt 2>/dev/null && \ + [ -s /host/run/awf-lib/system-ca-certificates.crt ]; then + local CA_PATH="/run/awf-lib/system-ca-certificates.crt" SYSTEM_CA_CHROOT="$CA_PATH" export SSL_CERT_FILE="$CA_PATH" export NODE_EXTRA_CA_CERTS="$CA_PATH" @@ -1062,7 +1077,7 @@ copy_system_ca_bundle() { echo "[entrypoint][WARN] Could not copy system CA bundle to chroot — TLS may fail" fi else - echo "[entrypoint][WARN] Could not create /host/tmp/awf-lib for system CA bundle" + echo "[entrypoint][WARN] Could not create /host/run/awf-lib for system CA bundle" fi } @@ -1503,8 +1518,13 @@ run_chroot_command() { fi # Write the command to a temporary script file in the chroot - # This avoids complex quoting issues with nested shells - SCRIPT_FILE="/tmp/awf-cmd-$$.sh" + # This avoids complex quoting issues with nested shells. + # It lives under /run rather than /tmp because filesystem.allowWrite can + # narrow the /tmp bind to read-only, which would make this write fail. /run + # inside the chroot is the container's own writable rootfs, so it is always + # writable regardless of the host write policy. + mkdir -p /host/run 2>/dev/null || true + SCRIPT_FILE="/run/awf-cmd-$$.sh" build_path_script "$@" # Execute inside chroot: @@ -1536,9 +1556,9 @@ run_chroot_command() { CLEANUP_CMD="${CLEANUP_CMD}; sed -i '/^[0-9.]\\+[[:space:]]\\+host\\.docker\\.internal\$/d' /etc/hosts 2>/dev/null || true" echo "[entrypoint] host.docker.internal will be removed from /etc/hosts on exit" fi - # Clean up /tmp/awf-lib if anything was copied (one-shot-token, CA cert, key helper) + # Clean up /run/awf-lib if anything was copied (one-shot-token, CA cert, key helper) if [ -n "${ONE_SHOT_TOKEN_LIB}" ] || [ -n "${AWF_CA_CHROOT}" ] || [ -n "${SYSTEM_CA_CHROOT}" ] || [ -n "${CHROOT_KEY_HELPER}" ] || [ -n "${STAGED_RUNNER_BINARY_CHROOT}" ]; then - CLEANUP_CMD="${CLEANUP_CMD}; rm -rf /tmp/awf-lib 2>/dev/null || true" + CLEANUP_CMD="${CLEANUP_CMD}; rm -rf /run/awf-lib 2>/dev/null || true" fi # NOTE: the /usr/local/bin overlay is torn down by cleanup_usr_local_bin_overlay(), # which is installed as an EXIT trap in the container's root shell — the chroot @@ -1606,16 +1626,20 @@ run_non_chroot_command() { # Drop capabilities and privileges, then execute the user command # Activate gh CLI proxy wrapper in non-chroot mode. - # Copy the wrapper to /tmp/awf-lib/gh so it takes precedence over - # the system gh at /usr/bin/gh (since /tmp/awf-lib is prepended to PATH). + # Copy the wrapper to /run/awf-lib/gh so it takes precedence over + # the system gh at /usr/bin/gh (since /run/awf-lib is prepended to PATH). if [ -n "$AWF_CLI_PROXY_URL" ] && [ -f /usr/local/bin/gh-cli-proxy-wrapper.sh ]; then - mkdir -p /tmp/awf-lib - if cp /usr/local/bin/gh-cli-proxy-wrapper.sh /tmp/awf-lib/gh 2>/dev/null && \ - chmod +x /tmp/awf-lib/gh 2>/dev/null; then - export PATH="/tmp/awf-lib:${PATH}" - echo "[entrypoint] gh CLI proxy wrapper installed at /tmp/awf-lib/gh" + mkdir -p /run/awf-lib + if cp /usr/local/bin/gh-cli-proxy-wrapper.sh /run/awf-lib/gh 2>/dev/null && \ + chmod +x /run/awf-lib/gh 2>/dev/null; then + export PATH="/run/awf-lib:${PATH}" + echo "[entrypoint] gh CLI proxy wrapper installed at /run/awf-lib/gh" else - echo "[entrypoint][WARN] Could not install gh CLI proxy wrapper" + # The CLI proxy is enabled, so an unwrapped gh would bypass credential + # mediation entirely. Fail closed rather than silently downgrade. + echo "[entrypoint][ERROR] Could not install gh CLI proxy wrapper at /run/awf-lib/gh" >&2 + echo "[entrypoint][ERROR] Refusing to start with an unmediated gh CLI" >&2 + exit 1 fi fi diff --git a/containers/agent/one-shot-token/README.md b/containers/agent/one-shot-token/README.md index 84c3190cc..7c93256b7 100644 --- a/containers/agent/one-shot-token/README.md +++ b/containers/agent/one-shot-token/README.md @@ -174,8 +174,8 @@ exec capsh --drop=$CAPS_TO_DROP -- -c "exec gosu awfuser $COMMAND" In chroot mode, the library must be accessible from within the chroot (host filesystem). The entrypoint: -1. Copies the library from container to `/host/tmp/awf-lib/one-shot-token.so` -2. Sets `LD_PRELOAD=/tmp/awf-lib/one-shot-token.so` inside the chroot +1. Copies the library from container to `/host/run/awf-lib/one-shot-token.so` +2. Sets `LD_PRELOAD=/run/awf-lib/one-shot-token.so` inside the chroot 3. Cleans up the library on exit ## Building diff --git a/containers/agent/setup-iptables.sh b/containers/agent/setup-iptables.sh index 92eee78c1..ccc6eabf2 100644 --- a/containers/agent/setup-iptables.sh +++ b/containers/agent/setup-iptables.sh @@ -497,7 +497,11 @@ dump_nat_rules_for_debugging() { dump_audit_state() { # Dump full iptables state for audit trail # Written to the init signal volume so it can be preserved by the host - local audit_file="/tmp/awf-init/iptables-audit.txt" + local audit_dir="${AWF_INIT_SIGNAL_DIR:-/run/awf-init}" + if [ ! -d "$audit_dir" ] && [ -d /tmp/awf-init ]; then + audit_dir="/tmp/awf-init" + fi + local audit_file="${audit_dir}/iptables-audit.txt" echo "# iptables audit dump - $(date -u '+%Y-%m-%dT%H:%M:%SZ')" > "$audit_file" echo "" >> "$audit_file" echo "## IPv4 NAT rules" >> "$audit_file" diff --git a/docs/chroot-mode.md b/docs/chroot-mode.md index 9083eb986..b3e2d09c9 100644 --- a/docs/chroot-mode.md +++ b/docs/chroot-mode.md @@ -193,7 +193,7 @@ When `chroot.binariesSourcePath` is set in stdin config, AWF also mounts: **Note:** As of v0.13.13, `/proc` is no longer bind-mounted. Instead, a fresh container-scoped procfs is mounted at `/host/proc` during entrypoint initialization. This provides dynamic `/proc/self/exe` resolution required by Java and .NET runtimes. -**System CA Bundle Detection:** The entrypoint automatically detects the host system CA bundle from common locations (Debian/Ubuntu `/etc/ssl/certs/ca-certificates.crt`, RHEL/Amazon Linux `/etc/pki/ca-trust/extracted/pem/tls-ca-bundle.pem`, `/etc/pki/tls/certs/ca-bundle.crt`, `/etc/pki/tls/cert.pem`, macOS `/etc/ssl/cert.pem`). If the bundle is not already accessible in the chroot via the mounted CA paths, it is copied to `/tmp/awf-lib/system-ca-certificates.crt` and `SSL_CERT_FILE`, `NODE_EXTRA_CA_CERTS`, `REQUESTS_CA_BUNDLE`, `CURL_CA_BUNDLE`, and `GIT_SSL_CAINFO` are set to point at it. +**System CA Bundle Detection:** The entrypoint automatically detects the host system CA bundle from common locations (Debian/Ubuntu `/etc/ssl/certs/ca-certificates.crt`, RHEL/Amazon Linux `/etc/pki/ca-trust/extracted/pem/tls-ca-bundle.pem`, `/etc/pki/tls/certs/ca-bundle.crt`, `/etc/pki/tls/cert.pem`, macOS `/etc/ssl/cert.pem`). If the bundle is not already accessible in the chroot via the mounted CA paths, it is copied to `/run/awf-lib/system-ca-certificates.crt` and `SSL_CERT_FILE`, `NODE_EXTRA_CA_CERTS`, `REQUESTS_CA_BUNDLE`, `CURL_CA_BUNDLE`, and `GIT_SSL_CAINFO` are set to point at it. ### Read-Write Mounts diff --git a/src/agent-helper-staging.test.ts b/src/agent-helper-staging.test.ts new file mode 100644 index 000000000..e8597125e --- /dev/null +++ b/src/agent-helper-staging.test.ts @@ -0,0 +1,87 @@ +import * as fs from 'fs'; +import * as path from 'path'; + +const entrypointSource = fs.readFileSync( + path.resolve(__dirname, '../containers/agent/entrypoint.sh'), + 'utf8', +); + +/** + * AWF stages several helpers on the host side of the chroot before the agent + * runs: the one-shot-token LD_PRELOAD library, the gh CLI proxy wrapper, the + * Claude key helper and CA bundles. + * + * These used to live under `/tmp/awf-lib`. That is a bind mount of the host's + * `/tmp`, so `filesystem.allowWrite` can narrow it to read-only, at which point + * every one of those copies fails. The failures were swallowed, silently + * disabling security controls while the run appeared healthy. + * + * They now stage under `/run/awf-lib`, which lives on the container's own + * writable rootfs and is therefore never affected by a user write policy. + */ +describe('agent helper staging location', () => { + it('never stages helpers under the host-bound /tmp', () => { + expect(entrypointSource).not.toContain('/tmp/awf-lib'); + }); + + it('stages helpers under /run/awf-lib on both sides of the chroot', () => { + expect(entrypointSource).toContain('mkdir -p /host/run/awf-lib'); + expect(entrypointSource).toContain('/host/run/awf-lib/one-shot-token.so'); + expect(entrypointSource).toContain('/host/run/awf-lib/gh'); + }); + + it('preloads the one-shot token library from the chroot-visible path', () => { + expect(entrypointSource).toContain('LD_PRELOAD=/run/awf-lib/one-shot-token.so'); + }); + + it('puts the staging directory on PATH so the gh wrapper shadows the host gh', () => { + expect(entrypointSource).toContain('export AWF_HOST_PATH="/run/awf-lib:${AWF_HOST_PATH:-$PATH}"'); + expect(entrypointSource).toContain('export PATH="/run/awf-lib:${PATH}"'); + }); + + it('cleans up the staging directory at its new location', () => { + expect(entrypointSource).toContain('rm -rf /run/awf-lib'); + }); + + it('no longer claims /tmp is always writable', () => { + expect(entrypointSource).not.toContain('/tmp is mounted read-write in chroot mode'); + expect(entrypointSource).not.toMatch(/\/tmp\/awf-lib\/ \(always writable\)/); + }); + + describe('fail-closed behaviour', () => { + function functionBody(name: string): string { + const start = entrypointSource.indexOf(`${name}() {`); + expect(start).toBeGreaterThan(-1); + const end = entrypointSource.indexOf('\n}\n', start); + return entrypointSource.slice(start, end); + } + + it('refuses to start when the one-shot token library cannot be staged', () => { + const body = functionBody('copy_preload_libs'); + + expect(body).toContain('Refusing to start without one-shot token protection'); + // Both the mkdir and the copy failure paths must abort. + expect(body.match(/exit 1/g) ?? []).toHaveLength(2); + expect(body).not.toContain('Token protection will be disabled (tokens may be readable multiple times)\n fi'); + }); + + it('still tolerates a host libc that cannot load the library', () => { + // An incompatible dynamic linker (musl/Alpine) is an environment property, + // not a staging failure, and must stay a warning. + const body = functionBody('copy_preload_libs'); + + expect(body).toContain('host libc incompatibility'); + expect(body).toContain('Token protection will be disabled'); + }); + + it('refuses to start with an unmediated gh CLI when the proxy is enabled', () => { + const occurrences = entrypointSource.match( + /Refusing to start with an unmediated gh CLI/g, + ) ?? []; + + // Once for the chroot path, once for the non-chroot path. + expect(occurrences).toHaveLength(2); + expect(entrypointSource).not.toContain('[entrypoint][WARN] Could not install gh CLI proxy wrapper'); + }); + }); +}); diff --git a/src/chroot-home-setup.ts b/src/chroot-home-setup.ts index b2b0322fb..e0d06bcba 100644 --- a/src/chroot-home-setup.ts +++ b/src/chroot-home-setup.ts @@ -116,4 +116,17 @@ export function prepareChrootHomeMounts(config: WrapperConfig): void { logger.debug(`Prepared chroot runner tool cache mountpoint: ${chrootToolCachePath} (${uid}:${gid})`); } } + + // On GitHub-hosted runners the workspace lives under $HOME + // (`/home/runner/work//`), so its `/host`-prefixed bind lands + // inside the chroot home. Docker used to create those parents itself, but + // once filesystem.allowWrite narrows the chroot home bind to read-only runc + // can no longer do so and container init fails with EROFS. Prepare the + // mountpoint up front, exactly as for the tool-cache paths above. + const workspaceDir = process.env.GITHUB_WORKSPACE || process.cwd(); + const relativeWorkspacePath = path.relative(effectiveHome, workspaceDir); + if (relativeWorkspacePath && !relativeWorkspacePath.startsWith('..') && !path.isAbsolute(relativeWorkspacePath)) { + const chrootWorkspacePath = prepareChrootHomeMountpoint(emptyHomeDir, relativeWorkspacePath, uid, gid); + logger.debug(`Prepared chroot workspace mountpoint: ${chrootWorkspacePath} (${uid}:${gid})`); + } } diff --git a/src/constants.ts b/src/constants.ts index fd4d63769..abe508483 100644 --- a/src/constants.ts +++ b/src/constants.ts @@ -12,6 +12,8 @@ export const CLI_PROXY_CONTAINER_NAME = 'awf-cli-proxy'; export const ENCLAVE_MCP_SERVER_CONTAINER_NAME = 'awf-enclave-mcp-server'; export const ENCLAVE_AGENT_API_PROXY_CONTAINER_NAME = 'awf-enclave-agent-api-proxy'; export const LOCAL_ENCLAVE_MCP_SERVER_IMAGE = 'awf-enclave-mcp-server:local'; +export const INIT_SIGNAL_DIR = '/run/awf-init'; +export const LEGACY_INIT_SIGNAL_DIR = '/tmp/awf-init'; // SQUID_PORT is centralized in src/config/sandbox-network-policy.json and // re-exported here so existing import sites keep working unchanged. diff --git a/src/fs-utils.ts b/src/fs-utils.ts index b62274318..c78d22386 100644 --- a/src/fs-utils.ts +++ b/src/fs-utils.ts @@ -111,7 +111,30 @@ export function createMissingOwnedDirectorySegments(dirPath: string, uid: number } if (created) { - fs.chownSync(currentPath, uid, gid); + // Only chown when ownership actually has to change, and only when this + // process could possibly succeed. A freshly created directory already + // belongs to the caller, so an owner-preserving chown is a no-op that + // macOS still denies to non-root; and a non-root caller can never hand a + // directory to a *different* owner, so attempting it only turns a usable + // directory into a hard EPERM failure. AWF runs privileged in production, + // where this is unchanged. + if (stat.uid !== uid || stat.gid !== gid) { + try { + fs.chownSync(currentPath, uid, gid); + } catch (error) { + // An unprivileged caller cannot hand a directory to another owner, so + // this can never succeed however often it is retried. Turning that + // into a hard failure would abort on a directory that is already + // usable, so tolerate exactly that case and nothing else. AWF runs + // privileged in production, where the chown still happens and still + // propagates any failure. + const code = (error as NodeJS.ErrnoException).code; + const unprivileged = process.getuid?.() !== 0; + if (!unprivileged || (code !== 'EPERM' && code !== 'EACCES')) { + throw error; + } + } + } fs.chmodSync(currentPath, 0o755); } } diff --git a/src/services/agent-environment/core-environment.ts b/src/services/agent-environment/core-environment.ts index cc9a2ad90..868d9c049 100644 --- a/src/services/agent-environment/core-environment.ts +++ b/src/services/agent-environment/core-environment.ts @@ -1,4 +1,4 @@ -import { SQUID_PORT } from '../../constants'; +import { INIT_SIGNAL_DIR, SQUID_PORT } from '../../constants'; import { getRealUserHome } from '../../host-identity'; import { AgentEnvironmentParams } from './types'; @@ -14,6 +14,7 @@ export function buildCoreEnvironment(params: AgentEnvironmentParams): Record { expect(initService.entrypoint).toEqual(['/bin/bash']); expect(initService.command).toEqual([ '-c', - '/usr/local/bin/setup-iptables.sh > /tmp/awf-init/output.log 2>&1 && touch /tmp/awf-init/ready', + 'mkdir -p "$$AWF_INIT_SIGNAL_DIR" && /usr/local/bin/setup-iptables.sh > "$$AWF_INIT_SIGNAL_DIR/output.log" 2>&1 && touch "$$AWF_INIT_SIGNAL_DIR/ready"', ]); expect(initService.security_opt).toBeUndefined(); expect(initService.restart).toBe('no'); @@ -136,8 +136,8 @@ describe('agent service', () => { const initService = result.services['iptables-init'] as any; const volumes = initService.volumes as string[]; - // Source path is the runner-side init-signal dir, container path is /tmp/awf-init - expect(volumes).toContain(`${mockConfig.workDir}/init-signal:/tmp/awf-init:rw`); + // Source path is the runner-side init-signal dir, container path is outside /tmp. + expect(volumes).toContain(`${mockConfig.workDir}/init-signal:/run/awf-init:rw`); }); it('should apply dockerHostPathPrefix to the iptables-init init-signal volume', () => { @@ -158,10 +158,10 @@ describe('agent service', () => { const agentVolumes = result.services.agent.volumes as string[]; const expectedSource = `/host${mockConfig.workDir}/init-signal`; - expect(initVolumes).toContain(`${expectedSource}:/tmp/awf-init:rw`); + expect(initVolumes).toContain(`${expectedSource}:/run/awf-init:rw`); // The agent must mount the SAME daemon-side source so they share the ready file. - expect(agentVolumes).toContain(`${expectedSource}:/tmp/awf-init:rw`); + expect(agentVolumes).toContain(`${expectedSource}:/run/awf-init:rw`); }); it('should normalize trailing slash in dockerHostPathPrefix for iptables-init mount', () => { @@ -174,7 +174,7 @@ describe('agent service', () => { const initService = result.services['iptables-init'] as any; const initVolumes = initService.volumes as string[]; - expect(initVolumes).toContain(`/host${mockConfig.workDir}/init-signal:/tmp/awf-init:rw`); + expect(initVolumes).toContain(`/host${mockConfig.workDir}/init-signal:/run/awf-init:rw`); }); // Symmetric invariant: every absolute, non-kernel-virtual bind-mount source on every diff --git a/src/services/agent-service.ts b/src/services/agent-service.ts index c21a7551c..f7f0cbbe7 100644 --- a/src/services/agent-service.ts +++ b/src/services/agent-service.ts @@ -1,7 +1,9 @@ import * as path from 'path'; import { AGENT_CONTAINER_NAME, + INIT_SIGNAL_DIR, IPTABLES_INIT_CONTAINER_NAME, + LEGACY_INIT_SIGNAL_DIR, SQUID_PORT, } from '../constants'; import { ACT_PRESET_BASE_IMAGE, getSafeHostUid, getSafeHostGid } from '../host-identity'; @@ -292,13 +294,30 @@ interface IptablesInitServiceParams { */ export function buildIptablesInitService(params: IptablesInitServiceParams): any { const { agentService, environment, networkConfig, initSignalDir, dockerHostPathPrefix, hostGatewayIp } = params; + const setupCommand = [ + 'mkdir -p "$$AWF_INIT_SIGNAL_DIR"', + '/usr/local/bin/setup-iptables.sh > "$$AWF_INIT_SIGNAL_DIR/output.log" 2>&1', + 'touch "$$AWF_INIT_SIGNAL_DIR/ready"', + ].join(' && '); // The init-signal mount must use the same source path that the agent container uses, // otherwise the two containers bind to different daemon-side directories and the // ready-file handshake fails. buildAgentVolumes() applies dockerHostPathPrefix to its // mounts, so do the same here via the shared helper. - const [initSignalMount] = applyHostPathPrefixToVolumes( - [`${initSignalDir}:/tmp/awf-init:rw`], + // + // The legacy path is bound here as well, and read-write. This container has its + // own mount namespace, so the agent-side legacy bind is invisible to it and + // cannot help. It matters because setup-iptables.sh in agent images released + // before the signal directory moved to /run writes its audit dump to a + // hardcoded LEGACY_INIT_SIGNAL_DIR path, under `set -e`: with the directory + // absent the redirection fails, the script exits non-zero before the `touch` + // above, and the agent waits out its full ready timeout and then fails. Both + // paths are the same host directory, so a new image simply ignores this one. + const [initSignalMount, legacyInitSignalMount] = applyHostPathPrefixToVolumes( + [ + `${initSignalDir}:${INIT_SIGNAL_DIR}:rw`, + `${initSignalDir}:${LEGACY_INIT_SIGNAL_DIR}:rw`, + ], dockerHostPathPrefix, ); @@ -312,6 +331,7 @@ export function buildIptablesInitService(params: IptablesInitServiceParams): any // Only mount the init signal volume and the iptables setup script volumes: [ initSignalMount, + legacyInitSignalMount, ], environment: { // Pass through environment variables needed by setup-iptables.sh @@ -335,6 +355,7 @@ export function buildIptablesInitService(params: IptablesInitServiceParams): any AWF_SSL_BUMP_ENABLED: environment.AWF_SSL_BUMP_ENABLED || '', AWF_SSL_BUMP_INTERCEPT_PORT: environment.AWF_SSL_BUMP_INTERCEPT_PORT || '', AWF_HOST_GATEWAY_IP: hostGatewayIp || '', + AWF_INIT_SIGNAL_DIR: INIT_SIGNAL_DIR, }, depends_on: { 'agent': { @@ -350,7 +371,7 @@ export function buildIptablesInitService(params: IptablesInitServiceParams): any // The init container only needs to run setup-iptables.sh directly. entrypoint: ['/bin/bash'], // Run setup-iptables.sh then signal readiness; log output to shared volume for diagnostics - command: ['-c', '/usr/local/bin/setup-iptables.sh > /tmp/awf-init/output.log 2>&1 && touch /tmp/awf-init/ready'], + command: ['-c', setupCommand], // Resource limits (init container exits quickly) mem_limit: '128m', pids_limit: 50, diff --git a/src/services/agent-volumes-basic.test.ts b/src/services/agent-volumes-basic.test.ts index 3a1cec599..487c5188f 100644 --- a/src/services/agent-volumes-basic.test.ts +++ b/src/services/agent-volumes-basic.test.ts @@ -63,6 +63,12 @@ describe('agent service', () => { expect(volumes).toContain(`${writablePath}:/host${writablePath}:rw`); expect(volumes).toContain('/tmp:/tmp:ro'); expect(volumes).toContain('/tmp:/host/tmp:ro'); + expect(volumes).toContain(`${getConfig().workDir}/init-signal:/run/awf-init:rw`); + // Legacy path stays exposed read-only so a newer CLI still works with an + // older pinned agent image, and it must not become a writable hole in the + // narrowed /tmp tree. + expect(volumes).toContain(`${getConfig().workDir}/init-signal:/tmp/awf-init:ro`); + expect(volumes).not.toContain(`${getConfig().workDir}/init-signal:/tmp/awf-init:rw`); expect(volumes.some((volume) => volume.includes('/.copilot/logs:rw'))).toBe(true); }); diff --git a/src/services/agent-volumes/credential-hiding.test.ts b/src/services/agent-volumes/credential-hiding.test.ts index 71116371f..fc60b35cb 100644 --- a/src/services/agent-volumes/credential-hiding.test.ts +++ b/src/services/agent-volumes/credential-hiding.test.ts @@ -1,5 +1,9 @@ -import { buildCredentialHidingOverlays } from './credential-hiding'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { buildCredentialHidingOverlays, pruneUnmountableCredentialOverlays } from './credential-hiding'; import { credentialFilesToHide } from '../../config/mount-policy'; +import { createLocalSourceResolver } from './mount-topology'; describe('buildCredentialHidingOverlays', () => { it('hides every policy credential file at both home and /host paths', () => { @@ -27,3 +31,162 @@ describe('buildCredentialHidingOverlays', () => { expect(overlays).toContain('/dev/null:/home/runner/.gemini/oauth_creds.json:ro'); }); }); + +describe('pruneUnmountableCredentialOverlays', () => { + const HOME = '/home/runner'; + const overlay = (target: string) => `/dev/null:${target}:ro`; + + let hostDir: string; + + beforeEach(() => { + hostDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cred-prune-')); + }); + + afterEach(() => { + fs.rmSync(hostDir, { recursive: true, force: true }); + }); + + it('keeps every overlay when no covering bind is read-only (the no-policy case)', () => { + const volumes = [ + `${hostDir}:/host${HOME}:rw`, + `${hostDir}/.config:/host${HOME}/.config:rw`, + overlay(`${HOME}/.docker/config.json`), + overlay(`/host${HOME}/.docker/config.json`), + overlay(`/host${HOME}/.config/gh/hosts.yml`), + ]; + + expect(pruneUnmountableCredentialOverlays(volumes)).toEqual(volumes); + }); + + it('keeps overlays whose mountpoint exists behind a read-only bind', () => { + fs.mkdirSync(path.join(hostDir, '.config/gh'), { recursive: true }); + fs.writeFileSync(path.join(hostDir, '.config/gh/hosts.yml'), 'DUMMY_SECRET_VALUE'); + const target = `/host${HOME}/.config/gh/hosts.yml`; + + const result = pruneUnmountableCredentialOverlays([ + `${hostDir}:/host${HOME}:ro`, + overlay(target), + ]); + + expect(result).toContain(overlay(target)); + }); + + it('drops overlays whose mountpoint is missing behind a read-only bind', () => { + const target = `/host${HOME}/.docker/config.json`; + + const result = pruneUnmountableCredentialOverlays([ + `${hostDir}:/host${HOME}:ro`, + overlay(target), + ]); + + expect(result).not.toContain(overlay(target)); + expect(result).toContain(`${hostDir}:/host${HOME}:ro`); + }); + + it('resolves the mountpoint against the innermost covering bind', () => { + // An outer read-only bind that does have the file, and an inner read-only + // bind that does not. The inner bind is what supplies the directory, so the + // overlay is unmountable and must be dropped. + fs.mkdirSync(path.join(hostDir, 'outer/.config/gh'), { recursive: true }); + fs.writeFileSync(path.join(hostDir, 'outer/.config/gh/hosts.yml'), 'DUMMY'); + fs.mkdirSync(path.join(hostDir, 'inner'), { recursive: true }); + const target = `/host${HOME}/.config/gh/hosts.yml`; + + const result = pruneUnmountableCredentialOverlays([ + `${hostDir}/outer:/host${HOME}:ro`, + `${hostDir}/inner:/host${HOME}/.config:ro`, + overlay(target), + ]); + + expect(result).not.toContain(overlay(target)); + }); + + it('keeps overlays that land on the container rootfs with no covering bind', () => { + const volumes = [ + `${hostDir}:/host${HOME}:ro`, + '/dev/null:/host/var/run/docker.sock:ro', + '/dev/null:/host/run/docker.sock:ro', + ]; + + const result = pruneUnmountableCredentialOverlays(volumes); + + expect(result).toContain('/dev/null:/host/var/run/docker.sock:ro'); + expect(result).toContain('/dev/null:/host/run/docker.sock:ro'); + }); + + it('keeps overlays covered by a named volume, which cannot be probed', () => { + const target = `/host${HOME}/.docker/config.json`; + + const result = pruneUnmountableCredentialOverlays([ + `awf-home:/host${HOME}:ro`, + overlay(target), + ]); + + expect(result).toContain(overlay(target)); + }); + + it('never drops non-overlay mounts', () => { + const volumes = [`${hostDir}:/host${HOME}:ro`, '/tmp:/tmp:ro', `${hostDir}:/workspace:rw`]; + + expect(pruneUnmountableCredentialOverlays(volumes)).toEqual(volumes); + }); + + describe('split-filesystem runners (--docker-host-path-prefix)', () => { + it('keeps an overlay when the covering custom mount maps to a real runner path', () => { + const localRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'awf-local-')); + fs.mkdirSync(path.join(localRoot, '.docker'), { recursive: true }); + fs.writeFileSync(path.join(localRoot, '.docker/config.json'), '{}'); + + const resolver = createLocalSourceResolver(new Map([['/daemon' + localRoot, localRoot]]), '/daemon'); + const target = `/host${HOME}/.docker/config.json`; + + const result = pruneUnmountableCredentialOverlays( + [`/daemon${localRoot}:/host${HOME}:ro`, overlay(target)], + resolver, + ); + + expect(result).toContain(overlay(target)); + fs.rmSync(localRoot, { recursive: true, force: true }); + }); + + it('keeps an overlay whose covering custom mount has no known runner path', () => { + // Fail closed: probing '/daemon/staged/...' with runner-local fs would + // report "missing" and silently unmask a real credential file. + const resolver = createLocalSourceResolver(new Map([['/daemon/staged', '']]), '/daemon'); + const target = `/host${HOME}/.docker/config.json`; + + const result = pruneUnmountableCredentialOverlays( + [`/daemon/staged:/host${HOME}:ro`, overlay(target)], + resolver, + ); + + expect(result).toContain(overlay(target)); + }); + + it('keeps an overlay behind an unattributable daemon-prefixed source', () => { + const resolver = createLocalSourceResolver(new Map(), '/daemon'); + const target = `/host${HOME}/.npmrc`; + + const result = pruneUnmountableCredentialOverlays( + [`/daemon/tmp/chroot-home:/host${HOME}:ro`, overlay(target)], + resolver, + ); + + expect(result).toContain(overlay(target)); + }); + + it('still drops an unreachable overlay when the runner path is known', () => { + const localRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'awf-local-')); + const resolver = createLocalSourceResolver(new Map([['/daemon' + localRoot, localRoot]]), '/daemon'); + const target = `/host${HOME}/.docker/config.json`; + + const result = pruneUnmountableCredentialOverlays( + [`/daemon${localRoot}:/host${HOME}:ro`, overlay(target)], + resolver, + ); + + expect(result).not.toContain(overlay(target)); + fs.rmSync(localRoot, { recursive: true, force: true }); + }); + }); +}); diff --git a/src/services/agent-volumes/credential-hiding.ts b/src/services/agent-volumes/credential-hiding.ts index 0b8c1460d..901f13c8c 100644 --- a/src/services/agent-volumes/credential-hiding.ts +++ b/src/services/agent-volumes/credential-hiding.ts @@ -1,5 +1,13 @@ +import * as fs from 'fs'; import { logger } from '../../logger'; import { credentialFilesToHide } from '../../config/mount-policy'; +import { + LocalSourceResolver, + ParsedMount, + identityLocalSourceResolver, + innermostCoveringMount, + parseMount, +} from './mount-topology'; /** * Builds the compose-mode `/dev/null` overlays that blank known on-disk @@ -22,3 +30,65 @@ export function buildCredentialHidingOverlays(effectiveHome: string): string[] { return mounts; } + +/** + * Drops `/dev/null` overlays whose mountpoint cannot physically be created. + * + * A bind mount requires its mountpoint to already exist; runc creates a missing + * one with `openat`/`mkdirat` on the parent directory. That silently works + * while every mount under `$HOME` is read-write, but once `filesystem.allowWrite` + * narrows those binds to read-only, runc fails with EROFS and the agent + * container dies before it starts. + * + * An overlay is kept unless its containing bind is read-only *and* the path it + * masks does not exist behind that bind. Dropping those loses no protection: + * the read-only bind is the only way the agent could reach the path, and there + * is nothing there to read. Every credential that is actually reachable is + * still masked, because mounting over a path that exists succeeds even inside a + * read-only bind. + * + * This is a no-op unless a write policy is active, since every covering bind is + * read-write otherwise. + */ +export function pruneUnmountableCredentialOverlays( + volumes: string[], + localSourceResolver: LocalSourceResolver = identityLocalSourceResolver, +): string[] { + const binds = volumes + .map(parseMount) + .filter((mount): mount is ParsedMount => mount !== undefined && mount.source !== '/dev/null'); + + const kept = volumes.filter((spec) => { + const overlay = parseMount(spec); + if (!overlay || overlay.source !== '/dev/null') return true; + + const cover = innermostCoveringMount(binds, overlay.target); + // No covering bind: the mountpoint lives on the container's own writable + // rootfs, so runc can always create it. + if (!cover) return true; + if (cover.mode !== 'ro') return true; + // Named volumes and other non-path sources cannot be probed on the host. + if (!cover.source.startsWith('/')) return true; + + const suffix = overlay.target.slice(cover.target.length); + if (!suffix) return true; + + // On split-filesystem runners the covering source may be a daemon-side path + // that means nothing to `fs` here. Keep the overlay rather than risk + // unmasking a credential based on an unrelated runner path. + const localCoverSource = localSourceResolver(cover.source); + if (localCoverSource === undefined) return true; + + return fs.existsSync(`${localCoverSource}${suffix}`); + }); + + const dropped = volumes.length - kept.length; + if (dropped > 0) { + logger.debug( + `Dropped ${dropped} credential overlay(s) whose target does not exist behind a read-only ` + + 'mount; those paths are unreadable in the container, so nothing is left unmasked', + ); + } + + return kept; +} diff --git a/src/services/agent-volumes/docker-host-staging.ts b/src/services/agent-volumes/docker-host-staging.ts index 139302d68..8cde0503c 100644 --- a/src/services/agent-volumes/docker-host-staging.ts +++ b/src/services/agent-volumes/docker-host-staging.ts @@ -1,22 +1,34 @@ import * as fs from 'fs'; import * as path from 'path'; import { logger } from '../../logger'; +import { isTmpRootedDockerHostPathPrefix, normalizeDockerHostPathPrefix } from '../host-path-prefix'; import { WrapperConfig } from '../../types'; const DOCKER_HOST_STAGE_DIR = 'awf-docker-host-stage'; const SAFE_BINARY_NAME_REGEX = /^[a-zA-Z0-9_][a-zA-Z0-9_.-]*$/; -function normalizeDockerHostPathPrefix(prefix: string): string { - const trimmed = prefix.trim(); - if (!trimmed) return ''; - const withLeadingSlash = trimmed.startsWith('/') ? trimmed : `/${trimmed}`; - return withLeadingSlash.replace(/\/+$/, '') || '/'; +/** + * Regular files AWF copied into the daemon staging root during this run. + * + * Staged sources live *under* `--docker-host-path-prefix`, so a topology pass + * cannot attribute them back to a runner path and must otherwise treat them as + * daemon-side. Recording them here preserves the one fact that would be lost: + * the source is a regular file on this filesystem, so a bind of it needs a + * *file* mountpoint rather than a directory. + */ +const stagedHostFiles = new Set(); + +export function isStagedHostFile(candidate: string): boolean { + return stagedHostFiles.has(candidate); +} + +/** Test seam: staging is process-global, so suites must be able to reset it. */ +export function clearStagedHostFiles(): void { + stagedHostFiles.clear(); } export function shouldUseDockerHostStaging(prefix: string | undefined): boolean { - if (!prefix) return false; - const normalized = normalizeDockerHostPathPrefix(prefix); - return normalized === '/tmp' || normalized.startsWith('/tmp/'); + return isTmpRootedDockerHostPathPrefix(prefix); } export function getDockerHostStageRoot(config: WrapperConfig): string { @@ -53,6 +65,7 @@ export function stageHostFile(config: WrapperConfig, sourcePath: string, relativ fs.mkdirSync(path.dirname(targetPath), { recursive: true }); fs.copyFileSync(sourcePath, targetPath); fs.chmodSync(targetPath, mode); + stagedHostFiles.add(targetPath); return targetPath; } catch (err) { logger.debug(`Could not stage ${sourcePath} for docker-host-path-prefix: ${err}`); diff --git a/src/services/agent-volumes/mount-topology.test.ts b/src/services/agent-volumes/mount-topology.test.ts new file mode 100644 index 000000000..60e8b1859 --- /dev/null +++ b/src/services/agent-volumes/mount-topology.test.ts @@ -0,0 +1,131 @@ +import { createLocalSourceResolver } from './mount-topology'; + +/** + * Pure string-level coverage of local-source resolution. + * + * The filesystem-backed suites cannot pin this down on their own: they build a + * prefix from `os.tmpdir()`, which is `/tmp/...` on Linux CI but + * `/private/var/...` on macOS after `realpathSync`. That makes "is this prefix + * shared?" answer differently per platform, so a Linux-only regression can hide + * behind a green local run. These cases hardcode both shapes instead. + */ +describe('createLocalSourceResolver', () => { + const workDirSource = '/tmp/awf-1730000000/init-signal'; + + describe('shared prefix (/tmp)', () => { + // Only the exact /tmp shape is structurally shared: AWF's own workDir then + // sits under the prefix and its binds are never translated, so the daemon + // has to resolve them to the same bytes. + it.each(['/tmp', '/tmp/', ' /tmp ', 'tmp'])( + 'keeps a source under %p runner-resolvable', + (prefix) => { + const resolve = createLocalSourceResolver(new Map(), prefix); + + expect(resolve('/tmp/awf-run/init-signal')).toBe('/tmp/awf-run/init-signal'); + }, + ); + + it('resolves the run workDir that sits inside the prefix', () => { + const resolve = createLocalSourceResolver(new Map(), '/tmp'); + + // Returning undefined here made the whole run directory unclassifiable: + // every nested bind under a policy-narrowed read-only cover then failed + // closed before launch. + expect(resolve(workDirSource)).toBe(workDirSource); + expect(resolve('/tmp/awf-1730000000-chroot-home')).toBe('/tmp/awf-1730000000-chroot-home'); + }); + }); + + describe('daemon-only prefix', () => { + // A /tmp *descendant* is daemon-only, not shared. `/tmp/gh-aw` is one of + // dind-probe's CANDIDATE_PREFIXES, and that loop is reached only after the + // probe confirms the daemon cannot see the runner's filesystem — it means + // the daemon sees runner path X at /tmp/gh-aw/X. buildCustomVolumeMounts + // agrees, translating sources that already start with /tmp/gh-aw. + it.each([ + ['/tmp/gh-aw', '/tmp/gh-aw/awf-docker-host-stage/bin/copilot'], + ['/tmp/gh-aw/', '/tmp/gh-aw/tmp/awf-1730000000/init-signal'], + ['/tmp/shared', '/tmp/shared/awf-run/init-signal'], + ['/tmp/gh-aw/nested', '/tmp/gh-aw/nested/anything'], + ])('fails closed for the /tmp descendant %p', (prefix, source) => { + const resolve = createLocalSourceResolver(new Map(), prefix); + + expect(resolve(source)).toBeUndefined(); + }); + + it('still resolves a plain /tmp path under a /tmp descendant prefix', () => { + const resolve = createLocalSourceResolver(new Map(), '/tmp/gh-aw'); + + // Not under the prefix, so it is an ordinary runner path that + // translation will rewrite later. + expect(resolve('/tmp/awf-1730000000/init-signal')).toBe('/tmp/awf-1730000000/init-signal'); + }); + + it.each(['/host', '/dind', '/var/runner'])( + 'refuses to attribute a source already under %p', + (prefix) => { + const resolve = createLocalSourceResolver(new Map(), prefix); + + expect(resolve(`${prefix}/tmp/awf-run/init-signal`)).toBeUndefined(); + }, + ); + + it.each(['/host/', ' /host ', 'host'])( + 'fails closed for %p, which the CLI only trims', + (prefix) => { + // The raw value reaches the resolver, so an unnormalised comparison + // would miss and hand back a daemon path as if it were runner-local. + const resolve = createLocalSourceResolver(new Map(), prefix); + + expect(resolve('/host/data/inner')).toBeUndefined(); + }, + ); + + it('still resolves a runner path that merely resembles the prefix', () => { + const resolve = createLocalSourceResolver(new Map(), '/host'); + + // Sibling, not nested: `/hostage` must not be mistaken for `/host/...`. + expect(resolve('/hostage/thing')).toBe('/hostage/thing'); + // The prefix itself is not "under" the prefix. + expect(resolve('/host')).toBe('/host'); + }); + }); + + describe('custom mounts', () => { + it('maps a daemon-side custom source back to its runner path', () => { + const resolve = createLocalSourceResolver(new Map([['/host/data', '/data']]), '/host'); + + expect(resolve('/host/data/inner')).toBe('/data/inner'); + }); + + it('fails closed when a custom source cannot be mapped back', () => { + const resolve = createLocalSourceResolver(new Map([['/host/data', '']]), '/host'); + + expect(resolve('/host/data/inner')).toBeUndefined(); + }); + + it('applies the custom mapping even under a shared prefix', () => { + // Custom mounts are resolved before any prefix reasoning, so a shared + // prefix must not quietly bypass their fail-closed mapping. + const resolve = createLocalSourceResolver(new Map([['/tmp/data', '']]), '/tmp'); + + expect(resolve('/tmp/data/inner')).toBeUndefined(); + }); + }); + + it('resolves to itself when no prefix is configured', () => { + const resolve = createLocalSourceResolver(new Map()); + + expect(resolve(workDirSource)).toBe(workDirSource); + }); + + // `/` normalises to `/`, which translateBindMountHostPath returns unchanged — + // it prefixes nothing. Reading it as a daemon root would make every absolute + // source unattributable and fail an otherwise ordinary run closed. + it.each(['/', '//', ' / '])('treats %p as no prefix at all', (prefix) => { + const resolve = createLocalSourceResolver(new Map(), prefix); + + expect(resolve(workDirSource)).toBe(workDirSource); + expect(resolve('/home/runner/work/repo/repo')).toBe('/home/runner/work/repo/repo'); + }); +}); diff --git a/src/services/agent-volumes/mount-topology.ts b/src/services/agent-volumes/mount-topology.ts new file mode 100644 index 000000000..294e20e98 --- /dev/null +++ b/src/services/agent-volumes/mount-topology.ts @@ -0,0 +1,116 @@ +/** + * Shared helpers for reasoning about the *final* compose bind topology. + * + * Both the credential-overlay prune and the nested-mountpoint preparation need + * to answer the same two questions about a generated volume list: + * + * 1. Which bind covers a given container path once runc has mounted parents + * first (the innermost strictly-shallower bind)? + * 2. Where does that bind's source live *on the runner*, so we can probe or + * prepare it before `docker compose up`? + * + * Question 2 is subtle on split-filesystem runners (`--docker-host-path-prefix`). + * Custom volume mounts are materialised with daemon-side sources, so a runner + * local `fs` call against them is meaningless. Everything else is still a + * runner-local path at this stage because the prefix is applied last. + * + * Being *under* the prefix is therefore not the same as being daemon-only. A + * shared prefix (see `isSharedDockerHostPathPrefix`) names one filesystem both + * sides can see at the same path, and the run's own workDir lives inside it — + * treating that as unresolvable would make the whole run directory + * unclassifiable. The mirror-image mistake is just as bad: a daemon-only + * prefix that merely looks shared (`/tmp/gh-aw`) must keep failing closed. + */ + +import { isSharedDockerHostPathPrefix, normalizeDockerHostPathPrefix } from '../host-path-prefix'; + +export interface ParsedMount { + source: string; + target: string; + mode: string; +} + +export function parseMount(spec: string): ParsedMount | undefined { + const parts = spec.split(':'); + if (parts.length < 2 || !parts[0] || !parts[1]) return undefined; + return { + source: parts[0], + target: parts[1].replace(/\/+$/, '') || '/', + mode: parts[2] || 'rw', + }; +} + +function isPathPrefix(parent: string, child: string): boolean { + return parent !== child && child.startsWith(parent === '/' ? '/' : `${parent}/`); +} + +/** + * Returns the deepest bind whose target strictly contains `target`. runc applies + * binds parent-first, so this is the mount that owns the directory entry runc + * must create for `target`. + */ +export function innermostCoveringMount(binds: ParsedMount[], target: string): ParsedMount | undefined { + let best: ParsedMount | undefined; + for (const bind of binds) { + if (!isPathPrefix(bind.target, target)) continue; + if (!best || bind.target.length > best.target.length) best = bind; + } + return best; +} + +/** + * Resolves a generated bind source to the equivalent path on the runner's own + * filesystem, or `undefined` when that cannot be known. + * + * `undefined` means "do not touch": callers must fail closed (keep a credential + * overlay, skip a mountpoint preparation) rather than act on a path that may + * belong to the Docker daemon's filesystem instead of the runner's. + */ +export type LocalSourceResolver = (source: string) => string | undefined; + +export function createLocalSourceResolver( + customSourceRoots: Map, + dockerHostPathPrefix?: string, +): LocalSourceResolver { + // The CLI only trims this value, so compare against the canonical form: a raw + // `/host/` would otherwise fail the `/host/` prefix test and let a daemon-side + // source be treated as runner-local. + const normalizedPrefix = dockerHostPathPrefix + ? normalizeDockerHostPathPrefix(dockerHostPathPrefix) + : ''; + // `/` prefixes nothing: `translateBindMountHostPath` returns every mount + // unchanged for it, so the generated sources are plain runner paths. Reading + // it as a daemon root instead would make *every* absolute source + // unattributable and fail the run closed before launch. + const isNoOpPrefix = normalizedPrefix === '' || normalizedPrefix === '/'; + const daemonOnlyPrefix = !isNoOpPrefix && !isSharedDockerHostPathPrefix(normalizedPrefix) + ? normalizedPrefix + : ''; + + return (source: string): string | undefined => { + for (const [daemonRoot, localRoot] of customSourceRoots) { + if (source !== daemonRoot && !isPathPrefix(daemonRoot, source)) continue; + // A custom mount we cannot map back to a runner path: fail closed. + if (!localRoot) return undefined; + return `${localRoot}${source.slice(daemonRoot.length)}`; + } + + // Not a custom mount. Every other source is still runner-local here because + // `applyHostPathPrefixToVolumes` runs after this stage. A source that + // already carries a *daemon-only* prefix cannot be attributed, so fail + // closed. + // + // A shared prefix is not that case: there the same path is valid on both + // sides, so the run's own workDir (and everything AWF stages) legitimately + // sits under the prefix and stays runner-resolvable. Failing closed on it + // instead would misclassify the entire run directory. + if (daemonOnlyPrefix && isPathPrefix(daemonOnlyPrefix, source)) { + return undefined; + } + + return source; + }; +} + +/** Default resolver for single-filesystem runs: sources are runner-local. */ +export const identityLocalSourceResolver: LocalSourceResolver = (source) => source; diff --git a/src/services/agent-volumes/nested-mountpoints.test.ts b/src/services/agent-volumes/nested-mountpoints.test.ts new file mode 100644 index 000000000..e736c592c --- /dev/null +++ b/src/services/agent-volumes/nested-mountpoints.test.ts @@ -0,0 +1,827 @@ +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { + ensureNestedMountpoints, + planNestedMountpoints, +} from './nested-mountpoints'; +import { createLocalSourceResolver } from './mount-topology'; +import { pruneUnmountableCredentialOverlays } from './credential-hiding'; +import { HOME_TOOL_PATHS } from '../../config/mount-policy'; +import { buildAgentVolumes } from './volume-builder'; +import { WrapperConfig } from '../../types'; + +jest.mock('../../host-env', () => ({ + ...jest.requireActual('../../host-env'), + // Real runs are root under sudo; the test process is not, so keep chown a no-op + // by targeting the identity the test already owns. + getSafeHostUid: () => String(process.getuid?.() ?? 0), + getSafeHostGid: () => String(process.getgid?.() ?? 0), +})); + +/** + * Models how runc materialises a compose bind list: parents first, and every + * mountpoint has to already exist inside whichever bind currently covers it. + * Returns the mounts runc would fail on. + */ +function simulateRuncMountFailures(volumes: string[]): string[] { + const parsed = volumes + .map((spec) => { + const [source, target, mode] = spec.split(':'); + return { spec, source, target, mode: mode || 'rw' }; + }) + .filter((mount) => Boolean(mount.source && mount.target)); + + const ordered = [...parsed].sort( + (a, b) => a.target.split('/').length - b.target.split('/').length, + ); + + const failures: string[] = []; + const established: typeof ordered = []; + + for (const mount of ordered) { + const cover = established + .filter((candidate) => mount.target.startsWith(`${candidate.target}/`)) + .sort((a, b) => b.target.length - a.target.length)[0]; + + if (cover) { + const hostPath = `${cover.source}${mount.target.slice(cover.target.length)}`; + // runc creates a missing mountpoint with mkdirat; that fails on a + // read-only cover. + if (!fs.existsSync(hostPath) && cover.mode === 'ro') failures.push(mount.spec); + } + + established.push(mount); + } + + return failures; +} + +/** + * Cleanup for the product-owned, *fixed* /tmp paths that a /tmp-rooted + * `--docker-host-path-prefix` writes to (`/tmp/awf-init`, + * `/tmp/awf-runner-bin`, `/tmp/awf-docker-host-stage`). + * + * Deliberately removes *files only*, and only files this run provably owns: + * the uniquely named staged binary, the mountpoint that was materialised for + * it, the per-run random `chroot-*` staging directory, and staged copies that + * did not exist before the build. + * + * It never removes the shared root directories themselves, for two measured + * reasons: + * - Several pre-existing suites create them concurrently under + * `maxWorkers`, so deleting one races with a worker that still needs it. + * - `ensureNestedMountpoints` chowns any directory it creates, and macOS + * denies chown to non-root. So on macOS a *missing* `/tmp/awf-init` makes + * unrelated suites fail with EPERM, where a pre-existing one succeeds. + * Tests must therefore never depend on these roots being absent or present; + * the assertions below read the planned `hostPath` instead of `existsSync`. + */ +function makeFixedTmpArtifactTracker(stagedCopies: string[] = []) { + const files: string[] = []; + const dirs: string[] = []; + + return { + /** Call before building, so pre-existing copies are never removed. */ + noteBeforeBuild(): void { + for (const copy of stagedCopies) { + if (!fs.existsSync(copy)) files.push(copy); + } + }, + /** Attribute this run's artefacts from the specs it actually generated. */ + trackGenerated(volumes: string[], binaryName: string): void { + for (const spec of volumes) { + const [source = '', target = ''] = spec.split(':'); + const chroot = /^(\/tmp\/awf-docker-host-stage\/chroot-[A-Za-z0-9_-]+)\//.exec(source); + if (chroot) { + dirs.push(chroot[1]); + continue; + } + if (!binaryName || path.basename(source) !== binaryName) continue; + // The staged copy is the bind *source*; the mountpoint that + // ensureNestedMountpoints had to materialise under the narrowed /tmp + // cover is the bind *target*. Both are real files on this machine. + for (const candidate of [source, target]) { + if (candidate.startsWith('/tmp/')) files.push(candidate); + } + } + }, + cleanup(): void { + files.splice(0).forEach((file) => { + try { + fs.unlinkSync(file); + } catch { + // Never created, or already gone. + } + }); + // Random per-run name, so a recursive remove here cannot reach anything + // another worker owns. + dirs.splice(0).forEach((dir) => fs.rmSync(dir, { recursive: true, force: true })); + }, + }; +} + +// Staged under fixed, stable names rather than per-run ones, so they can only +// be reclaimed when this suite is the run that created them. +const FIXED_TMP_STAGED_COPIES = [ + '/tmp/awf-docker-host-stage/etc/passwd', + '/tmp/awf-docker-host-stage/etc/group', + '/tmp/awf-docker-host-stage/identity/passwd', + '/tmp/awf-docker-host-stage/identity/group', +]; + +describe('nested mountpoint preparation', () => { + let tmpRoot: string; + + beforeEach(() => { + tmpRoot = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), 'awf-nested-'))); + }); + + afterEach(() => { + fs.rmSync(tmpRoot, { recursive: true, force: true }); + }); + + describe('planNestedMountpoints', () => { + it('reports nothing when every covering bind is writable', () => { + const volumes = [ + '/host/home/runner:/host/home/runner:rw', + '/logs:/host/home/runner/.copilot/logs:rw', + ]; + + expect(planNestedMountpoints(volumes)).toEqual([]); + }); + + it('reports the mountpoint a read-only cover cannot create', () => { + const emptyHome = path.join(tmpRoot, 'chroot-home'); + const logs = path.join(tmpRoot, 'agent-logs'); + [emptyHome, logs].forEach((dir) => fs.mkdirSync(dir, { recursive: true })); + const volumes = [ + `${emptyHome}:/host/home/runner:ro`, + `${logs}:/host/home/runner/.copilot/logs:rw`, + ]; + + expect(planNestedMountpoints(volumes)).toEqual([ + expect.objectContaining({ + containerTarget: '/host/home/runner/.copilot/logs', + coveringTarget: '/host/home/runner', + coveringSource: emptyHome, + hostPath: path.join(emptyHome, '.copilot/logs'), + kind: 'directory', + credentialOverlay: false, + }), + ]); + }); + + it('cannot classify a source that this filesystem cannot see at all', () => { + const volumes = [ + '/empty-home:/host/home/runner:ro', + '/daemon-only/logs:/host/home/runner/.copilot/logs:rw', + ]; + // Under --docker-host-path-prefix a source can belong to the daemon's + // filesystem, not the runner's. There is nothing here to stat, so the kind + // is reported as unknown rather than assumed. A source that is merely + // absent is different: the daemon materialises those as directories, and + // `resolveSourceKind` classifies them accordingly. + const resolver = (source: string): string | undefined => + source.startsWith('/daemon-only/') ? undefined : source; + + expect(planNestedMountpoints(volumes, resolver)[0]).toEqual( + expect.objectContaining({ kind: 'unknown', credentialOverlay: false }), + ); + }); + + it('attributes the mountpoint to the innermost cover, not the outermost', () => { + const volumes = [ + '/empty-home:/host/home/runner:ro', + '/real-copilot:/host/home/runner/.copilot:ro', + '/logs:/host/home/runner/.copilot/logs:rw', + ]; + + const requirement = planNestedMountpoints(volumes).find( + (candidate) => candidate.containerTarget === '/host/home/runner/.copilot/logs', + ); + + expect(requirement?.hostPath).toBe('/real-copilot/logs'); + }); + + it('classifies /dev/null credential overlays as files, not directories', () => { + const volumes = [ + '/empty-home:/host/home/runner:ro', + '/dev/null:/host/home/runner/.netrc:ro', + ]; + + expect(planNestedMountpoints(volumes)).toEqual([ + expect.objectContaining({ containerTarget: '/host/home/runner/.netrc', kind: 'file' }), + ]); + }); + + it('reports no host path when the cover is a daemon-side custom mount', () => { + const resolver = createLocalSourceResolver(new Map(), '/daemon'); + const volumes = [ + '/daemon/mnt:/host/home/runner:ro', + '/logs:/host/home/runner/.copilot/logs:rw', + ]; + + expect(planNestedMountpoints(volumes, resolver)[0].hostPath).toBeUndefined(); + }); + + // GitHub-hosted runners bind `/opt` read-only and nest the tool cache + // inside it, so a requirement exists even with no write policy. It is + // already satisfied — source and mountpoint are the same host path — which + // is why this has always worked and must keep costing nothing. + it('reports already-satisfied system binds without asking for preparation', () => { + const volumes = [ + '/opt:/host/opt:ro', + '/opt/hostedtoolcache:/host/opt/hostedtoolcache:rw', + ]; + + const [requirement] = planNestedMountpoints(volumes); + expect(requirement.containerTarget).toBe('/host/opt/hostedtoolcache'); + // The mountpoint resolves to the mount's own source, so it exists exactly + // when the mount does and never needs preparing. + expect(requirement.hostPath).toBe(requirement.source); + }); + }); + + describe('ensureNestedMountpoints', () => { + const uid = process.getuid?.() ?? 0; + const gid = process.getgid?.() ?? 0; + + function makeTree() { + const emptyHome = path.join(tmpRoot, 'chroot-home'); + const logs = path.join(tmpRoot, 'agent-logs'); + const sessionState = path.join(tmpRoot, 'agent-session-state'); + [emptyHome, logs, sessionState].forEach((dir) => fs.mkdirSync(dir, { recursive: true })); + return { emptyHome, logs, sessionState }; + } + + it('creates the nested .copilot mountpoints runc could not create itself', () => { + const { emptyHome, logs, sessionState } = makeTree(); + const volumes = [ + `${emptyHome}:/host/home/runner:ro`, + `${logs}:/host/home/runner/.copilot/logs:rw`, + `${sessionState}:/host/home/runner/.copilot/session-state:rw`, + ]; + + expect(simulateRuncMountFailures(volumes)).toHaveLength(2); + + const created = ensureNestedMountpoints(volumes, uid, gid); + + expect(created).toEqual([ + path.join(emptyHome, '.copilot/logs'), + path.join(emptyHome, '.copilot/session-state'), + ]); + expect(simulateRuncMountFailures(volumes)).toEqual([]); + }); + + it('prepares mountpoints inside a real ~/.copilot cover as well as an empty home', () => { + const { logs } = makeTree(); + const realCopilot = path.join(tmpRoot, 'home', '.copilot'); + fs.mkdirSync(realCopilot, { recursive: true }); + + const volumes = [ + `${path.join(tmpRoot, 'home')}:/host/home/runner:ro`, + `${realCopilot}:/host/home/runner/.copilot:ro`, + `${logs}:/host/home/runner/.copilot/logs:rw`, + ]; + + ensureNestedMountpoints(volumes, uid, gid); + + expect(fs.existsSync(path.join(realCopilot, 'logs'))).toBe(true); + expect(simulateRuncMountFailures(volumes)).toEqual([]); + }); + + it('creates nothing when no write policy narrowed a cover to read-only', () => { + const { emptyHome, logs } = makeTree(); + const volumes = [ + `${emptyHome}:/host/home/runner:rw`, + `${logs}:/host/home/runner/.copilot/logs:rw`, + ]; + + expect(ensureNestedMountpoints(volumes, uid, gid)).toEqual([]); + expect(fs.existsSync(path.join(emptyHome, '.copilot'))).toBe(false); + }); + + it('never fabricates a credential file for a /dev/null overlay', () => { + const { emptyHome } = makeTree(); + const volumes = [ + `${emptyHome}:/host/home/runner:ro`, + '/dev/null:/host/home/runner/.netrc:ro', + ]; + + // The real pipeline prunes unmountable overlays first, which is what makes + // the mask safe to drop: the path is unreachable behind a read-only bind, + // so nothing is left unmasked. + const pruned = pruneUnmountableCredentialOverlays(volumes); + expect(pruned).not.toContain('/dev/null:/host/home/runner/.netrc:ro'); + expect(ensureNestedMountpoints(pruned, uid, gid)).toEqual([]); + expect(fs.existsSync(path.join(emptyHome, '.netrc'))).toBe(false); + }); + + it('refuses to launch rather than create a credential mountpoint itself', () => { + const { emptyHome } = makeTree(); + // Same list, but without the prune step: AWF must not quietly paper over + // an overlay it cannot satisfy, and it must not create the credential path. + const volumes = [ + `${emptyHome}:/host/home/runner:ro`, + '/dev/null:/host/home/runner/.netrc:ro', + ]; + + expect(() => ensureNestedMountpoints(volumes, uid, gid)).toThrow(/must not create/); + expect(fs.existsSync(path.join(emptyHome, '.netrc'))).toBe(false); + }); + + it('creates a directory for a source the daemon has not materialised yet', () => { + const { emptyHome } = makeTree(); + // AWF creates the init signal directory after the volume list is built, + // so at this point the source legitimately does not exist. This is the + // real shape of the agent's legacy `/tmp/awf-init` bind under a narrowed + // `/tmp`, and it must not be mistaken for an unclassifiable source. + const notYetCreated = path.join(tmpRoot, 'init-signal'); + const volumes = [ + `${emptyHome}:/host/home/runner:ro`, + `${notYetCreated}:/host/home/runner/.copilot/logs:rw`, + ]; + + expect(() => ensureNestedMountpoints(volumes, uid, gid)).not.toThrow(); + const mountpoint = path.join(emptyHome, '.copilot/logs'); + expect(fs.statSync(mountpoint).isDirectory()).toBe(true); + }); + + it('fails closed on a required mountpoint whose source cannot be classified', () => { + const { emptyHome } = makeTree(); + const volumes = [ + `${emptyHome}:/host/home/runner:ro`, + `/daemon-only/opaque:/host/home/runner/.copilot/logs:rw`, + ]; + // A daemon-side source we never staged: there is nothing on this + // filesystem to inspect. Guessing could create the wrong node type, and + // skipping it silently is what produced the opaque EROFS this pass exists + // to prevent. + const resolver = (source: string): string | undefined => + source.startsWith('/daemon-only/') ? undefined : source; + + expect(() => ensureNestedMountpoints(volumes, uid, gid, resolver)) + .toThrow(/could not be classified/); + expect(fs.existsSync(path.join(emptyHome, '.copilot'))).toBe(false); + }); + + it('creates a file mountpoint for a regular-file bind under a read-only cover', () => { + const { emptyHome } = makeTree(); + const sourceFile = path.join(tmpRoot, 'runner-binary'); + fs.writeFileSync(sourceFile, '#!/bin/sh\n', { mode: 0o755 }); + const volumes = [ + `${emptyHome}:/host/home/runner:ro`, + `${sourceFile}:/host/home/runner/bin/tool:ro`, + ]; + + const created = ensureNestedMountpoints(volumes, uid, gid); + + const mountpoint = path.join(emptyHome, 'bin/tool'); + expect(created).toEqual([mountpoint]); + // A single stat answers both questions: an empty regular file. The + // placeholder is never written to, so its size is the assertion. + const placeholder = fs.statSync(mountpoint); + expect(placeholder.isFile()).toBe(true); + expect(placeholder.size).toBe(0); + // Owner-only: these paths can land in a world-writable directory, and a + // bind does not need its mountpoint to be readable by anyone else. + expect(placeholder.mode & 0o077).toBe(0); + // The parent directory has to be created too, or the file cannot land. + expect(fs.statSync(path.join(emptyHome, 'bin')).isDirectory()).toBe(true); + }); + + it('refuses a symlink planted where a file mountpoint belongs', () => { + const { emptyHome } = makeTree(); + const sourceFile = path.join(tmpRoot, 'runner-binary'); + fs.writeFileSync(sourceFile, '#!/bin/sh\n', { mode: 0o755 }); + fs.mkdirSync(path.join(emptyHome, 'bin'), { recursive: true }); + // A dangling symlink needs no race to plant: existsSync() follows it and + // reports false, so preparation proceeds, and an exclusive create then + // fails with EEXIST. Returning quietly there would hand runc an + // attacker-chosen mountpoint in a world-writable directory. + fs.symlinkSync('/nonexistent-target', path.join(emptyHome, 'bin/tool')); + const volumes = [ + `${emptyHome}:/host/home/runner:ro`, + `${sourceFile}:/host/home/runner/bin/tool:ro`, + ]; + + expect(() => ensureNestedMountpoints(volumes, uid, gid)).toThrow(/symlink/i); + // The plant is reported, never followed and never replaced. + expect(fs.lstatSync(path.join(emptyHome, 'bin/tool')).isSymbolicLink()).toBe(true); + }); + + it('refuses a directory planted where a file mountpoint belongs', () => { + const { emptyHome } = makeTree(); + const sourceFile = path.join(tmpRoot, 'runner-binary'); + fs.writeFileSync(sourceFile, '#!/bin/sh\n', { mode: 0o755 }); + fs.mkdirSync(path.join(emptyHome, 'bin/tool'), { recursive: true }); + const volumes = [ + `${emptyHome}:/host/home/runner:ro`, + `${sourceFile}:/host/home/runner/bin/tool:ro`, + ]; + + // A directory standing in for a file is not a usable mountpoint: the + // daemon rejects the bind with "not a directory" once the run is under + // way, which is far later and far more opaque than failing here. + expect(() => ensureNestedMountpoints(volumes, uid, gid)).toThrow(/not a regular file/i); + }); + + it('leaves an existing file mountpoint untouched', () => { + const { emptyHome } = makeTree(); + const sourceFile = path.join(tmpRoot, 'runner-binary'); + fs.writeFileSync(sourceFile, '#!/bin/sh\n', { mode: 0o755 }); + fs.mkdirSync(path.join(emptyHome, 'bin'), { recursive: true }); + fs.writeFileSync(path.join(emptyHome, 'bin/tool'), 'PRE-EXISTING'); + const volumes = [ + `${emptyHome}:/host/home/runner:ro`, + `${sourceFile}:/host/home/runner/bin/tool:ro`, + ]; + + expect(ensureNestedMountpoints(volumes, uid, gid)).toEqual([]); + expect(fs.readFileSync(path.join(emptyHome, 'bin/tool'), 'utf8')).toBe('PRE-EXISTING'); + }); + + it('skips daemon-side covers instead of creating a runner-local tree', () => { + const { logs } = makeTree(); + // A *daemon-only* prefix. This deliberately does not use the suite's own + // tmpRoot: under `TMPDIR=/tmp` that is a shared prefix, which is runner + // resolvable by design, so the test would assert the opposite of its name + // on Linux while still passing on macOS (where realpath yields + // /private/var/...). + const daemonHome = '/daemon-only/home/runner'; + const resolver = createLocalSourceResolver(new Map(), '/daemon-only'); + const volumes = [ + `${daemonHome}:/host/home/runner:ro`, + `${logs}:/host/home/runner/.copilot/logs:rw`, + ]; + + expect(ensureNestedMountpoints(volumes, uid, gid, resolver)).toEqual([]); + // The cover was left alone rather than fabricated on this filesystem. + expect(fs.existsSync('/daemon-only')).toBe(false); + }); + }); + + describe('buildAgentVolumes (end-to-end topology)', () => { + // Narrowing /tmp makes the legacy init bind materialise the shared + // /tmp/awf-init mountpoint. That root is intentionally left in place; see + // makeFixedTmpArtifactTracker for why removing it is unsafe. + function stageRunnerLayout() { + const home = path.join(tmpRoot, 'home', 'runner'); + const workspaceDir = path.join(home, 'work', 'repo', 'repo'); + const workDir = path.join(tmpRoot, 'awf-run'); + const emptyHome = `${workDir}-chroot-home`; + const agentLogsPath = path.join(workDir, 'agent-logs'); + const sessionStatePath = path.join(workDir, 'agent-session-state'); + const initSignalDir = path.join(workDir, 'init-signal'); + + [workspaceDir, workDir, emptyHome, agentLogsPath, sessionStatePath, initSignalDir] + .forEach((dir) => fs.mkdirSync(dir, { recursive: true })); + // Mirror prepareChrootHomeMounts: host tool dirs plus their chroot placeholders. + for (const toolPath of HOME_TOOL_PATHS) { + fs.mkdirSync(path.join(home, toolPath), { recursive: true }); + fs.mkdirSync(path.join(emptyHome, toolPath), { recursive: true }); + } + fs.mkdirSync(path.join(emptyHome, 'work', 'repo', 'repo'), { recursive: true }); + + const config = { + agentCommand: 'true', + allowedDomains: [], + workDir, + volumeMounts: [], + } as unknown as WrapperConfig; + + return { + home, + workspaceDir, + emptyHome, + build: (filesystemAllowWrite?: string[]) => buildAgentVolumes({ + config: { ...config, filesystemAllowWrite } as WrapperConfig, + projectRoot: process.cwd(), + effectiveHome: home, + workspaceDir, + agentLogsPath, + sessionStatePath, + initSignalDir, + }), + }; + } + + it('leaves every nested mountpoint satisfiable when a write policy narrows the home tree', () => { + const layout = stageRunnerLayout(); + const writable = path.join(layout.workspaceDir, 'allowed'); + fs.mkdirSync(writable, { recursive: true }); + + const volumes = layout.build([writable]); + + expect(volumes.some((spec) => spec.includes('/.copilot/logs:rw'))).toBe(true); + expect(simulateRuncMountFailures(volumes)).toEqual([]); + }); + + it('creates the .copilot log and session-state mountpoints runc cannot', () => { + const layout = stageRunnerLayout(); + const writable = path.join(layout.workspaceDir, 'allowed'); + fs.mkdirSync(writable, { recursive: true }); + + layout.build([writable]); + + // The real ~/.copilot bind is the innermost cover for both nested mounts. + expect(fs.existsSync(path.join(layout.home, '.copilot/logs'))).toBe(true); + expect(fs.existsSync(path.join(layout.home, '.copilot/session-state'))).toBe(true); + }); + + it('changes nothing on disk and needs no preparation without a policy', () => { + const layout = stageRunnerLayout(); + + const volumes = layout.build(undefined); + + // `ensureNestedMountpoints` only ever creates a requirement's `hostPath`, + // and that is always `coveringSource` + suffix. No requirement is covered + // by an AWF-owned bind, so nothing in the staged tree can have been + // created... + const staged = planNestedMountpoints(volumes) + .filter((requirement) => requirement.coveringSource.startsWith(tmpRoot)); + expect(staged).toEqual([]); + expect(fs.existsSync(path.join(layout.home, '.copilot/logs'))).toBe(false); + expect(fs.existsSync(path.join(layout.home, '.copilot/session-state'))).toBe(false); + // ...and every remaining requirement is a system bind AWF has always + // emitted (on a GitHub-hosted runner `/opt` covers `/opt/hostedtoolcache`) + // whose mountpoint already exists, so nothing was created there either. + expect(simulateRuncMountFailures(volumes)).toEqual([]); + }); + }); + + // A regular-file bind nested inside a read-only cover needs a *file* + // mountpoint. runc cannot create one inside a read-only bind any more than it + // can create a directory, so this has to be prepared too. It is reachable on + // split-filesystem runners: the agent binary is staged under the daemon path + // prefix and published at /tmp/awf-runner-bin/, while a write policy + // narrows the /tmp:/tmp bind to read-only. + describe('buildAgentVolumes (staged runner binary under --docker-host-path-prefix)', () => { + const prefixRoots: string[] = []; + const runnerBinPaths: string[] = []; + // This case narrows /tmp too, so it also materialises the fixed + // /tmp/awf-init and /tmp/awf-runner-bin mountpoints. + const fixedTmp = makeFixedTmpArtifactTracker(FIXED_TMP_STAGED_COPIES); + + afterEach(() => { + // Staging and the /tmp cover are both real paths by construction: the + // staging root has to sit under the daemon prefix, and the cover bind is + // hardcoded to /tmp. + prefixRoots.splice(0).forEach((dir) => fs.rmSync(dir, { recursive: true, force: true })); + runnerBinPaths.splice(0).forEach((file) => fs.rmSync(file, { force: true })); + fixedTmp.cleanup(); + }); + + function stageSplitFsLayout() { + const unique = path.basename(tmpRoot).replace(/[^a-zA-Z0-9]/g, ''); + const binaryName = `awfprobe${unique}`; + // shouldUseDockerHostStaging only engages for a prefix under /tmp, so this + // cannot be redirected into the test's own sandbox. + const dockerHostPathPrefix = `/tmp/awf-prefix-${unique}`; + prefixRoots.push(dockerHostPathPrefix); + runnerBinPaths.push(path.join('/tmp/awf-runner-bin', binaryName)); + + const home = path.join(tmpRoot, 'home', 'runner'); + const workspaceDir = path.join(home, 'work', 'repo', 'repo'); + const workDir = path.join(tmpRoot, 'awf-run'); + const emptyHome = `${workDir}-chroot-home`; + const agentLogsPath = path.join(workDir, 'agent-logs'); + const sessionStatePath = path.join(workDir, 'agent-session-state'); + const initSignalDir = path.join(workDir, 'init-signal'); + const binDir = path.join(tmpRoot, 'runner-bin'); + + [workspaceDir, workDir, emptyHome, agentLogsPath, sessionStatePath, initSignalDir, binDir] + .forEach((dir) => fs.mkdirSync(dir, { recursive: true })); + for (const toolPath of HOME_TOOL_PATHS) { + fs.mkdirSync(path.join(home, toolPath), { recursive: true }); + fs.mkdirSync(path.join(emptyHome, toolPath), { recursive: true }); + } + fs.mkdirSync(path.join(emptyHome, 'work', 'repo', 'repo'), { recursive: true }); + + const binarySourcePath = path.join(binDir, binaryName); + fs.writeFileSync(binarySourcePath, '#!/bin/sh\n', { mode: 0o755 }); + + fixedTmp.noteBeforeBuild(); + + const config = { + agentCommand: binarySourcePath, + allowedDomains: [], + workDir, + volumeMounts: [], + dockerHostPathPrefix, + } as unknown as WrapperConfig; + + return { + binaryName, + workspaceDir, + build: (filesystemAllowWrite?: string[]) => { + const volumes = buildAgentVolumes({ + config: { ...config, filesystemAllowWrite } as WrapperConfig, + projectRoot: process.cwd(), + effectiveHome: home, + workspaceDir, + agentLogsPath, + sessionStatePath, + initSignalDir, + }); + fixedTmp.trackGenerated(volumes, binaryName); + return volumes; + }, + }; + } + + it('classifies the staged binary mountpoint as a file, not a directory', () => { + const layout = stageSplitFsLayout(); + const writable = path.join(layout.workspaceDir, 'allowed'); + fs.mkdirSync(writable, { recursive: true }); + + const volumes = layout.build([writable]); + const requirement = planNestedMountpoints(volumes) + .find((candidate) => candidate.containerTarget.endsWith(`/awf-runner-bin/${layout.binaryName}`)); + + expect(requirement).toBeDefined(); + expect(requirement?.kind).toBe('file'); + }); + + it('creates the file mountpoint runc cannot create under a read-only /tmp', () => { + const layout = stageSplitFsLayout(); + const writable = path.join(layout.workspaceDir, 'allowed'); + fs.mkdirSync(writable, { recursive: true }); + + const volumes = layout.build([writable]); + + const mountpoint = path.join('/tmp/awf-runner-bin', layout.binaryName); + // An empty placeholder: it exists only so runc has something to bind over. + const placeholder = fs.statSync(mountpoint); + expect(placeholder.isFile()).toBe(true); + expect(placeholder.size).toBe(0); + // The bind really is published, and the /tmp cover really is read-only — + // otherwise this test would pass without exercising anything. + expect(volumes.some((spec) => spec.endsWith(':/tmp:ro'))).toBe(true); + expect(volumes.some((spec) => spec.endsWith(`:/tmp/awf-runner-bin/${layout.binaryName}:ro`))) + .toBe(true); + }); + }); + + // A `--docker-host-path-prefix` of exactly /tmp is *shared*: the runner and + // the daemon see the same paths there, which is why prefix translation leaves + // an already-/tmp source unrewritten. The run's own workDir then sits *inside* + // the prefix, so topology passes still have to resolve it locally. (A /tmp + // *descendant* such as /tmp/gh-aw is daemon-only and must keep failing + // closed — covered in mount-topology.test.ts.) + describe('buildAgentVolumes (--docker-host-path-prefix shares /tmp with the runner)', () => { + const sharedRoots: string[] = []; + const fixedTmp = makeFixedTmpArtifactTracker(FIXED_TMP_STAGED_COPIES); + + afterEach(() => { + sharedRoots.splice(0).forEach((dir) => fs.rmSync(dir, { recursive: true, force: true })); + fixedTmp.cleanup(); + }); + + function stageSharedTmpLayout() { + // The prefix under test is the literal /tmp, and the whole run has to sit + // beneath it, so this cannot be redirected into the suite's sandbox (on + // macOS os.tmpdir() is /private/var/... and would not nest). mkdtemp + // rather than a predictable name, so nothing here can be pre-created or + // redirected by another user on a shared machine. + const sharedRoot = fs.mkdtempSync('/tmp/awf-shared-'); + sharedRoots.push(sharedRoot); + const unique = path.basename(sharedRoot).replace(/[^a-zA-Z0-9]/g, ''); + + const home = path.join(sharedRoot, 'home', 'runner'); + const workspaceDir = path.join(home, 'work', 'repo', 'repo'); + const workDir = path.join(sharedRoot, 'awf-run'); + const emptyHome = `${workDir}-chroot-home`; + const agentLogsPath = path.join(workDir, 'agent-logs'); + const sessionStatePath = path.join(workDir, 'agent-session-state'); + const initSignalDir = path.join(workDir, 'init-signal'); + const binDir = path.join(sharedRoot, 'runner-bin'); + + [workspaceDir, workDir, emptyHome, agentLogsPath, sessionStatePath, initSignalDir, binDir] + .forEach((dir) => fs.mkdirSync(dir, { recursive: true })); + for (const toolPath of HOME_TOOL_PATHS) { + fs.mkdirSync(path.join(home, toolPath), { recursive: true }); + fs.mkdirSync(path.join(emptyHome, toolPath), { recursive: true }); + } + fs.mkdirSync(path.join(emptyHome, 'work', 'repo', 'repo'), { recursive: true }); + + // A /tmp-rooted prefix also turns on ARC/DinD binary staging, which + // publishes to fixed paths named after the command. A unique name keeps + // this run's artefacts distinguishable from a concurrent suite's, and + // means they can never be mistaken for pre-existing ones. + const binaryName = `awfshared${unique}`; + const binarySourcePath = path.join(binDir, binaryName); + fs.writeFileSync(binarySourcePath, '#!/bin/sh\n', { mode: 0o755 }); + + fixedTmp.noteBeforeBuild(); + + const config = { + agentCommand: binarySourcePath, + allowedDomains: [], + workDir, + volumeMounts: [], + dockerHostPathPrefix: '/tmp', + } as unknown as WrapperConfig; + + return { + workspaceDir, + initSignalDir, + build: (filesystemAllowWrite?: string[]) => { + const volumes = buildAgentVolumes({ + config: { ...config, filesystemAllowWrite } as WrapperConfig, + projectRoot: process.cwd(), + effectiveHome: home, + workspaceDir, + agentLogsPath, + sessionStatePath, + initSignalDir, + }); + fixedTmp.trackGenerated(volumes, binaryName); + return volumes; + }, + }; + } + + function stageWritable(layout: ReturnType): string { + const writable = path.join(layout.workspaceDir, 'allowed'); + fs.mkdirSync(writable, { recursive: true }); + return writable; + } + + /** + * Plans against the resolver the production pipeline actually builds for + * this config. `planNestedMountpoints` defaults to an identity resolver, + * which would resolve every source regardless of prefix handling and so + * could not detect a regression in `createLocalSourceResolver` at all. + */ + function planWithProductionResolver(volumes: string[]) { + return planNestedMountpoints(volumes, createLocalSourceResolver(new Map(), '/tmp')); + } + + it('resolves a workDir nested inside the prefix instead of failing closed', () => { + const layout = stageSharedTmpLayout(); + const writable = stageWritable(layout); + + expect(() => layout.build([writable])).not.toThrow(); + }); + + it('classifies the legacy init signal mountpoint nested under a narrowed /tmp', () => { + const layout = stageSharedTmpLayout(); + const volumes = layout.build([stageWritable(layout)]); + + const legacy = planWithProductionResolver(volumes) + .find((requirement) => requirement.containerTarget === '/tmp/awf-init'); + + // The source is the run's own init-signal directory, which the CLI just + // created locally — being under the shared prefix must not make it + // unclassifiable. Asserted on the plan rather than on disk: a + // /tmp/awf-init left behind by another suite would otherwise satisfy an + // existsSync check without this code path ever running. + expect(legacy?.source).toBe(layout.initSignalDir); + expect(legacy?.kind).toBe('directory'); + expect(legacy?.hostPath).toBe('/tmp/awf-init'); + }); + + it('prepares the nested home mountpoints when the chroot home is under the prefix', () => { + const layout = stageSharedTmpLayout(); + const volumes = layout.build([stageWritable(layout)]); + + const nestedHomeMounts = planWithProductionResolver(volumes) + .filter((requirement) => requirement.containerTarget.includes('/.copilot/')); + + // Guards against passing vacuously: the policy really does narrow a home + // bind that covers these, so there is something to prepare. + expect(nestedHomeMounts.length).toBeGreaterThan(0); + for (const requirement of nestedHomeMounts) { + // An unresolvable covering source leaves hostPath undefined, which + // silently skips preparation and restores the EROFS failure. + expect(requirement.hostPath).toBeDefined(); + expect(fs.existsSync(requirement.hostPath as string)).toBe(true); + } + }); + + it('leaves no unsatisfiable mountpoint for runc', () => { + const layout = stageSharedTmpLayout(); + const volumes = layout.build([stageWritable(layout)]); + const requirements = planWithProductionResolver(volumes); + + // Only the mounts landing inside the guest's /tmp are asserted here. + // ensureNestedMountpoints runs on the topology *before* the prefix is + // applied, so re-planning the returned list is only faithful where the + // prefix changes nothing -- which is exactly the already-/tmp-rooted + // paths this case is about. Anything else is rewritten: on a GitHub + // runner /opt/hostedtoolcache becomes the daemon-only + // /tmp/opt/hostedtoolcache, which no runner-local check can resolve. + const guestTmpMounts = requirements.filter((requirement) => + requirement.containerTarget.startsWith('/tmp/')); + + expect(guestTmpMounts.length).toBeGreaterThan(0); + for (const requirement of guestTmpMounts) { + expect(requirement.kind).not.toBe('unknown'); + expect(requirement.hostPath).toBeDefined(); + expect(fs.existsSync(requirement.hostPath as string)).toBe(true); + } + }); + }); +}); diff --git a/src/services/agent-volumes/nested-mountpoints.ts b/src/services/agent-volumes/nested-mountpoints.ts new file mode 100644 index 000000000..4d3ebb7b2 --- /dev/null +++ b/src/services/agent-volumes/nested-mountpoints.ts @@ -0,0 +1,295 @@ +import * as fs from 'fs'; +import * as path from 'path'; +import { logger } from '../../logger'; +import { createMissingOwnedDirectorySegments } from '../../fs-utils'; +import { isStagedHostFile } from './docker-host-staging'; +import { + LocalSourceResolver, + ParsedMount, + identityLocalSourceResolver, + innermostCoveringMount, + parseMount, +} from './mount-topology'; + +export interface NestedMountpointRequirement { + /** Container path whose mountpoint runc must create. */ + containerTarget: string; + /** Source of the mount that needs the mountpoint. */ + source: string; + /** Target of the read-only bind that owns the directory entry. */ + coveringTarget: string; + /** Source of that covering bind, as written into the compose file. */ + coveringSource: string; + /** Path on the runner that must exist so runc does not have to create it. */ + hostPath?: string; + /** + * What runc needs at the mountpoint. A bind's target must match its source's + * type, so this is derived from the source, not guessed from the target. + * `unknown` means the source could not be classified on this filesystem. + */ + kind: 'directory' | 'file' | 'unknown'; + /** + * `/dev/null` credential masks. These need a file mountpoint too, but AWF must + * never fabricate one: creating the credential path is exactly what the mask + * exists to prevent. Unmountable overlays are dropped upstream instead. + */ + credentialOverlay: boolean; +} + +function statKind(candidate: string): 'directory' | 'file' | 'missing' | 'unknown' { + try { + const stats = fs.statSync(candidate); + if (stats.isDirectory()) return 'directory'; + if (stats.isFile()) return 'file'; + return 'unknown'; + } catch (err) { + if ((err as NodeJS.ErrnoException).code === 'ENOENT') return 'missing'; + return 'unknown'; + } +} + +function resolveSourceKind( + source: string, + localSource: string | undefined, +): 'directory' | 'file' | 'unknown' { + // Staged files sit under the daemon prefix, so `localSource` is deliberately + // undefined for them. The staging record is the only surviving evidence of + // their type. + if (isStagedHostFile(source)) return 'file'; + // No local path at all: a daemon-side source we cannot inspect and did not + // stage. Guessing here is what the fail-closed path exists to prevent. + if (localSource === undefined) return 'unknown'; + + const kind = statKind(localSource); + // A source that simply does not exist yet is not ambiguous. Some AWF-owned + // sources (the init signal directory among them) are materialised after the + // volume list is built, and the daemon creates a *directory* for any bind + // source still missing at launch — so the mountpoint has to be a directory to + // match. Verified against a real daemon: binding a missing source yields a + // directory on both the host and inside the guest. + if (kind === 'missing') return 'directory'; + return kind; +} + +/** + * Derives every mountpoint runc would have to create inside a read-only bind. + * + * runc applies binds parent-first and creates a missing mountpoint with + * `mkdirat` against the *destination*, which resolves into whichever bind + * already covers that path. While every covering bind is read-write this always + * succeeds, which is why the nesting went unnoticed. As soon as + * `filesystem.allowWrite` narrows a covering bind to read-only the same + * `mkdirat` returns EROFS and container init dies before the agent runs. + * + * Deriving the list from the final, policy-applied volumes (rather than from a + * hand-maintained list of known nested paths) means any internal mount added + * later is covered automatically. + * + * Pure: this only reports requirements, it does not touch the filesystem. + */ +export function planNestedMountpoints( + volumes: string[], + localSourceResolver: LocalSourceResolver = identityLocalSourceResolver, +): NestedMountpointRequirement[] { + const mounts = volumes + .map(parseMount) + .filter((mount): mount is ParsedMount => mount !== undefined); + const binds = mounts.filter((mount) => mount.source !== '/dev/null'); + + const requirements: NestedMountpointRequirement[] = []; + for (const mount of mounts) { + const cover = innermostCoveringMount(binds, mount.target); + // No covering bind: the mountpoint lives on the container's own writable + // rootfs and runc can always create it. + if (!cover) continue; + // A read-write cover can still create its own mountpoints. + if (cover.mode !== 'ro') continue; + if (!cover.source.startsWith('/')) continue; + + const suffix = mount.target.slice(cover.target.length); + if (!suffix) continue; + + const localCoverSource = localSourceResolver(cover.source); + const credentialOverlay = mount.source === '/dev/null'; + requirements.push({ + containerTarget: mount.target, + source: mount.source, + coveringTarget: cover.target, + coveringSource: cover.source, + hostPath: localCoverSource === undefined ? undefined : `${localCoverSource}${suffix}`, + kind: credentialOverlay + ? 'file' + : resolveSourceKind(mount.source, localSourceResolver(mount.source)), + credentialOverlay, + }); + } + + return requirements; +} + +function isExistingDirectory(candidate: string): boolean { + try { + return fs.statSync(candidate).isDirectory(); + } catch { + return false; + } +} + +/** + * Creates an empty file for runc to bind over. + * + * Deliberately exclusive (`wx`): if something already occupies the path we must + * not truncate it. Contents are never written — the bind replaces the file's + * contents wholesale, so the placeholder only has to exist and be of the right + * type. The mode is owner-only because these paths can land in a world-writable + * directory (`/tmp/awf-runner-bin` under a narrowed `/tmp`), and a bind does not + * care about its mountpoint's permissions. + * + * An exclusive create is also what makes the symlink check below reachable + * rather than theoretical: `O_CREAT | O_EXCL` refuses to follow a symlink, so a + * planted one surfaces as `EEXIST` instead of being silently written through. + */ +function createOwnedPlaceholderFile(filePath: string, uid: number, gid: number): void { + let handle: number | undefined; + try { + handle = fs.openSync(filePath, 'wx', 0o600); + } catch (err) { + if ((err as NodeJS.ErrnoException).code === 'EEXIST') { + assertUsableFileMountpoint(filePath); + return; + } + throw err; + } finally { + if (handle !== undefined) fs.closeSync(handle); + } + + try { + fs.chownSync(filePath, uid, gid); + } catch (err) { + // Ownership is a convenience for the in-container user; the mountpoint + // itself is what runc needs, and the bind masks the placeholder entirely. + logger.debug(`Could not chown placeholder mountpoint ${filePath}: ${err}`); + } +} + +/** + * Accepts an entry that appeared underneath us only if runc could actually bind + * over it. + * + * Reaching here means the path was absent when preparation looked and present a + * moment later. A regular file is the benign explanation and is fine to reuse. + * Anything else is not: a symlink hands runc a target of someone else's + * choosing, and a directory makes the daemon reject the bind with "not a + * directory" once the run is already under way. Neither is repaired here — + * deleting a path we did not create is how a mountpoint helper turns into a + * destructive one — so both fail closed while the context is still useful. + */ +function assertUsableFileMountpoint(filePath: string): void { + const entry = fs.lstatSync(filePath); + if (entry.isSymbolicLink()) { + throw new Error(`Refusing to use symlink as bind mountpoint: ${filePath}`); + } + if (!entry.isFile()) { + throw new Error(`Bind mountpoint exists but is not a regular file: ${filePath}`); + } +} + +/** + * Creates the mountpoints reported by {@link planNestedMountpoints}. + * + * A requirement is *inert* only when the covering bind's source is not a real + * directory on this filesystem: that means a fabricated path or a daemon-side + * one, and in both cases there is nothing here we could or should write into. + * + * Everything else is *required*. A required mountpoint is either already + * present, or gets created as the same type as its source — a directory for a + * directory bind, an empty placeholder file for a regular-file bind. Nothing + * required is skipped silently: a requirement we cannot classify or cannot + * satisfy fails the launch, because the alternative is an opaque EROFS from + * container init long after the useful context is gone. + * + * The one thing never fabricated is a `/dev/null` credential mask. Creating the + * credential path is precisely what the mask exists to prevent, so unmountable + * overlays are dropped upstream by + * {@link ../agent-volumes/credential-hiding.pruneUnmountableCredentialOverlays}. + * That runs first and probes the same paths, so a surviving overlay always has + * an existing mountpoint; if one ever does not, that is a real inconsistency and + * is reported rather than papered over. + */ +export function ensureNestedMountpoints( + volumes: string[], + uid: number, + gid: number, + localSourceResolver: LocalSourceResolver = identityLocalSourceResolver, +): string[] { + const created: string[] = []; + const requirements = planNestedMountpoints(volumes, localSourceResolver); + const unmet: string[] = []; + + for (const requirement of requirements) { + const localCoverSource = localSourceResolver(requirement.coveringSource); + // Not a place we can write: fabricated or daemon-side cover. + if (localCoverSource === undefined || !isExistingDirectory(localCoverSource)) { + logger.debug( + `Skipping mountpoint preparation for ${requirement.containerTarget}: covering bind ` + + `source ${requirement.coveringSource} is not a directory on this filesystem`, + ); + continue; + } + + const { hostPath } = requirement; + // The cover resolved, so `planNestedMountpoints` always produced a hostPath. + if (hostPath === undefined) continue; + if (fs.existsSync(hostPath)) { + // An existing directory mountpoint is the overwhelmingly common case and + // is left exactly as it was found. A *file* mountpoint is different: the + // daemon rejects the bind outright if the entry is the wrong type, so the + // one kind this pass introduces is also the one kind worth checking. + if (requirement.kind === 'file' && !requirement.credentialOverlay) { + assertUsableFileMountpoint(hostPath); + } + continue; + } + + if (requirement.credentialOverlay) { + unmet.push( + `${requirement.containerTarget} (credential mask needs ${hostPath}, which AWF must not create)`, + ); + continue; + } + + if (requirement.kind === 'unknown') { + unmet.push( + `${requirement.containerTarget} (needs ${hostPath}, but source ${requirement.source} ` + + 'could not be classified as a file or a directory)', + ); + continue; + } + + if (requirement.kind === 'file') { + createMissingOwnedDirectorySegments(path.dirname(hostPath), uid, gid); + createOwnedPlaceholderFile(hostPath, uid, gid); + } else { + createMissingOwnedDirectorySegments(hostPath, uid, gid); + } + + created.push(hostPath); + logger.debug( + `Prepared nested ${requirement.kind} mountpoint ${hostPath} for ` + + `${requirement.containerTarget} (inside read-only bind ${requirement.coveringTarget})`, + ); + + if (!fs.existsSync(hostPath)) { + unmet.push(`${requirement.containerTarget} (needs ${hostPath})`); + } + } + + if (unmet.length > 0) { + throw new Error( + `Could not prepare bind mountpoints nested inside a read-only mount: ${unmet.join(', ')}. ` + + 'The agent container would fail to start with a read-only filesystem error.', + ); + } + + return created; +} diff --git a/src/services/agent-volumes/volume-builder.ts b/src/services/agent-volumes/volume-builder.ts index 7be25e840..13bb945f9 100644 --- a/src/services/agent-volumes/volume-builder.ts +++ b/src/services/agent-volumes/volume-builder.ts @@ -1,8 +1,11 @@ import { SslConfig } from '../../host-env'; +import { getSafeHostGid, getSafeHostUid } from '../../host-env'; import { logger } from '../../logger'; import { WrapperConfig } from '../../types'; import { applyHostPathPrefixToVolumes } from '../host-path-prefix'; -import { buildCredentialHidingOverlays } from './credential-hiding'; +import { buildCredentialHidingOverlays, pruneUnmountableCredentialOverlays } from './credential-hiding'; +import { createLocalSourceResolver } from './mount-topology'; +import { ensureNestedMountpoints } from './nested-mountpoints'; import { buildDockerSocketMount } from './docker-socket'; import { buildEtcMounts } from './etc-mounts'; import { buildHomeMounts } from './home-strategy'; @@ -66,6 +69,7 @@ export function buildAgentVolumes(params: AgentVolumesParams): string[] { const alwaysWritableMounts = new Set(agentVolumes.filter((spec) => spec.startsWith(`${agentLogsPath}:`) || spec.startsWith(`${sessionStatePath}:`) || + spec.startsWith(`${initSignalDir}:`) || spec === '/dev/null:/host/dev/null:rw' )); const customMounts = buildCustomVolumeMounts(config.volumeMounts, config.dockerHostPathPrefix); @@ -79,11 +83,31 @@ export function buildAgentVolumes(params: AgentVolumesParams): string[] { const localSourceRoots = new Map( customMounts.map((spec, index) => [spec, localCustomMounts[index]?.split(':')[0] ?? '']), ); - const policyVolumes = applyFilesystemWritePolicy( + // Same pairing, keyed by source root, so topology passes can map a daemon-side + // custom-mount source back to the runner path (or refuse to, and fail closed). + const customSourceRoots = new Map( + customMounts.map((spec, index) => [ + spec.split(':')[0] ?? '', + localCustomMounts[index]?.split(':')[0] ?? '', + ]), + ); + const localSourceResolver = createLocalSourceResolver(customSourceRoots, config.dockerHostPathPrefix); + + const policyVolumes = pruneUnmountableCredentialOverlays(applyFilesystemWritePolicy( agentVolumes, resolveComposeFilesystemAllowWrite(config), alwaysWritableMounts, localSourceRoots, + ), localSourceResolver); + + // A read-only bind cannot host a mountpoint runc still has to create, so any + // internal mount nested inside one must exist up front. Derived from the final + // topology, so mounts added later are covered without another targeted fix. + ensureNestedMountpoints( + policyVolumes, + parseInt(getSafeHostUid(), 10), + parseInt(getSafeHostGid(), 10), + localSourceResolver, ); if (config.dockerHostPathPrefix) { diff --git a/src/services/agent-volumes/workspace-mounts.test.ts b/src/services/agent-volumes/workspace-mounts.test.ts index e8a260f9e..93b2f8eb5 100644 --- a/src/services/agent-volumes/workspace-mounts.test.ts +++ b/src/services/agent-volumes/workspace-mounts.test.ts @@ -48,7 +48,7 @@ describe('buildWorkspaceMounts', () => { expect(mounts).toContain('/workspace:/workspace:rw'); expect(mounts).toContain('/tmp/awf-logs:/home/runner/.copilot/logs:rw'); expect(mounts).toContain('/tmp/awf-session:/home/runner/.copilot/session-state:rw'); - expect(mounts).toContain('/tmp/awf-init:/tmp/awf-init:rw'); + expect(mounts).toContain('/tmp/awf-init:/run/awf-init:rw'); }); }); diff --git a/src/services/agent-volumes/workspace-mounts.ts b/src/services/agent-volumes/workspace-mounts.ts index 4254fd21b..336418824 100644 --- a/src/services/agent-volumes/workspace-mounts.ts +++ b/src/services/agent-volumes/workspace-mounts.ts @@ -2,6 +2,7 @@ import * as fs from 'fs'; import * as path from 'path'; import { logger } from '../../logger'; import { WrapperConfig } from '../../types'; +import { INIT_SIGNAL_DIR, LEGACY_INIT_SIGNAL_DIR } from '../../constants'; import { applyHostPathPrefixToVolumes } from '../host-path-prefix'; import { extractCommandBinaryName, @@ -27,7 +28,13 @@ export function buildWorkspaceMounts(params: WorkspaceMountsParams): string[] { `${workspaceDir}:${workspaceDir}:rw`, `${agentLogsPath}:${effectiveHome}/.copilot/logs:rw`, `${sessionStatePath}:${effectiveHome}/.copilot/session-state:rw`, - `${initSignalDir}:/tmp/awf-init:rw`, + `${initSignalDir}:${INIT_SIGNAL_DIR}:rw`, + // Agent images released before the signal directory moved to /run wait on + // the legacy path instead. Exposing the same source there keeps a newer CLI + // working with an older pinned `--image-tag`. Read-only on purpose: the + // agent only polls for `ready`, and the init container writes through its + // own read-write mount at INIT_SIGNAL_DIR. + `${initSignalDir}:${LEGACY_INIT_SIGNAL_DIR}:ro`, ]; if (config.enableApiProxy) { diff --git a/src/services/host-path-prefix.ts b/src/services/host-path-prefix.ts index 32d6b90c6..be148634c 100644 --- a/src/services/host-path-prefix.ts +++ b/src/services/host-path-prefix.ts @@ -12,7 +12,12 @@ // squid, api-proxy, cli-proxy) so the rewrite is symmetric across services // that share daemon-side directories. -function normalizeDockerHostPathPrefix(prefix: string): string { +/** + * Canonical form of a `--docker-host-path-prefix` value: leading slash, no + * trailing slash. Exported so every pass compares against the same string — + * a raw `/host/` fails a `/host/`-prefix test that a normalised `/host` passes. + */ +export function normalizeDockerHostPathPrefix(prefix: string): string { const trimmed = prefix.trim(); if (!trimmed) return ''; const withLeadingSlash = trimmed.startsWith('/') ? trimmed : `/${trimmed}`; @@ -20,9 +25,53 @@ function normalizeDockerHostPathPrefix(prefix: string): string { return withoutTrailingSlash || '/'; } +/** + * Is this prefix a directory the runner and the Docker daemon see at the *same + * path*, rather than a daemon-only view of the runner's filesystem? + * + * Only the exact `/tmp` shape qualifies, and the reason is structural rather + * than conventional. AWF's own workDir (`/tmp/awf-`) then sits under the + * prefix, so `translateBindMountHostPath` leaves every one of its binds + * unrewritten — which can only work if the daemon resolves those paths to the + * same bytes. A prefix of `/tmp` is therefore a claim that /tmp is shared. + * + * Descendants are the opposite. `/tmp/gh-aw` is one of `dind-probe`'s + * CANDIDATE_PREFIXES, and that loop runs *only* after the probe has confirmed + * the daemon cannot see the runner's filesystem: it means the daemon sees + * runner path `X` at `/tmp/gh-aw/X`. `buildCustomVolumeMounts` says the same, + * translating sources that already start with `/tmp/gh-aw`. Treating such a + * prefix as shared would hand back a daemon-namespace path as if it were + * runner-local, skipping the fail-closed guard that exists to prevent exactly + * that. + * + * Note this is deliberately narrower than `isTmpRootedDockerHostPathPrefix`, + * which gates staging-root selection. Those two questions look alike but are + * not the same, so they must not share an answer. + */ +export function isSharedDockerHostPathPrefix(prefix: string | undefined): boolean { + if (!prefix) return false; + return normalizeDockerHostPathPrefix(prefix) === '/tmp'; +} + +/** + * Does this prefix select the /tmp-rooted staging layout introduced for ARC and + * DinD (see docs/arc-dind.md)? + * + * This gates *where AWF stages* chroot prerequisites, not whether a path can be + * attributed to the runner, so it keeps its original `/tmp`-or-below meaning. + * It is intentionally not narrowed to the shared case: doing so would silently + * disable binary and /etc staging for `/tmp/gh-aw` runners, which is precisely + * the topology that feature was built for. + */ +export function isTmpRootedDockerHostPathPrefix(prefix: string | undefined): boolean { + if (!prefix) return false; + const normalized = normalizeDockerHostPathPrefix(prefix); + return normalized === '/tmp' || normalized.startsWith('/tmp/'); +} + function shouldPreserveUnprefixedEtcIdentityFile(hostPath: string, dockerHostPathPrefix: string): boolean { return ( - (dockerHostPathPrefix === '/tmp' || dockerHostPathPrefix.startsWith('/tmp/')) && + isTmpRootedDockerHostPathPrefix(dockerHostPathPrefix) && (hostPath === '/etc/passwd' || hostPath === '/etc/group') ); } diff --git a/src/services/init-signal-compatibility.test.ts b/src/services/init-signal-compatibility.test.ts new file mode 100644 index 000000000..8fd0be535 --- /dev/null +++ b/src/services/init-signal-compatibility.test.ts @@ -0,0 +1,190 @@ +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { execFileSync } from 'child_process'; +import { generateDockerCompose, mockNetworkConfig, useAgentVolumesTestConfig } from './service-test-setup.test-utils'; +import { INIT_SIGNAL_DIR, LEGACY_INIT_SIGNAL_DIR } from '../constants'; + +// eslint-disable-next-line @typescript-eslint/no-require-imports +jest.mock('execa', () => require('../test-helpers/mock-execa.test-utils').execaMockFactory()); + +const { getConfig } = useAgentVolumesTestConfig(); + +const entrypointSource = fs.readFileSync( + path.resolve(__dirname, '../../containers/agent/entrypoint.sh'), + 'utf8', +); + +/** + * The init-signal handshake spans two independently versioned artefacts: the + * CLI, which decides where the ready-file is mounted, and the agent image, + * which decides where it waits. These tests pin both directions of that + * contract so a move like `/tmp/awf-init` -> `/run/awf-init` cannot silently + * strand a pinned `--image-tag`. + */ +describe('init signal directory compatibility', () => { + describe('new CLI + new agent image', () => { + it('mounts the ready-file source at the current path, writable', () => { + const volumes = generateDockerCompose(getConfig(), mockNetworkConfig) + .services.agent.volumes as string[]; + + expect(volumes).toContain(`${getConfig().workDir}/init-signal:${INIT_SIGNAL_DIR}:rw`); + }); + + it('waits on the current path', () => { + expect(entrypointSource).toContain('INIT_SIGNAL_DIR="${AWF_INIT_SIGNAL_DIR:-/run/awf-init}"'); + }); + }); + + describe('new CLI + old agent image', () => { + it('also exposes the same source at the legacy path an older image polls', () => { + const volumes = generateDockerCompose(getConfig(), mockNetworkConfig) + .services.agent.volumes as string[]; + + expect(volumes).toContain(`${getConfig().workDir}/init-signal:${LEGACY_INIT_SIGNAL_DIR}:ro`); + }); + + it('keeps the legacy view read-only, since older images only poll it', () => { + const volumes = generateDockerCompose(getConfig(), mockNetworkConfig) + .services.agent.volumes as string[]; + + expect(volumes).not.toContain(`${getConfig().workDir}/init-signal:${LEGACY_INIT_SIGNAL_DIR}:rw`); + }); + + it('keeps the legacy path exposed when a write policy narrows /tmp', () => { + const workspaceDir = process.env.GITHUB_WORKSPACE || process.cwd(); + const volumes = generateDockerCompose( + { ...getConfig(), filesystemAllowWrite: [`${workspaceDir}/src`] }, + mockNetworkConfig, + ).services.agent.volumes as string[]; + + expect(volumes).toContain('/tmp:/tmp:ro'); + expect(volumes).toContain(`${getConfig().workDir}/init-signal:${LEGACY_INIT_SIGNAL_DIR}:ro`); + }); + }); + + describe('old CLI + new agent image', () => { + it('still accepts a ready-file delivered at the legacy path', () => { + expect(entrypointSource).toContain('LEGACY_INIT_SIGNAL_DIR="/tmp/awf-init"'); + // Escaped `\${...}` so this stays a literal shell expansion rather than a + // JavaScript one: the assertion is a byte-for-byte excerpt of the script. + expect(entrypointSource).toContain( + `while [ ! -f "\${INIT_SIGNAL_DIR}/ready" ] && [ ! -f "\${LEGACY_INIT_SIGNAL_DIR}/ready" ]; do`, + ); + }); + }); + + describe('new CLI + old agent image: the init container runs the old script', () => { + /** + * The audit step of `setup-iptables.sh` as shipped in agent images released + * before the signal directory moved to /run. Two properties matter and both + * are load-bearing: the script runs under `set -e`, and the audit file path + * is hardcoded to the legacy directory rather than read from + * `$AWF_INIT_SIGNAL_DIR`. A redirection into a missing directory is a + * command failure, so `set -e` aborts the script before the CLI's + * `&& touch "$AWF_INIT_SIGNAL_DIR/ready"` ever runs, and the agent then + * waits out its full ready timeout. + */ + const OLD_AUDIT_STEP = [ + 'set -e', + `audit_file="${LEGACY_INIT_SIGNAL_DIR}/iptables-audit.txt"`, + 'echo "# iptables audit dump" > "$audit_file"', + 'echo "## IPv4 NAT rules" >> "$audit_file"', + 'echo OLD_SCRIPT_COMPLETED', + ].join('\n'); + + /** + * Materialises the init container's filesystem view: every bind target in + * the generated service exists as a directory, and nothing else does. + */ + function stageInitContainerRootfs(volumes: string[]): string { + const rootfs = fs.mkdtempSync(path.join(os.tmpdir(), 'awf-initns-')); + for (const spec of volumes) { + const target = spec.split(':')[1]; + if (target) fs.mkdirSync(path.join(rootfs, target), { recursive: true }); + } + return rootfs; + } + + function runOldSetupScript(rootfs: string): { stdout: string; readyExists: boolean } { + const script = [ + `cd "${rootfs}"`, + // Rebase the container-absolute paths onto the staged rootfs. + OLD_AUDIT_STEP.replace( + `audit_file="${LEGACY_INIT_SIGNAL_DIR}`, + `audit_file="${rootfs}${LEGACY_INIT_SIGNAL_DIR}`, + ), + // Exactly how the CLI chains the ready signal after the script. + `touch "${rootfs}${INIT_SIGNAL_DIR}/ready"`, + ].join('\n'); + + let stdout = ''; + try { + stdout = execFileSync('/bin/sh', ['-c', script], { encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'] }); + } catch (err) { + stdout = String((err as { stdout?: string }).stdout ?? ''); + } + return { + stdout, + readyExists: fs.existsSync(path.join(rootfs, INIT_SIGNAL_DIR.slice(1), 'ready')), + }; + } + + it('lets the old audit dump succeed so the ready signal is still written', () => { + const volumes = generateDockerCompose(getConfig(), mockNetworkConfig) + .services['iptables-init'].volumes as string[]; + const rootfs = stageInitContainerRootfs(volumes); + + try { + const result = runOldSetupScript(rootfs); + + expect(result.stdout).toContain('OLD_SCRIPT_COMPLETED'); + expect(result.readyExists).toBe(true); + } finally { + fs.rmSync(rootfs, { recursive: true, force: true }); + } + }); + + it('proves the test would catch the regression it is guarding', () => { + // Same script, but with only the current signal directory mounted: this is + // the state that stranded an older image, and it must be detectable. + const rootfs = stageInitContainerRootfs([`/src:${INIT_SIGNAL_DIR}:rw`]); + + try { + const result = runOldSetupScript(rootfs); + + expect(result.stdout).not.toContain('OLD_SCRIPT_COMPLETED'); + expect(result.readyExists).toBe(false); + } finally { + fs.rmSync(rootfs, { recursive: true, force: true }); + } + }); + + it('mounts the legacy path writable, because the old script writes there', () => { + const volumes = generateDockerCompose(getConfig(), mockNetworkConfig) + .services['iptables-init'].volumes as string[]; + + expect(volumes).toContain(`${getConfig().workDir}/init-signal:${LEGACY_INIT_SIGNAL_DIR}:rw`); + expect(volumes).toContain(`${getConfig().workDir}/init-signal:${INIT_SIGNAL_DIR}:rw`); + }); + + it('keeps both init-signal views on one source, so either path signals the agent', () => { + const volumes = (generateDockerCompose(getConfig(), mockNetworkConfig) + .services['iptables-init'].volumes as string[]) + .filter((spec) => spec.includes('init-signal')) + .map((spec) => spec.split(':')[0]); + + expect(new Set(volumes).size).toBe(1); + }); + }); + + it('does not rely on a symlink inside the init container, which the agent cannot see', () => { + const command = generateDockerCompose(getConfig(), mockNetworkConfig) + .services['iptables-init'].command as string[]; + + // The init container has its own rootfs and mount namespace: anything it + // creates at the legacy path is invisible to the agent container. + expect(command.join(' ')).not.toContain('ln -s'); + expect(command.join(' ')).not.toContain(LEGACY_INIT_SIGNAL_DIR); + }); +}); diff --git a/src/workdir-setup-branches.test.ts b/src/workdir-setup-branches.test.ts index 6964e23d8..b91dd86e6 100644 --- a/src/workdir-setup-branches.test.ts +++ b/src/workdir-setup-branches.test.ts @@ -144,6 +144,91 @@ describe('workdir-setup – createMissingOwnedDirectorySegments non-directory se workdirSetupTestHelpers.createMissingOwnedDirectorySegments(childPath, 1000, 1000) ).toThrow(`Expected directory but found non-directory path: ${fileSegment}`); }); + + it('skips an owner-preserving chown so non-root callers do not hit EPERM', () => { + // A freshly created directory already belongs to the caller, so chowning it + // back to the same owner changes nothing -- but macOS still denies that + // syscall to non-root, which used to abort mountpoint preparation. + const created = path.join(tempDir, 'nested', 'mountpoint'); + // Read the ownership the platform actually assigns to a new directory here + // rather than assuming the caller's ids: BSD (macOS) gives a new directory + // its parent's gid, so under /tmp that is wheel, not the caller's group. + const probe = path.join(tempDir, 'ownership-probe'); + fs.mkdirSync(probe); + const { uid, gid } = fs.statSync(probe); + + workdirSetupTestHelpers.createMissingOwnedDirectorySegments(created, uid, gid); + + expect(fs.existsSync(created)).toBe(true); + expect(fs.chownSync as unknown as jest.Mock).not.toHaveBeenCalled(); + }); + + it('still chowns when a privileged caller must change owner', () => { + const created = path.join(tempDir, 'other-owner'); + const foreignUid = (process.getuid?.() ?? 0) + 1; + const getuid = jest.spyOn(process, 'getuid').mockReturnValue(0); + + try { + workdirSetupTestHelpers.createMissingOwnedDirectorySegments(created, foreignUid, 0); + } finally { + getuid.mockRestore(); + } + + expect(fs.chownSync as unknown as jest.Mock).toHaveBeenCalledWith(created, foreignUid, 0); + }); + + it('tolerates an EPERM chown a non-root caller could never satisfy', () => { + // getSafeHostGid can map the caller onto a different gid (macOS maps a + // system gid into the regular range). Unprivileged AWF cannot grant a + // directory away, so this must not abort mountpoint preparation. + const created = path.join(tempDir, 'unprivileged'); + const foreignGid = (process.getgid?.() ?? 0) + 1000; + const getuid = jest.spyOn(process, 'getuid').mockReturnValue(501); + (fs.chownSync as unknown as jest.Mock).mockImplementation(() => { + const error = new Error('EPERM') as NodeJS.ErrnoException; + error.code = 'EPERM'; + throw error; + }); + + try { + workdirSetupTestHelpers.createMissingOwnedDirectorySegments(created, 501, foreignGid); + } finally { + getuid.mockRestore(); + } + + expect(fs.existsSync(created)).toBe(true); + }); + + it('still propagates a chown failure that is not a privilege limit', () => { + const created = path.join(tempDir, 'broken'); + (fs.chownSync as unknown as jest.Mock).mockImplementation(() => { + const error = new Error('EIO') as NodeJS.ErrnoException; + error.code = 'EIO'; + throw error; + }); + + expect(() => + workdirSetupTestHelpers.createMissingOwnedDirectorySegments(created, 4242, 4242) + ).toThrow('EIO'); + }); + + it('still propagates an EPERM chown when running privileged', () => { + const created = path.join(tempDir, 'privileged-eperm'); + const getuid = jest.spyOn(process, 'getuid').mockReturnValue(0); + (fs.chownSync as unknown as jest.Mock).mockImplementation(() => { + const error = new Error('EPERM') as NodeJS.ErrnoException; + error.code = 'EPERM'; + throw error; + }); + + try { + expect(() => + workdirSetupTestHelpers.createMissingOwnedDirectorySegments(created, 4242, 4242) + ).toThrow('EPERM'); + } finally { + getuid.mockRestore(); + } + }); }); describe('workdir-setup – prepareLogDirectories mcp-logs already-exists branch (lines 194-195)', () => { diff --git a/src/workdir-setup.test.ts b/src/workdir-setup.test.ts index 637d816cf..e1c265c03 100644 --- a/src/workdir-setup.test.ts +++ b/src/workdir-setup.test.ts @@ -503,8 +503,7 @@ describe('prepareChrootHomeMounts (sub-function)', () => { expect(fs.existsSync(geminiDir)).toBe(false); }); - it('refuses existing symlinked nested home tool paths', () => { - const sandboxState = path.join(fixture.tempDir, '.local', 'state', 'sandboxes'); + it('refuses existing symlinked nested home tool paths', () => { const sandboxState = path.join(fixture.tempDir, '.local', 'state', 'sandboxes'); const localBin = path.join(fixture.tempDir, '.local', 'bin'); fs.mkdirSync(sandboxState, { recursive: true }); fs.symlinkSync(sandboxState, localBin); @@ -512,6 +511,41 @@ describe('prepareChrootHomeMounts (sub-function)', () => { expect(() => workdirSetupTestHelpers.prepareChrootHomeMounts(buildConfig())) .toThrow(`Refusing to use symlink as directory: ${localBin}`); }); + + // Regression: on GitHub-hosted runners the workspace lives under $HOME, so + // its /host-prefixed bind lands inside the chroot home. Docker used to create + // those parents, but filesystem.allowWrite narrows the chroot home bind to + // read-only and runc then fails container init with EROFS. + describe('workspace mountpoint', () => { + const originalWorkspace = process.env.GITHUB_WORKSPACE; + + afterEach(() => { + if (originalWorkspace === undefined) delete process.env.GITHUB_WORKSPACE; + else process.env.GITHUB_WORKSPACE = originalWorkspace; + }); + + it('prepares it inside the chroot home when the workspace is under $HOME', () => { + const workspace = path.join(fixture.tempDir, 'work', 'gh-aw-firewall', 'gh-aw-firewall'); + process.env.GITHUB_WORKSPACE = workspace; + + workdirSetupTestHelpers.prepareChrootHomeMounts(buildConfig()); + + const chrootHome = `${fixture.tempDir}-chroot-home`; + expect(fs.existsSync(path.join(chrootHome, 'work'))).toBe(true); + expect( + fs.existsSync(path.join(chrootHome, 'work', 'gh-aw-firewall', 'gh-aw-firewall')), + ).toBe(true); + }); + + it('leaves the chroot home alone when the workspace is outside $HOME', () => { + process.env.GITHUB_WORKSPACE = path.join(fixture.tempDir, '..', 'elsewhere', 'workspace'); + + workdirSetupTestHelpers.prepareChrootHomeMounts(buildConfig()); + + expect(fs.existsSync(path.join(`${fixture.tempDir}-chroot-home`, 'elsewhere'))).toBe(false); + expect(fs.existsSync(path.join(`${fixture.tempDir}-chroot-home`, 'workspace'))).toBe(false); + }); + }); }); describe('ensureDirectory EACCES diagnostic', () => { diff --git a/tests/fixtures/awf-runner.ts b/tests/fixtures/awf-runner.ts index 6bf994b6e..8248af131 100644 --- a/tests/fixtures/awf-runner.ts +++ b/tests/fixtures/awf-runner.ts @@ -13,6 +13,7 @@ export interface AwfOptions { imageTag?: string; timeout?: number; // milliseconds env?: Record; + configFile?: string; volumeMounts?: string[]; // Volume mounts in format: host_path:container_path[:mode] containerWorkDir?: string; // Working directory inside the container tty?: boolean; // Allocate pseudo-TTY (required for interactive tools like Claude Code) @@ -60,6 +61,10 @@ export class AwfRunner { async run(command: string, options: AwfOptions = {}): Promise { const args: string[] = []; + if (options.configFile) { + args.push('--config', options.configFile); + } + // Add allow-domains if (options.allowDomains && options.allowDomains.length > 0) { args.push('--allow-domains', options.allowDomains.join(',')); @@ -284,6 +289,10 @@ export class AwfRunner { // Add awf path args.push('node', this.awfPath); + if (options.configFile) { + args.push('--config', options.configFile); + } + // runWithSudo uses the legacy iptables path args.push('--legacy-security'); diff --git a/tests/integration/filesystem-allowwrite.test.ts b/tests/integration/filesystem-allowwrite.test.ts new file mode 100644 index 000000000..a14d55f93 --- /dev/null +++ b/tests/integration/filesystem-allowwrite.test.ts @@ -0,0 +1,123 @@ +/** + * filesystem.allowWrite integration tests + * + * These tests cover live container startup behavior that unit tests of volume + * rewriting cannot catch, such as Docker/runc mountpoint creation order. + */ + +/// + +import { describe, test, expect, beforeAll, afterAll } from '@jest/globals'; +import { createRunner, AwfRunner } from '../fixtures/awf-runner'; +import { cleanup } from '../fixtures/cleanup'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; + +describe('filesystem.allowWrite', () => { + let runner: AwfRunner; + let testDir: string; + + beforeAll(async () => { + await cleanup(false); + runner = createRunner(); + }); + + afterAll(async () => { + await cleanup(false); + if (testDir && fs.existsSync(testDir)) { + fs.rmSync(testDir, { recursive: true, force: true }); + } + }); + + test('starts the legacy Docker agent when /tmp is narrowed read-only', async () => { + testDir = fs.mkdtempSync(path.join(os.tmpdir(), 'awf-allowwrite-')); + const writableDir = path.join(testDir, 'agent'); + fs.mkdirSync(writableDir, { recursive: true }); + const configPath = path.join(testDir, 'awf-config.json'); + fs.writeFileSync(configPath, JSON.stringify({ + network: { + allowDomains: ['github.com'], + }, + filesystem: { + allowWrite: [writableDir], + }, + container: { + buildLocal: true, + }, + logging: { + logLevel: 'debug', + }, + security: { + legacySecurity: true, + }, + })); + + const result = await runner.runWithSudo( + `sh -c 'echo started > ${writableDir}/started.txt'`, + { + configFile: configPath, + timeout: 120000, + } + ); + + expect(result).toSucceed(); + expect(fs.readFileSync(path.join(writableDir, 'started.txt'), 'utf8')).toContain('started'); + }, 180000); + + test('keeps security helpers installed when /tmp is narrowed read-only', async () => { + const helperDir = fs.mkdtempSync(path.join(os.tmpdir(), 'awf-allowwrite-helpers-')); + const writableDir = path.join(helperDir, 'agent'); + fs.mkdirSync(writableDir, { recursive: true }); + const configPath = path.join(helperDir, 'awf-config.json'); + fs.writeFileSync(configPath, JSON.stringify({ + network: { allowDomains: ['github.com'] }, + filesystem: { allowWrite: [writableDir] }, + container: { buildLocal: true }, + logging: { logLevel: 'debug' }, + security: { legacySecurity: true }, + })); + + // The helpers used to stage under /tmp/awf-lib. Once a write policy narrows + // /tmp to read-only those copies failed silently, disabling one-shot token + // protection. Prove the library is really present at its new location, not + // merely that the container started. + const result = await runner.runWithSudo( + "sh -c 'ls -l /run/awf-lib/ > " + writableDir + "/helpers.txt 2>&1; " + + "test -s /run/awf-lib/one-shot-token.so && echo ONE_SHOT_TOKEN_PRESENT >> " + writableDir + "/helpers.txt'", + { configFile: configPath, timeout: 120000 }, + ); + + expect(result).toSucceed(); + const helpers = fs.readFileSync(path.join(writableDir, 'helpers.txt'), 'utf8'); + expect(helpers).toContain('ONE_SHOT_TOKEN_PRESENT'); + + fs.rmSync(helperDir, { recursive: true, force: true }); + }, 180000); + + test('leaves no AWF mount residue on the host after teardown', async () => { + const residueDir = fs.mkdtempSync(path.join(os.tmpdir(), 'awf-allowwrite-residue-')); + const writableDir = path.join(residueDir, 'agent'); + fs.mkdirSync(writableDir, { recursive: true }); + const configPath = path.join(residueDir, 'awf-config.json'); + fs.writeFileSync(configPath, JSON.stringify({ + network: { allowDomains: ['github.com'] }, + filesystem: { allowWrite: [writableDir] }, + container: { buildLocal: true }, + logging: { logLevel: 'debug' }, + security: { legacySecurity: true }, + })); + + const result = await runner.runWithSudo( + `sh -c 'echo done > ${writableDir}/done.txt'`, + { configFile: configPath, timeout: 120000 }, + ); + expect(result).toSucceed(); + + const mounts = fs.readFileSync('/proc/mounts', 'utf8'); + expect(mounts).not.toContain(writableDir); + expect(mounts).not.toContain('/run/awf-lib'); + + fs.rmSync(residueDir, { recursive: true, force: true }); + }, 180000); +});