fix: use granular workspace mounting instead of entire HOME directory#699
fix: use granular workspace mounting instead of entire HOME directory#699
Conversation
Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
|
Chroot tests failed Smoke Chroot failed - See logs for details. |
|
💫 TO BE CONTINUED... Smoke Claude failed! Our hero faces unexpected challenges... |
|
📰 DEVELOPING STORY: Smoke Copilot reports failed. Our correspondents are investigating the incident... |
|
| Metric | Base | PR | Delta |
|---|---|---|---|
| Lines | 82.34% | 82.34% | ➡️ +0.00% |
| Statements | 82.38% | 82.38% | ➡️ +0.00% |
| Functions | 82.14% | 82.14% | ➡️ +0.00% |
| Branches | 74.70% | 74.65% | 📉 -0.05% |
Coverage comparison generated by scripts/ci/compare-coverage.ts
🔒 Security Review: Critical Vulnerability FoundI've reviewed PR #699 and identified one critical security vulnerability and one test bug that need to be addressed before merging.
|
| Issue | Severity | Status | Action Required |
|---|---|---|---|
| Local execution vulnerability (process.cwd() fallback) | HIGH | Add validation to prevent running from $HOME | |
| Test assertion expects HOME mount | Low | Update test to check for workspace mount |
Recommendation: DO NOT MERGE until the local execution vulnerability is fixed with proper validation.
Security Review completed by: Claude Code Security Guard
Date: 2026-02-11
AI generated by Security Guard
There was a problem hiding this comment.
Pull request overview
This PR updates AWF’s Docker Compose generation to avoid mounting the entire host $HOME into the agent container, aiming to reduce exposure of sensitive files (including GitHub Actions runner credentials) by mounting only the workspace path instead.
Changes:
- Switch agent volume mounts from
${HOME}:${HOME}to${GITHUB_WORKSPACE || cwd}:${same}(and the corresponding/host...mount in chroot mode). - Update chroot-mode unit test to assert the new workspace-under-
/hostmount. - Expand selective mounting documentation to describe the granular workspace-only approach and the historical vulnerability.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/docker-manager.ts | Replaces blanket $HOME mounting with workspace-only mounts (normal + chroot) and updates security commentary. |
| src/docker-manager.test.ts | Updates a chroot-mode volumes test to check for workspace mount under /host. |
| docs/selective-mounting.md | Updates docs to describe the workspace-only mounting approach and compares before/after behavior. |
Comments suppressed due to low confidence (2)
src/docker-manager.ts:485
- In chroot mode, this change removes the
/host${userHome}bind mount but the entrypoint still tries to augment PATH with user-home tool locations (e.g.$HOME/.local/bin,$HOME/.cargo/bin, and optionally$AWF_CARGO_HOME/bin). Without mounting those directories, common GH Actions setups where toolchains are installed under the user home will stop working. If the intent is workspace-only, consider mounting specific tool directories read-only (e.g.${userHome}/.cargo,${userHome}/.rustup,${userHome}/.local) or updating the chroot PATH logic/docs to reflect that these tool locations are no longer supported unless explicitly mounted.
// SECURITY FIX: Mount only workspace directory instead of entire user home
// This prevents access to credential files in $HOME
// Mount workspace directory at /host path for chroot
agentVolumes.push(`${workspaceDir}:/host${workspaceDir}:rw`);
docs/selective-mounting.md:277
- The example
ls ~/output is incorrect in this configuration: even though the host home isn't bind-mounted, the container will still have a$HOMEdirectory created by other mounts (workspace bind mount under/home/...,.copilot/logsmount, and/dev/nulloverlays). The right description is that host home contents aren’t accessible, not that the directory path is missing.
$ ls ~/
ls: cannot access '/home/runner/': No such file or directory
# ✓ HOME directory not mounted at all!
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/docker-manager.ts
Outdated
| const effectiveHome = config.enableChroot ? getRealUserHome() : (process.env.HOME || '/root'); | ||
|
|
||
| // SECURITY FIX: Use granular mounting instead of blanket HOME directory mount | ||
| // Only mount the workspace directory ($GITHUB_WORKSPACE or current working directory) | ||
| // to prevent access to credential files in $HOME | ||
| const workspaceDir = process.env.GITHUB_WORKSPACE || process.cwd(); | ||
| const agentVolumes: string[] = [ | ||
| // Essential mounts that are always included | ||
| '/tmp:/tmp:rw', | ||
| `${effectiveHome}:${effectiveHome}:rw`, | ||
| // Mount only the workspace directory (not entire HOME) | ||
| // This prevents access to ~/.docker/, ~/.config/gh/, ~/.npmrc, etc. | ||
| `${workspaceDir}:${workspaceDir}:rw`, | ||
| // Mount agent logs directory to workDir for persistence | ||
| `${config.workDir}/agent-logs:${effectiveHome}/.copilot/logs:rw`, |
There was a problem hiding this comment.
HOME is still set to the host home (homeDir), but the host home directory is no longer bind-mounted. Docker will create ${effectiveHome} as root-owned (via the .copilot/logs mount and /dev/null overlays), which can make $HOME effectively read-only for awfuser and break tools that write under $HOME (git/npm/pip caches, configs, etc.). Consider either (a) setting HOME to a writable in-container home (e.g. /home/awfuser) when not mounting host home, or (b) mounting a dedicated writable volume/tmpfs at ${effectiveHome} and ensuring ownership for awfuser in entrypoint.
This issue also appears on line 482 of the same file.
docs/selective-mounting.md
Outdated
| **Primary security mechanism**: Credential files are never mounted because `$HOME` is not mounted. The `/dev/null` overlays provide defense-in-depth in case a credential file somehow exists in the workspace directory. | ||
|
|
||
| **Result:** Commands like `cat ~/.docker/config.json` will fail with "No such file or directory" because the home directory is not mounted. |
There was a problem hiding this comment.
This section claims cat ~/.docker/config.json will fail with "No such file or directory" because $HOME isn't mounted, but the implementation still adds explicit /dev/null volume mounts for these credential paths in normal mode. With those mounts, the file exists and cat will succeed with empty output (exit code 0). Please update the documented behavior to match the actual /dev/null overlay approach (or adjust the implementation if the goal is a hard "not found").
This issue also appears on line 275 of the same file.
| **Primary security mechanism**: Credential files are never mounted because `$HOME` is not mounted. The `/dev/null` overlays provide defense-in-depth in case a credential file somehow exists in the workspace directory. | |
| **Result:** Commands like `cat ~/.docker/config.json` will fail with "No such file or directory" because the home directory is not mounted. | |
| **Primary security mechanism**: Host credential files are never exposed because `$HOME` is not mounted from the host, and known credential paths inside the container are overlaid with `/dev/null`. This means those paths resolve to empty, read-only files that cannot reveal or modify host credentials. | |
| **Result:** Commands like `cat ~/.docker/config.json` will succeed but print nothing (empty output), because the file in the container is backed by `/dev/null` rather than any host credential file. |
src/docker-manager.test.ts
Outdated
| @@ -637,9 +637,9 @@ describe('docker-manager', () => { | |||
| const agent = result.services.agent; | |||
| const volumes = agent.volumes as string[]; | |||
|
|
|||
| // Should mount home directory under /host for chroot access (read-write) | |||
| const homeDir = process.env.HOME || '/root'; | |||
| expect(volumes).toContain(`${homeDir}:/host${homeDir}:rw`); | |||
| // SECURITY FIX: Should mount only workspace directory under /host for chroot access (not entire HOME) | |||
| const workspaceDir = process.env.GITHUB_WORKSPACE || process.cwd(); | |||
| expect(volumes).toContain(`${workspaceDir}:/host${workspaceDir}:rw`); | |||
There was a problem hiding this comment.
This test verifies the presence of the new workspace /host mount, but it no longer asserts the key security property that the host home directory is not mounted in chroot mode. Consider adding an expectation that volumes do not contain ${homeDir}:/host${homeDir} (and, in normal mode tests, do not contain ${homeDir}:${homeDir}), so future regressions back to blanket home mounting are caught.
…nd Claude (#702) * Initial plan * fix(docker): mount ~/.copilot in chroot mode for copilot cli GitHub Copilot CLI needs to extract bundled packages to ~/.copilot/pkg directory. In chroot mode, this directory was not accessible, causing "EACCES: permission denied" errors when trying to create directories. The fix mounts ~/.copilot at /host~/.copilot in chroot mode, allowing the CLI to create necessary directories while maintaining security (no full HOME mount). Added tests to verify ~/.copilot is writable in chroot mode. Fixes: https://github.com/github/gh-aw-firewall/actions/runs/21912194865/job/63270212999 Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> * feat(one-shot-token): add logging for cached token accesses Add print statements whenever getenv() or secure_getenv() are called for sensitive tokens, even when returning cached values. Previously, only the first access was logged. Now subsequent accesses also log "accessed (cached value)" for better debugging visibility. This helps diagnose token access patterns and verify that the one-shot-token library is working correctly. Example output: - First access: "Token GITHUB_TOKEN accessed and cached (value: ghp_...)" - Subsequent: "Token GITHUB_TOKEN accessed (cached value: ghp_...)" Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> * fix(one-shot-token): temporarily disable ANTHROPIC_API_KEY Comment out ANTHROPIC_API_KEY from DEFAULT_SENSITIVE_TOKENS to address smoke CI timeout issues with Claude. This is a temporary change that will be reverted once the underlying issue is resolved. The token is commented out rather than removed to make it easy to restore later. Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> * fix(one-shot-token): re-enable ANTHROPIC_API_KEY Uncomment ANTHROPIC_API_KEY in DEFAULT_SENSITIVE_TOKENS to restore token protection for Anthropic API keys. The token was temporarily disabled to address Claude timeout issues, but is now being re-enabled. Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> * fix(docker): mount ~/.cache, ~/.config, ~/.local in chroot mode Claude Code and other CLI tools need write access to ~/.cache, ~/.config, and ~/.local directories for state management, caching, and configuration. In chroot mode, these directories were not mounted, causing tools to fail when trying to write state. This resulted in Claude Code timing out after 10 minutes. The fix mounts these directories at /host paths in chroot mode, similar to the ~/.copilot mount. These directories are safe to mount as they contain application state, not credentials. Specific credential files within ~/.config (like ~/.config/gh/hosts.yml) remain blocked via /dev/null overlays. Verified fix: - ~/.cache: ✓ Writable - ~/.config: ✓ Writable - ~/.local: ✓ Writable Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> * fix(docker): mount ~/.anthropic in chroot mode for Claude Code Add read-write mount for ~/.anthropic directory in chroot mode to allow Claude Code to store Anthropic-specific state and configuration. This directory is safe to mount as it contains only Claude-specific application state, not credentials. Verified: ~/.anthropic is writable in chroot mode. Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> * fix(one-shot-token): initialize token list in secure_getenv Critical security fix: secure_getenv() was calling get_token_index() before initializing the token list. If secure_getenv() was the first function called for a sensitive token, tokens_initialized would be 0, causing the token to pass through unprotected and remain exposed in /proc/self/environ. The fix mirrors getenv()'s initialization flow: 1. Take token_mutex 2. Call init_token_list() if not initialized 3. Get token_idx while holding mutex This ensures sensitive tokens are always properly cached and removed from the environment, regardless of which function is called first. Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> * fix(docker): mount ~/.claude in chroot mode for Claude CLI Add read-write mount for ~/.claude directory in chroot mode to allow Claude CLI to store state and configuration. This directory is safe to mount as it contains only Claude-specific application state, not credentials. Verified: ~/.claude is writable in chroot mode. Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
|
Chroot tests passed! Smoke Chroot - All security and functionality tests succeeded. |
|
💫 TO BE CONTINUED... Smoke Claude failed! Our hero faces unexpected challenges... |
|
✨ The prophecy is fulfilled... Smoke Codex has completed its mystical journey. The stars align. 🌟 |
|
| Metric | Base | PR | Delta |
|---|---|---|---|
| Lines | 82.34% | 82.39% | 📈 +0.05% |
| Statements | 82.38% | 82.44% | 📈 +0.06% |
| Functions | 82.14% | 82.14% | ➡️ +0.00% |
| Branches | 74.70% | 74.65% | 📉 -0.05% |
📁 Per-file Coverage Changes (1 files)
| File | Lines (Before → After) | Statements (Before → After) |
|---|---|---|
src/docker-manager.ts |
83.9% → 84.1% (+0.22%) | 83.3% → 83.5% (+0.22%) |
Coverage comparison generated by scripts/ci/compare-coverage.ts
C++ Build Test Results
Overall: PASS ✅ All C++ projects built successfully.
|
Go Build Test Results
Overall: PASS ✅ All Go projects successfully downloaded dependencies and passed tests.
|
Deno Build Test Results
Overall: ✅ PASS All Deno tests completed successfully.
|
|
Smoke Test Results Last 2 merged PRs:
Test Results:
Overall Status: PASS
|
Bun Build Test Results
Overall: PASS ✅ All Bun projects built and tested successfully!
|
.NET Build Test Results
Overall: PASS All .NET projects successfully restored, built, and ran with expected output.
|
✅ Java Build Test ResultsAll Java projects compiled and tested successfully!
Overall: PASS Maven proxy configuration worked correctly with
|
Go Build Test Results
Overall: PASS ✅ All Go projects successfully downloaded dependencies and passed tests.
|
Chroot Mode Runtime Version Test ResultsThe chroot mode test compared runtime versions between the host system and the chroot environment to verify transparent binary access.
Overall Result: ❌ FAILED The test expects all runtimes in chroot mode to match the host versions exactly. Only Go matched successfully, while Python and Node.js versions differed between host and chroot environments.
|
|
PR titles: fix: harden one-shot-token binary against ELF reconnaissance; feat: add secret-digger red team workflows
|
Smoke Test Results ✅Last 2 Merged PRs:
Test Results:
Status: PASS cc
|
Build Test: Rust - ❌ FAILEDIssue: Network Access RequiredThe Rust build test could not complete because Test Results
Overall: FAILED - Network configuration issue Error DetailsCargo hangs indefinitely when attempting to build projects because it cannot access:
Required ActionTo enable Rust builds through the firewall, the following domains need to be whitelisted:
Repository Details
|
Root cause: when rustc loads LLVM (which calls secure_getenv during constructor initialization), our intercepted secure_getenv acquires token_mutex, calls init_token_list/fprintf, which internally calls secure_getenv again for locale -> deadlock on the non-recursive mutex. Fix: simplify secure_getenv to a passthrough to real_secure_getenv. Token protection is already handled by getenv() which intercepts all env var reads. The unsetenv() call in getenv removes tokens from both getenv and secure_getenv code paths. Also: eagerly resolve dlsym pointers in __attribute__((constructor)) to prevent dlsym deadlocks during other libraries' constructor execution. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Chroot tests passed! Smoke Chroot - All security and functionality tests succeeded. |
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
|
🎬 THE END — Smoke Claude MISSION: ACCOMPLISHED! The hero saves the day! ✨ |
|
✨ The prophecy is fulfilled... Smoke Codex has completed its mystical journey. The stars align. 🌟 |
✅ Coverage Check PassedOverall Coverage
📁 Per-file Coverage Changes (2 files)
Coverage comparison generated by |
Build Test Results: Node.js
Overall: PASS ✅ All Node.js test projects built and tested successfully.
|
Deno Build Test Results
Overall: ✅ PASS All Deno tests passed successfully.
|
Smoke Test ResultsLast 2 Merged PRs:
Test Results:
Status: PASS
|
Go Build Test Results
Overall: PASS ✅ All Go projects built and tested successfully.
|
Rust Build Test Results
Overall: PASS ✅ All Rust projects built successfully and all tests passed.
|
✅ Bun Build Test Results
Overall: PASS ✅ All Bun build tests completed successfully!
|
|
PR titles: fix: harden one-shot-token binary against ELF reconnaissance | test: add comprehensive coverage for TOCTOU fix error paths
|
Smoke Test ResultsLast 2 Merged PRs:
Tests:
Overall Status: FAIL (Playwright timeout) cc
|
Chroot Mode Version Comparison Test Results
Overall Result: Some version mismatches detected between host and chroot environments. The test validates that chroot mode correctly accesses host binaries. Python and Node.js show minor version differences, likely due to different installation sources or update timing. Go versions match exactly.
|
C++ Build Test Results
Overall: PASS ✅ All C++ builds completed successfully.
|
Analysis: Runner JWT Token Vulnerability Assessment
Vulnerability Summary
Secret Digger Run 32 discovered a GitHub Actions Runner JWT token stored in:
/home/runner/actions-runner/cached/.credentials-rw-r--r--(world-readable)eyJhbGciOiJSUzI1NiI...)Current Protection Status: ✅ PROTECTED
Our granular selective mounting implementation (v0.14.1+) successfully prevents access to this credential.
Why It's Protected
Path Analysis:
/home/runner/actions-runner/cached/.credentials/home/runner/(HOME directory)$GITHUB_WORKSPACE(/home/runner/work/gh-aw-firewall/gh-aw-firewall)Mount Hierarchy:
Security Mechanism:
$GITHUB_WORKSPACE, not entire$HOMEactions-runner/directory is a sibling towork/, not within workspace/home/runner/actions-runner/cached/.credentialsis completely inaccessibleVerification
Commands that would expose the credential under old implementation:
Under current implementation (v0.14.1+):
Defense-in-Depth Status
/dev/nulloverlaysComparison: Before vs After Fix
Before Fix (v0.13.0-v0.14.0 - VULNERABLE):
/home/runner:/home/runner:rw(entire HOME)actions-runner/cached/.credentialsfully accessibleAfter Fix (v0.14.1+ - PROTECTED):
/home/runner/work/repo/repo:/home/runner/work/repo/repo:rw(workspace only)actions-runner/cached/.credentialsnot mounted, inaccessibleRecommendations
✅ No immediate action required - Current implementation protects against this vulnerability
Additional Hardening (Optional):
Conclusion
The granular selective mounting fix implemented in v0.14.1 successfully protects against the runner JWT token vulnerability discovered by Secret Digger. The credential file is located outside the workspace directory and is therefore not accessible to the agent container.