diff --git a/scripts/test-layout/layout.json b/scripts/test-layout/layout.json index c0ec0cbd94f..1f00b4019aa 100644 --- a/scripts/test-layout/layout.json +++ b/scripts/test-layout/layout.json @@ -1936,7 +1936,7 @@ "windows-popup-fix.test.ts": "windows", "windows-process-priority.test.ts": "windows", "windows-scheduler-install-verification.test.ts": "windows", - "windows-secret-acl.test.ts": "windows", "windows-secret-acl-removal-flight.test.ts": "windows", + "windows-secret-acl-diagnostics.test.ts": "windows", "windows-secret-acl.test.ts": "windows", "windows-secret-acl-removal-flight.test.ts": "windows", "windows-config-read-acl-memo.test.ts": "windows", "windows-service-mutation-lock.test.ts": "windows", "windows-service-wrappers.test.ts": "windows", diff --git a/src/lib/windows-secret-acl.ts b/src/lib/windows-secret-acl.ts index 5841a656fb7..91654be801b 100644 --- a/src/lib/windows-secret-acl.ts +++ b/src/lib/windows-secret-acl.ts @@ -331,11 +331,67 @@ export interface IcaclsResult { exitCode: number | null; timedOut: boolean; stdout: string; + /** + * Diagnostics only: how a failure was produced when it is not a child + * result. `spawn_failed` means the spawn call itself threw — no process + * existed; `runner_rejected` means the runner promise rejected without a + * result; `runner_watchdog` means the caller-side belt fired before the + * runner settled — a killed child may still be reaping. Absent means + * `child_result` (a spawned child produced a verdict, including timeouts). + */ + readonly failureStage?: "spawn_failed" | "runner_rejected" | "runner_watchdog"; + /** + * Allowlisted OS code from the failed spawn (e.g. ENOENT), never an error + * object, message, or path. Present only alongside `failureStage === + * "spawn_failed"`. + */ + readonly spawnErrorCode?: string; } type IcaclsRunner = (args: string[], timeoutMs: number) => IcaclsResult; type AsyncIcaclsRunner = (args: string[], timeoutMs: number) => Promise; +/** The only icacls operations this module runs; argv is never derived from it. */ +export type IcaclsStep = "/grant:r" | "/inheritance:r" | "/remove:g" | "/findsid"; +const ICACLS_STEPS: ReadonlySet = new Set(["/grant:r", "/inheritance:r", "/remove:g", "/findsid"]); + +/** + * Fixed failure stages for `aclFailure` diagnostics: the shared deadline ran + * out before a spawn was attempted, the spawn itself threw, the runner + * rejected, the caller-side watchdog fired before the runner settled, or a + * child process produced a result. Enum literals only — never parsed from + * errors, and each value is written only by the code that can prove it. + */ +export type AclFailureStage = + | "deadline_before_spawn" + | "spawn_failed" + | "runner_rejected" + | "runner_watchdog" + | "child_result"; + +/** + * Bounded, allowlisted failure metadata carried on a sanitized ACL error. + * Every field is a fixed enum literal, a boolean, or a finite number — no + * path, argv, SID, stdout/stderr, message or stack ever crosses this boundary, + * so a caller may log or branch on it without re-sanitizing. + */ +export interface AclFailureDiagnostics { + /** The icacls step being attempted — one of the fixed literals used by runIcacls. */ + readonly step: IcaclsStep; + readonly stage: AclFailureStage; + /** + * For spawned steps, the measured runner time; for `deadline_before_spawn`, + * how much of the shared sequence had already elapsed. + */ + readonly elapsedMs: number; + /** The exact remaining budget handed to the runner (<= 0 when refused pre-spawn). */ + readonly budgetMs: number; + readonly success: boolean; + readonly exitCode: number | null; + readonly timedOut: boolean; + readonly spawnErrorCode?: string; +} + function resolveIcaclsExecutable(): string { // Same authority as schtasks/powershell: never take icacls from PATH. // A bun-shim or stripped PATH makes `icacls.exe` throw ENOENT, which used to @@ -343,8 +399,55 @@ function resolveIcaclsExecutable(): string { return resolveTrustedWindowsIcaclsExe(); } -function spawnFailedResult(): IcaclsResult { - return { success: false, exitCode: null, timedOut: false, stdout: "" }; +/** + * OS spawn codes safe to surface in diagnostics: fixed symbolic names that + * say WHY the child could not start. Anything else — messages, paths, + * platform-specific strings — stays inside the process. + */ +const ALLOWED_SPAWN_ERROR_CODES: ReadonlySet = new Set([ + "ENOENT", + "EACCES", + "EPERM", + "EAGAIN", + "ENOMEM", +]); + +function allowlistedSpawnErrorCode(code: unknown): string | undefined { + return typeof code === "string" && ALLOWED_SPAWN_ERROR_CODES.has(code) ? code : undefined; +} + +/** Extract an allowlisted spawn code from a thrown spawn error; never the error itself. */ +function spawnErrorCodeOf(error: unknown): string | undefined { + if (error && typeof error === "object" && "code" in error) { + return allowlistedSpawnErrorCode((error as { code?: unknown }).code); + } + return undefined; +} + +function spawnFailedResult(spawnErrorCode?: string): IcaclsResult { + return { + success: false, + exitCode: null, + timedOut: false, + stdout: "", + failureStage: "spawn_failed", + ...(spawnErrorCode !== undefined ? { spawnErrorCode } : {}), + }; +} + +/** + * The async runner rejected instead of producing a result. That is not spawn + * evidence — nothing proves a child ever existed — so it keeps its own stage + * and carries no spawnErrorCode. + */ +function runnerRejectedResult(): IcaclsResult { + return { + success: false, + exitCode: null, + timedOut: false, + stdout: "", + failureStage: "runner_rejected", + }; } /** @@ -354,14 +457,15 @@ function spawnFailedResult(): IcaclsResult { */ function trySpawnIcacls(args: string[]) { try { - return Bun.spawn([resolveIcaclsExecutable(), ...args], { + const proc = Bun.spawn([resolveIcaclsExecutable(), ...args], { stdin: "ignore", stdout: "pipe", stderr: "ignore", windowsHide: true, }); - } catch { - return null; + return { proc, spawnErrorCode: undefined }; + } catch (error) { + return { proc: null, spawnErrorCode: spawnErrorCodeOf(error) }; } } @@ -382,8 +486,8 @@ function defaultIcaclsRunner(args: string[], timeoutMs: number): IcaclsResult { timedOut: result.exitedDueToTimeout ?? false, stdout: result.stdout ? result.stdout.toString() : "", }; - } catch { - return spawnFailedResult(); + } catch (error) { + return spawnFailedResult(spawnErrorCodeOf(error)); } } @@ -393,8 +497,9 @@ function defaultIcaclsRunner(args: string[], timeoutMs: number): IcaclsResult { * classifies the deadline and keeps waiting for a killed child to actually exit. */ async function defaultAsyncIcaclsRunner(args: string[], timeoutMs: number): Promise { - const proc = trySpawnIcacls(args); - if (!proc) return spawnFailedResult(); + const spawned = trySpawnIcacls(args); + if (!spawned.proc) return spawnFailedResult(spawned.spawnErrorCode); + const proc = spawned.proc; const { exitCode, timedOut } = await waitForSubprocessExit(proc, timeoutMs); const stdout = !timedOut && proc.stdout ? await new Response(proc.stdout).text().catch(() => "") @@ -419,7 +524,7 @@ function awaitAsyncIcaclsRunner(args: string[], timeoutMs: number): Promise asyncIcaclsRunner(args, timeoutMs).then( result => { finish(result); }, - () => { finish(spawnFailedResult()); }, + () => { finish(runnerRejectedResult()); }, ); // Capture and register ownership before the child can start or mutate its alias. const tracked = args[0] ? startTrackedAsyncAclWork(args[0], run) : undefined; @@ -432,7 +537,13 @@ function awaitAsyncIcaclsRunner(args: string[], timeoutMs: number): Promise { const targetPath = args[0]; if (targetPath && tracked) registerPendingAsyncIcaclsReap(targetPath, tracked.entry); - finish({ success: false, exitCode: null, timedOut: true, stdout: "" }); + finish({ + success: false, + exitCode: null, + timedOut: true, + stdout: "", + failureStage: "runner_watchdog", + }); }, Math.max(1, timeoutMs) + SUBPROCESS_KILL_GRACE_MS + ASYNC_ICACLS_BELT_MARGIN_MS, ); @@ -730,12 +841,65 @@ export function windowsSecretAclApplies(): boolean { return (platformOverride ?? process.platform) === "win32"; } +/** + * Per-spawn observation measured by run(): which stage produced the result, + * how long it took, and the exact budget it was given. Measured at the call + * site — not inside the runner — so injected runners are observed identically. + */ +interface IcaclsRunMeta { + readonly stage: AclFailureStage; + readonly elapsedMs: number; + readonly budgetMs: number; +} + +/** Runner-reported failure provenance; anything else is treated as a child result. */ +const RUNNER_FAILURE_STAGES: ReadonlySet = new Set([ + "spawn_failed", + "runner_rejected", + "runner_watchdog", +]); + +/** + * An injected runner may write any string into failureStage, so the enum is + * re-validated at the boundary instead of trusting the declared type. + */ +function runnerFailureStageOf(result: IcaclsResult): AclFailureStage { + return typeof result.failureStage === "string" && RUNNER_FAILURE_STAGES.has(result.failureStage) + ? result.failureStage + : "child_result"; +} + +function finiteOrZero(value: number): number { + return Number.isFinite(value) ? value : 0; +} + /** Error carrying an honest code: ETIMEDOUT only for real timeouts, EICACLS otherwise. */ -function icaclsError(step: string, result: IcaclsResult): NodeJS.ErrnoException { +function icaclsError(step: IcaclsStep, result: IcaclsResult, meta: IcaclsRunMeta): NodeJS.ErrnoException { + // Coerce once so the error code and the diagnostics record can never disagree. + // Boolean() keeps the original truthy semantics the memo/retry callers relied + // on — a mistyped truthy timedOut still means "timeout", not "child ok". + const timedOut = Boolean(result.timedOut); const err = new Error( - result.timedOut ? `icacls ${step} timed out` : `icacls ${step} exited ${result.exitCode ?? "null"}`, - ) as NodeJS.ErrnoException; - err.code = result.timedOut ? "ETIMEDOUT" : "EICACLS"; + timedOut ? `icacls ${step} timed out` : `icacls ${step} exited ${result.exitCode ?? "null"}`, + ) as NodeJS.ErrnoException & { aclFailure: AclFailureDiagnostics }; + err.code = timedOut ? "ETIMEDOUT" : "EICACLS"; + // Injected runners are untrusted: every field is coerced back to its declared + // enum/number/boolean shape so only allowlisted values reach the boundary. + const spawnErrorCode = result.failureStage === "spawn_failed" + ? allowlistedSpawnErrorCode(result.spawnErrorCode) + : undefined; + err.aclFailure = { + step, + stage: meta.stage, + elapsedMs: finiteOrZero(meta.elapsedMs), + budgetMs: finiteOrZero(meta.budgetMs), + success: Boolean(result.success), + exitCode: typeof result.exitCode === "number" && Number.isFinite(result.exitCode) + ? result.exitCode + : null, + timedOut, + ...(spawnErrorCode !== undefined ? { spawnErrorCode } : {}), + }; return err; } @@ -871,18 +1035,36 @@ async function existingAclAlreadyCompliantAsync( function runIcacls(targetPath: string, directory: boolean, deadline: number, extraReadAces: readonly string[]): void { const principal = currentWindowsPrincipal(deadline); + const sequenceStartedAt = nowFn(); // The deadline is owned by hardenEntry (total budget incl. retry + verification). - const run = (step: string, args: string[]): IcaclsResult => { + const run = (step: IcaclsStep, args: string[]): { result: IcaclsResult; meta: IcaclsRunMeta } => { const remaining = deadline - nowFn(); if (remaining <= 0) { - throw icaclsError(step, { success: false, exitCode: null, timedOut: true, stdout: "" }); + throw icaclsError( + step, + { success: false, exitCode: null, timedOut: true, stdout: "" }, + { + stage: "deadline_before_spawn", + elapsedMs: Math.max(0, nowFn() - sequenceStartedAt), + budgetMs: remaining, + }, + ); } - return icaclsRunner(args, remaining); + const started = nowFn(); + const result = icaclsRunner(args, remaining); + return { + result, + meta: { + stage: runnerFailureStageOf(result), + elapsedMs: Math.max(0, nowFn() - started), + budgetMs: remaining, + }, + }; }; - const runOrThrow = (step: string, args: string[]): void => { - const result = run(step, args); - if (!result.success) throw icaclsError(step, result); + const runOrThrow = (step: IcaclsStep, args: string[]): void => { + const { result, meta } = run(step, args); + if (!result.success) throw icaclsError(step, result, meta); }; // Step 1: grant current user full control BEFORE any destructive ACL change. @@ -898,17 +1080,17 @@ function runIcacls(targetPath: string, directory: boolean, deadline: number, ext // before accepting the failure as harmless — a swallowed real failure would leave // Everyone/Users/Authenticated Users grants while reporting hardened. // `/remove:g` cannot remove the explicit current-user ACE installed in step 1. - const removal = run("/remove:g", [targetPath, "/remove:g", ...BROAD_SIDS]); + const { result: removal, meta: removalMeta } = run("/remove:g", [targetPath, "/remove:g", ...BROAD_SIDS]); if (!removal.success) { - if (removal.timedOut) throw icaclsError("/remove:g", removal); + if (removal.timedOut) throw icaclsError("/remove:g", removal, removalMeta); for (const sid of BROAD_SIDS) { - const found = run("/findsid", [targetPath, "/findsid", sid]); - if (!found.success) throw icaclsError("/findsid", found); + const { result: found, meta: foundMeta } = run("/findsid", [targetPath, "/findsid", sid]); + if (!found.success) throw icaclsError("/findsid", found, foundMeta); // icacls /findsid echoes the target path in its "SID Found" line only when the SID // still holds an ACE; the summary lines carry only counts. Matching the path echo — // not the (localized) prose — keeps the check locale-independent. if (found.stdout.includes(targetPath)) { - throw icaclsError("/remove:g", removal); + throw icaclsError("/remove:g", removal, removalMeta); } } } @@ -917,30 +1099,48 @@ function runIcacls(targetPath: string, directory: boolean, deadline: number, ext /** Async counterpart of runIcacls — same step order and timeout/error classification (#612). */ async function runIcaclsAsync(targetPath: string, directory: boolean, deadline: number, extraReadAces: readonly string[]): Promise { const principal = await currentWindowsPrincipalAsync(deadline); + const sequenceStartedAt = nowFn(); - const run = async (step: string, args: string[]): Promise => { + const run = async (step: IcaclsStep, args: string[]): Promise<{ result: IcaclsResult; meta: IcaclsRunMeta }> => { const remaining = deadline - nowFn(); if (remaining <= 0) { - throw icaclsError(step, { success: false, exitCode: null, timedOut: true, stdout: "" }); + throw icaclsError( + step, + { success: false, exitCode: null, timedOut: true, stdout: "" }, + { + stage: "deadline_before_spawn", + elapsedMs: Math.max(0, nowFn() - sequenceStartedAt), + budgetMs: remaining, + }, + ); } - return awaitAsyncIcaclsRunner(args, remaining); + const started = nowFn(); + const result = await awaitAsyncIcaclsRunner(args, remaining); + return { + result, + meta: { + stage: runnerFailureStageOf(result), + elapsedMs: Math.max(0, nowFn() - started), + budgetMs: remaining, + }, + }; }; - const runOrThrow = async (step: string, args: string[]): Promise => { - const result = await run(step, args); - if (!result.success) throw icaclsError(step, result); + const runOrThrow = async (step: IcaclsStep, args: string[]): Promise => { + const { result, meta } = await run(step, args); + if (!result.success) throw icaclsError(step, result, meta); }; await runOrThrow("/grant:r", [targetPath, "/grant:r", grantAce(principal, directory), ...extraReadAces]); await runOrThrow("/inheritance:r", [targetPath, "/inheritance:r"]); - const removal = await run("/remove:g", [targetPath, "/remove:g", ...BROAD_SIDS]); + const { result: removal, meta: removalMeta } = await run("/remove:g", [targetPath, "/remove:g", ...BROAD_SIDS]); if (!removal.success) { - if (removal.timedOut) throw icaclsError("/remove:g", removal); + if (removal.timedOut) throw icaclsError("/remove:g", removal, removalMeta); for (const sid of BROAD_SIDS) { - const found = await run("/findsid", [targetPath, "/findsid", sid]); - if (!found.success) throw icaclsError("/findsid", found); + const { result: found, meta: foundMeta } = await run("/findsid", [targetPath, "/findsid", sid]); + if (!found.success) throw icaclsError("/findsid", found, foundMeta); if (found.stdout.includes(targetPath)) { - throw icaclsError("/remove:g", removal); + throw icaclsError("/remove:g", removal, removalMeta); } } } @@ -976,9 +1176,48 @@ function isTimeoutError(error: unknown): boolean { && String((error as NodeJS.ErrnoException).code) === "ETIMEDOUT"; } +/** + * Field-by-field allowlist copy of an aclFailure record from an untrusted cause. + * A malformed field invalidates the whole record except `spawnErrorCode`, + * which is optional and simply omitted when it is not an allowlisted code. + */ +function sanitizeAclFailure(cause: unknown): AclFailureDiagnostics | undefined { + if (!cause || typeof cause !== "object" || !("aclFailure" in cause)) return undefined; + const raw = (cause as { aclFailure?: unknown }).aclFailure; + if (!raw || typeof raw !== "object") return undefined; + const f = raw as Record; + if (typeof f.step !== "string" || !ICACLS_STEPS.has(f.step)) return undefined; + if ( + f.stage !== "deadline_before_spawn" && + f.stage !== "spawn_failed" && + f.stage !== "runner_rejected" && + f.stage !== "runner_watchdog" && + f.stage !== "child_result" + ) { + return undefined; + } + if (typeof f.elapsedMs !== "number" || !Number.isFinite(f.elapsedMs)) return undefined; + if (typeof f.budgetMs !== "number" || !Number.isFinite(f.budgetMs)) return undefined; + if (typeof f.success !== "boolean" || typeof f.timedOut !== "boolean") return undefined; + if (f.exitCode !== null && (typeof f.exitCode !== "number" || !Number.isFinite(f.exitCode))) { + return undefined; + } + const spawnErrorCode = allowlistedSpawnErrorCode(f.spawnErrorCode); + return { + step: f.step as IcaclsStep, + stage: f.stage, + elapsedMs: f.elapsedMs, + budgetMs: f.budgetMs, + success: f.success, + exitCode: f.exitCode as number | null, + timedOut: f.timedOut, + ...(spawnErrorCode !== undefined ? { spawnErrorCode } : {}), + }; +} + /** Preserve only the bounded machine-readable cause on a sanitized public error. */ function sanitizedAclError(diagnostics: string, cause: unknown): NodeJS.ErrnoException { - const error = new Error(diagnostics) as NodeJS.ErrnoException; + const error = new Error(diagnostics) as NodeJS.ErrnoException & { aclFailure?: AclFailureDiagnostics }; const code = cause && typeof cause === "object" && "code" in cause ? String((cause as { code?: unknown }).code) : ""; @@ -995,6 +1234,8 @@ function sanitizedAclError(diagnostics: string, cause: unknown): NodeJS.ErrnoExc ) { error.code = code; } + const aclFailure = sanitizeAclFailure(cause); + if (aclFailure) error.aclFailure = aclFailure; return error; } diff --git a/structure/ops/service-and-sidecars.md b/structure/ops/service-and-sidecars.md index 3f71bb3aa3e..5fc84e34777 100644 --- a/structure/ops/service-and-sidecars.md +++ b/structure/ops/service-and-sidecars.md @@ -120,7 +120,7 @@ enumeration twice made a measured 12.3-second fallback cost roughly 25 seconds b and every async ACL hardening flight and deadline-surviving `icacls.exe` runner under that home have settled. `src/config/paths.ts` owns the barrier: a timeout verdict alone does not make the home removable. The contract is exercised by `tests/server/server-stop-config-hardening.test.ts`. -`src/lib/windows-secret-acl.ts` retains removal ownership for an entire async harden, including principal lookup, successive ACL commands, retries and diagnostics, as well as tracking runners that outlive their deadlines. The exact-path timeout indicator keeps its existing meaning for atomic-file fallback. The Bun regression in `tests/windows/windows-secret-acl-removal-flight.test.ts` holds a normal command and verifies that removal remains blocked between successive commands. +`src/lib/windows-secret-acl.ts` retains removal ownership for an entire async harden, including principal lookup, successive ACL commands, retries and diagnostics, as well as tracking runners that outlive their deadlines. The exact-path timeout indicator keeps its existing meaning for atomic-file fallback. The Bun regression in `tests/windows/windows-secret-acl-removal-flight.test.ts` holds a normal command and verifies that removal remains blocked between successive commands. Required-mode failures keep their sanitized ETIMEDOUT/EICACLS semantics and additionally carry a bounded `aclFailure` record — fixed step and stage enums (`deadline_before_spawn`/`spawn_failed`/`runner_rejected`/`runner_watchdog`/`child_result`), measured elapsed and requested budget, success, exit code, and an allowlisted spawn code — copied field-by-field so no path, argv, SID, or child output crosses into the thrown error; `tests/windows/windows-secret-acl-diagnostics.test.ts` covers the boundary. Removal ownership captures lexical and canonical path identities before each work item or child starts, including Windows short names and directory aliases; deadline survivors retain the same identities. A deleted temporary file is captured through its existing parent. Unreadable identity at capture stays conservatively matched until settlement. Removal compares the requested root against those retained identities, so deleting or retargeting an alias cannot release the original root early. The same Bun regression verifies aliases changed mid-flight still block removal. Matching covers captured and current alias destinations; a second retarget by a writer of the protected directory is out of scope. diff --git a/tests/codex-integration/bearer-admission-routed-provider.test.ts b/tests/codex-integration/bearer-admission-routed-provider.test.ts index 21d036a0544..848368eb3fd 100644 --- a/tests/codex-integration/bearer-admission-routed-provider.test.ts +++ b/tests/codex-integration/bearer-admission-routed-provider.test.ts @@ -122,6 +122,7 @@ function cursorForwardConfig(baseUrl: string, apiKey?: string): OcxConfig { async function withCursorCaptureServer( run: (baseUrl: string, capturedAuth: Array) => Promise, beforeResponse?: () => Promise, + closeTrace?: BearerIterationTrace, ): Promise { const capturedAuth: Array = []; const sessions = new Set(); @@ -157,8 +158,15 @@ async function withCursorCaptureServer( try { return await run(`http://127.0.0.1:${address.port}`, capturedAuth); } finally { - for (const session of sessions) session.destroy(); - await new Promise(resolve => server.close(() => resolve())); + closeTrace?.begin("capture_close"); + try { + for (const session of sessions) session.destroy(); + await new Promise(resolve => server.close(() => resolve())); + closeTrace?.end("capture_close"); + } catch (error) { + closeTrace?.error("capture_close"); + throw error; + } } } @@ -273,6 +281,376 @@ async function startOwnedServer(): Promise> { return server; } +/** + * Opt-in stage tracing for the multi-iteration admission loops below. One of + * these tests consumed its whole SERVER_BUDGET_MS in CI with no evidence of + * which stage ate it — the per-test timeout reports only that the test died. + * + * OPENCODEX_BEARER_STAGE_TRACE=1 emits one JSON record per stage event on + * stderr: a fixed stage name with begin/end/error, the case index, this + * iteration's elapsed and the whole-test elapsed against the single 30s + * budget the test timeout actually enforces. Records carry enum literals and + * numbers only — no path, header, token, or error text — an elapsed + * correlation alone never proves an external kill. The request stage spans + * the body read, and test_end is emitted from a finally so a failed stage + * or assertion still closes the record after capture_close and + * iteration_end. Tracing adds no timers, retries, or files and never alters + * the call sequence or cleanup: an undefined trace emits nothing. + */ +type BearerTraceStage = "save_config" | "startup_gate" | "request" | "server_stop" | "capture_close"; + +interface BearerIterationTrace { + begin(stage: BearerTraceStage): void; + end(stage: BearerTraceStage): void; + error(stage: BearerTraceStage): void; + finish(): void; +} + +interface BearerTestTrace { + iteration(caseIndex: number, caseCount: number): BearerIterationTrace; + finish(): void; +} + +function bearerStageTracer(label: string): BearerTestTrace | undefined { + if (process.env.OPENCODEX_BEARER_STAGE_TRACE !== "1") return undefined; + const startedAt = Date.now(); + const emit = (record: Record): void => { + console.error(`[bearer-stage-trace] ${JSON.stringify({ label, budgetMs: SERVER_BUDGET_MS, ...record })}`); + }; + emit({ event: "test_begin" }); + return { + finish: () => emit({ event: "test_end", testElapsedMs: Date.now() - startedAt }), + iteration: (caseIndex, caseCount) => { + const iterationStartedAt = Date.now(); + const stageStartedAt = new Map(); + emit({ + event: "iteration_begin", + caseIndex, + caseCount, + testElapsedMs: iterationStartedAt - startedAt, + }); + const mark = (stage: BearerTraceStage, event: "begin" | "end" | "error"): void => { + const now = Date.now(); + const record: Record = { + event: `${stage}_${event}`, + caseIndex, + iterationElapsedMs: now - iterationStartedAt, + testElapsedMs: now - startedAt, + }; + if (event === "begin") { + stageStartedAt.set(stage, now); + } else { + const began = stageStartedAt.get(stage); + if (began !== undefined) record.stageElapsedMs = now - began; + } + emit(record); + }; + return { + begin: stage => mark(stage, "begin"), + end: stage => mark(stage, "end"), + error: stage => mark(stage, "error"), + finish: () => emit({ + event: "iteration_end", + caseIndex, + iterationElapsedMs: Date.now() - iterationStartedAt, + testElapsedMs: Date.now() - startedAt, + }), + }; + }, + }; +} + +/** Run `fn` inside a traced stage; an undefined trace changes nothing. */ +async function tracedBearerStage( + iteration: BearerIterationTrace | undefined, + stage: BearerTraceStage, + fn: () => T | Promise, +): Promise { + if (!iteration) return fn(); + iteration.begin(stage); + try { + const value = await fn(); + iteration.end(stage); + return value; + } catch (error) { + iteration.error(stage); + throw error; + } +} + +/** + * Sync counterpart for stages whose body is already synchronous. Routing a + * sync call through an async wrapper would add a microtask on every + * iteration even with tracing off, so this keeps the original direct call. + */ +function tracedBearerStageSync( + iteration: BearerIterationTrace | undefined, + stage: BearerTraceStage, + fn: () => T, +): T { + if (!iteration) return fn(); + iteration.begin(stage); + try { + const value = fn(); + iteration.end(stage); + return value; + } catch (error) { + iteration.error(stage); + throw error; + } +} + +/** + * Opt-in trace capture for the seam tests: enables the env switch, records + * parsed stderr records, and always restores both even when a check fails. + */ +async function withTraceLines( + run: (records: () => Array>) => Promise, +): Promise { + const previous = process.env.OPENCODEX_BEARER_STAGE_TRACE; + process.env.OPENCODEX_BEARER_STAGE_TRACE = "1"; + const lines: string[] = []; + const originalError = console.error; + console.error = (line: unknown) => { lines.push(String(line)); }; + try { + await run(() => lines + .filter(line => line.startsWith("[bearer-stage-trace] ")) + .map(line => JSON.parse(line.slice("[bearer-stage-trace] ".length)) as Record)); + } finally { + console.error = originalError; + if (previous === undefined) delete process.env.OPENCODEX_BEARER_STAGE_TRACE; + else process.env.OPENCODEX_BEARER_STAGE_TRACE = previous; + } +} + +type BearerAdmissionPost = (url: string | URL, model: string, headers: HeadersInit) => Promise; + +/** + * The request stage used by the matrix below — and exercised directly by the + * seam tests through the injected `post`. The body read belongs to the + * stage: a stalled or rejected body is still request work, not unmeasured + * time after request_end. Chat keeps the original order — the status + * refusal precedes the drain, so an unexpected 200 fails fast instead of + * hanging on a stalled body. + */ +async function runBearerRequestStage( + iteration: BearerIterationTrace | undefined, + surface: "Responses" | "Chat", + url: string | URL, + headers: HeadersInit, + post: BearerAdmissionPost, +): Promise<{ body: unknown }> { + return tracedBearerStage(iteration, "request", async () => { + const res = await post(url, "cursorcustom/auto", headers); + if (surface === "Responses") return { body: await res.json() }; + expect(res.status).not.toBe(200); + return { body: await res.text() }; + }); +} + +interface AdmissionCaseDeps { + readonly post?: BearerAdmissionPost; +} + +/** One matrix iteration: real config, real servers, real stage tracing. */ +async function runTracedAdmissionCase( + iteration: BearerIterationTrace | undefined, + surface: "Responses" | "Chat", + extra: Record, + deps: AdmissionCaseDeps = {}, +): Promise { + await withCursorCaptureServer(async (baseUrl, capturedAuth) => { + tracedBearerStageSync(iteration, "save_config", () => saveConfig(cursorForwardConfig(baseUrl))); + writeFileSync(join(codexHome, "auth.json"), JSON.stringify({ tokens: {} })); + const server = await tracedBearerStage(iteration, "startup_gate", () => startOwnedServer()); + try { + const headers = { "x-opencodex-api-key": ADMISSION_SECRET, ...extra }; + const post = deps.post ?? (surface === "Chat" ? postChatCompletions : postResponses); + const { body } = await runBearerRequestStage(iteration, surface, server.url, headers, post); + if (surface === "Responses") { + expect(body).toMatchObject({ status: "failed" }); + } + expect(capturedAuth).toEqual([]); + } finally { + await tracedBearerStage(iteration, "server_stop", () => server.stop(true)); + } + }, undefined, iteration); +} + +/** + * The matrix loop itself, shared by the parameterized test and the seam + * tests so failure-order regressions are checked on the real control flow. + */ +async function runTracedAdmissionMatrix( + surface: "Responses" | "Chat", + cases: ReadonlyArray>, + deps: AdmissionCaseDeps = {}, +): Promise { + const trace = bearerStageTracer(`ChatGPT-claimed bearer (${surface})`); + try { + for (const [caseIndex, extra] of cases.entries()) { + const iteration = trace?.iteration(caseIndex, cases.length); + try { + await runTracedAdmissionCase(iteration, surface, extra, deps); + } finally { + iteration?.finish(); + } + } + } finally { + // test_end closes every run — including a failed stage or assertion, + // after that iteration's capture_close and iteration_end. + trace?.finish(); + } +} + +describe("bearer stage tracer seam", () => { + test("a disabled trace runs a sync stage as a direct call — no microtask, same order", () => { + // The whole point of the sync wrapper: a sync body must come back as a + // value, not a resolved Promise, when tracing is off. + const order: string[] = []; + const value = tracedBearerStageSync(undefined, "save_config", () => { + order.push("save_config"); + return "saved"; + }); + order.push("auth_write"); + + expect(value).toBe("saved"); + expect(order).toEqual(["save_config", "auth_write"]); + }); + + test("an enabled trace emits the fixed event sequence without altering call order", async () => { + await withTraceLines(async records => { + const trace = bearerStageTracer("seam-check"); + const iteration = trace?.iteration(0, 1); + const order: string[] = []; + + tracedBearerStageSync(iteration, "save_config", () => { order.push("save_config"); }); + await tracedBearerStage(iteration, "request", async () => { order.push("request"); }); + iteration?.finish(); + trace?.finish(); + + expect(order).toEqual(["save_config", "request"]); + const events = records(); + expect(events.map(record => record.event)).toEqual([ + "test_begin", + "iteration_begin", + "save_config_begin", + "save_config_end", + "request_begin", + "request_end", + "iteration_end", + "test_end", + ]); + for (const record of events) { + expect(record.label).toBe("seam-check"); + expect(Number.isFinite(record.budgetMs)).toBe(true); + expect(Number.isFinite(record.testElapsedMs ?? 0)).toBe(true); + } + }); + }); + + test("the shared request stage holds request_end until the body settles", async () => { + await withTraceLines(async records => { + const trace = bearerStageTracer("body-attribution"); + const iteration = trace!.iteration(0, 1); + const events = (name: string) => records().filter(record => record.event === name); + + // Headers resolved but the body is still in flight: the stage must not + // end until the read releases. The gate is released in finally so a + // failed count check cannot leave the stage promise hanging. + let release!: () => void, entered!: () => void; + const bodyEntered = new Promise(resolve => { entered = resolve; }); + const gate = new Promise(resolve => { release = () => resolve("payload"); }); + const post: BearerAdmissionPost = () => Promise.resolve({ + status: 401, + text: () => { entered(); return gate; }, + json: () => Promise.resolve({}), + } as unknown as Response); + const pending = runBearerRequestStage(iteration, "Chat", "http://fixture.invalid", {}, post); + try { + await Promise.race([bodyEntered, pending.then(() => { throw new Error("request settled before body entry"); })]); + expect(events("request_end")).toHaveLength(0); + } finally { + release(); + await pending.catch(() => {}); + } + expect(events("request_end")).toHaveLength(1); + + const rejectedPost: BearerAdmissionPost = () => Promise.resolve({ + status: 401, + text: () => Promise.reject(new Error("synthetic-stall")), + json: () => Promise.reject(new Error("synthetic-stall")), + } as unknown as Response); + const parseError = await runBearerRequestStage(iteration, "Responses", "http://fixture.invalid", {}, rejectedPost) + .then(() => null, error => error); + expect(parseError).toBeInstanceOf(Error); + expect(parseError.message).toBe("synthetic-stall"); + expect(events("request_error")).toHaveLength(1); + + iteration.finish(); + trace!.finish(); + }); + }); + + test("an unexpected Chat 200 fails inside the shared request stage before the body drain", async () => { + await withTraceLines(async records => { + const trace = bearerStageTracer("chat-200-fast-fail"); + const iteration = trace!.iteration(0, 1); + const events = (name: string) => records().filter(record => record.event === name); + + // A 200 with a body that never settles must fail on the status check + // immediately — the assertion precedes the drain inside the stage. + let bodyRead = false; + const post: BearerAdmissionPost = () => Promise.resolve({ + status: 200, + text: () => { bodyRead = true; return new Promise(() => {}); }, + json: () => { bodyRead = true; return new Promise(() => {}); }, + } as unknown as Response); + const statusError = await runBearerRequestStage(iteration, "Chat", "http://fixture.invalid", {}, post) + .then(() => null, error => error); + expect(statusError).toBeInstanceOf(Error); + expect(bodyRead).toBe(false); + expect(events("request_error")).toHaveLength(1); + expect(events("request_end")).toHaveLength(0); + + iteration.finish(); + trace!.finish(); + }); + }); + + test("an injected stage failure still ends the real matrix with test_end last", async () => { + await withTraceLines(async records => { + // Real saveConfig, real servers, real capture — only the wire response + // is injected, with a Chat 200 the shared request stage must refuse. + const post: BearerAdmissionPost = () => Promise.resolve({ + status: 200, + text: () => Promise.resolve("{}"), + json: () => Promise.resolve({}), + } as unknown as Response); + + const failure = await runTracedAdmissionMatrix("Chat", [{}], { post }).then(() => null, error => error); + expect(failure).toBeInstanceOf(Error); + + expect(records().map(record => record.event)).toEqual([ + "test_begin", + "iteration_begin", + "save_config_begin", + "save_config_end", + "startup_gate_begin", + "startup_gate_end", + "request_begin", + "request_error", + "server_stop_begin", + "server_stop_end", + "capture_close_begin", + "capture_close_end", + "iteration_end", + "test_end", + ]); + }); + }); +}); + describe("#2132 bearer admission does not require a ChatGPT credential for routed providers", () => { test("a key-authenticated route is served with no stored main credential", async () => { saveConfig(mixedConfig()); @@ -746,28 +1124,7 @@ describe("bearer admission is not reused as a Cursor upstream credential", () => { authorization: `Bearer ${fakeChatGptJwt({ "https://api.openai.com/auth": {} })}` }, { authorization: `Bearer ${fakeChatGptJwt({ "https://api.openai.com/auth": { user_id: "u_1" } })}` }, ]; - for (const extra of cases) { - await withCursorCaptureServer(async (baseUrl, capturedAuth) => { - saveConfig(cursorForwardConfig(baseUrl)); - writeFileSync(join(codexHome, "auth.json"), JSON.stringify({ tokens: {} })); - const server = await startOwnedServer(); - try { - const headers = { "x-opencodex-api-key": ADMISSION_SECRET, ...extra }; - const response = surface === "Chat" - ? await postChatCompletions(server.url, "cursorcustom/auto", headers) - : await postResponses(server.url, "cursorcustom/auto", headers); - if (surface === "Responses") { - expect(await response.json()).toMatchObject({ status: "failed" }); - } else { - expect(response.status).not.toBe(200); - await response.text(); - } - expect(capturedAuth).toEqual([]); - } finally { - await server.stop(true); - } - }); - } + await runTracedAdmissionMatrix(surface, cases); }); test.each(["Responses", "Chat"])("%s keeps an unmarked JWT as the Cursor credential", async surface => { diff --git a/tests/fixtures/test-layout-expected.json b/tests/fixtures/test-layout-expected.json index d1f3dad7a49..8af2a85f198 100644 --- a/tests/fixtures/test-layout-expected.json +++ b/tests/fixtures/test-layout-expected.json @@ -1341,7 +1341,7 @@ "windows-popup-fix.test.ts": "windows", "windows-process-priority.test.ts": "windows", "windows-scheduler-install-verification.test.ts": "windows", - "windows-secret-acl.test.ts": "windows", "windows-secret-acl-removal-flight.test.ts": "windows", + "windows-secret-acl-diagnostics.test.ts": "windows", "windows-secret-acl.test.ts": "windows", "windows-secret-acl-removal-flight.test.ts": "windows", "windows-config-read-acl-memo.test.ts": "windows", "windows-service-mutation-lock.test.ts": "windows", "windows-service-wrappers.test.ts": "windows", diff --git a/tests/windows/windows-secret-acl-diagnostics.test.ts b/tests/windows/windows-secret-acl-diagnostics.test.ts new file mode 100644 index 00000000000..c671ed9b1f7 --- /dev/null +++ b/tests/windows/windows-secret-acl-diagnostics.test.ts @@ -0,0 +1,396 @@ +/** + * Failure-diagnostics contract for src/lib/windows-secret-acl.ts. + * + * A required ACL failure used to surface only a sanitized message and a code — + * "icacls exited 5" with no record of WHICH step or stage failed, what budget + * it was given, or how long it ran. The error now also carries `aclFailure`: + * a bounded record of fixed enum literals and finite numbers, copied + * field-by-field through the same allowlist boundary as the error code. + * + * What these tests defend is the boundary, not just the fields: nothing a + * child, a spawn error, or an injected runner produces — path, argv, SID, + * stdout, stderr, message or stack — may reach `aclFailure`, the sanitized + * message, or the diagnostics string. + */ +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { existsSync, mkdtempSync, renameSync, rmSync, truncateSync, unlinkSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { + flushWindowsSecretAclReapsBeforeRemoval, + hardenSecretPath, + hardenSecretPathAsync, + resetHardenedStateForTests, + setAsyncIcaclsBeltSchedulerForTests, + setAsyncIcaclsRunnerForTests, + setIcaclsRunnerForTests, + setNowForTests, + setPlatformForTests, + windowsSecretAclReapPendingForPath, + type AclFailureDiagnostics, + type IcaclsResult, +} from "../../src/lib/windows-secret-acl"; +import { atomicWriteFile } from "../../src/config"; +import { setTrustedWindowsElevationExecutablesForTests } from "../../src/lib/windows-elevation"; +import { + resetWindowsPrincipalForTests, + setAsyncWindowsPrincipalRunnerForTests, + setWindowsPrincipalRunnerForTests, +} from "../../src/lib/windows-user-principal"; + +type ThrownAclError = NodeJS.ErrnoException & { aclFailure?: AclFailureDiagnostics }; + +const ownerSid = "S-1-5-21-111-222-333-1001"; +const ownerName = "EXAMPLE\\Owner"; + +const ok: IcaclsResult = { success: true, exitCode: 0, timedOut: false, stdout: "" }; +const denied = (exitCode = 5): IcaclsResult => ({ success: false, exitCode, timedOut: false, stdout: "" }); +const timedOut: IcaclsResult = { success: false, exitCode: null, timedOut: true, stdout: "" }; +const principalResult = { success: true, exitCode: 0, timedOut: false, stdout: `${ownerSid}\r\n${ownerName}\r\n` }; + +let testDir = ""; +let previousAclTimeout: string | undefined; +let previousAclVerifyExisting: string | undefined; + +function secretFile(name = "secret.json"): string { + const filePath = join(testDir, name); + writeFileSync(filePath, "data", "utf-8"); + return filePath; +} + +function thrownAclError(fn: () => unknown): ThrownAclError { + try { + fn(); + } catch (error) { + return error as ThrownAclError; + } + throw new Error("expected the ACL harden to throw"); +} + +beforeEach(() => { + previousAclTimeout = process.env.OPENCODEX_ACL_TIMEOUT_MS; + delete process.env.OPENCODEX_ACL_TIMEOUT_MS; + previousAclVerifyExisting = process.env.OPENCODEX_ACL_VERIFY_EXISTING; + delete process.env.OPENCODEX_ACL_VERIFY_EXISTING; + testDir = mkdtempSync(join(tmpdir(), "ocx-acl-diag-")); + setPlatformForTests("win32"); + resetHardenedStateForTests(); + resetWindowsPrincipalForTests(); + setWindowsPrincipalRunnerForTests(() => ({ ...principalResult })); + setAsyncWindowsPrincipalRunnerForTests(async () => ({ ...principalResult })); +}); + +afterEach(() => { + if (previousAclTimeout === undefined) delete process.env.OPENCODEX_ACL_TIMEOUT_MS; + else process.env.OPENCODEX_ACL_TIMEOUT_MS = previousAclTimeout; + if (previousAclVerifyExisting === undefined) delete process.env.OPENCODEX_ACL_VERIFY_EXISTING; + else process.env.OPENCODEX_ACL_VERIFY_EXISTING = previousAclVerifyExisting; + setIcaclsRunnerForTests(null); + setAsyncIcaclsRunnerForTests(null); + setAsyncIcaclsBeltSchedulerForTests(null); + setNowForTests(null); + setPlatformForTests(null); + setTrustedWindowsElevationExecutablesForTests(null); + setWindowsPrincipalRunnerForTests(null); + setAsyncWindowsPrincipalRunnerForTests(null); + resetWindowsPrincipalForTests(); + resetHardenedStateForTests(); + if (testDir && existsSync(testDir)) rmSync(testDir, { recursive: true, force: true }); + testDir = ""; +}); + +describe("aclFailure diagnostics on required harden errors", () => { + test("a mid-sequence child failure reports step, stage, exit code, budget and elapsed", () => { + const filePath = secretFile(); + setIcaclsRunnerForTests(args => (args.includes("/inheritance:r") ? denied(5) : ok)); + + const error = thrownAclError(() => hardenSecretPath(filePath, { required: true })); + + expect(error.code).toBe("EICACLS"); + expect(error.aclFailure).toMatchObject({ + step: "/inheritance:r", + stage: "child_result", + success: false, + exitCode: 5, + timedOut: false, + }); + expect(error.aclFailure!.elapsedMs).toBeGreaterThanOrEqual(0); + expect(Number.isFinite(error.aclFailure!.elapsedMs)).toBe(true); + expect(error.aclFailure!.budgetMs).toBeGreaterThan(0); + expect(error.aclFailure!.spawnErrorCode).toBeUndefined(); + }); + + test("the icacls step order is preserved and a /remove:g timeout is attributed to /remove:g", () => { + const filePath = secretFile(); + const calls: string[][] = []; + setIcaclsRunnerForTests(args => { + calls.push(args); + return args.includes("/remove:g") ? timedOut : ok; + }); + + const error = thrownAclError(() => hardenSecretPath(filePath, { required: true })); + + expect(error.code).toBe("ETIMEDOUT"); + // The post-timeout /findsid state probe appends more calls after the throw; + // the mutation prefix is the order contract. + expect(calls.slice(0, 3).map(args => args[1])).toEqual(["/grant:r", "/inheritance:r", "/remove:g"]); + expect(error.aclFailure).toMatchObject({ + step: "/remove:g", + stage: "child_result", + timedOut: true, + exitCode: null, + success: false, + }); + }); + + test("a /remove:g denial with surviving SIDs is attributed to /remove:g, not /findsid", () => { + const filePath = secretFile(); + setIcaclsRunnerForTests(args => { + if (args.includes("/remove:g")) return denied(3); + if (args.includes("/findsid")) return { ...ok, stdout: `SID Found: ${filePath}\n` }; + return ok; + }); + + const error = thrownAclError(() => hardenSecretPath(filePath, { required: true })); + + expect(error.code).toBe("EICACLS"); + expect(error.aclFailure).toMatchObject({ step: "/remove:g", stage: "child_result", exitCode: 3 }); + }); + + test("a deadline cutoff before the next spawn is reported as deadline_before_spawn", () => { + const filePath = secretFile(); + let now = 0; + setNowForTests(() => now); + setIcaclsRunnerForTests(args => { + if (args.includes("/grant:r")) now += 2_000; // the grant eats the whole 1s budget + return ok; + }); + + const error = thrownAclError(() => hardenSecretPath(filePath, { required: true, deadlineMs: 1_000 })); + + expect(error.code).toBe("ETIMEDOUT"); + // The /inheritance:r step never reached the runner: the shared deadline was + // already gone, which used to read identically to a real child timeout. + expect(error.aclFailure).toMatchObject({ + step: "/inheritance:r", + stage: "deadline_before_spawn", + timedOut: true, + exitCode: null, + success: false, + }); + expect(error.aclFailure!.budgetMs).toBeLessThanOrEqual(0); + expect(error.aclFailure!.elapsedMs).toBeGreaterThan(0); + }); + + test("a spawn that cannot start is reported as spawn_failed with an allowlisted code", () => { + const filePath = secretFile(); + // No icacls runner injection: the REAL default runner tries to spawn the + // trusted executable, which this seam points at a path that cannot exist. + setTrustedWindowsElevationExecutablesForTests({ icacls: join(testDir, "missing-icacls.exe") }); + + const error = thrownAclError(() => hardenSecretPath(filePath, { required: true })); + + expect(error.code).toBe("EICACLS"); + expect(error.aclFailure).toMatchObject({ + step: "/grant:r", + stage: "spawn_failed", + success: false, + exitCode: null, + timedOut: false, + }); + // The only acceptable payload is an allowlisted symbolic code; ENOENT is + // what a missing executable reports on every supported platform. + expect( + error.aclFailure!.spawnErrorCode === undefined || error.aclFailure!.spawnErrorCode === "ENOENT", + ).toBe(true); + }); + + test("a non-allowlisted spawn code is dropped, and no child output reaches the record", () => { + const filePath = secretFile(); + const hostile = testDir; // a raw filesystem path masquerading as a spawn code + setIcaclsRunnerForTests(() => ({ + success: false, + exitCode: null, + timedOut: false, + stdout: "marker-stdout", + failureStage: "spawn_failed", + spawnErrorCode: hostile, + })); + + const error = thrownAclError(() => hardenSecretPath(filePath, { required: true })); + + expect(error.aclFailure!.stage).toBe("spawn_failed"); + expect(error.aclFailure!.spawnErrorCode).toBeUndefined(); + const serialized = JSON.stringify(error.aclFailure); + expect(serialized).not.toContain(hostile); + expect(serialized).not.toContain("marker-stdout"); + expect(serialized).not.toContain(filePath); + expect(serialized).not.toContain(ownerSid); + expect(error.message).not.toContain(filePath); + }); + + test("an allowlisted spawn code survives verbatim — ENOENT proves the contract, not just its absence", () => { + const filePath = secretFile(); + setIcaclsRunnerForTests(() => ({ + success: false, + exitCode: null, + timedOut: false, + stdout: "", + failureStage: "spawn_failed", + spawnErrorCode: "ENOENT", + })); + + const error = thrownAclError(() => hardenSecretPath(filePath, { required: true })); + + expect(error.code).toBe("EICACLS"); + expect(error.aclFailure).toMatchObject({ + step: "/grant:r", + stage: "spawn_failed", + timedOut: false, + exitCode: null, + }); + expect(error.aclFailure!.spawnErrorCode).toBe("ENOENT"); + }); + + test("mistyped runner fields are coerced, never propagated", () => { + const filePath = secretFile(); + setIcaclsRunnerForTests(() => ({ + success: false, + exitCode: "5" as unknown as number, + timedOut: "yes" as unknown as boolean, + stdout: "", + })); + + const error = thrownAclError(() => hardenSecretPath(filePath, { required: true })); + + // The original truthy contract is kept: a truthy timedOut — even a + // mistyped one — is a timeout, and the code and the record are coerced + // from the same Boolean() so they can never disagree. A non-numeric + // exitCode still collapses to null rather than propagating. + expect(error.code).toBe("ETIMEDOUT"); + expect(error.aclFailure).toMatchObject({ + stage: "child_result", + success: false, + exitCode: null, + timedOut: true, + }); + }); + + test("the async runner path reports the same bounded record", async () => { + const filePath = secretFile(); + setAsyncIcaclsRunnerForTests(async args => (args.includes("/grant:r") ? denied(9) : ok)); + + const error = await hardenSecretPathAsync(filePath, { required: true }).then( + () => { throw new Error("expected the async harden to reject"); }, + cause => cause as ThrownAclError, + ); + + expect(error.code).toBe("EICACLS"); + expect(error.aclFailure).toMatchObject({ + step: "/grant:r", + stage: "child_result", + success: false, + exitCode: 9, + timedOut: false, + }); + }); + + test("an async runner that rejects is runner_rejected, not spawn evidence", async () => { + const filePath = secretFile(); + setAsyncIcaclsRunnerForTests(async () => { + throw new Error(`marker-rejection ${filePath}`); + }); + + const error = await hardenSecretPathAsync(filePath, { required: true }).then( + () => { throw new Error("expected the async harden to reject"); }, + cause => cause as ThrownAclError, + ); + + // Nothing proves a child ever existed, so the stage must not claim a + // spawn failure — and the raw rejection carries no path to the boundary. + expect(error.code).toBe("EICACLS"); + expect(error.aclFailure).toMatchObject({ + step: "/grant:r", + stage: "runner_rejected", + success: false, + exitCode: null, + timedOut: false, + }); + expect(error.aclFailure!.spawnErrorCode).toBeUndefined(); + expect(JSON.stringify(error.aclFailure)).not.toContain("marker-rejection"); + expect(error.message).not.toContain(filePath); + }); + + test("a caller-side belt that fires first is runner_watchdog and the late reap is tracked", async () => { + const filePath = secretFile(); + const releases: Array<() => void> = []; + setAsyncIcaclsRunnerForTests(() => new Promise(resolve => { + releases.push(() => resolve(ok)); + })); + // Fire the caller-facing belt immediately through the existing scheduler + // seam — no timer, no sleep, and the runner stays in flight. + setAsyncIcaclsBeltSchedulerForTests(callback => { + callback(); + return () => {}; + }); + + try { + const error = await hardenSecretPathAsync(filePath, { required: true }).then( + () => { throw new Error("expected the async harden to reject"); }, + cause => cause as ThrownAclError, + ); + + // The belt released the caller while the runner was still unsettled; that + // is a watchdog verdict, never a fabricated child_result timeout. + expect(error.code).toBe("ETIMEDOUT"); + expect(error.aclFailure).toMatchObject({ + step: "/grant:r", + stage: "runner_watchdog", + timedOut: true, + exitCode: null, + success: false, + }); + // The unsettled runner is registered as a pending reap for this path — + // removal must wait for it, not treat the caller release as proof of exit. + expect(windowsSecretAclReapPendingForPath(filePath)).toBe(true); + + } finally { + for (const release of releases) release(); + await flushWindowsSecretAclReapsBeforeRemoval(testDir); + } + expect(windowsSecretAclReapPendingForPath(filePath)).toBe(false); + }); + + test("an ACL failure still blocks publication — now with the bounded record attached", () => { + setIcaclsRunnerForTests(args => (args.includes("/grant:r") ? denied(5) : ok)); + const destination = join(testDir, "atomic-secret.json"); + let renamed = false; + let scrubbed = false; + + const error = thrownAclError(() => atomicWriteFile(destination, "secret", { + write: (path, content) => writeFileSync(path, content, { mode: 0o600 }), + harden: path => { hardenSecretPath(path, { required: true }); }, + rename: (source, target) => { renamed = true; renameSync(source, target); }, + truncate: path => { scrubbed = true; truncateSync(path, 0); }, + unlink: unlinkSync, + })); + + expect(error.code).toBe("EICACLS"); + expect(error.aclFailure!.step).toBe("/grant:r"); + expect(renamed).toBe(false); // the rename — the publish — never ran + expect(scrubbed).toBe(true); + expect(existsSync(destination)).toBe(false); + }); + + test("optional mode keeps its path-free diagnostic string and gains nothing new", () => { + const filePath = secretFile(); + setIcaclsRunnerForTests(() => denied(5)); + + const result = hardenSecretPath(filePath, { required: false }); + + expect(result.ok).toBe(false); + expect(result.diagnostics).toContain("EICACLS"); + expect(result.diagnostics).not.toContain(filePath); + expect(result.diagnostics).not.toContain(ownerSid); + }); +});