Skip to content

fix: use 0o777 permissions for mcp-logs and squid-logs directories #710

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.

Sign up for GitHub

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

Merged
lpcox merged 3 commits into from
Feb 12, 2026
Merged

fix: use 0o777 permissions for mcp-logs and squid-logs directories #710

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
832a767
Initial plan
Copilot Feb 12, 2026
a228abf
fix: set /tmp/gh-aw/mcp-logs to world-writable (0o777)
Copilot Feb 12, 2026
10134c4
fix: ensure squid logs dir has 777 permissions
Copilot Feb 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion src/docker-manager.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1495,7 +1495,7 @@ describe('docker-manager', () => {
expect(fs.existsSync(path.join(testDir, 'squid-logs'))).toBe(true);
});

it('should create /tmp/gh-aw/mcp-logs directory', async () => {
it('should create /tmp/gh-aw/mcp-logs directory with world-writable permissions', async () => {
const config: WrapperConfig = {
allowedDomains: ['github.com'],
agentCommand: 'echo test',
Expand All @@ -1514,6 +1514,8 @@ describe('docker-manager', () => {
expect(fs.existsSync('/tmp/gh-aw/mcp-logs')).toBe(true);
const stats = fs.statSync('/tmp/gh-aw/mcp-logs');
expect(stats.isDirectory()).toBe(true);
// Verify permissions are 0o777 (rwxrwxrwx) to allow non-root users to create subdirectories
expect((stats.mode & 0o777).toString(8)).toBe('777');
});

it('should write squid.conf file', async () => {
Expand Down
12 changes: 11 additions & 1 deletion src/docker-manager.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -872,16 +872,26 @@ export async function writeConfigs(config: WrapperConfig): Promise<void> {
const squidLogsDir = config.proxyLogsDir || path.join(config.workDir, 'squid-logs');
if (!fs.existsSync(squidLogsDir)) {
fs.mkdirSync(squidLogsDir, { recursive: true, mode: 0o777 });
// Explicitly set permissions to 0o777 (not affected by umask)
fs.chmodSync(squidLogsDir, 0o777);
}
logger.debug(`Squid logs directory created at: ${squidLogsDir}`);

// Create /tmp/gh-aw/mcp-logs directory
// This directory exists on the HOST for MCP gateway to write logs
// Inside the AWF container, it's hidden via tmpfs mount (see generateDockerCompose)
// Uses mode 0o777 to allow GitHub Actions workflows and MCP gateway to create subdirectories
// even when AWF runs as root (e.g., sudo awf --enable-chroot)
const mcpLogsDir = '/tmp/gh-aw/mcp-logs';
if (!fs.existsSync(mcpLogsDir)) {
fs.mkdirSync(mcpLogsDir, { recursive: true, mode: 0o755 });
fs.mkdirSync(mcpLogsDir, { recursive: true, mode: 0o777 });
// Explicitly set permissions to 0o777 (not affected by umask)
fs.chmodSync(mcpLogsDir, 0o777);
logger.debug(`MCP logs directory created at: ${mcpLogsDir}`);
} else {
// Fix permissions if directory already exists (e.g., created by a previous run)
fs.chmodSync(mcpLogsDir, 0o777);
logger.debug(`MCP logs directory permissions fixed at: ${mcpLogsDir}`);
Comment on lines +893 to +894
Copy link

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the else branch, chmodSync(mcpLogsDir, 0o777) is unconditional. If /tmp/gh-aw/mcp-logs already exists and is owned by a different user (e.g., created by a prior sudo run) but already has 0777, a non-root run will still throw EPERM here and fail the workflow unnecessarily. Consider stat’ing first and only chmod’ing when the mode isn’t already 0777, and handle EPERM with a clearer error or a non-fatal warning when permissions are already sufficient.

This issue also appears on line 872 of the same file.

Suggested change
fs.chmodSync(mcpLogsDir, 0o777);
logger.debug(`MCP logs directory permissions fixed at: ${mcpLogsDir}`);
try {
const stats = fs.statSync(mcpLogsDir);
const currentMode = stats.mode & 0o777;
if (currentMode !== 0o777) {
try {
fs.chmodSync(mcpLogsDir, 0o777);
logger.debug(`MCP logs directory permissions fixed at: ${mcpLogsDir}`);
} catch (err: any) {
const code = (err as NodeJS.ErrnoException).code;
if (code === 'EPERM') {
logger.warn(
`MCP logs directory exists at ${mcpLogsDir} but permissions could not be changed to 0o777 due to EPERM. ` +
`Current mode is ${currentMode.toString(8)}.`
);
} else {
throw err;
}
}
} else {
logger.debug(`MCP logs directory permissions already set to 0o777: ${mcpLogsDir}`);
}
} catch (err) {
logger.warn(`Unable to verify or adjust MCP logs directory permissions at ${mcpLogsDir}: ${err}`);
}

Copilot uses AI. Check for mistakes.
}

// Use fixed network configuration (network is created by host-iptables.ts)
Expand Down
Loading