fix(sandbox): revoke post-apply grant failures, free init SID allocations, and name the Windows runner in SANDBOX_UNAVAILABLE

2026-08-08 17:39:25 +08:00
parent 43d53d339d
commit 07c7e2f0bd
6 changed files with 45 additions and 14 deletions
--- 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. */
--- 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
--- 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
--- 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 "<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 "<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
--- a/packages/sandbox/sandbox/README.zh.md
+++ b/packages/sandbox/sandbox/README.zh.md
@@ -23,7 +23,7 @@
 ##### 精确错误

 ```markdown
-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.
+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), or ensure the ACL restricted-token runner can start (Windows) — otherwise switch the consumer to danger-full-access.
 ```

 #### Token 影响
--- 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,
    )