From d1c37dc5c7a4d855bb0734b938b0ab0ed8072323 Mon Sep 17 00:00:00 2001 From: luvs01 Date: Thu, 13 Aug 2026 09:31:36 +0900 Subject: [PATCH] fix(windows): fail closed on principal resolver errors --- src/lib/windows-user-principal.ts | 65 +------------ structure/02_config-and-codex-home.md | 20 ++-- tests/windows-user-principal.test.ts | 134 ++------------------------ 3 files changed, 17 insertions(+), 202 deletions(-) diff --git a/src/lib/windows-user-principal.ts b/src/lib/windows-user-principal.ts index ecb92ca670..ac1359ff9c 100644 --- a/src/lib/windows-user-principal.ts +++ b/src/lib/windows-user-principal.ts @@ -20,72 +20,11 @@ * would mean passing an absolute deadline through the runner interface. */ -import { existsSync } from "node:fs"; -import { win32 as windowsPath } from "node:path"; - -import { - resolveTrustedWindowsPowerShellExe, - WindowsSystemDirectoryFfiUnavailableError, -} from "./windows-elevation"; +import { resolveTrustedWindowsPowerShellExe } from "./windows-elevation"; const SID_PATTERN = /^S-1-(?:\d+-)+\d+$/i; const SID_EXPRESSION = "[System.Security.Principal.WindowsIdentity]::GetCurrent().User.Value"; -const DEFAULT_WINDOWS_ARM64_POWERSHELL = windowsPath.join( - "C:\\Windows\\System32", - "WindowsPowerShell", - "v1.0", - "powershell.exe", -); - -type PrincipalExecutableResolution = Readonly<{ - platform: NodeJS.Platform; - arch: string; - resolveTrusted: () => string; - pathExists: (path: string) => boolean; -}>; - -/** - * Bun's Windows ARM64 build cannot currently execute the `bun:ffi` call used - * by the general System32 resolver. The ACL identity lookup has a narrower - * authority than the elevation helpers: it starts one non-elevated PowerShell - * command and accepts only a SID-shaped result. Keep its fallback equally - * narrow by using the fixed, OS-protected default installation path only. - * - * Environment and PATH lookup are deliberately absent. A Windows installation - * outside C:\\Windows keeps failing closed rather than executing a binary chosen - * by caller-controlled `SystemRoot`, `WINDIR`, or `PATH` values. - */ -function resolveWindowsPrincipalPowerShellExecutable( - resolution: PrincipalExecutableResolution = { - platform: process.platform, - arch: process.arch, - resolveTrusted: resolveTrustedWindowsPowerShellExe, - pathExists: existsSync, - }, -): string { - try { - return resolution.resolveTrusted(); - } catch (error) { - if ( - !(error instanceof WindowsSystemDirectoryFfiUnavailableError) || - resolution.platform !== "win32" || - resolution.arch !== "arm64" || - !resolution.pathExists(DEFAULT_WINDOWS_ARM64_POWERSHELL) - ) { - throw error; - } - return DEFAULT_WINDOWS_ARM64_POWERSHELL; - } -} - -/** Test-only dependency-injected view of the Windows ARM64 executable boundary. */ -export function resolveWindowsPrincipalPowerShellExecutableForTests( - resolution: PrincipalExecutableResolution, -): string { - return resolveWindowsPrincipalPowerShellExecutable(resolution); -} - export interface WindowsPrincipalLookupResult { success: boolean; exitCode: number | null; @@ -112,7 +51,7 @@ const POWERSHELL_ARGS = [ ] as const; function windowsPrincipalPowerShellCommand(): string[] { - return [resolveWindowsPrincipalPowerShellExecutable(), ...POWERSHELL_ARGS]; + return [resolveTrustedWindowsPowerShellExe(), ...POWERSHELL_ARGS]; } /** Test-only readback of the exact trusted executable and static arguments. */ diff --git a/structure/02_config-and-codex-home.md b/structure/02_config-and-codex-home.md index f3346020ca..2336984ccd 100644 --- a/structure/02_config-and-codex-home.md +++ b/structure/02_config-and-codex-home.md @@ -104,19 +104,17 @@ shutdown handler) both restore Codex config simultaneously. The temp is renamed Windows secret-file hardening resolves the effective token SID through an absolute, trusted PowerShell path before granting the owner and removing inherited broad ACL entries. The normal -path obtains System32 from `GetSystemDirectoryW`. Windows ARM64 Bun builds that cannot execute -`bun:ffi` use a narrower ACL-only fallback to the fixed protected default installation path -`C:\Windows\System32\WindowsPowerShell\v1.0\powershell.exe`. The fallback never applies to UAC or -Task Scheduler launch, never consults environment variables or `PATH`, and fails closed when the -fixed executable is absent. +path obtains System32 from `GetSystemDirectoryW`. If that trusted lookup is unavailable, including +on a runtime without working `bun:ffi`, the ACL operation fails closed rather than selecting an +executable through a fixed path, environment variable, or `PATH` lookup. [Decision Log] -- 목적과 의도: Preserve required Windows ACL hardening on the bundled Windows ARM64 runtime without weakening executable trust. -- 기존 구현 및 제약 조건: The effective-SID query depended on the shared `GetSystemDirectoryW` FFI resolver; Bun 1.3.14 Windows ARM64 has no working `bun:ffi`, so config mutation reached `EACLIDENTITY` before PowerShell could start. -- 검토한 주요 대안: Restore `USERDOMAIN\\USERNAME`; trust `SystemRoot`, `WINDIR`, or `PATH`; weaken required ACL writes; broaden the shared elevation resolver; or add a fixed-path fallback only for the non-elevated SID query. -- 선택한 방식: Keep FFI authoritative, then allow only Windows ARM64 to use the existing default `C:\Windows\System32` PowerShell binary for the SID query when that exact file exists. -- 다른 대안 대신 이 방식을 선택한 이유: Names and environment paths are caller-controlled, required secret writes must not silently skip ACLs, and elevation has a larger authority boundary that should remain FFI-only. -- 장점, 단점 및 영향: Default Windows ARM64 installations can start and harden secrets; non-default Windows roots continue to fail closed until Bun exposes a trustworthy native system-directory API without FFI. +- 목적과 의도: Preserve the trusted-executable boundary for Windows ACL hardening. +- 기존 구현 및 제약 조건: The effective-SID query depends on the shared `GetSystemDirectoryW` FFI resolver, which may be unavailable on some runtimes. +- 검토한 주요 대안: Restore `USERDOMAIN\\USERNAME`; trust a fixed path, `SystemRoot`, `WINDIR`, or `PATH`; or weaken required ACL writes. +- 선택한 방식: Require the `GetSystemDirectoryW`-derived PowerShell path and fail closed when it cannot be resolved. +- 다른 대안 대신 이 방식을 선택한 이유: Names, environment paths, and fixed paths not tied to the actual system directory cannot establish executable trust. +- 장점, 단점 및 영향: Secret ACL operations never launch an untrusted identity helper; affected runtimes fail with `EACLIDENTITY` until they provide trustworthy system-directory resolution. Response-state loading performs a bounded recovery pass for interrupted snapshot writes. It only matches regular files named `responses-state.json.ocx...tmp`, waits at least 15 diff --git a/tests/windows-user-principal.test.ts b/tests/windows-user-principal.test.ts index d1d04ca66c..c4ccb25688 100644 --- a/tests/windows-user-principal.test.ts +++ b/tests/windows-user-principal.test.ts @@ -4,14 +4,13 @@ import { resetWindowsPrincipalForTests, resolveCurrentWindowsPrincipal, resolveCurrentWindowsPrincipalAsync, - resolveWindowsPrincipalPowerShellExecutableForTests, setAsyncWindowsPrincipalRunnerForTests, setWindowsPrincipalRunnerForTests, windowsPrincipalPowerShellCommandForTests, } from "../src/lib/windows-user-principal"; import { setTrustedWindowsElevationExecutablesForTests, - WindowsSystemDirectoryFfiUnavailableError, + setTrustedWindowsSystemDirectoryResolverForTests, } from "../src/lib/windows-elevation"; const ok = (stdout = "S-1-5-21-111-222-333-1001\r\n") => ({ @@ -25,6 +24,7 @@ afterEach(() => { setWindowsPrincipalRunnerForTests(null); setAsyncWindowsPrincipalRunnerForTests(null); setTrustedWindowsElevationExecutablesForTests(null); + setTrustedWindowsSystemDirectoryResolverForTests(null); resetWindowsPrincipalForTests(); }); @@ -44,132 +44,10 @@ describe("Windows effective ACL principal", () => { ]); }); - test("Windows ARM64 uses only the fixed default PowerShell path when FFI resolution is unavailable", () => { - const lookupError = new WindowsSystemDirectoryFfiUnavailableError(); - const previousSystemRoot = process.env.SystemRoot; - const previousWindir = process.env.WINDIR; - const previousPath = process.env.PATH; - process.env.SystemRoot = "C:\\attacker-controlled"; - process.env.WINDIR = "D:\\attacker-controlled"; - process.env.PATH = "E:\\attacker-controlled"; - try { - let observedPath = ""; - const resolved = resolveWindowsPrincipalPowerShellExecutableForTests({ - platform: "win32", - arch: "arm64", - resolveTrusted: () => { throw lookupError; }, - pathExists: path => { - observedPath = path; - return true; - }, - }); - expect(observedPath).toBe("C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe"); - expect(resolved).toBe(observedPath); - expect(resolved).not.toContain("attacker-controlled"); - } finally { - if (previousSystemRoot === undefined) delete process.env.SystemRoot; - else process.env.SystemRoot = previousSystemRoot; - if (previousWindir === undefined) delete process.env.WINDIR; - else process.env.WINDIR = previousWindir; - if (previousPath === undefined) delete process.env.PATH; - else process.env.PATH = previousPath; - } - }); - - test("a GetSystemDirectoryW call failure is rethrown without probing the fixed fallback", () => { - const lookupError = new Error( - "GetSystemDirectoryW failed while resolving the trusted system directory.", - ); - let fallbackProbes = 0; - expect(() => resolveWindowsPrincipalPowerShellExecutableForTests({ - platform: "win32", - arch: "arm64", - resolveTrusted: () => { throw lookupError; }, - pathExists: () => { - fallbackProbes += 1; - return true; - }, - })).toThrow(lookupError); - expect(fallbackProbes).toBe(0); - }); - - test("an unusable non-default system directory is rethrown without probing the fixed fallback", () => { - const lookupError = new Error("GetSystemDirectoryW returned an unusable system directory."); - let fallbackProbes = 0; - expect(() => resolveWindowsPrincipalPowerShellExecutableForTests({ - platform: "win32", - arch: "arm64", - resolveTrusted: () => { throw lookupError; }, - pathExists: () => { - fallbackProbes += 1; - return true; - }, - })).toThrow(lookupError); - expect(fallbackProbes).toBe(0); - }); - - test("trusted PowerShell validation failures are rethrown without probing the fixed fallback", () => { - const validationErrors = [ - new Error("Trusted PowerShell was not found at D:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe."), - new Error("PowerShell resolved outside the trusted Windows system directory."), - ]; - for (const lookupError of validationErrors) { - let fallbackProbes = 0; - expect(() => resolveWindowsPrincipalPowerShellExecutableForTests({ - platform: "win32", - arch: "arm64", - resolveTrusted: () => { throw lookupError; }, - pathExists: () => { - fallbackProbes += 1; - return true; - }, - })).toThrow(lookupError); - expect(fallbackProbes).toBe(0); - } - }); - - test("an arbitrary trusted resolver error is rethrown without probing the fixed fallback", () => { - const lookupError = new Error("unexpected trusted resolver failure"); - let fallbackProbes = 0; - expect(() => resolveWindowsPrincipalPowerShellExecutableForTests({ - platform: "win32", - arch: "arm64", - resolveTrusted: () => { throw lookupError; }, - pathExists: () => { - fallbackProbes += 1; - return true; - }, - })).toThrow(lookupError); - expect(fallbackProbes).toBe(0); - }); - - test("a successful trusted resolver always wins without probing the ARM64 fallback", () => { - let fallbackProbes = 0; - expect(resolveWindowsPrincipalPowerShellExecutableForTests({ - platform: "win32", - arch: "arm64", - resolveTrusted: () => "D:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe", - pathExists: () => { - fallbackProbes += 1; - return true; - }, - })).toBe("D:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe"); - expect(fallbackProbes).toBe(0); - }); - - test("the FFI-unavailable sentinel fails closed off Windows ARM64 or without the fixed executable", () => { - const lookupError = new WindowsSystemDirectoryFfiUnavailableError(); - const resolve = (platform: NodeJS.Platform, arch: string, present: boolean) => - resolveWindowsPrincipalPowerShellExecutableForTests({ - platform, - arch, - resolveTrusted: () => { throw lookupError; }, - pathExists: () => present, - }); - - expect(() => resolve("win32", "x64", true)).toThrow(lookupError); - expect(() => resolve("linux", "arm64", true)).toThrow(lookupError); - expect(() => resolve("win32", "arm64", false)).toThrow(lookupError); + test("fails closed when trusted PowerShell resolution fails", () => { + const lookupError = new Error("trusted system directory unavailable"); + setTrustedWindowsSystemDirectoryResolverForTests(() => { throw lookupError; }); + expect(() => windowsPrincipalPowerShellCommandForTests()).toThrow(lookupError); }); test("the default trusted runner resolves the real token on Windows", () => {