From 06ca53c92eba0487fca4f2f1e52950b7659f8970 Mon Sep 17 00:00:00 2001 From: ant-kurt <209710463+ant-kurt@users.noreply.github.com> Date: Thu, 20 Aug 2026 22:56:27 -0700 Subject: [PATCH 1/2] linux: pin ancestor directories of deny binds against rename Each denyWrite bind or read-deny file mask makes only its destination a mountpoint; the directories between it and the covering allowed write root could be renamed, carrying the bind along and leaving the path recreatable unprotected. Emit a self --bind for each such directory so rename/rmdir on it fail EBUSY, seeded from both deny binds and file masks, skipping allowed write roots, deny dests and any directory that contains a read-deny tmpfs. Pins ride the existing emission filter and tmpfs/mask re-application passes, which now compare recorded and canonical spellings, replay the read section's actual restores instead of re-deriving them, and refuse restores that would bury an earlier read-deny mount. Only ENOENT/ENOTDIR count as absence in the pin walk; an unverifiable component aborts the wrap. --- src/sandbox/linux-sandbox-utils.ts | 421 ++++++++++- test/sandbox/linux-ancestor-pin-errno.test.ts | 215 ++++++ .../linux-ancestor-pin-implicit-tmpfs.test.ts | 49 ++ test/sandbox/linux-ancestor-pin.test.ts | 681 ++++++++++++++++++ test/sandbox/linux-mount-plan-record.test.ts | 175 +++++ 5 files changed, 1501 insertions(+), 40 deletions(-) create mode 100644 test/sandbox/linux-ancestor-pin-errno.test.ts create mode 100644 test/sandbox/linux-ancestor-pin-implicit-tmpfs.test.ts create mode 100644 test/sandbox/linux-ancestor-pin.test.ts create mode 100644 test/sandbox/linux-mount-plan-record.test.ts diff --git a/src/sandbox/linux-sandbox-utils.ts b/src/sandbox/linux-sandbox-utils.ts index e31685cf..bd460139 100644 --- a/src/sandbox/linux-sandbox-utils.ts +++ b/src/sandbox/linux-sandbox-utils.ts @@ -830,6 +830,18 @@ function buildSandboxCommand( * carve-outs expressed against the symlink path (e.g. /bin on usr-merged * systems). */ +/** Prefix that a strict descendant of `dir` starts with ('/' for the root). */ +function pathSep(dir: string): string { + return dir === '/' ? '/' : dir + '/' +} + +/** True when an fs error means the path is absent, as opposed to + * unreadable (EACCES), looping (ELOOP) or otherwise unverifiable. */ +function isAbsenceError(err: unknown): boolean { + const code = (err as NodeJS.ErrnoException | undefined)?.code + return code === 'ENOENT' || code === 'ENOTDIR' +} + function resolveSymlinkDenyDest(normalizedPath: string): string { try { if (fs.lstatSync(normalizedPath).isSymbolicLink()) { @@ -852,14 +864,46 @@ function pushReadDenyDirMounts( normalizedPath: string, allowedWritePaths: string[], readAllowPaths: string[], + pathForms: (p: string) => string[], + readDenyLocations: string[], + restoredWritePathsOut?: string[], ): void { - const denySep = normalizedPath === '/' ? '/' : normalizedPath + '/' + // Mounts land at canonical locations while recorded spellings may be + // symlinked, so containment tests expand both sides to both forms. + const denyForms = pathForms(normalizedPath) + const underDeniedDir = (p: string): boolean => + pathForms(p).some(pForm => + denyForms.some(form => pForm === form || pForm.startsWith(pathSep(form))), + ) + // A restore at-or-above an already-emitted read-deny mount would bind host + // content over it; skip it (the unit's own location is exempt). + const wouldBuryReadDeny = (restorePath: string): boolean => + readDenyLocations.some( + location => + !denyForms.includes(location) && + pathForms(restorePath).some( + restoreForm => + location === restoreForm || + location.startsWith(pathSep(restoreForm)), + ), + ) args.push('--tmpfs', normalizedPath) // tmpfs wiped any earlier write binds under this path — restore them. + // Record of what was actually re-bound; consumers read the record rather + // than re-deriving the predicate. + const restoredWrites: string[] = [] for (const writePath of allowedWritePaths) { - if (writePath.startsWith(denySep) || writePath === normalizedPath) { + if (underDeniedDir(writePath)) { + if (wouldBuryReadDeny(writePath)) { + logForDebugging( + `[Sandbox Linux] Skipping write-path restore that would bury a read-deny mount: ${writePath}`, + ) + continue + } args.push('--bind', writePath, writePath) + restoredWrites.push(writePath) + restoredWritePathsOut?.push(writePath) logForDebugging( `[Sandbox Linux] Re-bound write path wiped by denyRead tmpfs: ${writePath}`, ) @@ -870,26 +914,54 @@ function pushReadDenyDirMounts( // 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(denySep) || allowPath === normalizedPath) { + if (underDeniedDir(allowPath)) { if (!fs.existsSync(allowPath)) { logForDebugging( `[Sandbox Linux] Skipping non-existent read allow path: ${allowPath}`, ) continue } + // bwrap resolves the dest: an allowRead symlink pointing outside its + // recorded location would re-bind the target over this tmpfs. + try { + const resolvedAllow = fs.realpathSync(allowPath) + const allowForComparison = allowPath.replace(/\/+$/, '') + if ( + resolvedAllow !== allowForComparison && + isSymlinkOutsideBoundary(allowPath, resolvedAllow) + ) { + logForDebugging( + `[Sandbox Linux] Skipping allowRead restore for symlink pointing outside expected location: ${allowPath} -> ${resolvedAllow}`, + ) + continue + } + } catch { + logForDebugging( + `[Sandbox Linux] Skipping allowRead restore that could not be resolved: ${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 + '/')), + restoredWrites.some(w => + pathForms(w).some(wForm => + pathForms(allowPath).some( + aForm => aForm === wForm || aForm.startsWith(wForm + '/'), + ), + ), ) ) { continue } + if (wouldBuryReadDeny(allowPath)) { + logForDebugging( + `[Sandbox Linux] Skipping allowRead restore that would bury a read-deny mount: ${allowPath}`, + ) + continue + } // Bind the allowed path back over the tmpfs so it's readable args.push('--ro-bind', allowPath, allowPath) logForDebugging( @@ -938,6 +1010,36 @@ async function generateFilesystemArgs( // symlink no longer matches them by string prefix. Both spellings name the // same inode once bwrap resolves them, so the comparisons below test both. const denyWriteRawDests = new Map() + // A mount given a symlink-spelled path lands at its canonical target while + // recorded lists keep their spelling; containment tests expand to both + // forms through this memoized helper. + const mountFormsCache = new Map() + const mountForms = (p: string): string[] => { + let forms = mountFormsCache.get(p) + if (forms === undefined) { + forms = [p] + try { + const canonical = fs.realpathSync(p) + if (canonical !== p) forms.push(canonical) + } catch { + // vanished or unresolvable: the recorded form suffices + } + mountFormsCache.set(p, forms) + } + return forms + } + const canonicalForm = (p: string): string => { + const forms = mountForms(p) + return forms[forms.length - 1]! + } + // Canonical locations of every read-deny mount (tmpfs, file masks, + // credential fakes); a write-path restore must never bury one. + const readDenyCanonicalLocations: string[] = [ + ...(readConfig?.denyOnly ?? []).map(p => + canonicalForm(normalizePathForSandbox(p)), + ), + ...(maskedFileBinds ?? []).map(bind => canonicalForm(bind.realPath)), + ] // Determine initial root mount based on write restrictions if (writeConfig) { @@ -1114,14 +1216,20 @@ async function generateFilesystemArgs( // 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. + // directory missing from the record only costs an abort). allowedWritePaths + // entries are recorded with trailing slashes stripped; a symlink-spelled + // entry's bind lands at its canonical target, so candidates (canonical) + // are tested against both forms of each entry. const isWithinAnyAllowedWritePath = (candidatePath: string): boolean => - allowedWritePaths.some( - allowedPath => - candidatePath.startsWith(allowedPath + '/') || - candidatePath === allowedPath, + allowedWritePaths.some(allowedPath => + mountForms(allowedPath).some( + form => + candidatePath.startsWith(pathSep(form)) || candidatePath === form, + ), + ) + const isAllowedWriteRoot = (candidatePath: string): boolean => + allowedWritePaths.some(allowedPath => + mountForms(allowedPath).some(form => candidatePath === form), ) // Deny writes within allowed paths (user-specified + mandatory denies) @@ -1199,12 +1307,16 @@ async function generateFilesystemArgs( // INCOMPARABLE with every read-deny tmpfs directory (neither // at-or-beneath it nor containing it or any spelling it was reached // through). Rationale: the only writable emissions that land after the - // buffered read-only binds are the denyRead re-applications - // (pushReadDenyDirMounts), which mount a tmpfs and re-bind allowed write - // paths beneath it WITHOUT re-emitting the binds it buries — so a - // comparable tmpfs is both the re-opening vector (beneath or around the - // dir) and the only way the dir's own --ro-bind gets dropped at emission - // as hidden-by-a-tmpfs. (The emission filter's other drop condition, + // buffered read-only binds are the denyRead re-application's write + // restores, and those fire only for a tmpfs whose canonical location + // sits inside an emitted dest — a tmpfs comparable with the covering + // directory — with a restore list filtered to exclude any write path + // that has an emitted deny bind at-or-under it. (Ancestor pins never + // land above a tmpfs by the pin walk's own exclusion, and pins below + // one ride the same emission filter as every deny bind.) A comparable + // tmpfs is thus both the re-opening vector (beneath or around the dir) + // and the only way the dir's own --ro-bind gets dropped at emission as + // hidden-by-a-tmpfs. (The emission filter's other drop condition, // maskedFiles, holds file dests only — /dev/null read-deny masks and // credential-mask fakes — while the pre-pass stat-verifies every // recorded dir as a directory, so it cannot drop a recorded dir short of @@ -1431,6 +1543,172 @@ async function generateFilesystemArgs( ) } } + + // Ancestor pinning: the directories strictly between a deny dest and its + // covering allowed write root carry no mount, so rename(2) on one moves + // the deny bind along with it and the path can be recreated unprotected. + // A self --bind on each makes it a mountpoint (rename/rmdir fail EBUSY) + // without changing reads or writes inside it. Allowed write roots and + // deny dests are skipped (already mountpoints); the walk continues past + // nested roots to the outermost one; pins are prepended shallow-first so + // deny binds land on top, and ride the emission filter and + // re-application passes like any other denyWrite entry. + const denyWriteDests = new Set() + for (let i = 0; i < denyWriteArgs.length; i += 3) { + denyWriteDests.add(denyWriteArgs[i + 2]!) + } + // File masks (non-directory denyRead entries, credential fakes) are the + // second protection channel and seed the walk too. + const maskPinSeeds: string[] = [] + for (const denyReadPath of readConfig?.denyOnly ?? []) { + let resolvedDenyRead: string + try { + resolvedDenyRead = normalizePathForSandbox(denyReadPath) + } catch { + continue + } + let seedIt: boolean + try { + // Every non-directory (sockets and FIFOs included) gets a mask. + seedIt = !fs.statSync(resolvedDenyRead).isDirectory() + } catch (err) { + // Absent paths get no mask; any other errno seeds conservatively + // (over-seeding only adds restriction-only pins). + seedIt = !isAbsenceError(err) + } + if (seedIt) { + // Seed the canonical mount location, not the raw spelling. + maskPinSeeds.push( + canonicalForm(resolveSymlinkDenyDest(resolvedDenyRead)), + ) + } + } + for (const maskedFileBind of maskedFileBinds ?? []) { + maskPinSeeds.push(canonicalForm(maskedFileBind.realPath)) + } + // No pin at or strictly inside a deny dest: the dest's read-only bind + // shadows it, and renames inside a read-only mount already fail EROFS. + // Exception: inside an allowWrite carve-out under the dest, which the + // denyRead machinery restores writable, the corridor down to a deeper + // leaf still needs pins — unless the carve-out's restore is itself + // vetoed (its subtree contains another read-deny location). + const carveOutSubtreeContainsReadDeny = (writePath: string): boolean => + readDenyCanonicalLocations.some(location => + mountForms(writePath).some( + form => location === form || location.startsWith(pathSep(form)), + ), + ) + const excludedFromPinning = (candidate: string): boolean => { + for (const dest of denyWriteDests) { + if (candidate === dest || candidate.startsWith(dest + '/')) { + const insideRestorableCarveOut = allowedWritePaths.some( + writePath => + mountForms(writePath).some( + form => + form.startsWith(dest + '/') && + (candidate === form || candidate.startsWith(form + '/')), + ) && !carveOutSubtreeContainsReadDeny(writePath), + ) + if (!insideRestorableCarveOut) return true + } + } + return false + } + // A pin at-or-above a denyRead tmpfs would land after it and bury it, so + // none is generated (such a directory stays renameable). Keyed on + // readConfig, not denyOnly: /etc/ssh/ssh_config.d is hidden whenever + // readConfig is defined. + const containsProspectiveReadDenyTmpfs = (candidate: string): boolean => { + if (!readConfig) return false + const { prospectiveReadDenyTmpfsDirsBothForms } = getStubSkipVetoInputs() + return prospectiveReadDenyTmpfsDirsBothForms.some( + tmpfsForm => + tmpfsForm === candidate || tmpfsForm.startsWith(candidate + '/'), + ) + } + // Only a genuinely absent ancestor skips its pin (bwrap fails on a missing + // bind source); an unverifiable one (EACCES) is pinned anyway. + const ancestorIsAbsent = (dir: string): boolean => { + try { + fs.statSync(dir) + return false + } catch (err) { + return isAbsenceError(err) + } + } + const ancestorPinDirs = new Set() + // Verdicts are dest-independent: a visited ancestor's chain is done. + const visitedAncestors = new Set() + for (const dest of [...denyWriteDests, ...maskPinSeeds]) { + let ancestorDir = path.dirname(dest) + while (ancestorDir !== '/' && isWithinAnyAllowedWritePath(ancestorDir)) { + if (visitedAncestors.has(ancestorDir)) break + visitedAncestors.add(ancestorDir) + if ( + !isAllowedWriteRoot(ancestorDir) && + !excludedFromPinning(ancestorDir) && + !containsProspectiveReadDenyTmpfs(ancestorDir) && + !ancestorIsAbsent(ancestorDir) + ) { + ancestorPinDirs.add(ancestorDir) + } + ancestorDir = path.dirname(ancestorDir) + } + } + // bwrap re-resolves the pin path at mount time, so re-verify that no + // component is a symlink (narrows, does not close, the check→mount race). + // A symlink component drops the pin; an lstat error other than absence + // aborts the wrap rather than mount through an unverifiable component. + const componentLstatMemo = new Map() + const componentIsSymlink = (prefix: string): boolean | null => { + let verdict = componentLstatMemo.get(prefix) + if (verdict === undefined) { + try { + verdict = fs.lstatSync(prefix).isSymbolicLink() + } catch (err) { + if (!isAbsenceError(err)) { + const code = (err as NodeJS.ErrnoException | undefined)?.code + throw new Error( + `Sandbox ancestor-pin verification failed: cannot lstat ${prefix} (${code ?? String(err)}). ` + + 'Refusing to build a mount plan with unverifiable pin components.', + ) + } + verdict = null + } + componentLstatMemo.set(prefix, verdict) + } + return verdict + } + const pinComponentsAreSymlinkFree = (pinDir: string): boolean => { + let prefix = '' + for (const component of pinDir.split('/')) { + if (component === '') continue + prefix += '/' + component + const verdict = componentIsSymlink(prefix) + if (verdict === null) { + logForDebugging( + `[Sandbox Linux] Dropping ancestor pin with vanished component: ${pinDir}`, + ) + return false + } + if (verdict) { + logForDebugging( + `[Sandbox Linux] Dropping ancestor pin with symlink component (changed since resolution): ${pinDir}`, + ) + return false + } + } + return true + } + const ancestorPinArgs: string[] = [] + for (const pinDir of [...ancestorPinDirs].sort( + (a, b) => a.split('/').length - b.split('/').length, + )) { + if (pinComponentsAreSymlinkFree(pinDir)) { + ancestorPinArgs.push('--bind', pinDir, pinDir) + } + } + denyWriteArgs.unshift(...ancestorPinArgs) } else { // No write restrictions: Allow all writes args.push('--bind', '/', '/') @@ -1451,6 +1729,12 @@ async function generateFilesystemArgs( // the mask, and to re-apply the correct source if a denyWrite ancestor // bind re-exposes the dest. const maskedFiles = new Map() + // Canonical locations of read-deny mounts emitted so far: only an + // already-emitted mount can be buried by a later unit's restore. + const emittedReadDenyLocations: string[] = [] + // Per tmpfs unit, in emission order: its mount forms and the write paths + // its restore loop actually re-bound. Replayed by the emission filter. + const readSectionPlan: Array<{ forms: string[]; restores: string[] }> = [] // Directories masked by --tmpfs below, in emission (shallow-first) order. // Used to filter denyWriteArgs the same way: a dir in both deny lists must // not get its host contents re-bound on top of its own tmpfs. @@ -1500,12 +1784,20 @@ async function generateFilesystemArgs( const readDenyStat = fs.statSync(normalizedPath) if (readDenyStat.isDirectory()) { tmpfsDirs.push(normalizedPath) + // Joins the emitted set before its own call (exempt from its own veto). + const unitForms = mountForms(normalizedPath) + emittedReadDenyLocations.push(unitForms[unitForms.length - 1]!) + const restored: string[] = [] pushReadDenyDirMounts( args, normalizedPath, allowedWritePaths, readAllowPaths, + mountForms, + emittedReadDenyLocations, + restored, ) + readSectionPlan.push({ forms: unitForms, restores: restored }) } else { // For files, only an exact allowRead match overrides the deny. A // directory allowRead does not un-deny a file specifically listed in @@ -1523,6 +1815,7 @@ async function generateFilesystemArgs( args.push('--ro-bind', '/dev/null', denyDest) maskedFiles.set(denyDest, '/dev/null') maskedFiles.set(normalizedPath, '/dev/null') + emittedReadDenyLocations.push(canonicalForm(normalizedPath)) } } @@ -1556,28 +1849,40 @@ async function generateFilesystemArgs( // if an allowed write path at-or-under that tmpfs covers the dest, the // denyRead loop re-bound it (the .git/hooks case) and the write-deny bind // is still required on top. - // tmpfsDirs and allowedWritePaths hold unresolved paths while dest has been - // canonicalized, so each dest is tested under both spellings: they name the - // same inode after bwrap resolves the mount destinations, and a hit on - // either means the tmpfs really does cover this bind. - const isHiddenByTmpfs = (dest: string): boolean => - tmpfsDirs.some(tmpfsDir => { - const underTmpfs = dest === tmpfsDir || dest.startsWith(tmpfsDir + '/') - if (!underTmpfs) return false - const reExposedByWriteBind = allowedWritePaths.some( - writePath => - (writePath === tmpfsDir || writePath.startsWith(tmpfsDir + '/')) && - (dest === writePath || dest.startsWith(writePath + '/')), - ) - return !reExposedByWriteBind - }) + // Replay the read section's plan in order — bwrap is last-mount-wins, so a + // dest hidden by one unit's tmpfs is re-exposed by a later unit's restore. + // Only the canonical dest is tested: a raw route can read as covered while + // the canonical mount location is exposed. + const writeRebindCovers = (writePath: string, dest: string): boolean => + mountForms(writePath).some( + writeForm => dest === writeForm || dest.startsWith(pathSep(writeForm)), + ) + const isHiddenByTmpfs = (dest: string): boolean => { + let hidden = false + for (const unit of readSectionPlan) { + if ( + unit.forms.some(form => dest === form || dest.startsWith(pathSep(form))) + ) { + hidden = true + } + if (unit.restores.some(writePath => writeRebindCovers(writePath, dest))) { + hidden = false + } + } + return hidden + } const emittedDenyWriteDests: string[] = [] + // Mask keys may be raw spellings while deny dests are canonical. + const maskedFileForms = new Set() + for (const maskedFile of maskedFiles.keys()) { + for (const form of mountForms(maskedFile)) maskedFileForms.add(form) + } for (let i = 0; i < denyWriteArgs.length; i += 3) { const dest = denyWriteArgs[i + 2]! const rawDest = denyWriteRawDests.get(dest) ?? dest - if (maskedFiles.has(dest)) continue - if (isHiddenByTmpfs(dest) || isHiddenByTmpfs(rawDest)) { + if (maskedFileForms.has(dest) || maskedFileForms.has(rawDest)) continue + if (isHiddenByTmpfs(dest)) { logForDebugging( `[Sandbox Linux] Skipping denyWrite bind already hidden by denyRead tmpfs: ${dest}`, ) @@ -1595,19 +1900,55 @@ async function generateFilesystemArgs( // contains a read-denied dir re-exposes that dir's real contents (the bind // landed after the tmpfs). Re-apply the tmpfs on top, with the same write // and allowRead re-binds the denyRead loop emitted. + // Runs over all emitted dests, pins included. Containment is tested at the + // tmpfs's canonical location (where it actually mounted); a raw-spelling + // match means the mount was never buried. for (const tmpfsDir of tmpfsDirs) { - if (emittedDenyWriteDests.some(dest => tmpfsDir.startsWith(dest + '/'))) { + const canonicalLocation = canonicalForm(tmpfsDir) + if ( + emittedDenyWriteDests.some(dest => + canonicalLocation.startsWith(dest + '/'), + ) + ) { logForDebugging( `[Sandbox Linux] Re-applying denyRead tmpfs re-exposed by denyWrite bind: ${tmpfsDir}`, ) - pushReadDenyDirMounts(args, tmpfsDir, allowedWritePaths, readAllowPaths) + // Restoring a write path with an emitted deny bind at-or-under it, or + // one at-or-inside an emitted deny dest, would bury that bind. + const restorableWritePaths = allowedWritePaths.filter( + writePath => + !mountForms(writePath).some(wForm => + emittedDenyWriteDests.some( + dest => + dest === wForm || + dest.startsWith(wForm + '/') || + wForm.startsWith(pathSep(dest)), + ), + ), + ) + pushReadDenyDirMounts( + args, + tmpfsDir, + restorableWritePaths, + readAllowPaths, + mountForms, + readDenyCanonicalLocations, + ) } } // Same problem for masked files: the mask landed before the denyWrite // ancestor bind, so the real file is back. Re-apply the mask with its // original source (/dev/null for read-deny, the fake for credential mask). + // Both forms of the mask key: a raw-spelled mask mounts (and is buried by a + // canonical pin) at its canonical target. for (const [maskedFile, source] of maskedFiles) { - if (emittedDenyWriteDests.some(dest => maskedFile.startsWith(dest + '/'))) { + if ( + mountForms(maskedFile).some(mForm => + emittedDenyWriteDests.some( + dest => mForm === dest || mForm.startsWith(pathSep(dest)), + ), + ) + ) { // maskedFiles holds both the symlink path and its resolved target so // the denyWrite skip-check above matches either. Re-emission must go // to the target only — bwrap rejects a symlink bind dest (see diff --git a/test/sandbox/linux-ancestor-pin-errno.test.ts b/test/sandbox/linux-ancestor-pin-errno.test.ts new file mode 100644 index 00000000..0e9288ea --- /dev/null +++ b/test/sandbox/linux-ancestor-pin-errno.test.ts @@ -0,0 +1,215 @@ +import { describe, it, expect, afterEach, spyOn } from 'bun:test' +import { spawnSync } from 'node:child_process' +import * as fs from 'fs' +import { + mkdirSync, + mkdtempSync, + realpathSync, + rmSync, + writeFileSync, +} from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { wrapCommandWithSandboxLinux } from '../../src/sandbox/linux-sandbox-utils.js' +import { isLinux } from '../helpers/platform.js' + +// The ancestor-pin walk distinguishes absence (ENOENT/ENOTDIR) from other +// errnos: an unreadable ancestor is still pinned, and an unreadable pin +// component aborts the wrap. Errnos are injected via fs spies (a root +// container sees no real EACCES); each spy asserts its own hit count so a +// non-intercepting mock cannot pass vacuously. Nothing here executes bwrap. +describe.if(isLinux)( + 'Linux sandbox — ancestor-pin errno discrimination', + () => { + const errnoError = (code: string, message: string) => + Object.assign(new Error(message), { code }) + const EACCES = () => errnoError('EACCES', 'EACCES: permission denied') + + const created: string[] = [] + const spies: Array<{ mockRestore: () => void }> = [] + afterEach(() => { + for (const spy of spies.splice(0)) spy.mockRestore() + for (const dir of created.splice(0)) { + rmSync(dir, { recursive: true, force: true }) + } + }) + + function makeTree(): string { + // proj/a/b/.git/config — pins expected for a, a/b, a/b/.git + const proj = realpathSync(mkdtempSync(join(tmpdir(), 'pin-errno-'))) + created.push(proj) + mkdirSync(join(proj, 'a', 'b', '.git'), { recursive: true }) + writeFileSync(join(proj, 'a', 'b', '.git', 'config'), '[core]\n') + return proj + } + + async function wrap( + proj: string, + extra: Partial[0]> = {}, + ): Promise { + return wrapCommandWithSandboxLinux({ + command: 'true', + needsNetworkRestriction: false, + allowAllUnixSockets: true, + writeConfig: { + allowOnly: [proj], + denyWithinAllow: [join(proj, 'a', 'b', '.git', 'config')], + }, + ...extra, + }) + } + + it('baseline: ancestors of a denyWrite target are pinned', async () => { + const proj = makeTree() + const wrapped = await wrap(proj) + const pin = join(proj, 'a', 'b') + expect(wrapped).toContain(`--bind ${pin} ${pin}`) + }) + + it('pins an ancestor whose stat fails with EACCES', async () => { + const proj = makeTree() + const target = join(proj, 'a', 'b') + const realStat = fs.statSync + const realExists = fs.existsSync + let statHits = 0 + let existsHits = 0 + spies.push( + spyOn(fs, 'statSync').mockImplementation((( + p: fs.PathLike, + ...rest: unknown[] + ) => { + if (String(p) === target) { + statHits++ + throw EACCES() + } + return (realStat as (...a: unknown[]) => unknown)(p, ...rest) + }) as typeof fs.statSync), + ) + spies.push( + spyOn(fs, 'existsSync').mockImplementation(((p: fs.PathLike) => { + if (String(p) === target) { + existsHits++ + return false + } + return realExists(p) + }) as typeof fs.existsSync), + ) + const wrapped = await wrap(proj) + expect(statHits + existsHits).toBeGreaterThan(0) + expect(wrapped).toContain(`--bind ${target} ${target}`) + }) + + it('skips the pin of a genuinely absent (ENOENT) ancestor', async () => { + const proj = makeTree() + const missingParent = join(proj, 'a', 'missing') + const wrapped = await wrap(proj, { + writeConfig: { + allowOnly: [proj], + denyWithinAllow: [join(missingParent, 'leaf')], + }, + }) + expect(wrapped).not.toContain(`--bind ${missingParent} ${missingParent}`) + }) + + it('aborts the wrap when lstat of a pin component fails with EACCES', async () => { + const proj = makeTree() + const component = join(proj, 'a') + const realLstat = fs.lstatSync + let lstatHits = 0 + spies.push( + spyOn(fs, 'lstatSync').mockImplementation((( + p: fs.PathLike, + ...rest: unknown[] + ) => { + if (String(p) === component) { + lstatHits++ + throw EACCES() + } + return (realLstat as (...a: unknown[]) => unknown)(p, ...rest) + }) as typeof fs.lstatSync), + ) + // eslint-disable-next-line @typescript-eslint/await-thenable -- bun:test types .rejects.toThrow() as void; the await is required at runtime + await expect(wrap(proj)).rejects.toThrow( + /cannot lstat .*Refusing to build a mount plan/s, + ) + expect(lstatHits).toBeGreaterThan(0) + }) + + it('drops only the pin when a component vanished (ENOENT) and still builds the wrap', async () => { + const proj = makeTree() + const component = join(proj, 'a') + const realLstat = fs.lstatSync + spies.push( + spyOn(fs, 'lstatSync').mockImplementation((( + p: fs.PathLike, + ...rest: unknown[] + ) => { + if (String(p) === component) { + throw errnoError('ENOENT', 'ENOENT: no such file or directory') + } + return (realLstat as (...a: unknown[]) => unknown)(p, ...rest) + }) as typeof fs.lstatSync), + ) + const wrapped = await wrap(proj) + expect(wrapped).not.toContain(`--bind ${component} ${component}`) + expect(wrapped).toContain('bwrap') + }) + + it('seeds ancestors of a denyRead file whose first stat fails with EACCES', async () => { + const proj = makeTree() + mkdirSync(join(proj, 'secrets')) + const denyReadFile = join(proj, 'secrets', 'token') + writeFileSync(denyReadFile, 'x') + const realStat = fs.statSync + let statHits = 0 + // Permission flip: unreadable while the seed walk stats (first), readable + // again when the denyRead loop stats and emits the mask. + let failedOnce = false + spies.push( + spyOn(fs, 'statSync').mockImplementation((( + p: fs.PathLike, + ...rest: unknown[] + ) => { + if (String(p) === denyReadFile && !failedOnce) { + failedOnce = true + statHits++ + throw EACCES() + } + return (realStat as (...a: unknown[]) => unknown)(p, ...rest) + }) as typeof fs.statSync), + ) + const wrapped = await wrap(proj, { + readConfig: { denyOnly: [denyReadFile], allowWithinDeny: [] }, + }) + const seedAncestor = join(proj, 'secrets') + expect(statHits).toBeGreaterThan(0) + expect(wrapped).toContain(`--bind ${seedAncestor} ${seedAncestor}`) + }) + + it('seeds ancestors of a FIFO denyRead entry (every non-directory is masked)', async () => { + const proj = makeTree() + mkdirSync(join(proj, 'secrets')) + const fifoPath = join(proj, 'secrets', 'pipe.fifo') + const mk = spawnSync('mkfifo', [fifoPath]) + if (mk.status !== 0) { + throw new Error('mkfifo unavailable') + } + const wrapped = await wrap(proj, { + readConfig: { denyOnly: [fifoPath], allowWithinDeny: [] }, + }) + const seedAncestor = join(proj, 'secrets') + expect(wrapped).toContain(`--bind ${seedAncestor} ${seedAncestor}`) + expect(wrapped).toContain(`--ro-bind /dev/null ${fifoPath}`) + }) + + it('seeds nothing for a genuinely absent denyRead file', async () => { + const proj = makeTree() + const absent = join(proj, 'secrets', 'gone') + const wrapped = await wrap(proj, { + readConfig: { denyOnly: [absent], allowWithinDeny: [] }, + }) + const seedAncestor = join(proj, 'secrets') + expect(wrapped).not.toContain(`--bind ${seedAncestor} ${seedAncestor}`) + }) + }, +) diff --git a/test/sandbox/linux-ancestor-pin-implicit-tmpfs.test.ts b/test/sandbox/linux-ancestor-pin-implicit-tmpfs.test.ts new file mode 100644 index 00000000..ada5bd63 --- /dev/null +++ b/test/sandbox/linux-ancestor-pin-implicit-tmpfs.test.ts @@ -0,0 +1,49 @@ +import { describe, it, expect } from 'bun:test' +import { existsSync } from 'node:fs' +import { wrapCommandWithSandboxLinux } from '../../src/sandbox/linux-sandbox-utils.js' +import { isLinux } from '../helpers/platform.js' + +// The read section mounts an implicit tmpfs at /etc/ssh/ssh_config.d whenever +// readConfig is defined, even with an empty denyOnly, so the pin walk's tmpfs +// exclusion must key on readConfig itself or it pins /etc/ssh above it. +const HOST_SHAPE_PRESENT = + isLinux && + existsSync('/etc/ssh/ssh_config.d') && + existsSync('/etc/ssh/ssh_config') + +describe.if(HOST_SHAPE_PRESENT)( + 'Linux sandbox — implicit ssh_config.d tmpfs vs ancestor-pin exclusion', + () => { + const baseParams = { + command: 'true', + needsNetworkRestriction: false, + allowAllUnixSockets: true, + } + + it('does not pin /etc/ssh above the implicit ssh_config.d tmpfs', async () => { + const wrapped = await wrapCommandWithSandboxLinux({ + ...baseParams, + readConfig: { denyOnly: [], allowWithinDeny: [] }, + writeConfig: { + allowOnly: ['/etc'], + denyWithinAllow: ['/etc/ssh/ssh_config'], + }, + }) + expect(wrapped).toMatch(/--tmpfs \/etc\/ssh\/ssh_config\.d(?: |$)/) + expect(wrapped).not.toMatch(/--bind \/etc\/ssh \/etc\/ssh(?: |$)/) + }) + + it('pins /etc/ssh when there is no readConfig and hence no implicit tmpfs', async () => { + const wrapped = await wrapCommandWithSandboxLinux({ + ...baseParams, + readConfig: undefined, + writeConfig: { + allowOnly: ['/etc'], + denyWithinAllow: ['/etc/ssh/ssh_config'], + }, + }) + expect(wrapped).not.toContain('--tmpfs /etc/ssh/ssh_config.d') + expect(wrapped).toMatch(/--bind \/etc\/ssh \/etc\/ssh(?: |$)/) + }) + }, +) diff --git a/test/sandbox/linux-ancestor-pin.test.ts b/test/sandbox/linux-ancestor-pin.test.ts new file mode 100644 index 00000000..fd7eb08b --- /dev/null +++ b/test/sandbox/linux-ancestor-pin.test.ts @@ -0,0 +1,681 @@ +import { describe, it, expect, beforeEach, afterEach } from 'bun:test' +import { spawnSync } from 'node:child_process' +import { + existsSync, + mkdirSync, + mkdtempSync, + readFileSync, + realpathSync, + rmSync, + symlinkSync, + writeFileSync, +} from 'node:fs' +import { tmpdir } from 'node:os' +import { dirname, join } from 'node:path' +import { + wrapCommandWithSandboxLinux, + cleanupBwrapMountPoints, +} from '../../src/sandbox/linux-sandbox-utils.js' +import { isLinux } from '../helpers/platform.js' + +// Every directory strictly between a deny bind's dest and its covering +// allowWrite root is pinned with a self --bind so it is a mountpoint: +// rename/rmdir of it fail EBUSY, so the deny cannot be moved aside and the +// path recreated unprotected. Reads and writes inside it are unchanged. +describe.if(isLinux)('Linux sandbox — denyWrite ancestor pinning', () => { + let BASE: string + let PROJECT: string + const savedCwd = process.cwd() + + const BWRAP_CAN_NAMESPACE = + spawnSync( + 'bwrap', + [ + '--unshare-pid', + '--unshare-user', + '--cap-drop', + 'ALL', + '--ro-bind', + '/', + '/', + '--proc', + '/proc', + 'true', + ], + { timeout: 5000 }, + ).status === 0 + + function mkTree(root: string, tree: Record): void { + for (const [name, value] of Object.entries(tree)) { + const p = join(root, name) + if (typeof value === 'string') { + writeFileSync(p, value) + } else { + mkdirSync(p, { recursive: true }) + mkTree(p, value as Record) + } + } + } + + beforeEach(() => { + BASE = realpathSync(mkdtempSync(join(tmpdir(), 'ancestor-pin-'))) + PROJECT = join(BASE, 'project') + mkdirSync(PROJECT) + writeFileSync(join(PROJECT, 'README.md'), '# test\n') + }) + + afterEach(() => { + process.chdir(savedCwd) + cleanupBwrapMountPoints({ force: true }) + rmSync(BASE, { recursive: true, force: true }) + }) + + async function wrap( + filesystem: { + allowWrite?: string[] + denyWrite?: string[] + denyRead?: string[] + } = {}, + command = 'echo ok', + ): Promise { + // Mandatory denies (.git/config, .git/hooks, dotfiles) are relative to + // process.cwd(); the project is the sandbox's cwd like a real session. + process.chdir(PROJECT) + return wrapCommandWithSandboxLinux({ + command, + needsNetworkRestriction: false, + allowAllUnixSockets: true, + readConfig: { denyOnly: filesystem.denyRead ?? [] }, + writeConfig: { + allowOnly: [PROJECT, ...(filesystem.allowWrite ?? [])], + denyWithinAllow: filesystem.denyWrite ?? [], + }, + }) + } + + function run(command: string) { + return spawnSync(command, { + shell: true, + encoding: 'utf8', + timeout: 15000, + cwd: PROJECT, + }) + } + + it('pins the directories between a mandatory deny leaf and the allowWrite root', async () => { + mkTree(PROJECT, { '.git': { hooks: {}, config: '[core]\n' } }) + const gitDir = join(PROJECT, '.git') + + const command = await wrap() + + expect(command).toContain('--ro-bind / /') + const configBind = `--ro-bind ${gitDir}/config ${gitDir}/config` + expect(command).toContain(configBind) + // .git sits strictly between the leaf denies and the allowWrite root. + const gitPin = `--bind ${gitDir} ${gitDir}` + expect(command).toContain(gitPin) + // The pin must land before the leaf ro-bind, or its writable bind would + // shadow the read-only one. + expect(command.indexOf(gitPin)).toBeLessThan(command.indexOf(configBind)) + // Nothing outside the writable set is pinned. + const outside = dirname(PROJECT) + expect(command).not.toContain(`--bind ${outside} ${outside}`) + // The allowWrite root is bound exactly once (its allow bind); a pin there + // would add nothing. + const rootBind = `--bind ${PROJECT} ${PROJECT}` + expect(command.split(rootBind).length - 1).toBe(1) + }) + + it('pins ancestors of absent-path stub dests', async () => { + // .git exists but neither config nor hooks does: every deny dest on this + // chain is a stub, so the .git pin can only come from stub dests. + mkTree(PROJECT, { '.git': {} }) + const gitDir = join(PROJECT, '.git') + + const command = await wrap() + + expect(command).toContain(`--ro-bind /dev/null ${gitDir}/hooks`) + expect(command).toContain(`--ro-bind /dev/null ${gitDir}/config`) + expect(command).toContain(`--bind ${gitDir} ${gitDir}`) + }) + + it('pins every intermediate directory above a nested repo found by the depth scan', async () => { + mkTree(PROJECT, { nested: { '.git': { hooks: {}, config: '[core]\n' } } }) + const nestedDir = join(PROJECT, 'nested') + const nestedGit = join(nestedDir, '.git') + + const command = await wrap() + + expect(command).toContain( + `--ro-bind ${nestedGit}/config ${nestedGit}/config`, + ) + expect(command).toContain(`--bind ${nestedGit} ${nestedGit}`) + expect(command).toContain(`--bind ${nestedDir} ${nestedDir}`) + }) + + it('keeps leaf denies enforced when a denyRead tmpfs sits between nested allowWrite roots', async () => { + // x contains the read-denied y, so x gets no pin (it would land after + // the tmpfs and bury it); the denyRead section's restore of the nested + // allowWrite z runs before the buffered deny binds, so the .git pin and + // the config deny land on top of it. + mkTree(PROJECT, { + x: { y: { z: { '.git': { hooks: {}, config: '[core]\n' } } } }, + }) + const yDir = join(PROJECT, 'x', 'y') + const zDir = join(yDir, 'z') + const configPath = join(zDir, '.git', 'config') + const filesystem = { + allowWrite: [zDir], + denyRead: [yDir], + denyWrite: [configPath], + } + + const command = await wrap( + filesystem, + `echo evil >> ${configPath} && echo PLANTED`, + ) + + const configBind = `--ro-bind ${configPath} ${configPath}` + const zBind = `--bind ${zDir} ${zDir}` + expect(command).toContain(configBind) + expect(command).toContain(zBind) + expect(command.lastIndexOf(configBind)).toBeGreaterThan( + command.lastIndexOf(zBind), + ) + + if (BWRAP_CAN_NAMESPACE) { + const result = run(command) + expect(result.stdout ?? '').not.toContain('PLANTED') + expect(result.status).not.toBe(0) + expect(readFileSync(configPath, 'utf8')).toBe('[core]\n') + + const writable = await wrap( + filesystem, + `echo n > ${zDir}/newfile.txt && echo Z_WRITE_OK`, + ) + expect(run(writable).stdout).toContain('Z_WRITE_OK') + expect(existsSync(join(zDir, 'newfile.txt'))).toBe(true) + } + }) + + it('does not pin an ancestor that contains a denyRead dir, but keeps pinning the repo chain below it', async () => { + mkTree(PROJECT, { + x: { + y: { + z: { + app: { + data: { 'secret.txt': 'TOPSECRET\n' }, + repo: { '.git': { hooks: {}, config: '[core]\n' } }, + }, + }, + }, + }, + }) + const yDir = join(PROJECT, 'x', 'y') + const zDir = join(yDir, 'z') + const appDir = join(zDir, 'app') + const dataDir = join(appDir, 'data') + const secretPath = join(dataDir, 'secret.txt') + const configPath = join(appDir, 'repo', '.git', 'config') + + const command = await wrap( + { + allowWrite: [zDir], + denyRead: [yDir, dataDir], + denyWrite: [configPath], + }, + `cat ${secretPath} 2>&1; echo evil >> ${secretPath} 2>&1; echo evil >> ${configPath} 2>&1; echo DONE`, + ) + + expect(command).toContain(`--tmpfs ${dataDir}`) + expect(command).not.toContain(`--bind ${appDir} ${appDir}`) + expect(command).toContain( + `--bind ${join(appDir, 'repo')} ${join(appDir, 'repo')}`, + ) + expect(command).not.toContain( + `--bind ${join(PROJECT, 'x')} ${join(PROJECT, 'x')}`, + ) + + if (BWRAP_CAN_NAMESPACE) { + const result = run(command) + expect(result.stdout ?? '').not.toContain('TOPSECRET') + expect(readFileSync(secretPath, 'utf8')).toBe('TOPSECRET\n') + expect(readFileSync(configPath, 'utf8')).toBe('[core]\n') + } + }) + + it('refuses to pin an ancestor that contains a symlink-spelled denyRead tmpfs location', async () => { + // A pin on data would land after the tmpfs (mounted at data/secrets via + // the symlink spelling) and bury it. data stays renameable — the known + // residual — while the nested .git is still pinned. + mkTree(PROJECT, { + data: { + secrets: { 'secret.txt': 'TOPSECRET\n' }, + '.git': { hooks: {}, config: '[core]\n' }, + }, + }) + const dataDir = join(PROJECT, 'data') + const secretsLink = join(PROJECT, 'secrets') + symlinkSync(join('data', 'secrets'), secretsLink) + const secretCanonical = join(dataDir, 'secrets', 'secret.txt') + + const command = await wrap( + { denyRead: [secretsLink] }, + `cat ${secretCanonical} 2>&1; cat ${secretsLink}/secret.txt 2>&1; echo evil >> ${secretCanonical} 2>&1; echo DONE`, + ) + + expect(command).toContain(`--tmpfs ${secretsLink}`) + expect(command).not.toContain(`--bind ${dataDir} ${dataDir}`) + expect(command).toContain( + `--bind ${join(dataDir, '.git')} ${join(dataDir, '.git')}`, + ) + + if (BWRAP_CAN_NAMESPACE) { + const result = run(command) + expect(result.stdout ?? '').not.toContain('TOPSECRET') + expect(readFileSync(secretCanonical, 'utf8')).toBe('TOPSECRET\n') + } + }) + + it('restores a canonical allowWrite carve-out inside a symlink-spelled denyRead', async () => { + mkTree(PROJECT, { + data: { + d: { w: { secret: 'DENYTEST\n' }, 'elsewhere.txt': 'ALSOSECRET\n' }, + }, + }) + const dDir = join(PROJECT, 'data', 'd') + const wDir = join(dDir, 'w') + const secretPath = join(wDir, 'secret') + const linkD = join(PROJECT, 'link-d') + symlinkSync(join('data', 'd'), linkD) + + const command = await wrap( + { allowWrite: [wDir], denyRead: [linkD], denyWrite: [secretPath] }, + `echo n > ${wDir}/newfile.txt 2>&1; echo evil >> ${secretPath} 2>&1; cat ${dDir}/elsewhere.txt 2>&1; echo DONE`, + ) + + // The carve-out's writable re-bind must follow the tmpfs. + const tmpfsOp = `--tmpfs ${linkD}` + const wBind = `--bind ${wDir} ${wDir}` + expect(command).toContain(tmpfsOp) + expect(command.lastIndexOf(wBind)).toBeGreaterThan( + command.lastIndexOf(tmpfsOp), + ) + + if (BWRAP_CAN_NAMESPACE) { + const result = run(command) + expect(existsSync(join(wDir, 'newfile.txt'))).toBe(true) + expect(readFileSync(secretPath, 'utf8')).toBe('DENYTEST\n') + expect(result.stdout ?? '').not.toContain('ALSOSECRET') + } + }) + + it('never pins inside a write-denied directory', async () => { + mkTree(PROJECT, { + x: { + y: { + z: { + app: { + repo: { '.git': { hooks: {}, config: '[core]\n' } }, + 'owned.txt': 'KEEP\n', + }, + }, + }, + }, + }) + const yDir = join(PROJECT, 'x', 'y') + const zDir = join(yDir, 'z') + const appDir = join(zDir, 'app') + const repoDir = join(appDir, 'repo') + const configPath = join(repoDir, '.git', 'config') + + const command = await wrap( + { allowWrite: [zDir], denyRead: [yDir], denyWrite: [appDir, configPath] }, + `echo evil > ${repoDir}/.git/planted 2>&1; echo evil >> ${appDir}/owned.txt 2>&1; echo DONE`, + ) + + expect(command).not.toContain(`--bind ${repoDir} ${repoDir}`) + expect(command).not.toContain(`--bind ${repoDir}/.git ${repoDir}/.git`) + expect(command).toContain(`--ro-bind ${appDir} ${appDir}`) + + if (BWRAP_CAN_NAMESPACE) { + run(command) + expect(existsSync(join(repoDir, '.git', 'planted'))).toBe(false) + expect(readFileSync(join(appDir, 'owned.txt'), 'utf8')).toBe('KEEP\n') + } + }) + + it('still pins inside a denied directory when an allowWrite carve-out inside it makes the corridor writable', async () => { + // repo is denyRead-hidden (its own read-only bind is dropped as + // tmpfs-hidden), but the carve-out sub inside it is restored writable, + // so the corridor between sub and the deeper leaf still needs its pin. + mkTree(PROJECT, { + repo: { sub: { x: { secret: 'DENYTEST\n' } }, 'hidden.txt': 'X\n' }, + }) + const repoDir = join(PROJECT, 'repo') + const subDir = join(repoDir, 'sub') + const xDir = join(subDir, 'x') + const secretPath = join(xDir, 'secret') + + const command = await wrap( + { + allowWrite: [subDir], + denyRead: [repoDir], + denyWrite: [repoDir, secretPath], + }, + `cd ${subDir} && mv x x2 && mkdir -p x && echo evil > ${secretPath}`, + ) + + expect(command).toContain(`--bind ${xDir} ${xDir}`) + + if (BWRAP_CAN_NAMESPACE) { + const result = run(command) + expect(result.status).not.toBe(0) + expect(result.stderr ?? '').toMatch(/busy/i) + expect(existsSync(join(subDir, 'x2'))).toBe(false) + expect(readFileSync(secretPath, 'utf8')).toBe('DENYTEST\n') + } + }) + + it('keeps a deny leaf protected when its carve-out sits inside a denyRead inside a denied directory', async () => { + // denyWrite dir ⊃ denyRead ⊃ allowWrite carve-out ⊃ denyWrite leaf: the + // tmpfs re-application must not restore a write path with an emitted + // deny bind under it. + mkTree(PROJECT, { d: { t: { w: { secret: 'PROTECT\n' } }, 'o.txt': 'X' } }) + const dDir = join(PROJECT, 'd') + const tDir = join(dDir, 't') + const wDir = join(tDir, 'w') + const secretPath = join(wDir, 'secret') + + const command = await wrap( + { allowWrite: [wDir], denyRead: [tDir], denyWrite: [dDir, secretPath] }, + `echo evil >> ${secretPath} 2>&1; echo DONE`, + ) + + const secretRo = `--ro-bind ${secretPath} ${secretPath}` + const wBind = `--bind ${wDir} ${wDir}` + expect(command).toContain(secretRo) + expect(command.lastIndexOf(wBind)).toBeLessThan( + command.lastIndexOf(secretRo), + ) + + if (BWRAP_CAN_NAMESPACE) { + run(command) + expect(readFileSync(secretPath, 'utf8')).toBe('PROTECT\n') + } + }) + + it('pins ancestors of denyRead file masks', async () => { + mkTree(PROJECT, { config: { 'secrets.json': '{"k":"REAL"}\n' } }) + const configDir = join(PROJECT, 'config') + const secretPath = join(configDir, 'secrets.json') + + const command = await wrap( + { denyRead: [secretPath] }, + `cd ${PROJECT} && mv config config-moved && mkdir config && echo attacker > ${secretPath}`, + ) + + expect(command).toContain(`--ro-bind /dev/null ${secretPath}`) + expect(command).toContain(`--bind ${configDir} ${configDir}`) + + if (BWRAP_CAN_NAMESPACE) { + const result = run(command) + expect(result.status).not.toBe(0) + expect(existsSync(join(PROJECT, 'config-moved'))).toBe(false) + expect(readFileSync(secretPath, 'utf8')).toBe('{"k":"REAL"}\n') + } + }) + + it('pins ancestors of credential masks and re-applies the buried mask', async () => { + mkTree(PROJECT, { + creds: { 'token.txt': 'REAL\n' }, + fakes: { 'token.txt': 'FAKE\n' }, + }) + const credsDir = join(PROJECT, 'creds') + const realPath = join(credsDir, 'token.txt') + const fakePath = join(PROJECT, 'fakes', 'token.txt') + process.chdir(PROJECT) + + const command = await wrapCommandWithSandboxLinux({ + command: 'true', + needsNetworkRestriction: false, + allowAllUnixSockets: true, + writeConfig: { allowOnly: [PROJECT], denyWithinAllow: [] }, + maskedFileBinds: [{ realPath, fakePath }], + }) + + expect(command).toContain(`--bind ${credsDir} ${credsDir}`) + // The pin lands after the mask and buries it; the mask re-application + // pass must re-emit the fake on top. + const maskBind = `--ro-bind ${fakePath} ${realPath}` + expect(command.lastIndexOf(maskBind)).toBeGreaterThan( + command.indexOf(`--bind ${credsDir} ${credsDir}`), + ) + }) + + it('never restores a write path that canonically contains another read-deny location', async () => { + mkTree(PROJECT, { d: { w: { secret: 'MASKME\n', 'other.txt': 'ok\n' } } }) + const wDir = join(PROJECT, 'd', 'w') + const secretCanonical = join(wDir, 'secret') + const sLink = join(PROJECT, 's-link') + const dLink = join(PROJECT, 'link') + symlinkSync(join('d', 'w', 'secret'), sLink) + symlinkSync('d', dLink) + + const command = await wrap( + { denyRead: [sLink, dLink], allowWrite: [wDir] }, + `cat ${secretCanonical} 2>&1; echo evil >> ${secretCanonical} 2>&1; echo DONE`, + ) + + const tmpfsOp = `--tmpfs ${dLink}` + const wBind = `--bind ${wDir} ${wDir}` + expect(command).toContain(tmpfsOp) + expect(command.lastIndexOf(wBind)).toBeLessThan( + command.lastIndexOf(tmpfsOp), + ) + + if (BWRAP_CAN_NAMESPACE) { + const result = run(command) + expect(result.stdout ?? '').not.toContain('MASKME') + expect(readFileSync(secretCanonical, 'utf8')).toBe('MASKME\n') + } + }) + + it('keeps a carve-out inside an emitted denied directory read-only across re-application', async () => { + mkTree(PROJECT, { D: { t: { w: { 'file.txt': 'KEEP\n' } } } }) + const DDir = join(PROJECT, 'D') + const tDir = join(DDir, 't') + const wDir = join(tDir, 'w') + + const command = await wrap( + { allowWrite: [wDir], denyWrite: [DDir], denyRead: [tDir] }, + `echo evil > ${wDir}/planted.txt 2>&1; echo DONE`, + ) + + const DRo = `--ro-bind ${DDir} ${DDir}` + const wBind = `--bind ${wDir} ${wDir}` + expect(command).toContain(DRo) + expect(command.lastIndexOf(wBind)).toBeLessThan(command.lastIndexOf(DRo)) + + if (BWRAP_CAN_NAMESPACE) { + run(command) + expect(existsSync(join(wDir, 'planted.txt'))).toBe(false) + } + }) + + it('never restores an allow entry sitting exactly at a masked location', async () => { + mkTree(PROJECT, { d: { w: { secret: 'MASKME\n' } } }) + const secretCanonical = join(PROJECT, 'd', 'w', 'secret') + const sLink = join(PROJECT, 's-link') + const dLink = join(PROJECT, 'link') + symlinkSync(join('d', 'w', 'secret'), sLink) + symlinkSync('d', dLink) + + const command = await wrap( + { denyRead: [sLink, dLink], allowWrite: [secretCanonical] }, + `cat ${secretCanonical} 2>&1; echo evil >> ${secretCanonical} 2>&1; echo DONE`, + ) + + const tmpfsOp = `--tmpfs ${dLink}` + const fileBind = `--bind ${secretCanonical} ${secretCanonical}` + expect(command).toContain(tmpfsOp) + expect(command.lastIndexOf(fileBind)).toBeLessThan( + command.lastIndexOf(tmpfsOp), + ) + + if (BWRAP_CAN_NAMESPACE) { + const result = run(command) + expect(result.stdout ?? '').not.toContain('MASKME') + expect(readFileSync(secretCanonical, 'utf8')).toBe('MASKME\n') + } + }) + + it.if(BWRAP_CAN_NAMESPACE)( + 'blocks renaming .git aside inside the sandbox, and the host tree is untouched', + async () => { + mkTree(PROJECT, { '.git': { hooks: {}, config: '[core]\n' } }) + + const command = await wrap( + {}, + `cd ${PROJECT} && mv .git .git-moved && mkdir .git && echo planted > .git/config`, + ) + const result = run(command) + + expect(result.status).not.toBe(0) + expect(result.stderr ?? '').toMatch(/busy/i) + expect(existsSync(join(PROJECT, '.git-moved'))).toBe(false) + expect(readFileSync(join(PROJECT, '.git', 'config'), 'utf8')).toBe( + '[core]\n', + ) + }, + ) + + it.if(BWRAP_CAN_NAMESPACE)( + 'blocks rmdir and exchange-rename of the pinned directory', + async () => { + mkTree(PROJECT, { '.git': { hooks: {}, config: '[core]\n' } }) + + // renameat2(RENAME_EXCHANGE) must fail EBUSY (errno 16): the python + // probe exits 0 only in that case. + const exchangeProbe = + Bun.which('python3') === null + ? 'true' + : `mkdir exch && python3 -c "import ctypes, os, sys; libc = ctypes.CDLL('libc.so.6', use_errno=True); r = libc.renameat2(-100, b'.git', -100, b'exch', 2); sys.exit(0 if r != 0 and ctypes.get_errno() == 16 else 1)"` + const command = await wrap( + {}, + `cd ${PROJECT} && rmdir .git 2>&1; ${exchangeProbe}`, + ) + const result = run(command) + + expect(result.status).toBe(0) + expect(result.stdout + (result.stderr ?? '')).toMatch(/busy/i) + expect(existsSync(join(PROJECT, '.git'))).toBe(true) + }, + ) + + it.if(BWRAP_CAN_NAMESPACE)( + 'leaves normal work inside the pinned directory and the project intact', + async () => { + mkTree(PROJECT, { + '.git': { hooks: {}, config: '[core]\n' }, + sub: { 'file.txt': 'data\n' }, + }) + + const command = await wrap( + {}, + `cd ${PROJECT} && echo idx > .git/index && mkdir .git/objects && echo n > newfile.txt && mv sub sub-renamed && echo ALL_OK`, + ) + const result = run(command) + + expect(result.stdout).toContain('ALL_OK') + expect(result.status).toBe(0) + expect(existsSync(join(PROJECT, '.git', 'index'))).toBe(true) + expect(existsSync(join(PROJECT, 'sub-renamed'))).toBe(true) + + // A rename STRADDLING the pin boundary crosses vfsmounts and fails + // EXDEV for callers without a copy fallback (os.rename); mv copies. + if (Bun.which('python3') !== null) { + const exdev = await wrap( + {}, + `cd ${PROJECT} && python3 -c "import os, errno, sys; e = 0\ntry: os.rename('README.md', '.git/README.md')\nexcept OSError as err: e = err.errno\nsys.exit(0 if e == errno.EXDEV else 1)"`, + ) + expect(run(exdev).status).toBe(0) + } + }, + ) + + it.if(BWRAP_CAN_NAMESPACE && Bun.which('git') !== null)( + 'lets git and cross-directory renames work in a nested repo whose ancestors are pinned', + async () => { + // packages/app is a nested repo: packages and packages/app/.git are + // pinned (packages/app/.git/config and hooks are mandatory denies + // found by the depth scan; packages/app itself is the .git's parent). + mkTree(PROJECT, { + packages: { + app: { + 'index.js': 'console.log(1)\n', + node_modules: { + '.staging': { 'left-pad-abc': { 'index.js': 'x' } }, + }, + }, + }, + }) + const appDir = join(PROJECT, 'packages', 'app') + const gitInit = spawnSync( + 'git', + ['-c', 'init.defaultBranch=main', 'init', '-q', appDir], + { encoding: 'utf8' }, + ) + expect(gitInit.status).toBe(0) + + const command = await wrap({}) + expect(command).toContain( + `--bind ${join(PROJECT, 'packages')} ${join(PROJECT, 'packages')}`, + ) + expect(command).toContain(`--bind ${appDir} ${appDir}`) + expect(command).toContain( + `--bind ${join(appDir, '.git')} ${join(appDir, '.git')}`, + ) + + // git's ordinary object/index/ref writes, including its atomic + // rename-into-place of lockfiles within .git, all stay inside one + // vfsmount and work as before. + const gitWork = await wrap( + {}, + `cd ${appDir} && git add index.js && git -c user.name=t -c user.email=t@t commit -q -m init && git log --oneline | wc -l && echo GIT_OK`, + ) + const gitResult = run(gitWork) + expect(gitResult.status).toBe(0) + expect(gitResult.stdout).toContain('GIT_OK') + expect(existsSync(join(appDir, '.git', 'refs', 'heads', 'main'))).toBe( + true, + ) + + // The npm-style staging rename (node_modules/.staging/x -> node_modules/ + // x) does not cross a pin: neither directory is a mountpoint, both + // live in the packages/app vfsmount. + const staged = join(appDir, 'node_modules', '.staging', 'left-pad-abc') + const final = join(appDir, 'node_modules', 'left-pad') + const npmLike = await wrap( + {}, + `${process.execPath} -e "require('fs').renameSync(${JSON.stringify(staged)}, ${JSON.stringify(final)})" && echo RENAME_OK`, + ) + const npmResult = run(npmLike) + expect(npmResult.stdout).toContain('RENAME_OK') + expect(existsSync(join(final, 'index.js'))).toBe(true) + + // A rename that straddles a pin boundary (packages/app -> packages, + // i.e. out of the app vfsmount into its parent's) fails EXDEV from + // fs.rename; mv detects EXDEV and falls back to copy + unlink. + const straddle = await wrap( + {}, + `cd ${appDir} && ${process.execPath} -e "try { require('fs').renameSync('index.js', '../index.js'); console.log('NO_ERROR') } catch (e) { console.log('CODE=' + e.code) }" && mv index.js ../moved.js && echo MV_OK`, + ) + const straddleResult = run(straddle) + expect(straddleResult.stdout).toContain('CODE=EXDEV') + expect(straddleResult.stdout).toContain('MV_OK') + expect(existsSync(join(PROJECT, 'packages', 'moved.js'))).toBe(true) + expect(existsSync(join(appDir, 'index.js'))).toBe(false) + }, + ) +}) diff --git a/test/sandbox/linux-mount-plan-record.test.ts b/test/sandbox/linux-mount-plan-record.test.ts new file mode 100644 index 00000000..a7b0616e --- /dev/null +++ b/test/sandbox/linux-mount-plan-record.test.ts @@ -0,0 +1,175 @@ +import { describe, it, expect, afterEach } from 'bun:test' +import { + mkdirSync, + mkdtempSync, + realpathSync, + rmSync, + symlinkSync, + writeFileSync, +} from 'node:fs' +import { tmpdir } from 'node:os' +import { dirname, join } from 'node:path' +import { wrapCommandWithSandboxLinux } from '../../src/sandbox/linux-sandbox-utils.js' +import { isLinux } from '../helpers/platform.js' + +// Arg-level checks of the read-section record the emission filter replays: +// the restore veto counts only mounts already emitted, the filter reads the +// record of restores actually made, and symlink-spelled file masks seed pins +// at their canonical location and are re-applied when a pin buries them. +describe.if(isLinux)('Linux sandbox — mount-plan record and ordering', () => { + const baseParams = { + command: 'true', + needsNetworkRestriction: false, + allowAllUnixSockets: true, + } + + const created: string[] = [] + afterEach(() => { + for (const dir of created.splice(0)) { + rmSync(dir, { recursive: true, force: true }) + } + }) + + function tempTree(files: Record): string { + const proj = realpathSync(mkdtempSync(join(tmpdir(), 'mount-plan-'))) + created.push(proj) + for (const [rel, content] of Object.entries(files)) { + mkdirSync(dirname(join(proj, rel)), { recursive: true }) + writeFileSync(join(proj, rel), content) + } + return proj + } + + it('preserves a carve-out under a denyRead dir when the inner deny mounts later', async () => { + const proj = tempTree({ 'data/build/logs/keep.txt': 'x' }) + const data = join(proj, 'data') + const build = join(proj, 'data', 'build') + const logs = join(proj, 'data', 'build', 'logs') + const wrapped = await wrapCommandWithSandboxLinux({ + ...baseParams, + readConfig: { denyOnly: [data, logs], allowWithinDeny: [] }, + writeConfig: { allowOnly: [build], denyWithinAllow: [] }, + }) + // The restore is a second occurrence of the allowWrite bind, after the + // outer tmpfs; the deeper tmpfs mounts after the restore and on top. + const bind = `--bind ${build} ${build}` + const restoreIdx = wrapped.lastIndexOf(bind) + expect(restoreIdx).toBeGreaterThan(wrapped.indexOf(`--tmpfs ${data}`)) + expect(wrapped.indexOf(`--tmpfs ${logs}`)).toBeGreaterThan(restoreIdx) + }) + + it('drops ancestor pins whose carve-out restore was vetoed', async () => { + const proj = tempTree({ + 'a/t/w/secret-dir/s.txt': 'x', + 'a/t/w/deep/file.txt': 'x', + }) + // Raw spelling sorts shallow; canonical target is deep inside the + // carve-out, so the carve-out's restore would bury it. + symlinkSync(join(proj, 'a/t/w/secret-dir'), join(proj, 's')) + const wrapped = await wrapCommandWithSandboxLinux({ + ...baseParams, + readConfig: { + denyOnly: [join(proj, 's'), join(proj, 'a/t')], + allowWithinDeny: [], + }, + writeConfig: { + allowOnly: [join(proj, 'a/t/w')], + denyWithinAllow: [join(proj, 'a/t/w/deep/file.txt')], + }, + }) + const wBind = `--bind ${join(proj, 'a/t/w')} ${join(proj, 'a/t/w')}` + expect(wrapped.lastIndexOf(wBind)).toBeLessThan( + wrapped.indexOf(`--tmpfs ${join(proj, 'a/t')}`), + ) + expect(wrapped).not.toContain( + `--bind ${join(proj, 'a/t/w/deep')} ${join(proj, 'a/t/w/deep')}`, + ) + }) + + it('emits a deny bind whose region a later unit re-exposed', async () => { + const proj = tempTree({ 'p/q/w/.git/config': 'x' }) + const W = join(proj, 'p/q/w') + const cfg = join(proj, 'p/q/w/.git/config') + // s hides W first; p/q hides it again with its restore vetoed; W's own + // unit then re-binds W host content, so the deny bind is still needed. + symlinkSync(W, join(proj, 's')) + const wrapped = await wrapCommandWithSandboxLinux({ + ...baseParams, + readConfig: { + denyOnly: [join(proj, 's'), join(proj, 'p/q'), W], + allowWithinDeny: [], + }, + writeConfig: { allowOnly: [W], denyWithinAllow: [cfg] }, + }) + const wBind = `--bind ${W} ${W}` + expect(wrapped.lastIndexOf(wBind)).toBeGreaterThan( + wrapped.indexOf(`--tmpfs ${W}`), + ) + expect(wrapped).toContain(`--ro-bind ${cfg} ${cfg}`) + expect(wrapped).toContain( + `--bind ${join(proj, 'p/q/w/.git')} ${join(proj, 'p/q/w/.git')}`, + ) + }) + + it('keeps a symlink-spelled file mask when a denyWrite names its canonical location', async () => { + const proj = tempTree({ 'data/secrets/key.pem': 'SECRET' }) + symlinkSync(join(proj, 'data/secrets'), join(proj, 'secrets')) + const rawSpelling = join(proj, 'secrets', 'key.pem') + const canonical = join(proj, 'data', 'secrets', 'key.pem') + const wrapped = await wrapCommandWithSandboxLinux({ + ...baseParams, + readConfig: { denyOnly: [rawSpelling], allowWithinDeny: [] }, + writeConfig: { allowOnly: [proj], denyWithinAllow: [canonical] }, + }) + expect(wrapped).toContain(`--ro-bind /dev/null ${rawSpelling}`) + expect(wrapped).not.toContain(`--ro-bind ${canonical} ${canonical}`) + }) + + it('emits a deny bind whose raw route is buried but whose canonical location is exposed', async () => { + const proj = tempTree({ 'x/W/secret': 'SECRET', 'z/foo': 'host-content' }) + // W's restore is vetoed (an earlier-sorted mask sits inside it), so the + // raw route reads as covered, but the bind mounts at the canonical dest. + symlinkSync(join(proj, 'x/W/secret'), join(proj, 's')) + symlinkSync(join(proj, 'z'), join(proj, 'x/W/link2')) + const canonicalFoo = join(proj, 'z', 'foo') + const wrapped = await wrapCommandWithSandboxLinux({ + ...baseParams, + readConfig: { + denyOnly: [join(proj, 's'), join(proj, 'x')], + allowWithinDeny: [], + }, + writeConfig: { + allowOnly: [proj, join(proj, 'x/W')], + denyWithinAllow: [join(proj, 'x/W/link2/foo')], + }, + }) + expect(wrapped).toContain(`--ro-bind /dev/null ${join(proj, 'x/W/secret')}`) + expect(wrapped).toContain(`--ro-bind ${canonicalFoo} ${canonicalFoo}`) + }) + + it('pins the canonical parents of a symlink-spelled denyRead file mask and re-applies the buried mask', async () => { + const proj = tempTree({ + 'data/secrets/key.pem': 'SECRET', + 'data/other/thing.txt': 'x', + }) + symlinkSync(join(proj, 'data/secrets'), join(proj, 'secrets')) + const rawSpelling = join(proj, 'secrets', 'key.pem') + const canonicalParent = join(proj, 'data', 'secrets') + const wrapped = await wrapCommandWithSandboxLinux({ + ...baseParams, + readConfig: { denyOnly: [rawSpelling], allowWithinDeny: [] }, + writeConfig: { + allowOnly: [proj], + denyWithinAllow: [join(proj, 'data/other/thing.txt')], + }, + }) + expect(wrapped).toContain(`--bind ${canonicalParent} ${canonicalParent}`) + expect(wrapped).toContain( + `--bind ${join(proj, 'data')} ${join(proj, 'data')}`, + ) + const maskBind = `--ro-bind /dev/null ${rawSpelling}` + const first = wrapped.indexOf(maskBind) + expect(first).toBeGreaterThan(-1) + expect(wrapped.lastIndexOf(maskBind)).toBeGreaterThan(first) + }) +}) From 815f52c912932692fdbfd7aebfd7ad17bd356fd2 Mon Sep 17 00:00:00 2001 From: ant-kurt <209710463+ant-kurt@users.noreply.github.com> Date: Thu, 20 Aug 2026 23:12:11 -0700 Subject: [PATCH 2/2] linux: extract ancestor-pin walk, fix nested-repo test depth and quoting computeAncestorPins is a pure, exported walk with injected probes and direct unit tests. The unverifiable-component abort now names the path and the remedy. The nested-repo behavioral test keeps the repo within the default scan depth, quotes its rename paths correctly, and a new case pins the depth rule. README documents pinned-directory behavior. --- README.md | 2 + src/sandbox/linux-sandbox-utils.ts | 99 ++++++++++++------- test/sandbox/compute-ancestor-pins.test.ts | 74 ++++++++++++++ test/sandbox/linux-ancestor-pin-errno.test.ts | 2 +- test/sandbox/linux-ancestor-pin.test.ts | 63 ++++++++---- 5 files changed, 186 insertions(+), 54 deletions(-) create mode 100644 test/sandbox/compute-ancestor-pins.test.ts diff --git a/README.md b/README.md index 14d89686..b6d3ac72 100644 --- a/README.md +++ b/README.md @@ -680,6 +680,8 @@ $ srt 'echo "bad" > .git/hooks/pre-commit' **Note (Linux):** On Linux, mandatory deny paths only block files that already exist. Non-existent files in these patterns cannot be blocked by bubblewrap's bind-mount approach. macOS uses glob patterns which block both existing and new files. +**Pinned directories (Linux):** Every directory between a protected path and the allowed write root is bind-mounted over itself so it cannot be renamed or removed from inside the sandbox: `mv` of a nested repository's parent fails with `EBUSY` ("Device or resource busy"), and `rm -rf` of a nested repository leaves an empty husk behind (as it already did for `.git/hooks`). Reads, writes and creation inside a pinned directory are unaffected, but a rename that crosses a pin boundary returns `EXDEV` to callers without a copy fallback (`mv` copies instead). + **Linux search depth:** On Linux, the sandbox uses `ripgrep` to scan for dangerous files in subdirectories within allowed write paths. By default, it searches up to 3 levels deep for performance. You can configure this with `mandatoryDenySearchDepth`: ```json diff --git a/src/sandbox/linux-sandbox-utils.ts b/src/sandbox/linux-sandbox-utils.ts index bd460139..680e41e9 100644 --- a/src/sandbox/linux-sandbox-utils.ts +++ b/src/sandbox/linux-sandbox-utils.ts @@ -817,6 +817,56 @@ function buildSandboxCommand( } } +/** Prefix that a strict descendant of `dir` starts with ('/' for the root). */ +function pathSep(dir: string): string { + return dir === '/' ? '/' : dir + '/' +} + +/** True when an fs error means the path is absent, as opposed to + * unreadable (EACCES), looping (ELOOP) or otherwise unverifiable. */ +function isAbsenceError(err: unknown): boolean { + const code = (err as NodeJS.ErrnoException | undefined)?.code + return code === 'ENOENT' || code === 'ENOTDIR' +} + +/** + * Directories to pin with a self --bind: every ancestor of a deny dest or + * mask seed up to (and excluding) the outermost covering allowed write root, + * shallow-first. Filesystem and policy probes are injected so the walk is + * testable without bwrap. + */ +export function computeAncestorPins( + seeds: Iterable, + probes: { + isWithinAllowedWrite: (dir: string) => boolean + isAllowedWriteRoot: (dir: string) => boolean + isExcluded: (dir: string) => boolean + containsReadDenyTmpfs: (dir: string) => boolean + isAbsent: (dir: string) => boolean + }, +): string[] { + const pins = new Set() + // Verdicts are seed-independent: a visited ancestor's chain is done. + const visited = new Set() + for (const seed of seeds) { + let dir = path.dirname(seed) + while (dir !== '/' && probes.isWithinAllowedWrite(dir)) { + if (visited.has(dir)) break + visited.add(dir) + if ( + !probes.isAllowedWriteRoot(dir) && + !probes.isExcluded(dir) && + !probes.containsReadDenyTmpfs(dir) && + !probes.isAbsent(dir) + ) { + pins.add(dir) + } + dir = path.dirname(dir) + } + } + return [...pins].sort((a, b) => a.split('/').length - b.split('/').length) +} + /** * bwrap cannot create a file bind mount point over a destination that is * itself a symlink — `--ro-bind /dev/null ` fails with "Can't create @@ -830,18 +880,6 @@ function buildSandboxCommand( * carve-outs expressed against the symlink path (e.g. /bin on usr-merged * systems). */ -/** Prefix that a strict descendant of `dir` starts with ('/' for the root). */ -function pathSep(dir: string): string { - return dir === '/' ? '/' : dir + '/' -} - -/** True when an fs error means the path is absent, as opposed to - * unreadable (EACCES), looping (ELOOP) or otherwise unverifiable. */ -function isAbsenceError(err: unknown): boolean { - const code = (err as NodeJS.ErrnoException | undefined)?.code - return code === 'ENOENT' || code === 'ENOTDIR' -} - function resolveSymlinkDenyDest(normalizedPath: string): string { try { if (fs.lstatSync(normalizedPath).isSymbolicLink()) { @@ -1636,25 +1674,16 @@ async function generateFilesystemArgs( return isAbsenceError(err) } } - const ancestorPinDirs = new Set() - // Verdicts are dest-independent: a visited ancestor's chain is done. - const visitedAncestors = new Set() - for (const dest of [...denyWriteDests, ...maskPinSeeds]) { - let ancestorDir = path.dirname(dest) - while (ancestorDir !== '/' && isWithinAnyAllowedWritePath(ancestorDir)) { - if (visitedAncestors.has(ancestorDir)) break - visitedAncestors.add(ancestorDir) - if ( - !isAllowedWriteRoot(ancestorDir) && - !excludedFromPinning(ancestorDir) && - !containsProspectiveReadDenyTmpfs(ancestorDir) && - !ancestorIsAbsent(ancestorDir) - ) { - ancestorPinDirs.add(ancestorDir) - } - ancestorDir = path.dirname(ancestorDir) - } - } + const ancestorPinDirs = computeAncestorPins( + [...denyWriteDests, ...maskPinSeeds], + { + isWithinAllowedWrite: isWithinAnyAllowedWritePath, + isAllowedWriteRoot, + isExcluded: excludedFromPinning, + containsReadDenyTmpfs: containsProspectiveReadDenyTmpfs, + isAbsent: ancestorIsAbsent, + }, + ) // bwrap re-resolves the pin path at mount time, so re-verify that no // component is a symlink (narrows, does not close, the check→mount race). // A symlink component drops the pin; an lstat error other than absence @@ -1669,8 +1698,8 @@ async function generateFilesystemArgs( if (!isAbsenceError(err)) { const code = (err as NodeJS.ErrnoException | undefined)?.code throw new Error( - `Sandbox ancestor-pin verification failed: cannot lstat ${prefix} (${code ?? String(err)}). ` + - 'Refusing to build a mount plan with unverifiable pin components.', + `Sandbox cannot verify ${prefix} (${code ?? String(err)}), which lies between an allowed write path and a protected path. ` + + 'Fix its permissions or remove the covering path from allowWrite.', ) } verdict = null @@ -1701,9 +1730,7 @@ async function generateFilesystemArgs( return true } const ancestorPinArgs: string[] = [] - for (const pinDir of [...ancestorPinDirs].sort( - (a, b) => a.split('/').length - b.split('/').length, - )) { + for (const pinDir of ancestorPinDirs) { if (pinComponentsAreSymlinkFree(pinDir)) { ancestorPinArgs.push('--bind', pinDir, pinDir) } diff --git a/test/sandbox/compute-ancestor-pins.test.ts b/test/sandbox/compute-ancestor-pins.test.ts new file mode 100644 index 00000000..4cb0f8de --- /dev/null +++ b/test/sandbox/compute-ancestor-pins.test.ts @@ -0,0 +1,74 @@ +import { describe, it, expect } from 'bun:test' +import { computeAncestorPins } from '../../src/sandbox/linux-sandbox-utils.js' + +// Pure walk over the deny-dest seeds; probes are injected so this runs on +// every platform. +describe('computeAncestorPins', () => { + const under = (root: string) => (dir: string) => + dir === root || dir.startsWith(root + '/') + const none = () => false + const probes = ( + roots: string[], + overrides: Partial[1]> = {}, + ) => ({ + isWithinAllowedWrite: (dir: string) => roots.some(r => under(r)(dir)), + isAllowedWriteRoot: (dir: string) => roots.includes(dir), + isExcluded: none, + containsReadDenyTmpfs: none, + isAbsent: none, + ...overrides, + }) + + it('pins every directory strictly between the dest and the write root', () => { + expect(computeAncestorPins(['/w/a/b/.git/config'], probes(['/w']))).toEqual( + ['/w/a', '/w/a/b', '/w/a/b/.git'], + ) + }) + + it('pins nothing for a dest outside every write root', () => { + expect(computeAncestorPins(['/x/a/leaf'], probes(['/w']))).toEqual([]) + }) + + it('skips directories at or below a deny dest via isExcluded', () => { + const pins = computeAncestorPins( + ['/w/app', '/w/app/repo/.git/config'], + probes(['/w'], { isExcluded: under('/w/app') }), + ) + expect(pins).toEqual([]) + }) + + it('skips a directory at or above a read-deny tmpfs', () => { + const tmpfs = '/w/x/y' + const pins = computeAncestorPins( + ['/w/x/y/z/.git/config'], + probes(['/w'], { + containsReadDenyTmpfs: dir => under(dir)(tmpfs), + }), + ) + expect(pins).toEqual(['/w/x/y/z', '/w/x/y/z/.git']) + }) + + it('skips absent directories', () => { + const pins = computeAncestorPins( + ['/w/a/missing/leaf'], + probes(['/w'], { isAbsent: dir => dir === '/w/a/missing' }), + ) + expect(pins).toEqual(['/w/a']) + }) + + it('continues past a nested write root up to the outermost one', () => { + const pins = computeAncestorPins( + ['/w/x/y/z/.git/config'], + probes(['/w', '/w/x/y/z']), + ) + expect(pins).toEqual(['/w/x', '/w/x/y', '/w/x/y/z/.git']) + }) + + it('orders pins shallow-first across seeds and dedupes shared prefixes', () => { + const pins = computeAncestorPins( + ['/w/a/b/c/leaf', '/w/a/d/leaf', '/w/e/leaf'], + probes(['/w']), + ) + expect(pins).toEqual(['/w/a', '/w/e', '/w/a/b', '/w/a/d', '/w/a/b/c']) + }) +}) diff --git a/test/sandbox/linux-ancestor-pin-errno.test.ts b/test/sandbox/linux-ancestor-pin-errno.test.ts index 0e9288ea..a7e71804 100644 --- a/test/sandbox/linux-ancestor-pin-errno.test.ts +++ b/test/sandbox/linux-ancestor-pin-errno.test.ts @@ -130,7 +130,7 @@ describe.if(isLinux)( ) // eslint-disable-next-line @typescript-eslint/await-thenable -- bun:test types .rejects.toThrow() as void; the await is required at runtime await expect(wrap(proj)).rejects.toThrow( - /cannot lstat .*Refusing to build a mount plan/s, + /cannot verify .*Fix its permissions/s, ) expect(lstatHits).toBeGreaterThan(0) }) diff --git a/test/sandbox/linux-ancestor-pin.test.ts b/test/sandbox/linux-ancestor-pin.test.ts index fd7eb08b..88eaddc3 100644 --- a/test/sandbox/linux-ancestor-pin.test.ts +++ b/test/sandbox/linux-ancestor-pin.test.ts @@ -604,23 +604,55 @@ describe.if(isLinux)('Linux sandbox — denyWrite ancestor pinning', () => { }, ) + it('does not pin above a nested repo deeper than the mandatory-deny scan depth', async () => { + // a/b/c/.git/config sits at depth 4; the default scan depth of 3 never + // finds it, so there is no deny bind there and nothing to pin. Raising + // the depth finds it and pins the whole chain. + mkTree(PROJECT, { + a: { b: { c: { '.git': { hooks: {}, config: '[core]\n' } } } }, + }) + const cDir = join(PROJECT, 'a', 'b', 'c') + const gitDir = join(cDir, '.git') + + const shallow = await wrap() + expect(shallow).not.toContain(`--ro-bind ${gitDir}/config`) + expect(shallow).not.toContain(`--bind ${gitDir} ${gitDir}`) + expect(shallow).not.toContain(`--bind ${cDir} ${cDir}`) + + process.chdir(PROJECT) + const deep = await wrapCommandWithSandboxLinux({ + command: 'true', + needsNetworkRestriction: false, + allowAllUnixSockets: true, + mandatoryDenySearchDepth: 4, + writeConfig: { allowOnly: [PROJECT], denyWithinAllow: [] }, + }) + expect(deep).toContain(`--ro-bind ${gitDir}/config ${gitDir}/config`) + for (const dir of [ + join(PROJECT, 'a'), + join(PROJECT, 'a', 'b'), + cDir, + gitDir, + ]) { + expect(deep).toContain(`--bind ${dir} ${dir}`) + } + }) + it.if(BWRAP_CAN_NAMESPACE && Bun.which('git') !== null)( 'lets git and cross-directory renames work in a nested repo whose ancestors are pinned', async () => { - // packages/app is a nested repo: packages and packages/app/.git are - // pinned (packages/app/.git/config and hooks are mandatory denies - // found by the depth scan; packages/app itself is the .git's parent). + // app is a nested repo at depth 1: app/.git/config and hooks are + // mandatory denies found by the depth scan, so app and app/.git are + // pinned. mkTree(PROJECT, { - packages: { - app: { - 'index.js': 'console.log(1)\n', - node_modules: { - '.staging': { 'left-pad-abc': { 'index.js': 'x' } }, - }, + app: { + 'index.js': 'console.log(1)\n', + node_modules: { + '.staging': { 'left-pad-abc': { 'index.js': 'x' } }, }, }, }) - const appDir = join(PROJECT, 'packages', 'app') + const appDir = join(PROJECT, 'app') const gitInit = spawnSync( 'git', ['-c', 'init.defaultBranch=main', 'init', '-q', appDir], @@ -629,9 +661,6 @@ describe.if(isLinux)('Linux sandbox — denyWrite ancestor pinning', () => { expect(gitInit.status).toBe(0) const command = await wrap({}) - expect(command).toContain( - `--bind ${join(PROJECT, 'packages')} ${join(PROJECT, 'packages')}`, - ) expect(command).toContain(`--bind ${appDir} ${appDir}`) expect(command).toContain( `--bind ${join(appDir, '.git')} ${join(appDir, '.git')}`, @@ -653,18 +682,18 @@ describe.if(isLinux)('Linux sandbox — denyWrite ancestor pinning', () => { // The npm-style staging rename (node_modules/.staging/x -> node_modules/ // x) does not cross a pin: neither directory is a mountpoint, both - // live in the packages/app vfsmount. + // live in the app vfsmount. const staged = join(appDir, 'node_modules', '.staging', 'left-pad-abc') const final = join(appDir, 'node_modules', 'left-pad') const npmLike = await wrap( {}, - `${process.execPath} -e "require('fs').renameSync(${JSON.stringify(staged)}, ${JSON.stringify(final)})" && echo RENAME_OK`, + `${process.execPath} -e "require('fs').renameSync('${staged}', '${final}')" && echo RENAME_OK`, ) const npmResult = run(npmLike) expect(npmResult.stdout).toContain('RENAME_OK') expect(existsSync(join(final, 'index.js'))).toBe(true) - // A rename that straddles a pin boundary (packages/app -> packages, + // A rename that straddles a pin boundary (app -> PROJECT, // i.e. out of the app vfsmount into its parent's) fails EXDEV from // fs.rename; mv detects EXDEV and falls back to copy + unlink. const straddle = await wrap( @@ -674,7 +703,7 @@ describe.if(isLinux)('Linux sandbox — denyWrite ancestor pinning', () => { const straddleResult = run(straddle) expect(straddleResult.stdout).toContain('CODE=EXDEV') expect(straddleResult.stdout).toContain('MV_OK') - expect(existsSync(join(PROJECT, 'packages', 'moved.js'))).toBe(true) + expect(existsSync(join(PROJECT, 'moved.js'))).toBe(true) expect(existsSync(join(appDir, 'index.js'))).toBe(false) }, )