From 2063cc10c9ea114e6e5e9cdffe1b7eadfba57fb8 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Sat, 29 Aug 2026 05:13:21 +0900 Subject: [PATCH 1/2] fix(codex): refresh native main credentials safely Co-authored-by: MarcTCruz <58499846+MarcTCruz@users.noreply.github.com> --- src/codex/account-store.ts | 2 +- src/codex/account-usability.ts | 15 +- src/codex/auth-context.ts | 77 ++++++- src/codex/main-account.ts | 225 +++++++++++++++++++- src/codex/model-entitlements.ts | 21 +- src/codex/routing.ts | 8 +- src/config/atomic-write.ts | 7 +- src/lib/test-home-guard.ts | 21 +- src/oauth/chatgpt.ts | 6 +- src/server/responses/codex-auth-error.ts | 26 +++ src/server/responses/compact.ts | 114 +++++++++- src/server/responses/core.ts | 140 +++++++++++- tests/codex-auth-context.test.ts | 34 ++- tests/codex-main-account-refresh.test.ts | 105 +++++++++ tests/responses-native-main-refresh.test.ts | 162 ++++++++++++++ tests/test-home-guard.test.ts | 22 +- 16 files changed, 949 insertions(+), 36 deletions(-) create mode 100644 tests/codex-main-account-refresh.test.ts create mode 100644 tests/responses-native-main-refresh.test.ts diff --git a/src/codex/account-store.ts b/src/codex/account-store.ts index 85eaab3ef8..32546f91a1 100644 --- a/src/codex/account-store.ts +++ b/src/codex/account-store.ts @@ -331,7 +331,7 @@ function isRefreshLockStale(path: string): boolean { } } -async function withCodexRefreshFileLock(lockKey: string, signal: AbortSignal, fn: () => Promise): Promise { +export async function withCodexRefreshFileLock(lockKey: string, signal: AbortSignal, fn: () => Promise): Promise { hardenConfigDir(); const dir = getConfigDir(); if (!existsSync(dir)) mkdirSync(dir, { recursive: true, mode: 0o700 }); diff --git a/src/codex/account-usability.ts b/src/codex/account-usability.ts index d508e19f4d..c2565a41aa 100644 --- a/src/codex/account-usability.ts +++ b/src/codex/account-usability.ts @@ -1,6 +1,11 @@ import { getCodexAccountCredential } from "./account-store"; import { isAccountNeedsReauth } from "./account-runtime-state"; -import { MAIN_CODEX_ACCOUNT_ID, isMainAccountTokenLive } from "./main-account"; +import { + MAIN_CODEX_ACCOUNT_ID, + hasMainAccountRefreshGrant, + isMainAccountCredentialUsable, + isMainAccountTokenLive, +} from "./main-account"; import { hasLegacyMainCodexPoolAccount, isSelectableCodexPoolAccount } from "./account-id"; import type { OcxConfig } from "../types"; import { isNativeMainTrafficBlocked } from "./native-profile-startup"; @@ -27,13 +32,15 @@ export function isCodexAccountUsable( // A legacy pool row with the sentinel makes an active `__main__` ambiguous. // Fail closed until the authenticated compatibility-delete path removes it. if (hasLegacyMainCodexPoolAccount(config.codexAccounts)) return false; - if (isAccountNeedsReauth(accountId)) return false; + if (isAccountNeedsReauth(accountId) && !hasMainAccountRefreshGrant()) return false; // A selection-only caller owns the recovery/drain fence and will reject main // before reservation or token materialization. Treat cached main as a routing // candidate without touching the credential file so affinity is not rebound. if (options.nativeMainSelectionOnly) return true; - // Main account: credential is the read-only ~/.codex/auth.json token (Option A). - return (options.isMainAccountTokenLive ?? isMainAccountTokenLive)(); + // Main account: a refresh grant is enough to route; materialization refreshes before I/O. + return options.isMainAccountTokenLive + ? options.isMainAccountTokenLive() + : isMainAccountCredentialUsable(); } const exists = (config.codexAccounts ?? []) .some(account => isSelectableCodexPoolAccount(account) && account.id === accountId); diff --git a/src/codex/auth-context.ts b/src/codex/auth-context.ts index 5f6a54f6d8..812cadd22f 100644 --- a/src/codex/auth-context.ts +++ b/src/codex/auth-context.ts @@ -12,7 +12,15 @@ import { isCodexAccountPaused } from "./account-pause"; import { ConfigMutationLockError } from "../config"; import { isCodexAccountUsable } from "./account-usability"; import { reconcileMainCodexAccountRuntimeState } from "./account-lifecycle"; -import { MAIN_CODEX_ACCOUNT_ID, getMainAccountToken, isMainAccountTokenLive } from "./main-account"; +import { + MAIN_CODEX_ACCOUNT_ID, + MainAccountTokenRefreshError, + MainAuthJsonChangedDuringRefreshError, + getMainAccountToken, + getValidMainAccountToken, + isMainAccountTokenLive, + type NativeMainRefreshDependencies, +} from "./main-account"; import { isNativeMainTrafficBlocked, nativeMainStartupGateSnapshot } from "./native-profile-startup"; import type { NativeMainStartupBlockReason } from "./native-profile-startup"; import { @@ -318,6 +326,8 @@ export function shouldMarkAccountNeedsReauthForCodexAuthFailure(cause: unknown): && !(cause instanceof CodexCredentialRefreshLockTimeoutError) && !(cause instanceof CodexCredentialRefreshBusyError) && !(cause instanceof CodexCredentialRefreshStaleError) + && !(cause instanceof MainAuthJsonChangedDuringRefreshError) + && !(cause instanceof MainAccountTokenRefreshError && cause.reason === "transient") && !(cause instanceof ConfigMutationLockError); } @@ -332,6 +342,9 @@ export interface ResolveCodexAuthContextOptions { /** Test-only native credential read seams. */ isMainAccountTokenLive?: () => boolean; getMainAccountToken?: typeof getMainAccountToken; + getValidMainAccountToken?: typeof getValidMainAccountToken; + nativeMainRefreshDependencies?: NativeMainRefreshDependencies; + signal?: AbortSignal; primeCodexPoolQuotas?: (config: OcxConfig, reason: string) => Promise; /** Test seam for account-gated native model discovery. */ resolveCodexModelEntitlements?: typeof resolveCodexModelEntitlements; @@ -395,7 +408,10 @@ export async function resolveCodexAuthContext( } if (options.modelId && ACCOUNT_GATED_NATIVE_OPENAI_MODELS.has(options.modelId)) { const entitled = entitledCodexAccountIdsForModel( - await (options.resolveCodexModelEntitlements ?? resolveCodexModelEntitlements)(config), + await (options.resolveCodexModelEntitlements ?? resolveCodexModelEntitlements)(config, { + signal: options.signal, + nativeMainRefreshDependencies: options.nativeMainRefreshDependencies, + }), options.modelId, )?.has(MAIN_CODEX_ACCOUNT_ID) === true; if (!entitled) { @@ -424,10 +440,13 @@ export async function resolveCodexAuthContext( ? codexPoolAffinityKey(headers) : undefined; // Retained startup recovery makes the physical main identity ineligible. Routing - // can still preserve service by selecting a healthy configured pool account. + // can still preserve service by selecting a healthy configured pool account. A + // request-owned bearer likewise cannot inspect or reconcile file-main state. const nativeMainTrafficBlocked = isNativeMainTrafficBlocked(); const selectionAdmission = options.beginCodexAccountSelection?.(); - const nativeMainReadsForbidden = nativeMainTrafficBlocked || selectionAdmission?.mainProfileDraining === true; + const nativeMainReadsForbidden = requestScopedMainCredential + || nativeMainTrafficBlocked + || selectionAdmission?.mainProfileDraining === true; const nativeMainSelectionOnly = !nativeMainTrafficBlocked && selectionAdmission?.mainProfileDraining === true; let accountId: string; @@ -437,7 +456,11 @@ export async function resolveCodexAuthContext( ? new Set([MAIN_CODEX_ACCOUNT_ID]) : undefined; const entitlementSnapshot = options.modelId && ACCOUNT_GATED_NATIVE_OPENAI_MODELS.has(options.modelId) - ? await (options.resolveCodexModelEntitlements ?? resolveCodexModelEntitlements)(config, { excludeAccountIds }) + ? await (options.resolveCodexModelEntitlements ?? resolveCodexModelEntitlements)(config, { + excludeAccountIds, + signal: options.signal, + nativeMainRefreshDependencies: options.nativeMainRefreshDependencies, + }) : undefined; const entitledAccountIds = entitlementSnapshot ? entitledCodexAccountIdsForModel(entitlementSnapshot, options.modelId) @@ -582,8 +605,21 @@ export async function resolveCodexAuthContext( } if (accountId === MAIN_CODEX_ACCOUNT_ID) { - // Main account in rotation: inject the read-only auth.json token and fail closed if it vanished. - const token = (options.getMainAccountToken ?? getMainAccountToken)(); + // Main account in rotation: refresh auth.json before upstream I/O and fail closed if it vanished. + let token: { accessToken: string; chatgptAccountId: string } | null; + try { + token = await (options.getValidMainAccountToken ?? getValidMainAccountToken)({ + signal: options.signal, + ...(options.nativeMainRefreshDependencies ?? {}), + }); + } catch (cause) { + if (probeLeaseId && probeQuotaScope) releaseCodexQuotaScopeProbeLease(accountId, probeQuotaScope, probeLeaseId); + else if (probeLeaseId) releaseCodexQuotaProbeLease(accountId, probeLeaseId); + if (shouldMarkAccountNeedsReauthForCodexAuthFailure(cause)) { + markAccountNeedsReauth(accountId, writerGeneration); + } + throw new CodexAuthContextError(accountId, cause); + } if (!token) { // Nothing will reach upstream, so give the probe back instead of burning it. if (probeLeaseId && probeQuotaScope) releaseCodexQuotaScopeProbeLease(accountId, probeQuotaScope, probeLeaseId); @@ -712,6 +748,33 @@ export function materializeCodexUpstreamAuth( return selected; } +export async function materializeCodexUpstreamAuthAsync( + headers: Headers, + ctx: CodexAuthContext, + options: { + substituteMainCredential?: boolean; + signal?: AbortSignal; + nativeMainRefreshDependencies?: NativeMainRefreshDependencies; + } = {}, +): Promise { + if (ctx.kind !== "main" || options.substituteMainCredential !== true) { + return materializeCodexUpstreamAuth(headers, ctx, options); + } + const selected = new Headers(); + for (const name of FORWARD_HEADERS) { + const value = headers.get(name); + if (value) selected.set(name, value); + } + const stored = await getValidMainAccountToken({ + signal: options.signal, + ...(options.nativeMainRefreshDependencies ?? {}), + }); + if (!stored?.accessToken) throw new CodexMainSubstitutionUnavailableError(); + selected.set("authorization", `Bearer ${stored.accessToken}`); + if (stored.chatgptAccountId) selected.set("chatgpt-account-id", stored.chatgptAccountId); + return selected; +} + /** @deprecated Prefer materializeCodexUpstreamAuth; kept for call sites without admission context. */ export function headersForCodexAuthContext(headers: Headers, ctx: CodexAuthContext): Headers { return materializeCodexUpstreamAuth(headers, ctx); diff --git a/src/codex/main-account.ts b/src/codex/main-account.ts index 30296b586a..b8604176cb 100644 --- a/src/codex/main-account.ts +++ b/src/codex/main-account.ts @@ -1,7 +1,23 @@ +import { createHash } from "node:crypto"; +import { readFileSync } from "node:fs"; +import { join } from "node:path"; import { readCodexTokens } from "./auth-collision"; -import { decodeJwtPayload } from "../oauth/chatgpt"; +import { + decodeJwtPayload, + extractAccountId, + refreshChatGPTToken, +} from "../oauth/chatgpt"; +import type { OAuthCredentials } from "../oauth/types"; import { extractChatgptPlanType } from "./plan"; import { MAIN_CODEX_ACCOUNT_ID } from "./account-id"; +import { + refreshGrantFingerprintForToken, + withCodexRefreshFileLock, +} from "./account-store"; +import { atomicWriteFile, resolveWriteTarget } from "../config/atomic-write"; +import { resolveCodexHomeDir } from "./home"; +import { assertNotRealCodexHomeUnderTest } from "../lib/test-home-guard"; +import { clearAccountNeedsReauth } from "./account-runtime-state"; export { MAIN_CODEX_ACCOUNT_ID } from "./account-id"; @@ -12,6 +28,213 @@ export { MAIN_CODEX_ACCOUNT_ID } from "./account-id"; */ let mainAccountPlan: string | null = null; let jwtPlanAttempted = false; +const MAIN_TOKEN_REFRESH_SKEW_MS = 60_000; +let beforeMainAuthJsonRenameForTests: (() => void) | null = null; + +type MainAuthJsonCredential = { + path: string; + rawSha256: string; + root: Record; + tokens: Record; + accessToken?: string; + refreshToken?: string; + chatgptAccountId: string; +}; + +export interface NativeMainRefreshDependencies { + refreshToken?: (refreshToken: string, options: { signal: AbortSignal }) => Promise; + signal?: AbortSignal; +} + +export class MainAuthJsonChangedDuringRefreshError extends Error { + constructor() { + super("Codex auth.json changed while its token was refreshing"); + this.name = "MainAuthJsonChangedDuringRefreshError"; + } +} + +export class MainAccountTokenRefreshError extends Error { + constructor(readonly reason: "reauth" | "transient", options?: ErrorOptions) { + super(reason === "reauth" + ? "Codex main account needs reauthentication" + : "Codex main token refresh did not complete", options); + this.name = "MainAccountTokenRefreshError"; + } +} + +function nonEmptyString(value: unknown): string | undefined { + return typeof value === "string" && value.trim() ? value : undefined; +} + +function sha256(value: string): string { + return createHash("sha256").update(value).digest("hex"); +} + +function readMainAuthJsonCredential(): MainAuthJsonCredential | null { + const path = resolveWriteTarget(join(resolveCodexHomeDir(), "auth.json")); + let raw: string; + try { + raw = readFileSync(path, "utf8"); + } catch { + return null; + } + try { + const parsed = JSON.parse(raw) as unknown; + if (!parsed || typeof parsed !== "object" || Array.isArray(parsed)) return null; + const root = parsed as Record; + const tokenValue = root.tokens; + if (!tokenValue || typeof tokenValue !== "object" || Array.isArray(tokenValue)) return null; + const tokens = tokenValue as Record; + const accessToken = nonEmptyString(tokens.access_token); + const refreshToken = nonEmptyString(tokens.refresh_token); + if (!accessToken && !refreshToken) return null; + const idToken = nonEmptyString(tokens.id_token); + const chatgptAccountId = extractAccountId(idToken, accessToken) + ?? nonEmptyString(tokens.account_id) + ?? ""; + return { + path, + rawSha256: sha256(raw), + root, + tokens, + ...(accessToken ? { accessToken } : {}), + ...(refreshToken ? { refreshToken } : {}), + chatgptAccountId, + }; + } catch { + return null; + } +} + +function mainAccessTokenFresh(accessToken: string | undefined, now: number, skewMs: number): boolean { + if (!accessToken) return false; + const payload = decodeJwtPayload(accessToken); + const exp = typeof payload?.exp === "number" ? payload.exp * 1000 : undefined; + return exp === undefined || exp > now + skewMs; +} + +/** A refresh grant makes native main routeable even when the current access token is expired. */ +export function isMainAccountCredentialUsable(now = Date.now()): boolean { + const current = readMainAuthJsonCredential(); + return !!current?.refreshToken || mainAccessTokenFresh(current?.accessToken, now, 0); +} + +export function hasMainAccountRefreshGrant(): boolean { + return !!readMainAuthJsonCredential()?.refreshToken; +} + +function assertMainAuthJsonSnapshotUnchanged(expected: MainAuthJsonCredential): void { + const current = readMainAuthJsonCredential(); + if (!current || current.path !== expected.path || current.rawSha256 !== expected.rawSha256) { + throw new MainAuthJsonChangedDuringRefreshError(); + } +} + +function persistRefreshedMainAuthJson( + expected: MainAuthJsonCredential, + refreshed: OAuthCredentials, +): { accessToken: string; chatgptAccountId: string } { + assertNotRealCodexHomeUnderTest(resolveCodexHomeDir()); + const accessToken = refreshed.access; + const refreshToken = refreshed.refresh || expected.refreshToken!; + const chatgptAccountId = refreshed.accountId + ?? extractAccountId(undefined, accessToken) + ?? expected.chatgptAccountId; + const tokens = { + ...expected.tokens, + access_token: accessToken, + refresh_token: refreshToken, + account_id: chatgptAccountId, + }; + atomicWriteFile( + expected.path, + JSON.stringify({ ...expected.root, tokens }, null, 2) + "\n", + undefined, + { + beforeRename: () => { + const hook = beforeMainAuthJsonRenameForTests; + beforeMainAuthJsonRenameForTests = null; + hook?.(); + assertMainAuthJsonSnapshotUnchanged(expected); + }, + }, + ); + return { accessToken, chatgptAccountId }; +} + +export function setMainAuthJsonBeforeRenameHookForTests(hook: (() => void) | null): void { + beforeMainAuthJsonRenameForTests = hook; +} + +async function resolveMainAccountToken( + dependencies: NativeMainRefreshDependencies = {}, + rejectedAccessToken?: string, +): Promise<{ accessToken: string; chatgptAccountId: string } | null> { + const initial = readMainAuthJsonCredential(); + if (!initial) return null; + const now = Date.now(); + if (initial.accessToken !== rejectedAccessToken + && mainAccessTokenFresh(initial.accessToken, now, MAIN_TOKEN_REFRESH_SKEW_MS)) { + return { accessToken: initial.accessToken!, chatgptAccountId: initial.chatgptAccountId }; + } + if (!initial.refreshToken) { + return initial.accessToken !== rejectedAccessToken + && mainAccessTokenFresh(initial.accessToken, now, 0) + ? { accessToken: initial.accessToken!, chatgptAccountId: initial.chatgptAccountId } + : null; + } + + const signal = dependencies.signal + ? AbortSignal.any([dependencies.signal, AbortSignal.timeout(30_000)]) + : AbortSignal.timeout(30_000); + const lockKey = refreshGrantFingerprintForToken(initial.refreshToken); + return withCodexRefreshFileLock(lockKey, signal, async () => { + const locked = readMainAuthJsonCredential(); + if (!locked) throw new MainAuthJsonChangedDuringRefreshError(); + if (!locked.refreshToken + || refreshGrantFingerprintForToken(locked.refreshToken) !== lockKey) { + if (locked.accessToken !== rejectedAccessToken + && mainAccessTokenFresh(locked.accessToken, Date.now(), 0)) { + return { accessToken: locked.accessToken!, chatgptAccountId: locked.chatgptAccountId }; + } + throw new MainAuthJsonChangedDuringRefreshError(); + } + if (locked.accessToken !== rejectedAccessToken + && mainAccessTokenFresh(locked.accessToken, Date.now(), MAIN_TOKEN_REFRESH_SKEW_MS)) { + return { accessToken: locked.accessToken!, chatgptAccountId: locked.chatgptAccountId }; + } + const refresh = dependencies.refreshToken + ?? ((refreshToken: string, options: { signal: AbortSignal }) => refreshChatGPTToken(refreshToken, options)); + let refreshed: OAuthCredentials; + try { + refreshed = await refresh(locked.refreshToken, { signal }); + } catch (cause) { + const message = cause instanceof Error ? cause.message.toLowerCase() : ""; + const reason = /invalid_grant|invalidated|revoked|expired/.test(message) + ? "reauth" as const + : "transient" as const; + throw new MainAccountTokenRefreshError(reason, { cause }); + } + const result = persistRefreshedMainAuthJson(locked, refreshed); + clearAccountNeedsReauth(MAIN_CODEX_ACCOUNT_ID); + return result; + }); +} + +/** Refresh the CLI-owned native credential before upstream I/O and publish it atomically. */ +export function getValidMainAccountToken( + dependencies: NativeMainRefreshDependencies = {}, +): Promise<{ accessToken: string; chatgptAccountId: string } | null> { + return resolveMainAccountToken(dependencies); +} + +/** Force refresh after upstream rejected this exact bearer once. */ +export function forceRefreshMainAccountToken( + rejectedAccessToken: string, + dependencies: NativeMainRefreshDependencies = {}, +): Promise<{ accessToken: string; chatgptAccountId: string } | null> { + return resolveMainAccountToken(dependencies, rejectedAccessToken); +} export function setMainAccountPlan(plan: string | null): void { mainAccountPlan = plan; diff --git a/src/codex/model-entitlements.ts b/src/codex/model-entitlements.ts index 5a649d335a..d6744ac030 100644 --- a/src/codex/model-entitlements.ts +++ b/src/codex/model-entitlements.ts @@ -3,7 +3,12 @@ import { readBoundedResponseBody } from "../lib/bounded-body"; import type { OcxConfig } from "../types"; import { isSelectableCodexPoolAccount } from "./account-id"; import { getValidCodexToken, readCodexAccountRecord } from "./account-store"; -import { getMainAccountToken, MAIN_CODEX_ACCOUNT_ID } from "./main-account"; +import { + getMainAccountToken, + getValidMainAccountToken, + MAIN_CODEX_ACCOUNT_ID, + type NativeMainRefreshDependencies, +} from "./main-account"; import { ACCOUNT_GATED_NATIVE_OPENAI_MODELS } from "./catalog/native-models"; const CODEX_MODELS_URL = "https://chatgpt.com/backend-api/codex/models?client_version=0.0.0"; @@ -37,7 +42,9 @@ export interface CodexModelEntitlementSnapshot { export interface CodexModelEntitlementResolveOptions { readonly fetcher?: typeof fetch; + readonly nativeMainRefreshDependencies?: NativeMainRefreshDependencies; readonly now?: number; + readonly signal?: AbortSignal; /** Test-only credential seam; production callers enumerate local main + Pool credentials. */ readonly credentials?: readonly CodexModelEntitlementCredentialSnapshot[]; /** Test-only seam for proving lifecycle exclusions happen before credential reads. */ @@ -92,9 +99,15 @@ function currentCredentialIdentity(accountId: string): string | undefined { return `pool:${record.generation}:${record.credential.chatgptAccountId}`; } -async function accountCredentialSnapshot(accountId: string): Promise { +async function accountCredentialSnapshot( + accountId: string, + options: Pick = {}, +): Promise { if (accountId === MAIN_CODEX_ACCOUNT_ID) { - const token = getMainAccountToken(); + const token = await getValidMainAccountToken({ + signal: options.signal, + ...(options.nativeMainRefreshDependencies ?? {}), + }); return token ? { accountId, @@ -261,7 +274,7 @@ export async function resolveCodexModelEntitlements( const credentialSnapshot = options.credentialSnapshot ?? accountCredentialSnapshot; const credentials = options.credentials ? [...options.credentials].filter(credential => !options.excludeAccountIds?.has(credential.accountId)) - : (await Promise.all(allowedAccountIds.map(credentialSnapshot))) + : (await Promise.all(allowedAccountIds.map(accountId => credentialSnapshot(accountId, options)))) .filter((value): value is CodexModelEntitlementCredentialSnapshot => value !== null); const results = await Promise.all(credentials.map(async credential => ({ credential, diff --git a/src/codex/routing.ts b/src/codex/routing.ts index b1d5a26b01..d38a40645f 100644 --- a/src/codex/routing.ts +++ b/src/codex/routing.ts @@ -19,7 +19,11 @@ import { } from "./pool-rotation"; import { CODEX_UNKNOWN_USAGE_SCORE, getAccountQuota } from "./quota"; import { isThirtyDayOnlyCodexPlan } from "./plan"; -import { MAIN_CODEX_ACCOUNT_ID, getMainAccountPlan } from "./main-account"; +import { + MAIN_CODEX_ACCOUNT_ID, + getMainAccountPlan, + hasMainAccountRefreshGrant, +} from "./main-account"; import { isSelectableCodexPoolAccount } from "./account-id"; import type { OcxConfig } from "../types"; import { captureConfigGeneration, type GenerationContext } from "../lib/state-store-sweeper"; @@ -1023,7 +1027,7 @@ function getEligiblePoolAccounts( if ( excludeId !== MAIN_CODEX_ACCOUNT_ID && !isCodexAccountPaused(config, MAIN_CODEX_ACCOUNT_ID) - && !isAccountNeedsReauth(MAIN_CODEX_ACCOUNT_ID) + && (!isAccountNeedsReauth(MAIN_CODEX_ACCOUNT_ID) || hasMainAccountRefreshGrant()) && getCodexQuotaHealthSnapshot(MAIN_CODEX_ACCOUNT_ID, quotaScope, now) === null && !isCodexAccountSoftAvoided(MAIN_CODEX_ACCOUNT_ID, now) && (!skipFailoverReadyCandidates || !shouldFailover(config, MAIN_CODEX_ACCOUNT_ID, now)) diff --git a/src/config/atomic-write.ts b/src/config/atomic-write.ts index b7e204580b..fae35872b9 100644 --- a/src/config/atomic-write.ts +++ b/src/config/atomic-write.ts @@ -42,6 +42,10 @@ export interface AtomicWriteIO { unlink: (path: string) => void; } +export interface AtomicWriteHooks { + beforeRename?: (tempPath: string, targetPath: string) => void; +} + export class AtomicWriteResidualTempError extends Error { constructor(readonly tempPath: string, readonly hardened = true, options?: ErrorOptions) { super(`Atomic config write left a ${hardened ? "hardened " : ""}zero-byte temporary file`, options); @@ -101,7 +105,7 @@ export function atomicWriteFile(path: string, content: string, io: AtomicWriteIO rename: renameAtomicFile, truncate: target => truncateSync(target, 0), unlink: unlinkSync, -}): void { +}, hooks: AtomicWriteHooks = {}): void { recordOwnedConfigPath(getConfigDir(), path); const target = resolveWriteTarget(path); assertResolvedTargetAllowed(path, target); @@ -111,6 +115,7 @@ export function atomicWriteFile(path: string, content: string, io: AtomicWriteIO io.write(tmp, content); io.harden(tmp); hardened = true; + hooks.beforeRename?.(tmp, target); io.rename(tmp, target); forgetEphemeralSecretPath(tmp); } catch (cause) { diff --git a/src/lib/test-home-guard.ts b/src/lib/test-home-guard.ts index 9b8c52d0dd..493e7f8297 100644 --- a/src/lib/test-home-guard.ts +++ b/src/lib/test-home-guard.ts @@ -58,15 +58,20 @@ function canonicalize(path: string): string { * `homedir()` later would return the sandbox and leave the real home unprotected — the * guard would be perfectly inverted while its tests still looked green. */ -const PROTECTED_HOME = canonicalize( - join(process.env[REAL_HOME_ENV]?.trim() || homedir(), ".opencodex"), -); +const REAL_HOME = process.env[REAL_HOME_ENV]?.trim() || homedir(); +const PROTECTED_HOME = canonicalize(join(REAL_HOME, ".opencodex")); +const PROTECTED_CODEX_HOME = canonicalize(join(REAL_HOME, ".codex")); /** The production home this process protects. Exported for the guard's own tests. */ export function protectedHomeForTests(): string { return PROTECTED_HOME; } +/** The production Codex home this process protects when tests write native credentials. */ +export function protectedCodexHomeForTests(): string { + return PROTECTED_CODEX_HOME; +} + export function isTestHomeGuardArmed(): boolean { return process.env[GUARD_ENV] === "1"; } @@ -88,3 +93,13 @@ export function assertNotRealHomeUnderTest(dir: string): void { + "instead of calling the global writer (see devlog 260730_codex_rs_upstream_v2_live_handoff/070).", ); } + +/** Throw when an armed test process is about to write the real native Codex home. */ +export function assertNotRealCodexHomeUnderTest(dir: string): void { + if (!isTestHomeGuardArmed()) return; + if (canonicalize(dir) !== PROTECTED_CODEX_HOME) return; + throw new Error( + `refusing to write the real Codex home (${PROTECTED_CODEX_HOME}) from a test process. ` + + "Point CODEX_HOME at a temp directory for this test before writing native auth.json.", + ); +} diff --git a/src/oauth/chatgpt.ts b/src/oauth/chatgpt.ts index f4ecc7f8a9..5dd01497db 100644 --- a/src/oauth/chatgpt.ts +++ b/src/oauth/chatgpt.ts @@ -143,7 +143,10 @@ export async function loginChatGPT(ctrl: OAuthController, opts?: { forceLogin?: // Note: uses form-urlencoded per OAuth 2.0 spec (RFC 6749 §6). // Codex-rs uses JSON for refresh — intentional divergence; both accepted by auth.openai.com. -export async function refreshChatGPTToken(refreshToken: string): Promise { +export async function refreshChatGPTToken( + refreshToken: string, + options: { signal?: AbortSignal } = {}, +): Promise { const resp = await fetch(TOKEN_URL, { method: "POST", headers: { "Content-Type": "application/x-www-form-urlencoded" }, @@ -152,6 +155,7 @@ export async function refreshChatGPTToken(refreshToken: string): Promise { + const { req, authCtx, provider, codexAccountMode, substituteMainCredential, options } = args; + if (authCtx.kind !== "main-pool") { + return { ok: false, response: formatErrorResponse(401, "authentication_error", "No native main credential to refresh") }; + } + try { + const refreshed = await forceRefreshMainAccountToken(authCtx.accessToken, { + signal: req.signal, + ...(options.nativeMainRefreshDependencies ?? {}), + }); + if (!refreshed) { + return { ok: false, response: formatErrorResponse(401, "authentication_error", "Codex main account needs reauthentication") }; + } + const refreshedAuthCtx: CodexAuthContext = { + ...authCtx, + accessToken: refreshed.accessToken, + chatgptAccountId: refreshed.chatgptAccountId, + }; + const refreshedProvider = applyCodexAuthContextToProvider( + stripCodexRuntimeProviderFields(provider), + refreshedAuthCtx, + codexAccountMode, + ); + const headers = new Headers({ "content-type": "application/json" }); + const selected = await materializeCodexUpstreamAuthAsync(req.headers, refreshedAuthCtx, { + substituteMainCredential, + signal: req.signal, + nativeMainRefreshDependencies: options.nativeMainRefreshDependencies, + }); + for (const name of FORWARD_HEADERS) { + const value = selected.get(name); + if (value) headers.set(name, value); + } + const override = (refreshedProvider as { _codexAccountOverride?: { accessToken: string; chatgptAccountId: string } })._codexAccountOverride; + if (override) { + headers.set("authorization", `Bearer ${override.accessToken}`); + headers.set("chatgpt-account-id", override.chatgptAccountId); + } + return { ok: true, authCtx: refreshedAuthCtx, provider: refreshedProvider, headers }; + } catch (error) { + return { ok: false, response: nativeMainRefreshFailureResponse(error) }; + } +} + /** @@ -272,6 +335,7 @@ export async function handleResponsesCompact( logCtx: RequestLogContext, turnAdmissionLease?: AdmissionLease, admission?: DataPlaneAdmission, + options: HandleResponsesCompactOptions = {}, ): Promise { let body: unknown; try { @@ -373,7 +437,7 @@ export async function handleResponsesCompact( // headers would run compaction on the wrong account (or 401) whenever a pool account is // active for this thread while normal turns succeed. let compactProvider = route.provider; - const headers = new Headers({ "content-type": "application/json" }); + let headers = new Headers({ "content-type": "application/json" }); try { if (route.codexAccountMode) { authCtx = await resolveCodexAuthContext(req.headers, config, route.codexAccountMode, { @@ -382,9 +446,15 @@ export async function handleResponsesCompact( substituteMainCredentialForDirect: substituteMainCredential, requestScopedMainCredential, beginCodexAccountSelection: codexAccountSelectionForTurn(turnAdmissionLease), + signal: req.signal, + nativeMainRefreshDependencies: options.nativeMainRefreshDependencies, }); logCtx.accountLogLabel = codexAuthContextLogLabel(authCtx, config); - const selected = materializeCodexUpstreamAuth(req.headers, authCtx, { substituteMainCredential }); + const selected = await materializeCodexUpstreamAuthAsync(req.headers, authCtx, { + substituteMainCredential, + signal: req.signal, + nativeMainRefreshDependencies: options.nativeMainRefreshDependencies, + }); compactProvider = applyCodexAuthContextToProvider(route.provider, authCtx, route.codexAccountMode); for (const name of FORWARD_HEADERS) { const value = selected.get(name); @@ -552,6 +622,42 @@ export async function handleResponsesCompact( return formatErrorResponse(502, "upstream_error", "Failed to connect to compact upstream"); } + if ( + upstream.status === 401 + && authCtx.kind === "main-pool" + && usesCodexForwardPoolAuth(authCtx, compactProvider) + && !req.signal.aborted + ) { + await upstream.body?.cancel().catch(() => undefined); + const replay = await refreshNativeMainCompactContext({ + req, + authCtx, + provider: compactProvider, + codexAccountMode: route.codexAccountMode, + substituteMainCredential, + options, + }); + if (!replay.ok) { + recordCompactPoolOutcome(outcomeCtx, replay.response.status === 401 ? 401 : "connect_neutral"); + return replay.response; + } + authCtx = replay.authCtx; + outcomeCtx = replay.authCtx; + compactProvider = replay.provider; + headers = replay.headers; + logCtx.accountLogLabel = codexAuthContextLogLabel(replay.authCtx, config); + try { + upstream = await sendCompactAttempt(compactProvider, headers, "single"); + } catch (err) { + if (req.signal.aborted) { + recordCompactPoolOutcome(outcomeCtx, 499); + return formatErrorResponse(499, "client_cancelled", "Client cancelled compact request"); + } + recordCompactPoolOutcome(outcomeCtx, classifyTransportFailureKind(err)); + return formatErrorResponse(502, "upstream_error", "Failed to connect to compact upstream"); + } + } + // Bounded same-request alternate: the regular /v1/responses path already does this // (core.ts:319-423) and recognizes exactly 429/402. Without it a pool rejection // surfaces to the client, which retries the compact task OUTSIDE the logical request diff --git a/src/server/responses/core.ts b/src/server/responses/core.ts index e04ed3802d..691d75ada4 100644 --- a/src/server/responses/core.ts +++ b/src/server/responses/core.ts @@ -140,7 +140,7 @@ import { CodexPoolAuthenticationError, CodexThreadAffinityExpiredError, headersForCodexAuthContext, - materializeCodexUpstreamAuth, + materializeCodexUpstreamAuthAsync, isCodexAuthContextUsable, resolveCodexAuthContext, codexProbeLeaseId, @@ -155,7 +155,11 @@ import { resolveCodexModelEntitlements, } from "../../codex/model-entitlements"; import { ACCOUNT_GATED_NATIVE_OPENAI_MODELS } from "../../codex/catalog/native-models"; -import { MAIN_CODEX_ACCOUNT_ID } from "../../codex/main-account"; +import { + MAIN_CODEX_ACCOUNT_ID, + forceRefreshMainAccountToken, + type NativeMainRefreshDependencies, +} from "../../codex/main-account"; import { captureCodexAffinityDiagnostic } from "../../codex/affinity-debug"; import { computeQuotaCooldown, @@ -306,7 +310,7 @@ import { createResponsesModelPayloadRewrite, rewriteResponsesModelJson } from ". import type { EffectiveSubagentRoster, SpawnAgentSurface } from "../../codex/catalog"; import { buildToolBridgeMaps, collabSurface, injectDeveloperMessage, multiAgentGuidanceText } from "./collaboration"; -import { mapCodexAuthContextErrorToResponse } from "./codex-auth-error"; +import { mapCodexAuthContextErrorToResponse, nativeMainRefreshFailureResponse } from "./codex-auth-error"; import { hasUnreadableEncryptedAgentTask, looksLikeBackendCiphertext, sanitizeEncryptedContentInPlace } from "./encrypted-payload"; import { fetchWithHeaderTimeout, providerFetch, safeHostLabel, safeOriginLabel } from "./fetch-helpers"; import { classifyTransportFailureKind, transportErrorCode } from "../../lib/upstream-reachability"; @@ -1380,6 +1384,8 @@ export interface HandleResponsesOptions { responsesTerminalRepairScheduler?: ResponsesTerminalRepairScheduler; /** Internal deterministic runtime-identity seam for Codex upstream WS selection tests. */ codexWsRuntimeIdentity?: BunRuntimeGateInput; + /** Test seam for native main refresh without live OAuth traffic. */ + nativeMainRefreshDependencies?: NativeMainRefreshDependencies; /** * When true, body `prompt_cache_key` is a Claude Desktop shared cache cohort * (system/tools hash), not a per-session id — do not use it for Anthropic pool affinity. @@ -1677,6 +1683,8 @@ async function resolveResponsesCodexAuth( requestScopedMainCredential, beginCodexAccountSelection: codexAccountSelectionForTurn(options.turnAdmissionLease), resolveCodexModelEntitlements: options.resolveCodexModelEntitlements, + signal: options.abortSignal, + nativeMainRefreshDependencies: options.nativeMainRefreshDependencies, }); options.onCodexAuthContextResolved?.(authCtx); } else { @@ -1707,7 +1715,11 @@ async function resolveResponsesCodexAuth( return { ok: true, authCtx, - headers: materializeCodexUpstreamAuth(req.headers, authCtx, { substituteMainCredential }), + headers: await materializeCodexUpstreamAuthAsync(req.headers, authCtx, { + substituteMainCredential, + signal: options.abortSignal, + nativeMainRefreshDependencies: options.nativeMainRefreshDependencies, + }), substituteMainCredential, }; } catch (err) { @@ -1729,6 +1741,49 @@ async function resolveResponsesCodexAuth( } } +async function refreshNativeMainForwardAuth(args: { + req: Request; + route: RouteResult; + authCtx: CodexAuthContext; + substituteMainCredential: boolean; + options: HandleResponsesOptions; +}): Promise< + | { ok: true; authCtx: CodexAuthContext; provider: OcxProviderConfig; headers: Headers } + | { ok: false; response: Response } +> { + const { req, route, authCtx, substituteMainCredential, options } = args; + if (authCtx.kind !== "main-pool") { + return { ok: false, response: formatErrorResponse(401, "authentication_error", "No native main credential to refresh") }; + } + try { + const refreshed = await forceRefreshMainAccountToken(authCtx.accessToken, { + signal: options.abortSignal, + ...(options.nativeMainRefreshDependencies ?? {}), + }); + if (!refreshed) { + return { ok: false, response: formatErrorResponse(401, "authentication_error", "Codex main account needs reauthentication") }; + } + const refreshedAuthCtx: CodexAuthContext = { + ...authCtx, + accessToken: refreshed.accessToken, + chatgptAccountId: refreshed.chatgptAccountId, + }; + const provider = applyCodexAuthContextToProvider( + stripCodexRuntimeProviderFields(route.provider), + refreshedAuthCtx, + route.codexAccountMode, + ); + const headers = await materializeCodexUpstreamAuthAsync(req.headers, refreshedAuthCtx, { + substituteMainCredential, + signal: options.abortSignal, + nativeMainRefreshDependencies: options.nativeMainRefreshDependencies, + }); + return { ok: true, authCtx: refreshedAuthCtx, provider, headers }; + } catch (error) { + return { ok: false, response: nativeMainRefreshFailureResponse(error) }; + } +} + async function resolveSubagentFallbackModelEligibility(args: { config: OcxConfig; fallbackChain: readonly string[] | null; @@ -3611,6 +3666,7 @@ async function handleResponsesInner( const opaqueBlobRecoveryGuard: OpaqueBlobRecoveryGuard = { attempted: false }; let oauth401ReplayAttempted = false; + let codexMain401ReplayAttempted = false; const rateLimitPolicy = rateLimitRetryPolicyFor(route.provider); let rateLimitRetries = 0; const rebuildAndRefetch = async ( @@ -3681,6 +3737,82 @@ async function handleResponsesInner( // Keep recovery kinds in sync with the generic `recovery:` loop below. passthroughRecovery: for (;;) { + if ( + upstreamResponse.status === 401 + && authCtx.kind === "main-pool" + && usesCodexForwardPoolAuth(authCtx, route.provider) + && !codexMain401ReplayAttempted + ) { + codexMain401ReplayAttempted = true; + try { void upstreamResponse.body?.cancel().catch(() => {}); } catch { /* already consumed */ } + const replay = await refreshNativeMainForwardAuth({ + req, + route, + authCtx, + substituteMainCredential, + options, + }); + if (!replay.ok) { + upstream.abort(); + releaseCodexAuthContextProbeLease(authCtx); + return replay.response; + } + authCtx = replay.authCtx; + route.provider = replay.provider; + selectedForwardHeaders = replay.headers; + const replayAdapter = resolveAdapter( + resolveWireProtocolOverride(route.providerName, route.modelId, replay.provider, inboundWire), + config.cacheRetention, + ); + if (!("passthrough" in replayAdapter) || !replayAdapter.passthrough) { + upstream.abort(); + return formatErrorResponse(502, "upstream_error", "Native main refresh changed the provider wire unexpectedly"); + } + bindRouteReasoningReplayScope({ + parsed, + providerName: route.providerName, + provider: replay.provider, + adapterName: replayAdapter.name, + codexAuthContext: authCtx, + forwardHeaders: selectedForwardHeaders, + }); + logCtx.providerAdapter = replayAdapter.name; + sealRequestAttemptIdentity(logCtx.activeAttempt, logCtx.provider, replayAdapter.name, logCtx.accountLogLabel); + try { + request = await replayAdapter.buildRequest(parsed, { + headers: selectedForwardHeaders, + translatorBudget, + }); + refreshRoutedNamespaceToolAliases(request); + recordAdapterReasoning(logCtx, request); + recordAdapterTier(logCtx, request); + refreshUndeclaredToolGuard(request); + noteAttemptSend(logCtx.activeAttempt, passthroughEstimate, "oauth-401"); + upstreamResponse = await fetchWithHeaderTimeout( + request.url, + { method: request.method, headers: request.headers, body: request.body }, + upstream.signal, + connectMs, + parsed.stream, + providerFetch(route.provider, options.codexWsRuntimeIdentity, { + providerName: route.providerName, + modelId: route.modelId, + }), + route.provider.authMode === "forward", + ).then(response => { + settleObservedHostResponse(); + return response; + }); + } catch (err) { + return transportFailureResponse(err); + } finally { + request.releaseBodyObservation?.(); + } + continue passthroughRecovery; + } + + if (codexMain401ReplayAttempted && upstreamResponse.status === 401) break; + // Native Responses providers return before the generic adapter recovery loop below. Keep // their OAuth contract identical: one pre-stream 401 forces a credential refresh and one // rebuilt replay. xAI's current subscription models use this branch now that their official diff --git a/tests/codex-auth-context.test.ts b/tests/codex-auth-context.test.ts index eec919927e..93653a3002 100644 --- a/tests/codex-auth-context.test.ts +++ b/tests/codex-auth-context.test.ts @@ -1,5 +1,5 @@ import { afterEach, beforeEach, describe, expect, spyOn, test } from "bun:test"; -import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { @@ -210,6 +210,11 @@ function liveJwt(): string { const payload = Buffer.from(JSON.stringify({ exp: Math.floor(Date.now() / 1000) + 86_400 })).toString("base64url"); return `header.${payload}.signature`; } + +function expiredJwt(): string { + const payload = Buffer.from(JSON.stringify({ exp: Math.floor(Date.now() / 1000) - 60 })).toString("base64url"); + return `header.${payload}.signature`; +} describe("Codex auth context", () => { test("main-profile drain routes a non-main pool account without native reads or quota priming", async () => { saveCodexAccountCredential("pool-a", { @@ -1024,12 +1029,24 @@ describe("Codex auth context", () => { const cfg = config(); cfg.codexAccounts = []; cfg.activeCodexAccountId = undefined; + const authPath = join(testDir, "auth.json"); + const originalAuth = JSON.stringify({ + tokens: { + access_token: expiredJwt(), + refresh_token: "operator-refresh-token", + account_id: "operator-main-account", + }, + }); + writeFileSync(authPath, originalAuth); const inbound = new Headers({ authorization: "Bearer caller-keyring-token", "chatgpt-account-id": "caller-keyring-account", "openai-beta": "responses=experimental", }); - markAccountNeedsReauth(MAIN_CODEX_ACCOUNT_ID); + // This assertion is about request isolation, not config-generation reconciliation. + // Use a definitely-current writer so earlier tests cannot make the setup a no-op. + markAccountNeedsReauth(MAIN_CODEX_ACCOUNT_ID, Number.MAX_SAFE_INTEGER); + let operatorRefreshes = 0; try { expect(hasForwardableCodexBearer(inbound, cfg)).toBe(true); @@ -1039,6 +1056,17 @@ describe("Codex auth context", () => { }), cfg)).toBe(false); const ctx = await resolveCodexAuthContext(inbound, cfg, "pool", { requestScopedMainCredential: true, + nativeMainRefreshDependencies: { + refreshToken: async () => { + operatorRefreshes += 1; + return { + access: "wrong-refreshed-access", + refresh: "wrong-rotated-refresh", + expires: Date.now() + 3_600_000, + accountId: "operator-main-account", + }; + }, + }, }); expect(ctx).toMatchObject({ kind: "main", @@ -1054,6 +1082,8 @@ describe("Codex auth context", () => { expect(upstream.get("chatgpt-account-id")).toBe("caller-keyring-account"); expect(upstream.get("openai-beta")).toBe("responses=experimental"); expect(isAccountNeedsReauth(MAIN_CODEX_ACCOUNT_ID)).toBe(true); + expect(operatorRefreshes).toBe(0); + expect(readFileSync(authPath, "utf8")).toBe(originalAuth); } finally { clearAccountNeedsReauth(MAIN_CODEX_ACCOUNT_ID); } diff --git a/tests/codex-main-account-refresh.test.ts b/tests/codex-main-account-refresh.test.ts new file mode 100644 index 0000000000..507c1b4370 --- /dev/null +++ b/tests/codex-main-account-refresh.test.ts @@ -0,0 +1,105 @@ +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { mkdtempSync, readFileSync, readdirSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { + getValidMainAccountToken, + setMainAuthJsonBeforeRenameHookForTests, +} from "../src/codex/main-account"; + +let home: string; +let previousCodexHome: string | undefined; + +function expiredJwt(): string { + const payload = Buffer.from(JSON.stringify({ exp: Math.floor(Date.now() / 1000) - 60 })).toString("base64url"); + return `header.${payload}.signature`; +} + +beforeEach(() => { + home = mkdtempSync(join(tmpdir(), "ocx-main-refresh-")); + previousCodexHome = process.env.CODEX_HOME; + process.env.CODEX_HOME = home; +}); + +afterEach(() => { + setMainAuthJsonBeforeRenameHookForTests(null); + if (previousCodexHome === undefined) delete process.env.CODEX_HOME; + else process.env.CODEX_HOME = previousCodexHome; + rmSync(home, { recursive: true, force: true }); +}); + +describe("native main token refresh", () => { + test("refreshes a refresh-only auth file and atomically preserves unrelated fields", async () => { + const authPath = join(home, "auth.json"); + const original = { + auth_mode: "chatgpt", + tokens: { + refresh_token: "old-refresh", + account_id: "account-main", + future_token_field: "preserve-token", + }, + future_root_field: { preserve: true }, + }; + writeFileSync(authPath, JSON.stringify(original)); + let targetDuringPublish = ""; + setMainAuthJsonBeforeRenameHookForTests(() => { + targetDuringPublish = readFileSync(authPath, "utf8"); + }); + + const token = await getValidMainAccountToken({ + refreshToken: async refreshToken => { + expect(refreshToken).toBe("old-refresh"); + return { + access: "new-access", + refresh: "rotated-refresh", + expires: Date.now() + 3_600_000, + accountId: "account-main", + }; + }, + }); + + expect(token).toEqual({ accessToken: "new-access", chatgptAccountId: "account-main" }); + expect(targetDuringPublish).toBe(JSON.stringify(original)); + expect(JSON.parse(readFileSync(authPath, "utf8"))).toEqual({ + ...original, + tokens: { + ...original.tokens, + access_token: "new-access", + refresh_token: "rotated-refresh", + account_id: "account-main", + }, + }); + expect(readdirSync(home).filter(name => name.includes(".tmp"))).toEqual([]); + }); + + test("refuses to overwrite an external auth writer after refresh", async () => { + const authPath = join(home, "auth.json"); + writeFileSync(authPath, JSON.stringify({ + tokens: { + access_token: expiredJwt(), + refresh_token: "old-refresh", + account_id: "account-main", + }, + })); + const external = JSON.stringify({ + tokens: { + access_token: "external-access", + refresh_token: "external-refresh", + account_id: "account-external", + }, + }); + setMainAuthJsonBeforeRenameHookForTests(() => writeFileSync(authPath, external)); + + await expect(getValidMainAccountToken({ + refreshToken: async () => ({ + access: "new-access", + refresh: "rotated-refresh", + expires: Date.now() + 3_600_000, + accountId: "account-main", + }), + })).rejects.toThrow("changed while its token was refreshing"); + + expect(readFileSync(authPath, "utf8")).toBe(external); + expect(readdirSync(home).filter(name => name.includes(".tmp"))).toEqual([]); + }); +}); diff --git a/tests/responses-native-main-refresh.test.ts b/tests/responses-native-main-refresh.test.ts new file mode 100644 index 0000000000..f700d7cae6 --- /dev/null +++ b/tests/responses-native-main-refresh.test.ts @@ -0,0 +1,162 @@ +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { clearAccountNeedsReauth } from "../src/codex/auth-api"; +import { MAIN_CODEX_ACCOUNT_ID } from "../src/codex/main-account"; +import { clearCodexUpstreamHealth, clearThreadAccountMap } from "../src/codex/routing"; +import { handleResponses, handleResponsesCompact } from "../src/server/responses"; +import type { RequestLogContext } from "../src/server/request-log"; +import type { OcxConfig } from "../src/types"; + +const originalFetch = globalThis.fetch; +let home = ""; +let previousOcxHome: string | undefined; +let previousCodexHome: string | undefined; + +function config(): OcxConfig { + return { + defaultProvider: "openai", + activeCodexAccountId: MAIN_CODEX_ACCOUNT_ID, + autoSwitchThreshold: 0, + providers: { + openai: { + adapter: "openai-responses", + baseUrl: "https://chatgpt.com/backend-api/codex", + authMode: "forward", + codexAccountMode: "pool", + }, + }, + codexAccounts: [], + } as OcxConfig; +} + +function request(path: "/v1/responses" | "/v1/responses/compact"): Request { + return new Request(`http://localhost${path}`, { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify(path.endsWith("compact") + ? { model: "gpt-5.5", input: [] } + : { model: "gpt-5.5", input: "hello", stream: false }), + }); +} + +beforeEach(() => { + home = mkdtempSync(join(tmpdir(), "ocx-responses-main-refresh-")); + previousOcxHome = process.env.OPENCODEX_HOME; + previousCodexHome = process.env.CODEX_HOME; + process.env.OPENCODEX_HOME = home; + process.env.CODEX_HOME = home; + clearAccountNeedsReauth(MAIN_CODEX_ACCOUNT_ID); + clearCodexUpstreamHealth(); + clearThreadAccountMap(); + writeFileSync(join(home, "auth.json"), JSON.stringify({ + tokens: { + access_token: "rejected-access", + refresh_token: "refresh-grant", + account_id: "account-main", + }, + })); +}); + +afterEach(() => { + globalThis.fetch = originalFetch; + clearAccountNeedsReauth(MAIN_CODEX_ACCOUNT_ID); + clearCodexUpstreamHealth(); + clearThreadAccountMap(); + if (previousOcxHome === undefined) delete process.env.OPENCODEX_HOME; + else process.env.OPENCODEX_HOME = previousOcxHome; + if (previousCodexHome === undefined) delete process.env.CODEX_HOME; + else process.env.CODEX_HOME = previousCodexHome; + rmSync(home, { recursive: true, force: true }); +}); + +function install401ThenRefreshHarness(): { sends: string[]; refreshes: string[] } { + const sends: string[] = []; + const refreshes: string[] = []; + globalThis.fetch = (async (input: RequestInfo | URL, init?: RequestInit) => { + const url = new URL(input instanceof Request ? input.url : String(input)); + if (url.hostname === "auth.openai.com") { + const refresh = new URLSearchParams(String(init?.body)).get("refresh_token") ?? ""; + refreshes.push(refresh); + return Response.json({ + access_token: "refreshed-access", + refresh_token: "rotated-refresh", + expires_in: 3600, + }); + } + if (!url.pathname.endsWith("/responses") && !url.pathname.endsWith("/responses/compact")) { + return Response.json({ rate_limit: { primary_window: { used_percent: 10 } } }); + } + const authorization = new Headers(init?.headers).get("authorization") ?? ""; + sends.push(authorization); + if (sends.length === 1) { + return Response.json({ error: { message: "expired bearer" } }, { status: 401 }); + } + return Response.json({ id: "resp_refreshed", object: "response", status: "completed", output: [] }); + }) as typeof fetch; + return { sends, refreshes }; +} + +describe("native main 401 refresh and replay", () => { + test("refreshes a refresh-only native main credential before upstream I/O", async () => { + writeFileSync(join(home, "auth.json"), JSON.stringify({ + tokens: { refresh_token: "refresh-grant", account_id: "account-main" }, + })); + const sends: string[] = []; + let refreshes = 0; + globalThis.fetch = (async (input: RequestInfo | URL, init?: RequestInit) => { + const url = new URL(input instanceof Request ? input.url : String(input)); + if (url.hostname === "auth.openai.com") { + refreshes += 1; + return Response.json({ + access_token: "refreshed-access", + refresh_token: "rotated-refresh", + expires_in: 3600, + }); + } + if (url.pathname.endsWith("/responses")) { + sends.push(new Headers(init?.headers).get("authorization") ?? ""); + } + return Response.json({ id: "resp_refreshed", object: "response", status: "completed", output: [] }); + }) as typeof fetch; + + const response = await handleResponses( + request("/v1/responses"), + config(), + { model: "", provider: "" } as RequestLogContext, + ); + + expect(response.status).toBe(200); + expect(refreshes).toBe(1); + expect(sends).toEqual(["Bearer refreshed-access"]); + }); + + test("Responses refreshes and performs exactly one physical replay", async () => { + const harness = install401ThenRefreshHarness(); + const response = await handleResponses( + request("/v1/responses"), + config(), + { model: "", provider: "" } as RequestLogContext, + ); + + expect(response.status).toBe(200); + expect(harness.sends).toEqual(["Bearer rejected-access", "Bearer refreshed-access"]); + expect(harness.refreshes).toEqual(["refresh-grant"]); + expect(JSON.parse(readFileSync(join(home, "auth.json"), "utf8")).tokens.refresh_token) + .toBe("rotated-refresh"); + }); + + test("compact refreshes and performs exactly one physical replay", async () => { + const harness = install401ThenRefreshHarness(); + const response = await handleResponsesCompact( + request("/v1/responses/compact"), + config(), + { model: "", provider: "" } as RequestLogContext, + ); + + expect(response.status).toBe(200); + expect(harness.sends).toEqual(["Bearer rejected-access", "Bearer refreshed-access"]); + expect(harness.refreshes).toEqual(["refresh-grant"]); + }); +}); diff --git a/tests/test-home-guard.test.ts b/tests/test-home-guard.test.ts index 5c8ab45f7d..91783dc0dc 100644 --- a/tests/test-home-guard.test.ts +++ b/tests/test-home-guard.test.ts @@ -57,11 +57,13 @@ function runProbe(source: string, env: Record): { co } /** A fake "real home" the guard will protect, so no deny case aims at the true one. */ -function sentinelHome(): { realHome: string; opencodexHome: string } { +function sentinelHome(): { realHome: string; opencodexHome: string; codexHome: string } { const realHome = mkdtempSync(join(tmpdir(), "ocx-sentinel-home-")); const opencodexHome = join(realHome, ".opencodex"); + const codexHome = join(realHome, ".codex"); mkdirSync(opencodexHome, { recursive: true }); - return { realHome, opencodexHome }; + mkdirSync(codexHome, { recursive: true }); + return { realHome, opencodexHome, codexHome }; } describe("real-home write guard", () => { @@ -111,6 +113,22 @@ const canSymlink = (() => { expect(() => readFileSync(join(opencodexHome, "codex-accounts.json"))).toThrow(); }); + test("armed native credential writes reject the protected Codex home", () => { + const { realHome, codexHome } = sentinelHome(); + const probe = runProbe(` + import { assertNotRealCodexHomeUnderTest } from "${REPO_ROOT_URL}src/lib/test-home-guard"; + try { + assertNotRealCodexHomeUnderTest("${codexHome}"); + console.log("WRITE_ALLOWED"); + } catch (err) { + console.log(String(err).includes("refusing to write the real Codex home") ? "REFUSED" : "OTHER"); + } + `, { OCX_TEST_HOME_GUARD: "1", OCX_REAL_HOME: realHome, CODEX_HOME: codexHome }); + + expect(probe.stdout).toContain("REFUSED"); + expect(probe.stdout).not.toContain("WRITE_ALLOWED"); + }); + test.skipIf(!canSymlink)("armed + a symlink escaping a temp home into the protected home: refused", () => { // Atomic writes resolve their destination through symlinks, so a temp home whose // config.json points into the protected home would otherwise pass the caller's From 1c949d75cd182e3e9b11d9357b64fdbf99f3eed7 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Sat, 29 Aug 2026 05:48:36 +0900 Subject: [PATCH 2/2] fix(codex): close native auth refresh write races Create atomic temp files exclusively with owner-only permissions and apply Windows ACLs before writing credentials. Revalidate native auth bytes at the final rename boundary so concurrent external writes abort publication. --- src/codex/main-account.ts | 3 +- src/config/atomic-write.ts | 121 ++++++++++++++++++----- tests/codex-main-account-refresh.test.ts | 15 +++ tests/config.test.ts | 60 ++++++++++- 4 files changed, 173 insertions(+), 26 deletions(-) diff --git a/src/codex/main-account.ts b/src/codex/main-account.ts index b8604176cb..b0b7ebb328 100644 --- a/src/codex/main-account.ts +++ b/src/codex/main-account.ts @@ -152,11 +152,12 @@ function persistRefreshedMainAuthJson( undefined, { beforeRename: () => { + assertMainAuthJsonSnapshotUnchanged(expected); const hook = beforeMainAuthJsonRenameForTests; beforeMainAuthJsonRenameForTests = null; hook?.(); - assertMainAuthJsonSnapshotUnchanged(expected); }, + validateBeforeRename: () => assertMainAuthJsonSnapshotUnchanged(expected), }, ); return { accessToken, chatgptAccountId }; diff --git a/src/config/atomic-write.ts b/src/config/atomic-write.ts index fae35872b9..0ec0831c4c 100644 --- a/src/config/atomic-write.ts +++ b/src/config/atomic-write.ts @@ -1,6 +1,11 @@ import { chmodSync, + closeSync, + constants, + fchmodSync, + fstatSync, lstatSync, + openSync, realpathSync, truncateSync, unlinkSync, @@ -43,7 +48,9 @@ export interface AtomicWriteIO { } export interface AtomicWriteHooks { + afterTempWrite?: (tempPath: string, targetPath: string) => void; beforeRename?: (tempPath: string, targetPath: string) => void; + validateBeforeRename?: (targetPath: string) => void; } export class AtomicWriteResidualTempError extends Error { @@ -96,53 +103,120 @@ function assertResolvedTargetAllowed(path: string, target: string): void { assertNotRealHomeUnderTest(dirname(target)); } -export function atomicWriteFile(path: string, content: string, io: AtomicWriteIO = { - write: (target, value) => writeFileSync(target, value, { encoding: "utf-8", mode: 0o600 }), - harden: target => { - try { chmodSync(target, 0o600); } catch { /* platform may ignore chmod */ } - if (process.platform === "win32") hardenSecretPath(target, { required: true, timeoutMemoKey: path }); - }, - rename: renameAtomicFile, - truncate: target => truncateSync(target, 0), - unlink: unlinkSync, -}, hooks: AtomicWriteHooks = {}): void { +function assertPrivateTempDescriptor(path: string, descriptor: number): void { + const opened = fstatSync(descriptor); + const linked = lstatSync(path); + if (!opened.isFile() || !linked.isFile() + || opened.dev !== linked.dev || opened.ino !== linked.ino) { + throw new Error("atomic temporary file identity changed before write"); + } + if (process.platform !== "win32" && (opened.mode & 0o777) !== 0o600) { + throw new Error("atomic temporary file permissions are not owner-only"); + } +} + +function writePrivateTempFile( + path: string, + content: string, + timeoutMemoKey: string, + onCreated: () => void, +): void { + const descriptor = openSync(path, constants.O_WRONLY | constants.O_CREAT | constants.O_EXCL, 0o600); + onCreated(); + try { + if (process.platform === "win32") { + hardenSecretPath(path, { required: true, timeoutMemoKey }); + } else { + fchmodSync(descriptor, 0o600); + } + assertPrivateTempDescriptor(path, descriptor); + writeFileSync(descriptor, content, { encoding: "utf-8" }); + } finally { + closeSync(descriptor); + } +} + +async function writePrivateTempFileAsync( + path: string, + content: string, + timeoutMemoKey: string, + onCreated: () => void, +): Promise { + const descriptor = openSync(path, constants.O_WRONLY | constants.O_CREAT | constants.O_EXCL, 0o600); + onCreated(); + try { + if (process.platform === "win32") { + await hardenSecretPathAsync(path, { required: true, timeoutMemoKey }); + } else { + fchmodSync(descriptor, 0o600); + } + assertPrivateTempDescriptor(path, descriptor); + writeFileSync(descriptor, content, { encoding: "utf-8" }); + } finally { + closeSync(descriptor); + } +} + +export function atomicWriteFile( + path: string, + content: string, + io?: AtomicWriteIO, + hooks: AtomicWriteHooks = {}, +): void { recordOwnedConfigPath(getConfigDir(), path); const target = resolveWriteTarget(path); assertResolvedTargetAllowed(path, target); const tmp = `${target}.ocx.${process.pid}.${nextAtomicTempSequence()}.tmp`; let hardened = false; + let ownsTemp = false; + 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") { + hardenSecretPath(tempPath, { required: true, timeoutMemoKey: path }); + } + }, + rename: renameAtomicFile, + truncate: tempPath => truncateSync(tempPath, 0), + unlink: unlinkSync, + }; try { - io.write(tmp, content); - io.harden(tmp); + if (io) ownsTemp = true; + effective.write(tmp, content); + hooks.afterTempWrite?.(tmp, target); + effective.harden(tmp); hardened = true; hooks.beforeRename?.(tmp, target); - io.rename(tmp, target); + hooks.validateBeforeRename?.(target); + effective.rename(tmp, target); forgetEphemeralSecretPath(tmp); } catch (cause) { + if (!ownsTemp) throw cause; let scrubbed = false; try { - io.truncate(tmp); + effective.truncate(tmp); scrubbed = true; } catch (error) { if (isMissingPathError(error)) scrubbed = true; else { - try { io.write(tmp, ""); scrubbed = true; } catch { /* removal may still succeed */ } + try { effective.write(tmp, ""); scrubbed = true; } catch { /* removal may still succeed */ } } } let removed = false; try { - io.unlink(tmp); + effective.unlink(tmp); removed = true; } catch (error) { if (isMissingPathError(error)) removed = true; else { - try { io.unlink(tmp); removed = true; } + try { effective.unlink(tmp); removed = true; } catch (retryError) { if (isMissingPathError(retryError)) removed = true; } } } if (!removed && !scrubbed) throw new AtomicWriteSecretResidualError(tmp, { cause }); if (!removed && !hardened) { - try { io.harden(tmp); hardened = true; } catch { /* reported below */ } + try { effective.harden(tmp); hardened = true; } catch { /* reported below */ } } if (removed) forgetEphemeralSecretPath(tmp); if (!removed) throw new AtomicWriteResidualTempError(tmp, hardened, { cause }); @@ -168,12 +242,13 @@ export async function atomicWriteFileAsync( io?: AtomicWriteAsyncIO, testSeam?: AtomicWriteAsyncTestSeam, ): Promise { + let ownsTemp = false; 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 */ } + 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") { - await hardenSecretPathAsync(target, { required: true, timeoutMemoKey: path }); + await hardenSecretPathAsync(tempPath, { required: true, timeoutMemoKey: path }); } }, rename: renameAtomicFileAsync, @@ -185,6 +260,7 @@ export async function atomicWriteFileAsync( const tmp = `${target}.ocx.${process.pid}.${nextAtomicTempSequence()}.tmp`; let hardened = false; try { + if (io) ownsTemp = true; await effective.write(tmp, content); await testSeam?.afterTempWrite?.(tmp); await effective.harden(tmp); @@ -192,6 +268,7 @@ export async function atomicWriteFileAsync( await effective.rename(tmp, target); forgetEphemeralSecretPath(tmp); } catch (cause) { + if (!ownsTemp) throw cause; let scrubbed = false; try { await effective.truncate(tmp); diff --git a/tests/codex-main-account-refresh.test.ts b/tests/codex-main-account-refresh.test.ts index 507c1b4370..45db2a670e 100644 --- a/tests/codex-main-account-refresh.test.ts +++ b/tests/codex-main-account-refresh.test.ts @@ -102,4 +102,19 @@ describe("native main token refresh", () => { expect(readFileSync(authPath, "utf8")).toBe(external); expect(readdirSync(home).filter(name => name.includes(".tmp"))).toEqual([]); }); + + test("refresh failure leaves the original auth file byte-identical", async () => { + const authPath = join(home, "auth.json"); + const original = Buffer.from(`{\n "tokens": {\n "access_token": "${expiredJwt()}",\n "refresh_token": "old-refresh",\n "account_id": "account-main"\n },\n "preserve": "spacing"\n}\n`); + writeFileSync(authPath, original); + + await expect(getValidMainAccountToken({ + refreshToken: async () => { + throw new Error("simulated refresh transport failure"); + }, + })).rejects.toThrow("did not complete"); + + expect(readFileSync(authPath)).toEqual(original); + expect(readdirSync(home).filter(name => name.includes(".tmp"))).toEqual([]); + }); }); diff --git a/tests/config.test.ts b/tests/config.test.ts index 6ebae31319..e3c55a0680 100644 --- a/tests/config.test.ts +++ b/tests/config.test.ts @@ -1,5 +1,5 @@ import { afterEach, beforeEach, describe, expect, spyOn, test } from "bun:test"; -import { chmodSync, existsSync, lstatSync, mkdirSync, mkdtempSync, readdirSync, readFileSync, readlinkSync, renameSync, rmSync, symlinkSync, truncateSync, unlinkSync, writeFileSync } from "node:fs"; +import { chmodSync, existsSync, lstatSync, mkdirSync, mkdtempSync, readdirSync, readFileSync, readlinkSync, renameSync, rmSync, statSync, symlinkSync, truncateSync, unlinkSync, writeFileSync } from "node:fs"; import { homedir, tmpdir } from "node:os"; import { delimiter, dirname, join, resolve } from "node:path"; import { @@ -34,6 +34,7 @@ import { 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 { providerManagementConfigError } from "../src/server/auth-cors"; let testDir = ""; @@ -2438,6 +2439,47 @@ describe("opencodex config defaults", () => { }); describe("config.ts – Windows ACL hardening integration", () => { + test("secret temp bytes are private at first observation and a pre-existing temp is refused", () => { + const destination = join(testDir, "atomic-private-secret.json"); + let observedSecret = false; + atomicWriteFile(destination, "new-secret", undefined, { + afterTempWrite: tempPath => { + expect(readFileSync(tempPath, "utf8")).toBe("new-secret"); + expect(statSync(tempPath).mode & 0o077).toBe(0); + observedSecret = true; + }, + }); + expect(observedSecret).toBe(true); + + const occupiedSequence = nextAtomicTempSequence() + 1; + const occupiedTemp = `${destination}.ocx.${process.pid}.${occupiedSequence}.tmp`; + writeFileSync(occupiedTemp, "pre-existing", { encoding: "utf8", mode: 0o644 }); + expect(() => atomicWriteFile(destination, "replacement-secret", undefined, { + afterTempWrite: tempPath => { + expect(readFileSync(tempPath, "utf8")).not.toBe("replacement-secret"); + expect(statSync(tempPath).mode & 0o077).toBe(0); + }, + })).toThrow(); + expect(readFileSync(occupiedTemp, "utf8")).toBe("pre-existing"); + }); + + test("Windows ACL hardening completes before secret temp bytes are observable", () => { + const originalPlatform = process.platform; + Object.defineProperty(process, "platform", { value: "win32", configurable: true }); + const hardenSpy = spyOn(windowsAcl, "hardenSecretPath").mockReturnValue({ ok: true }); + try { + atomicWriteFile(join(testDir, "atomic-private-windows.json"), "windows-secret", undefined, { + afterTempWrite: tempPath => { + expect(readFileSync(tempPath, "utf8")).toBe("windows-secret"); + expect(hardenSpy).toHaveBeenCalled(); + }, + }); + } finally { + hardenSpy.mockRestore(); + Object.defineProperty(process, "platform", { value: originalPlatform, configurable: true }); + } + }); + test("successive atomic temps for one destination are each hardened and then forgotten", () => { const destination = join(testDir, "atomic-secret.json"); const previousUsername = process.env.USERNAME; @@ -2645,8 +2687,20 @@ describe("config.ts – Windows ACL hardening integration", () => { describe("config.ts – sync writer timeout keying (#840 refinement)", () => { test("the production sync harden keys timeouts by destination", () => { - const source = readFileSync(join(import.meta.dir, "..", "src", "config", "atomic-write.ts"), "utf-8"); - expect(source).toContain("hardenSecretPath(target, { required: true, timeoutMemoKey: path })"); + const destination = join(testDir, "sync-timeout-key.json"); + const originalPlatform = process.platform; + Object.defineProperty(process, "platform", { value: "win32", configurable: true }); + const hardenSpy = spyOn(windowsAcl, "hardenSecretPath").mockReturnValue({ ok: true }); + try { + atomicWriteFile(destination, "secret"); + expect(hardenSpy).toHaveBeenCalledWith( + expect.stringContaining(".tmp"), + { required: true, timeoutMemoKey: destination }, + ); + } finally { + hardenSpy.mockRestore(); + Object.defineProperty(process, "platform", { value: originalPlatform, configurable: true }); + } }); test("timed-out write with a RESIDUAL temp retains both memos (fail-closed)", () => {