mirror of
https://github.com/google-gemini/gemini-cli.git
synced 2026-05-13 05:12:55 -07:00
feat(admin): Implement admin allowlist for MCP server configurations (#18311)
This commit is contained in:
@@ -995,6 +995,10 @@ their corresponding top-level category object in your `settings.json` file.
|
|||||||
- **Description:** If false, disallows MCP servers from being used.
|
- **Description:** If false, disallows MCP servers from being used.
|
||||||
- **Default:** `true`
|
- **Default:** `true`
|
||||||
|
|
||||||
|
- **`admin.mcp.config`** (object):
|
||||||
|
- **Description:** Admin-configured MCP servers.
|
||||||
|
- **Default:** `{}`
|
||||||
|
|
||||||
- **`admin.skills.enabled`** (boolean):
|
- **`admin.skills.enabled`** (boolean):
|
||||||
- **Description:** If false, disallows agent skills from being used.
|
- **Description:** If false, disallows agent skills from being used.
|
||||||
- **Default:** `true`
|
- **Default:** `true`
|
||||||
|
|||||||
@@ -18,6 +18,7 @@ import {
|
|||||||
type ExtensionLoader,
|
type ExtensionLoader,
|
||||||
debugLogger,
|
debugLogger,
|
||||||
ApprovalMode,
|
ApprovalMode,
|
||||||
|
type MCPServerConfig,
|
||||||
} from '@google/gemini-cli-core';
|
} from '@google/gemini-cli-core';
|
||||||
import { loadCliConfig, parseArguments, type CliArgs } from './config.js';
|
import { loadCliConfig, parseArguments, type CliArgs } from './config.js';
|
||||||
import { type Settings, createTestMergedSettings } from './settings.js';
|
import { type Settings, createTestMergedSettings } from './settings.js';
|
||||||
@@ -1441,6 +1442,211 @@ describe('loadCliConfig with allowed-mcp-server-names', () => {
|
|||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe('loadCliConfig with admin.mcp.config', () => {
|
||||||
|
beforeEach(() => {
|
||||||
|
vi.resetAllMocks();
|
||||||
|
vi.mocked(os.homedir).mockReturnValue('/mock/home/user');
|
||||||
|
vi.stubEnv('GEMINI_API_KEY', 'test-api-key');
|
||||||
|
vi.spyOn(ExtensionManager.prototype, 'getExtensions').mockReturnValue([]);
|
||||||
|
});
|
||||||
|
|
||||||
|
afterEach(() => {
|
||||||
|
vi.unstubAllEnvs();
|
||||||
|
vi.restoreAllMocks();
|
||||||
|
});
|
||||||
|
|
||||||
|
const localMcpServers: Record<string, MCPServerConfig> = {
|
||||||
|
serverA: {
|
||||||
|
command: 'npx',
|
||||||
|
args: ['-y', '@mcp/server-a'],
|
||||||
|
env: { KEY: 'VALUE' },
|
||||||
|
cwd: '/local/cwd',
|
||||||
|
trust: false,
|
||||||
|
},
|
||||||
|
serverB: {
|
||||||
|
command: 'npx',
|
||||||
|
args: ['-y', '@mcp/server-b'],
|
||||||
|
trust: false,
|
||||||
|
},
|
||||||
|
};
|
||||||
|
|
||||||
|
const baseSettings = createTestMergedSettings({
|
||||||
|
mcp: { serverCommand: 'npx -y @mcp/default-server' },
|
||||||
|
mcpServers: localMcpServers,
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should use local configuration if admin allowlist is empty', async () => {
|
||||||
|
process.argv = ['node', 'script.js'];
|
||||||
|
const argv = await parseArguments(createTestMergedSettings());
|
||||||
|
const settings = createTestMergedSettings({
|
||||||
|
mcp: baseSettings.mcp,
|
||||||
|
mcpServers: localMcpServers,
|
||||||
|
admin: {
|
||||||
|
...baseSettings.admin,
|
||||||
|
mcp: { enabled: true, config: {} },
|
||||||
|
},
|
||||||
|
});
|
||||||
|
const config = await loadCliConfig(settings, 'test-session', argv);
|
||||||
|
expect(config.getMcpServers()).toEqual(localMcpServers);
|
||||||
|
expect(config.getMcpServerCommand()).toBe('npx -y @mcp/default-server');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should ignore locally configured servers not present in the allowlist', async () => {
|
||||||
|
process.argv = ['node', 'script.js'];
|
||||||
|
const argv = await parseArguments(createTestMergedSettings());
|
||||||
|
const adminAllowlist: Record<string, MCPServerConfig> = {
|
||||||
|
serverA: {
|
||||||
|
type: 'sse',
|
||||||
|
url: 'https://admin-server-a.com/sse',
|
||||||
|
trust: true,
|
||||||
|
},
|
||||||
|
};
|
||||||
|
const settings = createTestMergedSettings({
|
||||||
|
mcp: baseSettings.mcp,
|
||||||
|
mcpServers: localMcpServers,
|
||||||
|
admin: {
|
||||||
|
...baseSettings.admin,
|
||||||
|
mcp: { enabled: true, config: adminAllowlist },
|
||||||
|
},
|
||||||
|
});
|
||||||
|
const config = await loadCliConfig(settings, 'test-session', argv);
|
||||||
|
|
||||||
|
const mergedServers = config.getMcpServers();
|
||||||
|
expect(mergedServers).toHaveProperty('serverA');
|
||||||
|
expect(mergedServers).not.toHaveProperty('serverB');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should clear command, args, env, and cwd for present servers', async () => {
|
||||||
|
process.argv = ['node', 'script.js'];
|
||||||
|
const argv = await parseArguments(createTestMergedSettings());
|
||||||
|
const adminAllowlist: Record<string, MCPServerConfig> = {
|
||||||
|
serverA: {
|
||||||
|
type: 'sse',
|
||||||
|
url: 'https://admin-server-a.com/sse',
|
||||||
|
trust: true,
|
||||||
|
},
|
||||||
|
};
|
||||||
|
const settings = createTestMergedSettings({
|
||||||
|
mcpServers: localMcpServers,
|
||||||
|
admin: {
|
||||||
|
...baseSettings.admin,
|
||||||
|
mcp: { enabled: true, config: adminAllowlist },
|
||||||
|
},
|
||||||
|
});
|
||||||
|
const config = await loadCliConfig(settings, 'test-session', argv);
|
||||||
|
|
||||||
|
const serverA = config.getMcpServers()?.['serverA'];
|
||||||
|
expect(serverA).toEqual({
|
||||||
|
...localMcpServers['serverA'],
|
||||||
|
type: 'sse',
|
||||||
|
url: 'https://admin-server-a.com/sse',
|
||||||
|
trust: true,
|
||||||
|
command: undefined,
|
||||||
|
args: undefined,
|
||||||
|
env: undefined,
|
||||||
|
cwd: undefined,
|
||||||
|
httpUrl: undefined,
|
||||||
|
tcp: undefined,
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should not initialize a server if it is in allowlist but missing locally', async () => {
|
||||||
|
process.argv = ['node', 'script.js'];
|
||||||
|
const argv = await parseArguments(createTestMergedSettings());
|
||||||
|
const adminAllowlist: Record<string, MCPServerConfig> = {
|
||||||
|
serverC: {
|
||||||
|
type: 'sse',
|
||||||
|
url: 'https://admin-server-c.com/sse',
|
||||||
|
trust: true,
|
||||||
|
},
|
||||||
|
};
|
||||||
|
const settings = createTestMergedSettings({
|
||||||
|
mcpServers: localMcpServers,
|
||||||
|
admin: {
|
||||||
|
...baseSettings.admin,
|
||||||
|
mcp: { enabled: true, config: adminAllowlist },
|
||||||
|
},
|
||||||
|
});
|
||||||
|
const config = await loadCliConfig(settings, 'test-session', argv);
|
||||||
|
|
||||||
|
const mergedServers = config.getMcpServers();
|
||||||
|
expect(mergedServers).not.toHaveProperty('serverC');
|
||||||
|
expect(Object.keys(mergedServers || {})).toHaveLength(0);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should merge local fields and prefer admin tool filters', async () => {
|
||||||
|
process.argv = ['node', 'script.js'];
|
||||||
|
const argv = await parseArguments(createTestMergedSettings());
|
||||||
|
const adminAllowlist: Record<string, MCPServerConfig> = {
|
||||||
|
serverA: {
|
||||||
|
type: 'sse',
|
||||||
|
url: 'https://admin-server-a.com/sse',
|
||||||
|
trust: true,
|
||||||
|
includeTools: ['admin_tool'],
|
||||||
|
},
|
||||||
|
};
|
||||||
|
const localMcpServersWithTools: Record<string, MCPServerConfig> = {
|
||||||
|
serverA: {
|
||||||
|
...localMcpServers['serverA'],
|
||||||
|
includeTools: ['local_tool'],
|
||||||
|
timeout: 1234,
|
||||||
|
},
|
||||||
|
};
|
||||||
|
const settings = createTestMergedSettings({
|
||||||
|
mcpServers: localMcpServersWithTools,
|
||||||
|
admin: {
|
||||||
|
...baseSettings.admin,
|
||||||
|
mcp: { enabled: true, config: adminAllowlist },
|
||||||
|
},
|
||||||
|
});
|
||||||
|
const config = await loadCliConfig(settings, 'test-session', argv);
|
||||||
|
|
||||||
|
const serverA = config.getMcpServers()?.['serverA'];
|
||||||
|
expect(serverA).toMatchObject({
|
||||||
|
timeout: 1234,
|
||||||
|
includeTools: ['admin_tool'],
|
||||||
|
type: 'sse',
|
||||||
|
url: 'https://admin-server-a.com/sse',
|
||||||
|
trust: true,
|
||||||
|
});
|
||||||
|
expect(serverA).not.toHaveProperty('command');
|
||||||
|
expect(serverA).not.toHaveProperty('args');
|
||||||
|
expect(serverA).not.toHaveProperty('env');
|
||||||
|
expect(serverA).not.toHaveProperty('cwd');
|
||||||
|
expect(serverA).not.toHaveProperty('httpUrl');
|
||||||
|
expect(serverA).not.toHaveProperty('tcp');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should use local tool filters when admin does not define them', async () => {
|
||||||
|
process.argv = ['node', 'script.js'];
|
||||||
|
const argv = await parseArguments(createTestMergedSettings());
|
||||||
|
const adminAllowlist: Record<string, MCPServerConfig> = {
|
||||||
|
serverA: {
|
||||||
|
type: 'sse',
|
||||||
|
url: 'https://admin-server-a.com/sse',
|
||||||
|
trust: true,
|
||||||
|
},
|
||||||
|
};
|
||||||
|
const localMcpServersWithTools: Record<string, MCPServerConfig> = {
|
||||||
|
serverA: {
|
||||||
|
...localMcpServers['serverA'],
|
||||||
|
includeTools: ['local_tool'],
|
||||||
|
},
|
||||||
|
};
|
||||||
|
const settings = createTestMergedSettings({
|
||||||
|
mcpServers: localMcpServersWithTools,
|
||||||
|
admin: {
|
||||||
|
...baseSettings.admin,
|
||||||
|
mcp: { enabled: true, config: adminAllowlist },
|
||||||
|
},
|
||||||
|
});
|
||||||
|
const config = await loadCliConfig(settings, 'test-session', argv);
|
||||||
|
|
||||||
|
const serverA = config.getMcpServers()?.['serverA'];
|
||||||
|
expect(serverA?.includeTools).toEqual(['local_tool']);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
describe('loadCliConfig model selection', () => {
|
describe('loadCliConfig model selection', () => {
|
||||||
beforeEach(() => {
|
beforeEach(() => {
|
||||||
vi.spyOn(ExtensionManager.prototype, 'getExtensions').mockReturnValue([]);
|
vi.spyOn(ExtensionManager.prototype, 'getExtensions').mockReturnValue([]);
|
||||||
|
|||||||
@@ -12,7 +12,6 @@ import { extensionsCommand } from '../commands/extensions.js';
|
|||||||
import { skillsCommand } from '../commands/skills.js';
|
import { skillsCommand } from '../commands/skills.js';
|
||||||
import { hooksCommand } from '../commands/hooks.js';
|
import { hooksCommand } from '../commands/hooks.js';
|
||||||
import {
|
import {
|
||||||
Config,
|
|
||||||
setGeminiMdFilename as setServerGeminiMdFilename,
|
setGeminiMdFilename as setServerGeminiMdFilename,
|
||||||
getCurrentGeminiMdFilename,
|
getCurrentGeminiMdFilename,
|
||||||
ApprovalMode,
|
ApprovalMode,
|
||||||
@@ -34,12 +33,16 @@ import {
|
|||||||
ASK_USER_TOOL_NAME,
|
ASK_USER_TOOL_NAME,
|
||||||
getVersion,
|
getVersion,
|
||||||
PREVIEW_GEMINI_MODEL_AUTO,
|
PREVIEW_GEMINI_MODEL_AUTO,
|
||||||
type HookDefinition,
|
|
||||||
type HookEventName,
|
|
||||||
type OutputFormat,
|
|
||||||
coreEvents,
|
coreEvents,
|
||||||
GEMINI_MODEL_ALIAS_AUTO,
|
GEMINI_MODEL_ALIAS_AUTO,
|
||||||
getAdminErrorMessage,
|
getAdminErrorMessage,
|
||||||
|
Config,
|
||||||
|
} from '@google/gemini-cli-core';
|
||||||
|
import type {
|
||||||
|
MCPServerConfig,
|
||||||
|
HookDefinition,
|
||||||
|
HookEventName,
|
||||||
|
OutputFormat,
|
||||||
} from '@google/gemini-cli-core';
|
} from '@google/gemini-cli-core';
|
||||||
import {
|
import {
|
||||||
type Settings,
|
type Settings,
|
||||||
@@ -687,6 +690,45 @@ export async function loadCliConfig(
|
|||||||
? mcpEnablementManager.getEnablementCallbacks()
|
? mcpEnablementManager.getEnablementCallbacks()
|
||||||
: undefined;
|
: undefined;
|
||||||
|
|
||||||
|
const adminAllowlist = settings.admin?.mcp?.config;
|
||||||
|
let mcpServerCommand = mcpEnabled ? settings.mcp?.serverCommand : undefined;
|
||||||
|
let mcpServers = mcpEnabled ? settings.mcpServers : {};
|
||||||
|
|
||||||
|
if (mcpEnabled && adminAllowlist && Object.keys(adminAllowlist).length > 0) {
|
||||||
|
const filteredMcpServers: Record<string, MCPServerConfig> = {};
|
||||||
|
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;
|
||||||
|
mcpServerCommand = undefined;
|
||||||
|
}
|
||||||
|
|
||||||
return new Config({
|
return new Config({
|
||||||
sessionId,
|
sessionId,
|
||||||
clientVersion: await getVersion(),
|
clientVersion: await getVersion(),
|
||||||
@@ -706,8 +748,8 @@ export async function loadCliConfig(
|
|||||||
excludeTools,
|
excludeTools,
|
||||||
toolDiscoveryCommand: settings.tools?.discoveryCommand,
|
toolDiscoveryCommand: settings.tools?.discoveryCommand,
|
||||||
toolCallCommand: settings.tools?.callCommand,
|
toolCallCommand: settings.tools?.callCommand,
|
||||||
mcpServerCommand: mcpEnabled ? settings.mcp?.serverCommand : undefined,
|
mcpServerCommand,
|
||||||
mcpServers: mcpEnabled ? settings.mcpServers : {},
|
mcpServers,
|
||||||
mcpEnablementCallbacks,
|
mcpEnablementCallbacks,
|
||||||
mcpEnabled,
|
mcpEnabled,
|
||||||
extensionsEnabled,
|
extensionsEnabled,
|
||||||
|
|||||||
@@ -76,7 +76,11 @@ import {
|
|||||||
LoadedSettings,
|
LoadedSettings,
|
||||||
sanitizeEnvVar,
|
sanitizeEnvVar,
|
||||||
} from './settings.js';
|
} from './settings.js';
|
||||||
import { FatalConfigError, GEMINI_DIR } from '@google/gemini-cli-core';
|
import {
|
||||||
|
FatalConfigError,
|
||||||
|
GEMINI_DIR,
|
||||||
|
type MCPServerConfig,
|
||||||
|
} from '@google/gemini-cli-core';
|
||||||
import { updateSettingsFilePreservingFormat } from '../utils/commentJson.js';
|
import { updateSettingsFilePreservingFormat } from '../utils/commentJson.js';
|
||||||
import {
|
import {
|
||||||
getSettingsSchema,
|
getSettingsSchema,
|
||||||
@@ -2350,6 +2354,28 @@ describe('Settings Loading and Merging', () => {
|
|||||||
expect(loadedSettings.merged.admin?.extensions?.enabled).toBe(true);
|
expect(loadedSettings.merged.admin?.extensions?.enabled).toBe(true);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('should un-nest MCP configuration from remote settings', () => {
|
||||||
|
const loadedSettings = loadSettings(MOCK_WORKSPACE_DIR);
|
||||||
|
const mcpServers: Record<string, MCPServerConfig> = {
|
||||||
|
'admin-server': {
|
||||||
|
url: 'http://admin-mcp.com',
|
||||||
|
type: 'sse',
|
||||||
|
trust: true,
|
||||||
|
},
|
||||||
|
};
|
||||||
|
|
||||||
|
loadedSettings.setRemoteAdminSettings({
|
||||||
|
mcpSetting: {
|
||||||
|
mcpEnabled: true,
|
||||||
|
mcpConfig: {
|
||||||
|
mcpServers,
|
||||||
|
},
|
||||||
|
},
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(loadedSettings.merged.admin?.mcp?.config).toEqual(mcpServers);
|
||||||
|
});
|
||||||
|
|
||||||
it('should set skills based on unmanagedCapabilitiesEnabled', () => {
|
it('should set skills based on unmanagedCapabilitiesEnabled', () => {
|
||||||
const loadedSettings = loadSettings();
|
const loadedSettings = loadSettings();
|
||||||
loadedSettings.setRemoteAdminSettings({
|
loadedSettings.setRemoteAdminSettings({
|
||||||
|
|||||||
@@ -412,7 +412,10 @@ export class LoadedSettings {
|
|||||||
}
|
}
|
||||||
|
|
||||||
admin.secureModeEnabled = !strictModeDisabled;
|
admin.secureModeEnabled = !strictModeDisabled;
|
||||||
admin.mcp = { enabled: mcpSetting?.mcpEnabled };
|
admin.mcp = {
|
||||||
|
enabled: mcpSetting?.mcpEnabled,
|
||||||
|
config: mcpSetting?.mcpConfig?.mcpServers,
|
||||||
|
};
|
||||||
admin.extensions = {
|
admin.extensions = {
|
||||||
enabled: cliFeatureSetting?.extensionsSetting?.extensionsEnabled,
|
enabled: cliFeatureSetting?.extensionsSetting?.extensionsEnabled,
|
||||||
};
|
};
|
||||||
|
|||||||
@@ -1867,6 +1867,20 @@ const SETTINGS_SCHEMA = {
|
|||||||
showInDialog: false,
|
showInDialog: false,
|
||||||
mergeStrategy: MergeStrategy.REPLACE,
|
mergeStrategy: MergeStrategy.REPLACE,
|
||||||
},
|
},
|
||||||
|
config: {
|
||||||
|
type: 'object',
|
||||||
|
label: 'MCP Config',
|
||||||
|
category: 'Admin',
|
||||||
|
requiresRestart: false,
|
||||||
|
default: {} as Record<string, MCPServerConfig>,
|
||||||
|
description: 'Admin-configured MCP servers.',
|
||||||
|
showInDialog: false,
|
||||||
|
mergeStrategy: MergeStrategy.REPLACE,
|
||||||
|
additionalProperties: {
|
||||||
|
type: 'object',
|
||||||
|
ref: 'MCPServerConfig',
|
||||||
|
},
|
||||||
|
},
|
||||||
},
|
},
|
||||||
},
|
},
|
||||||
skills: {
|
skills: {
|
||||||
|
|||||||
@@ -1712,6 +1712,16 @@
|
|||||||
"markdownDescription": "If false, disallows MCP servers from being used.\n\n- Category: `Admin`\n- Requires restart: `no`\n- Default: `true`",
|
"markdownDescription": "If false, disallows MCP servers from being used.\n\n- Category: `Admin`\n- Requires restart: `no`\n- Default: `true`",
|
||||||
"default": true,
|
"default": true,
|
||||||
"type": "boolean"
|
"type": "boolean"
|
||||||
|
},
|
||||||
|
"config": {
|
||||||
|
"title": "MCP Config",
|
||||||
|
"description": "Admin-configured MCP servers.",
|
||||||
|
"markdownDescription": "Admin-configured MCP servers.\n\n- Category: `Admin`\n- Requires restart: `no`\n- Default: `{}`",
|
||||||
|
"default": {},
|
||||||
|
"type": "object",
|
||||||
|
"additionalProperties": {
|
||||||
|
"$ref": "#/$defs/MCPServerConfig"
|
||||||
|
}
|
||||||
}
|
}
|
||||||
},
|
},
|
||||||
"additionalProperties": false
|
"additionalProperties": false
|
||||||
|
|||||||
Reference in New Issue
Block a user