Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
86 changes: 85 additions & 1 deletion src/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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<void>;
harden: (path: string) => void | Promise<void>;
rename: (source: string, destination: string) => void | Promise<void>;
truncate: (path: string) => void | Promise<void>;
unlink: (path: string) => void | Promise<void>;
}

async function renameAtomicFileAsync(source: string, destination: string): Promise<void> {
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<void> {
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"; }
}
Expand Down
177 changes: 173 additions & 4 deletions src/lib/windows-secret-acl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/

Expand All @@ -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;
}

/**
Expand Down Expand Up @@ -72,6 +83,7 @@ export interface IcaclsResult {
}

type IcaclsRunner = (args: string[], timeoutMs: number) => IcaclsResult;
type AsyncIcaclsRunner = (args: string[], timeoutMs: number) => Promise<IcaclsResult>;

function defaultIcaclsRunner(args: string[], timeoutMs: number): IcaclsResult {
// Bun.spawnSync with windowsHide: Node execFileSync has hung under the GUI/proxy even
Expand All @@ -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<IcaclsResult> {
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;

Expand All @@ -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;
Expand Down Expand Up @@ -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) {
Expand All @@ -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.
Expand All @@ -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<void> {
const user = currentWindowsUser();
if (!user) {
throw new Error("Cannot determine current Windows user for ACL hardening");
}

const run = async (step: string, args: string[]): Promise<IcaclsResult> => {
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<void> => {
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
Expand Down Expand Up @@ -254,6 +345,27 @@ function describeAclStateAfterTimeout(targetPath: string, deadline: number): str
}
}

async function describeAclStateAfterTimeoutAsync(targetPath: string, deadline: number): Promise<string> {
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.
Expand All @@ -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" };
}

Expand All @@ -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.
Expand All @@ -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<string>,
): Promise<HardenResult> {
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).
Expand All @@ -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<HardenResult> {
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).
Expand All @@ -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<HardenResult> {
return hardenEntryAsync(targetPath, true, opts, hardenedDirectories);
}
Loading
Loading