From bbe6f7fdd91e6dd710d48614a7ff789a255a0b9f Mon Sep 17 00:00:00 2001 From: giulio-leone Date: Tue, 24 Mar 2026 11:13:27 +0100 Subject: [PATCH] fix(auth): protect fresher codex reauth state - invalidate cached Codex CLI credentials when auth.json changes within the TTL window - skip external CLI sync when the stored Codex OAuth credential is newer - cover both behaviors with focused regression tests Refs #53466 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../auth-profiles.external-cli-sync.test.ts | 36 +++++++++++ src/agents/auth-profiles/external-cli-sync.ts | 24 +++++++ src/agents/cli-credentials.test.ts | 62 +++++++++++++++++++ src/agents/cli-credentials.ts | 26 +++++++- 4 files changed, 145 insertions(+), 3 deletions(-) diff --git a/src/agents/auth-profiles.external-cli-sync.test.ts b/src/agents/auth-profiles.external-cli-sync.test.ts index 7c1e05f5fd1..34f15d50a62 100644 --- a/src/agents/auth-profiles.external-cli-sync.test.ts +++ b/src/agents/auth-profiles.external-cli-sync.test.ts @@ -95,4 +95,40 @@ describe("syncExternalCliCredentials", () => { expires: freshExpiry, }); }); + + it("does not overwrite newer stored Codex credentials with older external CLI credentials", () => { + const staleExpiry = Date.now() + 30 * 60_000; + const freshExpiry = Date.now() + 5 * 24 * 60 * 60_000; + mocks.readCodexCliCredentialsCached.mockReturnValue({ + type: "oauth", + provider: "openai-codex", + access: "stale-access-token", + refresh: "stale-refresh-token", + expires: staleExpiry, + accountId: "acct_789", + }); + + const store: AuthProfileStore = { + version: 1, + profiles: { + [OPENAI_CODEX_DEFAULT_PROFILE_ID]: { + type: "oauth", + provider: "openai-codex", + access: "fresh-access-token", + refresh: "fresh-refresh-token", + expires: freshExpiry, + accountId: "acct_789", + }, + }, + }; + + const mutated = syncExternalCliCredentials(store); + + expect(mutated).toBe(false); + expect(store.profiles[OPENAI_CODEX_DEFAULT_PROFILE_ID]).toMatchObject({ + access: "fresh-access-token", + refresh: "fresh-refresh-token", + expires: freshExpiry, + }); + }); }); diff --git a/src/agents/auth-profiles/external-cli-sync.ts b/src/agents/auth-profiles/external-cli-sync.ts index ff43b586b48..717df9f31d9 100644 --- a/src/agents/auth-profiles/external-cli-sync.ts +++ b/src/agents/auth-profiles/external-cli-sync.ts @@ -36,6 +36,18 @@ function shallowEqualOAuthCredentials(a: OAuthCredential | undefined, b: OAuthCr ); } +function hasNewerStoredOAuthCredential( + existing: OAuthCredential | undefined, + incoming: OAuthCredential, +): boolean { + return Boolean( + existing && + existing.provider === incoming.provider && + Number.isFinite(existing.expires) && + (!Number.isFinite(incoming.expires) || existing.expires > incoming.expires), + ); +} + /** Sync external CLI credentials into the store for a given provider. */ function syncExternalCliCredentialsForProvider( store: AuthProfileStore, @@ -54,6 +66,18 @@ function syncExternalCliCredentialsForProvider( if (shallowEqualOAuthCredentials(existingOAuth, creds)) { return false; } + if (hasNewerStoredOAuthCredential(existingOAuth, creds)) { + if (options.log !== false) { + log.debug(`kept newer stored ${provider} credentials over external cli sync`, { + profileId, + storedExpires: new Date(existingOAuth!.expires).toISOString(), + externalExpires: Number.isFinite(creds.expires) + ? new Date(creds.expires).toISOString() + : null, + }); + } + return false; + } store.profiles[profileId] = creds; if (options.log !== false) { diff --git a/src/agents/cli-credentials.test.ts b/src/agents/cli-credentials.test.ts index 53be1581b13..51f94f4d953 100644 --- a/src/agents/cli-credentials.test.ts +++ b/src/agents/cli-credentials.test.ts @@ -7,6 +7,7 @@ const execSyncMock = vi.fn(); const execFileSyncMock = vi.fn(); const CLI_CREDENTIALS_CACHE_TTL_MS = 15 * 60 * 1000; let readClaudeCliCredentialsCached: typeof import("./cli-credentials.js").readClaudeCliCredentialsCached; +let readCodexCliCredentialsCached: typeof import("./cli-credentials.js").readCodexCliCredentialsCached; let resetCliCredentialCachesForTest: typeof import("./cli-credentials.js").resetCliCredentialCachesForTest; let writeClaudeCliKeychainCredentials: typeof import("./cli-credentials.js").writeClaudeCliKeychainCredentials; let writeClaudeCliCredentials: typeof import("./cli-credentials.js").writeClaudeCliCredentials; @@ -56,6 +57,7 @@ describe("cli credentials", () => { beforeAll(async () => { ({ readClaudeCliCredentialsCached, + readCodexCliCredentialsCached, resetCliCredentialCachesForTest, writeClaudeCliKeychainCredentials, writeClaudeCliCredentials, @@ -292,4 +294,64 @@ describe("cli credentials", () => { expires: expSeconds * 1000, }); }); + + it("invalidates cached Codex credentials when auth.json changes within the TTL window", () => { + const tempHome = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-codex-cache-")); + process.env.CODEX_HOME = tempHome; + const authPath = path.join(tempHome, "auth.json"); + const firstExpiry = Math.floor(Date.parse("2026-03-24T12:34:56Z") / 1000); + const secondExpiry = Math.floor(Date.parse("2026-03-25T12:34:56Z") / 1000); + try { + fs.mkdirSync(tempHome, { recursive: true, mode: 0o700 }); + fs.writeFileSync( + authPath, + JSON.stringify({ + tokens: { + access_token: createJwtWithExp(firstExpiry), + refresh_token: "stale-refresh", + }, + }), + "utf8", + ); + fs.utimesSync(authPath, new Date("2026-03-24T10:00:00Z"), new Date("2026-03-24T10:00:00Z")); + vi.setSystemTime(new Date("2026-03-24T10:00:00Z")); + + const first = readCodexCliCredentialsCached({ + ttlMs: CLI_CREDENTIALS_CACHE_TTL_MS, + platform: "linux", + execSync: execSyncMock, + }); + + expect(first).toMatchObject({ + refresh: "stale-refresh", + expires: firstExpiry * 1000, + }); + + fs.writeFileSync( + authPath, + JSON.stringify({ + tokens: { + access_token: createJwtWithExp(secondExpiry), + refresh_token: "fresh-refresh", + }, + }), + "utf8", + ); + fs.utimesSync(authPath, new Date("2026-03-24T10:05:00Z"), new Date("2026-03-24T10:05:00Z")); + vi.advanceTimersByTime(60_000); + + const second = readCodexCliCredentialsCached({ + ttlMs: CLI_CREDENTIALS_CACHE_TTL_MS, + platform: "linux", + execSync: execSyncMock, + }); + + expect(second).toMatchObject({ + refresh: "fresh-refresh", + expires: secondExpiry * 1000, + }); + } finally { + fs.rmSync(tempHome, { recursive: true, force: true }); + } + }); }); diff --git a/src/agents/cli-credentials.ts b/src/agents/cli-credentials.ts index 8ded765346a..2f667ed3b2e 100644 --- a/src/agents/cli-credentials.ts +++ b/src/agents/cli-credentials.ts @@ -21,6 +21,7 @@ type CachedValue = { value: T | null; readAt: number; cacheKey: string; + sourceMtimeMs?: number | null; }; let claudeCliCache: CachedValue | null = null; @@ -148,6 +149,14 @@ function resolveMiniMaxCliCredentialsPath(homeDir?: string) { return path.join(baseDir, MINIMAX_CLI_CREDENTIALS_RELATIVE_PATH); } +function readFileMtimeMs(filePath: string): number | null { + try { + return fs.statSync(filePath).mtimeMs; + } catch { + return null; + } +} + function computeCodexKeychainAccount(codexHome: string) { const hash = createHash("sha256").update(codexHome).digest("hex"); return `cli|${hash.slice(0, 16)}`; @@ -526,11 +535,14 @@ export function readCodexCliCredentialsCached(options?: { }): CodexCliCredential | null { const ttlMs = options?.ttlMs ?? 0; const now = Date.now(); - const cacheKey = `${options?.platform ?? process.platform}|${resolveCodexCliAuthPath()}`; + const authPath = resolveCodexCliAuthPath(); + const cacheKey = `${options?.platform ?? process.platform}|${authPath}`; + const sourceMtimeMs = readFileMtimeMs(authPath); if ( ttlMs > 0 && codexCliCache && codexCliCache.cacheKey === cacheKey && + codexCliCache.sourceMtimeMs === sourceMtimeMs && now - codexCliCache.readAt < ttlMs ) { return codexCliCache.value; @@ -539,8 +551,16 @@ export function readCodexCliCredentialsCached(options?: { platform: options?.platform, execSync: options?.execSync, }); - if (ttlMs > 0) { - codexCliCache = { value, readAt: now, cacheKey }; + const cachedSourceMtimeMs = readFileMtimeMs(authPath); + if (ttlMs > 0 && cachedSourceMtimeMs === sourceMtimeMs) { + codexCliCache = { + value, + readAt: now, + cacheKey, + sourceMtimeMs: cachedSourceMtimeMs, + }; + } else if (ttlMs > 0) { + codexCliCache = null; } return value; }