A / write root (Linux) or read root (macOS) no longer switches off the denies beneath it

src/sandbox/linux-sandbox-utils.ts

@@ -124,9 +124,8 @@ function findSymlinkInPath(
       const stats = fs.lstatSync(nextPath)
       if (stats.isSymbolicLink()) {
         // Check if this symlink is within an allowed write path
-        const isWithinAllowedPath = allowedWritePaths.some(
-          allowedPath =>
-            nextPath.startsWith(allowedPath + '/') || nextPath === allowedPath,
+        const isWithinAllowedPath = allowedWritePaths.some(allowedPath =>
+          isAtOrUnder(nextPath, allowedPath),
         )
         if (isWithinAllowedPath) {
           return nextPath
@@ -1156,18 +1155,22 @@ async function generateFilesystemArgs(
       return stubSkipVetoInputs
     }
     // The ONE predicate deciding whether a deny dest lies inside the write
-    // allowlist. The read-only pre-pass below and the loop's --ro-bind gate
-    // MUST share it: the pre-pass is only sound if it records exactly the
-    // directories the loop re-binds read-only (a recorded directory that is
-    // never re-bound read-only would suppress stubs unsafely; a re-bound
-    // directory missing from the record only costs an abort). No spelling
-    // handling is needed here: allowedWritePaths entries are recorded with
-    // trailing slashes stripped, and candidates are resolved deny dests.
+    // allowlist, and so whether the deny is applied at all: a dest outside it
+    // is left read-only by the initial --ro-bind / /. The read-only pre-pass
+    // below and the loop's --ro-bind gate MUST share it, so that the pre-pass
+    // records the directories the loop re-binds read-only.
+    //
+    // Containment is root-aware (isAtOrUnder) because '/' is a legal
+    // allowOnly entry: an `allowedPath + '/'` prefix test spells it '//' and
+    // matches nothing, so under a '/' write root every deny that no other
+    // allow entry covers would be judged outside the allowlist and silently
+    // lose its bind — over a root the allow loop has already bound writable.
+    // Spelling needs no further handling here: allowedWritePaths entries are
+    // recorded with trailing slashes stripped, and candidates are resolved
+    // deny dests.
     const isWithinAnyAllowedWritePath = (candidatePath: string): boolean =>
-      allowedWritePaths.some(
-        allowedPath =>
-          candidatePath.startsWith(allowedPath + '/') ||
-          candidatePath === allowedPath,
+      allowedWritePaths.some(allowedPath =>
+        isAtOrUnder(candidatePath, allowedPath),
       )
 
     // Deny writes within allowed paths (user-specified + mandatory denies)
@@ -1196,9 +1199,13 @@ async function generateFilesystemArgs(
     // allowed write path strictly beneath it; incomparable with every
     // read-deny tmpfs in any spelling), which exclude every way its subtree
     // could be writable in the sandbox. Keep the two passes in lockstep: a
-    // directory recorded here but never re-bound read-only AND not vetoed
-    // would suppress stubs unsafely, while an emitted one missing from the
-    // record only costs a spurious abort.
+    // directory recorded here is either re-bound read-only by the loop or
+    // skipped because a recorded directory above it survived the vetoes and
+    // is bound in its place, so every record still stands for a bind that
+    // lands — unless a symlink appears in its path between the two passes,
+    // where the loop masks that component and emits no bind for the
+    // directory (the re-check below); an emitted one missing from the record
+    // only costs a spurious abort.
     for (const pathPattern of denyPaths) {
       const rawPath = normalizePathForSandbox(pathPattern)
       if (rawPath.startsWith('/dev/')) {
@@ -1317,9 +1324,7 @@ async function generateFilesystemArgs(
           // recorded directory: everything lies beneath it, so it would
           // veto every skip and stub each absent mandatory-deny path of a
           // write-denied cwd after that cwd's own bind — the startup abort.
-          // Its descendants are decided by their own recorded directories,
-          // as before, when the string-prefix filter matched '/' only for a
-          // path directly beneath it and the vetoes never fired for it.
+          // Its descendants are decided by their own recorded directories.
           if (denyDir === '/') continue
           return false
         }

src/sandbox/macos-sandbox-utils.ts

@@ -12,6 +12,7 @@ import {
   decodeSandboxedCommand,
   containsGlobChars,
   globToRegex,
+  isStrictlyUnder as isPathStrictlyUnder,
   DANGEROUS_FILES,
   getDangerousDirectories,
 } from './sandbox-utils.js'
@@ -201,8 +202,10 @@ function denyGlobCovers(denyRegex: RegExp, entry: PathEntry): boolean {
 
 /** Is `entry`'s region strictly inside the literal directory `dir`? */
 function isStrictlyUnder(entry: PathEntry, dir: string): boolean {
-  const probe = entry.glob ? globSamplePath(entry.path) : entry.path
-  return probe.startsWith(dir === '/' ? '/' : dir + '/') && probe !== dir
+  return isPathStrictlyUnder(
+    entry.glob ? globSamplePath(entry.path) : entry.path,
+    dir,
+  )
 }
 
 /**
@@ -270,7 +273,7 @@ function lateReadDenyFilters(resolved: ResolvedReadConfig): {
   const literalAllowDirs = resolved.allows.filter(a => !a.glob).map(a => a.path)
   for (const deny of resolved.denies) {
     if (!deny.glob) {
-      if (literalAllowDirs.some(a => deny.path.startsWith(a + '/'))) {
+      if (literalAllowDirs.some(a => isStrictlyUnder(deny, a))) {
         filters.push(denyPathFilter(deny.path))
       }
       continue

src/sandbox/sandbox-utils.ts

@@ -53,8 +53,8 @@ export function normalizeCaseForComparison(pathStr: string): string {
 }
 
 /**
- * `p` is `dir` itself or lies beneath it, by path segment ('/x' is not under
- * '/xy'); root-aware, since '/' + '/' is a prefix of nothing.
+ * `p` is `dir` itself or lies beneath it, by path segment ('/xy' is not under
+ * '/x'); root-aware, since '/' + '/' is a prefix of nothing.
  */
 export function isAtOrUnder(p: string, dir: string): boolean {
   return p === dir || p.startsWith(dir === '/' ? '/' : dir + '/')

test/sandbox/macos-glob-deny-reemit.test.ts

@@ -189,6 +189,19 @@ describe.if(!isWindows)('macOS read profile: glob denies vs allowRead', () => {
     expect(lateBlock(read)).toContain(filter)
   })
 
+  it('still re-emits a literal deny nested under a literal allow of "/"', () => {
+    // '/' is a legal allowWithinDeny entry and re-opens every denied path,
+    // so the nested deny only survives if it lands after the allow block.
+    // Containment against it has to be root-aware: a `dir + '/'` prefix
+    // spells '//' and matches nothing.
+    const read = readSection(
+      wrap({ denyOnly: ['/work/proj/secrets'], allowWithinDeny: ['/'] }),
+    )
+    const filter = '(subpath "/work/proj/secrets")'
+    expect(read.indexOf(filter)).toBeLessThan(allowBlockIndex(read))
+    expect(lateBlock(read)).toContain(filter)
+  })
+
   it('does not re-emit a literal deny that no allow is nested in', () => {
     const read = readSection(
       wrap({