diff --git a/packages/cli/src/config/settings.test.ts b/packages/cli/src/config/settings.test.ts index be5f3d14f9..41514f18fe 100644 --- a/packages/cli/src/config/settings.test.ts +++ b/packages/cli/src/config/settings.test.ts @@ -2215,8 +2215,8 @@ describe('Settings Loading and Merging', () => { // and missing properties revert to schema defaults. loadedSettings.setRemoteAdminSettings({ secureModeEnabled: false }); expect(loadedSettings.merged.admin?.secureModeEnabled).toBe(false); - expect(loadedSettings.merged.admin?.mcp?.enabled).toBe(true); // Reverts to default: true - expect(loadedSettings.merged.admin?.extensions?.enabled).toBe(true); // Reverts to default: true + 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', () => { @@ -2276,10 +2276,10 @@ describe('Settings Loading and Merging', () => { secureModeEnabled: true, }); - // Verify secureModeEnabled is updated, others remain defaults + // Verify secureModeEnabled is updated, others default to false expect(loadedSettings.merged.admin?.secureModeEnabled).toBe(true); - expect(loadedSettings.merged.admin?.mcp?.enabled).toBe(true); - expect(loadedSettings.merged.admin?.extensions?.enabled).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({ @@ -2289,7 +2289,7 @@ describe('Settings Loading and Merging', () => { // Verify mcpEnabled is updated, others remain defaults (secureModeEnabled reverts to default:false) expect(loadedSettings.merged.admin?.secureModeEnabled).toBe(false); expect(loadedSettings.merged.admin?.mcp?.enabled).toBe(false); - expect(loadedSettings.merged.admin?.extensions?.enabled).toBe(true); + expect(loadedSettings.merged.admin?.extensions?.enabled).toBe(false); // Set remote settings with only cliFeatureSetting.extensionsSetting.extensionsEnabled loadedSettings.setRemoteAdminSettings({ @@ -2298,7 +2298,7 @@ describe('Settings Loading and Merging', () => { // Verify extensionsEnabled is updated, others remain defaults expect(loadedSettings.merged.admin?.secureModeEnabled).toBe(false); - expect(loadedSettings.merged.admin?.mcp?.enabled).toBe(true); + expect(loadedSettings.merged.admin?.mcp?.enabled).toBe(false); expect(loadedSettings.merged.admin?.extensions?.enabled).toBe(false); }); @@ -2318,6 +2318,62 @@ 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 false if undefined, overriding schema defaults', () => { + // Mock schema to have secureModeEnabled default to true to verify the override + const originalSchema = getSettingsSchema(); + const modifiedSchema = JSON.parse(JSON.stringify(originalSchema)); + if (modifiedSchema.admin?.properties?.secureModeEnabled) { + modifiedSchema.admin.properties.secureModeEnabled.default = true; + } + 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 secureModeEnabled + loadedSettings.setRemoteAdminSettings({ + mcpSetting: {}, + }); + + // It should be forced to false by the logic, overriding the mock default of true + expect(loadedSettings.merged.admin?.secureModeEnabled).toBe(false); + } finally { + vi.mocked(getSettingsSchema).mockReturnValue(originalSchema); + } + }); + + it('should handle completely empty remote admin settings response', () => { + const loadedSettings = loadSettings(MOCK_WORKSPACE_DIR); + + loadedSettings.setRemoteAdminSettings({}); + + // Should default to schema defaults (standard defaults) + expect(loadedSettings.merged.admin?.secureModeEnabled).toBe(false); + expect(loadedSettings.merged.admin?.mcp?.enabled).toBe(true); + expect(loadedSettings.merged.admin?.extensions?.enabled).toBe(true); + }); }); describe('getDefaultsFromSchema', () => { diff --git a/packages/cli/src/config/settings.ts b/packages/cli/src/config/settings.ts index d7da64195f..dcfaf27f41 100644 --- a/packages/cli/src/config/settings.ts +++ b/packages/cli/src/config/settings.ts @@ -350,18 +350,17 @@ export class LoadedSettings { const admin: Settings['admin'] = {}; const { secureModeEnabled, mcpSetting, cliFeatureSetting } = remoteSettings; - if (secureModeEnabled !== undefined) { - admin.secureModeEnabled = secureModeEnabled; + if (Object.keys(remoteSettings).length === 0) { + this._remoteAdminSettings = { admin }; + this._merged = this.computeMergedSettings(); + return; } - if (mcpSetting?.mcpEnabled !== undefined) { - admin.mcp = { enabled: mcpSetting.mcpEnabled }; - } - - const extensionsSetting = cliFeatureSetting?.extensionsSetting; - if (extensionsSetting?.extensionsEnabled !== undefined) { - admin.extensions = { enabled: extensionsSetting.extensionsEnabled }; - } + admin.secureModeEnabled = secureModeEnabled ?? false; + admin.mcp = { enabled: mcpSetting?.mcpEnabled ?? false }; + admin.extensions = { + enabled: cliFeatureSetting?.extensionsSetting?.extensionsEnabled ?? false, + }; if (cliFeatureSetting?.advancedFeaturesEnabled !== undefined) { admin.skills = { enabled: cliFeatureSetting.advancedFeaturesEnabled }; diff --git a/packages/cli/src/ui/AppContainer.tsx b/packages/cli/src/ui/AppContainer.tsx index 45ccd33ad0..f08d947eca 100644 --- a/packages/cli/src/ui/AppContainer.tsx +++ b/packages/cli/src/ui/AppContainer.tsx @@ -1888,6 +1888,15 @@ Logging in with Google... Restarting Gemini CLI to continue. setEmbeddedShellFocused, setAuthContext, handleRestart: async () => { + if (process.send) { + const remoteSettings = config.getRemoteAdminSettings(); + if (remoteSettings) { + process.send({ + type: 'admin-settings-update', + settings: remoteSettings, + }); + } + } await runExitCleanup(); process.exit(RELAUNCH_EXIT_CODE); }, @@ -1963,6 +1972,7 @@ Logging in with Google... Restarting Gemini CLI to continue. setAuthContext({}); setAuthState(AuthState.Updating); }} + config={config} /> ); } diff --git a/packages/cli/src/ui/auth/LoginWithGoogleRestartDialog.test.tsx b/packages/cli/src/ui/auth/LoginWithGoogleRestartDialog.test.tsx index 907f1447db..ac0966c111 100644 --- a/packages/cli/src/ui/auth/LoginWithGoogleRestartDialog.test.tsx +++ b/packages/cli/src/ui/auth/LoginWithGoogleRestartDialog.test.tsx @@ -10,6 +10,7 @@ import { LoginWithGoogleRestartDialog } from './LoginWithGoogleRestartDialog.js' import { useKeypress } from '../hooks/useKeypress.js'; import { runExitCleanup } from '../../utils/cleanup.js'; import { RELAUNCH_EXIT_CODE } from '../../utils/processUtils.js'; +import { type Config } from '@google/gemini-cli-core'; // Mocks vi.mock('../hooks/useKeypress.js', () => ({ @@ -29,6 +30,10 @@ describe('LoginWithGoogleRestartDialog', () => { .spyOn(process, 'exit') .mockImplementation(() => undefined as never); + const mockConfig = { + getRemoteAdminSettings: vi.fn(), + } as unknown as Config; + beforeEach(() => { vi.clearAllMocks(); exitSpy.mockClear(); @@ -37,13 +42,21 @@ describe('LoginWithGoogleRestartDialog', () => { it('renders correctly', () => { const { lastFrame } = render( - , + , ); expect(lastFrame()).toMatchSnapshot(); }); it('calls onDismiss when escape is pressed', () => { - render(); + render( + , + ); const keypressHandler = mockedUseKeypress.mock.calls[0][0]; keypressHandler({ @@ -62,7 +75,12 @@ describe('LoginWithGoogleRestartDialog', () => { async (keyName) => { vi.useFakeTimers(); - render(); + render( + , + ); const keypressHandler = mockedUseKeypress.mock.calls[0][0]; keypressHandler({ diff --git a/packages/cli/src/ui/auth/LoginWithGoogleRestartDialog.tsx b/packages/cli/src/ui/auth/LoginWithGoogleRestartDialog.tsx index 0418e3f3f3..03a18bced7 100644 --- a/packages/cli/src/ui/auth/LoginWithGoogleRestartDialog.tsx +++ b/packages/cli/src/ui/auth/LoginWithGoogleRestartDialog.tsx @@ -4,6 +4,7 @@ * SPDX-License-Identifier: Apache-2.0 */ +import { type Config } from '@google/gemini-cli-core'; import { Box, Text } from 'ink'; import { theme } from '../semantic-colors.js'; import { useKeypress } from '../hooks/useKeypress.js'; @@ -12,10 +13,12 @@ import { RELAUNCH_EXIT_CODE } from '../../utils/processUtils.js'; interface LoginWithGoogleRestartDialogProps { onDismiss: () => void; + config: Config; } export const LoginWithGoogleRestartDialog = ({ onDismiss, + config, }: LoginWithGoogleRestartDialogProps) => { useKeypress( (key) => { @@ -23,6 +26,15 @@ export const LoginWithGoogleRestartDialog = ({ onDismiss(); } else if (key.name === 'r' || key.name === 'R') { setTimeout(async () => { + if (process.send) { + const remoteSettings = config.getRemoteAdminSettings(); + if (remoteSettings) { + process.send({ + type: 'admin-settings-update', + settings: remoteSettings, + }); + } + } await runExitCleanup(); process.exit(RELAUNCH_EXIT_CODE); }, 100); diff --git a/packages/cli/src/utils/relaunch.ts b/packages/cli/src/utils/relaunch.ts index 131caf9bae..c2d987845d 100644 --- a/packages/cli/src/utils/relaunch.ts +++ b/packages/cli/src/utils/relaunch.ts @@ -40,6 +40,8 @@ export async function relaunchAppInChildProcess( return; } + let latestAdminSettings = remoteAdminSettings; + const runner = () => { // process.argv is [node, script, ...args] // We want to construct [ ...nodeArgs, script, ...scriptArgs] @@ -63,10 +65,16 @@ export async function relaunchAppInChildProcess( env: newEnv, }); - if (remoteAdminSettings) { - child.send({ type: 'admin-settings', settings: remoteAdminSettings }); + if (latestAdminSettings) { + child.send({ type: 'admin-settings', settings: latestAdminSettings }); } + child.on('message', (msg: { type?: string; settings?: unknown }) => { + if (msg.type === 'admin-settings-update' && msg.settings) { + latestAdminSettings = msg.settings as FetchAdminControlsResponse; + } + }); + return new Promise((resolve, reject) => { child.on('error', reject); child.on('close', (code) => { 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 22876f10a2..a72fd53fc3 100644 --- a/packages/core/src/code_assist/admin/admin_controls.test.ts +++ b/packages/core/src/code_assist/admin/admin_controls.test.ts @@ -170,6 +170,25 @@ describe('Admin Controls', () => { expect(mockServer.fetchAdminControls).toHaveBeenCalledTimes(2); // Initial + poll }); + it('should return empty object on 403 fetch error and STOP polling', async () => { + const error403 = new Error('Forbidden'); + Object.assign(error403, { status: 403 }); + (mockServer.fetchAdminControls as Mock).mockRejectedValue(error403); + + const result = await fetchAdminControls( + mockServer, + undefined, + true, + mockOnSettingsChanged, + ); + + expect(result).toEqual({}); + + // Advance time - should NOT poll because of 403 + await vi.advanceTimersByTimeAsync(5 * 60 * 1000); + expect(mockServer.fetchAdminControls).toHaveBeenCalledTimes(1); // Only the initial call + }); + it('should sanitize server response', async () => { (mockServer.fetchAdminControls as Mock).mockResolvedValue({ secureModeEnabled: true, @@ -302,6 +321,32 @@ describe('Admin Controls', () => { secureModeEnabled: true, }); }); + + it('should STOP polling if server returns 403', async () => { + // Initial fetch is successful + (mockServer.fetchAdminControls as Mock).mockResolvedValue({ + secureModeEnabled: false, + }); + await fetchAdminControls( + mockServer, + undefined, + true, + mockOnSettingsChanged, + ); + expect(mockServer.fetchAdminControls).toHaveBeenCalledTimes(1); + + // Next poll returns 403 + const error403 = new Error('Forbidden'); + Object.assign(error403, { status: 403 }); + (mockServer.fetchAdminControls as Mock).mockRejectedValue(error403); + + await vi.advanceTimersByTimeAsync(5 * 60 * 1000); + expect(mockServer.fetchAdminControls).toHaveBeenCalledTimes(2); + + // Advance time again - should NOT poll again + await vi.advanceTimersByTimeAsync(5 * 60 * 1000); + expect(mockServer.fetchAdminControls).toHaveBeenCalledTimes(2); + }); }); describe('stopAdminControlsPolling', () => { diff --git a/packages/core/src/code_assist/admin/admin_controls.ts b/packages/core/src/code_assist/admin/admin_controls.ts index 93af330ecb..e2feaf9414 100644 --- a/packages/core/src/code_assist/admin/admin_controls.ts +++ b/packages/core/src/code_assist/admin/admin_controls.ts @@ -25,6 +25,15 @@ export function sanitizeAdminSettings( return result.data; } +function isGaxiosError(error: unknown): error is { status: number } { + return ( + typeof error === 'object' && + error !== null && + 'status' in error && + typeof (error as { status: unknown }).status === 'number' + ); +} + /** * Fetches the admin controls from the server if enabled by experiment flag. * Safely handles polling start/stop based on the flag and server availability. @@ -64,6 +73,12 @@ export async function fetchAdminControls( startAdminControlsPolling(server, server.projectId, onSettingsChanged); return sanitizedSettings; } catch (e) { + // Non-enterprise users don't have access to fetch settings. + if (isGaxiosError(e) && e.status === 403) { + stopAdminControlsPolling(); + currentSettings = undefined; + return {}; + } debugLogger.error('Failed to fetch admin controls: ', e); // If initial fetch fails, start polling to retry. currentSettings = {}; @@ -95,6 +110,12 @@ function startAdminControlsPolling( onSettingsChanged(newSettings); } } catch (e) { + // Non-enterprise users don't have access to fetch settings. + if (isGaxiosError(e) && e.status === 403) { + stopAdminControlsPolling(); + currentSettings = undefined; + return; + } debugLogger.error('Failed to poll admin controls: ', e); } },