From 0ecf6ad0cb6bdcd599cd60e215556ea1d8933a33 Mon Sep 17 00:00:00 2001 From: Wibias <37517432+Wibias@users.noreply.github.com> Date: Wed, 29 Jul 2026 09:37:06 +0200 Subject: [PATCH] fix(windows): async ACL harden for response-state writes (#645) Fixes #612. Async ACL runner for response-state writes; destination-path timeout memo; owner-first grant. Conflict-resolved onto current dig. --- src/config.ts | 86 ++++++++++++++- src/lib/windows-secret-acl.ts | 177 ++++++++++++++++++++++++++++++- src/responses/state.ts | 30 ++++-- src/server/lifecycle.ts | 2 +- tests/responses-state.test.ts | 24 ++--- tests/windows-secret-acl.test.ts | 85 +++++++++++++++ 6 files changed, 378 insertions(+), 26 deletions(-) diff --git a/src/config.ts b/src/config.ts index a30ee879b..b7364df49 100644 --- a/src/config.ts +++ b/src/config.ts @@ -11,7 +11,7 @@ import { MAIN_CODEX_ACCOUNT_NAMESPACE_TARGET, } from "./codex/account-namespace-match"; import { COMBO_NAMESPACE, comboConfigIssues } from "./combos/types"; -import { hardenSecretDir, hardenSecretPath } from "./lib/windows-secret-acl"; +import { hardenSecretDir, hardenSecretPath, hardenSecretPathAsync } from "./lib/windows-secret-acl"; import { providerDestinationConfigError } from "./lib/destination-policy"; import { openRouterRoutingConfigError } from "./providers/openrouter-routing"; import { @@ -135,6 +135,90 @@ export function atomicWriteFile(path: string, content: string, io: AtomicWriteIO } } +/** Async atomic-write I/O: harden may await icacls without blocking the event loop (#612). */ +export interface AtomicWriteAsyncIO { + write: (path: string, content: string) => void | Promise; + harden: (path: string) => void | Promise; + rename: (source: string, destination: string) => void | Promise; + truncate: (path: string) => void | Promise; + unlink: (path: string) => void | Promise; +} + +async function renameAtomicFileAsync(source: string, destination: string): Promise { + for (let attempt = 0; ; attempt += 1) { + try { + renameSync(source, destination); + return; + } catch (error) { + const code = (error as NodeJS.ErrnoException).code; + const transientWindowsError = process.platform === "win32" + && (code === "EBUSY" || code === "EPERM" || code === "EACCES"); + if (!transientWindowsError || attempt >= 2) throw error; + await Bun.sleep(25 * (attempt + 1)); + } + } +} + +/** + * Async atomic write (#612): same temp+harden+rename and residual-temp policy as + * atomicWriteFile, but Windows ACL harden yields the event loop. Timeout memo is keyed + * by the final destination path (not the unique temp, not the parent directory). + */ +export async function atomicWriteFileAsync( + path: string, + content: string, + io?: AtomicWriteAsyncIO, +): Promise { + const effective: AtomicWriteAsyncIO = io ?? { + write: (target, value) => writeFileSync(target, value, { encoding: "utf-8", mode: 0o600 }), + harden: async target => { + try { chmodSync(target, 0o600); } catch { /* platform may ignore chmod */ } + if (process.platform === "win32") { + await hardenSecretPathAsync(target, { required: true, timeoutMemoKey: path }); + } + }, + rename: renameAtomicFileAsync, + truncate: target => truncateSync(target, 0), + unlink: unlinkSync, + }; + const tmp = `${path}.ocx.${process.pid}.${++_atomicSeq}.tmp`; + let hardened = false; + try { + await effective.write(tmp, content); + await effective.harden(tmp); + hardened = true; + await effective.rename(tmp, path); + } catch (cause) { + let scrubbed = false; + try { + await effective.truncate(tmp); + scrubbed = true; + } catch (error) { + if (isMissingPathError(error)) scrubbed = true; + else { + try { await effective.write(tmp, ""); scrubbed = true; } catch { /* removal may still succeed */ } + } + } + let removed = false; + try { + await effective.unlink(tmp); + removed = true; + } catch (error) { + if (isMissingPathError(error)) removed = true; + else { + try { await effective.unlink(tmp); removed = true; } + catch (retryError) { if (isMissingPathError(retryError)) removed = true; } + } + } + if (!removed && !scrubbed) throw new AtomicWriteSecretResidualError(tmp, { cause }); + if (!removed && !hardened) { + try { await effective.harden(tmp); hardened = true; } catch { /* zero-byte residual is reported honestly */ } + } + if (!removed) throw new AtomicWriteResidualTempError(tmp, hardened, { cause }); + throw cause; + } +} + export class OpenAiTierBackupCleanupError extends Error { constructor() { super("OpenAI tier backup temporary cleanup failed"); this.name = "OpenAiTierBackupCleanupError"; } } diff --git a/src/lib/windows-secret-acl.ts b/src/lib/windows-secret-acl.ts index 7cdc48a85..e5ce2eee9 100644 --- a/src/lib/windows-secret-acl.ts +++ b/src/lib/windows-secret-acl.ts @@ -24,6 +24,10 @@ * icacls cannot block OAuth logins or token refresh (field report: Kimi auth * stuck behind ETIMEDOUT). Real EPERM/EACCES/exit-code failures still throw: * availability never silently overrides confidentiality for those. + * hardenSecretPathAsync / hardenSecretDirAsync — same policy, async icacls + * runner so the event loop is not held for the child lifetime (#612). + * HardenOptions.timeoutMemoKey — optional destination-path key for the + * timeout memo (atomic writers mint unique temps; never a parent directory). * hardenSecretDir — same contract for directories. */ @@ -42,6 +46,13 @@ export interface HardenResult { export interface HardenOptions { required: boolean; + /** + * Optional timeout-memo key distinct from `targetPath` (issue #612). + * Atomic writers mint a fresh `.tmp` path per write; keying the timeout cache by the + * final destination path prevents re-stalling the event loop on every subsequent temp. + * Must NOT be a parent directory — directory ACLs are not authoritative for new files. + */ + timeoutMemoKey?: string; } /** @@ -72,6 +83,7 @@ export interface IcaclsResult { } type IcaclsRunner = (args: string[], timeoutMs: number) => IcaclsResult; +type AsyncIcaclsRunner = (args: string[], timeoutMs: number) => Promise; function defaultIcaclsRunner(args: string[], timeoutMs: number): IcaclsResult { // Bun.spawnSync with windowsHide: Node execFileSync has hung under the GUI/proxy even @@ -91,7 +103,43 @@ function defaultIcaclsRunner(args: string[], timeoutMs: number): IcaclsResult { }; } +/** + * Async icacls runner (#612): yields the event loop while waiting for the child. + * Timeout provenance is recorded by our timer (async Subprocess has no exitedDueToTimeout); + * we still await process exit before classifying so settlement is confirmed. + */ +async function defaultAsyncIcaclsRunner(args: string[], timeoutMs: number): Promise { + const proc = Bun.spawn(["icacls.exe", ...args], { + stdin: "ignore", + stdout: "pipe", + stderr: "ignore", + windowsHide: true, + }); + let timedOutByUs = false; + const timer = setTimeout(() => { + timedOutByUs = true; + try { proc.kill(); } catch { /* already exited */ } + }, Math.max(1, timeoutMs)); + let exitCode: number | null = null; + try { + exitCode = await proc.exited; + } finally { + clearTimeout(timer); + } + const stdout = proc.stdout + ? await new Response(proc.stdout).text().catch(() => "") + : ""; + const timedOut = timedOutByUs; + return { + success: !timedOut && exitCode === 0, + exitCode: timedOut ? null : exitCode, + timedOut, + stdout, + }; +} + let icaclsRunner: IcaclsRunner = defaultIcaclsRunner; +let asyncIcaclsRunner: AsyncIcaclsRunner = defaultAsyncIcaclsRunner; let platformOverride: string | null = null; let nowFn: () => number = Date.now; @@ -100,6 +148,11 @@ export function setIcaclsRunnerForTests(runner: IcaclsRunner | null): void { icaclsRunner = runner ?? defaultIcaclsRunner; } +/** Test seam: replace the async icacls runner. Pass null to restore the default. */ +export function setAsyncIcaclsRunnerForTests(runner: AsyncIcaclsRunner | null): void { + asyncIcaclsRunner = runner ?? defaultAsyncIcaclsRunner; +} + /** Test seam: force the platform gate (e.g. "win32") so CI on POSIX reaches the runner. */ export function setPlatformForTests(value: string | null): void { platformOverride = value; @@ -156,6 +209,10 @@ function currentWindowsUser(): string | undefined { */ const BROAD_SIDS = ["*S-1-1-0", "*S-1-5-11", "*S-1-5-32-545"] as const; +function grantAce(user: string, directory: boolean): string { + return directory ? `${user}:(OI)(CI)(F)` : `${user}:(F)`; +} + function runIcacls(targetPath: string, directory: boolean, deadline: number): void { const user = currentWindowsUser(); if (!user) { @@ -177,8 +234,7 @@ function runIcacls(targetPath: string, directory: boolean, deadline: number): vo // Step 1: grant current user full control BEFORE any destructive ACL change. // If this fails, inheritance is untouched and the writer keeps inherited access. - const grant = directory ? `${user}:(OI)(CI)(F)` : `${user}:(F)`; - runOrThrow("/grant:r", [targetPath, "/grant:r", grant]); + runOrThrow("/grant:r", [targetPath, "/grant:r", grantAce(user, directory)]); // Step 2: disable inheritance and remove inherited ACEs. The explicit owner ACE // from step 1 survives this transition, so a later failure still leaves cleanup access. @@ -205,6 +261,41 @@ function runIcacls(targetPath: string, directory: boolean, deadline: number): vo } } +/** Async counterpart of runIcacls — same step order and timeout/error classification (#612). */ +async function runIcaclsAsync(targetPath: string, directory: boolean, deadline: number): Promise { + const user = currentWindowsUser(); + if (!user) { + throw new Error("Cannot determine current Windows user for ACL hardening"); + } + + const run = async (step: string, args: string[]): Promise => { + const remaining = deadline - nowFn(); + if (remaining <= 0) { + throw icaclsError(step, { success: false, exitCode: null, timedOut: true, stdout: "" }); + } + return asyncIcaclsRunner(args, remaining); + }; + const runOrThrow = async (step: string, args: string[]): Promise => { + const result = await run(step, args); + if (!result.success) throw icaclsError(step, result); + }; + + await runOrThrow("/grant:r", [targetPath, "/grant:r", grantAce(user, directory)]); + await runOrThrow("/inheritance:r", [targetPath, "/inheritance:r"]); + + const removal = await run("/remove:g", [targetPath, "/remove:g", ...BROAD_SIDS]); + if (!removal.success) { + if (removal.timedOut) throw icaclsError("/remove:g", removal); + for (const sid of BROAD_SIDS) { + const found = await run("/findsid", [targetPath, "/findsid", sid]); + if (!found.success) throw icaclsError("/findsid", found); + if (found.stdout.includes(targetPath)) { + throw icaclsError("/remove:g", removal); + } + } + } +} + /** * Sanitize an error from a failed ACL operation into a safe diagnostic string. * The raw path must not appear in the returned string (it may contain @@ -254,6 +345,27 @@ function describeAclStateAfterTimeout(targetPath: string, deadline: number): str } } +async function describeAclStateAfterTimeoutAsync(targetPath: string, deadline: number): Promise { + try { + for (const sid of BROAD_SIDS) { + const remaining = deadline - nowFn(); + if (remaining <= 0) return "ACL state unverified (budget exhausted)"; + const found = await asyncIcaclsRunner([targetPath, "/findsid", sid], remaining); + if (!found.success) return "ACL state unverified (probe failed)"; + if (found.stdout.includes(targetPath)) return "broad ACL grants still present"; + } + return "no broad ACL grants detected (hardening still incomplete)"; + } catch { + return "ACL state unverified (probe failed)"; + } +} + +function timeoutMemoKey(targetPath: string, opts: HardenOptions): string { + // Destination-path memo only (issue #612). Never a parent directory — directory ACLs + // are not authoritative for newly created temps. + return opts.timeoutMemoKey ?? targetPath; +} + /** * Shared harden flow for files and directories: one total budget (env-configurable) * covering the initial attempt, ONE timeout retry, and the diagnostic verification. @@ -269,7 +381,8 @@ function hardenEntry( if (!existsSync(targetPath)) return { ok: true }; if (effectivePlatform() !== "win32") return { ok: true }; if (cache.has(targetPath)) return { ok: true }; - if (timedOutPaths.has(targetPath)) { + const memoKey = timeoutMemoKey(targetPath, opts); + if (timedOutPaths.has(memoKey)) { return { ok: false, diagnostics: "ACL hardening skipped — previous attempt timed out" }; } @@ -289,7 +402,7 @@ function hardenEntry( const diagnostics = sanitizeDiagnostics(lastErr); if (isTimeoutError(lastErr)) { - timedOutPaths.add(targetPath); + timedOutPaths.add(memoKey); const state = describeAclStateAfterTimeout(targetPath, deadline); const annotated = `${diagnostics}; ${state}`; // Timeout-only soft-fail: a hung icacls must not block OAuth/token writes. @@ -301,6 +414,47 @@ function hardenEntry( return { ok: false, diagnostics }; } +/** Async counterpart of hardenEntry — yields while waiting on icacls (#612). */ +async function hardenEntryAsync( + targetPath: string, + directory: boolean, + opts: HardenOptions, + cache: Set, +): Promise { + if (!existsSync(targetPath)) return { ok: true }; + if (effectivePlatform() !== "win32") return { ok: true }; + if (cache.has(targetPath)) return { ok: true }; + const memoKey = timeoutMemoKey(targetPath, opts); + if (timedOutPaths.has(memoKey)) { + return { ok: false, diagnostics: "ACL hardening skipped — previous attempt timed out" }; + } + + const deadline = nowFn() + resolveHardenDeadlineMs(); + let lastErr: unknown; + for (let attempt = 0; attempt < 2; attempt++) { + if (attempt > 0 && deadline - nowFn() <= 0) break; + try { + await runIcaclsAsync(targetPath, directory, deadline); + cache.add(targetPath); + return { ok: true }; + } catch (err) { + lastErr = err; + if (!isTimeoutError(err)) break; + } + } + + const diagnostics = sanitizeDiagnostics(lastErr); + if (isTimeoutError(lastErr)) { + timedOutPaths.add(memoKey); + const state = await describeAclStateAfterTimeoutAsync(targetPath, deadline); + const annotated = `${diagnostics}; ${state}`; + console.warn(`[opencodex] ${annotated} — continuing without NTFS ACL harden`); + return { ok: false, diagnostics: annotated }; + } + if (opts.required) throw new Error(diagnostics); + return { ok: false, diagnostics }; +} + /** * Harden a single file path with per-user NTFS ACLs on Windows. * On non-Windows platforms, returns ok:true immediately (caller owns chmod). @@ -312,6 +466,14 @@ export function hardenSecretPath(targetPath: string, opts: HardenOptions): Harde return hardenEntry(targetPath, false, opts, hardenedPaths); } +/** + * Async harden for write paths that must not block the event loop (#612). + * Same success/timeout/error policy as hardenSecretPath. + */ +export function hardenSecretPathAsync(targetPath: string, opts: HardenOptions): Promise { + return hardenEntryAsync(targetPath, false, opts, hardenedPaths); +} + /** * Harden a directory path with per-user NTFS ACLs on Windows. * On non-Windows platforms, returns ok:true immediately (caller owns chmod). @@ -322,3 +484,10 @@ export function hardenSecretPath(targetPath: string, opts: HardenOptions): Harde export function hardenSecretDir(targetPath: string, opts: HardenOptions): HardenResult { return hardenEntry(targetPath, true, opts, hardenedDirectories); } + +/** + * Async directory harden (#612). Same policy as hardenSecretDir. + */ +export function hardenSecretDirAsync(targetPath: string, opts: HardenOptions): Promise { + return hardenEntryAsync(targetPath, true, opts, hardenedDirectories); +} diff --git a/src/responses/state.ts b/src/responses/state.ts index c9201805b..3345e4626 100644 --- a/src/responses/state.ts +++ b/src/responses/state.ts @@ -1,6 +1,6 @@ import { chmodSync, existsSync, lstatSync, mkdirSync, opendirSync, readFileSync, unlinkSync } from "node:fs"; import { dirname, join } from "node:path"; -import { atomicWriteFile, getConfigDir } from "../config"; +import { atomicWriteFileAsync, getConfigDir } from "../config"; import type { OcxProviderContinuationState } from "../types"; const MAX_STORED_RESPONSES = 1_000; @@ -83,6 +83,8 @@ const replayedInputPrefixLengths = new WeakMap(); let loaded = false; let persistTimer: ReturnType | null = null; let pendingPersistPath: string | null = null; +/** Single-flight gate: overlapping response-state writes serialize (#612). */ +let persistGate: Promise = Promise.resolve(); function now(): number { return Date.now(); @@ -247,12 +249,18 @@ function ensureLoaded(): void { } } -function persistNow(path: string): void { +async function persistNow(path: string): Promise { if (persistTimer) { clearTimeout(persistTimer); persistTimer = null; } pendingPersistPath = null; + + // Serialize writers so concurrent flush + debounce cannot race on temps / ACL (#612). + const previous = persistGate; + let release!: () => void; + persistGate = new Promise(resolve => { release = resolve; }); + await previous; try { const entries: [string, StoredResponseState][] = []; let total = 0; @@ -273,9 +281,11 @@ function persistNow(path: string): void { // mkdirSync's mode only applies on creation — re-harden an existing config dir so the // conversation-content snapshot never lands in a group/world-readable directory. try { chmodSync(dirname(path), 0o700); } catch { /* best-effort (e.g. Windows) */ } - atomicWriteFile(path, JSON.stringify({ version: 2, states: entries })); + await atomicWriteFileAsync(path, JSON.stringify({ version: 2, states: entries })); } catch { /* best-effort: disk trouble must never affect request handling */ + } finally { + release(); } } @@ -285,15 +295,18 @@ function schedulePersist(): void { // debounce fires, and a late write must land in the home that owned the recorded state. pendingPersistPath = snapshotPath(); const path = pendingPersistPath; - persistTimer = setTimeout(() => persistNow(path), SNAPSHOT_DEBOUNCE_MS); + persistTimer = setTimeout(() => { void persistNow(path); }, SNAPSHOT_DEBOUNCE_MS); (persistTimer as { unref?: () => void }).unref?.(); } /** Flush any pending debounced snapshot write (graceful shutdown / deterministic tests). */ -export function flushResponseState(): void { - if (!persistTimer) return; - // Use the path captured when the write was scheduled — OPENCODEX_HOME may have moved since. - persistNow(pendingPersistPath ?? snapshotPath()); +export async function flushResponseState(): Promise { + if (persistTimer) { + await persistNow(pendingPersistPath ?? snapshotPath()); + return; + } + // No pending timer: still await any in-flight write so shutdown does not race (#612). + await persistGate; } function inputItems(input: unknown): unknown[] { @@ -448,6 +461,7 @@ export function clearResponseStateMemoryForTests(): void { clearTimeout(persistTimer); persistTimer = null; } + pendingPersistPath = null; states.clear(); storedResponseBytes = 0; loaded = false; diff --git a/src/server/lifecycle.ts b/src/server/lifecycle.ts index a07b493e4..cc063240d 100644 --- a/src/server/lifecycle.ts +++ b/src/server/lifecycle.ts @@ -86,7 +86,7 @@ export async function drainAndShutdown( } // Debounced replay-state snapshot may still be pending; flush so the last completed turn's // previous_response_id chain survives the restart this shutdown is usually part of. - flushResponseState(); + await flushResponseState(); // Tear down opt-in storage policy timers / worker / live-config sink so they cannot fire after stop. stopStorageCleanupScheduler(); abortStorageCleanupPolicyJob(); diff --git a/tests/responses-state.test.ts b/tests/responses-state.test.ts index 1e3e200ce..705b71121 100644 --- a/tests/responses-state.test.ts +++ b/tests/responses-state.test.ts @@ -522,14 +522,14 @@ describe("Responses previous_response_id state", () => { } }); - test("byte accounting survives restart (sizes recomputed on load)", () => { + test("byte accounting survives restart (sizes recomputed on load)", async () => { setResponseStateByteCapForTests(4_000); try { const bulk = "y".repeat(1_500); const bodyA = { model: "cursor/grok-4.5", input: `${bulk}-a`, store: false }; const jsonA = buildResponseJSON([{ type: "text_delta", text: "ok" }, { type: "done" }], "cursor/grok-4.5"); rememberResponseState(bodyA, jsonA, { cursor: { conversationId: "conv_a" } }, { force: true }); - flushResponseState(); + await flushResponseState(); // Simulated restart: memory wiped, snapshot reloaded lazily. clearResponseStateMemoryForTests(); @@ -625,14 +625,14 @@ describe("Responses previous_response_id state", () => { } }); - test("snapshot survives a simulated restart (memory clear + disk load)", () => { + test("snapshot survives a simulated restart (memory clear + disk load)", async () => { const firstBody = { model: "gpt-5.5", input: "hello" }; const first = buildResponseJSON([ { type: "text_delta", text: "hi" }, { type: "done" }, ], "gpt-5.5"); rememberResponseState(firstBody, first, "cursor_conv_9"); - flushResponseState(); + await flushResponseState(); // Simulate restart: wipe memory, keep the snapshot file. clearResponseStateMemoryForTests(); @@ -747,7 +747,7 @@ describe("Responses previous_response_id state", () => { expect(previousResponseConversationId("resp_v1")).toBe("cursor_v1"); }); - test("persists provider-keyed Cursor and Kiro continuation state across restart", () => { + test("persists provider-keyed Cursor and Kiro continuation state across restart", async () => { const first = buildResponseJSON([ { type: "text_delta", text: "answer", phase: "final_answer" }, { type: "done", endTurn: true }, @@ -760,7 +760,7 @@ describe("Responses previous_response_id state", () => { kiro: { conversationId: "kiro_conv_2" }, }, ); - flushResponseState(); + await flushResponseState(); clearResponseStateMemoryForTests(); expect(previousResponseProviderState(first.id as string)).toEqual({ @@ -771,13 +771,13 @@ describe("Responses previous_response_id state", () => { expect(snapshot.version).toBe(2); }); - test("stale snapshot entries are pruned on load", () => { + test("stale snapshot entries are pruned on load", async () => { const first = buildResponseJSON([ { type: "text_delta", text: "old" }, { type: "done" }, ], "gpt-5.5"); rememberResponseState({ model: "gpt-5.5", input: "old turn" }, first); - flushResponseState(); + await flushResponseState(); clearResponseStateMemoryForTests(); // Rewrite the snapshot with an expired createdAt (2h ago > 1h TTL). @@ -821,7 +821,7 @@ describe("Responses previous_response_id state", () => { expect(expanded.input).toHaveLength(3); }); - test("oversized entries stay in memory but are skipped on disk", () => { + test("oversized entries stay in memory but are skipped on disk", async () => { const big = "x".repeat(3 * 1024 * 1024); // > 2MiB per-entry cap const first = buildResponseJSON([ { type: "text_delta", text: big }, @@ -834,7 +834,7 @@ describe("Responses previous_response_id state", () => { { type: "done" }, ], "gpt-5.5"); rememberResponseState({ model: "gpt-5.5", input: "small turn" }, small); - flushResponseState(); + await flushResponseState(); // In-memory: both expand. expect((expandPreviousResponseInput({ @@ -964,10 +964,10 @@ describe("Responses previous_response_id state", () => { } }); - test("is side-effect free: it never lazy-loads the disk snapshot, prunes, or evicts", () => { + test("is side-effect free: it never lazy-loads the disk snapshot, prunes, or evicts", async () => { const first = buildResponseJSON([{ type: "text_delta", text: "persisted" }, { type: "done" }], "gpt-5.5"); rememberResponseState({ model: "gpt-5.5", input: "persisted turn" }, first); - flushResponseState(); + await flushResponseState(); // Simulated restart: memory wiped, snapshot on disk, `loaded` reset to false. clearResponseStateMemoryForTests(); diff --git a/tests/windows-secret-acl.test.ts b/tests/windows-secret-acl.test.ts index e3a19b1d8..95871b44b 100644 --- a/tests/windows-secret-acl.test.ts +++ b/tests/windows-secret-acl.test.ts @@ -16,7 +16,9 @@ import { join } from "node:path"; import { hardenSecretDir, hardenSecretPath, + hardenSecretPathAsync, resetHardenedStateForTests, + setAsyncIcaclsRunnerForTests, setIcaclsRunnerForTests, setNowForTests, setPlatformForTests, @@ -475,3 +477,86 @@ describe("icacls failure paths (injected seams)", () => { expect(ownerHasExplicitAce).toBe(true); }); }); + +// --------------------------------------------------------------------------- +// Async harden (#612): same policy as sync, but yields via asyncIcaclsRunner. +// --------------------------------------------------------------------------- + +describe("async hardenSecretPath (issue #612)", () => { + const ok: IcaclsResult = { success: true, exitCode: 0, timedOut: false, stdout: "" }; + const timeout: IcaclsResult = { success: false, exitCode: null, timedOut: true, stdout: "" }; + const denied: IcaclsResult = { success: false, exitCode: 5, timedOut: false, stdout: "" }; + let warnings: string[] = []; + const realWarn = console.warn; + + beforeEach(() => { + setPlatformForTests("win32"); + resetHardenedStateForTests(); + process.env.USERNAME ??= "tester"; + warnings = []; + console.warn = (...args: unknown[]) => { warnings.push(args.join(" ")); }; + }); + + afterEach(() => { + setPlatformForTests(null); + setIcaclsRunnerForTests(null); + setAsyncIcaclsRunnerForTests(null); + setNowForTests(null); + resetHardenedStateForTests(); + console.warn = realWarn; + }); + + function secretFile(name = "secret.json"): string { + const filePath = join(testDir, name); + writeFileSync(filePath, "data", "utf-8"); + return filePath; + } + + test("async timeout soft-fails with the same policy as sync", async () => { + setAsyncIcaclsRunnerForTests(async () => timeout); + const result = await hardenSecretPathAsync(secretFile(), { required: true }); + expect(result.ok).toBe(false); + expect(result.diagnostics).toContain("ETIMEDOUT"); + expect(warnings.some(w => w.includes("continuing without NTFS ACL harden"))).toBe(true); + }); + + test("async permission failure still throws on required paths", async () => { + setAsyncIcaclsRunnerForTests(async () => denied); + await expect(hardenSecretPathAsync(secretFile(), { required: true })).rejects.toThrow(/EICACLS/); + }); + + test("timeoutMemoKey shares the timeout cache across distinct temp paths", async () => { + setAsyncIcaclsRunnerForTests(async () => timeout); + const dest = join(testDir, "responses-state.json"); + const tempA = join(testDir, "responses-state.json.ocx.1.1.tmp"); + const tempB = join(testDir, "responses-state.json.ocx.1.2.tmp"); + writeFileSync(tempA, "a", "utf-8"); + writeFileSync(tempB, "b", "utf-8"); + + const first = await hardenSecretPathAsync(tempA, { required: true, timeoutMemoKey: dest }); + expect(first.ok).toBe(false); + expect(first.diagnostics).toContain("ETIMEDOUT"); + + let calls = 0; + setAsyncIcaclsRunnerForTests(async () => { + calls += 1; + return timeout; + }); + const second = await hardenSecretPathAsync(tempB, { required: true, timeoutMemoKey: dest }); + expect(second.ok).toBe(false); + expect(second.diagnostics).toContain("skipped"); + expect(calls).toBe(0); // destination-keyed memo; not a parent-directory shortcut + }); + + test("async harden still grants owner before inheritance removal", async () => { + const steps: string[] = []; + setAsyncIcaclsRunnerForTests(async args => { + if (args.includes("/grant:r")) steps.push("grant-owner"); + else if (args.includes("/inheritance:r")) steps.push("remove-inheritance"); + else if (args.includes("/remove:g")) steps.push("remove-broad"); + return ok; + }); + expect(await hardenSecretPathAsync(secretFile(), { required: true })).toEqual({ ok: true }); + expect(steps).toEqual(["grant-owner", "remove-inheritance", "remove-broad"]); + }); +});