From aec717722716537feab2f36c67525af974c095f4 Mon Sep 17 00:00:00 2001 From: Ingwannu Date: Sun, 30 Aug 2026 16:12:14 +0000 Subject: [PATCH] fix(windows): move response spill ACL work off event loop --- src/config/paths.ts | 21 ++- src/responses/spill-store.ts | 179 ++++++++++++++++++--- src/responses/state.ts | 219 +++++++++++++++++++++++++- structure/02_config-and-codex-home.md | 20 +++ tests/config.test.ts | 25 ++- tests/responses-state.test.ts | 108 +++++++++++++ 6 files changed, 540 insertions(+), 32 deletions(-) diff --git a/src/config/paths.ts b/src/config/paths.ts index b8b494ecd7..4c351a6ae8 100644 --- a/src/config/paths.ts +++ b/src/config/paths.ts @@ -1,7 +1,7 @@ import { chmodSync, existsSync } from "node:fs"; import { homedir } from "node:os"; import { join, resolve } from "node:path"; -import { hardenSecretDir } from "../lib/windows-secret-acl"; +import { hardenSecretDirAsync, windowsSecretAclApplies } from "../lib/windows-secret-acl"; import { assertNotRealHomeUnderTest } from "../lib/test-home-guard"; /** @@ -14,6 +14,7 @@ export function expandUserPath(raw: string): string { return raw; } let resolvedConfigDirCache: { raw: string | undefined; path: string } | null = null; +const configDirHardeningFlights = new Map>(); export function getConfigDir(): string { const raw = process.env["OPENCODEX_HOME"]?.trim() || undefined; @@ -34,7 +35,21 @@ export function hardenConfigDir(): void { assertNotRealHomeUnderTest(dir); if (!existsSync(dir)) return; try { chmodSync(dir, 0o700); } catch { /* best-effort */ } - if (process.platform === "win32") { - hardenSecretDir(dir, { required: false }); + if (windowsSecretAclApplies() && !configDirHardeningFlights.has(dir)) { + // This is an optional read-path harden. Waiting synchronously here used to stop the Bun + // event loop (including /healthz) for the full icacls timeout. Required mutation paths keep + // their own awaited/fail-closed hardening; ordinary config reads only start one soft flight. + const flight = hardenSecretDirAsync(dir, { required: false }) + .then(() => undefined) + .catch(() => undefined) + .finally(() => { + if (configDirHardeningFlights.get(dir) === flight) configDirHardeningFlights.delete(dir); + }); + configDirHardeningFlights.set(dir, flight); } } + +/** Test-only: settle optional config-directory hardening without exposing it to production callers. */ +export async function flushConfigDirHardeningForTests(): Promise { + await Promise.all([...configDirHardeningFlights.values()]); +} diff --git a/src/responses/spill-store.ts b/src/responses/spill-store.ts index 825a3da3a3..5475636f5d 100644 --- a/src/responses/spill-store.ts +++ b/src/responses/spill-store.ts @@ -17,7 +17,15 @@ import { import { createHash, randomBytes } from "node:crypto"; import { join } from "node:path"; import { getConfigDir } from "../config"; -import { forgetEphemeralSecretPath, forgetHardenedSecretPath, hardenSecretDir, hardenSecretPath } from "../lib/windows-secret-acl"; +import { + forgetEphemeralSecretPath, + forgetHardenedSecretPath, + hardenSecretDir, + hardenSecretDirAsync, + hardenSecretPath, + hardenSecretPathAsync, + windowsSecretAclApplies, +} from "../lib/windows-secret-acl"; import { isValidProviderContinuationOwner } from "./provider-continuation"; import type { OcxProviderContinuationState } from "../types"; @@ -189,6 +197,20 @@ function harden(path: string, mode: number): void { } } +async function hardenAsync(path: string, mode: number, retryTimedOutOnce = false): Promise { + try { + chmodSync(path, mode); + } catch { + if (!windowsSecretAclApplies()) throw new Error("Response spill permission hardening failed"); + } + if (windowsSecretAclApplies()) { + const result = mode === 0o700 + ? await hardenSecretDirAsync(path, { required: true, retryTimedOutOnce }) + : await hardenSecretPathAsync(path, { required: true, retryTimedOutOnce }); + if (!result.ok) throw new Error("Response spill permission hardening failed"); + } +} + function writeAll(fd: number, bytes: Uint8Array): void { if (spillIoForTest?.write) spillIoForTest.write(fd, bytes); else { @@ -261,6 +283,78 @@ function publishNoReplace(tempPath: string, destinationPath: string): void { record("publish"); } +async function publishNoReplaceAsync( + tempPath: string, + destinationPath: string, + retryTimedOutOnce: boolean, +): Promise { + try { + if (spillIoForTest?.link) spillIoForTest.link(tempPath, destinationPath); + else linkSync(tempPath, destinationPath); + } catch (error) { + if (isErrno(error, "EEXIST")) throw error; + if (!canUseExclusiveCopyFallback(error)) throw error; + let copied = false; + try { + if (spillIoForTest?.copyFileExcl) spillIoForTest.copyFileExcl(tempPath, destinationPath); + else copyFileSync(tempPath, destinationPath, constants.COPYFILE_EXCL); + copied = true; + await hardenAsync(destinationPath, 0o600, retryTimedOutOnce); + const copyFd = openSync(destinationPath, "r"); + try { + if (spillIoForTest?.fsync) spillIoForTest.fsync(copyFd); + else fsyncSync(copyFd); + } finally { + closeSync(copyFd); + } + } catch (copyError) { + if (copied) { + try { unlink(destinationPath); } catch { /* startup GC reclaims an incomplete publication */ } + } + throw copyError; + } + } + record("publish"); +} + +function serializedSpill( + responseId: string, + state: Omit, +): { + bytes: Buffer; + digest: string; + idDigest: string; + contentDigest: string; +} { + const payload: ResponseSpillPayload = { + version: 1, + responseId, + createdAt: state.createdAt, + ...(state.clientThreadId ? { clientThreadId: state.clientThreadId } : {}), + items: state.items, + ...(state.providerOutputStart !== undefined ? { providerOutputStart: state.providerOutputStart } : {}), + ...(state.providers ? { providers: state.providers } : {}), + }; + const serialized = JSON.stringify(payload); + if (serialized === undefined) throw new Error("Response spill serialization failed"); + const bytes = Buffer.from(serialized, "utf8"); + const digest = sha256(bytes); + return { + bytes, + digest, + idDigest: sha256(responseId).slice(0, 12), + contentDigest: digest.slice(0, 24), + }; +} + +function responseSpillWriteError(cause: unknown): NodeJS.ErrnoException { + const error = new Error("Response spill write failed", { cause }) as NodeJS.ErrnoException; + if (cause && typeof cause === "object" && "code" in cause) { + error.code = String((cause as { code?: unknown }).code); + } + return error; +} + function validSpillRef(ref: ResponseSpillRef): boolean { return ref.version === 1 && OWNED_SPILL_NAME.test(ref.fileName) @@ -306,21 +400,7 @@ export function writeResponseSpillDurably( let tempPath: string | null = null; let fd: number | null = null; try { - const payload: ResponseSpillPayload = { - version: 1, - responseId, - createdAt: state.createdAt, - ...(state.clientThreadId ? { clientThreadId: state.clientThreadId } : {}), - items: state.items, - ...(state.providerOutputStart !== undefined ? { providerOutputStart: state.providerOutputStart } : {}), - ...(state.providers ? { providers: state.providers } : {}), - }; - const serialized = JSON.stringify(payload); - if (serialized === undefined) throw new Error("Response spill serialization failed"); - const bytes = Buffer.from(serialized, "utf8"); - const digest = sha256(bytes); - const idDigest = sha256(responseId).slice(0, 12); - const contentDigest = digest.slice(0, 24); + const { bytes, digest, idDigest, contentDigest } = serializedSpill(responseId, state); const dir = responseSpillDirectory(); mkdirSync(dir, { recursive: true, mode: 0o700 }); harden(dir, 0o700); @@ -352,14 +432,77 @@ export function writeResponseSpillDurably( } } throw new Error("Response spill publication retries exhausted"); - } catch { + } catch (cause) { + if (fd !== null) { + try { closeSync(fd); } catch { /* best effort */ } + } + if (tempPath) { + try { unlinkEphemeral(tempPath); } catch { /* best effort */ } + } + throw responseSpillWriteError(cause); + } +} + +/** + * Windows runtime counterpart of `writeResponseSpillDurably`. + * + * The filesystem publication contract stays identical, but required NTFS ACL subprocesses are + * awaited through Bun.spawn instead of Bun.spawnSync. State ownership and serialization remain in + * `state.ts`; callers must compare the resident generation again before installing the returned + * reference because another response can replace it while ACL hardening is pending. + */ +export async function writeResponseSpillDurablyAsync( + responseId: string, + state: Omit, + options: { retryTimedOutOnce?: boolean } = {}, +): Promise { + let tempPath: string | null = null; + let fd: number | null = null; + try { + const { bytes, digest, idDigest, contentDigest } = serializedSpill(responseId, state); + const dir = responseSpillDirectory(); + mkdirSync(dir, { recursive: true, mode: 0o700 }); + await hardenAsync(dir, 0o700, options.retryTimedOutOnce === true); + + tempPath = join(dir, `.response-spill.${process.pid}.${randomBytes(8).toString("hex")}.tmp`); + fd = openSync(tempPath, "wx", 0o600); + writeAll(fd, bytes); + fsyncFile(fd); + closeFile(fd); + fd = null; + await hardenAsync(tempPath, 0o600, options.retryTimedOutOnce === true); + record("harden"); + const publishTempPath = tempPath; + + for (let attempt = 0; attempt < RESPONSE_SPILL_PUBLISH_RETRIES; attempt++) { + spillGeneration += 1; + const fileName = `${sanitizeResponseId(responseId)}.${idDigest}.${contentDigest}.${spillGeneration}.${bytes.byteLength}.spill.json`; + if (!OWNED_SPILL_NAME.test(fileName)) throw new Error("Response spill name allocation failed"); + const destinationPath = join(dir, fileName); + try { + await publishNoReplaceAsync( + publishTempPath, + destinationPath, + options.retryTimedOutOnce === true, + ); + fsyncDirectoryBestEffort(dir); + unlinkEphemeral(publishTempPath); + tempPath = null; + return { version: 1, fileName, digest, payloadBytes: bytes.byteLength }; + } catch (error) { + if (isErrno(error, "EEXIST")) continue; + throw error; + } + } + throw new Error("Response spill publication retries exhausted"); + } catch (cause) { if (fd !== null) { try { closeSync(fd); } catch { /* best effort */ } } if (tempPath) { try { unlinkEphemeral(tempPath); } catch { /* best effort */ } } - throw new Error("Response spill write failed"); + throw responseSpillWriteError(cause); } } diff --git a/src/responses/state.ts b/src/responses/state.ts index 940de10e69..f0b6b6c2c4 100644 --- a/src/responses/state.ts +++ b/src/responses/state.ts @@ -3,9 +3,11 @@ import { uptime } from "node:os"; import { dirname, join } from "node:path"; import { atomicWriteFileAsync, getConfigDir, resolveWriteTarget } from "../config"; import { enforceAppOwnedMemoryBudget, type RetainedStoreSnapshot } from "../lib/app-owned-memory"; +import { windowsSecretAclApplies } from "../lib/windows-secret-acl"; import type { OcxProviderContinuationState } from "../types"; import { deleteResponseSpill, + MAX_RESPONSE_SPILL_PAYLOAD_BYTES, noteStubSwapForTest, readResponseSpill, recoverOrphanedResponseSpills, @@ -13,6 +15,7 @@ import { responseSpillPayloadCap, type ResponseSpillRef, writeResponseSpillDurably, + writeResponseSpillDurablyAsync, } from "./spill-store"; const MAX_STORED_RESPONSES = 1_000; @@ -160,6 +163,176 @@ const pendingSpillUnlinks: ResponseSpillRef[] = []; // structured 400 — bounded-loss, never silent corruption or unbounded disk. const PENDING_SPILL_UNLINKS_MAX = 128; +/** + * Windows keeps the candidate replayable while required ACL hardening runs off the event loop. + * Pending bytes are pinned, not evictable; cap them below the process-owned 512 MiB ceiling so an + * icacls outage cannot turn the serialized queue into an unbounded resident backlog. + */ +const MAX_PENDING_RESPONSE_SPILL_BYTES = MAX_RESPONSE_SPILL_PAYLOAD_BYTES; + +interface PendingResponseSpill { + id: string; + candidate: ResidentResponseState | null; + supersededSpill?: ResponseSpillRef; + directAdmission: boolean; + running: boolean; + cancelled: boolean; + released: boolean; + sizeBytes: number; +} + +const pendingResponseSpills = new Set(); +const pendingResponseSpillById = new Map(); +let pendingResponseSpillBytes = 0; +let responseSpillPublicationTail: Promise = Promise.resolve(); + +function deferSupersededSpill(ref: ResponseSpillRef | undefined): void { + if (!ref) return; + pendingSpillUnlinks.push(ref); + while (pendingSpillUnlinks.length > PENDING_SPILL_UNLINKS_MAX) { + deleteResponseSpill(pendingSpillUnlinks.shift()!); + } +} + +function releasePendingResponseSpill(job: PendingResponseSpill): void { + if (job.released) return; + job.released = true; + pendingResponseSpillBytes = Math.max(0, pendingResponseSpillBytes - job.sizeBytes); + pendingResponseSpills.delete(job); + if (pendingResponseSpillById.get(job.id) === job) pendingResponseSpillById.delete(job.id); + job.candidate = null; +} + +function cancelPendingResponseSpill(id: string): ResponseSpillRef | undefined { + const job = pendingResponseSpillById.get(id); + if (!job) return undefined; + pendingResponseSpillById.delete(id); + job.cancelled = true; + const superseded = job.supersededSpill; + // A queued job has not captured the candidate in an async frame yet, so release it now. + // A running job retains its accounting until settlement and will discard its stale file. + if (!job.running) releasePendingResponseSpill(job); + return superseded; +} + +function isAclTimeout(error: unknown): boolean { + return !!error && typeof error === "object" && "code" in error + && String((error as { code?: unknown }).code) === "ETIMEDOUT"; +} + +async function runPendingResponseSpill(job: PendingResponseSpill): Promise { + if (job.cancelled || !job.candidate) return; + job.running = true; + const candidate = job.candidate; + let ref: ResponseSpillRef | null = null; + try { + const state = { + createdAt: candidate.createdAt, + ...(candidate.clientThreadId ? { clientThreadId: candidate.clientThreadId } : {}), + items: candidate.items, + ...(candidate.providerOutputStart !== undefined ? { providerOutputStart: candidate.providerOutputStart } : {}), + ...(candidate.providers ? { providers: candidate.providers } : {}), + }; + try { + ref = await writeResponseSpillDurablyAsync(job.id, state); + } catch (error) { + if (!isAclTimeout(error)) throw error; + // The ACL helper permits exactly one caller-owned recovery budget. The resident generation + // remains replayable during both attempts, so a transient timeout never becomes a tombstone. + ref = await writeResponseSpillDurablyAsync(job.id, state, { retryTimedOutOnce: true }); + } + if (ref.payloadBytes > responseSpillPayloadCap()) { + deleteResponseSpill(ref); + ref = null; + if (job.directAdmission) admissionCounters.oversizedDrops += 1; + throw Object.assign(new Error("Response spill payload exceeds replay ceiling"), { code: "EFBIG" }); + } + if (states.get(job.id) !== candidate || job.cancelled) { + deleteResponseSpill(ref); + ref = null; + return; + } + if (swapResidentForSpill(job.id, candidate, ref)) { + ref = null; + spillCounters.writes += 1; + if (job.directAdmission) admissionCounters.directSpills += 1; + deferSupersededSpill(job.supersededSpill); + } + } catch { + if (ref) deleteResponseSpill(ref); + if (states.get(job.id) === candidate && !job.cancelled) { + spillCounters.writeFailures += 1; + replaceWithSpillFailure(job.id, candidate); + deferSupersededSpill(job.supersededSpill); + } + } finally { + const cancelled = job.cancelled; + releasePendingResponseSpill(job); + recomputeOldestResident(); + if (!cancelled) { + schedulePersist(); + pruneResponses(); + enforceAppOwnedMemoryBudget(); + } + } +} + +function queuePendingResponseSpill( + id: string, + candidate: ResidentResponseState, + options: { supersededSpill?: ResponseSpillRef; directAdmission?: boolean } = {}, +): void { + const inheritedSpill = cancelPendingResponseSpill(id) ?? options.supersededSpill; + if (pendingResponseSpillBytes + candidate.sizeBytes > MAX_PENDING_RESPONSE_SPILL_BYTES) { + spillCounters.writeFailures += 1; + replaceWithSpillFailure(id, candidate); + deferSupersededSpill(inheritedSpill); + return; + } + const job: PendingResponseSpill = { + id, + candidate, + ...(inheritedSpill ? { supersededSpill: inheritedSpill } : {}), + directAdmission: options.directAdmission === true, + running: false, + cancelled: false, + released: false, + sizeBytes: candidate.sizeBytes, + }; + pendingResponseSpills.add(job); + pendingResponseSpillById.set(id, job); + pendingResponseSpillBytes += job.sizeBytes; + recomputeOldestResident(); + responseSpillPublicationTail = responseSpillPublicationTail + .then(() => runPendingResponseSpill(job), () => runPendingResponseSpill(job)); +} + +function replaceWithPendingResponseSpill( + id: string, + candidate: ResidentResponseState, + expected: StoredResponseState | undefined, + options: { directAdmission?: boolean } = {}, +): boolean { + const inheritedSpill = pendingResponseSpillById.get(id)?.supersededSpill + ?? (expected?.kind === "spill" ? expected.spill : undefined); + if (!replaceMapEntry(id, candidate, expected)) return false; + queuePendingResponseSpill(id, candidate, { + ...(inheritedSpill ? { supersededSpill: inheritedSpill } : {}), + directAdmission: options.directAdmission === true, + }); + return true; +} + +/** Test-only: settle every serialized Windows spill publication. */ +export async function flushPendingResponseSpillsForTests(): Promise { + await responseSpillPublicationTail; +} + +/** Test-only: observe the bounded queue without exposing payloads. */ +export function pendingResponseSpillMetricsForTests(): { count: number; bytes: number } { + return { count: pendingResponseSpills.size, bytes: pendingResponseSpillBytes }; +} + function byteCap(): number { return byteCapOverride ?? MAX_STORED_RESPONSE_BYTES; } @@ -200,6 +373,7 @@ function recomputeOldestResident(): void { oldestResidentAt = null; for (const [id, state] of states) { if (state.kind !== "resident") continue; + if (pendingResponseSpillById.get(id)?.candidate === state) continue; if (oldestResidentAt !== null && state.createdAt >= oldestResidentAt) continue; oldestResidentId = id; oldestResidentAt = state.createdAt; @@ -248,6 +422,7 @@ function deleteOwnedSpills(entry: StoredResponseState): void { function deleteEntry(id: string, options: { deleteSpill?: boolean } = {}): void { const existing = states.get(id); if (!existing) return; + const supersededSpill = cancelPendingResponseSpill(id); storedResponseBytes -= existing.sizeBytes; if (existing.kind === "resident") { residentResponseBytes -= existing.sizeBytes; @@ -258,6 +433,7 @@ function deleteEntry(id: string, options: { deleteSpill?: boolean } = {}): void if (oldestResidentId === id) recomputeOldestResident(); stateRevision += 1; if (options.deleteSpill !== false) deleteOwnedSpills(existing); + if (options.deleteSpill !== false && supersededSpill) deleteResponseSpill(supersededSpill); } function replaceWithSpillFailure( @@ -362,11 +538,18 @@ function setResidentEntry(id: string, entry: ResidentInput): void { pruneResponses(); return; } + const pending = pendingResponseSpillById.get(id); + if (windowsSecretAclApplies() && (expected?.kind === "spill" || pending?.supersededSpill)) { + replaceWithPendingResponseSpill(id, candidate, expected); + pruneResponses(); + return; + } if (expected?.kind === "spill") { replaceSpillEntryAtomically(id, expected, candidate); pruneResponses(); return; } + if (windowsSecretAclApplies()) cancelPendingResponseSpill(id); if (!replaceMapEntry(id, candidate, expected)) return; pruneResponses(); } @@ -389,6 +572,10 @@ function admitOversizedCandidate( replaceWithSpillFailure(id, expected, { deferSpillUnlink: true }); return; } + if (windowsSecretAclApplies()) { + replaceWithPendingResponseSpill(id, candidate, expected, { directAdmission: true }); + return; + } try { const ref = writeResponseSpillDurably(id, { createdAt: candidate.createdAt, @@ -1048,7 +1235,11 @@ function pruneResponses(at = now()): void { // Unconditional RAM cap. Resident payloads demote durably; stubs/tombstones are // deleted only when even their bounded metadata cannot fit the override. while (storedResponseBytes > byteCap() && states.size > 0) { - const oldestResident = [...states].find(([, entry]) => entry.kind === "resident"); + const oldestResident = [...states].find(([id, entry]) => entry.kind === "resident" + && pendingResponseSpillById.get(id)?.candidate !== entry); + const hasPendingResident = !oldestResident && [...states].some(([id, entry]) => entry.kind === "resident" + && pendingResponseSpillById.get(id)?.candidate === entry); + if (hasPendingResident) break; const oldestId = oldestResident?.[0] ?? states.keys().next().value as string | undefined; if (!oldestId) break; const entry = states.get(oldestId)!; @@ -1056,6 +1247,10 @@ function pruneResponses(at = now()): void { deleteEntry(oldestId); continue; } + if (windowsSecretAclApplies()) { + queuePendingResponseSpill(oldestId, entry); + continue; + } try { const ref = writeResponseSpillDurably(oldestId, { createdAt: entry.createdAt, @@ -1143,11 +1338,18 @@ export function sweepAbandonedResponseStateTemps(): number { } export function responseContinuationRetainedStoreSnapshot(): RetainedStoreSnapshot { + let currentPendingBytes = 0; + for (const job of pendingResponseSpills) { + if (job.candidate && states.get(job.id) === job.candidate) currentPendingBytes += job.sizeBytes; + } + const detachedPendingBytes = Math.max(0, pendingResponseSpillBytes - currentPendingBytes); + const bytes = storedResponseBytes + detachedPendingBytes; + const evictableBytes = Math.max(0, residentResponseBytes - currentPendingBytes); return { count: states.size, - bytes: storedResponseBytes, - evictableBytes: residentResponseBytes, - pinnedBytes: Math.max(0, storedResponseBytes - residentResponseBytes), + bytes, + evictableBytes, + pinnedBytes: Math.max(0, bytes - evictableBytes), oldestAt: oldestResidentAt, }; } @@ -1157,6 +1359,11 @@ export function evictOldestResponseContinuationForBudget(): number { const id = oldestResidentId; const entry = states.get(id); if (!entry || entry.kind !== "resident") return 0; + if (windowsSecretAclApplies()) { + queuePendingResponseSpill(id, entry); + schedulePersist(); + return 0; + } try { const ref = writeResponseSpillDurably(id, { createdAt: entry.createdAt, @@ -1380,7 +1587,7 @@ export function responseStateMetrics(): ResponseStateMetrics { residentCount, spillStubCount, tombstoneCount, - totalBytes: storedResponseBytes, + totalBytes: responseContinuationRetainedStoreSnapshot().bytes, spillPayloadBytes, largestBytes, oldestAgeMs: states.size > 0 ? at - oldestCreatedAt : 0, @@ -1492,6 +1699,8 @@ export function clearResponseStateMemoryForTests(): void { persistTimer = null; } pendingPersistPath = null; + for (const id of [...pendingResponseSpillById.keys()]) cancelPendingResponseSpill(id); + pendingResponseSpillById.clear(); states.clear(); storedResponseBytes = 0; residentResponseBytes = 0; diff --git a/structure/02_config-and-codex-home.md b/structure/02_config-and-codex-home.md index 3685fb5bb4..419dddc9de 100644 --- a/structure/02_config-and-codex-home.md +++ b/structure/02_config-and-codex-home.md @@ -173,6 +173,26 @@ are consumed incrementally and at most 512 stale files are attempted per process - 다른 대안 대신 이 방식을 선택한 이유: It repairs known remnants without broad authority over unrelated temp files or active writers. - 장점, 단점 및 영향: Old dead-PID files are reclaimed automatically; locked or conservatively classified files remain for a later retry. +Windows runtime response spills never wait on `icacls` through `Bun.spawnSync`. Linux and macOS +retain the immediate synchronous publication path. On Windows, the resident continuation enters one +serialized publication queue and remains replayable while `hardenSecretDirAsync` and +`hardenSecretPathAsync` run. Publication installs a spill stub only when the map still contains the +same resident object; a superseded job deletes its newly published file instead of overwriting newer +state. Pending payloads are pinned and capped at 256 MiB, so an ACL outage cannot grow an unbounded +queue or be misreported as evictable memory. One caller-owned retry is allowed after a real +`ETIMEDOUT`; the first timeout does not install a `spill-failed` tombstone. Required ACL failures +remain fail-closed after that bounded recovery. Optional config-directory hardening uses a separate +per-directory async single-flight, while required config mutation writers retain their existing +awaited or synchronous fail-closed boundary. + +[Decision Log] +- 목적과 의도: Keep `/healthz` and unrelated requests responsive during intermittent Windows ACL stalls without publishing an unhardened continuation. +- 기존 구현 및 제약 조건: Response demotion called the synchronous spill writer from request-time state mutations; `Bun.spawnSync(icacls)` could block the only Bun event loop for the full timeout and immediately replace replayable state with a tombstone. +- 검토한 주요 대안: Increase the ACL timeout, weaken required ACL checks, publish before hardening, move every platform to async state mutation, or isolate only the Windows ACL-dependent publication boundary. +- 선택한 방식: Preserve non-Windows behavior; serialize Windows publications through async ACL APIs, retain the exact resident generation until compare-before-swap succeeds, cap pending bytes, and retry one proven timeout. +- 다른 대안 대신 이 방식을 선택한 이유: Longer waits worsen liveness, early publication weakens secret-file ACLs, and a cross-platform async rewrite would disturb mature immediate memory and crash-ordering contracts that do not cause this incident. +- 장점, 단점 및 영향: Windows health stays schedulable and transient ACL stalls retain continuation replay; pending payloads can temporarily exceed the 64 MiB resident target but are pinned under a 256 MiB local ceiling and remain inside the documented 512 MiB process-owned worst case. + ## Config surface ### OpenCodex home and live process state diff --git a/tests/config.test.ts b/tests/config.test.ts index 6f0416b35f..62d3f6d782 100644 --- a/tests/config.test.ts +++ b/tests/config.test.ts @@ -35,6 +35,7 @@ import * as windowsAcl from "../src/lib/windows-secret-acl"; import { setTrustedWindowsSystemDirectoryResolverForTests } from "../src/lib/windows-elevation"; import { AtomicWriteResidualTempError, atomicWriteFile, atomicWriteFileAsync, hardenConfigDir, hardenExistingSecret, renameAtomicFile, saveConfig } from "../src/config"; import { nextAtomicTempSequence } from "../src/config/atomic-write"; +import { flushConfigDirHardeningForTests } from "../src/config/paths"; import { providerManagementConfigError } from "../src/server/auth-cors"; let testDir = ""; @@ -2613,25 +2614,37 @@ describe("config.ts – Windows ACL hardening integration", () => { } }); - test("hardenConfigDir delegates to hardenSecretDir with required:false on win32", () => { + test("hardenConfigDir delegates to one async optional flight on win32", async () => { const origPlatform = process.platform; Object.defineProperty(process, "platform", { value: "win32", configurable: true }); try { - const spy = spyOn(windowsAcl, "hardenSecretDir").mockReturnValue({ ok: true }); + let release!: () => void; + const pending = new Promise(resolve => { release = resolve; }); + const spy = spyOn(windowsAcl, "hardenSecretDirAsync").mockImplementation(async () => { + await pending; + return { ok: true }; + }); mkdirSync(testDir, { recursive: true }); hardenConfigDir(); - expect(spy).toHaveBeenCalledWith(testDir, { required: false }); - spy.mockRestore(); + hardenConfigDir(); + try { + expect(spy).toHaveBeenCalledTimes(1); + expect(spy).toHaveBeenCalledWith(testDir, { required: false }); + } finally { + release(); + await flushConfigDirHardeningForTests(); + spy.mockRestore(); + } } finally { Object.defineProperty(process, "platform", { value: origPlatform, configurable: true }); } }); - test("hardenConfigDir does not call hardenSecretDir on non-Windows", () => { + test("hardenConfigDir does not call async ACL hardening on non-Windows", () => { const origPlatform = process.platform; Object.defineProperty(process, "platform", { value: "linux", configurable: true }); try { - const spy = spyOn(windowsAcl, "hardenSecretDir"); + const spy = spyOn(windowsAcl, "hardenSecretDirAsync"); mkdirSync(testDir, { recursive: true }); hardenConfigDir(); expect(spy).not.toHaveBeenCalled(); diff --git a/tests/responses-state.test.ts b/tests/responses-state.test.ts index aa05690e38..b5fca3fb5d 100644 --- a/tests/responses-state.test.ts +++ b/tests/responses-state.test.ts @@ -46,6 +46,8 @@ import { setResponseStateByteCapForTests, setResponseStatePersistAttemptHookForTests, getStoredResponseBytesForTests, + flushPendingResponseSpillsForTests, + pendingResponseSpillMetricsForTests, } from "../src/responses/state"; import { readResponseSpill, @@ -80,7 +82,9 @@ import { hardenSecretPath, hardenedSecretPathCountForTests, resetHardenedStateForTests, + setAsyncIcaclsRunnerForTests, setIcaclsRunnerForTests, + setNowForTests, setPlatformForTests, timedOutSecretPathCountForTests, } from "../src/lib/windows-secret-acl"; @@ -168,9 +172,12 @@ describe("Responses previous_response_id state", () => { afterEach(() => { setSpillIoForTest(null); + setAsyncIcaclsRunnerForTests(null); setIcaclsRunnerForTests(null); + setNowForTests(null); setPlatformForTests(null); resetHardenedStateForTests(); + delete process.env.OPENCODEX_ACL_TIMEOUT_MS; setResponseStateByteCapForTests(null); clearResponseStateForTests(); rmSync(home, { recursive: true, force: true }); @@ -710,6 +717,107 @@ describe("Responses previous_response_id state", () => { expect(events).toEqual(["write", "fsync", "close", "harden", "publish", "dir-fsync", "stub-swap"]); }); + test("Windows spill ACL hardening yields the event loop and swaps only after publication", async () => { + setPlatformForTests("win32"); + let release!: () => void; + let entered!: () => void; + const gate = new Promise(resolve => { release = resolve; }); + const started = new Promise(resolve => { entered = resolve; }); + let announced = false; + setAsyncIcaclsRunnerForTests(async () => { + if (!announced) { + announced = true; + entered(); + } + await gate; + return { success: true, exitCode: 0, timedOut: false, stdout: "" }; + }); + setResponseStateByteCapForTests(1_024); + + rememberLarge("resp_async_acl", "x".repeat(8_000)); + await started; + try { + let unrelatedTickRan = false; + setTimeout(() => { unrelatedTickRan = true; }, 0); + await new Promise(resolve => setTimeout(resolve, 0)); + + expect(unrelatedTickRan).toBe(true); + expect(pendingResponseSpillMetricsForTests()).toMatchObject({ count: 1 }); + expect(responseStateMetrics()).toMatchObject({ residentCount: 1, spillStubCount: 0, spillWriteFailures: 0 }); + expect(JSON.stringify(expandPreviousResponseInput({ previous_response_id: "resp_async_acl", input: "next" }))) + .toContain("xxxxxxxx"); + } finally { + release(); + } + await flushPendingResponseSpillsForTests(); + expect(pendingResponseSpillMetricsForTests()).toEqual({ count: 0, bytes: 0 }); + expect(responseStateMetrics()).toMatchObject({ residentCount: 0, spillStubCount: 1, spillWrites: 1, spillWriteFailures: 0 }); + }); + + test("Windows spill retries one transient ACL timeout without installing a tombstone", async () => { + setPlatformForTests("win32"); + process.env.OPENCODEX_ACL_TIMEOUT_MS = "1000"; + let clock = 0; + let grantCalls = 0; + setNowForTests(() => clock); + setAsyncIcaclsRunnerForTests(async args => { + if (args.includes("/grant:r")) { + grantCalls += 1; + if (grantCalls === 1) { + clock = 1_000; + return { success: false, exitCode: null, timedOut: true, stdout: "" }; + } + } + return { success: true, exitCode: 0, timedOut: false, stdout: "" }; + }); + setResponseStateByteCapForTests(1_024); + + rememberLarge("resp_async_acl_retry", "r".repeat(8_000)); + await flushPendingResponseSpillsForTests(); + + expect(grantCalls).toBeGreaterThanOrEqual(2); + expect(responseStateMetrics()).toMatchObject({ + residentCount: 0, + spillStubCount: 1, + tombstoneCount: 0, + spillWrites: 1, + spillWriteFailures: 0, + }); + }); + + test("Windows pending spill publication cannot overwrite a newer same-id generation", async () => { + setPlatformForTests("win32"); + let release!: () => void; + let entered!: () => void; + const gate = new Promise(resolve => { release = resolve; }); + const started = new Promise(resolve => { entered = resolve; }); + let announced = false; + setAsyncIcaclsRunnerForTests(async () => { + if (!announced) { + announced = true; + entered(); + } + await gate; + return { success: true, exitCode: 0, timedOut: false, stdout: "" }; + }); + setResponseStateByteCapForTests(1_024); + + rememberLarge("resp_async_replace", `old-${"a".repeat(8_000)}`); + await started; + rememberLarge("resp_async_replace", `new-${"b".repeat(8_000)}`); + release(); + await flushPendingResponseSpillsForTests(); + + const replay = JSON.stringify(expandPreviousResponseInput({ + previous_response_id: "resp_async_replace", + input: "next", + })); + expect(replay).toContain("new-bbbbbbbb"); + expect(replay).not.toContain("old-aaaaaaaa"); + expect(spillFileNames(home)).toHaveLength(1); + expect(responseStateMetrics()).toMatchObject({ spillStubCount: 1, spillWriteFailures: 0 }); + }); + test("directory fsync follows spill unlink", () => { const ref = writeResponseSpillDurably("resp_unlink_order", { createdAt: Date.now(), items: ["x"] }); const events: string[] = [];