diff --git a/src/sandbox/linux-sandbox-utils.ts b/src/sandbox/linux-sandbox-utils.ts index 7dd31b3..f71aa1b 100644 --- a/src/sandbox/linux-sandbox-utils.ts +++ b/src/sandbox/linux-sandbox-utils.ts @@ -101,6 +101,36 @@ function findSymlinkInPath( return null } +/** + * Check if any existing component in the path is a file (not a directory). + * If so, the target path can never be created because you can't mkdir under a file. + * + * This handles the git worktree case: .git is a file, so .git/hooks can never + * exist and there's nothing to deny. + */ +function hasFileAncestor(targetPath: string): boolean { + const parts = targetPath.split(path.sep) + let currentPath = '' + + for (const part of parts) { + if (!part) continue // Skip empty parts (leading /) + const nextPath = currentPath + path.sep + part + try { + const stat = fs.statSync(nextPath) + if (stat.isFile() || stat.isSymbolicLink()) { + // This component exists as a file — nothing below it can be created + return true + } + } catch { + // Path doesn't exist — stop checking + break + } + currentPath = nextPath + } + + return false +} + /** * Find the first non-existent path component. * E.g., for "/existing/parent/nonexistent/child/file.txt" where /existing/parent exists, @@ -148,13 +178,29 @@ async function linuxGetMandatoryDenyPaths( ...DANGEROUS_FILES.map(f => path.resolve(cwd, f)), // Dangerous directories in CWD ...dangerousDirectories.map(d => path.resolve(cwd, d)), - // Git hooks always blocked for security - path.resolve(cwd, '.git/hooks'), ] - // Git config conditionally blocked based on allowGitConfig setting - if (!allowGitConfig) { - denyPaths.push(path.resolve(cwd, '.git/config')) + // Git hooks and config are only denied when .git exists as a directory. + // In git worktrees, .git is a file (e.g., "gitdir: /path/..."), so + // .git/hooks can never exist — denying it would cause bwrap to fail. + // When .git doesn't exist at all, mounting at .git would block its + // creation and break git init. + const dotGitPath = path.resolve(cwd, '.git') + let dotGitIsDirectory = false + try { + dotGitIsDirectory = fs.statSync(dotGitPath).isDirectory() + } catch { + // .git doesn't exist + } + + if (dotGitIsDirectory) { + // Git hooks always blocked for security + denyPaths.push(path.resolve(cwd, '.git/hooks')) + + // Git config conditionally blocked based on allowGitConfig setting + if (!allowGitConfig) { + denyPaths.push(path.resolve(cwd, '.git/config')) + } } // Build iglob args for all patterns in one ripgrep call @@ -284,15 +330,25 @@ function registerExitCleanupHandler(): void { export function cleanupBwrapMountPoints(): void { for (const mountPoint of bwrapMountPoints) { try { - // Only remove if it's still the empty file bwrap created. + // Only remove if it's still the empty file/directory bwrap created. // If something else has written real content, leave it alone. if (fs.existsSync(mountPoint)) { const stat = fs.statSync(mountPoint) if (stat.isFile() && stat.size === 0) { fs.unlinkSync(mountPoint) logForDebugging( - `[Sandbox Linux] Cleaned up bwrap mount point: ${mountPoint}`, + `[Sandbox Linux] Cleaned up bwrap mount point (file): ${mountPoint}`, ) + } else if (stat.isDirectory()) { + // Empty directory mount points are created for intermediate + // components (Fix 2). Only remove if still empty. + const entries = fs.readdirSync(mountPoint) + if (entries.length === 0) { + fs.rmdirSync(mountPoint) + logForDebugging( + `[Sandbox Linux] Cleaned up bwrap mount point (dir): ${mountPoint}`, + ) + } } } } catch { @@ -662,6 +718,17 @@ async function generateFilesystemArgs( // We track them in bwrapMountPoints so cleanupBwrapMountPoints() can // remove them after the command exits. if (!fs.existsSync(normalizedPath)) { + // Fix 1 (worktree): If any existing component in the deny path is a + // file (not a directory), skip the deny entirely. You can't mkdir + // under a file, so the deny path can never be created. This handles + // git worktrees where .git is a file. + if (hasFileAncestor(normalizedPath)) { + logForDebugging( + `[Sandbox Linux] Skipping deny path with file ancestor (cannot create paths under a file): ${normalizedPath}`, + ) + continue + } + // Find the deepest existing ancestor directory let ancestorPath = path.dirname(normalizedPath) while (ancestorPath !== '/' && !fs.existsSync(ancestorPath)) { @@ -679,12 +746,29 @@ async function generateFilesystemArgs( if (ancestorIsWithinAllowedPath) { const firstNonExistent = findFirstNonExistentComponent(normalizedPath) - args.push('--ro-bind', '/dev/null', firstNonExistent) - bwrapMountPoints.add(firstNonExistent) - registerExitCleanupHandler() - logForDebugging( - `[Sandbox Linux] Mounted /dev/null at ${firstNonExistent} to block creation of ${normalizedPath}`, - ) + + // Fix 2: If firstNonExistent is an intermediate component (not the + // leaf deny path itself), mount a read-only empty directory instead + // of /dev/null. This prevents the component from appearing as a file + // which breaks tools that expect to traverse it as a directory. + if (firstNonExistent !== normalizedPath) { + const emptyDir = fs.mkdtempSync( + path.join(tmpdir(), 'claude-empty-'), + ) + args.push('--ro-bind', emptyDir, firstNonExistent) + bwrapMountPoints.add(firstNonExistent) + registerExitCleanupHandler() + logForDebugging( + `[Sandbox Linux] Mounted empty dir at ${firstNonExistent} to block creation of ${normalizedPath}`, + ) + } else { + args.push('--ro-bind', '/dev/null', firstNonExistent) + bwrapMountPoints.add(firstNonExistent) + registerExitCleanupHandler() + logForDebugging( + `[Sandbox Linux] Mounted /dev/null at ${firstNonExistent} to block creation of ${normalizedPath}`, + ) + } } else { logForDebugging( `[Sandbox Linux] Skipping non-existent deny path not within allowed paths: ${normalizedPath}`, diff --git a/test/sandbox/mandatory-deny-paths.test.ts b/test/sandbox/mandatory-deny-paths.test.ts index c2b778e..d07b793 100644 --- a/test/sandbox/mandatory-deny-paths.test.ts +++ b/test/sandbox/mandatory-deny-paths.test.ts @@ -7,6 +7,7 @@ import { readFileSync, symlinkSync, existsSync, + statSync, } from 'node:fs' import { tmpdir } from 'node:os' import { join } from 'node:path' @@ -568,9 +569,10 @@ describe('Mandatory Deny Paths - Integration Tests', () => { ) expect(result.success).toBe(false) - // bwrap mounts /dev/null at first non-existent component, blocking mkdir - const content = readFileSync('nonexistent-dir', 'utf8') - expect(content).toBe('') + // bwrap mounts an empty read-only directory at first non-existent + // intermediate component, blocking mkdir inside it + const stat = statSync('nonexistent-dir') + expect(stat.isDirectory()).toBe(true) cleanupBwrapMountPoints() }) @@ -586,9 +588,10 @@ describe('Mandatory Deny Paths - Integration Tests', () => { ) expect(result.success).toBe(false) - // bwrap mounts /dev/null at 'a', blocking the entire subtree - const content = readFileSync('a', 'utf8') - expect(content).toBe('') + // bwrap mounts an empty read-only directory at 'a', blocking the + // entire subtree + const stat = statSync('a') + expect(stat.isDirectory()).toBe(true) cleanupBwrapMountPoints() }) @@ -637,7 +640,7 @@ describe('Mandatory Deny Paths - Integration Tests', () => { expect(existsSync('ghost-dir-b')).toBe(false) }) - it('cleanupBwrapMountPoints preserves files with real content', async () => { + it('cleanupBwrapMountPoints preserves non-empty directories', async () => { if (getPlatform() !== 'linux') return const nonExistentPath = 'preserve-test-dir/file.txt' @@ -646,23 +649,23 @@ describe('Mandatory Deny Paths - Integration Tests', () => { join(TEST_DIR, nonExistentPath), ]) - // Simulate something else writing real content to the mount point - // (e.g., another process created this path legitimately) + // Simulate something else creating content in the mount point directory + // (e.g., another process created files here legitimately) const mountPoint = join(TEST_DIR, 'preserve-test-dir') if (existsSync(mountPoint)) { - // Write real content to it — cleanup should NOT delete non-empty files - const { writeFileSync: writeSync } = await import('node:fs') - writeSync(mountPoint, 'real content') + // Create a file inside — cleanup should NOT delete non-empty directories + writeFileSync(join(mountPoint, 'real-file.txt'), 'real content') } cleanupBwrapMountPoints() - // File with real content should be preserved + // Directory with real content should be preserved if (existsSync(mountPoint)) { - const content = readFileSync(mountPoint, 'utf8') + expect(statSync(mountPoint).isDirectory()).toBe(true) + const content = readFileSync(join(mountPoint, 'real-file.txt'), 'utf8') expect(content).toBe('real content') // Manual cleanup for this test - rmSync(mountPoint, { force: true }) + rmSync(mountPoint, { recursive: true, force: true }) } }) @@ -672,6 +675,114 @@ describe('Mandatory Deny Paths - Integration Tests', () => { cleanupBwrapMountPoints() }) + it('non-existent .git/hooks deny does not turn .git into a file, breaking git', async () => { + if (getPlatform() !== 'linux') return + + // When .git doesn't exist yet, denying .git/hooks causes + // findFirstNonExistentComponent to return .git itself. bwrap then does + // --ro-bind /dev/null .git, creating .git as a FILE (not a directory). + // Inside the sandbox, every git command fails because .git is a file. + + // Use a clean directory with NO .git + const noGitDir = join(TEST_DIR, 'no-git-dir') + mkdirSync(noGitDir, { recursive: true }) + + const originalDir = process.cwd() + process.chdir(noGitDir) + + try { + const writeConfig = { + allowOnly: ['.'], + denyWithinAllow: [] as string[], + } + + // This calls linuxGetMandatoryDenyPaths which unconditionally adds + // .git/hooks to the deny list. When .git doesn't exist, + // findFirstNonExistentComponent returns .git and bwrap mounts + // /dev/null there — making .git a file. + const wrappedCommand = await wrapCommandWithSandboxLinux({ + command: 'git init && git status', + needsNetworkRestriction: false, + readConfig: undefined, + writeConfig, + enableWeakerNestedSandbox: true, + }) + + const result = spawnSync(wrappedCommand, { + shell: true, + encoding: 'utf8', + timeout: 10000, + }) + + // git init + git status should succeed — .git must be creatable as + // a directory, not blocked by a /dev/null file mount. + expect(result.status).toBe(0) + + cleanupBwrapMountPoints() + } finally { + process.chdir(originalDir) + rmSync(noGitDir, { recursive: true, force: true }) + } + }) + + it('git worktree with .git as a file does not break sandboxed commands', async () => { + if (getPlatform() !== 'linux') return + + // Reproduces the bug reported by nvidia/netflix with git worktrees: + // In a worktree, .git is a FILE (e.g., "gitdir: /path/to/.git/worktrees/foo"), + // not a directory. The mandatory deny list includes .git/hooks, but since + // .git is a file, .git/hooks doesn't exist. The non-existent path handling + // tries to mount /dev/null at .git/hooks, but bwrap can't create a mount + // point under .git because it's a file — causing every command to fail. + + const worktreeDir = join(TEST_DIR, 'fake-worktree') + mkdirSync(worktreeDir, { recursive: true }) + + // Simulate a git worktree: .git is a file, not a directory + writeFileSync( + join(worktreeDir, '.git'), + 'gitdir: /tmp/fake-main-repo/.git/worktrees/my-branch', + ) + + const originalDir = process.cwd() + process.chdir(worktreeDir) + + try { + const writeConfig = { + allowOnly: ['.'], + denyWithinAllow: [] as string[], + } + + // linuxGetMandatoryDenyPaths adds .git/hooks to deny list. + // .git exists as a file, so .git/hooks doesn't exist. + // The code will try to mount /dev/null at .git/hooks, but bwrap + // can't create a mount point there because .git is a file. + const wrappedCommand = await wrapCommandWithSandboxLinux({ + command: 'echo hello', + needsNetworkRestriction: false, + readConfig: undefined, + writeConfig, + enableWeakerNestedSandbox: true, + }) + + const result = spawnSync(wrappedCommand, { + shell: true, + encoding: 'utf8', + timeout: 10000, + }) + + // A simple echo should succeed — the .git-as-file worktree layout + // should not cause the sandbox to fail. + expect(result.status).toBe(0) + expect(result.stdout.trim()).toBe('hello') + + cleanupBwrapMountPoints() + } finally { + process.chdir(originalDir) + rmSync(worktreeDir, { recursive: true, force: true }) + } + }) + it('does not leave ghost dotfiles after command + cleanup cycle', async () => { if (getPlatform() !== 'linux') return