fix(oauth): guard local token expiry parsing against NaN

[Bruce-Yii]

Summary

Fixes #1366: an imported local CLI credential (Grok ~/.grok/auth.json or Claude .credentials.json) whose expires_at is invalid/unparseable (e.g. "not-a-date", NaN, epoch-garbage like 1970-01-22) was treated as valid and never refreshed, and ocx status showed ✓ logged in for it.

Root cause

src/oauth/local-token-detect.ts — new Date(entry.expires_at as string).getTime() returns NaN for unparseable strings. Downstream comparisons then misbehave:

• shouldAdoptGrokGeneration(): disk.expires <= now + refreshSkewMs is always false for NaN (passes the expiry gate); bothExpiriesExist is false because NaN > 0 is false, so it falls through to return true — the garbage credential is adopted as the authoritative generation (authoritative() in src/oauth/index.ts), and the refresh path never re-validates it.
• getLoginStatus() reported loggedIn: !!cred (credential exists), ignoring needsReauth — the CLI "OAuth logins" section printed ✓ logged in for reauth-required credentials.

Changes

1. src/oauth/local-token-detect.ts — detectGrokCliToken(): Number.isFinite guard on the parsed expires_at; non-finite → 0 (unknown → forces the refresh-validation path downstream instead of "valid forever").
2. src/oauth/local-token-detect.ts — shouldAdoptGrokGeneration(): explicit !Number.isFinite(disk.expires) → false (defense-in-depth; a garbage disk credential is never adopted as an upgrade).
3. src/oauth/local-token-detect.ts — parseClaudeOauthPayload(): Number.isFinite guard on expiresAt (strings/NaN → 0).
4. src/oauth/index.ts — getLoginStatus(): loggedIn = !!cred && !needsReauth. needsReauth is the authoritative login-failure signal; an expired-but-refreshable access token stays logged in per the lazy-refresh contract (request resolution refreshes it on demand). Invalid/unknown local-import expiries are handled solely at parse/adoption time (points 1–3), never by over-reporting login state.

Tests

• tests/local-token-detect.test.ts: +9 tests (string/NaN expiresAt → 0; detectGrokCliToken with unparseable/future expires_at; shouldAdoptGrokGeneration with NaN/0/expired/newer-valid disk expiry).
• tests/oauth-status-privacy.test.ts: +4 tests — expired / unknown-0 / non-finite (1e999) expiries stay logged in (lazy-refresh contract); needsReauth reports not logged in.
• tests/oauth-refresh.test.ts: +1 lifecycle regression — stored local-cli xAI credential + malformed disk expires_at: "not-a-date" → generation not adopted, refresh resolves with the stored refresh token (1 discovery + 1 token exchange), detaches to source: "oauth".

Verification (head 9468a57b, rebased onto dev 0757b10):

• Targeted: 51/51 pass
• OAuth suite (tests/oauth-* + local-token + cli-status + login-summary): 174/174 pass
• bun x tsc --noEmit → clean
• Full suite runs on CI (ubuntu/macos/windows)

Fixes #1366

Review readiness checklist

This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:

• All CI tests are green on my local testing.

• I pushed my PR to the latest dev commit.

• I resolved all correct Codex and CodeRabbit findings.

• My PR is ready for review.

Summary by CodeRabbit

• Bug Fixes

  • Login status now remains active for expired credentials that can be refreshed automatically.
  • Accounts requiring reauthentication are correctly reported as logged out.
  • Invalid or non-finite credential expiration values are handled safely.
  • Malformed stored credentials no longer prevent successful OAuth refreshes.

• Tests

  • Added coverage for credential expiration, refresh behavior, reauthentication, and local token detection.

[github-actions]

⚠️ Deterministic hygiene checks failed.

• unsponsored_surface — This changes an authentication, workflow, release-automation, or dependency surface. MAINTAINERS.md requires security review for these; ask a maintainer to apply maintainer-sponsored once they have reviewed it. Paths: src/oauth/index.ts, src/oauth/local-token-detect.ts.

[github-actions]

✅ READY

• all PR quality gates passed; the review readiness checklist is complete.

Review readiness checklist

• ✅ All CI tests are green on my local testing.
• ✅ I pushed my PR to the latest dev commit.
• ✅ I resolved all correct Codex and CodeRabbit findings.
• ✅ My PR is ready for review.

✅ 4/4 boxes ticked.

This pull request is already Ready for Review.
The review-ready label marks this PR as ready; review automation runs independently. If no CodeRabbit review appears, comment @coderabbitai review to request one.
Maintainers: @lidge-jun @Ingwannu @Wibias

[coderabbitai]

📝 Walkthrough

Walkthrough

OAuth credential imports now normalize invalid expiry values and reject unusable disk credentials. Login status remains true for credentials that can refresh lazily and becomes false when the active account requires reauthentication.

Changes

OAuth credential validity

Layer / File(s)	Summary
Local token expiry normalization and adoption src/oauth/local-token-detect.ts, tests/local-token-detect.test.ts	Grok and Claude expiry parsing converts invalid values to 0. Grok adoption rejects non-finite, unknown, and expired disk credentials. Tests cover parsing, missing files, adoption, and environment restoration.
Invalid credential refresh path tests/oauth-refresh.test.ts	A malformed Grok credential is rejected, then refreshed through the OAuth discovery and token endpoints with the stored refresh token.
OAuth login status validation src/oauth/index.ts, tests/oauth-status-privacy.test.ts	getLoginStatus checks needsReauth instead of expiry values when setting loggedIn. Tests cover expired, zero, non-finite, and reauthentication-required accounts.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant LocalCredential
  participant OAuthDiscovery
  participant OAuthTokenEndpoint
  participant LoginStatus
  LocalCredential->>OAuthDiscovery: reject malformed disk credential
  OAuthDiscovery->>OAuthTokenEndpoint: refresh with stored refresh token
  OAuthTokenEndpoint-->>LocalCredential: return OAuth credential
  LocalCredential->>LoginStatus: evaluate active account
  LoginStatus-->>LocalCredential: loggedIn depends on needsReauth

Loading

Possibly related PRs

• lidge-jun/opencodex#479: Directly overlaps with OAuth expiry, refresh, and credential adoption handling.

Suggested reviewers: ingwannu, lidge-jun

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The changes address issue #1366 by normalizing invalid expiries, rejecting invalid Grok tokens, preserving refresh flow, and honoring needsReauth in login status.
Out of Scope Changes check	✅ Passed	The source changes and regression tests remain within issue #1366 and the stated OAuth token validation objectives.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly identifies the OAuth fix for invalid local token expiry parsing, which is a central change in the pull request.

✨ Finishing Touches 💡 1

🛠️ Fix failing CI checks 💡

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

@github-actions[bot] The pull request is marked ready for review. I will review the changes.

⚠️ Action not completed

Already reviewed.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[Wibias]

Request changes — one behavioral regression plus a missing lifecycle regression test.

1. High: getLoginStatus() now treats any expired access token as logged out, even when the account still has a valid refresh token and needsReauth is false. That conflicts with the existing lazy-refresh contract: request resolution refreshes expired/near-expiry credentials on demand. This can make ocx status say “not logged in” for a normal refreshable account that will successfully refresh on the next request.

Please narrow this change so ordinary expired-but-refreshable OAuth credentials remain logged in. The invalid/unknown local-import expiry case should be handled separately, with needsReauth remaining authoritative for a true login failure. Add a regression covering: expired access token + valid refresh token + needsReauth === false => still logged in / refreshable.

2. Medium: the new tests cover parsing and shouldAdoptGrokGeneration(), but not the actual lifecycle where an existing source: "local-cli" credential encounters a malformed/new external generation during refresh/reconciliation. Initial xAI/Anthropic local import already attempts refresh when the expiry is invalid because the freshness comparison fails, so the important regression is the stored-local-credential -> malformed external generation -> refresh-resolution path. Please add an integration-level test for that lifecycle.

The Number.isFinite normalization itself looks good and scoped. After the behavioral fix/test, this branch also needs rebasing onto current dev and exact-head CI; the auth-surface hygiene gate additionally requires maintainer sponsorship/security review.

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

@github-actions The PR is marked ready for review.

The hygiene check remains pending. A maintainer must review the OAuth changes in src/oauth/index.ts and src/oauth/local-token-detect.ts for security impact. After that review, the maintainer can apply the maintainer-sponsored label.

_{You are interacting with an AI system.}

[Bruce-Yii]

Addressed the review (Wibias): both points are fixed in the new commits, rebased onto current dev (0757b10), head 9468a57b.

1. High — expired-but-refreshable credentials stay logged in. getLoginStatus() no longer inspects expiry: loggedIn = !!cred && !needsReauth. Expired access token with a valid refresh token remains "logged in" per the lazy-refresh contract; needsReauth is the authoritative login-failure signal. Invalid/unknown local-import expiries are handled only at parse/adoption time (Number.isFinite guards + shouldAdoptGrokGeneration refusing non-finite disk expiries → malformed generation never adopted, refresh path resolves it).

2. Medium — lifecycle regression test added in tests/oauth-refresh.test.ts: seeds a stored source: "local-cli" xAI credential + disk auth.json with expires_at: "not-a-date", asserts getValidAccessToken refreshes using the stored refresh token (1 discovery + 1 token exchange, no adoption, detaches to source: "oauth").

tests/oauth-status-privacy.test.ts updated to assert lazy-refresh semantics (expired / unknown-0 / non-finite 1e999 expiries all stay logged in). Verification: targeted 51/51, OAuth suite 174/174, tsc --noEmit clean. Rebase done; exact-head CI runs on this push; auth-surface hygiene gate awaits maintainer sponsorship.

[coderabbitai]

@github-actions The PR is ready for review. I will review the current head commit 9468a57b7312df75b214e94a70b5f48f06d0074b.

⚠️ Action not completed

Already reviewed.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

@github-actions[bot] Acknowledged. The readiness gate is complete for head commit 9468a57b7312df75b214e94a70b5f48f06d0074b.

_{You are interacting with an AI system.}

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

@github-actions[bot] Acknowledged. The PR readiness checklist and deterministic hygiene checks passed for head commit 9468a57b7312df75b214e94a70b5f48f06d0074b.

The PR remains ready for maintainer review.

_{You are interacting with an AI system.}

done

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

@github-actions[bot] Acknowledged. The readiness checklist and deterministic hygiene checks passed for head commit 9468a57b7312df75b214e94a70b5f48f06d0074b.

The PR is ready for maintainer review.

_{You are interacting with an AI system.}

[coderabbitai]

@github-actions[bot] The PR readiness gate passed for head 4d737e53c5a733bcd6c6c9d53dff5e040878a3e6. I will review the current changes.

⚠️ Action not completed

No files to review.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

@github-actions[bot] Acknowledged. The readiness gate passed for head 4d737e53c5a733bcd6c6c9d53dff5e040878a3e6.

The PR remains ready for maintainer review.

_{You are interacting with an AI system.}

[Wibias]

Thanks @Bruce-Yii — nice work on this. 🙌

The final revision fixed the malformed/non-finite local-token expiry handling, preserved the lazy-refresh contract for expired-but-refreshable credentials, and added the missing lifecycle coverage to prove malformed disk generations are rejected while refresh continues from the stored credential.

I re-reviewed the rebased head and no remaining code/security blocker was found. Merged — thank you for the careful follow-up fixes.