diff --git a/packages/sandbox/sandbox-windows-acl/src/index.ts b/packages/sandbox/sandbox-windows-acl/src/index.ts index 231d1cb0c3..ecd7eb4cce 100644 --- a/packages/sandbox/sandbox-windows-acl/src/index.ts +++ b/packages/sandbox/sandbox-windows-acl/src/index.ts @@ -391,24 +391,35 @@ export class AclSandbox { return { pid: native.pid, wait: () => (settlement ??= (async () => { - const drains = await Promise.allSettled([stdout, stderr]) + let drains: PromiseSettledResult[] + try { + const [stdoutBuffer, stderrBuffer] = await Promise.all([stdout, stderr]) + drains = [ + { status: 'fulfilled', value: stdoutBuffer }, + { status: 'fulfilled', value: stderrBuffer }, + ] + } catch (firstDrainFailure) { + if (api.terminateProcess(native.process, 1) === 0) { + const failures: unknown[] = [firstDrainFailure] + const terminationCode = api.getLastError() + try { + closeHandleChecked(api, native.process, 'piped child after drain failure') + } catch (error) { + failures.push(error) + } + failures.push(new Win32Error('TerminateProcess', terminationCode, `pid ${native.pid} after drain failure`)) + void Promise.allSettled([stdout, stderr]) + throw new AggregateError(failures, 'piped child settlement failed') + } + drains = await Promise.allSettled([stdout, stderr]) + } const failures = drains.flatMap(outcome => outcome.status === 'rejected' ? [outcome.reason as unknown] : []) let exitCode = 0 - if (failures.length > 0 && api.terminateProcess(native.process, 1) === 0) { - const terminationCode = api.getLastError() - try { - closeHandleChecked(api, native.process, 'piped child after drain failure') - } catch (error) { - failures.push(error) - } - failures.push(new Win32Error('TerminateProcess', terminationCode, `pid ${native.pid} after drain failure`)) - } else { - try { - exitCode = waitForExit(api, native.process) - } catch (error) { - failures.push(error) - } + try { + exitCode = waitForExit(api, native.process) + } catch (error) { + failures.push(error) } if (failures.length === 1) throw failures[0] if (failures.length > 1) throw new AggregateError(failures, 'piped child settlement failed') diff --git a/packages/sandbox/sandbox-windows-acl/tests/index-failure-paths.spec.ts b/packages/sandbox/sandbox-windows-acl/tests/index-failure-paths.spec.ts index a75f3546d3..78032e2c32 100644 --- a/packages/sandbox/sandbox-windows-acl/tests/index-failure-paths.spec.ts +++ b/packages/sandbox/sandbox-windows-acl/tests/index-failure-paths.spec.ts @@ -466,6 +466,38 @@ describe('AclSandbox spawn', () => { expect(waitForSingleObject).toHaveBeenCalledOnce() }) + it('pipe spawn terminates promptly when one drain fails and the sibling remains open', async () => { + const { api } = state.stubs as HappyStubs + let peekCount = 0 + let terminated = false + let lastError = 5 + api.peekNamedPipe = vi.fn((_handle, _buffer, _size, _read, totalAvail: NativePtr) => { + peekCount += 1 + if (peekCount === 1) return 0 + if (terminated) { + lastError = ERROR_BROKEN_PIPE + return 0 + } + koffi.encode(totalAvail, 'uint32', 0) + return 1 + }) + api.getLastError = vi.fn(() => lastError) + const terminateProcess = vi.fn(() => { + terminated = true + return 1 + }) + api.terminateProcess = terminateProcess + const waitForSingleObject = vi.fn(() => 0) + api.waitForSingleObject = waitForSingleObject + const workspace = scratch() + const sandbox = new AclSandbox({ writableDirs: [workspace], tempDir: null, writeSid: 'S-1-4-9000-14-2-1', mode: 'workspace-write' }) + await sandbox.init() + const child = sandbox.spawn({ command: 'probe.exe' }) + await expect(child.wait()).rejects.toMatchObject({ api: 'PeekNamedPipe' }) + expect(terminateProcess).toHaveBeenCalledOnce() + expect(waitForSingleObject).toHaveBeenCalledOnce() + }) + it('pipe spawn closes the process without waiting when termination after a drain failure fails', async () => { const { api, closeHandle } = state.stubs as HappyStubs api.getLastError = vi.fn(() => 5) @@ -480,6 +512,23 @@ describe('AclSandbox spawn', () => { expect(waitForSingleObject).not.toHaveBeenCalled() expect(closeHandle).toHaveBeenCalled() }) + + it('pipe spawn aggregates process-handle closure failure after termination failure', async () => { + const { api } = state.stubs as HappyStubs + api.getLastError = vi.fn(() => 5) + api.terminateProcess = vi.fn(() => 0) + const workspace = scratch() + const sandbox = new AclSandbox({ writableDirs: [workspace], tempDir: null, writeSid: 'S-1-4-9000-14-4', mode: 'workspace-write' }) + await sandbox.init() + const child = sandbox.spawn({ command: 'probe.exe' }) + api.closeHandle = vi.fn(() => 0) + await expect(child.wait()).rejects.toMatchObject({ + errors: expect.arrayContaining([ + expect.objectContaining({ api: 'CloseHandle' }), + expect.objectContaining({ api: 'TerminateProcess' }), + ]), + }) + }) }) describe('AclSandbox dispose', () => {