diff --git a/packages/a2a-server/src/config/config.test.ts b/packages/a2a-server/src/config/config.test.ts index d3f0bfdbdd..87da1e2b5e 100644 --- a/packages/a2a-server/src/config/config.test.ts +++ b/packages/a2a-server/src/config/config.test.ts @@ -123,7 +123,7 @@ describe('loadConfig', () => { await loadConfig(mockSettings, mockExtensionLoader, taskId); - expect(Config).toHaveBeenCalledWith( + expect(Config).toHaveBeenLastCalledWith( expect.objectContaining({ disableYoloMode: !mockAdminSettings.strictModeDisabled, mcpEnabled: mockAdminSettings.mcpSetting?.mcpEnabled, @@ -144,11 +144,11 @@ describe('loadConfig', () => { await loadConfig(mockSettings, mockExtensionLoader, taskId); - expect(Config).toHaveBeenCalledWith( + expect(Config).toHaveBeenLastCalledWith( expect.objectContaining({ disableYoloMode: !false, mcpEnabled: mockAdminSettings.mcpSetting?.mcpEnabled, - extensionsEnabled: false, + extensionsEnabled: undefined, }), ); }); @@ -159,7 +159,7 @@ describe('loadConfig', () => { await loadConfig(mockSettings, mockExtensionLoader, taskId); - expect(Config).toHaveBeenCalledWith(expect.objectContaining({})); + expect(Config).toHaveBeenLastCalledWith(expect.objectContaining({})); }); it('should fetch admin controls using the code assist server when available', async () => { @@ -182,11 +182,11 @@ describe('loadConfig', () => { mockCodeAssistServer, true, ); - expect(Config).toHaveBeenCalledWith( + expect(Config).toHaveBeenLastCalledWith( expect.objectContaining({ disableYoloMode: !mockAdminSettings.strictModeDisabled, mcpEnabled: mockAdminSettings.mcpSetting?.mcpEnabled, - extensionsEnabled: false, + extensionsEnabled: undefined, }), ); }); diff --git a/packages/a2a-server/src/config/config.ts b/packages/a2a-server/src/config/config.ts index d98ae4fb7c..5b8793d15e 100644 --- a/packages/a2a-server/src/config/config.ts +++ b/packages/a2a-server/src/config/config.ts @@ -157,14 +157,10 @@ export async function loadConfig( // If NONE are present, disregard admin settings entirely, and pass the // final config as is. if (Object.keys(adminSettings).length !== 0) { - finalConfigParams.disableYoloMode = !( - adminSettings.strictModeDisabled ?? false - ); - finalConfigParams.mcpEnabled = - adminSettings.mcpSetting?.mcpEnabled ?? false; + finalConfigParams.disableYoloMode = !adminSettings.strictModeDisabled; + finalConfigParams.mcpEnabled = adminSettings.mcpSetting?.mcpEnabled; finalConfigParams.extensionsEnabled = - adminSettings.cliFeatureSetting?.extensionsSetting?.extensionsEnabled ?? - false; + adminSettings.cliFeatureSetting?.extensionsSetting?.extensionsEnabled; } } diff --git a/packages/cli/src/config/settings.test.ts b/packages/cli/src/config/settings.test.ts index 8b4d6f64c5..449a5e0b0b 100644 --- a/packages/cli/src/config/settings.test.ts +++ b/packages/cli/src/config/settings.test.ts @@ -2216,8 +2216,11 @@ describe('Settings Loading and Merging', () => { // 2. Now, set remote admin settings. loadedSettings.setRemoteAdminSettings({ strictModeDisabled: false, - mcpSetting: { mcpEnabled: false }, - cliFeatureSetting: { extensionsSetting: { extensionsEnabled: false } }, + mcpSetting: { mcpEnabled: false, mcpConfig: {} }, + cliFeatureSetting: { + extensionsSetting: { extensionsEnabled: false }, + unmanagedCapabilitiesEnabled: false, + }, }); // 3. Verify that remote admin settings take precedence. @@ -2257,8 +2260,11 @@ describe('Settings Loading and Merging', () => { const newRemoteSettings = { strictModeDisabled: false, - mcpSetting: { mcpEnabled: false }, - cliFeatureSetting: { extensionsSetting: { extensionsEnabled: false } }, + mcpSetting: { mcpEnabled: false, mcpConfig: {} }, + cliFeatureSetting: { + extensionsSetting: { extensionsEnabled: false }, + unmanagedCapabilitiesEnabled: false, + }, }; loadedSettings.setRemoteAdminSettings(newRemoteSettings); @@ -2269,13 +2275,6 @@ describe('Settings Loading and Merging', () => { expect(loadedSettings.merged.admin?.extensions?.enabled).toBe(false); // Non-admin settings should remain untouched expect(loadedSettings.merged.ui?.theme).toBe('initial-theme'); - - // Verify that calling setRemoteAdminSettings with partial data overwrites previous remote settings - // and missing properties revert to schema defaults. - loadedSettings.setRemoteAdminSettings({ strictModeDisabled: true }); - expect(loadedSettings.merged.admin?.secureModeEnabled).toBe(false); - expect(loadedSettings.merged.admin?.mcp?.enabled).toBe(false); // Defaulting to false if missing - expect(loadedSettings.merged.admin?.extensions?.enabled).toBe(false); // Defaulting to false if missing }); it('should correctly handle undefined remote admin settings', () => { @@ -2307,84 +2306,6 @@ describe('Settings Loading and Merging', () => { expect(loadedSettings.merged.admin?.extensions?.enabled).toBe(true); }); - it('should correctly handle missing properties in remote admin settings', () => { - (mockFsExistsSync as Mock).mockReturnValue(true); - const systemSettingsContent = { - admin: { - secureModeEnabled: true, - }, - }; - - (fs.readFileSync as Mock).mockImplementation( - (p: fs.PathOrFileDescriptor) => { - if (p === getSystemSettingsPath()) { - return JSON.stringify(systemSettingsContent); - } - return '{}'; - }, - ); - - const loadedSettings = loadSettings(MOCK_WORKSPACE_DIR); - // Ensure initial state from defaults (as file-based admin settings are ignored) - expect(loadedSettings.merged.admin?.secureModeEnabled).toBe(false); - expect(loadedSettings.merged.admin?.mcp?.enabled).toBe(true); - expect(loadedSettings.merged.admin?.extensions?.enabled).toBe(true); - - // Set remote settings with only strictModeDisabled (false -> secureModeEnabled: true) - loadedSettings.setRemoteAdminSettings({ - strictModeDisabled: false, - }); - - // Verify secureModeEnabled is updated, others default to false - expect(loadedSettings.merged.admin?.secureModeEnabled).toBe(true); - expect(loadedSettings.merged.admin?.mcp?.enabled).toBe(false); - expect(loadedSettings.merged.admin?.extensions?.enabled).toBe(false); - - // Set remote settings with only mcpSetting.mcpEnabled - loadedSettings.setRemoteAdminSettings({ - mcpSetting: { mcpEnabled: false }, - }); - - // Verify mcpEnabled is updated, others remain defaults (secureModeEnabled defaults to true if strictModeDisabled is missing) - expect(loadedSettings.merged.admin?.secureModeEnabled).toBe(true); - expect(loadedSettings.merged.admin?.mcp?.enabled).toBe(false); - expect(loadedSettings.merged.admin?.extensions?.enabled).toBe(false); - - // Set remote settings with only cliFeatureSetting.extensionsSetting.extensionsEnabled - loadedSettings.setRemoteAdminSettings({ - cliFeatureSetting: { extensionsSetting: { extensionsEnabled: false } }, - }); - - // Verify extensionsEnabled is updated, others remain defaults - expect(loadedSettings.merged.admin?.secureModeEnabled).toBe(true); - expect(loadedSettings.merged.admin?.mcp?.enabled).toBe(false); - expect(loadedSettings.merged.admin?.extensions?.enabled).toBe(false); - - // Verify that missing strictModeDisabled falls back to secureModeEnabled - loadedSettings.setRemoteAdminSettings({ - secureModeEnabled: false, - }); - expect(loadedSettings.merged.admin?.secureModeEnabled).toBe(false); - - loadedSettings.setRemoteAdminSettings({ - secureModeEnabled: true, - }); - expect(loadedSettings.merged.admin?.secureModeEnabled).toBe(true); - - // Verify strictModeDisabled takes precedence over secureModeEnabled - loadedSettings.setRemoteAdminSettings({ - strictModeDisabled: false, - secureModeEnabled: false, - }); - expect(loadedSettings.merged.admin?.secureModeEnabled).toBe(true); - - loadedSettings.setRemoteAdminSettings({ - strictModeDisabled: true, - secureModeEnabled: true, - }); - expect(loadedSettings.merged.admin?.secureModeEnabled).toBe(false); - }); - it('should set skills based on unmanagedCapabilitiesEnabled', () => { const loadedSettings = loadSettings(); loadedSettings.setRemoteAdminSettings({ @@ -2402,51 +2323,6 @@ describe('Settings Loading and Merging', () => { expect(loadedSettings.merged.admin.skills?.enabled).toBe(false); }); - it('should default mcp.enabled to false if mcpSetting is present but mcpEnabled is undefined', () => { - const loadedSettings = loadSettings(MOCK_WORKSPACE_DIR); - loadedSettings.setRemoteAdminSettings({ - mcpSetting: {}, - }); - expect(loadedSettings.merged.admin?.mcp?.enabled).toBe(false); - }); - - it('should default extensions.enabled to false if extensionsSetting is present but extensionsEnabled is undefined', () => { - const loadedSettings = loadSettings(MOCK_WORKSPACE_DIR); - loadedSettings.setRemoteAdminSettings({ - cliFeatureSetting: { - extensionsSetting: {}, - }, - }); - expect(loadedSettings.merged.admin?.extensions?.enabled).toBe(false); - }); - - it('should force secureModeEnabled to true if undefined, overriding schema defaults', () => { - // Mock schema to have secureModeEnabled default to false to verify the override - const originalSchema = getSettingsSchema(); - const modifiedSchema = JSON.parse(JSON.stringify(originalSchema)); - if (modifiedSchema.admin?.properties?.secureModeEnabled) { - modifiedSchema.admin.properties.secureModeEnabled.default = false; - } - vi.mocked(getSettingsSchema).mockReturnValue(modifiedSchema); - - try { - (mockFsExistsSync as Mock).mockReturnValue(true); - (fs.readFileSync as Mock).mockImplementation(() => '{}'); - - const loadedSettings = loadSettings(MOCK_WORKSPACE_DIR); - - // Pass a non-empty object that doesn't have strictModeDisabled - loadedSettings.setRemoteAdminSettings({ - mcpSetting: {}, - }); - - // It should be forced to true by the logic (default secure), overriding the mock default of false - expect(loadedSettings.merged.admin?.secureModeEnabled).toBe(true); - } finally { - vi.mocked(getSettingsSchema).mockReturnValue(originalSchema); - } - }); - it('should handle completely empty remote admin settings response', () => { const loadedSettings = loadSettings(MOCK_WORKSPACE_DIR); diff --git a/packages/cli/src/config/settings.ts b/packages/cli/src/config/settings.ts index b1dc449219..90cef7c7fe 100644 --- a/packages/cli/src/config/settings.ts +++ b/packages/cli/src/config/settings.ts @@ -16,7 +16,7 @@ import { Storage, coreEvents, homedir, - type FetchAdminControlsResponse, + type AdminControlsSettings, } from '@google/gemini-cli-core'; import stripJsonComments from 'strip-json-comments'; import { DefaultLight } from '../ui/themes/default-light.js'; @@ -348,14 +348,10 @@ export class LoadedSettings { coreEvents.emitSettingsChanged(); } - setRemoteAdminSettings(remoteSettings: FetchAdminControlsResponse): void { + setRemoteAdminSettings(remoteSettings: AdminControlsSettings): void { const admin: Settings['admin'] = {}; - const { - secureModeEnabled, - strictModeDisabled, - mcpSetting, - cliFeatureSetting, - } = remoteSettings; + const { strictModeDisabled, mcpSetting, cliFeatureSetting } = + remoteSettings; if (Object.keys(remoteSettings).length === 0) { this._remoteAdminSettings = { admin }; @@ -363,19 +359,13 @@ export class LoadedSettings { return; } - if (strictModeDisabled !== undefined) { - admin.secureModeEnabled = !strictModeDisabled; - } else if (secureModeEnabled !== undefined) { - admin.secureModeEnabled = secureModeEnabled; - } else { - admin.secureModeEnabled = true; - } - admin.mcp = { enabled: mcpSetting?.mcpEnabled ?? false }; + admin.secureModeEnabled = !strictModeDisabled; + admin.mcp = { enabled: mcpSetting?.mcpEnabled }; admin.extensions = { - enabled: cliFeatureSetting?.extensionsSetting?.extensionsEnabled ?? false, + enabled: cliFeatureSetting?.extensionsSetting?.extensionsEnabled, }; admin.skills = { - enabled: cliFeatureSetting?.unmanagedCapabilitiesEnabled ?? false, + enabled: cliFeatureSetting?.unmanagedCapabilitiesEnabled, }; this._remoteAdminSettings = { admin }; diff --git a/packages/cli/src/gemini.tsx b/packages/cli/src/gemini.tsx index 25e3909fe3..494b857656 100644 --- a/packages/cli/src/gemini.tsx +++ b/packages/cli/src/gemini.tsx @@ -67,7 +67,7 @@ import { getVersion, ValidationCancelledError, ValidationRequiredError, - type FetchAdminControlsResponse, + type AdminControlsSettings, } from '@google/gemini-cli-core'; import { initializeApp, @@ -809,13 +809,13 @@ export function initializeOutputListenersAndFlush() { } function setupAdminControlsListener() { - let pendingSettings: FetchAdminControlsResponse | undefined; + let pendingSettings: AdminControlsSettings | undefined; let config: Config | undefined; const messageHandler = (msg: unknown) => { const message = msg as { type?: string; - settings?: FetchAdminControlsResponse; + settings?: AdminControlsSettings; }; if (message?.type === 'admin-settings' && message.settings) { if (config) { diff --git a/packages/cli/src/utils/relaunch.ts b/packages/cli/src/utils/relaunch.ts index c2d987845d..7e287e4565 100644 --- a/packages/cli/src/utils/relaunch.ts +++ b/packages/cli/src/utils/relaunch.ts @@ -8,7 +8,7 @@ import { spawn } from 'node:child_process'; import { RELAUNCH_EXIT_CODE } from './processUtils.js'; import { writeToStderr, - type FetchAdminControlsResponse, + type AdminControlsSettings, } from '@google/gemini-cli-core'; export async function relaunchOnExitCode(runner: () => Promise) { @@ -34,7 +34,7 @@ export async function relaunchOnExitCode(runner: () => Promise) { export async function relaunchAppInChildProcess( additionalNodeArgs: string[], additionalScriptArgs: string[], - remoteAdminSettings?: FetchAdminControlsResponse, + remoteAdminSettings?: AdminControlsSettings, ) { if (process.env['GEMINI_CLI_NO_RELAUNCH']) { return; @@ -71,7 +71,7 @@ export async function relaunchAppInChildProcess( child.on('message', (msg: { type?: string; settings?: unknown }) => { if (msg.type === 'admin-settings-update' && msg.settings) { - latestAdminSettings = msg.settings as FetchAdminControlsResponse; + latestAdminSettings = msg.settings as AdminControlsSettings; } }); 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 f30d8b7e58..57849ae3a4 100644 --- a/packages/core/src/code_assist/admin/admin_controls.test.ts +++ b/packages/core/src/code_assist/admin/admin_controls.test.ts @@ -4,6 +4,7 @@ * SPDX-License-Identifier: Apache-2.0 */ +import { isDeepStrictEqual } from 'node:util'; import { describe, it, @@ -23,6 +24,10 @@ import { import type { CodeAssistServer } from '../server.js'; import type { Config } from '../../config/config.js'; import { getCodeAssistServer } from '../codeAssist.js'; +import type { + FetchAdminControlsResponse, + AdminControlsSettings, +} from '../types.js'; vi.mock('../codeAssist.js', () => ({ getCodeAssistServer: vi.fn(), @@ -50,37 +55,243 @@ describe('Admin Controls', () => { }); describe('sanitizeAdminSettings', () => { - it('should strip unknown fields', () => { + it('should strip unknown fields and pass through mcpConfigJson when valid', () => { + const mcpConfig = { + mcpServers: { + 'server-1': { + url: 'http://example.com', + type: 'sse' as const, + trust: true, + includeTools: ['tool1'], + }, + }, + }; + const input = { strictModeDisabled: false, extraField: 'should be removed', mcpSetting: { - mcpEnabled: false, + mcpEnabled: true, + mcpConfigJson: JSON.stringify(mcpConfig), unknownMcpField: 'remove me', }, }; + const result = sanitizeAdminSettings( + input as unknown as FetchAdminControlsResponse, + ); + + expect(result).toEqual({ + strictModeDisabled: false, + cliFeatureSetting: { + extensionsSetting: { extensionsEnabled: false }, + unmanagedCapabilitiesEnabled: false, + }, + mcpSetting: { + mcpEnabled: true, + mcpConfig, + }, + }); + }); + + it('should ignore mcpConfigJson if it is invalid JSON', () => { + const input: FetchAdminControlsResponse = { + mcpSetting: { + mcpEnabled: true, + mcpConfigJson: '{ invalid json }', + }, + }; + + const result = sanitizeAdminSettings(input); + expect(result.mcpSetting).toEqual({ + mcpEnabled: true, + mcpConfig: {}, + }); + }); + + it('should ignore mcpConfigJson if it does not match schema', () => { + const invalidConfig = { + mcpServers: { + 'server-1': { + url: 123, // should be string + type: 'invalid-type', // should be sse or http + }, + }, + }; + const input: FetchAdminControlsResponse = { + mcpSetting: { + mcpEnabled: true, + mcpConfigJson: JSON.stringify(invalidConfig), + }, + }; + + const result = sanitizeAdminSettings(input); + expect(result.mcpSetting).toEqual({ + mcpEnabled: true, + mcpConfig: {}, + }); + }); + + it('should apply default values when fields are missing', () => { + const input = {}; + const result = sanitizeAdminSettings(input as FetchAdminControlsResponse); + + expect(result).toEqual({ + strictModeDisabled: false, + cliFeatureSetting: { + extensionsSetting: { extensionsEnabled: false }, + unmanagedCapabilitiesEnabled: false, + }, + mcpSetting: { + mcpEnabled: false, + mcpConfig: {}, + }, + }); + }); + + it('should default mcpEnabled to false if mcpSetting is present but mcpEnabled is undefined', () => { + const input = { mcpSetting: {} }; + const result = sanitizeAdminSettings(input as FetchAdminControlsResponse); + expect(result.mcpSetting?.mcpEnabled).toBe(false); + expect(result.mcpSetting?.mcpConfig).toEqual({}); + }); + + it('should default extensionsEnabled to false if extensionsSetting is present but extensionsEnabled is undefined', () => { + const input = { + cliFeatureSetting: { + extensionsSetting: {}, + }, + }; + const result = sanitizeAdminSettings(input as FetchAdminControlsResponse); + expect( + result.cliFeatureSetting?.extensionsSetting?.extensionsEnabled, + ).toBe(false); + }); + + it('should default unmanagedCapabilitiesEnabled to false if cliFeatureSetting is present but unmanagedCapabilitiesEnabled is undefined', () => { + const input = { + cliFeatureSetting: {}, + }; + const result = sanitizeAdminSettings(input as FetchAdminControlsResponse); + expect(result.cliFeatureSetting?.unmanagedCapabilitiesEnabled).toBe( + false, + ); + }); + + it('should reflect explicit values', () => { + const input: FetchAdminControlsResponse = { + strictModeDisabled: true, + cliFeatureSetting: { + extensionsSetting: { extensionsEnabled: true }, + unmanagedCapabilitiesEnabled: true, + }, + mcpSetting: { + mcpEnabled: true, + }, + }; + const result = sanitizeAdminSettings(input); expect(result).toEqual({ - strictModeDisabled: false, + strictModeDisabled: true, + cliFeatureSetting: { + extensionsSetting: { extensionsEnabled: true }, + unmanagedCapabilitiesEnabled: true, + }, mcpSetting: { - mcpEnabled: false, + mcpEnabled: true, + mcpConfig: {}, }, }); - // Explicitly check that unknown fields are gone - expect((result as Record)['extraField']).toBeUndefined(); }); - it('should preserve valid nested fields', () => { - const input = { - cliFeatureSetting: { - extensionsSetting: { - extensionsEnabled: true, + it('should prioritize strictModeDisabled over secureModeEnabled', () => { + const input: FetchAdminControlsResponse = { + strictModeDisabled: true, + secureModeEnabled: true, // Should be ignored because strictModeDisabled takes precedence for backwards compatibility if both exist (though usually they shouldn't) + }; + + const result = sanitizeAdminSettings(input); + expect(result.strictModeDisabled).toBe(true); + }); + + it('should use secureModeEnabled if strictModeDisabled is undefined', () => { + const input: FetchAdminControlsResponse = { + secureModeEnabled: false, + }; + + const result = sanitizeAdminSettings(input); + expect(result.strictModeDisabled).toBe(true); + }); + }); + + describe('isDeepStrictEqual verification', () => { + it('should consider AdminControlsSettings with different key orders as equal', () => { + const settings1: AdminControlsSettings = { + strictModeDisabled: false, + mcpSetting: { mcpEnabled: true }, + cliFeatureSetting: { unmanagedCapabilitiesEnabled: true }, + }; + const settings2: AdminControlsSettings = { + cliFeatureSetting: { unmanagedCapabilitiesEnabled: true }, + mcpSetting: { mcpEnabled: true }, + strictModeDisabled: false, + }; + expect(isDeepStrictEqual(settings1, settings2)).toBe(true); + }); + + it('should consider nested settings objects with different key orders as equal', () => { + const settings1: AdminControlsSettings = { + mcpSetting: { + mcpEnabled: true, + mcpConfig: { + mcpServers: { + server1: { url: 'url', type: 'sse' }, + }, }, }, }; - expect(sanitizeAdminSettings(input)).toEqual(input); + + // Order swapped in mcpConfig and mcpServers items + const settings2: AdminControlsSettings = { + mcpSetting: { + mcpConfig: { + mcpServers: { + server1: { type: 'sse', url: 'url' }, + }, + }, + mcpEnabled: true, + }, + }; + expect(isDeepStrictEqual(settings1, settings2)).toBe(true); + }); + + it('should consider arrays in options as order-independent and equal if shuffled after sanitization', () => { + const mcpConfig1 = { + mcpServers: { + server1: { includeTools: ['a', 'b'] }, + }, + }; + const mcpConfig2 = { + mcpServers: { + server1: { includeTools: ['b', 'a'] }, + }, + }; + + const settings1 = sanitizeAdminSettings({ + mcpSetting: { + mcpEnabled: true, + mcpConfigJson: JSON.stringify(mcpConfig1), + }, + }); + const settings2 = sanitizeAdminSettings({ + mcpSetting: { + mcpEnabled: true, + mcpConfigJson: JSON.stringify(mcpConfig2), + }, + }); + + expect(isDeepStrictEqual(settings1, settings2)).toBe(true); }); }); @@ -112,7 +323,14 @@ describe('Admin Controls', () => { }); it('should use cachedSettings and start polling if provided', async () => { - const cachedSettings = { strictModeDisabled: false }; + const cachedSettings = { + strictModeDisabled: false, + mcpSetting: { mcpEnabled: false, mcpConfig: {} }, + cliFeatureSetting: { + extensionsSetting: { extensionsEnabled: false }, + unmanagedCapabilitiesEnabled: false, + }, + }; const result = await fetchAdminControls( mockServer, cachedSettings, @@ -153,7 +371,17 @@ describe('Admin Controls', () => { true, mockOnSettingsChanged, ); - expect(result).toEqual(serverResponse); + expect(result).toEqual({ + strictModeDisabled: false, + cliFeatureSetting: { + extensionsSetting: { extensionsEnabled: false }, + unmanagedCapabilitiesEnabled: false, + }, + mcpSetting: { + mcpEnabled: false, + mcpConfig: {}, + }, + }); expect(mockServer.fetchAdminControls).toHaveBeenCalledTimes(1); }); @@ -209,7 +437,17 @@ describe('Admin Controls', () => { true, mockOnSettingsChanged, ); - expect(result).toEqual({ strictModeDisabled: false }); + expect(result).toEqual({ + strictModeDisabled: false, + cliFeatureSetting: { + extensionsSetting: { extensionsEnabled: false }, + unmanagedCapabilitiesEnabled: false, + }, + mcpSetting: { + mcpEnabled: false, + mcpConfig: {}, + }, + }); expect( (result as Record)['unknownField'], ).toBeUndefined(); @@ -279,7 +517,17 @@ describe('Admin Controls', () => { (mockServer.fetchAdminControls as Mock).mockResolvedValue(serverResponse); const result = await fetchAdminControlsOnce(mockServer, true); - expect(result).toEqual({ strictModeDisabled: true }); + expect(result).toEqual({ + strictModeDisabled: true, + cliFeatureSetting: { + extensionsSetting: { extensionsEnabled: false }, + unmanagedCapabilitiesEnabled: false, + }, + mcpSetting: { + mcpEnabled: false, + mcpConfig: {}, + }, + }); expect(mockServer.fetchAdminControls).toHaveBeenCalledTimes(1); }); @@ -337,6 +585,14 @@ describe('Admin Controls', () => { expect(mockOnSettingsChanged).toHaveBeenCalledWith({ strictModeDisabled: false, + cliFeatureSetting: { + extensionsSetting: { extensionsEnabled: false }, + unmanagedCapabilitiesEnabled: false, + }, + mcpSetting: { + mcpEnabled: false, + mcpConfig: {}, + }, }); }); @@ -362,7 +618,6 @@ describe('Admin Controls', () => { expect(mockOnSettingsChanged).not.toHaveBeenCalled(); expect(mockServer.fetchAdminControls).toHaveBeenCalledTimes(2); }); - it('should continue polling after a fetch error', async () => { // Initial fetch is successful (mockServer.fetchAdminControls as Mock).mockResolvedValue({ @@ -392,6 +647,14 @@ describe('Admin Controls', () => { expect(mockServer.fetchAdminControls).toHaveBeenCalledTimes(3); expect(mockOnSettingsChanged).toHaveBeenCalledWith({ strictModeDisabled: false, + cliFeatureSetting: { + extensionsSetting: { extensionsEnabled: false }, + unmanagedCapabilitiesEnabled: false, + }, + mcpSetting: { + mcpEnabled: false, + mcpConfig: {}, + }, }); }); diff --git a/packages/core/src/code_assist/admin/admin_controls.ts b/packages/core/src/code_assist/admin/admin_controls.ts index 7d9c46861c..cfd34225a6 100644 --- a/packages/core/src/code_assist/admin/admin_controls.ts +++ b/packages/core/src/code_assist/admin/admin_controls.ts @@ -10,21 +10,74 @@ import { isDeepStrictEqual } from 'node:util'; import { type FetchAdminControlsResponse, FetchAdminControlsResponseSchema, + McpConfigDefinitionSchema, + type AdminControlsSettings, } from '../types.js'; import { getCodeAssistServer } from '../codeAssist.js'; import type { Config } from '../../config/config.js'; let pollingInterval: NodeJS.Timeout | undefined; -let currentSettings: FetchAdminControlsResponse | undefined; +let currentSettings: AdminControlsSettings | undefined; export function sanitizeAdminSettings( settings: FetchAdminControlsResponse, -): FetchAdminControlsResponse { +): AdminControlsSettings { const result = FetchAdminControlsResponseSchema.safeParse(settings); if (!result.success) { return {}; } - return result.data; + const sanitized = result.data; + let mcpConfig; + + if (sanitized.mcpSetting?.mcpConfigJson) { + try { + const parsed = JSON.parse(sanitized.mcpSetting.mcpConfigJson); + const validationResult = McpConfigDefinitionSchema.safeParse(parsed); + + if (validationResult.success) { + mcpConfig = validationResult.data; + // Sort include/exclude tools for stable comparison + if (mcpConfig.mcpServers) { + for (const server of Object.values(mcpConfig.mcpServers)) { + if (server.includeTools) { + server.includeTools.sort(); + } + if (server.excludeTools) { + server.excludeTools.sort(); + } + } + } + } + } catch (_e) { + // Ignore parsing errors + } + } + + // Apply defaults (secureModeEnabled is supported for backward compatibility) + let strictModeDisabled = false; + if (sanitized.strictModeDisabled !== undefined) { + strictModeDisabled = sanitized.strictModeDisabled; + } else if (sanitized.secureModeEnabled !== undefined) { + strictModeDisabled = !sanitized.secureModeEnabled; + } + + return { + strictModeDisabled, + cliFeatureSetting: { + ...sanitized.cliFeatureSetting, + extensionsSetting: { + extensionsEnabled: + sanitized.cliFeatureSetting?.extensionsSetting?.extensionsEnabled ?? + false, + }, + unmanagedCapabilitiesEnabled: + sanitized.cliFeatureSetting?.unmanagedCapabilitiesEnabled ?? false, + }, + mcpSetting: { + mcpEnabled: sanitized.mcpSetting?.mcpEnabled ?? false, + mcpConfig: mcpConfig ?? {}, + }, + }; } function isGaxiosError(error: unknown): error is { status: number } { @@ -48,10 +101,10 @@ function isGaxiosError(error: unknown): error is { status: number } { */ export async function fetchAdminControls( server: CodeAssistServer | undefined, - cachedSettings: FetchAdminControlsResponse | undefined, + cachedSettings: AdminControlsSettings | undefined, adminControlsEnabled: boolean, - onSettingsChanged: (settings: FetchAdminControlsResponse) => void, -): Promise { + onSettingsChanged: (settings: AdminControlsSettings) => void, +): Promise { if (!server || !server.projectId || !adminControlsEnabled) { stopAdminControlsPolling(); currentSettings = undefined; @@ -129,7 +182,7 @@ export async function fetchAdminControlsOnce( function startAdminControlsPolling( server: CodeAssistServer, project: string, - onSettingsChanged: (settings: FetchAdminControlsResponse) => void, + onSettingsChanged: (settings: AdminControlsSettings) => void, ) { stopAdminControlsPolling(); diff --git a/packages/core/src/code_assist/types.ts b/packages/core/src/code_assist/types.ts index ccf54921cf..a5a452ee76 100644 --- a/packages/core/src/code_assist/types.ts +++ b/packages/core/src/code_assist/types.ts @@ -311,11 +311,39 @@ const CliFeatureSettingSchema = z.object({ unmanagedCapabilitiesEnabled: z.boolean().optional(), }); +const McpServerConfigSchema = z.object({ + url: z.string().optional(), + type: z.enum(['sse', 'http']).optional(), + trust: z.boolean().optional(), + includeTools: z.array(z.string()).optional(), + excludeTools: z.array(z.string()).optional(), +}); + +export const McpConfigDefinitionSchema = z.object({ + mcpServers: z.record(McpServerConfigSchema).optional(), +}); + +export type McpConfigDefinition = z.infer; + const McpSettingSchema = z.object({ mcpEnabled: z.boolean().optional(), - overrideMcpConfigJson: z.string().optional(), + mcpConfigJson: z.string().optional(), }); +// Schema for internal application use (parsed mcpConfig) +export const AdminControlsSettingsSchema = z.object({ + strictModeDisabled: z.boolean().optional(), + mcpSetting: z + .object({ + mcpEnabled: z.boolean().optional(), + mcpConfig: McpConfigDefinitionSchema.optional(), + }) + .optional(), + cliFeatureSetting: CliFeatureSettingSchema.optional(), +}); + +export type AdminControlsSettings = z.infer; + export const FetchAdminControlsResponseSchema = z.object({ // TODO: deprecate once backend stops sending this field secureModeEnabled: z.boolean().optional(), diff --git a/packages/core/src/config/config.ts b/packages/core/src/config/config.ts index 2995078677..6ec0444d80 100644 --- a/packages/core/src/config/config.ts +++ b/packages/core/src/config/config.ts @@ -100,7 +100,7 @@ import { ApprovalMode, type PolicyEngineConfig } from '../policy/types.js'; import { HookSystem } from '../hooks/index.js'; import type { UserTierId } from '../code_assist/types.js'; import type { RetrieveUserQuotaResponse } from '../code_assist/types.js'; -import type { FetchAdminControlsResponse } from '../code_assist/types.js'; +import type { AdminControlsSettings } from '../code_assist/types.js'; import { getCodeAssistServer } from '../code_assist/codeAssist.js'; import type { Experiments } from '../code_assist/experiments/experiments.js'; import { AgentRegistry } from '../agents/registry.js'; @@ -623,7 +623,7 @@ export class Config { private readonly planEnabled: boolean; private contextManager?: ContextManager; private terminalBackground: string | undefined = undefined; - private remoteAdminSettings: FetchAdminControlsResponse | undefined; + private remoteAdminSettings: AdminControlsSettings | undefined; private latestApiRequest: GenerateContentParameters | undefined; private lastModeSwitchTime: number = Date.now(); @@ -1025,7 +1025,7 @@ export class Config { codeAssistServer, this.getRemoteAdminSettings(), adminControlsEnabled, - (newSettings: FetchAdminControlsResponse) => { + (newSettings: AdminControlsSettings) => { this.setRemoteAdminSettings(newSettings); coreEvents.emitAdminSettingsChanged(); }, @@ -1094,11 +1094,11 @@ export class Config { this.latestApiRequest = req; } - getRemoteAdminSettings(): FetchAdminControlsResponse | undefined { + getRemoteAdminSettings(): AdminControlsSettings | undefined { return this.remoteAdminSettings; } - setRemoteAdminSettings(settings: FetchAdminControlsResponse): void { + setRemoteAdminSettings(settings: AdminControlsSettings): void { this.remoteAdminSettings = settings; }