Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions docs/get-started/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -995,6 +995,10 @@ their corresponding top-level category object in your `settings.json` file.
- **Description:** If false, disallows MCP servers from being used.
- **Default:** `true`

- **`admin.mcp.config`** (object):
- **Description:** Admin-configured MCP servers.
- **Default:** `{}`

- **`admin.skills.enabled`** (boolean):
- **Description:** If false, disallows agent skills from being used.
- **Default:** `true`
Expand Down
206 changes: 206 additions & 0 deletions packages/cli/src/config/config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
type ExtensionLoader,
debugLogger,
ApprovalMode,
type MCPServerConfig,
} from '@google/gemini-cli-core';
import { loadCliConfig, parseArguments, type CliArgs } from './config.js';
import { type Settings, createTestMergedSettings } from './settings.js';
Expand Down Expand Up @@ -1431,6 +1432,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', () => {
beforeEach(() => {
vi.spyOn(ExtensionManager.prototype, 'getExtensions').mockReturnValue([]);
Expand Down
54 changes: 48 additions & 6 deletions packages/cli/src/config/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import { extensionsCommand } from '../commands/extensions.js';
import { skillsCommand } from '../commands/skills.js';
import { hooksCommand } from '../commands/hooks.js';
import {
Config,
setGeminiMdFilename as setServerGeminiMdFilename,
getCurrentGeminiMdFilename,
ApprovalMode,
Expand All @@ -33,12 +32,16 @@ import {
WEB_FETCH_TOOL_NAME,
getVersion,
PREVIEW_GEMINI_MODEL_AUTO,
type HookDefinition,
type HookEventName,
type OutputFormat,
coreEvents,
GEMINI_MODEL_ALIAS_AUTO,
getAdminErrorMessage,
Config,
} from '@google/gemini-cli-core';
import type {
MCPServerConfig,
HookDefinition,
HookEventName,
OutputFormat,
} from '@google/gemini-cli-core';
import {
type Settings,
Expand Down Expand Up @@ -682,6 +685,45 @@ export async function loadCliConfig(
? mcpEnablementManager.getEnablementCallbacks()
: 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,
};
Comment thread
skeshive marked this conversation as resolved.

// 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;
}
Comment thread
skeshive marked this conversation as resolved.

filteredMcpServers[serverId] = mergedConfig;
}
}
mcpServers = filteredMcpServers;
mcpServerCommand = undefined;
}

return new Config({
sessionId,
clientVersion: await getVersion(),
Expand All @@ -701,8 +743,8 @@ export async function loadCliConfig(
excludeTools,
toolDiscoveryCommand: settings.tools?.discoveryCommand,
toolCallCommand: settings.tools?.callCommand,
mcpServerCommand: mcpEnabled ? settings.mcp?.serverCommand : undefined,
mcpServers: mcpEnabled ? settings.mcpServers : {},
mcpServerCommand,
mcpServers,
mcpEnablementCallbacks,
mcpEnabled,
extensionsEnabled,
Expand Down
28 changes: 27 additions & 1 deletion packages/cli/src/config/settings.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,11 @@ import {
SettingScope,
LoadedSettings,
} 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 {
getSettingsSchema,
Expand Down Expand Up @@ -2306,6 +2310,28 @@ describe('Settings Loading and Merging', () => {
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', () => {
const loadedSettings = loadSettings();
loadedSettings.setRemoteAdminSettings({
Expand Down
5 changes: 4 additions & 1 deletion packages/cli/src/config/settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -360,7 +360,10 @@ export class LoadedSettings {
}

admin.secureModeEnabled = !strictModeDisabled;
admin.mcp = { enabled: mcpSetting?.mcpEnabled };
admin.mcp = {
enabled: mcpSetting?.mcpEnabled,
config: mcpSetting?.mcpConfig?.mcpServers,
};
admin.extensions = {
enabled: cliFeatureSetting?.extensionsSetting?.extensionsEnabled,
};
Expand Down
14 changes: 14 additions & 0 deletions packages/cli/src/config/settingsSchema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1867,6 +1867,20 @@ const SETTINGS_SCHEMA = {
showInDialog: false,
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: {
Expand Down
Loading
Loading