fix(codex): protect the desktop authority key in-process on Windows x64

src/codex/desktop-compatibility/windows-key-protection.ts

@@ -1,12 +1,14 @@
 import { execFile } from "node:child_process";
-import { resolveTrustedWindowsPowerShellExe } from "../../lib/windows-elevation";
+import { join } from "node:path";
+import { dlopen, ptr, toArrayBuffer, type Pointer } from "bun:ffi";
+import { resolveTrustedWindowsPowerShellExe, resolveTrustedWindowsSystemDirectory } from "../../lib/windows-elevation";
 
 const MAX_INPUT = 65_536;
 const MAX_OUTPUT = 131_072;
 const PURPOSE = "opencodex/codex-desktop-compatibility/authority/v1";
-// A cold Windows PowerShell that must also load System.Security has been measured past 10s on a
-// loaded machine (Windows CI run 38591626343). The call is async and off the request path, so a
-// wider bound costs nothing while healthy and turns a slow start into success instead of a refusal.
+const ENTROPY = new TextEncoder().encode(PURPOSE);
+// Only the PowerShell fallback is bounded by a clock. A cold Windows PowerShell that must also load
+// System.Security has been measured past 10s on a loaded machine (Windows CI run 38591626343).
 const DPAPI_TIMEOUT_MS = 20_000;
 const SCRIPT = [
   "$ErrorActionPreference='Stop'",
@@ -24,18 +26,94 @@ const SCRIPT = [
   "[Console]::Out.Write([Convert]::ToBase64String($out))",
 ].join("\n");
 
+type Operation = "protect" | "unprotect";
+
 export interface AuthorityKeyProtection {
   protect(cleartext: Uint8Array): Promise<Uint8Array>;
   unprotect(ciphertext: Uint8Array): Promise<Uint8Array>;
 }
 
+// CRYPTPROTECT_UI_FORBIDDEN and no CRYPTPROTECT_LOCAL_MACHINE: the CurrentUser scope that
+// ProtectedData.Protect(..., CurrentUser) requests, so blobs written by either path read in the other.
+const CRYPTPROTECT_UI_FORBIDDEN = 0x1;
+// DATA_BLOB on x64: DWORD cbData, four bytes of padding, BYTE* pbData.
+const DATA_BLOB_BYTES = 16;
+const DPAPI_SIGNATURE = { args: ["ptr", "ptr", "ptr", "ptr", "ptr", "u32", "ptr"], returns: "i32" } as const;
+
+type NativeDpapi = (operation: Operation, data: Uint8Array) => Uint8Array;
+let nativeDpapi: NativeDpapi | undefined;
+
+function dataBlob(bytes: Uint8Array): Buffer {
+  const blob = Buffer.alloc(DATA_BLOB_BYTES);
+  blob.writeUInt32LE(bytes.byteLength, 0);
+  blob.writeBigUInt64LE(BigInt(ptr(bytes)), 8);
+  return blob;
+}
+
+/**
+ * In-process CryptProtectData/CryptUnprotectData. Spawning Windows PowerShell for every call cost
+ * seconds of cold start (past 20s on a loaded CI runner) for a call that takes milliseconds.
+ * crypt32 is loaded from the GetSystemDirectoryW path so the DLL search order cannot substitute it.
+ * Returns null where Bun's FFI is unavailable (Windows ARM64), leaving the PowerShell helper.
+ */
+function loadNativeDpapi(): NativeDpapi | null {
+  if (nativeDpapi) return nativeDpapi;
+  if (process.platform !== "win32" || process.arch !== "x64") return null;
+  let crypt32;
+  let kernel32;
+  try {
+    const systemDirectory = resolveTrustedWindowsSystemDirectory();
+    crypt32 = dlopen(join(systemDirectory, "crypt32.dll"), {
+      CryptProtectData: DPAPI_SIGNATURE,
+      CryptUnprotectData: DPAPI_SIGNATURE,
+    }).symbols;
+    kernel32 = dlopen(join(systemDirectory, "kernel32.dll"), {
+      LocalFree: { args: ["ptr"], returns: "ptr" },
+      // Wipes by address, so clearing never depends on a JS view having been created.
+      RtlZeroMemory: { args: ["ptr", "u64"], returns: "void" },
+    }).symbols;
+  } catch {
+    return null;
+  }
+  const { CryptProtectData, CryptUnprotectData } = crypt32;
+  const { LocalFree, RtlZeroMemory } = kernel32;
+  nativeDpapi = (operation, data) => {
+    const input = dataBlob(data);
+    const entropy = dataBlob(ENTROPY);
+    const output = Buffer.alloc(DATA_BLOB_BYTES);
+    const call = operation === "protect" ? CryptProtectData : CryptUnprotectData;
+    // Description, reserved and prompt are null; Windows reports no detail worth propagating.
+    if (!call(ptr(input), null, ptr(entropy), null, null, CRYPTPROTECT_UI_FORBIDDEN, ptr(output))) {
+      throw new Error("desktop_compatibility_key_protection_failed");
+    }
+    const length = output.readUInt32LE(0);
+    const address = Number(output.readBigUInt64LE(8)) as Pointer;
+    // Every step after a successful call sits inside try so the native buffer is always
+    // released, even if creating the view over it throws.
+    let view: Uint8Array | null = null;
+    try {
+      view = length > 0 ? new Uint8Array(toArrayBuffer(address, 0, length)) : null;
+      if (length === 0 || length > MAX_INPUT) throw new Error("desktop_compatibility_key_size");
+      return Uint8Array.from(view!);
+    } finally {
+      // The unprotected buffer is cleartext key material; wipe it natively before LocalFree
+      // releases it, including when the size check or view creation fails. LocalFree still
+      // runs if wiping throws.
+      try {
+        if (operation === "unprotect" && length > 0) RtlZeroMemory(address, length);
+      } finally {
+        LocalFree(address);
+      }
+    }
+  };
+  return nativeDpapi;
+}
+
 /** The payload travels on stdin, never in process arguments, environment or logs. */
-function runDpapi(operation: "protect" | "unprotect", data: Uint8Array): Promise<Uint8Array> {
-  if (process.platform !== "win32") return Promise.reject(new Error("desktop_compatibility_windows_required"));
-  if (data.byteLength === 0 || data.byteLength > MAX_INPUT) return Promise.reject(new Error("desktop_compatibility_key_size"));
+function runPowerShellDpapi(operation: Operation, data: Uint8Array, timeoutMs = DPAPI_TIMEOUT_MS): Promise<Uint8Array> {
   return new Promise((resolve, reject) => {
-    const child = execFile(resolveTrustedWindowsPowerShellExe(), ["-NoProfile", "-NonInteractive", "-Command", SCRIPT], {
-      timeout: DPAPI_TIMEOUT_MS, maxBuffer: MAX_OUTPUT, windowsHide: true, encoding: "utf8",
+    const child = execFile(resolveTrustedWindowsPowerShellExe(), ["-NoLogo", "-NoProfile", "-NonInteractive", "-Command", SCRIPT], {
+      timeout: timeoutMs, maxBuffer: MAX_OUTPUT, windowsHide: true, encoding: "utf8",
     }, (error, stdout) => {
       // Do not include PowerShell stderr, command output or the input in a propagated error.
       if (error || !/^[A-Za-z0-9+/]+={0,2}$/.test(stdout.trim())) {
@@ -52,8 +130,36 @@ function runDpapi(operation: "protect" | "unprotect", data: Uint8Array): Promise
   });
 }
 
+function validated(data: Uint8Array): Uint8Array {
+  if (process.platform !== "win32") throw new Error("desktop_compatibility_windows_required");
+  if (data.byteLength === 0 || data.byteLength > MAX_INPUT) throw new Error("desktop_compatibility_key_size");
+  return data;
+}
+
+async function runDpapi(operation: Operation, data: Uint8Array): Promise<Uint8Array> {
+  validated(data);
+  const native = loadNativeDpapi();
+  return native ? native(operation, data) : runPowerShellDpapi(operation, data);
+}
+
 /** CurrentUser protects against other OS identities, not other processes of the same user. */
 export const windowsAuthorityKeyProtection: AuthorityKeyProtection = {
   protect: bytes => runDpapi("protect", bytes),
   unprotect: bytes => runDpapi("unprotect", bytes),
 };
+
+/**
+ * Test-only: the PowerShell helper (Windows ARM64, and every key stored before the native path).
+ * The caller owns the bound because a compatibility check must not depend on cold-start latency.
+ */
+export function powerShellAuthorityKeyProtectionForTests(timeoutMs: number): AuthorityKeyProtection {
+  return {
+    protect: async bytes => runPowerShellDpapi("protect", validated(bytes), timeoutMs),
+    unprotect: async bytes => runPowerShellDpapi("unprotect", validated(bytes), timeoutMs),
+  };
+}
+
+/** Test-only: whether this process protects keys in-process rather than through PowerShell. */
+export function usesNativeDpapiForTests(): boolean {
+  return process.platform === "win32" && loadNativeDpapi() !== null;
+}

structure/clients/codex-desktop.md

@@ -52,11 +52,16 @@ Failure before publication preserves the original removable identity; staging is
 An older authority without the server-only EKU reports `renewal-required`: serving and trust
 registration refuse it, while exact trust removal and deliberate renewal remain available.
 
-`src/codex/desktop-compatibility/windows-key-protection.ts` uses trusted PowerShell
-and bounded stdin/stdout, never command-line secrets or plaintext fallback. CurrentUser
-protects against other OS identities, not another process using the same credentials.
+`src/codex/desktop-compatibility/windows-key-protection.ts` calls CurrentUser
+`CryptProtectData`/`CryptUnprotectData` in-process on Windows x64, loading `crypt32.dll`
+from the `GetSystemDirectoryW` path and clearing unprotected output before `LocalFree`.
+Windows ARM64, where Bun FFI is unavailable, keeps the trusted PowerShell helper with
+bounded stdin/stdout. Both write the same blob format; neither puts secrets on a command
+line or falls back to plaintext. CurrentUser protects against other OS identities, not
+another process using the same credentials.
 `tests/clients/desktop-compatibility-authority.test.ts` covers persistence, refusal,
-and a real Windows DPAPI round trip with synthetic data.
+a real Windows DPAPI round trip with synthetic data, and a PowerShell-written key opening
+in-process.
 
 ## Certificate setup API
 

tests/clients/desktop-compatibility-authority.test.ts

@@ -5,7 +5,9 @@ import { existsSync, mkdtempSync, readFileSync, readdirSync, rmSync, writeFileSy
 import { tmpdir } from "node:os";
 import { join } from "node:path";
 import { ensureDesktopCompatibilityAuthority, inspectDesktopCompatibilityAuthority, loadDesktopCompatibilityAuthority, renewDesktopCompatibilityAuthority } from "../../src/codex/desktop-compatibility/certificate-store";
-import { windowsAuthorityKeyProtection, type AuthorityKeyProtection } from "../../src/codex/desktop-compatibility/windows-key-protection";
+import {
+  powerShellAuthorityKeyProtectionForTests, usesNativeDpapiForTests, windowsAuthorityKeyProtection, type AuthorityKeyProtection,
+} from "../../src/codex/desktop-compatibility/windows-key-protection";
 import { setAsyncIcaclsRunnerForTests, setIcaclsRunnerForTests } from "../../src/lib/windows-secret-acl";
 
 const roots: string[] = [];
@@ -164,11 +166,20 @@ describe("Codex Desktop compatibility authority lifecycle", () => {
   });
 
   test.skipIf(process.platform !== "win32")("real CurrentUser DPAPI survives separate helper invocations and rejects corruption", async () => {
+    // x64 protects in-process; spawning PowerShell per call is what exceeded its bound on CI.
+    expect(usesNativeDpapiForTests()).toBe(process.arch === "x64");
     const original = Buffer.from("synthetic authority secret; no real credentials");
     const encrypted = await windowsAuthorityKeyProtection.protect(original);
     expect(Buffer.from(encrypted).includes(original)).toBe(false);
     expect(Buffer.from(await windowsAuthorityKeyProtection.unprotect(encrypted))).toEqual(original);
     const corrupted = Uint8Array.from(encrypted); corrupted[corrupted.length - 1] ^= 1;
     await expect(windowsAuthorityKeyProtection.unprotect(corrupted)).rejects.toThrow("desktop_compatibility_key_protection_failed");
-  }, 65_000); // three PowerShell calls, each bounded at 20s, plus margin
+  }, 65_000); // ARM64 still takes the PowerShell path: three calls, each bounded at 20s
+
+  test.skipIf(process.platform !== "win32" || process.arch !== "x64")("a key protected by the PowerShell helper still opens in-process", async () => {
+    // Every key stored before the native path was written by ProtectedData through PowerShell.
+    const original = Buffer.from("synthetic legacy authority secret; no real credentials");
+    const legacy = await powerShellAuthorityKeyProtectionForTests(50_000).protect(original);
+    expect(Buffer.from(await windowsAuthorityKeyProtection.unprotect(legacy))).toEqual(original);
+  }, 60_000); // one cold PowerShell; the bound is the test's, not the production path's
 });