From 2f414dcc9f96ac788f40a1d63af66e0f9039c74c Mon Sep 17 00:00:00 2001 From: Landon Cox Date: Tue, 7 Jul 2026 05:47:44 -0700 Subject: [PATCH] fix: pre-flight reclaim of stale root-owned directories before writeConfigs On persistent GitHub Actions runners, a previous AWF run (or Docker daemon) can leave /tmp/gh-aw/sandbox/firewall/ owned by root. When the next AWF invocation runs as non-root (rootless/network-isolation mode), mkdirSync fails with EACCES before any container is started: EACCES: permission denied, mkdir '/tmp/gh-aw/sandbox/firewall/logs' This adds a pre-flight reclaim step that detects non-writable ancestor directories and removes them (via sudo rm -rf, then fs.rmSync fallback) before mkdirSync is called. The reclaim is applied to: - workDir (validateAndPrepareWorkDir in config-writer.ts) - All log directories (prepareLogDirectories in workdir-setup.ts) - /tmp/gh-aw/mcp-logs (hardcoded MCP gateway path) Safety guards: - Only acts when running as non-root (root never gets EACCES) - Only removes the narrowest non-writable ancestor - Refuses to operate on protected system paths (/, /tmp, /home/runner, etc.) - Logs all actions for auditability - Falls back gracefully if neither sudo nor rmSync succeeds This is the AWF-side complement to gh-aw's compiled workflow chmod steps (#30414, #27742) which fix artifact upload permissions post-run. This fix addresses the pre-run failure path that those PRs cannot cover. Fixes: github/gh-aw#42400 (supersedes the closed, unmerged approach) Ref: https://github.com/github/gh-aw/actions/runs/28856007222/job/85583002595 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/config-writer.ts | 10 +++ src/preflight-reclaim.test.ts | 163 ++++++++++++++++++++++++++++++++++ src/preflight-reclaim.ts | 159 +++++++++++++++++++++++++++++++++ src/workdir-setup.ts | 12 +++ 4 files changed, 344 insertions(+) create mode 100644 src/preflight-reclaim.test.ts create mode 100644 src/preflight-reclaim.ts diff --git a/src/config-writer.ts b/src/config-writer.ts index 8f8851309..6712c9f8d 100644 --- a/src/config-writer.ts +++ b/src/config-writer.ts @@ -18,6 +18,7 @@ import { SQUID_IP, } from './host-iptables-shared'; import { prepareWorkDirectories } from './workdir-setup'; +import { reclaimStaleDirectory } from './preflight-reclaim'; // When bundled with esbuild, this global is replaced at build time with the // JSON content of containers/agent/seccomp-profile.json. In normal (tsc) @@ -41,8 +42,17 @@ interface NetworkConfig { * symlink injection, and re-applies the permission mask on pre-existing dirs. * Security-critical: docker-compose.yml (which contains plaintext secrets) is * written here, so non-root host processes must not be able to read it. + * + * Pre-flight: on persistent runners, a previous AWF run may leave the workDir + * (or its ancestors) owned by root. We detect and reclaim these before mkdirSync + * to prevent EACCES failures in rootless/network-isolation mode. */ function validateAndPrepareWorkDir(config: WrapperConfig): void { + // Pre-flight: reclaim stale root-owned directories from previous runs. + // This handles the case where Docker containers (or AWF running as root) + // leave directories that the current non-root user cannot write to. + reclaimStaleDirectory(config.workDir); + // 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 diff --git a/src/preflight-reclaim.test.ts b/src/preflight-reclaim.test.ts new file mode 100644 index 000000000..2e5e3f5d3 --- /dev/null +++ b/src/preflight-reclaim.test.ts @@ -0,0 +1,163 @@ +import * as fs from 'fs'; +import * as path from 'path'; +import * as os from 'os'; +import execa from 'execa'; +import { preflightReclaimTestHelpers } from './preflight-reclaim'; + +const { findNonWritableAncestor, isProtectedPath, reclaimStaleDirectory } = preflightReclaimTestHelpers; + +jest.mock('execa', () => ({ + sync: jest.fn(), +})); + +const mockExecaSync = execa.sync as jest.MockedFunction; + +describe('preflight-reclaim', () => { + let tempDir: string; + + beforeEach(() => { + tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'awf-reclaim-test-')); + jest.clearAllMocks(); + }); + + afterEach(() => { + fs.rmSync(tempDir, { recursive: true, force: true }); + }); + + describe('isProtectedPath', () => { + it('returns true for root', () => { + expect(isProtectedPath('/')).toBe(true); + }); + + it('returns true for /tmp', () => { + expect(isProtectedPath('/tmp')).toBe(true); + }); + + it('returns true for /home/runner', () => { + expect(isProtectedPath('/home/runner')).toBe(true); + }); + + it('returns true for /home/runner/work', () => { + expect(isProtectedPath('/home/runner/work')).toBe(true); + }); + + it('returns false for a subdirectory of /tmp', () => { + expect(isProtectedPath('/tmp/gh-aw')).toBe(false); + }); + + it('returns false for a user working directory', () => { + expect(isProtectedPath('/tmp/gh-aw/sandbox/firewall')).toBe(false); + }); + }); + + describe('findNonWritableAncestor', () => { + it('returns undefined when target does not exist and parent is writable', () => { + const target = path.join(tempDir, 'does', 'not', 'exist'); + // tempDir is writable, so finding the first existing writable ancestor means no issue + expect(findNonWritableAncestor(target)).toBeUndefined(); + }); + + it('returns undefined when target exists and is writable', () => { + const target = path.join(tempDir, 'subdir'); + fs.mkdirSync(target); + expect(findNonWritableAncestor(target)).toBeUndefined(); + }); + + it('returns the non-writable directory when target exists but is not writable', () => { + const target = path.join(tempDir, 'readonly'); + fs.mkdirSync(target); + fs.chmodSync(target, 0o555); // read+execute only + try { + const result = findNonWritableAncestor(path.join(target, 'child')); + expect(result).toBe(target); + } finally { + fs.chmodSync(target, 0o755); // restore for cleanup + } + }); + + it('returns the non-writable ancestor when only parent is non-writable', () => { + const parent = path.join(tempDir, 'parent'); + fs.mkdirSync(parent); + fs.chmodSync(parent, 0o555); // make parent non-writable + try { + // target does not exist yet, and its parent (parent) is not writable + const target = path.join(parent, 'newchild', 'grandchild'); + const result = findNonWritableAncestor(target); + expect(result).toBe(parent); + } finally { + fs.chmodSync(parent, 0o755); // restore for cleanup + } + }); + }); + + describe('reclaimStaleDirectory', () => { + it('returns false when running as root', () => { + const originalGetuid = process.getuid; + process.getuid = () => 0; + try { + expect(reclaimStaleDirectory('/tmp/gh-aw/sandbox')).toBe(false); + } finally { + process.getuid = originalGetuid; + } + }); + + it('returns false when target path is fully writable', () => { + const target = path.join(tempDir, 'writable', 'path'); + fs.mkdirSync(path.join(tempDir, 'writable')); + expect(reclaimStaleDirectory(target)).toBe(false); + expect(mockExecaSync).not.toHaveBeenCalled(); + }); + + it('returns false for protected paths', () => { + // Even if /tmp were non-writable, we should not try to remove it + const result = reclaimStaleDirectory('/tmp'); + expect(result).toBe(false); + }); + + it('attempts sudo rm when directory is not writable', () => { + const staleDir = path.join(tempDir, 'stale'); + fs.mkdirSync(staleDir); + fs.chmodSync(staleDir, 0o555); + + mockExecaSync.mockReturnValue({ exitCode: 0 } as any); + + try { + const target = path.join(staleDir, 'child'); + const result = reclaimStaleDirectory(target); + expect(result).toBe(true); + expect(mockExecaSync).toHaveBeenCalledWith( + 'sudo', + ['rm', '-rf', staleDir], + expect.objectContaining({ reject: false, timeout: 10_000 }), + ); + } finally { + // Restore permissions if sudo mock didn't actually remove it + if (fs.existsSync(staleDir)) { + fs.chmodSync(staleDir, 0o755); + } + } + }); + + it('falls back to fs.rmSync when sudo fails', () => { + const staleDir = path.join(tempDir, 'stale2'); + fs.mkdirSync(staleDir); + fs.chmodSync(staleDir, 0o555); + + mockExecaSync.mockReturnValue({ exitCode: 1, stderr: 'sudo: not found' } as any); + + try { + const target = path.join(staleDir, 'child'); + // fs.rmSync will likely also fail on the non-writable dir, but we test the flow + const result = reclaimStaleDirectory(target); + // Result depends on whether fs.rmSync succeeds (it won't on a dir we just chmod'd 555) + expect(mockExecaSync).toHaveBeenCalled(); + // The important thing is it doesn't throw + expect(typeof result).toBe('boolean'); + } finally { + if (fs.existsSync(staleDir)) { + fs.chmodSync(staleDir, 0o755); + } + } + }); + }); +}); diff --git a/src/preflight-reclaim.ts b/src/preflight-reclaim.ts new file mode 100644 index 000000000..877976208 --- /dev/null +++ b/src/preflight-reclaim.ts @@ -0,0 +1,159 @@ +import * as fs from 'fs'; +import * as path from 'path'; +import execa from 'execa'; +import { logger } from './logger'; + +/** + * Attempts to reclaim a directory tree that is not writable by the current user. + * + * On persistent GitHub Actions runners, a previous AWF run (or Docker daemon) + * can leave directories under workDir owned by root. When the next AWF + * invocation runs as non-root (rootless/network-isolation mode), mkdirSync + * fails with EACCES before any container is started. + * + * This function detects non-writable ancestors in the workDir path and attempts + * to reclaim them via: + * 1. `sudo rm -rf` (available on GitHub-hosted runners without a password) + * 2. Plain `rm -rf` fallback (may work if only leaf permissions are wrong) + * + * The function is intentionally conservative: + * - Only acts when the directory actually exists AND is not writable + * - Only removes the narrowest non-writable ancestor (not arbitrary paths) + * - Refuses to operate on system paths (/, /tmp, /var, etc.) + * - Logs all actions for auditability + * + * @param targetDir - The directory AWF wants to create/use (e.g., workDir) + * @returns true if reclaim was attempted, false if no action was needed + */ +export function reclaimStaleDirectory(targetDir: string): boolean { + const currentUid = process.getuid?.(); + // If running as root, EACCES won't happen — skip preflight + if (currentUid === 0) { + return false; + } + + // Walk from the target up to find the first existing ancestor + // that is not writable by the current user + const nonWritableDir = findNonWritableAncestor(targetDir); + if (!nonWritableDir) { + return false; + } + + // Safety: refuse to remove system directories + if (isProtectedPath(nonWritableDir)) { + logger.warn( + `Pre-flight: stale directory ${nonWritableDir} is not writable, but refusing to remove protected path` + ); + return false; + } + + logger.info( + `Pre-flight: reclaiming stale directory ${nonWritableDir} (not writable by UID ${currentUid})` + ); + + // Attempt 1: sudo rm -rf (works on GitHub-hosted runners without password) + try { + const result = execa.sync('sudo', ['rm', '-rf', nonWritableDir], { + reject: false, + timeout: 10_000, + }); + if (result.exitCode === 0) { + logger.debug(`Pre-flight: successfully removed ${nonWritableDir} via sudo`); + return true; + } + logger.debug( + `Pre-flight: sudo rm failed (exit ${result.exitCode}): ${result.stderr || '(no stderr)'}` + ); + } catch (error) { + logger.debug(`Pre-flight: sudo rm threw:`, error); + } + + // Attempt 2: plain rm -rf (may work if only leaf permissions are wrong) + try { + fs.rmSync(nonWritableDir, { recursive: true, force: true }); + logger.debug(`Pre-flight: successfully removed ${nonWritableDir} via fs.rmSync`); + return true; + } catch (error) { + logger.debug(`Pre-flight: fs.rmSync failed:`, error); + } + + logger.warn( + `Pre-flight: could not reclaim ${nonWritableDir} — writeConfigs may fail with EACCES` + ); + return false; +} + +/** + * Walks from targetDir upward to find the shallowest existing directory + * that is not writable by the current process. + * + * Returns undefined if all existing ancestors are writable (normal case). + */ +function findNonWritableAncestor(targetDir: string): string | undefined { + const resolvedTarget = path.resolve(targetDir); + let current = resolvedTarget; + + // Collect path segments from target down to the first existing directory + const nonExistentSegments: string[] = []; + while (!fs.existsSync(current)) { + nonExistentSegments.unshift(path.basename(current)); + const parent = path.dirname(current); + if (parent === current) break; // reached root + current = parent; + } + + // `current` is now the first existing ancestor of targetDir + if (!fs.existsSync(current)) { + return undefined; + } + + // Check if this existing ancestor is writable + try { + fs.accessSync(current, fs.constants.W_OK); + // The existing ancestor is writable — mkdirSync should succeed + return undefined; + } catch { + // Not writable — this is the directory we need to reclaim + return current; + } +} + +/** + * Returns true for paths that must never be removed, even if they're + * not writable. Prevents catastrophic damage from bugs or unexpected state. + */ +function isProtectedPath(dirPath: string): boolean { + const resolved = path.resolve(dirPath); + const protectedPaths = new Set([ + '/', + '/tmp', + '/var', + '/var/tmp', + '/home', + '/root', + '/usr', + '/etc', + '/opt', + '/bin', + '/sbin', + '/lib', + '/lib64', + '/dev', + '/sys', + '/proc', + '/run', + // GitHub Actions runner paths (protect the runner itself) + '/home/runner', + '/home/runner/work', + ]); + + return protectedPaths.has(resolved); +} + +/** @internal Exposed only for unit tests — not part of the public API. */ +// ts-prune-ignore-next +export const preflightReclaimTestHelpers = { + findNonWritableAncestor, + isProtectedPath, + reclaimStaleDirectory, +}; diff --git a/src/workdir-setup.ts b/src/workdir-setup.ts index b7066a8a7..8e8c62a12 100644 --- a/src/workdir-setup.ts +++ b/src/workdir-setup.ts @@ -5,6 +5,7 @@ import { logger } from './logger'; import { getSafeHostUid, getSafeHostGid, getRealUserHome } from './host-env'; import { LogPaths } from './log-paths'; import { resolveRunnerToolCachePath } from './runner-tool-cache'; +import { reclaimStaleDirectory } from './preflight-reclaim'; interface EnsureDirectoryOptions { mode?: number; @@ -113,8 +114,18 @@ function prepareChrootHomeMountpoint(emptyHomeDir: string, relativeMountPath: st /** * Creates all log and session-state directories required before container * startup, setting ownership and permissions for the respective service users. + * + * Pre-flight: attempts to reclaim any root-owned directories from previous runs + * before calling ensureDirectory, preventing EACCES on persistent runners. */ function prepareLogDirectories(logPaths: LogPaths): void { + // Pre-flight: reclaim any stale root-owned log directories from previous runs. + // On persistent runners (or shared /tmp), Docker containers may leave these + // directories owned by root, causing ensureDirectory to fail with EACCES. + for (const dir of [logPaths.agentLogs, logPaths.sessionState, logPaths.squidLogs, logPaths.apiProxyLogs, logPaths.cliProxyLogs]) { + reclaimStaleDirectory(dir); + } + // Create agent logs directory for persistence // Chown to host user so Copilot CLI can write logs (AWF runs as root, agent runs as host user) ensureDirectory(logPaths.agentLogs, { @@ -203,6 +214,7 @@ function prepareLogDirectories(logPaths: LogPaths): void { // Uses mode 0o777 to allow GitHub Actions workflows and MCP gateway to create subdirectories // even when AWF runs as root (e.g., sudo awf) const mcpLogsDir = '/tmp/gh-aw/mcp-logs'; + reclaimStaleDirectory(mcpLogsDir); if (ensureDirectory(mcpLogsDir, { mode: 0o777 })) { // Explicitly set permissions to 0o777 (not affected by umask) fs.chmodSync(mcpLogsDir, 0o777);