From 07c7e2f0bd626b4d1138e6e87714beb19d270de1 Mon Sep 17 00:00:00 2001 From: Huanqi Cao Date: Sat, 8 Aug 2026 17:39:25 +0800 Subject: [PATCH] fix(sandbox): revoke post-apply grant failures, free init SID allocations, and name the Windows runner in SANDBOX_UNAVAILABLE --- .../sandbox/sandbox-windows-acl/src/grant.ts | 11 +++--- .../sandbox/sandbox-windows-acl/src/index.ts | 35 ++++++++++++++++--- packages/sandbox/sandbox/README.i18n.yaml | 4 +-- packages/sandbox/sandbox/README.md | 2 +- packages/sandbox/sandbox/README.zh.md | 2 +- packages/sandbox/sandbox/src/index.ts | 5 +-- 6 files changed, 45 insertions(+), 14 deletions(-) diff --git a/packages/sandbox/sandbox-windows-acl/src/grant.ts b/packages/sandbox/sandbox-windows-acl/src/grant.ts index 4a0f6f4dec..7826922f1d 100644 --- a/packages/sandbox/sandbox-windows-acl/src/grant.ts +++ b/packages/sandbox/sandbox-windows-acl/src/grant.ts @@ -57,14 +57,17 @@ export class AclWriteGrant { /** * Grant the write ACE on one directory (idempotent: an already-standing * exact ACE skips the eager full-tree re-propagation — see - * {@link grantWrite}) and record the path for {@link dispose}. Callers - * treat a throw as a failed materialization and dispose the instance to - * revoke the paths granted so far. + * {@link grantWrite}) and record the path for {@link dispose}. The path is + * recorded BEFORE the grant: a post-apply throw (a LocalFree failure after + * SetNamedSecurityInfoW succeeded) must still revoke it, and revoking an + * ungranted path is a no-op merge. Callers treat a throw as a failed + * materialization and dispose the instance to revoke the paths granted so + * far. * @param path - the directory whose DACL gains the grant. */ add(path: string): void { - grantWrite(this.api, path, this.sidPtr) this.grantedPaths.push(path) + grantWrite(this.api, path, this.sidPtr) } /** Every directory currently carrying the grant, in grant order. */ diff --git a/packages/sandbox/sandbox-windows-acl/src/index.ts b/packages/sandbox/sandbox-windows-acl/src/index.ts index 1ff712f24e..07f35ed177 100644 --- a/packages/sandbox/sandbox-windows-acl/src/index.ts +++ b/packages/sandbox/sandbox-windows-acl/src/index.ts @@ -130,6 +130,8 @@ export class AclSandbox { private api: Win32Bindings | undefined private token: NativePtr | undefined private writeSidPtr: NativePtr | undefined + /** The well-known/logon SID allocations init() makes; freed by dispose() alongside the write SID. */ + private sidAllocations: NativePtr[] = [] private grantedPaths: string[] = [] constructor(options: AclSandboxOptions) { @@ -182,16 +184,24 @@ export class AclSandbox { // remove any (its dispose() must not revoke the caller's standing grant). if (this.manageDacls) { for (const path of tempDir !== null ? [...this.writableDirs, tempDir] : this.writableDirs) { - grantWrite(api, path, writeSidPtr) + // Record BEFORE granting: grantWrite can throw after a successful + // apply (a LocalFree failure), and the fail-closed catch must still + // revoke that path (revoking an ungranted path is a no-op merge). this.grantedPaths.push(path) + grantWrite(api, path, writeSidPtr) } } const logonSid = findLogonSid(api, currentToken) + this.sidAllocations.push(logonSid) + const worldSid = makeWellKnownSid(api, abi.WinWorldSid) + this.sidAllocations.push(worldSid) + const authUserSid = makeWellKnownSid(api, abi.WinAuthenticatedUserSid) + this.sidAllocations.push(authUserSid) const restricted = createRestrictedToken( api, currentToken, logonSid, writeSidPtr, { - world: makeWellKnownSid(api, abi.WinWorldSid), - authUser: makeWellKnownSid(api, abi.WinAuthenticatedUserSid), + world: worldSid, + authUser: authUserSid, }, this.mode, ) @@ -201,7 +211,8 @@ export class AclSandbox { } catch (error) { // Best-effort close on the failure path (last error already captured in `error`). api.closeHandle(currentToken) - // Fail-closed cleanup: never leave standing grants behind a failed init. + // Fail-closed cleanup: never leave standing grants or SID allocations + // behind a failed init. const cleanupFailures: unknown[] = [] const writeSidPtr = this.writeSidPtr if (writeSidPtr !== undefined) { @@ -213,6 +224,14 @@ export class AclSandbox { } } } + for (const sidPtr of this.sidAllocations.splice(0)) { + try { + const freed = api.localFree(sidPtr) + if (!isNullPtr(freed)) throwLastError(api, 'LocalFree', 'init SID allocation') + } catch (cleanupError) { + cleanupFailures.push(cleanupError) + } + } if (cleanupFailures.length > 0) { throw new AggregateError( [error, ...cleanupFailures], @@ -304,6 +323,14 @@ export class AclSandbox { failures.push(error) } } + for (const sidPtr of this.sidAllocations.splice(0)) { + try { + const freed = api.localFree(sidPtr) + if (!isNullPtr(freed)) throwLastError(api, 'LocalFree', 'init SID allocation') + } catch (error) { + failures.push(error) + } + } this.api = undefined this.token = undefined this.writeSidPtr = undefined diff --git a/packages/sandbox/sandbox/README.i18n.yaml b/packages/sandbox/sandbox/README.i18n.yaml index 11f2a2ed86..6f3d1ca4dd 100644 --- a/packages/sandbox/sandbox/README.i18n.yaml +++ b/packages/sandbox/sandbox/README.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 packages/sandbox/sandbox/README.md -README.md: 1b522b2c72d00bfed89650aa7f22b65a72d26085 -README.zh.md: adccd4421a74ef073ad3ffc3a23bccb0354d99aa +README.md: d6bf873d559fe0933e7e7ee7d966e2f27053ca05 +README.zh.md: a618fe435d0c9d862c8dab4ded390de45e1e5d74 diff --git a/packages/sandbox/sandbox/README.md b/packages/sandbox/sandbox/README.md index 1b522b2c72..d6bf873d55 100644 --- a/packages/sandbox/sandbox/README.md +++ b/packages/sandbox/sandbox/README.md @@ -23,7 +23,7 @@ Through [`dsh-bash-sandbox`](../../bash/bash-sandbox/README.md) and [`dsh-tool-b ##### Exact error ```markdown -sandbox mode "" is requested but no sandbox backend is usable on this host; refusing to run the command unconfined. Install bubblewrap or run a Landlock-enforcing kernel (Linux), ensure sandbox-exec is usable (macOS) — Windows has no confinement backend yet — or switch the consumer to danger-full-access. +sandbox mode "" is requested but no sandbox backend is usable on this host; refusing to run the command unconfined. Install bubblewrap or run a Landlock-enforcing kernel (Linux), ensure sandbox-exec is usable (macOS), or ensure the ACL restricted-token runner can start (Windows) — otherwise switch the consumer to danger-full-access. ``` #### Token effect diff --git a/packages/sandbox/sandbox/README.zh.md b/packages/sandbox/sandbox/README.zh.md index adccd4421a..a618fe435d 100644 --- a/packages/sandbox/sandbox/README.zh.md +++ b/packages/sandbox/sandbox/README.zh.md @@ -23,7 +23,7 @@ ##### 精确错误 ```markdown -sandbox mode "" is requested but no sandbox backend is usable on this host; refusing to run the command unconfined. Install bubblewrap or run a Landlock-enforcing kernel (Linux), ensure sandbox-exec is usable (macOS) — Windows has no confinement backend yet — or switch the consumer to danger-full-access. +sandbox mode "" is requested but no sandbox backend is usable on this host; refusing to run the command unconfined. Install bubblewrap or run a Landlock-enforcing kernel (Linux), ensure sandbox-exec is usable (macOS), or ensure the ACL restricted-token runner can start (Windows) — otherwise switch the consumer to danger-full-access. ``` #### Token 影响 diff --git a/packages/sandbox/sandbox/src/index.ts b/packages/sandbox/sandbox/src/index.ts index 6170482a37..fee9164ead 100644 --- a/packages/sandbox/sandbox/src/index.ts +++ b/packages/sandbox/sandbox/src/index.ts @@ -131,8 +131,9 @@ export class SandboxUnavailableError extends HarnessError { super( `sandbox mode "${mode}" is requested but no sandbox backend is usable on this host; ` + 'refusing to run the command unconfined. Install bubblewrap or run a Landlock-enforcing ' - + 'kernel (Linux), ensure sandbox-exec is usable (macOS) — Windows has no confinement ' - + 'backend yet — or switch the consumer to danger-full-access.' + + 'kernel (Linux), ensure sandbox-exec is usable (macOS), or ensure the ACL ' + + 'restricted-token runner can start (Windows) — otherwise switch the consumer to ' + + 'danger-full-access.' + (detail === undefined ? '' : ` Runner failure: ${detail}`), SANDBOX_UNAVAILABLE, )