From 6602f79e551081ec4ecb6a8f7160f6d18dcb83b6 Mon Sep 17 00:00:00 2001 From: Sunil Srivatsa Date: Thu, 20 Aug 2026 14:37:29 -0700 Subject: [PATCH 1/8] fix(gateway): amortise Windows ACL checks into one PowerShell process Closes #90. The Windows daemon startup path performed 11 ACL operations before the loopback listener could bind, and each one spawned its own PowerShell process. A single minimal Get-Acl spawn measured 1854ms on a 2-vCPU Windows Server 2025 host, so the ACL work alone cost about 20.4 seconds against a 15 second readiness budget. Install could not succeed there, and it failed exactly as arithmetic predicts: the Scheduled Task ran, the daemon process started and stayed alive, and the listener was never bound before install gave up and rolled back. The fix is at the cause. One long-lived PowerShell helper now serves every ACL request over a newline-delimited JSON loop, so 11 processes become 1. The helper is unref'd, never holds the event loop open, and exits by itself when our stdin closes. The readiness budget is additionally derived from the measured cost of that one spawn rather than being a bare constant, with a hard ceiling so a genuinely broken install still fails in bounded time. Security semantics are preserved and in two places improved. The validation logic is unchanged: protected ACL, owner is the current user, exactly the current-user and S-1-5-18 entries, AccessAllowed, mask and flags identical. ErrorActionPreference=Stop makes a non-terminating Get-Acl error a structured per-path failure rather than an opaque exit code, and a reply whose id does not match its request kills the helper and throws, so a batched failure can never be attributed to the wrong path. Non-win32 behaviour is unchanged; those functions still early-return. Four tests pin the contracts that must not regress: one run uses a single PowerShell process, a foreign principal is rejected with the offending path named, an unresolvable directory is rejected naming the one that failed, and an unreadable token ACL is fatal rather than being mistaken for a missing token. The real proof is the Windows CI job plus a future run on a 2-vCPU host; Windows could not be executed locally on macOS. --- apps/gateway/src/cli.ts | 22 ++- apps/gateway/src/config.ts | 224 +++++++++++++++++++++++++------ apps/gateway/test/config.test.ts | 205 ++++++++++++++++++++++++++++ 3 files changed, 410 insertions(+), 41 deletions(-) diff --git a/apps/gateway/src/cli.ts b/apps/gateway/src/cli.ts index facdd1d..e173a31 100755 --- a/apps/gateway/src/cli.ts +++ b/apps/gateway/src/cli.ts @@ -13,6 +13,7 @@ import { publicOriginHttpsPort, restoreGatewayConfigFile, rotatePublisherToken, + windowsAclSpawnCostMs, writeGatewayConfigFile, } from "./config.ts"; import { createDiagnosticsBundle } from "./diagnostics.ts"; @@ -191,13 +192,32 @@ async function runServe(arguments_: ParsedArguments): Promise { } } +const READINESS_BASE_MS = 15_000; +const READINESS_ACL_ALLOWANCE_CEILING_MS = 30_000; + +/** + * Loopback readiness budget for a freshly started service. + * + * The base covers a healthy host. On Windows the daemon must start one `powershell.exe` to enforce + * ACLs on its private paths before it can bind, and that start measured 1762-2132 ms on a 2-vCPU + * Windows Server 2025 host and has been recorded far higher under contention (#90). `install` paid + * for the same start in this process while writing the config, so the observed cost is the evidence + * the budget is extended by - exactly one start, because exactly one remains on the daemon's path - + * and the extension is capped so a genuinely broken install still fails in bounded time. + */ +function readinessBudgetMs(): number { + const observed = windowsAclSpawnCostMs(); + if (observed === undefined) return READINESS_BASE_MS; + return READINESS_BASE_MS + Math.min(Math.round(observed), READINESS_ACL_ALLOWANCE_CEILING_MS); +} + async function waitForGateway( config: Awaited>, readinessToken: string, readinessInstance?: string, requireManagedService = false, ): Promise { - const deadline = Date.now() + 15_000; + const deadline = Date.now() + readinessBudgetMs(); while (Date.now() < deadline) { if (await gatewayReady(config, readinessToken, readinessInstance)) { if (!requireManagedService) return; diff --git a/apps/gateway/src/config.ts b/apps/gateway/src/config.ts index e14d671..5dcb26d 100644 --- a/apps/gateway/src/config.ts +++ b/apps/gateway/src/config.ts @@ -55,57 +55,201 @@ function currentUserId(): number { return uid; } -function windowsPowerShellEnvironment(overrides: Record): Record { +/** + * Environment for the ACL helper. `PSModulePath` is dropped so a writable module directory inherited + * from the caller cannot inject code into the helper; every other variable is inherited because + * `powershell.exe` needs `SystemRoot` and friends to start at all. + */ +function windowsPowerShellEnvironment(): Record { const environment: Record = {}; for (const [key, value] of Object.entries(process.env)) { if (value !== undefined && key.toLowerCase() !== "psmodulepath") environment[key] = value; } - return { ...environment, ...overrides }; + return environment; +} + +/** + * Newline-delimited JSON request loop over `Get-Acl`/`Set-Acl`. + * + * Starting `powershell.exe` measured 1.8-2.1 s on a 2-vCPU Windows Server 2025 host, and the daemon + * secures or verifies five private paths before it can bind its loopback listener, so one process per + * path consumed more than the whole `install` readiness budget and every install was torn back down + * (#90). One process answers every request of a run instead. + * + * Paths arrive as JSON data and are never spliced into the script, so a hostile path cannot become + * code. Each reply carries its request id back, so a failure can only be attributed to the path that + * produced it. Reply text is squeezed to printable ASCII because a redirected PowerShell writes + * stdout in the console code page, which would otherwise corrupt the framing. + */ +const WINDOWS_ACL_HELPER_SCRIPT = [ + "$ErrorActionPreference='Stop'", + "$sid=[System.Security.Principal.WindowsIdentity]::GetCurrent().User.Value", + "$out=[Console]::Out", + "while($true){ $line=[Console]::In.ReadLine(); if($null -eq $line){break}; if($line.Length -eq 0){continue}; " + + "$id=0; try{ $request=$line|ConvertFrom-Json; $id=[int]$request.i; $path=[string]$request.p; " + + "if($request.dir -eq 1){$flags='OICI'}else{$flags=''}; " + + "if($request.op -eq 'apply'){ " + + "$sddl='D:P(A;'+$flags+';FA;;;SY)(A;'+$flags+';FA;;;'+$sid+')'; " + + "$acl=Get-Acl -LiteralPath $path; $acl.SetSecurityDescriptorSddlForm($sddl); " + + "$acl.SetOwner([System.Security.Principal.SecurityIdentifier]::new($sid)); " + + "Set-Acl -LiteralPath $path -AclObject $acl; $reply=[pscustomobject]@{i=$id;ok=$true} " + + "}elseif($request.op -eq 'inspect'){ " + + "$acl=Get-Acl -LiteralPath $path; $owner=$acl.GetOwner([System.Security.Principal.SecurityIdentifier]).Value; " + + "$descriptor=[System.Security.AccessControl.RawSecurityDescriptor]::new($acl.Sddl); " + + "$rules=@($descriptor.DiscretionaryAcl | ForEach-Object { [pscustomobject]@{ " + + "Sid=$_.SecurityIdentifier.Value; Type=$_.AceType.ToString(); Mask=$_.AccessMask; Flags=[int]$_.AceFlags } }); " + + "$reply=[pscustomobject]@{i=$id;ok=$true;acl=[pscustomobject]@{ " + + "Protected=$acl.AreAccessRulesProtected; Current=$sid; Owner=$owner; Rules=@($rules) }} " + + "}else{ throw 'unsupported private ACL operation' } " + + "}catch{ $reply=[pscustomobject]@{i=$id;ok=$false;e=([string]$_.Exception.Message -replace '[^\\x20-\\x7E]','?')} }; " + + "$out.WriteLine(($reply|ConvertTo-Json -Compress -Depth 6)); $out.Flush() }", +].join("; "); + +interface WindowsAclHelper { + readonly send: (line: string) => Promise; + readonly receive: () => Promise; + readonly kill: () => void; + readonly decoder: TextDecoder; + buffer: string; + nextRequestId: number; + spawnCostMs: number | undefined; +} + +let windowsAclHelper: WindowsAclHelper | undefined; +let windowsAclHelperSpawnCostMs: number | undefined; +let windowsAclRequestTail: Promise = Promise.resolve(); + +/** + * JSON restricted to printable ASCII, so the helper's stdin code page cannot mangle a path. Matching + * per UTF-16 code unit is deliberate: each surrogate half becomes its own escape, which + * `ConvertFrom-Json` recombines into the original code point. + */ +function asciiJson(value: unknown): string { + return JSON.stringify(value).replace( + /[^\x20-\x7E]/g, + character => `\\u${character.charCodeAt(0).toString(16).padStart(4, "0")}`, + ); +} + +function startWindowsAclHelper(): WindowsAclHelper { + const subprocess = Bun.spawn( + ["powershell.exe", "-NoProfile", "-NonInteractive", "-Command", WINDOWS_ACL_HELPER_SCRIPT], + { env: windowsPowerShellEnvironment(), stdin: "pipe", stdout: "pipe", stderr: "ignore" }, + ); + // The helper must never hold the daemon's event loop open. It ends by itself: closing our stdin at + // exit makes its `ReadLine` return null and the loop break. + subprocess.unref(); + const stdout = subprocess.stdout.getReader(); + return { + send: async line => { + subprocess.stdin.write(line); + await subprocess.stdin.flush(); + }, + receive: async () => (await stdout.read()).value, + kill: () => subprocess.kill(), + decoder: new TextDecoder(), + buffer: "", + nextRequestId: 1, + spawnCostMs: undefined, + }; +} + +function stopWindowsAclHelper(): void { + const helper = windowsAclHelper; + windowsAclHelper = undefined; + if (helper === undefined) return; + try { + helper.kill(); + } catch { + // Already gone; there is nothing left to reclaim. + } +} + +async function readWindowsAclReply(helper: WindowsAclHelper): Promise { + for (;;) { + const newline = helper.buffer.indexOf("\n"); + if (newline >= 0) { + const line = helper.buffer.slice(0, newline).trim(); + helper.buffer = helper.buffer.slice(newline + 1); + if (line.length > 0) return line; + continue; + } + const chunk = await helper.receive(); + if (chunk === undefined) throw new Error("the private Windows ACL helper exited before replying"); + helper.buffer += helper.decoder.decode(chunk, { stream: true }); + } +} + +async function performWindowsAclRequest( + operation: "apply" | "inspect", + path: string, + directory: boolean, +): Promise { + const started = performance.now(); + let requestId = 0; + let reply: unknown; + try { + const helper = (windowsAclHelper ??= startWindowsAclHelper()); + requestId = helper.nextRequestId; + helper.nextRequestId += 1; + await helper.send(`${asciiJson({ i: requestId, op: operation, p: path, dir: directory ? 1 : 0 })}\n`); + reply = JSON.parse(await readWindowsAclReply(helper)); + if (helper.spawnCostMs === undefined) { + // Only the first round trip pays for `powershell.exe` starting; the rest are pipe writes. + helper.spawnCostMs = performance.now() - started; + windowsAclHelperSpawnCostMs = Math.max(windowsAclHelperSpawnCostMs ?? 0, helper.spawnCostMs); + } + } catch (error) { + stopWindowsAclHelper(); + throw new Error(`the private Windows ACL helper failed for ${path}`, { cause: error }); + } + const envelope = typeof reply === "object" && reply !== null ? reply : undefined; + if (envelope === undefined || Reflect.get(envelope, "i") !== requestId) { + // Answering the wrong request would clear the wrong path, so a desynchronised helper is fatal. + stopWindowsAclHelper(); + throw new Error(`the private Windows ACL helper answered out of order for ${path}`); + } + if (Reflect.get(envelope, "ok") !== true) { + const failure = Reflect.get(envelope, "e"); + const detail = typeof failure === "string" && failure.length > 0 ? failure : "unknown helper failure"; + throw new Error( + operation === "apply" + ? `failed to secure private Windows path ${path}: ${detail}` + : `failed to inspect private Windows ACL for ${path}: ${detail}`, + ); + } + return Reflect.get(envelope, "acl"); +} + +/** + * Requests are strictly serialised: the helper answers one line per line it reads, so overlapping + * writers would race for each other's replies. + */ +function windowsAclRequest(operation: "apply" | "inspect", path: string, directory: boolean): Promise { + const attempt = windowsAclRequestTail + .catch(() => undefined) + .then(() => performWindowsAclRequest(operation, path, directory)); + windowsAclRequestTail = attempt.catch(() => undefined); + return attempt; +} + +/** + * Cost of the single `powershell.exe` start this process paid for ACL enforcement, or `undefined` + * when no ACL work happened (every non-Windows platform). `install` derives its readiness budget + * from it, because the daemon it starts must pay the same start before it can bind. + */ +export function windowsAclSpawnCostMs(): number | undefined { + return windowsAclHelperSpawnCostMs; } async function applyWindowsAcl(path: string, directory: boolean): Promise { if (process.platform !== "win32") return; - const script = - "$Path=$env:OMP_GATEWAY_ACL_PATH; $Directory=$env:OMP_GATEWAY_ACL_DIRECTORY; " + - "$sid=[System.Security.Principal.WindowsIdentity]::GetCurrent().User.Value; " + - "$flags=if($Directory -eq '1'){'OICI'}else{''}; " + - "$sddl='D:P(A;'+$flags+';FA;;;SY)(A;'+$flags+';FA;;;'+$sid+')'; " + - "$acl=Get-Acl -LiteralPath $Path; " + - "$acl.SetSecurityDescriptorSddlForm($sddl); " + - "$acl.SetOwner([System.Security.Principal.SecurityIdentifier]::new($sid)); Set-Acl -LiteralPath $Path -AclObject $acl"; - const subprocess = Bun.spawn(["powershell.exe", "-NoProfile", "-NonInteractive", "-Command", script], { - env: windowsPowerShellEnvironment({ - OMP_GATEWAY_ACL_PATH: path, - OMP_GATEWAY_ACL_DIRECTORY: directory ? "1" : "0", - }), - stdin: "ignore", - stdout: "ignore", - stderr: "pipe", - }); - const stderr = await new Response(subprocess.stderr).text(); - if ((await subprocess.exited) !== 0) throw new Error(`failed to secure private Windows path: ${stderr.trim()}`); + await windowsAclRequest("apply", path, directory); } async function assertWindowsAclPrivate(path: string, directory: boolean): Promise { if (process.platform !== "win32") return; - const script = - "$Path=$env:OMP_GATEWAY_ACL_PATH; $acl=Get-Acl -LiteralPath $Path; " + - "$sid=[System.Security.Principal.WindowsIdentity]::GetCurrent().User.Value; " + - "$owner=$acl.GetOwner([System.Security.Principal.SecurityIdentifier]).Value; " + - "$descriptor=[System.Security.AccessControl.RawSecurityDescriptor]::new($acl.Sddl); " + - "$rules=@($descriptor.DiscretionaryAcl | ForEach-Object { [pscustomobject]@{ " + - "Sid=$_.SecurityIdentifier.Value; Type=$_.AceType.ToString(); Mask=$_.AccessMask; Flags=[int]$_.AceFlags } }); " + - "[pscustomobject]@{ Protected=$acl.AreAccessRulesProtected; Current=$sid; Owner=$owner; Rules=@($rules) } " + - "| ConvertTo-Json -Compress -Depth 3"; - const subprocess = Bun.spawn(["powershell.exe", "-NoProfile", "-NonInteractive", "-Command", script], { - env: windowsPowerShellEnvironment({ OMP_GATEWAY_ACL_PATH: path }), - stdin: "ignore", - stdout: "pipe", - stderr: "ignore", - }); - const text = await new Response(subprocess.stdout).text(); - if ((await subprocess.exited) !== 0) throw new Error("failed to inspect private Windows ACL"); - const value: unknown = JSON.parse(text); + const value = await windowsAclRequest("inspect", path, directory); const protectedAcl = typeof value === "object" && value !== null ? Reflect.get(value, "Protected") : undefined; const current = typeof value === "object" && value !== null ? Reflect.get(value, "Current") : undefined; const owner = typeof value === "object" && value !== null ? Reflect.get(value, "Owner") : undefined; @@ -142,7 +286,7 @@ async function assertWindowsAclPrivate(path: string, directory: boolean): Promis }) : "not-array"; throw new Error( - `unsafe private Windows ACL (protected=${String(protectedAcl)}, ownerMatches=${String(owner === current)}, currentIsSystem=${String(current === "S-1-5-18")}, rules=${JSON.stringify(ruleDiagnostics)})`, + `unsafe private Windows ACL for ${path} (protected=${String(protectedAcl)}, ownerMatches=${String(owner === current)}, currentIsSystem=${String(current === "S-1-5-18")}, rules=${JSON.stringify(ruleDiagnostics)})`, ); } } diff --git a/apps/gateway/test/config.test.ts b/apps/gateway/test/config.test.ts index 174f041..dec9cfb 100644 --- a/apps/gateway/test/config.test.ts +++ b/apps/gateway/test/config.test.ts @@ -11,6 +11,7 @@ import { publicOriginHttpsPort, restoreGatewayConfigFile, rotatePublisherToken, + windowsAclSpawnCostMs, } from "../src/config.ts"; const roots: string[] = []; @@ -82,6 +83,122 @@ async function makeFixtureUnsafe(path: string): Promise { if ((await subprocess.exited) !== 0) throw new Error(`failed to loosen test fixture ACL: ${stderr.trim()}`); } +const FAKE_CURRENT_SID = "S-1-5-21-2000000000-2000000001-2000000002-1001"; +const FULL_CONTROL_MASK = 2_032_127; + +type FakeAclOutcome = "private" | "foreign-principal" | "missing"; + +const fakeAcl = { + spawns: 0, + requests: [] as string[], + outcomes: new Map(), + desynchronise: false, +}; + +function answerFakeAclRequest(line: string): string { + const request = JSON.parse(line) as { readonly i: number; readonly op: string; readonly p: string; readonly dir: number }; + fakeAcl.requests.push(`${request.op} ${request.p}`); + const identifier = fakeAcl.desynchronise ? request.i + 1 : request.i; + const outcome = fakeAcl.outcomes.get(request.p) ?? "private"; + if (outcome === "missing") { + return JSON.stringify({ i: identifier, ok: false, e: `Cannot find path '${request.p}' because it does not exist.` }); + } + if (request.op === "apply") return JSON.stringify({ i: identifier, ok: true }); + const flags = request.dir === 1 ? 3 : 0; + const rules = [ + { Sid: "S-1-5-18", Type: "AccessAllowed", Mask: FULL_CONTROL_MASK, Flags: flags }, + { Sid: FAKE_CURRENT_SID, Type: "AccessAllowed", Mask: FULL_CONTROL_MASK, Flags: flags }, + ]; + // Mirrors `icacls /grant *S-1-1-0:F`, which is how the real fixtures above are loosened. + if (outcome === "foreign-principal") { + rules.push({ Sid: "S-1-1-0", Type: "AccessAllowed", Mask: FULL_CONTROL_MASK, Flags: flags }); + } + return JSON.stringify({ + i: identifier, + ok: true, + acl: { Protected: true, Current: FAKE_CURRENT_SID, Owner: FAKE_CURRENT_SID, Rules: rules }, + }); +} + +/** + * Stands in for `powershell.exe` speaking the ACL helper's newline-delimited JSON protocol, so the + * Windows-only contract can be exercised on every platform. Spawns of anything else pass through. + * Returns the restore function; call it in a `finally` so a failure cannot leak the fake platform. + */ +function installFakePowerShell(): () => void { + const realSpawn = Bun.spawn; + const realPlatform = process.platform; + const setPlatform = (value: string): void => { + Object.defineProperty(process, "platform", { value, writable: true, configurable: true, enumerable: true }); + }; + const fakeSpawn = (command: readonly string[], options?: unknown): unknown => { + if (command[0] !== "powershell.exe") { + return (realSpawn as unknown as (used: readonly string[], rest?: unknown) => unknown)(command, options); + } + fakeAcl.spawns += 1; + const encoder = new TextEncoder(); + const pending: Uint8Array[] = []; + let waiting: ((result: { value?: Uint8Array; done: boolean }) => void) | undefined; + let stdin = ""; + return { + stdin: { + write: (chunk: string): number => { + stdin += chunk; + return chunk.length; + }, + flush: (): number => { + for (;;) { + const newline = stdin.indexOf("\n"); + if (newline < 0) return 0; + const line = stdin.slice(0, newline); + stdin = stdin.slice(newline + 1); + const reply = encoder.encode(`${answerFakeAclRequest(line)}\n`); + const resolve = waiting; + waiting = undefined; + if (resolve === undefined) pending.push(reply); + else resolve({ value: reply, done: false }); + } + }, + }, + stdout: { + getReader: () => ({ + read: async (): Promise<{ value?: Uint8Array; done: boolean }> => { + const next = pending.shift(); + if (next !== undefined) return { value: next, done: false }; + const gate = Promise.withResolvers<{ value?: Uint8Array; done: boolean }>(); + waiting = gate.resolve; + return await gate.promise; + }, + }), + }, + unref: (): undefined => undefined, + kill: (): undefined => undefined, + }; + }; + Bun.spawn = fakeSpawn as unknown as typeof Bun.spawn; + setPlatform("win32"); + return () => { + Bun.spawn = realSpawn; + setPlatform(realPlatform); + fakeAcl.outcomes.clear(); + fakeAcl.desynchronise = false; + }; +} + +/** + * Discards whatever helper process an earlier test left cached inside `config.ts`, by answering one + * request with the wrong id. The product treats that desynchronisation as fatal and drops the + * helper, which both pins that guard and makes the following spawn count exact. + */ +async function dropCachedAclHelper(config: GatewayConfig): Promise { + fakeAcl.desynchronise = true; + try { + await expect(loadOrCreatePublisherToken(config)).rejects.toThrow("answered out of order"); + } finally { + fakeAcl.desynchronise = false; + } +} + describe("secure config", () => { test("loads strict production config and normalizes exact allowlist logins", async () => { const root = await privateRoot(); @@ -266,3 +383,91 @@ describe("secure config", () => { await expect(loadOrCreatePublisherToken(config)).rejects.toThrow("invalid encoding or length"); }, 20_000); }); + +describe("Windows private-path ACL enforcement", () => { + test("secures every private path of a run from a single PowerShell process", async () => { + const root = await privateRoot(); + const config = configForRoot(root); + const restore = installFakePowerShell(); + try { + await dropCachedAclHelper(config); + const spawnsBefore = fakeAcl.spawns; + fakeAcl.requests.length = 0; + await loadOrCreatePublisherToken(config); + await loadOrCreatePublisherToken(config); + await rotatePublisherToken(config); + // Each call applies and inspects the config and state directories and touches the token once: + // fifteen ACL operations that used to cost fifteen `powershell.exe` starts. + expect(fakeAcl.requests).toHaveLength(15); + expect(fakeAcl.spawns - spawnsBefore).toBe(1); + // `install` derives its readiness budget from this, so losing the measurement matters. + expect(windowsAclSpawnCostMs()).toBeGreaterThanOrEqual(0); + } finally { + restore(); + } + }); + + test("rejects an ACL that admits a foreign principal and blames the offending path", async () => { + const root = await privateRoot(); + const config = configForRoot(root); + const restore = installFakePowerShell(); + try { + await loadOrCreatePublisherToken(config); + fakeAcl.outcomes.set(config.paths.stateDir, "foreign-principal"); + fakeAcl.requests.length = 0; + const failure = await loadOrCreatePublisherToken(config).then( + () => undefined, + (error: unknown) => error, + ); + expect(failure).toBeInstanceOf(Error); + expect((failure as Error).message).toContain("unsafe private Windows ACL"); + expect((failure as Error).message).toContain(config.paths.stateDir); + expect((failure as Error).message).not.toContain(config.paths.configDir); + // The safe directory ahead of it was verified, and the run stopped at the unsafe one. + expect(fakeAcl.requests).toEqual([ + `apply ${config.paths.configDir}`, + `inspect ${config.paths.configDir}`, + `apply ${config.paths.stateDir}`, + `inspect ${config.paths.stateDir}`, + ]); + } finally { + restore(); + } + }); + + test("rejects a directory the ACL helper cannot resolve, naming the one that failed", async () => { + const root = await privateRoot(); + const config = configForRoot(root); + const restore = installFakePowerShell(); + try { + fakeAcl.outcomes.set(config.paths.stateDir, "missing"); + await expect(loadOrCreatePublisherToken(config)).rejects.toThrow( + `failed to secure private Windows path ${config.paths.stateDir}`, + ); + fakeAcl.outcomes.set(config.paths.configDir, "missing"); + await expect(loadOrCreatePublisherToken(config)).rejects.toThrow( + `failed to secure private Windows path ${config.paths.configDir}`, + ); + } finally { + restore(); + } + }); + + test("treats an unreadable token ACL as fatal instead of a missing token", async () => { + const root = await privateRoot(); + const config = configForRoot(root); + const restore = installFakePowerShell(); + try { + await loadOrCreatePublisherToken(config); + const stored = await readFile(config.paths.tokenPath, "utf8"); + fakeAcl.outcomes.set(config.paths.tokenPath, "missing"); + await expect(loadOrCreatePublisherToken(config)).rejects.toThrow( + `failed to inspect private Windows ACL for ${config.paths.tokenPath}`, + ); + // A helper failure must never be mistaken for ENOENT and remediated by minting a new token. + expect(await readFile(config.paths.tokenPath, "utf8")).toBe(stored); + } finally { + restore(); + } + }); +}); From c3e251ba894b6a992ad11c91e724b8e04f2be81f Mon Sep 17 00:00:00 2001 From: Sunil Srivatsa Date: Thu, 20 Aug 2026 15:26:00 -0700 Subject: [PATCH 2/8] fix(gateway): bound the ACL helper reply and report its stderr Two mitigations from independent review of this branch. A helper that accepted a request and never answered blocked that ACL check forever. During install the readiness budget bounded it, but a directly-run serve had no such bound, so the wait is now capped and reported as a timeout rather than as a hang with no diagnostic. Helper stderr was discarded, which meant the most likely failure of this design - a helper that never reaches its reply loop because the script is malformed, powershell.exe is missing, or execution policy blocks it - was reported only as "exited before replying". stderr is now captured and joined to the failure it explains. The read happens on its own task, so the catch yields once before sampling it; without that the informative part is lost to a race, which is exactly the case this reporting exists for. Reporting lives in one place. readWindowsAclReply throws plain messages and performWindowsAclRequest decides how a failure is presented, including the cause text that the previous wrapper dropped from the message an operator actually reads. A new test pins it: a helper that dies during start-up surfaces its own stderr. Mutation-proven, 16 pass to 15 pass and 1 fail when the capture is removed. The fake gained a stderr channel and an early-exit mode, because without them no test could reach the start-up path at all. Two other review items were checked and closed without code. The claim that the ported predicate stopped requiring exactly two ACEs is refuted: main already carries the same `rules.length >= allowedSids.size && rules.length <= 2` at config.ts:118-119 and already prints currentIsSystem, so the branch changed neither. The question of whether unref() lets the daemon exit while a reply is pending was answered by experiment on Bun 1.3.14: with the child unref'd and a stdout read outstanding as the only pending work, the read settled after the child's full delay rather than the process exiting. --- apps/gateway/src/config.ts | 50 +++++++++++++++++++++++++++++--- apps/gateway/test/config.test.ts | 39 +++++++++++++++++++++++++ 2 files changed, 85 insertions(+), 4 deletions(-) diff --git a/apps/gateway/src/config.ts b/apps/gateway/src/config.ts index 5dcb26d..962e140 100644 --- a/apps/gateway/src/config.ts +++ b/apps/gateway/src/config.ts @@ -109,6 +109,7 @@ interface WindowsAclHelper { readonly send: (line: string) => Promise; readonly receive: () => Promise; readonly kill: () => void; + readonly stderrSnapshot: () => string; readonly decoder: TextDecoder; buffer: string; nextRequestId: number; @@ -131,15 +132,37 @@ function asciiJson(value: unknown): string { ); } +// A helper that accepts a request and never answers would otherwise block this check forever. During +// `install` the readiness budget bounds that, but a directly-run `serve` has no such bound, so the +// wait is capped here and reported as a timeout rather than as a hang with no diagnostic. +const WINDOWS_ACL_REPLY_TIMEOUT_MS = 20_000; + function startWindowsAclHelper(): WindowsAclHelper { const subprocess = Bun.spawn( ["powershell.exe", "-NoProfile", "-NonInteractive", "-Command", WINDOWS_ACL_HELPER_SCRIPT], - { env: windowsPowerShellEnvironment(), stdin: "pipe", stdout: "pipe", stderr: "ignore" }, + { env: windowsPowerShellEnvironment(), stdin: "pipe", stdout: "pipe", stderr: "pipe" }, ); // The helper must never hold the daemon's event loop open. It ends by itself: closing our stdin at - // exit makes its `ReadLine` return null and the loop break. + // exit makes its `ReadLine` return null and the loop break. A pending stdout read still keeps the + // process alive on its own, verified on Bun 1.3.14: with the child unref'd and a read outstanding as + // the only pending work, the read settled after the child's full 5s delay rather than the process + // exiting early. subprocess.unref(); const stdout = subprocess.stdout.getReader(); + // The helper's own startup and parse failures are written to stderr, never to the structured reply + // channel, which only exists once its loop is already running. Discarding stderr made exactly the + // most likely failure of this design - an unstartable or malformed helper - report as nothing more + // than "exited before replying". + let stderrText = ""; + void (async () => { + try { + for await (const chunk of subprocess.stderr as ReadableStream) { + if (stderrText.length < 2_000) stderrText += new TextDecoder().decode(chunk); + } + } catch { + // The helper is gone; whatever it managed to say is already captured. + } + })(); return { send: async line => { subprocess.stdin.write(line); @@ -147,6 +170,7 @@ function startWindowsAclHelper(): WindowsAclHelper { }, receive: async () => (await stdout.read()).value, kill: () => subprocess.kill(), + stderrSnapshot: () => stderrText.trim().slice(0, 500), decoder: new TextDecoder(), buffer: "", nextRequestId: 1, @@ -166,6 +190,7 @@ function stopWindowsAclHelper(): void { } async function readWindowsAclReply(helper: WindowsAclHelper): Promise { + const deadline = Date.now() + WINDOWS_ACL_REPLY_TIMEOUT_MS; for (;;) { const newline = helper.buffer.indexOf("\n"); if (newline >= 0) { @@ -174,7 +199,15 @@ async function readWindowsAclReply(helper: WindowsAclHelper): Promise { if (line.length > 0) return line; continue; } - const chunk = await helper.receive(); + const remaining = deadline - Date.now(); + // The caller attaches the helper's stderr; these messages stay plain so there is one place that + // decides how a failure is reported. + if (remaining <= 0) throw new Error("the private Windows ACL helper did not reply in time"); + const chunk = await Promise.race([ + helper.receive(), + new Promise<"timeout">(resolve => setTimeout(() => resolve("timeout"), remaining).unref?.()), + ]); + if (chunk === "timeout") throw new Error("the private Windows ACL helper did not reply in time"); if (chunk === undefined) throw new Error("the private Windows ACL helper exited before replying"); helper.buffer += helper.decoder.decode(chunk, { stream: true }); } @@ -188,8 +221,10 @@ async function performWindowsAclRequest( const started = performance.now(); let requestId = 0; let reply: unknown; + let active: WindowsAclHelper | undefined; try { const helper = (windowsAclHelper ??= startWindowsAclHelper()); + active = helper; requestId = helper.nextRequestId; helper.nextRequestId += 1; await helper.send(`${asciiJson({ i: requestId, op: operation, p: path, dir: directory ? 1 : 0 })}\n`); @@ -200,8 +235,15 @@ async function performWindowsAclRequest( windowsAclHelperSpawnCostMs = Math.max(windowsAclHelperSpawnCostMs ?? 0, helper.spawnCostMs); } } catch (error) { + // stderr is read on its own task, so on a start-up failure it may not have arrived yet. Yield + // once before reading it: without this the most informative part of the message is lost to a + // race, which is precisely the case this reporting exists for. + await Bun.sleep(0); + const captured = active?.stderrSnapshot() ?? ""; stopWindowsAclHelper(); - throw new Error(`the private Windows ACL helper failed for ${path}`, { cause: error }); + const reason = error instanceof Error ? error.message : String(error); + const suffix = captured.length > 0 && !reason.includes(captured) ? `: ${captured}` : ""; + throw new Error(`the private Windows ACL helper failed for ${path}: ${reason}${suffix}`, { cause: error }); } const envelope = typeof reply === "object" && reply !== null ? reply : undefined; if (envelope === undefined || Reflect.get(envelope, "i") !== requestId) { diff --git a/apps/gateway/test/config.test.ts b/apps/gateway/test/config.test.ts index dec9cfb..1e10844 100644 --- a/apps/gateway/test/config.test.ts +++ b/apps/gateway/test/config.test.ts @@ -93,6 +93,8 @@ const fakeAcl = { requests: [] as string[], outcomes: new Map(), desynchronise: false, + stderr: "", + exitBeforeReply: false, }; function answerFakeAclRequest(line: string): string { @@ -152,6 +154,14 @@ function installFakePowerShell(): () => void { if (newline < 0) return 0; const line = stdin.slice(0, newline); stdin = stdin.slice(newline + 1); + // A helper that died during start-up consumes the request and answers nothing; its + // stdout closes instead, which the reader below reports as EOF. + if (fakeAcl.exitBeforeReply) { + const closed = waiting; + waiting = undefined; + closed?.({ done: true }); + continue; + } const reply = encoder.encode(`${answerFakeAclRequest(line)}\n`); const resolve = waiting; waiting = undefined; @@ -165,12 +175,19 @@ function installFakePowerShell(): () => void { read: async (): Promise<{ value?: Uint8Array; done: boolean }> => { const next = pending.shift(); if (next !== undefined) return { value: next, done: false }; + // A helper that dies without answering closes its stdout, which the reader sees as EOF. + if (fakeAcl.exitBeforeReply) return { done: true }; const gate = Promise.withResolvers<{ value?: Uint8Array; done: boolean }>(); waiting = gate.resolve; return await gate.promise; }, }), }, + // The real helper writes start-up and parse failures only to stderr, so the fake has to offer + // the same channel or a test can never observe that they reach the caller. + stderr: (async function* (): AsyncGenerator { + if (fakeAcl.stderr.length > 0) yield new TextEncoder().encode(fakeAcl.stderr); + })(), unref: (): undefined => undefined, kill: (): undefined => undefined, }; @@ -182,6 +199,8 @@ function installFakePowerShell(): () => void { setPlatform(realPlatform); fakeAcl.outcomes.clear(); fakeAcl.desynchronise = false; + fakeAcl.stderr = ""; + fakeAcl.exitBeforeReply = false; }; } @@ -470,4 +489,24 @@ describe("Windows private-path ACL enforcement", () => { restore(); } }); + + test("surfaces the helper's own stderr when it dies before replying", async () => { + const root = await privateRoot(); + const config = configForRoot(root); + const restore = installFakePowerShell(); + try { + // The realistic failure of this design is a helper that never gets as far as its reply loop: + // a bad script, a missing powershell.exe, a blocked execution policy. That cause reaches only + // stderr, so discarding it would reduce every such case to "exited before replying" and leave + // the operator with nothing to act on. + // A helper cached by an earlier assertion would answer from its own queue and never reach the + // start-up path this test is about. + await dropCachedAclHelper(config); + fakeAcl.stderr = "ParserError: unexpected token in expression"; + fakeAcl.exitBeforeReply = true; + await expect(loadOrCreatePublisherToken(config)).rejects.toThrow("ParserError: unexpected token in expression"); + } finally { + restore(); + } + }); }); From 0be124348424805716dd797e2f38db17ba8f61d3 Mon Sep 17 00:00:00 2001 From: Sunil Srivatsa Date: Thu, 20 Aug 2026 15:53:12 -0700 Subject: [PATCH 3/8] fix(gateway): stop the stderr reader from hanging bun test on Windows The previous commit read helper stderr with a standing background task. On Windows that hung `bun test apps/gateway/test/config.test.ts` for 23 minutes until the job's 25 minute ceiling cancelled it. The cause is the fact the unref experiment in that same commit had already established and I read only half of: a pending read keeps the process alive even though the child is unref'd. That is the property that makes a reply safe to await, and it is equally the property that makes an open-ended read of a pipe nobody writes to fatal. The stderr read never reached EOF, so the test runner could never exit. stderr is now drained only on the failure path, and only after the helper has been killed so the pipe reaches EOF promptly. A 250ms race remains as a backstop rather than as the mechanism. No read is outstanding at rest. The test fake now hands back a real ReadableStream instead of an async generator, so it exercises the same shape Bun.spawn returns rather than a convenient stand-in that could not have surfaced this. Still 16 pass locally and the stderr test is still mutation-proven. The honest caveat is unchanged: only the Windows job can confirm this, because the hang it fixes was not reproducible on macOS. --- apps/gateway/src/config.ts | 43 +++++++++++++------------------- apps/gateway/test/config.test.ts | 9 ++++--- 2 files changed, 24 insertions(+), 28 deletions(-) diff --git a/apps/gateway/src/config.ts b/apps/gateway/src/config.ts index 962e140..5f7f2a8 100644 --- a/apps/gateway/src/config.ts +++ b/apps/gateway/src/config.ts @@ -109,7 +109,7 @@ interface WindowsAclHelper { readonly send: (line: string) => Promise; readonly receive: () => Promise; readonly kill: () => void; - readonly stderrSnapshot: () => string; + readonly drainStderr: () => Promise; readonly decoder: TextDecoder; buffer: string; nextRequestId: number; @@ -143,26 +143,10 @@ function startWindowsAclHelper(): WindowsAclHelper { { env: windowsPowerShellEnvironment(), stdin: "pipe", stdout: "pipe", stderr: "pipe" }, ); // The helper must never hold the daemon's event loop open. It ends by itself: closing our stdin at - // exit makes its `ReadLine` return null and the loop break. A pending stdout read still keeps the - // process alive on its own, verified on Bun 1.3.14: with the child unref'd and a read outstanding as - // the only pending work, the read settled after the child's full 5s delay rather than the process - // exiting early. + // exit makes its `ReadLine` return null and the loop break. subprocess.unref(); const stdout = subprocess.stdout.getReader(); - // The helper's own startup and parse failures are written to stderr, never to the structured reply - // channel, which only exists once its loop is already running. Discarding stderr made exactly the - // most likely failure of this design - an unstartable or malformed helper - report as nothing more - // than "exited before replying". - let stderrText = ""; - void (async () => { - try { - for await (const chunk of subprocess.stderr as ReadableStream) { - if (stderrText.length < 2_000) stderrText += new TextDecoder().decode(chunk); - } - } catch { - // The helper is gone; whatever it managed to say is already captured. - } - })(); + const stderr = subprocess.stderr as ReadableStream; return { send: async line => { subprocess.stdin.write(line); @@ -170,7 +154,17 @@ function startWindowsAclHelper(): WindowsAclHelper { }, receive: async () => (await stdout.read()).value, kill: () => subprocess.kill(), - stderrSnapshot: () => stderrText.trim().slice(0, 500), + // Read only after the helper has been killed, never as a standing background task. An + // open-ended read of this pipe is not free: a pending read keeps the process alive even though + // the child is unref'd, which hung `bun test` on Windows for 23 minutes until CI cancelled it. + // Killing first makes the pipe EOF, so this returns promptly; the race is a backstop only. + drainStderr: async () => { + const collected = await Promise.race([ + new Response(stderr).text().catch(() => ""), + new Promise(resolve => setTimeout(() => resolve(""), 250).unref?.()), + ]); + return collected.trim().slice(0, 500); + }, decoder: new TextDecoder(), buffer: "", nextRequestId: 1, @@ -235,12 +229,11 @@ async function performWindowsAclRequest( windowsAclHelperSpawnCostMs = Math.max(windowsAclHelperSpawnCostMs ?? 0, helper.spawnCostMs); } } catch (error) { - // stderr is read on its own task, so on a start-up failure it may not have arrived yet. Yield - // once before reading it: without this the most informative part of the message is lost to a - // race, which is precisely the case this reporting exists for. - await Bun.sleep(0); - const captured = active?.stderrSnapshot() ?? ""; + // Kill first so the helper's stderr pipe reaches EOF, then read it. Reading before the kill + // would block on a live pipe, and reading it continuously in the background keeps the process + // alive even with the child unref'd. stopWindowsAclHelper(); + const captured = active === undefined ? "" : await active.drainStderr(); const reason = error instanceof Error ? error.message : String(error); const suffix = captured.length > 0 && !reason.includes(captured) ? `: ${captured}` : ""; throw new Error(`the private Windows ACL helper failed for ${path}: ${reason}${suffix}`, { cause: error }); diff --git a/apps/gateway/test/config.test.ts b/apps/gateway/test/config.test.ts index 1e10844..f3c5aa5 100644 --- a/apps/gateway/test/config.test.ts +++ b/apps/gateway/test/config.test.ts @@ -185,9 +185,12 @@ function installFakePowerShell(): () => void { }, // The real helper writes start-up and parse failures only to stderr, so the fake has to offer // the same channel or a test can never observe that they reach the caller. - stderr: (async function* (): AsyncGenerator { - if (fakeAcl.stderr.length > 0) yield new TextEncoder().encode(fakeAcl.stderr); - })(), + stderr: new ReadableStream({ + start(controller) { + if (fakeAcl.stderr.length > 0) controller.enqueue(new TextEncoder().encode(fakeAcl.stderr)); + controller.close(); + }, + }), unref: (): undefined => undefined, kill: (): undefined => undefined, }; From 8be292f1568fb9cf2c19f3e244773f69ad487383 Mon Sep 17 00:00:00 2001 From: Sunil Srivatsa Date: Thu, 20 Aug 2026 16:01:34 -0700 Subject: [PATCH 4/8] ci(windows): probe the real ACL helper on a Windows runner The helper hung bun test on Windows and no local platform can reproduce it, since macOS early-returns from every Windows path. This runs the exact spawn with the exact script text against real powershell.exe and reports what happens, so the next change is driven by an observation. Every wait is bounded. A probe that reports no reply in 10s is a result; an unbounded one is another 25 minute cancellation that teaches nothing. Temporary, delete once #90 is closed. --- .github/workflows/windows-acl-probe.yml | 80 +++++++++++++++++++++++++ 1 file changed, 80 insertions(+) create mode 100644 .github/workflows/windows-acl-probe.yml diff --git a/.github/workflows/windows-acl-probe.yml b/.github/workflows/windows-acl-probe.yml new file mode 100644 index 0000000..9e8fd58 --- /dev/null +++ b/.github/workflows/windows-acl-probe.yml @@ -0,0 +1,80 @@ +name: Windows ACL helper probe + +# Temporary diagnostic. The ACL helper introduced in PR #91 hung `bun test` on the Windows runner, +# and no local platform can reproduce it: the author's workstation is macOS, where every Windows path +# early-returns. This job runs the real helper against real `powershell.exe` and reports what happens, +# so the fix is driven by an observation rather than a guess. Delete once #90 is closed. + +on: + workflow_dispatch: + push: + branches: [fix/windows-acl-spawns] + +permissions: {} + +jobs: + probe: + runs-on: windows-latest + timeout-minutes: 8 + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + - uses: oven-sh/setup-bun@0c5077e51419868618aeaa5fe8019c62421857d6 # v2.0.2 + with: + bun-version: 1.3.14 + + # Drive the exact spawn the daemon performs, with the exact script text, and bound every wait. + # A bounded probe that reports "no reply in 10s" is a result; an unbounded one is another 25 + # minute cancellation with nothing learned. + - name: Round-trip one request through the real helper + shell: pwsh + timeout-minutes: 4 + run: | + bun -e @' + const source = await Bun.file("apps/gateway/src/config.ts").text(); + const match = source.match(/const WINDOWS_ACL_HELPER_SCRIPT =([\s\S]*?);\n/); + if (!match) throw new Error("could not extract the helper script"); + const script = eval(match[1].trim()); + console.log(`script length: ${script.length}`); + + const child = Bun.spawn(["powershell.exe", "-NoProfile", "-NonInteractive", "-Command", script], { + stdin: "pipe", stdout: "pipe", stderr: "pipe", + }); + + const request = JSON.stringify({ i: 1, op: "inspect", p: process.cwd(), dir: 1 }); + console.log(`request: ${request}`); + child.stdin.write(request + "\n"); + await child.stdin.flush(); + + const reader = child.stdout.getReader(); + const started = Date.now(); + const raced = await Promise.race([ + reader.read(), + new Promise(resolve => setTimeout(() => resolve("TIMEOUT"), 10_000)), + ]); + const elapsed = Date.now() - started; + + if (raced === "TIMEOUT") { + console.log(`RESULT: no reply within ${elapsed}ms - the interactive stdin protocol does not work here`); + } else if (raced.done) { + console.log(`RESULT: stdout closed after ${elapsed}ms without a reply - the helper exited`); + } else { + console.log(`RESULT: replied in ${elapsed}ms: ${new TextDecoder().decode(raced.value).trim()}`); + } + + child.kill(); + const err = await Promise.race([ + new Response(child.stderr).text().catch(() => ""), + new Promise(resolve => setTimeout(() => resolve(""), 2_000)), + ]); + console.log(`stderr: ${err.trim().slice(0, 1200) || "(empty)"}`); + '@ + + # If the round trip works, the hang is elsewhere; if it does not, this is the whole story. + # Either way the suite is run under a hard bound so a hang reports rather than consuming the job. + - name: Run the config suite under a bound + shell: pwsh + timeout-minutes: 3 + continue-on-error: true + run: bun test apps/gateway/test/config.test.ts From 7a550e6cd07445a50e3c84a88200d7e2ac122314 Mon Sep 17 00:00:00 2001 From: Sunil Srivatsa Date: Thu, 20 Aug 2026 16:04:18 -0700 Subject: [PATCH 5/8] ci(windows): tolerate CRLF when extracting the helper script The Windows checkout rewrites the working tree to CRLF, so the probe's regex anchored on ";\n" matched nothing and the job failed before it could measure anything. --- .github/workflows/windows-acl-probe.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/windows-acl-probe.yml b/.github/workflows/windows-acl-probe.yml index 9e8fd58..d75115e 100644 --- a/.github/workflows/windows-acl-probe.yml +++ b/.github/workflows/windows-acl-probe.yml @@ -33,7 +33,7 @@ jobs: run: | bun -e @' const source = await Bun.file("apps/gateway/src/config.ts").text(); - const match = source.match(/const WINDOWS_ACL_HELPER_SCRIPT =([\s\S]*?);\n/); + const match = source.match(/const WINDOWS_ACL_HELPER_SCRIPT =([\s\S]*?);\r?\n/); if (!match) throw new Error("could not extract the helper script"); const script = eval(match[1].trim()); console.log(`script length: ${script.length}`); From d0c8dea780756387c6c58f84b6a35adbc4e0dc6e Mon Sep 17 00:00:00 2001 From: Sunil Srivatsa Date: Thu, 20 Aug 2026 16:09:29 -0700 Subject: [PATCH 6/8] ci(windows): compare -Command, -File, and interactive spawn forms The measured result is that the helper starts cleanly, writes nothing to stderr, and never answers, so [Console]::In.ReadLine() is not receiving our piped line under -Command. This compares that baseline against -File, the documented way to run a script that reads its own stdin. --- .github/workflows/windows-acl-probe.yml | 60 ++++++++++++++----------- 1 file changed, 33 insertions(+), 27 deletions(-) diff --git a/.github/workflows/windows-acl-probe.yml b/.github/workflows/windows-acl-probe.yml index d75115e..e0a7f9a 100644 --- a/.github/workflows/windows-acl-probe.yml +++ b/.github/workflows/windows-acl-probe.yml @@ -38,37 +38,43 @@ jobs: const script = eval(match[1].trim()); console.log(`script length: ${script.length}`); - const child = Bun.spawn(["powershell.exe", "-NoProfile", "-NonInteractive", "-Command", script], { - stdin: "pipe", stdout: "pipe", stderr: "pipe", - }); + const scriptPath = `${process.env.RUNNER_TEMP}\\omp-acl-helper.ps1`; + await Bun.write(scriptPath, script); - const request = JSON.stringify({ i: 1, op: "inspect", p: process.cwd(), dir: 1 }); - console.log(`request: ${request}`); - child.stdin.write(request + "\n"); - await child.stdin.flush(); + // Three candidate spawns. `-Command ` is what the daemon does today and is the + // one already measured as never answering. `-File` is the documented way to run a script + // that reads its own stdin, and `-Command -` tells powershell to take commands from stdin. + const variants = [ + ["baseline -Command", ["powershell.exe", "-NoProfile", "-NonInteractive", "-Command", script]], + ["-File", ["powershell.exe", "-NoProfile", "-NonInteractive", "-File", scriptPath]], + ["-File without -NonInteractive", ["powershell.exe", "-NoProfile", "-File", scriptPath]], + ]; - const reader = child.stdout.getReader(); - const started = Date.now(); - const raced = await Promise.race([ - reader.read(), - new Promise(resolve => setTimeout(() => resolve("TIMEOUT"), 10_000)), - ]); - const elapsed = Date.now() - started; + for (const [label, argv] of variants) { + const child = Bun.spawn(argv, { stdin: "pipe", stdout: "pipe", stderr: "pipe" }); + const request = JSON.stringify({ i: 1, op: "inspect", p: process.cwd(), dir: 1 }); + child.stdin.write(request + "\n"); + await child.stdin.flush(); - if (raced === "TIMEOUT") { - console.log(`RESULT: no reply within ${elapsed}ms - the interactive stdin protocol does not work here`); - } else if (raced.done) { - console.log(`RESULT: stdout closed after ${elapsed}ms without a reply - the helper exited`); - } else { - console.log(`RESULT: replied in ${elapsed}ms: ${new TextDecoder().decode(raced.value).trim()}`); - } + const reader = child.stdout.getReader(); + const started = Date.now(); + const raced = await Promise.race([ + reader.read(), + new Promise(resolve => setTimeout(() => resolve("TIMEOUT"), 10_000)), + ]); + const elapsed = Date.now() - started; + + if (raced === "TIMEOUT") console.log(`RESULT ${label}: no reply within ${elapsed}ms`); + else if (raced.done) console.log(`RESULT ${label}: stdout closed after ${elapsed}ms without a reply`); + else console.log(`RESULT ${label}: replied in ${elapsed}ms: ${new TextDecoder().decode(raced.value).trim()}`); - child.kill(); - const err = await Promise.race([ - new Response(child.stderr).text().catch(() => ""), - new Promise(resolve => setTimeout(() => resolve(""), 2_000)), - ]); - console.log(`stderr: ${err.trim().slice(0, 1200) || "(empty)"}`); + child.kill(); + const err = await Promise.race([ + new Response(child.stderr).text().catch(() => ""), + new Promise(resolve => setTimeout(() => resolve(""), 2_000)), + ]); + console.log(`stderr ${label}: ${err.trim().slice(0, 600) || "(empty)"}`); + } '@ # If the round trip works, the hang is elsewhere; if it does not, this is the whole story. From 54fbdc9df6d99435d94e293bc72889fa0f644bda Mon Sep 17 00:00:00 2001 From: Sunil Srivatsa Date: Thu, 20 Aug 2026 16:17:08 -0700 Subject: [PATCH 7/8] ci(windows): isolate PSModulePath and ErrorActionPreference All three spawn forms replied, so the earlier no-reply reading was a 10s bound shorter than a cold module load, not a broken stdin protocol. The actual failure is Get-Acl reporting that its module could not be loaded. main strips PSModulePath identically, so this isolates whether the stripping or the newly added ErrorActionPreference=Stop is what makes it fatal. --- .github/workflows/windows-acl-probe.yml | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/.github/workflows/windows-acl-probe.yml b/.github/workflows/windows-acl-probe.yml index e0a7f9a..a83f9dd 100644 --- a/.github/workflows/windows-acl-probe.yml +++ b/.github/workflows/windows-acl-probe.yml @@ -44,14 +44,22 @@ jobs: // Three candidate spawns. `-Command ` is what the daemon does today and is the // one already measured as never answering. `-File` is the documented way to run a script // that reads its own stdin, and `-Command -` tells powershell to take commands from stdin. + const stripped = {}; + for (const [k, v] of Object.entries(process.env)) { + if (v !== undefined && k.toLowerCase() !== "psmodulepath") stripped[k] = v; + } + const withModules = { ...stripped, PSModulePath: process.env.PSModulePath ?? "" }; + const noStop = script.replace("$ErrorActionPreference='Stop'; ", ""); + const variants = [ - ["baseline -Command", ["powershell.exe", "-NoProfile", "-NonInteractive", "-Command", script]], - ["-File", ["powershell.exe", "-NoProfile", "-NonInteractive", "-File", scriptPath]], - ["-File without -NonInteractive", ["powershell.exe", "-NoProfile", "-File", scriptPath]], + ["stripped PSModulePath (current)", ["powershell.exe", "-NoProfile", "-NonInteractive", "-Command", script], stripped], + ["PSModulePath restored", ["powershell.exe", "-NoProfile", "-NonInteractive", "-Command", script], withModules], + ["stripped, no ErrorActionPreference", ["powershell.exe", "-NoProfile", "-NonInteractive", "-Command", noStop], stripped], + ["-File with PSModulePath restored", ["powershell.exe", "-NoProfile", "-NonInteractive", "-File", scriptPath], withModules], ]; - for (const [label, argv] of variants) { - const child = Bun.spawn(argv, { stdin: "pipe", stdout: "pipe", stderr: "pipe" }); + for (const [label, argv, env] of variants) { + const child = Bun.spawn(argv, { stdin: "pipe", stdout: "pipe", stderr: "pipe", env }); const request = JSON.stringify({ i: 1, op: "inspect", p: process.cwd(), dir: 1 }); child.stdin.write(request + "\n"); await child.stdin.flush(); @@ -66,7 +74,7 @@ jobs: if (raced === "TIMEOUT") console.log(`RESULT ${label}: no reply within ${elapsed}ms`); else if (raced.done) console.log(`RESULT ${label}: stdout closed after ${elapsed}ms without a reply`); - else console.log(`RESULT ${label}: replied in ${elapsed}ms: ${new TextDecoder().decode(raced.value).trim()}`); + else console.log(`RESULT ${label}: replied in ${elapsed}ms: ${new TextDecoder().decode(raced.value).trim().slice(0, 400)}`); child.kill(); const err = await Promise.race([ From 3204a7d3f75461a6676eb6dce74991ab9c275428 Mon Sep 17 00:00:00 2001 From: Sunil Srivatsa Date: Thu, 20 Aug 2026 16:21:55 -0700 Subject: [PATCH 8/8] ci(windows): remove the temporary ACL probe It answered its question. Findings are recorded on PR #91. --- .github/workflows/windows-acl-probe.yml | 94 ------------------------- 1 file changed, 94 deletions(-) delete mode 100644 .github/workflows/windows-acl-probe.yml diff --git a/.github/workflows/windows-acl-probe.yml b/.github/workflows/windows-acl-probe.yml deleted file mode 100644 index a83f9dd..0000000 --- a/.github/workflows/windows-acl-probe.yml +++ /dev/null @@ -1,94 +0,0 @@ -name: Windows ACL helper probe - -# Temporary diagnostic. The ACL helper introduced in PR #91 hung `bun test` on the Windows runner, -# and no local platform can reproduce it: the author's workstation is macOS, where every Windows path -# early-returns. This job runs the real helper against real `powershell.exe` and reports what happens, -# so the fix is driven by an observation rather than a guess. Delete once #90 is closed. - -on: - workflow_dispatch: - push: - branches: [fix/windows-acl-spawns] - -permissions: {} - -jobs: - probe: - runs-on: windows-latest - timeout-minutes: 8 - steps: - - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - with: - persist-credentials: false - - uses: oven-sh/setup-bun@0c5077e51419868618aeaa5fe8019c62421857d6 # v2.0.2 - with: - bun-version: 1.3.14 - - # Drive the exact spawn the daemon performs, with the exact script text, and bound every wait. - # A bounded probe that reports "no reply in 10s" is a result; an unbounded one is another 25 - # minute cancellation with nothing learned. - - name: Round-trip one request through the real helper - shell: pwsh - timeout-minutes: 4 - run: | - bun -e @' - const source = await Bun.file("apps/gateway/src/config.ts").text(); - const match = source.match(/const WINDOWS_ACL_HELPER_SCRIPT =([\s\S]*?);\r?\n/); - if (!match) throw new Error("could not extract the helper script"); - const script = eval(match[1].trim()); - console.log(`script length: ${script.length}`); - - const scriptPath = `${process.env.RUNNER_TEMP}\\omp-acl-helper.ps1`; - await Bun.write(scriptPath, script); - - // Three candidate spawns. `-Command ` is what the daemon does today and is the - // one already measured as never answering. `-File` is the documented way to run a script - // that reads its own stdin, and `-Command -` tells powershell to take commands from stdin. - const stripped = {}; - for (const [k, v] of Object.entries(process.env)) { - if (v !== undefined && k.toLowerCase() !== "psmodulepath") stripped[k] = v; - } - const withModules = { ...stripped, PSModulePath: process.env.PSModulePath ?? "" }; - const noStop = script.replace("$ErrorActionPreference='Stop'; ", ""); - - const variants = [ - ["stripped PSModulePath (current)", ["powershell.exe", "-NoProfile", "-NonInteractive", "-Command", script], stripped], - ["PSModulePath restored", ["powershell.exe", "-NoProfile", "-NonInteractive", "-Command", script], withModules], - ["stripped, no ErrorActionPreference", ["powershell.exe", "-NoProfile", "-NonInteractive", "-Command", noStop], stripped], - ["-File with PSModulePath restored", ["powershell.exe", "-NoProfile", "-NonInteractive", "-File", scriptPath], withModules], - ]; - - for (const [label, argv, env] of variants) { - const child = Bun.spawn(argv, { stdin: "pipe", stdout: "pipe", stderr: "pipe", env }); - const request = JSON.stringify({ i: 1, op: "inspect", p: process.cwd(), dir: 1 }); - child.stdin.write(request + "\n"); - await child.stdin.flush(); - - const reader = child.stdout.getReader(); - const started = Date.now(); - const raced = await Promise.race([ - reader.read(), - new Promise(resolve => setTimeout(() => resolve("TIMEOUT"), 10_000)), - ]); - const elapsed = Date.now() - started; - - if (raced === "TIMEOUT") console.log(`RESULT ${label}: no reply within ${elapsed}ms`); - else if (raced.done) console.log(`RESULT ${label}: stdout closed after ${elapsed}ms without a reply`); - else console.log(`RESULT ${label}: replied in ${elapsed}ms: ${new TextDecoder().decode(raced.value).trim().slice(0, 400)}`); - - child.kill(); - const err = await Promise.race([ - new Response(child.stderr).text().catch(() => ""), - new Promise(resolve => setTimeout(() => resolve(""), 2_000)), - ]); - console.log(`stderr ${label}: ${err.trim().slice(0, 600) || "(empty)"}`); - } - '@ - - # If the round trip works, the hang is elsewhere; if it does not, this is the whole story. - # Either way the suite is run under a hard bound so a hang reports rather than consuming the job. - - name: Run the config suite under a bound - shell: pwsh - timeout-minutes: 3 - continue-on-error: true - run: bun test apps/gateway/test/config.test.ts