fix: create /tmp/gh-aw/mcp-logs before Docker mount#707
Conversation
Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Fixes a Docker startup failure caused by mounting /dev/null onto a non-existent target directory (/tmp/gh-aw/mcp-logs) used for hiding MCP server logs in selective mounting mode.
Changes:
- Create
/tmp/gh-aw/mcp-logsduringwriteConfigs()before containers start. - Add a Jest test intended to verify the directory is created.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/docker-manager.ts | Ensures /tmp/gh-aw/mcp-logs exists prior to Docker volume mounts used for MCP log hiding. |
| src/docker-manager.test.ts | Adds a unit test checking that /tmp/gh-aw/mcp-logs exists after writeConfigs(). |
Comments suppressed due to low confidence (1)
src/docker-manager.ts:876
writeConfigs()currently creates/tmp/gh-aw/mcp-logsunconditionally, even whenallowFullFilesystemAccessis enabled (i.e., when the/dev/nullhiding mount isn’t added). If possible, gate this directory creation behind the same condition that adds the mount to reduce side effects on the host filesystem.
// Create /tmp/gh-aw/mcp-logs directory for hiding via /dev/null mount
// This directory must exist before Docker tries to mount /dev/null over it
// (selective mounting mode hides this directory to prevent MCP log exfiltration)
const mcpLogsDir = '/tmp/gh-aw/mcp-logs';
if (!fs.existsSync(mcpLogsDir)) {
fs.mkdirSync(mcpLogsDir, { recursive: true, mode: 0o755 });
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const mcpLogsDir = '/tmp/gh-aw/mcp-logs'; | ||
| if (!fs.existsSync(mcpLogsDir)) { | ||
| fs.mkdirSync(mcpLogsDir, { recursive: true, mode: 0o755 }); | ||
| logger.debug(`MCP logs directory created at: ${mcpLogsDir}`); | ||
| } |
There was a problem hiding this comment.
fs.existsSync(mcpLogsDir) only checks that the path exists, not that it’s a directory. If /tmp/gh-aw/mcp-logs exists as a regular file (or symlink), this code will skip mkdirSync and Docker will still fail with the same “not a directory” mount error. Consider validating with fs.statSync/lstatSync and either replacing non-directories with a directory or throwing a clear error.
This issue also appears on line 871 of the same file.
| it('should create /tmp/gh-aw/mcp-logs directory', async () => { | ||
| const config: WrapperConfig = { | ||
| allowedDomains: ['github.com'], | ||
| agentCommand: 'echo test', | ||
| logLevel: 'info', | ||
| keepContainers: false, | ||
| workDir: testDir, | ||
| }; | ||
|
|
||
| try { | ||
| await writeConfigs(config); | ||
| } catch { | ||
| // May fail, but directories should still be created | ||
| } | ||
|
|
||
| // Verify /tmp/gh-aw/mcp-logs directory was created | ||
| expect(fs.existsSync('/tmp/gh-aw/mcp-logs')).toBe(true); | ||
| }); |
There was a problem hiding this comment.
This test can pass even if /tmp/gh-aw/mcp-logs already existed before the test ran, so it may not actually validate the new directory-creation behavior. It also leaves /tmp/gh-aw/mcp-logs behind after the suite completes. Consider making the test hermetic by ensuring the path does not exist in setup (and cleaning it up in teardown), or by mocking/spying on fs.existsSync/fs.mkdirSync to assert the creation call happens.
* Initial plan * feat: hide /tmp/gh-aw/mcp-logs/ directory from agent container Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> * test: improve mcp-logs tests to verify mount protection * fix: create /tmp/gh-aw/mcp-logs before Docker mount (#707) * Initial plan * fix: create /tmp/gh-aw/mcp-logs directory before mounting Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> * test: add test for /tmp/gh-aw/mcp-logs directory creation 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> * fix: use tmpfs to hide /tmp/gh-aw/mcp-logs directory from containers (#709) * Initial plan * fix: revert file approach, need directory solution for mcp-logs Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> * fix: use tmpfs to hide /tmp/gh-aw/mcp-logs directory from container Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> * test: verify tmpfs mount solution works end-to-end Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> * fix: use 0o777 permissions for mcp-logs and squid-logs directories (#710) * Initial plan * fix: set /tmp/gh-aw/mcp-logs to world-writable (0o777) Fixes permission denied error when GitHub Actions workflows try to create subdirectories in /tmp/gh-aw/mcp-logs after AWF runs with sudo. Changes: - Set directory permissions to 0o777 (rwxrwxrwx) instead of 0o755 - Explicitly call chmodSync after mkdirSync to bypass umask - Fix permissions if directory already exists from previous run - Update test to verify 777 permissions Root cause: When AWF runs with sudo (e.g., --enable-chroot), it creates /tmp/gh-aw/mcp-logs owned by root. With 755 permissions, non-root users cannot create subdirectories. Using 777 allows workflows to create subdirectories like /tmp/gh-aw/mcp-logs/safeoutputs without sudo. Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> * fix: ensure squid logs dir has 777 permissions Apply same fix to squidLogsDir for consistency with mcpLogsDir. Explicitly calls chmodSync to bypass umask effects. 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> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> 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>
Docker mount fails when target directory doesn't exist, causing container startup to fail with "not a directory" error when attempting to mount
/dev/nullover/tmp/gh-aw/mcp-logsfor credential hiding.Changes
/tmp/gh-aw/mcp-logsinwriteConfigs()before container startupContext
The firewall hides MCP server logs via
/dev/nulloverlay mounts (lines 670, 703) to prevent exfiltration. Docker requires the mount target to exist beforehand.✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.