From 3ed0995fa9f019c7200a5022f560bf13e0880ffd Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Fri, 17 Apr 2026 10:19:56 -0700 Subject: [PATCH] fix(auth): keep codex oauth canonical in openclaw --- ...auth.openai-codex-refresh-fallback.test.ts | 85 ++++++++++++++++--- src/agents/auth-profiles/oauth.ts | 54 +++--------- 2 files changed, 86 insertions(+), 53 deletions(-) diff --git a/src/agents/auth-profiles/oauth.openai-codex-refresh-fallback.test.ts b/src/agents/auth-profiles/oauth.openai-codex-refresh-fallback.test.ts index 073b68c6e29..273d1605be8 100644 --- a/src/agents/auth-profiles/oauth.openai-codex-refresh-fallback.test.ts +++ b/src/agents/auth-profiles/oauth.openai-codex-refresh-fallback.test.ts @@ -257,7 +257,7 @@ describe("resolveApiKeyForProfile openai-codex refresh fallback", () => { expect(writeCodexCliCredentialsMock).not.toHaveBeenCalled(); }); - it("refreshes expired Codex-managed credentials and persists them back to auth-profiles", async () => { + it("refreshes imported Codex credentials into the canonical auth store without writing back to .codex", async () => { const profileId = "openai-codex:default"; saveAuthProfileStore( { @@ -302,17 +302,7 @@ describe("resolveApiKeyForProfile openai-codex refresh fallback", () => { provider: "openai-codex", email: undefined, }); - expect(writeCodexCliCredentialsMock).toHaveBeenCalledTimes(1); - expect(writeCodexCliCredentialsMock).toHaveBeenCalledWith( - expect.objectContaining({ - type: "oauth", - provider: "openai-codex", - access: "rotated-cli-access-token", - refresh: "rotated-cli-refresh-token", - accountId: "acct-rotated", - managedBy: "codex-cli", - }), - ); + expect(writeCodexCliCredentialsMock).not.toHaveBeenCalled(); const persisted = await readPersistedStore(agentDir); expect(persisted.profiles[profileId]).toMatchObject({ @@ -322,6 +312,11 @@ describe("resolveApiKeyForProfile openai-codex refresh fallback", () => { refresh: "rotated-cli-refresh-token", accountId: "acct-rotated", }); + expect(persisted.profiles[profileId]).not.toEqual( + expect.objectContaining({ + managedBy: "codex-cli", + }), + ); expect(persisted.profiles[profileId]).not.toEqual( expect.objectContaining({ provider: "openai-codex", @@ -330,6 +325,72 @@ describe("resolveApiKeyForProfile openai-codex refresh fallback", () => { ); }); + it("keeps the canonical refresh token when imported Codex CLI state is stale", async () => { + const profileId = "openai-codex:default"; + saveAuthProfileStore( + { + version: 1, + profiles: { + [profileId]: { + type: "oauth", + provider: "openai-codex", + access: "expired-access-token", + refresh: "canonical-refresh-token", + expires: Date.now() - 60_000, + }, + }, + }, + agentDir, + ); + readCodexCliCredentialsCachedMock.mockReturnValue({ + type: "oauth", + provider: "openai-codex", + access: "stale-cli-access-token", + refresh: "stale-cli-refresh-token", + expires: Date.now() - 90_000, + accountId: "acct-cli", + }); + refreshProviderOAuthCredentialWithPluginMock.mockImplementationOnce( + async (params?: { context?: unknown }) => { + expect(params?.context).toMatchObject({ + access: "expired-access-token", + refresh: "canonical-refresh-token", + }); + return { + type: "oauth", + provider: "openai-codex", + access: "fresh-access-token", + refresh: "fresh-refresh-token", + expires: Date.now() + 86_400_000, + }; + }, + ); + + await expect( + resolveApiKeyForProfile({ + store: ensureAuthProfileStore(agentDir), + profileId, + agentDir, + }), + ).resolves.toEqual({ + apiKey: "fresh-access-token", + provider: "openai-codex", + email: undefined, + }); + + const persisted = await readPersistedStore(agentDir); + expect(persisted.profiles[profileId]).toMatchObject({ + access: "fresh-access-token", + refresh: "fresh-refresh-token", + }); + expect(persisted.profiles[profileId]).not.toEqual( + expect.objectContaining({ + access: "stale-cli-access-token", + refresh: "stale-cli-refresh-token", + }), + ); + }); + it("adopts fresher stored credentials after refresh_token_reused", async () => { const profileId = "openai-codex:default"; saveAuthProfileStore( diff --git a/src/agents/auth-profiles/oauth.ts b/src/agents/auth-profiles/oauth.ts index c8c73dc0f73..52226ce60cb 100644 --- a/src/agents/auth-profiles/oauth.ts +++ b/src/agents/auth-profiles/oauth.ts @@ -16,7 +16,6 @@ import { import { resolveSecretRefString, type SecretRefResolveCache } from "../../secrets/resolve.js"; import { normalizeLowercaseStringOrEmpty } from "../../shared/string-coerce.js"; import { refreshChutesTokens } from "../chutes-oauth.js"; -import { writeCodexCliCredentials } from "../cli-credentials.js"; import { AUTH_STORE_LOCK_OPTIONS, OAUTH_REFRESH_CALL_TIMEOUT_MS, @@ -28,6 +27,7 @@ import { formatAuthDoctorHint } from "./doctor.js"; import { areOAuthCredentialsEquivalent, readManagedExternalCliCredential, + shouldReplaceStoredOAuthCredential, } from "./external-cli-sync.js"; import { ensureAuthStoreFile, resolveAuthStorePath, resolveOAuthRefreshLockPath } from "./paths.js"; import { assertNoOAuthSecretRefPolicyViolations } from "./policy.js"; @@ -151,6 +151,13 @@ function hasOAuthCredentialChanged( ); } +function clearExternalOAuthManager( + credential: OAuthCredential, +): OAuthCredentials & { type: "oauth"; provider: string; email?: string } { + const { managedBy: _managedBy, ...canonicalCredential } = credential; + return canonicalCredential; +} + async function loadFreshStoredOAuthCredential(params: { profileId: string; agentDir?: string; @@ -622,7 +629,10 @@ async function doRefreshOAuthTokenWithLock(params: { credential: cred, }); if (externallyManaged) { - if (!areOAuthCredentialsEquivalent(cred, externallyManaged)) { + if ( + shouldReplaceStoredOAuthCredential(cred, externallyManaged) && + !areOAuthCredentialsEquivalent(cred, externallyManaged) + ) { store.profiles[params.profileId] = externallyManaged; saveAuthProfileStore(store, params.agentDir); } @@ -632,44 +642,6 @@ async function doRefreshOAuthTokenWithLock(params: { newCredentials: externallyManaged, }; } - if (externallyManaged.managedBy === "codex-cli") { - const pluginRefreshed = await withRefreshCallTimeout( - `refreshProviderOAuthCredentialWithPlugin(${externallyManaged.provider}, codex-cli)`, - OAUTH_REFRESH_CALL_TIMEOUT_MS, - () => - refreshProviderOAuthCredentialWithPlugin({ - provider: externallyManaged.provider, - context: externallyManaged, - }), - ); - if (pluginRefreshed) { - const refreshedCredentials: OAuthCredential = { - ...externallyManaged, - ...pluginRefreshed, - type: "oauth", - managedBy: "codex-cli", - }; - if (!writeCodexCliCredentials(refreshedCredentials)) { - log.warn("failed to persist refreshed codex credentials back to Codex storage", { - profileId: params.profileId, - }); - } - store.profiles[params.profileId] = refreshedCredentials; - saveAuthProfileStore(store, params.agentDir); - return { - apiKey: await buildOAuthApiKey(refreshedCredentials.provider, refreshedCredentials), - newCredentials: refreshedCredentials, - }; - } - } - throw new Error( - `${externallyManaged.managedBy} credential is expired; refresh it in the external CLI and retry.`, - ); - } - if (cred.managedBy) { - throw new Error( - `${cred.managedBy} credential is unavailable; re-authenticate in the external CLI and retry.`, - ); } const pluginRefreshed = await withRefreshCallTimeout( @@ -683,7 +655,7 @@ async function doRefreshOAuthTokenWithLock(params: { ); if (pluginRefreshed) { const refreshedCredentials: OAuthCredential = { - ...cred, + ...clearExternalOAuthManager(cred), ...pluginRefreshed, type: "oauth", };