fix(sandbox): terminate on first drain failure

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<Buffer>[]
+        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<unknown>(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')

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', () => {