diff --git a/README.md b/README.md index 14d89686..6c316f59 100644 --- a/README.md +++ b/README.md @@ -215,7 +215,7 @@ child.on('exit', async code => { }) ``` -**Violation attribution (`commandId` / `commandText`).** Violations observed while a wrapped command runs (seatbelt log lines, seccomp events, proxy denies) are stored under an attribution key, and `annotateStderrWithSandboxFailures(key, stderr)` / `getViolationsForCommand(key)` look them up by that same key. By default the key is the wrapped string itself. Pass an opaque per-invocation `commandId` (e.g. a tool-use id) to key by that instead — recommended: keys compare on their first 100 characters, so long commands sharing a prefix would otherwise cross-attribute, and a rerun of the same text would inherit the earlier run's events. If the string you *execute* is not the command the invocation *represents* (e.g. you wrap an assembled `source && eval ''`), also pass `commandText: ''`: it is what `ignoreViolations` command patterns match against and what each violation reports as its `command`. +**Violation attribution (`commandId` / `commandText`).** Violations observed while a wrapped command runs (seatbelt log lines, seccomp events, proxy denies) are stored under an attribution key, and `annotateStderrWithSandboxFailures(key, stderr)` / `getViolationsForCommand(key)` look them up by that same key. By default the key is the wrapped string itself. Pass an opaque per-invocation `commandId` (e.g. a tool-use id) to key by that instead — recommended: keys compare on their first 100 characters, so long commands sharing a prefix would otherwise cross-attribute, and a rerun of the same text would inherit the earlier run's events. If the string you _execute_ is not the command the invocation _represents_ (e.g. you wrap an assembled `source && eval ''`), also pass `commandText: ''`: it is what `ignoreViolations` command patterns match against and what each violation reports as its `command`. ```typescript const wrapped = await SandboxManager.wrapWithSandbox( @@ -226,7 +226,10 @@ const wrapped = await SandboxManager.wrapWithSandbox( { commandId: invocationId, commandText: rawCommand }, ) // ... run it ... -const annotated = SandboxManager.annotateStderrWithSandboxFailures(invocationId, stderr) +const annotated = SandboxManager.annotateStderrWithSandboxFailures( + invocationId, + stderr, +) ``` #### Available exports @@ -353,6 +356,8 @@ Uses two different patterns: - `filesystem.allowWrite` - Array of paths to allow write access. Empty array = no write access. - `filesystem.denyWrite` - Array of paths to deny write access within allowed paths (takes precedence over allowWrite) +A few paths are writable without being listed: the child's stdio and `/tmp/claude`, and as a convenience `~/.npm/_logs` and `~/.claude/debug`. Those two home directories are dropped when a `denyRead` entry covers them (and kept when an `allowRead` entry beneath that deny re-opens them), so list them in `allowWrite` if you want them writable under a home read-deny. + **Path Syntax (macOS):** Paths support git-style glob patterns on macOS, similar to `.gitignore` syntax: diff --git a/src/sandbox/macos-sandbox-utils.ts b/src/sandbox/macos-sandbox-utils.ts index 2cc6e5f9..9fc0839c 100644 --- a/src/sandbox/macos-sandbox-utils.ts +++ b/src/sandbox/macos-sandbox-utils.ts @@ -12,6 +12,7 @@ import { decodeSandboxedCommand, containsGlobChars, globToRegex, + denyGlobRegex, DANGEROUS_FILES, getDangerousDirectories, } from './sandbox-utils.js' @@ -137,20 +138,6 @@ function pathFilter(normalizedPath: string): string { : `(subpath ${escapePath(normalizedPath)})` } -/** - * Regex for a glob used in a DENY rule: {@link globToRegex} plus an optional - * `/…` tail, so the deny covers everything beneath each match the way - * `subpath` does for literals. Callers strip a trailing `/**` before the - * pattern gets here (removeTrailingGlobSuffix), so `**\/secrets/**` arrives - * as `**\/secrets` and, matched exactly, would deny only the directory - * vnode while `secrets/key` stayed readable. This is what the Linux backend - * already does (a deny masks the whole subtree). Only ever widens a deny. - */ -function denyGlobRegex(normalizedGlob: string): string { - // globToRegex() always returns '^…$'. - return globToRegex(normalizedGlob).slice(0, -1) + '(/.*)?$' -} - /** {@link pathFilter} for deny rules: globs get {@link denyGlobRegex}. */ function denyPathFilter(normalizedPath: string): string { return containsGlobChars(normalizedPath) diff --git a/src/sandbox/sandbox-manager.ts b/src/sandbox/sandbox-manager.ts index 3cf0d103..d1118493 100644 --- a/src/sandbox/sandbox-manager.ts +++ b/src/sandbox/sandbox-manager.ts @@ -670,6 +670,9 @@ async function initialize( // apply-seccomp's observer reports every write-intent syscall // (allowed or not). Only paths bwrap would actually refuse — outside // allowWrite or inside a denyWrite carve-out — go to the store. + // The defaults are listed in full: a home directory a wrap leaves + // out because it is read-denied sits under that deny's writable + // tmpfs, so a write there succeeds and is not a violation. allowWritePaths: [ ...getDefaultWritePaths(), ...config.filesystem.allowWrite, @@ -1150,6 +1153,30 @@ function getCredentialDenyReadPaths( return [...new Set(files.filter(f => f.mode === 'deny').map(f => f.path))] } +/** + * The default write paths under a filesystem policy's read rules, for + * getFsWriteConfig() and wrapWithSandbox() alike. Fed the entries as + * configured, through the pure {@link getCredentialDenyReadPaths}: no glob + * expansion and no credential masking, because getFsWriteConfig() is a + * getter callers reach from permission checks and render paths. A mask entry + * that degrades to a deny is a file and cannot cover a directory, so + * leaving those out changes nothing. + */ +function defaultWritePathsUnder({ + denyRead, + allowRead, + credentials, +}: { + denyRead: readonly string[] + allowRead: readonly string[] | undefined + credentials: CredentialsConfig | undefined +}): string[] { + return getDefaultWritePaths({ + denyRead: [...denyRead, ...getCredentialDenyReadPaths(credentials)], + allowRead, + }) +} + /** * Union the explicit `filesystem.denyRead` with credential-derived * deny paths. The single source of "what files does this config @@ -1246,8 +1273,14 @@ function getFsWriteConfig(): FsWriteRestrictionConfig { return true }) - // Build allowOnly list: default paths + configured allow paths - const allowOnly = [...getDefaultWritePaths(), ...allowPaths] + const allowOnly = [ + ...defaultWritePathsUnder({ + denyRead: config.filesystem.denyRead, + allowRead: config.filesystem.allowRead, + credentials: config.credentials, + }), + ...allowPaths, + ] return { allowOnly, @@ -1576,7 +1609,18 @@ async function wrapWithSandbox( [], ) writeConfig = { - allowOnly: [...getDefaultWritePaths(), ...userAllowWrite], + allowOnly: [ + ...defaultWritePathsUnder({ + denyRead: + customConfig?.filesystem?.denyRead ?? + config?.filesystem.denyRead ?? + [], + allowRead: + customConfig?.filesystem?.allowRead ?? config?.filesystem.allowRead, + credentials: customConfig?.credentials ?? config?.credentials, + }), + ...userAllowWrite, + ], denyWithinAllow: stripWriteGlobs( customConfig?.filesystem?.denyWrite ?? config?.filesystem.denyWrite ?? diff --git a/src/sandbox/sandbox-utils.ts b/src/sandbox/sandbox-utils.ts index f5d9bd8f..b44b0438 100644 --- a/src/sandbox/sandbox-utils.ts +++ b/src/sandbox/sandbox-utils.ts @@ -325,7 +325,6 @@ export function expandWindowsEnvRefs(p: string): string { * Returns the absolute path with symlinks resolved (or normalized glob pattern) */ export function normalizePathForSandbox(pathPattern: string): string { - const cwd = process.cwd() // Windows pre-processing: expand `%USERPROFILE%` / `%HOMEDRIVE%` / // `%HOMEPATH%`, strip the `\\?\` / `\\?\UNC\` extended prefix (its // `?` is a literal, not a glob char), and uppercase the drive @@ -366,10 +365,10 @@ export function normalizePathForSandbox(pathPattern: string): string { // tilde was expanded above } else if (pathPattern.startsWith('./') || pathPattern.startsWith('../')) { // Convert relative to absolute based on current working directory - normalizedPath = path.resolve(cwd, pathPattern) + normalizedPath = path.resolve(process.cwd(), pathPattern) } else if (!path.isAbsolute(pathPattern)) { // Handle other relative paths (e.g., ".", "..", "foo/bar") - normalizedPath = path.resolve(cwd, pathPattern) + normalizedPath = path.resolve(process.cwd(), pathPattern) } // For glob patterns, resolve symlinks for the directory portion only @@ -420,29 +419,122 @@ export function normalizePathForSandbox(pathPattern: string): string { return normalizedPath } +/** + * What the sandbox itself needs writable: the child's stdio and the TMPDIR it + * is handed (generateProxyEnvVars). Kept whatever is read-denied. + */ +const SANDBOX_OWN_WRITE_PATHS: readonly string[] = [ + '/dev/stdout', + '/dev/stderr', + '/dev/null', + '/dev/tty', + '/dev/dtracehelper', + '/dev/autofs_nowait', + '/tmp/claude', + '/private/tmp/claude', +] + +/** + * Directories under the home directory made writable as a convenience the + * caller never asked for. Every entry is subject to the read-rule check in + * {@link getDefaultWritePaths}. + */ +const HOME_CONVENIENCE_WRITE_DIRS: readonly string[] = [ + '.npm/_logs', + '.claude/debug', +] + /** * Get recommended system paths that should be writable for commands to work properly * * WARNING: These default paths are intentionally broad for compatibility but may * allow access to files from other processes. In highly security-sensitive * environments, you should configure more restrictive write paths. + * + * With no argument this is the whole list. Given the read rules of a + * filesystem policy, a home convenience directory (~/.npm/_logs, + * ~/.claude/debug) is left out when a `denyRead` entry names it or a + * directory above it: kept, it would be bound back over that deny on Linux + * (readable and writable again) and stay writable on macOS, so the explicit + * denyRead wins over the implicit write allow. It is kept when an `allowRead` + * entry beneath that deny re-opens it, because the caller has already made it + * readable. A caller who wants it writable regardless lists it in + * `allowWrite`. What the sandbox itself needs (stdio, /tmp/claude) is never + * left out. + * + * Pass the entries as configured, not expanded. `dir/**` counts as `dir`, + * and a glob covers a directory when it matches that directory or one above + * it; nothing is listed from disk. On Linux, where the backend expands globs + * against the disk, two things follow: a glob that matches nothing there + * still counts (which only ever drops a convenience path), and a glob whose + * match is a symlink to one of these directories is not seen. A glob + * `allowRead` entry is not counted as re-opening anything. */ -export function getDefaultWritePaths(): string[] { - const homeDir = homedir() - const recommendedPaths = [ - '/dev/stdout', - '/dev/stderr', - '/dev/null', - '/dev/tty', - '/dev/dtracehelper', - '/dev/autofs_nowait', - '/tmp/claude', - '/private/tmp/claude', - path.join(homeDir, '.npm/_logs'), - path.join(homeDir, '.claude/debug'), +export function getDefaultWritePaths(readRules?: { + denyRead: readonly string[] + allowRead?: readonly string[] +}): string[] { + const home = homedir() + const keptDirs = + !readRules || readRules.denyRead.length === 0 + ? HOME_CONVENIENCE_WRITE_DIRS + : homeDirsNotReadDenied(home, readRules.denyRead, readRules.allowRead) + return [ + ...SANDBOX_OWN_WRITE_PATHS, + ...keptDirs.map(rel => path.join(home, rel)), ] +} + +/** + * The {@link HOME_CONVENIENCE_WRITE_DIRS} no `denyRead` entry covers, or + * that an `allowRead` entry beneath the covering deny re-opens. + */ +function homeDirsNotReadDenied( + home: string, + denyRead: readonly string[], + allowRead: readonly string[] = [], +): readonly string[] { + // Rules are compared as normalizePathForSandbox spells them, and on macOS + // that resolves /tmp and /var to /private/... for a path that exists. A + // convenience directory may not exist yet, so its second spelling is built + // from the home directory, which does. + const homes = [...new Set([home, normalizePathForSandbox(home)])] + const denies = denyRead.map(entry => readRuleCovers(entry)) + const reopened = allowRead + .map(entry => removeTrailingGlobSuffix(entry)) + .filter(entry => !containsGlobCharsForPlatform(entry)) + .map(entry => normalizePathForSandbox(entry)) + return HOME_CONVENIENCE_WRITE_DIRS.filter(rel => { + const spellings = homes.map(h => path.join(h, rel)) + return !denies.some( + denyCovers => + spellings.some(denyCovers) && + !reopened.some( + allow => + denyCovers(allow) && spellings.some(s => isAtOrUnder(s, allow)), + ), + ) + }) +} - return recommendedPaths +/** + * Whether a read rule covers a path: the path is the rule's own or lies + * beneath it. `dir/**` is `dir`, and a glob covers whatever + * {@link denyGlobRegex} matches. + */ +function readRuleCovers(entry: string): (p: string) => boolean { + const stripped = removeTrailingGlobSuffix(entry) + const rule = normalizePathForSandbox(stripped) + if (containsGlobCharsForPlatform(stripped)) { + try { + const regex = new RegExp(denyGlobRegex(rule)) + return p => regex.test(p) + } catch { + // Brackets that do not form a valid class. The entry may be a literal + // file name, so it is compared as one. + } + } + return p => isAtOrUnder(p, rule) } /** @@ -876,6 +968,20 @@ export function globToRegex(globPattern: string): string { ) } +/** + * Regex for a glob used in a DENY rule: {@link globToRegex} plus an optional + * `/…` tail, so the deny covers everything beneath each match the way + * `subpath` does for literals. Callers strip a trailing `/**` before the + * pattern gets here (removeTrailingGlobSuffix), so `**\/secrets/**` arrives + * as `**\/secrets` and, matched exactly, would deny only the directory + * vnode while `secrets/key` stayed readable. This is what the Linux backend + * already does (a deny masks the whole subtree). Only ever widens a deny. + */ +export function denyGlobRegex(normalizedGlob: string): string { + // globToRegex() always returns '^…$'. + return globToRegex(normalizedGlob).slice(0, -1) + '(/.*)?$' +} + export interface ExpandGlobOptions { /** * Match case-insensitively. Set this on Windows where the diff --git a/test/sandbox/default-write-paths.test.ts b/test/sandbox/default-write-paths.test.ts new file mode 100644 index 00000000..ca4c0f28 --- /dev/null +++ b/test/sandbox/default-write-paths.test.ts @@ -0,0 +1,275 @@ +import { describe, it, expect, afterAll, beforeAll } from 'bun:test' +import { spawnSync } from 'node:child_process' +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import { homedir, tmpdir } from 'node:os' +import { join } from 'node:path' +import { SandboxManager } from '../../src/sandbox/sandbox-manager.js' +import { + getDefaultWritePaths, + isAtOrUnder, +} from '../../src/sandbox/sandbox-utils.js' +import { isLinux, isMacOS, isWindows } from '../helpers/platform.js' + +const REPO_ROOT = join(import.meta.dir, '../..') +const NPM_LOGS = '.npm/_logs' +const CLAUDE_DEBUG = '.claude/debug' +const SANDBOX_OWN = [ + '/dev/stdout', + '/dev/stderr', + '/dev/null', + '/dev/tty', + '/dev/dtracehelper', + '/dev/autofs_nowait', + '/tmp/claude', + '/private/tmp/claude', +] + +/** + * The two home directories among the default write paths are conveniences + * the caller never asked for; one that a read-deny covers must not be bound + * back over that deny. What the sandbox itself needs is never dropped. + */ +describe.if(!isWindows)('getDefaultWritePaths', () => { + const home = homedir() + const npmLogs = join(home, NPM_LOGS) + const claudeDebug = join(home, CLAUDE_DEBUG) + + it('is the whole list when given no read rules', () => { + expect(getDefaultWritePaths()).toEqual([ + ...SANDBOX_OWN, + npmLogs, + claudeDebug, + ]) + expect(getDefaultWritePaths({ denyRead: [] })).toEqual( + getDefaultWritePaths(), + ) + }) + + it.each([home, '~/', '~/**', join(home, '**'), '/', '/**'])( + 'drops both home conveniences, and nothing else, under denyRead %s', + deny => { + expect(getDefaultWritePaths({ denyRead: [deny] })).toEqual(SANDBOX_OWN) + }, + ) + + it.each(['/tmp', '/private/tmp', '/dev'])( + 'drops nothing under denyRead %s', + deny => { + expect(getDefaultWritePaths({ denyRead: [deny] })).toEqual( + getDefaultWritePaths(), + ) + }, + ) + + it('leaves nothing under a read-denied home', () => { + expect( + getDefaultWritePaths({ denyRead: ['~'] }).filter(p => + isAtOrUnder(p, home), + ), + ).toEqual([]) + }) + + it('drops only the convenience a deny names', () => { + expect(getDefaultWritePaths({ denyRead: [npmLogs] })).toEqual([ + ...SANDBOX_OWN, + claudeDebug, + ]) + }) + + it('drops a convenience under a directory a glob matches', () => { + expect(getDefaultWritePaths({ denyRead: [join(home, '.n*')] })).toEqual([ + ...SANDBOX_OWN, + claudeDebug, + ]) + }) + + it('keeps a convenience beside, not beneath, what is read-denied', () => { + expect( + getDefaultWritePaths({ + denyRead: [ + join(home, '.npmrc'), + join(home, '.np'), + join(home, '**/*.log'), + ], + }), + ).toEqual(getDefaultWritePaths()) + }) + + it('compares an entry whose brackets are no valid class as a literal', () => { + expect( + getDefaultWritePaths({ denyRead: [join(home, 'backup[2024-01-15]')] }), + ).toEqual(getDefaultWritePaths()) + }) + + it.each([ + { + name: 'an allowRead above the directory, beneath the deny', + denyRead: ['~'], + allowRead: ['~/.claude'], + kept: [claudeDebug], + }, + { + name: 'an allowRead of the directory itself', + denyRead: ['~/.npm'], + allowRead: ['~/.npm/_logs/**'], + kept: [npmLogs, claudeDebug], + }, + { + name: 'no allowRead above the deny', + denyRead: ['~/.claude'], + allowRead: ['~'], + kept: [npmLogs], + }, + { + name: 'no glob allowRead', + denyRead: ['~'], + allowRead: ['~/.c*'], + kept: [], + }, + ])('counts $name as re-opening it', ({ denyRead, allowRead, kept }) => { + expect(getDefaultWritePaths({ denyRead, allowRead })).toEqual([ + ...SANDBOX_OWN, + ...kept, + ]) + }) +}) + +/** + * Runs `script` (an ES module body) under bun with HOME set to `fakeHome` + * and returns the JSON it prints last. os.homedir() does not follow a HOME + * changed at runtime, so these cases need a process of their own. + */ +function runWithHome(fakeHome: string, script: string): unknown { + const result = spawnSync(process.execPath, ['-e', script], { + cwd: REPO_ROOT, + env: { ...process.env, HOME: fakeHome }, + encoding: 'utf8', + timeout: 60_000, + }) + if (result.status !== 0) { + throw new Error(`exit ${result.status}: ${result.stderr}`) + } + return JSON.parse(result.stdout.trim().split('\n').at(-1) ?? '') +} + +describe.if(isLinux || isMacOS)( + 'default write paths under another HOME', + () => { + let fakeHome: string + + beforeAll(() => { + // On macOS tmpdir() is under /var, which normalizes to /private/var. + fakeHome = mkdtempSync(join(tmpdir(), 'srt-home-')) + mkdirSync(join(fakeHome, '.npm')) + mkdirSync(join(fakeHome, CLAUDE_DEBUG), { recursive: true }) + }) + + afterAll(() => { + rmSync(fakeHome, { recursive: true, force: true }) + }) + + it('drops a convenience directory that does not exist yet', () => { + const paths = runWithHome( + fakeHome, + `const { getDefaultWritePaths } = await import('./src/sandbox/sandbox-utils.ts') + console.log(JSON.stringify(getDefaultWritePaths({ denyRead: ['~/.npm'] })))`, + ) + expect(paths).toEqual([...SANDBOX_OWN, join(fakeHome, CLAUDE_DEBUG)]) + }) + + /** Whether the wrapped command makes `dir` writable. */ + function grantsWrite(wrapped: string, dir: string): boolean { + if (isLinux) return wrapped.includes(`--bind ${dir} ${dir}`) + const rule = wrapped.slice(wrapped.indexOf('(allow file-write*')) + return rule.slice(0, rule.indexOf('(with message')).includes(dir) + } + + it('reaches getFsWriteConfig() and the wrapped command', () => { + mkdirSync(join(fakeHome, NPM_LOGS), { recursive: true }) + const { allowOnly, wrapped, reopened } = runWithHome( + fakeHome, + `const { SandboxManager } = await import('./src/sandbox/sandbox-manager.ts') + const filesystem = { denyRead: ['~/.npm'], allowWrite: [], denyWrite: [] } + await SandboxManager.initialize({ + network: { allowedDomains: [], deniedDomains: [] }, + filesystem, + credentials: { files: [{ path: '~/.claude', mode: 'deny' }] }, + }) + const out = { + allowOnly: SandboxManager.getFsWriteConfig().allowOnly, + wrapped: await SandboxManager.wrapWithSandbox('true'), + reopened: await SandboxManager.wrapWithSandbox('true', undefined, { + filesystem: { ...filesystem, allowRead: ['~/.npm/_logs'] }, + }), + } + await SandboxManager.reset() + process.stdout.write(JSON.stringify(out) + '\\n', () => process.exit(0))`, + ) as { allowOnly: string[]; wrapped: string; reopened: string } + + // A filesystem deny and a credential file deny each drop theirs. + expect(allowOnly).toEqual(SANDBOX_OWN) + expect(grantsWrite(wrapped, join(fakeHome, NPM_LOGS))).toBe(false) + expect(grantsWrite(wrapped, join(fakeHome, CLAUDE_DEBUG))).toBe(false) + // A per-call allowRead of the directory keeps its write allow. + expect(grantsWrite(reopened, join(fakeHome, NPM_LOGS))).toBe(true) + }) + }, +) + +describe.if(isLinux)('getFsWriteConfig() stays a plain getter', () => { + let dir: string + + beforeAll(() => { + dir = mkdtempSync(join(tmpdir(), 'srt-write-getter-')) + writeFileSync(join(dir, 'a.env'), 'A=1') + writeFileSync(join(dir, 'backup[2024-01-15].env'), 'B=2') + writeFileSync(join(dir, 'hosts.yml'), 'k: v\n') + }) + + afterAll(async () => { + await SandboxManager.reset() + rmSync(dir, { recursive: true, force: true }) + }) + + it('is untouched by what a denyRead glob expands to, or by a credential mask', async () => { + await SandboxManager.reset() + await SandboxManager.initialize({ + network: { allowedDomains: [], deniedDomains: [] }, + filesystem: { + denyRead: [join(dir, '**/*.env')], + allowWrite: [], + denyWrite: [], + }, + credentials: { + files: [ + { + path: join(dir, 'hosts.yml'), + mode: 'mask', + extract: 'nope: (\\S+)', + onExtractNoMatch: 'error', + }, + ], + }, + }) + + expect(SandboxManager.getFsWriteConfig().allowOnly).toEqual( + getDefaultWritePaths(), + ) + }) + + it('wraps when a denyRead glob matches a file name with brackets', async () => { + await SandboxManager.reset() + await SandboxManager.initialize({ + network: { allowedDomains: [], deniedDomains: [] }, + filesystem: { + denyRead: [join(dir, '**/*.env')], + allowWrite: [], + denyWrite: [], + }, + }) + + expect(await SandboxManager.wrapWithSandbox('true')).toContain( + join(dir, 'backup[2024-01-15].env'), + ) + }) +})