fix: only hide credential files if parent directory exists#737
Conversation
Prevents Docker mount errors when credential file parent directories don't exist on the host. This fixes the "read-only file system" error when Docker tries to create mountpoints for non-existent parents. The fix checks if the parent directory exists before adding /dev/null mounts for credential files in both normal and chroot modes. This maintains security while avoiding mount failures. Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
|
💫 TO BE CONTINUED... Smoke Claude failed! Our hero faces unexpected challenges... |
|
Chroot tests failed Smoke Chroot failed - See logs for details. |
|
📰 DEVELOPING STORY: Smoke Copilot reports failed. Our correspondents are investigating the incident... |
* Initial plan * fix: replace /dev/null mounts with tmpfs for credential hiding This fixes Docker mount errors like "read-only file system" that occur when mounting /dev/null over credential files whose parent directories don't exist in the container's filesystem. The solution uses tmpfs mounts instead, which create empty in-memory filesystems that overlay directories without requiring the target path to exist first. Changes: - Normal mode: Hide credential directories (~/.docker, ~/.ssh, ~/.aws, etc.) using tmpfs - Chroot mode: Hide credential directories at /host paths using tmpfs - Updated DockerService type to include tmpfs property - Updated tests to verify tmpfs behavior instead of /dev/null mounts - Fixed config mutation bug by using local variable instead of mutating config object Closes the GitHub Actions failure where cargo credentials mounting failed with: "error mounting "/dev/null" to rootfs at "/host/home/runner/.cargo/credentials": create mountpoint ... read-only file system" Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> * fix: prevent .cargo volume/tmpfs mount conflict in chroot mode (#740) * Initial plan * fix: prevent .cargo volume/tmpfs mount conflict in chroot mode Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> * fix: handle missing docker-compose.yml during cleanup gracefully (#741) * Initial plan * fix: handle missing docker-compose.yml during cleanup gracefully Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> --------- Co-authored-by: anthropic-code-agent[bot] <242468646+Claude@users.noreply.github.com> Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> --------- Co-authored-by: anthropic-code-agent[bot] <242468646+Claude@users.noreply.github.com> Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> --------- Co-authored-by: anthropic-code-agent[bot] <242468646+Claude@users.noreply.github.com> Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
|
💫 TO BE CONTINUED... Smoke Claude failed! Our hero faces unexpected challenges... |
|
📰 DEVELOPING STORY: Smoke Copilot reports failed. Our correspondents are investigating the incident... |
|
Chroot tests failed Smoke Chroot failed - See logs for details. |
* Initial plan * fix: create .copilot/logs mountpoint before docker mount The agent container mount at ~/.copilot:ro followed by ~/agent-logs:~/.copilot/logs:rw fails in GitHub Actions because Docker cannot create the logs subdirectory inside a read-only mount. Fix by creating ~/.copilot/logs on the host before mounting, so Docker doesn't need to create the mountpoint inside the read-only parent. This affects GitHub Actions runners where ~/.copilot doesn't exist before AWF runs. Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> * fix: only hide credential files if parent directory exists (#737) * Initial plan * fix: only hide credential files if parent directory exists Prevents Docker mount errors when credential file parent directories don't exist on the host. This fixes the "read-only file system" error when Docker tries to create mountpoints for non-existent parents. The fix checks if the parent directory exists before adding /dev/null mounts for credential files in both normal and chroot modes. This maintains security while avoiding mount failures. Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> * fix: replace /dev/null mounts with tmpfs for credential hiding (#738) * Initial plan * fix: replace /dev/null mounts with tmpfs for credential hiding This fixes Docker mount errors like "read-only file system" that occur when mounting /dev/null over credential files whose parent directories don't exist in the container's filesystem. The solution uses tmpfs mounts instead, which create empty in-memory filesystems that overlay directories without requiring the target path to exist first. Changes: - Normal mode: Hide credential directories (~/.docker, ~/.ssh, ~/.aws, etc.) using tmpfs - Chroot mode: Hide credential directories at /host paths using tmpfs - Updated DockerService type to include tmpfs property - Updated tests to verify tmpfs behavior instead of /dev/null mounts - Fixed config mutation bug by using local variable instead of mutating config object Closes the GitHub Actions failure where cargo credentials mounting failed with: "error mounting "/dev/null" to rootfs at "/host/home/runner/.cargo/credentials": create mountpoint ... read-only file system" Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> * fix: prevent .cargo volume/tmpfs mount conflict in chroot mode (#740) * Initial plan * fix: prevent .cargo volume/tmpfs mount conflict in chroot mode Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> * fix: handle missing docker-compose.yml during cleanup gracefully (#741) * Initial plan * fix: handle missing docker-compose.yml during cleanup gracefully Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> --------- Co-authored-by: anthropic-code-agent[bot] <242468646+Claude@users.noreply.github.com> Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> --------- Co-authored-by: anthropic-code-agent[bot] <242468646+Claude@users.noreply.github.com> Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> --------- Co-authored-by: anthropic-code-agent[bot] <242468646+Claude@users.noreply.github.com> Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> --------- Co-authored-by: anthropic-code-agent[bot] <242468646+Claude@users.noreply.github.com> Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> --------- Co-authored-by: anthropic-code-agent[bot] <242468646+Claude@users.noreply.github.com> Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
|
🌑 The shadows whisper... Smoke Codex failed. The oracle requires further meditation... |
There was a problem hiding this comment.
Pull request overview
This PR aims to prevent Docker mount failures during container initialization while preserving AWF’s credential-hiding security model by changing how sensitive paths are hidden in the generated Docker Compose config.
Changes:
- Add
tmpfs?: string[]support to theDockerServicetype for Docker Compose generation. - Switch credential hiding from
/dev/nullbind mounts to tmpfs-based mounts (and update unit tests accordingly). - Add a defensive fallback in
stopContainers()to stop containers by name whendocker-compose.ymlis missing.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/types.ts | Extends the Docker Compose service type to support tmpfs mounts and documents intended usage. |
| src/docker-manager.ts | Updates compose generation to use tmpfs mounts for hiding credential locations; also adds safer container shutdown fallback. |
| src/docker-manager.test.ts | Updates/extends tests to assert tmpfs-based credential hiding and .cargo mount behavior. |
Comments suppressed due to low confidence (1)
src/docker-manager.ts:751
- Same issue as normal mode: adding a tmpfs mount at
/host${userHome}/.npmrctargets what is normally a file path. This can cause Docker mount failures (file vs directory) or turn.npmrcinto an empty directory. Use a/dev/nullbind mount for the file (with a parent-dir existence check) if you need to hide only that file, or adjust the tmpfs strategy to mount a directory path.
credentialTmpfsMounts.push(`/host${userHome}/.npmrc:rw,noexec,nosuid,size=1m`);
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Also hide ~/.npmrc file (NPM registry tokens) - needs special handling as it's a file | ||
| // Mount its parent directory as tmpfs to hide it | ||
| const npmrcParent = effectiveHome; | ||
| if (!credentialTmpfsMounts.some(mount => mount.startsWith(`${npmrcParent}:`))) { | ||
| // Only add if we're not already mounting the entire home directory | ||
| // In practice, we'll mount ~/.npmrc as a tmpfs (which will be an empty directory) | ||
| credentialTmpfsMounts.push(`${effectiveHome}/.npmrc:rw,noexec,nosuid,size=1m`); | ||
| } |
There was a problem hiding this comment.
tmpfs mounts are directory mounts, but this adds a tmpfs at ${effectiveHome}/.npmrc which is typically a file. If .npmrc exists as a file in the image/home, Docker will likely fail with a "not a directory" error; if it doesn’t exist, Docker may create a directory named .npmrc, which can break tooling in unexpected ways. Prefer hiding .npmrc with a file-level /dev/null bind mount guarded by a parent-dir existence check, or mount a directory tmpfs at a valid directory path and handle .npmrc within that design.
This issue also appears on line 751 of the same file.
| // Also hide ~/.npmrc file (NPM registry tokens) - needs special handling as it's a file | |
| // Mount its parent directory as tmpfs to hide it | |
| const npmrcParent = effectiveHome; | |
| if (!credentialTmpfsMounts.some(mount => mount.startsWith(`${npmrcParent}:`))) { | |
| // Only add if we're not already mounting the entire home directory | |
| // In practice, we'll mount ~/.npmrc as a tmpfs (which will be an empty directory) | |
| credentialTmpfsMounts.push(`${effectiveHome}/.npmrc:rw,noexec,nosuid,size=1m`); | |
| } | |
| // NOTE: ~/.npmrc is typically a file, not a directory. Avoid adding it to the tmpfs | |
| // directory mounts here to prevent Docker "not a directory" errors when it exists | |
| // as a file in the image or home directory. File-level hiding (e.g. bind-mounting | |
| // /dev/null over ~/.npmrc) should be implemented via a separate bind-mount mechanism. |
| // SECURITY: Hide credential directories using tmpfs (empty in-memory filesystem) | ||
| // This prevents prompt-injected commands from reading sensitive tokens | ||
| // even if the attacker knows the file paths | ||
| const credentialFiles = [ | ||
| `${effectiveHome}/.docker/config.json`, // Docker Hub tokens | ||
| `${effectiveHome}/.npmrc`, // NPM registry tokens | ||
| `${effectiveHome}/.cargo/credentials`, // Rust crates.io tokens | ||
| `${effectiveHome}/.composer/auth.json`, // PHP Composer tokens | ||
| `${effectiveHome}/.config/gh/hosts.yml`, // GitHub CLI OAuth tokens | ||
| // SSH private keys (CRITICAL - server access, git operations) | ||
| `${effectiveHome}/.ssh/id_rsa`, | ||
| `${effectiveHome}/.ssh/id_ed25519`, | ||
| `${effectiveHome}/.ssh/id_ecdsa`, | ||
| `${effectiveHome}/.ssh/id_dsa`, | ||
| // Cloud provider credentials (CRITICAL - infrastructure access) | ||
| `${effectiveHome}/.aws/credentials`, | ||
| `${effectiveHome}/.aws/config`, | ||
| `${effectiveHome}/.kube/config`, | ||
| `${effectiveHome}/.azure/credentials`, | ||
| `${effectiveHome}/.config/gcloud/credentials.db`, | ||
| // even if the attacker knows the file paths. | ||
| // Using tmpfs instead of /dev/null mounts avoids Docker errors when parent directories | ||
| // don't exist in the container filesystem. | ||
| const credentialDirs = [ |
There was a problem hiding this comment.
The PR description/title talk about conditionally adding /dev/null mounts based on parent directory existence and logging skipped paths, but the implementation here switches to tmpfs-based directory hiding and doesn’t perform existence checks or log skipped credential paths. Either update the PR description/title to match the new approach or adjust the code to implement the described parent-dir checks + skip logging.
| // 1. MCP server logs at /tmp/gh-aw/mcp-logs | ||
| // 2. Credential files/directories (when not using --allow-full-filesystem-access) | ||
| tmpfs: (() => { | ||
| const tmpfsMounts = []; |
There was a problem hiding this comment.
const tmpfsMounts = [] is inferred as any[] under TypeScript, which weakens type-safety in a strict project. Declare this as string[] so accidental non-string entries (or later refactors) are caught at compile time.
| const tmpfsMounts = []; | |
| const tmpfsMounts: string[] = []; |
Docker mount failures occur when AWF attempts to mount
/dev/nullover credential files whose parent directories don't exist on the host. The container's rootfs is read-only during initialization, preventing Docker from creating missing parent directories as mountpoints.Changes
/dev/nullmounts for credential files~/.cargo/credentials) and chroot mode (/host/.cargo/credentials)Implementation
This prevents mount failures in GitHub Actions runners and other environments where credential directories may not be pre-created, while preserving the security model of hiding sensitive files where their parent directories do exist.