mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-06 08:50:43 +00:00
[codex] fix(auth): harden OAuth refresh and Codex CLI bootstrap flows (#68396)
* Harden OAuth refresh and Codex CLI bootstrap flows - Treat near-expiry OAuth credentials as unusable for bootstrap and refresh - Add clearer timeout and callback validation handling for OpenAI Codex OAuth - Tighten file lock retry behavior for stale OAuth refresh contention * fix(auth): address PR review threads * fix(auth): adopt fresher imported refresh tokens * test(auth): align oauth expiry fixtures with refresh margin * fix(auth): tighten Codex OAuth bootstrap and local fallback * Keep explicit local auth over CLI bootstrap - Preserve existing non-OAuth local profiles during external CLI OAuth sync - Add regression coverage for OpenAI Codex and generic external OAuth overlays * fix(auth): distinguish oauth lock timeout sources * fix(auth): reject cross-account external oauth bootstrap * fix(auth): narrow refresh contention classification
This commit is contained in:
@@ -1,2 +1,2 @@
|
||||
e5c3d6eb56304164434a8b29670a6f02490f359eb7a5f3f4034e61a1b8421c54 plugin-sdk-api-baseline.json
|
||||
14d07997a35902bbd3c94d528aafa53185bc66a0ddd9b518f87c85a352feb476 plugin-sdk-api-baseline.jsonl
|
||||
d08f0e793e66192fdbc377183ce0d94adcbec53cf334522bce8c0c457b90b0a8 plugin-sdk-api-baseline.json
|
||||
924f20b350a9f1997e95b3d7249cbb6720c9576c63e6c0c15cca0164734fd93d plugin-sdk-api-baseline.jsonl
|
||||
|
||||
@@ -87,7 +87,116 @@ describe("readOpenAICodexCliOAuthProfile", () => {
|
||||
provider: "openai-codex",
|
||||
access: "local-access",
|
||||
refresh: "local-refresh",
|
||||
expires: Date.now() + 10 * 60_000,
|
||||
},
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
expect(parsed).toBeNull();
|
||||
});
|
||||
|
||||
it("does not override explicit local non-oauth auth with Codex CLI bootstrap", () => {
|
||||
vi.spyOn(fs, "readFileSync").mockReturnValue(
|
||||
JSON.stringify({
|
||||
auth_mode: "chatgpt",
|
||||
tokens: {
|
||||
access_token: "access-token",
|
||||
refresh_token: "refresh-token",
|
||||
},
|
||||
}),
|
||||
);
|
||||
|
||||
const parsed = readOpenAICodexCliOAuthProfile({
|
||||
store: {
|
||||
version: 1,
|
||||
profiles: {
|
||||
[OPENAI_CODEX_DEFAULT_PROFILE_ID]: {
|
||||
type: "api_key",
|
||||
provider: "openai-codex",
|
||||
key: "sk-local",
|
||||
},
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
expect(parsed).toBeNull();
|
||||
});
|
||||
|
||||
it("allows Codex CLI bootstrap when the stored default is expired", () => {
|
||||
const accessToken = buildJwt({
|
||||
exp: Math.floor(Date.now() / 1000) + 600,
|
||||
"https://api.openai.com/profile": {
|
||||
email: "codex@example.com",
|
||||
},
|
||||
});
|
||||
vi.spyOn(fs, "readFileSync").mockReturnValue(
|
||||
JSON.stringify({
|
||||
auth_mode: "chatgpt",
|
||||
tokens: {
|
||||
access_token: accessToken,
|
||||
refresh_token: "refresh-token",
|
||||
account_id: "acct_123",
|
||||
},
|
||||
}),
|
||||
);
|
||||
|
||||
const parsed = readOpenAICodexCliOAuthProfile({
|
||||
store: {
|
||||
version: 1,
|
||||
profiles: {
|
||||
[OPENAI_CODEX_DEFAULT_PROFILE_ID]: {
|
||||
type: "oauth",
|
||||
provider: "openai-codex",
|
||||
access: "expired-local-access",
|
||||
refresh: "expired-local-refresh",
|
||||
expires: Date.now() - 60_000,
|
||||
},
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
expect(parsed).toMatchObject({
|
||||
profileId: OPENAI_CODEX_DEFAULT_PROFILE_ID,
|
||||
credential: {
|
||||
access: accessToken,
|
||||
refresh: "refresh-token",
|
||||
accountId: "acct_123",
|
||||
email: "codex@example.com",
|
||||
},
|
||||
});
|
||||
});
|
||||
|
||||
it("refuses Codex CLI bootstrap when an expired local default belongs to a different account", () => {
|
||||
const accessToken = buildJwt({
|
||||
exp: Math.floor(Date.now() / 1000) + 600,
|
||||
"https://api.openai.com/profile": {
|
||||
email: "codex-b@example.com",
|
||||
},
|
||||
});
|
||||
vi.spyOn(fs, "readFileSync").mockReturnValue(
|
||||
JSON.stringify({
|
||||
auth_mode: "chatgpt",
|
||||
tokens: {
|
||||
access_token: accessToken,
|
||||
refresh_token: "refresh-token",
|
||||
account_id: "acct_b",
|
||||
},
|
||||
}),
|
||||
);
|
||||
|
||||
const parsed = readOpenAICodexCliOAuthProfile({
|
||||
store: {
|
||||
version: 1,
|
||||
profiles: {
|
||||
[OPENAI_CODEX_DEFAULT_PROFILE_ID]: {
|
||||
type: "oauth",
|
||||
provider: "openai-codex",
|
||||
access: "near-expiry-local-access",
|
||||
refresh: "near-expiry-local-refresh",
|
||||
expires: Date.now() + 60_000,
|
||||
accountId: "acct_a",
|
||||
email: "codex-a@example.com",
|
||||
},
|
||||
},
|
||||
},
|
||||
|
||||
@@ -1,7 +1,11 @@
|
||||
import fs from "node:fs";
|
||||
import path from "node:path";
|
||||
import type { AuthProfileStore, OAuthCredential } from "openclaw/plugin-sdk/provider-auth";
|
||||
import { resolveRequiredHomeDir } from "openclaw/plugin-sdk/provider-auth";
|
||||
import {
|
||||
hasUsableOAuthCredential,
|
||||
resolveRequiredHomeDir,
|
||||
type AuthProfileStore,
|
||||
type OAuthCredential,
|
||||
} from "openclaw/plugin-sdk/provider-auth";
|
||||
import { createSubsystemLogger } from "openclaw/plugin-sdk/runtime-env";
|
||||
import {
|
||||
resolveCodexAccessTokenExpiry,
|
||||
@@ -76,6 +80,40 @@ function oauthCredentialMatches(a: OAuthCredential, b: OAuthCredential): boolean
|
||||
);
|
||||
}
|
||||
|
||||
function normalizeAuthIdentityToken(value: string | undefined): string | undefined {
|
||||
const trimmed = value?.trim();
|
||||
return trimmed ? trimmed : undefined;
|
||||
}
|
||||
|
||||
function normalizeAuthEmailToken(value: string | undefined): string | undefined {
|
||||
return normalizeAuthIdentityToken(value)?.toLowerCase();
|
||||
}
|
||||
|
||||
// Keep this overwrite guard aligned with the canonical OAuth identity-copy rule
|
||||
// in src/agents/auth-profiles/oauth.ts without widening the plugin SDK surface.
|
||||
function isSafeToReplaceStoredIdentity(
|
||||
existing: Pick<OAuthCredential, "accountId" | "email">,
|
||||
incoming: Pick<OAuthCredential, "accountId" | "email">,
|
||||
): boolean {
|
||||
const existingAccountId = normalizeAuthIdentityToken(existing.accountId);
|
||||
const incomingAccountId = normalizeAuthIdentityToken(incoming.accountId);
|
||||
const existingEmail = normalizeAuthEmailToken(existing.email);
|
||||
const incomingEmail = normalizeAuthEmailToken(incoming.email);
|
||||
|
||||
if (existingAccountId !== undefined && incomingAccountId !== undefined) {
|
||||
return existingAccountId === incomingAccountId;
|
||||
}
|
||||
if (existingEmail !== undefined && incomingEmail !== undefined) {
|
||||
return existingEmail === incomingEmail;
|
||||
}
|
||||
|
||||
const existingHasIdentity = existingAccountId !== undefined || existingEmail !== undefined;
|
||||
if (existingHasIdentity) {
|
||||
return false;
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
export function readOpenAICodexCliOAuthProfile(params: {
|
||||
env?: NodeJS.ProcessEnv;
|
||||
store: AuthProfileStore;
|
||||
@@ -104,7 +142,28 @@ export function readOpenAICodexCliOAuthProfile(params: {
|
||||
...(identity.profileName ? { displayName: identity.profileName } : {}),
|
||||
};
|
||||
const existing = params.store.profiles[OPENAI_CODEX_DEFAULT_PROFILE_ID];
|
||||
if (existing && (existing.type !== "oauth" || !oauthCredentialMatches(existing, credential))) {
|
||||
const existingOAuth =
|
||||
existing?.type === "oauth" && existing.provider === PROVIDER_ID ? existing : undefined;
|
||||
if (existing && !existingOAuth) {
|
||||
log.debug("kept explicit local auth over Codex CLI bootstrap", {
|
||||
profileId: OPENAI_CODEX_DEFAULT_PROFILE_ID,
|
||||
localType: existing.type,
|
||||
localProvider: existing.provider,
|
||||
});
|
||||
return null;
|
||||
}
|
||||
if (
|
||||
existingOAuth &&
|
||||
hasUsableOAuthCredential(existingOAuth) &&
|
||||
!oauthCredentialMatches(existingOAuth, credential)
|
||||
) {
|
||||
return null;
|
||||
}
|
||||
if (
|
||||
existingOAuth &&
|
||||
!oauthCredentialMatches(existingOAuth, credential) &&
|
||||
!isSafeToReplaceStoredIdentity(existingOAuth, credential)
|
||||
) {
|
||||
return null;
|
||||
}
|
||||
|
||||
|
||||
@@ -173,6 +173,63 @@ describe("buildAuthHealthSummary", () => {
|
||||
expect(profile?.expiresAt).toBe(now + DEFAULT_OAUTH_WARN_MS + 10_000);
|
||||
});
|
||||
|
||||
it("marks oauth as expiring when it falls within the shared refresh margin", () => {
|
||||
vi.spyOn(Date, "now").mockReturnValue(now);
|
||||
const store = {
|
||||
version: 1,
|
||||
profiles: {
|
||||
"openai-codex:default": {
|
||||
type: "oauth" as const,
|
||||
provider: "openai-codex",
|
||||
access: "near-expiry-access",
|
||||
refresh: "near-expiry-refresh",
|
||||
expires: now + 2 * 60_000,
|
||||
},
|
||||
},
|
||||
};
|
||||
|
||||
const summary = buildAuthHealthSummary({
|
||||
store,
|
||||
warnAfterMs: 60_000,
|
||||
});
|
||||
|
||||
const profile = summary.profiles.find((entry) => entry.profileId === "openai-codex:default");
|
||||
expect(profile?.status).toBe("expiring");
|
||||
});
|
||||
|
||||
it("prefers fresher imported external OAuth when the local credential is near expiry", () => {
|
||||
vi.spyOn(Date, "now").mockReturnValue(now);
|
||||
readCodexCliCredentialsCachedMock.mockReturnValue({
|
||||
type: "oauth",
|
||||
provider: "openai-codex",
|
||||
access: "fresh-cli-access",
|
||||
refresh: "fresh-cli-refresh",
|
||||
expires: now + DEFAULT_OAUTH_WARN_MS + 60_000,
|
||||
accountId: "acct-cli",
|
||||
});
|
||||
const store = {
|
||||
version: 1,
|
||||
profiles: {
|
||||
"openai-codex:default": {
|
||||
type: "oauth" as const,
|
||||
provider: "openai-codex",
|
||||
access: "near-expiry-local-access",
|
||||
refresh: "near-expiry-local-refresh",
|
||||
expires: now + 2 * 60_000,
|
||||
},
|
||||
},
|
||||
};
|
||||
|
||||
const summary = buildAuthHealthSummary({
|
||||
store,
|
||||
warnAfterMs: 60_000,
|
||||
});
|
||||
|
||||
const profile = summary.profiles.find((entry) => entry.profileId === "openai-codex:default");
|
||||
expect(profile?.status).toBe("ok");
|
||||
expect(profile?.expiresAt).toBe(now + DEFAULT_OAUTH_WARN_MS + 60_000);
|
||||
});
|
||||
|
||||
it("marks token profiles with invalid expires as missing with reason code", () => {
|
||||
vi.spyOn(Date, "now").mockReturnValue(now);
|
||||
const store = {
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import type { OpenClawConfig } from "../config/types.openclaw.js";
|
||||
import {
|
||||
DEFAULT_OAUTH_REFRESH_MARGIN_MS,
|
||||
type AuthCredentialReasonCode,
|
||||
evaluateStoredCredentialEligibility,
|
||||
resolveTokenExpiryState,
|
||||
@@ -79,16 +80,22 @@ export function formatRemainingShort(
|
||||
function resolveOAuthStatus(
|
||||
expiresAt: number | undefined,
|
||||
now: number,
|
||||
warnAfterMs: number,
|
||||
expiringWithinMs: number,
|
||||
): { status: AuthProfileHealthStatus; remainingMs?: number } {
|
||||
if (!expiresAt || !Number.isFinite(expiresAt) || expiresAt <= 0) {
|
||||
return { status: "missing" };
|
||||
}
|
||||
const remainingMs = expiresAt - now;
|
||||
if (remainingMs <= 0) {
|
||||
const expiryState = resolveTokenExpiryState(expiresAt, now, {
|
||||
expiringWithinMs,
|
||||
});
|
||||
if (expiryState === "invalid_expires" || expiryState === "missing") {
|
||||
return { status: "missing" };
|
||||
}
|
||||
if (expiryState === "expired") {
|
||||
return { status: "expired", remainingMs };
|
||||
}
|
||||
if (remainingMs <= warnAfterMs) {
|
||||
if (expiryState === "expiring") {
|
||||
return { status: "expiring", remainingMs };
|
||||
}
|
||||
return { status: "ok", remainingMs };
|
||||
@@ -166,10 +173,11 @@ function buildProfileHealth(params: {
|
||||
profileId,
|
||||
credential,
|
||||
});
|
||||
const oauthWarnAfterMs = Math.max(warnAfterMs, DEFAULT_OAUTH_REFRESH_MARGIN_MS);
|
||||
const { status: rawStatus, remainingMs } = resolveOAuthStatus(
|
||||
effectiveCredential.expires,
|
||||
now,
|
||||
warnAfterMs,
|
||||
oauthWarnAfterMs,
|
||||
);
|
||||
return {
|
||||
profileId,
|
||||
|
||||
@@ -22,7 +22,7 @@ function makeOAuthCredential(
|
||||
provider: overrides.provider,
|
||||
access: overrides.access ?? `${overrides.provider}-access`,
|
||||
refresh: overrides.refresh ?? `${overrides.provider}-refresh`,
|
||||
expires: overrides.expires ?? Date.now() + 60_000,
|
||||
expires: overrides.expires ?? Date.now() + 10 * 60_000,
|
||||
accountId: overrides.accountId,
|
||||
email: overrides.email,
|
||||
enterpriseUrl: overrides.enterpriseUrl,
|
||||
@@ -109,13 +109,13 @@ describe("external cli oauth resolution", () => {
|
||||
});
|
||||
|
||||
describe("external cli bootstrap policy", () => {
|
||||
it("treats only non-expired access tokens as usable local oauth", () => {
|
||||
it("treats only non-expired and non-near-expiry access tokens as usable local oauth", () => {
|
||||
expect(
|
||||
hasUsableOAuthCredential(
|
||||
makeOAuthCredential({
|
||||
provider: "openai-codex",
|
||||
access: "live-access",
|
||||
expires: Date.now() + 60_000,
|
||||
expires: Date.now() + 10 * 60_000,
|
||||
}),
|
||||
),
|
||||
).toBe(true);
|
||||
@@ -128,6 +128,15 @@ describe("external cli oauth resolution", () => {
|
||||
}),
|
||||
),
|
||||
).toBe(false);
|
||||
expect(
|
||||
hasUsableOAuthCredential(
|
||||
makeOAuthCredential({
|
||||
provider: "openai-codex",
|
||||
access: "near-expiry-access",
|
||||
expires: Date.now() + 60_000,
|
||||
}),
|
||||
),
|
||||
).toBe(false);
|
||||
expect(
|
||||
hasUsableOAuthCredential(
|
||||
makeOAuthCredential({
|
||||
@@ -153,7 +162,7 @@ describe("external cli oauth resolution", () => {
|
||||
provider: "openai-codex",
|
||||
access: "healthy-local-access",
|
||||
refresh: "healthy-local-refresh",
|
||||
expires: Date.now() + 60_000,
|
||||
expires: Date.now() + 10 * 60_000,
|
||||
}),
|
||||
imported,
|
||||
}),
|
||||
@@ -169,6 +178,40 @@ describe("external cli oauth resolution", () => {
|
||||
imported,
|
||||
}),
|
||||
).toBe(true);
|
||||
expect(
|
||||
shouldBootstrapFromExternalCliCredential({
|
||||
existing: makeOAuthCredential({
|
||||
provider: "openai-codex",
|
||||
access: "near-expiry-local-access",
|
||||
refresh: "near-expiry-local-refresh",
|
||||
expires: Date.now() + 60_000,
|
||||
}),
|
||||
imported,
|
||||
}),
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it("does not bootstrap across different known oauth identities", () => {
|
||||
const imported = makeOAuthCredential({
|
||||
provider: "openai-codex",
|
||||
access: "fresh-cli-access",
|
||||
refresh: "fresh-cli-refresh",
|
||||
expires: Date.now() + 5 * 24 * 60 * 60_000,
|
||||
accountId: "acct-external",
|
||||
});
|
||||
|
||||
expect(
|
||||
shouldBootstrapFromExternalCliCredential({
|
||||
existing: makeOAuthCredential({
|
||||
provider: "openai-codex",
|
||||
access: "expired-local-access",
|
||||
refresh: "expired-local-refresh",
|
||||
expires: Date.now() - 60_000,
|
||||
accountId: "acct-local",
|
||||
}),
|
||||
imported,
|
||||
}),
|
||||
).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -296,7 +339,7 @@ describe("external cli oauth resolution", () => {
|
||||
provider: "openai-codex",
|
||||
access: "healthy-local-access",
|
||||
refresh: "healthy-local-refresh",
|
||||
expires: Date.now() + 60_000,
|
||||
expires: Date.now() + 10 * 60_000,
|
||||
}),
|
||||
),
|
||||
);
|
||||
|
||||
@@ -33,9 +33,13 @@ export const AUTH_STORE_LOCK_OPTIONS = {
|
||||
// so a legitimate refresh's critical section always finishes well before
|
||||
// peers would treat the lock as reclaimable. Violating this invariant re-
|
||||
// introduces the `refresh_token_reused` race the lock is meant to prevent.
|
||||
//
|
||||
// Retry budget note: keep the MINIMUM cumulative retry window comfortably
|
||||
// above OAUTH_REFRESH_CALL_TIMEOUT_MS so waiters do not give up while a
|
||||
// legitimate slow refresh is still within its allowed runtime budget.
|
||||
export const OAUTH_REFRESH_LOCK_OPTIONS = {
|
||||
retries: {
|
||||
retries: 10,
|
||||
retries: 20,
|
||||
factor: 2,
|
||||
minTimeout: 100,
|
||||
maxTimeout: 10_000,
|
||||
|
||||
@@ -1,6 +1,8 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import {
|
||||
DEFAULT_OAUTH_REFRESH_MARGIN_MS,
|
||||
evaluateStoredCredentialEligibility,
|
||||
hasUsableOAuthCredential,
|
||||
resolveTokenExpiryState,
|
||||
} from "./credential-state.js";
|
||||
|
||||
@@ -25,6 +27,33 @@ describe("resolveTokenExpiryState", () => {
|
||||
it("returns valid when expires is in the future", () => {
|
||||
expect(resolveTokenExpiryState(now + 1, now)).toBe("valid");
|
||||
});
|
||||
|
||||
it("returns expiring when expires falls within the configured margin", () => {
|
||||
expect(
|
||||
resolveTokenExpiryState(now + DEFAULT_OAUTH_REFRESH_MARGIN_MS - 1, now, {
|
||||
expiringWithinMs: DEFAULT_OAUTH_REFRESH_MARGIN_MS,
|
||||
}),
|
||||
).toBe("expiring");
|
||||
});
|
||||
});
|
||||
|
||||
describe("hasUsableOAuthCredential", () => {
|
||||
const now = 1_700_000_000_000;
|
||||
|
||||
it("treats near-expiry oauth credentials as no longer usable", () => {
|
||||
expect(
|
||||
hasUsableOAuthCredential(
|
||||
{
|
||||
type: "oauth",
|
||||
provider: "openai-codex",
|
||||
access: "access-token",
|
||||
refresh: "refresh-token",
|
||||
expires: now + DEFAULT_OAUTH_REFRESH_MARGIN_MS - 1,
|
||||
},
|
||||
{ now },
|
||||
),
|
||||
).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("evaluateStoredCredentialEligibility", () => {
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import { coerceSecretRef, normalizeSecretInputString } from "../../config/types.secrets.js";
|
||||
import type { AuthProfileCredential } from "./types.js";
|
||||
import type { AuthProfileCredential, OAuthCredential } from "./types.js";
|
||||
|
||||
export type AuthCredentialReasonCode =
|
||||
| "ok"
|
||||
@@ -8,9 +8,17 @@ export type AuthCredentialReasonCode =
|
||||
| "expired"
|
||||
| "unresolved_ref";
|
||||
|
||||
export type TokenExpiryState = "missing" | "valid" | "expired" | "invalid_expires";
|
||||
export const DEFAULT_OAUTH_REFRESH_MARGIN_MS = 5 * 60 * 1000;
|
||||
|
||||
export function resolveTokenExpiryState(expires: unknown, now = Date.now()): TokenExpiryState {
|
||||
export type TokenExpiryState = "missing" | "valid" | "expiring" | "expired" | "invalid_expires";
|
||||
|
||||
export function resolveTokenExpiryState(
|
||||
expires: unknown,
|
||||
now = Date.now(),
|
||||
opts?: {
|
||||
expiringWithinMs?: number;
|
||||
},
|
||||
): TokenExpiryState {
|
||||
if (expires === undefined) {
|
||||
return "missing";
|
||||
}
|
||||
@@ -20,7 +28,37 @@ export function resolveTokenExpiryState(expires: unknown, now = Date.now()): Tok
|
||||
if (!Number.isFinite(expires) || expires <= 0) {
|
||||
return "invalid_expires";
|
||||
}
|
||||
return now >= expires ? "expired" : "valid";
|
||||
const remainingMs = expires - now;
|
||||
if (remainingMs <= 0) {
|
||||
return "expired";
|
||||
}
|
||||
const expiringWithinMs = Math.max(0, opts?.expiringWithinMs ?? 0);
|
||||
if (expiringWithinMs > 0 && remainingMs <= expiringWithinMs) {
|
||||
return "expiring";
|
||||
}
|
||||
return "valid";
|
||||
}
|
||||
|
||||
export function hasUsableOAuthCredential(
|
||||
credential: OAuthCredential | undefined,
|
||||
opts?: {
|
||||
now?: number;
|
||||
refreshMarginMs?: number;
|
||||
},
|
||||
): boolean {
|
||||
if (!credential || credential.type !== "oauth") {
|
||||
return false;
|
||||
}
|
||||
if (typeof credential.access !== "string" || credential.access.trim().length === 0) {
|
||||
return false;
|
||||
}
|
||||
const now = opts?.now ?? Date.now();
|
||||
const refreshMarginMs = Math.max(0, opts?.refreshMarginMs ?? DEFAULT_OAUTH_REFRESH_MARGIN_MS);
|
||||
return (
|
||||
resolveTokenExpiryState(credential.expires, now, {
|
||||
expiringWithinMs: refreshMarginMs,
|
||||
}) === "valid"
|
||||
);
|
||||
}
|
||||
|
||||
function hasConfiguredSecretRef(value: unknown): boolean {
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import { log } from "./constants.js";
|
||||
import {
|
||||
hasUsableOAuthCredential,
|
||||
isSafeToUseExternalCliCredential,
|
||||
readExternalCliBootstrapCredential,
|
||||
shouldBootstrapFromExternalCliCredential,
|
||||
} from "./external-cli-sync.js";
|
||||
@@ -26,6 +27,13 @@ export function resolveEffectiveOAuthCredential(params: {
|
||||
});
|
||||
return params.credential;
|
||||
}
|
||||
if (!isSafeToUseExternalCliCredential(params.credential, imported)) {
|
||||
log.warn("refused external cli oauth bootstrap: identity mismatch", {
|
||||
profileId: params.profileId,
|
||||
provider: params.credential.provider,
|
||||
});
|
||||
return params.credential;
|
||||
}
|
||||
const shouldBootstrap = shouldBootstrapFromExternalCliCredential({
|
||||
existing: params.credential,
|
||||
imported,
|
||||
|
||||
@@ -8,7 +8,7 @@ import {
|
||||
OPENAI_CODEX_DEFAULT_PROFILE_ID,
|
||||
} from "./constants.js";
|
||||
import { log } from "./constants.js";
|
||||
import { resolveTokenExpiryState } from "./credential-state.js";
|
||||
import { hasUsableOAuthCredential as hasUsableOAuthCredentialShared } from "./credential-state.js";
|
||||
import type { AuthProfileStore, OAuthCredential } from "./types.js";
|
||||
|
||||
export type ExternalCliResolvedProfile = {
|
||||
@@ -41,6 +41,46 @@ export function areOAuthCredentialsEquivalent(
|
||||
);
|
||||
}
|
||||
|
||||
function normalizeAuthIdentityToken(value: string | undefined): string | undefined {
|
||||
const trimmed = value?.trim();
|
||||
return trimmed ? trimmed : undefined;
|
||||
}
|
||||
|
||||
function normalizeAuthEmailToken(value: string | undefined): string | undefined {
|
||||
return normalizeAuthIdentityToken(value)?.toLowerCase();
|
||||
}
|
||||
|
||||
// Keep this gate aligned with the canonical identity-copy rule in oauth.ts.
|
||||
export function isSafeToUseExternalCliCredential(
|
||||
existing: OAuthCredential | undefined,
|
||||
imported: OAuthCredential,
|
||||
): boolean {
|
||||
if (!existing) {
|
||||
return true;
|
||||
}
|
||||
if (existing.provider !== imported.provider) {
|
||||
return false;
|
||||
}
|
||||
|
||||
const existingAccountId = normalizeAuthIdentityToken(existing.accountId);
|
||||
const importedAccountId = normalizeAuthIdentityToken(imported.accountId);
|
||||
const existingEmail = normalizeAuthEmailToken(existing.email);
|
||||
const importedEmail = normalizeAuthEmailToken(imported.email);
|
||||
|
||||
if (existingAccountId !== undefined && importedAccountId !== undefined) {
|
||||
return existingAccountId === importedAccountId;
|
||||
}
|
||||
if (existingEmail !== undefined && importedEmail !== undefined) {
|
||||
return existingEmail === importedEmail;
|
||||
}
|
||||
|
||||
const existingHasIdentity = existingAccountId !== undefined || existingEmail !== undefined;
|
||||
if (existingHasIdentity) {
|
||||
return false;
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
function hasNewerStoredOAuthCredential(
|
||||
existing: OAuthCredential | undefined,
|
||||
incoming: OAuthCredential,
|
||||
@@ -70,13 +110,7 @@ export function hasUsableOAuthCredential(
|
||||
credential: OAuthCredential | undefined,
|
||||
now = Date.now(),
|
||||
): boolean {
|
||||
if (!credential || credential.type !== "oauth") {
|
||||
return false;
|
||||
}
|
||||
if (typeof credential.access !== "string" || credential.access.trim().length === 0) {
|
||||
return false;
|
||||
}
|
||||
return resolveTokenExpiryState(credential.expires, now) === "valid";
|
||||
return hasUsableOAuthCredentialShared(credential, { now });
|
||||
}
|
||||
|
||||
export function shouldBootstrapFromExternalCliCredential(params: {
|
||||
@@ -85,6 +119,9 @@ export function shouldBootstrapFromExternalCliCredential(params: {
|
||||
now?: number;
|
||||
}): boolean {
|
||||
const now = params.now ?? Date.now();
|
||||
if (!isSafeToUseExternalCliCredential(params.existing, params.imported)) {
|
||||
return false;
|
||||
}
|
||||
if (hasUsableOAuthCredential(params.existing, now)) {
|
||||
return false;
|
||||
}
|
||||
@@ -144,7 +181,26 @@ export function resolveExternalCliAuthProfiles(
|
||||
continue;
|
||||
}
|
||||
const existing = store.profiles[providerConfig.profileId];
|
||||
const existingOAuth = existing?.type === "oauth" ? existing : undefined;
|
||||
const existingOAuth =
|
||||
existing?.type === "oauth" && existing.provider === providerConfig.provider
|
||||
? existing
|
||||
: undefined;
|
||||
if (existing && !existingOAuth) {
|
||||
log.debug("kept explicit local auth over external cli bootstrap", {
|
||||
profileId: providerConfig.profileId,
|
||||
provider: providerConfig.provider,
|
||||
localType: existing.type,
|
||||
localProvider: existing.provider,
|
||||
});
|
||||
continue;
|
||||
}
|
||||
if (existingOAuth && !isSafeToUseExternalCliCredential(existingOAuth, creds)) {
|
||||
log.warn("refused external cli oauth bootstrap: identity mismatch", {
|
||||
profileId: providerConfig.profileId,
|
||||
provider: providerConfig.provider,
|
||||
});
|
||||
continue;
|
||||
}
|
||||
if (
|
||||
!shouldBootstrapFromExternalCliCredential({
|
||||
existing: existingOAuth,
|
||||
|
||||
@@ -34,6 +34,11 @@ function createCredential(overrides: Partial<OAuthCredential> = {}): OAuthCreden
|
||||
};
|
||||
}
|
||||
|
||||
function createUsableOAuthExpiry(): number {
|
||||
// Keep fixtures comfortably outside the shared near-expiry refresh margin.
|
||||
return Date.now() + 30 * 60 * 1000;
|
||||
}
|
||||
|
||||
describe("auth external oauth helpers", () => {
|
||||
beforeEach(() => {
|
||||
resolveExternalAuthProfilesWithPluginsMock.mockReset();
|
||||
@@ -124,7 +129,7 @@ describe("auth external oauth helpers", () => {
|
||||
createCredential({
|
||||
access: "fresh-cli-access-token",
|
||||
refresh: "fresh-cli-refresh-token",
|
||||
expires: Date.now() + 60_000,
|
||||
expires: createUsableOAuthExpiry(),
|
||||
}),
|
||||
);
|
||||
|
||||
@@ -159,7 +164,7 @@ describe("auth external oauth helpers", () => {
|
||||
"openai-codex:default": createCredential({
|
||||
access: "healthy-local-access-token",
|
||||
refresh: "healthy-local-refresh-token",
|
||||
expires: Date.now() + 60_000,
|
||||
expires: createUsableOAuthExpiry(),
|
||||
}),
|
||||
}),
|
||||
);
|
||||
@@ -169,4 +174,58 @@ describe("auth external oauth helpers", () => {
|
||||
refresh: "healthy-local-refresh-token",
|
||||
});
|
||||
});
|
||||
|
||||
it("keeps explicit local non-oauth auth over external cli oauth overlays", () => {
|
||||
readCodexCliCredentialsCachedMock.mockReturnValue(
|
||||
createCredential({
|
||||
access: "fresh-cli-access-token",
|
||||
refresh: "fresh-cli-refresh-token",
|
||||
expires: Date.now() + 5 * 24 * 60 * 60_000,
|
||||
}),
|
||||
);
|
||||
|
||||
const overlaid = overlayExternalOAuthProfiles(
|
||||
createStore({
|
||||
"openai-codex:default": {
|
||||
type: "api_key",
|
||||
provider: "openai-codex",
|
||||
key: "sk-local",
|
||||
},
|
||||
}),
|
||||
);
|
||||
|
||||
expect(overlaid.profiles["openai-codex:default"]).toMatchObject({
|
||||
type: "api_key",
|
||||
provider: "openai-codex",
|
||||
key: "sk-local",
|
||||
});
|
||||
});
|
||||
|
||||
it("keeps expired local oauth when external cli belongs to a different account", () => {
|
||||
readCodexCliCredentialsCachedMock.mockReturnValue(
|
||||
createCredential({
|
||||
access: "fresh-cli-access-token",
|
||||
refresh: "fresh-cli-refresh-token",
|
||||
expires: createUsableOAuthExpiry(),
|
||||
accountId: "acct-external",
|
||||
}),
|
||||
);
|
||||
|
||||
const overlaid = overlayExternalOAuthProfiles(
|
||||
createStore({
|
||||
"openai-codex:default": createCredential({
|
||||
access: "expired-local-access-token",
|
||||
refresh: "expired-local-refresh-token",
|
||||
expires: Date.now() - 60_000,
|
||||
accountId: "acct-local",
|
||||
}),
|
||||
}),
|
||||
);
|
||||
|
||||
expect(overlaid.profiles["openai-codex:default"]).toMatchObject({
|
||||
access: "expired-local-access-token",
|
||||
refresh: "expired-local-refresh-token",
|
||||
accountId: "acct-local",
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -0,0 +1,208 @@
|
||||
import fs from "node:fs/promises";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { FILE_LOCK_TIMEOUT_ERROR_CODE, type FileLockTimeoutError } from "../../infra/file-lock.js";
|
||||
import { captureEnv } from "../../test-utils/env.js";
|
||||
import { resolveAuthStorePath, resolveOAuthRefreshLockPath } from "./paths.js";
|
||||
import { clearRuntimeAuthProfileStoreSnapshots, saveAuthProfileStore } from "./store.js";
|
||||
import type { AuthProfileStore, OAuthCredential } from "./types.js";
|
||||
|
||||
let resolveApiKeyForProfile: typeof import("./oauth.js").resolveApiKeyForProfile;
|
||||
let resetOAuthRefreshQueuesForTest: typeof import("./oauth.js").resetOAuthRefreshQueuesForTest;
|
||||
|
||||
async function loadOAuthModuleForTest() {
|
||||
({ resolveApiKeyForProfile, resetOAuthRefreshQueuesForTest } = await import("./oauth.js"));
|
||||
}
|
||||
|
||||
function resolveApiKeyForProfileInTest(
|
||||
params: Omit<Parameters<typeof resolveApiKeyForProfile>[0], "cfg">,
|
||||
) {
|
||||
return resolveApiKeyForProfile({ cfg: {}, ...params });
|
||||
}
|
||||
|
||||
const { withFileLockMock } = vi.hoisted(() => ({
|
||||
withFileLockMock: vi.fn(
|
||||
async <T>(_filePath: string, _options: unknown, run: () => Promise<T>) => await run(),
|
||||
),
|
||||
}));
|
||||
|
||||
vi.mock("../cli-credentials.js", () => ({
|
||||
readCodexCliCredentialsCached: () => null,
|
||||
readMiniMaxCliCredentialsCached: () => null,
|
||||
resetCliCredentialCachesForTest: () => undefined,
|
||||
writeCodexCliCredentials: () => true,
|
||||
}));
|
||||
|
||||
vi.mock("@mariozechner/pi-ai/oauth", () => ({
|
||||
getOAuthApiKey: vi.fn(async () => null),
|
||||
getOAuthProviders: () => [{ id: "openai-codex" }],
|
||||
}));
|
||||
|
||||
vi.mock("../../plugins/provider-runtime.runtime.js", () => ({
|
||||
formatProviderAuthProfileApiKeyWithPlugin: (params: { context?: { access?: string } }) =>
|
||||
params?.context?.access,
|
||||
refreshProviderOAuthCredentialWithPlugin: async () => undefined,
|
||||
}));
|
||||
|
||||
vi.mock("../../infra/file-lock.js", () => ({
|
||||
FILE_LOCK_TIMEOUT_ERROR_CODE: "file_lock_timeout",
|
||||
resetFileLockStateForTest: () => undefined,
|
||||
withFileLock: withFileLockMock,
|
||||
}));
|
||||
|
||||
vi.mock("../../plugin-sdk/file-lock.js", () => ({
|
||||
FILE_LOCK_TIMEOUT_ERROR_CODE: "file_lock_timeout",
|
||||
resetFileLockStateForTest: () => undefined,
|
||||
withFileLock: withFileLockMock,
|
||||
}));
|
||||
|
||||
vi.mock("./doctor.js", () => ({
|
||||
formatAuthDoctorHint: async () => undefined,
|
||||
}));
|
||||
|
||||
vi.mock("./external-auth.js", () => ({
|
||||
overlayExternalAuthProfiles: <T>(store: T) => store,
|
||||
shouldPersistExternalAuthProfile: () => true,
|
||||
}));
|
||||
|
||||
vi.mock("./external-cli-sync.js", async () => {
|
||||
const actual =
|
||||
await vi.importActual<typeof import("./external-cli-sync.js")>("./external-cli-sync.js");
|
||||
return {
|
||||
...actual,
|
||||
syncExternalCliCredentials: () => false,
|
||||
readManagedExternalCliCredential: () => null,
|
||||
resolveExternalCliAuthProfiles: () => [],
|
||||
areOAuthCredentialsEquivalent: (a: unknown, b: unknown) => a === b,
|
||||
};
|
||||
});
|
||||
|
||||
function createExpiredOauthStore(params: {
|
||||
profileId: string;
|
||||
provider: string;
|
||||
}): AuthProfileStore {
|
||||
return {
|
||||
version: 1,
|
||||
profiles: {
|
||||
[params.profileId]: {
|
||||
type: "oauth",
|
||||
provider: params.provider,
|
||||
access: "stale-access",
|
||||
refresh: "stale-refresh",
|
||||
expires: Date.now() - 60_000,
|
||||
} satisfies OAuthCredential,
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
function createLockTimeoutError(lockPath: string): FileLockTimeoutError {
|
||||
return Object.assign(new Error(`file lock timeout for ${lockPath.slice(0, -5)}`), {
|
||||
code: FILE_LOCK_TIMEOUT_ERROR_CODE as typeof FILE_LOCK_TIMEOUT_ERROR_CODE,
|
||||
lockPath,
|
||||
});
|
||||
}
|
||||
|
||||
describe("OAuth refresh lock timeout classification", () => {
|
||||
const envSnapshot = captureEnv([
|
||||
"OPENCLAW_STATE_DIR",
|
||||
"OPENCLAW_AGENT_DIR",
|
||||
"PI_CODING_AGENT_DIR",
|
||||
]);
|
||||
let tempRoot = "";
|
||||
let agentDir = "";
|
||||
|
||||
beforeEach(async () => {
|
||||
withFileLockMock.mockReset();
|
||||
withFileLockMock.mockImplementation(
|
||||
async <T>(_filePath: string, _options: unknown, run: () => Promise<T>) => await run(),
|
||||
);
|
||||
clearRuntimeAuthProfileStoreSnapshots();
|
||||
tempRoot = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-oauth-lock-timeout-"));
|
||||
process.env.OPENCLAW_STATE_DIR = tempRoot;
|
||||
agentDir = path.join(tempRoot, "agents", "main", "agent");
|
||||
process.env.OPENCLAW_AGENT_DIR = agentDir;
|
||||
process.env.PI_CODING_AGENT_DIR = agentDir;
|
||||
await fs.mkdir(agentDir, { recursive: true });
|
||||
await loadOAuthModuleForTest();
|
||||
resetOAuthRefreshQueuesForTest();
|
||||
});
|
||||
|
||||
afterEach(async () => {
|
||||
envSnapshot.restore();
|
||||
clearRuntimeAuthProfileStoreSnapshots();
|
||||
if (resetOAuthRefreshQueuesForTest) {
|
||||
resetOAuthRefreshQueuesForTest();
|
||||
}
|
||||
if (tempRoot) {
|
||||
await fs.rm(tempRoot, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("maps only global refresh lock timeouts to refresh_contention", async () => {
|
||||
const profileId = "openai-codex:default";
|
||||
const provider = "openai-codex";
|
||||
const store = createExpiredOauthStore({ profileId, provider });
|
||||
saveAuthProfileStore(store, agentDir);
|
||||
|
||||
const refreshLockPath = `${resolveOAuthRefreshLockPath(provider, profileId)}.lock`;
|
||||
withFileLockMock.mockImplementationOnce(async () => {
|
||||
throw createLockTimeoutError(refreshLockPath);
|
||||
});
|
||||
|
||||
try {
|
||||
await resolveApiKeyForProfileInTest({
|
||||
store,
|
||||
profileId,
|
||||
agentDir,
|
||||
});
|
||||
throw new Error("expected refresh contention error");
|
||||
} catch (error) {
|
||||
expect((error as Error).message).toContain("another process is already refreshing");
|
||||
expect((error as Error).message).toContain(
|
||||
"Please wait for the in-flight refresh to finish and retry.",
|
||||
);
|
||||
expect((error as Error & { cause?: unknown }).cause).toMatchObject({
|
||||
code: "refresh_contention",
|
||||
});
|
||||
expect(
|
||||
((error as Error & { cause?: { cause?: unknown } }).cause as { cause?: unknown }).cause,
|
||||
).toMatchObject({
|
||||
code: FILE_LOCK_TIMEOUT_ERROR_CODE,
|
||||
lockPath: refreshLockPath,
|
||||
});
|
||||
}
|
||||
});
|
||||
|
||||
it("preserves auth-store lock timeouts instead of remapping them to refresh_contention", async () => {
|
||||
const profileId = "openai-codex:default";
|
||||
const provider = "openai-codex";
|
||||
const store = createExpiredOauthStore({ profileId, provider });
|
||||
saveAuthProfileStore(store, agentDir);
|
||||
|
||||
const authStoreLockPath = `${resolveAuthStorePath(agentDir)}.lock`;
|
||||
withFileLockMock
|
||||
.mockImplementationOnce(
|
||||
async <T>(_filePath: string, _options: unknown, run: () => Promise<T>) => await run(),
|
||||
)
|
||||
.mockImplementationOnce(async () => {
|
||||
throw createLockTimeoutError(authStoreLockPath);
|
||||
});
|
||||
|
||||
try {
|
||||
await resolveApiKeyForProfileInTest({
|
||||
store,
|
||||
profileId,
|
||||
agentDir,
|
||||
});
|
||||
throw new Error("expected auth-store lock timeout");
|
||||
} catch (error) {
|
||||
expect((error as Error).message).toContain("file lock timeout");
|
||||
expect((error as Error).message).toContain("Please try again or re-authenticate.");
|
||||
expect((error as Error & { cause?: unknown }).cause).toMatchObject({
|
||||
code: FILE_LOCK_TIMEOUT_ERROR_CODE,
|
||||
lockPath: authStoreLockPath,
|
||||
});
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -1,6 +1,21 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { OAUTH_REFRESH_CALL_TIMEOUT_MS, OAUTH_REFRESH_LOCK_OPTIONS } from "./constants.js";
|
||||
|
||||
function computeMinimumRetryBudgetMs(): number {
|
||||
let total = 0;
|
||||
for (let attempt = 0; attempt < OAUTH_REFRESH_LOCK_OPTIONS.retries.retries; attempt += 1) {
|
||||
total += Math.min(
|
||||
OAUTH_REFRESH_LOCK_OPTIONS.retries.maxTimeout,
|
||||
Math.max(
|
||||
OAUTH_REFRESH_LOCK_OPTIONS.retries.minTimeout,
|
||||
OAUTH_REFRESH_LOCK_OPTIONS.retries.minTimeout *
|
||||
OAUTH_REFRESH_LOCK_OPTIONS.retries.factor ** attempt,
|
||||
),
|
||||
);
|
||||
}
|
||||
return total;
|
||||
}
|
||||
|
||||
// Invariant tests for the two constants that together bound the OAuth
|
||||
// refresh critical section. Behavioural tests for the inner `setTimeout`
|
||||
// mechanics are deliberately omitted: the implementation is a thin
|
||||
@@ -41,4 +56,10 @@ describe("OAuth refresh call timeout (invariants)", () => {
|
||||
// lock during a legitimate slow-but-successful refresh.
|
||||
expect(OAUTH_REFRESH_LOCK_OPTIONS.stale).toBeGreaterThan(60_000);
|
||||
});
|
||||
|
||||
it("OAUTH_REFRESH_LOCK_OPTIONS retry budget outlasts the refresh call timeout", () => {
|
||||
// Waiters should not exhaust their retry budget while a legitimate slow
|
||||
// refresh is still within its allowed runtime budget.
|
||||
expect(computeMinimumRetryBudgetMs()).toBeGreaterThan(OAUTH_REFRESH_CALL_TIMEOUT_MS);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -55,6 +55,10 @@ async function loadFreshOAuthModuleForTest() {
|
||||
({ resolveApiKeyForProfile } = await import("./oauth.js"));
|
||||
}
|
||||
|
||||
function createUsableOAuthExpiry(): number {
|
||||
return Date.now() + 30 * 60 * 1000;
|
||||
}
|
||||
|
||||
describe("resolveApiKeyForProfile fallback to main agent", () => {
|
||||
const envSnapshot = captureEnv([
|
||||
"OPENCLAW_STATE_DIR",
|
||||
@@ -139,7 +143,7 @@ describe("resolveApiKeyForProfile fallback to main agent", () => {
|
||||
provider: "anthropic",
|
||||
access: "oauth-token",
|
||||
refresh: "refresh-token",
|
||||
expires: Date.now() + 60_000,
|
||||
expires: createUsableOAuthExpiry(),
|
||||
},
|
||||
},
|
||||
};
|
||||
|
||||
@@ -87,6 +87,9 @@ function createExpiredOauthStore(params: {
|
||||
profileId: string;
|
||||
provider: string;
|
||||
access?: string;
|
||||
refresh?: string;
|
||||
accountId?: string;
|
||||
email?: string;
|
||||
}): AuthProfileStore {
|
||||
return {
|
||||
version: 1,
|
||||
@@ -95,8 +98,10 @@ function createExpiredOauthStore(params: {
|
||||
type: "oauth",
|
||||
provider: params.provider,
|
||||
access: params.access ?? "cached-access-token",
|
||||
refresh: "refresh-token",
|
||||
refresh: params.refresh ?? "refresh-token",
|
||||
expires: Date.now() - 60_000,
|
||||
accountId: params.accountId,
|
||||
email: params.email,
|
||||
},
|
||||
},
|
||||
};
|
||||
@@ -174,6 +179,46 @@ describe("resolveApiKeyForProfile openai-codex refresh fallback", () => {
|
||||
expect(refreshProviderOAuthCredentialWithPluginMock).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("refreshes near-expiry openai-codex credentials before hard expiry", async () => {
|
||||
const profileId = "openai-codex:default";
|
||||
saveAuthProfileStore(
|
||||
{
|
||||
version: 1,
|
||||
profiles: {
|
||||
[profileId]: {
|
||||
type: "oauth",
|
||||
provider: "openai-codex",
|
||||
access: "near-expiry-access-token",
|
||||
refresh: "near-expiry-refresh-token",
|
||||
expires: Date.now() + 60_000,
|
||||
},
|
||||
},
|
||||
},
|
||||
agentDir,
|
||||
);
|
||||
refreshProviderOAuthCredentialWithPluginMock.mockResolvedValueOnce({
|
||||
type: "oauth",
|
||||
provider: "openai-codex",
|
||||
access: "rotated-access-token",
|
||||
refresh: "rotated-refresh-token",
|
||||
expires: Date.now() + 86_400_000,
|
||||
accountId: "acct-rotated",
|
||||
});
|
||||
|
||||
const result = await resolveApiKeyForProfile({
|
||||
store: ensureAuthProfileStore(agentDir),
|
||||
profileId,
|
||||
agentDir,
|
||||
});
|
||||
|
||||
expect(result).toEqual({
|
||||
apiKey: "rotated-access-token",
|
||||
provider: "openai-codex",
|
||||
email: undefined,
|
||||
});
|
||||
expect(refreshProviderOAuthCredentialWithPluginMock).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("persists plugin-refreshed openai-codex credentials before returning", async () => {
|
||||
const profileId = "openai-codex:default";
|
||||
saveAuthProfileStore(
|
||||
@@ -320,6 +365,71 @@ describe("resolveApiKeyForProfile openai-codex refresh fallback", () => {
|
||||
);
|
||||
});
|
||||
|
||||
it("ignores mismatched fresh Codex CLI credentials when canonical local auth is bound to another account", async () => {
|
||||
const profileId = "openai-codex:default";
|
||||
saveAuthProfileStore(
|
||||
createExpiredOauthStore({
|
||||
profileId,
|
||||
provider: "openai-codex",
|
||||
access: "expired-local-access-token",
|
||||
refresh: "local-refresh-token",
|
||||
accountId: "acct-local",
|
||||
}),
|
||||
agentDir,
|
||||
);
|
||||
readCodexCliCredentialsCachedMock.mockReturnValueOnce({
|
||||
type: "oauth",
|
||||
provider: "openai-codex",
|
||||
access: "fresh-cli-access-token",
|
||||
refresh: "fresh-cli-refresh-token",
|
||||
expires: Date.now() + 86_400_000,
|
||||
accountId: "acct-external",
|
||||
});
|
||||
refreshProviderOAuthCredentialWithPluginMock.mockImplementationOnce(
|
||||
async (params?: { context?: unknown }) => {
|
||||
expect(params?.context).toMatchObject({
|
||||
access: "expired-local-access-token",
|
||||
refresh: "local-refresh-token",
|
||||
accountId: "acct-local",
|
||||
});
|
||||
return {
|
||||
type: "oauth",
|
||||
provider: "openai-codex",
|
||||
access: "fresh-local-access-token",
|
||||
refresh: "fresh-local-refresh-token",
|
||||
expires: Date.now() + 86_400_000,
|
||||
accountId: "acct-local",
|
||||
};
|
||||
},
|
||||
);
|
||||
|
||||
await expect(
|
||||
resolveApiKeyForProfile({
|
||||
store: ensureAuthProfileStore(agentDir),
|
||||
profileId,
|
||||
agentDir,
|
||||
}),
|
||||
).resolves.toEqual({
|
||||
apiKey: "fresh-local-access-token",
|
||||
provider: "openai-codex",
|
||||
email: undefined,
|
||||
});
|
||||
|
||||
const persisted = await readPersistedStore(agentDir);
|
||||
expect(persisted.profiles[profileId]).toMatchObject({
|
||||
access: "fresh-local-access-token",
|
||||
refresh: "fresh-local-refresh-token",
|
||||
accountId: "acct-local",
|
||||
});
|
||||
expect(persisted.profiles[profileId]).not.toEqual(
|
||||
expect.objectContaining({
|
||||
access: "fresh-cli-access-token",
|
||||
refresh: "fresh-cli-refresh-token",
|
||||
accountId: "acct-external",
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("keeps healthy local Codex OAuth over fresher imported CLI credentials", async () => {
|
||||
const profileId = "openai-codex:default";
|
||||
saveAuthProfileStore(
|
||||
@@ -331,7 +441,7 @@ describe("resolveApiKeyForProfile openai-codex refresh fallback", () => {
|
||||
provider: "openai-codex",
|
||||
access: "healthy-local-access-token",
|
||||
refresh: "healthy-local-refresh-token",
|
||||
expires: Date.now() + 60_000,
|
||||
expires: Date.now() + 10 * 60_000,
|
||||
},
|
||||
},
|
||||
},
|
||||
@@ -427,6 +537,135 @@ describe("resolveApiKeyForProfile openai-codex refresh fallback", () => {
|
||||
);
|
||||
});
|
||||
|
||||
it("adopts a fresher imported refresh token even when its access token is already expired", async () => {
|
||||
const profileId = "openai-codex:default";
|
||||
saveAuthProfileStore(
|
||||
{
|
||||
version: 1,
|
||||
profiles: {
|
||||
[profileId]: {
|
||||
type: "oauth",
|
||||
provider: "openai-codex",
|
||||
access: "expired-local-access-token",
|
||||
refresh: "stale-local-refresh-token",
|
||||
expires: Date.now() - 120_000,
|
||||
},
|
||||
},
|
||||
},
|
||||
agentDir,
|
||||
);
|
||||
readCodexCliCredentialsCachedMock.mockReturnValue({
|
||||
type: "oauth",
|
||||
provider: "openai-codex",
|
||||
access: "newer-but-expired-cli-access-token",
|
||||
refresh: "fresh-cli-refresh-token",
|
||||
expires: Date.now() - 30_000,
|
||||
accountId: "acct-cli",
|
||||
});
|
||||
refreshProviderOAuthCredentialWithPluginMock.mockImplementationOnce(
|
||||
async (params?: { context?: unknown }) => {
|
||||
expect(params?.context).toMatchObject({
|
||||
access: "newer-but-expired-cli-access-token",
|
||||
refresh: "fresh-cli-refresh-token",
|
||||
});
|
||||
return {
|
||||
type: "oauth",
|
||||
provider: "openai-codex",
|
||||
access: "fresh-access-token",
|
||||
refresh: "fresh-refresh-token",
|
||||
expires: Date.now() + 86_400_000,
|
||||
};
|
||||
},
|
||||
);
|
||||
|
||||
await expect(
|
||||
resolveApiKeyForProfile({
|
||||
store: ensureAuthProfileStore(agentDir),
|
||||
profileId,
|
||||
agentDir,
|
||||
}),
|
||||
).resolves.toEqual({
|
||||
apiKey: "fresh-access-token",
|
||||
provider: "openai-codex",
|
||||
email: undefined,
|
||||
});
|
||||
|
||||
const persisted = await readPersistedStore(agentDir);
|
||||
expect(persisted.profiles[profileId]).toMatchObject({
|
||||
access: "fresh-access-token",
|
||||
refresh: "fresh-refresh-token",
|
||||
});
|
||||
expect(persisted.profiles[profileId]).not.toEqual(
|
||||
expect.objectContaining({
|
||||
refresh: "stale-local-refresh-token",
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("does not use mismatched imported Codex CLI refresh state as refresh context", async () => {
|
||||
const profileId = "openai-codex:default";
|
||||
saveAuthProfileStore(
|
||||
createExpiredOauthStore({
|
||||
profileId,
|
||||
provider: "openai-codex",
|
||||
access: "expired-local-access-token",
|
||||
refresh: "local-refresh-token",
|
||||
accountId: "acct-local",
|
||||
}),
|
||||
agentDir,
|
||||
);
|
||||
readCodexCliCredentialsCachedMock.mockReturnValue({
|
||||
type: "oauth",
|
||||
provider: "openai-codex",
|
||||
access: "expired-cli-access-token",
|
||||
refresh: "external-refresh-token",
|
||||
expires: Date.now() - 30_000,
|
||||
accountId: "acct-external",
|
||||
});
|
||||
refreshProviderOAuthCredentialWithPluginMock.mockImplementationOnce(
|
||||
async (params?: { context?: unknown }) => {
|
||||
expect(params?.context).toMatchObject({
|
||||
access: "expired-local-access-token",
|
||||
refresh: "local-refresh-token",
|
||||
accountId: "acct-local",
|
||||
});
|
||||
return {
|
||||
type: "oauth",
|
||||
provider: "openai-codex",
|
||||
access: "fresh-local-access-token",
|
||||
refresh: "fresh-local-refresh-token",
|
||||
expires: Date.now() + 86_400_000,
|
||||
accountId: "acct-local",
|
||||
};
|
||||
},
|
||||
);
|
||||
|
||||
await expect(
|
||||
resolveApiKeyForProfile({
|
||||
store: ensureAuthProfileStore(agentDir),
|
||||
profileId,
|
||||
agentDir,
|
||||
}),
|
||||
).resolves.toEqual({
|
||||
apiKey: "fresh-local-access-token",
|
||||
provider: "openai-codex",
|
||||
email: undefined,
|
||||
});
|
||||
|
||||
const persisted = await readPersistedStore(agentDir);
|
||||
expect(persisted.profiles[profileId]).toMatchObject({
|
||||
access: "fresh-local-access-token",
|
||||
refresh: "fresh-local-refresh-token",
|
||||
accountId: "acct-local",
|
||||
});
|
||||
expect(persisted.profiles[profileId]).not.toEqual(
|
||||
expect.objectContaining({
|
||||
refresh: "external-refresh-token",
|
||||
accountId: "acct-external",
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("adopts fresher stored credentials after refresh_token_reused", async () => {
|
||||
const profileId = "openai-codex:default";
|
||||
saveAuthProfileStore(
|
||||
@@ -446,7 +685,7 @@ describe("resolveApiKeyForProfile openai-codex refresh fallback", () => {
|
||||
provider: "openai-codex",
|
||||
access: "reloaded-access-token",
|
||||
refresh: "reloaded-refresh-token",
|
||||
expires: Date.now() + 60_000,
|
||||
expires: Date.now() + 10 * 60_000,
|
||||
},
|
||||
},
|
||||
},
|
||||
@@ -510,7 +749,7 @@ describe("resolveApiKeyForProfile openai-codex refresh fallback", () => {
|
||||
newCredentials: {
|
||||
access: "retried-access-token",
|
||||
refresh: "retried-refresh-token",
|
||||
expires: Date.now() + 60_000,
|
||||
expires: Date.now() + 10 * 60_000,
|
||||
},
|
||||
};
|
||||
});
|
||||
|
||||
@@ -116,6 +116,10 @@ afterAll(() => {
|
||||
vi.doUnmock("../../plugins/provider-runtime.runtime.js");
|
||||
});
|
||||
|
||||
function createUsableOAuthExpiry(): number {
|
||||
return Date.now() + 30 * 60 * 1000;
|
||||
}
|
||||
|
||||
describe("resolveApiKeyForProfile config compatibility", () => {
|
||||
it("accepts token credentials when config mode is oauth", async () => {
|
||||
const profileId = "anthropic:token";
|
||||
@@ -183,7 +187,7 @@ describe("resolveApiKeyForProfile config compatibility", () => {
|
||||
provider: "anthropic",
|
||||
access: "access-123",
|
||||
refresh: "refresh-123",
|
||||
expires: Date.now() + 60_000,
|
||||
expires: createUsableOAuthExpiry(),
|
||||
},
|
||||
},
|
||||
};
|
||||
|
||||
@@ -8,7 +8,11 @@ import { loadConfig } from "../../config/config.js";
|
||||
import type { OpenClawConfig } from "../../config/types.openclaw.js";
|
||||
import { coerceSecretRef } from "../../config/types.secrets.js";
|
||||
import { formatErrorMessage } from "../../infra/errors.js";
|
||||
import { withFileLock } from "../../infra/file-lock.js";
|
||||
import {
|
||||
FILE_LOCK_TIMEOUT_ERROR_CODE,
|
||||
type FileLockTimeoutError,
|
||||
withFileLock,
|
||||
} from "../../infra/file-lock.js";
|
||||
import {
|
||||
formatProviderAuthProfileApiKeyWithPlugin,
|
||||
refreshProviderOAuthCredentialWithPlugin,
|
||||
@@ -28,6 +32,7 @@ import { resolveEffectiveOAuthCredential } from "./effective-oauth.js";
|
||||
import {
|
||||
areOAuthCredentialsEquivalent,
|
||||
hasUsableOAuthCredential,
|
||||
isSafeToUseExternalCliCredential,
|
||||
readExternalCliBootstrapCredential,
|
||||
shouldReplaceStoredOAuthCredential,
|
||||
} from "./external-cli-sync.js";
|
||||
@@ -286,6 +291,28 @@ async function withRefreshCallTimeout<T>(
|
||||
}
|
||||
}
|
||||
|
||||
function createOAuthRefreshContentionError(params: {
|
||||
profileId: string;
|
||||
provider: string;
|
||||
cause?: unknown;
|
||||
}): Error & { code: "refresh_contention" } {
|
||||
const error = new Error(
|
||||
`OAuth refresh failed (refresh_contention): another process is already refreshing ${params.provider} for ${params.profileId}`,
|
||||
{ cause: params.cause },
|
||||
);
|
||||
return Object.assign(error, { code: "refresh_contention" as const });
|
||||
}
|
||||
|
||||
function isGlobalOAuthRefreshLockTimeoutError(
|
||||
error: unknown,
|
||||
refreshLockPath: string,
|
||||
): error is FileLockTimeoutError {
|
||||
return (
|
||||
(error as { code?: string } | undefined)?.code === FILE_LOCK_TIMEOUT_ERROR_CODE &&
|
||||
(error as { lockPath?: string } | undefined)?.lockPath === `${refreshLockPath}.lock`
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Drop any in-flight entries in the module-level refresh queue. Intended
|
||||
* exclusively for tests that exercise the concurrent-refresh surface; a
|
||||
@@ -549,183 +576,207 @@ async function doRefreshOAuthTokenWithLock(params: {
|
||||
// paths only take the per-store lock, so no cycle is possible.
|
||||
const globalRefreshLockPath = resolveOAuthRefreshLockPath(params.provider, params.profileId);
|
||||
|
||||
return await withFileLock(globalRefreshLockPath, OAUTH_REFRESH_LOCK_OPTIONS, async () =>
|
||||
withFileLock(authPath, AUTH_STORE_LOCK_OPTIONS, async () => {
|
||||
// Locked refresh must bypass runtime snapshots so we can adopt fresher
|
||||
// on-disk credentials written by another refresh attempt.
|
||||
const store = loadAuthProfileStoreForSecretsRuntime(params.agentDir);
|
||||
const cred = store.profiles[params.profileId];
|
||||
if (!cred || cred.type !== "oauth") {
|
||||
return null;
|
||||
}
|
||||
try {
|
||||
return await withFileLock(globalRefreshLockPath, OAUTH_REFRESH_LOCK_OPTIONS, async () =>
|
||||
withFileLock(authPath, AUTH_STORE_LOCK_OPTIONS, async () => {
|
||||
// Locked refresh must bypass runtime snapshots so we can adopt fresher
|
||||
// on-disk credentials written by another refresh attempt.
|
||||
const store = loadAuthProfileStoreForSecretsRuntime(params.agentDir);
|
||||
const cred = store.profiles[params.profileId];
|
||||
if (!cred || cred.type !== "oauth") {
|
||||
return null;
|
||||
}
|
||||
|
||||
if (hasUsableOAuthCredential(cred)) {
|
||||
return {
|
||||
apiKey: await buildOAuthApiKey(cred.provider, cred),
|
||||
newCredentials: cred,
|
||||
};
|
||||
}
|
||||
if (hasUsableOAuthCredential(cred)) {
|
||||
return {
|
||||
apiKey: await buildOAuthApiKey(cred.provider, cred),
|
||||
newCredentials: cred,
|
||||
};
|
||||
}
|
||||
|
||||
// Inside-the-lock recheck: a prior agent that already held this lock may
|
||||
// have completed a refresh and mirrored its fresh credential into the
|
||||
// main store. If so, adopt into the local store and return without
|
||||
// issuing another HTTP refresh. This is what turns N serialized
|
||||
// refreshes into 1 refresh + (N-1) adoptions, preventing the
|
||||
// `refresh_token_reused` storm reported in #26322.
|
||||
if (params.agentDir) {
|
||||
try {
|
||||
const mainStore = loadAuthProfileStoreForSecretsRuntime(undefined);
|
||||
const mainCred = mainStore.profiles[params.profileId];
|
||||
if (
|
||||
mainCred?.type === "oauth" &&
|
||||
mainCred.provider === cred.provider &&
|
||||
hasUsableOAuthCredential(mainCred) &&
|
||||
// Defense-in-depth identity gate. Tolerates the pure upgrade
|
||||
// case (sub predates identity capture) but refuses positive
|
||||
// mismatch, identity regression, and non-overlapping fields.
|
||||
isSafeToCopyOAuthIdentity(cred, mainCred)
|
||||
) {
|
||||
store.profiles[params.profileId] = { ...mainCred };
|
||||
saveAuthProfileStore(store, params.agentDir);
|
||||
log.info("adopted fresh OAuth credential from main store (under refresh lock)", {
|
||||
// Inside-the-lock recheck: a prior agent that already held this lock may
|
||||
// have completed a refresh and mirrored its fresh credential into the
|
||||
// main store. If so, adopt into the local store and return without
|
||||
// issuing another HTTP refresh. This is what turns N serialized
|
||||
// refreshes into 1 refresh + (N-1) adoptions, preventing the
|
||||
// `refresh_token_reused` storm reported in #26322.
|
||||
if (params.agentDir) {
|
||||
try {
|
||||
const mainStore = loadAuthProfileStoreForSecretsRuntime(undefined);
|
||||
const mainCred = mainStore.profiles[params.profileId];
|
||||
if (
|
||||
mainCred?.type === "oauth" &&
|
||||
mainCred.provider === cred.provider &&
|
||||
hasUsableOAuthCredential(mainCred) &&
|
||||
// Defense-in-depth identity gate. Tolerates the pure upgrade
|
||||
// case (sub predates identity capture) but refuses positive
|
||||
// mismatch, identity regression, and non-overlapping fields.
|
||||
isSafeToCopyOAuthIdentity(cred, mainCred)
|
||||
) {
|
||||
store.profiles[params.profileId] = { ...mainCred };
|
||||
saveAuthProfileStore(store, params.agentDir);
|
||||
log.info("adopted fresh OAuth credential from main store (under refresh lock)", {
|
||||
profileId: params.profileId,
|
||||
agentDir: params.agentDir,
|
||||
expires: new Date(mainCred.expires).toISOString(),
|
||||
});
|
||||
return {
|
||||
apiKey: await buildOAuthApiKey(mainCred.provider, mainCred),
|
||||
newCredentials: mainCred,
|
||||
};
|
||||
} else if (
|
||||
mainCred?.type === "oauth" &&
|
||||
mainCred.provider === cred.provider &&
|
||||
hasUsableOAuthCredential(mainCred) &&
|
||||
!isSafeToCopyOAuthIdentity(cred, mainCred)
|
||||
) {
|
||||
// Main has fresh creds but they belong to a DIFFERENT account —
|
||||
// record the refusal so operators can diagnose, then proceed to
|
||||
// our own refresh rather than leaking credentials.
|
||||
log.warn("refused to adopt fresh main-store OAuth credential: identity mismatch", {
|
||||
profileId: params.profileId,
|
||||
agentDir: params.agentDir,
|
||||
});
|
||||
}
|
||||
} catch (err) {
|
||||
log.debug("inside-lock main-store adoption failed; proceeding to refresh", {
|
||||
profileId: params.profileId,
|
||||
agentDir: params.agentDir,
|
||||
expires: new Date(mainCred.expires).toISOString(),
|
||||
error: formatErrorMessage(err),
|
||||
});
|
||||
return {
|
||||
apiKey: await buildOAuthApiKey(mainCred.provider, mainCred),
|
||||
newCredentials: mainCred,
|
||||
};
|
||||
} else if (
|
||||
mainCred?.type === "oauth" &&
|
||||
mainCred.provider === cred.provider &&
|
||||
hasUsableOAuthCredential(mainCred) &&
|
||||
!isSafeToCopyOAuthIdentity(cred, mainCred)
|
||||
) {
|
||||
// Main has fresh creds but they belong to a DIFFERENT account —
|
||||
// record the refusal so operators can diagnose, then proceed to
|
||||
// our own refresh rather than leaking credentials.
|
||||
log.warn("refused to adopt fresh main-store OAuth credential: identity mismatch", {
|
||||
}
|
||||
}
|
||||
|
||||
const externallyManaged = readExternalCliBootstrapCredential({
|
||||
profileId: params.profileId,
|
||||
credential: cred,
|
||||
});
|
||||
let refreshCred = cred;
|
||||
if (externallyManaged) {
|
||||
if (isSafeToUseExternalCliCredential(cred, externallyManaged)) {
|
||||
const hasUsableExternalCredential = hasUsableOAuthCredential(externallyManaged);
|
||||
const shouldAdoptExternalCredential =
|
||||
shouldReplaceStoredOAuthCredential(cred, externallyManaged) &&
|
||||
!areOAuthCredentialsEquivalent(cred, externallyManaged);
|
||||
if (shouldAdoptExternalCredential) {
|
||||
store.profiles[params.profileId] = externallyManaged;
|
||||
saveAuthProfileStore(store, params.agentDir);
|
||||
refreshCred = externallyManaged;
|
||||
}
|
||||
if (hasUsableExternalCredential) {
|
||||
return {
|
||||
apiKey: await buildOAuthApiKey(externallyManaged.provider, externallyManaged),
|
||||
newCredentials: externallyManaged,
|
||||
};
|
||||
}
|
||||
} else {
|
||||
log.warn("refused to adopt external cli OAuth credential: identity mismatch", {
|
||||
profileId: params.profileId,
|
||||
provider: params.provider,
|
||||
agentDir: params.agentDir,
|
||||
});
|
||||
}
|
||||
} catch (err) {
|
||||
log.debug("inside-lock main-store adoption failed; proceeding to refresh", {
|
||||
profileId: params.profileId,
|
||||
error: formatErrorMessage(err),
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
const externallyManaged = readExternalCliBootstrapCredential({
|
||||
profileId: params.profileId,
|
||||
credential: cred,
|
||||
});
|
||||
if (externallyManaged) {
|
||||
if (
|
||||
shouldReplaceStoredOAuthCredential(cred, externallyManaged) &&
|
||||
!areOAuthCredentialsEquivalent(cred, externallyManaged)
|
||||
) {
|
||||
store.profiles[params.profileId] = externallyManaged;
|
||||
const pluginRefreshed = await withRefreshCallTimeout(
|
||||
`refreshProviderOAuthCredentialWithPlugin(${refreshCred.provider})`,
|
||||
OAUTH_REFRESH_CALL_TIMEOUT_MS,
|
||||
() =>
|
||||
refreshProviderOAuthCredentialWithPlugin({
|
||||
provider: refreshCred.provider,
|
||||
context: refreshCred,
|
||||
}),
|
||||
);
|
||||
if (pluginRefreshed) {
|
||||
const refreshedCredentials: OAuthCredential = {
|
||||
...refreshCred,
|
||||
...pluginRefreshed,
|
||||
type: "oauth",
|
||||
};
|
||||
store.profiles[params.profileId] = refreshedCredentials;
|
||||
saveAuthProfileStore(store, params.agentDir);
|
||||
}
|
||||
if (hasUsableOAuthCredential(externallyManaged)) {
|
||||
if (params.agentDir) {
|
||||
const mainPath = resolveAuthStorePath(undefined);
|
||||
if (mainPath !== authPath) {
|
||||
await mirrorRefreshedCredentialIntoMainStore({
|
||||
profileId: params.profileId,
|
||||
refreshed: refreshedCredentials,
|
||||
});
|
||||
}
|
||||
}
|
||||
return {
|
||||
apiKey: await buildOAuthApiKey(externallyManaged.provider, externallyManaged),
|
||||
newCredentials: externallyManaged,
|
||||
apiKey: await buildOAuthApiKey(cred.provider, refreshedCredentials),
|
||||
newCredentials: refreshedCredentials,
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
const pluginRefreshed = await withRefreshCallTimeout(
|
||||
`refreshProviderOAuthCredentialWithPlugin(${cred.provider})`,
|
||||
OAUTH_REFRESH_CALL_TIMEOUT_MS,
|
||||
() =>
|
||||
refreshProviderOAuthCredentialWithPlugin({
|
||||
provider: cred.provider,
|
||||
context: cred,
|
||||
}),
|
||||
);
|
||||
if (pluginRefreshed) {
|
||||
const refreshedCredentials: OAuthCredential = {
|
||||
...cred,
|
||||
...pluginRefreshed,
|
||||
const oauthCreds: Record<string, OAuthCredentials> = {
|
||||
[refreshCred.provider]: refreshCred,
|
||||
};
|
||||
const result =
|
||||
refreshCred.provider === "chutes"
|
||||
? await (async () => {
|
||||
const newCredentials = await withRefreshCallTimeout(
|
||||
`refreshChutesTokens(${refreshCred.provider})`,
|
||||
OAUTH_REFRESH_CALL_TIMEOUT_MS,
|
||||
() => refreshChutesTokens({ credential: refreshCred }),
|
||||
);
|
||||
return { apiKey: newCredentials.access, newCredentials };
|
||||
})()
|
||||
: await (async () => {
|
||||
const oauthProvider = resolveOAuthProvider(refreshCred.provider);
|
||||
if (!oauthProvider) {
|
||||
return null;
|
||||
}
|
||||
if (typeof getOAuthApiKey !== "function") {
|
||||
return null;
|
||||
}
|
||||
return await withRefreshCallTimeout(
|
||||
`getOAuthApiKey(${oauthProvider})`,
|
||||
OAUTH_REFRESH_CALL_TIMEOUT_MS,
|
||||
() => getOAuthApiKey(oauthProvider, oauthCreds),
|
||||
);
|
||||
})();
|
||||
if (!result) {
|
||||
return null;
|
||||
}
|
||||
const mergedCred: OAuthCredential = {
|
||||
...refreshCred,
|
||||
...result.newCredentials,
|
||||
type: "oauth",
|
||||
};
|
||||
store.profiles[params.profileId] = refreshedCredentials;
|
||||
store.profiles[params.profileId] = mergedCred;
|
||||
saveAuthProfileStore(store, params.agentDir);
|
||||
|
||||
// Mirror the refreshed credential back into the main-agent store while
|
||||
// both locks are still held (refresh lock + this agent's store lock)
|
||||
// plus we'll take main-store lock inside the mirror. Doing this inside
|
||||
// the refresh lock closes the cross-process race window where a second
|
||||
// agent could acquire the refresh lock between our lock release and
|
||||
// our main-store write, see only stale main creds, and redundantly
|
||||
// refresh (reproducing refresh_token_reused).
|
||||
if (params.agentDir) {
|
||||
const mainPath = resolveAuthStorePath(undefined);
|
||||
if (mainPath !== authPath) {
|
||||
await mirrorRefreshedCredentialIntoMainStore({
|
||||
profileId: params.profileId,
|
||||
refreshed: refreshedCredentials,
|
||||
refreshed: mergedCred,
|
||||
});
|
||||
}
|
||||
}
|
||||
return {
|
||||
apiKey: await buildOAuthApiKey(cred.provider, refreshedCredentials),
|
||||
newCredentials: refreshedCredentials,
|
||||
};
|
||||
}
|
||||
|
||||
const oauthCreds: Record<string, OAuthCredentials> = { [cred.provider]: cred };
|
||||
const result =
|
||||
cred.provider === "chutes"
|
||||
? await (async () => {
|
||||
const newCredentials = await withRefreshCallTimeout(
|
||||
`refreshChutesTokens(${cred.provider})`,
|
||||
OAUTH_REFRESH_CALL_TIMEOUT_MS,
|
||||
() => refreshChutesTokens({ credential: cred }),
|
||||
);
|
||||
return { apiKey: newCredentials.access, newCredentials };
|
||||
})()
|
||||
: await (async () => {
|
||||
const oauthProvider = resolveOAuthProvider(cred.provider);
|
||||
if (!oauthProvider) {
|
||||
return null;
|
||||
}
|
||||
if (typeof getOAuthApiKey !== "function") {
|
||||
return null;
|
||||
}
|
||||
return await withRefreshCallTimeout(
|
||||
`getOAuthApiKey(${oauthProvider})`,
|
||||
OAUTH_REFRESH_CALL_TIMEOUT_MS,
|
||||
() => getOAuthApiKey(oauthProvider, oauthCreds),
|
||||
);
|
||||
})();
|
||||
if (!result) {
|
||||
return null;
|
||||
}
|
||||
const mergedCred: OAuthCredential = {
|
||||
...cred,
|
||||
...result.newCredentials,
|
||||
type: "oauth",
|
||||
};
|
||||
store.profiles[params.profileId] = mergedCred;
|
||||
saveAuthProfileStore(store, params.agentDir);
|
||||
|
||||
// Mirror the refreshed credential back into the main-agent store while
|
||||
// both locks are still held (refresh lock + this agent's store lock)
|
||||
// plus we'll take main-store lock inside the mirror. Doing this inside
|
||||
// the refresh lock closes the cross-process race window where a second
|
||||
// agent could acquire the refresh lock between our lock release and
|
||||
// our main-store write, see only stale main creds, and redundantly
|
||||
// refresh (reproducing refresh_token_reused).
|
||||
if (params.agentDir) {
|
||||
const mainPath = resolveAuthStorePath(undefined);
|
||||
if (mainPath !== authPath) {
|
||||
await mirrorRefreshedCredentialIntoMainStore({
|
||||
profileId: params.profileId,
|
||||
refreshed: mergedCred,
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
return result;
|
||||
}),
|
||||
);
|
||||
return result;
|
||||
}),
|
||||
);
|
||||
} catch (error) {
|
||||
if (isGlobalOAuthRefreshLockTimeoutError(error, globalRefreshLockPath)) {
|
||||
throw createOAuthRefreshContentionError({
|
||||
profileId: params.profileId,
|
||||
provider: params.provider,
|
||||
cause: error,
|
||||
});
|
||||
}
|
||||
throw error;
|
||||
}
|
||||
}
|
||||
|
||||
async function tryResolveOAuthProfile(
|
||||
@@ -1029,6 +1080,7 @@ export async function resolveApiKeyForProfile(
|
||||
}
|
||||
|
||||
const message = extractErrorMessage(error);
|
||||
const errorCode = (error as { code?: string } | undefined)?.code;
|
||||
const hint = await formatAuthDoctorHint({
|
||||
cfg,
|
||||
store: refreshedStore,
|
||||
@@ -1037,7 +1089,9 @@ export async function resolveApiKeyForProfile(
|
||||
});
|
||||
throw new Error(
|
||||
`OAuth token refresh failed for ${cred.provider}: ${message}. ` +
|
||||
"Please try again or re-authenticate." +
|
||||
(errorCode === "refresh_contention"
|
||||
? "Please wait for the in-flight refresh to finish and retry."
|
||||
: "Please try again or re-authenticate.") +
|
||||
(hint ? `\n\n${hint}` : ""),
|
||||
{ cause: error },
|
||||
);
|
||||
|
||||
@@ -92,10 +92,14 @@ afterEach(() => {
|
||||
|
||||
const envVar = (...parts: string[]) => parts.join("_");
|
||||
|
||||
function createUsableOAuthExpiry(): number {
|
||||
return Date.now() + 30 * 60 * 1000;
|
||||
}
|
||||
|
||||
const oauthFixture = {
|
||||
access: "access-token",
|
||||
refresh: "refresh-token",
|
||||
expires: Date.now() + 60_000,
|
||||
expires: createUsableOAuthExpiry(),
|
||||
accountId: "acct_123",
|
||||
};
|
||||
|
||||
|
||||
@@ -247,6 +247,22 @@ describe("formatAssistantErrorText", () => {
|
||||
);
|
||||
});
|
||||
|
||||
it("returns a contention-specific message for OAuth refresh lock timeouts", () => {
|
||||
const msg = makeAssistantError("file lock timeout for /tmp/openclaw-oauth-refresh.lock");
|
||||
expect(formatAssistantErrorText(msg)).toBe(
|
||||
"Authentication refresh is already in progress elsewhere and this attempt timed out waiting for it. Retry in a moment.",
|
||||
);
|
||||
});
|
||||
|
||||
it("returns a timeout-specific message for OAuth refresh hard timeouts", () => {
|
||||
const msg = makeAssistantError(
|
||||
'OAuth refresh call "refreshProviderOAuthCredentialWithPlugin(openai-codex)" exceeded hard timeout (120000ms)',
|
||||
);
|
||||
expect(formatAssistantErrorText(msg)).toBe(
|
||||
"Authentication refresh timed out before the provider completed. Retry in a moment; re-authenticate only if it keeps failing.",
|
||||
);
|
||||
});
|
||||
|
||||
it("returns a missing-scope message for OpenAI Codex scope failures", () => {
|
||||
const msg = makeAssistantError(
|
||||
'401 {"type":"error","error":{"type":"permission_error","message":"Missing scopes: api.responses.write model.request"}}',
|
||||
|
||||
@@ -1228,6 +1228,37 @@ describe("classifyProviderRuntimeFailureKind", () => {
|
||||
).toBe("auth_refresh");
|
||||
});
|
||||
|
||||
it("classifies OAuth refresh timeouts and lock contention distinctly", () => {
|
||||
expect(
|
||||
classifyProviderRuntimeFailureKind(
|
||||
'OAuth refresh call "refreshProviderOAuthCredentialWithPlugin(openai-codex)" exceeded hard timeout (120000ms)',
|
||||
),
|
||||
).toBe("refresh_timeout");
|
||||
expect(
|
||||
classifyProviderRuntimeFailureKind("file lock timeout for /tmp/openclaw-oauth-refresh.lock"),
|
||||
).toBe("refresh_contention");
|
||||
expect(
|
||||
classifyProviderRuntimeFailureKind({
|
||||
code: "refresh_contention",
|
||||
message:
|
||||
"OAuth token refresh failed for openai-codex: OAuth refresh failed (refresh_contention): another process is already refreshing openai-codex for openai-codex:default. Please wait for the in-flight refresh to finish and retry.",
|
||||
}),
|
||||
).toBe("refresh_contention");
|
||||
expect(
|
||||
classifyProviderRuntimeFailureKind(
|
||||
"OAuth token refresh failed for openai-codex: file lock timeout for /tmp/agent/auth-profiles.json. Please try again or re-authenticate.",
|
||||
),
|
||||
).toBe("auth_refresh");
|
||||
});
|
||||
|
||||
it("classifies wrapped OpenAI Codex callback validation failures distinctly", () => {
|
||||
expect(
|
||||
classifyProviderRuntimeFailureKind(
|
||||
"OpenAI Codex OAuth failed (callback_validation_failed): State mismatch",
|
||||
),
|
||||
).toBe("callback_validation");
|
||||
});
|
||||
|
||||
it("classifies HTML 403 auth failures", () => {
|
||||
expect(
|
||||
classifyProviderRuntimeFailureKind(
|
||||
|
||||
@@ -256,6 +256,10 @@ export type FailoverClassification =
|
||||
export type ProviderRuntimeFailureKind =
|
||||
| "auth_scope"
|
||||
| "auth_refresh"
|
||||
| "refresh_timeout"
|
||||
| "refresh_contention"
|
||||
| "callback_timeout"
|
||||
| "callback_validation"
|
||||
| "auth_html_403"
|
||||
| "upstream_html"
|
||||
| "proxy"
|
||||
@@ -441,6 +445,26 @@ function isTimeoutTransportErrorMessage(raw: string, status?: number): boolean {
|
||||
return false;
|
||||
}
|
||||
|
||||
function isOAuthRefreshTimeoutMessage(raw: string): boolean {
|
||||
return /\boauth refresh call\b.*\bexceeded hard timeout\b/i.test(raw);
|
||||
}
|
||||
|
||||
function isOAuthRefreshContentionMessage(raw: string): boolean {
|
||||
return (
|
||||
/\brefresh_contention\b/i.test(raw) ||
|
||||
(/\bfile lock timeout\b/i.test(raw) &&
|
||||
/(?:\/|\\|^)(?:oauth-refresh|openclaw-oauth-refresh)[^/\n\\]*?(?:\.lock)?\b/i.test(raw))
|
||||
);
|
||||
}
|
||||
|
||||
function isOAuthCallbackTimeoutMessage(raw: string): boolean {
|
||||
return /\bcallback_timeout\b/i.test(raw);
|
||||
}
|
||||
|
||||
function isOAuthCallbackValidationMessage(raw: string): boolean {
|
||||
return /\bcallback_validation_failed\b/i.test(raw);
|
||||
}
|
||||
|
||||
function includesAnyHint(text: string, hints: readonly string[]): boolean {
|
||||
return hints.some((hint) => text.includes(hint));
|
||||
}
|
||||
@@ -825,6 +849,21 @@ export function classifyProviderRuntimeFailureKind(
|
||||
if (!message && typeof status !== "number") {
|
||||
return "unknown";
|
||||
}
|
||||
if (normalizedSignal.code === "refresh_contention") {
|
||||
return "refresh_contention";
|
||||
}
|
||||
if (message && isOAuthRefreshContentionMessage(message)) {
|
||||
return "refresh_contention";
|
||||
}
|
||||
if (message && isOAuthRefreshTimeoutMessage(message)) {
|
||||
return "refresh_timeout";
|
||||
}
|
||||
if (message && isOAuthCallbackTimeoutMessage(message)) {
|
||||
return "callback_timeout";
|
||||
}
|
||||
if (message && isOAuthCallbackValidationMessage(message)) {
|
||||
return "callback_validation";
|
||||
}
|
||||
if (message && classifyOAuthRefreshFailure(message)) {
|
||||
return "auth_refresh";
|
||||
}
|
||||
@@ -911,6 +950,34 @@ export function formatAssistantErrorText(
|
||||
return "Authentication refresh failed. Re-authenticate this provider and try again.";
|
||||
}
|
||||
|
||||
if (providerRuntimeFailureKind === "refresh_contention") {
|
||||
return (
|
||||
"Authentication refresh is already in progress elsewhere and this attempt " +
|
||||
"timed out waiting for it. Retry in a moment."
|
||||
);
|
||||
}
|
||||
|
||||
if (providerRuntimeFailureKind === "refresh_timeout") {
|
||||
return (
|
||||
"Authentication refresh timed out before the provider completed. " +
|
||||
"Retry in a moment; re-authenticate only if it keeps failing."
|
||||
);
|
||||
}
|
||||
|
||||
if (providerRuntimeFailureKind === "callback_timeout") {
|
||||
return (
|
||||
"Browser OAuth did not complete before manual fallback kicked in. " +
|
||||
"Retry the login flow and paste the redirect URL if prompted."
|
||||
);
|
||||
}
|
||||
|
||||
if (providerRuntimeFailureKind === "callback_validation") {
|
||||
return (
|
||||
"Browser OAuth returned an invalid or incomplete callback. " +
|
||||
"Retry the login flow and make sure the full redirect URL is pasted if prompted."
|
||||
);
|
||||
}
|
||||
|
||||
if (providerRuntimeFailureKind === "auth_scope") {
|
||||
return (
|
||||
"Authentication is missing the required OpenAI Codex scopes. " +
|
||||
|
||||
@@ -1,7 +1,12 @@
|
||||
export type { FileLockHandle, FileLockOptions } from "../plugin-sdk/file-lock.js";
|
||||
export type {
|
||||
FileLockHandle,
|
||||
FileLockOptions,
|
||||
FileLockTimeoutError,
|
||||
} from "../plugin-sdk/file-lock.js";
|
||||
export {
|
||||
acquireFileLock,
|
||||
drainFileLockStateForTest,
|
||||
FILE_LOCK_TIMEOUT_ERROR_CODE,
|
||||
resetFileLockStateForTest,
|
||||
withFileLock,
|
||||
} from "../plugin-sdk/file-lock.js";
|
||||
|
||||
58
src/plugin-sdk/file-lock.test.ts
Normal file
58
src/plugin-sdk/file-lock.test.ts
Normal file
@@ -0,0 +1,58 @@
|
||||
import fs from "node:fs/promises";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
import { afterEach, beforeEach, describe, expect, it } from "vitest";
|
||||
import {
|
||||
acquireFileLock,
|
||||
drainFileLockStateForTest,
|
||||
FILE_LOCK_TIMEOUT_ERROR_CODE,
|
||||
resetFileLockStateForTest,
|
||||
} from "./file-lock.js";
|
||||
|
||||
describe("acquireFileLock", () => {
|
||||
let tempDir = "";
|
||||
|
||||
beforeEach(async () => {
|
||||
resetFileLockStateForTest();
|
||||
tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-file-lock-"));
|
||||
});
|
||||
|
||||
afterEach(async () => {
|
||||
await drainFileLockStateForTest();
|
||||
if (tempDir) {
|
||||
await fs.rm(tempDir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("respects the configured retry budget even when stale windows are much larger", async () => {
|
||||
const filePath = path.join(tempDir, "oauth-refresh");
|
||||
const lockPath = `${filePath}.lock`;
|
||||
const options = {
|
||||
retries: {
|
||||
retries: 1,
|
||||
factor: 1,
|
||||
minTimeout: 20,
|
||||
maxTimeout: 20,
|
||||
},
|
||||
stale: 100,
|
||||
} as const;
|
||||
|
||||
await fs.writeFile(
|
||||
lockPath,
|
||||
JSON.stringify({ pid: process.pid, createdAt: new Date().toISOString() }, null, 2),
|
||||
"utf8",
|
||||
);
|
||||
setTimeout(() => {
|
||||
void fs.rm(lockPath, { force: true });
|
||||
}, 50);
|
||||
|
||||
await expect(acquireFileLock(filePath, options)).rejects.toSatisfy((error) => {
|
||||
expect(error).toMatchObject({
|
||||
code: FILE_LOCK_TIMEOUT_ERROR_CODE,
|
||||
});
|
||||
expect((error as { lockPath?: string }).lockPath).toBeTruthy();
|
||||
expect((error as { lockPath?: string }).lockPath).toMatch(/oauth-refresh\.lock$/);
|
||||
return true;
|
||||
});
|
||||
}, 5_000);
|
||||
});
|
||||
@@ -123,6 +123,24 @@ export type FileLockHandle = {
|
||||
release: () => Promise<void>;
|
||||
};
|
||||
|
||||
export const FILE_LOCK_TIMEOUT_ERROR_CODE = "file_lock_timeout";
|
||||
|
||||
export type FileLockTimeoutError = Error & {
|
||||
code: typeof FILE_LOCK_TIMEOUT_ERROR_CODE;
|
||||
lockPath: string;
|
||||
};
|
||||
|
||||
function createFileLockTimeoutError(
|
||||
normalizedFile: string,
|
||||
lockPath: string,
|
||||
): FileLockTimeoutError {
|
||||
const error = new Error(`file lock timeout for ${normalizedFile}`);
|
||||
return Object.assign(error, {
|
||||
code: FILE_LOCK_TIMEOUT_ERROR_CODE,
|
||||
lockPath,
|
||||
}) as FileLockTimeoutError;
|
||||
}
|
||||
|
||||
async function releaseHeldLock(normalizedFile: string): Promise<void> {
|
||||
const current = HELD_LOCKS.get(normalizedFile);
|
||||
if (!current) {
|
||||
@@ -162,8 +180,7 @@ export async function acquireFileLock(
|
||||
};
|
||||
}
|
||||
|
||||
const attempts = Math.max(1, options.retries.retries + 1);
|
||||
for (let attempt = 0; attempt < attempts; attempt += 1) {
|
||||
for (let attempt = 0; attempt <= options.retries.retries; attempt += 1) {
|
||||
try {
|
||||
const handle = await fs.open(lockPath, "wx");
|
||||
await handle.writeFile(
|
||||
@@ -184,14 +201,14 @@ export async function acquireFileLock(
|
||||
await fs.rm(lockPath, { force: true }).catch(() => undefined);
|
||||
continue;
|
||||
}
|
||||
if (attempt >= attempts - 1) {
|
||||
if (attempt >= options.retries.retries) {
|
||||
break;
|
||||
}
|
||||
await new Promise((resolve) => setTimeout(resolve, computeDelayMs(options.retries, attempt)));
|
||||
}
|
||||
}
|
||||
|
||||
throw new Error(`file lock timeout for ${normalizedFile}`);
|
||||
throw createFileLockTimeoutError(normalizedFile, lockPath);
|
||||
}
|
||||
|
||||
/** Run an async callback while holding a file lock, always releasing the lock afterward. */
|
||||
|
||||
@@ -69,6 +69,10 @@ export {
|
||||
} from "../secrets/provider-env-vars.js";
|
||||
export { buildOauthProviderAuthResult } from "./provider-auth-result.js";
|
||||
export { generatePkceVerifierChallenge, toFormUrlEncoded } from "./oauth-utils.js";
|
||||
export {
|
||||
DEFAULT_OAUTH_REFRESH_MARGIN_MS,
|
||||
hasUsableOAuthCredential,
|
||||
} from "../agents/auth-profiles/credential-state.js";
|
||||
|
||||
export function isProviderApiKeyConfigured(params: {
|
||||
provider: string;
|
||||
|
||||
@@ -215,6 +215,92 @@ describe("loginOpenAICodexOAuth", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("waits briefly before prompting for manual input after the local browser flow starts", async () => {
|
||||
vi.useFakeTimers();
|
||||
const { prompter } = createPrompter();
|
||||
const runtime = createRuntime();
|
||||
mocks.loginOpenAICodex.mockImplementation(
|
||||
async (opts: {
|
||||
onAuth: (event: { url: string }) => Promise<void>;
|
||||
onManualCodeInput?: () => Promise<string>;
|
||||
}) => {
|
||||
await opts.onAuth({
|
||||
url: "https://auth.openai.com/oauth/authorize?state=abc",
|
||||
});
|
||||
expect(opts.onManualCodeInput).toBeTypeOf("function");
|
||||
const manualPromise = opts.onManualCodeInput?.();
|
||||
await vi.advanceTimersByTimeAsync(14_000);
|
||||
expect(manualPromise).toBeDefined();
|
||||
expect(prompter.text).not.toHaveBeenCalled();
|
||||
await vi.advanceTimersByTimeAsync(1_000);
|
||||
expect(prompter.text).not.toHaveBeenCalled();
|
||||
await vi.advanceTimersByTimeAsync(1_000);
|
||||
return {
|
||||
provider: "openai-codex" as const,
|
||||
access: "access-token",
|
||||
refresh: "refresh-token",
|
||||
expires: Date.now() + 60_000,
|
||||
email: "user@example.com",
|
||||
manualCode: await manualPromise,
|
||||
};
|
||||
},
|
||||
);
|
||||
|
||||
await expect(
|
||||
loginOpenAICodexOAuth({
|
||||
prompter,
|
||||
runtime,
|
||||
isRemote: false,
|
||||
openUrl: async () => {},
|
||||
}),
|
||||
).resolves.toMatchObject({
|
||||
access: "access-token",
|
||||
refresh: "refresh-token",
|
||||
});
|
||||
|
||||
expect(prompter.text).toHaveBeenCalledWith({
|
||||
message: "Paste the authorization code (or full redirect URL):",
|
||||
validate: expect.any(Function),
|
||||
});
|
||||
expect(runtime.log).toHaveBeenCalledWith(
|
||||
"OpenAI Codex OAuth callback did not arrive within 15000ms; switching to manual entry (callback_timeout).",
|
||||
);
|
||||
vi.useRealTimers();
|
||||
});
|
||||
|
||||
it("clears the local manual fallback timer when browser callback settles first", async () => {
|
||||
vi.useFakeTimers();
|
||||
mocks.loginOpenAICodex.mockImplementation(
|
||||
async (opts: {
|
||||
onAuth: (event: { url: string }) => Promise<void>;
|
||||
onManualCodeInput?: () => Promise<string>;
|
||||
}) => {
|
||||
await opts.onAuth({
|
||||
url: "https://auth.openai.com/oauth/authorize?state=abc",
|
||||
});
|
||||
expect(opts.onManualCodeInput).toBeTypeOf("function");
|
||||
void opts.onManualCodeInput?.();
|
||||
return {
|
||||
provider: "openai-codex" as const,
|
||||
access: "access-token",
|
||||
refresh: "refresh-token",
|
||||
expires: Date.now() + 60_000,
|
||||
email: "user@example.com",
|
||||
};
|
||||
},
|
||||
);
|
||||
|
||||
await expect(runCodexOAuth({ isRemote: false })).resolves.toMatchObject({
|
||||
result: expect.objectContaining({
|
||||
access: "access-token",
|
||||
refresh: "refresh-token",
|
||||
}),
|
||||
});
|
||||
|
||||
expect(vi.getTimerCount()).toBe(0);
|
||||
vi.useRealTimers();
|
||||
});
|
||||
|
||||
it("continues OAuth flow on non-certificate preflight failures", async () => {
|
||||
const creds = {
|
||||
provider: "openai-codex" as const,
|
||||
@@ -238,7 +324,7 @@ describe("loginOpenAICodexOAuth", () => {
|
||||
expect(prompter.note).not.toHaveBeenCalledWith("tls fix", "OAuth prerequisites");
|
||||
});
|
||||
|
||||
it("surfaces TLS preflight guidance but still attempts OAuth login", async () => {
|
||||
it("fails fast on TLS certificate preflight failures before starting OAuth login", async () => {
|
||||
mocks.runOpenAIOAuthTlsPreflight.mockResolvedValue({
|
||||
ok: false,
|
||||
kind: "tls-cert",
|
||||
@@ -265,14 +351,108 @@ describe("loginOpenAICodexOAuth", () => {
|
||||
isRemote: false,
|
||||
openUrl: async () => {},
|
||||
}),
|
||||
).resolves.toEqual(creds);
|
||||
).rejects.toThrow(/OAuth prerequisites/i);
|
||||
|
||||
expect(mocks.loginOpenAICodex).toHaveBeenCalledOnce();
|
||||
expect(runtime.log).toHaveBeenCalledWith("Run brew postinstall openssl@3");
|
||||
expect(runtime.error).not.toHaveBeenCalledWith("Run brew postinstall openssl@3");
|
||||
expect(mocks.loginOpenAICodex).not.toHaveBeenCalled();
|
||||
expect(prompter.note).toHaveBeenCalledWith(
|
||||
"Run brew postinstall openssl@3",
|
||||
"OAuth prerequisites",
|
||||
);
|
||||
});
|
||||
|
||||
it("prompts for manual input immediately when the local callback flow never starts", async () => {
|
||||
vi.useFakeTimers();
|
||||
const { prompter } = createPrompter();
|
||||
const runtime = createRuntime();
|
||||
mocks.loginOpenAICodex.mockImplementation(
|
||||
async (opts: { onManualCodeInput?: () => Promise<string> }) => {
|
||||
expect(opts.onManualCodeInput).toBeTypeOf("function");
|
||||
const manualCode = await opts.onManualCodeInput?.();
|
||||
return {
|
||||
provider: "openai-codex" as const,
|
||||
access: "access-token",
|
||||
refresh: "refresh-token",
|
||||
expires: Date.now() + 60_000,
|
||||
email: "user@example.com",
|
||||
manualCode,
|
||||
};
|
||||
},
|
||||
);
|
||||
|
||||
await expect(
|
||||
loginOpenAICodexOAuth({
|
||||
prompter,
|
||||
runtime,
|
||||
isRemote: false,
|
||||
openUrl: async () => {},
|
||||
}),
|
||||
).resolves.toMatchObject({
|
||||
access: "access-token",
|
||||
refresh: "refresh-token",
|
||||
});
|
||||
|
||||
expect(prompter.text).toHaveBeenCalledWith({
|
||||
message: "Paste the authorization code (or full redirect URL):",
|
||||
validate: expect.any(Function),
|
||||
});
|
||||
expect(vi.getTimerCount()).toBe(0);
|
||||
vi.useRealTimers();
|
||||
});
|
||||
|
||||
it("suppresses the local manual prompt when oauth settles just after the fallback deadline", async () => {
|
||||
vi.useFakeTimers();
|
||||
const { prompter } = createPrompter();
|
||||
const runtime = createRuntime();
|
||||
mocks.loginOpenAICodex.mockImplementation(
|
||||
async (opts: {
|
||||
onAuth: (event: { url: string }) => Promise<void>;
|
||||
onManualCodeInput?: () => Promise<string>;
|
||||
}) => {
|
||||
await opts.onAuth({
|
||||
url: "https://auth.openai.com/oauth/authorize?state=abc",
|
||||
});
|
||||
void opts.onManualCodeInput?.();
|
||||
await vi.advanceTimersByTimeAsync(15_500);
|
||||
return {
|
||||
provider: "openai-codex" as const,
|
||||
access: "access-token",
|
||||
refresh: "refresh-token",
|
||||
expires: Date.now() + 60_000,
|
||||
email: "user@example.com",
|
||||
};
|
||||
},
|
||||
);
|
||||
|
||||
await expect(
|
||||
loginOpenAICodexOAuth({
|
||||
prompter,
|
||||
runtime,
|
||||
isRemote: false,
|
||||
openUrl: async () => {},
|
||||
}),
|
||||
).resolves.toMatchObject({
|
||||
access: "access-token",
|
||||
refresh: "refresh-token",
|
||||
});
|
||||
|
||||
expect(prompter.text).not.toHaveBeenCalled();
|
||||
vi.useRealTimers();
|
||||
});
|
||||
|
||||
it("rewrites callback validation failures with a stable internal code", async () => {
|
||||
mocks.loginOpenAICodex.mockRejectedValue(new Error("State mismatch"));
|
||||
|
||||
const { prompter, spin } = createPrompter();
|
||||
const runtime = createRuntime();
|
||||
await expect(
|
||||
loginOpenAICodexOAuth({
|
||||
prompter,
|
||||
runtime,
|
||||
isRemote: false,
|
||||
openUrl: async () => {},
|
||||
}),
|
||||
).rejects.toThrow(/callback_validation_failed/i);
|
||||
|
||||
expect(spin.stop).toHaveBeenCalledWith("OpenAI OAuth failed");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,7 +1,9 @@
|
||||
import { loginOpenAICodex, type OAuthCredentials } from "@mariozechner/pi-ai/oauth";
|
||||
import { formatErrorMessage } from "../infra/errors.js";
|
||||
import { ensureGlobalUndiciEnvProxyDispatcher } from "../infra/net/undici-global-dispatcher.js";
|
||||
import type { RuntimeEnv } from "../runtime.js";
|
||||
import type { WizardPrompter } from "../wizard/prompts.js";
|
||||
import type { OAuthPrompt } from "./provider-oauth-flow.js";
|
||||
import { createVpsAwareOAuthHandlers } from "./provider-oauth-flow.js";
|
||||
import {
|
||||
formatOpenAIOAuthTlsPreflightFix,
|
||||
@@ -10,6 +12,111 @@ import {
|
||||
|
||||
const manualInputPromptMessage = "Paste the authorization code (or full redirect URL):";
|
||||
const openAICodexOAuthOriginator = "openclaw";
|
||||
const localManualFallbackDelayMs = 15_000;
|
||||
const localManualFallbackGraceMs = 1_000;
|
||||
|
||||
type OpenAICodexOAuthFailureCode = "callback_timeout" | "callback_validation_failed";
|
||||
|
||||
function waitForDelayOrLoginSettle(params: {
|
||||
delayMs: number;
|
||||
waitForLoginToSettle: Promise<void>;
|
||||
}): Promise<"delay" | "settled"> {
|
||||
return new Promise((resolve) => {
|
||||
let finished = false;
|
||||
const finish = (outcome: "delay" | "settled") => {
|
||||
if (finished) {
|
||||
return;
|
||||
}
|
||||
finished = true;
|
||||
clearTimeout(timeoutHandle);
|
||||
resolve(outcome);
|
||||
};
|
||||
const timeoutHandle = setTimeout(() => finish("delay"), params.delayMs);
|
||||
params.waitForLoginToSettle.then(
|
||||
() => finish("settled"),
|
||||
() => finish("settled"),
|
||||
);
|
||||
});
|
||||
}
|
||||
|
||||
function createNeverSettlingPromptResult(): Promise<string> {
|
||||
return new Promise<string>(() => undefined);
|
||||
}
|
||||
|
||||
function createOpenAICodexOAuthError(
|
||||
code: OpenAICodexOAuthFailureCode,
|
||||
message: string,
|
||||
cause?: unknown,
|
||||
): Error & { code: OpenAICodexOAuthFailureCode } {
|
||||
const error = new Error(`OpenAI Codex OAuth failed (${code}): ${message}`, { cause });
|
||||
return Object.assign(error, { code });
|
||||
}
|
||||
|
||||
function rewriteOpenAICodexOAuthError(error: unknown): Error {
|
||||
const message = formatErrorMessage(error);
|
||||
if (/state mismatch|missing authorization code/i.test(message)) {
|
||||
return createOpenAICodexOAuthError("callback_validation_failed", message, error);
|
||||
}
|
||||
return error instanceof Error ? error : new Error(message);
|
||||
}
|
||||
|
||||
function createManualCodeInputHandler(params: {
|
||||
isRemote: boolean;
|
||||
onPrompt: (prompt: OAuthPrompt) => Promise<string>;
|
||||
runtime: RuntimeEnv;
|
||||
spin: ReturnType<WizardPrompter["progress"]>;
|
||||
waitForLoginToSettle: Promise<void>;
|
||||
hasBrowserAuthStarted: () => boolean;
|
||||
}): (() => Promise<string>) | undefined {
|
||||
if (params.isRemote) {
|
||||
return async () =>
|
||||
await params.onPrompt({
|
||||
message: manualInputPromptMessage,
|
||||
});
|
||||
}
|
||||
|
||||
return async () => {
|
||||
if (!params.hasBrowserAuthStarted()) {
|
||||
params.spin.update(
|
||||
"Local OAuth callback was unavailable. Paste the redirect URL to continue…",
|
||||
);
|
||||
params.runtime.log(
|
||||
"OpenAI Codex OAuth local callback did not start; switching to manual entry immediately.",
|
||||
);
|
||||
return await params.onPrompt({
|
||||
message: manualInputPromptMessage,
|
||||
});
|
||||
}
|
||||
|
||||
const outcome = await waitForDelayOrLoginSettle({
|
||||
delayMs: localManualFallbackDelayMs,
|
||||
waitForLoginToSettle: params.waitForLoginToSettle,
|
||||
});
|
||||
if (outcome === "settled") {
|
||||
// markLoginSettled() runs in loginOpenAICodexOAuth's finally block, so
|
||||
// reaching this branch means the outer login call has already completed.
|
||||
// Return a never-settling promise to suppress an unnecessary manual
|
||||
// prompt without feeding placeholder input back into the upstream flow.
|
||||
return await createNeverSettlingPromptResult();
|
||||
}
|
||||
|
||||
const settledDuringGraceWindow = await waitForDelayOrLoginSettle({
|
||||
delayMs: localManualFallbackGraceMs,
|
||||
waitForLoginToSettle: params.waitForLoginToSettle,
|
||||
});
|
||||
if (settledDuringGraceWindow === "settled") {
|
||||
return await createNeverSettlingPromptResult();
|
||||
}
|
||||
|
||||
params.spin.update("Browser callback did not finish. Paste the redirect URL to continue…");
|
||||
params.runtime.log(
|
||||
`OpenAI Codex OAuth callback did not arrive within ${localManualFallbackDelayMs}ms; switching to manual entry (callback_timeout).`,
|
||||
);
|
||||
return await params.onPrompt({
|
||||
message: manualInputPromptMessage,
|
||||
});
|
||||
};
|
||||
}
|
||||
|
||||
export async function loginOpenAICodexOAuth(params: {
|
||||
prompter: WizardPrompter;
|
||||
@@ -25,8 +132,9 @@ export async function loginOpenAICodexOAuth(params: {
|
||||
const preflight = await runOpenAIOAuthTlsPreflight();
|
||||
if (!preflight.ok && preflight.kind === "tls-cert") {
|
||||
const hint = formatOpenAIOAuthTlsPreflightFix(preflight);
|
||||
runtime.log(hint);
|
||||
await prompter.note(hint, "OAuth prerequisites");
|
||||
runtime.error(hint);
|
||||
throw new Error(`OpenAI Codex OAuth prerequisites failed: ${preflight.message}`);
|
||||
}
|
||||
|
||||
await prompter.note(
|
||||
@@ -45,6 +153,11 @@ export async function loginOpenAICodexOAuth(params: {
|
||||
);
|
||||
|
||||
const spin = prompter.progress("Starting OAuth flow…");
|
||||
let browserAuthStarted = false;
|
||||
let markLoginSettled!: () => void;
|
||||
const waitForLoginToSettle = new Promise<void>((resolve) => {
|
||||
markLoginSettled = resolve;
|
||||
});
|
||||
try {
|
||||
const { onAuth: baseOnAuth, onPrompt } = createVpsAwareOAuthHandlers({
|
||||
isRemote,
|
||||
@@ -55,25 +168,34 @@ export async function loginOpenAICodexOAuth(params: {
|
||||
localBrowserMessage: localBrowserMessage ?? "Complete sign-in in browser…",
|
||||
manualPromptMessage: manualInputPromptMessage,
|
||||
});
|
||||
const onAuth: typeof baseOnAuth = async (event) => {
|
||||
browserAuthStarted = true;
|
||||
await baseOnAuth(event);
|
||||
};
|
||||
|
||||
const creds = await loginOpenAICodex({
|
||||
onAuth: baseOnAuth,
|
||||
onAuth,
|
||||
onPrompt,
|
||||
originator: openAICodexOAuthOriginator,
|
||||
onManualCodeInput: isRemote
|
||||
? async () =>
|
||||
await onPrompt({
|
||||
message: manualInputPromptMessage,
|
||||
})
|
||||
: undefined,
|
||||
onManualCodeInput: createManualCodeInputHandler({
|
||||
isRemote,
|
||||
onPrompt,
|
||||
runtime,
|
||||
spin,
|
||||
waitForLoginToSettle,
|
||||
hasBrowserAuthStarted: () => browserAuthStarted,
|
||||
}),
|
||||
onProgress: (msg: string) => spin.update(msg),
|
||||
});
|
||||
spin.stop("OpenAI OAuth complete");
|
||||
return creds ?? null;
|
||||
} catch (err) {
|
||||
spin.stop("OpenAI OAuth failed");
|
||||
runtime.error(String(err));
|
||||
const rewrittenError = rewriteOpenAICodexOAuthError(err);
|
||||
runtime.error(String(rewrittenError));
|
||||
await prompter.note("Trouble with OAuth? See https://docs.openclaw.ai/start/faq", "OAuth help");
|
||||
throw err;
|
||||
throw rewrittenError;
|
||||
} finally {
|
||||
markLoginSettled();
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user