From 0dc4322cda84b09d4a1398c09e7c19c48e90c822 Mon Sep 17 00:00:00 2001 From: David Dworken Date: Sun, 8 Feb 2026 19:21:15 -0800 Subject: [PATCH] Re-introduce non-existent deny path protection with mount point cleanup PR #80 hardened the sandbox by mounting /dev/null over non-existent deny paths to prevent their creation, but this caused bwrap to leave empty "ghost dotfiles" on the host (issue #85), which PR #91 reverted. This re-introduces the protection with proper cleanup: mount points are tracked and removed via cleanupBwrapMountPoints(). A new lightweight cleanupAfterCommand() API is exposed on SandboxManager for callers to invoke after each command, and the srt CLI calls it on child exit. Co-Authored-By: Claude Opus 4.6 --- package-lock.json | 4 +- package.json | 2 +- src/cli.ts | 5 + src/sandbox/linux-sandbox-utils.ts | 111 +++++++++- src/sandbox/sandbox-manager.ts | 20 ++ test/sandbox/mandatory-deny-paths.test.ts | 241 +++++++++++++++++++++- 6 files changed, 372 insertions(+), 11 deletions(-) diff --git a/package-lock.json b/package-lock.json index b3ac4f3..0cad39f 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@anthropic-ai/sandbox-runtime", - "version": "0.0.35", + "version": "0.0.36", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@anthropic-ai/sandbox-runtime", - "version": "0.0.35", + "version": "0.0.36", "license": "Apache-2.0", "dependencies": { "@pondwader/socks5-server": "^1.0.10", diff --git a/package.json b/package.json index ee5965d..63a3b01 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@anthropic-ai/sandbox-runtime", - "version": "0.0.35", + "version": "0.0.36", "description": "Anthropic Sandbox Runtime (ASRT) - A general-purpose tool for wrapping security boundaries around arbitrary processes", "type": "module", "main": "./dist/index.js", diff --git a/src/cli.ts b/src/cli.ts index 0fd510a..ec8ac03 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -175,6 +175,11 @@ async function main(): Promise { // Handle process exit child.on('exit', (code, signal) => { + // Clean up bwrap mount point artifacts before exiting. + // On Linux, bwrap creates empty files on the host when protecting + // non-existent deny paths. This removes them. + SandboxManager.cleanupAfterCommand() + if (signal) { if (signal === 'SIGINT' || signal === 'SIGTERM') { process.exit(0) diff --git a/src/sandbox/linux-sandbox-utils.ts b/src/sandbox/linux-sandbox-utils.ts index 5f6955b..7dd31b3 100644 --- a/src/sandbox/linux-sandbox-utils.ts +++ b/src/sandbox/linux-sandbox-utils.ts @@ -101,6 +101,30 @@ function findSymlinkInPath( return null } +/** + * Find the first non-existent path component. + * E.g., for "/existing/parent/nonexistent/child/file.txt" where /existing/parent exists, + * returns "/existing/parent/nonexistent" + * + * This is used to block creation of non-existent deny paths by mounting /dev/null + * at the first missing component, preventing mkdir from creating the parent directories. + */ +function findFirstNonExistentComponent(targetPath: string): string { + 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 + if (!fs.existsSync(nextPath)) { + return nextPath + } + currentPath = nextPath + } + + return targetPath // Shouldn't reach here if called correctly +} + /** * Get mandatory deny paths using ripgrep (Linux only). * Uses a SINGLE ripgrep call with multiple glob patterns for efficiency. @@ -212,12 +236,19 @@ async function linuxGetMandatoryDenyPaths( // Track generated seccomp filters for cleanup on process exit const generatedSeccompFilters: Set = new Set() + +// Track mount points created by bwrap for non-existent deny paths. +// When bwrap does --ro-bind /dev/null /nonexistent/path, it creates an empty +// file on the host as a mount point. These persist after bwrap exits and must +// be cleaned up explicitly. +const bwrapMountPoints: Set = new Set() + let exitHandlerRegistered = false /** - * Register cleanup handler for generated seccomp filters + * Register cleanup handler for generated seccomp filters and bwrap mount points */ -function registerSeccompCleanupHandler(): void { +function registerExitCleanupHandler(): void { if (exitHandlerRegistered) { return } @@ -230,11 +261,47 @@ function registerSeccompCleanupHandler(): void { // Ignore cleanup errors during exit } } + cleanupBwrapMountPoints() }) exitHandlerRegistered = true } +/** + * Clean up mount point files created by bwrap for non-existent deny paths. + * + * When protecting non-existent deny paths, bwrap creates empty files on the + * host filesystem as mount points for --ro-bind. These files persist after + * bwrap exits. This function removes them. + * + * This should be called after each sandboxed command completes to prevent + * ghost dotfiles (e.g. .bashrc, .gitconfig) from appearing in the working + * directory. It is also called automatically on process exit as a safety net. + * + * Safe to call at any time — it only removes files that were tracked during + * generateFilesystemArgs() and skips any that no longer exist. + */ +export function cleanupBwrapMountPoints(): void { + for (const mountPoint of bwrapMountPoints) { + try { + // Only remove if it's still the empty file 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}`, + ) + } + } + } catch { + // Ignore cleanup errors — the file may have already been removed + } + } + bwrapMountPoints.clear() +} + /** * Detailed status of Linux sandbox dependencies */ @@ -587,12 +654,42 @@ async function generateFilesystemArgs( continue } - // Skip non-existent paths - no protection needed - // Mounting /dev/null over non-existent paths creates empty files on host + // Handle non-existent paths by mounting /dev/null to block creation. + // Without this, a sandboxed process could mkdir+write a denied path that + // doesn't exist yet, bypassing the deny rule entirely. + // + // bwrap creates empty files on the host as mount points for these binds. + // We track them in bwrapMountPoints so cleanupBwrapMountPoints() can + // remove them after the command exits. if (!fs.existsSync(normalizedPath)) { - logForDebugging( - `[Sandbox Linux] Skipping non-existent deny path: ${normalizedPath}`, + // Find the deepest existing ancestor directory + let ancestorPath = path.dirname(normalizedPath) + while (ancestorPath !== '/' && !fs.existsSync(ancestorPath)) { + ancestorPath = path.dirname(ancestorPath) + } + + // Only protect if the existing ancestor is within an allowed write path. + // If not, the path is already read-only from --ro-bind / /. + const ancestorIsWithinAllowedPath = allowedWritePaths.some( + allowedPath => + ancestorPath.startsWith(allowedPath + '/') || + ancestorPath === allowedPath || + normalizedPath.startsWith(allowedPath + '/'), ) + + 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}`, + ) + } else { + logForDebugging( + `[Sandbox Linux] Skipping non-existent deny path not within allowed paths: ${normalizedPath}`, + ) + } continue } @@ -761,7 +858,7 @@ export async function wrapCommandWithSandboxLinux( // Only track runtime-generated filters (not pre-generated ones from vendor/) if (!seccompFilterPath.includes('/vendor/seccomp/')) { generatedSeccompFilters.add(seccompFilterPath) - registerSeccompCleanupHandler() + registerExitCleanupHandler() } logForDebugging( diff --git a/src/sandbox/sandbox-manager.ts b/src/sandbox/sandbox-manager.ts index c39579e..1fe5d97 100644 --- a/src/sandbox/sandbox-manager.ts +++ b/src/sandbox/sandbox-manager.ts @@ -19,6 +19,7 @@ import { type LinuxNetworkBridgeContext, checkLinuxDependencies, type SandboxDependencyCheck, + cleanupBwrapMountPoints, } from './linux-sandbox-utils.js' import { wrapCommandWithSandboxMacOS, @@ -638,7 +639,24 @@ function updateConfig(newConfig: SandboxRuntimeConfig): void { logForDebugging('Sandbox configuration updated') } +/** + * Lightweight cleanup to call after each sandboxed command completes. + * + * On Linux, bwrap creates empty files on the host filesystem as mount points + * when protecting non-existent deny paths (e.g. ~/.bashrc, ~/.gitconfig). + * These persist after bwrap exits. This function removes them. + * + * Safe to call on any platform — it's a no-op on macOS. + * Also called automatically by reset() and on process exit as safety nets. + */ +function cleanupAfterCommand(): void { + cleanupBwrapMountPoints() +} + async function reset(): Promise { + // Clean up any leftover bwrap mount points + cleanupAfterCommand() + // Stop log monitor if (logMonitorShutdown) { logMonitorShutdown() @@ -905,6 +923,7 @@ export interface ISandboxManager { getLinuxGlobPatternWarnings(): string[] getConfig(): SandboxRuntimeConfig | undefined updateConfig(newConfig: SandboxRuntimeConfig): void + cleanupAfterCommand(): void reset(): Promise } @@ -934,6 +953,7 @@ export const SandboxManager: ISandboxManager = { getLinuxSocksSocketPath, waitForNetworkInitialization, wrapWithSandbox, + cleanupAfterCommand, reset, getSandboxViolationStore, annotateStderrWithSandboxFailures, diff --git a/test/sandbox/mandatory-deny-paths.test.ts b/test/sandbox/mandatory-deny-paths.test.ts index 1e44daf..c2b778e 100644 --- a/test/sandbox/mandatory-deny-paths.test.ts +++ b/test/sandbox/mandatory-deny-paths.test.ts @@ -15,7 +15,10 @@ import { wrapCommandWithSandboxMacOS, macGetMandatoryDenyPatterns, } from '../../src/sandbox/macos-sandbox-utils.js' -import { wrapCommandWithSandboxLinux } from '../../src/sandbox/linux-sandbox-utils.js' +import { + wrapCommandWithSandboxLinux, + cleanupBwrapMountPoints, +} from '../../src/sandbox/linux-sandbox-utils.js' /** * Integration tests for mandatory deny paths. @@ -486,6 +489,242 @@ describe('Mandatory Deny Paths - Integration Tests', () => { }) }) + describe('Non-existent deny path protection and cleanup (Linux only)', () => { + // This tests that: + // 1. Non-existent deny paths within writable areas are blocked by mounting + // /dev/null at the first non-existent component + // 2. The mount point artifacts bwrap creates on the host are cleaned up + // by cleanupBwrapMountPoints() + // + // Background: When bwrap does --ro-bind /dev/null /nonexistent/path, it + // creates an empty file on the host as a mount point. Without cleanup, + // these "ghost dotfiles" persist and pollute the working directory. + + async function runSandboxedWriteWithDenyPaths( + command: string, + denyPaths: string[], + ): Promise<{ success: boolean; stdout: string; stderr: string }> { + const platform = getPlatform() + if (platform !== 'linux') { + return { success: true, stdout: '', stderr: '' } + } + + const writeConfig = { + allowOnly: ['.'], + denyWithinAllow: denyPaths, + } + + const wrappedCommand = await wrapCommandWithSandboxLinux({ + command, + needsNetworkRestriction: false, + readConfig: undefined, + writeConfig, + enableWeakerNestedSandbox: true, + }) + + const result = spawnSync(wrappedCommand, { + shell: true, + encoding: 'utf8', + timeout: 10000, + }) + + return { + success: result.status === 0, + stdout: result.stdout || '', + stderr: result.stderr || '', + } + } + + // --- Security: deny path blocking --- + + it('blocks creation of non-existent file when parent dir exists', async () => { + if (getPlatform() !== 'linux') return + + // .claude directory exists from beforeAll setup + // .claude/settings.json does NOT exist + const nonExistentFile = '.claude/settings.json' + + const result = await runSandboxedWriteWithDenyPaths( + `echo '{"hooks":{}}' > '${nonExistentFile}'`, + [join(TEST_DIR, nonExistentFile)], + ) + + expect(result.success).toBe(false) + // Verify file content was NOT written (bwrap creates empty mount point) + const content = readFileSync(nonExistentFile, 'utf8') + expect(content).toBe('') + + cleanupBwrapMountPoints() + }) + + it('blocks creation of non-existent file when parent dir also does not exist', async () => { + if (getPlatform() !== 'linux') return + + const nonExistentPath = 'nonexistent-dir/settings.json' + + const result = await runSandboxedWriteWithDenyPaths( + `mkdir -p nonexistent-dir && echo '{"hooks":{}}' > '${nonExistentPath}'`, + [join(TEST_DIR, nonExistentPath)], + ) + + 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('') + + cleanupBwrapMountPoints() + }) + + it('blocks creation of deeply nested non-existent path', async () => { + if (getPlatform() !== 'linux') return + + const nonExistentPath = 'a/b/c/file.txt' + + const result = await runSandboxedWriteWithDenyPaths( + `mkdir -p a/b/c && echo 'test' > '${nonExistentPath}'`, + [join(TEST_DIR, nonExistentPath)], + ) + + expect(result.success).toBe(false) + // bwrap mounts /dev/null at 'a', blocking the entire subtree + const content = readFileSync('a', 'utf8') + expect(content).toBe('') + + cleanupBwrapMountPoints() + }) + + // --- Cleanup: mount point artifact removal --- + + it('cleanupBwrapMountPoints removes mount point artifacts', async () => { + if (getPlatform() !== 'linux') return + + const nonExistentPath = 'cleanup-test-dir/file.txt' + + await runSandboxedWriteWithDenyPaths(`echo test > '${nonExistentPath}'`, [ + join(TEST_DIR, nonExistentPath), + ]) + + // Mount point artifact should exist on host after bwrap exits + expect(existsSync('cleanup-test-dir')).toBe(true) + + // Clean up + cleanupBwrapMountPoints() + + // Artifact should be gone + expect(existsSync('cleanup-test-dir')).toBe(false) + }) + + it('cleanupBwrapMountPoints removes multiple mount points from a single command', async () => { + if (getPlatform() !== 'linux') return + + // Two non-existent deny paths in different subtrees + const path1 = 'ghost-dir-a/secret.txt' + const path2 = 'ghost-dir-b/secret.txt' + + await runSandboxedWriteWithDenyPaths(`mkdir -p ghost-dir-a ghost-dir-b`, [ + join(TEST_DIR, path1), + join(TEST_DIR, path2), + ]) + + // Both mount point artifacts should exist + expect(existsSync('ghost-dir-a')).toBe(true) + expect(existsSync('ghost-dir-b')).toBe(true) + + cleanupBwrapMountPoints() + + // Both should be cleaned up + expect(existsSync('ghost-dir-a')).toBe(false) + expect(existsSync('ghost-dir-b')).toBe(false) + }) + + it('cleanupBwrapMountPoints preserves files with real content', async () => { + if (getPlatform() !== 'linux') return + + const nonExistentPath = 'preserve-test-dir/file.txt' + + await runSandboxedWriteWithDenyPaths(`echo test > '${nonExistentPath}'`, [ + join(TEST_DIR, nonExistentPath), + ]) + + // Simulate something else writing real content to the mount point + // (e.g., another process created this path 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') + } + + cleanupBwrapMountPoints() + + // File with real content should be preserved + if (existsSync(mountPoint)) { + const content = readFileSync(mountPoint, 'utf8') + expect(content).toBe('real content') + // Manual cleanup for this test + rmSync(mountPoint, { force: true }) + } + }) + + it('cleanupBwrapMountPoints is safe to call when there are no mount points', () => { + // Should not throw + cleanupBwrapMountPoints() + cleanupBwrapMountPoints() + }) + + it('does not leave ghost dotfiles after command + cleanup cycle', async () => { + if (getPlatform() !== 'linux') return + + // This is the exact scenario from issue #85: running a sandboxed command + // should NOT leave .bashrc, .gitconfig, etc. in the working directory. + // + // The mandatory deny list includes paths like ~/.bashrc, ~/.gitconfig. + // When CWD is within an allowed write path and these dotfiles don't exist + // in CWD, the old code left empty mount point files behind. + + // Use a clean subdirectory with no dotfiles + const cleanDir = join(TEST_DIR, 'clean-subdir') + mkdirSync(cleanDir, { recursive: true }) + + const originalDir = process.cwd() + process.chdir(cleanDir) + + try { + // Run a simple command through the sandbox + const writeConfig = { + allowOnly: ['.'], + denyWithinAllow: [] as string[], + } + + const wrappedCommand = await wrapCommandWithSandboxLinux({ + command: 'echo hello', + needsNetworkRestriction: false, + readConfig: undefined, + writeConfig, + enableWeakerNestedSandbox: true, + }) + + spawnSync(wrappedCommand, { + shell: true, + encoding: 'utf8', + timeout: 10000, + }) + + // Run cleanup (as the CLI / Claude Code would) + cleanupBwrapMountPoints() + + // Verify no ghost dotfiles were left behind + const { readdirSync } = await import('node:fs') + const files = readdirSync(cleanDir) + const ghostDotfiles = files.filter(f => f.startsWith('.')) + expect(ghostDotfiles).toEqual([]) + } finally { + process.chdir(originalDir) + rmSync(cleanDir, { recursive: true, force: true }) + } + }) + }) + describe('Symlink replacement attack protection (Linux only)', () => { // This tests the fix for symlink replacement attacks where an attacker // could delete a symlink and create a real directory with malicious content