From 831a120ea520e62a100f8009fa6d55d9247c7ddc Mon Sep 17 00:00:00 2001 From: Bruce-Yii <298228875+Bruce-Yii@users.noreply.github.com> Date: Mon, 10 Aug 2026 00:12:23 +0800 Subject: [PATCH 1/3] fix(oauth): guard local token expiry parsing against NaN --- src/oauth/index.ts | 12 +++- src/oauth/local-token-detect.ts | 13 ++++- tests/local-token-detect.test.ts | 90 +++++++++++++++++++++++++++++- tests/oauth-status-privacy.test.ts | 40 +++++++++++++ 4 files changed, 151 insertions(+), 4 deletions(-) diff --git a/src/oauth/index.ts b/src/oauth/index.ts index 0238797c7f..59231f4512 100644 --- a/src/oauth/index.ts +++ b/src/oauth/index.ts @@ -1179,8 +1179,18 @@ export function getLoginStatus(provider: string): { loggedIn: boolean; email?: s ...(a.needsReauth ? { needsReauth: true } : {}), expiresAt: a.credential.expires, })); + + // A stored credential only counts as "logged in" when it is still usable: not marked for + // re-auth and not already expired (or unknown-expiry, which needs refresh validation). + const active = set?.accounts.find(a => a.id === set.activeAccountId); + const activeExpired = active + ? Number.isFinite(active.credential.expires) + && active.credential.expires > 0 + && active.credential.expires <= Date.now() + : false; + const activeNeedsReauth = active?.needsReauth === true; return { - loggedIn: !!cred, + loggedIn: !!cred && !activeNeedsReauth && !activeExpired, email: maskEmail(cred?.email) ?? undefined, source: cred?.source, error: st?.error, diff --git a/src/oauth/local-token-detect.ts b/src/oauth/local-token-detect.ts index f0422b63b8..aca20b8216 100644 --- a/src/oauth/local-token-detect.ts +++ b/src/oauth/local-token-detect.ts @@ -24,7 +24,10 @@ export function detectGrokCliToken(): OAuthCredentials | null { const accessToken = entry.key as string; const refreshToken = entry.refresh_token as string; - const expiresAt = entry.expires_at ? new Date(entry.expires_at as string).getTime() : 0; + const parsedExpiresAt = entry.expires_at ? new Date(entry.expires_at as string).getTime() : 0; + // Guard against unparseable/NaN expiries: a non-finite value must never be treated as + // "valid forever". Unknown → 0, which forces the refresh-validation path downstream. + const expiresAt = Number.isFinite(parsedExpiresAt) ? parsedExpiresAt : 0; return { refresh: refreshToken, @@ -55,6 +58,9 @@ export function shouldAdoptGrokGeneration( now = Date.now(), refreshSkewMs = 60_000, ): boolean { + // A non-finite disk expiry means we cannot reason about the generation: the credential is + // either garbage or unknown. Treat it as requiring refresh validation, never as an upgrade. + if (!Number.isFinite(disk.expires)) return false; if (disk.expires <= now + refreshSkewMs) return false; const bothExpiriesExist = stored.expires > 0 && disk.expires > 0; if (bothExpiriesExist) return disk.expires >= stored.expires; @@ -108,7 +114,10 @@ export function parseClaudeOauthPayload(raw: string): OAuthCredentials | null { const data = JSON.parse(raw) as { claudeAiOauth?: { accessToken?: string; refreshToken?: string; expiresAt?: number } }; const o = data.claudeAiOauth; if (!o?.accessToken || !o?.refreshToken) return null; - return { access: o.accessToken, refresh: o.refreshToken, expires: o.expiresAt ?? 0, source: "local-cli" }; + // Number.isFinite guard: a string/NaN expiresAt must not flow into downstream time + // comparisons as a "valid forever" value. Unknown → 0 (refresh-validation path). + const expires = typeof o.expiresAt === "number" && Number.isFinite(o.expiresAt) ? o.expiresAt : 0; + return { access: o.accessToken, refresh: o.refreshToken, expires, source: "local-cli" }; } catch { return null; } diff --git a/tests/local-token-detect.test.ts b/tests/local-token-detect.test.ts index d4e062e7f3..fc37fe0ddd 100644 --- a/tests/local-token-detect.test.ts +++ b/tests/local-token-detect.test.ts @@ -2,14 +2,21 @@ import { afterEach, beforeAll, afterAll, describe, expect, test } from "bun:test import { mkdtempSync, rmSync, writeFileSync, mkdirSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; -import { parseClaudeOauthPayload, readClaudeCredentialsFile } from "../src/oauth/local-token-detect"; +import { + detectGrokCliToken, + parseClaudeOauthPayload, + readClaudeCredentialsFile, + shouldAdoptGrokGeneration, +} from "../src/oauth/local-token-detect"; let tmp: string; let prevConfigDir: string | undefined; +let prevHome: string | undefined; beforeAll(() => { tmp = mkdtempSync(join(tmpdir(), "ocx-claude-detect-")); prevConfigDir = process.env.CLAUDE_CONFIG_DIR; + prevHome = process.env.HOME; }); afterAll(() => { @@ -19,6 +26,8 @@ afterAll(() => { afterEach(() => { if (prevConfigDir === undefined) delete process.env.CLAUDE_CONFIG_DIR; else process.env.CLAUDE_CONFIG_DIR = prevConfigDir; + if (prevHome === undefined) delete process.env.HOME; + else process.env.HOME = prevHome; }); describe("Claude Code credentials file fallback (Linux/Windows)", () => { @@ -45,3 +54,82 @@ describe("Claude Code credentials file fallback (Linux/Windows)", () => { expect(parseClaudeOauthPayload("not json")).toBeNull(); }); }); + +describe("invalid expiry handling (NaN guard)", () => { + test("parseClaudeOauthPayload coerces a string expiresAt to unknown (0) instead of NaN", () => { + const raw = JSON.stringify({ claudeAiOauth: { accessToken: "at-1", refreshToken: "rt-1", expiresAt: "garbage" } }); + const creds = parseClaudeOauthPayload(raw); + expect(creds).not.toBeNull(); + expect(creds!.expires).toBe(0); + expect(Number.isFinite(creds!.expires)).toBe(true); + }); + + test("parseClaudeOauthPayload coerces a non-finite expiresAt to unknown (0)", () => { + const raw = JSON.stringify({ claudeAiOauth: { accessToken: "at-1", refreshToken: "rt-1", expiresAt: NaN } }); + const creds = parseClaudeOauthPayload(raw); + expect(creds).not.toBeNull(); + expect(creds!.expires).toBe(0); + }); + + test("detectGrokCliToken coerces an unparseable expires_at to unknown (0) instead of NaN", () => { + const grokHome = join(tmp, "grok-home"); + mkdirSync(join(grokHome, ".grok"), { recursive: true }); + writeFileSync(join(grokHome, ".grok", "auth.json"), JSON.stringify({ + "https://auth.x.ai::1": { + key: "xai-stub-access", + refresh_token: "xai-stub-refresh", + expires_at: "not-a-date", + }, + })); + process.env.HOME = grokHome; + + const creds = detectGrokCliToken(); + expect(creds).not.toBeNull(); + expect(creds!.expires).toBe(0); + expect(Number.isFinite(creds!.expires)).toBe(true); + }); + + test("detectGrokCliToken passes through a parseable future expires_at", () => { + const grokHome = join(tmp, "grok-home-future"); + mkdirSync(join(grokHome, ".grok"), { recursive: true }); + const future = new Date(Date.now() + 3600_000).toISOString(); + writeFileSync(join(grokHome, ".grok", "auth.json"), JSON.stringify({ + "https://auth.x.ai::1": { + key: "xai-stub-access", + refresh_token: "xai-stub-refresh", + expires_at: future, + }, + })); + process.env.HOME = grokHome; + + const creds = detectGrokCliToken(); + expect(creds).not.toBeNull(); + expect(creds!.expires).toBeGreaterThan(Date.now()); + }); + + test("detectGrokCliToken returns null when auth.json is absent", () => { + process.env.HOME = join(tmp, "grok-home-missing"); + expect(detectGrokCliToken()).toBeNull(); + }); +}); + +describe("shouldAdoptGrokGeneration with NaN/unknown expiries", () => { + const stored = { refresh: "stored-refresh", access: "stored-access", expires: Date.now() + 3600_000 }; + + test("never adopts a disk credential with a non-finite expiry", () => { + expect(shouldAdoptGrokGeneration(stored, { ...stored, expires: Number.NaN }, Date.now(), 60_000)).toBe(false); + }); + + test("rejects an unknown (0) disk expiry as requiring refresh", () => { + expect(shouldAdoptGrokGeneration(stored, { ...stored, expires: 0 }, Date.now(), 60_000)).toBe(false); + }); + + test("rejects an already-expired disk credential", () => { + expect(shouldAdoptGrokGeneration(stored, { ...stored, expires: Date.now() - 60_000 }, Date.now(), 60_000)).toBe(false); + }); + + test("adopts a newer valid disk credential", () => { + const disk = { ...stored, expires: Date.now() + 7200_000 }; + expect(shouldAdoptGrokGeneration(stored, disk, Date.now(), 60_000)).toBe(true); + }); +}); diff --git a/tests/oauth-status-privacy.test.ts b/tests/oauth-status-privacy.test.ts index 57880d93c1..fc3e56e6b8 100644 --- a/tests/oauth-status-privacy.test.ts +++ b/tests/oauth-status-privacy.test.ts @@ -96,6 +96,46 @@ describe("OAuth status privacy", () => { expect(JSON.stringify(status)).not.toContain("oauth