diff --git a/src/cli.ts b/src/cli.ts index 2a099ad38..8109b56d5 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -426,6 +426,20 @@ async function main(): Promise { controlStream.on('error', onControlError) } + // Until there is a child to pass an interrupt on to, it stops the + // run. The library's own handlers, which initialize() installs, only + // clean up: left to them the command would be run all the same. + const interrupted = new AbortController() + const interrupt = (signal: NodeJS.Signals): void => + interrupted.abort(new Error(`interrupted by ${signal}`)) + process.once('SIGINT', interrupt) + process.once('SIGTERM', interrupt) + const aboutToSpawn = (): void => { + interrupted.signal.throwIfAborted() + process.off('SIGINT', interrupt) + process.off('SIGTERM', interrupt) + } + // Initialize sandbox with config logForDebugging('Initializing sandbox...') await SandboxManager.initialize(runtimeConfig) @@ -515,14 +529,21 @@ async function main(): Promise { // entries to the child as CRT descriptors, so the control fd // is not among them (an inheritable HANDLE still reaches the // child, but unnamed — nothing there can find it). + aboutToSpawn() child = spawn(argv[0], argv.slice(1), { shell: false, stdio: 'inherit', env, }) } else { - const sandboxedCommand = - await SandboxManager.wrapWithSandbox(command) + // The wrap can take seconds over a large tree. + const sandboxedCommand = await SandboxManager.wrapWithSandbox( + command, + undefined, + undefined, + interrupted.signal, + ) + aboutToSpawn() child = spawn(sandboxedCommand, { shell: true, stdio: sandboxedStdio(controlFd), diff --git a/src/sandbox/linux-sandbox-utils.ts b/src/sandbox/linux-sandbox-utils.ts index 4b22a9557..6a6a2ab12 100644 --- a/src/sandbox/linux-sandbox-utils.ts +++ b/src/sandbox/linux-sandbox-utils.ts @@ -372,6 +372,9 @@ async function linuxGetMandatoryDenyPaths( ripgrepConfig, ) } catch (error) { + // Stopped, it found nothing: the caller must not be handed a command + // without the denies it would have found. + signal.throwIfAborted() logForDebugging(`[Sandbox] ripgrep scan failed: ${error}`) } diff --git a/src/sandbox/read-deny-glob.ts b/src/sandbox/read-deny-glob.ts index 0a1d7915c..34c83bf08 100644 --- a/src/sandbox/read-deny-glob.ts +++ b/src/sandbox/read-deny-glob.ts @@ -5,7 +5,9 @@ import { normalizePathForSandbox, pathSpellings, properAncestors, - walkGlobPattern, + type Steps, + finish, + walkGlobPatternSteps, } from './sandbox-utils.js' /** @@ -79,7 +81,24 @@ export function expandReadDenyGlobLinux( unlistableDirs?: Set, listings?: GlobWalkListings, ): string[] { - const walk = walkGlobPattern(globPattern, { + return finish( + expandReadDenyGlobLinuxSteps( + globPattern, + reExposedPaths, + unlistableDirs, + listings, + ), + ) +} + +/** {@link expandReadDenyGlobLinux}, in the walk's steps. */ +export function* expandReadDenyGlobLinuxSteps( + globPattern: string, + reExposedPaths: readonly string[], + unlistableDirs?: Set, + listings?: GlobWalkListings, +): Steps { + const walk = yield* walkGlobPatternSteps(globPattern, { withDirectoryForm: true, followSymlinkedDirectories: true, listings, diff --git a/src/sandbox/sandbox-manager.ts b/src/sandbox/sandbox-manager.ts index b8e6c8611..1c0d8a97b 100644 --- a/src/sandbox/sandbox-manager.ts +++ b/src/sandbox/sandbox-manager.ts @@ -49,7 +49,7 @@ import { cleanupBwrapMountPoints, linuxGetCwdMandatoryDenyPaths, } from './linux-sandbox-utils.js' -import { expandReadDenyGlobLinux } from './read-deny-glob.js' +import { expandReadDenyGlobLinuxSteps } from './read-deny-glob.js' import { wrapCommandWithSandboxMacOS, startMacOSSandboxLogMonitor, @@ -84,8 +84,11 @@ import { globPatternBaseDir, normalizePathForSandbox, removeTrailingGlobSuffix, - expandGlobPattern, + walkGlobPatternSteps, type GlobWalkListings, + type Steps, + finish, + finishInTurns, attributionKeyFor, decodeSandboxedCommand, encodeSandboxedCommand, @@ -1265,20 +1268,26 @@ function unionDenyReadPaths( * it is passed through whatever characters it contains. Expanded as a * pattern, a name holding `[` matches nothing and the deny is lost. */ -function resolveReadPathEntries( +function* resolveReadPathEntries( paths: readonly string[], - expandGlob: (pattern: string) => string[], + expandGlob: (pattern: string) => Steps, literalPaths: readonly string[] = [], -): string[] { +): Steps { const literal = new Set(literalPaths) - return paths.flatMap(p => { + const resolved: string[] = [] + for (const p of paths) { const stripped = removeTrailingGlobSuffix(p) - return getPlatform() === 'linux' && + if ( + getPlatform() === 'linux' && !literal.has(p) && containsGlobChars(stripped) - ? expandGlob(p) - : [stripped] - }) + ) { + resolved.push(...(yield* expandGlob(p))) + } else { + resolved.push(stripped) + } + } + return resolved } /** @@ -1298,8 +1307,8 @@ function stripWriteGlobs(paths: readonly string[]): string[] { }) } -function expandAllowReadGlob(pattern: string): string[] { - const expanded = expandGlobPattern(pattern) +function* expandAllowReadGlob(pattern: string): Steps { + const expanded = (yield* walkGlobPatternSteps(pattern)).matches logForDebugging( `[Sandbox] Expanded allowRead glob pattern "${pattern}" to ${expanded.length} paths on Linux`, ) @@ -1326,23 +1335,27 @@ function getFsReadConfig(): FsReadRestrictionConfig { ) // allowRead (re-allow within denied regions) is resolved first: the // denyRead glob expansion collapses against it. - const allowPaths = resolveReadPathEntries( - config.filesystem.allowRead ?? [], - expandAllowReadGlob, + const allowPaths = finish( + resolveReadPathEntries( + config.filesystem.allowRead ?? [], + expandAllowReadGlob, + ), ) const reExposedPaths = [...allowPaths, ...getFsWriteConfig().allowOnly] const unlistableDenyDirs = new Set() const listings: GlobWalkListings = new Map() - const denyPaths = resolveReadPathEntries( - unionDenyReadPaths(config.filesystem.denyRead, credentialRestrictions), - pattern => - expandReadDenyGlobLinux( - pattern, - reExposedPaths, - unlistableDenyDirs, - listings, - ), - credentialRestrictions.degradeToDenyPaths, + const denyPaths = finish( + resolveReadPathEntries( + unionDenyReadPaths(config.filesystem.denyRead, credentialRestrictions), + pattern => + expandReadDenyGlobLinuxSteps( + pattern, + reExposedPaths, + unlistableDenyDirs, + listings, + ), + credentialRestrictions.degradeToDenyPaths, + ), ) return { @@ -1649,6 +1662,7 @@ async function wrapWithSandbox( const platform = getPlatform() const commandId = options?.commandId registerCommandText(command, options) + const startedWith = config // filesystem.disabled bypasses ALL filesystem rule generation. Both // platform wrappers treat readConfig/writeConfig === undefined as "no @@ -1708,9 +1722,14 @@ async function wrapWithSandbox( // allowRead is resolved first: on Linux a denyRead glob's expansion is // collapsed against the paths that re-expose contents under a denied // directory (allowRead + allowWrite), so both must be final here. - const expandedAllowRead = resolveReadPathEntries( - customConfig?.filesystem?.allowRead ?? config?.filesystem.allowRead ?? [], - expandAllowReadGlob, + const expandedAllowRead = await finishInTurns( + resolveReadPathEntries( + customConfig?.filesystem?.allowRead ?? + config?.filesystem.allowRead ?? + [], + expandAllowReadGlob, + ), + abortSignal, ) // The TLS-termination CA cert and the trust bundle the env vars point at // (NODE_EXTRA_CA_CERTS etc.) must be readable by the child, even if their @@ -1725,19 +1744,24 @@ async function wrapWithSandbox( const reExposedPaths = [...expandedAllowRead, ...writeConfig.allowOnly] const unlistableDenyDirs = new Set() const listings: GlobWalkListings = new Map() - const expandedDenyRead = resolveReadPathEntries( - unionDenyReadPaths( - customConfig?.filesystem?.denyRead ?? config?.filesystem.denyRead ?? [], - credentialRestrictions, - ), - pattern => - expandReadDenyGlobLinux( - pattern, - reExposedPaths, - unlistableDenyDirs, - listings, + const expandedDenyRead = await finishInTurns( + resolveReadPathEntries( + unionDenyReadPaths( + customConfig?.filesystem?.denyRead ?? + config?.filesystem.denyRead ?? + [], + credentialRestrictions, ), - credentialRestrictions.degradeToDenyPaths, + pattern => + expandReadDenyGlobLinuxSteps( + pattern, + reExposedPaths, + unlistableDenyDirs, + listings, + ), + credentialRestrictions.degradeToDenyPaths, + ), + abortSignal, ) readConfig = { denyOnly: expandedDenyRead, @@ -1770,6 +1794,21 @@ async function wrapWithSandbox( await waitForNetworkInitialization() } + // INVARIANT: one wrap, one configuration. updateConfig() can run in any of + // the turns above, and what was read before it must not be put together + // with what is read after it: the write list of one configuration with the + // allowGitConfig of another is a policy nobody chose. So the wrap starts + // over. Nothing from here to the platform's wrapper awaits. + if (config !== startedWith) { + return wrapWithSandbox( + command, + binShell, + customConfig, + abortSignal, + options, + ) + } + // Check custom config to allow pseudo-terminal (can be applied dynamically) const allowPty = customConfig?.allowPty ?? config?.allowPty diff --git a/src/sandbox/sandbox-utils.ts b/src/sandbox/sandbox-utils.ts index 7fa101423..e17a72854 100644 --- a/src/sandbox/sandbox-utils.ts +++ b/src/sandbox/sandbox-utils.ts @@ -1141,6 +1141,50 @@ export interface ExpandGlobOptions { * pattern, reads the same entries. */ export type GlobWalkListings = Map +export type GlobWalkOptions = ExpandGlobOptions & { + withDirectoryForm?: boolean + followSymlinkedDirectories?: boolean + /** Handed to every walk of one configuration, so that patterns with a + * base in common list each directory once between them. */ + listings?: GlobWalkListings +} + +/** Work that can be left between two steps and taken up again. */ +export type Steps = Generator + +/** Runs `steps` to the end, on the spot. */ +export function finish(steps: Steps): T { + for (;;) { + const step = steps.next() + if (step.done) return step.value + } +} + +/** How long {@link finishInTurns} works before it lets others have a turn. */ +const TURN_MS = 10 + +/** + * Runs `steps` to the end, letting the event loop have a turn every + * {@link TURN_MS}: a walk of a large tree takes seconds, and a caller whose + * thread is held that long can neither draw nor hear that it should stop. + * Rejects with `signal`'s reason once that is aborted, however far it got. + */ +export async function finishInTurns( + steps: Steps, + signal?: AbortSignal, +): Promise { + signal?.throwIfAborted() + let since = performance.now() + for (;;) { + const step = steps.next() + if (!step.done && performance.now() - since < TURN_MS) continue + if (!step.done) await new Promise(resolve => setImmediate(resolve)) + signal?.throwIfAborted() + if (step.done) return step.value + since = performance.now() + } +} + /** What one recursive walk of a glob's base directory found; see {@link walkGlobPattern}. */ export interface GlobWalk { /** Where the walk started, with symlinks resolved (the spelling itself @@ -1489,14 +1533,16 @@ export function toForwardSlashes(s: string): string { */ export function walkGlobPattern( globPath: string, - opts: ExpandGlobOptions & { - withDirectoryForm?: boolean - followSymlinkedDirectories?: boolean - /** Handed to every walk of one configuration, so that patterns with a - * base in common list each directory once between them. */ - listings?: GlobWalkListings - } = {}, + opts: GlobWalkOptions = {}, ): GlobWalk { + return finish(walkGlobPatternSteps(globPath, opts)) +} + +/** {@link walkGlobPattern}, a step to a directory. */ +export function* walkGlobPatternSteps( + globPath: string, + opts: GlobWalkOptions = {}, +): Steps { const walk: GlobWalk = { baseLocation: '', matches: [], @@ -1621,6 +1667,7 @@ export function walkGlobPattern( // others it came with, so only the ones new to this directory are taken. const fresh = frame.positions.filter(p => !listed.has(p)) if (fresh.length === 0) continue + yield for (const p of fresh) listed.add(p) let entries = listings.get(real) try { diff --git a/test/cli.test.ts b/test/cli.test.ts index 654490796..fc8b8c9bf 100644 --- a/test/cli.test.ts +++ b/test/cli.test.ts @@ -1,5 +1,5 @@ import { describe, test, expect } from 'bun:test' -import { spawnSync } from 'node:child_process' +import { spawn, spawnSync } from 'node:child_process' import { mkdirSync, mkdtempSync, @@ -9,6 +9,7 @@ import { } from 'node:fs' import { tmpdir } from 'node:os' import { join } from 'node:path' +import { isLinux } from './helpers/platform.js' /** * Get the path to the CLI entry point @@ -306,4 +307,63 @@ describe('CLI', () => { expect(result.status).toBe(0) }) }) + + describe.if(isLinux)('an interrupt before the command has started', () => { + for (const [when, lastLine] of [ + // The library's handlers are in place, and there is more to wait for. + ['the sandbox is being initialized', 'Starting HTTP bridge'], + // The last thing the CLI says before it wraps the command. + ['the command is being wrapped', 'Original command'], + ] as const) { + test(`stops the run while ${when}, and the command is not run`, async () => { + // Enough directories for the walk of the pattern to outlast the signal. + const root = mkdtempSync(join(tmpdir(), 'cli-interrupt-')) + try { + for (let i = 0; i < 200; i++) { + for (let j = 0; j < 100; j++) { + mkdirSync(join(root, 'tree', `d${i}`, `e${j}`), { + recursive: true, + }) + } + } + const settings = join(root, 'settings.json') + writeFileSync( + settings, + JSON.stringify({ + network: { allowedDomains: [], deniedDomains: [] }, + filesystem: { + denyRead: [join(root, 'tree', '**/*.pem')], + allowWrite: [], + denyWrite: [], + }, + }), + ) + const child = spawn( + 'bun', + ['run', getCliPath(), '-s', settings, 'echo', 'RAN'], + { env: { ...process.env, SRT_DEBUG: 'true' } }, + ) + let stdout = '' + let stderr = '' + let signalled = false + child.stdout.on('data', (chunk: Buffer) => (stdout += String(chunk))) + child.stderr.on('data', (chunk: Buffer) => { + stderr += String(chunk) + if (!signalled && stderr.includes(lastLine)) { + signalled = true + child.kill('SIGINT') + } + }) + const status = await new Promise(resolve => child.on('exit', resolve)) + + expect(signalled).toBe(true) + expect(stderr).toContain('interrupted by SIGINT') + expect(stdout).not.toContain('RAN') + expect(status).toBe(1) + } finally { + rmSync(root, { recursive: true, force: true }) + } + }, 60_000) + } + }) }) diff --git a/test/sandbox/glob-expand.test.ts b/test/sandbox/glob-expand.test.ts index 9f0d90ef8..96ff621da 100644 --- a/test/sandbox/glob-expand.test.ts +++ b/test/sandbox/glob-expand.test.ts @@ -18,10 +18,14 @@ import { join } from 'node:path' import { expandGlobPattern, expandTilde, + finish, + finishInTurns, globPatternBaseDir, globToRegex, normalizePathForSandbox, + type Steps, walkGlobPattern, + walkGlobPatternSteps, } from '../../src/sandbox/sandbox-utils.js' import { containsGlobCharsWin, @@ -762,6 +766,88 @@ describe.if(!isWindows)('walkGlobPattern', () => { // expandTilde — `~\` form is Windows-only // ============================================================================ +describe('finishInTurns', () => { + /** `count` steps of about `ms` each; returns how many it took. */ + function* busy( + count: number, + ms: number, + taken = { steps: 0 }, + ): Steps { + for (let i = 0; i < count; i++) { + const until = performance.now() + ms + while (performance.now() < until); + taken.steps++ + yield + } + return taken.steps + } + + it('lets the event loop have a turn while the steps go on', async () => { + const taken = { steps: 0 } + let stepsWhenTheTimerFired = -1 + setTimeout(() => (stepsWhenTheTimerFired = taken.steps), 0) + expect(await finishInTurns(busy(30, 3, taken))).toBe(30) + expect(stepsWhenTheTimerFired).toBeGreaterThan(0) + expect(stepsWhenTheTimerFired).toBeLessThan(30) + }) + + it('takes no step for a signal already aborted', async () => { + const taken = { steps: 0 } + const reason = new Error('stopped') + expect( + await finishInTurns(busy(3, 0, taken), AbortSignal.abort(reason)).catch( + (e: unknown) => e, + ), + ).toBe(reason) + expect(taken.steps).toBe(0) + }) + + it("stops at the next turn with the signal's reason", async () => { + const taken = { steps: 0 } + const controller = new AbortController() + const reason = new Error('stopped') + setTimeout(() => controller.abort(reason), 0) + expect( + await finishInTurns(busy(30, 3, taken), controller.signal).catch( + (e: unknown) => e, + ), + ).toBe(reason) + expect(taken.steps).toBeLessThan(30) + }) + + it('hands nothing out when the signal is aborted in the last step', async () => { + const controller = new AbortController() + const reason = new Error('stopped') + function* abortsAtTheEnd(): Steps { + yield + controller.abort(reason) + return 'done' + } + expect( + await finishInTurns(abortsAtTheEnd(), controller.signal).catch( + (e: unknown) => e, + ), + ).toBe(reason) + }) + + it('finds what the walk finds on the spot', async () => { + const root = realPath(mkdtempSync(join(tmpdir(), 'glob-walk-turns-'))) + try { + mkdirSync(join(root, 'a', 'b'), { recursive: true }) + writeFileSync(join(root, 'a', 'b', 'id.pem'), '') + symlinkSync(join(root, 'a'), join(root, 'link')) + const opts = { followSymlinkedDirectories: true, withDirectoryForm: true } + const pattern = join(root, '**/*.pem') + const inTurns = await finishInTurns(walkGlobPatternSteps(pattern, opts)) + expect(inTurns).toEqual(walkGlobPattern(pattern, opts)) + expect(inTurns).toEqual(finish(walkGlobPatternSteps(pattern, opts))) + expect(inTurns.matches).toEqual([join(root, 'a', 'b', 'id.pem')]) + } finally { + rmSync(root, { recursive: true, force: true }) + } + }) +}) + describe('expandTilde', () => { it.if(!isWindows)( 'should NOT expand `~\\` on POSIX (literal filename byte)', @@ -1272,6 +1358,164 @@ describe.if(isLinux)('getFsReadConfig with glob patterns on Linux', () => { } }) + for (const list of ['denyRead', 'allowRead'] as const) { + it(`gives up a wrap whose signal is aborted while a ${list} pattern is walked`, async () => { + const { SandboxManager } = await import( + '../../src/sandbox/sandbox-manager.js' + ) + const root = realPath(mkdtempSync(join(tmpdir(), 'glob-walk-abort-'))) + const dirs = Array.from({ length: 12 }, (_, i) => join(root, `d${i}`)) + for (const dir of dirs) mkdirSync(dir) + + await SandboxManager.reset() + await SandboxManager.initialize({ + network: { allowedDomains: [], deniedDomains: [] }, + filesystem: { + denyRead: list === 'denyRead' ? [join(root, '**/*.pem')] : [root], + allowRead: list === 'allowRead' ? [join(root, '**/*.pem')] : [], + allowWrite: [], + denyWrite: [], + }, + }) + + // Each listing takes longer than a turn, and the third one aborts. + const controller = new AbortController() + const reason = new Error('stopped') + let listed = 0 + const readdirSync = fs.readdirSync + const readdirSpy = spyOn(fs, 'readdirSync').mockImplementation((( + ...args: Parameters + ) => { + if (String(args[0]).startsWith(root)) { + if (++listed === 3) controller.abort(reason) + const until = performance.now() + 15 + while (performance.now() < until); + } + return readdirSync(...args) + }) as typeof fs.readdirSync) + try { + expect( + await SandboxManager.wrapWithSandbox( + 'true', + undefined, + undefined, + controller.signal, + ).catch((e: unknown) => e), + ).toBe(reason) + expect(listed).toBe(3) + } finally { + readdirSpy.mockRestore() + await SandboxManager.reset() + rmSync(root, { recursive: true, force: true }) + } + }) + } + + it('wraps by one configuration when that is replaced in a turn', async () => { + const { SandboxManager } = await import( + '../../src/sandbox/sandbox-manager.js' + ) + const root = realPath(mkdtempSync(join(tmpdir(), 'glob-walk-replaced-'))) + for (let i = 0; i < 6; i++) { + mkdirSync(join(root, 'tree', `d${i}`), { recursive: true }) + } + mkdirSync(join(root, '.git')) + for (const name of ['first', 'second', '.git/config']) { + writeFileSync(join(root, name), '') + } + // They differ in what the wrap reads before its first turn and after its + // last: put together, the first's write list and the second's + // allowGitConfig would leave .git/config writable, which neither does. + const network = { allowedDomains: [], deniedDomains: [] } + const allowRead = [join(root, 'tree', '**/*.pem')] + const first = { + network, + filesystem: { + denyRead: [join(root, 'first')], + allowRead, + allowWrite: [root], + denyWrite: [], + }, + } + const second = { + network, + filesystem: { + denyRead: [join(root, 'second')], + allowRead, + allowWrite: [], + denyWrite: [], + allowGitConfig: true, + }, + } + + const cwd = process.cwd() + process.chdir(root) + await SandboxManager.reset() + await SandboxManager.initialize(first) + + // Each listing takes longer than a turn; the configuration is replaced + // while the allowRead pattern, the first to be walked, is under way. + let listed = 0 + const readdirSync = fs.readdirSync + const readdirSpy = spyOn(fs, 'readdirSync').mockImplementation((( + ...args: Parameters + ) => { + if (String(args[0]).startsWith(root)) { + if (++listed === 2) SandboxManager.updateConfig(second) + const until = performance.now() + 15 + while (performance.now() < until); + } + return readdirSync(...args) + }) as typeof fs.readdirSync) + try { + const disturbed = await SandboxManager.wrapWithSandbox('true') + readdirSpy.mockRestore() + + expect(listed).toBeGreaterThan(2) + expect(disturbed).toBe(await SandboxManager.wrapWithSandbox('true')) + expect(disturbed).toContain(join(root, 'second')) + expect(disturbed).not.toContain(join(root, 'first')) + } finally { + readdirSpy.mockRestore() + process.chdir(cwd) + await SandboxManager.reset() + rmSync(root, { recursive: true, force: true }) + } + }) + + it('gives up a wrap whose signal is aborted while the mandatory denies are looked for', async () => { + const { SandboxManager } = await import( + '../../src/sandbox/sandbox-manager.js' + ) + const root = realPath(mkdtempSync(join(tmpdir(), 'glob-walk-scan-'))) + // Stands in for ripgrep, and outlasts the abort. + const slowScan = join(root, 'slow-scan') + writeFileSync(slowScan, '#!/bin/sh\nsleep 5\n', { mode: 0o755 }) + + await SandboxManager.reset() + await SandboxManager.initialize({ + network: { allowedDomains: [], deniedDomains: [] }, + filesystem: { denyRead: [], allowWrite: [root], denyWrite: [] }, + ripgrep: { command: slowScan }, + }) + const controller = new AbortController() + const reason = new Error('stopped') + setTimeout(() => controller.abort(reason), 200) + try { + expect( + await SandboxManager.wrapWithSandbox( + 'true', + undefined, + undefined, + controller.signal, + ).catch((e: unknown) => e), + ).toBe(reason) + } finally { + await SandboxManager.reset() + rmSync(root, { recursive: true, force: true }) + } + }) + it('should pass non-glob paths through unchanged on Linux', async () => { const { SandboxManager } = await import( '../../src/sandbox/sandbox-manager.js'