-
Notifications
You must be signed in to change notification settings - Fork 6
fix: hide workDir from agent container to prevent secrets exposure #718
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Sensitive tokens (GITHUB_TOKEN, ANTHROPIC_API_KEY, etc.) are written in plaintext to docker-compose.yml inside the workDir (/tmp/awf-*). Since the agent container mounts /tmp:/tmp:rw, any code inside the container could read these secrets via `cat /tmp/awf-*/docker-compose.yml`. Primary fix: Add tmpfs overlay on workDir (same pattern as mcp-logs hiding) so the agent sees an empty in-memory filesystem instead of the real directory containing docker-compose.yml with all tokens. Secondary fix (defense-in-depth): Restrict file permissions on workDir (0o700) and config files (0o600) so non-root processes on the host cannot read them either. Both normal mode and chroot mode are covered with appropriate paths. Closes #62, closes #206, closes #210 Co-Authored-By: Claude Opus 4.6 <[email protected]>
|
🎬 THE END — Smoke Claude MISSION: ACCOMPLISHED! The hero saves the day! ✨ |
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
|
Chroot tests passed! Smoke Chroot - All security and functionality tests succeeded. |
|
✨ The prophecy is fulfilled... Smoke Codex has completed its mystical journey. The stars align. 🌟 |
✅ Coverage Check PassedOverall Coverage
📁 Per-file Coverage Changes (1 files)
Coverage comparison generated by |
Smoke Test ResultsLast 2 merged PRs:
✅ GitHub MCP - Read PRs successfully Overall Status: PASS
|
Bun Build Test Results
Overall: PASS ✅ All Bun projects successfully installed dependencies and passed tests.
|
Deno Build Test Results
Overall: ✅ PASS All Deno tests completed successfully.
|
Smoke Test ResultsLast 2 merged PRs:
Test Results: Status: PASS cc @Mossaka
|
Node.js Build Test Results
Overall: PASS ✅ All Node.js projects successfully installed dependencies and passed their test suites.
|
Go Build Test Results ✅All Go projects passed successfully!
Overall: PASS ✅ All modules downloaded successfully and all tests passed.
|
Rust Build Test Results
Overall: PASS ✅ All Rust projects built successfully and all tests passed.
|
C++ Build Test Results
Overall: PASS ✅ All C++ projects built successfully with CMake and make.
|
|
GitHub MCP ✅ - feat: add release highlights generator agentic workflow | feat: hide /tmp/gh-aw/mcp-logs/ from agent containers
|
Chroot Test ResultsChroot mode verification comparing host vs container runtime versions:
Status: Tests FAILED - Not all runtimes matched between host and container. Details
The chroot mode successfully accessed host binaries for Go, but Python and Node.js are running different versions, indicating they may be using container-provided binaries instead of host binaries.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR attempts to fix a security vulnerability where sensitive environment variables (GITHUB_TOKEN, ANTHROPIC_API_KEY, etc.) stored in /tmp/awf-*/docker-compose.yml could be read by malicious code running inside the agent container via the /tmp:/tmp:rw mount. However, the implementation contains a critical bug that will break essential container functionality.
Changes:
- Added tmpfs overlays on workDir to hide docker-compose.yml from agent container (broken implementation)
- Restricted file permissions on workDir (0o700), squid.conf (0o600), and docker-compose.yml (0o600)
- Added tmpfs property to DockerService TypeScript interface
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| src/types.ts | Added tmpfs property to DockerService interface for tmpfs mount configuration |
| src/docker-manager.ts | Added tmpfs overlays on workDir paths and restricted permissions on directory and config files |
| src/docker-manager.test.ts | Added 5 unit tests for tmpfs overlay and file permission restrictions |
Comments suppressed due to low confidence (1)
src/docker-manager.ts:744
- Critical bug: The tmpfs overlay on workDir will break volume mounts that depend on subdirectories within workDir.
The agent container mounts several subdirectories from workDir:
- Line 439:
${config.workDir}/agent-logs→${effectiveHome}/.copilot/logs(agent logs persistence) - Line 560:
${config.workDir}/chroot-*/hosts→/host/etc/hosts(chroot mode DNS resolution) - Line 574:
${config.workDir}/ssl/ca-cert.pem→/usr/local/share/ca-certificates/awf-ca.crt(SSL bump CA trust)
Docker processes tmpfs mounts before bind mounts. When tmpfs creates an empty in-memory filesystem at workDir inside the agent container, the host's workDir subdirectories become inaccessible. Subsequent volume mount declarations that reference these subdirectories will mount empty directories or fail.
This will cause:
- Agent logs to not persist (agent-logs mount fails)
- SSL bump to fail (CA certificate mount fails)
- Chroot DNS resolution to fail (hosts file mount fails)
Alternative solutions:
- Move sensitive files (docker-compose.yml, squid.conf) to a separate subdirectory (e.g., workDir/secrets/) and only tmpfs that subdirectory
- Don't use tmpfs - rely only on file permissions (0o600) as defense-in-depth since /tmp mounting is necessary for other functionality
- Use a completely separate directory outside /tmp for docker-compose.yml that isn't mounted into containers
Note: The squid container also mounts from workDir (squid.conf, SSL files) but is not affected by agent's tmpfs since each container has independent filesystem configuration.
tmpfs: config.enableChroot
? [
'/tmp/gh-aw/mcp-logs:rw,noexec,nosuid,size=1m',
'/host/tmp/gh-aw/mcp-logs:rw,noexec,nosuid,size=1m',
`${config.workDir}:rw,noexec,nosuid,size=1m`,
`/host${config.workDir}:rw,noexec,nosuid,size=1m`,
]
: [
'/tmp/gh-aw/mcp-logs:rw,noexec,nosuid,size=1m',
`${config.workDir}:rw,noexec,nosuid,size=1m`,
],
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // SECURITY: Hide sensitive directories from agent using tmpfs overlays (empty in-memory filesystems) | ||
| // | ||
| // 1. Hide /tmp/gh-aw/mcp-logs - prevents agent from accessing MCP server logs | ||
| // while still allowing the host to write logs there | ||
| // | ||
| // 2. Hide workDir (e.g., /tmp/awf-<timestamp>) - prevents agent from reading | ||
| // docker-compose.yml which contains all environment variables (tokens, API keys) | ||
| // in plaintext. Without this, any code inside the container could extract secrets via: | ||
| // cat /tmp/awf-*/docker-compose.yml | ||
| // This is the primary fix for the secrets exposure vulnerability. | ||
| // |
Copilot
AI
Feb 12, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment incorrectly states that mounting tmpfs over workDir "while still allowing the host to write logs to /tmp/gh-aw/mcp-logs/".
However, workDir is different from /tmp/gh-aw/mcp-logs. The tmpfs overlay on workDir doesn't affect /tmp/gh-aw/mcp-logs at all - they are separate directories with independent tmpfs overlays. This comment conflates two different security measures:
- MCP logs hiding: tmpfs on /tmp/gh-aw/mcp-logs
- WorkDir hiding: tmpfs on workDir (e.g., /tmp/awf-12345)
The comment should clarify that the host writes to workDir subdirectories (agent-logs, ssl, etc.) which are then mounted as volumes, but these won't work correctly with the tmpfs overlay on the parent directory.
This issue also appears on line 734 of the same file.
| // SECURITY: Hide sensitive directories from agent using tmpfs overlays (empty in-memory filesystems) | |
| // | |
| // 1. Hide /tmp/gh-aw/mcp-logs - prevents agent from accessing MCP server logs | |
| // while still allowing the host to write logs there | |
| // | |
| // 2. Hide workDir (e.g., /tmp/awf-<timestamp>) - prevents agent from reading | |
| // docker-compose.yml which contains all environment variables (tokens, API keys) | |
| // in plaintext. Without this, any code inside the container could extract secrets via: | |
| // cat /tmp/awf-*/docker-compose.yml | |
| // This is the primary fix for the secrets exposure vulnerability. | |
| // | |
| // SECURITY: Hide sensitive directories from the agent using tmpfs overlays (empty in-memory filesystems) | |
| // | |
| // 1. MCP logs hiding: mount tmpfs over /tmp/gh-aw/mcp-logs to prevent the agent | |
| // from reading MCP server logs inside the container. This tmpfs only affects | |
| // the container’s view of /tmp/gh-aw/mcp-logs; the host can still write logs | |
| // to its own /tmp/gh-aw/mcp-logs directory on the host filesystem. | |
| // | |
| // 2. WorkDir hiding: mount tmpfs over workDir (e.g., /tmp/awf-<timestamp>) to | |
| // prevent the agent from reading docker-compose.yml, which contains all | |
| // environment variables (tokens, API keys) in plaintext. Without this, any | |
| // code inside the container could extract secrets via: | |
| // cat /tmp/awf-*/docker-compose.yml | |
| // This is the primary fix for the secrets exposure vulnerability. | |
| // | |
| // The host creates subdirectories under workDir (e.g., agent-logs, ssl, etc.) | |
| // and mounts them into the container as volumes. These volume mounts depend | |
| // on the real host directories and will not behave as expected if the parent | |
| // workDir inside the container is overlaid with tmpfs. | |
| // |
src/types.ts
Outdated
| * In-memory filesystems mounted over directories to hide their contents. | ||
| * Used as a security measure to prevent the agent from reading sensitive | ||
| * files (e.g., docker-compose.yml containing tokens, MCP logs). | ||
| * | ||
| * @example ['/tmp/awf-123:rw,noexec,nosuid,size=1m'] | ||
| */ |
Copilot
AI
Feb 12, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The documentation states tmpfs is used to hide "directories" but the example shows a directory path. This is technically correct for Docker's tmpfs implementation, but it's important to note that tmpfs can also overlay individual files (not just directories).
Given the critical bug in the actual implementation (see comment on docker-manager.ts lines 734-744), this documentation should be updated to reflect that tmpfs overlays can target both files and directories, and should warn about the implications of overlaying parent directories when subdirectories are mounted as volumes.
| * In-memory filesystems mounted over directories to hide their contents. | |
| * Used as a security measure to prevent the agent from reading sensitive | |
| * files (e.g., docker-compose.yml containing tokens, MCP logs). | |
| * | |
| * @example ['/tmp/awf-123:rw,noexec,nosuid,size=1m'] | |
| */ | |
| * In-memory filesystems mounted over files or directories to hide or shadow | |
| * their contents. Used as a security measure to prevent the agent from | |
| * reading sensitive files (e.g., docker-compose.yml containing tokens, MCP logs). | |
| * | |
| * Be careful when mounting tmpfs over parent directories (for example, /tmp or | |
| * /workspace) if subpaths are also mounted as bind volumes. A tmpfs overlay on | |
| * the parent directory will hide any underlying files and may also make | |
| * mounted subdirectories inaccessible from inside the container. | |
| * | |
| * @example ['/tmp/awf-123:rw,noexec,nosuid,size=1m'] | |
| */ | |
| */ |
| fs.mkdirSync(config.workDir, { recursive: true }); | ||
| fs.mkdirSync(config.workDir, { recursive: true, mode: 0o700 }); | ||
| } else { | ||
| fs.chmodSync(config.workDir, 0o700); |
Copilot
AI
Feb 12, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential issue: chmodSync on existing workDir may be too restrictive if the directory was created by a different user or with different intentions.
If workDir already exists (e.g., from a previous run, or manually created), forcing it to 0o700 could:
- Break access for other processes that legitimately need to read the directory
- Cause issues in multi-user environments
- Override intentionally-set permissions
Consider checking the current permissions and only applying chmod if they're too permissive (e.g., world-readable). Alternatively, document that workDir should not be reused between runs, and consider using a unique timestamp-based directory for each run (which appears to already be the case based on the /tmp/awf-* pattern mentioned in comments).
| fs.chmodSync(config.workDir, 0o700); | |
| // If the directory already exists, only tighten permissions if it is | |
| // world-accessible. This avoids unexpectedly overriding intentionally-set | |
| // permissions (e.g., in shared or multi-user environments) while still | |
| // preventing access by "others". | |
| const stat = fs.statSync(config.workDir); | |
| const currentMode = stat.mode & 0o777; | |
| if ((currentMode & 0o007) !== 0) { | |
| fs.chmodSync(config.workDir, 0o700); | |
| } |
| tmpfs: config.enableChroot | ||
| ? [ | ||
| '/tmp/gh-aw/mcp-logs:rw,noexec,nosuid,size=1m', | ||
| '/host/tmp/gh-aw/mcp-logs:rw,noexec,nosuid,size=1m', | ||
| `${config.workDir}:rw,noexec,nosuid,size=1m`, | ||
| `/host${config.workDir}:rw,noexec,nosuid,size=1m`, | ||
| ] | ||
| : ['/tmp/gh-aw/mcp-logs:rw,noexec,nosuid,size=1m'], | ||
| : [ | ||
| '/tmp/gh-aw/mcp-logs:rw,noexec,nosuid,size=1m', | ||
| `${config.workDir}:rw,noexec,nosuid,size=1m`, | ||
| ], |
Copilot
AI
Feb 12, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Discrepancy: The PR description claims to close #62, #206, and #210, but the linked PR #62 is about "docs: add mermaid diagram rendering for Astro Starlight site" which is completely unrelated to this security fix.
This appears to be a copy-paste error in the PR description. The correct issues that this PR addresses should be verified and the description should be corrected to reference the actual security vulnerability issues.
| // Ensure work directory exists with restricted permissions (owner-only access) | ||
| // Defense-in-depth: even if tmpfs overlay fails, non-root processes on the host | ||
| // cannot read the docker-compose.yml which contains sensitive tokens | ||
| if (!fs.existsSync(config.workDir)) { | ||
| fs.mkdirSync(config.workDir, { recursive: true }); | ||
| fs.mkdirSync(config.workDir, { recursive: true, mode: 0o700 }); | ||
| } else { | ||
| fs.chmodSync(config.workDir, 0o700); | ||
| } |
Copilot
AI
Feb 12, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing explicit permissions on agent-logs subdirectory. The agent-logs directory at line 878-882 is created without specifying mode, so it will inherit from the parent workDir's umask.
For consistency with the pattern established for squid-logs (mode 0o777) and mcp-logs (mode 0o777), and considering this directory needs to be writable by the agent container running as awfuser, this should explicitly set appropriate permissions.
However, note that this comment may be superseded by the critical bug in the tmpfs implementation that would prevent this subdirectory from being accessible anyway.
See below for a potential fix:
// Make directory writable by the agent container user (awfuser)
const agentLogsDir = path.join(config.workDir, 'agent-logs');
if (!fs.existsSync(agentLogsDir)) {
fs.mkdirSync(agentLogsDir, { recursive: true, mode: 0o777 });
// Explicitly set permissions to 0o777 (not affected by umask)
fs.chmodSync(agentLogsDir, 0o777);
| describe('workDir tmpfs overlay (secrets protection)', () => { | ||
| it('should hide workDir from agent container via tmpfs in normal mode', () => { | ||
| const result = generateDockerCompose(mockConfig, mockNetworkConfig); | ||
| const agent = result.services.agent; | ||
| const tmpfs = agent.tmpfs as string[]; | ||
|
|
||
| // workDir should be hidden via tmpfs overlay to prevent reading docker-compose.yml | ||
| expect(tmpfs).toContainEqual(expect.stringContaining(mockConfig.workDir)); | ||
| expect(tmpfs.some((t: string) => t.startsWith(`${mockConfig.workDir}:`))).toBe(true); | ||
| }); | ||
|
|
||
| it('should hide workDir at both paths in chroot mode', () => { | ||
| const configWithChroot = { ...mockConfig, enableChroot: true }; | ||
| const result = generateDockerCompose(configWithChroot, mockNetworkConfig); | ||
| const agent = result.services.agent; | ||
| const tmpfs = agent.tmpfs as string[]; | ||
|
|
||
| // Both /tmp/awf-test and /host/tmp/awf-test should be hidden | ||
| expect(tmpfs.some((t: string) => t.startsWith(`${mockConfig.workDir}:`))).toBe(true); | ||
| expect(tmpfs.some((t: string) => t.startsWith(`/host${mockConfig.workDir}:`))).toBe(true); | ||
| }); | ||
|
|
||
| it('should still hide mcp-logs alongside workDir', () => { | ||
| const result = generateDockerCompose(mockConfig, mockNetworkConfig); | ||
| const agent = result.services.agent; | ||
| const tmpfs = agent.tmpfs as string[]; | ||
|
|
||
| // Both mcp-logs and workDir should be hidden | ||
| expect(tmpfs.some((t: string) => t.includes('/tmp/gh-aw/mcp-logs'))).toBe(true); | ||
| expect(tmpfs.some((t: string) => t.startsWith(`${mockConfig.workDir}:`))).toBe(true); | ||
| }); | ||
| }); |
Copilot
AI
Feb 12, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The tests verify that tmpfs overlays are added, but they don't test for the critical bug where tmpfs on workDir will break volume mounts of subdirectories.
These tests should also verify that essential volume mounts (like agent-logs, SSL certificates, seccomp profile) are not broken by the tmpfs overlay. Consider adding integration tests that actually start containers to verify the tmpfs implementation doesn't interfere with required functionality.
The current unit tests only check that tmpfs entries exist in the configuration but don't validate the runtime behavior or interaction with volume mounts.
❌ Build Test: Java - FAILEDTest Results
Overall: FAILED Error DetailsCritical Issue: Maven proxy configuration error prevents builds from completing. Root Cause: The Maven Impact: Without proper proxy configuration, Maven cannot download dependencies from Maven Central, blocking all compile and test operations. Next Steps:
Repository Status: ✅ Test repository cloned successfully to
|
- Improve tmpfs comment to clarify two separate security measures
(mcp-logs hiding vs workDir hiding) and note that volume mounts
to different container paths are unaffected by the tmpfs overlay
- Update types.ts tmpfs docstring per review feedback
- Fix build-test-java.md: use literal proxy values (squid-proxy:3128)
instead of shell variables ${SQUID_PROXY_HOST}/${SQUID_PROXY_PORT}
which AI agents write literally without expansion, causing Maven
NumberFormatException
Co-Authored-By: Claude Opus 4.6 <[email protected]>
|
💫 TO BE CONTINUED... Smoke Claude failed! Our hero faces unexpected challenges... |
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
|
Chroot tests passed! Smoke Chroot - All security and functionality tests succeeded. |
|
✨ The prophecy is fulfilled... Smoke Codex has completed its mystical journey. The stars align. 🌟 |
Node.js Build Test Results
Overall: PASS ✅ All Node.js projects built and tested successfully.
|
Go Build Test Results
Overall: PASS ✅ All Go projects successfully built and tested.
|
C++ Build Test Results
Overall: PASS All C++ projects built successfully.
|
Deno Build Test Results
Overall: ✅ PASS All Deno tests completed successfully.
|
Build Test: Rust - FAILED ❌Status: Test execution blocked Issue: Rust toolchain (cargo, rustc) is not installed in the execution environment. Results
Overall: FAILED Error DetailsRequired ActionThe workflow execution environment needs the Rust toolchain installed. Consider:
|
Bun Build Test Results
Overall: PASS All Bun build tests completed successfully.
|
|
✅ GitHub MCP: #197 "feat: add release highlights generator agentic workflow", #706 "feat: hide /tmp/gh-aw/mcp-logs/ from agent containers" Overall: PASS cc @Mossaka
|
.NET Build Test Results
Overall: PASS ✅ All .NET projects successfully restored, built, and ran with expected output.
|
Java Build Test Results ✅All Java projects successfully compiled and tested through the AWF firewall.
Overall: PASS All dependencies were successfully downloaded through the Squid proxy (172.30.0.10:3128) and all tests passed.
|
|
PR titles:
|
Chroot Version Comparison Test Results
Overall Result: Some versions do not match between host and chroot environments. The chroot mode successfully provides access to host binaries, but version mismatches in Python and Node.js indicate that the container's binaries are being used instead of the host's for these runtimes.
|
Smoke Test Results ✅Last 2 Merged PRs:
Test Results:
Status: PASS 🎉 cc
|
Summary
Fixes a security vulnerability where sensitive tokens (GITHUB_TOKEN, GH_TOKEN, ANTHROPIC_API_KEY, COPILOT_GITHUB_TOKEN via
--env-all) written to/tmp/awf-*/docker-compose.ymlin plaintext could be read by code running inside the agent container, since/tmp:/tmp:rwis mounted.Primary fix: Add a tmpfs overlay on the workDir path (same pattern already used for hiding
/tmp/gh-aw/mcp-logs/). The agent container sees an empty in-memory filesystem instead of the real directory containingdocker-compose.ymlwith all tokens. Both normal mode and chroot mode paths are covered. Volume mounts of workDir subdirectories (agent-logs, squid-logs, etc.) map to different container paths, so they are unaffected.Secondary fix (defense-in-depth): Restrict file permissions on:
0o700(owner-only access)squid.conf:0o600(owner-only read/write)docker-compose.yml:0o600(owner-only read/write)Java workflow fix: Use literal proxy values (
squid-proxy:3128) in Maven settings.xml instead of shell variables (${SQUID_PROXY_HOST}/${SQUID_PROXY_PORT}) which AI agents write literally without expansion, causingNumberFormatException.Changes
src/docker-manager.ts: Added workDir to tmpfs overlay list (normal + chroot paths), restricted directory and file permissionssrc/types.ts: Addedtmpfsproperty toDockerServiceinterfacesrc/docker-manager.test.ts: Added 5 new tests covering tmpfs overlay behavior and file permission restrictions.github/workflows/build-test-java.md: Fixed Maven proxy configuration to use literal valuesTest plan
sudo awf --allow-domains github.com 'cat /tmp/awf-*/docker-compose.yml'should show empty directory🤖 Generated with Claude Code