fix(config): re-arm the atomic writer's ACL timeout memo per destination

docs-site/src/content/docs/troubleshooting/windows-acl-timeouts.md

@@ -0,0 +1,25 @@
+---
+title: Windows File Permission Timeouts
+description: How atomic file writes recover after a temporary icacls stall.
+---
+
+On Windows, opencodex uses `icacls` to protect temporary files before writing
+configuration, credentials, or cached state. If permission hardening fails, the
+write fails and the previous destination stays in place.
+
+A temporary `icacls` timeout does not require a proxy restart to recover. A later
+atomic write to the same destination may make one recovery attempt. If that
+attempt also times out, further writes to that destination are refused until
+five minutes after the recovery attempt started. The next write can then try
+hardening again; recovery happens when a write is requested, without a timer.
+
+If an earlier `icacls` process has outlived its caller's timeout belt and has not
+yet been reaped, writes to the same destination are refused, including after
+the five-minute window. These refusals return promptly and preserve the next
+recovery attempt. Healthy overlapping writes can proceed, including while an
+earlier write is hardening its temporary file or waiting to rename it.
+
+Every recovery attempt must complete permission hardening before publishing the
+file. Permission-denied errors remain failures and do not trigger an automatic
+retry. Cleanup of a failed write does not consume the next write's recovery
+attempt. Other file destinations have independent timeout tracking.

scripts/test-layout/layout.json

@@ -1927,6 +1927,7 @@
     "win-exec.test.ts": "windows",
     "win-paths.test.ts": "windows",
     "windows-acl-start-cost.test.ts": "windows",
+    "windows-atomic-write-acl-rearm.test.ts": "windows",
     "windows-atomic-replace.test.ts": "windows",
     "windows-deploy-close-regressions.test.ts": "windows",
     "windows-elevation-spawn.test.ts": "windows",

src/config/atomic-write.ts

@@ -18,10 +18,12 @@ import { recordOwnedConfigPath } from "../lib/config-ownership";
 import { assertNotRealCodexHomeUnderTest, assertNotRealHomeUnderTest } from "../lib/test-home-guard";
 import {
   forgetEphemeralSecretPath,
+  flushWindowsSecretAclReapsBeforeRemoval,
   hardenSecretPath,
   hardenSecretPathAsync,
   reattributeHardenedSecretPath,
   windowsSecretAclReapPendingForPath,
+  WindowsSecretAclSettleTimeoutError,
 } from "../lib/windows-secret-acl";
 import {
   renameAtomicFile,
@@ -30,6 +32,28 @@ import {
 import { getConfigDir } from "./paths";
 
 let atomicSequence = 0;
+const aclDestinations = new Map<string, Set<string>>();
+
+/** Track overlapping writes, refusing only children retained past their timeout belt. */
+function admitAtomicAclWork(target: string, temp: string, enabled: boolean): (() => void) | undefined {
+  if (!enabled || !windowsHardeningApplies()) return undefined;
+  const destination = join(realpathSync(dirname(target)), basename(target));
+  const key = process.platform === "win32" ? destination.toLowerCase() : destination;
+  const temps = aclDestinations.get(key) ?? new Set<string>();
+  if ([...temps].some(windowsSecretAclReapPendingForPath)) throw new WindowsSecretAclSettleTimeoutError();
+  temps.add(temp);
+  aclDestinations.set(key, temps);
+  const release = (): void => {
+    temps.delete(temp);
+    if (temps.size === 0) aclDestinations.delete(key);
+  };
+  return () => {
+    if (windowsSecretAclReapPendingForPath(temp)) {
+      // Caller settlement is bounded; admission resumes only after actual handle release.
+      void flushWindowsSecretAclReapsBeforeRemoval(temp).then(release);
+    } else release();
+  };
+}
 
 /**
  * Whether this writer's Windows ACL branch applies.
@@ -207,7 +231,7 @@ function writePrivateTempFile(
   try {
     onCreated(descriptor);
     if (windowsHardeningApplies()) {
-      hardenSecretPath(path, { required: true, timeoutMemoKey });
+      hardenSecretPath(path, { required: true, timeoutMemoKey, retryTimedOutOnce: true });
     }
     // Keyed on the REAL platform, not the override: on a POSIX host the mode is
     // the boundary and `assertPrivateTempDescriptor` demands 0o600, which an
@@ -248,7 +272,7 @@ function writePrivateTempFileWith(
   try {
     onCreated(descriptor);
     if (windowsHardeningApplies()) {
-      hardenSecretPath(path, { required: true, timeoutMemoKey });
+      hardenSecretPath(path, { required: true, timeoutMemoKey, retryTimedOutOnce: true });
     }
     if (process.platform !== "win32") fchmodSync(descriptor, 0o600);
     assertPrivateTempDescriptor(path, descriptor, requireStableIdentity);
@@ -271,7 +295,7 @@ async function writePrivateTempFileAsync(
   try {
     onCreated(descriptor);
     if (windowsHardeningApplies()) {
-      await hardenSecretPathAsync(path, { required: true, timeoutMemoKey });
+      await hardenSecretPathAsync(path, { required: true, timeoutMemoKey, retryTimedOutOnce: true });
     }
     if (process.platform !== "win32") {
       fchmodSync(descriptor, 0o600);
@@ -297,8 +321,11 @@ function atomicWriteFileToTarget(
   if (recordOwnership) recordOwnedConfigPath(getConfigDir(), path);
   assertResolvedTargetAllowed(path, target);
   const tmp = `${target}.ocx.${process.pid}.${nextAtomicTempSequence()}.tmp`;
+  const releaseAcl = admitAtomicAclWork(target, tmp, io === undefined);
   let hardened = false;
   let ownsTemp = false;
+  // Cleanup must not consume the destination's recovery attempt for a later write.
+  let cleanupHarden = false;
   let publicationAttempted = false;
   let identity: BigIntStats | undefined;
   const created = (descriptor: number): void => {
@@ -321,7 +348,7 @@ function atomicWriteFileToTarget(
       // its ChangeTime bump is what used to invalidate the harden memo one line
       // later. On POSIX the mode IS the boundary, so it stays.
       if (windowsHardeningApplies()) {
-        hardenSecretPath(tempPath, { required: true, timeoutMemoKey: path });
+        hardenSecretPath(tempPath, { required: true, timeoutMemoKey: path, retryTimedOutOnce: !cleanupHarden });
         return;
       }
       try { chmodSync(tempPath, 0o600); } catch { /* platform may ignore chmod */ }
@@ -365,6 +392,7 @@ function atomicWriteFileToTarget(
     forgetEphemeralSecretPath(tmp);
   } catch (cause) {
     if (!ownsTemp) throw cause;
+    cleanupHarden = true;
     let sharedTemp = false;
     if (exclusive) {
       try { sharedTemp = assertIdentity(tmp).nlink > 1n; } catch (error) {
@@ -403,7 +431,7 @@ function atomicWriteFileToTarget(
     if (removed) forgetEphemeralSecretPath(tmp);
     if (!removed) throw new AtomicWriteResidualTempError(tmp, hardened, { cause });
     throw cause;
-  }
+  } finally { releaseAcl?.(); }
 }
 
 export function atomicWriteFile(
@@ -490,12 +518,13 @@ export async function atomicWriteFileAsync(
   testSeam?: AtomicWriteAsyncTestSeam,
 ): Promise<void> {
   let ownsTemp = false;
+  let cleanupHarden = false;
   const effective: AtomicWriteAsyncIO = io ?? {
     write: (tempPath, value) => writePrivateTempFileAsync(tempPath, value, path, () => { ownsTemp = true; }),
     harden: async tempPath => {
       // Same reasoning as the synchronous writer above.
       if (windowsHardeningApplies()) {
-        await hardenSecretPathAsync(tempPath, { required: true, timeoutMemoKey: path });
+        await hardenSecretPathAsync(tempPath, { required: true, timeoutMemoKey: path, retryTimedOutOnce: !cleanupHarden });
         return;
       }
       try { chmodSync(tempPath, 0o600); } catch { /* platform may ignore chmod */ }
@@ -507,6 +536,7 @@ export async function atomicWriteFileAsync(
   const target = resolveWriteTarget(path);
   assertResolvedTargetAllowed(path, target);
   const tmp = `${target}.ocx.${process.pid}.${nextAtomicTempSequence()}.tmp`;
+  const releaseAcl = admitAtomicAclWork(target, tmp, io === undefined);
   let hardened = false;
   let tempWasHardenedBeforeContent = false;
   try {
@@ -520,6 +550,7 @@ export async function atomicWriteFileAsync(
     forgetEphemeralSecretPath(tmp);
   } catch (cause) {
     if (!ownsTemp) throw cause;
+    cleanupHarden = true;
     // The async ACL belt bounds the writer, but it is not evidence that icacls released this
     // path. Leave the temp in the existing residual state instead of racing an unlink against a
     // live Windows handle. The default Windows writer hardens before writing secret bytes; a
@@ -555,5 +586,5 @@ export async function atomicWriteFileAsync(
     if (removed) forgetEphemeralSecretPath(tmp);
     if (!removed) throw new AtomicWriteResidualTempError(tmp, hardened, { cause });
     throw cause;
-  }
+  } finally { releaseAcl?.(); }
 }

structure/config.md

@@ -70,7 +70,7 @@ identity, and snapshot-guarded removal. `RuntimePortState.attestationSecret` and
 owner-only state and are validated before a record is returned. `src/config.ts` re-exports the same
 symbols for compatibility, but new lifecycle-only callers import the process-state leaf directly.
 
-Replacing config and process-state writes use `src/config/atomic-write.ts`. The leaf preserves the shared
+Replacing config and process-state writes use `src/config/atomic-write.ts`. Normal Windows writes opt into the destination's single ACL timeout recovery attempt; a consumed attempt re-arms after `TIMEOUT_MEMO_REARM_MS` in `src/lib/windows-secret-acl.ts`. Default writers track temporary paths per resolved destination across synchronous, streamed and asynchronous writes. A new write refuses immediately with sanitized `ETIMEDOUT` only while one of that destination's tracked paths has an icacls child retained past its caller's timeout belt and not yet reaped, including across re-arm windows. Healthy overlapping writes proceed, including during ACL hardening or a pause before rename. Refusal consumes no recovery attempt and leaves removal ownership intact; completing one write does not drop another write's retained-child tracking. Both cleanup paths leave that recovery unconsumed. Every admitted attempt still requires a successful harden before secret bytes or publication; non-timeout failures retain their existing fail-closed behavior. `tests/windows/windows-atomic-write-acl-rearm.test.ts` covers all five harden calls, the re-arm boundary, healthy overlap, retained child admission and cleanup. The leaf preserves the shared
 process-wide temp sequence, optional synchronous caller destination validation before every rename attempt, symlink target resolution, no-follow directory-entry replacement for
 externally writable integration directories, real-home test guard, owner manifest, Windows ACL hardening,
 scrub-before-unlink failure path, and explicit residual-temp errors. No-follow writes also accept raw bytes and exclusive publication: the same protected descriptor writer checks bigint file identity before and after content, then an atomic hard link refuses any occupied destination and verifies the published identity. Exclusive cleanup never truncates a shared inode or any inode after a link attempt, even if the syscall reports failure; a substituted temporary entry is neither scrubbed nor removed. A caller must not replace it with a local temp-and-rename shortcut. Publication failures in

tests/fixtures/test-layout-expected.json

@@ -1,5 +1,6 @@
 { "update-reclaim-authorizer.test.ts": "update", "update-capability-policy.test.ts": "update", "update-service-home.test.ts": "update", "update-service-home-policy.test.ts": "update", "update-home-recovery.test.ts": "update", "update-recovery-authority.test.ts": "update", "update-recovery-boundaries.test.ts": "update", "update-home-launcher.test.ts": "update", "update-home-worker.test.ts": "update", "update-mutation-boundaries.test.ts": "update", "update-mutation-guard.test.ts": "update", "update-guarded-ops.test.ts": "update", "update-effect-authority.test.ts": "update", "update-windows-wrapper-authority.test.ts": "update", "update-mutation-scanner.test.ts": "update", "refreshed-upstream-deadlines.test.ts": "responses", "local-upstream-deadlines.test.ts": "responses", "service-desktop-recovery.test.ts": "service", "provider-input-modalities.test.ts": "providers", "turn-membership.test.ts": "responses", "turn-membership-ingress.test.ts": "responses", "workbuddy-quota.test.ts": "providers", "workbuddy-login.test.ts": "oauth", "workbuddy-adapter.test.ts": "adapters", "workbuddy-provider.test.ts": "providers", "link-tunnel-history.test.ts": "clients", "link-readiness.test.ts": "clients", "link-diagnostics.test.ts": "clients", "combo-description.test.ts": "routing", "combo-description-catalog.test.ts": "codex-integration", "catalog-auto-review-override.test.ts": "codex-integration", "management-auto-review-settings.test.ts": "server", "management-auto-review-auth.test.ts": "server", "prefer-model-alias-ids.test.ts": "server", "responses-mimo-envelope.test.ts": "responses", "responses-call-id-linkage.test.ts": "responses", "responses-phase-inference.test.ts": "responses",
     "windows-config-root-async-writers.test.ts": "windows",
+    "windows-atomic-write-acl-rearm.test.ts": "windows",
     "t3code-client.test.ts": "clients",
     "t3code-client-lifecycle.test.ts": "clients",
   "fix-jsonc-cr-comments.test.ts": "lib",

tests/server/config.test.ts

@@ -3440,7 +3440,7 @@ describe("config.ts – sync writer timeout keying (#840 refinement)", () => {
       atomicWriteFile(destination, "secret");
       expect(hardenSpy).toHaveBeenCalledWith(
         expect.stringContaining(".tmp"),
-        { required: true, timeoutMemoKey: destination },
+        { required: true, timeoutMemoKey: destination, retryTimedOutOnce: true },
       );
     } finally {
       hardenSpy.mockRestore();