fix(core): harden user account caching (#6501)

packages/core/src/utils/user_account.test.ts

@@ -99,18 +99,37 @@ describe('user_account', () => {
     it('should handle corrupted JSON by starting fresh', async () => {
       fs.mkdirSync(path.dirname(accountsFile()), { recursive: true });
       fs.writeFileSync(accountsFile(), 'not valid json');
-      const consoleDebugSpy = vi
+      const consoleLogSpy = vi
-        .spyOn(console, 'debug')
+        .spyOn(console, 'log')
         .mockImplementation(() => {});
 
       await cacheGoogleAccount('test1@google.com');
 
-      expect(consoleDebugSpy).toHaveBeenCalled();
+      expect(consoleLogSpy).toHaveBeenCalled();
       expect(JSON.parse(fs.readFileSync(accountsFile(), 'utf-8'))).toEqual({
         active: 'test1@google.com',
         old: [],
       });
     });
+
+    it('should handle valid JSON with incorrect schema by starting fresh', async () => {
+      fs.mkdirSync(path.dirname(accountsFile()), { recursive: true });
+      fs.writeFileSync(
+        accountsFile(),
+        JSON.stringify({ active: 'test1@google.com', old: 'not-an-array' }),
+      );
+      const consoleLogSpy = vi
+        .spyOn(console, 'log')
+        .mockImplementation(() => {});
+
+      await cacheGoogleAccount('test2@google.com');
+
+      expect(consoleLogSpy).toHaveBeenCalled();
+      expect(JSON.parse(fs.readFileSync(accountsFile(), 'utf-8'))).toEqual({
+        active: 'test2@google.com',
+        old: [],
+      });
+    });
   });
 
   describe('getCachedGoogleAccount', () => {
@@ -139,14 +158,21 @@ describe('user_account', () => {
     it('should return null and log if file is corrupted', () => {
       fs.mkdirSync(path.dirname(accountsFile()), { recursive: true });
       fs.writeFileSync(accountsFile(), '{ "active": "test@google.com"'); // Invalid JSON
-      const consoleDebugSpy = vi
+      const consoleLogSpy = vi
-        .spyOn(console, 'debug')
+        .spyOn(console, 'log')
         .mockImplementation(() => {});
 
       const account = getCachedGoogleAccount();
 
       expect(account).toBeNull();
-      expect(consoleDebugSpy).toHaveBeenCalled();
+      expect(consoleLogSpy).toHaveBeenCalled();
+    });
+
+    it('should return null if active key is missing', () => {
+      fs.mkdirSync(path.dirname(accountsFile()), { recursive: true });
+      fs.writeFileSync(accountsFile(), JSON.stringify({ old: [] }));
+      const account = getCachedGoogleAccount();
+      expect(account).toBeNull();
     });
   });
 
@@ -177,6 +203,56 @@ describe('user_account', () => {
       expect(stored.active).toBeNull();
       expect(stored.old).toEqual([]);
     });
+
+    it('should handle corrupted JSON by creating a fresh file', async () => {
+      fs.mkdirSync(path.dirname(accountsFile()), { recursive: true });
+      fs.writeFileSync(accountsFile(), 'not valid json');
+      const consoleLogSpy = vi
+        .spyOn(console, 'log')
+        .mockImplementation(() => {});
+
+      await clearCachedGoogleAccount();
+
+      expect(consoleLogSpy).toHaveBeenCalled();
+      const stored = JSON.parse(fs.readFileSync(accountsFile(), 'utf-8'));
+      expect(stored.active).toBeNull();
+      expect(stored.old).toEqual([]);
+    });
+
+    it('should be idempotent if active account is already null', async () => {
+      fs.mkdirSync(path.dirname(accountsFile()), { recursive: true });
+      fs.writeFileSync(
+        accountsFile(),
+        JSON.stringify({ active: null, old: ['old1@google.com'] }, null, 2),
+      );
+
+      await clearCachedGoogleAccount();
+
+      const stored = JSON.parse(fs.readFileSync(accountsFile(), 'utf-8'));
+      expect(stored.active).toBeNull();
+      expect(stored.old).toEqual(['old1@google.com']);
+    });
+
+    it('should not add a duplicate to the old list', async () => {
+      fs.mkdirSync(path.dirname(accountsFile()), { recursive: true });
+      fs.writeFileSync(
+        accountsFile(),
+        JSON.stringify(
+          {
+            active: 'active@google.com',
+            old: ['active@google.com'],
+          },
+          null,
+          2,
+        ),
+      );
+
+      await clearCachedGoogleAccount();
+
+      const stored = JSON.parse(fs.readFileSync(accountsFile(), 'utf-8'));
+      expect(stored.active).toBeNull();
+      expect(stored.old).toEqual(['active@google.com']);
+    });
   });
 
   describe('getLifetimeGoogleAccounts', () => {
@@ -193,12 +269,12 @@ describe('user_account', () => {
     it('should return 0 if the file is corrupted', () => {
       fs.mkdirSync(path.dirname(accountsFile()), { recursive: true });
       fs.writeFileSync(accountsFile(), 'invalid json');
-      const consoleDebugSpy = vi
+      const consoleLogSpy = vi
-        .spyOn(console, 'debug')
+        .spyOn(console, 'log')
         .mockImplementation(() => {});
 
       expect(getLifetimeGoogleAccounts()).toBe(0);
-      expect(consoleDebugSpy).toHaveBeenCalled();
+      expect(consoleLogSpy).toHaveBeenCalled();
     });
 
     it('should return 1 if there is only an active account', () => {
@@ -233,5 +309,31 @@ describe('user_account', () => {
       );
       expect(getLifetimeGoogleAccounts()).toBe(3);
     });
+
+    it('should handle valid JSON with incorrect schema by returning 0', () => {
+      fs.mkdirSync(path.dirname(accountsFile()), { recursive: true });
+      fs.writeFileSync(
+        accountsFile(),
+        JSON.stringify({ active: null, old: 1 }),
+      );
+      const consoleLogSpy = vi
+        .spyOn(console, 'log')
+        .mockImplementation(() => {});
+
+      expect(getLifetimeGoogleAccounts()).toBe(0);
+      expect(consoleLogSpy).toHaveBeenCalled();
+    });
+
+    it('should not double count if active account is also in old list', () => {
+      fs.mkdirSync(path.dirname(accountsFile()), { recursive: true });
+      fs.writeFileSync(
+        accountsFile(),
+        JSON.stringify({
+          active: 'test1@google.com',
+          old: ['test1@google.com', 'test2@google.com'],
+        }),
+      );
+      expect(getLifetimeGoogleAccounts()).toBe(2);
+    });
   });
 });

packages/core/src/utils/user_account.ts

@@ -5,7 +5,7 @@
  */
 
 import path from 'node:path';
-import { promises as fsp, existsSync, readFileSync } from 'node:fs';
+import { promises as fsp, readFileSync } from 'node:fs';
 import * as os from 'os';
 import { GEMINI_DIR, GOOGLE_ACCOUNTS_FILENAME } from './paths.js';
 
@@ -18,21 +18,66 @@ function getGoogleAccountsCachePath(): string {
   return path.join(os.homedir(), GEMINI_DIR, GOOGLE_ACCOUNTS_FILENAME);
 }
 
-async function readAccounts(filePath: string): Promise<UserAccounts> {
+/**
+ * Parses and validates the string content of an accounts file.
+ * @param content The raw string content from the file.
+ * @returns A valid UserAccounts object.
+ */
+function parseAndValidateAccounts(content: string): UserAccounts {
+  const defaultState = { active: null, old: [] };
+  if (!content.trim()) {
+    return defaultState;
+  }
+
+  const parsed = JSON.parse(content);
+
+  // Inlined validation logic
+  if (typeof parsed !== 'object' || parsed === null) {
+    console.log('Invalid accounts file schema, starting fresh.');
+    return defaultState;
+  }
+  const { active, old } = parsed as Partial<UserAccounts>;
+  const isValid =
+    (active === undefined || active === null || typeof active === 'string') &&
+    (old === undefined ||
+      (Array.isArray(old) && old.every((i) => typeof i === 'string')));
+
+  if (!isValid) {
+    console.log('Invalid accounts file schema, starting fresh.');
+    return defaultState;
+  }
+
+  return {
+    active: parsed.active ?? null,
+    old: parsed.old ?? [],
+  };
+}
+
+function readAccountsSync(filePath: string): UserAccounts {
+  const defaultState = { active: null, old: [] };
   try {
-    const content = await fsp.readFile(filePath, 'utf-8');
+    const content = readFileSync(filePath, 'utf-8');
-    if (!content.trim()) {
+    return parseAndValidateAccounts(content);
-      return { active: null, old: [] };
-    }
-    return JSON.parse(content) as UserAccounts;
   } catch (error) {
     if (error instanceof Error && 'code' in error && error.code === 'ENOENT') {
-      // File doesn't exist, which is fine.
+      return defaultState;
-      return { active: null, old: [] };
     }
-    // File is corrupted or not valid JSON, start with a fresh object.
+    console.log('Error during sync read of accounts, starting fresh.', error);
-    console.debug('Could not parse accounts file, starting fresh.', error);
+    return defaultState;
-    return { active: null, old: [] };
+  }
+}
+
+async function readAccounts(filePath: string): Promise<UserAccounts> {
+  const defaultState = { active: null, old: [] };
+  try {
+    const content = await fsp.readFile(filePath, 'utf-8');
+    return parseAndValidateAccounts(content);
+  } catch (error) {
+    if (error instanceof Error && 'code' in error && error.code === 'ENOENT') {
+      return defaultState;
+    }
+    console.log('Could not parse accounts file, starting fresh.', error);
+    return defaultState;
   }
 }
 
@@ -56,52 +101,23 @@ export async function cacheGoogleAccount(email: string): Promise<void> {
 }
 
 export function getCachedGoogleAccount(): string | null {
-  try {
+  const filePath = getGoogleAccountsCachePath();
-    const filePath = getGoogleAccountsCachePath();
+  const accounts = readAccountsSync(filePath);
-    if (existsSync(filePath)) {
+  return accounts.active;
-      const content = readFileSync(filePath, 'utf-8').trim();
-      if (!content) {
-        return null;
-      }
-      const accounts: UserAccounts = JSON.parse(content);
-      return accounts.active;
-    }
-    return null;
-  } catch (error) {
-    console.debug('Error reading cached Google Account:', error);
-    return null;
-  }
 }
 
 export function getLifetimeGoogleAccounts(): number {
-  try {
+  const filePath = getGoogleAccountsCachePath();
-    const filePath = getGoogleAccountsCachePath();
+  const accounts = readAccountsSync(filePath);
-    if (!existsSync(filePath)) {
+  const allAccounts = new Set(accounts.old);
-      return 0;
+  if (accounts.active) {
-    }
+    allAccounts.add(accounts.active);
-
-    const content = readFileSync(filePath, 'utf-8').trim();
-    if (!content) {
-      return 0;
-    }
-    const accounts: UserAccounts = JSON.parse(content);
-    let count = accounts.old.length;
-    if (accounts.active) {
-      count++;
-    }
-    return count;
-  } catch (error) {
-    console.debug('Error reading lifetime Google Accounts:', error);
-    return 0;
   }
+  return allAccounts.size;
 }
 
 export async function clearCachedGoogleAccount(): Promise<void> {
   const filePath = getGoogleAccountsCachePath();
-  if (!existsSync(filePath)) {
-    return;
-  }
-
   const accounts = await readAccounts(filePath);
 
   if (accounts.active) {