mirror of https://github.com/openclaw/openclaw.git
fix(auth): protect fresher codex reauth state
- invalidate cached Codex CLI credentials when auth.json changes within the TTL window - skip external CLI sync when the stored Codex OAuth credential is newer - cover both behaviors with focused regression tests Refs #53466 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
parent
559b3a5fd4
commit
bbe6f7fdd9
|
|
@ -95,4 +95,40 @@ describe("syncExternalCliCredentials", () => {
|
|||
expires: freshExpiry,
|
||||
});
|
||||
});
|
||||
|
||||
it("does not overwrite newer stored Codex credentials with older external CLI credentials", () => {
|
||||
const staleExpiry = Date.now() + 30 * 60_000;
|
||||
const freshExpiry = Date.now() + 5 * 24 * 60 * 60_000;
|
||||
mocks.readCodexCliCredentialsCached.mockReturnValue({
|
||||
type: "oauth",
|
||||
provider: "openai-codex",
|
||||
access: "stale-access-token",
|
||||
refresh: "stale-refresh-token",
|
||||
expires: staleExpiry,
|
||||
accountId: "acct_789",
|
||||
});
|
||||
|
||||
const store: AuthProfileStore = {
|
||||
version: 1,
|
||||
profiles: {
|
||||
[OPENAI_CODEX_DEFAULT_PROFILE_ID]: {
|
||||
type: "oauth",
|
||||
provider: "openai-codex",
|
||||
access: "fresh-access-token",
|
||||
refresh: "fresh-refresh-token",
|
||||
expires: freshExpiry,
|
||||
accountId: "acct_789",
|
||||
},
|
||||
},
|
||||
};
|
||||
|
||||
const mutated = syncExternalCliCredentials(store);
|
||||
|
||||
expect(mutated).toBe(false);
|
||||
expect(store.profiles[OPENAI_CODEX_DEFAULT_PROFILE_ID]).toMatchObject({
|
||||
access: "fresh-access-token",
|
||||
refresh: "fresh-refresh-token",
|
||||
expires: freshExpiry,
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -36,6 +36,18 @@ function shallowEqualOAuthCredentials(a: OAuthCredential | undefined, b: OAuthCr
|
|||
);
|
||||
}
|
||||
|
||||
function hasNewerStoredOAuthCredential(
|
||||
existing: OAuthCredential | undefined,
|
||||
incoming: OAuthCredential,
|
||||
): boolean {
|
||||
return Boolean(
|
||||
existing &&
|
||||
existing.provider === incoming.provider &&
|
||||
Number.isFinite(existing.expires) &&
|
||||
(!Number.isFinite(incoming.expires) || existing.expires > incoming.expires),
|
||||
);
|
||||
}
|
||||
|
||||
/** Sync external CLI credentials into the store for a given provider. */
|
||||
function syncExternalCliCredentialsForProvider(
|
||||
store: AuthProfileStore,
|
||||
|
|
@ -54,6 +66,18 @@ function syncExternalCliCredentialsForProvider(
|
|||
if (shallowEqualOAuthCredentials(existingOAuth, creds)) {
|
||||
return false;
|
||||
}
|
||||
if (hasNewerStoredOAuthCredential(existingOAuth, creds)) {
|
||||
if (options.log !== false) {
|
||||
log.debug(`kept newer stored ${provider} credentials over external cli sync`, {
|
||||
profileId,
|
||||
storedExpires: new Date(existingOAuth!.expires).toISOString(),
|
||||
externalExpires: Number.isFinite(creds.expires)
|
||||
? new Date(creds.expires).toISOString()
|
||||
: null,
|
||||
});
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
store.profiles[profileId] = creds;
|
||||
if (options.log !== false) {
|
||||
|
|
|
|||
|
|
@ -7,6 +7,7 @@ const execSyncMock = vi.fn();
|
|||
const execFileSyncMock = vi.fn();
|
||||
const CLI_CREDENTIALS_CACHE_TTL_MS = 15 * 60 * 1000;
|
||||
let readClaudeCliCredentialsCached: typeof import("./cli-credentials.js").readClaudeCliCredentialsCached;
|
||||
let readCodexCliCredentialsCached: typeof import("./cli-credentials.js").readCodexCliCredentialsCached;
|
||||
let resetCliCredentialCachesForTest: typeof import("./cli-credentials.js").resetCliCredentialCachesForTest;
|
||||
let writeClaudeCliKeychainCredentials: typeof import("./cli-credentials.js").writeClaudeCliKeychainCredentials;
|
||||
let writeClaudeCliCredentials: typeof import("./cli-credentials.js").writeClaudeCliCredentials;
|
||||
|
|
@ -56,6 +57,7 @@ describe("cli credentials", () => {
|
|||
beforeAll(async () => {
|
||||
({
|
||||
readClaudeCliCredentialsCached,
|
||||
readCodexCliCredentialsCached,
|
||||
resetCliCredentialCachesForTest,
|
||||
writeClaudeCliKeychainCredentials,
|
||||
writeClaudeCliCredentials,
|
||||
|
|
@ -292,4 +294,64 @@ describe("cli credentials", () => {
|
|||
expires: expSeconds * 1000,
|
||||
});
|
||||
});
|
||||
|
||||
it("invalidates cached Codex credentials when auth.json changes within the TTL window", () => {
|
||||
const tempHome = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-codex-cache-"));
|
||||
process.env.CODEX_HOME = tempHome;
|
||||
const authPath = path.join(tempHome, "auth.json");
|
||||
const firstExpiry = Math.floor(Date.parse("2026-03-24T12:34:56Z") / 1000);
|
||||
const secondExpiry = Math.floor(Date.parse("2026-03-25T12:34:56Z") / 1000);
|
||||
try {
|
||||
fs.mkdirSync(tempHome, { recursive: true, mode: 0o700 });
|
||||
fs.writeFileSync(
|
||||
authPath,
|
||||
JSON.stringify({
|
||||
tokens: {
|
||||
access_token: createJwtWithExp(firstExpiry),
|
||||
refresh_token: "stale-refresh",
|
||||
},
|
||||
}),
|
||||
"utf8",
|
||||
);
|
||||
fs.utimesSync(authPath, new Date("2026-03-24T10:00:00Z"), new Date("2026-03-24T10:00:00Z"));
|
||||
vi.setSystemTime(new Date("2026-03-24T10:00:00Z"));
|
||||
|
||||
const first = readCodexCliCredentialsCached({
|
||||
ttlMs: CLI_CREDENTIALS_CACHE_TTL_MS,
|
||||
platform: "linux",
|
||||
execSync: execSyncMock,
|
||||
});
|
||||
|
||||
expect(first).toMatchObject({
|
||||
refresh: "stale-refresh",
|
||||
expires: firstExpiry * 1000,
|
||||
});
|
||||
|
||||
fs.writeFileSync(
|
||||
authPath,
|
||||
JSON.stringify({
|
||||
tokens: {
|
||||
access_token: createJwtWithExp(secondExpiry),
|
||||
refresh_token: "fresh-refresh",
|
||||
},
|
||||
}),
|
||||
"utf8",
|
||||
);
|
||||
fs.utimesSync(authPath, new Date("2026-03-24T10:05:00Z"), new Date("2026-03-24T10:05:00Z"));
|
||||
vi.advanceTimersByTime(60_000);
|
||||
|
||||
const second = readCodexCliCredentialsCached({
|
||||
ttlMs: CLI_CREDENTIALS_CACHE_TTL_MS,
|
||||
platform: "linux",
|
||||
execSync: execSyncMock,
|
||||
});
|
||||
|
||||
expect(second).toMatchObject({
|
||||
refresh: "fresh-refresh",
|
||||
expires: secondExpiry * 1000,
|
||||
});
|
||||
} finally {
|
||||
fs.rmSync(tempHome, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -21,6 +21,7 @@ type CachedValue<T> = {
|
|||
value: T | null;
|
||||
readAt: number;
|
||||
cacheKey: string;
|
||||
sourceMtimeMs?: number | null;
|
||||
};
|
||||
|
||||
let claudeCliCache: CachedValue<ClaudeCliCredential> | null = null;
|
||||
|
|
@ -148,6 +149,14 @@ function resolveMiniMaxCliCredentialsPath(homeDir?: string) {
|
|||
return path.join(baseDir, MINIMAX_CLI_CREDENTIALS_RELATIVE_PATH);
|
||||
}
|
||||
|
||||
function readFileMtimeMs(filePath: string): number | null {
|
||||
try {
|
||||
return fs.statSync(filePath).mtimeMs;
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
function computeCodexKeychainAccount(codexHome: string) {
|
||||
const hash = createHash("sha256").update(codexHome).digest("hex");
|
||||
return `cli|${hash.slice(0, 16)}`;
|
||||
|
|
@ -526,11 +535,14 @@ export function readCodexCliCredentialsCached(options?: {
|
|||
}): CodexCliCredential | null {
|
||||
const ttlMs = options?.ttlMs ?? 0;
|
||||
const now = Date.now();
|
||||
const cacheKey = `${options?.platform ?? process.platform}|${resolveCodexCliAuthPath()}`;
|
||||
const authPath = resolveCodexCliAuthPath();
|
||||
const cacheKey = `${options?.platform ?? process.platform}|${authPath}`;
|
||||
const sourceMtimeMs = readFileMtimeMs(authPath);
|
||||
if (
|
||||
ttlMs > 0 &&
|
||||
codexCliCache &&
|
||||
codexCliCache.cacheKey === cacheKey &&
|
||||
codexCliCache.sourceMtimeMs === sourceMtimeMs &&
|
||||
now - codexCliCache.readAt < ttlMs
|
||||
) {
|
||||
return codexCliCache.value;
|
||||
|
|
@ -539,8 +551,16 @@ export function readCodexCliCredentialsCached(options?: {
|
|||
platform: options?.platform,
|
||||
execSync: options?.execSync,
|
||||
});
|
||||
if (ttlMs > 0) {
|
||||
codexCliCache = { value, readAt: now, cacheKey };
|
||||
const cachedSourceMtimeMs = readFileMtimeMs(authPath);
|
||||
if (ttlMs > 0 && cachedSourceMtimeMs === sourceMtimeMs) {
|
||||
codexCliCache = {
|
||||
value,
|
||||
readAt: now,
|
||||
cacheKey,
|
||||
sourceMtimeMs: cachedSourceMtimeMs,
|
||||
};
|
||||
} else if (ttlMs > 0) {
|
||||
codexCliCache = null;
|
||||
}
|
||||
return value;
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue