diff --git a/src/agents/auth-profiles/oauth-manager.ts b/src/agents/auth-profiles/oauth-manager.ts index a3263eed251..7a6d58ad17b 100644 --- a/src/agents/auth-profiles/oauth-manager.ts +++ b/src/agents/auth-profiles/oauth-manager.ts @@ -6,6 +6,7 @@ import { OAUTH_REFRESH_LOCK_OPTIONS, log, } from "./constants.js"; +import { shouldMirrorRefreshedOAuthCredential } from "./oauth-identity.js"; import { areOAuthCredentialsEquivalent, hasUsableOAuthCredential, @@ -18,7 +19,6 @@ import { shouldReplaceStoredOAuthCredential, type RuntimeExternalOAuthProfile, } from "./oauth-shared.js"; -import { shouldMirrorRefreshedOAuthCredential } from "./oauth-identity.js"; import { ensureAuthStoreFile, resolveAuthStorePath, resolveOAuthRefreshLockPath } from "./paths.js"; import { ensureAuthProfileStore, @@ -317,12 +317,6 @@ export function createOAuthManager(adapter: OAuthManagerAdapter) { } return false; } - if (existing && !isSafeToAdoptMainStoreOAuthIdentity(existing, params.refreshed)) { - log.warn("refused to mirror OAuth credential: identity mismatch or regression", { - profileId: params.profileId, - }); - return false; - } store.profiles[params.profileId] = { ...params.refreshed }; log.debug("mirrored refreshed OAuth credential to main agent store", { profileId: params.profileId, 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 b95fda28bf3..d4c352bf129 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 @@ -387,7 +387,7 @@ describe("resolveApiKeyForProfile openai-codex refresh fallback", () => { ); }); - it("adopts a fresher imported refresh token even when its access token is already expired", async () => { + it("keeps the canonical refresh token when imported Codex CLI state is expired", async () => { const profileId = "openai-codex:default"; saveAuthProfileStore( { @@ -415,8 +415,8 @@ describe("resolveApiKeyForProfile openai-codex refresh fallback", () => { refreshProviderOAuthCredentialWithPluginMock.mockImplementationOnce( async (params?: { context?: unknown }) => { expect(params?.context).toMatchObject({ - access: "newer-but-expired-cli-access-token", - refresh: "fresh-cli-refresh-token", + access: "expired-local-access-token", + refresh: "stale-local-refresh-token", }); return { type: "oauth", @@ -447,7 +447,7 @@ describe("resolveApiKeyForProfile openai-codex refresh fallback", () => { }); expect(persisted.profiles[profileId]).not.toEqual( expect.objectContaining({ - refresh: "stale-local-refresh-token", + refresh: "fresh-cli-refresh-token", }), ); }); diff --git a/src/agents/auth-profiles/oauth.ts b/src/agents/auth-profiles/oauth.ts index f637acf2458..b93b086bb9e 100644 --- a/src/agents/auth-profiles/oauth.ts +++ b/src/agents/auth-profiles/oauth.ts @@ -130,138 +130,6 @@ type ResolveApiKeyForProfileParams = { }; type SecretDefaults = NonNullable["defaults"]; -export function normalizeAuthIdentityToken(value: string | undefined): string | undefined { - const trimmed = value?.trim(); - return trimmed ? trimmed : undefined; -} - -export function normalizeAuthEmailToken(value: string | undefined): string | undefined { - return normalizeAuthIdentityToken(value)?.toLowerCase(); -} - -/** - * Returns true if `existing` and `incoming` provably belong to the same - * account. Used to gate cross-agent credential mirroring. - * - * The rule is intentionally strict to satisfy the CWE-284 model: - * 1. If one side carries identity metadata (accountId or email) and the - * other does not, refuse — we have no evidence they match. - * 2. If both sides carry identity, a shared field must match (accountId - * wins over email when both present). If the two sides carry identity - * in non-overlapping fields (one has only accountId, the other only - * email), refuse. - * 3. If neither side carries identity, return true: no evidence of - * mismatch and provider equality is checked separately by the caller. - * - * The previous permissive behaviour (fall back to `true` whenever a strict - * comparison could not be made) was unsafe: a sub-agent whose refreshed - * credential lacked identity metadata could overwrite a known-account main - * credential that had it, allowing cross-account poisoning through the - * mirror path. - */ -export function isSameOAuthIdentity( - existing: Pick, - incoming: Pick, -): boolean { - const aAcct = normalizeAuthIdentityToken(existing.accountId); - const bAcct = normalizeAuthIdentityToken(incoming.accountId); - const aEmail = normalizeAuthEmailToken(existing.email); - const bEmail = normalizeAuthEmailToken(incoming.email); - const aHasIdentity = aAcct !== undefined || aEmail !== undefined; - const bHasIdentity = bAcct !== undefined || bEmail !== undefined; - - // Asymmetric identity evidence — refuse. We cannot prove the two - // credentials belong to the same account. - if (aHasIdentity !== bHasIdentity) { - return false; - } - - // Both sides carry identity — require a positive match on a shared field. - if (aHasIdentity) { - if (aAcct !== undefined && bAcct !== undefined) { - return aAcct === bAcct; - } - if (aEmail !== undefined && bEmail !== undefined) { - return aEmail === bEmail; - } - // Identity metadata is present on both sides but in non-overlapping - // fields (one has accountId, the other has only email, or vice versa). - // No shared field to compare — refuse rather than guess. - return false; - } - - // Neither side carries identity metadata — provider equality is checked - // separately by the caller; no evidence of mismatch here. - return true; -} - -/** - * Identity gate used for both directions of credential copy: - * - mirror (sub-agent refresh -> main agent store) - * - adopt (main agent store -> sub-agent store) - * - * Rule: allow the copy iff - * 1. no positive identity mismatch — if both sides expose the same - * identity field (accountId or email), the values must match, AND - * 2. the incoming credential carries at least as much identity - * evidence as the existing one — if existing has accountId/email, - * incoming must carry the same field, AND - * 3. when both sides carry identity but in non-overlapping fields - * (existing has only accountId, incoming has only email, or vice - * versa) we cannot positively prove the same account and the copy - * is refused. - * - * Accepts: - * - matching accountId (positive match on strongest field) - * - matching email when accountId is absent on both sides - * - neither side carries identity (no evidence of mismatch) - * - existing has no identity, incoming has identity (UPGRADE: adds - * the marker without dropping anything) - * - * Refuses: - * - mismatching accountId or email on a shared field (CWE-284 core) - * - incoming drops an identity field present on existing (regression - * that would later let a wrong-account peer pass this gate) - * - non-overlapping fields (no comparable positive match) - * - * Design note: this is a single unified rule for both copy directions. - * The rule is deliberately one-sided because "existing" is whatever is - * about to be overwritten and "incoming" is the new data — the - * constraint is the same regardless of whether existing is main or sub. - */ -export function isSafeToCopyOAuthIdentity( - existing: Pick, - incoming: Pick, -): boolean { - const aAcct = normalizeAuthIdentityToken(existing.accountId); - const bAcct = normalizeAuthIdentityToken(incoming.accountId); - const aEmail = normalizeAuthEmailToken(existing.email); - const bEmail = normalizeAuthEmailToken(incoming.email); - - // (1) Positive match on a shared field, if one exists. - if (aAcct !== undefined && bAcct !== undefined) { - return aAcct === bAcct; - } - if (aEmail !== undefined && bEmail !== undefined) { - return aEmail === bEmail; - } - - // No shared comparable field beyond this point. - const aHasIdentity = aAcct !== undefined || aEmail !== undefined; - - // (2) Refuse if existing has any identity evidence that incoming lacks. - // That covers both the "drop" case (incoming has nothing) and the - // "non-overlapping fields" case (existing has accountId only, - // incoming has email only, or vice versa). - if (aHasIdentity) { - return false; - } - - // (3) Existing has no identity. Either incoming has none either - // (allowed: no evidence of mismatch) or incoming adds identity - // (allowed: pure upgrade, no loss). - return true; -} async function refreshOAuthCredential( credential: OAuthCredential,