Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
193 changes: 193 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,198 @@ 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({
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({
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({
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,
});
});

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({
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?.command).toBeUndefined();
});

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
44 changes: 40 additions & 4 deletions packages/cli/src/config/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ import { mcpCommand } from '../commands/mcp.js';
import { extensionsCommand } from '../commands/extensions.js';
import { skillsCommand } from '../commands/skills.js';
import { hooksCommand } from '../commands/hooks.js';
import {
import type {
MCPServerConfig,
Config,
setGeminiMdFilename as setServerGeminiMdFilename,
getCurrentGeminiMdFilename,
Expand All @@ -38,7 +39,7 @@ import {
type OutputFormat,
coreEvents,
GEMINI_MODEL_ALIAS_AUTO,
getAdminErrorMessage,
getAdminErrorMessage
} from '@google/gemini-cli-core';
import {
type Settings,
Expand Down Expand Up @@ -682,6 +683,41 @@ 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,
command: undefined,
args: undefined,
env: undefined,
cwd: undefined,
};

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({
sessionId,
clientVersion: await getVersion(),
Expand All @@ -701,8 +737,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