lywsvip/sandbox-runtime
1
0
Fork 0
mirror of https://github.com/anthropic-experimental/sandbox-runtime.git synced 2026-05-06 13:40:59 +08:00
Code Issues Packages Projects Releases Wiki Activity

fix: allow filesystem root traversal when denyRead includes '/' (#190)

Browse Source 
* Fix allowRead carve-outs when denyRead covers filesystem root

denyRead: ['/'] + allowRead: [<project>] denied everything on both
platforms (follow-up to #166, closes #10).

macOS: (deny file-read* (subpath "/")) blocks the root inode itself; no
allowWithinDeny subpath covers "/", so dyld SIGABRTs before exec. Emit
(allow file-read* (literal "/")) so path traversal through root works.
Exposes `ls /` dirent names but no subtree contents.

Linux: two issues. --tmpfs / wipes every prior mount (ro-bind /, write
binds, denyWrite ro-binds), and the carve-out prefix check
startsWith('/' + '/') never matches. Expand a root deny into its
children (minus /proc, /dev, /sys) so the existing per-dir tmpfs +
re-bind logic applies. Also re-bind any allowWrite paths that land
under a tmpfs'd deny dir (previously they went read-only), and buffer
denyWrite ro-binds until after denyRead processing so .git/hooks
protection survives a tmpfs over an ancestor.

* Dedup denyWrite entries post-normalization to prevent bwrap failure

Two denyWrite entries that converge to the same path after
normalizePathForSandbox() produced a duplicate
--ro-bind /dev/null <dest>. On the second bind, <dest> is a char device
(the first bind's mount); bwrap's ensure_file() only short-circuits on
S_ISREG, so it falls through to creat() and fails on the now-read-only
mount. Every sandboxed command errors out.

Common trigger: same path specified in two config surfaces that feed
into denyWrite, e.g. a file-edit permission deny and a
sandbox.filesystem.denyWrite entry for the same file.

linuxGetMandatoryDenyPaths() already does [...new Set(denyPaths)] but
that's pre-normalization — ~/.foo and the expanded absolute path differ
as strings there.

* Wire allow-read and wrap-with-sandbox tests into CI

test:integration only listed integration.test.ts explicitly; these two
files were never run despite containing sandbox-exec/bwrap integration
tests. The root-deny and dedup tests added in this PR need CI coverage.

* Fix Linux test assumptions: seccomp binary path, ro-bind src+dest count

Root deny hides the repo's vendor/seccomp/ dir, so apply-seccomp
can't load inside the sandbox and bash returns 127. Bypass seccomp
with allowAllUnixSockets: true — socket blocking is orthogonal here.

Dedup assertion was off by 2x: --ro-bind <p> <p> contains <p> twice.
The fix works (was 4 occurrences, now 2).

* Narrow allowRead skip check to writes actually re-bound under this tmpfs

The skip check introduced in 3cdb468 was too broad: it skipped any
allowPath under any allowWrite, not just writes that were re-bound in
the current tmpfs iteration. With allowWrite as an ancestor of denyRead
(e.g. allowWrite: [~], denyRead: [~/.ssh], allowRead: [~/.ssh/known_hosts])
the write path isn't wiped and isn't re-bound, but the skip check still
matched — known_hosts was left sitting in the empty tmpfs.

Narrow the predicate: only skip if the write path itself is under the
tmpfs'd deny dir (i.e. it was re-bound just above).
This commit is contained in:
Alice T'Poteat
2026-03-30 15:42:43 -07:00
committed by GitHub
parent fd74a3f012
commit 41a57927c7
5 changed files with 344 additions and 24 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
package.json
View File

@@ -18,7 +18,7 @@
"clean": "rm -rf dist",
"test": "bun test",
"test:unit": "bun test test/config-validation.test.ts test/sandbox/seccomp-filter.test.ts",
"test:integration": "bun test test/sandbox/integration.test.ts",
"test:integration": "bun test test/sandbox/integration.test.ts test/sandbox/allow-read.test.ts test/sandbox/wrap-with-sandbox.test.ts",
"typecheck": "tsc --noEmit",
"lint": "eslint 'src/**/*.ts' --fix --cache --cache-location=node_modules/.cache/.eslintcache",
"lint:check": "eslint 'src/**/*.ts' --cache --cache-location=node_modules/.cache/.eslintcache",

78
src/sandbox/linux-sandbox-utils.ts
View File

@@ -644,14 +644,18 @@ async function generateFilesystemArgs(
const args: string[] = []
// fs already imported
// Collect normalized allowed write paths. Populated in the writeConfig
// block, read again in the denyRead loop to re-bind writes under tmpfs.
const allowedWritePaths: string[] = []
// denyWrite binds are buffered and emitted after denyRead processing so that
// a denyRead tmpfs over an ancestor directory doesn't wipe them out.
const denyWriteArgs: string[] = []
// Determine initial root mount based on write restrictions
if (writeConfig) {
// Write restrictions: Start with read-only root, then allow writes to specific paths
args.push('--ro-bind', '/', '/')
// Collect normalized allowed write paths for later checking
const allowedWritePaths: string[] = []
// Allow writes to specific paths
for (const pathPattern of writeConfig.allowOnly || []) {
const normalizedPath = normalizePathForSandbox(pathPattern)
@@ -714,8 +718,15 @@ async function generateFilesystemArgs(
)),
]
// Dedup post-normalization: entries like ['~/.foo', '/home/user/.foo']
// converge to the same path here. A duplicate --ro-bind /dev/null <dest>
// hits a char device on the second pass and bwrap's ensure_file() falls
// through to creat() on a read-only mount.
const seenDenyWrite = new Set<string>()
for (const pathPattern of denyPaths) {
const normalizedPath = normalizePathForSandbox(pathPattern)
if (seenDenyWrite.has(normalizedPath)) continue
seenDenyWrite.add(normalizedPath)
// Skip /dev/* paths since --dev /dev already handles them
if (normalizedPath.startsWith('/dev/')) {
@@ -728,7 +739,7 @@ async function generateFilesystemArgs(
// symlink and creates real .claude/settings.json with malicious hooks.
const symlinkInPath = findSymlinkInPath(normalizedPath, allowedWritePaths)
if (symlinkInPath) {
args.push('--ro-bind', '/dev/null', symlinkInPath)
denyWriteArgs.push('--ro-bind', '/dev/null', symlinkInPath)
logForDebugging(
`[Sandbox Linux] Mounted /dev/null at symlink ${symlinkInPath} to prevent symlink replacement attack`,
)
@@ -780,14 +791,14 @@ async function generateFilesystemArgs(
const emptyDir = fs.mkdtempSync(
path.join(tmpdir(), 'claude-empty-'),
)
args.push('--ro-bind', emptyDir, firstNonExistent)
denyWriteArgs.push('--ro-bind', emptyDir, firstNonExistent)
bwrapMountPoints.add(firstNonExistent)
registerExitCleanupHandler()
logForDebugging(
`[Sandbox Linux] Mounted empty dir at ${firstNonExistent} to block creation of ${normalizedPath}`,
)
} else {
args.push('--ro-bind', '/dev/null', firstNonExistent)
denyWriteArgs.push('--ro-bind', '/dev/null', firstNonExistent)
bwrapMountPoints.add(firstNonExistent)
registerExitCleanupHandler()
logForDebugging(
@@ -811,7 +822,7 @@ async function generateFilesystemArgs(
)
if (isWithinAllowedPath) {
args.push('--ro-bind', normalizedPath, normalizedPath)
denyWriteArgs.push('--ro-bind', normalizedPath, normalizedPath)
} else {
logForDebugging(
`[Sandbox Linux] Skipping deny path not within allowed paths: ${normalizedPath}`,
@@ -822,13 +833,30 @@ async function generateFilesystemArgs(
// No write restrictions: Allow all writes
args.push('--bind', '/', '/')
}
// denyWriteArgs is emitted after the denyRead loop below.
// Handle read restrictions by mounting tmpfs over denied paths
const readDenyPaths = [...(readConfig?.denyOnly || [])]
const readDenyPaths: string[] = []
const readAllowPaths = (readConfig?.allowWithinDeny || []).map(p =>
normalizePathForSandbox(p),
)
// --tmpfs / would wipe all prior mounts (ro-bind /, write binds, deny binds).
// Expand a root deny into its direct children so the existing per-dir tmpfs
// + re-bind logic applies. Skip /proc and /dev: they're remounted by the
// caller after this function returns. Skip /sys: kernel interface, tmpfs
// over it breaks tooling and the host /sys is already read-only via ro-bind.
const rootSkip = new Set(['proc', 'dev', 'sys'])
for (const p of readConfig?.denyOnly || []) {
if (normalizePathForSandbox(p) === '/') {
for (const child of fs.readdirSync('/')) {
if (!rootSkip.has(child)) readDenyPaths.push('/' + child)
}
} else {
readDenyPaths.push(p)
}
}
// Always hide /etc/ssh/ssh_config.d to avoid permission issues with OrbStack
// SSH is very strict about config file permissions and ownership, and they can
// appear wrong inside the sandbox causing "Bad owner or permissions" errors
@@ -845,24 +873,45 @@ async function generateFilesystemArgs(
continue
}
const denySep = normalizedPath === '/' ? '/' : normalizedPath + '/'
const readDenyStat = fs.statSync(normalizedPath)
if (readDenyStat.isDirectory()) {
args.push('--tmpfs', normalizedPath)
// tmpfs wiped any earlier write binds under this path — restore them.
for (const writePath of allowedWritePaths) {
if (writePath.startsWith(denySep) || writePath === normalizedPath) {
args.push('--bind', writePath, writePath)
logForDebugging(
`[Sandbox Linux] Re-bound write path wiped by denyRead tmpfs: ${writePath}`,
)
}
}
// Re-allow specific paths within the denied directory (allowRead overrides denyRead).
// After mounting tmpfs over the denied dir, bind back the allowed subdirectories
// so they are readable again.
for (const allowPath of readAllowPaths) {
if (
allowPath.startsWith(normalizedPath + '/') ||
allowPath === normalizedPath
) {
if (allowPath.startsWith(denySep) || allowPath === normalizedPath) {
if (!fs.existsSync(allowPath)) {
logForDebugging(
`[Sandbox Linux] Skipping non-existent read allow path: ${allowPath}`,
)
continue
}
// Skip only if a write path was re-bound just above AND covers
// allowPath. A write path that's an ancestor of the deny dir isn't
// re-bound (it wasn't wiped), so allowPath under it still needs
// its own ro-bind here.
if (
allowedWritePaths.some(
w =>
(w.startsWith(denySep) || w === normalizedPath) &&
(allowPath === w || allowPath.startsWith(w + '/')),
)
) {
continue
}
// Bind the allowed path back over the tmpfs so it's readable
args.push('--ro-bind', allowPath, allowPath)
logForDebugging(
@@ -888,6 +937,11 @@ async function generateFilesystemArgs(
}
}
// Emitting denyWrite last means these ro-binds layer on top of any write
// paths the denyRead loop just re-bound. Before this ordering, tmpfs over
// an ancestor of cwd would wipe the .git/hooks protection.
args.push(...denyWriteArgs)
return args
}

10
src/sandbox/macos-sandbox-utils.ts
View File

@@ -213,6 +213,7 @@ function generateReadRules(
}
const rules: string[] = []
let deniesRoot = false
// Start by allowing everything
rules.push(`(allow file-read*)`)
@@ -221,6 +222,8 @@ function generateReadRules(
for (const pathPattern of config.denyOnly || []) {
const normalizedPath = normalizePathForSandbox(pathPattern)
if (normalizedPath === '/') deniesRoot = true
if (containsGlobChars(normalizedPath)) {
// Use regex matching for glob patterns
const regexPattern = globToRegex(normalizedPath)
@@ -239,6 +242,13 @@ function generateReadRules(
}
}
// (subpath "/") denies the root inode itself; allowWithinDeny subpaths don't
// cover "/", so dyld aborts before exec. Re-allow the literal root so path
// traversal works. This exposes `ls /` dirent names but no subtree contents.
if (deniesRoot) {
rules.push(`(allow file-read* (literal "/"))`)
}
// Re-allow specific paths within denied regions (allowWithinDeny takes precedence)
for (const pathPattern of config.allowWithinDeny || []) {
const normalizedPath = normalizePathForSandbox(pathPattern)

238
test/sandbox/allow-read.test.ts
View File

@@ -1,19 +1,12 @@
import { describe, it, expect, beforeAll, afterAll } from 'bun:test'
import { spawnSync } from 'node:child_process'
import {
existsSync,
mkdirSync,
rmSync,
writeFileSync,
} from 'node:fs'
import { tmpdir } from 'node:os'
import { existsSync, mkdirSync, rmSync, writeFileSync } from 'node:fs'
import { homedir, tmpdir } from 'node:os'
import { join } from 'node:path'
import { getPlatform } from '../../src/utils/platform.js'
import { wrapCommandWithSandboxMacOS } from '../../src/sandbox/macos-sandbox-utils.js'
import { wrapCommandWithSandboxLinux } from '../../src/sandbox/linux-sandbox-utils.js'
import type {
FsReadRestrictionConfig,
} from '../../src/sandbox/sandbox-schemas.js'
import type { FsReadRestrictionConfig } from '../../src/sandbox/sandbox-schemas.js'
function skipIfNotMacOS(): boolean {
return getPlatform() !== 'macos'
@@ -219,6 +212,231 @@ describe('allowRead precedence over denyRead', () => {
expect(result.status).not.toBe(0)
expect(result.stdout).not.toContain(TEST_SECRET_CONTENT)
})
// Regression: the write-path skip check in the allowRead re-bind loop was
// too broad — it skipped any allowPath under ANY allowWrite, not just
// writes actually re-bound under this tmpfs. With allowWrite as an
// ancestor of denyRead (not wiped, not re-bound), allowRead under it was
// skipped and left sitting in the empty tmpfs.
// Shape: allowWrite: [~], denyRead: [~/.ssh], allowRead: [~/.ssh/known_hosts].
it('should re-allow under denyRead when allowWrite is an ancestor of the deny', async () => {
if (skipIfNotLinux()) {
return
}
const wrappedCommand = await wrapCommandWithSandboxLinux({
command: `cat ${TEST_ALLOWED_FILE}`,
needsNetworkRestriction: false,
readConfig: {
denyOnly: [TEST_DENIED_DIR],
allowWithinDeny: [TEST_ALLOWED_SUBDIR],
},
writeConfig: {
allowOnly: [TEST_BASE_DIR], // ancestor of denyRead
denyWithinAllow: [],
},
})
const result = spawnSync(wrappedCommand, {
shell: true,
encoding: 'utf8',
timeout: 5000,
})
expect(result.status).toBe(0)
expect(result.stdout).toContain(TEST_ALLOWED_CONTENT)
})
})
})
/**
* Regression: denyRead: ['/'] + allowRead: [<project>] used to deny everything.
*
* macOS: (subpath "/") denies the root inode; no allowWithinDeny subpath covers
* "/", so dyld SIGABRTs before exec. Fix emits (allow file-read* (literal "/")).
* Linux: --tmpfs / wiped all prior mounts, and the carve-out prefix check
* startsWith('/' + '/') never matched. Fix expands '/' into its children.
*
* Test dir lives under $HOME (not tmpdir) so the macOS /tmp → /private/tmp
* symlink doesn't confuse Seatbelt path matching.
*/
describe('allowRead carve-out with denyRead at filesystem root (issue #10)', () => {
const TEST_DIR = join(
homedir(),
'.sandbox-runtime-test-root-deny-' + Date.now(),
)
const TEST_FILE = join(TEST_DIR, 'visible.txt')
const TEST_CONTENT = 'ROOT_CARVE_OUT'
// Paths needed for sh/cat to load at all when the whole filesystem is denied.
// /private covers /tmp and /var (macOS symlinks). /lib* for Linux ld.so.
const EXEC_DEPS = [
'/bin',
'/usr',
'/lib',
'/lib64',
'/System',
'/private',
'/dev',
'/etc',
]
beforeAll(() => {
if (getPlatform() !== 'macos' && getPlatform() !== 'linux') {
return
}
mkdirSync(TEST_DIR, { recursive: true })
writeFileSync(TEST_FILE, TEST_CONTENT)
})
afterAll(() => {
if (existsSync(TEST_DIR)) {
rmSync(TEST_DIR, { recursive: true, force: true })
}
})
it('macOS: re-allows carve-out under a root-level deny', () => {
if (skipIfNotMacOS()) {
return
}
const readConfig: FsReadRestrictionConfig = {
denyOnly: ['/'],
allowWithinDeny: [TEST_DIR, ...EXEC_DEPS],
}
const wrappedCommand = wrapCommandWithSandboxMacOS({
command: `cat ${TEST_FILE}`,
needsNetworkRestriction: false,
readConfig,
writeConfig: undefined,
})
const result = spawnSync(wrappedCommand, {
shell: true,
encoding: 'utf8',
timeout: 5000,
})
expect(result.status).toBe(0)
expect(result.stdout).toContain(TEST_CONTENT)
})
it('macOS: still denies paths outside the carve-out under a root-level deny', () => {
if (skipIfNotMacOS()) {
return
}
const outside = join(homedir(), '.bashrc')
const readConfig: FsReadRestrictionConfig = {
denyOnly: ['/'],
allowWithinDeny: [TEST_DIR, ...EXEC_DEPS],
}
const wrappedCommand = wrapCommandWithSandboxMacOS({
command: `cat ${outside} 2>/dev/null; true`,
needsNetworkRestriction: false,
readConfig,
writeConfig: undefined,
})
const result = spawnSync(wrappedCommand, {
shell: true,
encoding: 'utf8',
timeout: 5000,
})
// Process must exec (no SIGABRT) and stdout must be empty (cat denied)
expect(result.status).toBe(0)
expect(result.stdout).toBe('')
})
it('Linux: re-allows carve-out under a root-level deny', async () => {
if (skipIfNotLinux()) {
return
}
const readConfig: FsReadRestrictionConfig = {
denyOnly: ['/'],
allowWithinDeny: [TEST_DIR, ...EXEC_DEPS],
}
// allowAllUnixSockets: true bypasses the seccomp path — otherwise the
// apply-seccomp binary under <repo>/vendor/ is hidden by the root deny.
const wrappedCommand = await wrapCommandWithSandboxLinux({
command: `cat ${TEST_FILE}`,
needsNetworkRestriction: false,
readConfig,
writeConfig: undefined,
allowAllUnixSockets: true,
})
const result = spawnSync(wrappedCommand, {
shell: true,
encoding: 'utf8',
timeout: 5000,
})
expect(result.status).toBe(0)
expect(result.stdout).toContain(TEST_CONTENT)
})
it('Linux: still denies paths outside the carve-out under a root-level deny', async () => {
if (skipIfNotLinux()) {
return
}
const outside = join(homedir(), '.bashrc')
const readConfig: FsReadRestrictionConfig = {
denyOnly: ['/'],
allowWithinDeny: [TEST_DIR, ...EXEC_DEPS],
}
const wrappedCommand = await wrapCommandWithSandboxLinux({
command: `cat ${outside} 2>/dev/null; true`,
needsNetworkRestriction: false,
readConfig,
writeConfig: undefined,
allowAllUnixSockets: true,
})
const result = spawnSync(wrappedCommand, {
shell: true,
encoding: 'utf8',
timeout: 5000,
})
expect(result.status).toBe(0)
expect(result.stdout).toBe('')
})
it('Linux: preserves write binds when denyRead ancestor wipes them', async () => {
if (skipIfNotLinux()) {
return
}
const writeTarget = join(TEST_DIR, 'written.txt')
const wrappedCommand = await wrapCommandWithSandboxLinux({
command: `echo WRITE_OK > ${writeTarget} && cat ${writeTarget}`,
needsNetworkRestriction: false,
readConfig: {
denyOnly: ['/'],
allowWithinDeny: [...EXEC_DEPS],
},
writeConfig: {
allowOnly: [TEST_DIR],
denyWithinAllow: [],
},
allowAllUnixSockets: true,
})
const result = spawnSync(wrappedCommand, {
shell: true,
encoding: 'utf8',
timeout: 5000,
})
expect(result.status).toBe(0)
expect(result.stdout).toContain('WRITE_OK')
})
})

40
test/sandbox/wrap-with-sandbox.test.ts
View File

@@ -4,7 +4,7 @@ import type { SandboxRuntimeConfig } from '../../src/sandbox/sandbox-config.js'
import { getPlatform } from '../../src/utils/platform.js'
import { wrapCommandWithSandboxLinux } from '../../src/sandbox/linux-sandbox-utils.js'
import { wrapCommandWithSandboxMacOS } from '../../src/sandbox/macos-sandbox-utils.js'
import { mkdirSync, rmSync } from 'node:fs'
import { mkdirSync, rmSync, writeFileSync } from 'node:fs'
import { tmpdir } from 'node:os'
import { join } from 'node:path'
@@ -717,4 +717,42 @@ describe('allowWrite glob suffix handling', () => {
rmSync(parentDir, { recursive: true, force: true })
}
})
// Regression: two denyWrite entries that converge to the same path after
// normalizePathForSandbox() produced a duplicate --ro-bind /dev/null <dest>.
// Second bind finds a char device at <dest>; bwrap's ensure_file() doesn't
// short-circuit on S_ISCHR and falls through to creat() on a read-only mount.
it('dedups denyWrite entries that normalize to the same path (Linux)', async () => {
if (getPlatform() !== 'linux') {
return
}
const parentDir = join(tmpdir(), `srt-test-dup-deny-${Date.now()}`)
const childFile = join(parentDir, 'dup.txt')
mkdirSync(parentDir, { recursive: true })
writeFileSync(childFile, '')
try {
await SandboxManager.reset()
await SandboxManager.initialize({
network: { allowedDomains: [], deniedDomains: [] },
filesystem: {
denyRead: [],
allowWrite: [parentDir],
// Trailing slash and bare form both realpath to childFile
denyWrite: [childFile, childFile + '/'],
},
})
const result = await SandboxManager.wrapWithSandbox(command)
// One --ro-bind <path> <path> contains the path twice (src + dest).
// Without dedup this was 4 occurrences (two binds).
const occurrences = result.split(childFile).length - 1
expect(occurrences).toBe(2)
} finally {
await SandboxManager.reset()
rmSync(parentDir, { recursive: true, force: true })
}
})
})