From 63b5cb488deecd1e5d39e239bfc3d6d9a4b5bd39 Mon Sep 17 00:00:00 2001 From: Ron Leizrowice Date: Sat, 10 Oct 2026 21:56:09 +0000 Subject: [PATCH 1/8] A write deny keeps the symbolic links on the way to it in place A denyWrite entry or mandatory deny path may have a symbolic link on the way to it, its last name included. Such a deny covers the path the links lead to, whether or not anything is there yet; each link, which cannot be removed, renamed or replaced; and the directories above them. macOS: the deny is emitted as written and where the links lead, and each link is kept by a literal filter. Linux: bubblewrap cannot mount on a link, so the seccomp helper gains --hold-link , which mounts the link on itself, read-only. A helper without the option is recognised and used as before, and SandboxManager.getLinuxUnheldLinks() then names the links. --- README.md | 8 +- src/sandbox/linux-sandbox-utils.ts | 88 ++- src/sandbox/macos-sandbox-utils.ts | 32 +- src/sandbox/sandbox-manager.ts | 8 + src/sandbox/sandbox-utils.ts | 43 ++ test/docker-weak-sandbox.test.ts | 20 +- test/sandbox/write-deny-through-links.test.ts | 564 ++++++++++++++++++ vendor/seccomp-src/apply-seccomp.c | 179 ++++-- 8 files changed, 887 insertions(+), 55 deletions(-) create mode 100644 test/sandbox/write-deny-through-links.test.ts diff --git a/README.md b/README.md index 5cafd7bcb..cb75488ad 100644 --- a/README.md +++ b/README.md @@ -587,7 +587,7 @@ Examples: - `allowPty` - macOS only: allow pseudo-terminal operations (boolean, default: false). The seatbelt profile then carries `(allow pseudo-tty)` plus read, write and `ioctl` on `/dev/ptmx` and `/dev/ttys*`, which a command that allocates a pty of its own needs. Not read on Linux or Windows. - `bwrapPath` - Linux only: absolute path to the `bwrap` (bubblewrap) binary, used instead of resolving `bwrap` on `PATH`. A path that is not executable is a dependency error, and no `PATH` lookup is tried after it. - `socatPath` - Linux only: absolute path to the `socat` binary, used instead of resolving `socat` on `PATH`, with the same rule for a path that is not executable. -- `seccomp` - Linux only: `applyPath` points at the `apply-seccomp` binary to use instead of the packaged one, which is still used when nothing exists at that path. `argv0` invokes it as a multicall binary that dispatches on the `ARGV0` environment variable; `applyPath` is then used verbatim (no existence check) and must resolve inside the bubblewrap namespace. +- `seccomp` - Linux only: `applyPath` points at the `apply-seccomp` binary to use instead of the packaged one, which is still used when nothing exists at that path. One built from older sources is used for what it can do (see "Write denies and symbolic links"). `argv0` invokes it as a multicall binary that dispatches on the `ARGV0` environment variable; `applyPath` is then used verbatim (no existence check) and must resolve inside the bubblewrap namespace. - `ripgrep` - Linux only: how to invoke ripgrep for the deny-path scan (default: `{ "command": "rg" }`). `args` are passed before srt's own arguments, and `argv0` overrides `argv[0]` for a multicall binary. - `git.safeDirectories` - Directories git should treat as `safe.directory` inside the sandbox, where the working tree is owned by another user — the repository top level when the command runs from a subdirectory, say. Injected through `GIT_CONFIG_*` environment variables, and it grants no write access of its own. - `credentials` - Environment variables (`credentials.envVars`) and files (`credentials.files`) the sandboxed command must not read the real values of. `mode: "deny"` withholds the value; `mode: "mask"` puts a sentinel in its place inside the sandbox and has the proxy substitute the real value back on egress to the hosts that entry's `injectHosts` lists (default: `network.allowedDomains`). Masking requires `network.tlsTerminate`, so the real value only leaves over a verified TLS connection, unless `credentials.allowPlaintextInject` opts out. Files are masked on Linux only; macOS denies a masked file instead. @@ -893,6 +893,12 @@ Filesystem restrictions are enforced at the OS level: **Writes through links (Linux and macOS):** a write is judged by where it lands, after symbolic links are followed. A link inside an allowed write path that leads out of it gives a command nothing: the write is refused. A link that leads into an allowed write path works like any other name for that place. A hard link is a second name for the file itself: on Linux a file outside the allowed write paths that already has a hard link inside one can be written through it, and a sandboxed command cannot create such a link. On macOS nothing is promised for such a file. +**Write denies and symbolic links (Linux and macOS):** a `denyWrite` entry or a [mandatory deny path](#mandatory-deny-paths-auto-protected-files) with a symbolic link on the way to it, its last name included, denies the path the links lead to, whether or not anything is there yet, and keeps each of those links where it is: it cannot be removed, renamed or replaced, and neither can the directories above it, as above any denied path. The link reads and leads as before, and everything else in the directory that holds it stays as writable as it was. The links are those found when the command is wrapped. + +- On Linux bubblewrap cannot mount on a link, so the seccomp helper does (`apply-seccomp --hold-link `): each link that lies in a directory the command can write to is mounted on itself, read-only, and removing or replacing it fails with `EBUSY` ("Device or resource busy"). A command is not started if a link has become something else since the wrap. +- Not held on Linux: with no helper in use (`allowAllUnixSockets`, or no binary for the architecture), or with one built before it had the option (`seccomp.applyPath`), what the links lead to is denied all the same, and the links are ordinary entries of a writable directory. `SandboxManager.getLinuxUnheldLinks()` returns every such link of the commands wrapped so far, for the embedder to show or to look at again after the command; the only other sign is a line under `SRT_DEBUG`. +- Not covered: a mandatory deny name _below_ the working directory that is itself a link. On Linux the scan lists files and does not see it. On macOS the pattern keeps the name in place and says nothing of what it leads to. + **Read-side rules (Linux):** an entry is matched by the name it is, not by what it points at. - An `allowRead` entry re-allows the name it names, so a link planted at an allowed path cannot re-open a denied one. One that is itself a symlink is bound back at its own name, from the target it was checked against: the target's own path stays hidden, and the target's contents are reachable only through the name. A `denyRead` entry or a credential mask that overlaps that target — at it, inside it, or covering it — wins over the carve-out, which is then not restored at all: the name is absent inside the sandbox rather than serving what the deny hides. The denied directory the carve-out is being restored into is not such an overlap, nor is anything above it: those are what the carve-out is an exception to. Neither is an entry that denies nothing — one naming a path that is not there, or a file an `allowRead` entry lifts. diff --git a/src/sandbox/linux-sandbox-utils.ts b/src/sandbox/linux-sandbox-utils.ts index 661530a83..03607be3e 100644 --- a/src/sandbox/linux-sandbox-utils.ts +++ b/src/sandbox/linux-sandbox-utils.ts @@ -19,6 +19,7 @@ import { encodeSandboxedCommand, attributionKeyFor, DANGEROUS_FILES, + followLinks, isAbsenceErrno, isAtOrUnder, isStrictlyUnder, @@ -1503,6 +1504,48 @@ export async function initializeLinuxNetworkBridge( } } +// Keyed by how the helper is run. Only an answer is kept: a probe that failed +// is made again the next time. +const holdsLinksProbes = new Map() + +/** + * Whether the seccomp helper takes `--hold-link`. The one in use need not be + * the one this package ships, and one built from older sources takes the word + * for the command to run. + */ +function seccompHelperHoldsLinks( + applyPath: string | undefined, + argv0: string | undefined, +): boolean { + const helper = argv0 ? applyPath : getApplySeccompBinaryPath(applyPath) + if (!helper) return false + const key = JSON.stringify([helper, argv0]) + const cached = holdsLinksProbes.get(key) + if (cached !== undefined) return cached + const probe = spawnSync(helper, ['--holds-links'], { + timeout: 5000, + stdio: 'ignore', + // THREAT: this runs outside any sandbox, and an older helper looks the + // word up on PATH as a program. So there is nowhere to find one. + env: { PATH: '/dev/null', ...(argv0 ? { ARGV0: argv0 } : {}) }, + }) + if (probe.error !== undefined || probe.status === null) return false + holdsLinksProbes.set(key, probe.status === 0) + return probe.status === 0 +} + +const unheldLinks = new Set() + +/** + * The symbolic links that a write deny of some command wrapped so far leads + * through, that lie in a directory that command could write to, and that + * nothing kept in place there: the command could put something else at the + * name. See "Write denies and symbolic links" in README.md. + */ +export function getLinuxUnheldLinks(): string[] { + return [...unheldLinks] +} + /** * Resolve how to invoke apply-seccomp: either a standalone binary path, or a * multicall-binary prefix that dispatches on the ARGV0 env var. @@ -1875,7 +1918,11 @@ async function generateFilesystemArgs( mandatoryDenySearchDepth: number = DEFAULT_MANDATORY_DENY_SEARCH_DEPTH, allowGitConfig = false, abortSignal?: AbortSignal, -): Promise<{ args: string[]; mandatoryDenyArgs: number }> { +): Promise<{ + args: string[] + mandatoryDenyArgs: number + linksToHold: string[] +}> { const args: string[] = [] const startedIn = workingDirectory() // fs already imported @@ -1888,6 +1935,9 @@ async function generateFilesystemArgs( const denyWriteArgs: string[] = [] // Where the mandatory denies' binds begin in it: after the configuration's. let mandatoryDenyArgsStart = Infinity + // Every symbolic link on the way to a write-denied path, where it lies. The + // deny's own mount is where they lead, and bubblewrap can put none on a link. + const linksToDenies = new Set() // Directories that a deny entry re-binds read-only inside the sandbox // (--ro-bind ), keyed by resolved dest, with every raw // (pre-resolution) spelling each was reached through. A non-existent deny @@ -2564,6 +2614,7 @@ async function generateFilesystemArgs( if (rawPath.startsWith('/dev/')) { continue } + for (const link of followLinks(rawPath).links) linksToDenies.add(link) // Resolve-before-mask: normalizePathForSandbox keeps the raw symlink // path whenever resolution crosses isSymlinkOutsideBoundary (the @@ -3025,9 +3076,21 @@ async function generateFilesystemArgs( // each: a symlink spelling would pin the chain of the link, not of where // the mount sits. Deny binds, tmpfs units and masks are emitted later and // land on top of both pins and covers. + // + // A link is held where the command could replace it: whoever replaces one + // decides what the denied name leads to. That is in a directory it can write + // to, and sees as it is on the host. The directories above a held link are + // pinned like those above a mount. + const linksToHold = [...linksToDenies].filter( + link => + isWithinAnyAllowedWritePath(path.dirname(link)) && + !KERNEL_TOP_LEVEL_DIRS.some(dir => isAtOrUnder(link, dir)) && + !isHiddenByTmpfs(path.dirname(link), 'writes and reads'), + ) const pinArgs = ancestorPinArgs( [ ...denyWriteRawDests.keys(), + ...linksToHold, ...fileMasks.map(mask => mask.landing), ...readDenyTmpfsUnits.map(unit => unit.landing), ], @@ -3204,7 +3267,7 @@ async function generateFilesystemArgs( args.push('--ro-bind', emptySource, emptySource) } - return { args, mandatoryDenyArgs } + return { args, mandatoryDenyArgs, linksToHold } } /** @@ -3496,7 +3559,11 @@ export async function wrapCommandWithSandboxLinux( } // ========== FILESYSTEM RESTRICTIONS ========== - const { args: fsArgs, mandatoryDenyArgs } = await generateFilesystemArgs( + const { + args: fsArgs, + mandatoryDenyArgs, + linksToHold, + } = await generateFilesystemArgs( readConfig, writeConfig, maskedFileBinds, @@ -3506,6 +3573,21 @@ export async function wrapCommandWithSandboxLinux( allowGitConfig, abortSignal, ) + if ( + linksToHold.length > 0 && + applySeccompPrefix !== undefined && + seccompHelperHoldsLinks(seccompConfig?.applyPath, seccompConfig?.argv0) + ) { + applySeccompPrefix += `${quote(linksToHold.flatMap(link => ['--hold-link', link]))} ` + } else { + for (const link of linksToHold) { + unheldLinks.add(link) + logForDebugging( + `[Sandbox Linux] A write deny leads through the symbolic link ${link}, which the command can replace: holding it takes a seccomp helper with --hold-link`, + { level: 'warn' }, + ) + } + } const mountsStart = bwrapArgs.length bwrapArgs.push(...fsArgs) const mounts = { diff --git a/src/sandbox/macos-sandbox-utils.ts b/src/sandbox/macos-sandbox-utils.ts index 8c84d5bfb..a93b04981 100644 --- a/src/sandbox/macos-sandbox-utils.ts +++ b/src/sandbox/macos-sandbox-utils.ts @@ -14,6 +14,7 @@ import { containsGlobChars, globToRegex, denyGlobRegex, + followLinks, isAtOrUnder, isStrictlyUnder as isPathStrictlyUnder, nameLocation, @@ -367,9 +368,17 @@ function inBothSpellings(kind: PathListKind, entry: PathEntry): PathEntry[] { kind === 'deny' || entry.glob ? resolved(base) : nameLocation(base, resolved) - if (canonical === base || canonical === '/') return [entry] + return [entry, ...startingFrom(canonical, entry)] +} + +/** + * `entry` with `canonical` for the path it is, or for the directory its + * pattern starts from; nothing where that is no other spelling, or the root. + */ +function startingFrom(canonical: string, entry: PathEntry): PathEntry[] { + const base = entryBaseDir(entry) + if (canonical === base || canonical === '/') return [] return [ - entry, entry.glob ? { glob: true, @@ -992,12 +1001,18 @@ function generateWriteRules( // Combine user-specified and mandatory deny patterns (no ripgrep needed on // macOS). The caller's spellings are patterns when they read as patterns; // the mandatory entries carry their own literal/glob split. - const denyEntries = [ + const walked = [ ...(config.denyWithinAllow || []).map(toPathEntry), ...(config.literalDenyWithinAllow ?? []).map(toLiteralPathEntry), ...macGetMandatoryDenyEntries(allowGitConfig), - ] - denyEntries.push(...alsoReadAs(config.denyWithinAllow, 'deny')) + ...alsoReadAs(config.denyWithinAllow, 'deny'), + ].map(entry => ({ entry, ...followLinks(entryBaseDir(entry)) })) + // As written, and where the links on the way lead, there or not: that is the + // path Seatbelt compares a write through them with. + const denyEntries = walked.flatMap(({ entry, end }) => [ + entry, + ...(end === undefined ? [] : startingFrom(end, entry)), + ]) const { groups, rest: ungrouped } = groupLiteralDenyPaths(denyEntries) const groupFilters = groups.map(literalDenyGroupFilters) @@ -1023,6 +1038,13 @@ function generateWriteRules( moveFilters.add(`(literal ${escapePath(ancestorDir)})`) } } + // Each of those links stays where it lies: whoever replaces one decides + // what the denied name leads to. + for (const link of new Set(walked.flatMap(({ links }) => links))) { + for (const kept of [link, ...getAncestorDirectories(link)]) { + moveFilters.add(`(literal ${escapePath(kept)})`) + } + } rules.push( ...renderRule( 'deny', diff --git a/src/sandbox/sandbox-manager.ts b/src/sandbox/sandbox-manager.ts index 04b9cdf10..643e5e1b1 100644 --- a/src/sandbox/sandbox-manager.ts +++ b/src/sandbox/sandbox-manager.ts @@ -63,6 +63,7 @@ import { checkLinuxDependencies, type SandboxDependencyCheck, cleanupBwrapMountPoints, + getLinuxUnheldLinks, linuxGetCwdMandatoryDenyPaths, } from './linux-sandbox-utils.js' import { expandReadDenyGlobLinuxSteps } from './read-deny-glob.js' @@ -3045,6 +3046,12 @@ export interface ISandboxManager { getSandboxViolationStore(): SandboxViolationStore annotateStderrWithSandboxFailures(command: string, stderr: string): string getLinuxGlobPatternWarnings(): string[] + /** + * Linux: the symbolic links on the way to a write-denied path that a + * command wrapped by this process could replace, for the embedder to show + * or to look at again afterwards. Empty where every one was held in place. + */ + getLinuxUnheldLinks(): string[] getConfig(): SandboxRuntimeConfig | undefined getMitmCA(): MitmCA | undefined getSentinelRegistry(): SentinelRegistry @@ -3097,6 +3104,7 @@ export const SandboxManager: ISandboxManager = { getSandboxViolationStore, annotateStderrWithSandboxFailures, getLinuxGlobPatternWarnings, + getLinuxUnheldLinks, getConfig, updateConfig, } as const diff --git a/src/sandbox/sandbox-utils.ts b/src/sandbox/sandbox-utils.ts index 03f6c688b..e0ea17b27 100644 --- a/src/sandbox/sandbox-utils.ts +++ b/src/sandbox/sandbox-utils.ts @@ -107,6 +107,49 @@ export function nameLocation( return `${parent === '/' ? '' : parent}/${path.basename(p)}` } +/** As many links as the kernel follows on the way to one path. */ +const MAX_LINKS_FOLLOWED = 40 + +/** + * The absolute path `p` walked name by name: the symbolic links met on the + * way, each where it lies, and where the way ends. A `..` in a link's target + * is the parent of the directory reached, as it is to the kernel. A name that + * is not there is taken as written, so a dangling link ends at the path a + * write through it would create. There is no end past the number of links the + * kernel follows, nor past a target that is no text. + */ +export function followLinks(p: string): { + links: string[] + end: string | undefined +} { + const links: string[] = [] + const rest = path.resolve(p).split('/') + // No link in it. + let reached = '' + for (let name = rest.shift(); name !== undefined; name = rest.shift()) { + if (name === '' || name === '.') continue + if (name === '..') { + reached = reached.slice(0, reached.lastIndexOf('/')) + continue + } + const next = `${reached}/${name}` + let target: string + try { + target = fs.readlinkSync(next) + } catch { + reached = next + continue + } + links.push(next) + if (links.length > MAX_LINKS_FOLLOWED || target.includes('\uFFFD')) { + return { links, end: undefined } + } + if (target.startsWith('/')) reached = '' + rest.unshift(...target.split('/')) + } + return { links, end: reached || '/' } +} + /** `process.cwd()`, or undefined where that throws: it has been removed. */ export function workingDirectory(): string | undefined { try { diff --git a/test/docker-weak-sandbox.test.ts b/test/docker-weak-sandbox.test.ts index 66cedbb65..6a192ab37 100644 --- a/test/docker-weak-sandbox.test.ts +++ b/test/docker-weak-sandbox.test.ts @@ -19,8 +19,10 @@ import { spawnSync } from 'node:child_process' import { mkdirSync, rmSync, + symlinkSync, writeFileSync, readFileSync, + readlinkSync, existsSync, } from 'node:fs' import { tmpdir } from 'node:os' @@ -33,6 +35,7 @@ describe.if(inDocker)('srt end-to-end as uid 0 in a container', () => { const ALLOWED = join(WORK, 'allowed') const DENIED = join(WORK, 'denied') const SECRET = join(WORK, 'secret') + const LINK = join(ALLOWED, 'link') const CONFIG = join(WORK, 'srt.json') // Same policy without the seccomp helper: the command then runs in bwrap's // own namespaces, where --cap-drop ALL is the only thing between it and the @@ -71,6 +74,7 @@ describe.if(inDocker)('srt end-to-end as uid 0 in a container', () => { mkdirSync(DENIED, { recursive: true }) mkdirSync(SECRET, { recursive: true }) writeFileSync(join(SECRET, 'key'), 'TOPSECRET') + symlinkSync('leads-to', LINK) writeFileSync( UMOUNT_PROBE, [ @@ -78,7 +82,8 @@ describe.if(inDocker)('srt end-to-end as uid 0 in a container', () => { "libc = ctypes.CDLL('libc.so.6', use_errno=True)", 'for target in sys.argv[1:]:', ' ctypes.set_errno(0)', - ' rc = libc.umount2(target.encode(), 0)', + // UMOUNT_NOFOLLOW, which also reaches a mount on a symbolic link. + ' rc = libc.umount2(target.encode(), 8)', ' e = ctypes.get_errno()', " print('umount2 %s rc=%d errno=%s (%s)' % (", ' target, rc, errno.errorcode.get(e, str(e)), os.strerror(e)))', @@ -89,7 +94,7 @@ describe.if(inDocker)('srt end-to-end as uid 0 in a container', () => { filesystem: { denyRead: [SECRET], allowWrite: [ALLOWED], - denyWrite: [], + denyWrite: [LINK], }, enableWeakerNestedSandbox: true, } @@ -160,6 +165,17 @@ describe.if(inDocker)('srt end-to-end as uid 0 in a container', () => { expect(existsSync(out)).toBe(false) }) + it('keeps a write-denied symbolic link in place', () => { + const r = srt( + `echo SANDBOX-RAN; python3 ${UMOUNT_PROBE} ${LINK}; rm ${LINK} 2>&1`, + ) + + expect(r.stdout).toContain('SANDBOX-RAN') + expect(r.stdout).toContain(`umount2 ${LINK} rc=-1 errno=EINVAL`) + expect(r.stdout).toContain('Device or resource busy') + expect(readlinkSync(LINK)).toBe('leads-to') + }) + // Without the helper there is no nested namespace and no locked copies: // --cap-drop ALL is the whole barrier, and the kernel refuses with EPERM // because the command holds no CAP_SYS_ADMIN in bwrap's user namespace. diff --git a/test/sandbox/write-deny-through-links.test.ts b/test/sandbox/write-deny-through-links.test.ts new file mode 100644 index 000000000..b0c2d989d --- /dev/null +++ b/test/sandbox/write-deny-through-links.test.ts @@ -0,0 +1,564 @@ +import { describe, it, expect, beforeEach, afterEach } from 'bun:test' +import { spawnSync } from 'node:child_process' +import { + chmodSync, + existsSync, + lstatSync, + mkdirSync, + mkdtempSync, + readFileSync, + readlinkSync, + realpathSync, + rmSync, + symlinkSync, + writeFileSync, +} from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { getApplySeccompBinaryPath } from '../../src/sandbox/generate-seccomp-filter.js' +import { + cleanupBwrapMountPoints, + getLinuxUnheldLinks, + wrapCommandWithSandboxLinux, +} from '../../src/sandbox/linux-sandbox-utils.js' +import { wrapCommandWithSandboxMacOS } from '../../src/sandbox/macos-sandbox-utils.js' +import { followLinks } from '../../src/sandbox/sandbox-utils.js' +import { bwrapCanNamespace } from '../helpers/bwrap-namespace.js' +import { withCapturedWarnings } from '../helpers/captured-warnings.js' +import { isLinux, isMacOS, isWindows } from '../helpers/platform.js' + +/** + * A write deny on a path with a symbolic link on the way to it, the last name + * included, holds what the links lead to and the links themselves. Everything + * beside them stays as writable as it was. + * + * Every link here is made on the host before the command is wrapped. + */ + +let BASE: string +let ROOT: string // the one write root +let OUTSIDE: string // beside it, under no write root + +/** Link name in ROOT -> what it says. */ +const LINKS = { + name: 'conf/x', + 'holder/commands': '../shared/commands', + dangling: 'later', + way: 'real', + first: 'middle', + middle: 'conf/y', + 'other-link': 'conf/z', +} +const FILES = [ + 'conf/x', + 'conf/y', + 'conf/z', + 'shared/commands/file', + 'real/secret', + 'beside', +] +const ABSENT = ['later', 'real/absent', 'shared/commands/new'] +const DENIED = [ + 'name', + 'holder/commands', + 'dangling', + 'way/secret', + 'way/absent', + 'first', +] + +beforeEach(() => { + if (isWindows) return + // Where the temp dir really is: macOS names it through a link. + BASE = realpathSync(mkdtempSync(join(tmpdir(), 'write-deny-links-'))) + ROOT = join(BASE, 'root') + OUTSIDE = join(BASE, 'outside') + for (const dir of ['conf', 'shared/commands', 'real', 'holder']) { + mkdirSync(join(ROOT, dir), { recursive: true }) + } + mkdirSync(OUTSIDE) + for (const file of FILES) writeFileSync(join(ROOT, file), 'original') + for (const [link, says] of Object.entries(LINKS)) { + symlinkSync(says, join(ROOT, link)) + } +}) + +afterEach(() => { + if (isWindows) return + if (isLinux) cleanupBwrapMountPoints({ force: true }) + rmSync(BASE, { recursive: true, force: true }) +}) + +const inRoot = (names: string[]): string[] => names.map(n => join(ROOT, n)) + +describe.if(!isWindows)('the links on the way to a path', () => { + it.each<[string, string[], string]>([ + ['beside', [], 'beside'], + ['name', ['name'], 'conf/x'], + ['holder/commands/file', ['holder/commands'], 'shared/commands/file'], + ['dangling', ['dangling'], 'later'], + ['way/absent/deeper', ['way'], 'real/absent/deeper'], + ['first', ['first', 'middle'], 'conf/y'], + ['absent/name', [], 'absent/name'], + ])('%s: %p, ending at %s', (p, links, end) => { + expect(followLinks(join(ROOT, p))).toEqual({ + links: inRoot(links), + end: join(ROOT, end), + }) + }) + + it('reads `..` in a target as the parent of the directory reached', () => { + symlinkSync(join(ROOT, 'shared', 'commands'), join(ROOT, 'absolute')) + symlinkSync('absolute/../made', join(ROOT, 'up')) + expect(followLinks(join(ROOT, 'up'))).toEqual({ + links: inRoot(['up', 'absolute']), + end: join(ROOT, 'shared', 'made'), + }) + }) + + it('names a link where it lies, whatever it was reached through', () => { + symlinkSync(ROOT, join(OUTSIDE, 'to-root')) + expect(followLinks(join(OUTSIDE, 'to-root', 'name'))).toEqual({ + links: [join(OUTSIDE, 'to-root'), join(ROOT, 'name')], + end: join(ROOT, 'conf', 'x'), + }) + }) + + // Not every file system takes such a target. + it.if(isLinux)('has no end past a target that is no text', () => { + symlinkSync(Buffer.from([0x78, 0xff]), join(ROOT, 'bytes')) + expect(followLinks(join(ROOT, 'bytes', 'file'))).toEqual({ + links: inRoot(['bytes']), + end: undefined, + }) + }) + + it('has no end for links that lead in a circle', () => { + symlinkSync('circle-b', join(ROOT, 'circle-a')) + symlinkSync('circle-a', join(ROOT, 'circle-b')) + expect(followLinks(join(ROOT, 'circle-a')).end).toBeUndefined() + }) +}) + +describe.if((isLinux && bwrapCanNamespace()) || isMacOS)( + 'A write deny on a path with a link on the way', + () => { + /** What `attempt` came to, run in ROOT with every one of `denied` denied. */ + async function outcomeOf( + attempt: string, + denied = DENIED, + ): Promise { + const options = { + command: `cd ${ROOT} && echo started && if ( ${attempt} ); then echo DONE; else echo refused; fi`, + needsNetworkRestriction: false, + readConfig: { denyOnly: [] }, + writeConfig: { allowOnly: [ROOT], denyWithinAllow: inRoot(denied) }, + } + const wrapped = isLinux + ? await wrapCommandWithSandboxLinux(options) + : wrapCommandWithSandboxMacOS(options) + const result = spawnSync(wrapped, { + shell: true, + encoding: 'utf8', + timeout: 30000, + // Under no write root, so nothing of the working directory's is + // protected or made. + cwd: OUTSIDE, + }) + const said = `${result.stdout}${result.stderr}` + console.log(`${attempt}\n ${said.trim().split('\n').join('\n ')}`) + expect(said).toContain('started') + if (isLinux) cleanupBwrapMountPoints({ force: true }) + return said.includes('DONE') ? 'DONE' : 'refused' + } + + it.each([ + ['a write through the link', 'echo changed >> name'], + [ + 'a write to what it leads to, by its own path', + 'echo changed >> conf/x', + ], + ['removing the link', 'rm name'], + ['renaming a file over the link', 'echo new > made && mv -f made name'], + ['renaming the link away', 'mv name aside'], + ['making the link anew', 'ln -sfn beside name'], + ['removing what it leads to', 'rm conf/x'], + ['renaming the directory of what it leads to', 'mv conf aside'], + [ + 'a create beneath a link to a directory', + 'echo n > holder/commands/new', + ], + ['the same by its own path', 'echo new > shared/commands/new'], + ['removing a link to a directory', 'rm holder/commands'], + ['renaming the directory that holds the link', 'mv holder aside'], + ['a create through a dangling link', 'echo new > dangling'], + ['a create of what it leads to, by its own path', 'echo new > later'], + ['removing a dangling link', 'rm dangling'], + ['a write beneath a link on the way', 'echo changed >> way/secret'], + ['the same by its own path', 'echo changed >> real/secret'], + ['a create beneath a link on the way', 'echo new > way/absent'], + ['the same by its own path', 'echo new > real/absent'], + ['removing a link on the way', 'rm way'], + ['a write through two links', 'echo changed >> first'], + ['removing the second of two links', 'rm middle'], + ['renaming a file over it', 'echo new > made && mv -f made middle'], + ])('refuses %s: %s', async (_what, attempt) => { + expect(await outcomeOf(attempt)).toBe('refused') + + for (const [link, says] of Object.entries(LINKS)) { + expect(lstatSync(join(ROOT, link)).isSymbolicLink()).toBe(true) + expect(readlinkSync(join(ROOT, link))).toBe(says) + } + for (const file of FILES) { + expect(readFileSync(join(ROOT, file), 'utf8')).toBe('original') + } + for (const name of ABSENT) + expect(existsSync(join(ROOT, name))).toBe(false) + }) + + it.each([ + ['a write to a file beside the link', 'echo changed >> beside'], + ['removing it', 'rm beside'], + ['a create beside the link', 'echo new > new && echo new > holder/new'], + ['a write beside what the link leads to', 'echo changed >> conf/z'], + ['replacing a link no deny leads through', 'ln -sfn beside other-link'], + [ + 'reading the link, and through it', + 'test "$(readlink name)$(cat name)$(ls holder/commands)" = conf/xoriginalfile', + ], + ])('allows %s: %s', async (_what, attempt) => { + expect(await outcomeOf(attempt)).toBe('DONE') + }) + + it.each([ + 'rm .bashrc', + 'echo changed >> conf/x', + 'rm .claude/commands', + 'echo new > shared/commands/new', + ])( + 'refuses the same of a name denied in every working directory: %s', + async attempt => { + mkdirSync(join(ROOT, '.claude')) + symlinkSync('conf/x', join(ROOT, '.bashrc')) + symlinkSync('../shared/commands', join(ROOT, '.claude', 'commands')) + const cwd = process.cwd() + process.chdir(ROOT) + try { + expect(await outcomeOf(attempt, [])).toBe('refused') + } finally { + process.chdir(cwd) + } + }, + ) + }, +) + +// Profile text only, so it runs on every POSIX host. +describe.if(!isWindows)('Seatbelt profile', () => { + /** The filters of the rule that denies `operations`, one to a line. */ + const deniedBy = (operations: string, denyWithinAllow: string[]): string => { + const profile = wrapCommandWithSandboxMacOS({ + command: 'true', + needsNetworkRestriction: false, + readConfig: undefined, + writeConfig: { allowOnly: [ROOT], denyWithinAllow }, + }) + const at = profile.indexOf(`(deny ${operations}\n`) + expect(at).toBeGreaterThan(-1) + return profile.slice(at, profile.indexOf('(with message', at)) + } + const subpath = (name: string): string => + ` (subpath "${join(ROOT, name)}")\n` + const literal = (name: string): string => + ` (literal "${join(ROOT, name)}")\n` + + it.each([ + ['name', 'conf/x'], + ['holder/commands', 'shared/commands'], + ['dangling', 'later'], + ['way/secret', 'real/secret'], + ['way/absent', 'real/absent'], + ['first', 'conf/y'], + ])('a deny on %s is one on %s too', (written, end) => { + const denies = deniedBy('file-write*', inRoot([written])) + expect(denies).toContain(subpath(written)) + expect(denies).toContain(subpath(end)) + }) + + it('so is one on a name denied in every working directory', () => { + symlinkSync('conf/x', join(ROOT, '.bashrc')) + const cwd = process.cwd() + process.chdir(ROOT) + try { + expect(deniedBy('file-write*', [])).toContain(subpath('conf/x')) + } finally { + process.chdir(cwd) + } + }) + + it('a pattern deny starts from both spellings of its directory', () => { + const denies = deniedBy('file-write*', [join(ROOT, 'way', '*.key')]) + const matches = (file: string): boolean => + [...denies.matchAll(/^ {2}\(regex (".*")\)$/gm)].some(([, text]) => + new RegExp(JSON.parse(text!) as string).test(join(ROOT, file)), + ) + expect(matches('way/a.key')).toBe(true) + expect(matches('real/a.key')).toBe(true) + expect(matches('conf/a.key')).toBe(false) + }) + + it('every link on the way, and what holds it, is kept in place', () => { + mkdirSync(join(ROOT, 'elsewhere')) + symlinkSync('../conf/x', join(ROOT, 'elsewhere', 'hop')) + symlinkSync('elsewhere/hop', join(ROOT, 'far')) + const kept = deniedBy( + 'file-write-unlink file-write-create', + inRoot(['first', 'far']), + ) + for (const name of ['middle', 'elsewhere/hop', 'elsewhere', 'conf']) { + expect(kept).toContain(literal(name)) + } + expect(kept).not.toContain(join(ROOT, 'other-link')) + }) +}) + +describe.if(isLinux)('what the seccomp helper is asked to hold', () => { + const wrap = ( + denyWithinAllow: string[], + rest: Partial[0]> = {}, + ): Promise => + wrapCommandWithSandboxLinux({ + command: 'echo started', + needsNetworkRestriction: false, + readConfig: { denyOnly: [] }, + writeConfig: { allowOnly: [ROOT], denyWithinAllow }, + ...rest, + }) + const held = (wrapped: string): string[] => + [...wrapped.matchAll(/--hold-link (\S+)/g)].map(match => match[1]!) + + it('each link once, where it lies', async () => { + symlinkSync(ROOT, join(OUTSIDE, 'to-root')) + expect( + held( + await wrap([ + ...inRoot(['first', 'way/secret', 'way/absent']), + join(OUTSIDE, 'to-root', 'middle'), + ]), + ).sort(), + ).toEqual(inRoot(['first', 'middle', 'way'])) + }) + + it('none for a path with no link on the way', async () => { + expect(held(await wrap(inRoot(['beside', 'conf/x', 'absent'])))).toEqual([]) + }) + + it('none that lies where the command cannot write', async () => { + symlinkSync(join(ROOT, 'conf', 'x'), join(OUTSIDE, 'into-root')) + expect(held(await wrap([join(OUTSIDE, 'into-root')]))).toEqual([]) + }) + + it('none in the directories the sandbox mounts anew', async () => { + expect( + held( + await wrap(['/proc/self/cwd/file'], { + writeConfig: { + allowOnly: ['/'], + denyWithinAllow: ['/proc/self/cwd/file'], + }, + }), + ), + ).toEqual([]) + }) + + it.if(bwrapCanNamespace())( + 'none that a read deny hides, and the command starts', + async () => { + const wrapped = await wrap(inRoot(['holder/commands', 'name']), { + readConfig: { denyOnly: [join(ROOT, 'holder')] }, + }) + expect(held(wrapped)).toEqual(inRoot(['name'])) + expect( + spawnSync(wrapped, { shell: true, encoding: 'utf8', cwd: OUTSIDE }) + .stdout, + ).toBe('started\n') + }, + ) + + it.if(bwrapCanNamespace())( + 'none that is bound back by itself into a directory a read deny hides', + async () => { + mkdirSync(join(ROOT, 'holder', 'inner')) + symlinkSync('inner', join(ROOT, 'holder', 'to-inner')) + const wrapped = await wrap(inRoot(['holder/to-inner/file']), { + readConfig: { + denyOnly: [join(ROOT, 'holder')], + allowWithinDeny: inRoot(['holder/to-inner']), + }, + }) + expect(held(wrapped)).toEqual([]) + expect( + spawnSync(wrapped, { shell: true, encoding: 'utf8', cwd: OUTSIDE }) + .stdout, + ).toBe('started\n') + }, + ) + + it('the directories above a held link are pinned', async () => { + const holder = join(ROOT, 'holder') + expect(await wrap(inRoot(['holder/commands']))).toContain( + `--ro-bind ${holder} ${holder} `, + ) + }) + + describe('where nothing can hold them', () => { + /** Runs its words, as a helper built before the option does. */ + const olderHelper = (): string => { + const helper = join(OUTSIDE, 'older-helper') + writeFileSync(helper, '#!/bin/sh\nexec "$@"\n') + chmodSync(helper, 0o755) + return helper + } + + it.each<[string, () => Parameters[1]]>([ + ['no helper in use', () => ({ allowAllUnixSockets: true })], + ['an older one', () => ({ seccompConfig: { applyPath: olderHelper() } })], + ])('%s: they are named, and the command starts', async (_what, rest) => { + const links = inRoot(['first', 'middle']) + expect(getLinuxUnheldLinks()).not.toContain(links[0]) + + const { result: wrapped, warnings } = await withCapturedWarnings(() => + wrap(inRoot(['first']), rest()), + ) + + expect(held(wrapped)).toEqual([]) + expect(getLinuxUnheldLinks()).toEqual(expect.arrayContaining(links)) + for (const link of links) { + expect(warnings.some(line => line.includes(`${link},`))).toBe(true) + } + if (bwrapCanNamespace()) { + expect( + spawnSync(wrapped, { shell: true, encoding: 'utf8', cwd: OUTSIDE }) + .stdout, + ).toBe('started\n') + } + }) + + it('a helper run under another name is asked under it, and again after no answer', async () => { + const multicall = join(OUTSIDE, 'multicall') + const seccompConfig = { applyPath: multicall, argv0: 'apply-seccomp' } + expect(held(await wrap(inRoot(['name']), { seccompConfig }))).toEqual([]) + + writeFileSync( + multicall, + `#!/bin/sh\n[ "$ARGV0" = apply-seccomp ] && exec ${getApplySeccompBinaryPath()} "$@"\n`, + ) + chmodSync(multicall, 0o755) + expect(held(await wrap(inRoot(['name']), { seccompConfig }))).toEqual( + inRoot(['name']), + ) + }) + + it('asking an older helper runs no program of that name', async () => { + const program = join(ROOT, '--holds-links') + writeFileSync(program, `#!/bin/sh\necho ran > ${ROOT}/ran\n`) + chmodSync(program, 0o755) + const [cwd, PATH] = [process.cwd(), process.env.PATH] + process.chdir(ROOT) + process.env.PATH = `${ROOT}:.:${PATH}` + try { + await wrap(inRoot(['name']), { + seccompConfig: { applyPath: olderHelper() }, + }) + } finally { + process.chdir(cwd) + process.env.PATH = PATH + } + expect(existsSync(join(ROOT, 'ran'))).toBe(false) + }) + }) +}) + +describe.if(isLinux && bwrapCanNamespace())('the seccomp helper', () => { + const helper = (): string => getApplySeccompBinaryPath()! + const run = ( + program: string, + words: string[], + ): { status: number | null; said: string } => { + const result = spawnSync(program, words, { + encoding: 'utf8', + timeout: 30000, + cwd: ROOT, + }) + return { status: result.status, said: result.stdout + result.stderr } + } + /** umount2() with UMOUNT_NOFOLLOW, which reaches a mount on a link. */ + const UNMOUNT = [ + 'python3', + '-c', + [ + 'import ctypes, errno, sys', + 'libc = ctypes.CDLL(None, use_errno=True)', + 'r = libc.umount2(sys.argv[1].encode(), 8)', + 'print("unmounted" if r == 0 else errno.errorcode[ctypes.get_errno()])', + ].join('\n'), + ] + + it('says that it holds links', () => { + expect(run(helper(), ['--holds-links'])).toEqual({ status: 0, said: '' }) + }) + + it.each([ + ['a file', 'beside', 'not a symbolic link'], + ['nothing', 'absent', 'No such file or directory'], + ])('runs no command where %s is at the name', (_what, name, why) => { + const { status, said } = run(helper(), [ + '--hold-link', + join(ROOT, name), + 'echo', + 'started', + ]) + expect(status).toBe(1) + expect(said).toContain(why) + expect(said).not.toContain('started') + }) + + it.each([ + ['this user', [], 'EPERM'], + ['uid 0, with every capability in its namespace', ['-Ur'], 'EINVAL'], + ])('a command of %s cannot take the hold off', (_who, as, errno) => { + const link = join(ROOT, 'name') + const { said } = run('unshare', [ + ...as, + helper(), + '--hold-link', + link, + ...UNMOUNT, + link, + ]) + expect(said.trim()).toBe(errno) + }) + + it('holds a link on a file system mounted with flags of its own', () => { + const link = join(OUTSIDE, 'link') + const { said } = run('unshare', [ + ...['-Urm', 'sh', '-c'], + `mount -t tmpfs -o noexec,noatime none ${OUTSIDE} && ln -s x ${link} && ` + + `exec ${helper()} --hold-link ${link} rm ${link}`, + ]) + expect(said).toContain('Device or resource busy') + }) + + it.each(['--ro-bind', '--bind', '--dev-bind'])( + 'a bubblewrap the command starts with %s / / starts, and the hold holds there', + bind => { + const { said } = run(helper(), [ + ...inRoot(['name', 'way', 'dangling']).flatMap(l => ['--hold-link', l]), + ...['bwrap', bind, '/', '/', '--bind', ROOT, ROOT, '--dev', '/dev'], + ...['sh', '-c', 'echo started; rm name way dangling'], + ]) + expect(said).toContain('started') + expect(said.match(/Device or resource busy/g)).toHaveLength(3) + }, + ) +}) diff --git a/vendor/seccomp-src/apply-seccomp.c b/vendor/seccomp-src/apply-seccomp.c index 74c12e55a..a9af1bdfb 100644 --- a/vendor/seccomp-src/apply-seccomp.c +++ b/vendor/seccomp-src/apply-seccomp.c @@ -1,7 +1,8 @@ /* * apply-seccomp.c - Apply seccomp BPF filter in an isolated PID namespace * - * Usage: apply-seccomp [args...] + * Usage: apply-seccomp [--hold-link ]... [args...] + * apply-seccomp --holds-links * * This program applies a baked-in seccomp BPF filter, isolates the * target command in a nested user+PID+mount namespace so it cannot see or @@ -26,6 +27,10 @@ * Any failure to set up the nested namespaces aborts with a non-zero exit * status; we never fall back to running the command without isolation. * + * --hold-link keeps a symbolic link where it is: see hold_link(). The second + * form exits 0 and does nothing else. A build that lacks the option takes the + * word for a command, finds none, and exits 1. + * * Compile: gcc -static -O2 -o apply-seccomp apply-seccomp.c */ @@ -45,6 +50,8 @@ #include #include #include +#include +#include #include #include #include @@ -657,13 +664,116 @@ static int reap_until(pid_t main_child) { } } +/* A new user namespace, with this process's uid and gid mapped to themselves + * and every capability in it. */ +static void enter_user_namespace(void) { + uid_t uid = geteuid(); + gid_t gid = getegid(); + + /* If this binary was exec'd without read permission (e.g. installed + * mode 0111), the kernel marked the process non-dumpable, which + * makes /proc/self/{setgroups,uid_map,gid_map} root-owned, so the + * writes below would fail with EACCES. Temporarily flip dumpable + * on for the uid/gid mapping and restore it right after. While + * dumpable is 1, a same-uid process can ptrace us (under yama + * ptrace_scope=0) and dump the mapped pages that mode 0111 is + * meant to hide; the save/restore keeps that exposure to a + * few-syscall race window — the same unavoidable window runc and + * systemd accept for this pattern. + * + * prctl failures here are ignored: they are next to impossible for + * these calls, and if raising dumpable did fail the map writes + * below fail with their own clearer errors. */ + int dumpable = prctl(PR_GET_DUMPABLE); + (void)prctl(PR_SET_DUMPABLE, 1); + + if (unshare(CLONE_NEWUSER) < 0) { + die("apply-seccomp: unshare(CLONE_NEWUSER)"); + } + if (write_file("/proc/self/setgroups", "deny") < 0) { + die("apply-seccomp: write /proc/self/setgroups " + "(nested userns is capability-restricted; " + "caller must provide CAP_SYS_ADMIN)"); + } + if (write_file("/proc/self/uid_map", "%u %u 1\n", uid, uid) < 0) { + die("apply-seccomp: write /proc/self/uid_map"); + } + if (write_file("/proc/self/gid_map", "%u %u 1\n", gid, gid) < 0) { + die("apply-seccomp: write /proc/self/gid_map"); + } + /* PR_SET_DUMPABLE only accepts 0 or 1; if the saved value was + * SUID_DUMP_ROOT (2) — or the read above failed — restore the more + * restrictive 0. */ + (void)prctl(PR_SET_DUMPABLE, dumpable == 1 ? 1 : 0); +} + +static void die_holding(const char *path, const char *why) { + fprintf(stderr, "apply-seccomp: --hold-link %s: %s\n", path, why); + _exit(1); +} + +/* + * Mount the symbolic link at `path` on itself, read-only. A name that is a + * mount point cannot be unlinked, renamed or renamed over (EBUSY), and the + * directory that holds it stays as writable as it was. The link reads and + * leads as before. + * + * mount(2) follows a link at its target, so the link is named through a + * descriptor of itself: the kernel does not follow what /proc/self/fd/N leads + * to any further. + */ +static void hold_link(const char *path) { + char self[32]; + struct stat st; + struct statvfs fs; + + int fd = open(path, O_PATH | O_NOFOLLOW | O_CLOEXEC); + if (fd < 0 || fstat(fd, &st) < 0) { + die_holding(path, strerror(errno)); + } + if (!S_ISLNK(st.st_mode)) { + die_holding(path, "not a symbolic link"); + } + snprintf(self, sizeof(self), "/proc/self/fd/%d", fd); + if (mount(self, self, NULL, MS_BIND, NULL) < 0) { + die_holding(path, strerror(errno)); + } + close(fd); + + /* Read-only, nosuid and nodev are all the flags bubblewrap ever adds. One + * that the command starts applies its flags to every mount it finds, by + * path, which for this mount leads elsewhere; it leaves a mount that has + * them already alone. noexec is locked where the mount came with it, and + * a remount keeps the access-time flags by itself. Opened anew: the first + * descriptor is of what now lies beneath the mount. */ + fd = open(path, O_PATH | O_NOFOLLOW | O_CLOEXEC); + if (fd < 0 || fstatvfs(fd, &fs) < 0) { + die_holding(path, strerror(errno)); + } + unsigned long flags = MS_RDONLY | MS_NOSUID | MS_NODEV; + if (fs.f_flag & ST_NOEXEC) flags |= MS_NOEXEC; + snprintf(self, sizeof(self), "/proc/self/fd/%d", fd); + if (mount(NULL, self, NULL, MS_REMOUNT | MS_BIND | flags, NULL) < 0) { + die_holding(path, strerror(errno)); + } + close(fd); +} + int main(int argc, char *argv[]) { - if (argc < 2) { - fprintf(stderr, "Usage: %s [args...]\n", argv[0]); + if (argc == 2 && strcmp(argv[1], "--holds-links") == 0) { + return 0; + } + int links = 0; + while (2 * links + 2 < argc && strcmp(argv[2 * links + 1], "--hold-link") == 0) { + links++; + } + if (argc < 2 * links + 2) { + fprintf(stderr, "Usage: %s [--hold-link ]... [args...]\n", + argv[0]); return 1; } - char **command_argv = &argv[1]; + char **command_argv = &argv[2 * links + 1]; _Static_assert(sizeof(unix_block_bpf) % sizeof(struct sock_filter) == 0, "BPF filter size must be a multiple of sock_filter"); @@ -693,7 +803,8 @@ int main(int argc, char *argv[]) { * map uid/gid, then unshare. This also works when apply-seccomp is * run standalone outside bwrap. * - * Path (a) is tried first. If we don't have the cap, the + * Path (a) is tried first, unless there are links to hold, which takes + * (b): see the invariant there. If we don't have the cap, the * kernel returns EPERM and we fall through to (b). Path (b) can itself * fail on hosts where unprivileged user namespaces are gated by an LSM * (Ubuntu 24.04's AppArmor restriction, for example) — the unshare @@ -708,49 +819,29 @@ int main(int argc, char *argv[]) { ? open("/proc", O_PATH | O_DIRECTORY | O_CLOEXEC) : -1; - if (unshare(CLONE_NEWPID | CLONE_NEWNS) < 0) { - if (errno != EPERM) { + if (links > 0 || unshare(CLONE_NEWPID | CLONE_NEWNS) < 0) { + if (links == 0 && errno != EPERM) { die("apply-seccomp: unshare(CLONE_NEWPID|CLONE_NEWNS)"); } - uid_t uid = geteuid(); - gid_t gid = getegid(); - - /* If this binary was exec'd without read permission (e.g. installed - * mode 0111), the kernel marked the process non-dumpable, which - * makes /proc/self/{setgroups,uid_map,gid_map} root-owned, so the - * writes below would fail with EACCES. Temporarily flip dumpable - * on for the uid/gid mapping and restore it right after. While - * dumpable is 1, a same-uid process can ptrace us (under yama - * ptrace_scope=0) and dump the mapped pages that mode 0111 is - * meant to hide; the save/restore keeps that exposure to a - * few-syscall race window — the same unavoidable window runc and - * systemd accept for this pattern. - * - * prctl failures here are ignored: they are next to impossible for - * these calls, and if raising dumpable did fail the map writes - * below fail with their own clearer errors. */ - int dumpable = prctl(PR_GET_DUMPABLE); - (void)prctl(PR_SET_DUMPABLE, 1); - - if (unshare(CLONE_NEWUSER) < 0) { - die("apply-seccomp: unshare(CLONE_NEWUSER)"); - } - if (write_file("/proc/self/setgroups", "deny") < 0) { - die("apply-seccomp: write /proc/self/setgroups " - "(nested userns is capability-restricted; " - "caller must provide CAP_SYS_ADMIN)"); - } - if (write_file("/proc/self/uid_map", "%u %u 1\n", uid, uid) < 0) { - die("apply-seccomp: write /proc/self/uid_map"); - } - if (write_file("/proc/self/gid_map", "%u %u 1\n", gid, gid) < 0) { - die("apply-seccomp: write /proc/self/gid_map"); + enter_user_namespace(); + if (links > 0) { + /* INVARIANT: a user namespace is entered after the last hold. The + * mount namespace made below is then a copy made across that + * boundary, in which every mount is locked: a command started by + * uid 0 holds all capabilities in its user namespace, and could + * unmount what was mounted in a mount namespace of that one. */ + if (unshare(CLONE_NEWNS) < 0) { + die("apply-seccomp: unshare(CLONE_NEWNS)"); + } + if (mount(NULL, "/", NULL, MS_REC | MS_PRIVATE, NULL) < 0) { + die("apply-seccomp: mount(MS_PRIVATE)"); + } + for (int i = 0; i < links; i++) { + hold_link(argv[2 * i + 2]); + } + enter_user_namespace(); } - /* PR_SET_DUMPABLE only accepts 0 or 1; if the saved value was - * SUID_DUMP_ROOT (2) — or the read above failed — restore the more - * restrictive 0. */ - (void)prctl(PR_SET_DUMPABLE, dumpable == 1 ? 1 : 0); if (unshare(CLONE_NEWPID | CLONE_NEWNS) < 0) { die("apply-seccomp: unshare(CLONE_NEWPID|CLONE_NEWNS) after userns"); } From 05a36839fce0ae49f0eaed9bb69cb65d695a8e77 Mon Sep 17 00:00:00 2001 From: Ron Leizrowice Date: Sat, 10 Oct 2026 23:34:22 +0000 Subject: [PATCH 2/8] Links on the way to read denies are kept too, and what a hold is said to do Links at denyRead entries, pattern matches, credential files and masks are held like those on the way to a write deny. Nothing among the devices is denied through a link. A directory that cannot be looked in is denied whole: "could not look" is never "no link". The helper compares what the link says with what it said at the wrap, on the descriptor it mounts. seccomp.holdsLinks is the caller's word on its helper, and a path given by applyPath is never run outside the sandbox. getLinuxUnheldLinks() gives a reason with each link and is emptied by reset() and updateConfig(). The README and the comments say what a hold refuses: unlink and rename of a mount point in the caller's mount namespace, and unmounting it. --- README.md | 15 +- src/index.ts | 2 + src/sandbox/linux-sandbox-utils.ts | 234 +++-- src/sandbox/macos-sandbox-utils.ts | 20 +- src/sandbox/read-deny-glob.ts | 15 +- src/sandbox/sandbox-config.ts | 10 + src/sandbox/sandbox-manager.ts | 17 +- src/sandbox/sandbox-schemas.ts | 6 + src/sandbox/sandbox-utils.ts | 60 +- test/sandbox/literal-path-entries.test.ts | 23 +- test/sandbox/write-deny-through-links.test.ts | 896 ++++++++++++++---- vendor/seccomp-src/apply-seccomp.c | 57 +- 12 files changed, 1059 insertions(+), 296 deletions(-) diff --git a/README.md b/README.md index cb75488ad..cad524d54 100644 --- a/README.md +++ b/README.md @@ -587,7 +587,7 @@ Examples: - `allowPty` - macOS only: allow pseudo-terminal operations (boolean, default: false). The seatbelt profile then carries `(allow pseudo-tty)` plus read, write and `ioctl` on `/dev/ptmx` and `/dev/ttys*`, which a command that allocates a pty of its own needs. Not read on Linux or Windows. - `bwrapPath` - Linux only: absolute path to the `bwrap` (bubblewrap) binary, used instead of resolving `bwrap` on `PATH`. A path that is not executable is a dependency error, and no `PATH` lookup is tried after it. - `socatPath` - Linux only: absolute path to the `socat` binary, used instead of resolving `socat` on `PATH`, with the same rule for a path that is not executable. -- `seccomp` - Linux only: `applyPath` points at the `apply-seccomp` binary to use instead of the packaged one, which is still used when nothing exists at that path. One built from older sources is used for what it can do (see "Write denies and symbolic links"). `argv0` invokes it as a multicall binary that dispatches on the `ARGV0` environment variable; `applyPath` is then used verbatim (no existence check) and must resolve inside the bubblewrap namespace. +- `seccomp` - Linux only: `applyPath` points at the `apply-seccomp` binary to use instead of the packaged one, which is still used when nothing exists at that path. `argv0` invokes it as a multicall binary that dispatches on the `ARGV0` environment variable; `applyPath` is then used verbatim (no existence check) and must resolve inside the bubblewrap namespace. `holdsLinks` says whether the helper takes `--hold-link` (see "Denied paths and symbolic links") and is taken at its word: nothing is run to find out. Not set, a helper the library found for itself is asked (`apply-seccomp --holds-links` exits 0), and one named by `applyPath`, which need not name the helper outside the sandbox, is never run there and is handed no link. Set it from such a probe of your own helper, not from a constant: a helper without the option takes `--hold-link` for the command, so under a wrong `true` no sandboxed command starts in a project with such a link. - `ripgrep` - Linux only: how to invoke ripgrep for the deny-path scan (default: `{ "command": "rg" }`). `args` are passed before srt's own arguments, and `argv0` overrides `argv[0]` for a multicall binary. - `git.safeDirectories` - Directories git should treat as `safe.directory` inside the sandbox, where the working tree is owned by another user — the repository top level when the command runs from a subdirectory, say. Injected through `GIT_CONFIG_*` environment variables, and it grants no write access of its own. - `credentials` - Environment variables (`credentials.envVars`) and files (`credentials.files`) the sandboxed command must not read the real values of. `mode: "deny"` withholds the value; `mode: "mask"` puts a sentinel in its place inside the sandbox and has the proxy substitute the real value back on egress to the hosts that entry's `injectHosts` lists (default: `network.allowedDomains`). Masking requires `network.tlsTerminate`, so the real value only leaves over a verified TLS connection, unless `credentials.allowPlaintextInject` opts out. Files are masked on Linux only; macOS denies a masked file instead. @@ -893,11 +893,18 @@ Filesystem restrictions are enforced at the OS level: **Writes through links (Linux and macOS):** a write is judged by where it lands, after symbolic links are followed. A link inside an allowed write path that leads out of it gives a command nothing: the write is refused. A link that leads into an allowed write path works like any other name for that place. A hard link is a second name for the file itself: on Linux a file outside the allowed write paths that already has a hard link inside one can be written through it, and a sandboxed command cannot create such a link. On macOS nothing is promised for such a file. -**Write denies and symbolic links (Linux and macOS):** a `denyWrite` entry or a [mandatory deny path](#mandatory-deny-paths-auto-protected-files) with a symbolic link on the way to it, its last name included, denies the path the links lead to, whether or not anything is there yet, and keeps each of those links where it is: it cannot be removed, renamed or replaced, and neither can the directories above it, as above any denied path. The link reads and leads as before, and everything else in the directory that holds it stays as writable as it was. The links are those found when the command is wrapped. +**Denied paths and symbolic links (Linux and macOS):** a `denyWrite` entry or a [mandatory deny path](#mandatory-deny-paths-auto-protected-files) with a symbolic link on the way to it, its last name included, denies the path the links lead to, whether or not anything is there yet, and keeps each of those links where it is: removing it, renaming it and putting something else at its name are refused, and so is renaming the directories above it, as above any denied path. The link reads and leads as before, and everything else in the directory that holds it stays as writable as it was. The links are those found when the command is wrapped. -- On Linux bubblewrap cannot mount on a link, so the seccomp helper does (`apply-seccomp --hold-link `): each link that lies in a directory the command can write to is mounted on itself, read-only, and removing or replacing it fails with `EBUSY` ("Device or resource busy"). A command is not started if a link has become something else since the wrap. -- Not held on Linux: with no helper in use (`allowAllUnixSockets`, or no binary for the architecture), or with one built before it had the option (`seccomp.applyPath`), what the links lead to is denied all the same, and the links are ordinary entries of a writable directory. `SandboxManager.getLinuxUnheldLinks()` returns every such link of the commands wrapped so far, for the embedder to show or to look at again after the command; the only other sign is a line under `SRT_DEBUG`. +- Nothing is denied among the devices, for reading or writing: a name blanked with a link to `/dev/null` is kept, and `/dev/null` stays as it was. +- A tool that replaces a file instead of writing into it is refused at such a name, as at any denied one: `sed -i`, `perl -i`, `mv -f`, a write to a temporary file that is then renamed, `install`, `ln -sfn`, `rsync`, `tar x`, `unzip -o`, `git checkout` where the tracked link differs, `git clean -fdx`. On Linux the error is `EBUSY` ("Device or resource busy"). +- A directory on the way that cannot be looked in (its search permission is gone, or the look fails for any reason but the name not being there) is denied whole in place of the path, its mode included: what is in it is not known. Never `/`: where that is the directory, the path is taken as written. +- On Linux the same links are kept on the way to a `denyRead` entry, to a link that a `denyRead` pattern matches, and to a `credentials.files` path, denied or masked. Each command's mounts are placed by the name, so a name that led elsewhere by the next command would leave what it led to unhidden. +- On Linux bubblewrap cannot mount on a link, so the seccomp helper does (`apply-seccomp --hold-link `): each link that lies in a directory the command can write to is mounted on itself, read-only. The hold is a mount like every deny of this sandbox and holds what those hold: it is in force in the mount namespaces made for the command. +- No command is started where a hold cannot be made: the name is no link any more, the link says something else than when the command was wrapped, or the mount is refused (a security module, a file system that does not take one). The helper exits 1 and its one line on stderr starts with `apply-seccomp: --hold-link : `. `seccomp.holdsLinks: false` turns holding off. +- Not held on Linux: `SandboxManager.getLinuxUnheldLinks()` returns each link that should be held and is not, as `{ path, reason }`, for the embedder to show or to look at again after the command; the only other sign is a line under `SRT_DEBUG`. What such a link leads to is denied all the same, and the link is an ordinary entry of a writable directory. A link is listed by the latest wrap that met it, so one that a later wrap holds is listed no longer, and `reset()` and `updateConfig()` empty the list. The reasons: `no_helper` (`allowAllUnixSockets`, or no binary for the architecture), `told_no` (`seccomp.holdsLinks` is false), `not_told` (the helper is named by `seccomp.applyPath` and `seccomp.holdsLinks` is not set), `older_helper` (the helper the library found for itself is built from older sources), `no_answer` (that helper could not be asked, twice; it is asked again at the next wrap). - Not covered: a mandatory deny name _below_ the working directory that is itself a link. On Linux the scan lists files and does not see it. On macOS the pattern keeps the name in place and says nothing of what it leads to. +- Not covered on Linux: a link on the way to what a `denyRead` pattern matches, other than the match itself. A `denyWrite` pattern is skipped there as a whole. +- The links' names stay on the command line (see the note on large profiles below), about as many bytes each as the path and the target together: some thousand of them, with the command, reach the limit of one argument, and the `command_too_long` refusal says how many bytes are theirs. **Read-side rules (Linux):** an entry is matched by the name it is, not by what it points at. diff --git a/src/index.ts b/src/index.ts index 45bac3933..78fbfa109 100644 --- a/src/index.ts +++ b/src/index.ts @@ -49,6 +49,8 @@ export { type SandboxDependencyCheck, LinuxSandboxProfileError, type LinuxSandboxProfileErrorCode, + type LinuxUnheldLink, + type LinuxUnheldLinkReason, } from './sandbox/linux-sandbox-utils.js' // Windows install/status API diff --git a/src/sandbox/linux-sandbox-utils.ts b/src/sandbox/linux-sandbox-utils.ts index 16d0700be..2b33af999 100644 --- a/src/sandbox/linux-sandbox-utils.ts +++ b/src/sandbox/linux-sandbox-utils.ts @@ -883,7 +883,13 @@ export class WorkingDirectoryChanged extends Error { function renderBwrapInvocation( bwrapBinary: string, bwrapArgs: string[], - mounts: { start: number; end: number; mandatoryDenyArgs: number }, + mounts: { + start: number + end: number + mandatoryDenyArgs: number + /** How much of the command word is the seccomp helper's `--hold-link`s. */ + holdWordsBytes: number + }, ): string { // Whose the arguments are and what lowers each share: the mandatory denies // follow from what the working directory holds, not from the configuration. @@ -948,7 +954,11 @@ function renderBwrapInvocation( closeBwrapArgsProfile(argsFd) throw new LinuxSandboxProfileError( 'command_too_long', - `Sandboxed command is too long for one shell argument even with the mounts passed through a file (${viaArgsFileBytes} bytes; the limit here is ${limit})`, + `Sandboxed command is too long for one shell argument even with the mounts passed through a file (${viaArgsFileBytes} bytes; the limit here is ${limit})${ + mounts.holdWordsBytes > 0 + ? `. At least ${mounts.holdWordsBytes} of them are not the caller's: they name the symbolic links on the way to denied paths, for the seccomp helper to hold, and stay on the line` + : '' + }`, ) } logForDebugging( @@ -1504,46 +1514,89 @@ export async function initializeLinuxNetworkBridge( } } -// Keyed by how the helper is run. Only an answer is kept: a probe that failed -// is made again the next time. +/** Why a symbolic link that should be held in place is not. */ +export type LinuxUnheldLinkReason = + /** No seccomp helper runs in the wrap. */ + | 'no_helper' + /** `seccomp.holdsLinks` is false. */ + | 'told_no' + /** The helper is the caller's, and `seccomp.holdsLinks` is not set. */ + | 'not_told' + /** The helper this library found for itself is built from older sources. */ + | 'older_helper' + /** That helper could not be asked: it did not start, or gave no status. */ + | 'no_answer' + +// Keyed by path. Only an answer is kept: a probe that got none is made again +// at the next wrap. const holdsLinksProbes = new Map() /** - * Whether the seccomp helper takes `--hold-link`. The one in use need not be - * the one this package ships, and one built from older sources takes the word - * for the command to run. + * Why the seccomp helper of a wrap is not handed the links to hold, or + * undefined where it is. One built from older sources takes `--hold-link` for + * the command to run, so the words go to no helper that is not known to take + * them: by the caller's word, or, of a helper this library found for itself, + * by asking it. */ -function seccompHelperHoldsLinks( - applyPath: string | undefined, - argv0: string | undefined, -): boolean { - const helper = argv0 ? applyPath : getApplySeccompBinaryPath(applyPath) - if (!helper) return false - const key = JSON.stringify([helper, argv0]) - const cached = holdsLinksProbes.get(key) - if (cached !== undefined) return cached - const probe = spawnSync(helper, ['--holds-links'], { - timeout: 5000, - stdio: 'ignore', - // THREAT: this runs outside any sandbox, and an older helper looks the - // word up on PATH as a program. So there is nowhere to find one. - env: { PATH: '/dev/null', ...(argv0 ? { ARGV0: argv0 } : {}) }, - }) - if (probe.error !== undefined || probe.status === null) return false - holdsLinksProbes.set(key, probe.status === 0) - return probe.status === 0 +function whyLinksAreNotHeld( + helperInUse: boolean, + config: SeccompConfig | undefined, +): LinuxUnheldLinkReason | undefined { + if (!helperInUse) return 'no_helper' + if (config?.holdsLinks !== undefined) { + return config.holdsLinks ? undefined : 'told_no' + } + // THREAT: a path the caller gave need name the helper only inside the + // sandbox. Here it may name another file, and asking would run that file + // outside any sandbox. + const helper = config?.argv0 + ? null + : getApplySeccompBinaryPath(config?.applyPath) + if (helper === null || helper === config?.applyPath) return 'not_told' + let holds = holdsLinksProbes.get(helper) + // Twice: no answer is not "no", and a busy host is a passing reason for none. + for (let attempt = 0; holds === undefined && attempt < 2; attempt++) { + const probe = spawnSync(helper, ['--holds-links'], { + timeout: 5000, + stdio: 'ignore', + // THREAT: an older helper looks the word up on PATH as a program, and + // this runs outside any sandbox. So there is nowhere to find one. + env: { PATH: '/dev/null' }, + }) + if (probe.error === undefined && probe.status !== null) { + holds = probe.status === 0 + holdsLinksProbes.set(helper, holds) + } else { + logForDebugging( + `[Sandbox Linux] ${helper} --holds-links gave no answer: ${probe.error?.message ?? probe.signal}`, + { level: 'warn' }, + ) + } + } + if (holds === undefined) return 'no_answer' + return holds ? undefined : 'older_helper' } -const unheldLinks = new Set() +/** A symbolic link where it lies, and why nothing holds it there. */ +export type LinuxUnheldLink = { path: string; reason: LinuxUnheldLinkReason } + +// By link: what the latest wrap that met it came to. +const unheldLinks = new Map() /** - * The symbolic links that a write deny of some command wrapped so far leads - * through, that lie in a directory that command could write to, and that - * nothing kept in place there: the command could put something else at the - * name. See "Write denies and symbolic links" in README.md. + * The symbolic links on the way to a denied path that lie in a directory the + * command could write to, and that nothing kept in place there: the command + * could put something else at the name. Each link is listed by the latest + * wrap that met it, so one that a later wrap holds is listed no longer. See + * "Denied paths and symbolic links" in README.md. */ -export function getLinuxUnheldLinks(): string[] { - return [...unheldLinks] +export function getLinuxUnheldLinks(): LinuxUnheldLink[] { + return [...unheldLinks].map(([path, reason]) => ({ path, reason })) +} + +/** For `reset()` and `updateConfig()`: the list is of one configuration. */ +export function forgetLinuxUnheldLinks(): void { + unheldLinks.clear() } /** @@ -1921,7 +1974,7 @@ async function generateFilesystemArgs( ): Promise<{ args: string[] mandatoryDenyArgs: number - linksToHold: string[] + linksToHold: Array<{ path: string; target: string }> }> { const args: string[] = [] const startedIn = workingDirectory() @@ -1935,9 +1988,18 @@ async function generateFilesystemArgs( const denyWriteArgs: string[] = [] // Where the mandatory denies' binds begin in it: after the configuration's. let mandatoryDenyArgsStart = Infinity - // Every symbolic link on the way to a write-denied path, where it lies. The - // deny's own mount is where they lead, and bubblewrap can put none on a link. + // Every symbolic link on the way to a denied path, where it lies. The deny's + // own mount is where they lead, and bubblewrap can put none on a link. const linksToDenies = new Set() + // What stands at each name those walks looked at: one look a name a wrap. + const looked = new Map() + /** Notes the links on the way to `name`, a denied path as written. Returns + * the directory the walk could not look in, if there was one. */ + const noteLinksTo = (name: string): string | undefined => { + const { links, unsearched } = followLinks(name, looked) + for (const link of links) linksToDenies.add(link) + return unsearched + } // Directories that a deny entry re-binds read-only inside the sandbox // (--ro-bind ), keyed by resolved dest, with every raw // (pre-resolution) spelling each was reached through. A non-existent deny @@ -2426,6 +2488,22 @@ async function generateFilesystemArgs( // against it on both sides of the await above, the only one here. if (workingDirectory() !== startedIn) throw new WorkingDirectoryChanged() + // Each deny path as the two loops below take it, from one walk, so that + // they agree. A directory the walk could not look in stands in for the + // path: what is in it is not known, and bound read-only it keeps its mode. + const rawDenyPaths = denyPaths.map(pathPattern => { + const rawPath = normalizePathForSandbox(pathPattern, { literal: true }) + // Both loops skip it. + if (rawPath.startsWith('/dev/')) return rawPath + const unsearched = noteLinksTo(rawPath) + if (unsearched === undefined) return rawPath + logForDebugging( + `[Sandbox Linux] Write deny path ${rawPath} cannot be looked at; denying ${unsearched} whole instead`, + { level: 'warn' }, + ) + return unsearched + }) + // Duplicate deny entries must be collapsed: a duplicate // --ro-bind /dev/null hits a char device on the second pass and // bwrap's ensure_file() falls through to creat() on a read-only mount. @@ -2452,8 +2530,7 @@ async function generateFilesystemArgs( // 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, { literal: true }) + for (const rawPath of rawDenyPaths) { if (rawPath.startsWith('/dev/')) { continue } @@ -2604,17 +2681,15 @@ async function generateFilesystemArgs( } return covered } - for (const [index, pathPattern] of denyPaths.entries()) { + for (const [index, rawPath] of rawDenyPaths.entries()) { if (index === configuredDenyPaths.length) { mandatoryDenyArgsStart = denyWriteArgs.length } - const rawPath = normalizePathForSandbox(pathPattern, { literal: true }) // Skip /dev/* paths since --dev /dev already handles them if (rawPath.startsWith('/dev/')) { continue } - for (const link of followLinks(rawPath).links) linksToDenies.add(link) // Resolve-before-mask: normalizePathForSandbox keeps the raw symlink // path whenever resolution crosses isSymlinkOutsideBoundary (the @@ -3077,20 +3152,40 @@ async function generateFilesystemArgs( // the mount sits. Deny binds, tmpfs units and masks are emitted later and // land on top of both pins and covers. // + // The read side's names count like the write denies': each wrap finds what + // to hide by the name, so a name that leads elsewhere by the next wrap + // leaves what it led to unhidden. A path that cannot be looked at has its + // stand-in already (readDenyTargetOf). One that is where it resolves to has + // no link on the way, which is every match of a pattern but the links. + for (const name of [ + ...readDenyPlan().map(entry => entry.normalizedPath), + ...(readConfig?.matchedLinks ?? []), + ...(maskedFileBinds ?? []).map(bind => bind.realPath), + ]) { + const { canonical, resolved } = canonicalLocationOf(name) + if (!resolved || canonical !== name) noteLinksTo(name) + } // A link is held where the command could replace it: whoever replaces one // decides what the denied name leads to. That is in a directory it can write // to, and sees as it is on the host. The directories above a held link are // pinned like those above a mount. - const linksToHold = [...linksToDenies].filter( - link => - isWithinAnyAllowedWritePath(path.dirname(link)) && - !KERNEL_TOP_LEVEL_DIRS.some(dir => isAtOrUnder(link, dir)) && - !isHiddenByTmpfs(path.dirname(link), 'writes and reads'), - ) + const linksToHold = [...linksToDenies] + .filter( + link => + isWithinAnyAllowedWritePath(path.dirname(link)) && + !KERNEL_TOP_LEVEL_DIRS.some(dir => isAtOrUnder(link, dir)) && + !isHiddenByTmpfs(path.dirname(link), 'writes and reads'), + ) + // With what the link said when the plan was made for it, or nothing: no + // string spells a target that is no text. + .map(link => { + const target = looked.get(link) ?? '' + return { path: link, target: target.includes('\uFFFD') ? '' : target } + }) const pinArgs = ancestorPinArgs( [ ...denyWriteRawDests.keys(), - ...linksToHold, + ...linksToHold.map(link => link.path), ...fileMasks.map(mask => mask.landing), ...readDenyTmpfsUnits.map(unit => unit.landing), ], @@ -3576,27 +3671,28 @@ export async function wrapCommandWithSandboxLinux( allowGitConfig, abortSignal, ) - if ( - linksToHold.length > 0 && - applySeccompPrefix !== undefined && - seccompHelperHoldsLinks(seccompConfig?.applyPath, seccompConfig?.argv0) - ) { - applySeccompPrefix += `${quote(linksToHold.flatMap(link => ['--hold-link', link]))} ` - } else { - for (const link of linksToHold) { - unheldLinks.add(link) - logForDebugging( - `[Sandbox Linux] A write deny leads through the symbolic link ${link}, which the command can replace: holding it takes a seccomp helper with --hold-link`, - { level: 'warn' }, - ) - } - } + const whyNotHeld = + linksToHold.length > 0 + ? whyLinksAreNotHeld(applySeccompPrefix !== undefined, seccompConfig) + : undefined + const holdWords = + whyNotHeld === undefined + ? quote( + linksToHold.flatMap(link => [ + '--hold-link', + link.path, + link.target, + ]), + ) + : '' + if (holdWords !== '') applySeccompPrefix += `${holdWords} ` const mountsStart = bwrapArgs.length bwrapArgs.push(...fsArgs) const mounts = { start: mountsStart, end: bwrapArgs.length, mandatoryDenyArgs, + holdWordsBytes: Buffer.byteLength(holdWords, 'utf8'), } // Always bind /dev @@ -3694,6 +3790,18 @@ export async function wrapCommandWithSandboxLinux( `[Sandbox Linux] Wrapped command with bwrap (${restrictions.join(', ')} restrictions)`, ) + for (const { path: link } of linksToHold) { + if (whyNotHeld === undefined) { + unheldLinks.delete(link) + continue + } + unheldLinks.set(link, whyNotHeld) + logForDebugging( + `[Sandbox Linux] A denied path leads through the symbolic link ${link}, which the command can replace: nothing holds it (${whyNotHeld})`, + { level: 'warn' }, + ) + } + // INVARIANT: a wrap that hands out a command is in the count when it does, // whatever a forced cleanup did meanwhile: its caller gives one back. if (forcedCleanups !== countedAfter) activeSandboxCount++ diff --git a/src/sandbox/macos-sandbox-utils.ts b/src/sandbox/macos-sandbox-utils.ts index a93b04981..a4aa87402 100644 --- a/src/sandbox/macos-sandbox-utils.ts +++ b/src/sandbox/macos-sandbox-utils.ts @@ -374,10 +374,18 @@ function inBothSpellings(kind: PathListKind, entry: PathEntry): PathEntry[] { /** * `entry` with `canonical` for the path it is, or for the directory its * pattern starts from; nothing where that is no other spelling, or the root. + * Nor among the devices, as on Linux: a name blanked with a link to /dev/null + * would take /dev/null from every command. */ function startingFrom(canonical: string, entry: PathEntry): PathEntry[] { const base = entryBaseDir(entry) - if (canonical === base || canonical === '/') return [] + if ( + canonical === base || + canonical === '/' || + isAtOrUnder(canonical, '/dev') + ) { + return [] + } return [ entry.glob ? { @@ -1001,17 +1009,20 @@ function generateWriteRules( // Combine user-specified and mandatory deny patterns (no ripgrep needed on // macOS). The caller's spellings are patterns when they read as patterns; // the mandatory entries carry their own literal/glob split. + const looked = new Map() const walked = [ ...(config.denyWithinAllow || []).map(toPathEntry), ...(config.literalDenyWithinAllow ?? []).map(toLiteralPathEntry), ...macGetMandatoryDenyEntries(allowGitConfig), ...alsoReadAs(config.denyWithinAllow, 'deny'), - ].map(entry => ({ entry, ...followLinks(entryBaseDir(entry)) })) + ].map(entry => ({ entry, ...followLinks(entryBaseDir(entry), looked) })) // As written, and where the links on the way lead, there or not: that is the - // path Seatbelt compares a write through them with. - const denyEntries = walked.flatMap(({ entry, end }) => [ + // path Seatbelt compares a write through them with. A directory the walk + // could not look in is denied whole, its mode with it. + const denyEntries = walked.flatMap(({ entry, end, unsearched }) => [ entry, ...(end === undefined ? [] : startingFrom(end, entry)), + ...(unsearched === undefined ? [] : [toLiteralPathEntry(unsearched)]), ]) const { groups, rest: ungrouped } = groupLiteralDenyPaths(denyEntries) @@ -1041,6 +1052,7 @@ function generateWriteRules( // Each of those links stays where it lies: whoever replaces one decides // what the denied name leads to. for (const link of new Set(walked.flatMap(({ links }) => links))) { + if (isAtOrUnder(link, '/dev')) continue for (const kept of [link, ...getAncestorDirectories(link)]) { moveFilters.add(`(literal ${escapePath(kept)})`) } diff --git a/src/sandbox/read-deny-glob.ts b/src/sandbox/read-deny-glob.ts index d200139db..44a6a8ca0 100644 --- a/src/sandbox/read-deny-glob.ts +++ b/src/sandbox/read-deny-glob.ts @@ -75,12 +75,18 @@ function collapseReadDenyLocations({ * come back unmasked. * @param opts.anchor - as in `ExpandGlobOptions.anchor`. * @param opts.listings - one map for all the patterns of a configuration. + * @param opts.matchedLinks - receives the matches that are symbolic links, by + * name: what is returned for one is where it leads. */ export function expandReadDenyGlobLinux( globPattern: string, reExposedPaths: readonly string[], unlistableDirs?: Set, - opts: { anchor?: string; listings?: GlobWalkListings } = {}, + opts: { + anchor?: string + listings?: GlobWalkListings + matchedLinks?: Set + } = {}, ): string[] { return finish( expandReadDenyGlobLinuxSteps( @@ -97,7 +103,11 @@ export function* expandReadDenyGlobLinuxSteps( globPattern: string, reExposedPaths: readonly string[], unlistableDirs?: Set, - opts: { anchor?: string; listings?: GlobWalkListings } = {}, + opts: { + anchor?: string + listings?: GlobWalkListings + matchedLinks?: Set + } = {}, ): Steps { const walk = yield* walkGlobPatternSteps(globPattern, { withDirectoryForm: true, @@ -137,6 +147,7 @@ export function* expandReadDenyGlobLinuxSteps( if (!namedBy.has(location)) namedBy.set(location, candidate) } for (const candidate of candidates) { + if (walk.symlinks.has(candidate)) opts.matchedLinks?.add(candidate) if (walk.symlinks.has(candidate) && !walk.realOf.has(candidate)) { if (!walk.uninspectableLinks.has(candidate)) { // A link that resolves to nothing denies nothing, and bwrap cannot diff --git a/src/sandbox/sandbox-config.ts b/src/sandbox/sandbox-config.ts index 122994d23..032c61f7a 100644 --- a/src/sandbox/sandbox-config.ts +++ b/src/sandbox/sandbox-config.ts @@ -1102,6 +1102,16 @@ export const SeccompConfigSchema = z.object({ 'applyPath resolves inside the bwrap namespace and that the target ' + 'binary implements the apply-seccomp interface when ARGV0 matches.', ), + holdsLinks: z + .boolean() + .optional() + .describe( + 'Whether the helper takes --hold-link, by which the symbolic links on ' + + 'the way to denied paths are kept in place. Taken at its word: ' + + 'nothing is run to find out. Not set, a helper the library found ' + + 'for itself is asked (--holds-links), and one named by applyPath is ' + + 'handed no link. false also serves to turn holding off.', + ), }) /** diff --git a/src/sandbox/sandbox-manager.ts b/src/sandbox/sandbox-manager.ts index 643e5e1b1..b09c4cca8 100644 --- a/src/sandbox/sandbox-manager.ts +++ b/src/sandbox/sandbox-manager.ts @@ -63,7 +63,9 @@ import { checkLinuxDependencies, type SandboxDependencyCheck, cleanupBwrapMountPoints, + forgetLinuxUnheldLinks, getLinuxUnheldLinks, + type LinuxUnheldLink, linuxGetCwdMandatoryDenyPaths, } from './linux-sandbox-utils.js' import { expandReadDenyGlobLinuxSteps } from './read-deny-glob.js' @@ -2189,6 +2191,7 @@ async function wrapWithSandboxOnce( writeConfig, ) const unlistableDenyDirs = new Set() + const matchedLinks = new Set() const listings: GlobWalkListings = new Map() const expandedDenyRead = await walked( resolveReadDenies( @@ -2199,7 +2202,7 @@ async function wrapWithSandboxOnce( pattern, reExposedPaths, unlistableDenyDirs, - { anchor, listings }, + { anchor, listings, matchedLinks }, ), ), ) @@ -2212,6 +2215,7 @@ async function wrapWithSandboxOnce( allowWithinDeny: expandedAllowRead, ...(ownAllowWithinDeny.length > 0 && { ownAllowWithinDeny }), unlistableDenyDirs: [...unlistableDenyDirs], + matchedLinks: [...matchedLinks], ...literalReadLists( customConfig?.filesystem?.denyRead ?? config?.filesystem.denyRead, customConfig?.filesystem?.allowRead ?? config?.filesystem.allowRead, @@ -2573,6 +2577,7 @@ function updateConfig(newConfig: SandboxRuntimeConfig): void { config = structuredClone({ ...newConfig, network: rest }) config.network.filterRequest = filterRequest configInstalls++ + forgetLinuxUnheldLinks() resolvedAddressGuard = nextGuard // Re-resolve parent proxy so hot-reload picks up changes. Note: the proxy // servers capture `parentProxy` by value at creation, so changes here take @@ -2748,6 +2753,7 @@ async function reset(): Promise { // Clean up any leftover bwrap mount points. Force past the // active-sandbox counter — reset() means the session is over. cleanupBwrapMountPoints({ force: true }) + forgetLinuxUnheldLinks() // Stop log monitor if (logMonitorShutdown) { @@ -3047,11 +3053,12 @@ export interface ISandboxManager { annotateStderrWithSandboxFailures(command: string, stderr: string): string getLinuxGlobPatternWarnings(): string[] /** - * Linux: the symbolic links on the way to a write-denied path that a - * command wrapped by this process could replace, for the embedder to show - * or to look at again afterwards. Empty where every one was held in place. + * Linux: the symbolic links on the way to a denied path that a command + * could replace, each with the reason nothing holds it, for the embedder to + * show or to look at again after the command. Each link is listed by the + * latest wrap that met it; `reset()` and `updateConfig()` empty the list. */ - getLinuxUnheldLinks(): string[] + getLinuxUnheldLinks(): LinuxUnheldLink[] getConfig(): SandboxRuntimeConfig | undefined getMitmCA(): MitmCA | undefined getSentinelRegistry(): SentinelRegistry diff --git a/src/sandbox/sandbox-schemas.ts b/src/sandbox/sandbox-schemas.ts index f286ec867..7221193bc 100644 --- a/src/sandbox/sandbox-schemas.ts +++ b/src/sandbox/sandbox-schemas.ts @@ -45,6 +45,12 @@ export interface FsReadRestrictionConfig { * unmasked. Linux only: the other backends match globs natively. */ unlistableDenyDirs?: string[] + /** + * The names a glob expansion matched that are symbolic links. `denyOnly` + * holds what each leads to, which is where its mount goes; the next + * expansion finds that by the name again. Linux only, likewise. + */ + matchedLinks?: string[] } /** diff --git a/src/sandbox/sandbox-utils.ts b/src/sandbox/sandbox-utils.ts index e0ea17b27..73002f05a 100644 --- a/src/sandbox/sandbox-utils.ts +++ b/src/sandbox/sandbox-utils.ts @@ -110,18 +110,38 @@ export function nameLocation( /** As many links as the kernel follows on the way to one path. */ const MAX_LINKS_FOLLOWED = 40 +/** What a look at a name fails with where it is settled that no link is there: + * nothing is, it is no link by now, or no call takes the name (a NUL byte). */ +const NO_LINK_THERE = ['ENOENT', 'ENOTDIR', 'EINVAL', 'ERR_INVALID_ARG_VALUE'] + /** * The absolute path `p` walked name by name: the symbolic links met on the - * way, each where it lies, and where the way ends. A `..` in a link's target - * is the parent of the directory reached, as it is to the kernel. A name that - * is not there is taken as written, so a dangling link ends at the path a - * write through it would create. There is no end past the number of links the - * kernel follows, nor past a target that is no text. + * way, each where it lies, and where the way ends. + * + * A `.`, `..` or `//` in `p` itself is folded in text before anything is + * looked at, as everywhere a configured name is read: `link/..` is the + * directory that holds the link, which is not where the kernel goes. A `..` in + * a link's target is read as the kernel reads it, the parent of the directory + * reached. + * + * A name that is not there is taken as written, so a dangling link ends at the + * path a write through it would create. There is no end past the number of + * links the kernel follows, nor past a target that is no text. + * + * INVARIANT: "could not look" is never "no link". Only a name that is not + * there (ENOENT, ENOTDIR), is no link, or is one no call takes (a NUL byte) + * counts as none. On any other failure the walk stops with no end, and + * `unsearched` is the directory reached, in which it could not look: a + * same-uid command can take a directory's search bit away and give it back + * from inside the next sandbox. Never the root, which stands in for nothing. + * + * `looked` holds what stands at each name looked at, a link's target or null. + * One map for the paths of one wrap has a directory they share looked at once. */ -export function followLinks(p: string): { - links: string[] - end: string | undefined -} { +export function followLinks( + p: string, + looked: Map = new Map(), +): { links: string[]; end: string | undefined; unsearched?: string } { const links: string[] = [] const rest = path.resolve(p).split('/') // No link in it. @@ -133,10 +153,24 @@ export function followLinks(p: string): { continue } const next = `${reached}/${name}` - let target: string - try { - target = fs.readlinkSync(next) - } catch { + let target = looked.get(next) + if (target === undefined) { + try { + // Nearly every name is no link, and asking one for its target throws. + target = fs.lstatSync(next, { throwIfNoEntry: false })?.isSymbolicLink() + ? fs.readlinkSync(next) + : null + } catch (err) { + if ( + !NO_LINK_THERE.includes((err as NodeJS.ErrnoException).code ?? '') + ) { + return { links, end: undefined, unsearched: reached || undefined } + } + target = null + } + looked.set(next, target) + } + if (target === null) { reached = next continue } diff --git a/test/sandbox/literal-path-entries.test.ts b/test/sandbox/literal-path-entries.test.ts index 668529ae6..ea047042b 100644 --- a/test/sandbox/literal-path-entries.test.ts +++ b/test/sandbox/literal-path-entries.test.ts @@ -1321,9 +1321,9 @@ describe.if(isLinux)( it.skipIf(!CAN_RUN || process.getuid?.() === 0)( 'takes any write deny with glob characters for the path it spells when the folder cannot be looked at', async () => { - // Whether anything in the folder has the name cannot be told, so a - // pattern is also applied like a path that is not there: the name is - // kept from being created, and what it would match is not denied. + // Whether anything in the folder has the name cannot be told, nor + // what: so the folder is denied whole, wherever it lies. Its mode + // cannot be given back, so what the pattern would match is safe too. const locked = join(root, 'locked') const elsewhere = join(root, 'elsewhere') mkdirSync(locked) @@ -1377,13 +1377,12 @@ describe.if(isLinux)( const { denied, stdout } = await wrapsWith(name, elsewhere) expect(denied).toEqual([join(locked, name)]) expect(stdout).not.toContain('CREATED') - expect(stdout).toContain('WROTE') + expect(stdout).not.toContain('WROTE') expect(stdout).toContain('END') expect(existsSync(join(locked, name))).toBe(false) - // Below the working directory the folder is denied whole, because - // the scan for dangerous names could not read it either: its mode - // cannot be given back, so what the pattern would match is safe too. + // Below the working directory the scan for dangerous names, which + // could not read it either, comes to the same. const below = await wrapsWith(name, root) expect(below.denied).toEqual([join(locked, name)]) expect(below.stdout).not.toContain('CREATED') @@ -1391,12 +1390,10 @@ describe.if(isLinux)( expect(below.stdout).toContain('END') expect(existsSync(join(locked, name))).toBe(false) } - // The folder cannot be cleaned by the host while it cannot be looked - // at, for a name and for a pattern alike. - const leftByName = await leftBy('absent') - expect(await leftBy('*.pem')).toEqual( - leftByName.map(entry => (entry === 'absent' ? '*.pem' : entry)), - ) + // Nothing is made in a folder the host could not clean while it cannot + // be looked at, for a name and for a pattern alike. + expect(await leftBy('absent')).toEqual([]) + expect(await leftBy('*.pem')).toEqual([]) }, WRAP_TIMEOUT_MS, ) diff --git a/test/sandbox/write-deny-through-links.test.ts b/test/sandbox/write-deny-through-links.test.ts index b0c2d989d..a154445e9 100644 --- a/test/sandbox/write-deny-through-links.test.ts +++ b/test/sandbox/write-deny-through-links.test.ts @@ -1,4 +1,5 @@ -import { describe, it, expect, beforeEach, afterEach } from 'bun:test' +import { describe, it, expect, beforeEach, afterEach, spyOn } from 'bun:test' +import * as fs from 'fs' import { spawnSync } from 'node:child_process' import { chmodSync, @@ -13,24 +14,31 @@ import { symlinkSync, writeFileSync, } from 'node:fs' -import { tmpdir } from 'node:os' -import { join } from 'node:path' -import { getApplySeccompBinaryPath } from '../../src/sandbox/generate-seccomp-filter.js' +import { homedir, tmpdir } from 'node:os' +import { join, relative } from 'node:path' +import type { LinuxUnheldLinkReason } from '../../src/index.js' +import * as seccomp from '../../src/sandbox/generate-seccomp-filter.js' import { cleanupBwrapMountPoints, + forgetLinuxUnheldLinks, getLinuxUnheldLinks, wrapCommandWithSandboxLinux, } from '../../src/sandbox/linux-sandbox-utils.js' import { wrapCommandWithSandboxMacOS } from '../../src/sandbox/macos-sandbox-utils.js' +import { + SandboxRuntimeConfigSchema, + type SandboxRuntimeConfig, +} from '../../src/sandbox/sandbox-config.js' +import { SandboxManager } from '../../src/sandbox/sandbox-manager.js' import { followLinks } from '../../src/sandbox/sandbox-utils.js' import { bwrapCanNamespace } from '../helpers/bwrap-namespace.js' import { withCapturedWarnings } from '../helpers/captured-warnings.js' import { isLinux, isMacOS, isWindows } from '../helpers/platform.js' /** - * A write deny on a path with a symbolic link on the way to it, the last name - * included, holds what the links lead to and the links themselves. Everything - * beside them stays as writable as it was. + * A deny on a path with a symbolic link on the way to it, the last name + * included, covers what the links lead to and keeps the links where they are. + * Everything beside them stays as writable as it was. * * Every link here is made on the host before the command is wrapped. */ @@ -47,6 +55,8 @@ const LINKS = { way: 'real', first: 'middle', middle: 'conf/y', + blank: '/dev/null', + out: '/dev/stdout', 'other-link': 'conf/z', } const FILES = [ @@ -65,7 +75,11 @@ const DENIED = [ 'way/secret', 'way/absent', 'first', + 'blank', + 'out', ] +/** No mode keeps uid 0 from looking. */ +const modesCount = process.getuid?.() !== 0 beforeEach(() => { if (isWindows) return @@ -86,11 +100,31 @@ beforeEach(() => { afterEach(() => { if (isWindows) return if (isLinux) cleanupBwrapMountPoints({ force: true }) + chmodSync(join(ROOT, 'holder'), 0o755) rmSync(BASE, { recursive: true, force: true }) }) const inRoot = (names: string[]): string[] => names.map(n => join(ROOT, n)) +/** `run` with ROOT for the working directory of this process. */ +async function fromRoot(run: () => T | Promise): Promise { + const cwd = process.cwd() + process.chdir(ROOT) + try { + return await run() + } finally { + process.chdir(cwd) + } +} + +/** An executable file in OUTSIDE that holds `script`. */ +function program(name: string, script: string): string { + const file = join(OUTSIDE, name) + writeFileSync(file, `#!/bin/sh\n${script}\n`) + chmodSync(file, 0o755) + return file +} + describe.if(!isWindows)('the links on the way to a path', () => { it.each<[string, string[], string]>([ ['beside', [], 'beside'], @@ -100,6 +134,8 @@ describe.if(!isWindows)('the links on the way to a path', () => { ['way/absent/deeper', ['way'], 'real/absent/deeper'], ['first', ['first', 'middle'], 'conf/y'], ['absent/name', [], 'absent/name'], + ['beside/name', [], 'beside/name'], + ['./way/./secret/', ['way'], 'real/secret'], ])('%s: %p, ending at %s', (p, links, end) => { expect(followLinks(join(ROOT, p))).toEqual({ links: inRoot(links), @@ -107,15 +143,35 @@ describe.if(!isWindows)('the links on the way to a path', () => { }) }) + it('takes a name with a NUL byte, which no call takes, as written', () => { + expect(followLinks(join(ROOT, 'way', 'nul\0byte'))).toEqual({ + links: inRoot(['way']), + end: join(ROOT, 'real', 'nul\0byte'), + }) + }) + it('reads `..` in a target as the parent of the directory reached', () => { symlinkSync(join(ROOT, 'shared', 'commands'), join(ROOT, 'absolute')) - symlinkSync('absolute/../made', join(ROOT, 'up')) + symlinkSync('./absolute/../made', join(ROOT, 'up')) expect(followLinks(join(ROOT, 'up'))).toEqual({ links: inRoot(['up', 'absolute']), end: join(ROOT, 'shared', 'made'), }) }) + it('folds `..` in the path it is given in text', () => { + expect(followLinks(`${ROOT}/holder/commands/../name`)).toEqual({ + links: [], + end: join(ROOT, 'holder', 'name'), + }) + }) + + it('takes a relative path from the working directory', async () => { + expect(await fromRoot(() => followLinks('name').end)).toBe( + join(ROOT, 'conf', 'x'), + ) + }) + it('names a link where it lies, whatever it was reached through', () => { symlinkSync(ROOT, join(OUTSIDE, 'to-root')) expect(followLinks(join(OUTSIDE, 'to-root', 'name'))).toEqual({ @@ -133,10 +189,72 @@ describe.if(!isWindows)('the links on the way to a path', () => { }) }) - it('has no end for links that lead in a circle', () => { - symlinkSync('circle-b', join(ROOT, 'circle-a')) - symlinkSync('circle-a', join(ROOT, 'circle-b')) - expect(followLinks(join(ROOT, 'circle-a')).end).toBeUndefined() + it('follows as many links as the kernel, and has no end past them', () => { + symlinkSync('beside', join(ROOT, 'hop-1')) + for (let i = 2; i <= 41; i++) { + symlinkSync(`hop-${i - 1}`, join(ROOT, `hop-${i}`)) + } + expect(followLinks(join(ROOT, 'hop-40')).end).toBe(join(ROOT, 'beside')) + expect(followLinks(join(ROOT, 'hop-41')).end).toBeUndefined() + expect(() => readFileSync(join(ROOT, 'hop-40'))).not.toThrow() + expect(() => readFileSync(join(ROOT, 'hop-41'))).toThrow('ELOOP') + }) + + it.if(modesCount)( + 'stops at a directory it cannot look in, and says which', + () => { + chmodSync(join(ROOT, 'holder'), 0) + symlinkSync('holder/commands', join(ROOT, 'to-holder')) + expect(followLinks(join(ROOT, 'to-holder', 'file'))).toEqual({ + links: inRoot(['to-holder']), + end: undefined, + unsearched: join(ROOT, 'holder'), + }) + }, + ) + + it.each([ + ['ENOENT', undefined], + ['EINVAL', undefined], + ['EIO', ''], + ['ESTALE', ''], + ['ENAMETOOLONG', ''], + ])('a look that fails with %s: could not look in %p', (code, unsearched) => { + const spy = spyOn(fs, 'readlinkSync').mockImplementation(() => { + throw Object.assign(new Error(code), { code }) + }) + try { + expect(followLinks(join(ROOT, 'name')).unsearched).toBe( + unsearched === undefined ? undefined : ROOT, + ) + } finally { + spy.mockRestore() + } + }) + + it('never gives the root for the directory it could not look in', () => { + const spy = spyOn(fs, 'lstatSync').mockImplementation(() => { + throw Object.assign(new Error('EIO'), { code: 'EIO' }) + }) + try { + expect(followLinks(ROOT)).toEqual({ links: [], end: undefined }) + } finally { + spy.mockRestore() + } + }) + + it('looks at a name once for all the paths that share a map', () => { + const looked = new Map() + followLinks(join(ROOT, 'name'), looked) + expect(looked.get(ROOT)).toBeNull() + expect(looked.get(join(ROOT, 'name'))).toBe('conf/x') + expect(looked.get(join(ROOT, 'conf', 'x'))).toBeNull() + + rmSync(join(ROOT, 'name')) + expect(followLinks(join(ROOT, 'name'), looked).links).toEqual( + inRoot(['name']), + ) + expect(followLinks(join(ROOT, 'name')).links).toEqual([]) }) }) @@ -151,8 +269,13 @@ describe.if((isLinux && bwrapCanNamespace()) || isMacOS)( const options = { command: `cd ${ROOT} && echo started && if ( ${attempt} ); then echo DONE; else echo refused; fi`, needsNetworkRestriction: false, - readConfig: { denyOnly: [] }, - writeConfig: { allowOnly: [ROOT], denyWithinAllow: inRoot(denied) }, + // Linux mounts a read deny where it resolves, under the /dev it then + // makes anew, and bubblewrap before 0.5.0 takes no device to mount on. + readConfig: { denyOnly: isMacOS ? inRoot(['blank']) : [] }, + writeConfig: { + allowOnly: [ROOT, '/dev/null', '/dev/stdout'], + denyWithinAllow: inRoot(denied), + }, } const wrapped = isLinux ? await wrapCommandWithSandboxLinux(options) @@ -202,6 +325,8 @@ describe.if((isLinux && bwrapCanNamespace()) || isMacOS)( ['a write through two links', 'echo changed >> first'], ['removing the second of two links', 'rm middle'], ['renaming a file over it', 'echo new > made && mv -f made middle'], + ['removing a link to a device', 'rm blank'], + ['making it anew', 'ln -sfn beside out'], ])('refuses %s: %s', async (_what, attempt) => { expect(await outcomeOf(attempt)).toBe('refused') @@ -226,28 +351,47 @@ describe.if((isLinux && bwrapCanNamespace()) || isMacOS)( 'reading the link, and through it', 'test "$(readlink name)$(cat name)$(ls holder/commands)" = conf/xoriginalfile', ], + ['a write to a device a denied name leads to', 'echo x > /dev/null'], + ['a read of it', 'cat /dev/null'], + // Linux opens what is behind it anew, which a socket refuses. + ...(isMacOS ? [['the same of the output', 'echo x > /dev/stdout']] : []), ])('allows %s: %s', async (_what, attempt) => { expect(await outcomeOf(attempt)).toBe('DONE') }) it.each([ - 'rm .bashrc', - 'echo changed >> conf/x', - 'rm .claude/commands', - 'echo new > shared/commands/new', + ['refused', 'rm .bashrc'], + ['refused', 'echo changed >> conf/x'], + ['refused', 'rm .claude/commands'], + ['refused', 'echo new > shared/commands/new'], + ['refused', 'rm .zshrc'], + ['DONE', 'echo x > /dev/null'], ])( - 'refuses the same of a name denied in every working directory: %s', - async attempt => { + 'the same of the names denied in every working directory: %s: %s', + async (outcome, attempt) => { mkdirSync(join(ROOT, '.claude')) symlinkSync('conf/x', join(ROOT, '.bashrc')) + symlinkSync('/dev/null', join(ROOT, '.zshrc')) symlinkSync('../shared/commands', join(ROOT, '.claude', 'commands')) - const cwd = process.cwd() - process.chdir(ROOT) - try { - expect(await outcomeOf(attempt, [])).toBe('refused') - } finally { - process.chdir(cwd) - } + expect(await fromRoot(() => outcomeOf(attempt, []))).toBe(outcome) + }, + ) + + it.if(modesCount)( + 'keeps a directory that cannot be looked in as it is, mode and all', + async () => { + expect(await outcomeOf('chmod 000 holder')).toBe('DONE') + expect( + await outcomeOf( + 'chmod 755 holder; rm holder/commands || rm -r holder', + ), + ).toBe('refused') + + expect(lstatSync(join(ROOT, 'holder')).mode & 0o777).toBe(0) + chmodSync(join(ROOT, 'holder'), 0o755) + expect(readlinkSync(join(ROOT, 'holder', 'commands'))).toBe( + LINKS['holder/commands'], + ) }, ) }, @@ -267,6 +411,7 @@ describe.if(!isWindows)('Seatbelt profile', () => { expect(at).toBeGreaterThan(-1) return profile.slice(at, profile.indexOf('(with message', at)) } + const KEPT = 'file-write-unlink file-write-create' const subpath = (name: string): string => ` (subpath "${join(ROOT, name)}")\n` const literal = (name: string): string => @@ -285,15 +430,11 @@ describe.if(!isWindows)('Seatbelt profile', () => { expect(denies).toContain(subpath(end)) }) - it('so is one on a name denied in every working directory', () => { + it('so is one on a name denied in every working directory', async () => { symlinkSync('conf/x', join(ROOT, '.bashrc')) - const cwd = process.cwd() - process.chdir(ROOT) - try { - expect(deniedBy('file-write*', [])).toContain(subpath('conf/x')) - } finally { - process.chdir(cwd) - } + expect(await fromRoot(() => deniedBy('file-write*', []))).toContain( + subpath('conf/x'), + ) }) it('a pattern deny starts from both spellings of its directory', () => { @@ -311,187 +452,504 @@ describe.if(!isWindows)('Seatbelt profile', () => { mkdirSync(join(ROOT, 'elsewhere')) symlinkSync('../conf/x', join(ROOT, 'elsewhere', 'hop')) symlinkSync('elsewhere/hop', join(ROOT, 'far')) - const kept = deniedBy( - 'file-write-unlink file-write-create', - inRoot(['first', 'far']), - ) + const kept = deniedBy(KEPT, inRoot(['first', 'far'])) for (const name of ['middle', 'elsewhere/hop', 'elsewhere', 'conf']) { expect(kept).toContain(literal(name)) } expect(kept).not.toContain(join(ROOT, 'other-link')) }) -}) -describe.if(isLinux)('what the seccomp helper is asked to hold', () => { - const wrap = ( - denyWithinAllow: string[], - rest: Partial[0]> = {}, - ): Promise => - wrapCommandWithSandboxLinux({ - command: 'echo started', + it('nothing among the devices is denied or kept, and the link to it is kept', () => { + symlinkSync('/dev', join(ROOT, 'devices')) + const denied = inRoot(['blank', 'out', 'devices']) + for (const operations of ['file-write*', KEPT]) { + expect(deniedBy(operations, denied)).not.toContain('"/dev') + } + for (const name of ['blank', 'out', 'devices']) { + expect(deniedBy(KEPT, denied)).toContain(literal(name)) + } + }) + + it('nor is a device hidden by a read deny on a link to it', () => { + const profile = wrapCommandWithSandboxMacOS({ + command: 'true', needsNetworkRestriction: false, - readConfig: { denyOnly: [] }, - writeConfig: { allowOnly: [ROOT], denyWithinAllow }, - ...rest, + readConfig: { denyOnly: inRoot(['blank', 'name']) }, + writeConfig: undefined, }) - const held = (wrapped: string): string[] => - [...wrapped.matchAll(/--hold-link (\S+)/g)].map(match => match[1]!) + const at = profile.indexOf('(deny file-read*\n') + const denies = profile.slice(at, profile.indexOf('(with message', at)) + expect(denies).toContain(subpath('conf/x')) + expect(denies).not.toContain('"/dev') + }) + + it.if(modesCount)( + 'a directory that cannot be looked in is denied whole', + () => { + chmodSync(join(ROOT, 'holder'), 0) + expect(deniedBy('file-write*', inRoot(['holder/commands']))).toContain( + subpath('holder'), + ) + expect(deniedBy('file-write*', inRoot(['name']))).not.toContain( + subpath('holder'), + ) + }, + ) +}) + +type LinuxOptions = Parameters[0] + +/** `denyWithinAllow` denied in ROOT, wrapped for Linux. */ +const wrapLinux = ( + denyWithinAllow: string[], + rest: Partial = {}, +): Promise => + wrapCommandWithSandboxLinux({ + command: 'echo started', + needsNetworkRestriction: false, + readConfig: { denyOnly: [] }, + writeConfig: { allowOnly: [ROOT], denyWithinAllow }, + ...rest, + }) - it('each link once, where it lies', async () => { +const stdoutOf = (wrapped: string): string => + spawnSync(wrapped, { + shell: true, + encoding: 'utf8', + cwd: OUTSIDE, + timeout: 30000, + }).stdout + +/** + * The words a wrap hands the seccomp helper before the command, as [link, + * target] pairs. Read by the shells themselves, through every level of + * quoting, and with no namespace made: a stand-in for bubblewrap runs what + * follows `--`, and one for the helper prints its words and runs nothing. + */ +async function heldBy( + denyWithinAllow: string[], + rest: Partial = {}, +): Promise> { + const words = stdoutOf( + await wrapLinux(denyWithinAllow, { + bwrapPath: program( + 'bwrap-stand-in', + 'while [ "$1" != -- ]; do shift; done; shift; exec "$@"', + ), + seccompConfig: { + applyPath: program('helper-stand-in', `printf '%s\\0' "$@"`), + holdsLinks: true, + }, + ...rest, + }), + ).split('\0') + const held: Array<[string, string]> = [] + for (let i = 0; words[i] === '--hold-link'; i += 3) { + held.push([words[i + 1]!, words[i + 2]!]) + } + expect(words.slice(3 * held.length + 1)).toEqual(['-c', 'echo started', '']) + return held.sort() +} +/** LINKS' names as {@link heldBy} gives them. */ +const pairs = (...names: Array): Array<[string, string]> => + names.sort().map(name => [join(ROOT, name), LINKS[name]]) + +describe.if(isLinux)('what the seccomp helper is asked to hold', () => { + it('each link once, where it lies, with what it says', async () => { symlinkSync(ROOT, join(OUTSIDE, 'to-root')) expect( - held( - await wrap([ - ...inRoot(['first', 'way/secret', 'way/absent']), - join(OUTSIDE, 'to-root', 'middle'), - ]), - ).sort(), - ).toEqual(inRoot(['first', 'middle', 'way'])) + await heldBy([ + ...inRoot(['first', 'way/secret', 'way/absent']), + join(OUTSIDE, 'to-root', 'middle'), + ]), + ).toEqual(pairs('first', 'middle', 'way')) + }) + + it('whatever characters the name and the target hold', async () => { + const odd = [ + "sp ace'quote", + 'new\nline', + '$(echo x)`y`', + '--hold-link', + '!', + ] + for (const name of odd) symlinkSync(`to ${name}`, join(ROOT, name)) + expect(await heldBy(inRoot(odd))).toEqual( + odd.sort().map(name => [join(ROOT, name), `to ${name}`]), + ) + }) + + it('with no word on a target that is no text', async () => { + symlinkSync(Buffer.from([0x78, 0xff]), join(ROOT, 'bytes')) + expect(await heldBy(inRoot(['bytes']))).toEqual([[join(ROOT, 'bytes'), '']]) + }) + + it('for a deny written from the home or the working directory', async () => { + const fromHome = `~/${relative(homedir(), join(ROOT, 'way', 'x'))}` + expect(await fromRoot(() => heldBy(['name', fromHome]))).toEqual( + pairs('name', 'way'), + ) + }) + + it('for a name denied in every working directory', async () => { + symlinkSync('conf/x', join(ROOT, '.bashrc')) + expect(await fromRoot(() => heldBy([]))).toEqual([ + [join(ROOT, '.bashrc'), 'conf/x'], + ]) + }) + + it('for a link that leads out of every write root, or to a device', async () => { + symlinkSync(OUTSIDE, join(ROOT, 'leads-out')) + expect(await heldBy(inRoot(['leads-out/file', 'blank']))).toEqual([ + ...pairs('blank'), + [join(ROOT, 'leads-out'), OUTSIDE], + ]) + }) + + it('under a write root that is the root', async () => { + expect( + await heldBy([], { + writeConfig: { allowOnly: ['/'], denyWithinAllow: inRoot(['name']) }, + }), + ).toEqual(pairs('name')) }) it('none for a path with no link on the way', async () => { - expect(held(await wrap(inRoot(['beside', 'conf/x', 'absent'])))).toEqual([]) + expect(await heldBy(inRoot(['beside', 'conf/x', 'absent']))).toEqual([]) }) it('none that lies where the command cannot write', async () => { symlinkSync(join(ROOT, 'conf', 'x'), join(OUTSIDE, 'into-root')) - expect(held(await wrap([join(OUTSIDE, 'into-root')]))).toEqual([]) + expect(await heldBy([join(OUTSIDE, 'into-root')])).toEqual([]) }) it('none in the directories the sandbox mounts anew', async () => { expect( - held( - await wrap(['/proc/self/cwd/file'], { - writeConfig: { - allowOnly: ['/'], - denyWithinAllow: ['/proc/self/cwd/file'], - }, - }), - ), - ).toEqual([]) + await heldBy([], { + writeConfig: { + allowOnly: ['/'], + denyWithinAllow: ['/proc/self/cwd/file', join(ROOT, 'out')], + }, + }), + ).toEqual(pairs('out')) }) - it.if(bwrapCanNamespace())( - 'none that a read deny hides, and the command starts', - async () => { - const wrapped = await wrap(inRoot(['holder/commands', 'name']), { - readConfig: { denyOnly: [join(ROOT, 'holder')] }, - }) - expect(held(wrapped)).toEqual(inRoot(['name'])) - expect( - spawnSync(wrapped, { shell: true, encoding: 'utf8', cwd: OUTSIDE }) - .stdout, - ).toBe('started\n') - }, - ) + it('none that a read deny hides', async () => { + const options = { readConfig: { denyOnly: [join(ROOT, 'holder')] } } + const denied = inRoot(['holder/commands', 'name']) + expect(await heldBy(denied, options)).toEqual(pairs('name')) + if (bwrapCanNamespace()) { + expect(stdoutOf(await wrapLinux(denied, options))).toBe('started\n') + } + }) + + it('none that is bound back by itself into a directory a read deny hides', async () => { + mkdirSync(join(ROOT, 'holder', 'inner')) + symlinkSync('inner', join(ROOT, 'holder', 'to-inner')) + const options = { + readConfig: { + denyOnly: [join(ROOT, 'holder')], + allowWithinDeny: inRoot(['holder/to-inner']), + }, + } + const denied = inRoot(['holder/to-inner/file']) + expect(await heldBy(denied, options)).toEqual([]) + if (bwrapCanNamespace()) { + expect(stdoutOf(await wrapLinux(denied, options))).toBe('started\n') + } + }) + + it('the directories above a held link are pinned', async () => { + const holder = join(ROOT, 'holder') + expect(await wrapLinux(inRoot(['holder/commands']))).toContain( + `--ro-bind ${holder} ${holder} `, + ) + }) - it.if(bwrapCanNamespace())( - 'none that is bound back by itself into a directory a read deny hides', + it.if(modesCount)( + 'a directory that cannot be looked in is bound read-only in place of the path', async () => { - mkdirSync(join(ROOT, 'holder', 'inner')) - symlinkSync('inner', join(ROOT, 'holder', 'to-inner')) - const wrapped = await wrap(inRoot(['holder/to-inner/file']), { - readConfig: { - denyOnly: [join(ROOT, 'holder')], - allowWithinDeny: inRoot(['holder/to-inner']), - }, - }) - expect(held(wrapped)).toEqual([]) + const holder = join(ROOT, 'holder') + chmodSync(holder, 0) + const { result: wrapped, warnings } = await withCapturedWarnings(() => + wrapLinux(inRoot(['holder/commands', 'holder/other'])), + ) + // Beneath the write root's bind it is a pin, after it a deny. expect( - spawnSync(wrapped, { shell: true, encoding: 'utf8', cwd: OUTSIDE }) - .stdout, - ).toBe('started\n') + wrapped.lastIndexOf(`--ro-bind ${holder} ${holder} `), + ).toBeGreaterThan(wrapped.indexOf(`--bind ${ROOT} ${ROOT} `)) + expect(wrapped).not.toContain(`${holder}/`) + expect(warnings.join()).toContain(`denying ${holder} whole`) }, ) - it('the directories above a held link are pinned', async () => { - const holder = join(ROOT, 'holder') - expect(await wrap(inRoot(['holder/commands']))).toContain( - `--ro-bind ${holder} ${holder} `, + it.each<[string, (name: string) => Partial]>([ + ['a read deny', name => ({ readConfig: { denyOnly: [name] } })], + [ + 'a match of a read-deny pattern', + name => ({ readConfig: { denyOnly: [], matchedLinks: [name] } }), + ], + [ + 'a masked file', + name => ({ maskedFileBinds: [{ realPath: name, fakePath: '/fakes/0' }] }), + ], + ])('the links on the way to %s', async (_what, options) => { + expect(await heldBy([], options(join(ROOT, 'first')))).toEqual( + pairs('first', 'middle'), ) }) - describe('where nothing can hold them', () => { - /** Runs its words, as a helper built before the option does. */ - const olderHelper = (): string => { - const helper = join(OUTSIDE, 'older-helper') - writeFileSync(helper, '#!/bin/sh\nexec "$@"\n') - chmodSync(helper, 0o755) - return helper + it('a link that leads to nothing yet, for a read deny on it', async () => { + expect( + await heldBy([], { readConfig: { denyOnly: inRoot(['dangling']) } }), + ).toEqual(pairs('dangling')) + }) + + it('a read deny that is where it resolves to is not walked', async () => { + const spy = spyOn(fs, 'lstatSync') + try { + await wrapLinux([], { readConfig: { denyOnly: inRoot(['conf/x']) } }) + expect( + spy.mock.calls.filter(([p]) => String(p).startsWith(ROOT)), + ).toEqual([]) + } finally { + spy.mockRestore() } + }) + + it('a refusal for length says how much of it names links', async () => { + const wrap = (denied: string[]): Promise => + wrapLinux(denied, { command: 'a'.repeat(2 ** 21) }).then( + () => 'wrapped', + (error: Error) => error.message, + ) + const words = `--hold-link ${ROOT}/name conf/x` + expect(await wrap(inRoot(['name']))).toContain( + `At least ${words.length} of them are not the caller's`, + ) + expect(await wrap([])).toMatch(/the limit here is \d+\)$/) + }) +}) - it.each<[string, () => Parameters[1]]>([ - ['no helper in use', () => ({ allowAllUnixSockets: true })], - ['an older one', () => ({ seccompConfig: { applyPath: olderHelper() } })], - ])('%s: they are named, and the command starts', async (_what, rest) => { - const links = inRoot(['first', 'middle']) - expect(getLinuxUnheldLinks()).not.toContain(links[0]) +describe.if(isLinux)('where no link is held', () => { + beforeEach(forgetLinuxUnheldLinks) + /** A helper that notes each time it is run, then does as `script` says. */ + const noting = (script: string): string => + program('noting-helper', `echo run >> ${OUTSIDE}/runs\n${script}`) + const runs = (): number => + existsSync(join(OUTSIDE, 'runs')) + ? readFileSync(join(OUTSIDE, 'runs'), 'utf8').split('\n').length - 1 + : 0 + /** As a helper built before the option does. */ + const RUNS_ITS_WORDS = 'exec "$@"' + /** `helper` for the one this library finds for itself. */ + const found = (helper: string): { mockRestore(): void } => + spyOn(seccomp, 'getApplySeccompBinaryPath').mockReturnValue(helper) + const unheld = ( + reason: LinuxUnheldLinkReason, + ): ReturnType => + inRoot(['first', 'middle']).map(path => ({ path, reason })) + + it.each<[LinuxUnheldLinkReason, string, () => Partial]>([ + ['no_helper', 'no helper runs', () => ({ allowAllUnixSockets: true })], + [ + 'told_no', + 'the caller says its helper does not hold links', + () => ({ + seccompConfig: { applyPath: noting('exit 0'), holdsLinks: false }, + }), + ], + [ + 'told_no', + 'the caller turns holding off', + () => ({ seccompConfig: { holdsLinks: false } }), + ], + [ + 'not_told', + "the helper is the caller's, which says nothing", + () => ({ seccompConfig: { applyPath: noting('exit 0') } }), + ], + [ + 'not_told', + 'the same of one that is run under another name', + () => ({ + seccompConfig: { applyPath: noting('exit 0'), argv0: 'apply-seccomp' }, + }), + ], + ])( + '%s, %s: they are named, nothing is run to find out, and the command starts', + async (reason, _what, rest) => { const { result: wrapped, warnings } = await withCapturedWarnings(() => - wrap(inRoot(['first']), rest()), + wrapLinux(inRoot(['first']), rest()), ) - expect(held(wrapped)).toEqual([]) - expect(getLinuxUnheldLinks()).toEqual(expect.arrayContaining(links)) - for (const link of links) { - expect(warnings.some(line => line.includes(`${link},`))).toBe(true) - } - if (bwrapCanNamespace()) { + expect(wrapped).not.toContain('--hold-link') + expect(getLinuxUnheldLinks()).toEqual(unheld(reason)) + for (const { path } of unheld(reason)) { expect( - spawnSync(wrapped, { shell: true, encoding: 'utf8', cwd: OUTSIDE }) - .stdout, - ).toBe('started\n') + warnings.some(l => l.includes(`${path},`) && l.includes(reason)), + ).toBe(true) + } + expect(runs()).toBe(0) + if (bwrapCanNamespace() && !wrapped.includes('noting-helper')) { + expect(stdoutOf(wrapped)).toBe('started\n') } + }, + ) + + it("the caller's word that its helper holds links is taken, and nothing is run", async () => { + const wrapped = await wrapLinux(inRoot(['first']), { + seccompConfig: { + applyPath: '/proc/self/fd/3', + argv0: 'apply-seccomp', + holdsLinks: true, + }, }) + expect(wrapped).toContain( + `ARGV0=apply-seccomp /proc/self/fd/3 --hold-link ${ROOT}/first middle `, + ) + expect(getLinuxUnheldLinks()).toEqual([]) + }) - it('a helper run under another name is asked under it, and again after no answer', async () => { - const multicall = join(OUTSIDE, 'multicall') - const seccompConfig = { applyPath: multicall, argv0: 'apply-seccomp' } - expect(held(await wrap(inRoot(['name']), { seccompConfig }))).toEqual([]) + it('a helper the library found for itself is asked once', async () => { + const spy = found(noting(RUNS_ITS_WORDS)) + try { + for (const _ of [1, 2]) { + expect(await wrapLinux(inRoot(['first']))).not.toContain('--hold-link') + expect(getLinuxUnheldLinks()).toEqual(unheld('older_helper')) + } + } finally { + spy.mockRestore() + } + expect(runs()).toBe(1) + }) - writeFileSync( - multicall, - `#!/bin/sh\n[ "$ARGV0" = apply-seccomp ] && exec ${getApplySeccompBinaryPath()} "$@"\n`, - ) - chmodSync(multicall, 0o755) - expect(held(await wrap(inRoot(['name']), { seccompConfig }))).toEqual( - inRoot(['name']), - ) - }) + it('it is not asked where there is nothing to hold', async () => { + const spy = found(noting(RUNS_ITS_WORDS)) + try { + await wrapLinux(inRoot(['beside'])) + } finally { + spy.mockRestore() + } + expect(runs()).toBe(0) + }) - it('asking an older helper runs no program of that name', async () => { - const program = join(ROOT, '--holds-links') - writeFileSync(program, `#!/bin/sh\necho ran > ${ROOT}/ran\n`) - chmodSync(program, 0o755) - const [cwd, PATH] = [process.cwd(), process.env.PATH] - process.chdir(ROOT) - process.env.PATH = `${ROOT}:.:${PATH}` + it.each([ + ['ends by a signal', 'kill -9 $$', 0o755, 2], + ['cannot be started', '', 0o644, 0], + ])( + 'one that %s is asked twice, and again at the next wrap', + async (_what, script, mode, perWrap) => { + const helper = noting(script) + chmodSync(helper, mode) + const spy = found(helper) try { - await wrap(inRoot(['name']), { - seccompConfig: { applyPath: olderHelper() }, - }) + for (const wraps of [1, 2]) { + const { warnings } = await withCapturedWarnings(() => + wrapLinux(inRoot(['first'])), + ) + expect(getLinuxUnheldLinks()).toEqual(unheld('no_answer')) + expect( + warnings.filter(l => l.includes('gave no answer')), + ).toHaveLength(2) + expect(runs()).toBe(perWrap * wraps) + } } finally { - process.chdir(cwd) - process.env.PATH = PATH + spy.mockRestore() } - expect(existsSync(join(ROOT, 'ran'))).toBe(false) + }, + ) + + it('asking an older helper runs no program of that name', async () => { + const named = join(ROOT, '--holds-links') + writeFileSync(named, `#!/bin/sh\necho ran > ${ROOT}/ran\n`) + chmodSync(named, 0o755) + const PATH = process.env.PATH + process.env.PATH = `${ROOT}:.:${PATH}` + const spy = found(noting(RUNS_ITS_WORDS)) + try { + await fromRoot(() => wrapLinux(inRoot(['name']))) + } finally { + spy.mockRestore() + process.env.PATH = PATH + } + expect(runs()).toBe(1) + expect(existsSync(join(ROOT, 'ran'))).toBe(false) + }) + + it('a link is listed by the latest wrap that met it', async () => { + await wrapLinux(inRoot(['first']), { allowAllUnixSockets: true }) + expect(await wrapLinux(inRoot(['middle']))).toContain('--hold-link') + expect(getLinuxUnheldLinks()).toEqual(unheld('no_helper').slice(0, 1)) + }) + + it('a wrap that throws names none', async () => { + const wrap = wrapLinux(inRoot(['first']), { + allowAllUnixSockets: true, + binShell: 'no-such-shell', + }) + expect(wrap).rejects.toThrow('no-such-shell') + await wrap.catch(() => {}) + expect(getLinuxUnheldLinks()).toEqual([]) + }) + + describe('as the manager lists them', () => { + const config = (allowAllUnixSockets: boolean): SandboxRuntimeConfig => ({ + network: { allowedDomains: [], deniedDomains: [], allowAllUnixSockets }, + filesystem: { + denyRead: [], + allowWrite: [ROOT], + denyWrite: inRoot(['first']), + }, + }) + afterEach(() => SandboxManager.reset()) + + it('takes the word on the helper from the configuration', async () => { + const { seccomp: parsed } = SandboxRuntimeConfigSchema.parse({ + ...config(false), + seccomp: { holdsLinks: false }, + }) + expect(parsed).toEqual({ holdsLinks: false }) + await SandboxManager.initialize({ ...config(false), seccomp: parsed }) + await SandboxManager.wrapWithSandbox('true') + expect(SandboxManager.getLinuxUnheldLinks()).toEqual(unheld('told_no')) + }) + + it('lists them', async () => { + await SandboxManager.initialize(config(true)) + await SandboxManager.wrapWithSandbox('true') + expect(SandboxManager.getLinuxUnheldLinks()).toEqual(unheld('no_helper')) + }) + + it.each<[string, () => void | Promise]>([ + ['reset()', () => SandboxManager.reset()], + ['updateConfig()', () => SandboxManager.updateConfig(config(true))], + ])('%s empties the list', async (_what, act) => { + await SandboxManager.initialize(config(true)) + await SandboxManager.wrapWithSandbox('true') + expect(SandboxManager.getLinuxUnheldLinks()).not.toEqual([]) + await act() + expect(SandboxManager.getLinuxUnheldLinks()).toEqual([]) }) }) }) describe.if(isLinux && bwrapCanNamespace())('the seccomp helper', () => { - const helper = (): string => getApplySeccompBinaryPath()! + const helper = (): string => seccomp.getApplySeccompBinaryPath()! const run = ( - program: string, + runnable: string, words: string[], ): { status: number | null; said: string } => { - const result = spawnSync(program, words, { + const result = spawnSync(runnable, words, { encoding: 'utf8', timeout: 30000, cwd: ROOT, }) return { status: result.status, said: result.stdout + result.stderr } } + const holding = (...names: Array): string[] => + names.flatMap(name => ['--hold-link', join(ROOT, name), LINKS[name]]) /** umount2() with UMOUNT_NOFOLLOW, which reaches a mount on a link. */ const UNMOUNT = [ 'python3', @@ -509,51 +967,63 @@ describe.if(isLinux && bwrapCanNamespace())('the seccomp helper', () => { }) it.each([ - ['a file', 'beside', 'not a symbolic link'], - ['nothing', 'absent', 'No such file or directory'], - ])('runs no command where %s is at the name', (_what, name, why) => { - const { status, said } = run(helper(), [ - '--hold-link', - join(ROOT, name), - 'echo', - 'started', - ]) - expect(status).toBe(1) - expect(said).toContain(why) - expect(said).not.toContain('started') + ['a file is at the name', 'beside', '', 'not a symbolic link'], + ['nothing is at the name', 'absent', '', 'No such file or directory'], + ['the link says more', 'name', 'conf', 'leads elsewhere than it did'], + ['the link says less', 'name', 'conf/xy', 'leads elsewhere than it did'], + [ + 'the link says another thing', + 'name', + 'conf/y', + 'leads elsewhere than it did', + ], + ])('runs no command where %s, and says why', (_what, name, target, why) => { + expect( + run(helper(), ['--hold-link', join(ROOT, name), target, 'echo', 'ran']), + ).toEqual({ + status: 1, + said: `apply-seccomp: --hold-link ${join(ROOT, name)}: ${why}\n`, + }) + }) + + it('holds a link it is given no word on', () => { + expect( + run(helper(), ['--hold-link', join(ROOT, 'name'), '', 'rm', 'name']).said, + ).toContain('Device or resource busy') }) it.each([ ['this user', [], 'EPERM'], ['uid 0, with every capability in its namespace', ['-Ur'], 'EINVAL'], - ])('a command of %s cannot take the hold off', (_who, as, errno) => { - const link = join(ROOT, 'name') + ])('a command of %s cannot unmount the hold', (_who, as, errno) => { const { said } = run('unshare', [ ...as, helper(), - '--hold-link', - link, + ...holding('name'), ...UNMOUNT, - link, + join(ROOT, 'name'), ]) expect(said.trim()).toBe(errno) }) - it('holds a link on a file system mounted with flags of its own', () => { + it('a hold keeps the flags of the file system that holds the link', () => { const link = join(OUTSIDE, 'link') const { said } = run('unshare', [ ...['-Urm', 'sh', '-c'], - `mount -t tmpfs -o noexec,noatime none ${OUTSIDE} && ln -s x ${link} && ` + - `exec ${helper()} --hold-link ${link} rm ${link}`, + `mount -t tmpfs -o noexec,noatime,nosymfollow none ${OUTSIDE} && ` + + `echo read > ${OUTSIDE}/x && ln -s x ${link} && ` + + `exec ${helper()} --hold-link ${link} x sh -c 'rm ${link}; cat ${link}'`, ]) expect(said).toContain('Device or resource busy') + expect(said).toContain('Too many levels of symbolic links') + expect(said).not.toContain('read') }) it.each(['--ro-bind', '--bind', '--dev-bind'])( 'a bubblewrap the command starts with %s / / starts, and the hold holds there', bind => { const { said } = run(helper(), [ - ...inRoot(['name', 'way', 'dangling']).flatMap(l => ['--hold-link', l]), + ...holding('name', 'way', 'dangling'), ...['bwrap', bind, '/', '/', '--bind', ROOT, ROOT, '--dev', '/dev'], ...['sh', '-c', 'echo started; rm name way dangling'], ]) @@ -561,4 +1031,80 @@ describe.if(isLinux && bwrapCanNamespace())('the seccomp helper', () => { expect(said.match(/Device or resource busy/g)).toHaveLength(3) }, ) + + it('a link that says another thing by the time the command runs: nothing runs', async () => { + const wrapped = await wrapLinux(inRoot(['name'])) + rmSync(join(ROOT, 'name')) + symlinkSync('conf/z', join(ROOT, 'name')) + const result = spawnSync(wrapped, { shell: true, encoding: 'utf8' }) + expect(result.stdout).toBe('') + expect(result.stderr).toContain( + `apply-seccomp: --hold-link ${ROOT}/name: leads elsewhere than it did`, + ) + }) }) + +describe.if(isLinux && bwrapCanNamespace())( + 'A read deny on a name that is a link, over two commands', + () => { + const SECRET = 'secret-content' + afterEach(() => SandboxManager.reset()) + + it.each< + [string, string, (name: string) => Partial, string] + >([ + ['denyRead', 'file', name => ({ filesystem: lists([name]) }), ''], + ['denyRead', 'dir', name => ({ filesystem: lists([name]) }), '/file'], + [ + 'a denyRead pattern', + 'file', + name => ({ filesystem: lists([`${name}*`]) }), + '', + ], + [ + 'a credential deny', + 'file', + path => ({ credentials: { files: [{ path, mode: 'deny' }] } }), + '', + ], + [ + 'a credential mask', + 'file', + path => ({ credentials: { files: [{ path, mode: 'mask' }] } }), + '', + ], + ])( + '%s on a link to a %s: the name stays, and what it leads to stays hidden', + async (_kind, leadsTo, listing, inside) => { + mkdirSync(join(OUTSIDE, 'dir')) + writeFileSync(join(OUTSIDE, 'file'), SECRET) + writeFileSync(join(OUTSIDE, 'dir', 'file'), SECRET) + const name = join(ROOT, 'linked') + symlinkSync(join(OUTSIDE, leadsTo), name) + await SandboxManager.initialize({ + network: { allowedDomains: [], deniedDomains: [] }, + filesystem: lists([]), + ...listing(name), + }) + const saidBy = async (command: string): Promise => { + const said = stdoutOf( + await SandboxManager.wrapWithSandbox(`echo ran; ${command}`), + ) + SandboxManager.cleanupAfterCommand() + expect(said).toStartWith('ran\n') + return said + } + + await saidBy(`rm ${name}; mv ${name} ${name}-aside; echo x > ${name}`) + expect(readlinkSync(name)).toBe(join(OUTSIDE, leadsTo)) + expect( + await saidBy(`cat ${join(OUTSIDE, leadsTo)}${inside}`), + ).not.toContain(SECRET) + }, + ) + + function lists(denyRead: string[]): SandboxRuntimeConfig['filesystem'] { + return { denyRead, allowWrite: [ROOT], denyWrite: [] } + } + }, +) diff --git a/vendor/seccomp-src/apply-seccomp.c b/vendor/seccomp-src/apply-seccomp.c index a9af1bdfb..7b201cb9a 100644 --- a/vendor/seccomp-src/apply-seccomp.c +++ b/vendor/seccomp-src/apply-seccomp.c @@ -1,7 +1,7 @@ /* * apply-seccomp.c - Apply seccomp BPF filter in an isolated PID namespace * - * Usage: apply-seccomp [--hold-link ]... [args...] + * Usage: apply-seccomp [--hold-link ]... [args...] * apply-seccomp --holds-links * * This program applies a baked-in seccomp BPF filter, isolates the @@ -78,6 +78,13 @@ #define SECCOMP_MODE_FILTER 2 #endif +#ifndef MS_NOSYMFOLLOW +#define MS_NOSYMFOLLOW 256 +#endif +#ifndef ST_NOSYMFOLLOW +#define ST_NOSYMFOLLOW 0x2000 +#endif + #ifndef SECCOMP_FILTER_FLAG_NEW_LISTENER #define SECCOMP_FILTER_FLAG_NEW_LISTENER (1UL << 3) #endif @@ -713,17 +720,21 @@ static void die_holding(const char *path, const char *why) { } /* - * Mount the symbolic link at `path` on itself, read-only. A name that is a - * mount point cannot be unlinked, renamed or renamed over (EBUSY), and the - * directory that holds it stays as writable as it was. The link reads and - * leads as before. + * Mount the symbolic link at `path` on itself, read-only. unlink and rename + * refuse a name that is a mount point in the caller's mount namespace (EBUSY), + * and the directory that holds it stays as writable as it was. The link reads + * and leads as before. + * + * `target` is what the link said when the denies were placed by it, or "" for + * no word on that. One that says something else by now leads past them. * * mount(2) follows a link at its target, so the link is named through a * descriptor of itself: the kernel does not follow what /proc/self/fd/N leads - * to any further. + * to any further. What is looked at and what is mounted are one descriptor. */ -static void hold_link(const char *path) { +static void hold_link(const char *path, const char *target) { char self[32]; + char says[PATH_MAX]; struct stat st; struct statvfs fs; @@ -734,6 +745,14 @@ static void hold_link(const char *path) { if (!S_ISLNK(st.st_mode)) { die_holding(path, "not a symbolic link"); } + ssize_t length = readlinkat(fd, "", says, sizeof(says)); + if (length < 0) { + die_holding(path, strerror(errno)); + } + if (*target && ((size_t)length != strlen(target) || + memcmp(says, target, (size_t)length) != 0)) { + die_holding(path, "leads elsewhere than it did"); + } snprintf(self, sizeof(self), "/proc/self/fd/%d", fd); if (mount(self, self, NULL, MS_BIND, NULL) < 0) { die_holding(path, strerror(errno)); @@ -743,15 +762,17 @@ static void hold_link(const char *path) { /* Read-only, nosuid and nodev are all the flags bubblewrap ever adds. One * that the command starts applies its flags to every mount it finds, by * path, which for this mount leads elsewhere; it leaves a mount that has - * them already alone. noexec is locked where the mount came with it, and - * a remount keeps the access-time flags by itself. Opened anew: the first - * descriptor is of what now lies beneath the mount. */ + * them already alone. The flags the mount came with are given again: + * noexec is locked, and without nosymfollow the link would lead where it + * did not. A remount keeps the access-time flags by itself. Opened anew: + * the first descriptor is of what now lies beneath the mount. */ fd = open(path, O_PATH | O_NOFOLLOW | O_CLOEXEC); if (fd < 0 || fstatvfs(fd, &fs) < 0) { die_holding(path, strerror(errno)); } unsigned long flags = MS_RDONLY | MS_NOSUID | MS_NODEV; if (fs.f_flag & ST_NOEXEC) flags |= MS_NOEXEC; + if (fs.f_flag & ST_NOSYMFOLLOW) flags |= MS_NOSYMFOLLOW; snprintf(self, sizeof(self), "/proc/self/fd/%d", fd); if (mount(NULL, self, NULL, MS_REMOUNT | MS_BIND | flags, NULL) < 0) { die_holding(path, strerror(errno)); @@ -764,16 +785,17 @@ int main(int argc, char *argv[]) { return 0; } int links = 0; - while (2 * links + 2 < argc && strcmp(argv[2 * links + 1], "--hold-link") == 0) { + while (3 * links + 3 < argc && strcmp(argv[3 * links + 1], "--hold-link") == 0) { links++; } - if (argc < 2 * links + 2) { - fprintf(stderr, "Usage: %s [--hold-link ]... [args...]\n", + if (argc < 3 * links + 2) { + fprintf(stderr, + "Usage: %s [--hold-link ]... [args...]\n", argv[0]); return 1; } - char **command_argv = &argv[2 * links + 1]; + char **command_argv = &argv[3 * links + 1]; _Static_assert(sizeof(unix_block_bpf) % sizeof(struct sock_filter) == 0, "BPF filter size must be a multiple of sock_filter"); @@ -828,8 +850,9 @@ int main(int argc, char *argv[]) { if (links > 0) { /* INVARIANT: a user namespace is entered after the last hold. The * mount namespace made below is then a copy made across that - * boundary, in which every mount is locked: a command started by - * uid 0 holds all capabilities in its user namespace, and could + * boundary, in which every mount is locked: it cannot be + * unmounted, which is all a lock refuses. A command started by uid + * 0 holds all capabilities in its user namespace, and could * unmount what was mounted in a mount namespace of that one. */ if (unshare(CLONE_NEWNS) < 0) { die("apply-seccomp: unshare(CLONE_NEWNS)"); @@ -838,7 +861,7 @@ int main(int argc, char *argv[]) { die("apply-seccomp: mount(MS_PRIVATE)"); } for (int i = 0; i < links; i++) { - hold_link(argv[2 * i + 2]); + hold_link(argv[3 * i + 2], argv[3 * i + 3]); } enter_user_namespace(); } From 154c32a72a57c1470e63c5642728277d1bedf228 Mon Sep 17 00:00:00 2001 From: Ron Leizrowice Date: Sat, 10 Oct 2026 23:40:55 +0000 Subject: [PATCH 3/8] test: macOS follows fewer links than Linux, and one row had nothing to be compared with The walk follows 40 links on every platform. macOS gives up sooner, so the comparison with the kernel at 40 is made on Linux. The row on /dev/stdout said what a command may do there with no run without the link beside it: it goes. The rows on /dev/null stay. --- test/sandbox/write-deny-through-links.test.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/test/sandbox/write-deny-through-links.test.ts b/test/sandbox/write-deny-through-links.test.ts index a154445e9..3e5e46291 100644 --- a/test/sandbox/write-deny-through-links.test.ts +++ b/test/sandbox/write-deny-through-links.test.ts @@ -189,14 +189,17 @@ describe.if(!isWindows)('the links on the way to a path', () => { }) }) - it('follows as many links as the kernel, and has no end past them', () => { + it('follows as many links as the Linux kernel, and has no end past them', () => { symlinkSync('beside', join(ROOT, 'hop-1')) for (let i = 2; i <= 41; i++) { symlinkSync(`hop-${i - 1}`, join(ROOT, `hop-${i}`)) } expect(followLinks(join(ROOT, 'hop-40')).end).toBe(join(ROOT, 'beside')) expect(followLinks(join(ROOT, 'hop-41')).end).toBeUndefined() - expect(() => readFileSync(join(ROOT, 'hop-40'))).not.toThrow() + // macOS gives up sooner, at 32: what is denied past that is denied in vain. + if (isLinux) { + expect(() => readFileSync(join(ROOT, 'hop-40'))).not.toThrow() + } expect(() => readFileSync(join(ROOT, 'hop-41'))).toThrow('ELOOP') }) @@ -353,8 +356,6 @@ describe.if((isLinux && bwrapCanNamespace()) || isMacOS)( ], ['a write to a device a denied name leads to', 'echo x > /dev/null'], ['a read of it', 'cat /dev/null'], - // Linux opens what is behind it anew, which a socket refuses. - ...(isMacOS ? [['the same of the output', 'echo x > /dev/stdout']] : []), ])('allows %s: %s', async (_what, attempt) => { expect(await outcomeOf(attempt)).toBe('DONE') }) From 438380fb6360e49479a93fcaa5c558536928d5ae Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 20 Oct 2026 06:25:52 +0000 Subject: [PATCH 4/8] test: read EBUSY as either C library words it The container suite is to run on musl as well, where rm says "Resource busy". Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01FydxqUESqxwSCVFnhk3gfv Co-authored-by: ronleizrowice --- test/docker-weak-sandbox.test.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/docker-weak-sandbox.test.ts b/test/docker-weak-sandbox.test.ts index 030b4e5b8..696b92714 100644 --- a/test/docker-weak-sandbox.test.ts +++ b/test/docker-weak-sandbox.test.ts @@ -195,7 +195,8 @@ describe.if(inDocker)('srt end-to-end as uid 0 in a container', () => { expect(r.stdout).toContain('SANDBOX-RAN') expect(r.stdout).toContain(`umount2 ${LINK} rc=-1 errno=${errno}`) - expect(r.stdout).toContain('Device or resource busy') + // EBUSY, as glibc words it ("Device or resource busy") or musl. + expect(r.stdout).toMatch(/resource busy/i) expect(readlinkSync(LINK)).toBe('leads-to') }, ) From dcc0f333612bfef1290737779e0df30fbd568092 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 20 Oct 2026 12:09:55 +0000 Subject: [PATCH 5/8] What a review of the whole diff found in the links on the way to a deny - The helper the library finds for itself is read for the name of --holds-links, never run: that was outside any sandbox, and in a checkout or a project's own node_modules the file is whatever an earlier command left. - One walker. followLinks() is wayTo() of real-path.ts with a watcher, so a target that holds U+FFFD itself is text, and a name too long to be there is a name that is not there and no longer makes its directory read-only. - A look that fails is tried again before a directory is denied whole. - A directory that a link's target goes into and out of again by `..` is kept in place, on both platforms; on macOS a name that is not there which the target goes up from can no longer be made. - Where nothing holds the links, the directories above them are not pinned. - getFsReadConfig() names the links its patterns match, as a wrap does. - The refusals for length and of a hold name seccomp.holdsLinks. - Tests: a rejection is awaited, EBUSY is read in musl's wording too, and the attempts are no longer printed. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01FydxqUESqxwSCVFnhk3gfv Co-authored-by: ronleizrowice --- README.md | 10 +- src/sandbox/linux-sandbox-utils.ts | 130 +++---- src/sandbox/macos-sandbox-utils.ts | 23 +- src/sandbox/sandbox-config.ts | 7 +- src/sandbox/sandbox-manager.ts | 4 +- src/sandbox/sandbox-utils.ts | 133 ++++--- src/utils/real-path.ts | 56 ++- test/sandbox/write-deny-through-links.test.ts | 340 +++++++++++------- vendor/seccomp-src/apply-seccomp.c | 5 +- 9 files changed, 410 insertions(+), 298 deletions(-) diff --git a/README.md b/README.md index 771a03572..357d0ee0c 100644 --- a/README.md +++ b/README.md @@ -595,7 +595,7 @@ Examples: - `allowPty` - macOS only: allow pseudo-terminal operations (boolean, default: false). The seatbelt profile then carries `(allow pseudo-tty)` plus read, write and `ioctl` on `/dev/ptmx` and `/dev/ttys*`, which a command that allocates a pty of its own needs. Not read on Linux or Windows. - `bwrapPath` - Linux only: absolute path to the `bwrap` (bubblewrap) binary, used instead of resolving `bwrap` on `PATH`. A path that is not executable is a dependency error, and no `PATH` lookup is tried after it. - `socatPath` - Linux only: absolute path to the `socat` binary, used instead of resolving `socat` on `PATH`, with the same rule for a path that is not executable. -- `seccomp` - Linux only: `applyPath` points at the `apply-seccomp` binary to use instead of the packaged one, which is still used when nothing exists at that path. `argv0` invokes it as a multicall binary that dispatches on the `ARGV0` environment variable; `applyPath` is then used verbatim (no existence check) and must resolve inside the bubblewrap namespace. `holdsLinks` says whether the helper takes `--hold-link` (see "Denied paths and symbolic links") and is taken at its word: nothing is run to find out. Not set, a helper the library found for itself is asked (`apply-seccomp --holds-links` exits 0), and one named by `applyPath`, which need not name the helper outside the sandbox, is never run there and is handed no link. Set it from such a probe of your own helper, not from a constant: a helper without the option takes `--hold-link` for the command, so under a wrong `true` no sandboxed command starts in a project with such a link. +- `seccomp` - Linux only: `applyPath` points at the `apply-seccomp` binary to use instead of the packaged one, which is still used when nothing exists at that path. `argv0` invokes it as a multicall binary that dispatches on the `ARGV0` environment variable; `applyPath` is then used verbatim (no existence check) and must resolve inside the bubblewrap namespace. `holdsLinks` says whether the helper takes `--hold-link` (see "Denied paths and symbolic links") and is taken at its word: nothing is run to find out. Not set, the file of a helper the library found for itself is read for the name of the option `--holds-links`, and one named by `applyPath`, which need not name the helper outside the sandbox, is handed no link. Set it from a probe of your own helper (`apply-seccomp --holds-links` exits 0), not from a constant: a helper without the option takes `--hold-link` for the command, so under a wrong `true` no sandboxed command starts in a project with such a link. - `ripgrep` - Linux only: how to invoke ripgrep for the deny-path scan (default: `{ "command": "rg" }`). `args` are passed before srt's own arguments, and `argv0` overrides `argv[0]` for a multicall binary. - `git.safeDirectories` - Directories git should treat as `safe.directory` inside the sandbox, where the working tree is owned by another user — the repository top level when the command runs from a subdirectory, say. Injected through `GIT_CONFIG_*` environment variables, and it grants no write access of its own. - `credentials` - Environment variables (`credentials.envVars`) and files (`credentials.files`) the sandboxed command must not read the real values of. `mode: "deny"` withholds the value; `mode: "mask"` puts a sentinel in its place inside the sandbox and has the proxy substitute the real value back on egress to the hosts that entry's `injectHosts` lists (default: `network.allowedDomains`). Masking requires `network.tlsTerminate`, so the real value only leaves over a verified TLS connection, unless `credentials.allowPlaintextInject` opts out. Files are masked on Linux only; macOS denies a masked file instead. @@ -1118,17 +1118,19 @@ Filesystem restrictions are enforced at the OS level: **Writes through links (Linux and macOS):** a write is judged by where it lands, after symbolic links are followed. A link inside an allowed write path that leads out of it gives a command nothing: the write is refused. A link that leads into an allowed write path works like any other name for that place. A hard link is a second name for the file itself: on Linux a file outside the allowed write paths that already has a hard link inside one can be written through it, and a sandboxed command cannot create such a link. On macOS nothing is promised for such a file. -**Denied paths and symbolic links (Linux and macOS):** a `denyWrite` entry or a [mandatory deny path](#mandatory-deny-paths-auto-protected-files) with a symbolic link on the way to it, its last name included, denies the path the links lead to, whether or not anything is there yet, and keeps each of those links where it is: removing it, renaming it and putting something else at its name are refused, and so is renaming the directories above it, as above any denied path. The link reads and leads as before, and everything else in the directory that holds it stays as writable as it was. The links are those found when the command is wrapped. +**Denied paths and symbolic links (Linux and macOS):** a `denyWrite` entry or a [mandatory deny path](#mandatory-deny-paths-auto-protected-files) with a symbolic link on the way to it, its last name included, denies the path the links lead to, whether or not anything is there yet, and keeps each of those links where it is: removing it, renaming it and putting something else at its name are refused, and so is renaming the directories above it, as above any denied path. Where a link's target goes into a directory and out of it again by `..`, that directory is kept in place too, and a name that is not there cannot be made where the target goes up from it. The link reads and leads as before, and everything else in the directory that holds it stays as writable as it was. The links are those found when the command is wrapped. - Nothing is denied among the devices, for reading or writing: a name blanked with a link to `/dev/null` is kept, and `/dev/null` stays as it was. - A tool that replaces a file instead of writing into it is refused at such a name, as at any denied one: `sed -i`, `perl -i`, `mv -f`, a write to a temporary file that is then renamed, `install`, `ln -sfn`, `rsync`, `tar x`, `unzip -o`, `git checkout` where the tracked link differs, `git clean -fdx`. On Linux the error is `EBUSY` ("Device or resource busy"). -- A directory on the way that cannot be looked in (its search permission is gone, or the look fails for any reason but the name not being there) is denied whole in place of the path, its mode included: what is in it is not known. Never `/`: where that is the directory, the path is taken as written. +- A directory on the way that cannot be looked in (its search permission is gone, or the look fails twice, for any reason but that nothing is or can be at the name) is denied whole in place of the path, its mode included: what is in it is not known. Never `/`: where that is the directory, the path is taken as written. - On Linux the same links are kept on the way to a `denyRead` entry, to a link that a `denyRead` pattern matches, and to a `credentials.files` path, denied or masked. Each command's mounts are placed by the name, so a name that led elsewhere by the next command would leave what it led to unhidden. - On Linux bubblewrap cannot mount on a link, so the seccomp helper does (`apply-seccomp --hold-link `): each link that lies in a directory the command can write to is mounted on itself, read-only. The hold is a mount like every deny of this sandbox and holds what those hold: it is in force in the mount namespaces made for the command. - No command is started where a hold cannot be made: the name is no link any more, the link says something else than when the command was wrapped, or the mount is refused (a security module, a file system that does not take one). The helper exits 1 and its one line on stderr starts with `apply-seccomp: --hold-link : `. `seccomp.holdsLinks: false` turns holding off. -- Not held on Linux: `SandboxManager.getLinuxUnheldLinks()` returns each link that should be held and is not, as `{ path, reason }`, for the embedder to show or to look at again after the command; the only other sign is a line under `SRT_DEBUG`. What such a link leads to is denied all the same, and the link is an ordinary entry of a writable directory. A link is listed by the latest wrap that met it, so one that a later wrap holds is listed no longer, and `reset()` and `updateConfig()` empty the list. The reasons: `no_helper` (`allowAllUnixSockets`, or no binary for the architecture), `told_no` (`seccomp.holdsLinks` is false), `not_told` (the helper is named by `seccomp.applyPath` and `seccomp.holdsLinks` is not set), `older_helper` (the helper the library found for itself is built from older sources), `no_answer` (that helper could not be asked, twice; it is asked again at the next wrap). +- Not held on Linux: `SandboxManager.getLinuxUnheldLinks()` returns each link that should be held and is not, as `{ path, reason }`, for the embedder to show or to look at again after the command; the only other sign is a line under `SRT_DEBUG`. What such a link leads to is denied all the same, and the link is an ordinary entry of a writable directory. A link is listed by the latest wrap that met it, so one that a later wrap holds is listed no longer, and `reset()` and `updateConfig()` empty the list. The reasons: `no_helper` (`allowAllUnixSockets`, or no binary for the architecture), `told_no` (`seccomp.holdsLinks` is false), `not_told` (the helper is named by `seccomp.applyPath` and `seccomp.holdsLinks` is not set), `older_helper` (the helper the library found for itself is built from older sources), `no_answer` (that helper's file could not be read). A wrap still in flight when `reset()` or `updateConfig()` runs lists its links afterwards. - Not covered: a mandatory deny name _below_ the working directory that is itself a link. On Linux the scan lists files and does not see it. On macOS the pattern keeps the name in place and says nothing of what it leads to. - Not covered on Linux: a link on the way to what a `denyRead` pattern matches, other than the match itself. A `denyWrite` pattern is skipped there as a whole. +- Not covered on Linux: a writer outside the sandbox that changes a link back and forth while the command is being wrapped. The deny's mount and the word the helper checks the link against come from two looks. +- Not reported on Linux: the violation monitor judges by the configured names, so a refused attempt on a link on the way to one, or on a directory kept for it, is no violation in its store. - The links' names stay on the command line (see the note on large profiles below), about as many bytes each as the path and the target together: some thousand of them, with the command, reach the limit of one argument, and the `command_too_long` refusal says how many bytes are theirs. **Read-side rules (Linux):** an entry is matched by the name it is, not by what it points at. diff --git a/src/sandbox/linux-sandbox-utils.ts b/src/sandbox/linux-sandbox-utils.ts index e2842c133..c084eba65 100644 --- a/src/sandbox/linux-sandbox-utils.ts +++ b/src/sandbox/linux-sandbox-utils.ts @@ -1,6 +1,6 @@ import { quote } from '../utils/shell-quote.js' import { logForDebugging } from '../utils/debug.js' -import { realPathOf, wayTo } from '../utils/real-path.js' +import { realPathOf, wayPastMissingNames } from '../utils/real-path.js' import { whichSync } from '../utils/which.js' import { randomBytes } from 'node:crypto' import * as fs from 'fs' @@ -151,9 +151,6 @@ function findSymlinkInPath( return null } -/** As many missing names as one way is followed up from again. */ -const MAX_MISSING_NAMES_PASSED = 40 - /** * Canonicalize a deny path through symlinks before any mask or bind is * computed (resolve-before-mask). Deny paths regularly contain symlinked @@ -226,21 +223,16 @@ function resolveSymlinkedDenyPath( } catch { return rejoined // Not a symlink — nothing left to resolve. } - let why: unknown = 'it goes up from too many names that are not there' + let why: unknown try { - let onward = rejoined - for (let i = 0; i < MAX_MISSING_NAMES_PASSED; i++) { - const { real, rest, notThere } = wayTo(onward) - if ((notThere as NodeJS.ErrnoException | undefined)?.code === 'EACCES') { - return real || '/' - } - const names = rest.filter(name => name !== '' && name !== '.') - if (names[1] !== '..') { - return [real, ...names.slice(0, 2)].join('/') || '/' - } - alsoDenied?.push(`${real}/${names[0]}`) - onward = [real, ...names.slice(2)].join('/') - } + const way = wayPastMissingNames(rejoined) + alsoDenied?.push(...way.passed) + const shut = (way.notThere as NodeJS.ErrnoException | undefined)?.code + return ( + [way.real, ...(shut === 'EACCES' ? [] : way.rest.slice(0, 2))].join( + '/', + ) || '/' + ) } catch (err) { why = err } @@ -1109,7 +1101,7 @@ function renderBwrapInvocation( 'command_too_long', `Sandboxed command is too long for one shell argument even with the mounts passed through a file (${viaArgsFileBytes} bytes; the limit here is ${limit})${ mounts.holdWordsBytes > 0 - ? `. At least ${mounts.holdWordsBytes} of them are not the caller's: they name the symbolic links on the way to denied paths, for the seccomp helper to hold, and stay on the line` + ? `. At least ${mounts.holdWordsBytes} of them are not the caller's: they name the symbolic links on the way to denied paths, for the seccomp helper to hold, and stay on the line (seccomp.holdsLinks: false leaves them unheld)` : '' }`, ) @@ -1684,19 +1676,15 @@ export type LinuxUnheldLinkReason = | 'not_told' /** The helper this library found for itself is built from older sources. */ | 'older_helper' - /** That helper could not be asked: it did not start, or gave no status. */ + /** That helper's file could not be read. */ | 'no_answer' -// Keyed by path. Only an answer is kept: a probe that got none is made again -// at the next wrap. -const holdsLinksProbes = new Map() - /** * Why the seccomp helper of a wrap is not handed the links to hold, or * undefined where it is. One built from older sources takes `--hold-link` for * the command to run, so the words go to no helper that is not known to take * them: by the caller's word, or, of a helper this library found for itself, - * by asking it. + * by its file holding the name of the option that says so. */ function whyLinksAreNotHeld( helperInUse: boolean, @@ -1706,35 +1694,24 @@ function whyLinksAreNotHeld( if (config?.holdsLinks !== undefined) { return config.holdsLinks ? undefined : 'told_no' } - // THREAT: a path the caller gave need name the helper only inside the - // sandbox. Here it may name another file, and asking would run that file - // outside any sandbox. + // A path the caller gave need name the helper only inside the sandbox. const helper = config?.argv0 ? null : getApplySeccompBinaryPath(config?.applyPath) if (helper === null || helper === config?.applyPath) return 'not_told' - let holds = holdsLinksProbes.get(helper) - // Twice: no answer is not "no", and a busy host is a passing reason for none. - for (let attempt = 0; holds === undefined && attempt < 2; attempt++) { - const probe = spawnSync(helper, ['--holds-links'], { - timeout: 5000, - stdio: 'ignore', - // THREAT: an older helper looks the word up on PATH as a program, and - // this runs outside any sandbox. So there is nowhere to find one. - env: { PATH: '/dev/null' }, + // THREAT: read, never run. This is outside any sandbox, and where the + // library lies in a path the command may write (a checkout, a project's own + // node_modules) the file is whatever an earlier command left there. + try { + return fs.readFileSync(helper).includes('--holds-links') + ? undefined + : 'older_helper' + } catch (err) { + logForDebugging(`[Sandbox Linux] ${helper} could not be read: ${err}`, { + level: 'warn', }) - if (probe.error === undefined && probe.status !== null) { - holds = probe.status === 0 - holdsLinksProbes.set(helper, holds) - } else { - logForDebugging( - `[Sandbox Linux] ${helper} --holds-links gave no answer: ${probe.error?.message ?? probe.signal}`, - { level: 'warn' }, - ) - } + return 'no_answer' } - if (holds === undefined) return 'no_answer' - return holds ? undefined : 'older_helper' } /** A symbolic link where it lies, and why nothing holds it there. */ @@ -2197,10 +2174,15 @@ async function generateFilesystemArgs( mandatoryDenySearchDepth: number = DEFAULT_MANDATORY_DENY_SEARCH_DEPTH, allowGitConfig = false, abortSignal?: AbortSignal, + /** Asked once there is a link to hold; see {@link whyLinksAreNotHeld}. */ + whyNoLinkIsHeld: () => LinuxUnheldLinkReason | undefined = () => 'no_helper', ): Promise<{ args: string[] mandatoryDenyArgs: number + /** The links the command could replace, each with what it said. */ linksToHold: Array<{ path: string; target: string }> + /** Why nothing holds them, where nothing does. */ + whyNotHeld: LinuxUnheldLinkReason | undefined }> { const args: string[] = [] const startedIn = workingDirectory() @@ -2214,16 +2196,17 @@ async function generateFilesystemArgs( const denyWriteArgs: string[] = [] // Where the mandatory denies' binds begin in it: after the configuration's. let mandatoryDenyArgsStart = Infinity - // Every symbolic link on the way to a denied path, where it lies. The deny's - // own mount is where they lead, and bubblewrap can put none on a link. - const linksToDenies = new Set() - // What stands at each name those walks looked at: one look a name a wrap. - const looked = new Map() - /** Notes the links on the way to `name`, a denied path as written. Returns - * the directory the walk could not look in, if there was one. */ + // Every symbolic link on the way to a denied path: where it lies, and what + // it said. The deny's own mount is where they lead, and bubblewrap can put + // none on a link. And every directory such a way goes into and out of again. + const linksToDenies = new Map() + const leftOnTheWay = new Set() + /** Notes what the way to `name`, a denied path as written, depends on. + * Returns the directory the walk could not look in, if there was one. */ const noteLinksTo = (name: string): string | undefined => { - const { links, unsearched } = followLinks(name, looked) - for (const link of links) linksToDenies.add(link) + const { links, left, unsearched } = followLinks(name) + for (const [link, target] of links) linksToDenies.set(link, target) + for (const dir of left) leftOnTheWay.add(dir) return unsearched } // Directories that a deny entry re-binds read-only inside the sandbox @@ -3438,33 +3421,36 @@ async function generateFilesystemArgs( // no link on the way, which is every match of a pattern but the links. for (const name of [ ...readDenyPlan().map(entry => entry.normalizedPath), - ...(readConfig?.matchedLinks ?? []), ...(maskedFileBinds ?? []).map(bind => bind.realPath), ]) { const { canonical, resolved } = canonicalLocationOf(name) if (!resolved || canonical !== name) noteLinksTo(name) } + readConfig?.matchedLinks?.forEach(noteLinksTo) // A link is held where the command could replace it: whoever replaces one // decides what the denied name leads to. That is in a directory it can write - // to, and sees as it is on the host. The directories above a held link are - // pinned like those above a mount. + // to, and sees as it is on the host. const linksToHold = [...linksToDenies] .filter( - link => + ([link]) => isWithinAnyAllowedWritePath(path.dirname(link)) && !KERNEL_TOP_LEVEL_DIRS.some(dir => isAtOrUnder(link, dir)) && !isHiddenByTmpfs(path.dirname(link), 'writes and reads'), ) - // With what the link said when the plan was made for it, or nothing: no - // string spells a target that is no text. - .map(link => { - const target = looked.get(link) ?? '' - return { path: link, target: target.includes('\uFFFD') ? '' : target } - }) + .map(([link, target]) => ({ path: link, target })) + const whyNotHeld = linksToHold.length > 0 ? whyNoLinkIsHeld() : undefined const pinArgs = ancestorPinArgs( [ ...denyWriteRawDests.keys(), - ...linksToHold.map(link => link.path), + // The directories above a held link are pinned like those above a + // mount, and so is each the way goes up from, itself included. Where + // nothing holds the links, pins would keep nothing from being led away. + ...(whyNotHeld === undefined + ? [ + ...linksToHold.map(link => link.path), + ...[...leftOnTheWay].map(dir => `${dir}/.`), + ] + : []), ...fileMasks.map(mask => mask.landing), ...readDenyTmpfsUnits.map(unit => unit.landing), ], @@ -3643,7 +3629,7 @@ async function generateFilesystemArgs( args.push('--ro-bind', emptySource, emptySource) } - return { args, mandatoryDenyArgs, linksToHold } + return { args, mandatoryDenyArgs, linksToHold, whyNotHeld } } /** @@ -3946,6 +3932,7 @@ export async function wrapCommandWithSandboxLinux( args: fsArgs, mandatoryDenyArgs, linksToHold, + whyNotHeld, } = await generateFilesystemArgs( readConfig, writeConfig, @@ -3955,11 +3942,8 @@ export async function wrapCommandWithSandboxLinux( mandatoryDenySearchDepth, allowGitConfig, abortSignal, + () => whyLinksAreNotHeld(applySeccompPrefix !== undefined, seccompConfig), ) - const whyNotHeld = - linksToHold.length > 0 - ? whyLinksAreNotHeld(applySeccompPrefix !== undefined, seccompConfig) - : undefined const holdWords = whyNotHeld === undefined ? quote( diff --git a/src/sandbox/macos-sandbox-utils.ts b/src/sandbox/macos-sandbox-utils.ts index 0f6c59650..fd1d5d3cc 100644 --- a/src/sandbox/macos-sandbox-utils.ts +++ b/src/sandbox/macos-sandbox-utils.ts @@ -1007,20 +1007,21 @@ function generateWriteRules( // Combine user-specified and mandatory deny patterns (no ripgrep needed on // macOS). The caller's spellings are patterns when they read as patterns; // the mandatory entries carry their own literal/glob split. - const looked = new Map() const walked = [ ...(config.denyWithinAllow || []).map(toPathEntry), ...(config.literalDenyWithinAllow ?? []).map(toLiteralPathEntry), ...macGetMandatoryDenyEntries(allowGitConfig), ...alsoReadAs(config.denyWithinAllow, 'deny'), - ].map(entry => ({ entry, ...followLinks(entryBaseDir(entry), looked) })) + ].map(entry => ({ entry, ...followLinks(entryBaseDir(entry)) })) // As written, and where the links on the way lead, there or not: that is the // path Seatbelt compares a write through them with. A directory the walk - // could not look in is denied whole, its mode with it. - const denyEntries = walked.flatMap(({ entry, end, unsearched }) => [ + // could not look in is denied whole, its mode with it, and so is a name that + // is not there and that the way goes up from: a link made there leads it off. + const denyEntries = walked.flatMap(({ entry, end, unsearched, passed }) => [ entry, ...(end === undefined ? [] : startingFrom(end, entry)), - ...(unsearched === undefined ? [] : [toLiteralPathEntry(unsearched)]), + ...(unsearched === undefined ? [] : [unsearched]).map(toLiteralPathEntry), + ...passed.map(toLiteralPathEntry), ]) const { groups, rest: ungrouped } = groupLiteralDenyPaths(denyEntries) @@ -1047,11 +1048,13 @@ function generateWriteRules( moveFilters.add(`(literal ${escapePath(ancestorDir)})`) } } - // Each of those links stays where it lies: whoever replaces one decides - // what the denied name leads to. - for (const link of new Set(walked.flatMap(({ links }) => links))) { - if (isAtOrUnder(link, '/dev')) continue - for (const kept of [link, ...getAncestorDirectories(link)]) { + // Each of those links stays where it lies, and each directory the way goes + // up from: whoever replaces one decides what the denied name leads to. + for (const stays of new Set( + walked.flatMap(({ links, left }) => [...links.keys(), ...left]), + )) { + if (isAtOrUnder(stays, '/dev')) continue + for (const kept of [stays, ...getAncestorDirectories(stays)]) { moveFilters.add(`(literal ${escapePath(kept)})`) } } diff --git a/src/sandbox/sandbox-config.ts b/src/sandbox/sandbox-config.ts index 569e97b9d..ac8a41926 100644 --- a/src/sandbox/sandbox-config.ts +++ b/src/sandbox/sandbox-config.ts @@ -1139,9 +1139,10 @@ export const SeccompConfigSchema = z.object({ .describe( 'Whether the helper takes --hold-link, by which the symbolic links on ' + 'the way to denied paths are kept in place. Taken at its word: ' + - 'nothing is run to find out. Not set, a helper the library found ' + - 'for itself is asked (--holds-links), and one named by applyPath is ' + - 'handed no link. false also serves to turn holding off.', + 'nothing is run to find out. Not set, the file of a helper the ' + + 'library found for itself is read for the name of --holds-links, and ' + + 'one named by applyPath is handed no link. false also serves to turn ' + + 'holding off.', ), }) diff --git a/src/sandbox/sandbox-manager.ts b/src/sandbox/sandbox-manager.ts index d8ddeaf34..c0aaab0f9 100644 --- a/src/sandbox/sandbox-manager.ts +++ b/src/sandbox/sandbox-manager.ts @@ -2002,6 +2002,7 @@ function getFsReadConfig(): FsReadRestrictionConfig { getFsWriteConfig(), ) const unlistableDenyDirs = new Set() + const matchedLinks = new Set() const listings: GlobWalkListings = new Map() const denyPaths = finish( resolveReadDenies( @@ -2012,7 +2013,7 @@ function getFsReadConfig(): FsReadRestrictionConfig { pattern, reExposedPaths, unlistableDenyDirs, - { anchor, listings }, + { anchor, listings, matchedLinks }, ), ), ) @@ -2021,6 +2022,7 @@ function getFsReadConfig(): FsReadRestrictionConfig { ...denyPaths, allowWithinDeny: allowPaths, unlistableDenyDirs: [...unlistableDenyDirs], + ...(matchedLinks.size > 0 && { matchedLinks: [...matchedLinks] }), ...literalReadLists( config.filesystem.denyRead, config.filesystem.allowRead, diff --git a/src/sandbox/sandbox-utils.ts b/src/sandbox/sandbox-utils.ts index 9d76807b5..281a3c265 100644 --- a/src/sandbox/sandbox-utils.ts +++ b/src/sandbox/sandbox-utils.ts @@ -3,7 +3,7 @@ import * as path from 'path' import * as fs from 'fs' import { getPlatform } from '../utils/platform.js' import { logForDebugging } from '../utils/debug.js' -import { realPathOf } from '../utils/real-path.js' +import { realPathOf, wayPastMissingNames } from '../utils/real-path.js' import { denyGlobRegex, globToRegex, @@ -116,81 +116,80 @@ export function nameLocation( return `${parent === '/' ? '' : parent}/${path.basename(p)}` } -/** As many links as the kernel follows on the way to one path. */ -const MAX_LINKS_FOLLOWED = 40 - -/** What a look at a name fails with where it is settled that no link is there: - * nothing is, it is no link by now, or no call takes the name (a NUL byte). */ -const NO_LINK_THERE = ['ENOENT', 'ENOTDIR', 'EINVAL', 'ERR_INVALID_ARG_VALUE'] - /** - * The absolute path `p` walked name by name: the symbolic links met on the - * way, each where it lies, and where the way ends. + * What has to stay as it is for the absolute path `p`, a denied name, to go on + * leading where it leads, and where that is. * * A `.`, `..` or `//` in `p` itself is folded in text before anything is * looked at, as everywhere a configured name is read: `link/..` is the - * directory that holds the link, which is not where the kernel goes. A `..` in - * a link's target is read as the kernel reads it, the parent of the directory - * reached. - * - * A name that is not there is taken as written, so a dangling link ends at the - * path a write through it would create. There is no end past the number of - * links the kernel follows, nor past a target that is no text. + * directory that holds the link, which is not where the kernel goes. * - * INVARIANT: "could not look" is never "no link". Only a name that is not - * there (ENOENT, ENOTDIR), is no link, or is one no call takes (a NUL byte) - * counts as none. On any other failure the walk stops with no end, and - * `unsearched` is the directory reached, in which it could not look: a - * same-uid command can take a directory's search bit away and give it back - * from inside the next sandbox. Never the root, which stands in for nothing. - * - * `looked` holds what stands at each name looked at, a link's target or null. - * One map for the paths of one wrap has a directory they share looked at once. + * INVARIANT: "could not look" is never "no link". A walk that stops for any + * reason but a name that is not there, or that nothing can be at, has no end, + * and `unsearched` is the directory it could not look in: a same-uid command + * can take a directory's search bit away and give it back from inside the next + * sandbox. Never the root, which stands in for nothing. */ -export function followLinks( - p: string, - looked: Map = new Map(), -): { links: string[]; end: string | undefined; unsearched?: string } { - const links: string[] = [] - const rest = path.resolve(p).split('/') - // No link in it. - let reached = '' - for (let name = rest.shift(); name !== undefined; name = rest.shift()) { - if (name === '' || name === '.') continue - if (name === '..') { - reached = reached.slice(0, reached.lastIndexOf('/')) - continue - } - const next = `${reached}/${name}` - let target = looked.get(next) - if (target === undefined) { - try { - // Nearly every name is no link, and asking one for its target throws. - target = fs.lstatSync(next, { throwIfNoEntry: false })?.isSymbolicLink() - ? fs.readlinkSync(next) - : null - } catch (err) { - if ( - !NO_LINK_THERE.includes((err as NodeJS.ErrnoException).code ?? '') - ) { - return { links, end: undefined, unsearched: reached || undefined } - } - target = null +export function followLinks(p: string): { + /** The symbolic links on the way: where each lies, and what it says, '' + * where that is no text. */ + links: Map + /** The directories a link's target goes into and, by `..`, out of again. */ + left: Set + /** The names that are not there which one goes up from in the same way. */ + passed: string[] + /** Where the way ends, what is not there taken as written: a dangling link + * ends at the path a write through it would create. A way that goes up + * again further on among such names ends at the first: with nothing made + * there, nothing beyond comes about. None past the number of links the + * kernel follows, nor past a target that is no text. */ + end?: string + unsearched?: string +} { + const links = new Map() + const left = new Set() + const walk = (): ReturnType => + wayPastMissingNames(path.resolve(p), { + link: (at, target) => links.set(at, target), + left: dir => left.add(dir), + }) + let way: ReturnType + try { + way = walk() + } catch { + // Once more: a look can fail in passing (EIO, ESTALE), and what would + // follow from that is a directory denied whole. + try { + way = walk() + } catch (err) { + const { code, path: at } = err as NodeJS.ErrnoException + // A link all the same, which says what no string spells. + if (code === 'EILSEQ' && at) links.set(at, '') + const nothingCanBeThere = + isAbsenceErrno(err) || + code === 'EILSEQ' || + code === 'ERR_INVALID_ARG_VALUE' + const dir = nothingCanBeThere ? '/' : path.dirname(at ?? path.resolve(p)) + return { + links, + left, + passed: [], + ...(dir !== '/' && { unsearched: dir }), } - looked.set(next, target) } - if (target === null) { - reached = next - continue - } - links.push(next) - if (links.length > MAX_LINKS_FOLLOWED || target.includes('\uFFFD')) { - return { links, end: undefined } - } - if (target.startsWith('/')) reached = '' - rest.unshift(...target.split('/')) } - return { links, end: reached || '/' } + const { real, rest, notThere, passed } = way + return (notThere as NodeJS.ErrnoException | undefined)?.code === 'EACCES' + ? { links, left, passed, ...(real !== '' && { unsearched: real }) } + : { + links, + left, + passed, + end: + [real, ...(rest.includes('..') ? rest.slice(0, 1) : rest)].join( + '/', + ) || '/', + } } /** `process.cwd()`, or undefined where that throws: it has been removed. */ diff --git a/src/utils/real-path.ts b/src/utils/real-path.ts index 8ffbceebd..feadc713e 100644 --- a/src/utils/real-path.ts +++ b/src/utils/real-path.ts @@ -14,6 +14,7 @@ function textOf(ask: typeof fs.readlinkSync, at: string): string { if (text.includes('\uFFFD') && !ask(at, 'buffer').equals(Buffer.from(text))) { throw Object.assign(new Error(`EILSEQ: illegal byte sequence, '${at}'`), { code: 'EILSEQ', + path: at, }) } return text @@ -50,6 +51,15 @@ export function realPathOf(p: string): string { return real || '/' } +/** What {@link wayTo} passes that decides where it leads: whoever can put + * something else at one of these can lead it elsewhere. */ +export type WayWatcher = { + /** A link it follows: where the link lies, and what it says. */ + link?: (at: string, target: string) => void + /** A directory it goes up from again, by a `..` in a link's target. */ + left?: (dir: string) => void +} + /** * The kernel's way to `folded`, an absolute path with no `.`, `..` or `//` of * its own: name by name, split at `/` alone, every link followed. `real` is @@ -58,7 +68,10 @@ export function realPathOf(p: string): string { * look for (EACCES), with what the kernel said of it. Whatever else stops the * way is thrown. POSIX only. */ -export function wayTo(folded: string): { +export function wayTo( + folded: string, + watcher: WayWatcher = {}, +): { real: string rest: string[] notThere?: unknown @@ -77,7 +90,10 @@ export function wayTo(folded: string): { } return { real, rest: [name, ...rest], notThere } } - if (name === '..') real = real.slice(0, real.lastIndexOf('/')) + if (name === '..' && real !== '') { + watcher.left?.(real) + real = real.slice(0, real.lastIndexOf('/')) + } continue } const next = `${real}/${name}` @@ -104,8 +120,44 @@ export function wayTo(folded: string): { ) } const target = textOf(fs.readlinkSync, next) + watcher.link?.(next, target) if (target.startsWith('/')) real = '' rest.unshift(...target.split('/')) } return { real, rest } } + +/** As many missing names as one way is followed up from again. */ +const MAX_MISSING_NAMES_PASSED = 40 + +/** + * {@link wayTo}, followed on where it goes up again at once from a name that + * is not there (`missing/../x`): a directory made at that name lets the way + * pass, and a link made there leads it elsewhere. `passed` holds each such + * name, and `rest` no `.` or empty name. ELOOP past the number of them. + */ +export function wayPastMissingNames( + folded: string, + watcher?: WayWatcher, +): { real: string; rest: string[]; notThere?: unknown; passed: string[] } { + const passed: string[] = [] + let onward = folded + while (passed.length <= MAX_MISSING_NAMES_PASSED) { + const { real, rest, notThere } = wayTo(onward, watcher) + const names = rest.filter(name => name !== '' && name !== '.') + if ( + names[1] !== '..' || + (notThere as NodeJS.ErrnoException | undefined)?.code === 'EACCES' + ) { + return { real, rest: names, notThere, passed } + } + passed.push(`${real}/${names[0]}`) + onward = [real, ...names.slice(2)].join('/') + } + throw Object.assign( + new Error( + `ELOOP: '${folded}' goes up from too many names that are not there`, + ), + { code: 'ELOOP', path: folded }, + ) +} diff --git a/test/sandbox/write-deny-through-links.test.ts b/test/sandbox/write-deny-through-links.test.ts index 2a18d5d8d..11bac921a 100644 --- a/test/sandbox/write-deny-through-links.test.ts +++ b/test/sandbox/write-deny-through-links.test.ts @@ -1,4 +1,12 @@ -import { describe, it, expect, beforeEach, afterEach, spyOn } from 'bun:test' +import { + describe, + it, + expect, + beforeEach, + afterEach, + mock, + spyOn, +} from 'bun:test' import * as fs from 'fs' import { spawnSync } from 'node:child_process' import { @@ -78,6 +86,8 @@ const DENIED = [ 'blank', 'out', ] +/** EBUSY, as glibc words it ("Device or resource busy") or musl. */ +const BUSY = /resource busy/i /** No mode keeps uid 0 from looking. */ const modesCount = process.getuid?.() !== 0 @@ -126,7 +136,24 @@ function program(name: string, script: string): string { } describe.if(!isWindows)('the links on the way to a path', () => { - it.each<[string, string[], string]>([ + /** What is said of `p`, a name in ROOT, with the names in ROOT again. */ + const followed = (p: string): object => { + const { links, left, ...rest } = followLinks(join(ROOT, p)) + const named = (at: string): string => relative(ROOT, at) + return JSON.parse( + JSON.stringify( + { links: [...links], left: [...left], ...rest }, + (_key, value: unknown) => + typeof value === 'string' && value.startsWith(ROOT) + ? named(value) + : value, + ), + ) as object + } + const said = (...names: Array): string[][] => + names.map(name => [name, LINKS[name]]) + + it.each<[string, Array, string]>([ ['beside', [], 'beside'], ['name', ['name'], 'conf/x'], ['holder/commands/file', ['holder/commands'], 'shared/commands/file'], @@ -136,33 +163,47 @@ describe.if(!isWindows)('the links on the way to a path', () => { ['absent/name', [], 'absent/name'], ['beside/name', [], 'beside/name'], ['./way/./secret/', ['way'], 'real/secret'], + // Folded in text, as the path it is given always is. + ['holder/commands/../name', [], 'holder/name'], ])('%s: %p, ending at %s', (p, links, end) => { - expect(followLinks(join(ROOT, p))).toEqual({ - links: inRoot(links), - end: join(ROOT, end), - }) - }) - - it('takes a name with a NUL byte, which no call takes, as written', () => { - expect(followLinks(join(ROOT, 'way', 'nul\0byte'))).toEqual({ - links: inRoot(['way']), - end: join(ROOT, 'real', 'nul\0byte'), - }) - }) - - it('reads `..` in a target as the parent of the directory reached', () => { - symlinkSync(join(ROOT, 'shared', 'commands'), join(ROOT, 'absolute')) - symlinkSync('./absolute/../made', join(ROOT, 'up')) - expect(followLinks(join(ROOT, 'up'))).toEqual({ - links: inRoot(['up', 'absolute']), - end: join(ROOT, 'shared', 'made'), + expect(followed(p)).toMatchObject({ + links: said(...links), + passed: [], + end, }) }) - it('folds `..` in the path it is given in text', () => { - expect(followLinks(`${ROOT}/holder/commands/../name`)).toEqual({ - links: [], - end: join(ROOT, 'holder', 'name'), + it.each([ + [ + 'a directory', + 'shared/commands/../../real/secret', + { left: ['shared/commands', 'shared'], passed: [], end: 'real/secret' }, + ], + [ + 'a link to one, whose parent it is the parent of', + 'holder/commands/../made', + { + links: [...said('holder/commands')], + left: ['holder', 'shared/commands'], + passed: [], + end: 'shared/made', + }, + ], + [ + 'a name that is not there', + 'missing/../gone/../real/secret', + { left: [], passed: ['missing', 'gone'], end: 'real/secret' }, + ], + [ + 'one further on among names that are not there, where it ends at the first', + 'missing/deeper/../../real/secret', + { left: [], passed: [], end: 'missing' }, + ], + ])('a target that goes up again from %s: %s', (_what, target, told) => { + symlinkSync(target, join(ROOT, 'up')) + expect(followed('up')).toEqual({ + ...told, + links: [['up', target], ...('links' in told ? told.links : [])], }) }) @@ -174,18 +215,27 @@ describe.if(!isWindows)('the links on the way to a path', () => { it('names a link where it lies, whatever it was reached through', () => { symlinkSync(ROOT, join(OUTSIDE, 'to-root')) - expect(followLinks(join(OUTSIDE, 'to-root', 'name'))).toEqual({ - links: [join(OUTSIDE, 'to-root'), join(ROOT, 'name')], - end: join(ROOT, 'conf', 'x'), + expect([...followLinks(join(OUTSIDE, 'to-root', 'name')).links]).toEqual([ + [join(OUTSIDE, 'to-root'), ROOT], + [join(ROOT, 'name'), LINKS.name], + ]) + }) + + it('reads a target that holds U+FFFD itself like any other', () => { + symlinkSync('x�y', join(ROOT, 'odd')) + expect(followed('odd/file')).toMatchObject({ + links: [['odd', 'x�y']], + end: 'x�y/file', }) }) // Not every file system takes such a target. it.if(isLinux)('has no end past a target that is no text', () => { symlinkSync(Buffer.from([0x78, 0xff]), join(ROOT, 'bytes')) - expect(followLinks(join(ROOT, 'bytes', 'file'))).toEqual({ - links: inRoot(['bytes']), - end: undefined, + expect(followed('bytes/file')).toEqual({ + links: [['bytes', '']], + left: [], + passed: [], }) }) @@ -194,8 +244,9 @@ describe.if(!isWindows)('the links on the way to a path', () => { for (let i = 2; i <= 41; i++) { symlinkSync(`hop-${i - 1}`, join(ROOT, `hop-${i}`)) } - expect(followLinks(join(ROOT, 'hop-40')).end).toBe(join(ROOT, 'beside')) - expect(followLinks(join(ROOT, 'hop-41')).end).toBeUndefined() + expect(followed('hop-40')).toMatchObject({ end: 'beside' }) + expect(followed('hop-41')).not.toHaveProperty('end') + expect(followed('hop-41')).not.toHaveProperty('unsearched') // macOS gives up sooner, at 32: what is denied past that is denied in vain. if (isLinux) { expect(() => readFileSync(join(ROOT, 'hop-40'))).not.toThrow() @@ -203,61 +254,72 @@ describe.if(!isWindows)('the links on the way to a path', () => { expect(() => readFileSync(join(ROOT, 'hop-41'))).toThrow('ELOOP') }) + it.each([ + ['longer than a name can be', 'a'.repeat(300)], + ['with a NUL byte', 'nul\0byte'], + ])('a name %s: nothing can be there, so it has looked', (_what, name) => { + expect(followed(`way/${name}`)).toEqual({ + links: said('way'), + left: [], + passed: [], + }) + }) + it.if(modesCount)( 'stops at a directory it cannot look in, and says which', () => { chmodSync(join(ROOT, 'holder'), 0) symlinkSync('holder/commands', join(ROOT, 'to-holder')) - expect(followLinks(join(ROOT, 'to-holder', 'file'))).toEqual({ - links: inRoot(['to-holder']), - end: undefined, - unsearched: join(ROOT, 'holder'), + expect(followed('to-holder/file')).toEqual({ + links: [['to-holder', 'holder/commands']], + left: [], + passed: [], + unsearched: 'holder', }) }, ) - it.each([ - ['ENOENT', undefined], - ['EINVAL', undefined], - ['EIO', ''], - ['ESTALE', ''], - ['ENAMETOOLONG', ''], - ])('a look that fails with %s: could not look in %p', (code, unsearched) => { - const spy = spyOn(fs, 'readlinkSync').mockImplementation(() => { - throw Object.assign(new Error(code), { code }) - }) - try { - expect(followLinks(join(ROOT, 'name')).unsearched).toBe( - unsearched === undefined ? undefined : ROOT, - ) - } finally { - spy.mockRestore() - } - }) + describe('a look that fails', () => { + afterEach(() => mock.restore()) + const failing = (code: string, path?: string): Error => + Object.assign(new Error(code), { code, path }) + + it.each(['EIO', 'ESTALE', 'EINVAL'])( + 'with %s both times: the directory of what it looked at', + code => { + spyOn(fs, 'readlinkSync').mockImplementation(at => { + throw failing(code, String(at)) + }) + expect(followed('holder/commands/file')).toMatchObject({ + unsearched: 'holder', + }) + }, + ) - it('never gives the root for the directory it could not look in', () => { - const spy = spyOn(fs, 'lstatSync').mockImplementation(() => { - throw Object.assign(new Error('EIO'), { code: 'EIO' }) + it('once: it looks again', () => { + spyOn(fs, 'readlinkSync').mockImplementationOnce(at => { + throw failing('EIO', String(at)) + }) + expect(followed('name')).toMatchObject({ end: 'conf/x' }) }) - try { - expect(followLinks(ROOT)).toEqual({ links: [], end: undefined }) - } finally { - spy.mockRestore() - } - }) - it('looks at a name once for all the paths that share a map', () => { - const looked = new Map() - followLinks(join(ROOT, 'name'), looked) - expect(looked.get(ROOT)).toBeNull() - expect(looked.get(join(ROOT, 'name'))).toBe('conf/x') - expect(looked.get(join(ROOT, 'conf', 'x'))).toBeNull() + it('and does not say where: the directory of the path as written', () => { + spyOn(fs, 'readlinkSync').mockImplementation(() => { + throw failing('EIO') + }) + expect(followed('way/secret')).toMatchObject({ unsearched: 'way' }) + }) - rmSync(join(ROOT, 'name')) - expect(followLinks(join(ROOT, 'name'), looked).links).toEqual( - inRoot(['name']), - ) - expect(followLinks(join(ROOT, 'name')).links).toEqual([]) + it('in the root: no directory, which would stand in for everything', () => { + spyOn(fs, 'lstatSync').mockImplementation(at => { + throw failing('EIO', String(at)) + }) + expect(followLinks(ROOT)).toEqual({ + links: new Map(), + left: new Set(), + passed: [], + }) + }) }) }) @@ -292,7 +354,6 @@ describe.if((isLinux && bwrapCanNamespace()) || isMacOS)( cwd: OUTSIDE, }) const said = `${result.stdout}${result.stderr}` - console.log(`${attempt}\n ${said.trim().split('\n').join('\n ')}`) expect(said).toContain('started') if (isLinux) cleanupBwrapMountPoints({ force: true }) return said.includes('DONE') ? 'DONE' : 'refused' @@ -378,6 +439,28 @@ describe.if((isLinux && bwrapCanNamespace()) || isMacOS)( }, ) + it.each([ + [ + 'a directory', + 'shared/commands/../../real/secret', + 'mv shared/commands aside', + ], + ['a directory', 'shared/commands/../../real/secret', 'mv shared aside'], + ['a name that is not there', 'missing/../real/secret', 'mkdir missing'], + [ + 'a name that is not there', + 'missing/../real/secret', + 'ln -s conf missing', + ], + ])( + 'where a link goes up again from %s (%s), refuses: %s', + async (_what, target, attempt) => { + symlinkSync(target, join(ROOT, 'up')) + expect(await outcomeOf(attempt, ['up'])).toBe('refused') + expect(await outcomeOf('echo changed >> up', ['up'])).toBe('refused') + }, + ) + it.if(modesCount)( 'keeps a directory that cannot be looked in as it is, mode and all', async () => { @@ -813,70 +896,43 @@ describe.if(isLinux)('where no link is held', () => { expect(getLinuxUnheldLinks()).toEqual([]) }) - it('a helper the library found for itself is asked once', async () => { - const spy = found(noting(RUNS_ITS_WORDS)) - try { - for (const _ of [1, 2]) { - expect(await wrapLinux(inRoot(['first']))).not.toContain('--hold-link') - expect(getLinuxUnheldLinks()).toEqual(unheld('older_helper')) - } - } finally { - spy.mockRestore() - } - expect(runs()).toBe(1) - }) - - it('it is not asked where there is nothing to hold', async () => { - const spy = found(noting(RUNS_ITS_WORDS)) - try { - await wrapLinux(inRoot(['beside'])) - } finally { - spy.mockRestore() - } - expect(runs()).toBe(0) - }) - - it.each([ - ['ends by a signal', 'kill -9 $$', 0o755, 2], - ['cannot be started', '', 0o644, 0], + it.each<[string, string, number, LinuxUnheldLinkReason | undefined]>([ + ['names the option that says so', '# --holds-links', 0o755, undefined], + ['does not', RUNS_ITS_WORDS, 0o755, 'older_helper'], + ...(modesCount + ? [ + ['cannot be read', '', 0, 'no_answer'] as [ + string, + string, + 0, + 'no_answer', + ], + ] + : []), ])( - 'one that %s is asked twice, and again at the next wrap', - async (_what, script, mode, perWrap) => { + 'a helper the library found for itself is read, never run: one that %s', + async (_what, script, mode, reason) => { const helper = noting(script) chmodSync(helper, mode) const spy = found(helper) try { - for (const wraps of [1, 2]) { - const { warnings } = await withCapturedWarnings(() => - wrapLinux(inRoot(['first'])), - ) - expect(getLinuxUnheldLinks()).toEqual(unheld('no_answer')) - expect( - warnings.filter(l => l.includes('gave no answer')), - ).toHaveLength(2) - expect(runs()).toBe(perWrap * wraps) - } + const wrapped = await wrapLinux(inRoot(['first'])) + expect(wrapped.includes('--hold-link')).toBe(reason === undefined) } finally { spy.mockRestore() } + expect(getLinuxUnheldLinks()).toEqual(reason ? unheld(reason) : []) + expect(runs()).toBe(0) }, ) - it('asking an older helper runs no program of that name', async () => { - const named = join(ROOT, '--holds-links') - writeFileSync(named, `#!/bin/sh\necho ran > ${ROOT}/ran\n`) - chmodSync(named, 0o755) - const PATH = process.env.PATH - process.env.PATH = `${ROOT}:.:${PATH}` - const spy = found(noting(RUNS_ITS_WORDS)) - try { - await fromRoot(() => wrapLinux(inRoot(['name']))) - } finally { - spy.mockRestore() - process.env.PATH = PATH - } - expect(runs()).toBe(1) - expect(existsSync(join(ROOT, 'ran'))).toBe(false) + it('what holds a link is not pinned for it either', async () => { + const pin = `--ro-bind ${ROOT}/holder ${ROOT}/holder ` + const denied = inRoot(['holder/commands']) + expect(await wrapLinux(denied)).toContain(pin) + expect( + await wrapLinux(denied, { seccompConfig: { holdsLinks: false } }), + ).not.toContain(pin) }) it('a link is listed by the latest wrap that met it', async () => { @@ -886,12 +942,11 @@ describe.if(isLinux)('where no link is held', () => { }) it('a wrap that throws names none', async () => { - const wrap = wrapLinux(inRoot(['first']), { + const refusal: unknown = await wrapLinux(inRoot(['first']), { allowAllUnixSockets: true, binShell: 'no-such-shell', - }) - expect(wrap).rejects.toThrow('no-such-shell') - await wrap.catch(() => {}) + }).catch((error: unknown) => error) + expect(String(refusal)).toContain('no-such-shell') expect(getLinuxUnheldLinks()).toEqual([]) }) @@ -917,6 +972,17 @@ describe.if(isLinux)('where no link is held', () => { expect(SandboxManager.getLinuxUnheldLinks()).toEqual(unheld('told_no')) }) + it('getFsReadConfig() names the links its patterns match, as a wrap does', async () => { + const { filesystem, ...rest } = config(false) + await SandboxManager.initialize({ + ...rest, + filesystem: { ...filesystem, denyRead: [join(ROOT, 'nam*')] }, + }) + expect(SandboxManager.getFsReadConfig().matchedLinks).toEqual( + inRoot(['name']), + ) + }) + it('lists them', async () => { await SandboxManager.initialize(config(true)) await SandboxManager.wrapWithSandbox('true') @@ -987,14 +1053,14 @@ describe.if(isLinux && bwrapCanNamespace())('the seccomp helper', () => { run(helper(), ['--hold-link', join(ROOT, name), target, 'echo', 'ran']), ).toEqual({ status: 1, - said: `apply-seccomp: --hold-link ${join(ROOT, name)}: ${why}\n`, + said: `apply-seccomp: --hold-link ${join(ROOT, name)}: ${why} (seccomp.holdsLinks: false in the sandbox configuration holds no link)\n`, }) }) it('holds a link it is given no word on', () => { expect( run(helper(), ['--hold-link', join(ROOT, 'name'), '', 'rm', 'name']).said, - ).toContain('Device or resource busy') + ).toMatch(BUSY) }) // The filter answers first, whoever asks. Where it is lifted, this user lacks @@ -1021,7 +1087,7 @@ describe.if(isLinux && bwrapCanNamespace())('the seccomp helper', () => { `echo read > ${OUTSIDE}/x && ln -s x ${link} && ` + `exec ${helper()} --hold-link ${link} x sh -c 'rm ${link}; cat ${link}'`, ]) - expect(said).toContain('Device or resource busy') + expect(said).toMatch(BUSY) expect(said).toContain('Too many levels of symbolic links') expect(said).not.toContain('read') }) @@ -1039,7 +1105,7 @@ describe.if(isLinux && bwrapCanNamespace())('the seccomp helper', () => { NAMESPACES_ALLOWED, ) expect(said).toContain('started') - expect(said.match(/Device or resource busy/g)).toHaveLength(3) + expect(said.match(new RegExp(BUSY, 'gi'))).toHaveLength(3) }, ) diff --git a/vendor/seccomp-src/apply-seccomp.c b/vendor/seccomp-src/apply-seccomp.c index 824fef9b9..39530aab0 100644 --- a/vendor/seccomp-src/apply-seccomp.c +++ b/vendor/seccomp-src/apply-seccomp.c @@ -728,7 +728,10 @@ static void enter_user_namespace(void) { } static void die_holding(const char *path, const char *why) { - fprintf(stderr, "apply-seccomp: --hold-link %s: %s\n", path, why); + fprintf(stderr, + "apply-seccomp: --hold-link %s: %s (seccomp.holdsLinks: false in the " + "sandbox configuration holds no link)\n", + path, why); _exit(1); } From 4c9a85d05e58ea7036f03156f0e2ee84a945d677 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 20 Oct 2026 12:51:03 +0000 Subject: [PATCH 6/8] A simplification pass over the source of the links on the way to a deny No behaviour changes but these: the packaged helper's file is read at every wrap that leaves seccomp.holdsLinks unset (0.3 ms), not only where there is a link to hold, and that it could not be read is told by the reason alone. - Whether links are held is asked before the mounts are planned and handed down as a boolean, in place of a function in and a reason out. - followLinks() is two flat tries, and wayPastMissingNames() says `shut` in place of an errno for three callers to cast. - macOS keeps main's inBothSpellings(), which takes the end of a walk. - Comments and the README paragraph are shorter. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01FydxqUESqxwSCVFnhk3gfv Co-authored-by: ronleizrowice --- README.md | 27 +++---- src/sandbox/linux-sandbox-utils.ts | 122 ++++++++++------------------- src/sandbox/macos-sandbox-utils.ts | 40 ++++------ src/sandbox/sandbox-config.ts | 10 +-- src/sandbox/sandbox-manager.ts | 5 +- src/sandbox/sandbox-schemas.ts | 5 +- src/sandbox/sandbox-utils.ts | 71 ++++++----------- src/utils/real-path.ts | 17 ++-- 8 files changed, 110 insertions(+), 187 deletions(-) diff --git a/README.md b/README.md index 357d0ee0c..ccb80f825 100644 --- a/README.md +++ b/README.md @@ -595,7 +595,7 @@ Examples: - `allowPty` - macOS only: allow pseudo-terminal operations (boolean, default: false). The seatbelt profile then carries `(allow pseudo-tty)` plus read, write and `ioctl` on `/dev/ptmx` and `/dev/ttys*`, which a command that allocates a pty of its own needs. Not read on Linux or Windows. - `bwrapPath` - Linux only: absolute path to the `bwrap` (bubblewrap) binary, used instead of resolving `bwrap` on `PATH`. A path that is not executable is a dependency error, and no `PATH` lookup is tried after it. - `socatPath` - Linux only: absolute path to the `socat` binary, used instead of resolving `socat` on `PATH`, with the same rule for a path that is not executable. -- `seccomp` - Linux only: `applyPath` points at the `apply-seccomp` binary to use instead of the packaged one, which is still used when nothing exists at that path. `argv0` invokes it as a multicall binary that dispatches on the `ARGV0` environment variable; `applyPath` is then used verbatim (no existence check) and must resolve inside the bubblewrap namespace. `holdsLinks` says whether the helper takes `--hold-link` (see "Denied paths and symbolic links") and is taken at its word: nothing is run to find out. Not set, the file of a helper the library found for itself is read for the name of the option `--holds-links`, and one named by `applyPath`, which need not name the helper outside the sandbox, is handed no link. Set it from a probe of your own helper (`apply-seccomp --holds-links` exits 0), not from a constant: a helper without the option takes `--hold-link` for the command, so under a wrong `true` no sandboxed command starts in a project with such a link. +- `seccomp` - Linux only: `applyPath` points at the `apply-seccomp` binary to use instead of the packaged one, which is still used when nothing exists at that path. `argv0` invokes it as a multicall binary that dispatches on the `ARGV0` environment variable; `applyPath` is then used verbatim (no existence check) and must resolve inside the bubblewrap namespace. `holdsLinks` says whether the helper takes `--hold-link` (see "Denied paths and symbolic links"), and is taken at its word: nothing is run to find out. Not set, a helper the library found for itself is read for the option's name, and one named by `applyPath` is handed no link. Set it from a probe of your own helper (`apply-seccomp --holds-links` exits 0), not from a constant: a helper without the option takes `--hold-link` for the command, so under a wrong `true` no sandboxed command starts where there is a link to hold. - `ripgrep` - Linux only: how to invoke ripgrep for the deny-path scan (default: `{ "command": "rg" }`). `args` are passed before srt's own arguments, and `argv0` overrides `argv[0]` for a multicall binary. - `git.safeDirectories` - Directories git should treat as `safe.directory` inside the sandbox, where the working tree is owned by another user — the repository top level when the command runs from a subdirectory, say. Injected through `GIT_CONFIG_*` environment variables, and it grants no write access of its own. - `credentials` - Environment variables (`credentials.envVars`) and files (`credentials.files`) the sandboxed command must not read the real values of. `mode: "deny"` withholds the value; `mode: "mask"` puts a sentinel in its place inside the sandbox and has the proxy substitute the real value back on egress to the hosts that entry's `injectHosts` lists (default: `network.allowedDomains`). Masking requires `network.tlsTerminate`, so the real value only leaves over a verified TLS connection, unless `credentials.allowPlaintextInject` opts out. Files are masked on Linux only; macOS denies a masked file instead. @@ -1118,20 +1118,17 @@ Filesystem restrictions are enforced at the OS level: **Writes through links (Linux and macOS):** a write is judged by where it lands, after symbolic links are followed. A link inside an allowed write path that leads out of it gives a command nothing: the write is refused. A link that leads into an allowed write path works like any other name for that place. A hard link is a second name for the file itself: on Linux a file outside the allowed write paths that already has a hard link inside one can be written through it, and a sandboxed command cannot create such a link. On macOS nothing is promised for such a file. -**Denied paths and symbolic links (Linux and macOS):** a `denyWrite` entry or a [mandatory deny path](#mandatory-deny-paths-auto-protected-files) with a symbolic link on the way to it, its last name included, denies the path the links lead to, whether or not anything is there yet, and keeps each of those links where it is: removing it, renaming it and putting something else at its name are refused, and so is renaming the directories above it, as above any denied path. Where a link's target goes into a directory and out of it again by `..`, that directory is kept in place too, and a name that is not there cannot be made where the target goes up from it. The link reads and leads as before, and everything else in the directory that holds it stays as writable as it was. The links are those found when the command is wrapped. - -- Nothing is denied among the devices, for reading or writing: a name blanked with a link to `/dev/null` is kept, and `/dev/null` stays as it was. -- A tool that replaces a file instead of writing into it is refused at such a name, as at any denied one: `sed -i`, `perl -i`, `mv -f`, a write to a temporary file that is then renamed, `install`, `ln -sfn`, `rsync`, `tar x`, `unzip -o`, `git checkout` where the tracked link differs, `git clean -fdx`. On Linux the error is `EBUSY` ("Device or resource busy"). -- A directory on the way that cannot be looked in (its search permission is gone, or the look fails twice, for any reason but that nothing is or can be at the name) is denied whole in place of the path, its mode included: what is in it is not known. Never `/`: where that is the directory, the path is taken as written. -- On Linux the same links are kept on the way to a `denyRead` entry, to a link that a `denyRead` pattern matches, and to a `credentials.files` path, denied or masked. Each command's mounts are placed by the name, so a name that led elsewhere by the next command would leave what it led to unhidden. -- On Linux bubblewrap cannot mount on a link, so the seccomp helper does (`apply-seccomp --hold-link `): each link that lies in a directory the command can write to is mounted on itself, read-only. The hold is a mount like every deny of this sandbox and holds what those hold: it is in force in the mount namespaces made for the command. -- No command is started where a hold cannot be made: the name is no link any more, the link says something else than when the command was wrapped, or the mount is refused (a security module, a file system that does not take one). The helper exits 1 and its one line on stderr starts with `apply-seccomp: --hold-link : `. `seccomp.holdsLinks: false` turns holding off. -- Not held on Linux: `SandboxManager.getLinuxUnheldLinks()` returns each link that should be held and is not, as `{ path, reason }`, for the embedder to show or to look at again after the command; the only other sign is a line under `SRT_DEBUG`. What such a link leads to is denied all the same, and the link is an ordinary entry of a writable directory. A link is listed by the latest wrap that met it, so one that a later wrap holds is listed no longer, and `reset()` and `updateConfig()` empty the list. The reasons: `no_helper` (`allowAllUnixSockets`, or no binary for the architecture), `told_no` (`seccomp.holdsLinks` is false), `not_told` (the helper is named by `seccomp.applyPath` and `seccomp.holdsLinks` is not set), `older_helper` (the helper the library found for itself is built from older sources), `no_answer` (that helper's file could not be read). A wrap still in flight when `reset()` or `updateConfig()` runs lists its links afterwards. -- Not covered: a mandatory deny name _below_ the working directory that is itself a link. On Linux the scan lists files and does not see it. On macOS the pattern keeps the name in place and says nothing of what it leads to. -- Not covered on Linux: a link on the way to what a `denyRead` pattern matches, other than the match itself. A `denyWrite` pattern is skipped there as a whole. -- Not covered on Linux: a writer outside the sandbox that changes a link back and forth while the command is being wrapped. The deny's mount and the word the helper checks the link against come from two looks. -- Not reported on Linux: the violation monitor judges by the configured names, so a refused attempt on a link on the way to one, or on a directory kept for it, is no violation in its store. -- The links' names stay on the command line (see the note on large profiles below), about as many bytes each as the path and the target together: some thousand of them, with the command, reach the limit of one argument, and the `command_too_long` refusal says how many bytes are theirs. +**Denied paths and symbolic links (Linux and macOS):** a `denyWrite` entry or a [mandatory deny path](#mandatory-deny-paths-auto-protected-files) with a symbolic link on the way to it, its last name included, denies the path the links lead to, whether or not anything is there yet, and keeps each link where it is: removing, renaming and replacing it are refused, and so is renaming a directory above it. A directory that a link's target goes into and out of again by `..` is kept in place too, and a name that is not there and that the target goes up from cannot be made. The link reads and leads as before, and the rest of its directory stays as writable as it was. The links are those found when the command is wrapped. + +- Nothing is denied among the devices: a name blanked with a link to `/dev/null` is kept, and `/dev/null` stays as it was. +- A tool that replaces a file instead of writing into it (`sed -i`, `mv -f`, `ln -sfn`, `rsync`, `tar x`, `git checkout` where the tracked link differs, `git clean -fdx`) is refused at such a name, as at any denied one, on Linux with `EBUSY` ("Device or resource busy"). +- A directory on the way that cannot be looked in (no search permission, or a look that fails twice for any reason but that nothing is or can be at the name) is denied whole in place of the path, its mode included. Never `/`: there the path is taken as written. +- On Linux the same links are kept on the way to a `denyRead` entry, to a link that a `denyRead` pattern matches, and to a `credentials.files` path: the next command's mounts are placed by the name again. +- On Linux bubblewrap cannot mount on a link, so the seccomp helper does (`apply-seccomp --hold-link `): each link in a directory the command can write to is mounted on itself, read-only, in the mount namespaces made for the command. Where that cannot be done (the name is no link any more, the link says something else than when the command was wrapped, the mount is refused), no command is started: the helper exits 1, and its one line on stderr starts with `apply-seccomp: --hold-link : `. `seccomp.holdsLinks: false` turns holding off. +- On Linux `SandboxManager.getLinuxUnheldLinks()` returns each link that should be held and is not, as `{ path, reason }`, to show or to look at again after the command; the only other sign is a line under `SRT_DEBUG`. What such a link leads to is denied all the same, and the command can replace the link. The reasons: `no_helper` (`allowAllUnixSockets`, or no binary for the architecture), `told_no` (`seccomp.holdsLinks` is false), `not_told` (the helper is named by `seccomp.applyPath` and `seccomp.holdsLinks` is not set), `older_helper` (the helper the library found for itself is built from older sources), `no_answer` (that helper's file could not be read). A link is listed by the latest wrap that met it. `reset()` and `updateConfig()` empty the list, and a wrap still in flight then lists its links afterwards. +- On Linux the held links' paths and targets stay on the command line (see the note on large profiles below): some thousand of them reach the limit of one argument, and the `command_too_long` refusal says how many bytes are theirs. +- Not covered: a mandatory deny name _below_ the working directory that is itself a link (the Linux scan does not see it; the macOS pattern keeps the name and says nothing of what it leads to). On Linux, a link on the way to what a `denyRead` pattern matches, other than the match itself (a `denyWrite` pattern is skipped there as a whole), and a writer outside the sandbox that changes a link while the command is being wrapped. +- Not reported on Linux: the violation monitor judges by the configured names, so a refused attempt on a kept link or directory is no violation in its store. **Read-side rules (Linux):** an entry is matched by the name it is, not by what it points at. diff --git a/src/sandbox/linux-sandbox-utils.ts b/src/sandbox/linux-sandbox-utils.ts index c084eba65..c881ab091 100644 --- a/src/sandbox/linux-sandbox-utils.ts +++ b/src/sandbox/linux-sandbox-utils.ts @@ -225,14 +225,9 @@ function resolveSymlinkedDenyPath( } let why: unknown try { - const way = wayPastMissingNames(rejoined) - alsoDenied?.push(...way.passed) - const shut = (way.notThere as NodeJS.ErrnoException | undefined)?.code - return ( - [way.real, ...(shut === 'EACCES' ? [] : way.rest.slice(0, 2))].join( - '/', - ) || '/' - ) + const { real, rest, shut, passed } = wayPastMissingNames(rejoined) + alsoDenied?.push(...passed) + return [real, ...(shut ? [] : rest.slice(0, 2))].join('/') || '/' } catch (err) { why = err } @@ -1666,25 +1661,18 @@ export async function initializeLinuxNetworkBridge( } } -/** Why a symbolic link that should be held in place is not. */ +/** Why a symbolic link that should be held in place is not; see README.md. */ export type LinuxUnheldLinkReason = - /** No seccomp helper runs in the wrap. */ | 'no_helper' - /** `seccomp.holdsLinks` is false. */ | 'told_no' - /** The helper is the caller's, and `seccomp.holdsLinks` is not set. */ | 'not_told' - /** The helper this library found for itself is built from older sources. */ | 'older_helper' - /** That helper's file could not be read. */ | 'no_answer' /** - * Why the seccomp helper of a wrap is not handed the links to hold, or - * undefined where it is. One built from older sources takes `--hold-link` for - * the command to run, so the words go to no helper that is not known to take - * them: by the caller's word, or, of a helper this library found for itself, - * by its file holding the name of the option that says so. + * Why the seccomp helper is not handed the links to hold, or undefined where + * it is. One built from older sources takes `--hold-link` for the command to + * run, so the words go to no helper that is not known to take them. */ function whyLinksAreNotHeld( helperInUse: boolean, @@ -1706,10 +1694,7 @@ function whyLinksAreNotHeld( return fs.readFileSync(helper).includes('--holds-links') ? undefined : 'older_helper' - } catch (err) { - logForDebugging(`[Sandbox Linux] ${helper} could not be read: ${err}`, { - level: 'warn', - }) + } catch { return 'no_answer' } } @@ -1720,13 +1705,7 @@ export type LinuxUnheldLink = { path: string; reason: LinuxUnheldLinkReason } // By link: what the latest wrap that met it came to. const unheldLinks = new Map() -/** - * The symbolic links on the way to a denied path that lie in a directory the - * command could write to, and that nothing kept in place there: the command - * could put something else at the name. Each link is listed by the latest - * wrap that met it, so one that a later wrap holds is listed no longer. See - * "Denied paths and symbolic links" in README.md. - */ +/** `SandboxManager.getLinuxUnheldLinks()`. */ export function getLinuxUnheldLinks(): LinuxUnheldLink[] { return [...unheldLinks].map(([path, reason]) => ({ path, reason })) } @@ -2174,15 +2153,13 @@ async function generateFilesystemArgs( mandatoryDenySearchDepth: number = DEFAULT_MANDATORY_DENY_SEARCH_DEPTH, allowGitConfig = false, abortSignal?: AbortSignal, - /** Asked once there is a link to hold; see {@link whyLinksAreNotHeld}. */ - whyNoLinkIsHeld: () => LinuxUnheldLinkReason | undefined = () => 'no_helper', + /** Whether the caller has `linksToHold` held. */ + linksAreHeld = false, ): Promise<{ args: string[] mandatoryDenyArgs: number /** The links the command could replace, each with what it said. */ - linksToHold: Array<{ path: string; target: string }> - /** Why nothing holds them, where nothing does. */ - whyNotHeld: LinuxUnheldLinkReason | undefined + linksToHold: Array<[link: string, target: string]> }> { const args: string[] = [] const startedIn = workingDirectory() @@ -2196,13 +2173,10 @@ async function generateFilesystemArgs( const denyWriteArgs: string[] = [] // Where the mandatory denies' binds begin in it: after the configuration's. let mandatoryDenyArgsStart = Infinity - // Every symbolic link on the way to a denied path: where it lies, and what - // it said. The deny's own mount is where they lead, and bubblewrap can put - // none on a link. And every directory such a way goes into and out of again. + // What the ways to the denied paths depend on; see followLinks. const linksToDenies = new Map() const leftOnTheWay = new Set() - /** Notes what the way to `name`, a denied path as written, depends on. - * Returns the directory the walk could not look in, if there was one. */ + /** Returns the directory the walk to `name` could not look in, if any. */ const noteLinksTo = (name: string): string | undefined => { const { links, left, unsearched } = followLinks(name) for (const [link, target] of links) linksToDenies.set(link, target) @@ -2714,10 +2688,9 @@ async function generateFilesystemArgs( // against it on both sides of the await above, the only one here. if (workingDirectory() !== startedIn) throw new WorkingDirectoryChanged() - // Each deny path as the two loops below take it, from one walk, so that - // they agree. A directory the walk could not look in stands in for the - // path: what is in it is not known, and bound read-only it keeps its mode. - const asTaken = (pathPattern: string): string => { + // Each deny path as both loops below take it. A directory the walk could + // not look in stands in for the path: bound read-only, it keeps its mode. + const rawDenyPaths = denyPaths.map(pathPattern => { const rawPath = normalizePathForSandbox(pathPattern, { literal: true }) // Both loops skip it. if (rawPath.startsWith('/dev/')) return rawPath @@ -2728,8 +2701,7 @@ async function generateFilesystemArgs( { level: 'warn' }, ) return unsearched - } - const rawDenyPaths = denyPaths.map(asTaken) + }) // Duplicate deny entries must be collapsed: a duplicate // --ro-bind /dev/null hits a char device on the second pass and @@ -2759,11 +2731,7 @@ async function generateFilesystemArgs( if (rawPath.startsWith('/dev/')) { continue } - // What it adds is at the end of the list this loop is going through, and - // so is met by it and by the one below. - const alsoDenied: string[] = [] - const resolvedPath = resolveSymlinkedDenyPath(rawPath, alsoDenied) - rawDenyPaths.push(...alsoDenied.map(asTaken)) + const resolvedPath = resolveSymlinkedDenyPath(rawPath, rawDenyPaths) if (resolvedPath === null || resolvedPath.startsWith('/dev/')) { continue } @@ -3414,11 +3382,10 @@ async function generateFilesystemArgs( // the mount sits. Deny binds, tmpfs units and masks are emitted later and // land on top of both pins and covers. // - // The read side's names count like the write denies': each wrap finds what - // to hide by the name, so a name that leads elsewhere by the next wrap - // leaves what it led to unhidden. A path that cannot be looked at has its - // stand-in already (readDenyTargetOf). One that is where it resolves to has - // no link on the way, which is every match of a pattern but the links. + // The read side's names count like the write denies': the next wrap finds + // what to hide by the name again. A path that cannot be looked at has its + // stand-in already (readDenyTargetOf), and one that is where it resolves to + // has no link on the way. for (const name of [ ...readDenyPlan().map(entry => entry.normalizedPath), ...(maskedFileBinds ?? []).map(bind => bind.realPath), @@ -3427,27 +3394,23 @@ async function generateFilesystemArgs( if (!resolved || canonical !== name) noteLinksTo(name) } readConfig?.matchedLinks?.forEach(noteLinksTo) - // A link is held where the command could replace it: whoever replaces one - // decides what the denied name leads to. That is in a directory it can write - // to, and sees as it is on the host. - const linksToHold = [...linksToDenies] - .filter( - ([link]) => - isWithinAnyAllowedWritePath(path.dirname(link)) && - !KERNEL_TOP_LEVEL_DIRS.some(dir => isAtOrUnder(link, dir)) && - !isHiddenByTmpfs(path.dirname(link), 'writes and reads'), - ) - .map(([link, target]) => ({ path: link, target })) - const whyNotHeld = linksToHold.length > 0 ? whyNoLinkIsHeld() : undefined + // A link is held where the command could replace it: in a directory it can + // write to, and sees as it is on the host. + const linksToHold = [...linksToDenies].filter( + ([link]) => + isWithinAnyAllowedWritePath(path.dirname(link)) && + !KERNEL_TOP_LEVEL_DIRS.some(dir => isAtOrUnder(link, dir)) && + !isHiddenByTmpfs(path.dirname(link), 'writes and reads'), + ) const pinArgs = ancestorPinArgs( [ ...denyWriteRawDests.keys(), // The directories above a held link are pinned like those above a // mount, and so is each the way goes up from, itself included. Where // nothing holds the links, pins would keep nothing from being led away. - ...(whyNotHeld === undefined + ...(linksAreHeld || linksToHold.length === 0 ? [ - ...linksToHold.map(link => link.path), + ...linksToHold.map(([link]) => link), ...[...leftOnTheWay].map(dir => `${dir}/.`), ] : []), @@ -3629,7 +3592,7 @@ async function generateFilesystemArgs( args.push('--ro-bind', emptySource, emptySource) } - return { args, mandatoryDenyArgs, linksToHold, whyNotHeld } + return { args, mandatoryDenyArgs, linksToHold } } /** @@ -3928,11 +3891,14 @@ export async function wrapCommandWithSandboxLinux( } // ========== FILESYSTEM RESTRICTIONS ========== + const whyNotHeld = whyLinksAreNotHeld( + applySeccompPrefix !== undefined, + seccompConfig, + ) const { args: fsArgs, mandatoryDenyArgs, linksToHold, - whyNotHeld, } = await generateFilesystemArgs( readConfig, writeConfig, @@ -3942,17 +3908,11 @@ export async function wrapCommandWithSandboxLinux( mandatoryDenySearchDepth, allowGitConfig, abortSignal, - () => whyLinksAreNotHeld(applySeccompPrefix !== undefined, seccompConfig), + whyNotHeld === undefined, ) const holdWords = whyNotHeld === undefined - ? quote( - linksToHold.flatMap(link => [ - '--hold-link', - link.path, - link.target, - ]), - ) + ? quote(linksToHold.flatMap(pair => ['--hold-link', ...pair])) : '' if (holdWords !== '') applySeccompPrefix += `${holdWords} ` const mountsStart = bwrapArgs.length @@ -4088,7 +4048,7 @@ export async function wrapCommandWithSandboxLinux( `[Sandbox Linux] Wrapped command with bwrap (${restrictions.join(', ')} restrictions)`, ) - for (const { path: link } of linksToHold) { + for (const [link] of linksToHold) { if (whyNotHeld === undefined) { unheldLinks.delete(link) continue diff --git a/src/sandbox/macos-sandbox-utils.ts b/src/sandbox/macos-sandbox-utils.ts index fd1d5d3cc..b80eeb46e 100644 --- a/src/sandbox/macos-sandbox-utils.ts +++ b/src/sandbox/macos-sandbox-utils.ts @@ -363,33 +363,31 @@ interface ResolvedReadConfig { * links on the way resolved. A deny is resolved to its end. An allow is a * name, so its last component stays as written. Of a pattern, the directory * it starts from is what is resolved. The root is never a second spelling. + * `end` is the second spelling where the caller has walked there itself. */ -function inBothSpellings(kind: PathListKind, entry: PathEntry): PathEntry[] { +function inBothSpellings( + kind: PathListKind, + entry: PathEntry, + end?: string, +): PathEntry[] { const resolved = (p: string): string => pathSpellings(p)[1] ?? p const base = entryBaseDir(entry) const canonical = - kind === 'deny' || entry.glob + end ?? + (kind === 'deny' || entry.glob ? resolved(base) - : nameLocation(base, resolved) - return [entry, ...startingFrom(canonical, entry)] -} - -/** - * `entry` with `canonical` for the path it is, or for the directory its - * pattern starts from; nothing where that is no other spelling, or the root. - * Nor among the devices, as on Linux: a name blanked with a link to /dev/null - * would take /dev/null from every command. - */ -function startingFrom(canonical: string, entry: PathEntry): PathEntry[] { - const base = entryBaseDir(entry) + : nameLocation(base, resolved)) + // Nor among the devices: a rule on a name blanked with a link to /dev/null + // would be one on /dev/null for every command. if ( canonical === base || canonical === '/' || isAtOrUnder(canonical, '/dev') ) { - return [] + return [entry] } return [ + entry, entry.glob ? { glob: true, @@ -1014,12 +1012,10 @@ function generateWriteRules( ...alsoReadAs(config.denyWithinAllow, 'deny'), ].map(entry => ({ entry, ...followLinks(entryBaseDir(entry)) })) // As written, and where the links on the way lead, there or not: that is the - // path Seatbelt compares a write through them with. A directory the walk - // could not look in is denied whole, its mode with it, and so is a name that - // is not there and that the way goes up from: a link made there leads it off. + // path Seatbelt compares a write through them with. Denied whole: a directory + // the walk could not look in, and a missing name the way goes up from. const denyEntries = walked.flatMap(({ entry, end, unsearched, passed }) => [ - entry, - ...(end === undefined ? [] : startingFrom(end, entry)), + ...inBothSpellings('deny', entry, end), ...(unsearched === undefined ? [] : [unsearched]).map(toLiteralPathEntry), ...passed.map(toLiteralPathEntry), ]) @@ -1050,9 +1046,7 @@ function generateWriteRules( } // Each of those links stays where it lies, and each directory the way goes // up from: whoever replaces one decides what the denied name leads to. - for (const stays of new Set( - walked.flatMap(({ links, left }) => [...links.keys(), ...left]), - )) { + for (const stays of walked.flatMap(w => [...w.links.keys(), ...w.left])) { if (isAtOrUnder(stays, '/dev')) continue for (const kept of [stays, ...getAncestorDirectories(stays)]) { moveFilters.add(`(literal ${escapePath(kept)})`) diff --git a/src/sandbox/sandbox-config.ts b/src/sandbox/sandbox-config.ts index ac8a41926..2acbfadd0 100644 --- a/src/sandbox/sandbox-config.ts +++ b/src/sandbox/sandbox-config.ts @@ -1137,12 +1137,10 @@ export const SeccompConfigSchema = z.object({ .boolean() .optional() .describe( - 'Whether the helper takes --hold-link, by which the symbolic links on ' + - 'the way to denied paths are kept in place. Taken at its word: ' + - 'nothing is run to find out. Not set, the file of a helper the ' + - 'library found for itself is read for the name of --holds-links, and ' + - 'one named by applyPath is handed no link. false also serves to turn ' + - 'holding off.', + 'Whether the helper takes --hold-link, which keeps the symbolic links ' + + 'on the way to denied paths in place. Taken at its word. Not set, a ' + + 'helper the library found for itself is read for the option, and ' + + 'one named by applyPath is handed no link. false turns holding off.', ), }) diff --git a/src/sandbox/sandbox-manager.ts b/src/sandbox/sandbox-manager.ts index c0aaab0f9..43a3f97f1 100644 --- a/src/sandbox/sandbox-manager.ts +++ b/src/sandbox/sandbox-manager.ts @@ -3520,9 +3520,8 @@ export interface ISandboxManager { getLinuxGlobPatternWarnings(): string[] /** * Linux: the symbolic links on the way to a denied path that a command - * could replace, each with the reason nothing holds it, for the embedder to - * show or to look at again after the command. Each link is listed by the - * latest wrap that met it; `reset()` and `updateConfig()` empty the list. + * could replace, each with the reason nothing holds it. Each is listed by + * the latest wrap that met it; `reset()` and `updateConfig()` empty the list. */ getLinuxUnheldLinks(): LinuxUnheldLink[] getConfig(): SandboxRuntimeConfig | undefined diff --git a/src/sandbox/sandbox-schemas.ts b/src/sandbox/sandbox-schemas.ts index 631206cd9..52340805c 100644 --- a/src/sandbox/sandbox-schemas.ts +++ b/src/sandbox/sandbox-schemas.ts @@ -46,9 +46,8 @@ export interface FsReadRestrictionConfig { */ unlistableDenyDirs?: string[] /** - * The names a glob expansion matched that are symbolic links. `denyOnly` - * holds what each leads to, which is where its mount goes; the next - * expansion finds that by the name again. Linux only, likewise. + * The names a glob expansion matched that are symbolic links: `denyOnly` + * holds what each leads to. Linux only, likewise. */ matchedLinks?: string[] } diff --git a/src/sandbox/sandbox-utils.ts b/src/sandbox/sandbox-utils.ts index 281a3c265..349043e55 100644 --- a/src/sandbox/sandbox-utils.ts +++ b/src/sandbox/sandbox-utils.ts @@ -117,12 +117,8 @@ export function nameLocation( } /** - * What has to stay as it is for the absolute path `p`, a denied name, to go on - * leading where it leads, and where that is. - * - * A `.`, `..` or `//` in `p` itself is folded in text before anything is - * looked at, as everywhere a configured name is read: `link/..` is the - * directory that holds the link, which is not where the kernel goes. + * What has to stay as it is for `p`, a denied name, to go on leading where it + * leads, and where that is. `p` is folded in text first, as by `realPathOf`. * * INVARIANT: "could not look" is never "no link". A walk that stops for any * reason but a name that is not there, or that nothing can be at, has no end, @@ -138,58 +134,43 @@ export function followLinks(p: string): { left: Set /** The names that are not there which one goes up from in the same way. */ passed: string[] - /** Where the way ends, what is not there taken as written: a dangling link - * ends at the path a write through it would create. A way that goes up - * again further on among such names ends at the first: with nothing made - * there, nothing beyond comes about. None past the number of links the - * kernel follows, nor past a target that is no text. */ + /** Where the way ends, what is not there taken as written: the path a write + * through a dangling link would create. A way that goes up again further on + * among such names ends at the first. None where the walk stopped. */ end?: string unsearched?: string } { const links = new Map() const left = new Set() - const walk = (): ReturnType => - wayPastMissingNames(path.resolve(p), { + const walk = (): ReturnType => { + const { real, rest, shut, passed } = wayPastMissingNames(path.resolve(p), { link: (at, target) => links.set(at, target), left: dir => left.add(dir), }) - let way: ReturnType + if (shut) return { links, left, passed, unsearched: real || undefined } + const names = rest.includes('..') ? rest.slice(0, 1) : rest + return { links, left, passed, end: [real, ...names].join('/') || '/' } + } try { - way = walk() + return walk() } catch { // Once more: a look can fail in passing (EIO, ESTALE), and what would // follow from that is a directory denied whole. - try { - way = walk() - } catch (err) { - const { code, path: at } = err as NodeJS.ErrnoException - // A link all the same, which says what no string spells. - if (code === 'EILSEQ' && at) links.set(at, '') - const nothingCanBeThere = - isAbsenceErrno(err) || - code === 'EILSEQ' || - code === 'ERR_INVALID_ARG_VALUE' - const dir = nothingCanBeThere ? '/' : path.dirname(at ?? path.resolve(p)) - return { - links, - left, - passed: [], - ...(dir !== '/' && { unsearched: dir }), - } - } } - const { real, rest, notThere, passed } = way - return (notThere as NodeJS.ErrnoException | undefined)?.code === 'EACCES' - ? { links, left, passed, ...(real !== '' && { unsearched: real }) } - : { - links, - left, - passed, - end: - [real, ...(rest.includes('..') ? rest.slice(0, 1) : rest)].join( - '/', - ) || '/', - } + try { + return walk() + } catch (err) { + const { code, path: at } = err as NodeJS.ErrnoException + // A link all the same, which says what no string spells. + if (code === 'EILSEQ' && at) links.set(at, '') + const nothingCanBeThere = + isAbsenceErrno(err) || + code === 'EILSEQ' || + code === 'ERR_INVALID_ARG_VALUE' + const dir = path.dirname(at ?? path.resolve(p)) + const unsearched = nothingCanBeThere || dir === '/' ? undefined : dir + return { links, left, passed: [], unsearched } + } } /** `process.cwd()`, or undefined where that throws: it has been removed. */ diff --git a/src/utils/real-path.ts b/src/utils/real-path.ts index feadc713e..d49b04c17 100644 --- a/src/utils/real-path.ts +++ b/src/utils/real-path.ts @@ -127,30 +127,25 @@ export function wayTo( return { real, rest } } -/** As many missing names as one way is followed up from again. */ -const MAX_MISSING_NAMES_PASSED = 40 - /** * {@link wayTo}, followed on where it goes up again at once from a name that * is not there (`missing/../x`): a directory made at that name lets the way * pass, and a link made there leads it elsewhere. `passed` holds each such - * name, and `rest` no `.` or empty name. ELOOP past the number of them. + * name, `rest` no `.` or empty name, and `shut` whether `real` lets nobody + * look (EACCES). ELOOP past as many of them as links. */ export function wayPastMissingNames( folded: string, watcher?: WayWatcher, -): { real: string; rest: string[]; notThere?: unknown; passed: string[] } { +): { real: string; rest: string[]; shut: boolean; passed: string[] } { const passed: string[] = [] let onward = folded - while (passed.length <= MAX_MISSING_NAMES_PASSED) { + while (passed.length <= MAX_LINKS) { const { real, rest, notThere } = wayTo(onward, watcher) const names = rest.filter(name => name !== '' && name !== '.') - if ( - names[1] !== '..' || + const shut = (notThere as NodeJS.ErrnoException | undefined)?.code === 'EACCES' - ) { - return { real, rest: names, notThere, passed } - } + if (shut || names[1] !== '..') return { real, rest: names, shut, passed } passed.push(`${real}/${names[0]}`) onward = [real, ...names.slice(2)].join('/') } From 08d11323b2649927b1b3e34af9a25936e700b452 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 20 Oct 2026 12:56:07 +0000 Subject: [PATCH 7/8] test: the links on the way to a deny in 671 lines, where there were 1,187 One table of ways (name, links, end, directories left, names passed) drives the walk, the Seatbelt profile and the real attempts, every link of the fixture is on the way to a deny, so that removing, moving and replacing are tried on all thirteen, and one table holds the reasons a link is not held. Of 136 mutations of the source 117 were caught before and 126 are now; none that was caught is lost. Four cases proved nothing and are replaced, and the read of a chain of 40 links, which the kernel itself refused now and then under load, is gone. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01FydxqUESqxwSCVFnhk3gfv Co-authored-by: ronleizrowice --- test/sandbox/write-deny-through-links.test.ts | 1432 ++++++----------- 1 file changed, 458 insertions(+), 974 deletions(-) diff --git a/test/sandbox/write-deny-through-links.test.ts b/test/sandbox/write-deny-through-links.test.ts index 11bac921a..02e1dd2e5 100644 --- a/test/sandbox/write-deny-through-links.test.ts +++ b/test/sandbox/write-deny-through-links.test.ts @@ -8,35 +8,14 @@ import { spyOn, } from 'bun:test' import * as fs from 'fs' -import { spawnSync } from 'node:child_process' -import { - chmodSync, - existsSync, - lstatSync, - mkdirSync, - mkdtempSync, - readFileSync, - readlinkSync, - realpathSync, - rmSync, - symlinkSync, - writeFileSync, -} from 'node:fs' +import { spawnSync, type SpawnSyncReturns } from 'node:child_process' import { homedir, tmpdir } from 'node:os' import { join, relative } from 'node:path' -import type { LinuxUnheldLinkReason } from '../../src/index.js' +import type { LinuxUnheldLink } from '../../src/index.js' import * as seccomp from '../../src/sandbox/generate-seccomp-filter.js' -import { - cleanupBwrapMountPoints, - forgetLinuxUnheldLinks, - getLinuxUnheldLinks, - wrapCommandWithSandboxLinux, -} from '../../src/sandbox/linux-sandbox-utils.js' +import { wrapCommandWithSandboxLinux } from '../../src/sandbox/linux-sandbox-utils.js' import { wrapCommandWithSandboxMacOS } from '../../src/sandbox/macos-sandbox-utils.js' -import { - SandboxRuntimeConfigSchema, - type SandboxRuntimeConfig, -} from '../../src/sandbox/sandbox-config.js' +import { SandboxRuntimeConfigSchema } from '../../src/sandbox/sandbox-config.js' import { SandboxManager } from '../../src/sandbox/sandbox-manager.js' import { followLinks } from '../../src/sandbox/sandbox-utils.js' import { bwrapCanNamespace } from '../helpers/bwrap-namespace.js' @@ -51,478 +30,294 @@ import { isLinux, isMacOS, isWindows } from '../helpers/platform.js' * Every link here is made on the host before the command is wrapped. */ -let BASE: string -let ROOT: string // the one write root -let OUTSIDE: string // beside it, under no write root +const CWD = process.cwd() +// Where the temp dir really is: macOS names it through a link. +const BASE = join(fs.realpathSync(tmpdir()), `write-deny-links-${process.pid}`) +const ROOT = join(BASE, 'root') // the one write root +const OUTSIDE = join(BASE, 'outside') // beside it, under no write root +const HOLDER = join(ROOT, 'holder') +const inRoot = (names: string[]): string[] => names.map(n => join(ROOT, n)) -/** Link name in ROOT -> what it says. */ +/** Link name in ROOT -> what it says. A deny leads through every one. */ const LINKS = { name: 'conf/x', 'holder/commands': '../shared/commands', dangling: 'later', way: 'real', + here: '.', first: 'middle', middle: 'conf/y', - blank: '/dev/null', + up: 'in/to/../../real/secret', + past: 'lost/../gone/../name', + deep: 'void/deeper/../../real/secret', + far: 'holder/blank', + 'holder/blank': '/dev/null', out: '/dev/stdout', - 'other-link': 'conf/z', } -const FILES = [ - 'conf/x', - 'conf/y', - 'conf/z', - 'shared/commands/file', - 'real/secret', - 'beside', -] -const ABSENT = ['later', 'real/absent', 'shared/commands/new'] -const DENIED = [ - 'name', - 'holder/commands', - 'dangling', - 'way/secret', - 'way/absent', - 'first', - 'blank', - 'out', +type Link = keyof typeof LINKS +const said = (names: Link[]): string[][] => names.map(n => [n, LINKS[n]]) +/** + * A name in ROOT, the links on the way to it and where they lead; then the + * directories the way goes up from, and the names that are not there and that + * it goes up from all the same. + */ +const WAYS: Array<[string, Link[], string, string[], string[]]> = [ + ['name', ['name'], 'conf/x', [], []], + ['holder/commands', ['holder/commands'], 'shared/commands', ['holder'], []], + ['dangling', ['dangling'], 'later', [], []], + ['way/secret', ['way'], 'real/secret', [], []], + ['way/absent', ['way'], 'real/absent', [], []], + // A link is named where it lies, whatever it was reached through. + ['here/name', ['here', 'name'], 'conf/x', [], []], + ['first', ['first', 'middle'], 'conf/y', [], []], + ['up', ['up'], 'real/secret', ['in/to', 'in'], []], + ['past', ['past', 'name'], 'conf/x', [], ['lost', 'gone']], + // Further on among names that are not there: it ends at the first. + ['deep', ['deep'], 'void', [], []], ] +const DENIED = inRoot([...WAYS.map(([name]) => name), 'far', 'out']) +const FILES = ['conf/x', 'conf/y', 'conf/z', 'real/secret', 'beside'] +const ABSENT = ['later', 'real/absent', 'shared/commands/new', 'lost', 'gone'] /** EBUSY, as glibc words it ("Device or resource busy") or musl. */ const BUSY = /resource busy/i /** No mode keeps uid 0 from looking. */ const modesCount = process.getuid?.() !== 0 +const namespaces = isLinux && bwrapCanNamespace() beforeEach(() => { if (isWindows) return - // Where the temp dir really is: macOS names it through a link. - BASE = realpathSync(mkdtempSync(join(tmpdir(), 'write-deny-links-'))) - ROOT = join(BASE, 'root') - OUTSIDE = join(BASE, 'outside') - for (const dir of ['conf', 'shared/commands', 'real', 'holder']) { - mkdirSync(join(ROOT, dir), { recursive: true }) + for (const dir of ['conf', 'shared/commands', 'real', 'holder', 'in/to']) { + fs.mkdirSync(join(ROOT, dir), { recursive: true }) } - mkdirSync(OUTSIDE) - for (const file of FILES) writeFileSync(join(ROOT, file), 'original') + fs.mkdirSync(OUTSIDE) + for (const file of FILES) fs.writeFileSync(join(ROOT, file), 'original') for (const [link, says] of Object.entries(LINKS)) { - symlinkSync(says, join(ROOT, link)) + fs.symlinkSync(says, join(ROOT, link)) } }) -afterEach(() => { +afterEach(async () => { if (isWindows) return - if (isLinux) cleanupBwrapMountPoints({ force: true }) - chmodSync(join(ROOT, 'holder'), 0o755) - rmSync(BASE, { recursive: true, force: true }) + mock.restore() + process.chdir(CWD) + // Removes what bubblewrap made to mount on, and empties the list of links. + await SandboxManager.reset() + fs.chmodSync(HOLDER, 0o755) + fs.rmSync(BASE, { recursive: true, force: true }) }) -const inRoot = (names: string[]): string[] => names.map(n => join(ROOT, n)) +/** `command`, run by a shell: unless told otherwise under no write root, so + * that nothing of the working directory's is protected or made. */ +const run = (command: string, cwd = OUTSIDE): SpawnSyncReturns => + spawnSync(command, { shell: true, encoding: 'utf8', cwd, timeout: 30000 }) -/** `run` with ROOT for the working directory of this process. */ -async function fromRoot(run: () => T | Promise): Promise { - const cwd = process.cwd() - process.chdir(ROOT) - try { - return await run() - } finally { - process.chdir(cwd) - } -} +type Options = Parameters[0] & + Parameters[0] +type Rest = Partial -/** An executable file in OUTSIDE that holds `script`. */ -function program(name: string, script: string): string { - const file = join(OUTSIDE, name) - writeFileSync(file, `#!/bin/sh\n${script}\n`) - chmodSync(file, 0o755) - return file +/** `denyWithinAllow` denied in ROOT, the write root beside two devices. */ +function denying(denyWithinAllow: string[], rest?: Rest): Options { + const allowOnly = [ROOT, '/dev/null', '/dev/stdout'] + return { + command: 'echo started', + needsNetworkRestriction: false, + readConfig: { denyOnly: [] }, + writeConfig: { allowOnly, denyWithinAllow }, + ...rest, + } } describe.if(!isWindows)('the links on the way to a path', () => { /** What is said of `p`, a name in ROOT, with the names in ROOT again. */ const followed = (p: string): object => { - const { links, left, ...rest } = followLinks(join(ROOT, p)) - const named = (at: string): string => relative(ROOT, at) - return JSON.parse( - JSON.stringify( - { links: [...links], left: [...left], ...rest }, - (_key, value: unknown) => - typeof value === 'string' && value.startsWith(ROOT) - ? named(value) - : value, - ), - ) as object + const { links, left, ...rest } = followLinks(`${ROOT}/${p}`) + const told = JSON.stringify({ links: [...links], left: [...left], ...rest }) + return JSON.parse(told.split(`${ROOT}/`).join('')) as object } - const said = (...names: Array): string[][] => - names.map(name => [name, LINKS[name]]) + const NONE = { left: [], passed: [] } - it.each<[string, Array, string]>([ - ['beside', [], 'beside'], - ['name', ['name'], 'conf/x'], - ['holder/commands/file', ['holder/commands'], 'shared/commands/file'], - ['dangling', ['dangling'], 'later'], - ['way/absent/deeper', ['way'], 'real/absent/deeper'], - ['first', ['first', 'middle'], 'conf/y'], - ['absent/name', [], 'absent/name'], - ['beside/name', [], 'beside/name'], - ['./way/./secret/', ['way'], 'real/secret'], - // Folded in text, as the path it is given always is. - ['holder/commands/../name', [], 'holder/name'], - ])('%s: %p, ending at %s', (p, links, end) => { - expect(followed(p)).toMatchObject({ - links: said(...links), - passed: [], - end, - }) + it.each<(typeof WAYS)[number]>([ + ...WAYS, + ['beside', [], 'beside', [], []], + ['absent/name', [], 'absent/name', [], []], + ['beside/name', [], 'beside/name', [], []], + ['way/absent/deeper', ['way'], 'real/absent/deeper', [], []], + // Folded in text, as everywhere a configured name is read. + ['./way/./secret/', ['way'], 'real/secret', [], []], + ['holder/commands/../name', [], 'holder/name', [], []], + ])('%s: %p, ending at %s', (p, links, end, left, passed) => { + expect(followed(p)).toEqual({ links: said(links), left, passed, end }) }) it.each([ - [ - 'a directory', - 'shared/commands/../../real/secret', - { left: ['shared/commands', 'shared'], passed: [], end: 'real/secret' }, - ], - [ - 'a link to one, whose parent it is the parent of', - 'holder/commands/../made', - { - links: [...said('holder/commands')], - left: ['holder', 'shared/commands'], - passed: [], - end: 'shared/made', - }, - ], - [ - 'a name that is not there', - 'missing/../gone/../real/secret', - { left: [], passed: ['missing', 'gone'], end: 'real/secret' }, - ], - [ - 'one further on among names that are not there, where it ends at the first', - 'missing/deeper/../../real/secret', - { left: [], passed: [], end: 'missing' }, - ], - ])('a target that goes up again from %s: %s', (_what, target, told) => { - symlinkSync(target, join(ROOT, 'up')) - expect(followed('up')).toEqual({ - ...told, - links: [['up', target], ...('links' in told ? told.links : [])], - }) - }) - - it('takes a relative path from the working directory', async () => { - expect(await fromRoot(() => followLinks('name').end)).toBe( - join(ROOT, 'conf', 'x'), - ) - }) - - it('names a link where it lies, whatever it was reached through', () => { - symlinkSync(ROOT, join(OUTSIDE, 'to-root')) - expect([...followLinks(join(OUTSIDE, 'to-root', 'name')).links]).toEqual([ - [join(OUTSIDE, 'to-root'), ROOT], - [join(ROOT, 'name'), LINKS.name], - ]) - }) - - it('reads a target that holds U+FFFD itself like any other', () => { - symlinkSync('x�y', join(ROOT, 'odd')) - expect(followed('odd/file')).toMatchObject({ - links: [['odd', 'x�y']], - end: 'x�y/file', - }) + ['longer than a name can be', 'a'.repeat(300)], + ['with a NUL byte', 'nul\0byte'], + ])('a name %s: nothing can be there, so it has looked', (_what, name) => { + expect(followed(`way/${name}`)).toEqual({ links: said(['way']), ...NONE }) }) // Not every file system takes such a target. it.if(isLinux)('has no end past a target that is no text', () => { - symlinkSync(Buffer.from([0x78, 0xff]), join(ROOT, 'bytes')) - expect(followed('bytes/file')).toEqual({ - links: [['bytes', '']], - left: [], - passed: [], - }) + fs.symlinkSync(Buffer.from([0x78, 0xff]), join(ROOT, 'raw')) + expect(followed('raw/file')).toEqual({ links: [['raw', '']], ...NONE }) }) - it('follows as many links as the Linux kernel, and has no end past them', () => { - symlinkSync('beside', join(ROOT, 'hop-1')) - for (let i = 2; i <= 41; i++) { - symlinkSync(`hop-${i - 1}`, join(ROOT, `hop-${i}`)) + it('follows 40 links, and has no end past them, where the kernel gives up too', () => { + for (let i = 1; i <= 41; i++) { + fs.symlinkSync(i > 1 ? `hop-${i - 1}` : 'beside', join(ROOT, `hop-${i}`)) } expect(followed('hop-40')).toMatchObject({ end: 'beside' }) - expect(followed('hop-41')).not.toHaveProperty('end') - expect(followed('hop-41')).not.toHaveProperty('unsearched') - // macOS gives up sooner, at 32: what is denied past that is denied in vain. - if (isLinux) { - expect(() => readFileSync(join(ROOT, 'hop-40'))).not.toThrow() - } - expect(() => readFileSync(join(ROOT, 'hop-41'))).toThrow('ELOOP') + expect(Object.keys(followed('hop-41'))).toEqual(['links', 'left', 'passed']) + expect(() => fs.readFileSync(join(ROOT, 'hop-41'))).toThrow('ELOOP') }) - it.each([ - ['longer than a name can be', 'a'.repeat(300)], - ['with a NUL byte', 'nul\0byte'], - ])('a name %s: nothing can be there, so it has looked', (_what, name) => { - expect(followed(`way/${name}`)).toEqual({ - links: said('way'), - left: [], - passed: [], - }) + it.if(modesCount)('says which directory it could not look in', () => { + fs.chmodSync(HOLDER, 0) + fs.symlinkSync('holder/commands/../made', join(ROOT, 'via')) + expect(followed('via')).toMatchObject({ ...NONE, unsearched: 'holder' }) }) - it.if(modesCount)( - 'stops at a directory it cannot look in, and says which', - () => { - chmodSync(join(ROOT, 'holder'), 0) - symlinkSync('holder/commands', join(ROOT, 'to-holder')) - expect(followed('to-holder/file')).toEqual({ - links: [['to-holder', 'holder/commands']], - left: [], - passed: [], - unsearched: 'holder', - }) - }, - ) - - describe('a look that fails', () => { - afterEach(() => mock.restore()) - const failing = (code: string, path?: string): Error => - Object.assign(new Error(code), { code, path }) + const failing = (at?: fs.PathLike): never => { + throw Object.assign(new Error('EIO'), { code: 'EIO', path: at }) + } - it.each(['EIO', 'ESTALE', 'EINVAL'])( - 'with %s both times: the directory of what it looked at', - code => { - spyOn(fs, 'readlinkSync').mockImplementation(at => { - throw failing(code, String(at)) - }) - expect(followed('holder/commands/file')).toMatchObject({ - unsearched: 'holder', - }) - }, - ) + it('looks again where a look fails', () => { + spyOn(fs, 'readlinkSync').mockImplementationOnce(failing) + expect(followed('name')).toMatchObject({ end: 'conf/x' }) + }) - it('once: it looks again', () => { - spyOn(fs, 'readlinkSync').mockImplementationOnce(at => { - throw failing('EIO', String(at)) - }) - expect(followed('name')).toMatchObject({ end: 'conf/x' }) - }) + it('where that fails too: the directory of what it looked at', () => { + spyOn(fs, 'readlinkSync').mockImplementation(failing) + expect(followed('way/secret')).toMatchObject({ unsearched: ROOT }) + }) - it('and does not say where: the directory of the path as written', () => { - spyOn(fs, 'readlinkSync').mockImplementation(() => { - throw failing('EIO') - }) - expect(followed('way/secret')).toMatchObject({ unsearched: 'way' }) - }) + it('and does not say where: the directory of the path as written', () => { + spyOn(fs, 'readlinkSync').mockImplementation(() => failing()) + expect(followed('way/secret')).toMatchObject({ unsearched: 'way' }) + }) - it('in the root: no directory, which would stand in for everything', () => { - spyOn(fs, 'lstatSync').mockImplementation(at => { - throw failing('EIO', String(at)) - }) - expect(followLinks(ROOT)).toEqual({ - links: new Map(), - left: new Set(), - passed: [], - }) - }) + it('in the root: no directory, which would stand in for everything', () => { + spyOn(fs, 'lstatSync').mockImplementation(failing) + expect(followed('name')).toEqual({ links: [], ...NONE }) }) }) -describe.if((isLinux && bwrapCanNamespace()) || isMacOS)( - 'A write deny on a path with a link on the way', - () => { - /** What `attempt` came to, run in ROOT with every one of `denied` denied. */ - async function outcomeOf( - attempt: string, - denied = DENIED, - ): Promise { - const options = { - command: `cd ${ROOT} && echo started && if ( ${attempt} ); then echo DONE; else echo refused; fi`, - needsNetworkRestriction: false, - // Linux mounts a read deny where it resolves, under the /dev it then - // makes anew, and bubblewrap before 0.5.0 takes no device to mount on. - readConfig: { denyOnly: isMacOS ? inRoot(['blank']) : [] }, - writeConfig: { - allowOnly: [ROOT, '/dev/null', '/dev/stdout'], - denyWithinAllow: inRoot(denied), - }, - } - const wrapped = isLinux - ? await wrapCommandWithSandboxLinux(options) - : wrapCommandWithSandboxMacOS(options) - const result = spawnSync(wrapped, { - shell: true, - encoding: 'utf8', - timeout: 30000, - // Under no write root, so nothing of the working directory's is - // protected or made. - cwd: OUTSIDE, - }) - const said = `${result.stdout}${result.stderr}` - expect(said).toContain('started') - if (isLinux) cleanupBwrapMountPoints({ force: true }) - return said.includes('DONE') ? 'DONE' : 'refused' - } - - it.each([ - ['a write through the link', 'echo changed >> name'], - [ - 'a write to what it leads to, by its own path', - 'echo changed >> conf/x', - ], - ['removing the link', 'rm name'], - ['renaming a file over the link', 'echo new > made && mv -f made name'], - ['renaming the link away', 'mv name aside'], - ['making the link anew', 'ln -sfn beside name'], - ['removing what it leads to', 'rm conf/x'], - ['renaming the directory of what it leads to', 'mv conf aside'], - [ - 'a create beneath a link to a directory', - 'echo n > holder/commands/new', - ], - ['the same by its own path', 'echo new > shared/commands/new'], - ['removing a link to a directory', 'rm holder/commands'], - ['renaming the directory that holds the link', 'mv holder aside'], - ['a create through a dangling link', 'echo new > dangling'], - ['a create of what it leads to, by its own path', 'echo new > later'], - ['removing a dangling link', 'rm dangling'], - ['a write beneath a link on the way', 'echo changed >> way/secret'], - ['the same by its own path', 'echo changed >> real/secret'], - ['a create beneath a link on the way', 'echo new > way/absent'], - ['the same by its own path', 'echo new > real/absent'], - ['removing a link on the way', 'rm way'], - ['a write through two links', 'echo changed >> first'], - ['removing the second of two links', 'rm middle'], - ['renaming a file over it', 'echo new > made && mv -f made middle'], - ['removing a link to a device', 'rm blank'], - ['making it anew', 'ln -sfn beside out'], - ])('refuses %s: %s', async (_what, attempt) => { - expect(await outcomeOf(attempt)).toBe('refused') - - for (const [link, says] of Object.entries(LINKS)) { - expect(lstatSync(join(ROOT, link)).isSymbolicLink()).toBe(true) - expect(readlinkSync(join(ROOT, link))).toBe(says) - } - for (const file of FILES) { - expect(readFileSync(join(ROOT, file), 'utf8')).toBe('original') - } - for (const name of ABSENT) - expect(existsSync(join(ROOT, name))).toBe(false) - }) - - it.each([ - ['a write to a file beside the link', 'echo changed >> beside'], - ['removing it', 'rm beside'], - ['a create beside the link', 'echo new > new && echo new > holder/new'], - ['a write beside what the link leads to', 'echo changed >> conf/z'], - ['replacing a link no deny leads through', 'ln -sfn beside other-link'], - [ - 'reading the link, and through it', - 'test "$(readlink name)$(cat name)$(ls holder/commands)" = conf/xoriginalfile', - ], - ['a write to a device a denied name leads to', 'echo x > /dev/null'], - ['a read of it', 'cat /dev/null'], - ])('allows %s: %s', async (_what, attempt) => { - expect(await outcomeOf(attempt)).toBe('DONE') +describe.if(namespaces || isMacOS)('a deny with a link on the way', () => { + /** What `attempt` came to, run in ROOT with every one of `denied` denied. */ + async function outcomeOf(attempt: string, denied = DENIED): Promise { + const options = denying(denied, { + command: `cd ${ROOT} && if ( ${attempt} ); then echo DONE; else echo refused; fi`, + // Linux mounts a read deny where it resolves, under the /dev it then + // makes anew, and bubblewrap before 0.5.0 takes no device to mount on. + readConfig: { denyOnly: isMacOS ? inRoot(['far']) : [] }, }) - - it.each([ - ['refused', 'rm .bashrc'], - ['refused', 'echo changed >> conf/x'], - ['refused', 'rm .claude/commands'], - ['refused', 'echo new > shared/commands/new'], - ['refused', 'rm .zshrc'], - ['DONE', 'echo x > /dev/null'], - ])( - 'the same of the names denied in every working directory: %s: %s', - async (outcome, attempt) => { - mkdirSync(join(ROOT, '.claude')) - symlinkSync('conf/x', join(ROOT, '.bashrc')) - symlinkSync('/dev/null', join(ROOT, '.zshrc')) - symlinkSync('../shared/commands', join(ROOT, '.claude', 'commands')) - expect(await fromRoot(() => outcomeOf(attempt, []))).toBe(outcome) - }, + const { stdout } = run( + isLinux + ? await wrapCommandWithSandboxLinux(options) + : wrapCommandWithSandboxMacOS(options), ) + SandboxManager.cleanupAfterCommand() + return stdout.trim() + } - it.each([ - [ - 'a directory', - 'shared/commands/../../real/secret', - 'mv shared/commands aside', - ], - ['a directory', 'shared/commands/../../real/secret', 'mv shared aside'], - ['a name that is not there', 'missing/../real/secret', 'mkdir missing'], - [ - 'a name that is not there', - 'missing/../real/secret', - 'ln -s conf missing', - ], - ])( - 'where a link goes up again from %s (%s), refuses: %s', - async (_what, target, attempt) => { - symlinkSync(target, join(ROOT, 'up')) - expect(await outcomeOf(attempt, ['up'])).toBe('refused') - expect(await outcomeOf('echo changed >> up', ['up'])).toBe('refused') - }, - ) + it.each([ + ...Object.keys(LINKS).flatMap(link => [ + `rm ${link}`, + `mv ${link} aside`, + `ln -sfn beside ${link}`, + ]), + ...['name', 'dangling', 'middle'].map( + link => `echo new > made && mv -f made ${link}`, + ), + // A write or a create through the links, and by the path they lead to. + ...[...new Set(WAYS.flatMap(([name, , end]) => [name, end]))].map( + at => `echo changed >> ${at} || echo new > ${at}/new`, + ), + 'rm conf/x', + // The directories above a link and above what it leads to, those the way + // goes up from, and the names that are not there. + 'mv holder aside', + 'mv conf aside', + 'mv in/to aside', + 'mv in aside', + 'mkdir lost', + 'ln -s conf gone', + ])('refuses: %s', async attempt => { + expect(await outcomeOf(attempt)).toBe('refused') + + for (const [link, says] of Object.entries(LINKS)) { + expect(fs.readlinkSync(join(ROOT, link))).toBe(says) + } + for (const file of FILES) { + expect(fs.readFileSync(join(ROOT, file), 'utf8')).toBe('original') + } + expect(inRoot(ABSENT).filter(fs.existsSync)).toEqual([]) + }) - it.if(modesCount)( - 'keeps a directory that cannot be looked in as it is, mode and all', - async () => { - expect(await outcomeOf('chmod 000 holder')).toBe('DONE') - expect( - await outcomeOf( - 'chmod 755 holder; rm holder/commands || rm -r holder', - ), - ).toBe('refused') + it.each([ + 'echo changed >> beside && rm beside', + 'echo new > new && echo new > holder/new', + 'echo changed >> conf/z', + 'ln -s conf/z free && ln -sfn beside free', + 'test "$(readlink name)$(cat name way/secret)" = conf/xoriginaloriginal', + 'echo x > /dev/null && cat /dev/null', + ])('allows: %s', async attempt => { + expect(await outcomeOf(attempt)).toBe('DONE') + }) + + it.each(['rm .bashrc', 'echo changed >> conf/z', 'rm .claude/commands'])( + 'the same of the names denied in every working directory: %s', + async attempt => { + fs.mkdirSync(join(ROOT, '.claude')) + fs.symlinkSync('conf/z', join(ROOT, '.bashrc')) + fs.symlinkSync('../shared/commands', join(ROOT, '.claude', 'commands')) + process.chdir(ROOT) + expect(await outcomeOf(attempt, [])).toBe('refused') + }, + ) - expect(lstatSync(join(ROOT, 'holder')).mode & 0o777).toBe(0) - chmodSync(join(ROOT, 'holder'), 0o755) - expect(readlinkSync(join(ROOT, 'holder', 'commands'))).toBe( - LINKS['holder/commands'], - ) - }, - ) - }, -) + it.if(modesCount)('a directory it cannot look in is kept whole', async () => { + expect(await outcomeOf('chmod 000 holder')).toBe('DONE') + const undoing = 'chmod 755 holder; rm holder/commands || rm -r holder' + expect(await outcomeOf(undoing)).toBe('refused') + expect(fs.lstatSync(HOLDER).mode & 0o777).toBe(0) + fs.chmodSync(HOLDER, 0o755) + expect(fs.readdirSync(HOLDER).sort()).toEqual(['blank', 'commands']) + }) +}) // Profile text only, so it runs on every POSIX host. describe.if(!isWindows)('Seatbelt profile', () => { - /** The filters of the rule that denies `operations`, one to a line. */ - const deniedBy = (operations: string, denyWithinAllow: string[]): string => { - const profile = wrapCommandWithSandboxMacOS({ - command: 'true', - needsNetworkRestriction: false, - readConfig: undefined, - writeConfig: { allowOnly: [ROOT], denyWithinAllow }, - }) - const at = profile.indexOf(`(deny ${operations}\n`) + /** The filters of the rule that denies `ops`, one to a line. */ + function deniedBy(ops: string, write: string[], read: string[] = []): string { + const profile = wrapCommandWithSandboxMacOS( + denying(write, { readConfig: { denyOnly: read } }), + ) + const at = profile.indexOf(`(deny ${ops}\n`) expect(at).toBeGreaterThan(-1) return profile.slice(at, profile.indexOf('(with message', at)) } const KEPT = 'file-write-unlink file-write-create' - const subpath = (name: string): string => - ` (subpath "${join(ROOT, name)}")\n` - const literal = (name: string): string => - ` (literal "${join(ROOT, name)}")\n` + const subpath = (name: string): string => `(subpath "${join(ROOT, name)}")\n` + const literal = (name: string): string => `(literal "${join(ROOT, name)}")\n` - it.each([ - ['name', 'conf/x'], - ['holder/commands', 'shared/commands'], - ['dangling', 'later'], - ['way/secret', 'real/secret'], - ['way/absent', 'real/absent'], - ['first', 'conf/y'], - ])('a deny on %s is one on %s too', (written, end) => { - const denies = deniedBy('file-write*', inRoot([written])) - expect(denies).toContain(subpath(written)) - expect(denies).toContain(subpath(end)) + it.each(WAYS)('%s keeps %p, denies %s', (name, links, end, left, passed) => { + const denies = deniedBy('file-write*', inRoot([name])) + for (const at of [name, end, ...passed]) { + expect(denies).toContain(subpath(at)) + } + const kept = deniedBy(KEPT, inRoot([name])) + for (const at of [...links, ...left]) expect(kept).toContain(literal(at)) }) - it('so is one on a name denied in every working directory', async () => { - symlinkSync('conf/x', join(ROOT, '.bashrc')) - expect(await fromRoot(() => deniedBy('file-write*', []))).toContain( - subpath('conf/x'), - ) + it('so is one on a name denied in every working directory', () => { + fs.symlinkSync('conf/x', join(ROOT, '.bashrc')) + process.chdir(ROOT) + expect(deniedBy('file-write*', [])).toContain(subpath('conf/x')) }) it('a pattern deny starts from both spellings of its directory', () => { - const denies = deniedBy('file-write*', [join(ROOT, 'way', '*.key')]) + const denies = deniedBy('file-write*', inRoot(['way/*.key'])) const matches = (file: string): boolean => [...denies.matchAll(/^ {2}\(regex (".*")\)$/gm)].some(([, text]) => new RegExp(JSON.parse(text!) as string).test(join(ROOT, file)), @@ -532,656 +327,345 @@ describe.if(!isWindows)('Seatbelt profile', () => { expect(matches('conf/a.key')).toBe(false) }) - it('every link on the way, and what holds it, is kept in place', () => { - mkdirSync(join(ROOT, 'elsewhere')) - symlinkSync('../conf/x', join(ROOT, 'elsewhere', 'hop')) - symlinkSync('elsewhere/hop', join(ROOT, 'far')) - const kept = deniedBy(KEPT, inRoot(['first', 'far'])) - for (const name of ['middle', 'elsewhere/hop', 'elsewhere', 'conf']) { + it('nothing among the devices is denied or kept; the links to them are kept, and what holds one', () => { + fs.symlinkSync('/dev', join(ROOT, 'devices')) + const denied = inRoot(['far', 'out', 'devices']) + const kept = deniedBy(KEPT, denied) + expect(deniedBy('file-write*', denied) + kept).not.toContain('"/dev') + for (const name of ['far', 'holder/blank', 'holder', 'out', 'devices']) { expect(kept).toContain(literal(name)) } - expect(kept).not.toContain(join(ROOT, 'other-link')) - }) - - it('nothing among the devices is denied or kept, and the link to it is kept', () => { - symlinkSync('/dev', join(ROOT, 'devices')) - const denied = inRoot(['blank', 'out', 'devices']) - for (const operations of ['file-write*', KEPT]) { - expect(deniedBy(operations, denied)).not.toContain('"/dev') - } - for (const name of ['blank', 'out', 'devices']) { - expect(deniedBy(KEPT, denied)).toContain(literal(name)) - } }) it('nor is a device hidden by a read deny on a link to it', () => { - const profile = wrapCommandWithSandboxMacOS({ - command: 'true', - needsNetworkRestriction: false, - readConfig: { denyOnly: inRoot(['blank', 'name']) }, - writeConfig: undefined, - }) - const at = profile.indexOf('(deny file-read*\n') - const denies = profile.slice(at, profile.indexOf('(with message', at)) + const denies = deniedBy('file-read*', [], inRoot(['far', 'name'])) expect(denies).toContain(subpath('conf/x')) expect(denies).not.toContain('"/dev') }) - it.if(modesCount)( - 'a directory that cannot be looked in is denied whole', - () => { - chmodSync(join(ROOT, 'holder'), 0) - expect(deniedBy('file-write*', inRoot(['holder/commands']))).toContain( - subpath('holder'), - ) - expect(deniedBy('file-write*', inRoot(['name']))).not.toContain( - subpath('holder'), - ) - }, - ) + it.if(modesCount)('a directory it cannot look in is denied whole', () => { + fs.chmodSync(HOLDER, 0) + expect(deniedBy('file-write*', inRoot(['holder/commands']))).toContain( + subpath('holder'), + ) + }) }) -type LinuxOptions = Parameters[0] - -/** `denyWithinAllow` denied in ROOT, wrapped for Linux. */ -const wrapLinux = ( - denyWithinAllow: string[], - rest: Partial = {}, -): Promise => - wrapCommandWithSandboxLinux({ - command: 'echo started', - needsNetworkRestriction: false, - readConfig: { denyOnly: [] }, - writeConfig: { allowOnly: [ROOT], denyWithinAllow }, - ...rest, - }) +function wrapLinux(denied: string[], rest?: Rest): Promise { + return wrapCommandWithSandboxLinux(denying(denied, rest)) +} -const stdoutOf = (wrapped: string): string => - spawnSync(wrapped, { - shell: true, - encoding: 'utf8', - cwd: OUTSIDE, - timeout: 30000, - }).stdout +/** An executable file in OUTSIDE that holds `script`. */ +function program(name: string, script: string): string { + const file = join(OUTSIDE, name) + fs.writeFileSync(file, `#!/bin/sh\n${script}\n`, { mode: 0o755 }) + return file +} /** - * The words a wrap hands the seccomp helper before the command, as [link, - * target] pairs. Read by the shells themselves, through every level of + * The words a wrap hands the seccomp helper before the command, as [link in + * ROOT, target] pairs. Read by the shells themselves, through every level of * quoting, and with no namespace made: a stand-in for bubblewrap runs what - * follows `--`, and one for the helper prints its words and runs nothing. + * follows `--`, three words, and one for the helper only prints its words. */ -async function heldBy( - denyWithinAllow: string[], - rest: Partial = {}, -): Promise> { - const words = stdoutOf( - await wrapLinux(denyWithinAllow, { - bwrapPath: program( - 'bwrap-stand-in', - 'while [ "$1" != -- ]; do shift; done; shift; exec "$@"', - ), - seccompConfig: { - applyPath: program('helper-stand-in', `printf '%s\\0' "$@"`), - holdsLinks: true, - }, - ...rest, - }), - ).split('\0') - const held: Array<[string, string]> = [] +async function heldBy(denied: string[], rest?: Rest): Promise { + const bwrapPath = program('bwrap-stand-in', 'shift $(($# - 3)); exec "$@"') + const applyPath = program('helper-stand-in', `printf '%s\\0' "$@"`) + const seccompConfig = { applyPath, holdsLinks: true } + const wrapped = await wrapLinux(denied, { bwrapPath, seccompConfig, ...rest }) + const words = run(wrapped).stdout.split('\0') + const held: string[][] = [] for (let i = 0; words[i] === '--hold-link'; i += 3) { - held.push([words[i + 1]!, words[i + 2]!]) + held.push([relative(ROOT, words[i + 1]!), words[i + 2]!]) } expect(words.slice(3 * held.length + 1)).toEqual(['-c', 'echo started', '']) return held.sort() } -/** LINKS' names as {@link heldBy} gives them. */ -const pairs = (...names: Array): Array<[string, string]> => - names.sort().map(name => [join(ROOT, name), LINKS[name]]) describe.if(isLinux)('what the seccomp helper is asked to hold', () => { - it('each link once, where it lies, with what it says', async () => { - symlinkSync(ROOT, join(OUTSIDE, 'to-root')) - expect( - await heldBy([ - ...inRoot(['first', 'way/secret', 'way/absent']), - join(OUTSIDE, 'to-root', 'middle'), - ]), - ).toEqual(pairs('first', 'middle', 'way')) - }) - - it('whatever characters the name and the target hold', async () => { - const odd = [ - "sp ace'quote", - 'new\nline', - '$(echo x)`y`', - '--hold-link', - '!', - ] - for (const name of odd) symlinkSync(`to ${name}`, join(ROOT, name)) - expect(await heldBy(inRoot(odd))).toEqual( - odd.sort().map(name => [join(ROOT, name), `to ${name}`]), + it('each link once, with what it says; none for a path with no link on the way, or that lies where the command cannot write', async () => { + fs.symlinkSync(join(ROOT, 'conf', 'x'), join(OUTSIDE, 'in')) + const none = [...inRoot(['beside', 'absent']), join(OUTSIDE, 'in')] + expect(await heldBy([...DENIED, ...none])).toEqual( + Object.entries(LINKS).sort(), ) }) - it('with no word on a target that is no text', async () => { - symlinkSync(Buffer.from([0x78, 0xff]), join(ROOT, 'bytes')) - expect(await heldBy(inRoot(['bytes']))).toEqual([[join(ROOT, 'bytes'), '']]) - }) - - it('for a deny written from the home or the working directory', async () => { - const fromHome = `~/${relative(homedir(), join(ROOT, 'way', 'x'))}` - expect(await fromRoot(() => heldBy(['name', fromHome]))).toEqual( - pairs('name', 'way'), + it('whatever characters the name and the target hold, U+FFFD too, with no word on a target that is no text', async () => { + const odd = ["sp ace'quote", 'new\nline', '$(echo x)`y`', 'x�y'] + for (const name of odd) fs.symlinkSync(`to ${name}`, join(ROOT, name)) + fs.symlinkSync(Buffer.from([0x78, 0xff]), join(ROOT, 'bytes')) + expect(await heldBy(inRoot([...odd, 'bytes']))).toEqual( + [...odd.map(name => [name, `to ${name}`]), ['bytes', '']].sort(), ) }) - it('for a name denied in every working directory', async () => { - symlinkSync('conf/x', join(ROOT, '.bashrc')) - expect(await fromRoot(() => heldBy([]))).toEqual([ - [join(ROOT, '.bashrc'), 'conf/x'], - ]) - }) - - it('for a link that leads out of every write root, or to a device', async () => { - symlinkSync(OUTSIDE, join(ROOT, 'leads-out')) - expect(await heldBy(inRoot(['leads-out/file', 'blank']))).toEqual([ - ...pairs('blank'), - [join(ROOT, 'leads-out'), OUTSIDE], + it('for a deny written from the home or the working directory, and a name denied in every one', async () => { + fs.symlinkSync('conf/x', join(ROOT, '.bashrc')) + process.chdir(ROOT) + const fromHome = `~/${relative(homedir(), join(ROOT, 'way', 'x'))}` + expect(await heldBy(['name', fromHome])).toEqual([ + ['.bashrc', 'conf/x'], + ...said(['name', 'way']), ]) }) - it('under a write root that is the root', async () => { - expect( - await heldBy([], { - writeConfig: { allowOnly: ['/'], denyWithinAllow: inRoot(['name']) }, - }), - ).toEqual(pairs('name')) - }) - - it('none for a path with no link on the way', async () => { - expect(await heldBy(inRoot(['beside', 'conf/x', 'absent']))).toEqual([]) - }) - - it('none that lies where the command cannot write', async () => { - symlinkSync(join(ROOT, 'conf', 'x'), join(OUTSIDE, 'into-root')) - expect(await heldBy([join(OUTSIDE, 'into-root')])).toEqual([]) - }) - - it('none in the directories the sandbox mounts anew', async () => { + it('under a write root that is the root, but none in the directories the sandbox mounts anew', async () => { + const denyWithinAllow = ['/proc/self/cwd/file', ...inRoot(['name', 'out'])] expect( - await heldBy([], { - writeConfig: { - allowOnly: ['/'], - denyWithinAllow: ['/proc/self/cwd/file', join(ROOT, 'out')], - }, - }), - ).toEqual(pairs('out')) - }) - - it('none that a read deny hides', async () => { - const options = { readConfig: { denyOnly: [join(ROOT, 'holder')] } } - const denied = inRoot(['holder/commands', 'name']) - expect(await heldBy(denied, options)).toEqual(pairs('name')) - if (bwrapCanNamespace()) { - expect(stdoutOf(await wrapLinux(denied, options))).toBe('started\n') - } - }) - - it('none that is bound back by itself into a directory a read deny hides', async () => { - mkdirSync(join(ROOT, 'holder', 'inner')) - symlinkSync('inner', join(ROOT, 'holder', 'to-inner')) - const options = { - readConfig: { - denyOnly: [join(ROOT, 'holder')], - allowWithinDeny: inRoot(['holder/to-inner']), - }, - } - const denied = inRoot(['holder/to-inner/file']) - expect(await heldBy(denied, options)).toEqual([]) - if (bwrapCanNamespace()) { - expect(stdoutOf(await wrapLinux(denied, options))).toBe('started\n') + await heldBy([], { writeConfig: { allowOnly: ['/'], denyWithinAllow } }), + ).toEqual(said(['name', 'out'])) + }) + + it('none in a directory that a read deny hides, though the link is bound back by itself', async () => { + fs.mkdirSync(join(HOLDER, 'inner')) + fs.symlinkSync('inner', join(HOLDER, 'to-inner')) + const denied = inRoot(['holder/to-inner/file', 'holder/commands', 'name']) + const allowWithinDeny = [join(HOLDER, 'to-inner')] + const hiding = { readConfig: { denyOnly: [HOLDER], allowWithinDeny } } + expect(await heldBy(denied, hiding)).toEqual(said(['name'])) + if (namespaces) { + expect(run(await wrapLinux(denied, hiding)).stdout).toBe('started\n') } }) - it('the directories above a held link are pinned', async () => { - const holder = join(ROOT, 'holder') - expect(await wrapLinux(inRoot(['holder/commands']))).toContain( - `--ro-bind ${holder} ${holder} `, - ) - }) - - it.if(modesCount)( - 'a directory that cannot be looked in is bound read-only in place of the path', - async () => { - const holder = join(ROOT, 'holder') - chmodSync(holder, 0) - const { result: wrapped, warnings } = await withCapturedWarnings(() => - wrapLinux(inRoot(['holder/commands', 'holder/other'])), - ) - // Beneath the write root's bind it is a pin, after it a deny. - expect( - wrapped.lastIndexOf(`--ro-bind ${holder} ${holder} `), - ).toBeGreaterThan(wrapped.indexOf(`--bind ${ROOT} ${ROOT} `)) - expect(wrapped).not.toContain(`${holder}/`) - expect(warnings.join()).toContain(`denying ${holder} whole`) - }, - ) - - it.each<[string, (name: string) => Partial]>([ - ['a read deny', name => ({ readConfig: { denyOnly: [name] } })], - [ - 'a match of a read-deny pattern', - name => ({ readConfig: { denyOnly: [], matchedLinks: [name] } }), - ], - [ - 'a masked file', - name => ({ maskedFileBinds: [{ realPath: name, fakePath: '/fakes/0' }] }), - ], - ])('the links on the way to %s', async (_what, options) => { - expect(await heldBy([], options(join(ROOT, 'first')))).toEqual( - pairs('first', 'middle'), + // In place of the paths in it. + it.if(modesCount)('denies a directory it cannot look in whole', async () => { + fs.chmodSync(HOLDER, 0) + const { result: wrapped, warnings } = await withCapturedWarnings(() => + wrapLinux(inRoot(['holder/commands', 'holder/other'])), ) + // Beneath the write root's bind it is a pin, after it a deny. + expect( + wrapped.lastIndexOf(`--ro-bind ${HOLDER} ${HOLDER} `), + ).toBeGreaterThan(wrapped.indexOf(`--bind ${ROOT} ${ROOT} `)) + expect(wrapped).not.toContain(`${HOLDER}/`) + expect(warnings.join()).toContain(`denying ${HOLDER} whole`) }) - it('a link that leads to nothing yet, for a read deny on it', async () => { + it('the links on the way to a read deny, one that leads to nothing yet too, to a match of a read-deny pattern and to a masked file', async () => { + const [realPath, ...denyOnly] = inRoot(['way/secret', 'first', 'dangling']) expect( - await heldBy([], { readConfig: { denyOnly: inRoot(['dangling']) } }), - ).toEqual(pairs('dangling')) + await heldBy([], { + readConfig: { denyOnly, matchedLinks: inRoot(['name']) }, + maskedFileBinds: [{ realPath: realPath!, fakePath: '/fakes/0' }], + }), + ).toEqual(said(['dangling', 'first', 'middle', 'name', 'way'])) }) it('a read deny that is where it resolves to is not walked', async () => { - const spy = spyOn(fs, 'lstatSync') - try { - await wrapLinux([], { readConfig: { denyOnly: inRoot(['conf/x']) } }) - expect( - spy.mock.calls.filter(([p]) => String(p).startsWith(ROOT)), - ).toEqual([]) - } finally { - spy.mockRestore() - } + const { calls } = spyOn(fs, 'lstatSync').mock + await wrapLinux([], { readConfig: { denyOnly: inRoot(['conf/x']) } }) + expect(calls.filter(([p]) => String(p).startsWith(ROOT))).toEqual([]) }) it('a refusal for length says how much of it names links', async () => { - const wrap = (denied: string[]): Promise => - wrapLinux(denied, { command: 'a'.repeat(2 ** 21) }).then( - () => 'wrapped', - (error: Error) => error.message, - ) + const refusal = (denied: string[]): Promise => + wrapLinux(denied, { command: 'a'.repeat(2 ** 21) }).catch(String) const words = `--hold-link ${ROOT}/name conf/x` - expect(await wrap(inRoot(['name']))).toContain( + expect(await refusal(inRoot(['name']))).toContain( `At least ${words.length} of them are not the caller's`, ) - expect(await wrap([])).toMatch(/the limit here is \d+\)$/) + expect(await refusal([])).toMatch(/the limit here is \d+\)$/) }) }) -describe.if(isLinux)('where no link is held', () => { - beforeEach(forgetLinuxUnheldLinks) - - /** A helper that notes each time it is run, then does as `script` says. */ - const noting = (script: string): string => - program('noting-helper', `echo run >> ${OUTSIDE}/runs\n${script}`) - const runs = (): number => - existsSync(join(OUTSIDE, 'runs')) - ? readFileSync(join(OUTSIDE, 'runs'), 'utf8').split('\n').length - 1 - : 0 - /** As a helper built before the option does. */ - const RUNS_ITS_WORDS = 'exec "$@"' - /** `helper` for the one this library finds for itself. */ - const found = (helper: string): { mockRestore(): void } => - spyOn(seccomp, 'getApplySeccompBinaryPath').mockReturnValue(helper) - const unheld = ( - reason: LinuxUnheldLinkReason, - ): ReturnType => - inRoot(['first', 'middle']).map(path => ({ path, reason })) +type Config = Parameters[0] - it.each<[LinuxUnheldLinkReason, string, () => Partial]>([ - ['no_helper', 'no helper runs', () => ({ allowAllUnixSockets: true })], - [ - 'told_no', - 'the caller says its helper does not hold links', - () => ({ - seccompConfig: { applyPath: noting('exit 0'), holdsLinks: false }, - }), - ], - [ - 'told_no', - 'the caller turns holding off', - () => ({ seccompConfig: { holdsLinks: false } }), - ], - [ - 'not_told', - "the helper is the caller's, which says nothing", - () => ({ seccompConfig: { applyPath: noting('exit 0') } }), - ], - [ - 'not_told', - 'the same of one that is run under another name', - () => ({ - seccompConfig: { applyPath: noting('exit 0'), argv0: 'apply-seccomp' }, - }), - ], - ])( - '%s, %s: they are named, nothing is run to find out, and the command starts', - async (reason, _what, rest) => { - const { result: wrapped, warnings } = await withCapturedWarnings(() => - wrapLinux(inRoot(['first']), rest()), - ) - - expect(wrapped).not.toContain('--hold-link') - expect(getLinuxUnheldLinks()).toEqual(unheld(reason)) - for (const { path } of unheld(reason)) { - expect( - warnings.some(l => l.includes(`${path},`) && l.includes(reason)), - ).toBe(true) - } - expect(runs()).toBe(0) - if (bwrapCanNamespace() && !wrapped.includes('noting-helper')) { - expect(stdoutOf(wrapped)).toBe('started\n') - } - }, - ) +/** ROOT for the one write root, and `first` denied in it. */ +const config = (denyRead: string[], rest: Partial = {}): Config => ({ + network: { allowedDomains: [], deniedDomains: [] }, + filesystem: { denyRead, allowWrite: [ROOT], denyWrite: inRoot(['first']) }, + ...rest, +}) - it("the caller's word that its helper holds links is taken, and nothing is run", async () => { - const wrapped = await wrapLinux(inRoot(['first']), { - seccompConfig: { - applyPath: '/proc/self/fd/3', - argv0: 'apply-seccomp', - holdsLinks: true, - }, - }) - expect(wrapped).toContain( - `ARGV0=apply-seccomp /proc/self/fd/3 --hold-link ${ROOT}/first middle `, +describe.if(isLinux)('where no link is held', () => { + const [NEWER, OLDER] = [join(OUTSIDE, 'newer'), join(OUTSIDE, 'older')] + const listed = SandboxManager.getLinuxUnheldLinks + type Reason = LinuxUnheldLink['reason'] + const unheld = (reason: Reason, ...names: string[]): LinuxUnheldLink[] => + inRoot(names).map(path => ({ path, reason })) + + // The reason if any, `seccomp`, and the helper the library finds for itself. + // The links are named, what holds one is not pinned for it either, and no + // helper is run to find out. + it.each<[Reason | '', Options['seccompConfig'], string]>([ + ['no_helper', {}, ''], + ['told_no', { holdsLinks: false }, ''], + ['told_no', { applyPath: NEWER, holdsLinks: false }, ''], + ['not_told', { applyPath: NEWER }, ''], + // What the library finds where nothing is at the path says nothing of it. + ['not_told', { applyPath: '/in/the/sandbox', argv0: 'apply-seccomp' }, ''], + ['', { applyPath: OLDER, holdsLinks: true }, ''], + ['', {}, NEWER], + ['older_helper', {}, OLDER], + ['no_answer', {}, OUTSIDE], + ])('"%s", told %j and finding "%s"', async (reason, seccompConfig, own) => { + // One names the option that says so, one runs its words as older builds do. + program('newer', `echo run > ${OUTSIDE}/run # --holds-links`) + program('older', `echo run > ${OUTSIDE}/run; exec "$@"`) + if (own) spyOn(seccomp, 'getApplySeccompBinaryPath').mockReturnValue(own) + const allowAllUnixSockets = reason === 'no_helper' + const { result: wrapped, warnings } = await withCapturedWarnings(() => + wrapLinux(inRoot(['far']), { seccompConfig, allowAllUnixSockets }), ) - expect(getLinuxUnheldLinks()).toEqual([]) - }) - - it.each<[string, string, number, LinuxUnheldLinkReason | undefined]>([ - ['names the option that says so', '# --holds-links', 0o755, undefined], - ['does not', RUNS_ITS_WORDS, 0o755, 'older_helper'], - ...(modesCount - ? [ - ['cannot be read', '', 0, 'no_answer'] as [ - string, - string, - 0, - 'no_answer', - ], - ] - : []), - ])( - 'a helper the library found for itself is read, never run: one that %s', - async (_what, script, mode, reason) => { - const helper = noting(script) - chmodSync(helper, mode) - const spy = found(helper) - try { - const wrapped = await wrapLinux(inRoot(['first'])) - expect(wrapped.includes('--hold-link')).toBe(reason === undefined) - } finally { - spy.mockRestore() - } - expect(getLinuxUnheldLinks()).toEqual(reason ? unheld(reason) : []) - expect(runs()).toBe(0) - }, - ) - it('what holds a link is not pinned for it either', async () => { - const pin = `--ro-bind ${ROOT}/holder ${ROOT}/holder ` - const denied = inRoot(['holder/commands']) - expect(await wrapLinux(denied)).toContain(pin) - expect( - await wrapLinux(denied, { seccompConfig: { holdsLinks: false } }), - ).not.toContain(pin) + for (const words of ['--hold-link', `--ro-bind ${HOLDER} ${HOLDER} `]) { + expect(wrapped.includes(words)).toBe(reason === '') + } + const links = reason ? unheld(reason, 'far', 'holder/blank') : [] + expect(listed()).toEqual(links) + for (const { path } of links) { + expect( + warnings.some(l => l.includes(`${path},`) && l.includes(reason)), + ).toBe(true) + } + expect(fs.existsSync(join(OUTSIDE, 'run'))).toBe(false) + if (namespaces && !seccompConfig?.applyPath && !own) { + expect(run(wrapped).stdout).toBe('started\n') + } }) - it('a link is listed by the latest wrap that met it', async () => { + it.each<[string, () => void | Promise]>([ + ['reset()', () => SandboxManager.reset()], + ['updateConfig()', () => SandboxManager.updateConfig(config([]))], + ])('listed by the latest wrap to meet it, until %s', async (_what, act) => { await wrapLinux(inRoot(['first']), { allowAllUnixSockets: true }) + await wrapLinux(inRoot(['first']), { seccompConfig: { holdsLinks: false } }) expect(await wrapLinux(inRoot(['middle']))).toContain('--hold-link') - expect(getLinuxUnheldLinks()).toEqual(unheld('no_helper').slice(0, 1)) + expect(listed()).toEqual(unheld('told_no', 'first')) + await act() + expect(listed()).toEqual([]) }) it('a wrap that throws names none', async () => { - const refusal: unknown = await wrapLinux(inRoot(['first']), { - allowAllUnixSockets: true, - binShell: 'no-such-shell', - }).catch((error: unknown) => error) - expect(String(refusal)).toContain('no-such-shell') - expect(getLinuxUnheldLinks()).toEqual([]) + const throwing = { allowAllUnixSockets: true, binShell: 'no-such-shell' } + const refusal = await wrapLinux(inRoot(['first']), throwing).catch(String) + expect(refusal).toContain("'no-such-shell' not found") + expect(listed()).toEqual([]) }) - describe('as the manager lists them', () => { - const config = (allowAllUnixSockets: boolean): SandboxRuntimeConfig => ({ - network: { allowedDomains: [], deniedDomains: [], allowAllUnixSockets }, - filesystem: { - denyRead: [], - allowWrite: [ROOT], - denyWrite: inRoot(['first']), - }, - }) - afterEach(() => SandboxManager.reset()) - - it('takes the word on the helper from the configuration', async () => { - const { seccomp: parsed } = SandboxRuntimeConfigSchema.parse({ - ...config(false), - seccomp: { holdsLinks: false }, - }) - expect(parsed).toEqual({ holdsLinks: false }) - await SandboxManager.initialize({ ...config(false), seccomp: parsed }) - await SandboxManager.wrapWithSandbox('true') - expect(SandboxManager.getLinuxUnheldLinks()).toEqual(unheld('told_no')) - }) - - it('getFsReadConfig() names the links its patterns match, as a wrap does', async () => { - const { filesystem, ...rest } = config(false) - await SandboxManager.initialize({ - ...rest, - filesystem: { ...filesystem, denyRead: [join(ROOT, 'nam*')] }, - }) - expect(SandboxManager.getFsReadConfig().matchedLinks).toEqual( - inRoot(['name']), - ) - }) - - it('lists them', async () => { - await SandboxManager.initialize(config(true)) - await SandboxManager.wrapWithSandbox('true') - expect(SandboxManager.getLinuxUnheldLinks()).toEqual(unheld('no_helper')) - }) - - it.each<[string, () => void | Promise]>([ - ['reset()', () => SandboxManager.reset()], - ['updateConfig()', () => SandboxManager.updateConfig(config(true))], - ])('%s empties the list', async (_what, act) => { - await SandboxManager.initialize(config(true)) - await SandboxManager.wrapWithSandbox('true') - expect(SandboxManager.getLinuxUnheldLinks()).not.toEqual([]) - await act() - expect(SandboxManager.getLinuxUnheldLinks()).toEqual([]) - }) + it('the manager takes the word on the helper from the configuration, and names the links its patterns match', async () => { + await SandboxManager.initialize( + SandboxRuntimeConfigSchema.parse( + config(inRoot(['nam*', 'besid*']), { seccomp: { holdsLinks: false } }), + ), + ) + expect(SandboxManager.getFsReadConfig().matchedLinks).toEqual( + inRoot(['name']), + ) + await SandboxManager.wrapWithSandbox('true') + expect(listed()).toEqual(unheld('told_no', 'first', 'middle', 'name')) }) }) -describe.if(isLinux && bwrapCanNamespace())('the seccomp helper', () => { +describe.if(namespaces)('the seccomp helper', () => { const helper = (): string => seccomp.getApplySeccompBinaryPath()! - const run = ( - runnable: string, - words: string[], - env: Record = {}, - ): { status: number | null; said: string } => { - const result = spawnSync(runnable, words, { - encoding: 'utf8', - timeout: 30000, - cwd: ROOT, - env: { ...process.env, ...env }, - }) - return { status: result.status, said: result.stdout + result.stderr } - } + /** For a shell in ROOT. With no word on what the first of them says. */ + const HOLDS = + '--hold-link name "" --hold-link way real --hold-link dangling later' /** Lifts the helper's namespaces filter, which refuses every mount call. */ - const NAMESPACES_ALLOWED = { SRT_ALLOW_NESTED_USERNS: '1' } - const holding = (...names: Array): string[] => - names.flatMap(name => ['--hold-link', join(ROOT, name), LINKS[name]]) - /** umount2() with UMOUNT_NOFOLLOW, which reaches a mount on a link. */ - const UNMOUNT = [ - 'python3', - '-c', - [ - 'import ctypes, errno, sys', - 'libc = ctypes.CDLL(None, use_errno=True)', - 'r = libc.umount2(sys.argv[1].encode(), 8)', - 'print("unmounted" if r == 0 else errno.errorcode[ctypes.get_errno()])', - ].join('\n'), - ] + const NAMESPACES_ALLOWED = 'SRT_ALLOW_NESTED_USERNS=1' + const ELSEWHERE = 'leads elsewhere than it did' it('says that it holds links', () => { - expect(run(helper(), ['--holds-links'])).toEqual({ status: 0, said: '' }) + expect(run(`${helper()} --holds-links`).status).toBe(0) }) it.each([ - ['a file is at the name', 'beside', '', 'not a symbolic link'], - ['nothing is at the name', 'absent', '', 'No such file or directory'], - ['the link says more', 'name', 'conf', 'leads elsewhere than it did'], - ['the link says less', 'name', 'conf/xy', 'leads elsewhere than it did'], - [ - 'the link says another thing', - 'name', - 'conf/y', - 'leads elsewhere than it did', - ], + ['a file is at the name', 'beside', '""', 'not a symbolic link'], + ['nothing is at the name', 'absent', '""', 'No such file or directory'], + ['the link says more', 'name', 'conf', ELSEWHERE], + ['the link says less', 'name', 'conf/xy', ELSEWHERE], + ['the link says another thing', 'name', 'conf/y', ELSEWHERE], ])('runs no command where %s, and says why', (_what, name, target, why) => { expect( - run(helper(), ['--hold-link', join(ROOT, name), target, 'echo', 'ran']), - ).toEqual({ + run(`${helper()} --hold-link ${name} ${target} echo ran 2>&1`, ROOT), + ).toMatchObject({ status: 1, - said: `apply-seccomp: --hold-link ${join(ROOT, name)}: ${why} (seccomp.holdsLinks: false in the sandbox configuration holds no link)\n`, + stdout: `apply-seccomp: --hold-link ${name}: ${why} (seccomp.holdsLinks: false in the sandbox configuration holds no link)\n`, }) }) - it('holds a link it is given no word on', () => { - expect( - run(helper(), ['--hold-link', join(ROOT, 'name'), '', 'rm', 'name']).said, - ).toMatch(BUSY) - }) - - // The filter answers first, whoever asks. Where it is lifted, this user lacks - // the capability, and uid 0 meets the lock. + // It has every capability in its namespace. The filter refuses the unmount; + // where it is lifted, the lock does. it.each([ - ['this user', [], {}, 'EPERM'], - ['uid 0, with every capability in its namespace', ['-Ur'], {}, 'EPERM'], - ['this user, allowed namespaces', [], NAMESPACES_ALLOWED, 'EPERM'], - ['uid 0, allowed namespaces', ['-Ur'], NAMESPACES_ALLOWED, 'EINVAL'], - ])('a command of %s cannot unmount the hold', (_who, as, env, errno) => { - const { said } = run( - 'unshare', - [...as, helper(), ...holding('name'), ...UNMOUNT, join(ROOT, 'name')], - env, + ['', 'EPERM'], + [NAMESPACES_ALLOWED, 'EINVAL'], + ])('%s a command of uid 0 cannot unmount the hold', (env, errno) => { + // umount2() with UMOUNT_NOFOLLOW, which reaches a mount on a link. + const unmount = + 'import ctypes, errno; ctypes.CDLL(None, use_errno=True).umount2(b"name", 8); print(errno.errorcode[ctypes.get_errno()])' + const { stdout } = run( + `${env} unshare -Ur ${helper()} ${HOLDS} sh -c 'python3 -c "$0"; rm name 2>&1' '${unmount}'`, + ROOT, ) - expect(said.trim()).toBe(errno) + expect(stdout).toStartWith(`${errno}\n`) + expect(stdout).toMatch(BUSY) }) it('a hold keeps the flags of the file system that holds the link', () => { - const link = join(OUTSIDE, 'link') - const { said } = run('unshare', [ - ...['-Urm', 'sh', '-c'], - `mount -t tmpfs -o noexec,noatime,nosymfollow none ${OUTSIDE} && ` + - `echo read > ${OUTSIDE}/x && ln -s x ${link} && ` + - `exec ${helper()} --hold-link ${link} x sh -c 'rm ${link}; cat ${link}'`, - ]) - expect(said).toMatch(BUSY) - expect(said).toContain('Too many levels of symbolic links') - expect(said).not.toContain('read') + const { stdout } = run( + `unshare -Urm sh -c 'mount -t tmpfs -o noexec,noatime,nosymfollow none ${OUTSIDE} && ` + + `cd ${OUTSIDE} && echo read > x && ln -s x link && ` + + `exec ${helper()} --hold-link link x sh -c "rm link; cat link"' 2>&1`, + ) + expect(stdout).toMatch(BUSY) + expect(stdout).toContain('Too many levels of symbolic links') + expect(stdout).not.toContain('read') }) it.each(['--ro-bind', '--bind', '--dev-bind'])( 'a bubblewrap the command may start, with %s / /, starts, and the hold holds there', bind => { - const { said } = run( - helper(), - [ - ...holding('name', 'way', 'dangling'), - ...['bwrap', bind, '/', '/', '--bind', ROOT, ROOT, '--dev', '/dev'], - ...['sh', '-c', 'echo started; rm name way dangling'], - ], - NAMESPACES_ALLOWED, + const { stdout } = run( + `${NAMESPACES_ALLOWED} ${helper()} ${HOLDS} bwrap ${bind} / / --bind ${ROOT} ${ROOT} --dev /dev sh -c 'echo started; rm name way dangling' 2>&1`, + ROOT, ) - expect(said).toContain('started') - expect(said.match(new RegExp(BUSY, 'gi'))).toHaveLength(3) + expect(stdout).toContain('started') + expect(stdout.match(new RegExp(BUSY, 'gi'))).toHaveLength(3) }, ) it('a link that says another thing by the time the command runs: nothing runs', async () => { const wrapped = await wrapLinux(inRoot(['name'])) - rmSync(join(ROOT, 'name')) - symlinkSync('conf/z', join(ROOT, 'name')) - const result = spawnSync(wrapped, { shell: true, encoding: 'utf8' }) - expect(result.stdout).toBe('') - expect(result.stderr).toContain( - `apply-seccomp: --hold-link ${ROOT}/name: leads elsewhere than it did`, - ) + fs.rmSync(join(ROOT, 'name')) + fs.symlinkSync('conf/z', join(ROOT, 'name')) + expect(run(wrapped)).toMatchObject({ + stdout: '', + stderr: expect.stringContaining(`--hold-link ${ROOT}/name: ${ELSEWHERE}`), + }) }) }) -describe.if(isLinux && bwrapCanNamespace())( - 'A read deny on a name that is a link, over two commands', - () => { - const SECRET = 'secret-content' - afterEach(() => SandboxManager.reset()) - - it.each< - [string, string, (name: string) => Partial, string] - >([ - ['denyRead', 'file', name => ({ filesystem: lists([name]) }), ''], - ['denyRead', 'dir', name => ({ filesystem: lists([name]) }), '/file'], - [ - 'a denyRead pattern', - 'file', - name => ({ filesystem: lists([`${name}*`]) }), - '', - ], - [ - 'a credential deny', - 'file', - path => ({ credentials: { files: [{ path, mode: 'deny' }] } }), - '', - ], - [ - 'a credential mask', - 'file', - path => ({ credentials: { files: [{ path, mode: 'mask' }] } }), - '', - ], - ])( - '%s on a link to a %s: the name stays, and what it leads to stays hidden', - async (_kind, leadsTo, listing, inside) => { - mkdirSync(join(OUTSIDE, 'dir')) - writeFileSync(join(OUTSIDE, 'file'), SECRET) - writeFileSync(join(OUTSIDE, 'dir', 'file'), SECRET) - const name = join(ROOT, 'linked') - symlinkSync(join(OUTSIDE, leadsTo), name) - await SandboxManager.initialize({ - network: { allowedDomains: [], deniedDomains: [] }, - filesystem: lists([]), - ...listing(name), - }) - const saidBy = async (command: string): Promise => { - const said = stdoutOf( - await SandboxManager.wrapWithSandbox(`echo ran; ${command}`), - ) - SandboxManager.cleanupAfterCommand() - expect(said).toStartWith('ran\n') - return said - } - - await saidBy(`rm ${name}; mv ${name} ${name}-aside; echo x > ${name}`) - expect(readlinkSync(name)).toBe(join(OUTSIDE, leadsTo)) - expect( - await saidBy(`cat ${join(OUTSIDE, leadsTo)}${inside}`), - ).not.toContain(SECRET) - }, - ) - - function lists(denyRead: string[]): SandboxRuntimeConfig['filesystem'] { - return { denyRead, allowWrite: [ROOT], denyWrite: [] } +// The name stays, and what it leads to stays hidden. +describe.if(namespaces)('a read deny on a link, over two commands', () => { + it.each<[string, string, string[], Array<'deny' | 'mask'>]>([ + ['denyRead', 'file', [''], []], + ['denyRead', 'dir', [''], []], + ['a denyRead pattern', 'file', ['*'], []], + ['a credential deny', 'file', [], ['deny']], + ['a credential mask', 'file', [], ['mask']], + ])('%s on a link to a %s', async (_kind, leadsTo, endings, modes) => { + const [name, target] = [join(ROOT, 'linked'), join(OUTSIDE, leadsTo)] + fs.mkdirSync(join(OUTSIDE, 'dir')) + fs.writeFileSync(join(OUTSIDE, 'file'), 'secret') + fs.writeFileSync(join(OUTSIDE, 'dir', 'file'), 'secret') + fs.symlinkSync(target, name) + const files = modes.map(mode => ({ path: name, mode })) + const denied = endings.map(ending => name + ending) + await SandboxManager.initialize(config(denied, { credentials: { files } })) + const saidBy = async (command: string): Promise => { + const { stdout } = run( + await SandboxManager.wrapWithSandbox(`echo ran; ${command}`), + ) + SandboxManager.cleanupAfterCommand() + expect(stdout).toStartWith('ran\n') + return stdout } - }, -) + + await saidBy(`rm ${name}; mv ${name} ${name}-aside; echo x > ${name}`) + expect(fs.readlinkSync(name)).toBe(target) + expect(await saidBy(`cat ${target} ${target}/file`)).not.toContain('secret') + }) +}) From 4ac7e7e39aa46ae389248c1743b5dabd6552512a Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 20 Oct 2026 13:59:29 +0000 Subject: [PATCH 8/8] linux: a shut directory on the way does not lift the deny of the missing names before it Where the walk to a denied path cannot look in a directory, that directory stands in for the path. The missing names the way had gone up from before it were dropped with the path, so a command could make one of them a link and lead the way elsewhere: l -> m1/../locked/x, chmod 000 locked, ln -s evil/sub m1. They are denied beside the directory, as they are where the walk goes through. Tests: that case; the directories a way goes up from are pinned where nothing holds links and the command can replace none; one refusal is recognised by the shell's name alone, whatever the words around it. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01FydxqUESqxwSCVFnhk3gfv Co-authored-by: ronleizrowice --- src/sandbox/linux-sandbox-utils.ts | 21 +++++++++++-------- test/sandbox/symlinked-deny-paths.test.ts | 4 ++++ test/sandbox/write-deny-through-links.test.ts | 13 +++++++++++- 3 files changed, 28 insertions(+), 10 deletions(-) diff --git a/src/sandbox/linux-sandbox-utils.ts b/src/sandbox/linux-sandbox-utils.ts index c881ab091..0103b9c1a 100644 --- a/src/sandbox/linux-sandbox-utils.ts +++ b/src/sandbox/linux-sandbox-utils.ts @@ -2176,12 +2176,15 @@ async function generateFilesystemArgs( // What the ways to the denied paths depend on; see followLinks. const linksToDenies = new Map() const leftOnTheWay = new Set() - /** Returns the directory the walk to `name` could not look in, if any. */ - const noteLinksTo = (name: string): string | undefined => { - const { links, left, unsearched } = followLinks(name) + /** + * Returns what stands in for `name` where the walk to it could not look in + * a directory: that directory, and the missing names it went up from before. + */ + const noteLinksTo = (name: string): string[] | undefined => { + const { links, left, passed, unsearched } = followLinks(name) for (const [link, target] of links) linksToDenies.set(link, target) for (const dir of left) leftOnTheWay.add(dir) - return unsearched + return unsearched === undefined ? undefined : [unsearched, ...passed] } // Directories that a deny entry re-binds read-only inside the sandbox // (--ro-bind ), keyed by resolved dest, with every raw @@ -2690,17 +2693,17 @@ async function generateFilesystemArgs( // Each deny path as both loops below take it. A directory the walk could // not look in stands in for the path: bound read-only, it keeps its mode. - const rawDenyPaths = denyPaths.map(pathPattern => { + const rawDenyPaths = denyPaths.flatMap(pathPattern => { const rawPath = normalizePathForSandbox(pathPattern, { literal: true }) // Both loops skip it. if (rawPath.startsWith('/dev/')) return rawPath - const unsearched = noteLinksTo(rawPath) - if (unsearched === undefined) return rawPath + const instead = noteLinksTo(rawPath) + if (instead === undefined) return rawPath logForDebugging( - `[Sandbox Linux] Write deny path ${rawPath} cannot be looked at; denying ${unsearched} whole instead`, + `[Sandbox Linux] Write deny path ${rawPath} cannot be looked at; denying ${instead[0]} whole instead`, { level: 'warn' }, ) - return unsearched + return instead }) // Duplicate deny entries must be collapsed: a duplicate diff --git a/test/sandbox/symlinked-deny-paths.test.ts b/test/sandbox/symlinked-deny-paths.test.ts index 722d868e2..4ee1b8548 100644 --- a/test/sandbox/symlinked-deny-paths.test.ts +++ b/test/sandbox/symlinked-deny-paths.test.ts @@ -269,6 +269,10 @@ describe.if(isLinux)('Symlinked deny paths (resolve-before-mask)', () => { const shut = join(PROJ, 'shut') expect(countMounts(result, '--ro-bind', shut, shut)).toBe(1) expect(result).not.toContain(`/dev/null ${join(PROJ, 'name')}`) + // Made a link, the missing name would lead to another `shut`. + expect( + countMounts(result, '--ro-bind', '/dev/null', join(PROJ, 'gone')), + ).toBe(target.startsWith('gone') ? 1 : 0) }, ) diff --git a/test/sandbox/write-deny-through-links.test.ts b/test/sandbox/write-deny-through-links.test.ts index 02e1dd2e5..fff4a4f4e 100644 --- a/test/sandbox/write-deny-through-links.test.ts +++ b/test/sandbox/write-deny-through-links.test.ts @@ -541,7 +541,18 @@ describe.if(isLinux)('where no link is held', () => { it('a wrap that throws names none', async () => { const throwing = { allowAllUnixSockets: true, binShell: 'no-such-shell' } const refusal = await wrapLinux(inRoot(['first']), throwing).catch(String) - expect(refusal).toContain("'no-such-shell' not found") + expect(refusal).toContain('no-such-shell') + expect(listed()).toEqual([]) + }) + + it('what the way goes up from is pinned all the same where the command can replace no link on it', async () => { + fs.symlinkSync(`${ROOT}/${LINKS.up}`, join(OUTSIDE, 'up')) + const wrapped = await wrapLinux([join(OUTSIDE, 'up')], { + seccompConfig: { holdsLinks: false }, + }) + for (const dir of inRoot(['in', 'in/to'])) { + expect(wrapped).toContain(`--ro-bind ${dir} ${dir} `) + } expect(listed()).toEqual([]) })