perf(windows): stop hardening every secret temp twice

scripts/test-layout/layout.json

@@ -1430,6 +1430,7 @@
     "web-search.test.ts": "web-search",
     "win-exec.test.ts": "windows",
     "win-paths.test.ts": "windows",
+    "windows-acl-start-cost.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

@@ -17,6 +17,7 @@ import {
   forgetEphemeralSecretPath,
   hardenSecretPath,
   hardenSecretPathAsync,
+  reattributeHardenedSecretPath,
 } from "../lib/windows-secret-acl";
 import {
   renameAtomicFile,
@@ -26,6 +27,26 @@ import { getConfigDir } from "./paths";
 
 let atomicSequence = 0;
 
+/**
+ * Whether this writer's Windows ACL branch applies.
+ *
+ * Deliberately a local override rather than the ACL module's
+ * `windowsSecretAclApplies()`. That one is flipped by many unrelated suites
+ * through `setPlatformForTests("win32")`, and honouring it here would make every
+ * such suite start running icacls against config writes on a POSIX runner. The
+ * gate a cost fix needs is one only the cost tests can open.
+ */
+let windowsHardeningOverride: boolean | null = null;
+
+function windowsHardeningApplies(): boolean {
+  return windowsHardeningOverride ?? process.platform === "win32";
+}
+
+/** Test seam: drive the Windows hardening branch on a POSIX runner. Null restores. */
+export function setWindowsHardeningForTests(enabled: boolean | null): void {
+  windowsHardeningOverride = enabled;
+}
+
 /** Shared process-wide suffix source for config-owned atomic sibling files. */
 export function nextAtomicTempSequence(): number {
   return ++atomicSequence;
@@ -114,6 +135,32 @@ function assertPrivateTempDescriptor(path: string, descriptor: number): void {
   }
 }
 
+/**
+ * Carry the harden applied to the empty temp across the content write.
+ *
+ * The temp is hardened before it holds a byte, and then `atomicWriteFile` hardens
+ * it again before the rename. The second call used to be a full icacls sequence
+ * rather than a memo hit, because the ACL memo's freshness component is
+ * `ctimeNs` and libuv reports that from the NTFS ChangeTime, which a data write
+ * moves. So every secret write on Windows applied the same three-step ACL twice:
+ * `/grant:r`, `/inheritance:r`, `/remove:g`, plus up to three `/findsid` probes,
+ * all of it to arrive at the ACL the file already had.
+ *
+ * It runs after the descriptor closes, not before, because Windows may not have
+ * published the new ChangeTime to a path query while the handle is still open;
+ * refreshing to a time the next reader will not see would leave the memo missing
+ * and change nothing. What licenses the shortcut is the identity assertion the
+ * caller makes immediately before the close, which proves this path resolves to
+ * the object the ACL was applied to, plus the object comparison inside
+ * `reattributeHardenedSecretPath`, which refuses to move the memo to a different
+ * object. Nothing is skipped on the strength of the pathname alone, and a
+ * refusal costs only the second full harden this is trying to avoid.
+ */
+function carryHardenAcrossContentWrite(path: string): void {
+  if (!windowsHardeningApplies()) return;
+  reattributeHardenedSecretPath(path);
+}
+
 function writePrivateTempFile(
   path: string,
   content: string,
@@ -123,16 +170,24 @@ function writePrivateTempFile(
   const descriptor = openSync(path, "wx", 0o600);
   onCreated();
   try {
-    if (process.platform === "win32") {
+    if (windowsHardeningApplies()) {
       hardenSecretPath(path, { required: true, timeoutMemoKey });
-    } else {
+    }
+    // Keyed on the REAL platform, not the override: on a POSIX host the mode is
+    // the boundary and `assertPrivateTempDescriptor` demands 0o600, which an
+    // ambient umask can otherwise take away from the open above.
+    if (process.platform !== "win32") {
       fchmodSync(descriptor, 0o600);
     }
     assertPrivateTempDescriptor(path, descriptor);
     writeFileSync(descriptor, content, { encoding: "utf-8" });
+    // Second assertion, after the content write: the object this path resolves
+    // to is still the object the ACL was applied to and the one we just wrote.
+    assertPrivateTempDescriptor(path, descriptor);
   } finally {
     closeSync(descriptor);
   }
+  carryHardenAcrossContentWrite(path);
 }
 
 async function writePrivateTempFileAsync(
@@ -144,16 +199,19 @@ async function writePrivateTempFileAsync(
   const descriptor = openSync(path, "wx", 0o600);
   onCreated();
   try {
-    if (process.platform === "win32") {
+    if (windowsHardeningApplies()) {
       await hardenSecretPathAsync(path, { required: true, timeoutMemoKey });
-    } else {
+    }
+    if (process.platform !== "win32") {
       fchmodSync(descriptor, 0o600);
     }
     assertPrivateTempDescriptor(path, descriptor);
     writeFileSync(descriptor, content, { encoding: "utf-8" });
+    assertPrivateTempDescriptor(path, descriptor);
   } finally {
     closeSync(descriptor);
   }
+  carryHardenAcrossContentWrite(path);
 }
 
 export function atomicWriteFile(
@@ -171,10 +229,15 @@ export function atomicWriteFile(
   const effective: AtomicWriteIO = io ?? {
     write: (tempPath, value) => writePrivateTempFile(tempPath, value, path, () => { ownsTemp = true; }),
     harden: tempPath => {
-      try { chmodSync(tempPath, 0o600); } catch { /* platform may ignore chmod */ }
-      if (process.platform === "win32") {
+      // No chmod on the Windows branch: there it sets the read-only ATTRIBUTE,
+      // which is not the secret boundary and is not what protects this file, and
+      // 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 });
+        return;
       }
+      try { chmodSync(tempPath, 0o600); } catch { /* platform may ignore chmod */ }
     },
     rename: renameAtomicFile,
     truncate: tempPath => truncateSync(tempPath, 0),
@@ -245,10 +308,12 @@ export async function atomicWriteFileAsync(
   const effective: AtomicWriteAsyncIO = io ?? {
     write: (tempPath, value) => writePrivateTempFileAsync(tempPath, value, path, () => { ownsTemp = true; }),
     harden: async tempPath => {
-      try { chmodSync(tempPath, 0o600); } catch { /* platform may ignore chmod */ }
-      if (process.platform === "win32") {
+      // Same reasoning as the synchronous writer above.
+      if (windowsHardeningApplies()) {
         await hardenSecretPathAsync(tempPath, { required: true, timeoutMemoKey: path });
+        return;
       }
+      try { chmodSync(tempPath, 0o600); } catch { /* platform may ignore chmod */ }
     },
     rename: renameAtomicFileAsync,
     truncate: target => truncateSync(target, 0),

src/lib/windows-secret-acl.ts

@@ -52,15 +52,21 @@ const timedOutPaths = new Map<string, boolean>();
 const ASYNC_ICACLS_BELT_MARGIN_MS = 250;
 
 /**
- * The memo value: `object:freshness` for a file a harden was actually attributed
- * to.
+ * The memo value: the `object` plus `freshness` of a file a harden was actually
+ * attributed to.
  *
  * There is deliberately no null member. An observation that cannot be read is
  * not stored at all — the entry is deleted — because a "recorded as unverifiable"
  * value was dead code the moment attribution became a before/after comparison,
  * and a branch nothing can reach is a branch no test can defend.
+ *
+ * It is the observation itself rather than a joined string so that the two
+ * questions stay separately askable after storage. `reattributeHardenedSecretPath`
+ * has to compare the object while deliberately ignoring the freshness, and
+ * recovering one half out of `dev:ino:ctimeNs` by counting colons would make that
+ * comparison depend on a format nothing declares.
  */
-type HardenedIdentity = string;
+type HardenedIdentity = PathObservation;
 
 /**
  * What a stat can tell us about WHICH OBJECT is at a path.
@@ -130,8 +136,8 @@ function observe(targetPath: string): PathObservation | null {
   }
 }
 
-function memoValue(seen: PathObservation): HardenedIdentity {
-  return `${seen.object}:${seen.freshness}`;
+function sameObservation(a: PathObservation, b: PathObservation): boolean {
+  return a.object === b.object && a.freshness === b.freshness;
 }
 
 /**
@@ -157,7 +163,7 @@ function memoSatisfied(cache: Map<string, HardenedIdentity>, targetPath: string)
   // without any ACL work. That needs exact-identity ABA to bite — outside the
   // proof bound this unit claims — but "the consequence is out of scope" is not a
   // reason to keep an entry we have just proven does not describe what is there.
-  if (current === null || memoValue(current) !== remembered) {
+  if (current === null || !sameObservation(current, remembered)) {
     cache.delete(targetPath);
     return false;
   }
@@ -205,7 +211,7 @@ function recordHarden(
     cache.delete(targetPath);
     return false;
   }
-  cache.set(targetPath, memoValue(after));
+  cache.set(targetPath, after);
   return true;
 }
 
@@ -429,6 +435,58 @@ export function forgetHardenedSecretPath(targetPath: string): void {
   hardenedPaths.delete(targetPath);
 }
 
+/**
+ * Re-attribute an existing file memo to the SAME object after the caller wrote
+ * content to it through a descriptor whose identity it verified.
+ *
+ * This exists because `freshness` is `ctimeNs`, and on Windows libuv reports
+ * `st_ctim` from the NTFS ChangeTime, which moves when file DATA is written. An
+ * atomic writer therefore invalidated its own memo between the harden that
+ * protects the empty temp and the harden before the rename, and paid a second
+ * full `/grant:r` + `/inheritance:r` + `/remove:g` sequence to reapply the ACL
+ * that was already on the file. Every secret write on Windows paid it twice.
+ *
+ * Only the freshness moves, and only for an unchanged object: a different object
+ * retires the entry instead. A caller must have proven, immediately beforehand,
+ * that `targetPath` resolves to the object its own descriptor refers to.
+ *
+ * The cost of this is worth stating exactly, because `PathObservation` documents
+ * that freshness also moves when PERMISSIONS change, and this call cannot tell
+ * the two apart. So a DACL change landing between the harden and this call is
+ * absorbed instead of forcing a re-harden. That window is the caller's own
+ * content write; every permission change after this call still moves ctime
+ * again and still misses the memo, so the detection this memo provides is
+ * relocated, not removed.
+ *
+ * What makes the absorbed window acceptable is who can be in it. Once the harden
+ * has run, the DACL is an explicit owner-only ACE with inheritance removed, so
+ * no other principal can open the file for `WRITE_DAC` at all. The one principal
+ * who can still rewrite that DACL is one holding a handle opened BEFORE the
+ * harden, and Windows keeps the access granted to an open handle: that principal
+ * can equally rewrite the DACL after any later harden, and after the rename, on
+ * the same object. A second mutation pass never bounded that capability — it
+ * stripped an ACE the holder could immediately re-add — so declining to repeat
+ * it removes no guarantee anyone had.
+ *
+ * Refusal is cheap and safe in either direction: an unmoved memo simply means the
+ * caller's next harden runs in full.
+ *
+ * Returns whether the memo now describes what is at the path.
+ */
+export function reattributeHardenedSecretPath(targetPath: string): boolean {
+  const remembered = hardenedPaths.get(targetPath);
+  if (remembered === undefined) return false;
+  const current = observe(targetPath);
+  // Unreadable, or a different object: this is exactly the case the memo must
+  // not cover. Retire it so the next harden is a real one.
+  if (current === null || current.object !== remembered.object) {
+    hardenedPaths.delete(targetPath);
+    return false;
+  }
+  hardenedPaths.set(targetPath, current);
+  return true;
+}
+
 /**
  * Ephemeral-path lifecycle release: clears the success memo AND any timeout
  * memo keyed by THIS TEMP path in both namespaces. Call only after the temp is

structure/config.md

@@ -52,6 +52,13 @@ process-wide temp sequence, symlink target resolution, real-home test guard, own
 Windows ACL hardening, scrub-before-unlink failure path, and explicit residual-temp errors. A caller
 must not replace it with a local temp-and-rename shortcut.
 
+Windows hardening there is applied once per write, not once per harden call. Both calls stay
+`required: true` and still fail the write closed, but the pre-rename call resolves through the
+`src/lib/windows-secret-acl.ts` success memo: after the content write the writer re-asserts that the
+path still resolves to the object its descriptor holds, then re-attributes the memo to that same
+object so the freshness the data write moved does not read as a replacement. A different object, or
+one that cannot be observed, retires the memo and the pre-rename call performs the full sequence.
+
 > Decision record: [ADR-0016](decisions/ADR-0016-config-surface.md)
 
 `src/types.ts` is the shape; the load/validate pipeline lives in the split config leaves — schema in `src/config/schema/` (`config-schema.ts`, `leaf-validators.ts`) and replace-path persistence in `src/config/persist-unlocked.ts`, with `src/config.ts` as the compatibility facade — and is not reproduced here. What

structure/runtime.md

@@ -55,7 +55,7 @@ The prefilter is only an optimization, not final process-membership authority.
 | `src/server/audio-live.ts`, `src/server/audio-dictation.ts` | External voice/dictation orchestration using the existing bounded socket relay, server-owned credentials, cancellation and opaque call ownership. See [streaming audio](data-planes/inbound-compat.md#streaming-audio). |
 | `src/config.ts` | Persisted `~/.opencodex/config.json` surface: the facade keeps the load/save/initialize entry points and re-exports, while schema lives in `src/config/schema/` (`config-schema.ts`, `leaf-validators.ts`), defaults in `src/config/proxy-env.ts`, and replace-path persistence in `src/config/persist-unlocked.ts`. |
 | `src/config/paths.ts` | Resolves `OPENCODEX_HOME`, `config.json`, and owner-only directory hardening. |
-| `src/config/atomic-write.ts` | Shared synchronous/asynchronous temp-harden-rename writer and residual-temp failure contract. |
+| `src/config/atomic-write.ts` | Shared synchronous/asynchronous temp-harden-rename writer and residual-temp failure contract. The temp is ACL-hardened before it holds a byte and again before the rename, both `required: true`; the second call is a memo hit rather than a second icacls sequence because the writer re-asserts descriptor/path identity after the content write and re-attributes the harden through `reattributeHardenedSecretPath`. Windows takes no `chmod` on that path — it sets the read-only attribute, not the DACL, and its ChangeTime bump is what used to retire the memo. |
 | `src/config/process-state.ts` | Owns `ocx.pid`, `runtime-port.json`, cheap liveness, full command-line identity verification, and snapshot-guarded cleanup. |
 | `src/server/ports.ts` | Owns bind availability and ephemeral-port selection. Temporary probes dispose accepted peers and wait for listener close before reporting success. |
 | `src/cli/status.ts` / `src/cli/status-probes.ts` | Status snapshot assembly and the shared read-only health/stale-process probes used by status and doctor. Probe evidence keeps recorded-port choice, before/after snapshots and per-call timer cleanup together. |

tests/fixtures/test-layout-expected.json

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