diff --git a/src/config/index.ts b/src/config/index.ts index 00765f766..a8696abb8 100644 --- a/src/config/index.ts +++ b/src/config/index.ts @@ -135,16 +135,17 @@ function applyPersistedOAuthDefaults( // OAuth entries in settings.json carry no credentials; they are only usable // while a matching auth-store profile exists. Drop orphans in memory so a // removed profile does not pin resolution to an unauthenticatable provider. -function dropOrphanedOAuthEntries( +export function dropOrphanedOAuthEntries( settings: Settings | null, projected: Record, ): Settings | null { if (settings === null) return null; const providers = Object.fromEntries( Object.entries(settings.providers).filter( - ([name]) => + ([name, provider]) => (!isCodexProviderName(name) && !isXaiProviderName(name)) || - projected[name] !== undefined, + projected[name] !== undefined || + isHandNamedProviderEntry(provider), ), ); const { defaultProvider, ...rest } = settings; @@ -158,6 +159,35 @@ function dropOrphanedOAuthEntries( }; } +// A codex/ or xai/ settings row carrying its own credential is the +// operator's explicit config, not an OAuth placeholder: OAuth profile +// projections must never overwrite it, orphan-sweep it, or drop it from the +// catalog. Credential-less namespaced rows stay placeholders (CL-6728). +function isHandNamedProviderEntry( + entry: Pick | undefined, +): boolean { + if (entry === undefined) return false; + if (entry.keyless === true) return true; + return typeof entry.apiKey === "string" && entry.apiKey.length > 0; +} + +// Overlay live OAuth profile projections onto settings for runtime provider +// resolution. Exported for tests; loadConfig is the only production caller. +export function overlayOAuthProjections( + settings: Settings | null, + projected: Record, +): Settings | null { + if (Object.keys(projected).length === 0) return settings; + const providers = { ...(settings?.providers ?? {}) }; + for (const [name, entry] of Object.entries(projected)) { + if (!isHandNamedProviderEntry(providers[name])) providers[name] = entry; + } + return { + ...(settings ?? { providers: {} }), + providers, + }; +} + function hasExaEntry(servers: MCPServerSettingsEntry[] | undefined): boolean { return ( servers?.some((server) => server.name === EXA_MCP_SERVER_NAME) === true @@ -936,16 +966,10 @@ export async function loadConfig( const liveSettings = useOAuthProfiles ? dropOrphanedOAuthEntries(settings, projectedOAuthProviders) : settings; - const settingsForResolution: Settings | null = - Object.keys(projectedOAuthProviders).length > 0 - ? { - ...(liveSettings ?? { providers: {} }), - providers: { - ...(liveSettings?.providers ?? {}), - ...projectedOAuthProviders, - }, - } - : liveSettings; + const settingsForResolution: Settings | null = overlayOAuthProjections( + liveSettings, + projectedOAuthProviders, + ); // The per-repo selection file still applies on top of a --config source: that // file supplies provider definitions, while .corbits/settings.json supplies @@ -1194,15 +1218,33 @@ export function mergeOAuthCatalog( ? ["xai"] : []), ]); + const settingsRows = buildProviderCatalog(settings, resolved); + // A hand-named codex/ or xai/ API-key row is the operator's + // explicit config, not an OAuth placeholder: keep it and skip the colliding + // live profile projection instead of overwriting it (CL-6728). Read the raw + // settings rows only: buildProviderCatalog synthesizes a [resolved] row when + // settings is null/empty, and when resolved is itself codex/ that row + // carries the live apiKey with no profile marker — treating it as hand-named + // would eject the real marked entry for a stale token snapshot. + const handNamed = new Set( + Object.entries(settings?.providers ?? {}) + .filter( + ([name, provider]) => + (isCodexProviderName(name) || isXaiProviderName(name)) && + isHandNamedProviderEntry(provider), + ) + .map(([name]) => name), + ); return [ - ...buildProviderCatalog(settings, resolved).filter( + ...settingsRows.filter( (e) => - !isCodexProviderName(e.name) && - !isXaiProviderName(e.name) && - !dropBare.has(e.name), + handNamed.has(e.name) || + (!isCodexProviderName(e.name) && + !isXaiProviderName(e.name) && + !dropBare.has(e.name)), ), - ...codexEntries, - ...xaiEntries, + ...codexEntries.filter((e) => !handNamed.has(e.name)), + ...xaiEntries.filter((e) => !handNamed.has(e.name)), ].map((entry) => isOpenCodeGoProvider(entry) ? { ...entry, models: [...selectableGoModelIds()] } @@ -1272,11 +1314,20 @@ export function runtimeSettingsWithCatalog( if (settings === undefined) { return { providers: fromCatalog }; } + // OAuth-marked catalog rows carry live profile tokens; they overlay + // credential-less placeholders but never a hand-named API-key row (CL-6728). + const overlaid = Object.fromEntries( + Object.entries(fromCatalog).filter( + ([name]) => + (!isCodexProviderName(name) && !isXaiProviderName(name)) || + !isHandNamedProviderEntry(settings.providers[name]), + ), + ); return { ...settings, providers: { ...settings.providers, - ...fromCatalog, + ...overlaid, }, }; } diff --git a/src/config/oauth-catalog.test.ts b/src/config/oauth-catalog.test.ts index fb0356931..ae0021f1d 100644 --- a/src/config/oauth-catalog.test.ts +++ b/src/config/oauth-catalog.test.ts @@ -3,7 +3,21 @@ import type { CodexProfile } from "../auth/codex/store.js"; import { CODEX_BASE_URL } from "../auth/codex/constants.js"; import type { XaiProfile } from "../auth/xai/store.js"; import { XAI_BASE_URL } from "../auth/xai/constants.js"; -import { mergeOAuthCatalog } from "./index.js"; +import { + codexProfilesToCatalogEntries, + codexProvidersAsSettings, +} from "./codex-providers.js"; +import { + xaiProfilesToCatalogEntries, + xaiProvidersAsSettings, +} from "./xai-providers.js"; +import { + dropOrphanedOAuthEntries, + mergeOAuthCatalog, + overlayOAuthProjections, + providerCatalogToSettings, + runtimeSettingsWithCatalog, +} from "./index.js"; import type { ProviderSettings, ResolvedProvider, @@ -132,3 +146,257 @@ describe("mergeOAuthCatalog legacy bare-row dedupe (CL-5606)", () => { expect(merged.map((p) => p.name)).toEqual(["xai", "xai/work"]); }); }); + +describe("CL-6728: OAuth projections do not overwrite hand-named provider entries", () => { + const handNamed = (): ProviderSettings => ({ + baseURL: "https://hand-named.example.com/v1", + apiKey: "hand-named-key", + models: ["hand-model"], + }); + const liveMine: CodexProfile = { + name: "mine", + tokens: { access: "live-token", refresh: "r", expiresAt: 1 }, + createdAt: 0, + }; + const liveProjected = () => codexProvidersAsSettings([liveMine]); + const liveCatalog = () => codexProfilesToCatalogEntries([liveMine]); + + test("overlayOAuthProjections keeps a hand-named codex/ API-key entry", () => { + const overlaid = overlayOAuthProjections( + settingsWith({ "codex/mine": handNamed() }), + liveProjected(), + ); + expect(overlaid?.providers["codex/mine"]?.apiKey).toBe("hand-named-key"); + }); + + test("overlayOAuthProjections still applies the live token over a credential-less OAuth placeholder", () => { + const overlaid = overlayOAuthProjections( + settingsWith({ "codex/mine": codexEntry() }), + liveProjected(), + ); + expect(overlaid?.providers["codex/mine"]?.apiKey).toBe("live-token"); + }); + + test("dropOrphanedOAuthEntries never drops a hand-named API-key entry", () => { + const kept = dropOrphanedOAuthEntries( + settingsWith({ "codex/mine": handNamed() }), + {}, + ); + expect(kept?.providers["codex/mine"]?.apiKey).toBe("hand-named-key"); + }); + + test("dropOrphanedOAuthEntries still drops a credential-less orphan", () => { + const dropped = dropOrphanedOAuthEntries( + settingsWith({ "codex/mine": codexEntry() }), + {}, + ); + expect(dropped?.providers["codex/mine"]).toBeUndefined(); + }); + + test("runtimeSettingsWithCatalog keeps a hand-named codex/ API-key entry", () => { + const runtime = runtimeSettingsWithCatalog( + settingsWith({ "codex/mine": handNamed() }), + liveCatalog(), + ); + expect(runtime.providers["codex/mine"]?.apiKey).toBe("hand-named-key"); + }); + + test("runtimeSettingsWithCatalog still overlays the live token over a placeholder", () => { + const runtime = runtimeSettingsWithCatalog( + settingsWith({ "codex/mine": codexEntry() }), + liveCatalog(), + ); + expect(runtime.providers["codex/mine"]?.apiKey).toBe("live-token"); + }); + + test("mergeOAuthCatalog keeps a hand-named codex/ entry when its profile is live", () => { + const merged = mergeOAuthCatalog( + settingsWith({ "codex/mine": handNamed() }), + resolved, + [liveMine], + [], + ); + const rows = merged.filter((p) => p.name === "codex/mine"); + expect(rows).toHaveLength(1); + expect(rows[0]?.apiKey).toBe("hand-named-key"); + expect(rows[0]?.codexProfile).toBeUndefined(); + }); + + test("mergeOAuthCatalog keeps a hand-named codex/ entry with no live profile", () => { + const merged = mergeOAuthCatalog( + settingsWith({ "codex/mine": handNamed() }), + resolved, + [], + [], + ); + expect(merged.find((p) => p.name === "codex/mine")?.apiKey).toBe( + "hand-named-key", + ); + }); + + test("persist round-trip keeps the hand-named key and no login token", () => { + const merged = mergeOAuthCatalog( + settingsWith({ "codex/mine": handNamed() }), + resolved, + [liveMine], + [], + ); + const persisted = providerCatalogToSettings(merged, undefined); + expect(persisted.providers["codex/mine"]?.apiKey).toBe("hand-named-key"); + expect(JSON.stringify(persisted)).not.toContain("live-token"); + }); + + test("mergeOAuthCatalog keeps the marked live entry when settings is null and resolved is codex/", () => { + const resolvedCodexMine: ResolvedProvider = { + providerName: "codex/mine", + baseURL: CODEX_BASE_URL, + apiKey: "live-token", + model: "gpt-5.1-codex-max", + }; + const merged = mergeOAuthCatalog(null, resolvedCodexMine, [liveMine], []); + const rows = merged.filter((p) => p.name === "codex/mine"); + expect(rows).toHaveLength(1); + expect(rows[0]?.codexProfile).toBe("mine"); + expect(rows[0]?.apiKey).toBe("live-token"); + }); + + test("mergeOAuthCatalog keeps the marked live entry when settings is empty and resolved is codex/", () => { + const resolvedCodexMine: ResolvedProvider = { + providerName: "codex/mine", + baseURL: CODEX_BASE_URL, + apiKey: "live-token", + model: "gpt-5.1-codex-max", + }; + const merged = mergeOAuthCatalog( + settingsWith({}), + resolvedCodexMine, + [liveMine], + [], + ); + const rows = merged.filter((p) => p.name === "codex/mine"); + expect(rows).toHaveLength(1); + expect(rows[0]?.codexProfile).toBe("mine"); + expect(rows[0]?.apiKey).toBe("live-token"); + }); + + test("persist round-trip from the empty-settings merge contains no live token", () => { + const resolvedCodexMine: ResolvedProvider = { + providerName: "codex/mine", + baseURL: CODEX_BASE_URL, + apiKey: "live-token", + model: "gpt-5.1-codex-max", + }; + const merged = mergeOAuthCatalog(null, resolvedCodexMine, [liveMine], []); + const persisted = providerCatalogToSettings(merged, undefined); + expect(JSON.stringify(persisted)).not.toContain("live-token"); + }); + + test("mergeOAuthCatalog keeps a hand-named xai/ entry when its profile is live", () => { + const handNamedXai = (): ProviderSettings => ({ + baseURL: "https://hand-named-xai.example.com/v1", + apiKey: "hand-named-xai-key", + models: ["hand-xai-model"], + }); + const merged = mergeOAuthCatalog( + settingsWith({ "xai/work": handNamedXai() }), + resolved, + [], + [xaiWork], + ); + const rows = merged.filter((p) => p.name === "xai/work"); + expect(rows).toHaveLength(1); + expect(rows[0]?.apiKey).toBe("hand-named-xai-key"); + expect(rows[0]?.xaiProfile).toBeUndefined(); + }); + + test("overlayOAuthProjections keeps a hand-named xai/ API-key entry", () => { + const overlaid = overlayOAuthProjections( + settingsWith({ + "xai/work": { + baseURL: "https://hand-named-xai.example.com/v1", + apiKey: "hand-named-xai-key", + models: ["hand-xai-model"], + }, + }), + xaiProvidersAsSettings([xaiWork]), + ); + expect(overlaid?.providers["xai/work"]?.apiKey).toBe("hand-named-xai-key"); + }); + + test("mergeOAuthCatalog keeps a keyless codex/ entry when its profile is live", () => { + const merged = mergeOAuthCatalog( + settingsWith({ + "codex/mine": { + baseURL: "https://hand-named.example.com/v1", + keyless: true, + models: ["hand-model"], + }, + }), + resolved, + [liveMine], + [], + ); + const rows = merged.filter((p) => p.name === "codex/mine"); + expect(rows).toHaveLength(1); + expect(rows[0]?.keyless).toBe(true); + expect(rows[0]?.codexProfile).toBeUndefined(); + }); + + test("runtimeSettingsWithCatalog keeps a keyless codex/ entry", () => { + const runtime = runtimeSettingsWithCatalog( + settingsWith({ + "codex/mine": { + baseURL: "https://hand-named.example.com/v1", + keyless: true, + models: ["hand-model"], + }, + }), + liveCatalog(), + ); + expect(runtime.providers["codex/mine"]?.keyless).toBe(true); + expect(runtime.providers["codex/mine"]?.apiKey).toBeUndefined(); + }); + + test("overlayOAuthProjections treats a whitespace apiKey as hand-named", () => { + const overlaid = overlayOAuthProjections( + settingsWith({ + "codex/mine": { + baseURL: "https://hand-named.example.com/v1", + apiKey: " ", + models: ["hand-model"], + }, + }), + liveProjected(), + ); + expect(overlaid?.providers["codex/mine"]?.apiKey).toBe(" "); + }); + + test("overlayOAuthProjections treats an empty-string apiKey as a placeholder", () => { + const overlaid = overlayOAuthProjections( + settingsWith({ + "codex/mine": { + baseURL: "https://hand-named.example.com/v1", + apiKey: "", + models: ["hand-model"], + }, + }), + liveProjected(), + ); + expect(overlaid?.providers["codex/mine"]?.apiKey).toBe("live-token"); + }); + + test("runtimeSettingsWithCatalog resolves a keyless xai/ entry from the catalog", () => { + const runtime = runtimeSettingsWithCatalog( + settingsWith({ + "xai/work": { + baseURL: "https://hand-named-xai.example.com/v1", + keyless: true, + models: ["hand-xai-model"], + }, + }), + xaiProfilesToCatalogEntries([xaiWork]), + ); + expect(runtime.providers["xai/work"]?.keyless).toBe(true); + expect(runtime.providers["xai/work"]?.apiKey).toBeUndefined(); + }); +});