From 6b28e5e4dae7d90d02ca67eab0480c8111997fac Mon Sep 17 00:00:00 2001
From: Andy <119136210+AndyMik90@users.noreply.github.com>
Date: Fri, 6 Feb 2026 23:50:33 +0100
Subject: [PATCH] auto-claude:
 198-critical-oauth-token-revocation-causes-infinite-40 (#1747)

* auto-claude: subtask-1-1 - Add errorCode propagation in reactiveTokenRefresh()

* auto-claude: subtask-1-2 - Return null instead of revoked token for permanent errors in ensureValidToken()

* auto-claude: subtask-1-3 - Clear credential cache on invalid_grant error in refreshOAuthToken()

* auto-claude: subtask-1-4 - Clear revoked credentials on persistence failure (Bug #5)

When token refresh succeeds but persistence to keychain fails, the old
credentials in the keychain are now revoked server-side. This commit adds
defensive cache clearing in both ensureValidToken() and reactiveTokenRefresh()
to prevent serving revoked tokens from cache.

On app restart, Bugs #3 and #4 fixes will handle the revoked credentials
properly by returning null and clearing the cache, forcing re-authentication.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

* auto-claude: subtask-2-1 - Move authFailedProfiles marking before early return

---------

Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
---
 .../src/main/claude-profile/token-refresh.ts  | 52 +++++++++++++++----
 .../src/main/claude-profile/usage-monitor.ts  | 17 +++---
 2 files changed, 52 insertions(+), 17 deletions(-)

diff --git a/apps/frontend/src/main/claude-profile/token-refresh.ts b/apps/frontend/src/main/claude-profile/token-refresh.ts
index ee31418c..f5d114b6 100644
--- a/apps/frontend/src/main/claude-profile/token-refresh.ts
+++ b/apps/frontend/src/main/claude-profile/token-refresh.ts
@@ -162,9 +162,13 @@ export function formatTimeRemaining(ms: number | null): string {
  * The new tokens must be stored immediately.
  *
  * @param refreshToken - The refresh token to use
+ * @param configDir - Optional config directory for the profile (used to clear cache on error)
  * @returns Result containing new tokens or error information
  */
-export async function refreshOAuthToken(refreshToken: string): Promise<TokenRefreshResult> {
+export async function refreshOAuthToken(
+  refreshToken: string,
+  configDir?: string
+): Promise<TokenRefreshResult> {
   const isDebug = process.env.DEBUG === 'true';
 
   if (isDebug) {
@@ -228,6 +232,11 @@ export async function refreshOAuthToken(refreshToken: string): Promise<TokenRefr
             errorCode,
             errorDescription
           });
+
+          // Clear credential cache to ensure stale tokens aren't reused
+          // This prevents infinite loops where cached invalid tokens are repeatedly used
+          clearKeychainCache(configDir);
+
           return {
             success: false,
             error: `Token refresh failed: ${errorDescription}`,
@@ -381,16 +390,29 @@ export async function ensureValidToken(
   }
 
   // Step 4: Refresh the token
-  const refreshResult = await refreshOAuthToken(creds.refreshToken);
+  const refreshResult = await refreshOAuthToken(creds.refreshToken, expandedConfigDir);
 
   if (!refreshResult.success || !refreshResult.accessToken || !refreshResult.refreshToken || !refreshResult.expiresAt) {
     console.error('[TokenRefresh:ensureValidToken] Token refresh failed:', refreshResult.error);
-    // CRITICAL: When token refresh fails server-side, the old token may already be revoked.
-    // Returning the old token here is a best-effort fallback, but callers should be aware
-    // that it will likely result in 401 errors. The comment "it might still work" is optimistic.
-    // This scenario indicates the user needs to re-authenticate via OAuth flow.
+
+    // Check for permanent errors (revoked/invalid tokens)
+    const isPermanentError = refreshResult.errorCode === 'invalid_grant' ||
+                             refreshResult.errorCode === 'invalid_client';
+
+    if (isPermanentError) {
+      // Return null for permanent errors to prevent infinite 401 loops
+      console.error('[TokenRefresh:ensureValidToken] Permanent error detected, returning null token');
+      return {
+        token: null,
+        wasRefreshed: false,
+        error: `Token refresh failed: ${refreshResult.error}`,
+        errorCode: refreshResult.errorCode
+      };
+    }
+
+    // For transient errors (network issues, etc.), return old token as best-effort fallback
     return {
-      token: creds.token,  // WARNING: This token may be invalid/revoked
+      token: creds.token,
       wasRefreshed: false,
       error: `Token refresh failed: ${refreshResult.error}`,
       errorCode: refreshResult.errorCode
@@ -413,7 +435,12 @@ export async function ensureValidToken(
     // This is a critical error - we have new tokens but can't persist them
     console.error('[TokenRefresh:ensureValidToken] CRITICAL: Failed to persist refreshed tokens:', updateResult.error);
     console.error('[TokenRefresh:ensureValidToken] The new token will be lost on next restart!');
+    console.error('[TokenRefresh:ensureValidToken] Old credentials in keychain are now REVOKED and must be cleared on restart');
     persistenceFailed = true;
+
+    // Clear credential cache immediately to prevent serving revoked tokens from cache
+    // On restart, the revoked tokens will trigger re-authentication via Bugs #3 and #4 fixes
+    clearKeychainCache(expandedConfigDir);
     // Still return the new token for this session
   } else {
     if (isDebug) {
@@ -490,13 +517,14 @@ export async function reactiveTokenRefresh(
   }
 
   // Perform refresh
-  const refreshResult = await refreshOAuthToken(creds.refreshToken);
+  const refreshResult = await refreshOAuthToken(creds.refreshToken, expandedConfigDir);
 
   if (!refreshResult.success || !refreshResult.accessToken || !refreshResult.refreshToken || !refreshResult.expiresAt) {
     return {
       token: null,
       wasRefreshed: false,
-      error: `Reactive refresh failed: ${refreshResult.error}`
+      error: `Reactive refresh failed: ${refreshResult.error}`,
+      errorCode: refreshResult.errorCode
     };
   }
 
@@ -512,9 +540,15 @@ export async function reactiveTokenRefresh(
   let persistenceFailed = false;
   if (!updateResult.success) {
     console.error('[TokenRefresh:reactive] CRITICAL: Failed to persist refreshed tokens:', updateResult.error);
+    console.error('[TokenRefresh:reactive] Old credentials in keychain are now REVOKED and must be cleared on restart');
     persistenceFailed = true;
+
+    // Clear credential cache immediately to prevent serving revoked tokens from cache
+    // On restart, the revoked tokens will trigger re-authentication via Bugs #3 and #4 fixes
+    clearKeychainCache(expandedConfigDir);
   }
 
+  // Also clear cache on success to ensure fresh data is loaded next time
   clearKeychainCache(expandedConfigDir);
 
   if (onRefreshed) {
diff --git a/apps/frontend/src/main/claude-profile/usage-monitor.ts b/apps/frontend/src/main/claude-profile/usage-monitor.ts
index a89397b6..150d8e56 100644
--- a/apps/frontend/src/main/claude-profile/usage-monitor.ts
+++ b/apps/frontend/src/main/claude-profile/usage-monitor.ts
@@ -1152,15 +1152,8 @@ export class UsageMonitor extends EventEmitter {
       }
     }
 
-    const settings = profileManager.getAutoSwitchSettings();
-
-    // Proactive swap is only supported for OAuth profiles, not API profiles
-    if (isAPIProfile || !settings.enabled || !settings.proactiveSwapEnabled) {
-      this.debugLog('[UsageMonitor] Auth failure detected but proactive swap is disabled or using API profile, skipping swap');
-      return;
-    }
-
     // Mark this profile as auth-failed to prevent swap loops
+    // This MUST happen before the early return to prevent infinite loops
     this.authFailedProfiles.set(profileId, Date.now());
     this.debugLog('[UsageMonitor] Auth failure detected, marked profile as failed: ' + profileId);
 
@@ -1172,6 +1165,14 @@ export class UsageMonitor extends EventEmitter {
       }
     });
 
+    const settings = profileManager.getAutoSwitchSettings();
+
+    // Proactive swap is only supported for OAuth profiles, not API profiles
+    if (isAPIProfile || !settings.enabled || !settings.proactiveSwapEnabled) {
+      this.debugLog('[UsageMonitor] Auth failure detected but proactive swap is disabled or using API profile, skipping swap');
+      return;
+    }
+
     try {
       const excludeProfiles = Array.from(this.authFailedProfiles.keys());
       this.debugLog('[UsageMonitor] Attempting proactive swap (excluding failed profiles):', excludeProfiles);