fix(cli): improve mcp list UX in untrusted folders (#26457)

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

packages/cli/src/commands/mcp/list.test.ts

@@ -14,16 +14,17 @@ import {
   type Mock,
 } from 'vitest';
 import { listMcpServers } from './list.js';
+import { loadSettings } from '../../config/settings.js';
 import {
-  loadSettings,
-  mergeSettings,
-  type LoadedSettings,
-} from '../../config/settings.js';
-import { createTransport, debugLogger } from '@google/gemini-cli-core';
+  createTransport,
+  debugLogger,
+  type AdminControlsSettings,
+} from '@google/gemini-cli-core';
 import { Client } from '@modelcontextprotocol/sdk/client/index.js';
 import { ExtensionStorage } from '../../config/extensions/storage.js';
 import { ExtensionManager } from '../../config/extension-manager.js';
 import { McpServerEnablementManager } from '../../config/mcp/index.js';
+import { createMockSettings } from '../../test-utils/settings.js';
 
 vi.mock('../../config/settings.js', async (importOriginal) => {
   const actual =
@@ -133,10 +134,7 @@ describe('mcp list command', () => {
   });
 
   it('should display message when no servers configured', async () => {
-    const defaultMergedSettings = mergeSettings({}, {}, {}, {}, true);
-    mockedLoadSettings.mockReturnValue({
-      merged: { ...defaultMergedSettings, mcpServers: {} },
-    });
+    mockedLoadSettings.mockReturnValue(createMockSettings({ mcpServers: {} }));
 
     await listMcpServers();
 
@@ -144,10 +142,8 @@ describe('mcp list command', () => {
   });
 
   it('should display different server types with connected status', async () => {
-    const defaultMergedSettings = mergeSettings({}, {}, {}, {}, true);
-    mockedLoadSettings.mockReturnValue({
-      merged: {
-        ...defaultMergedSettings,
+    mockedLoadSettings.mockReturnValue(
+      createMockSettings({
         mcpServers: {
           'stdio-server': { command: '/path/to/server', args: ['arg1'] },
           'sse-server': { url: 'https://example.com/sse', type: 'sse' },
@@ -158,9 +154,9 @@ describe('mcp list command', () => {
             type: 'http',
           },
         },
-      },
-      isTrusted: true,
-    });
+        isTrusted: true,
+      }),
+    );
 
     mockClient.connect.mockResolvedValue(undefined);
     mockClient.ping.mockResolvedValue(undefined);
@@ -196,15 +192,14 @@ describe('mcp list command', () => {
   });
 
   it('should display disconnected status when connection fails', async () => {
-    const defaultMergedSettings = mergeSettings({}, {}, {}, {}, true);
-    mockedLoadSettings.mockReturnValue({
-      merged: {
-        ...defaultMergedSettings,
+    mockedLoadSettings.mockReturnValue(
+      createMockSettings({
         mcpServers: {
           'test-server': { command: '/test/server' },
         },
-      },
-    });
+        isTrusted: true,
+      }),
+    );
 
     mockClient.connect.mockRejectedValue(new Error('Connection failed'));
 
@@ -218,16 +213,14 @@ describe('mcp list command', () => {
   });
 
   it('should display connected status even if ping fails', async () => {
-    const defaultMergedSettings = mergeSettings({}, {}, {}, {}, true);
-    mockedLoadSettings.mockReturnValue({
-      merged: {
-        ...defaultMergedSettings,
+    mockedLoadSettings.mockReturnValue(
+      createMockSettings({
         mcpServers: {
           'test-server': { command: '/test/server' },
         },
-      },
-      isTrusted: true,
-    });
+        isTrusted: true,
+      }),
+    );
 
     mockClient.connect.mockResolvedValue(undefined);
     mockClient.ping.mockRejectedValue(new Error('Ping failed'));
@@ -240,16 +233,14 @@ describe('mcp list command', () => {
   });
 
   it('should use configured timeout for connection', async () => {
-    const defaultMergedSettings = mergeSettings({}, {}, {}, {}, true);
-    mockedLoadSettings.mockReturnValue({
-      merged: {
-        ...defaultMergedSettings,
+    mockedLoadSettings.mockReturnValue(
+      createMockSettings({
         mcpServers: {
           'test-server': { command: '/test/server', timeout: 12345 },
         },
-      },
-      isTrusted: true,
-    });
+        isTrusted: true,
+      }),
+    );
 
     mockClient.connect.mockResolvedValue(undefined);
 
@@ -265,16 +256,14 @@ describe('mcp list command', () => {
   });
 
   it('should use default timeout for connection when not configured', async () => {
-    const defaultMergedSettings = mergeSettings({}, {}, {}, {}, true);
-    mockedLoadSettings.mockReturnValue({
-      merged: {
-        ...defaultMergedSettings,
+    mockedLoadSettings.mockReturnValue(
+      createMockSettings({
         mcpServers: {
           'test-server': { command: '/test/server' },
         },
-      },
-      isTrusted: true,
-    });
+        isTrusted: true,
+      }),
+    );
 
     mockClient.connect.mockResolvedValue(undefined);
 
@@ -290,16 +279,14 @@ describe('mcp list command', () => {
   });
 
   it('should merge extension servers with config servers', async () => {
-    const defaultMergedSettings = mergeSettings({}, {}, {}, {}, true);
-    mockedLoadSettings.mockReturnValue({
-      merged: {
-        ...defaultMergedSettings,
+    mockedLoadSettings.mockReturnValue(
+      createMockSettings({
         mcpServers: {
           'config-server': { command: '/config/server' },
         },
-      },
-      isTrusted: true,
-    });
+        isTrusted: true,
+      }),
+    );
 
     mockExtensionManager.loadExtensions.mockReturnValue([
       {
@@ -326,36 +313,40 @@ describe('mcp list command', () => {
   });
 
   it('should filter servers based on admin allowlist passed in settings', async () => {
-    const settingsWithAllowlist = mergeSettings({}, {}, {}, {}, true);
-    settingsWithAllowlist.admin = {
-      secureModeEnabled: false,
-      extensions: { enabled: true },
-      skills: { enabled: true },
-      mcp: {
-        enabled: true,
-        config: {
-          'allowed-server': { url: 'http://allowed' },
+    const adminControls = {
+      strictModeDisabled: true,
+      mcpSetting: {
+        mcpEnabled: true,
+        mcpConfig: {
+          mcpServers: {
+            'allowed-server': { url: 'http://allowed' },
+          },
         },
-        requiredConfig: {},
       },
     };
 
-    settingsWithAllowlist.mcpServers = {
+    const mcpServers = {
       'allowed-server': { command: 'cmd1' },
       'forbidden-server': { command: 'cmd2' },
     };
 
-    mockedLoadSettings.mockReturnValue({
-      merged: settingsWithAllowlist,
+    const mockSettings = createMockSettings({
+      mcpServers,
+      isTrusted: true,
     });
+    // setRemoteAdminSettings is the correct way to set admin settings in tests
+    (
+      mockSettings as unknown as {
+        setRemoteAdminSettings: (controls: AdminControlsSettings) => void;
+      }
+    ).setRemoteAdminSettings(adminControls as unknown as AdminControlsSettings);
+
+    mockedLoadSettings.mockReturnValue(mockSettings);
 
     mockClient.connect.mockResolvedValue(undefined);
     mockClient.ping.mockResolvedValue(undefined);
 
-    await listMcpServers({
-      merged: settingsWithAllowlist,
-      isTrusted: true,
-    } as unknown as LoadedSettings);
+    await listMcpServers(mockSettings);
 
     expect(debugLogger.log).toHaveBeenCalledWith(
       expect.stringContaining('allowed-server'),
@@ -371,44 +362,40 @@ describe('mcp list command', () => {
     );
   });
 
-  it('should show stdio servers as disconnected in untrusted folders', async () => {
-    const defaultMergedSettings = mergeSettings({}, {}, {}, {}, true);
-    mockedLoadSettings.mockReturnValue({
-      merged: {
-        ...defaultMergedSettings,
+  it('should show stdio servers as disabled in untrusted folders', async () => {
+    mockedLoadSettings.mockReturnValue(
+      createMockSettings({
         mcpServers: {
           'test-server': { command: '/test/server' },
         },
-      },
-      isTrusted: false,
-    });
-
-    // createTransport will throw in core if not trusted
-    mockedCreateTransport.mockRejectedValue(new Error('Folder not trusted'));
+        isTrusted: false,
+      }),
+    );
 
     await listMcpServers();
 
     expect(debugLogger.log).toHaveBeenCalledWith(
       expect.stringContaining(
-        'test-server: /test/server  (stdio) - Disconnected',
+        'Warning: MCP servers are configured but disabled because this folder is untrusted.',
       ),
     );
+    expect(debugLogger.log).toHaveBeenCalledWith(
+      expect.stringContaining('test-server: /test/server  (stdio) - Disabled'),
+    );
   });
 
   it('should display blocked status for servers in excluded list', async () => {
-    const defaultMergedSettings = mergeSettings({}, {}, {}, {}, true);
-    mockedLoadSettings.mockReturnValue({
-      merged: {
-        ...defaultMergedSettings,
+    mockedLoadSettings.mockReturnValue(
+      createMockSettings({
         mcp: {
           excluded: ['blocked-server'],
         },
         mcpServers: {
           'blocked-server': { command: '/test/server' },
         },
-      },
-      isTrusted: true,
-    });
+        isTrusted: true,
+      }),
+    );
 
     await listMcpServers();
 
@@ -421,16 +408,14 @@ describe('mcp list command', () => {
   });
 
   it('should display disabled status for servers disabled via enablement manager', async () => {
-    const defaultMergedSettings = mergeSettings({}, {}, {}, {}, true);
-    mockedLoadSettings.mockReturnValue({
-      merged: {
-        ...defaultMergedSettings,
+    mockedLoadSettings.mockReturnValue(
+      createMockSettings({
         mcpServers: {
           'disabled-server': { command: '/test/server' },
         },
-      },
-      isTrusted: true,
-    });
+        isTrusted: true,
+      }),
+    );
 
     vi.spyOn(
       McpServerEnablementManager.prototype,
@@ -446,4 +431,48 @@ describe('mcp list command', () => {
     );
     expect(mockedCreateTransport).not.toHaveBeenCalled();
   });
+
+  it('should display warning and disabled status in untrusted folders', async () => {
+    const userMcpServers = {
+      'user-server': { url: 'https://example.com/user' },
+    };
+    const workspaceMcpServers = {
+      'project-server': { command: '/path/to/project/server' },
+    };
+
+    mockedLoadSettings.mockReturnValue(
+      createMockSettings({
+        user: {
+          settings: { mcpServers: userMcpServers },
+          originalSettings: { mcpServers: userMcpServers },
+          path: '/mock/user/settings.json',
+        },
+        workspace: {
+          settings: { mcpServers: workspaceMcpServers },
+          originalSettings: { mcpServers: workspaceMcpServers },
+          path: '/mock/workspace/settings.json',
+        },
+        isTrusted: false,
+      }),
+    );
+
+    await listMcpServers();
+
+    expect(debugLogger.log).toHaveBeenCalledWith(
+      expect.stringContaining(
+        'Warning: MCP servers are configured but disabled because this folder is untrusted.',
+      ),
+    );
+    expect(debugLogger.log).toHaveBeenCalledWith(
+      expect.stringContaining(
+        'project-server: /path/to/project/server  (stdio) - Disabled',
+      ),
+    );
+    expect(debugLogger.log).toHaveBeenCalledWith(
+      expect.stringContaining(
+        'user-server: https://example.com/user (http) - Disabled',
+      ),
+    );
+    expect(mockedCreateTransport).not.toHaveBeenCalled();
+  });
 });

packages/cli/src/commands/mcp/list.ts

@@ -179,6 +179,10 @@ async function getServerStatus(
     return MCPServerStatus.DISABLED;
   }
 
+  if (!isTrusted) {
+    return MCPServerStatus.DISABLED;
+  }
+
   // Test all server types by attempting actual connection
   return testMCPConnection(serverName, server, isTrusted, activeSettings);
 }
@@ -189,8 +193,14 @@ export async function listMcpServers(
   const loadedSettings = loadedSettingsArg ?? loadSettings();
   const activeSettings = loadedSettings.merged;
 
+  // If the folder is untrusted, we want to show all configured servers (including
+  // project-scoped ones) as disabled.
+  const allSettings = !loadedSettings.isTrusted
+    ? loadedSettings.getMergedSettingsAsIfTrusted()
+    : activeSettings;
+
   const { mcpServers, blockedServerNames } =
-    await getMcpServersFromConfig(activeSettings);
+    await getMcpServersFromConfig(allSettings);
   const serverNames = Object.keys(mcpServers);
 
   if (blockedServerNames.length > 0) {
@@ -208,6 +218,15 @@ export async function listMcpServers(
     return;
   }
 
+  if (!loadedSettings.isTrusted) {
+    debugLogger.log(
+      chalk.yellow(
+        'Warning: MCP servers are configured but disabled because this folder is untrusted.\n' +
+          'User-level servers are also suppressed in untrusted folders to prevent accidental side-effects.\n',
+      ),
+    );
+  }
+
   debugLogger.log('Configured MCP servers:\n');
 
   for (const serverName of serverNames) {

packages/cli/src/config/settings.ts

@@ -348,6 +348,15 @@ export class LoadedSettings {
     return this._merged;
   }
 
+  /**
+   * Returns a merged settings object as if the folder were trusted.
+   * This is useful for commands like 'mcp list' that want to show
+   * what's configured even if it's currently disabled for security reasons.
+   */
+  getMergedSettingsAsIfTrusted(): MergedSettings {
+    return this.computeMergedSettings(true);
+  }
+
   setTrusted(isTrusted: boolean): void {
     if (this.isTrusted === isTrusted) {
       return;
@@ -368,13 +377,16 @@ export class LoadedSettings {
     };
   }
 
-  private computeMergedSettings(): MergedSettings {
+  private computeMergedSettings(forceTrusted = false): MergedSettings {
+    const isTrusted = forceTrusted || this.isTrusted;
+    const workspace = forceTrusted ? this._workspaceFile : this.workspace;
+
     const merged = mergeSettings(
       this.system.settings,
       this.systemDefaults.settings,
       this.user.settings,
-      this.workspace.settings,
-      this.isTrusted,
+      workspace.settings,
+      isTrusted,
     );
 
     // Remote admin settings always take precedence and file-based admin settings