[Bug]: synchronous Windows ACL hardening stalls the proxy on response-state writes #612

Client or integration

Codex App

Area

Proxy and routing

Summary

On Windows, response-state persistence can block the OpenCodex proxy event loop while NTFS ACL hardening runs. The current path is schedulePersist() -> persistNow() -> atomicWriteFile() -> hardenSecretPath() -> synchronous Bun.spawnSync(["icacls.exe", ...]). While that child process is running or stalled, local /healthz requests and other proxy work cannot be serviced.

The shared ACL deadline from #160 limits an individual hardening call, but it does not make the wait non-blocking. In addition, the timeout cache is keyed by the exact temporary pathname. atomicWriteFile() generates a new .ocx.<pid>.<sequence>.tmp name on every write, so a timeout cached for one response-state temp file does not suppress the same stall on the next response-state write.

I expected the ACL attempt and its existing result policy to remain ordered before publishing the atomic file, while the proxy event loop stayed responsive during the child-process wait. If a later ACL child stalls, a new temporary pathname should not make the proxy event loop block again.

Reproduction

The timing-dependent production symptom can be reduced to a deterministic test using the existing ACL test seams. This does not run or modify real ACLs:

1. Use Windows, OpenCodex 2.7.42 or current dev commit 7710185c0548f50ea5a1c0034c2a6ea398fd865e, and the package's bundled Bun 1.3.14.
2. Set OPENCODEX_ACL_TIMEOUT_MS=1000, force the test platform to win32, and replace the icacls runner with a runner that synchronously waits for the supplied timeout and returns { success: false, exitCode: null, timedOut: true, stdout: "" }.
3. Before each hardenSecretPath() call, schedule a 10 ms timer, record when it actually fires, and await that timer before starting the next measurement.
4. Call hardenSecretPath(pathA, { required: false }), repeat the same call for pathA, then call it for a second existing path pathB.
5. Observe that the first unique path blocks the event loop for the full deadline, the identical path is skipped by timedOutPaths, and the second unique path blocks for the full deadline again.

Minimal runner shape:

process.env.OPENCODEX_ACL_TIMEOUT_MS = "1000";
setPlatformForTests("win32");
resetHardenedStateForTests();
setIcaclsRunnerForTests((_args, timeoutMs) => {
  Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, timeoutMs);
  return { success: false, exitCode: null, timedOut: true, stdout: "" };
});

Measured result from the bundled Bun 1.3.14 run:

Every response-state write uses a new temp path and therefore misses both exact-path caches. If icacls.exe stalls or times out, a later write can incur the synchronous wait again.

As supporting live evidence, one correlation sample observed a live icacls.exe child while loopback /healthz took 742 ms; idle samples in the same run were about 61 ms. That service used a Bun 1.4 canary override, so the deterministic unit-level reproduction on the bundled Bun 1.3.14 runtime above is the primary evidence.

Version

• Released package: 2.7.42
• Also present on current dev: 7710185c0548f50ea5a1c0034c2a6ea398fd865e
• Deterministic reproduction runtime: bundled Bun 1.3.14 (1.3.14+0d9b296af)

Operating system

Microsoft Windows 11 Pro 25H2, x64 (build 26200.8894)

Provider and model

Not provider-specific.

Logs or error output

[opencodex] ACL hardening timed out (ETIMEDOUT) — transient icacls stall; the volume may still support per-user NTFS ACLs; ACL state unverified (budget exhausted) — continuing without NTFS ACL harden

Deterministic result:

first unique temp path: harden=1015ms, 10ms timer=1017ms
same path again:        harden=0ms,    10ms timer=12ms
second unique temp path:harden=1010ms, 10ms timer=1025ms

Screenshots and supporting files

Relevant current source paths:

• src/lib/windows-secret-acl.ts: the default runner uses synchronous Bun.spawnSync; timedOutPaths is keyed by exact path.
• src/config.ts: atomicWriteFile() generates a unique temp pathname and hardens it synchronously before rename.
• src/responses/state.ts: the debounced response-state snapshot calls atomicWriteFile() directly from the main event loop.

This is related to, but not a duplicate of, #160, #499 / PR #502, and #596 / PR #601. #160 bounds and diagnoses ACL timeouts. #499 / PR #502 handles stale response-state temp cleanup. #596 / PR #601 preserves the current-user ACE by making the sequence owner-first. None changes the synchronous event-loop wait or the unique-temp timeout-cache behavior.

Redacted configuration

{
  "provider_specific": false,
  "OPENCODEX_ACL_TIMEOUT_MS": "1000 for deterministic reproduction; default remains 5000"
}

Checks

• I searched existing issues and documentation.
• I removed secrets, tokens, account details, request credentials, and personal data.

Root cause

Bun.spawnSync() runs each icacls.exe step on the proxy's main thread. The total deadline prevents an unbounded wait, but the JavaScript event loop cannot service health checks or requests until the synchronous call returns.

The timeout memoization only helps when the exact same path is retried. Response-state atomic writes use a fresh sequence-numbered temp path for every snapshot, so each subsequent write is treated as unrelated even though it serves the same logical destination.

Suggested safe direction

A narrow asynchronous path for high-frequency response-state persistence appears safer than increasing GUI health-probe timeouts or weakening ACLs:

1. Add an asynchronous ACL runner for this path using non-blocking process creation with windowsHide, the same shared deadline, and the same honest timeout classification.
2. Add an asynchronous atomic writer that awaits the ACL attempt and applies the existing success, timeout-only soft-fail, or real-error policy before rename.
3. Serialize response-state persistence with a single-flight queue so overlapping snapshots cannot publish out of order, and await the final flush during graceful shutdown.
4. Preserve the owner-first invariant from PR fix(windows): grant owner ACE before ACL inheritance removal #601 and the current fail-closed behavior for real ACL errors. Keep the existing timeout-only policy explicit.

Skipping per-file hardening based only on the parent ACL is not a safe general fix. A parent directory may legitimately grant read access to additional principals, so a new response-state temp file can inherit a broader ACE until file-level hardening removes it.

The mutation steps that do not need output can use ignored stdout, while /findsid still needs captured output. An isolated Windows benchmark showed that changing stdout handling alone did not remove the stall; the synchronous process wait is the material issue.