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.
2026-05-07 22:20:26 +08:00 · 2026-03-30 14:20:00 -07:00
parent fd74a3f012
commit 3cdb468014
3 changed files with 253 additions and 22 deletions
--- a/src/sandbox/linux-sandbox-utils.ts
+++ b/src/sandbox/linux-sandbox-utils.ts
@@ -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)
@@ -728,7 +732,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 +784,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 +815,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 +826,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 +866,40 @@ 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 if already re-bound as writable above
+          if (
+            allowedWritePaths.some(
+              w => 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 +925,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
 }

--- a/src/sandbox/macos-sandbox-utils.ts
+++ b/src/sandbox/macos-sandbox-utils.ts
@@ -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)
--- a/test/sandbox/allow-read.test.ts
+++ b/test/sandbox/allow-read.test.ts
@@ -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'
@@ -222,6 +215,192 @@ describe('allowRead precedence over denyRead', () => {
  })
 })

+/**
+ * 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],
+    }
+
+    const wrappedCommand = await wrapCommandWithSandboxLinux({
+      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('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,
+    })
+
+    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: [],
+      },
+    })
+
+    const result = spawnSync(wrappedCommand, {
+      shell: true,
+      encoding: 'utf8',
+      timeout: 5000,
+    })
+
+    expect(result.status).toBe(0)
+    expect(result.stdout).toContain('WRITE_OK')
+  })
+})
+
 /**
 * Tests that allowRead-only configs (no denyRead) do not trigger sandbox overhead.
 */