Skip to content

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.

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 2 commits into from
Feb 12, 2026
Merged

fix: hide workDir from agent container to prevent secrets exposure #718

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
374f138
fix: hide workDir from agent container to prevent secrets exposure
Mossaka Feb 12, 2026
7369f45
fix: address review comments and fix Java workflow Maven proxy
Mossaka 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
8 changes: 4 additions & 4 deletions .github/workflows/build-test-java.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,19 +50,19 @@ Clone and test the following projects from the test repository:
1. **Clone Repository**: `gh repo clone Mossaka/gh-aw-firewall-test-java /tmp/test-java`
- **CRITICAL**: If clone fails, immediately call `safeoutputs-missing_tool` with message "CLONE_FAILED: Unable to clone test repository" and stop execution

2. **Configure Maven Proxy**: Maven ignores Java system properties for proxy configuration, so you must create `~/.m2/settings.xml` before running any Maven commands:
2. **Configure Maven Proxy**: Maven ignores Java system properties for proxy configuration, so you must create `~/.m2/settings.xml` before running any Maven commands. **IMPORTANT**: Use the literal values `squid-proxy` and `3128` directly in the XML - do NOT use shell variables or environment variable syntax:
```bash
mkdir -p ~/.m2
cat > ~/.m2/settings.xml << SETTINGS
cat > ~/.m2/settings.xml << 'SETTINGS'
<settings>
<proxies>
<proxy>
<id>awf-http</id><active>true</active><protocol>http</protocol>
<host>${SQUID_PROXY_HOST}</host><port>${SQUID_PROXY_PORT}</port>
<host>squid-proxy</host><port>3128</port>
</proxy>
<proxy>
<id>awf-https</id><active>true</active><protocol>https</protocol>
<host>${SQUID_PROXY_HOST}</host><port>${SQUID_PROXY_PORT}</port>
<host>squid-proxy</host><port>3128</port>
</proxy>
</proxies>
</settings>
Expand Down
85 changes: 85 additions & 0 deletions src/docker-manager.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1420,6 +1420,39 @@ describe('docker-manager', () => {
expect(env.AWF_DNS_SERVERS).toBe('8.8.8.8,8.8.4.4');
});
});

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);
});
});
Comment on lines +1424 to +1455
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.

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.

Copilot uses AI. Check for mistakes.
});

describe('writeConfigs', () => {
Expand Down Expand Up @@ -1566,6 +1599,58 @@ describe('docker-manager', () => {
}
});

it('should create work directory with restricted permissions (0o700)', async () => {
const newWorkDir = path.join(testDir, 'restricted-dir');
const config: WrapperConfig = {
allowedDomains: ['github.com'],
agentCommand: 'echo test',
logLevel: 'info',
keepContainers: false,
workDir: newWorkDir,
};

try {
await writeConfigs(config);
} catch {
// May fail if seccomp profile not found
}

// Verify directory was created with restricted permissions
expect(fs.existsSync(newWorkDir)).toBe(true);
const stats = fs.statSync(newWorkDir);
expect((stats.mode & 0o777).toString(8)).toBe('700');
});

it('should write config files with restricted permissions (0o600)', async () => {
const config: WrapperConfig = {
allowedDomains: ['github.com'],
agentCommand: 'echo test',
logLevel: 'info',
keepContainers: false,
workDir: testDir,
};

try {
await writeConfigs(config);
} catch {
// May fail after writing configs
}

// Verify squid.conf has restricted permissions
const squidConfPath = path.join(testDir, 'squid.conf');
if (fs.existsSync(squidConfPath)) {
const stats = fs.statSync(squidConfPath);
expect((stats.mode & 0o777).toString(8)).toBe('600');
}

// Verify docker-compose.yml has restricted permissions
const dockerComposePath = path.join(testDir, 'docker-compose.yml');
if (fs.existsSync(dockerComposePath)) {
const stats = fs.statSync(dockerComposePath);
expect((stats.mode & 0o777).toString(8)).toBe('600');
}
});

it('should use proxyLogsDir when specified', async () => {
const proxyLogsDir = path.join(testDir, 'custom-proxy-logs');
const config: WrapperConfig = {
Expand Down
42 changes: 32 additions & 10 deletions src/docker-manager.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -718,17 +718,33 @@ export function generateDockerCompose(
dns_search: [], // Disable DNS search domains to prevent embedded DNS fallback
volumes: agentVolumes,
environment,
// Hide /tmp/gh-aw/mcp-logs directory using tmpfs (empty in-memory filesystem)
// This prevents the agent from accessing MCP server logs while still allowing
// the host to write logs to /tmp/gh-aw/mcp-logs/ (e.g., /tmp/gh-aw/mcp-logs/safeoutputs/)
// For normal mode: hide /tmp/gh-aw/mcp-logs
// For chroot mode: hide both /tmp/gh-aw/mcp-logs and /host/tmp/gh-aw/mcp-logs
// SECURITY: Hide sensitive directories from agent using tmpfs overlays (empty in-memory filesystems)
//
// 1. MCP logs: tmpfs over /tmp/gh-aw/mcp-logs prevents the agent from reading
// MCP server logs inside the container. The host can still write to its own
// /tmp/gh-aw/mcp-logs directory since tmpfs only affects the container's view.
//
// 2. WorkDir: tmpfs over workDir (e.g., /tmp/awf-<timestamp>) prevents the agent
// from reading docker-compose.yml which contains environment variables (tokens,
// API keys) in plaintext. Without this overlay, code inside the container could
// extract secrets via: cat /tmp/awf-*/docker-compose.yml
// Note: volume mounts of workDir subdirectories (agent-logs, squid-logs, etc.)
// are mapped to different container paths (e.g., ~/.copilot/logs, /var/log/squid)
// so they are unaffected by the tmpfs overlay on workDir.
//
// For chroot mode: hide both normal and /host-prefixed paths since /tmp is
// mounted at both /tmp and /host/tmp
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`,
],
Comment on lines 737 to +747
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.

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.

Copilot uses AI. Check for mistakes.
depends_on: {
'squid-proxy': {
condition: 'service_healthy',
Expand Down Expand Up @@ -852,9 +868,13 @@ export function generateDockerCompose(
export async function writeConfigs(config: WrapperConfig): Promise<void> {
logger.debug('Writing configuration files...');

// Ensure work directory exists
// 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);
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.

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:

  1. Break access for other processes that legitimately need to read the directory
  2. Cause issues in multi-user environments
  3. 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).

Suggested change
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);
}

Copilot uses AI. Check for mistakes.
}
Comment on lines +871 to 878
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.

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);

Copilot uses AI. Check for mistakes.

// Create agent logs directory for persistence
Expand Down Expand Up @@ -960,13 +980,15 @@ export async function writeConfigs(config: WrapperConfig): Promise<void> {
allowHostPorts: config.allowHostPorts,
});
const squidConfigPath = path.join(config.workDir, 'squid.conf');
fs.writeFileSync(squidConfigPath, squidConfig);
fs.writeFileSync(squidConfigPath, squidConfig, { mode: 0o600 });
logger.debug(`Squid config written to: ${squidConfigPath}`);

// Write Docker Compose config
// Uses mode 0o600 (owner-only read/write) because this file contains sensitive
// environment variables (tokens, API keys) in plaintext
const dockerCompose = generateDockerCompose(config, networkConfig, sslConfig);
const dockerComposePath = path.join(config.workDir, 'docker-compose.yml');
fs.writeFileSync(dockerComposePath, yaml.dump(dockerCompose));
fs.writeFileSync(dockerComposePath, yaml.dump(dockerCompose), { mode: 0o600 });
logger.debug(`Docker Compose config written to: ${dockerComposePath}`);
}

Expand Down
14 changes: 14 additions & 0 deletions src/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -856,6 +856,20 @@ export interface DockerService {
* @example '/workspace'
*/
working_dir?: string;

/**
* Tmpfs mounts for the container
*
* In-memory filesystems mounted over files or directories to 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).
*
* Note: volume mounts of subdirectories that map to different container
* paths are unaffected by a tmpfs overlay on the parent directory.
*
* @example ['/tmp/awf-123:rw,noexec,nosuid,size=1m']
*/
tmpfs?: string[];
}

/**
Expand Down
Loading