diff options
| author | christine betts <[email protected]> | 2025-07-09 21:16:42 +0000 |
|---|---|---|
| committer | GitHub <[email protected]> | 2025-07-09 21:16:42 +0000 |
| commit | da50a1eefbd0751aaf137882595d500e6b6b4179 (patch) | |
| tree | 4a07182ca01f20074a5544db5caf5a198cabae6c /packages/cli/src/config | |
| parent | 063481faa4b1c86868689580ff0fbd8cb04141e3 (diff) | |
Add system-wide settings config for administrators (#3498)
Co-authored-by: Jack Wotherspoon <[email protected]>
Diffstat (limited to 'packages/cli/src/config')
| -rw-r--r-- | packages/cli/src/config/config.ts | 1 | ||||
| -rw-r--r-- | packages/cli/src/config/settings.test.ts | 167 | ||||
| -rw-r--r-- | packages/cli/src/config/settings.ts | 42 |
3 files changed, 206 insertions, 4 deletions
diff --git a/packages/cli/src/config/config.ts b/packages/cli/src/config/config.ts index ca38814a..20a8afcf 100644 --- a/packages/cli/src/config/config.ts +++ b/packages/cli/src/config/config.ts @@ -168,6 +168,7 @@ async function parseArguments(): Promise<CliArgs> { type: 'boolean', description: 'List all available extensions and exit.', }) + .version(await getCliVersion()) // This will enable the --version flag based on package.json .alias('v', 'version') .help() diff --git a/packages/cli/src/config/settings.test.ts b/packages/cli/src/config/settings.test.ts index e7565457..44de24fe 100644 --- a/packages/cli/src/config/settings.test.ts +++ b/packages/cli/src/config/settings.test.ts @@ -13,6 +13,7 @@ vi.mock('os', async (importOriginal) => { return { ...actualOs, homedir: vi.fn(() => '/mock/home/user'), + platform: vi.fn(() => 'linux'), }; }); @@ -45,6 +46,7 @@ import stripJsonComments from 'strip-json-comments'; // Will be mocked separatel import { loadSettings, USER_SETTINGS_PATH, // This IS the mocked path. + SYSTEM_SETTINGS_PATH, SETTINGS_DIRECTORY_NAME, // This is from the original module, but used by the mock. SettingScope, } from './settings.js'; @@ -90,12 +92,41 @@ describe('Settings Loading and Merging', () => { describe('loadSettings', () => { it('should load empty settings if no files exist', () => { const settings = loadSettings(MOCK_WORKSPACE_DIR); + expect(settings.system.settings).toEqual({}); expect(settings.user.settings).toEqual({}); expect(settings.workspace.settings).toEqual({}); expect(settings.merged).toEqual({}); expect(settings.errors.length).toBe(0); }); + it('should load system settings if only system file exists', () => { + (mockFsExistsSync as Mock).mockImplementation( + (p: fs.PathLike) => p === SYSTEM_SETTINGS_PATH, + ); + const systemSettingsContent = { + theme: 'system-default', + sandbox: false, + }; + (fs.readFileSync as Mock).mockImplementation( + (p: fs.PathOrFileDescriptor) => { + if (p === SYSTEM_SETTINGS_PATH) + return JSON.stringify(systemSettingsContent); + return '{}'; + }, + ); + + const settings = loadSettings(MOCK_WORKSPACE_DIR); + + expect(fs.readFileSync).toHaveBeenCalledWith( + SYSTEM_SETTINGS_PATH, + 'utf-8', + ); + expect(settings.system.settings).toEqual(systemSettingsContent); + expect(settings.user.settings).toEqual({}); + expect(settings.workspace.settings).toEqual({}); + expect(settings.merged).toEqual(systemSettingsContent); + }); + it('should load user settings if only user file exists', () => { const expectedUserSettingsPath = USER_SETTINGS_PATH; // Use the path actually resolved by the (mocked) module @@ -187,6 +218,50 @@ describe('Settings Loading and Merging', () => { }); }); + it('should merge system, user and workspace settings, with system taking precedence over workspace, and workspace over user', () => { + (mockFsExistsSync as Mock).mockReturnValue(true); + const systemSettingsContent = { + theme: 'system-theme', + sandbox: false, + telemetry: { enabled: false }, + }; + const userSettingsContent = { + theme: 'dark', + sandbox: true, + contextFileName: 'USER_CONTEXT.md', + }; + const workspaceSettingsContent = { + sandbox: false, + coreTools: ['tool1'], + contextFileName: 'WORKSPACE_CONTEXT.md', + }; + + (fs.readFileSync as Mock).mockImplementation( + (p: fs.PathOrFileDescriptor) => { + if (p === SYSTEM_SETTINGS_PATH) + return JSON.stringify(systemSettingsContent); + if (p === USER_SETTINGS_PATH) + return JSON.stringify(userSettingsContent); + if (p === MOCK_WORKSPACE_SETTINGS_PATH) + return JSON.stringify(workspaceSettingsContent); + return ''; + }, + ); + + const settings = loadSettings(MOCK_WORKSPACE_DIR); + + expect(settings.system.settings).toEqual(systemSettingsContent); + expect(settings.user.settings).toEqual(userSettingsContent); + expect(settings.workspace.settings).toEqual(workspaceSettingsContent); + expect(settings.merged).toEqual({ + theme: 'system-theme', + sandbox: false, + telemetry: { enabled: false }, + coreTools: ['tool1'], + contextFileName: 'WORKSPACE_CONTEXT.md', + }); + }); + it('should handle contextFileName correctly when only in user settings', () => { (mockFsExistsSync as Mock).mockImplementation( (p: fs.PathLike) => p === USER_SETTINGS_PATH, @@ -409,6 +484,50 @@ describe('Settings Loading and Merging', () => { delete process.env.WORKSPACE_ENDPOINT; }); + it('should prioritize user env variables over workspace env variables if keys clash after resolution', () => { + const userSettingsContent = { configValue: '$SHARED_VAR' }; + const workspaceSettingsContent = { configValue: '$SHARED_VAR' }; + + (mockFsExistsSync as Mock).mockReturnValue(true); + const originalSharedVar = process.env.SHARED_VAR; + // Temporarily delete to ensure a clean slate for the test's specific manipulations + delete process.env.SHARED_VAR; + + (fs.readFileSync as Mock).mockImplementation( + (p: fs.PathOrFileDescriptor) => { + if (p === USER_SETTINGS_PATH) { + process.env.SHARED_VAR = 'user_value_for_user_read'; // Set for user settings read + return JSON.stringify(userSettingsContent); + } + if (p === MOCK_WORKSPACE_SETTINGS_PATH) { + process.env.SHARED_VAR = 'workspace_value_for_workspace_read'; // Set for workspace settings read + return JSON.stringify(workspaceSettingsContent); + } + return '{}'; + }, + ); + + const settings = loadSettings(MOCK_WORKSPACE_DIR); + + expect(settings.user.settings.configValue).toBe( + 'user_value_for_user_read', + ); + expect(settings.workspace.settings.configValue).toBe( + 'workspace_value_for_workspace_read', + ); + // Merged should take workspace's resolved value + expect(settings.merged.configValue).toBe( + 'workspace_value_for_workspace_read', + ); + + // Restore original environment variable state + if (originalSharedVar !== undefined) { + process.env.SHARED_VAR = originalSharedVar; + } else { + delete process.env.SHARED_VAR; // Ensure it's deleted if it wasn't there before + } + }); + it('should prioritize workspace env variables over user env variables if keys clash after resolution', () => { const userSettingsContent = { configValue: '$SHARED_VAR' }; const workspaceSettingsContent = { configValue: '$SHARED_VAR' }; @@ -453,6 +572,48 @@ describe('Settings Loading and Merging', () => { } }); + it('should prioritize system env variables over workspace env variables if keys clash after resolution', () => { + const workspaceSettingsContent = { configValue: '$SHARED_VAR' }; + const systemSettingsContent = { configValue: '$SHARED_VAR' }; + + (mockFsExistsSync as Mock).mockReturnValue(true); + const originalSharedVar = process.env.SHARED_VAR; + // Temporarily delete to ensure a clean slate for the test's specific manipulations + delete process.env.SHARED_VAR; + + (fs.readFileSync as Mock).mockImplementation( + (p: fs.PathOrFileDescriptor) => { + if (p === SYSTEM_SETTINGS_PATH) { + process.env.SHARED_VAR = 'system_value_for_system_read'; // Set for system settings read + return JSON.stringify(systemSettingsContent); + } + if (p === MOCK_WORKSPACE_SETTINGS_PATH) { + process.env.SHARED_VAR = 'workspace_value_for_workspace_read'; // Set for workspace settings read + return JSON.stringify(workspaceSettingsContent); + } + return '{}'; + }, + ); + + const settings = loadSettings(MOCK_WORKSPACE_DIR); + + expect(settings.system.settings.configValue).toBe( + 'system_value_for_system_read', + ); + expect(settings.workspace.settings.configValue).toBe( + 'workspace_value_for_workspace_read', + ); + // Merged should take workspace's resolved value + expect(settings.merged.configValue).toBe('system_value_for_system_read'); + + // Restore original environment variable state + if (originalSharedVar !== undefined) { + process.env.SHARED_VAR = originalSharedVar; + } else { + delete process.env.SHARED_VAR; // Ensure it's deleted if it wasn't there before + } + }); + it('should leave unresolved environment variables as is', () => { const userSettingsContent = { apiKey: '$UNDEFINED_VAR' }; (mockFsExistsSync as Mock).mockImplementation( @@ -624,10 +785,10 @@ describe('Settings Loading and Merging', () => { 'utf-8', ); - // Workspace theme overrides user theme - loadedSettings.setValue(SettingScope.Workspace, 'theme', 'ocean'); + // System theme overrides user and workspace themes + loadedSettings.setValue(SettingScope.System, 'theme', 'ocean'); - expect(loadedSettings.workspace.settings.theme).toBe('ocean'); + expect(loadedSettings.system.settings.theme).toBe('ocean'); expect(loadedSettings.merged.theme).toBe('ocean'); }); }); diff --git a/packages/cli/src/config/settings.ts b/packages/cli/src/config/settings.ts index c2d03167..133701f5 100644 --- a/packages/cli/src/config/settings.ts +++ b/packages/cli/src/config/settings.ts @@ -6,7 +6,7 @@ import * as fs from 'fs'; import * as path from 'path'; -import { homedir } from 'os'; +import { homedir, platform } from 'os'; import * as dotenv from 'dotenv'; import { MCPServerConfig, @@ -24,9 +24,22 @@ export const SETTINGS_DIRECTORY_NAME = '.gemini'; export const USER_SETTINGS_DIR = path.join(homedir(), SETTINGS_DIRECTORY_NAME); export const USER_SETTINGS_PATH = path.join(USER_SETTINGS_DIR, 'settings.json'); +function getSystemSettingsPath(): string { + if (platform() === 'darwin') { + return '/Library/Application Support/GeminiCli/settings.json'; + } else if (platform() === 'win32') { + return 'C:\\ProgramData\\gemini-cli\\settings.json'; + } else { + return '/etc/gemini-cli/settings.json'; + } +} + +export const SYSTEM_SETTINGS_PATH = getSystemSettingsPath(); + export enum SettingScope { User = 'User', Workspace = 'Workspace', + System = 'System', } export interface CheckpointingSettings { @@ -81,16 +94,19 @@ export interface SettingsFile { } export class LoadedSettings { constructor( + system: SettingsFile, user: SettingsFile, workspace: SettingsFile, errors: SettingsError[], ) { + this.system = system; this.user = user; this.workspace = workspace; this.errors = errors; this._merged = this.computeMergedSettings(); } + readonly system: SettingsFile; readonly user: SettingsFile; readonly workspace: SettingsFile; readonly errors: SettingsError[]; @@ -105,6 +121,7 @@ export class LoadedSettings { return { ...this.user.settings, ...this.workspace.settings, + ...this.system.settings, }; } @@ -114,6 +131,8 @@ export class LoadedSettings { return this.user; case SettingScope.Workspace: return this.workspace; + case SettingScope.System: + return this.system; default: throw new Error(`Invalid scope: ${scope}`); } @@ -243,10 +262,27 @@ export function loadEnvironment(): void { */ export function loadSettings(workspaceDir: string): LoadedSettings { loadEnvironment(); + let systemSettings: Settings = {}; let userSettings: Settings = {}; let workspaceSettings: Settings = {}; const settingsErrors: SettingsError[] = []; + // Load system settings + try { + if (fs.existsSync(SYSTEM_SETTINGS_PATH)) { + const systemContent = fs.readFileSync(SYSTEM_SETTINGS_PATH, 'utf-8'); + const parsedSystemSettings = JSON.parse( + stripJsonComments(systemContent), + ) as Settings; + systemSettings = resolveEnvVarsInObject(parsedSystemSettings); + } + } catch (error: unknown) { + settingsErrors.push({ + message: getErrorMessage(error), + path: SYSTEM_SETTINGS_PATH, + }); + } + // Load user settings try { if (fs.existsSync(USER_SETTINGS_PATH)) { @@ -301,6 +337,10 @@ export function loadSettings(workspaceDir: string): LoadedSettings { return new LoadedSettings( { + path: SYSTEM_SETTINGS_PATH, + settings: systemSettings, + }, + { path: USER_SETTINGS_PATH, settings: userSettings, }, |
