Skip to content

Commit 1bfae9d

Browse files
steipetesteipete
authored
fix(models): keep auth login out of main config
Store provider login profiles in auth-state, preserve configured auth order/profile constraints, and keep legacy credential/keyRef normalization durable. Fixes #88565.
1 parent 2b61d38 commit 1bfae9d

13 files changed

Lines changed: 554 additions & 57 deletions

‎src/agents/auth-profiles.resolve-auth-profile-order.does-not-prioritize-lastgood-round-robin-ordering.test.ts‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -478,6 +478,43 @@ describe("resolveAuthProfileOrder", () => {
478478
});
479479
expect(order).toEqual(["anthropic:work", "anthropic:default"]);
480480
});
481+
it("prefers store order over stale configured profiles", () => {
482+
const order = resolveAuthProfileOrder({
483+
cfg: {
484+
auth: {
485+
profiles: {
486+
"openai:old-login": {
487+
provider: "openai",
488+
mode: "oauth",
489+
},
490+
},
491+
},
492+
},
493+
store: {
494+
version: 1,
495+
order: { openai: ["openai:new-login", "openai:old-login"] },
496+
profiles: {
497+
"openai:new-login": {
498+
type: "oauth",
499+
provider: "openai",
500+
access: "new-access",
501+
refresh: "new-refresh",
502+
expires: Date.now() + 60_000,
503+
},
504+
"openai:old-login": {
505+
type: "oauth",
506+
provider: "openai",
507+
access: "old-access",
508+
refresh: "old-refresh",
509+
expires: Date.now() + 60_000,
510+
},
511+
},
512+
},
513+
provider: "openai",
514+
});
515+
516+
expect(order).toEqual(["openai:new-login", "openai:old-login"]);
517+
});
481518
it.each(["store", "config"] as const)(
482519
"pushes cooldown profiles to the end even with %s order",
483520
(orderSource) => {

‎src/agents/auth-profiles/order.test.ts‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -204,6 +204,63 @@ describe("resolveAuthProfileOrder", () => {
204204
expect(order).toStrictEqual([]);
205205
});
206206

207+
it("falls back to stored profiles when a stored order only has missing credentials", async () => {
208+
const store: AuthProfileStore = {
209+
version: 1,
210+
profiles: {
211+
"fixture-provider:key": {
212+
type: "api_key",
213+
provider: "fixture-provider",
214+
key: "sk-primary",
215+
},
216+
"fixture-provider:oauth": {
217+
type: "oauth",
218+
provider: "fixture-provider",
219+
access: "access-token",
220+
refresh: "refresh-token",
221+
expires: Date.now() + 60_000,
222+
},
223+
},
224+
order: {
225+
"fixture-provider": ["fixture-provider:deleted"],
226+
},
227+
};
228+
229+
const order = resolveAuthProfileOrder({
230+
store,
231+
provider: "fixture-provider",
232+
});
233+
234+
expect(order).toStrictEqual(["fixture-provider:oauth", "fixture-provider:key"]);
235+
});
236+
237+
it("does not fall back past an explicit configured auth order", async () => {
238+
const store: AuthProfileStore = {
239+
version: 1,
240+
profiles: {
241+
"fixture-provider:primary": {
242+
type: "api_key",
243+
provider: "fixture-provider",
244+
key: "sk-primary",
245+
},
246+
},
247+
};
248+
249+
const order = resolveAuthProfileOrder({
250+
cfg: {
251+
auth: {
252+
order: {
253+
"fixture-provider": ["fixture-provider:missing"],
254+
},
255+
},
256+
},
257+
store,
258+
provider: "fixture-provider",
259+
});
260+
261+
expect(order).toStrictEqual([]);
262+
});
263+
207264
it("lets Codex auth use friendly OpenAI auth order entries", async () => {
208265
const store: AuthProfileStore = {
209266
version: 1,

‎src/agents/auth-profiles/order.ts‎

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -246,6 +246,9 @@ export function resolveAuthProfileOrder(params: {
246246
: undefined;
247247
const directExplicitOrder = directStoredOrder ?? directConfiguredOrder;
248248
const aliasExplicitOrder = aliasStoredOrder ?? aliasConfiguredOrder;
249+
const explicitOrderFromStore =
250+
directStoredOrder !== undefined ||
251+
(directExplicitOrder === undefined && aliasStoredOrder !== undefined);
249252
const explicitProfiles = cfg?.auth?.profiles
250253
? Object.entries(cfg.auth.profiles)
251254
.filter(([profileId, profile]) =>
@@ -298,21 +301,27 @@ export function resolveAuthProfileOrder(params: {
298301
now,
299302
}).eligible;
300303
let filtered = baseOrder.filter(isValidProfile);
304+
let repairedFallbackToStoreProfiles = false;
301305

302-
// Repair config/store profile-id drift from older setup flows:
303-
// if configured profile ids no longer exist in auth-profiles.json, scan the
304-
// provider's stored credentials and use any valid entries.
306+
// Repair stored-order and config-profile drift from older setup flows:
307+
// bare config auth.order is a hard constraint, but configured profile ids
308+
// can drift from their stored credential ids and still need repair.
305309
const allBaseProfilesMissing = baseOrder.every((profileId) => !store.profiles[profileId]);
306-
if (filtered.length === 0 && explicitProfiles.length > 0 && allBaseProfilesMissing) {
310+
if (
311+
filtered.length === 0 &&
312+
allBaseProfilesMissing &&
313+
(explicitOrderFromStore || explicitProfiles.length > 0)
314+
) {
307315
filtered = storeProfiles.filter(isValidProfile);
316+
repairedFallbackToStoreProfiles = true;
308317
}
309318

310319
const deduped = dedupeProfileIds(filtered);
311320

312321
// If user specified explicit order (store override or config), respect it
313322
// exactly, but still apply cooldown sorting to avoid repeatedly selecting
314323
// known-bad/rate-limited keys as the first candidate.
315-
if (explicitOrder && explicitOrder.length > 0) {
324+
if (explicitOrder && explicitOrder.length > 0 && !repairedFallbackToStoreProfiles) {
316325
// ...but still respect cooldown tracking to avoid repeatedly selecting a
317326
// known-bad/rate-limited key as the first candidate.
318327
const available: string[] = [];

‎src/agents/auth-profiles/persisted-boundary.test.ts‎

Lines changed: 53 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,15 +8,26 @@ describe("persisted auth profile boundary", () => {
88
version: "not-a-version",
99
profiles: {
1010
"openai:default": {
11-
type: "api_key",
11+
type: "apiKey",
1212
provider: " OpenAI ",
13-
key: 42,
13+
apiKey: "demo-openai-key",
1414
keyRef: { source: "env", id: "OPENAI_API_KEY" },
1515
metadata: { account: "acct_123", bad: 123 },
1616
copyToAgents: "yes",
1717
email: ["wrong"],
1818
displayName: "Work",
1919
},
20+
"openai:legacy-api-key": {
21+
type: "apiKey",
22+
provider: "openai",
23+
apiKey: "legacy-openai-key",
24+
},
25+
"openai:legacy-malformed-ref": {
26+
type: "apiKey",
27+
provider: "openai",
28+
apiKey: "legacy-fallback-key",
29+
keyRef: { source: "env", id: "" },
30+
},
2031
"minimax:default": {
2132
type: "token",
2233
provider: "minimax",
@@ -70,6 +81,16 @@ describe("persisted auth profile boundary", () => {
7081
metadata: { account: "acct_123" },
7182
displayName: "Work",
7283
},
84+
"openai:legacy-api-key": {
85+
type: "api_key",
86+
provider: "openai",
87+
key: "legacy-openai-key",
88+
},
89+
"openai:legacy-malformed-ref": {
90+
type: "api_key",
91+
provider: "openai",
92+
key: "legacy-fallback-key",
93+
},
7394
"minimax:default": {
7495
type: "token",
7596
provider: "minimax",
@@ -98,7 +119,6 @@ describe("persisted auth profile boundary", () => {
98119
},
99120
});
100121
expect(store?.profiles["broken:array"]).toBeUndefined();
101-
expect(store?.profiles["openai:default"]).not.toHaveProperty("key");
102122
expect(store?.profiles["openai:default"]).not.toHaveProperty("copyToAgents");
103123
expect(store?.profiles["openai:oauth"]).not.toHaveProperty("oauthRef");
104124
});
@@ -194,6 +214,36 @@ describe("persisted auth profile boundary", () => {
194214
expect(merged.lastGood?.anthropic).toBe(profileId);
195215
});
196216

217+
it("preserves config-only order fallbacks during agent-store merges", () => {
218+
const merged = mergeAuthProfileStores(
219+
{
220+
version: AUTH_STORE_VERSION,
221+
profiles: {},
222+
order: {
223+
openai: ["openai:aws-sdk"],
224+
},
225+
},
226+
{
227+
version: AUTH_STORE_VERSION,
228+
profiles: {
229+
"openai:new-login": {
230+
type: "oauth",
231+
provider: "openai",
232+
access: "new-access",
233+
refresh: "new-refresh",
234+
expires: 1,
235+
},
236+
},
237+
order: {
238+
openai: ["openai:new-login", "openai:aws-sdk"],
239+
},
240+
},
241+
{ preserveBaseRuntimeExternalProfiles: true },
242+
);
243+
244+
expect(merged.order?.openai).toEqual(["openai:new-login", "openai:aws-sdk"]);
245+
});
246+
197247
it("preserves inherited base runtime external profiles during agent-store merges", () => {
198248
const profileId = "anthropic:claude-cli";
199249
const merged = mergeAuthProfileStores(

‎src/agents/auth-profiles/persisted.ts‎

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,14 @@ function normalizeRawCredentialEntry(raw: Record<string, unknown>): Partial<Auth
106106
if (!("type" in entry) && typeof entry["mode"] === "string") {
107107
entry["type"] = entry["mode"];
108108
}
109-
if (!("key" in entry) && typeof entry["apiKey"] === "string") {
109+
if (entry.type === "apiKey") {
110+
entry.type = "api_key";
111+
}
112+
if (
113+
!("key" in entry) &&
114+
!coerceSecretRef(entry["keyRef"]) &&
115+
typeof entry["apiKey"] === "string"
116+
) {
110117
entry["key"] = entry["apiKey"];
111118
}
112119
normalizeSecretBackedField({ entry, valueField: "key", refField: "keyRef" });
@@ -119,11 +126,10 @@ function normalizeRawCredentialEntry(raw: Record<string, unknown>): Partial<Auth
119126
const key = normalizeOptionalCredentialString(entry.key);
120127
const keyRef = coerceSecretRef(entry.keyRef);
121128
const metadata = normalizeCredentialMetadata(entry.metadata);
122-
if (key !== undefined) {
123-
normalized.key = key;
124-
}
125129
if (keyRef) {
126130
normalized.keyRef = keyRef;
131+
} else if (key !== undefined) {
132+
normalized.key = key;
127133
}
128134
if (metadata) {
129135
normalized.metadata = metadata;
@@ -524,7 +530,10 @@ export function mergeAuthProfileStores(
524530
Object.entries(mergedOrder)
525531
.map(([provider, profileIds]) => [
526532
provider,
527-
profileIds.filter((profileId) => profiles[profileId]),
533+
profileIds.filter(
534+
(profileId) =>
535+
profiles[profileId] || !removedRuntimeExternalProfileIds.has(profileId),
536+
),
528537
])
529538
.filter(([, profileIds]) => profileIds.length > 0),
530539
)

0 commit comments

Comments
 (0)