diff --git a/README.md b/README.md index 14d89686..a645082d 100644 --- a/README.md +++ b/README.md @@ -666,7 +666,7 @@ Certain sensitive files and directories are **always blocked from writes**, even - IDE directories: `.vscode/`, `.idea/` - Claude config directories: `.claude/commands/`, `.claude/agents/` -- Git hooks and config: `.git/hooks/`, `.git/config` +- Git hooks and config: `hooks/`, `config`, `config.worktree` and `commondir` of a git directory — the working directory's repository, nested repositories, the submodule git directories they keep under `.git/modules/`, and a linked worktree's git directory. `commondir` and `config.worktree` are denied because git reads the hooks and config through them: `commondir` moves them to another directory entirely, and `config.worktree` is read instead of `config` wherever `extensions.worktreeConfig` is on (`git sparse-checkout init` turns it on). An existing `.git` _file_ (a linked worktree's or submodule checkout's `gitdir:` pointer) is read-only and cannot be removed or renamed over; creating a new one inside an allowed write path is still possible. The hooks and config a pointer leads to (the main repository's, for a worktree) are blocked as well, and so is filling in a git directory a pointer names but that does not exist yet; a pointer naming a directory that is not a git directory is not followed. On macOS only the working directory's own `.git` file is followed, since nested pointers are matched by pattern; the working directory's own submodule git directories are enumerated exactly, while a nested repository's are matched as `.git/modules//`, which covers a single-segment submodule name. These paths are blocked automatically - you don't need to add them to `denyWrite`. For example, even with `allowWrite: ["."]`, writing to `.bashrc` or `.git/hooks/pre-commit` will fail: @@ -678,9 +678,19 @@ $ 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. +**Git operations these denies break.** A git directory's `hooks/` and `config` are what a hook or a `core.fsmonitor` would be written to, so anything that writes or removes them fails inside the sandbox: -**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`: +- removing a tree that holds a submodule checkout or a linked worktree (`rm -rf lib`, `git clean -ffdx`), because its `.git` pointer file cannot be removed; +- `git worktree remove`, `git worktree move`, `git worktree repair`, `git submodule deinit`, for the same reason; +- `git submodule update --init` for a submodule that has not been cloned yet, which copies template hooks into `.git/modules//hooks/` and writes its config; +- from a linked worktree, anything writing the main repository's config: `git push -u`, `git checkout -b x origin/y`; +- `git init` and `git clone` into a subdirectory, which create `.git/hooks/`. + +**Known limit (both platforms).** A pointer file or a pattern-matched path is protected where it is: a command may still rename the directory _holding_ it aside and create a fresh one in its place (`mv lib lib.old && mkdir lib && echo 'gitdir: …' > lib/.git`). On Linux a path found by the scan has its ancestor directories pinned within the scan depth, so this is blocked there for what the scan reached; on macOS it is blocked for the literal denies (the working directory's own repository and its submodule git directories) and not for the pattern ones. + +**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 (a blocked _directory_, such as a repository's `.git/hooks/`, does cover files created in it later). macOS uses glob patterns which block both existing and new files. The Linux scan ignores `.gitignore` and similar ignore files, since the sandboxed command can write those. It fails closed: a directory it cannot read is denied whole, and a scan that does not finish in time aborts the command rather than sandboxing it with a partial deny list (a scan that cannot run at all — no `ripgrep` — is still logged and not fatal). + +**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, which reaches a nested repository directly beneath the working directory. You can configure this with `mandatoryDenySearchDepth`: ```json { diff --git a/src/sandbox/linux-sandbox-utils.ts b/src/sandbox/linux-sandbox-utils.ts index adab73a7..7a8b0876 100644 --- a/src/sandbox/linux-sandbox-utils.ts +++ b/src/sandbox/linux-sandbox-utils.ts @@ -7,7 +7,8 @@ import { spawn } from 'node:child_process' import type { ChildProcess } from 'node:child_process' import { tmpdir } from 'node:os' import path, { join } from 'node:path' -import { ripGrep } from '../utils/ripgrep.js' +import { ripGrep, RipgrepError } from '../utils/ripgrep.js' +import type { RipgrepConfig } from '../utils/ripgrep.js' import { buildJavaToolOptions } from './java-proxy-agent.js' import { generateProxyEnvVars, @@ -21,6 +22,11 @@ import { isStrictlyUnder, getDangerousDirectories, } from './sandbox-utils.js' +import { + gitDirDenyPaths, + gitFileDenyPaths, + submoduleGitDirs, +} from './mandatory-deny-paths.js' import type { FsReadRestrictionConfig, FsWriteRestrictionConfig, @@ -72,7 +78,7 @@ export interface LinuxSandboxParams { enableWeakerNestedSandbox?: boolean allowAllUnixSockets?: boolean binShell?: string - ripgrepConfig?: { command: string; args?: string[] } + ripgrepConfig?: RipgrepConfig /** Maximum directory depth to search for dangerous files (default: 3) */ mandatoryDenySearchDepth?: number /** Allow writes to .git/config files (default: false) */ @@ -269,13 +275,50 @@ function findFirstNonExistentComponent(targetPath: string): string { return targetPath // Shouldn't reach here if called correctly } +/** Where `parts` first occurs as consecutive segments of `segments`, or -1. */ +function indexOfSegmentRun(segments: string[], parts: string[]): number { + return segments.findIndex((_, i) => + parts.every((part, j) => segments[i + j] === part), + ) +} + +/** + * The paths under `cwd` a failed ripgrep run named in its diagnostics — the + * directories it could not read. Denying them is how the scan fails closed: + * their contents are unknown, so a nested repository inside one must not stay + * writable. rg reports `: `; a path holding `: ` is cut short + * at it, which denies an ancestor and so only ever denies more. + */ +function unreadablePathsFromRipgrepStderr( + stderr: string, + cwd: string, +): string[] { + const prefix = cwd + path.sep + const paths = new Set() + for (const line of stderr.split('\n')) { + const start = line.indexOf(prefix) + if (start === -1) continue + const rest = line.slice(start) + const end = rest.indexOf(': ') + const candidate = (end === -1 ? rest : rest.slice(0, end)).trimEnd() + if (candidate.length > prefix.length) paths.add(candidate) + } + return [...paths] +} + /** * Get mandatory deny paths using ripgrep (Linux only). * Uses a SINGLE ripgrep call with multiple glob patterns for efficiency. - * With --max-depth limiting, this is fast enough to run on each command without memoization. + * + * Runs on each command without memoization. `--max-depth` keeps that to + * milliseconds on ordinary trees, but `--no-ignore` means gitignored data + * within the depth is walked too: measured at about +100 ms per command on a + * tree with 150k ignored files three levels down. A scan that cannot finish + * inside {@link ripGrep}'s timeout aborts the wrap rather than sandboxing + * with a deny list of unknown completeness. */ async function linuxGetMandatoryDenyPaths( - ripgrepConfig: { command: string; args?: string[] } = { command: 'rg' }, + ripgrepConfig: RipgrepConfig = { command: 'rg' }, maxDepth: number = DEFAULT_MANDATORY_DENY_SEARCH_DEPTH, allowGitConfig = false, abortSignal?: AbortSignal, @@ -294,27 +337,33 @@ async function linuxGetMandatoryDenyPaths( ...dangerousDirectories.map(d => path.resolve(cwd, d)), ] - // 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. + // A repository's hooks/ and config, and those of the submodule git + // directories under its .git/modules (what a commit inside the submodule + // runs). Called for cwd's .git and for each nested one the scan finds. + const seenGitDirs = new Set() + const denyGitDir = (gitDir: string): void => { + if (seenGitDirs.has(gitDir)) return + seenGitDirs.add(gitDir) + const modules = submoduleGitDirs(path.join(gitDir, 'modules')) + denyPaths.push(...modules.unreadableDirs) + for (const dir of [gitDir, ...modules.gitDirs]) { + denyPaths.push(...gitDirDenyPaths(dir, allowGitConfig)) + } + } + const dotGitPath = path.resolve(cwd, '.git') - let dotGitIsDirectory = false + let dotGitStat: fs.Stats | undefined try { - dotGitIsDirectory = fs.statSync(dotGitPath).isDirectory() + dotGitStat = fs.statSync(dotGitPath) } catch { - // .git doesn't exist + // No .git: nothing is denied, since a mount at .git would block `git init`. } - - 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')) - } + if (dotGitStat?.isDirectory()) { + denyGitDir(dotGitPath) + } else if (dotGitStat?.isFile()) { + // A pointer file (linked worktree, submodule checkout) has no hooks/ + // beneath it, and binding a path under a file makes bwrap fail. + denyPaths.push(...gitFileDenyPaths(dotGitPath, allowGitConfig)) } // Build iglob args for all patterns in one ripgrep call @@ -325,13 +374,15 @@ async function linuxGetMandatoryDenyPaths( for (const dirName of dangerousDirectories) { iglobArgs.push('--iglob', `**/${dirName}/**`) } - // Git hooks always blocked in nested repos - iglobArgs.push('--iglob', '**/.git/hooks/**') - - // Git config conditionally blocked in nested repos - if (!allowGitConfig) { - iglobArgs.push('--iglob', '**/.git/config') - } + // A nested repository is recognised by ANY regular file directly inside its + // .git directory, so its hooks/ and config are denied at the depth the + // repository itself is found, not one level further down where the hook + // files sit (with the default depth, a repository directly under cwd has + // .git/config within reach but .git/hooks/* beyond it). Detection must not + // depend on any one file the sandboxed command could move aside, nor on + // allowGitConfig, which governs what is denied and not what is found. + // A FILE named .git is a worktree/submodule pointer (gitFileDenyPaths). + iglobArgs.push('--iglob', '**/.git/*', '--iglob', '**/.git') // Single ripgrep call to find all dangerous paths in subdirectories // Limit depth for performance - deeply nested dangerous files are rare @@ -342,6 +393,10 @@ async function linuxGetMandatoryDenyPaths( [ '--files', '--hidden', + // .gitignore, .ignore and .rgignore are writable inside the sandbox: + // honouring them would let one command hide a nested repository + // from the next command's scan. + '--no-ignore', '--max-depth', String(maxDepth), ...iglobArgs, @@ -353,41 +408,54 @@ async function linuxGetMandatoryDenyPaths( ripgrepConfig, ) } catch (error) { - logForDebugging(`[Sandbox] ripgrep scan failed: ${error}`) + if (error instanceof RipgrepError && error.timedOut) { + // The command that runs next is the one that could have made the tree + // slow to walk, so a truncated listing is not something to sandbox on: + // an unreached nested repository would be one with writable hooks. + throw new Error( + `[Sandbox] ripgrep scan of ${cwd} did not finish; refusing to sandbox with mandatory denies of unknown completeness: ${error.message}`, + ) + } + if (error instanceof RipgrepError) { + // An unreadable directory makes rg exit non-zero after listing the rest + // of the tree; those matches still count, and each directory it could + // not read is denied whole, since what it holds is unknown. + matches = error.partialMatches + denyPaths.push(...unreadablePathsFromRipgrepStderr(error.stderr, cwd)) + } + logForDebugging( + `[Sandbox] ripgrep scan failed, kept ${matches.length} partial matches; mandatory denies below cwd may be incomplete: ${error}`, + { level: 'warn' }, + ) } - // Process matches + const dirPatterns = dangerousDirectories.map(d => + normalizeCaseForComparison(d).split('/'), + ) for (const match of matches) { - const absolutePath = path.resolve(cwd, match) - - // File inside a dangerous directory -> add the directory path - let foundDir = false - for (const dirName of [...dangerousDirectories, '.git']) { - const normalizedDirName = normalizeCaseForComparison(dirName) - const segments = absolutePath.split(path.sep) - const dirIndex = segments.findIndex( - s => normalizeCaseForComparison(s) === normalizedDirName, - ) - if (dirIndex !== -1) { - // For .git, we want hooks/ or config, not the whole .git dir - if (dirName === '.git') { - const gitDir = segments.slice(0, dirIndex + 1).join(path.sep) - if (match.includes('.git/hooks')) { - denyPaths.push(path.join(gitDir, 'hooks')) - } else if (match.includes('.git/config')) { - denyPaths.push(path.join(gitDir, 'config')) - } - } else { - denyPaths.push(segments.slice(0, dirIndex + 1).join(path.sep)) - } - foundDir = true - break - } + // rg prefixes each match with its target, cwd, and does not follow + // symlinks, so every line is under it. Segments are compared relative to + // cwd, so a dangerous name in cwd's own location never counts. + const relative = path.relative(cwd, match).split(path.sep) + const lowered = relative.map(normalizeCaseForComparison) + + const dirRun = dirPatterns + .map(parts => ({ parts, at: indexOfSegmentRun(lowered, parts) })) + .find(({ at }) => at !== -1) + if (dirRun) { + // The directory, not the file, so files created in it later are covered. + const end = dirRun.at + dirRun.parts.length + denyPaths.push(path.join(cwd, ...relative.slice(0, end))) + continue } - - // Dangerous file match - if (!foundDir) { - denyPaths.push(absolutePath) + const gitAt = lowered.indexOf('.git') + if (gitAt === -1) { + denyPaths.push(match) + } else if (gitAt < relative.length - 1) { + denyGitDir(path.join(cwd, ...relative.slice(0, gitAt + 1))) + } else if (relative.length > 1) { + // cwd's own pointer file is handled above, before the scan. + denyPaths.push(...gitFileDenyPaths(match, allowGitConfig)) } } @@ -950,7 +1018,7 @@ async function generateFilesystemArgs( writeConfig: FsWriteRestrictionConfig | undefined, maskedFileBinds: Array<{ realPath: string; fakePath: string }> | undefined, maskedFileStoreDir: string | undefined, - ripgrepConfig: { command: string; args?: string[] } = { command: 'rg' }, + ripgrepConfig: RipgrepConfig = { command: 'rg' }, mandatoryDenySearchDepth: number = DEFAULT_MANDATORY_DENY_SEARCH_DEPTH, allowGitConfig = false, abortSignal?: AbortSignal, @@ -1458,6 +1526,13 @@ async function generateFilesystemArgs( // 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) { + // Absent deny paths under one absent directory share this + // destination (a git directory's hooks/ and config, say). A + // second bind would hit the first's mount point and bwrap + // aborts, taking every command in this cwd with it. The leaf + // case needs no check: normalizedPath is deduped above. + if (seenDenyWrite.has(firstNonExistent)) continue + seenDenyWrite.add(firstNonExistent) const emptyDir = fs.mkdtempSync( path.join(tmpdir(), 'claude-empty-'), ) diff --git a/src/sandbox/macos-sandbox-utils.ts b/src/sandbox/macos-sandbox-utils.ts index 2cc6e5f9..f5a21ae3 100644 --- a/src/sandbox/macos-sandbox-utils.ts +++ b/src/sandbox/macos-sandbox-utils.ts @@ -1,5 +1,6 @@ import { quote } from '../utils/shell-quote.js' import { spawn } from 'child_process' +import * as fs from 'fs' import * as path from 'path' import { logForDebugging } from '../utils/debug.js' import { whichSync } from '../utils/which.js' @@ -15,6 +16,11 @@ import { DANGEROUS_FILES, getDangerousDirectories, } from './sandbox-utils.js' +import { + gitDirDenyPaths, + gitFileDenyPaths, + submoduleGitDirs, +} from './mandatory-deny-paths.js' import { shouldIgnoreViolation } from './sandbox-violation-store.js' import type { @@ -73,8 +79,10 @@ export interface MacOSSandboxParams { } /** - * Get mandatory deny patterns as glob patterns (no filesystem scanning). - * macOS sandbox profile supports regex/glob matching directly via globToRegex(). + * Get mandatory deny patterns: glob patterns for what sits below cwd, which + * the macOS sandbox profile matches directly via globToRegex(), plus literal + * paths for the working directory's own repository. Reads cwd's `.git` and + * walks its `.git/modules`, so the result depends on the tree at cwd. */ export function macGetMandatoryDenyPatterns(allowGitConfig = false): string[] { const cwd = process.cwd() @@ -92,19 +100,54 @@ export function macGetMandatoryDenyPatterns(allowGitConfig = false): string[] { denyPaths.push(`**/${dirName}/**`) } - // Git hooks are always blocked for security - denyPaths.push(path.resolve(cwd, '.git/hooks')) - denyPaths.push('**/.git/hooks/**') + // Nested repositories and the submodule git directories they keep under + // .git/modules/ are matched by pattern: there is no scan on macOS, and a + // glob covers a git directory that does not exist yet as well. The + // submodule name is a single segment here — a nested repository's + // `vendor/lib` submodule is not covered — because a `**` in the middle + // would also match any component named config or hooks (a branch named + // feature/config, a submodule named config), which fails ordinary git + // operations that worked before. + for (const gitDirPattern of ['**/.git', '**/.git/modules/*']) { + denyPaths.push(...gitDirDenyPaths(gitDirPattern, allowGitConfig)) + } - // Git config - conditionally blocked based on allowGitConfig setting - if (!allowGitConfig) { - denyPaths.push(path.resolve(cwd, '.git/config')) - denyPaths.push('**/.git/config') + // The working directory's own repository is enumerated instead: literals + // are exact whatever a submodule is named, and each one pins its + // directories against being renamed out from under the deny. + const dotGit = path.resolve(cwd, '.git') + denyPaths.push(...gitDirDenyPaths(dotGit, allowGitConfig)) + let dotGitStat: fs.Stats | undefined + try { + dotGitStat = fs.statSync(dotGit) + } catch { + // no .git here + } + if (dotGitStat?.isFile()) { + // cwd checked out as a linked worktree or submodule: .git is a pointer + // file. Nested pointer files are matched by vnode type instead + // (gitPointerFilter), which cannot follow them. + denyPaths.push(...gitFileDenyPaths(dotGit, allowGitConfig)) + } else if (dotGitStat?.isDirectory()) { + const modules = submoduleGitDirs(path.join(dotGit, 'modules')) + denyPaths.push(...modules.unreadableDirs) + for (const gitDir of modules.gitDirs) { + denyPaths.push(...gitDirDenyPaths(gitDir, allowGitConfig)) + } } return [...new Set(denyPaths)] } +/** + * SBPL filter matching a regular file named `.git` anywhere under cwd, a + * `gitdir:` pointer. Matched by vnode type so a repository's .git DIRECTORY + * stays writable. + */ +function gitPointerFilter(): string { + return `(require-all (vnode-type REGULAR-FILE) ${pathFilter(normalizePathForSandbox('**/.git'))})` +} + export interface SandboxViolationEvent { line: string command?: string @@ -794,7 +837,25 @@ function generateWriteRules( for (const normalizedPath of ungrouped) { denyFilters.add(denyPathFilter(normalizedPath)) } + const gitPointer = gitPointerFilter() + denyFilters.add(gitPointer) rules.push(...renderRule('deny', ['file-write*'], denyFilters, logTag)) + // An existing pointer cannot be rewritten; `git worktree add` still creates + // new ones, but only inside the write roots, since this allow follows the + // denies. User and mandatory denies are re-applied to creation by the rule + // below, and removing or renaming over an existing pointer by the + // file-write-unlink deny after it. + if (allowFilters.size > 0) { + const createPointer = `(require-all ${gitPointer} (require-any ${[...allowFilters].join(' ')}))` + rules.push( + ...renderRule( + 'allow', + ['file-write-create'], + new Set([createPointer]), + logTag, + ), + ) + } // Block file movement to prevent bypass via mv/rename. A grouped path // contributes its regex, the pin for its parent directory, and the @@ -817,6 +878,15 @@ function generateWriteRules( ), ) + // Unlink only, so creating a pointer stays allowed: the rule above and the + // read section's re-allow of file-write-unlink for write roots would + // otherwise leave `rm lib/.git` and `mv evil lib/.git` open, which is the + // same rewrite the file-write* deny blocks. Emitted last; nothing after it + // in the profile re-allows unlink. + rules.push( + ...renderRule('deny', ['file-write-unlink'], new Set([gitPointer]), logTag), + ) + return rules } diff --git a/src/sandbox/mandatory-deny-paths.ts b/src/sandbox/mandatory-deny-paths.ts new file mode 100644 index 00000000..cb2774eb --- /dev/null +++ b/src/sandbox/mandatory-deny-paths.ts @@ -0,0 +1,330 @@ +import * as fs from 'fs' +import * as path from 'path' +import { logForDebugging } from '../utils/debug.js' + +/** The path is absent, as opposed to unreadable or otherwise unverifiable. */ +function isAbsenceError(err: unknown): boolean { + const code = (err as NodeJS.ErrnoException | undefined)?.code + return code === 'ENOENT' || code === 'ENOTDIR' +} + +/** + * How much of a `.git` pointer or a `commondir` is read. Both hold a single + * path, and git refuses a gitfile larger than 1 MiB, so a file this size is + * not one git would follow either. + */ +const MAX_GIT_METADATA_BYTES = 8192 + +/** + * Depth bound for the `.git/modules` walk. A submodule's name is its path + * (`vendor/lib`) and submodules nest, so the walk descends both name segments + * and nested `modules` directories; this bounds a hostile or looping tree, not + * a real one, and is deliberately unrelated to the ripgrep scan's depth. + */ +const MAX_SUBMODULE_WALK_DEPTH = 10 + +/** + * Entries whose presence makes a directory a git directory. git needs HEAD + * and objects; config and hooks are what this file protects, so a directory + * holding either is treated as one even if HEAD has been moved aside. + */ +const GIT_DIR_MARKERS = new Set(['HEAD', 'config', 'hooks', 'objects']) + +/** What {@link gitDirKind} concluded about a `gitdir:`/`commondir` target. */ +type GitDirKind = 'git-dir' | 'absent' | 'other' | 'unreadable' + +/** Directories found under a `.git/modules`, and what could not be read. */ +export interface SubmoduleScan { + /** The submodule git directories. */ + gitDirs: string[] + /** + * Directories the walk could not list. Their contents are unknown, so they + * are denied whole rather than left writable with a git directory possibly + * inside them. + */ + unreadableDirs: string[] +} + +/** + * The paths inside a git directory through which a write becomes code the + * host's git runs later: hooks/ always, `commondir` always (it redirects the + * hooks and config git reads to another directory entirely), and config plus + * `config.worktree` (core.fsmonitor, core.editor, core.hooksPath and the + * like, the latter read when extensions.worktreeConfig is on) unless the + * caller allows config writes. + */ +export function gitDirDenyPaths( + gitDir: string, + allowGitConfig: boolean, +): string[] { + const denyPaths = [path.join(gitDir, 'hooks'), path.join(gitDir, 'commondir')] + if (!allowGitConfig) { + denyPaths.push( + path.join(gitDir, 'config'), + path.join(gitDir, 'config.worktree'), + ) + } + return denyPaths +} + +/** + * Deny paths for a `.git` file, the `gitdir:` pointer of a linked worktree or + * submodule checkout: the file itself plus the hooks/ and config git reads + * through it (the named git directory's, and for a linked worktree its + * commondir's as well). + */ +export function gitFileDenyPaths( + gitFile: string, + allowGitConfig: boolean, +): string[] { + const denyPaths = [gitFile] + try { + const pointer = readGitMetadataFile(gitFile) + const target = + pointer === undefined ? undefined : parseGitdirPointer(pointer) + if (target === undefined) return denyPaths + const gitDir = path.resolve(path.dirname(gitFile), target) + denyPaths.push(...gitDirTargetDenyPaths(gitDir, allowGitConfig, gitFile)) + + // A linked worktree's git directory holds the path of the main one, whose + // hooks and config its commits run. + const commonFile = path.join(gitDir, 'commondir') + const common = readGitMetadataFile(commonFile) + const commonDir = + common === undefined + ? undefined + : path.resolve(gitDir, firstLine(common).trim()) + if (commonDir !== undefined && commonDir !== gitDir) { + denyPaths.push( + ...gitDirTargetDenyPaths(commonDir, allowGitConfig, commonFile), + ) + } + } catch (err) { + // A dangling pointer names nothing git would read. A pointer this process + // cannot read is one the host's git cannot read either, so the file + // itself is the whole deny; an unreadable TARGET is denied whole by + // gitDirTargetDenyPaths instead. + if (!isAbsenceError(err)) { + logForDebugging( + `[Sandbox] Could not follow ${gitFile}, denying only the file itself: ${err}`, + { level: 'warn' }, + ) + } + } + return denyPaths +} + +/** + * Git directories of the submodules under `modulesDir` (a repository's + * .git/modules), nested submodules included. A submodule's name is its path, + * so one can sit several levels down (modules/vendor/lib), hence the walk. + */ +export function submoduleGitDirs(modulesDir: string): SubmoduleScan { + const scan: SubmoduleScan = { gitDirs: [], unreadableDirs: [] } + collectSubmoduleGitDirs(modulesDir, 0, scan, new Set()) + return scan +} + +function collectSubmoduleGitDirs( + dir: string, + depth: number, + scan: SubmoduleScan, + visited: Set, +): void { + const entries = listDirectory(dir, scan) + if (entries === undefined) return + for (const entry of entries) { + const child = path.join(dir, entry.name) + // git accepts a symlinked entry under .git/modules, and Dirent.isDirectory + // is false for one, so the link is followed — and the realpath recorded, + // since a link back up would otherwise loop until the depth bound. + if (!isDirectory(entry, child, scan)) continue + const visitKey = realPathOrSelf(child) + if (visited.has(visitKey)) continue + visited.add(visitKey) + + const childEntries = listDirectory(child, scan) + if (childEntries === undefined) continue + const isGitDir = childEntries.some(e => GIT_DIR_MARKERS.has(e.name)) + if (isGitDir) scan.gitDirs.push(child) + + if (depth + 1 >= MAX_SUBMODULE_WALK_DEPTH) { + logForDebugging( + `[Sandbox] Stopped the .git/modules walk below ${child} at depth ${MAX_SUBMODULE_WALK_DEPTH}; submodule git directories beneath it are not denied`, + { level: 'warn' }, + ) + continue + } + collectSubmoduleGitDirs( + isGitDir ? path.join(child, 'modules') : child, + depth + 1, + scan, + visited, + ) + } +} + +/** Entries of `dir`, or undefined when it is absent or (recorded) unreadable. */ +function listDirectory( + dir: string, + scan: SubmoduleScan, +): fs.Dirent[] | undefined { + try { + return fs.readdirSync(dir, { withFileTypes: true }) + } catch (err) { + // Absent is the common case: no submodules, or none nested in this one. + if (!isAbsenceError(err)) { + const denied = deepestReachableAncestor(dir) ?? dir + scan.unreadableDirs.push(denied) + logForDebugging( + `[Sandbox] Could not list ${dir}, denying ${denied} whole: ${err}`, + { level: 'warn' }, + ) + } + return undefined + } +} + +/** Whether `entry` is a directory, following a symlink to one. */ +function isDirectory( + entry: fs.Dirent, + entryPath: string, + scan: SubmoduleScan, +): boolean { + if (entry.isDirectory()) return true + if (!entry.isSymbolicLink()) return false + try { + return fs.statSync(entryPath).isDirectory() + } catch (err) { + if (!isAbsenceError(err)) { + scan.unreadableDirs.push(deepestReachableAncestor(entryPath) ?? entryPath) + } + return false + } +} + +/** + * Deny paths for a directory a `gitdir:` or `commondir` names. An existing + * directory that is not a git directory is left alone: file content must not + * be able to point the deny list at, say, a Rails `config/`. An absent one is + * still denied, so the sandboxed command cannot create the target and fill it + * with hooks before the host's git first uses it. + */ +function gitDirTargetDenyPaths( + target: string, + allowGitConfig: boolean, + source: string, +): string[] { + const kind = gitDirKind(target) + switch (kind) { + case 'git-dir': + case 'absent': + return gitDirDenyPaths(target, allowGitConfig) + case 'unreadable': { + const denied = deepestReachableAncestor(target) ?? target + logForDebugging( + `[Sandbox] Could not read ${target} named by ${source}, denying ${denied} whole`, + { level: 'warn' }, + ) + return [denied] + } + case 'other': + logForDebugging( + `[Sandbox] ${source} names ${target}, which is not a git directory; denying only ${source}`, + { level: 'warn' }, + ) + return [] + } +} + +function gitDirKind(dir: string): GitDirKind { + let entries: fs.Dirent[] + try { + entries = fs.readdirSync(dir, { withFileTypes: true }) + } catch (err) { + return isAbsenceError(err) ? 'absent' : 'unreadable' + } + return entries.some(e => e.name === 'HEAD' || e.name === 'objects') + ? 'git-dir' + : 'other' +} + +/** + * At most {@link MAX_GIT_METADATA_BYTES} of `file`, or undefined when it is + * absent, is not a regular file, or is longer than that. The path is one a + * sandboxed command may create: a FIFO there would block the host on every + * later wrap (hence O_NONBLOCK and the type check), and an arbitrarily large + * file would be buffered whole on every command. + */ +function readGitMetadataFile(file: string): string | undefined { + // O_NONBLOCK is POSIX-only; this file's callers are the Linux and macOS + // backends, and 0 leaves the flags as they were. + const nonBlocking = fs.constants.O_NONBLOCK ?? 0 + let fd: number + try { + fd = fs.openSync(file, fs.constants.O_RDONLY | nonBlocking) + } catch (err) { + if (isAbsenceError(err)) return undefined + throw err + } + try { + // From the open file description, so it describes what was actually + // opened rather than what the path named a moment ago. + if (!fs.fstatSync(fd).isFile()) return undefined + const buffer = Buffer.alloc(MAX_GIT_METADATA_BYTES) + const read = fs.readSync(fd, buffer, 0, buffer.length, 0) + if (read === buffer.length) { + logForDebugging( + `[Sandbox] ${file} is larger than ${MAX_GIT_METADATA_BYTES} bytes, which is not a path git would follow; ignoring it`, + { level: 'warn' }, + ) + return undefined + } + return buffer.toString('utf8', 0, read) + } finally { + fs.closeSync(fd) + } +} + +/** + * The `gitdir:` target of a pointer file. git requires the prefix at byte 0 + * and trims only newline bytes from the end, and a path never spans lines. + */ +function parseGitdirPointer(contents: string): string | undefined { + const prefix = 'gitdir: ' + const line = firstLine(contents) + if (!line.startsWith(prefix)) return undefined + const target = line.slice(prefix.length) + return target.length > 0 ? target : undefined +} + +/** The first line, without the newline bytes git strips (`\n`, `\r`). */ +function firstLine(contents: string): string { + const end = contents.indexOf('\n') + return (end === -1 ? contents : contents.slice(0, end)).replace(/\r+$/, '') +} + +/** + * The deepest ancestor of `target` (itself included) this process can still + * stat. Denying that directory fails closed when the path below it cannot be + * inspected: nothing under it is writable in the sandbox. + */ +function deepestReachableAncestor(target: string): string | undefined { + for (let dir = target; ; dir = path.dirname(dir)) { + try { + if (fs.lstatSync(dir).isDirectory()) return dir + } catch { + // Unreachable at this level; try the parent. + } + if (path.dirname(dir) === dir) return undefined + } +} + +/** `target` with symlinks resolved, or itself when that fails. */ +function realPathOrSelf(target: string): string { + try { + return fs.realpathSync(target) + } catch { + return target + } +} diff --git a/src/utils/ripgrep.ts b/src/utils/ripgrep.ts index c246d917..dd9ed71a 100644 --- a/src/utils/ripgrep.ts +++ b/src/utils/ripgrep.ts @@ -7,8 +7,12 @@ export interface RipgrepConfig { args?: string[] /** Override argv[0] when spawning (for multicall binaries that dispatch on argv[0]) */ argv0?: string + /** How long the run may take before it is killed (default: 10 s). */ + timeoutMs?: number } +const DEFAULT_RIPGREP_TIMEOUT_MS = 10_000 + /** * Check if ripgrep (rg) is available synchronously * Returns true if rg is installed, false otherwise @@ -18,13 +22,44 @@ export function hasRipgrepSync(): boolean { } /** - * Execute ripgrep with the given arguments + * ripgrep exited with an error status. `partialMatches` is what it listed + * before that: rg reports an unreadable directory with exit code 2 after + * printing every match it could reach, and names each one in `stderr`. + * `timedOut` says the run was killed instead of finishing, so what it listed + * is a prefix of an unknown whole rather than everything it could reach. + */ +export class RipgrepError extends Error { + readonly partialMatches: string[] + readonly stderr: string + readonly timedOut: boolean + + constructor( + message: string, + partialMatches: string[], + stderr: string, + timedOut: boolean, + ) { + super(message) + this.partialMatches = partialMatches + this.stderr = stderr + this.timedOut = timedOut + } +} + +/** + * Execute ripgrep with the given arguments. + * + * The run is `--null`-delimited: a path may contain a newline, and a run cut + * short by the timeout can end mid-path, so line splitting would turn one + * path into two and hand back a truncated one. Output is split on NUL and an + * unterminated tail is dropped. + * * @param args Command-line arguments to pass to rg * @param target Target directory or file to search * @param abortSignal AbortSignal to cancel the operation * @param config Ripgrep configuration (command and optional args) - * @returns Array of matching lines (one per line of output) - * @throws Error if ripgrep exits with non-zero status (except exit code 1 which means no matches) + * @returns Array of matching paths + * @throws RipgrepError if ripgrep exits with non-zero status (except exit code 1 which means no matches) */ export async function ripGrep( args: string[], @@ -32,30 +67,57 @@ export async function ripGrep( abortSignal: AbortSignal, config: RipgrepConfig = { command: 'rg' }, ): Promise { - const { command, args: commandArgs = [], argv0 } = config + const { + command, + args: commandArgs = [], + argv0, + timeoutMs = DEFAULT_RIPGREP_TIMEOUT_MS, + } = config - const child = spawn(command, [...commandArgs, ...args, target], { + const child = spawn(command, [...commandArgs, '--null', ...args, target], { argv0, signal: abortSignal, - timeout: 10_000, + timeout: timeoutMs, windowsHide: true, }) - const [stdout, stderr, code] = await Promise.all([ + const [stdout, stderr, exit] = await Promise.all([ text(child.stdout), text(child.stderr), - new Promise((resolve, reject) => { - child.on('close', resolve) - child.on('error', reject) - }), + new Promise<{ code: number | null; signal: NodeJS.Signals | null }>( + (resolve, reject) => { + child.on('close', (code, signal) => resolve({ code, signal })) + child.on('error', reject) + }, + ), ]) - if (code === 0) { - return stdout.trim().split('\n').filter(Boolean) + const matches = splitNullDelimited(stdout) + if (exit.code === 0) { + return matches } - if (code === 1) { + if (exit.code === 1) { // Exit code 1 means "no matches found" - this is normal return [] } - throw new Error(`ripgrep failed with exit code ${code}: ${stderr}`) + // A null exit code means the child was killed rather than exiting. An + // abort rejects through the error handler above, so here it is the timeout. + const timedOut = exit.code === null + throw new RipgrepError( + timedOut + ? `ripgrep was killed by ${exit.signal ?? 'a signal'} after ${timeoutMs} ms: ${stderr}` + : `ripgrep failed with exit code ${exit.code}: ${stderr}`, + matches, + stderr, + timedOut, + ) +} + +/** NUL-terminated records, dropping an unterminated (truncated) last one. */ +function splitNullDelimited(output: string): string[] { + const records = output.split('\0') + // A complete run ends with a terminator, so the tail is empty; anything + // else is a record the run was cut off in the middle of. + records.pop() + return records.filter(Boolean) } diff --git a/test/sandbox/macos-glob-deny-reemit.test.ts b/test/sandbox/macos-glob-deny-reemit.test.ts index 1dc31973..2910f9c0 100644 --- a/test/sandbox/macos-glob-deny-reemit.test.ts +++ b/test/sandbox/macos-glob-deny-reemit.test.ts @@ -440,10 +440,10 @@ describe.if(isMacOS)('macOS write enforcement for glob denies', () => { expect(readFileSync(join(PROJECT, 'plain.txt'), 'utf8')).toBe('X\n') }) - it("mandatory **/.git/hooks/** still blocks a nested repo's hooks (regression guard)", () => { - // The mandatory patterns are anchored at process.cwd(); this pattern - // already carried its own /** tail before the subtree change, so this - // guards that the change keeps it working rather than fixing it. + it("mandatory **/.git/hooks still blocks a nested repo's hooks (regression guard)", () => { + // The mandatory patterns are anchored at process.cwd(). The pattern + // names the hooks directory itself, and the deny covers everything + // beneath it the way a literal subpath deny does. process.chdir(PROJECT) const hook = join(PROJECT, 'vendor', 'dep', '.git', 'hooks', 'pre-commit') const newHook = join( diff --git a/test/sandbox/mandatory-deny-paths.test.ts b/test/sandbox/mandatory-deny-paths.test.ts index e82ca64b..02d0d943 100644 --- a/test/sandbox/mandatory-deny-paths.test.ts +++ b/test/sandbox/mandatory-deny-paths.test.ts @@ -9,7 +9,10 @@ import { } from 'bun:test' import { spawn, spawnSync } from 'node:child_process' import { + chmodSync, mkdirSync, + mkdtempSync, + renameSync, rmSync, writeFileSync, readFileSync, @@ -28,7 +31,12 @@ import { wrapCommandWithSandboxLinux, cleanupBwrapMountPoints, } from '../../src/sandbox/linux-sandbox-utils.js' -import { isLinux, isSupportedPlatform } from '../helpers/platform.js' +import { + gitDirDenyPaths, + gitFileDenyPaths, + submoduleGitDirs, +} from '../../src/sandbox/mandatory-deny-paths.js' +import { isLinux, isSupportedPlatform, isWindows } from '../helpers/platform.js' /** * Integration tests for mandatory deny paths. @@ -45,6 +53,12 @@ describe.if(isSupportedPlatform)( 'Mandatory Deny Paths - Integration Tests', () => { const TEST_DIR = join(tmpdir(), `mandatory-deny-integration-${Date.now()}`) + // A read-denied region outside cwd, so the read section emits its + // operation-specific unlink/create rules (which a deny has to survive). + const READ_DENY_DIR = join( + tmpdir(), + `mandatory-deny-readdeny-${Date.now()}`, + ) const ORIGINAL_CONTENT = 'ORIGINAL' const MODIFIED_CONTENT = 'MODIFIED' let originalCwd: string @@ -52,6 +66,8 @@ describe.if(isSupportedPlatform)( beforeAll(() => { originalCwd = process.cwd() mkdirSync(TEST_DIR, { recursive: true }) + mkdirSync(READ_DENY_DIR, { recursive: true }) + writeFileSync(join(READ_DENY_DIR, 'secret.txt'), ORIGINAL_CONTENT) // Create ALL dangerous files from DANGEROUS_FILES writeFileSync(join(TEST_DIR, '.bashrc'), ORIGINAL_CONTENT) @@ -112,6 +128,84 @@ describe.if(isSupportedPlatform)( ) writeFileSync(join(TEST_DIR, '.git', 'index'), ORIGINAL_CONTENT) + // A nested repository directly under cwd, with `nested/` gitignored: + // its config sits at the default scan depth, its hook files one past + // it, and an ignore file must not hide either from the scan. + mkdirSync(join(TEST_DIR, 'nested', '.git', 'hooks'), { recursive: true }) + mkdirSync(join(TEST_DIR, 'nested', 'src'), { recursive: true }) + writeFileSync( + join(TEST_DIR, 'nested', '.git', 'HEAD'), + 'ref: refs/heads/main', + ) + writeFileSync( + join(TEST_DIR, 'nested', '.git', 'config'), + ORIGINAL_CONTENT, + ) + writeFileSync( + join(TEST_DIR, 'nested', '.git', 'hooks', 'pre-commit'), + ORIGINAL_CONTENT, + ) + writeFileSync(join(TEST_DIR, 'nested', 'src', 'ok.txt'), ORIGINAL_CONTENT) + writeFileSync(join(TEST_DIR, '.gitignore'), 'nested/\nlib/\n') + // The nested repository has a submodule of its own. + mkdirSync(join(TEST_DIR, 'nested', '.git', 'modules', 'dep', 'hooks'), { + recursive: true, + }) + writeFileSync( + join(TEST_DIR, 'nested', '.git', 'modules', 'dep', 'HEAD'), + 'ref: x', + ) + // A submodule: its git directory lives under cwd's .git/modules and its + // checkout has a .git FILE pointing there. + mkdirSync(join(TEST_DIR, '.git', 'modules', 'lib', 'hooks'), { + recursive: true, + }) + writeFileSync(join(TEST_DIR, '.git', 'modules', 'lib', 'HEAD'), 'ref: x') + writeFileSync( + join(TEST_DIR, '.git', 'modules', 'lib', 'config'), + ORIGINAL_CONTENT, + ) + writeFileSync( + join(TEST_DIR, '.git', 'modules', 'lib', 'hooks', 'pre-commit'), + ORIGINAL_CONTENT, + ) + mkdirSync(join(TEST_DIR, 'lib'), { recursive: true }) + writeFileSync( + join(TEST_DIR, 'lib', '.git'), + 'gitdir: ../.git/modules/lib', + ) + // A linked worktree of this repository checked out inside it: its + // .git file points at .git/worktrees/wt, whose commondir is the main + // .git, so the hooks a commit in the worktree runs are the main ones. + mkdirSync(join(TEST_DIR, '.git', 'worktrees', 'wt'), { recursive: true }) + writeFileSync(join(TEST_DIR, '.git', 'worktrees', 'wt', 'HEAD'), 'ref: x') + writeFileSync( + join(TEST_DIR, '.git', 'worktrees', 'wt', 'commondir'), + '../..\n', + ) + mkdirSync(join(TEST_DIR, 'wt-checkout'), { recursive: true }) + writeFileSync( + join(TEST_DIR, 'wt-checkout', '.git'), + `gitdir: ${join(TEST_DIR, '.git', 'worktrees', 'wt')}`, + ) + // A nested .claude/commands one level down (a name spanning two + // segments), within reach only of a deeper scan. + mkdirSync(join(TEST_DIR, 'pkg', '.claude', 'commands'), { + recursive: true, + }) + writeFileSync( + join(TEST_DIR, 'pkg', '.claude', 'commands', 'x.md'), + ORIGINAL_CONTENT, + ) + // A working directory whose own location has a dangerous name in it. + mkdirSync(join(TEST_DIR, '.vscode', 'ext', 'foo', 'sub'), { + recursive: true, + }) + writeFileSync( + join(TEST_DIR, '.vscode', 'ext', 'foo', 'sub', '.gitconfig'), + ORIGINAL_CONTENT, + ) + // Create safe file within .claude that SHOULD be writable (not commands/agents) writeFileSync( join(TEST_DIR, '.claude', 'some-other-file.txt'), @@ -122,6 +216,7 @@ describe.if(isSupportedPlatform)( afterAll(() => { process.chdir(originalCwd) rmSync(TEST_DIR, { recursive: true, force: true }) + rmSync(READ_DENY_DIR, { recursive: true, force: true }) }) beforeEach(() => { @@ -136,16 +231,27 @@ describe.if(isSupportedPlatform)( cleanupBwrapMountPoints({ force: true }) }) - async function runSandboxedWrite( - filePath: string, - content: string, + interface SandboxRunOptions { + mandatoryDenySearchDepth?: number + allowGitConfig?: boolean + allowOnly?: string[] + /** + * A read config makes the read section emit its own + * operation-specific unlink/create rules, which the write section's + * denies have to survive. + */ + readConfig?: { denyOnly: string[]; allowWithinDeny?: string[] } + } + + async function runSandboxed( + command: string, + opts: SandboxRunOptions = {}, ): Promise<{ success: boolean; stderr: string }> { const platform = getPlatform() - const command = `echo '${content}' > '${filePath}'` // Allow writes to current directory, but mandatory denies should still block dangerous files const writeConfig = { - allowOnly: ['.'], + allowOnly: opts.allowOnly ?? ['.'], denyWithinAllow: [], // Empty - relying on mandatory denies } @@ -154,15 +260,18 @@ describe.if(isSupportedPlatform)( wrappedCommand = wrapCommandWithSandboxMacOS({ command, needsNetworkRestriction: false, - readConfig: undefined, + readConfig: opts.readConfig, writeConfig, + allowGitConfig: opts.allowGitConfig, }) } else { wrappedCommand = await wrapCommandWithSandboxLinux({ command, needsNetworkRestriction: false, - readConfig: undefined, + readConfig: opts.readConfig, writeConfig, + mandatoryDenySearchDepth: opts.mandatoryDenySearchDepth, + allowGitConfig: opts.allowGitConfig, }) } @@ -178,6 +287,25 @@ describe.if(isSupportedPlatform)( } } + /** + * The write did not land. On Linux bwrap leaves the empty file it + * mounted over the absent deny path; on macOS nothing is created. + */ + function expectNotWritten(absolutePath: string): void { + const content = existsSync(absolutePath) + ? readFileSync(absolutePath, 'utf8') + : '' + expect(content).toBe('') + } + + async function runSandboxedWrite( + filePath: string, + content: string, + opts: SandboxRunOptions = {}, + ): Promise<{ success: boolean; stderr: string }> { + return runSandboxed(`echo '${content}' > '${filePath}'`, opts) + } + describe('Dangerous files should be blocked', () => { it('blocks writes to .bashrc', async () => { const result = await runSandboxedWrite('.bashrc', MODIFIED_CONTENT) @@ -267,6 +395,505 @@ describe.if(isSupportedPlatform)( }) }) + describe('Nested repositories, submodules and worktree pointers', () => { + it("blocks writes to a nested repository's .git/config even when gitignored", async () => { + const result = await runSandboxedWrite( + 'nested/.git/config', + MODIFIED_CONTENT, + ) + + expect(result.success).toBe(false) + expect(readFileSync('nested/.git/config', 'utf8')).toBe( + ORIGINAL_CONTENT, + ) + }) + + it("blocks writes to a nested repository's existing hook at the default depth", async () => { + const result = await runSandboxedWrite( + 'nested/.git/hooks/pre-commit', + MODIFIED_CONTENT, + ) + + expect(result.success).toBe(false) + expect(readFileSync('nested/.git/hooks/pre-commit', 'utf8')).toBe( + ORIGINAL_CONTENT, + ) + }) + + it("blocks a nested repository's hooks when only its HEAD is within the scan depth", async () => { + // With allowGitConfig the scan does not look for .git/config, and the + // hook files lie one level past the default depth. + const result = await runSandboxedWrite( + 'nested/.git/hooks/pre-commit', + MODIFIED_CONTENT, + { allowGitConfig: true }, + ) + + expect(result.success).toBe(false) + expect(readFileSync('nested/.git/hooks/pre-commit', 'utf8')).toBe( + ORIGINAL_CONTENT, + ) + }) + + it('blocks creating a new hook in a nested repository', async () => { + const result = await runSandboxedWrite( + 'nested/.git/hooks/post-checkout', + MODIFIED_CONTENT, + ) + + expect(result.success).toBe(false) + expect(existsSync('nested/.git/hooks/post-checkout')).toBe(false) + }) + + it('keeps the rest of a nested repository writable', async () => { + const result = await runSandboxedWrite( + 'nested/src/ok.txt', + MODIFIED_CONTENT, + ) + + expect(result.success).toBe(true) + expect(readFileSync('nested/src/ok.txt', 'utf8').trim()).toBe( + MODIFIED_CONTENT, + ) + }) + + it.if(isLinux && process.getuid?.() !== 0)( + 'keeps what the scan found when a directory under cwd is unreadable', + async () => { + mkdirSync('unreadable', { mode: 0o000 }) + try { + const result = await runSandboxedWrite( + 'nested/.git/config', + MODIFIED_CONTENT, + ) + + expect(result.success).toBe(false) + expect(readFileSync('nested/.git/config', 'utf8')).toBe( + ORIGINAL_CONTENT, + ) + } finally { + chmodSync('unreadable', 0o755) + rmSync('unreadable', { recursive: true, force: true }) + } + }, + ) + + it("blocks writes to a submodule's config under .git/modules", async () => { + const result = await runSandboxedWrite( + '.git/modules/lib/config', + MODIFIED_CONTENT, + ) + + expect(result.success).toBe(false) + expect(readFileSync('.git/modules/lib/config', 'utf8')).toBe( + ORIGINAL_CONTENT, + ) + }) + + it("blocks creating a hook in a submodule's git directory", async () => { + const result = await runSandboxedWrite( + '.git/modules/lib/hooks/post-checkout', + MODIFIED_CONTENT, + ) + + expect(result.success).toBe(false) + expect(existsSync('.git/modules/lib/hooks/post-checkout')).toBe(false) + }) + + it("blocks creating a hook in a nested repository's submodule", async () => { + const result = await runSandboxedWrite( + 'nested/.git/modules/dep/hooks/post-checkout', + MODIFIED_CONTENT, + ) + + expect(result.success).toBe(false) + expect(existsSync('nested/.git/modules/dep/hooks/post-checkout')).toBe( + false, + ) + }) + + it("blocks repointing a submodule checkout's .git file", async () => { + const result = await runSandboxedWrite( + 'lib/.git', + 'gitdir: /tmp/elsewhere', + ) + + expect(result.success).toBe(false) + expect(readFileSync('lib/.git', 'utf8')).toBe( + 'gitdir: ../.git/modules/lib', + ) + }) + + it('blocks removing an existing .git pointer file', async () => { + const result = await runSandboxed('rm -f lib/.git', { + readConfig: { denyOnly: [READ_DENY_DIR] }, + }) + + expect(result.success).toBe(false) + expect(readFileSync('lib/.git', 'utf8')).toBe( + 'gitdir: ../.git/modules/lib', + ) + }) + + it('blocks renaming a file over an existing .git pointer', async () => { + writeFileSync(join(TEST_DIR, 'lib', 'decoy'), 'gitdir: /tmp/elsewhere') + try { + const result = await runSandboxed('mv -f lib/decoy lib/.git', { + readConfig: { denyOnly: [READ_DENY_DIR] }, + }) + + expect(result.success).toBe(false) + expect(readFileSync('lib/.git', 'utf8')).toBe( + 'gitdir: ../.git/modules/lib', + ) + } finally { + rmSync(join(TEST_DIR, 'lib', 'decoy'), { force: true }) + } + }) + + it('still removes an ordinary file with the same read config', async () => { + writeFileSync(join(TEST_DIR, 'lib', 'plain.txt'), ORIGINAL_CONTENT) + try { + const result = await runSandboxed('rm -f lib/plain.txt', { + readConfig: { denyOnly: [READ_DENY_DIR] }, + }) + + expect(result.success).toBe(true) + expect(existsSync(join(TEST_DIR, 'lib', 'plain.txt'))).toBe(false) + } finally { + rmSync(join(TEST_DIR, 'lib', 'plain.txt'), { force: true }) + } + }) + + it("blocks creating a commondir in the repository's git directory", async () => { + // git reads hooks and config through commondir, so a write here + // moves every deny below to a directory of the command's choosing. + const result = await runSandboxedWrite('.git/commondir', 'decoy') + + expect(result.success).toBe(false) + expectNotWritten(join(TEST_DIR, '.git', 'commondir')) + }) + + it("blocks creating a commondir in a submodule's git directory", async () => { + const result = await runSandboxedWrite( + '.git/modules/lib/commondir', + 'decoy', + ) + + expect(result.success).toBe(false) + expectNotWritten(join(TEST_DIR, '.git', 'modules', 'lib', 'commondir')) + }) + + it("blocks creating a nested repository's commondir", async () => { + const result = await runSandboxedWrite('nested/.git/commondir', 'decoy') + + expect(result.success).toBe(false) + expectNotWritten(join(TEST_DIR, 'nested', '.git', 'commondir')) + }) + + it('blocks creating .git/config.worktree', async () => { + // Read instead of .git/config wherever extensions.worktreeConfig is + // on, which `git sparse-checkout init` turns on. + const result = await runSandboxedWrite( + '.git/config.worktree', + 'fsmonitor = touch pwned', + ) + + expect(result.success).toBe(false) + expectNotWritten(join(TEST_DIR, '.git', 'config.worktree')) + }) + + it('allows .git/config.worktree when allowGitConfig is true', async () => { + try { + const result = await runSandboxedWrite( + '.git/config.worktree', + 'bare = false', + { allowGitConfig: true }, + ) + + expect(result.success).toBe(true) + } finally { + rmSync(join(TEST_DIR, '.git', 'config.worktree'), { force: true }) + } + }) + + it('finds a submodule git directory whose HEAD was moved aside', async () => { + const head = join(TEST_DIR, '.git', 'modules', 'lib', 'HEAD') + renameSync(head, `${head}.bak`) + try { + const result = await runSandboxedWrite( + '.git/modules/lib/hooks/pre-commit', + MODIFIED_CONTENT, + ) + + expect(result.success).toBe(false) + expect( + readFileSync( + join(TEST_DIR, '.git', 'modules', 'lib', 'hooks', 'pre-commit'), + 'utf8', + ), + ).toBe(ORIGINAL_CONTENT) + } finally { + renameSync(`${head}.bak`, head) + } + }) + + it.if(isLinux)( + 'finds a nested repository whose HEAD was moved aside', + async () => { + // With allowGitConfig the scan does not look for config either, and + // the hook files are one level past the default depth: the + // repository has to be recognised by whatever else its .git holds. + const head = join(TEST_DIR, 'nested', '.git', 'HEAD') + renameSync(head, `${head}.bak`) + try { + const result = await runSandboxedWrite( + 'nested/.git/hooks/pre-commit', + MODIFIED_CONTENT, + { allowGitConfig: true }, + ) + + expect(result.success).toBe(false) + expect(readFileSync('nested/.git/hooks/pre-commit', 'utf8')).toBe( + ORIGINAL_CONTENT, + ) + } finally { + renameSync(`${head}.bak`, head) + } + }, + ) + + it.if(isLinux)( + 'does not follow a .git file that names an ordinary directory', + async () => { + // `gitdir: ..` from app/tools would otherwise make app/config and + // app/hooks — an ordinary Rails-shaped tree — read-only. + mkdirSync(join(TEST_DIR, 'app', 'config'), { recursive: true }) + mkdirSync(join(TEST_DIR, 'app', 'tools'), { recursive: true }) + writeFileSync(join(TEST_DIR, 'app', 'tools', '.git'), 'gitdir: ..') + try { + const result = await runSandboxedWrite( + 'app/config/settings.yml', + MODIFIED_CONTENT, + ) + + expect(result.success).toBe(true) + } finally { + rmSync(join(TEST_DIR, 'app'), { recursive: true, force: true }) + } + }, + ) + + it.if(isLinux)( + 'blocks filling in the git directory a dangling .git file names', + async () => { + mkdirSync(join(TEST_DIR, 'dangling'), { recursive: true }) + writeFileSync( + join(TEST_DIR, 'dangling', '.git'), + 'gitdir: ../dangling-gitdir', + ) + try { + const result = await runSandboxed( + 'mkdir -p dangling-gitdir/hooks && echo X > dangling-gitdir/hooks/pre-commit', + ) + + expect(result.success).toBe(false) + expect( + existsSync( + join(TEST_DIR, 'dangling-gitdir', 'hooks', 'pre-commit'), + ), + ).toBe(false) + } finally { + rmSync(join(TEST_DIR, 'dangling'), { recursive: true, force: true }) + rmSync(join(TEST_DIR, 'dangling-gitdir'), { + recursive: true, + force: true, + }) + } + }, + ) + + it.if(isLinux)( + 'refuses to sandbox at all when the scan does not finish', + async () => { + // One complete path, one the run was cut off in the middle of, and + // then a run that outlives its timeout: what it did not reach is + // unknown, so there is nothing safe to wrap the next command with. + const error = await wrapCommandWithSandboxLinux({ + command: 'echo hi', + needsNetworkRestriction: false, + readConfig: undefined, + writeConfig: { allowOnly: ['.'], denyWithinAllow: [] }, + ripgrepConfig: { + command: '/bin/sh', + args: [ + '-c', + 'printf "%s\\0%s" "$PWD/a/.git/HEAD" "$PWD/b/.gi"; exec sleep 30', + ], + timeoutMs: 200, + }, + }).catch((e: unknown) => e) + + expect(error).toBeInstanceOf(Error) + expect((error as Error).message).toMatch(/did not finish/) + }, + ) + + it.if(isLinux && process.getuid?.() !== 0)( + 'denies a directory the scan could not read', + async () => { + mkdirSync(join(TEST_DIR, 'locked', '.git', 'hooks'), { + recursive: true, + }) + writeFileSync( + join(TEST_DIR, 'locked', '.git', 'HEAD'), + 'ref: refs/heads/main', + ) + chmodSync(join(TEST_DIR, 'locked'), 0o000) + try { + const result = await runSandboxed( + 'chmod 755 locked && echo X > locked/.git/hooks/pre-commit', + ) + + expect(result.success).toBe(false) + expect(result.stderr).not.toBe('') + } finally { + chmodSync(join(TEST_DIR, 'locked'), 0o755) + rmSync(join(TEST_DIR, 'locked'), { recursive: true, force: true }) + } + }, + ) + + it('still lets a command create a .git file where none exists', async () => { + mkdirSync('fresh-checkout', { recursive: true }) + try { + const result = await runSandboxedWrite( + 'fresh-checkout/.git', + 'gitdir: ../.git/modules/fresh', + ) + + expect(result.success).toBe(true) + expect(readFileSync('fresh-checkout/.git', 'utf8').trim()).toBe( + 'gitdir: ../.git/modules/fresh', + ) + } finally { + rmSync('fresh-checkout', { recursive: true, force: true }) + } + }) + + it('does not let a .git file be created outside the allowed write paths', async () => { + const opts = { allowOnly: [join(TEST_DIR, 'nested', 'src')] } + mkdirSync('unlisted', { recursive: true }) + try { + const pointer = await runSandboxedWrite( + 'unlisted/.git', + 'gitdir: /tmp/elsewhere', + opts, + ) + expect(pointer.success).toBe(false) + expect(existsSync('unlisted/.git')).toBe(false) + + const control = await runSandboxedWrite( + 'nested/src/ok.txt', + MODIFIED_CONTENT, + opts, + ) + expect(control.success).toBe(true) + } finally { + rmSync('unlisted', { recursive: true, force: true }) + } + }) + + describe('from a linked worktree checkout', () => { + const opts = { allowOnly: [TEST_DIR] } + beforeEach(() => { + process.chdir(join(TEST_DIR, 'wt-checkout')) + }) + afterEach(() => { + rmSync(join(TEST_DIR, 'wt-checkout', 'notes.txt'), { force: true }) + }) + + it("blocks the main repository's hooks, which the worktree's commits run", async () => { + const hook = join(TEST_DIR, '.git', 'hooks', 'pre-commit') + const denied = await runSandboxedWrite(hook, MODIFIED_CONTENT, opts) + expect(denied.success).toBe(false) + expect(readFileSync(hook, 'utf8')).toBe(ORIGINAL_CONTENT) + + const control = await runSandboxedWrite( + 'notes.txt', + MODIFIED_CONTENT, + opts, + ) + expect(control.success).toBe(true) + }) + + it("blocks rewriting the worktree's commondir", async () => { + // It names the git directory whose hooks and config a commit here + // runs, so it chooses what the denies below apply to. + const commondir = join( + TEST_DIR, + '.git', + 'worktrees', + 'wt', + 'commondir', + ) + const denied = await runSandboxedWrite(commondir, '../../decoy', opts) + + expect(denied.success).toBe(false) + expect(readFileSync(commondir, 'utf8')).toBe('../..\n') + }) + + it("blocks repointing the checkout's own .git file", async () => { + const original = readFileSync('.git', 'utf8') + const result = await runSandboxedWrite( + '.git', + 'gitdir: /tmp/elsewhere', + opts, + ) + + expect(result.success).toBe(false) + expect(readFileSync('.git', 'utf8')).toBe(original) + }) + }) + + it("matches dangerous names below cwd only, not in cwd's own location", async () => { + process.chdir(join(TEST_DIR, '.vscode', 'ext', 'foo')) + + const denied = await runSandboxedWrite( + 'sub/.gitconfig', + MODIFIED_CONTENT, + ) + expect(denied.success).toBe(false) + expect(readFileSync('sub/.gitconfig', 'utf8')).toBe(ORIGINAL_CONTENT) + + const control = await runSandboxedWrite('sub/ok.txt', MODIFIED_CONTENT) + expect(control.success).toBe(true) + }) + + it('denies a nested .claude/commands as a directory once the scan reaches it', async () => { + // pkg/.claude/commands/x.md is four segments deep: found with a + // depth of 4 (macOS matches by pattern at any depth), and then the + // whole directory is read-only, new files included. + const existing = await runSandboxedWrite( + 'pkg/.claude/commands/x.md', + MODIFIED_CONTENT, + { mandatoryDenySearchDepth: 4 }, + ) + expect(existing.success).toBe(false) + expect(readFileSync('pkg/.claude/commands/x.md', 'utf8')).toBe( + ORIGINAL_CONTENT, + ) + + const created = await runSandboxedWrite( + 'pkg/.claude/commands/new.md', + MODIFIED_CONTENT, + { mandatoryDenySearchDepth: 4 }, + ) + expect(created.success).toBe(false) + expect(existsSync('pkg/.claude/commands/new.md')).toBe(false) + }) + }) + describe('Dangerous directories should be blocked', () => { it('blocks writes to .vscode/', async () => { const result = await runSandboxedWrite( @@ -926,10 +1553,10 @@ describe.if(isSupportedPlatform)( 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. + // .git is a pointer file here, so it goes through + // gitFileDenyPaths: the file itself and the hooks and config it + // leads to are denied, and nothing is mounted under the file + // (bwrap could not create a mount point there). const wrappedCommand = await wrapCommandWithSandboxLinux({ command: 'echo hello', needsNetworkRestriction: false, @@ -948,7 +1575,6 @@ describe.if(isSupportedPlatform)( // should not cause the sandbox to fail. expect(result.status).toBe(0) expect(result.stdout.trim()).toBe('hello') - cleanupBwrapMountPoints() } finally { process.chdir(originalDir) @@ -1181,3 +1807,171 @@ describe('macGetMandatoryDenyPatterns - Unit Tests', () => { expect(hasGitConfigPattern).toBe(true) }) }) +describe('Git metadata deny paths - Unit Tests', () => { + let dir: string + + beforeEach(() => { + dir = mkdtempSync(join(tmpdir(), 'git-deny-paths-')) + }) + + afterEach(() => { + rmSync(dir, { recursive: true, force: true }) + }) + + /** A directory git would accept as a git directory. */ + function makeGitDir(gitDir: string): string { + mkdirSync(join(gitDir, 'hooks'), { recursive: true }) + writeFileSync(join(gitDir, 'HEAD'), 'ref: refs/heads/main') + return gitDir + } + + function makePointer(checkout: string, target: string): string { + mkdirSync(join(dir, checkout), { recursive: true }) + const pointer = join(dir, checkout, '.git') + writeFileSync(pointer, `gitdir: ${target}\n`) + return pointer + } + + it('denies commondir in every git directory, and config.worktree with config', () => { + expect(gitDirDenyPaths('/repo/.git', false)).toEqual([ + '/repo/.git/hooks', + '/repo/.git/commondir', + '/repo/.git/config', + '/repo/.git/config.worktree', + ]) + expect(gitDirDenyPaths('/repo/.git', true)).toEqual([ + '/repo/.git/hooks', + '/repo/.git/commondir', + ]) + }) + + it('follows a pointer to the git directory it names', () => { + const gitDir = makeGitDir(join(dir, 'gitdir')) + const pointer = makePointer('checkout', '../gitdir') + + expect(gitFileDenyPaths(pointer, false)).toEqual([ + pointer, + ...gitDirDenyPaths(gitDir, false), + ]) + }) + + it("follows a linked worktree's commondir as well", () => { + const main = makeGitDir(join(dir, 'main.git')) + const worktreeGitDir = makeGitDir(join(dir, 'main.git', 'worktrees', 'wt')) + writeFileSync(join(worktreeGitDir, 'commondir'), '../..\n') + const pointer = makePointer('wt-checkout', worktreeGitDir) + + expect(gitFileDenyPaths(pointer, false)).toEqual([ + pointer, + ...gitDirDenyPaths(worktreeGitDir, false), + ...gitDirDenyPaths(main, false), + ]) + }) + + it('leaves a pointer that names an ordinary directory alone', () => { + // `gitdir: ..` from app/tools would otherwise deny app/config and + // app/hooks, which are an ordinary tree and not a git directory. + mkdirSync(join(dir, 'app', 'config'), { recursive: true }) + const pointer = makePointer(join('app', 'tools'), '..') + + expect(gitFileDenyPaths(pointer, false)).toEqual([pointer]) + }) + + it('blocks the git directory a dangling pointer names from being filled in', () => { + const pointer = makePointer('checkout', '../not-created-yet') + + expect(gitFileDenyPaths(pointer, false)).toEqual([ + pointer, + ...gitDirDenyPaths(join(dir, 'not-created-yet'), false), + ]) + }) + + it('ignores a .git file longer than a path git would follow', () => { + const gitDir = makeGitDir(join(dir, 'gitdir')) + mkdirSync(join(dir, 'checkout'), { recursive: true }) + const pointer = join(dir, 'checkout', '.git') + writeFileSync(pointer, `gitdir: ${gitDir}${' '.repeat(9000)}\n`) + + // git trims only newline bytes, so the padded path is not one it follows + // either — and the file is never read whole on the way to finding out. + expect(gitFileDenyPaths(pointer, false)).toEqual([pointer]) + }) + + it.if(!isWindows)( + 'does not block on a FIFO left where a git directory keeps its commondir', + () => { + const gitDir = makeGitDir(join(dir, 'gitdir')) + expect(spawnSync('mkfifo', [join(gitDir, 'commondir')]).status).toBe(0) + const pointer = makePointer('checkout', '../gitdir') + + expect(gitFileDenyPaths(pointer, false)).toEqual([ + pointer, + ...gitDirDenyPaths(gitDir, false), + ]) + }, + ) + + it.if(!isWindows)( + 'does not block on a FIFO left where a pointer file goes', + () => { + mkdirSync(join(dir, 'checkout'), { recursive: true }) + const pointer = join(dir, 'checkout', '.git') + expect(spawnSync('mkfifo', [pointer]).status).toBe(0) + + expect(gitFileDenyPaths(pointer, false)).toEqual([pointer]) + }, + ) + + it('recognises a submodule git directory without a HEAD', () => { + const gitDir = join(dir, 'modules', 'lib') + mkdirSync(join(gitDir, 'hooks'), { recursive: true }) + + expect(submoduleGitDirs(join(dir, 'modules'))).toEqual({ + gitDirs: [gitDir], + unreadableDirs: [], + }) + }) + + it('walks a submodule name that spans several segments, and nested ones', () => { + const outer = makeGitDir(join(dir, 'modules', 'vendor', 'lib')) + const inner = makeGitDir(join(outer, 'modules', 'dep')) + + const scan = submoduleGitDirs(join(dir, 'modules')) + expect(scan.gitDirs.sort()).toEqual([outer, inner].sort()) + }) + + it.if(!isWindows)('follows a symlinked entry under modules', () => { + const gitDir = makeGitDir(join(dir, 'elsewhere')) + mkdirSync(join(dir, 'modules'), { recursive: true }) + symlinkSync(gitDir, join(dir, 'modules', 'lib')) + + expect(submoduleGitDirs(join(dir, 'modules')).gitDirs).toEqual([ + join(dir, 'modules', 'lib'), + ]) + }) + + it.if(!isWindows && process.getuid?.() !== 0)( + 'denies a directory under modules it could not list', + () => { + const locked = join(dir, 'modules', 'locked') + makeGitDir(join(locked, 'deep')) + chmodSync(locked, 0o000) + try { + const scan = submoduleGitDirs(join(dir, 'modules')) + expect(scan.gitDirs).toEqual([]) + expect(scan.unreadableDirs).toEqual([locked]) + } finally { + chmodSync(locked, 0o755) + } + }, + ) + + it('stops walking modules at its own depth bound', () => { + // Deeper than the bound: a name of 12 segments, which no real submodule + // has, and which a symlink loop could otherwise spin on. + const deep = join(dir, 'modules', ...Array.from({ length: 12 }, () => 'x')) + makeGitDir(deep) + + expect(submoduleGitDirs(join(dir, 'modules')).gitDirs).toEqual([]) + }) +}) diff --git a/test/utils/ripgrep.test.ts b/test/utils/ripgrep.test.ts index 26136d32..ace45b94 100644 --- a/test/utils/ripgrep.test.ts +++ b/test/utils/ripgrep.test.ts @@ -1,8 +1,9 @@ import { describe, it, expect } from 'bun:test' -import { writeFileSync, mkdtempSync, rmSync } from 'fs' +import { chmodSync, mkdirSync, writeFileSync, mkdtempSync, rmSync } from 'fs' import { tmpdir } from 'os' import { join } from 'path' -import { ripGrep } from '../../src/utils/ripgrep.js' +import { ripGrep, RipgrepError } from '../../src/utils/ripgrep.js' +import { isWindows } from '../helpers/platform.js' describe('ripGrep', () => { it('finds matches with default config', async () => { @@ -43,7 +44,7 @@ describe('ripGrep', () => { try { const script = join(dir, 'echo-argv0.cjs') // ripGrep appends target as the last arg; ignore it and print argv0 - writeFileSync(script, 'process.stdout.write(process.argv0)') + writeFileSync(script, "process.stdout.write(process.argv0 + '\\0')") const results = await ripGrep([], dir, new AbortController().signal, { command: process.execPath, @@ -60,7 +61,7 @@ describe('ripGrep', () => { const dir = mkdtempSync(join(tmpdir(), 'rg-noargv0-')) try { const script = join(dir, 'echo-argv0.cjs') - writeFileSync(script, 'process.stdout.write(process.argv0)') + writeFileSync(script, "process.stdout.write(process.argv0 + '\\0')") const results = await ripGrep([], dir, new AbortController().signal, { command: process.execPath, @@ -79,4 +80,71 @@ describe('ripGrep', () => { ripGrep(['--invalid-flag-xyz'], '.', new AbortController().signal), ).rejects.toThrow(/ripgrep failed/) }) + + it.if(!isWindows)( + 'drops a path a killed run was cut off in the middle of', + async () => { + const dir = mkdtempSync(join(tmpdir(), 'rg-timeout-')) + try { + const error = await ripGrep([], dir, new AbortController().signal, { + command: '/bin/sh', + // A complete record, then a truncated one, then a run that outlives + // the timeout. exec so the kill reaches whatever holds stdout. + args: ['-c', 'printf "/found/a\\0/trunc"; exec sleep 30'], + timeoutMs: 200, + }).catch((e: unknown) => e) + + expect(error).toBeInstanceOf(RipgrepError) + expect((error as RipgrepError).timedOut).toBe(true) + expect((error as RipgrepError).partialMatches).toEqual(['/found/a']) + } finally { + rmSync(dir, { recursive: true }) + } + }, + ) + + it.if(!isWindows)( + 'keeps a path containing a newline in one piece', + async () => { + const dir = mkdtempSync(join(tmpdir(), 'rg-newline-')) + try { + const results = await ripGrep([], dir, new AbortController().signal, { + command: '/bin/sh', + args: ['-c', 'printf "/a/nl\\ndir/.git/HEAD\\0"'], + }) + + expect(results).toEqual(['/a/nl\ndir/.git/HEAD']) + } finally { + rmSync(dir, { recursive: true }) + } + }, + ) + + it.if(!isWindows && process.getuid?.() !== 0)( + 'hands back what rg listed before an unreadable directory failed the run', + async () => { + const dir = mkdtempSync(join(tmpdir(), 'rg-test-')) + mkdirSync(join(dir, 'locked')) + writeFileSync(join(dir, 'a.txt'), 'hello') + chmodSync(join(dir, 'locked'), 0o000) + try { + const error = await ripGrep( + ['--files'], + dir, + new AbortController().signal, + ).catch((e: unknown) => e) + + expect(error).toBeInstanceOf(RipgrepError) + expect((error as RipgrepError).partialMatches).toEqual([ + join(dir, 'a.txt'), + ]) + // The caller denies what rg could not read, so the paths must survive. + expect((error as RipgrepError).stderr).toContain(join(dir, 'locked')) + expect((error as RipgrepError).timedOut).toBe(false) + } finally { + chmodSync(join(dir, 'locked'), 0o755) + rmSync(dir, { recursive: true }) + } + }, + ) })