diff --git a/packages/cli/src/commands/mcp/list.test.ts b/packages/cli/src/commands/mcp/list.test.ts index 30d88af995..60912c51f5 100644 --- a/packages/cli/src/commands/mcp/list.test.ts +++ b/packages/cli/src/commands/mcp/list.test.ts @@ -32,6 +32,7 @@ vi.mock('@google/gemini-cli-core', async (importOriginal) => { return { ...original, createTransport: vi.fn(), + MCPServerStatus: { CONNECTED: 'CONNECTED', CONNECTING: 'CONNECTING', @@ -223,4 +224,46 @@ 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' }, + }, + }, + }; + + settingsWithAllowlist.mcpServers = { + 'allowed-server': { command: 'cmd1' }, + 'forbidden-server': { command: 'cmd2' }, + }; + + mockedLoadSettings.mockReturnValue({ + merged: settingsWithAllowlist, + }); + + mockClient.connect.mockResolvedValue(undefined); + mockClient.ping.mockResolvedValue(undefined); + + await listMcpServers(settingsWithAllowlist); + + expect(debugLogger.log).toHaveBeenCalledWith( + expect.stringContaining('allowed-server'), + ); + expect(debugLogger.log).not.toHaveBeenCalledWith( + expect.stringContaining('forbidden-server'), + ); + expect(mockedCreateTransport).toHaveBeenCalledWith( + 'allowed-server', + expect.objectContaining({ url: 'http://allowed' }), // Should use admin config + false, + expect.anything(), + ); + }); }); diff --git a/packages/cli/src/commands/mcp/list.ts b/packages/cli/src/commands/mcp/list.ts index 50fc222f71..d51093fbfa 100644 --- a/packages/cli/src/commands/mcp/list.ts +++ b/packages/cli/src/commands/mcp/list.ts @@ -6,12 +6,14 @@ // File for 'gemini mcp list' command import type { CommandModule } from 'yargs'; -import { loadSettings } from '../../config/settings.js'; +import { type MergedSettings, loadSettings } from '../../config/settings.js'; import type { MCPServerConfig } from '@google/gemini-cli-core'; import { MCPServerStatus, createTransport, debugLogger, + applyAdminAllowlist, + getAdminBlockedMcpServersMessage, } from '@google/gemini-cli-core'; import { Client } from '@modelcontextprotocol/sdk/client/index.js'; import { ExtensionManager } from '../../config/extension-manager.js'; @@ -24,18 +26,24 @@ const COLOR_YELLOW = '\u001b[33m'; const COLOR_RED = '\u001b[31m'; const RESET_COLOR = '\u001b[0m'; -export async function getMcpServersFromConfig(): Promise< - Record -> { - const settings = loadSettings(); +export async function getMcpServersFromConfig( + settings?: MergedSettings, +): Promise<{ + mcpServers: Record; + blockedServerNames: string[]; +}> { + if (!settings) { + settings = loadSettings().merged; + } + const extensionManager = new ExtensionManager({ - settings: settings.merged, + settings, workspaceDir: process.cwd(), requestConsent: requestConsentNonInteractive, requestSetting: promptForSetting, }); const extensions = await extensionManager.loadExtensions(); - const mcpServers = { ...settings.merged.mcpServers }; + const mcpServers = { ...settings.mcpServers }; for (const extension of extensions) { Object.entries(extension.mcpServers || {}).forEach(([key, server]) => { if (mcpServers[key]) { @@ -47,7 +55,11 @@ export async function getMcpServersFromConfig(): Promise< }; }); } - return mcpServers; + + const adminAllowlist = settings.admin?.mcp?.config; + const filteredResult = applyAdminAllowlist(mcpServers, adminAllowlist); + + return filteredResult; } async function testMCPConnection( @@ -103,12 +115,23 @@ async function getServerStatus( return testMCPConnection(serverName, server); } -export async function listMcpServers(): Promise { - const mcpServers = await getMcpServersFromConfig(); +export async function listMcpServers(settings?: MergedSettings): Promise { + const { mcpServers, blockedServerNames } = + await getMcpServersFromConfig(settings); const serverNames = Object.keys(mcpServers); + if (blockedServerNames.length > 0) { + const message = getAdminBlockedMcpServersMessage( + blockedServerNames, + undefined, + ); + debugLogger.log(COLOR_YELLOW + message + RESET_COLOR + '\n'); + } + if (serverNames.length === 0) { - debugLogger.log('No MCP servers configured.'); + if (blockedServerNames.length === 0) { + debugLogger.log('No MCP servers configured.'); + } return; } @@ -154,11 +177,15 @@ export async function listMcpServers(): Promise { } } -export const listCommand: CommandModule = { +interface ListArgs { + settings?: MergedSettings; +} + +export const listCommand: CommandModule = { command: 'list', describe: 'List all configured MCP servers', - handler: async () => { - await listMcpServers(); + handler: async (argv) => { + await listMcpServers(argv.settings); await exitCli(); }, }; diff --git a/packages/cli/src/config/config.test.ts b/packages/cli/src/config/config.test.ts index bc1c582a23..4342675500 100644 --- a/packages/cli/src/config/config.test.ts +++ b/packages/cli/src/config/config.test.ts @@ -1511,7 +1511,7 @@ describe('loadCliConfig with admin.mcp.config', () => { }); const config = await loadCliConfig(settings, 'test-session', argv); - const mergedServers = config.getMcpServers(); + const mergedServers = config.getMcpServers() ?? {}; expect(mergedServers).toHaveProperty('serverA'); expect(mergedServers).not.toHaveProperty('serverB'); }); @@ -1569,9 +1569,9 @@ describe('loadCliConfig with admin.mcp.config', () => { }); const config = await loadCliConfig(settings, 'test-session', argv); - const mergedServers = config.getMcpServers(); + const mergedServers = config.getMcpServers() ?? {}; expect(mergedServers).not.toHaveProperty('serverC'); - expect(Object.keys(mergedServers || {})).toHaveLength(0); + expect(Object.keys(mergedServers)).toHaveLength(0); }); it('should merge local fields and prefer admin tool filters', async () => { @@ -1601,7 +1601,7 @@ describe('loadCliConfig with admin.mcp.config', () => { }); const config = await loadCliConfig(settings, 'test-session', argv); - const serverA = config.getMcpServers()?.['serverA']; + const serverA = (config.getMcpServers() ?? {})['serverA']; expect(serverA).toMatchObject({ timeout: 1234, includeTools: ['admin_tool'], diff --git a/packages/cli/src/config/config.ts b/packages/cli/src/config/config.ts index f904922ba9..dec86e980c 100755 --- a/packages/cli/src/config/config.ts +++ b/packages/cli/src/config/config.ts @@ -36,9 +36,10 @@ import { GEMINI_MODEL_ALIAS_AUTO, getAdminErrorMessage, Config, + applyAdminAllowlist, + getAdminBlockedMcpServersMessage, } from '@google/gemini-cli-core'; import type { - MCPServerConfig, HookDefinition, HookEventName, OutputFormat, @@ -692,38 +693,17 @@ export async function loadCliConfig( let mcpServers = mcpEnabled ? settings.mcpServers : {}; if (mcpEnabled && adminAllowlist && Object.keys(adminAllowlist).length > 0) { - const filteredMcpServers: Record = {}; - for (const [serverId, localConfig] of Object.entries(mcpServers)) { - const adminConfig = adminAllowlist[serverId]; - if (adminConfig) { - const mergedConfig = { - ...localConfig, - url: adminConfig.url, - type: adminConfig.type, - trust: adminConfig.trust, - }; - - // Remove local connection details - delete mergedConfig.command; - delete mergedConfig.args; - delete mergedConfig.env; - delete mergedConfig.cwd; - delete mergedConfig.httpUrl; - delete mergedConfig.tcp; - - if ( - (adminConfig.includeTools && adminConfig.includeTools.length > 0) || - (adminConfig.excludeTools && adminConfig.excludeTools.length > 0) - ) { - mergedConfig.includeTools = adminConfig.includeTools; - mergedConfig.excludeTools = adminConfig.excludeTools; - } - - filteredMcpServers[serverId] = mergedConfig; - } - } - mcpServers = filteredMcpServers; + const result = applyAdminAllowlist(mcpServers, adminAllowlist); + mcpServers = result.mcpServers; mcpServerCommand = undefined; + + if (result.blockedServerNames && result.blockedServerNames.length > 0) { + const message = getAdminBlockedMcpServersMessage( + result.blockedServerNames, + undefined, + ); + coreEvents.emitConsoleLog('warn', message); + } } return new Config({ diff --git a/packages/cli/src/config/extension-manager.ts b/packages/cli/src/config/extension-manager.ts index 88edb500fe..820e4d4182 100644 --- a/packages/cli/src/config/extension-manager.ts +++ b/packages/cli/src/config/extension-manager.ts @@ -48,6 +48,8 @@ import { type HookEventName, type ResolvedExtensionSetting, coreEvents, + applyAdminAllowlist, + getAdminBlockedMcpServersMessage, } from '@google/gemini-cli-core'; import { maybeRequestConsentOrFail } from './extensions/consent.js'; import { resolveEnvVarsInObject } from '../utils/envVarResolver.js'; @@ -661,12 +663,33 @@ Would you like to attempt to install via "git clone" instead?`, if (this.settings.admin.mcp.enabled === false) { config.mcpServers = undefined; } else { - config.mcpServers = Object.fromEntries( - Object.entries(config.mcpServers).map(([key, value]) => [ - key, - filterMcpConfig(value), - ]), - ); + // Apply admin allowlist if configured + const adminAllowlist = this.settings.admin.mcp.config; + if (adminAllowlist && Object.keys(adminAllowlist).length > 0) { + const result = applyAdminAllowlist( + config.mcpServers, + adminAllowlist, + ); + config.mcpServers = result.mcpServers; + + if (result.blockedServerNames.length > 0) { + const message = getAdminBlockedMcpServersMessage( + result.blockedServerNames, + undefined, + ); + coreEvents.emitConsoleLog('warn', message); + } + } + + // Then apply local filtering/sanitization + if (config.mcpServers) { + config.mcpServers = Object.fromEntries( + Object.entries(config.mcpServers).map(([key, value]) => [ + key, + filterMcpConfig(value), + ]), + ); + } } } diff --git a/packages/cli/src/deferred.test.ts b/packages/cli/src/deferred.test.ts index 08cbb3a093..99b86c9827 100644 --- a/packages/cli/src/deferred.test.ts +++ b/packages/cli/src/deferred.test.ts @@ -167,7 +167,15 @@ describe('deferred', () => { // Now manually run it to verify it captured correctly await runDeferredCommand(createMockSettings().merged); - expect(originalHandler).toHaveBeenCalledWith(argv); + expect(originalHandler).toHaveBeenCalledWith( + expect.objectContaining({ + settings: expect.objectContaining({ + admin: expect.objectContaining({ + extensions: expect.objectContaining({ enabled: true }), + }), + }), + }), + ); expect(mockExit).toHaveBeenCalledWith(ExitCodes.SUCCESS); }); diff --git a/packages/cli/src/deferred.ts b/packages/cli/src/deferred.ts index 309233ba45..dec6d9d114 100644 --- a/packages/cli/src/deferred.ts +++ b/packages/cli/src/deferred.ts @@ -63,7 +63,13 @@ export async function runDeferredCommand(settings: MergedSettings) { process.exit(ExitCodes.FATAL_CONFIG_ERROR); } - await deferredCommand.handler(deferredCommand.argv); + // Inject settings into argv + const argvWithSettings = { + ...deferredCommand.argv, + settings, + }; + + await deferredCommand.handler(argvWithSettings); await runExitCleanup(); process.exit(ExitCodes.SUCCESS); } diff --git a/packages/core/src/code_assist/admin/admin_controls.test.ts b/packages/core/src/code_assist/admin/admin_controls.test.ts index 57849ae3a4..0606d7f255 100644 --- a/packages/core/src/code_assist/admin/admin_controls.test.ts +++ b/packages/core/src/code_assist/admin/admin_controls.test.ts @@ -20,6 +20,7 @@ import { sanitizeAdminSettings, stopAdminControlsPolling, getAdminErrorMessage, + getAdminBlockedMcpServersMessage, } from './admin_controls.js'; import type { CodeAssistServer } from '../server.js'; import type { Config } from '../../config/config.js'; @@ -759,4 +760,55 @@ describe('Admin Controls', () => { ); }); }); + + describe('getAdminBlockedMcpServersMessage', () => { + let mockConfig: Config; + + beforeEach(() => { + mockConfig = {} as Config; + }); + + it('should show count for a single blocked server', () => { + vi.mocked(getCodeAssistServer).mockReturnValue({ + projectId: 'test-project-123', + } as CodeAssistServer); + + const message = getAdminBlockedMcpServersMessage( + ['server-1'], + mockConfig, + ); + + expect(message).toBe( + '1 MCP server is not allowlisted by your administrator. To enable it, please request an update to the settings at: https://goo.gle/manage-gemini-cli?project=test-project-123', + ); + }); + + it('should show count for multiple blocked servers', () => { + vi.mocked(getCodeAssistServer).mockReturnValue({ + projectId: 'test-project-123', + } as CodeAssistServer); + + const message = getAdminBlockedMcpServersMessage( + ['server-1', 'server-2', 'server-3'], + mockConfig, + ); + + expect(message).toBe( + '3 MCP servers are not allowlisted by your administrator. To enable them, please request an update to the settings at: https://goo.gle/manage-gemini-cli?project=test-project-123', + ); + }); + + it('should format message correctly with no project ID', () => { + vi.mocked(getCodeAssistServer).mockReturnValue(undefined); + + const message = getAdminBlockedMcpServersMessage( + ['server-1', 'server-2'], + mockConfig, + ); + + expect(message).toBe( + '2 MCP servers are not allowlisted by your administrator. To enable them, please request an update to the settings at: https://goo.gle/manage-gemini-cli', + ); + }); + }); }); diff --git a/packages/core/src/code_assist/admin/admin_controls.ts b/packages/core/src/code_assist/admin/admin_controls.ts index cfd34225a6..43816215a1 100644 --- a/packages/core/src/code_assist/admin/admin_controls.ts +++ b/packages/core/src/code_assist/admin/admin_controls.ts @@ -238,3 +238,25 @@ export function getAdminErrorMessage( const projectParam = projectId ? `?project=${projectId}` : ''; return `${featureName} is disabled by your administrator. To enable it, please request an update to the settings at: https://goo.gle/manage-gemini-cli${projectParam}`; } + +/** + * Returns a standardized error message for MCP servers blocked by the admin allowlist. + * + * @param blockedServers List of blocked server names + * @param config The application config + * @returns The formatted error message + */ +export function getAdminBlockedMcpServersMessage( + blockedServers: string[], + config: Config | undefined, +): string { + const server = config ? getCodeAssistServer(config) : undefined; + const projectId = server?.projectId; + const projectParam = projectId ? `?project=${projectId}` : ''; + const count = blockedServers.length; + const serverText = count === 1 ? 'server is' : 'servers are'; + + return `${count} MCP ${serverText} not allowlisted by your administrator. To enable ${ + count === 1 ? 'it' : 'them' + }, please request an update to the settings at: https://goo.gle/manage-gemini-cli${projectParam}`; +} diff --git a/packages/core/src/code_assist/admin/mcpUtils.test.ts b/packages/core/src/code_assist/admin/mcpUtils.test.ts new file mode 100644 index 0000000000..313e654d7d --- /dev/null +++ b/packages/core/src/code_assist/admin/mcpUtils.test.ts @@ -0,0 +1,113 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect } from 'vitest'; +import { applyAdminAllowlist } from './mcpUtils.js'; +import type { MCPServerConfig } from '../../config/config.js'; + +describe('applyAdminAllowlist', () => { + it('should return original servers if no allowlist provided', () => { + const localServers: Record = { + server1: { command: 'cmd1' }, + }; + expect(applyAdminAllowlist(localServers, undefined)).toEqual({ + mcpServers: localServers, + blockedServerNames: [], + }); + }); + + it('should return original servers if allowlist is empty', () => { + const localServers: Record = { + server1: { command: 'cmd1' }, + }; + expect(applyAdminAllowlist(localServers, {})).toEqual({ + mcpServers: localServers, + blockedServerNames: [], + }); + }); + + it('should filter servers not in allowlist', () => { + const localServers: Record = { + server1: { command: 'cmd1' }, + server2: { command: 'cmd2' }, + }; + const allowlist: Record = { + server1: { url: 'http://server1' }, + }; + + const result = applyAdminAllowlist(localServers, allowlist); + expect(Object.keys(result.mcpServers)).toEqual(['server1']); + expect(result.blockedServerNames).toEqual(['server2']); + }); + + it('should override connection details with allowlist values', () => { + const localServers: Record = { + server1: { + command: 'local-cmd', + args: ['local-arg'], + env: { LOCAL: 'true' }, + description: 'Local description', + }, + }; + const allowlist: Record = { + server1: { + url: 'http://admin-url', + type: 'sse', + trust: true, + }, + }; + + const result = applyAdminAllowlist(localServers, allowlist); + const server = result.mcpServers['server1']; + + expect(server).toBeDefined(); + expect(server?.url).toBe('http://admin-url'); + expect(server?.type).toBe('sse'); + expect(server?.trust).toBe(true); + // Should preserve other local fields + expect(server?.description).toBe('Local description'); + // Should remove local connection fields + expect(server?.command).toBeUndefined(); + expect(server?.args).toBeUndefined(); + expect(server?.env).toBeUndefined(); + }); + + it('should apply tool restrictions from allowlist', () => { + const localServers: Record = { + server1: { command: 'cmd1' }, + }; + const allowlist: Record = { + server1: { + url: 'http://url', + includeTools: ['tool1'], + excludeTools: ['tool2'], + }, + }; + + const result = applyAdminAllowlist(localServers, allowlist); + expect(result.mcpServers['server1']?.includeTools).toEqual(['tool1']); + expect(result.mcpServers['server1']?.excludeTools).toEqual(['tool2']); + }); + + it('should not apply empty tool restrictions from allowlist', () => { + const localServers: Record = { + server1: { + command: 'cmd1', + includeTools: ['local-tool'], + }, + }; + const allowlist: Record = { + server1: { + url: 'http://url', + includeTools: [], + }, + }; + + const result = applyAdminAllowlist(localServers, allowlist); + // Should keep local tool restrictions if admin ones are empty/undefined + expect(result.mcpServers['server1']?.includeTools).toEqual(['local-tool']); + }); +}); diff --git a/packages/core/src/code_assist/admin/mcpUtils.ts b/packages/core/src/code_assist/admin/mcpUtils.ts new file mode 100644 index 0000000000..12c5845d5b --- /dev/null +++ b/packages/core/src/code_assist/admin/mcpUtils.ts @@ -0,0 +1,67 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import type { MCPServerConfig } from '../../config/config.js'; + +/** + * Applies the admin allowlist to the local MCP servers. + * + * If an admin allowlist is provided and not empty, this function filters the + * local servers to only those present in the allowlist. It also overrides + * connection details (url, type, trust) with the admin configuration and + * removes local execution details (command, args, env, cwd). + * + * @param localMcpServers The locally configured MCP servers. + * @param adminAllowlist The admin allowlist configuration. + * @returns The filtered and merged MCP servers. + */ +export function applyAdminAllowlist( + localMcpServers: Record, + adminAllowlist: Record | undefined, +): { + mcpServers: Record; + blockedServerNames: string[]; +} { + if (!adminAllowlist || Object.keys(adminAllowlist).length === 0) { + return { mcpServers: localMcpServers, blockedServerNames: [] }; + } + + const filteredMcpServers: Record = {}; + const blockedServerNames: string[] = []; + + for (const [serverId, localConfig] of Object.entries(localMcpServers)) { + const adminConfig = adminAllowlist[serverId]; + if (adminConfig) { + const mergedConfig = { + ...localConfig, + url: adminConfig.url, + type: adminConfig.type, + trust: adminConfig.trust, + }; + + // Remove local connection details + delete mergedConfig.command; + delete mergedConfig.args; + delete mergedConfig.env; + delete mergedConfig.cwd; + delete mergedConfig.httpUrl; + delete mergedConfig.tcp; + + if ( + (adminConfig.includeTools && adminConfig.includeTools.length > 0) || + (adminConfig.excludeTools && adminConfig.excludeTools.length > 0) + ) { + mergedConfig.includeTools = adminConfig.includeTools; + mergedConfig.excludeTools = adminConfig.excludeTools; + } + + filteredMcpServers[serverId] = mergedConfig; + } else { + blockedServerNames.push(serverId); + } + } + return { mcpServers: filteredMcpServers, blockedServerNames }; +} diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index b06a416176..856a896b3a 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -51,6 +51,7 @@ export * from './code_assist/setup.js'; export * from './code_assist/types.js'; export * from './code_assist/telemetry.js'; export * from './code_assist/admin/admin_controls.js'; +export * from './code_assist/admin/mcpUtils.js'; export * from './core/apiKeyCredentialStorage.js'; // Export utilities