fix: replace /dev/null mounts with tmpfs for credential hiding#738
Conversation
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>
|
💫 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: 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>
2f8fc0d
into
claude/fix-github-actions-workflow-again
|
💫 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: 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>
* 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 updates the Docker Compose generation logic to hide host credentials more reliably inside the agent container by switching from per-file /dev/null bind mounts to tmpfs overlays, avoiding mountpoint creation failures when paths don’t exist or are read-only.
Changes:
- Add
tmpfs?: string[]toDockerServiceinsrc/types.tsand document tmpfs mount semantics. - Replace credential hiding from
/dev/null:/path/to/file:romounts to tmpfs directory overlays ingenerateDockerCompose(); update unit tests accordingly. - Update
stopContainers()to fall back to stopping 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 compose service typing to include tmpfs mounts and documents intended usage. |
| src/docker-manager.ts | Implements tmpfs-based credential hiding and wires credential tmpfs mounts into the agent service; also adds a stopContainers() fallback path. |
| src/docker-manager.test.ts | Updates expectations from /dev/null volume mounts to tmpfs mounts; adds coverage for .cargo mount behavior under chroot/full-access combinations. |
Comments suppressed due to low confidence (1)
src/docker-manager.ts:751
- Same issue in chroot mode:
/host${userHome}/.npmrcis a file path. A tmpfs mount here will fail if the file exists (or will create a directory and potentially break tools that expect a file). Use an empty-file bind mount (or/dev/nullwith safe parent-dir handling) for file targets.
// Also hide ~/.npmrc file (NPM registry tokens) - mount as tmpfs
credentialTmpfsMounts.push(`/host${userHome}/.npmrc:rw,noexec,nosuid,size=1m`);
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| 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. Mounting tmpfs at ${effectiveHome}/.npmrc will fail if .npmrc exists as a file (common when users have NPM auth configured), because tmpfs can’t be mounted over a file path. Consider hiding .npmrc via a bind-mount of an empty file (or /dev/null if the parent dir is guaranteed to exist and be writable) instead of tmpfs, or adjust the approach so the mount point is always a directory.
This issue also appears on line 750 of the same file.
| // 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 = []; infers any[] in TypeScript. Please type this as string[] (and similarly ensure the returned value is string[]) so DockerService.tmpfs stays strongly typed and you don’t lose type-safety in this block.
| const tmpfsMounts = []; | |
| const tmpfsMounts: string[] = []; |
| // Check if workDir and docker-compose.yml exist before using docker compose | ||
| const composeFile = path.join(workDir, 'docker-compose.yml'); | ||
| if (fs.existsSync(workDir) && fs.existsSync(composeFile)) { | ||
| // Normal path: use docker compose down | ||
| await execa('docker', ['compose', 'down', '-v'], { | ||
| cwd: workDir, | ||
| stdio: 'inherit', | ||
| }); | ||
| logger.success('Containers stopped successfully'); |
There was a problem hiding this comment.
This PR also changes stopContainers() behavior by adding a fallback path that stops/removes containers by name when docker-compose.yml is missing. This is not mentioned in the PR description/title (which focuses on credential hiding), so it’s easy to miss during review/release notes—please either update the PR description to call this out or move it into a separate PR.
Docker mount errors occur when mounting
/dev/nullover credential files whose parent directories don't exist in the container filesystem. The error manifests as:error mounting "/dev/null" to rootfs at "/host/home/runner/.cargo/credentials": create mountpoint ... read-only file systemChanges
Credential hiding mechanism: Replaced individual file mounts (
/dev/null:/path/to/file:ro) with directory-level tmpfs overlays (/path/to/dir:rw,noexec,nosuid,size=1m)~/.docker,~/.ssh,~/.aws,~/.kube,~/.azure,~/.config/gcloud,~/.config/gh,~/.cargo,~/.composer,~/.npmrc/hostpathsType system: Added
tmpfs?: string[]toDockerServiceinterface insrc/types.tsConfig mutation fix: Changed from storing tmpfs arrays on config object (which caused test pollution) to using local variable
credentialTmpfsMountsTechnical Detail
tmpfs creates an empty in-memory filesystem overlay that doesn't require the target path to exist first, unlike
/dev/nullbind mounts which fail during mountpoint creation when parent directories are missing in the container.Before:
After: