Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 16 additions & 10 deletions src/sandbox/sandbox-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -672,7 +672,7 @@ async function initialize(
// (allowed or not). Only paths bwrap would actually refuse — outside
// allowWrite or inside a denyWrite carve-out — go to the store.
allowWritePaths: [
...getDefaultWritePaths(),
...getDefaultWritePaths(config.filesystem.denyRead),
...config.filesystem.allowWrite,
],
denyWritePaths: config.filesystem.denyWrite,
Expand Down Expand Up @@ -1247,8 +1247,12 @@ function getFsWriteConfig(): FsWriteRestrictionConfig {
return true
})

// Build allowOnly list: default paths + configured allow paths
const allowOnly = [...getDefaultWritePaths(), ...allowPaths]
// Build allowOnly list: default paths (less any the config read-denies)
// + configured allow paths
const allowOnly = [
...getDefaultWritePaths(config.filesystem.denyRead),
...allowPaths,
]

return {
allowOnly,
Expand Down Expand Up @@ -1576,21 +1580,23 @@ async function wrapWithSandbox(
config?.filesystem.allowWrite ??
[],
)
// Credential deny paths are unioned with the caller's denyRead — never
// replacing it — so explicit filesystem restrictions always survive.
// Computed ahead of the write config: a default write path under a
// read-denied directory is dropped from it.
const rawDenyRead = unionDenyReadPaths(
customConfig?.filesystem?.denyRead ?? config?.filesystem.denyRead ?? [],
credentialRestrictions,
)
writeConfig = {
allowOnly: [...getDefaultWritePaths(), ...userAllowWrite],
allowOnly: [...getDefaultWritePaths(rawDenyRead), ...userAllowWrite],
denyWithinAllow: stripWriteGlobs(
customConfig?.filesystem?.denyWrite ??
config?.filesystem.denyWrite ??
[],
),
}

// Credential deny paths are unioned with the caller's denyRead — never
// replacing it — so explicit filesystem restrictions always survive.
const rawDenyRead = unionDenyReadPaths(
customConfig?.filesystem?.denyRead ?? config?.filesystem.denyRead ?? [],
credentialRestrictions,
)
const expandedDenyRead: string[] = []
for (const p of rawDenyRead) {
const stripped = removeTrailingGlobSuffix(p)
Expand Down
25 changes: 22 additions & 3 deletions src/sandbox/sandbox-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -414,7 +414,9 @@ export function normalizePathForSandbox(pathPattern: string): string {
* allow access to files from other processes. In highly security-sensitive
* environments, you should configure more restrictive write paths.
*/
export function getDefaultWritePaths(): string[] {
export function getDefaultWritePaths(
denyRead: readonly string[] = [],
): string[] {
const homeDir = homedir()
const recommendedPaths = [
'/dev/stdout',
Expand All @@ -428,8 +430,25 @@ export function getDefaultWritePaths(): string[] {
path.join(homeDir, '.npm/_logs'),
path.join(homeDir, '.claude/debug'),
]

return recommendedPaths
if (denyRead.length === 0) return recommendedPaths

// These are conveniences the caller never asked for. One that lies at or
// under a directory the caller read-denies would be bound back over that
// deny on Linux (readable and writable again) and writable on macOS, so
// an explicit denyRead wins over an implicit allowWrite. A caller who
// wants the path writable lists it in allowWrite.
const denied = denyRead
.filter(p => !containsGlobChars(removeTrailingGlobSuffix(p)))
.map(p => normalizePathForSandbox(p))
const atOrUnder = (p: string, dir: string): boolean =>
p === dir || p.startsWith(dir === '/' ? '/' : dir + '/')
// Both spellings of the recommended path: as listed, and as the sandbox
// normalizes it (symlinks such as /tmp -> /private/tmp resolved).
return recommendedPaths.filter(recommended =>
[recommended, normalizePathForSandbox(recommended)].every(
form => !denied.some(d => atOrUnder(form, d)),
),
)
}

/**
Expand Down
48 changes: 48 additions & 0 deletions test/sandbox/default-write-paths.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
import { describe, it, expect } from 'bun:test'
import { homedir } from 'node:os'
import { join } from 'node:path'
import { getDefaultWritePaths } from '../../src/sandbox/sandbox-utils.js'

/**
* The default write paths are conveniences the caller never asked for; one
* that lies at or under a directory the caller read-denies must not be bound
* back over that deny.
*/
describe('getDefaultWritePaths', () => {
const npmLogs = join(homedir(), '.npm/_logs')
const claudeDebug = join(homedir(), '.claude/debug')

it('includes the home conveniences by default', () => {
expect(getDefaultWritePaths()).toContain(npmLogs)
expect(getDefaultWritePaths([])).toContain(claudeDebug)
})

it('drops a convenience under a read-denied directory', () => {
const underDeniedHome = getDefaultWritePaths([homedir()])
expect(underDeniedHome).not.toContain(npmLogs)
expect(underDeniedHome).not.toContain(claudeDebug)
expect(underDeniedHome).toContain('/dev/null')

// Tilde and trailing-slash spellings name the same directory.
expect(getDefaultWritePaths(['~/'])).not.toContain(npmLogs)
// The denied directory itself, exactly.
expect(getDefaultWritePaths([join(homedir(), '.npm', '_logs')])).toEqual(
getDefaultWritePaths().filter(p => p !== npmLogs),
)
})

it('keeps a convenience beside, not beneath, a read-denied directory', () => {
const kept = getDefaultWritePaths([
join(homedir(), '.npmrc'),
join(homedir(), '.np'),
])
expect(kept).toContain(npmLogs)
expect(kept).toContain(claudeDebug)
})

it('ignores glob entries, which name no directory to compare against', () => {
expect(getDefaultWritePaths([join(homedir(), '**/*.log')])).toContain(
npmLogs,
)
})
})
Loading