From 03186fe93f1215fb2f322a42802aa6993a91b7c9 Mon Sep 17 00:00:00 2001 From: pku-xht Date: Wed, 19 Aug 2026 08:45:54 +0800 Subject: [PATCH] fix(sandbox): cancel sibling drain after child termination --- ...-shared-win32-process-primitives.i18n.yaml | 4 ++-- ...6-08-19-shared-win32-process-primitives.md | 2 +- ...8-19-shared-win32-process-primitives.zh.md | 2 +- .../sandbox/sandbox-windows-acl/src/index.ts | 22 +++++++++++-------- .../tests/index-failure-paths.spec.ts | 16 +++++--------- .../subprocess/win32-process/src/process.ts | 4 ++-- .../win32-process/tests/process.spec.ts | 5 +++-- 7 files changed, 27 insertions(+), 28 deletions(-) diff --git a/.agents/notes/implemented/architecture/2026-08-19-shared-win32-process-primitives.i18n.yaml b/.agents/notes/implemented/architecture/2026-08-19-shared-win32-process-primitives.i18n.yaml index 5e1aeb4d0f..4e5dac463c 100644 --- a/.agents/notes/implemented/architecture/2026-08-19-shared-win32-process-primitives.i18n.yaml +++ b/.agents/notes/implemented/architecture/2026-08-19-shared-win32-process-primitives.i18n.yaml @@ -2,5 +2,5 @@ # side as of the last confirmed-consistent state. Both languages carry equal authority; # after editing either side, bring the other along and re-record with: # pnpm run verify-translation-pairing --write .agents/notes/implemented/architecture/2026-08-19-shared-win32-process-primitives.md -2026-08-19-shared-win32-process-primitives.md: ae7720273333f675b9fb4178405bedbc32982a59 -2026-08-19-shared-win32-process-primitives.zh.md: 2d8b9d3421fa4eb4100f4f016018301e21de1eef +2026-08-19-shared-win32-process-primitives.md: a190fbd78d3f6e33e5626b01a38a9c2cfbb8216f +2026-08-19-shared-win32-process-primitives.zh.md: 79e1ae576bc0d14d0e4f152825171d3d8510bb70 diff --git a/.agents/notes/implemented/architecture/2026-08-19-shared-win32-process-primitives.md b/.agents/notes/implemented/architecture/2026-08-19-shared-win32-process-primitives.md index ae77202733..a190fbd78d 100644 --- a/.agents/notes/implemented/architecture/2026-08-19-shared-win32-process-primitives.md +++ b/.agents/notes/implemented/architecture/2026-08-19-shared-win32-process-primitives.md @@ -14,7 +14,7 @@ The Windows ACL sandbox owns restricted-token, SID, DACL, grant, and workspace p The Windows ACL sandbox remains the only owner of restricted-token creation, SID and DACL policy, grants, writable-path decisions, temporary-directory policy, and the public sandbox child result. It extends the shared binding context with policy-specific APIs, supplies the primary token, combines pipe drains and waits, and closes the caller-owned Job at its lifecycle boundary. -Every native allocation and HANDLE has one owner. A process operation frees its Koffi out-parameters and closes every pipe, thread, process, or Job handle acquired before a failure. Successful pipe creation returns the process plus stdout/stderr read handles to the sandbox; if either drain fails, sandbox settlement terminates the child before its synchronous wait. When termination itself fails, settlement cancels and joins the sibling drain before closing the process handle and reporting the failure, so rejection leaves no polling timer alive. Inherited-stdio creation puts the kill-on-close Job in `STARTUPINFOEXW`, so the child is already Job-owned before any user code can run; attribute or creation failure therefore has one deterministic cleanup owner. The sandbox owns returned process, pipe, and Job handles until wait or disposal. +Every native allocation and HANDLE has one owner. A process operation frees its Koffi out-parameters and closes every pipe, thread, process, or Job handle acquired before a failure. Successful pipe creation returns the process plus stdout/stderr read handles to the sandbox; if either drain fails, sandbox settlement requests direct-child termination, cancels and joins the sibling drain, then performs the direct-child wait only when termination succeeded. A termination failure instead closes the process handle and reports both failures. Either result leaves no polling timer alive even when a descendant inherited a pipe writer. Inherited-stdio creation puts the kill-on-close Job in `STARTUPINFOEXW`, so the child is already Job-owned before any user code can run; attribute or creation failure therefore has one deterministic cleanup owner. The sandbox owns returned process, pipe, and Job handles until wait or disposal. The package exports only operations used by the sandbox production path. Ordinary `CreateProcessW`, exact `applicationName`, parent-stdio release, and whole-Job settlement remain absent until an ordinary process consumer needs them. The package is a library, not a Cordis service or a public Windows SDK. diff --git a/.agents/notes/implemented/architecture/2026-08-19-shared-win32-process-primitives.zh.md b/.agents/notes/implemented/architecture/2026-08-19-shared-win32-process-primitives.zh.md index 2d8b9d3421..79e1ae576b 100644 --- a/.agents/notes/implemented/architecture/2026-08-19-shared-win32-process-primitives.zh.md +++ b/.agents/notes/implemented/architecture/2026-08-19-shared-win32-process-primitives.zh.md @@ -14,7 +14,7 @@ Windows ACL sandbox 拥有 restricted token、SID、DACL、grant 与 workspace p Windows ACL sandbox 继续唯一拥有 restricted-token 创建、SID 与 DACL policy、grants、可写路径裁定、临时目录 policy 和公共 sandbox child result。它通过共享 binding context 扩展 policy-specific API，提供 primary token，组合 pipe drain 与 wait，并在自己的生命周期边界关闭调用方拥有的 Job。 -每项 native allocation 与 HANDLE 都只有一个 owner。process operation 会释放 Koffi out-parameter，并在失败前关闭已经取得的每个 pipe、thread、process 或 Job handle。pipe 创建成功时，把 process 与 stdout/stderr read handles 返回给 sandbox；任一 drain 失败时，sandbox settlement 会在同步 wait 前终止 child。若终止本身失败，settlement 会先取消并等待 sibling drain 结束，再关闭 process handle 并报告失败，因此 rejection 不会留下持续轮询的 timer。inherited-stdio 创建会把 kill-on-close Job 放进 `STARTUPINFOEXW`，因此 child 在任何用户代码运行前已经归属 Job；attribute 或创建失败都有唯一且确定的 cleanup owner。sandbox 在 wait 或 disposal 前拥有返回的 process、pipe 与 Job handles。 +每项 native allocation 与 HANDLE 都只有一个 owner。process operation 会释放 Koffi out-parameter，并在失败前关闭已经取得的每个 pipe、thread、process 或 Job handle。pipe 创建成功时，把 process 与 stdout/stderr read handles 返回给 sandbox；任一 drain 失败时，sandbox settlement 会请求终止 direct child，取消并等待 sibling drain，再只在终止成功时执行 direct-child wait。若终止本身失败，则关闭 process handle 并同时报告两项失败。即使 descendant 继承了 pipe writer，两种结果也都不会留下持续轮询的 timer。inherited-stdio 创建会把 kill-on-close Job 放进 `STARTUPINFOEXW`，因此 child 在任何用户代码运行前已经归属 Job；attribute 或创建失败都有唯一且确定的 cleanup owner。sandbox 在 wait 或 disposal 前拥有返回的 process、pipe 与 Job handles。 该包只导出 sandbox 生产路径已使用的操作。ordinary `CreateProcessW`、精确 `applicationName`、parent-stdio release 与 whole-Job settlement 在 ordinary process consumer 出现前保持缺席。该包是 library，不是 Cordis service 或公共 Windows SDK。 diff --git a/packages/sandbox/sandbox-windows-acl/src/index.ts b/packages/sandbox/sandbox-windows-acl/src/index.ts index 2dc33306af..64d49faf35 100644 --- a/packages/sandbox/sandbox-windows-acl/src/index.ts +++ b/packages/sandbox/sandbox-windows-acl/src/index.ts @@ -381,13 +381,14 @@ export class AclSandbox { const native = spawnSandboxed(api, token, { command: options.command, args, cwd }) const drainAbort = new AbortController() + const drainCancellation = new Error('piped child drain cancelled after peer failure') const stdout = drainPipe(api, native.stdoutRead, drainAbort.signal) const stderr = drainPipe(api, native.stderrRead, drainAbort.signal) // WaitForSingleObject blocks the thread, so settlement starts it only after // both drains settle. Successful drains mean the child closed its pipe ends - // and the wait returns immediately. A failed drain terminates the child - // before waiting, so a native pipe failure cannot pin the event loop on a - // still-running command. + // and the wait returns immediately. A failed drain cancels its sibling and + // terminates the child before waiting, so inherited pipe writers cannot pin + // the event loop after settlement. let settlement: Promise | undefined return { pid: native.pid, @@ -400,11 +401,12 @@ export class AclSandbox { { status: 'fulfilled', value: stderrBuffer }, ] } catch (firstDrainFailure) { - if (api.terminateProcess(native.process, 1) === 0) { + const terminated = api.terminateProcess(native.process, 1) + const terminationCode = terminated === 0 ? api.getLastError() : 0 + drainAbort.abort(drainCancellation) + const settledDrains = await Promise.allSettled([stdout, stderr]) + if (terminated === 0) { const failures: unknown[] = [firstDrainFailure] - const terminationCode = api.getLastError() - drainAbort.abort() - await Promise.allSettled([stdout, stderr]) try { closeHandleChecked(api, native.process, 'piped child after drain failure') } catch (error) { @@ -413,10 +415,12 @@ export class AclSandbox { failures.push(new Win32Error('TerminateProcess', terminationCode, `pid ${native.pid} after drain failure`)) throw new AggregateError(failures, 'piped child settlement failed') } - drains = await Promise.allSettled([stdout, stderr]) + drains = settledDrains } const failures = drains.flatMap(outcome => - outcome.status === 'rejected' ? [outcome.reason as unknown] : []) + outcome.status === 'rejected' && outcome.reason !== drainCancellation + ? [outcome.reason as unknown] + : []) let exitCode = 0 try { exitCode = waitForExit(api, native.process) 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 41ca5463f1..159b36428f 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 @@ -469,23 +469,14 @@ describe('AclSandbox spawn', () => { 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.getLastError = vi.fn(() => 5) + const terminateProcess = vi.fn(() => 1) api.terminateProcess = terminateProcess const waitForSingleObject = vi.fn(() => 0) api.waitForSingleObject = waitForSingleObject @@ -494,6 +485,9 @@ describe('AclSandbox spawn', () => { await sandbox.init() const child = sandbox.spawn({ command: 'probe.exe' }) await expect(child.wait()).rejects.toMatchObject({ api: 'PeekNamedPipe' }) + const settledPeekCount = peekCount + await new Promise(resolve => setTimeout(resolve, 5)) + expect(peekCount).toBe(settledPeekCount) expect(terminateProcess).toHaveBeenCalledOnce() expect(waitForSingleObject).toHaveBeenCalledOnce() }) diff --git a/packages/subprocess/win32-process/src/process.ts b/packages/subprocess/win32-process/src/process.ts index f63943728e..4b948e1849 100644 --- a/packages/subprocess/win32-process/src/process.ts +++ b/packages/subprocess/win32-process/src/process.ts @@ -242,7 +242,7 @@ export function spawnPipedProcess( * @param handle - caller-owned pipe read end. * @param signal - optional cancellation that stops polling and closes the read end. * @returns complete bytes read before EOF; the handle is always closed. - * @throws when cancellation or a Win32 pipe operation fails. + * @throws when the drain is cancelled or a Win32 pipe operation fails. */ export async function drainPipe( api: Win32ProcessBindings, @@ -254,7 +254,7 @@ export async function drainPipe( try { countSlot = allocUint32() for (;;) { - if (signal?.aborted === true) throw new Error('pipe drain aborted') + signal?.throwIfAborted() const peeked = api.peekNamedPipe(handle, null, 0, null, countSlot, null) if (peeked === 0) { const win32Code = api.getLastError() diff --git a/packages/subprocess/win32-process/tests/process.spec.ts b/packages/subprocess/win32-process/tests/process.spec.ts index 15cf2e62c6..e2f151c8e1 100644 --- a/packages/subprocess/win32-process/tests/process.spec.ts +++ b/packages/subprocess/win32-process/tests/process.spec.ts @@ -246,8 +246,9 @@ describe('wait and pipe cleanup', () => { closeHandle, } as unknown as Win32ProcessBindings const draining = drainPipe(api, 80n as NativePtr, controller.signal) - controller.abort() - await expect(draining).rejects.toThrow('pipe drain aborted') + const cancellation = new Error('stop pipe drain') + controller.abort(cancellation) + await expect(draining).rejects.toBe(cancellation) expect(peekNamedPipe).toHaveBeenCalledOnce() expect(closeHandle).toHaveBeenCalledWith(80n) })