Skip to content

Commit f062f56

Browse files
skeshiveskeshive
authored
feat(admin): apply MCP allowlist to extensions & gemini mcp list command (#18442)
1 parent 61d92c4 commit f062f56

12 files changed

Lines changed: 399 additions & 57 deletions

File tree

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

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ vi.mock('@google/gemini-cli-core', async (importOriginal) => {
3232
return {
3333
...original,
3434
createTransport: vi.fn(),
35+
3536
MCPServerStatus: {
3637
CONNECTED: 'CONNECTED',
3738
CONNECTING: 'CONNECTING',
@@ -223,4 +224,46 @@ describe('mcp list command', () => {
223224
),
224225
);
225226
});
227+
228+
it('should filter servers based on admin allowlist passed in settings', async () => {
229+
const settingsWithAllowlist = mergeSettings({}, {}, {}, {}, true);
230+
settingsWithAllowlist.admin = {
231+
secureModeEnabled: false,
232+
extensions: { enabled: true },
233+
skills: { enabled: true },
234+
mcp: {
235+
enabled: true,
236+
config: {
237+
'allowed-server': { url: 'http://allowed' },
238+
},
239+
},
240+
};
241+
242+
settingsWithAllowlist.mcpServers = {
243+
'allowed-server': { command: 'cmd1' },
244+
'forbidden-server': { command: 'cmd2' },
245+
};
246+
247+
mockedLoadSettings.mockReturnValue({
248+
merged: settingsWithAllowlist,
249+
});
250+
251+
mockClient.connect.mockResolvedValue(undefined);
252+
mockClient.ping.mockResolvedValue(undefined);
253+
254+
await listMcpServers(settingsWithAllowlist);
255+
256+
expect(debugLogger.log).toHaveBeenCalledWith(
257+
expect.stringContaining('allowed-server'),
258+
);
259+
expect(debugLogger.log).not.toHaveBeenCalledWith(
260+
expect.stringContaining('forbidden-server'),
261+
);
262+
expect(mockedCreateTransport).toHaveBeenCalledWith(
263+
'allowed-server',
264+
expect.objectContaining({ url: 'http://allowed' }), // Should use admin config
265+
false,
266+
expect.anything(),
267+
);
268+
});
226269
});

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

Lines changed: 41 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,14 @@
66

77
// File for 'gemini mcp list' command
88
import type { CommandModule } from 'yargs';
9-
import { loadSettings } from '../../config/settings.js';
9+
import { type MergedSettings, loadSettings } from '../../config/settings.js';
1010
import type { MCPServerConfig } from '@google/gemini-cli-core';
1111
import {
1212
MCPServerStatus,
1313
createTransport,
1414
debugLogger,
15+
applyAdminAllowlist,
16+
getAdminBlockedMcpServersMessage,
1517
} from '@google/gemini-cli-core';
1618
import { Client } from '@modelcontextprotocol/sdk/client/index.js';
1719
import { ExtensionManager } from '../../config/extension-manager.js';
@@ -24,18 +26,24 @@ const COLOR_YELLOW = '\u001b[33m';
2426
const COLOR_RED = '\u001b[31m';
2527
const RESET_COLOR = '\u001b[0m';
2628

27-
export async function getMcpServersFromConfig(): Promise<
28-
Record<string, MCPServerConfig>
29-
> {
30-
const settings = loadSettings();
29+
export async function getMcpServersFromConfig(
30+
settings?: MergedSettings,
31+
): Promise<{
32+
mcpServers: Record<string, MCPServerConfig>;
33+
blockedServerNames: string[];
34+
}> {
35+
if (!settings) {
36+
settings = loadSettings().merged;
37+
}
38+
3139
const extensionManager = new ExtensionManager({
32-
settings: settings.merged,
40+
settings,
3341
workspaceDir: process.cwd(),
3442
requestConsent: requestConsentNonInteractive,
3543
requestSetting: promptForSetting,
3644
});
3745
const extensions = await extensionManager.loadExtensions();
38-
const mcpServers = { ...settings.merged.mcpServers };
46+
const mcpServers = { ...settings.mcpServers };
3947
for (const extension of extensions) {
4048
Object.entries(extension.mcpServers || {}).forEach(([key, server]) => {
4149
if (mcpServers[key]) {
@@ -47,7 +55,11 @@ export async function getMcpServersFromConfig(): Promise<
4755
};
4856
});
4957
}
50-
return mcpServers;
58+
59+
const adminAllowlist = settings.admin?.mcp?.config;
60+
const filteredResult = applyAdminAllowlist(mcpServers, adminAllowlist);
61+
62+
return filteredResult;
5163
}
5264

5365
async function testMCPConnection(
@@ -103,12 +115,23 @@ async function getServerStatus(
103115
return testMCPConnection(serverName, server);
104116
}
105117

106-
export async function listMcpServers(): Promise<void> {
107-
const mcpServers = await getMcpServersFromConfig();
118+
export async function listMcpServers(settings?: MergedSettings): Promise<void> {
119+
const { mcpServers, blockedServerNames } =
120+
await getMcpServersFromConfig(settings);
108121
const serverNames = Object.keys(mcpServers);
109122

123+
if (blockedServerNames.length > 0) {
124+
const message = getAdminBlockedMcpServersMessage(
125+
blockedServerNames,
126+
undefined,
127+
);
128+
debugLogger.log(COLOR_YELLOW + message + RESET_COLOR + '\n');
129+
}
130+
110131
if (serverNames.length === 0) {
111-
debugLogger.log('No MCP servers configured.');
132+
if (blockedServerNames.length === 0) {
133+
debugLogger.log('No MCP servers configured.');
134+
}
112135
return;
113136
}
114137

@@ -154,11 +177,15 @@ export async function listMcpServers(): Promise<void> {
154177
}
155178
}
156179

157-
export const listCommand: CommandModule = {
180+
interface ListArgs {
181+
settings?: MergedSettings;
182+
}
183+
184+
export const listCommand: CommandModule<object, ListArgs> = {
158185
command: 'list',
159186
describe: 'List all configured MCP servers',
160-
handler: async () => {
161-
await listMcpServers();
187+
handler: async (argv) => {
188+
await listMcpServers(argv.settings);
162189
await exitCli();
163190
},
164191
};

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1511,7 +1511,7 @@ describe('loadCliConfig with admin.mcp.config', () => {
15111511
});
15121512
const config = await loadCliConfig(settings, 'test-session', argv);
15131513

1514-
const mergedServers = config.getMcpServers();
1514+
const mergedServers = config.getMcpServers() ?? {};
15151515
expect(mergedServers).toHaveProperty('serverA');
15161516
expect(mergedServers).not.toHaveProperty('serverB');
15171517
});
@@ -1569,9 +1569,9 @@ describe('loadCliConfig with admin.mcp.config', () => {
15691569
});
15701570
const config = await loadCliConfig(settings, 'test-session', argv);
15711571

1572-
const mergedServers = config.getMcpServers();
1572+
const mergedServers = config.getMcpServers() ?? {};
15731573
expect(mergedServers).not.toHaveProperty('serverC');
1574-
expect(Object.keys(mergedServers || {})).toHaveLength(0);
1574+
expect(Object.keys(mergedServers)).toHaveLength(0);
15751575
});
15761576

15771577
it('should merge local fields and prefer admin tool filters', async () => {
@@ -1601,7 +1601,7 @@ describe('loadCliConfig with admin.mcp.config', () => {
16011601
});
16021602
const config = await loadCliConfig(settings, 'test-session', argv);
16031603

1604-
const serverA = config.getMcpServers()?.['serverA'];
1604+
const serverA = (config.getMcpServers() ?? {})['serverA'];
16051605
expect(serverA).toMatchObject({
16061606
timeout: 1234,
16071607
includeTools: ['admin_tool'],

‎packages/cli/src/config/config.ts‎

Lines changed: 11 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,10 @@ import {
3636
GEMINI_MODEL_ALIAS_AUTO,
3737
getAdminErrorMessage,
3838
Config,
39+
applyAdminAllowlist,
40+
getAdminBlockedMcpServersMessage,
3941
} from '@google/gemini-cli-core';
4042
import type {
41-
MCPServerConfig,
4243
HookDefinition,
4344
HookEventName,
4445
OutputFormat,
@@ -692,38 +693,17 @@ export async function loadCliConfig(
692693
let mcpServers = mcpEnabled ? settings.mcpServers : {};
693694

694695
if (mcpEnabled && adminAllowlist && Object.keys(adminAllowlist).length > 0) {
695-
const filteredMcpServers: Record<string, MCPServerConfig> = {};
696-
for (const [serverId, localConfig] of Object.entries(mcpServers)) {
697-
const adminConfig = adminAllowlist[serverId];
698-
if (adminConfig) {
699-
const mergedConfig = {
700-
...localConfig,
701-
url: adminConfig.url,
702-
type: adminConfig.type,
703-
trust: adminConfig.trust,
704-
};
705-
706-
// Remove local connection details
707-
delete mergedConfig.command;
708-
delete mergedConfig.args;
709-
delete mergedConfig.env;
710-
delete mergedConfig.cwd;
711-
delete mergedConfig.httpUrl;
712-
delete mergedConfig.tcp;
713-
714-
if (
715-
(adminConfig.includeTools && adminConfig.includeTools.length > 0) ||
716-
(adminConfig.excludeTools && adminConfig.excludeTools.length > 0)
717-
) {
718-
mergedConfig.includeTools = adminConfig.includeTools;
719-
mergedConfig.excludeTools = adminConfig.excludeTools;
720-
}
696+
const result = applyAdminAllowlist(mcpServers, adminAllowlist);
697+
mcpServers = result.mcpServers;
698+
mcpServerCommand = undefined;
721699

722-
filteredMcpServers[serverId] = mergedConfig;
723-
}
700+
if (result.blockedServerNames && result.blockedServerNames.length > 0) {
701+
const message = getAdminBlockedMcpServersMessage(
702+
result.blockedServerNames,
703+
undefined,
704+
);
705+
coreEvents.emitConsoleLog('warn', message);
724706
}
725-
mcpServers = filteredMcpServers;
726-
mcpServerCommand = undefined;
727707
}
728708

729709
return new Config({

‎packages/cli/src/config/extension-manager.ts‎

Lines changed: 29 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,8 @@ import {
4848
type HookEventName,
4949
type ResolvedExtensionSetting,
5050
coreEvents,
51+
applyAdminAllowlist,
52+
getAdminBlockedMcpServersMessage,
5153
} from '@google/gemini-cli-core';
5254
import { maybeRequestConsentOrFail } from './extensions/consent.js';
5355
import { resolveEnvVarsInObject } from '../utils/envVarResolver.js';
@@ -661,12 +663,33 @@ Would you like to attempt to install via "git clone" instead?`,
661663
if (this.settings.admin.mcp.enabled === false) {
662664
config.mcpServers = undefined;
663665
} else {
664-
config.mcpServers = Object.fromEntries(
665-
Object.entries(config.mcpServers).map(([key, value]) => [
666-
key,
667-
filterMcpConfig(value),
668-
]),
669-
);
666+
// Apply admin allowlist if configured
667+
const adminAllowlist = this.settings.admin.mcp.config;
668+
if (adminAllowlist && Object.keys(adminAllowlist).length > 0) {
669+
const result = applyAdminAllowlist(
670+
config.mcpServers,
671+
adminAllowlist,
672+
);
673+
config.mcpServers = result.mcpServers;
674+
675+
if (result.blockedServerNames.length > 0) {
676+
const message = getAdminBlockedMcpServersMessage(
677+
result.blockedServerNames,
678+
undefined,
679+
);
680+
coreEvents.emitConsoleLog('warn', message);
681+
}
682+
}
683+
684+
// Then apply local filtering/sanitization
685+
if (config.mcpServers) {
686+
config.mcpServers = Object.fromEntries(
687+
Object.entries(config.mcpServers).map(([key, value]) => [
688+
key,
689+
filterMcpConfig(value),
690+
]),
691+
);
692+
}
670693
}
671694
}
672695

‎packages/cli/src/deferred.test.ts‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,15 @@ describe('deferred', () => {
167167

168168
// Now manually run it to verify it captured correctly
169169
await runDeferredCommand(createMockSettings().merged);
170-
expect(originalHandler).toHaveBeenCalledWith(argv);
170+
expect(originalHandler).toHaveBeenCalledWith(
171+
expect.objectContaining({
172+
settings: expect.objectContaining({
173+
admin: expect.objectContaining({
174+
extensions: expect.objectContaining({ enabled: true }),
175+
}),
176+
}),
177+
}),
178+
);
171179
expect(mockExit).toHaveBeenCalledWith(ExitCodes.SUCCESS);
172180
});
173181

‎packages/cli/src/deferred.ts‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,13 @@ export async function runDeferredCommand(settings: MergedSettings) {
6363
process.exit(ExitCodes.FATAL_CONFIG_ERROR);
6464
}
6565

66-
await deferredCommand.handler(deferredCommand.argv);
66+
// Inject settings into argv
67+
const argvWithSettings = {
68+
...deferredCommand.argv,
69+
settings,
70+
};
71+
72+
await deferredCommand.handler(argvWithSettings);
6773
await runExitCleanup();
6874
process.exit(ExitCodes.SUCCESS);
6975
}

‎packages/core/src/code_assist/admin/admin_controls.test.ts‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import {
2020
sanitizeAdminSettings,
2121
stopAdminControlsPolling,
2222
getAdminErrorMessage,
23+
getAdminBlockedMcpServersMessage,
2324
} from './admin_controls.js';
2425
import type { CodeAssistServer } from '../server.js';
2526
import type { Config } from '../../config/config.js';
@@ -759,4 +760,55 @@ describe('Admin Controls', () => {
759760
);
760761
});
761762
});
763+
764+
describe('getAdminBlockedMcpServersMessage', () => {
765+
let mockConfig: Config;
766+
767+
beforeEach(() => {
768+
mockConfig = {} as Config;
769+
});
770+
771+
it('should show count for a single blocked server', () => {
772+
vi.mocked(getCodeAssistServer).mockReturnValue({
773+
projectId: 'test-project-123',
774+
} as CodeAssistServer);
775+
776+
const message = getAdminBlockedMcpServersMessage(
777+
['server-1'],
778+
mockConfig,
779+
);
780+
781+
expect(message).toBe(
782+
'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',
783+
);
784+
});
785+
786+
it('should show count for multiple blocked servers', () => {
787+
vi.mocked(getCodeAssistServer).mockReturnValue({
788+
projectId: 'test-project-123',
789+
} as CodeAssistServer);
790+
791+
const message = getAdminBlockedMcpServersMessage(
792+
['server-1', 'server-2', 'server-3'],
793+
mockConfig,
794+
);
795+
796+
expect(message).toBe(
797+
'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',
798+
);
799+
});
800+
801+
it('should format message correctly with no project ID', () => {
802+
vi.mocked(getCodeAssistServer).mockReturnValue(undefined);
803+
804+
const message = getAdminBlockedMcpServersMessage(
805+
['server-1', 'server-2'],
806+
mockConfig,
807+
);
808+
809+
expect(message).toBe(
810+
'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',
811+
);
812+
});
813+
});
762814
});

0 commit comments

Comments
 (0)