diff --git a/README.md b/README.md index 14d89686..3b2071cf 100644 --- a/README.md +++ b/README.md @@ -680,6 +680,8 @@ $ srt 'echo "bad" > .git/hooks/pre-commit' **Note (Linux):** On Linux, mandatory deny paths only block files that already exist. Non-existent files in these patterns cannot be blocked by bubblewrap's bind-mount approach. macOS uses glob patterns which block both existing and new files. +**Note (Linux):** The wrapped string becomes the one argument of `sh -c`, and Linux caps a single argument at 128 KiB (`MAX_ARG_STRLEN` on 4 KiB-page kernels). A profile past that is handed to bubblewrap through `--args` from a file in a per-process temporary directory (created under `os.tmpdir()` on the process's first sandboxed wrap) that every profile of that process ro-binds over itself, so a sandboxed command it launched cannot rewrite a profile bubblewrap has yet to read; a sandbox launched by another `srt` process with tmpdir writable is not covered. The string then opens the file on fd 9, unlinks it, and runs bubblewrap, so it must be run unmodified as the whole `sh -c` script under a POSIX `sh`, and fd 9 is consumed there (fd 8 instead when `seccompConfig.applyPath` is `/proc/self/fd/9`); a file never spawned is removed with the other per-command artifacts (`cleanupAfterCommand()`, process exit). The command itself must still fit one argument, and bubblewrap caps a profile at 9000 parsed arguments (about 3000 mounts). + **Linux search depth:** On Linux, the sandbox uses `ripgrep` to scan for dangerous files in subdirectories within allowed write paths. By default, it searches up to 3 levels deep for performance. You can configure this with `mandatoryDenySearchDepth`: ```json diff --git a/src/sandbox/linux-sandbox-utils.ts b/src/sandbox/linux-sandbox-utils.ts index e31685cf..2c13f18e 100644 --- a/src/sandbox/linux-sandbox-utils.ts +++ b/src/sandbox/linux-sandbox-utils.ts @@ -398,6 +398,72 @@ async function linuxGetMandatoryDenyPaths( // be cleaned up explicitly. const bwrapMountPoints: Set = new Set() +/** Linux's per-argument cap (MAX_ARG_STRLEN, 32 pages) on 4 KiB-page kernels. */ +const LINUX_MAX_ARG_STRLEN = 128 * 1024 + +/** + * The fd an over-long profile's `--args` file is opened on. A single digit, + * since dash (Debian/Ubuntu's /bin/sh) rejects multi-digit redirections, and + * high, since embedders hand the command low fds of their own (an extra + * stdio pipe, or a helper binary passed as `/proc/self/fd/3`). When the + * caller's seccompConfig.applyPath names this very fd, the next one down is + * used instead. + */ +const BWRAP_ARGS_FD = 9 + +function bwrapArgsFdFor(seccompApplyPath: string | undefined): number { + const taken = seccompApplyPath?.match(/^\/(?:proc\/self|dev)\/fd\/(\d+)$/) + return Number(taken?.[1]) === BWRAP_ARGS_FD + ? BWRAP_ARGS_FD - 1 + : BWRAP_ARGS_FD +} + +/** + * Per-process directory for the `bwrap --args` files of profiles too large + * for one shell argument, created on the first sandboxed wrap and ro-bound + * over itself in EVERY profile this process generates (the INVARIANT at the + * end of generateFilesystemArgs). bwrap reads a file only when the embedder + * spawns the string; until then no sandbox this process launched may be + * able to write there, or a sandboxed command could rewrite the next + * command's profile. The rendered string unlinks its file as soon as the + * shell has opened it, so a file lives from the wrap to the spawn; one never + * spawned goes with the mount points. The directory lives for the whole + * process — never removed at reset(), since a sandbox launched before a + * reset may still be running with it bound — and goes at exit. + */ +let bwrapArgsDir: string | undefined +const bwrapArgsFiles: Set = new Set() +let bwrapArgsFileCount = 0 + +function ensureBwrapArgsDir(): string { + if (bwrapArgsDir !== undefined && fs.existsSync(bwrapArgsDir)) { + return bwrapArgsDir + } + if (bwrapArgsDir !== undefined) { + // Removed under us (an age-based clean of os.tmpdir()): a profile + // binding a gone path would never start, so a fresh one is made — but + // a sandbox launched earlier that is still running never bound it, and + // could write there. Say so. + logForDebugging( + `[Sandbox Linux] --args directory ${bwrapArgsDir} was removed; re-creating it. Sandboxes started before this point do not have the new directory read-only.`, + { level: 'warn' }, + ) + } + bwrapArgsDir = fs.mkdtempSync(path.join(tmpdir(), 'srt-bwrap-args-')) + registerExitCleanupHandler() + return bwrapArgsDir +} + +function removeBwrapArgsDir(): void { + if (bwrapArgsDir === undefined) return + try { + fs.rmSync(bwrapArgsDir, { recursive: true, force: true }) + } catch { + // Unremovable: nothing left to do at exit. + } + bwrapArgsDir = undefined +} + // Number of wrapped commands that have been generated but whose cleanup has // not yet run. cleanupBwrapMountPoints() defers file deletion while this is // positive, because deleting a mount point file on the host while another @@ -417,6 +483,7 @@ function registerExitCleanupHandler(): void { process.on('exit', () => { cleanupBwrapMountPoints({ force: true }) + removeBwrapArgsDir() }) exitHandlerRegistered = true @@ -484,6 +551,16 @@ export function cleanupBwrapMountPoints(opts?: { force?: boolean }): void { } } bwrapMountPoints.clear() + + for (const argsFile of bwrapArgsFiles) { + try { + fs.rmSync(argsFile, { force: true }) + } catch { + // Unremovable (a permission change under the directory): cleanup + // must not throw at the caller, as for the mount points above. + } + } + bwrapArgsFiles.clear() } /** @@ -1631,6 +1708,16 @@ async function generateFilesystemArgs( if (maskedFileStoreDir !== undefined) { args.push('--ro-bind', maskedFileStoreDir, maskedFileStoreDir) } + // The same invariant for the --args directory (bwrapArgsDir): a profile + // bwrap has yet to read sits there, and an earlier sandbox of this + // process may still be running with the directory's parent writable. + // Like the store's bind, this one lands even beneath a denyRead tmpfs + // over tmpdir, so the pending profiles (deny paths, --setenv values) are + // readable there — the same bytes a sandbox already sees in its own + // /proc/1/cmdline and environment. + if (bwrapArgsDir !== undefined) { + args.push('--ro-bind', bwrapArgsDir, bwrapArgsDir) + } return args } @@ -1752,6 +1839,9 @@ export async function wrapCommandWithSandboxLinux( let applySeccompPrefix: string | undefined try { + // Before the profile is generated, so this one ro-binds it too; inside + // the try, so a failure gives the count back. + const argsDir = ensureBwrapArgsDir() // ========== SECCOMP FILTER (Unix Socket Blocking) ========== // apply-seccomp wraps the workload and applies the baked-in BPF filter // that blocks socket(AF_UNIX, ...). Skipped when allowAllUnixSockets is true. @@ -1979,6 +2069,7 @@ export async function wrapCommandWithSandboxLinux( if (!shell) { throw new Error(`Shell '${shellName}' not found in PATH`) } + const trailerStart = bwrapArgs.length bwrapArgs.push('--', shell, '-c') // With network restrictions, route the command through buildSandboxCommand @@ -2001,7 +2092,60 @@ export async function wrapCommandWithSandboxLinux( bwrapArgs.push(command) } - const wrappedCommand = quote([bwrapPath ?? 'bwrap', ...bwrapArgs]) + let wrappedCommand = quote([bwrapPath ?? 'bwrap', ...bwrapArgs]) + const oneArgumentBytes = Buffer.byteLength(wrappedCommand, 'utf8') + if (oneArgumentBytes + 1 > LINUX_MAX_ARG_STRLEN) { + // The caller runs this string as the one argument of `sh -c`, and + // Linux caps a single argv element at MAX_ARG_STRLEN (the byte count + // plus its NUL), so a profile this large would fail every spawn with + // E2BIG. Hand the options to bwrap through `--args`, which reads them + // NUL-separated from an fd (and closes it before the command starts); + // only the trailer stays on the line. bwrap still caps the number of + // parsed arguments (MAX_ARGS, 9000: about 3000 mounts), so a profile + // should still be kept small at the source. + const argsFd = bwrapArgsFdFor(seccompConfig?.applyPath) + // The directory this profile ro-binds (created above, before the + // filesystem arguments were generated). + const argsFile = path.join( + argsDir, + `args-${process.pid}-${++bwrapArgsFileCount}`, + ) + // The redirect opens the file before the group runs, and rm unlinks + // it at once (the open fd keeps it readable for bwrap), so the file + // lives only until the spawn — the window the ro-bind covers — and + // never accumulates while a sandbox stays active. `command` keeps an + // rm alias of the embedder's shell (zsh reads .zshenv for -c) out of + // the way. + const viaArgsFile = + `{ command rm -f -- ${quote([argsFile])}; exec ` + + quote([ + bwrapPath ?? 'bwrap', + '--args', + String(argsFd), + ...bwrapArgs.slice(trailerStart), + ]) + + `; } ${argsFd}<${quote([argsFile])}` + if (Buffer.byteLength(viaArgsFile, 'utf8') + 1 > LINUX_MAX_ARG_STRLEN) { + // The command itself does not fit one argument; nothing here can + // help, and the caller's spawn would fail with an opaque E2BIG. + throw new Error( + `Sandboxed command is too long for one shell argument (${Buffer.byteLength(viaArgsFile, 'utf8')} bytes with the bwrap options already moved to a file; the limit is ${LINUX_MAX_ARG_STRLEN - 1})`, + ) + } + fs.writeFileSync( + argsFile, + bwrapArgs + .slice(0, trailerStart) + .map(arg => arg + '\0') + .join(''), + { mode: 0o600, flag: 'wx' }, + ) + bwrapArgsFiles.add(argsFile) + wrappedCommand = viaArgsFile + logForDebugging( + `[Sandbox Linux] bwrap options moved to ${argsFile} (fd ${argsFd}): the command line would be ${oneArgumentBytes} bytes as one argument`, + ) + } const restrictions = [] if (needsNetworkRestriction) restrictions.push('network') diff --git a/test/sandbox/linux-bwrap-args-file.test.ts b/test/sandbox/linux-bwrap-args-file.test.ts new file mode 100644 index 00000000..2edf9b3d --- /dev/null +++ b/test/sandbox/linux-bwrap-args-file.test.ts @@ -0,0 +1,282 @@ +import { describe, it, expect, beforeEach, afterEach } from 'bun:test' +import { + existsSync, + mkdirSync, + mkdtempSync, + readFileSync, + realpathSync, + rmSync, + writeFileSync, +} from 'node:fs' +import { spawnSync } from 'node:child_process' +import { tmpdir } from 'node:os' +import { dirname, join } from 'node:path' +import { + wrapCommandWithSandboxLinux, + cleanupBwrapMountPoints, +} from '../../src/sandbox/linux-sandbox-utils.js' +import { isLinux } from '../helpers/platform.js' + +/** + * A bwrap profile too large for one shell argument (Linux's 128 KiB + * MAX_ARG_STRLEN) is handed to bwrap through `--args` from a file in a + * per-process directory that every profile ro-binds over itself; a profile + * that fits stays on the command line. + */ +describe.if(isLinux)('bwrap --args for over-long profiles', () => { + let BASE: string + const savedCwd = process.cwd() + + // Runtime arm, as in readonly-deny-dir-stubs.test.ts: only where bwrap can + // run the namespace/proc surface the wrapped commands use. + const BWRAP_CAN_NAMESPACE = + spawnSync( + 'bwrap', + [ + '--unshare-pid', + '--unshare-user', + '--cap-drop', + 'ALL', + '--ro-bind', + '/', + '/', + '--proc', + '/proc', + 'true', + ], + { timeout: 5000 }, + ).status === 0 + + beforeEach(() => { + BASE = realpathSync(mkdtempSync(join(tmpdir(), 'bwrap-args-'))) + // cwd outside the write allowlist keeps the mandatory-deny scan from + // adding mounts of its own. + process.chdir(BASE) + }) + + afterEach(() => { + process.chdir(savedCwd) + cleanupBwrapMountPoints({ force: true }) + rmSync(BASE, { recursive: true, force: true }) + }) + + // `count` files, each its own /dev/null mask, as the concrete list the + // wrapper takes (glob expansion happens a layer up, in SandboxManager). + function flatFiles( + count: number, + stem = 'a-reasonably-long-file-name-to-fill-the-profile-', + ): string[] { + const dir = join(BASE, 'many') + mkdirSync(dir, { recursive: true }) + const files: string[] = [] + for (let i = 0; i < count; i++) { + const file = join(dir, `${stem}${i}.log`) + // Content, so a masked read of 0 bytes proves the mask applied. + writeFileSync(file, 'secret\n') + files.push(file) + } + return files + } + + async function wrap( + files: string[], + opts: { + command?: string + allowOnly?: string[] + setEnvVars?: Record + mandatoryDenySearchDepth?: number + seccompConfig?: { applyPath: string; argv0?: string } + } = {}, + ): Promise { + return wrapCommandWithSandboxLinux({ + command: opts.command ?? 'echo hello', + needsNetworkRestriction: false, + readConfig: { denyOnly: files }, + writeConfig: { allowOnly: opts.allowOnly ?? [], denyWithinAllow: [] }, + setEnvVars: opts.setEnvVars, + mandatoryDenySearchDepth: opts.mandatoryDenySearchDepth, + seccompConfig: opts.seccompConfig, + }) + } + + // The per-process --args directory, from the trailing ro-bind every + // profile that fits the command line carries (an over-long profile + // carries it inside the file). + function argsDirOf(wrapped: string): string { + const bind = wrapped.match(/--ro-bind (\S*srt-bwrap-args-\S+) \1(?: |$)/) + expect(bind).not.toBeNull() + return bind![1]! + } + + // The file an over-long profile was written to, from the redirect. + function argsFileOf(wrapped: string): string { + const redirect = wrapped.match(/ 9<(\S+)$/) + expect(redirect).not.toBeNull() + return redirect![1]! + } + + it('keeps a profile that fits on the command line and still ro-binds the --args directory', async () => { + const files = flatFiles(20) + const wrapped = await wrap(files) + expect(wrapped).not.toContain('--args') + expect(wrapped).toContain(`--ro-bind /dev/null ${files[0]}`) + // Ro-bound in every profile, not only the ones that use it: a sandbox + // launched with a small profile may still be running when a later + // over-long one is written there. + const argsDir = argsDirOf(wrapped) + expect(existsSync(argsDir)).toBe(true) + expect(wrapped.lastIndexOf(`--ro-bind ${argsDir} ${argsDir}`)).toBe( + wrapped.lastIndexOf('--ro-bind'), + ) + }) + + it('moves the options to a NUL-separated file bwrap reads through --args', async () => { + // 2000 masks of ~80 bytes each: well past 128 KiB as one argument. + const files = flatFiles(2000) + const wrapped = await wrap(files, { + setEnvVars: { SRT_TEST_VAR: "value with spaces and 'quotes'" }, + }) + + expect(Buffer.byteLength(wrapped)).toBeLessThan(128 * 1024) + // The shell opens the file on fd 9 (dash takes single-digit fds only; + // low fds belong to the embedder), unlinks it, and execs bwrap. + expect(wrapped).toMatch( + /^\{ command rm -f -- (\S+); exec bwrap --args 9 -- \S+ -c /, + ) + const argsFile = argsFileOf(wrapped) + expect(wrapped.match(/^\{ command rm -f -- (\S+); /)![1]).toBe(argsFile) + expect(existsSync(argsFile)).toBe(true) + // Inside the per-process directory. + const argsDir = dirname(argsFile) + expect(argsDir).toMatch(/srt-bwrap-args-/) + + const words = readFileSync(argsFile, 'utf8').split('\0') + expect(words[words.length - 1]).toBe('') // every word NUL-terminated + const options = words.slice(0, -1) + // The profile, one word per element, unquoted: 2000 masks between the + // fixed plumbing at either end, and a value bwrap must receive verbatim. + expect(options.slice(0, 2)).toEqual(['--new-session', '--die-with-parent']) + expect(options.slice(-2)).toEqual(['--proc', '/proc']) + // The directory's own ro-bind, last of the binds, rides in the file. + const lastBind = options.lastIndexOf('--ro-bind') + expect(options.slice(lastBind, lastBind + 3)).toEqual([ + '--ro-bind', + argsDir, + argsDir, + ]) + expect( + options.filter(w => w === '--ro-bind').length, + ).toBeGreaterThanOrEqual(2000) + expect(options).toContain(files[0]) + const setenv = options.indexOf('--setenv') + expect(options.slice(setenv, setenv + 3)).toEqual([ + '--setenv', + 'SRT_TEST_VAR', + "value with spaces and 'quotes'", + ]) + // The trailer stays on the line, not in the file. + expect(options).not.toContain('--') + expect(options).not.toContain('-c') + + // A file never spawned goes with the other per-command artifacts; the + // directory stays for the process (a sandbox launched earlier may still + // have it bound) and goes at exit. + cleanupBwrapMountPoints() + expect(existsSync(argsFile)).toBe(false) + expect(existsSync(argsDir)).toBe(true) + cleanupBwrapMountPoints({ force: true }) + expect(existsSync(argsDir)).toBe(true) + }) + + it('switches to --args exactly where one argument would exceed 128 KiB', async () => { + // The command is the last word on the line; a trailing two-byte + // character keeps the shell quoter's output constant while every + // added 'a' adds one byte, so the padding sets the rendered size byte + // for byte — and a regression to string length (UTF-16 units) would + // miscount it by one. + const files = flatFiles(20) + const base = await wrap(files, { command: 'é' }) + expect(base).not.toContain('--args') + const renderedAt = (bytes: number) => + wrap(files, { + command: 'a'.repeat(bytes - Buffer.byteLength(base)) + 'é', + }) + + const fits = await renderedAt(128 * 1024 - 1) + expect(Buffer.byteLength(fits)).toBe(128 * 1024 - 1) + expect(fits).not.toContain('--args') + + const overflows = await renderedAt(128 * 1024) + expect(overflows).toMatch( + /^\{ command rm -f -- \S+; exec bwrap --args 9 -- /, + ) + }) + + it('steps aside from an fd the seccomp helper is passed on, and refuses a command that cannot fit at all', async () => { + // An embedder that hands its helper binary over as /proc/self/fd/9 + // would lose it to the redirect; the args move to fd 8. + const files = flatFiles(2000) + const wrapped = await wrap(files, { + seccompConfig: { applyPath: '/proc/self/fd/9', argv0: 'apply-seccomp' }, + }) + expect(wrapped).toMatch(/^\{ command rm -f -- \S+; exec bwrap --args 8 -- /) + expect(wrapped).toMatch(/ 8<\S+$/) + expect(wrapped).toContain('/proc/self/fd/9') + + // The options are already in the file; a command past 128 KiB on its + // own has nowhere to go, and the caller hears why instead of E2BIG. + let thrown: unknown + try { + await wrap(files, { command: 'a'.repeat(128 * 1024) }) + } catch (err) { + thrown = err + } + expect(String(thrown)).toMatch(/too long for one shell argument/) + }) + + it.if(BWRAP_CAN_NAMESPACE)( + 'e2e: bwrap applies the profile from the file, and the command sees neither the fd nor a writable --args directory', + async () => { + // The directory is created by the first wrap of the process, so a + // small one names it for the command below. + const argsDir = argsDirOf(await wrap(flatFiles(1))) + const probe = join(argsDir, 'srt-args-probe') + // tmpdir writable inside the sandbox: the case the trailing ro-bind + // exists for. Fewer, longer-named masks than the shape test: every + // mount costs bwrap time, and the runner's tmpdir is scanned shallowly + // for the same reason. + const files = flatFiles(700, `${'a'.repeat(150)}-`) + const wrapped = await wrap(files, { + allowOnly: [tmpdir()], + mandatoryDenySearchDepth: 1, + command: [ + // The mask is a bind of /dev/null: a character device in place + // of the file (opening a device node inside the user namespace + // is not portable across hosts, so its type is the oracle). + `[ -c ${files[0]} ] && echo MASKED || echo UNMASKED`, + '[ -e /proc/self/fd/9 ] && echo FD9_OPEN || echo FD9_CLOSED', + `touch ${probe} 2>/dev/null && echo ARGS_WRITABLE || echo ARGS_READONLY`, + ].join('; '), + }) + const argsFile = argsFileOf(wrapped) + expect(dirname(argsFile)).toBe(argsDir) + const run = spawnSync(wrapped, { + shell: true, + encoding: 'utf8', + timeout: 60000, + cwd: BASE, + }) + expect(run.stderr ?? '').not.toMatch(/--args|Exceeded maximum/) + expect(run.status).toBe(0) + const lines = run.stdout.trim().split('\n') + expect(lines[0]).toBe('MASKED') // the mask from the file applied + expect(readFileSync(files[0]!, 'utf8')).toBe('secret\n') // host intact + expect(lines[1]).toBe('FD9_CLOSED') + expect(lines[2]).toBe('ARGS_READONLY') + expect(existsSync(probe)).toBe(false) + // Unlinked by the spawn itself. + expect(existsSync(argsFile)).toBe(false) + }, + 60_000, + ) +})