Skip to content

Commit ed546bd

Browse files
YangManBOBOsteipete
YangManBOBO
and
steipete
authored
fix(auth): expired OAuth credentials survive per-provider credential discovery and silently break background operations (#110678)
* fix(auth): reject expired OAuth credentials in provider credential discovery * test(auth): verify expired first profile is skipped for same-provider validation * fix(auth): prefer non-expired OAuth profile in per-provider credential map * fix(auth): use canonical profile order in discovery * docs(auth): document expired OAuth ordering * test(auth): use synthetic credential fixtures * test(auth): clarify resolved profile fixtures * test(auth): keep profile result names consistent * test(cli): relax ACP process deadlines under load * style(cli): format ACP process timeout --------- Co-authored-by: Peter Steinberger <[email protected]>
1 parent c2a9579 commit ed546bd

12 files changed

Lines changed: 242 additions & 25 deletions

File tree

‎docs/concepts/model-failover.md‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,10 @@ When a provider has multiple profiles, OpenClaw chooses an order like this:
129129
If no explicit order is configured, OpenClaw uses a round-robin order:
130130

131131
- **Primary key:** profile type (**OAuth, then static token, then API key**).
132-
- **Secondary key:** `usageStats.lastUsed` (oldest first, within each type).
132+
- **Secondary key for OAuth:** profiles with a currently usable access token before
133+
profiles whose access token is expired. Expired OAuth profiles stay eligible so
134+
the runtime can refresh them when no usable peer is available.
135+
- **Next key:** `usageStats.lastUsed` (oldest first, within each type/state tier).
133136
- **Cooldown/disabled profiles** are moved to the end, ordered by soonest expiry.
134137

135138
### Session stickiness (cache-friendly)

‎src/agents/agent-auth-credentials.ts‎

Lines changed: 27 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,10 @@
22
import { normalizeProviderId } from "@openclaw/model-catalog-core/provider-id";
33
import { asDateTimestampMs } from "@openclaw/normalization-core/number-coercion";
44
import { normalizeOptionalString } from "@openclaw/normalization-core/string-coerce";
5+
import type { OpenClawConfig } from "../config/types.openclaw.js";
56
import { coerceSecretRef } from "../config/types.secrets.js";
6-
import type { AuthProfileCredential, AuthProfileStore } from "./auth-profiles.js";
7+
import { resolveAuthProfileOrder } from "./auth-profiles/order.js";
8+
import type { AuthProfileCredential, AuthProfileStore } from "./auth-profiles/types.js";
79

810
// Converts auth-profile credentials into the compact credential map consumed by
911
// agent runtimes. Secret refs can be represented by markers without reading
@@ -22,6 +24,7 @@ export type AgentCredentialMap = Record<string, AgentCredential>;
2224

2325
type ResolveAgentCredentialMapOptions = {
2426
includeSecretRefPlaceholders?: boolean;
27+
config?: OpenClawConfig;
2528
};
2629

2730
const AGENT_SECRET_REF_CONFIGURED_MARKER = "openclaw-secret-ref-configured";
@@ -85,20 +88,38 @@ function convertAuthProfileCredentialToAgent(
8588
return null;
8689
}
8790

88-
/** Build one credential per normalized provider from an auth profile store. */
91+
/** Build one canonically selected credential per normalized provider. */
8992
export function resolveAgentCredentialMapFromStore(
9093
store: AuthProfileStore,
9194
options?: ResolveAgentCredentialMapOptions,
9295
): AgentCredentialMap {
9396
const credentials: AgentCredentialMap = {};
9497
for (const credential of Object.values(store.profiles)) {
9598
const provider = normalizeProviderId(credential.provider ?? "");
96-
if (!provider || credentials[provider]) {
99+
if (!provider) {
97100
continue;
98101
}
99-
const converted = convertAuthProfileCredentialToAgent(credential, options);
100-
if (converted) {
101-
credentials[provider] = converted;
102+
if (credentials[provider]) {
103+
continue;
104+
}
105+
// Discovery must not grow a second auth policy: explicit order, provider
106+
// aliases, eligibility, and automatic preference all belong to this resolver.
107+
const profileIds = resolveAuthProfileOrder({
108+
cfg: options?.config,
109+
store,
110+
provider,
111+
...(options?.includeSecretRefPlaceholders === true ? { readinessMode: "read-only" } : {}),
112+
});
113+
for (const profileId of profileIds) {
114+
const profile = store.profiles[profileId];
115+
if (!profile) {
116+
continue;
117+
}
118+
const converted = convertAuthProfileCredentialToAgent(profile, options);
119+
if (converted) {
120+
credentials[provider] = converted;
121+
break;
122+
}
102123
}
103124
}
104125
return credentials;

‎src/agents/agent-auth-discovery.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ export function resolveAgentCredentialsForDiscovery(
5252
const credentials = addEnvBackedAgentCredentials(
5353
resolveAgentCredentialMapFromStore(store, {
5454
includeSecretRefPlaceholders: options?.readOnly === true,
55+
config: options?.config,
5556
}),
5657
{
5758
config: options?.config,

‎src/agents/agent-model-discovery.auth.test.ts‎

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,96 @@ describe("discoverAuthStorage", () => {
130130
expect(credentials.openai).toBeUndefined();
131131
});
132132

133+
it("keeps expired OAuth when it is the sole profile for a provider", () => {
134+
const resolved = resolveAgentCredentialMapFromStore({
135+
version: 1,
136+
profiles: {
137+
"openai:sole-expired": {
138+
type: "oauth",
139+
provider: "openai",
140+
access: "fake",
141+
refresh: "sample",
142+
expires: Date.now() - 3600_000,
143+
},
144+
},
145+
});
146+
147+
expect(resolved.openai).toEqual({
148+
type: "oauth",
149+
access: "fake",
150+
refresh: "sample",
151+
expires: expect.any(Number),
152+
});
153+
});
154+
155+
it("uses canonical mode and expiry ordering instead of profile insertion order", () => {
156+
const resolved = resolveAgentCredentialMapFromStore({
157+
version: 1,
158+
profiles: {
159+
"openai:key": {
160+
type: "api_key",
161+
provider: "openai",
162+
key: "test-key",
163+
},
164+
"openai:expired": {
165+
type: "oauth",
166+
provider: "openai",
167+
access: "dummy",
168+
refresh: "placeholder",
169+
expires: Date.now() - 3600_000,
170+
},
171+
"openai:valid": {
172+
type: "oauth",
173+
provider: "openai",
174+
access: "fake",
175+
refresh: "sample",
176+
expires: Date.now() + 3600_000,
177+
},
178+
},
179+
});
180+
181+
expect(resolved.openai).toEqual({
182+
type: "oauth",
183+
access: "fake",
184+
refresh: "sample",
185+
expires: expect.any(Number),
186+
});
187+
});
188+
189+
it("passes configured auth order through discovery selection", async () => {
190+
await withAgentDir(async (agentDir) => {
191+
writeAuthProfilesSqlite(agentDir, {
192+
version: 1,
193+
profiles: {
194+
"openai:oauth": {
195+
type: "oauth",
196+
provider: "openai",
197+
access: "fake",
198+
refresh: "sample",
199+
expires: Date.now() + 3600_000,
200+
},
201+
"openai:key": {
202+
type: "api_key",
203+
provider: "openai",
204+
key: "test-key",
205+
},
206+
},
207+
});
208+
const authStorage = discoverAuthStorage(agentDir, {
209+
skipExternalAuthProfiles: true,
210+
env: {},
211+
config: {
212+
auth: { order: { openai: ["openai:key", "openai:oauth"] } },
213+
},
214+
});
215+
216+
expect(authStorage.get("openai")).toEqual({
217+
type: "api_key",
218+
key: "test-key",
219+
});
220+
});
221+
});
222+
133223
it("keeps keyRef and tokenRef profiles visible only for read-only agent discovery", () => {
134224
const credentials = resolveAgentCredentialMapFromStore({
135225
version: 1,

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

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -254,6 +254,79 @@ describe("resolveAuthProfileOrder", () => {
254254
expect(order).toStrictEqual(["fixture-provider:oauth", "fixture-provider:key"]);
255255
});
256256

257+
it.each([
258+
["expired first", ["openai:expired", "openai:valid"]],
259+
["valid first", ["openai:valid", "openai:expired"]],
260+
])("prefers live OAuth before expired OAuth when %s", (_caseName, profileIds) => {
261+
const now = Date.now();
262+
const profiles: AuthProfileStore["profiles"] = {
263+
"openai:expired": {
264+
type: "oauth",
265+
provider: "openai",
266+
access: "expired-access",
267+
refresh: "expired-refresh",
268+
expires: now - 60_000,
269+
},
270+
"openai:valid": {
271+
type: "oauth",
272+
provider: "openai",
273+
access: "valid-access",
274+
refresh: "valid-refresh",
275+
expires: now + 60_000,
276+
},
277+
};
278+
const orderedProfiles: AuthProfileStore["profiles"] = {};
279+
for (const profileId of profileIds) {
280+
const profile = profiles[profileId];
281+
if (profile) {
282+
orderedProfiles[profileId] = profile;
283+
}
284+
}
285+
286+
expect(
287+
resolveAuthProfileOrder({
288+
store: {
289+
version: 1,
290+
profiles: orderedProfiles,
291+
usageStats: {
292+
"openai:expired": { lastUsed: 0 },
293+
"openai:valid": { lastUsed: 10_000 },
294+
},
295+
},
296+
provider: "openai",
297+
}),
298+
).toStrictEqual(["openai:valid", "openai:expired"]);
299+
});
300+
301+
it("keeps an explicit order authoritative across OAuth expiry state", () => {
302+
const now = Date.now();
303+
const store: AuthProfileStore = {
304+
version: 1,
305+
profiles: {
306+
"openai:expired": {
307+
type: "oauth",
308+
provider: "openai",
309+
access: "expired-access",
310+
refresh: "expired-refresh",
311+
expires: now - 60_000,
312+
},
313+
"openai:valid": {
314+
type: "oauth",
315+
provider: "openai",
316+
access: "valid-access",
317+
refresh: "valid-refresh",
318+
expires: now + 60_000,
319+
},
320+
},
321+
order: { openai: ["openai:expired", "openai:valid"] },
322+
};
323+
324+
expect(resolveAuthProfileOrder({ store, provider: "openai" })).toStrictEqual([
325+
"openai:expired",
326+
"openai:valid",
327+
]);
328+
});
329+
257330
it("does not fall back past an explicit configured auth order", async () => {
258331
const store: AuthProfileStore = {
259332
version: 1,

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

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import {
1414
} from "../provider-auth-aliases.js";
1515
import {
1616
evaluateStoredCredentialEligibility,
17+
resolveTokenExpiryState,
1718
type AuthCredentialReasonCode,
1819
} from "./credential-state.js";
1920
import { dedupeProfileIds, listProfilesForProvider } from "./profile-list.js";
@@ -487,22 +488,31 @@ function orderProfilesByMode(
487488
}
488489
}
489490

490-
// Sort available profiles by type preference, then by lastUsed (oldest first = round-robin within type)
491+
// Sort by type, OAuth expiry state, then lastUsed for round-robin within each tier.
491492
const scored = available.map((profileId) => {
492-
const type = store.profiles[profileId]?.type;
493+
const profile = store.profiles[profileId];
494+
const type = profile?.type;
493495
const typeScore = type === "oauth" ? 0 : type === "token" ? 1 : type === "api_key" ? 2 : 3;
496+
// A refreshable expired OAuth profile remains eligible, but refreshing an
497+
// obsolete profile can rotate a one-time refresh token while a live peer exists.
498+
const expiryScore =
499+
profile?.type === "oauth" && resolveTokenExpiryState(profile.expires, now) === "expired"
500+
? 1
501+
: 0;
494502
const lastUsed = store.usageStats?.[profileId]?.lastUsed ?? 0;
495-
return { profileId, typeScore, lastUsed };
503+
return { profileId, typeScore, expiryScore, lastUsed };
496504
});
497505

498506
// Primary sort: type preference (oauth > token > api_key).
499-
// Secondary sort: lastUsed (oldest first for round-robin within type).
500507
const sorted = scored
501508
.toSorted((a, b) => {
502509
// First by type (oauth > token > api_key)
503510
if (a.typeScore !== b.typeScore) {
504511
return a.typeScore - b.typeScore;
505512
}
513+
if (a.expiryScore !== b.expiryScore) {
514+
return a.expiryScore - b.expiryScore;
515+
}
506516
// Then by lastUsed (oldest first)
507517
return a.lastUsed - b.lastUsed;
508518
})

‎src/agents/btw.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -488,7 +488,10 @@ async function resolveRuntimeModel(params: {
488488
}> {
489489
const modelsOptions = params.workspaceDir ? { workspaceDir: params.workspaceDir } : undefined;
490490
await ensureOpenClawModelsJson(params.cfg, params.agentDir, modelsOptions);
491-
const authStorage = discoverAuthStorage(params.agentDir);
491+
const authStorage = discoverAuthStorage(params.agentDir, {
492+
config: params.cfg,
493+
...(params.workspaceDir ? { workspaceDir: params.workspaceDir } : {}),
494+
});
492495
const modelRegistry = discoverModels(authStorage, params.agentDir, {
493496
config: params.cfg,
494497
...modelsOptions,

‎src/agents/embedded-agent-runner/model-discovery-cache.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,10 @@ function discoverFreshAgentStores(
142142
options: Pick<DiscoverCachedAgentStoresOptions, "config" | "workspaceDir">,
143143
pluginMetadataSnapshot: PluginMetadataSnapshot | undefined,
144144
): DiscoveryStores {
145-
const authStorage = discoverAuthStorage(agentDir);
145+
const authStorage = discoverAuthStorage(agentDir, {
146+
...(options.config ? { config: options.config } : {}),
147+
...(options.workspaceDir ? { workspaceDir: options.workspaceDir } : {}),
148+
});
146149
const modelRegistry = discoverModels(authStorage, agentDir, {
147150
...(options.config ? { config: options.config } : {}),
148151
...(pluginMetadataSnapshot ? { pluginMetadataSnapshot } : {}),

‎src/agents/embedded-agent-runner/model.ts‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1614,7 +1614,12 @@ export function resolveModel(
16141614
? discoverCachedAgentStoresForAgent(resolvedAgentDir, cfg, workspaceDir)
16151615
: undefined;
16161616
const authStorage =
1617-
options?.authStorage ?? cachedStores?.authStorage ?? discoverAuthStorage(resolvedAgentDir);
1617+
options?.authStorage ??
1618+
cachedStores?.authStorage ??
1619+
discoverAuthStorage(resolvedAgentDir, {
1620+
...(cfg ? { config: cfg } : {}),
1621+
...(workspaceDir ? { workspaceDir } : {}),
1622+
});
16181623
const modelRegistry =
16191624
options?.modelRegistry ??
16201625
cachedStores?.modelRegistry ??
@@ -1695,7 +1700,10 @@ export async function resolveModelAsync(
16951700
options?.authStorage ??
16961701
emptyDiscoveryStores?.authStorage ??
16971702
cachedStores?.authStorage ??
1698-
discoverAuthStorage(resolvedAgentDir);
1703+
discoverAuthStorage(resolvedAgentDir, {
1704+
...(cfg ? { config: cfg } : {}),
1705+
...(workspaceDir ? { workspaceDir } : {}),
1706+
});
16991707
const modelRegistry =
17001708
options?.modelRegistry ??
17011709
emptyDiscoveryStores?.modelRegistry ??

‎src/agents/model-catalog.ts‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -791,10 +791,11 @@ export async function loadModelCatalogSnapshot(
791791
logStage("agent-discovery-imported");
792792
const { buildShouldSuppressBuiltInModel } = await loadModelSuppression();
793793
logStage("catalog-deps-ready");
794-
const authStorage = agentDiscovery.discoverAuthStorage(
795-
agentDir,
796-
readOnly ? { readOnly: true } : undefined,
797-
);
794+
const authStorage = agentDiscovery.discoverAuthStorage(agentDir, {
795+
...(readOnly ? { readOnly: true } : {}),
796+
config: cfg,
797+
workspaceDir,
798+
});
798799
logStage("auth-storage-ready");
799800
const registry = agentDiscovery.discoverModels(authStorage, agentDir, {
800801
config: cfg,

0 commit comments

Comments
 (0)