diff --git a/README.md b/README.md index 14d89686..d7cd0711 100644 --- a/README.md +++ b/README.md @@ -678,9 +678,16 @@ $ srt 'echo "bad" > .git/hooks/pre-commit' /bin/bash: .git/hooks/pre-commit: Operation not permitted ``` -**Note (Linux):** On Linux, mandatory deny paths only block files that already exist. Non-existent files in these patterns cannot be blocked by bubblewrap's bind-mount approach. macOS uses glob patterns which block both existing and new files. +On macOS the directories that hold these paths (`.git`, `.vscode`, `.idea`, `.claude`) are also pinned at any depth, so a sandboxed command cannot create, rename, or delete them — for example `mv decoy packages/app/.git` or `git init` in a subdirectory fails, while writes inside an existing nested `.git` still work. -**Linux search depth:** On Linux, the sandbox uses `ripgrep` to scan for dangerous files in subdirectories within allowed write paths. By default, it searches up to 3 levels deep for performance. You can configure this with `mandatoryDenySearchDepth`: +| Platform | Covers | Paths created after the scan | +| --- | --- | --- | +| macOS | Pattern rules, any depth | Blocked | +| Linux | Direct writes to existing paths, up to `mandatoryDenySearchDepth` | Not blocked | + +A nested repository whose `.git` sits `L` directories below the project is within the scan when `mandatoryDenySearchDepth >= L + 2`. `node_modules` is never scanned. + +**Search depth (Linux):** the project directory is scanned for these paths with `ripgrep`, 3 levels deep by default. Configure with `mandatoryDenySearchDepth`: ```json { diff --git a/src/sandbox/macos-sandbox-utils.ts b/src/sandbox/macos-sandbox-utils.ts index 876661f9..bd6b7390 100644 --- a/src/sandbox/macos-sandbox-utils.ts +++ b/src/sandbox/macos-sandbox-utils.ts @@ -412,6 +412,28 @@ function renderRule( * Get all ancestor directories for a path, up to (but not including) root * Example: /private/tmp/test/file.txt -> ["/private/tmp/test", "/private/tmp", "/private"] */ +/** + * Glob-containing ancestors of a glob, nearest first, stopping before a + * bare `**`: `**\/.git/hooks` → [`**\/.git`]; `**\/.env` → []. + */ +function getGlobAncestorPatterns(normalizedGlob: string): string[] { + const ancestors: string[] = [] + let current = path.posix.dirname(normalizedGlob) + while ( + current !== '/' && + current !== '.' && + current !== '**' && + !current.endsWith('/**') && + containsGlobChars(current) + ) { + ancestors.push(current) + const parent = path.posix.dirname(current) + if (parent === current) break + current = parent + } + return ancestors +} + function getAncestorDirectories(pathStr: string): string[] { const ancestors: string[] = [] let currentPath = path.dirname(pathStr) @@ -462,6 +484,12 @@ function generateMoveBlockingRules( if (containsGlobChars(normalizedPath)) { // For glob patterns, block moves of the directory containing the // pattern's static prefix, then of its ancestors + // Pin the glob's ancestor directories too (exact match, no subtree): + // a rename of a prepared directory onto `/.git` is checked + // against the target only, not the children it carries. + for (const ancestor of getGlobAncestorPatterns(normalizedPath)) { + filters.add(`(regex ${escapePath(globToRegex(ancestor))})`) + } baseDir = globBaseDir(normalizedPath) if (baseDir === '/') continue filters.add(`(literal ${escapePath(baseDir)})`) diff --git a/test/sandbox/macos-seatbelt.test.ts b/test/sandbox/macos-seatbelt.test.ts index 1f2d8c4b..a26a3da0 100644 --- a/test/sandbox/macos-seatbelt.test.ts +++ b/test/sandbox/macos-seatbelt.test.ts @@ -1023,3 +1023,95 @@ describe.if(isMacOS)('macOS Seatbelt allowMachLookup', () => { expect(result.status).toBe(0) }) }) + +/** + * Nested `.git` directory swap: the mandatory deny globs have no static + * prefix, so only the literal `/.git` was move-protected and a prepared + * directory could be renamed onto `/.git`. The fix pins every + * glob-shaped ancestor (`**\/.git`) against create/unlink at any depth. + */ +describe.if(isMacOS)('macOS Seatbelt Nested .git Swap Prevention', () => { + const TEST_BASE_DIR = join( + realpathSync(tmpdir()), + 'seatbelt-git-swap-test-' + Date.now(), + ) + const TEST_ALLOWED_DIR = join(TEST_BASE_DIR, 'allowed') + const NESTED_DIR = join(TEST_ALLOWED_DIR, 'packages', 'app') + const DECOY_DIR = join(NESTED_DIR, '_gd') + const NESTED_GIT = join(NESTED_DIR, '.git') + const EXISTING_DIR = join(TEST_ALLOWED_DIR, 'packages', 'lib') + const EXISTING_GIT = join(EXISTING_DIR, '.git') + + // The mandatory `.git` globs are anchored to process.cwd(); mirror them here. + const writeConfig: FsWriteRestrictionConfig = { + allowOnly: [TEST_ALLOWED_DIR], + denyWithinAllow: [ + join(TEST_ALLOWED_DIR, '**/.git/config'), + join(TEST_ALLOWED_DIR, '**/.git/hooks/**'), + ], + } + + function run(command: string) { + const wrapped = wrapCommandWithSandboxMacOS({ + command, + needsNetworkRestriction: false, + readConfig: undefined, + writeConfig, + }) + const result = spawnSync(wrapped, { + shell: true, + encoding: 'utf8', + timeout: 5000, + }) + // A nested Seatbelt fails with "sandbox_apply: Operation not permitted", + // which would satisfy the deny assertions without running the command. + expect(result.stderr || '').not.toContain('sandbox_apply') + return result + } + + beforeAll(() => { + mkdirSync(DECOY_DIR, { recursive: true }) + writeFileSync( + join(DECOY_DIR, 'config'), + '[core]\n\tfsmonitor = /tmp/payload\n', + ) + mkdirSync(EXISTING_GIT, { recursive: true }) + writeFileSync(join(EXISTING_GIT, 'index'), 'old') + }) + + afterAll(() => { + if (existsSync(TEST_BASE_DIR)) { + rmSync(TEST_BASE_DIR, { recursive: true, force: true }) + } + }) + + it('blocks renaming a decoy directory onto a nested .git', () => { + const result = run(`mv ${DECOY_DIR} ${NESTED_GIT}`) + expect(result.status).not.toBe(0) + expect((result.stderr || '').toLowerCase()).toContain( + 'operation not permitted', + ) + expect(existsSync(NESTED_GIT)).toBe(false) + expect(existsSync(DECOY_DIR)).toBe(true) + }) + + it('blocks creating a nested .git directory or symlink directly', () => { + expect(run(`mkdir ${NESTED_GIT}`).status).not.toBe(0) + expect(run(`ln -s ${DECOY_DIR} ${NESTED_GIT}`).status).not.toBe(0) + expect(existsSync(NESTED_GIT)).toBe(false) + }) + + it('blocks renaming an existing nested .git away', () => { + const result = run(`mv ${EXISTING_GIT} ${EXISTING_DIR}/gone`) + expect(result.status).not.toBe(0) + expect(existsSync(EXISTING_GIT)).toBe(true) + }) + + it('still allows ordinary writes inside a nested .git', () => { + const result = run( + `echo new > ${EXISTING_GIT}/index.lock && mv ${EXISTING_GIT}/index.lock ${EXISTING_GIT}/index && mkdir -p ${EXISTING_GIT}/refs/heads`, + ) + expect(result.status).toBe(0) + expect(readFileSync(join(EXISTING_GIT, 'index'), 'utf8').trim()).toBe('new') + }) +})