fix(cli): Handle formatting errors in trustedFolders.json similar to settings file (#9167)

packages/cli/src/config/trustedFolders.test.ts

@@ -5,7 +5,7 @@
  */
 
 import * as osActual from 'node:os';
-import { ideContextStore } from '@google/gemini-cli-core';
+import { FatalConfigError, ideContextStore } from '@google/gemini-cli-core';
 import {
   describe,
   it,
@@ -24,6 +24,7 @@ import {
   getTrustedFoldersPath,
   TrustLevel,
   isWorkspaceTrusted,
+  resetTrustedFoldersForTesting,
 } from './trustedFolders.js';
 import type { Settings } from './settings.js';
 
@@ -55,6 +56,7 @@ describe('Trusted Folders Loading', () => {
   let mockFsWriteFileSync: Mocked<typeof fs.writeFileSync>;
 
   beforeEach(() => {
+    resetTrustedFoldersForTesting();
     vi.resetAllMocks();
     mockFsExistsSync = vi.mocked(fs.existsSync);
     mockStripJsonComments = vi.mocked(stripJsonComments);
@@ -208,6 +210,7 @@ describe('isWorkspaceTrusted', () => {
   };
 
   beforeEach(() => {
+    resetTrustedFoldersForTesting();
     vi.spyOn(process, 'cwd').mockImplementation(() => mockCwd);
     vi.spyOn(fs, 'readFileSync').mockImplementation((p) => {
       if (p === getTrustedFoldersPath()) {
@@ -226,6 +229,35 @@ describe('isWorkspaceTrusted', () => {
     Object.keys(mockRules).forEach((key) => delete mockRules[key]);
   });
 
+  it('should throw a fatal error if the config is malformed', () => {
+    mockCwd = '/home/user/projectA';
+    // This mock needs to be specific to this test to override the one in beforeEach
+    vi.spyOn(fs, 'readFileSync').mockImplementation((p) => {
+      if (p === getTrustedFoldersPath()) {
+        return '{"foo": "bar",}'; // Malformed JSON with trailing comma
+      }
+      return '{}';
+    });
+    expect(() => isWorkspaceTrusted(mockSettings)).toThrow(FatalConfigError);
+    expect(() => isWorkspaceTrusted(mockSettings)).toThrow(
+      /Please fix the configuration file/,
+    );
+  });
+
+  it('should throw a fatal error if the config is not a JSON object', () => {
+    mockCwd = '/home/user/projectA';
+    vi.spyOn(fs, 'readFileSync').mockImplementation((p) => {
+      if (p === getTrustedFoldersPath()) {
+        return 'null';
+      }
+      return '{}';
+    });
+    expect(() => isWorkspaceTrusted(mockSettings)).toThrow(FatalConfigError);
+    expect(() => isWorkspaceTrusted(mockSettings)).toThrow(
+      /not a valid JSON object/,
+    );
+  });
+
   it('should return true for a directly trusted folder', () => {
     mockCwd = '/home/user/projectA';
     mockRules['/home/user/projectA'] = TrustLevel.TRUST_FOLDER;
@@ -298,7 +330,9 @@ describe('isWorkspaceTrusted', () => {
 
 describe('isWorkspaceTrusted with IDE override', () => {
   afterEach(() => {
+    vi.clearAllMocks();
     ideContextStore.clear();
+    resetTrustedFoldersForTesting();
   });
 
   const mockSettings: Settings = {
@@ -359,3 +393,34 @@ describe('isWorkspaceTrusted with IDE override', () => {
     });
   });
 });
+
+describe('Trusted Folders Caching', () => {
+  beforeEach(() => {
+    resetTrustedFoldersForTesting();
+    vi.mocked(fs.existsSync).mockReturnValue(true);
+    vi.mocked(fs.readFileSync).mockReturnValue('{}');
+  });
+
+  afterEach(() => {
+    vi.restoreAllMocks();
+  });
+
+  it('should cache the loaded folders object', () => {
+    const readSpy = vi.spyOn(fs, 'readFileSync');
+
+    // First call should read the file
+    loadTrustedFolders();
+    expect(readSpy).toHaveBeenCalledTimes(1);
+
+    // Second call should use the cache
+    loadTrustedFolders();
+    expect(readSpy).toHaveBeenCalledTimes(1);
+
+    // Resetting should clear the cache
+    resetTrustedFoldersForTesting();
+
+    // Third call should read the file again
+    loadTrustedFolders();
+    expect(readSpy).toHaveBeenCalledTimes(2);
+  });
+});

packages/cli/src/config/trustedFolders.ts

@@ -8,6 +8,7 @@ import * as fs from 'node:fs';
 import * as path from 'node:path';
 import { homedir } from 'node:os';
 import {
+  FatalConfigError,
   getErrorMessage,
   isWithinRoot,
   ideContextStore,
@@ -114,9 +115,23 @@ export class LoadedTrustedFolders {
   }
 }
 
+let loadedTrustedFolders: LoadedTrustedFolders | undefined;
+
+/**
+ * FOR TESTING PURPOSES ONLY.
+ * Resets the in-memory cache of the trusted folders configuration.
+ */
+export function resetTrustedFoldersForTesting(): void {
+  loadedTrustedFolders = undefined;
+}
+
 export function loadTrustedFolders(): LoadedTrustedFolders {
+  if (loadedTrustedFolders) {
+    return loadedTrustedFolders;
+  }
+
   const errors: TrustedFoldersError[] = [];
-  const userConfig: Record<string, TrustLevel> = {};
+  let userConfig: Record<string, TrustLevel> = {};
 
   const userPath = getTrustedFoldersPath();
 
@@ -124,12 +139,19 @@ export function loadTrustedFolders(): LoadedTrustedFolders {
   try {
     if (fs.existsSync(userPath)) {
       const content = fs.readFileSync(userPath, 'utf-8');
-      const parsed = JSON.parse(stripJsonComments(content)) as Record<
-        string,
-        TrustLevel
-      >;
-      if (parsed) {
-        Object.assign(userConfig, parsed);
+      const parsed: unknown = JSON.parse(stripJsonComments(content));
+
+      if (
+        typeof parsed !== 'object' ||
+        parsed === null ||
+        Array.isArray(parsed)
+      ) {
+        errors.push({
+          message: 'Trusted folders file is not a valid JSON object.',
+          path: userPath,
+        });
+      } else {
+        userConfig = parsed as Record<string, TrustLevel>;
       }
     }
   } catch (error: unknown) {
@@ -139,10 +161,11 @@ export function loadTrustedFolders(): LoadedTrustedFolders {
     });
   }
 
-  return new LoadedTrustedFolders(
+  loadedTrustedFolders = new LoadedTrustedFolders(
     { path: userPath, config: userConfig },
     errors,
   );
+  return loadedTrustedFolders;
 }
 
 export function saveTrustedFolders(
@@ -181,11 +204,12 @@ function getWorkspaceTrustFromLocalConfig(
   }
 
   if (folders.errors.length > 0) {
-    for (const error of folders.errors) {
-      console.error(
-        `Error loading trusted folders config from ${error.path}: ${error.message}`,
-      );
-    }
+    const errorMessages = folders.errors.map(
+      (error) => `Error in ${error.path}: ${error.message}`,
+    );
+    throw new FatalConfigError(
+      `${errorMessages.join('\n')}\nPlease fix the configuration file and try again.`,
+    );
   }
 
   const isTrusted = folders.isPathTrusted(process.cwd());