From edc344834ba0f4e3b561c8919cf84bdd50ff8b8b Mon Sep 17 00:00:00 2001 From: Yuval Dinodia <102706514+yetval@users.noreply.github.com> Date: Thu, 23 Jul 2026 23:21:59 -0400 Subject: [PATCH] fix(auth-profiles): preserve secret refs and OAuth fields during doctor auth migration (#97881) Both legacy auth migration writers rebuilt each credential from a hardcoded field subset, dropping keyRef/tokenRef and the OAuth clientId/idToken/ chatgptPlanType, even though the canonical reader already preserves them. applyLegacyAuthStore now keeps the already-parsed credential as-is, and coerceLegacyFlatCredential delegates field extraction to the shared parseLegacyCredentialEntry while keeping its existing type and usability gates. The flat writer also no longer drops a secret-ref-only credential. This routes both migration paths through one normalization path so doctor --fix stops silently degrading or destroying credentials before removing the legacy files. --- .../auth-profiles/persisted-boundary.test.ts | 91 ++++++++++++++++++- src/agents/auth-profiles/persisted.ts | 43 +++------ .../doctor-auth-flat-profiles.test.ts | 49 ++++++++++ src/commands/doctor-auth-flat-profiles.ts | 55 +++-------- 4 files changed, 164 insertions(+), 74 deletions(-) diff --git a/src/agents/auth-profiles/persisted-boundary.test.ts b/src/agents/auth-profiles/persisted-boundary.test.ts index 7466b1c603d1..20932c3684b1 100644 --- a/src/agents/auth-profiles/persisted-boundary.test.ts +++ b/src/agents/auth-profiles/persisted-boundary.test.ts @@ -3,10 +3,19 @@ * Covers malformed credential coercion, state merging, legacy OAuth refs, and * main/agent store drift repair. */ -import { describe, expect, it } from "vitest"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { afterEach, describe, expect, it } from "vitest"; import { AUTH_STORE_VERSION } from "./constants.js"; import { resolveAuthProfileOrder } from "./order.js"; -import { coercePersistedAuthProfileStore, mergeAuthProfileStores } from "./persisted.js"; +import { + applyLegacyAuthStore, + coercePersistedAuthProfileStore, + loadLegacyAuthProfileStore, + mergeAuthProfileStores, +} from "./persisted.js"; +import type { AuthProfileStore } from "./types.js"; describe("persisted auth profile boundary", () => { it("normalizes malformed persisted credentials and state before runtime use", () => { @@ -420,3 +429,81 @@ describe("persisted auth profile boundary", () => { expect(merged.lastGood?.anthropic).toBe(profileId); }); }); + +describe("applyLegacyAuthStore", () => { + const agentDirs: string[] = []; + + afterEach(() => { + for (const dir of agentDirs.splice(0)) { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + function writeLegacyAuthJson(value: unknown): string { + const agentDir = fs.mkdtempSync(path.join(os.tmpdir(), "oc-legacy-auth-")); + agentDirs.push(agentDir); + fs.writeFileSync(path.join(agentDir, "auth.json"), JSON.stringify(value), "utf8"); + return agentDir; + } + + it("preserves OAuth refresh material when migrating legacy auth.json", () => { + const agentDir = writeLegacyAuthJson({ + chutes: { + type: "oauth", + provider: "chutes", + access: "ACCESS_TOKEN", + refresh: "REFRESH_TOKEN", + expires: 1_900_000_000_000, + clientId: "chutes-client-id-123", + idToken: "ID_TOKEN_xyz", + chatgptPlanType: "pro", + }, + }); + const legacy = loadLegacyAuthProfileStore(agentDir); + expect(legacy).not.toBeNull(); + + const store: AuthProfileStore = { version: AUTH_STORE_VERSION, profiles: {} }; + applyLegacyAuthStore(store, legacy ?? {}); + + expect(store.profiles["chutes:default"]).toMatchObject({ + type: "oauth", + provider: "chutes", + access: "ACCESS_TOKEN", + refresh: "REFRESH_TOKEN", + clientId: "chutes-client-id-123", + idToken: "ID_TOKEN_xyz", + chatgptPlanType: "pro", + }); + }); + + it("preserves secret-ref credentials when migrating legacy auth.json", () => { + const agentDir = writeLegacyAuthJson({ + openai: { + type: "api_key", + provider: "openai", + keyRef: { source: "env", id: "OPENAI_API_KEY" }, + }, + anthropic: { + type: "token", + provider: "anthropic", + tokenRef: { source: "env", id: "ANTHROPIC_TOKEN" }, + }, + }); + const legacy = loadLegacyAuthProfileStore(agentDir); + expect(legacy).not.toBeNull(); + + const store: AuthProfileStore = { version: AUTH_STORE_VERSION, profiles: {} }; + applyLegacyAuthStore(store, legacy ?? {}); + + expect(store.profiles["openai:default"]).toMatchObject({ + type: "api_key", + provider: "openai", + keyRef: { source: "env", id: "OPENAI_API_KEY" }, + }); + expect(store.profiles["anthropic:default"]).toMatchObject({ + type: "token", + provider: "anthropic", + tokenRef: { source: "env", id: "ANTHROPIC_TOKEN" }, + }); + }); +}); diff --git a/src/agents/auth-profiles/persisted.ts b/src/agents/auth-profiles/persisted.ts index bd880bc6d796..7e85a6cc858d 100644 --- a/src/agents/auth-profiles/persisted.ts +++ b/src/agents/auth-profiles/persisted.ts @@ -227,6 +227,15 @@ function parseCredentialEntry( }; } +/** Normalizes a single legacy credential entry into a canonical credential. */ +export function parseLegacyCredentialEntry( + raw: unknown, + fallbackProvider?: string, +): AuthProfileCredential | null { + const parsed = parseCredentialEntry(raw, fallbackProvider); + return parsed.ok ? parsed.credential : null; +} + function warnRejectedCredentialEntries(source: string, rejected: RejectedCredentialEntry[]): void { if (rejected.length === 0) { return; @@ -744,37 +753,9 @@ export function buildPersistedAuthProfileSecretsStore( /** Applies legacy auth.json credentials into an auth profile store. */ export function applyLegacyAuthStore(store: AuthProfileStore, legacy: LegacyAuthStore): void { for (const [provider, cred] of Object.entries(legacy)) { - const profileId = `${provider}:default`; - const credentialProvider = cred.provider ?? provider; - if (cred.type === "api_key") { - store.profiles[profileId] = { - type: "api_key", - provider: credentialProvider, - key: cred.key, - ...(cred.email ? { email: cred.email } : {}), - }; - continue; - } - if (cred.type === "token") { - store.profiles[profileId] = { - type: "token", - provider: credentialProvider, - token: cred.token, - ...(typeof cred.expires === "number" ? { expires: cred.expires } : {}), - ...(cred.email ? { email: cred.email } : {}), - }; - continue; - } - store.profiles[profileId] = { - type: "oauth", - provider: credentialProvider, - access: cred.access, - refresh: cred.refresh, - expires: cred.expires, - ...(cred.enterpriseUrl ? { enterpriseUrl: cred.enterpriseUrl } : {}), - ...(cred.projectId ? { projectId: cred.projectId } : {}), - ...(cred.accountId ? { accountId: cred.accountId } : {}), - ...(cred.email ? { email: cred.email } : {}), + store.profiles[`${provider}:default`] = { + ...cred, + provider: cred.provider ?? provider, }; } } diff --git a/src/commands/doctor-auth-flat-profiles.test.ts b/src/commands/doctor-auth-flat-profiles.test.ts index cbcb32fd85ef..a1515701604a 100644 --- a/src/commands/doctor-auth-flat-profiles.test.ts +++ b/src/commands/doctor-auth-flat-profiles.test.ts @@ -117,6 +117,55 @@ describe("maybeMigrateAuthProfileJsonStoresToSqlite", () => { expect(fs.existsSync(`${statePath}.sqlite-import.456.bak`)).toBe(true); }); + it("preserves secret refs and OAuth material when migrating a flat auth-profiles.json", async () => { + const state = await makeTestState(); + const authPath = await writeLegacyAuthProfilesJson(state, { + chutes: { + type: "oauth", + provider: "chutes", + access: "ACCESS_TOKEN", + refresh: "REFRESH_TOKEN", + expires: 1_900_000_000_000, + clientId: "chutes-client-id-123", + idToken: "ID_TOKEN_xyz", + chatgptPlanType: "pro", + }, + openai: { + type: "api_key", + provider: "openai", + keyRef: { source: "env", id: "OPENAI_API_KEY" }, + }, + }); + + const result = await maybeMigrateAuthProfileJsonStoresToSqlite({ + cfg: {}, + prompter: makePrompter(true), + now: () => 472, + }); + + expect(result.warnings).toStrictEqual([]); + expect(loadPersistedAuthProfileStore(state.agentDir())).toMatchObject({ + profiles: { + "chutes:default": { + type: "oauth", + provider: "chutes", + access: "ACCESS_TOKEN", + refresh: "REFRESH_TOKEN", + clientId: "chutes-client-id-123", + idToken: "ID_TOKEN_xyz", + chatgptPlanType: "pro", + }, + "openai:default": { + type: "api_key", + provider: "openai", + keyRef: { source: "env", id: "OPENAI_API_KEY" }, + }, + }, + }); + expect(fs.existsSync(authPath)).toBe(false); + expect(fs.existsSync(`${authPath}.sqlite-import.472.bak`)).toBe(true); + }); + it("moves legacy aws-sdk auth markers to config before removing JSON", async () => { const state = await makeTestState(); const cfg = {}; diff --git a/src/commands/doctor-auth-flat-profiles.ts b/src/commands/doctor-auth-flat-profiles.ts index 622a418f31b3..e774aa062df0 100644 --- a/src/commands/doctor-auth-flat-profiles.ts +++ b/src/commands/doctor-auth-flat-profiles.ts @@ -16,6 +16,7 @@ import { coercePersistedAuthProfileStore, loadLegacyAuthProfileStore, loadPersistedAuthProfileStore, + parseLegacyCredentialEntry, } from "../agents/auth-profiles/persisted.js"; import { coerceAuthProfileState } from "../agents/auth-profiles/state.js"; import { @@ -188,9 +189,15 @@ function inferLegacyCredentialType( if (readNonEmptyString(record.key) ?? readNonEmptyString(record.apiKey)) { return "api_key"; } + if (coerceSecretRef(record.keyRef)) { + return "api_key"; + } if (readNonEmptyString(record.token)) { return "token"; } + if (coerceSecretRef(record.tokenRef)) { + return "token"; + } if ( readNonEmptyString(record.access) && readNonEmptyString(record.refresh) && @@ -208,50 +215,16 @@ function coerceLegacyFlatCredential( if (!isRecord(raw)) { return null; } - const provider = readNonEmptyString(raw.provider) ?? providerId; const type = inferLegacyCredentialType(raw); - const email = readNonEmptyString(raw.email); - if (type === "api_key") { - const key = readNonEmptyString(raw.key) ?? readNonEmptyString(raw.apiKey); - return key ? { type, provider, key, ...(email ? { email } : {}) } : null; + if (!type) { + return null; } - if (type === "token") { - const token = readNonEmptyString(raw.token); - return token - ? { - type, - provider, - token, - ...(typeof raw.expires === "number" ? { expires: raw.expires } : {}), - ...(email ? { email } : {}), - } - : null; + const provider = readNonEmptyString(raw.provider) ?? providerId; + const credential = parseLegacyCredentialEntry({ ...raw, type, provider }, providerId); + if (!credential || !hasUsableAuthProfileCredential(credential)) { + return null; } - if (type === "oauth") { - const access = readNonEmptyString(raw.access); - const refresh = readNonEmptyString(raw.refresh); - if (!access || !refresh || typeof raw.expires !== "number") { - return null; - } - return { - type, - provider, - access, - refresh, - expires: raw.expires, - ...(readNonEmptyString(raw.enterpriseUrl) - ? { enterpriseUrl: readNonEmptyString(raw.enterpriseUrl) } - : {}), - ...(readNonEmptyString(raw.projectId) - ? { projectId: readNonEmptyString(raw.projectId) } - : {}), - ...(readNonEmptyString(raw.accountId) - ? { accountId: readNonEmptyString(raw.accountId) } - : {}), - ...(email ? { email } : {}), - }; - } - return null; + return credential; } function coerceLegacyFlatAuthProfileStore(raw: unknown): AuthProfileStore | null {