diff --git a/src/sandbox/sandbox-manager.ts b/src/sandbox/sandbox-manager.ts index de9a5b84e..ec380ee1f 100644 --- a/src/sandbox/sandbox-manager.ts +++ b/src/sandbox/sandbox-manager.ts @@ -1876,6 +1876,47 @@ const RESTARTS_IN_TURNS = 2 /** In place of what a walk finds: the configuration was replaced under it. */ const REPLACED = Symbol('replaced') +/** Whether a wrap is walking, and the ones waiting to, first come first. */ +let walking = false +const waitingToWalk: (() => void)[] = [] + +/** + * Resolves once no other wrap is walking, with what says that this one's walk + * has ended; only the first call of it counts. Walks that take turns on one + * thread end no sooner for it, while each keeps all it has listed until it + * ends and has its turn before the event loop gets one: the memory and the + * thread held would both grow with the wraps in flight. + * Rejects with `signal`'s reason as soon as that is aborted during the wait. + * Nothing else takes a wrap out of the line, reset() included. + */ +async function waitToWalk(signal?: AbortSignal): Promise<() => void> { + if (walking) { + signal?.throwIfAborted() + await new Promise((resolve, reject) => { + const go = (): void => { + signal?.removeEventListener('abort', leave) + resolve() + } + const leave = (): void => { + waitingToWalk.splice(waitingToWalk.indexOf(go), 1) + reject(signal?.reason) + } + waitingToWalk.push(go) + signal?.addEventListener('abort', leave, { once: true }) + }) + } + walking = true + let ended = false + return () => { + if (ended) return + ended = true + // Handed on while `walking` stays set, so that no newcomer gets in first. + const next = waitingToWalk.shift() + if (next) next() + else walking = false + } +} + async function wrapWithSandbox( command: string, binShell?: string, @@ -1901,6 +1942,35 @@ async function wrapWithSandboxAgain( customConfig?: Partial, abortSignal?: AbortSignal, options?: WrapWithSandboxOptions, +): Promise { + // INVARIANT: an attempt waits before it reads anything, so that it goes by + // the configuration, the working directory and the disk it finds afterwards. + const walkEnded = await waitToWalk(abortSignal) + try { + return await wrapWithSandboxOnce( + walkEnded, + restarts, + command, + binShell, + customConfig, + abortSignal, + options, + ) + } finally { + walkEnded() + } +} + +/** One attempt of {@link wrapWithSandboxAgain}, which calls `walkEnded` once + * it walks no more. */ +async function wrapWithSandboxOnce( + walkEnded: () => void, + restarts: number, + command: string, + binShell?: string, + customConfig?: Partial, + abortSignal?: AbortSignal, + options?: WrapWithSandboxOptions, ): Promise { const platform = getPlatform() const commandId = options?.commandId @@ -1912,8 +1982,10 @@ async function wrapWithSandboxAgain( config !== startedWith || configInstalls !== startedAt || workingDirectory() !== startedIn - const startOver = (): Promise => - wrapWithSandboxAgain( + const startOver = (): Promise => { + // The next attempt waits like any other wrap, behind the ones there are. + walkEnded() + return wrapWithSandboxAgain( restarts + 1, command, binShell, @@ -1921,6 +1993,7 @@ async function wrapWithSandboxAgain( abortSignal, options, ) + } /** `steps`, left as soon as the host is seen to have moved on: what they * would go on to find is for a wrap that starts over anyway. */ function* whileCurrent(steps: Steps): Steps { @@ -2051,6 +2124,9 @@ async function wrapWithSandboxAgain( credentialRestrictions.degradeToDenyPaths, ), ) + // Emptied by hand: a runtime may keep what this function has named for as + // long as it is suspended, which is until the scan below has ended. + listings.clear() if (expandedDenyRead === REPLACED) return startOver() readConfig = { denyOnly: expandedDenyRead, @@ -2062,6 +2138,8 @@ async function wrapWithSandboxAgain( ), } } + // Nothing below walks, and what it waits for is not this thread. + walkEnded() // Check if network config is specified - this determines if we need network restrictions // Network restriction is needed when: diff --git a/test/sandbox/glob-expand.test.ts b/test/sandbox/glob-expand.test.ts index fc87b7f78..302ffcb86 100644 --- a/test/sandbox/glob-expand.test.ts +++ b/test/sandbox/glob-expand.test.ts @@ -1,4 +1,14 @@ -import { describe, it, expect, beforeAll, afterAll, spyOn } from 'bun:test' +/// +import { + describe, + it, + expect, + beforeAll, + beforeEach, + afterAll, + afterEach, + spyOn, +} from 'bun:test' import * as fc from 'fast-check' // The namespace of the same module production binds (sandbox-utils.ts does // `import * as fs from 'fs'`), so a spy on it is seen by the code under test. @@ -20,6 +30,7 @@ import type { FilesystemPathEntry, SandboxRuntimeConfig, } from '../../src/sandbox/sandbox-config.js' +import type { ISandboxManager } from '../../src/sandbox/sandbox-manager.js' import { expandGlobPattern, expandTilde, @@ -2304,6 +2315,207 @@ describe.if(isLinux)('getFsReadConfig with glob patterns on Linux', () => { }) }) +// ============================================================================ +// Tests for wraps asked for while another one walks, on Linux +// ============================================================================ + +describe.if(isLinux)('wrapWithSandbox while another wrap walks', () => { + let SandboxManager: ISandboxManager + const cwd = process.cwd() + let root: string + /** The tree each listing was in: `a`, `b` or `c`, three listings each. */ + let listed: string + /** Runs in each listing, with how many there have been and what it found. */ + let inListing: (nth: number, entries: object) => void + let readdirSpy: { mockClear(): void; mockRestore(): void } + + const configured = (denyRead: string[], allowWrite: string[]) => ({ + network: { allowedDomains: [], deniedDomains: [] }, + filesystem: { + denyRead: denyRead.map(pattern => join(root, pattern)), + allowWrite, + denyWrite: [], + }, + }) + /** A wrap that walks `tree`, whatever is configured. */ + const wrapWalking = (tree: string, signal?: AbortSignal): Promise => + SandboxManager.wrapWithSandbox( + 'true', + undefined, + { filesystem: configured([`${tree}/**/*.pem`], []).filesystem }, + signal, + ) + + beforeEach(async () => { + ;({ SandboxManager } = await import('../../src/sandbox/sandbox-manager.js')) + root = realPath(mkdtempSync(join(tmpdir(), 'glob-walk-line-'))) + for (const tree of ['a', 'b', 'c']) { + mkdirSync(join(root, tree, 'from'), { recursive: true }) + mkdirSync(join(root, tree, 'to')) + } + listed = '' + inListing = () => {} + // Each listing takes longer than a turn. + const readdirSync = fs.readdirSync + readdirSpy = spyOn(fs, 'readdirSync').mockImplementation((( + ...args: Parameters + ) => { + const entries = readdirSync(...args) + if (String(args[0]).startsWith(root)) { + listed += String(args[0])[root.length + 1] + inListing(listed.length, entries) + const until = performance.now() + 15 + while (performance.now() < until); + } + return entries + }) as typeof fs.readdirSync) + process.chdir(join(root, 'c', 'from')) + await SandboxManager.reset() + await SandboxManager.initialize(configured(['b/**/.env'], ['.', root])) + }) + + afterEach(async () => { + readdirSpy.mockRestore() + process.chdir(cwd) + await SandboxManager.reset() + rmSync(root, { recursive: true, force: true }) + }) + + it('walks for one wrap at a time, in the order they were asked for', async () => { + await Promise.all(['a', 'b', 'c'].map(tree => wrapWalking(tree))) + expect(listed).toBe('aaabbbccc') + }) + + // Aborted before it is asked for, and while it waits. + it.each([0, 2])( + 'gives up an aborted wrap before its place comes, and goes on to the next (aborted after %d listings)', + async at => { + const controller = new AbortController() + const reason = new Error('stopped') + inListing = nth => { + if (nth === at) controller.abort(reason) + } + inListing(0, []) + let listedWhenGivenUp = '' + const [, givenUp] = await Promise.all([ + wrapWalking('a'), + wrapWalking('b', controller.signal).catch((e: unknown) => { + listedWhenGivenUp = listed + return e + }), + wrapWalking('c'), + ]) + expect(givenUp).toBe(reason) + expect(listedWhenGivenUp.length).toBeLessThan(3) + expect(listed).toBe('aaaccc') + }, + ) + + it('goes on to the next when the wrap that walks is aborted, and when it throws', async () => { + const controller = new AbortController() + inListing = nth => { + if (nth === 2) controller.abort(new Error('stopped')) + } + const outcomes = await Promise.allSettled([ + wrapWalking('a', controller.signal), + // Neither a path nor a marked one. + SandboxManager.wrapWithSandbox('true', undefined, { + filesystem: { denyRead: [{} as never], allowWrite: [], denyWrite: [] }, + }), + wrapWalking('b'), + ]) + expect(outcomes.map(outcome => outcome.status)).toEqual([ + 'rejected', + 'rejected', + 'fulfilled', + ]) + expect(listed).toBe('aabbb') + }) + + // What the host does in the second listing of `a`, with a wrap by the + // configuration waiting. That one has read nothing yet, so it walks `b` once; + // the wrap that was walking starts over, behind it. + it.each([ + [ + 'the configuration installed', + () => { + writeFileSync(join(root, 'b', 'key.pem'), '') + SandboxManager.updateConfig(configured(['b/**/.env', 'b/**/*.pem'], [])) + }, + 'b/key.pem', + 'c/from', + ], + [ + 'the working directory the host moved to', + () => process.chdir(join(root, 'c', 'to')), + 'c/to', + 'c/from', + ], + ])('goes by %s while it waited', async (_, disturb, holds, lacks) => { + inListing = nth => { + if (nth === 2) disturb() + } + const [, waited] = await Promise.all([ + wrapWalking('a'), + SandboxManager.wrapWithSandbox('true'), + ]) + expect(listed).toBe('aabbbaaa') + expect(waited).toContain(join(root, holds)) + expect(waited).not.toContain(join(root, lacks)) + expect(waited).toBe(await SandboxManager.wrapWithSandbox('true')) + }) + + it('keeps the order through a reset()', async () => { + let reset: Promise | undefined + inListing = nth => { + if (nth === 2) reset = SandboxManager.reset() + } + // Either may find the network bridge gone, as any wrap during a reset(). + await Promise.allSettled([wrapWalking('a'), wrapWalking('b')]) + await reset + expect(listed).toBe('aaabbb') + }) + + it('has walked, and let go of what it listed, before the mandatory denies are looked for', async () => { + // Stands in for ripgrep: says that it runs, and goes on until told. + const scans = join(root, 'scans') + const enough = join(root, 'enough') + const slowScan = join(root, 'slow-scan') + writeFileSync( + slowScan, + `#!/bin/sh\necho ran >> ${scans}\nfor i in $(seq 300); do [ -e ${enough} ] && exit; sleep 0.01; done\n`, + { mode: 0o755 }, + ) + writeFileSync(scans, '') + SandboxManager.updateConfig({ + ...configured([], []), + ripgrep: { command: slowScan }, + }) + const listings: WeakRef[] = [] + inListing = (_, entries) => listings.push(new WeakRef(entries)) + + let wrapped = 0 + const wrapping = Promise.all( + ['a', 'b'].map(tree => wrapWalking(tree).then(() => wrapped++)), + ) + try { + while (fs.readFileSync(scans, 'utf8') !== 'ran\nran\n') await Bun.sleep(5) + // The second has walked while the first was scanning. + expect(wrapped).toBe(0) + readdirSpy.mockClear() // It keeps what each call returned. + Bun.gc(true) + // Not none: a collector that reads the stack can take a stale word for a + // reference. Held by the wraps, it would be all six. + expect(listings.filter(l => l.deref() !== undefined).length).toBeLessThan( + 6, + ) + } finally { + writeFileSync(enough, '') + await wrapping + } + }) +}) + // ============================================================================ // Tests for getLinuxGlobPatternWarnings // ============================================================================