From 86134e9970a80c0e97e563616fc44192defb8de1 Mon Sep 17 00:00:00 2001 From: Nemo Date: Fri, 12 Dec 2025 14:30:34 +0800 Subject: [PATCH] feat(settings-validation): add validation for settings schema (#12929) --- .../src/config/settings-validation.test.ts | 398 ++++++++++++++++++ .../cli/src/config/settings-validation.ts | 331 +++++++++++++++ packages/cli/src/config/settings.ts | 21 +- 3 files changed, 749 insertions(+), 1 deletion(-) create mode 100644 packages/cli/src/config/settings-validation.test.ts create mode 100644 packages/cli/src/config/settings-validation.ts diff --git a/packages/cli/src/config/settings-validation.test.ts b/packages/cli/src/config/settings-validation.test.ts new file mode 100644 index 0000000000..8d4cc641f5 --- /dev/null +++ b/packages/cli/src/config/settings-validation.test.ts @@ -0,0 +1,398 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +/// + +import { describe, it, expect } from 'vitest'; +import { + validateSettings, + formatValidationError, + settingsZodSchema, +} from './settings-validation.js'; +import { z } from 'zod'; + +describe('settings-validation', () => { + describe('validateSettings', () => { + it('should accept valid settings with correct model.name as string', () => { + const validSettings = { + model: { + name: 'gemini-2.0-flash-exp', + maxSessionTurns: 10, + }, + ui: { + theme: 'dark', + }, + }; + + const result = validateSettings(validSettings); + expect(result.success).toBe(true); + }); + + it('should reject model.name as object instead of string', () => { + const invalidSettings = { + model: { + name: { + skipNextSpeakerCheck: true, + }, + }, + }; + + const result = validateSettings(invalidSettings); + expect(result.success).toBe(false); + expect(result.error).toBeDefined(); + + if (result.error) { + const issues = result.error.issues; + expect(issues.length).toBeGreaterThan(0); + expect(issues[0]?.path).toEqual(['model', 'name']); + expect(issues[0]?.code).toBe('invalid_type'); + } + }); + + it('should accept valid model.summarizeToolOutput structure', () => { + const validSettings = { + model: { + summarizeToolOutput: { + run_shell_command: { + tokenBudget: 500, + }, + }, + }, + }; + + const result = validateSettings(validSettings); + expect(result.success).toBe(true); + }); + + it('should reject invalid model.summarizeToolOutput structure', () => { + const invalidSettings = { + model: { + summarizeToolOutput: { + run_shell_command: { + tokenBudget: 500, + }, + }, + }, + }; + + // First test with valid structure + let result = validateSettings(invalidSettings); + expect(result.success).toBe(true); + + // Now test with wrong type (string instead of object) + const actuallyInvalidSettings = { + model: { + summarizeToolOutput: 'invalid', + }, + }; + + result = validateSettings(actuallyInvalidSettings); + expect(result.success).toBe(false); + if (result.error) { + expect(result.error.issues.length).toBeGreaterThan(0); + } + }); + + it('should accept empty settings object', () => { + const emptySettings = {}; + const result = validateSettings(emptySettings); + expect(result.success).toBe(true); + }); + + it('should accept unknown top-level keys (for migration compatibility)', () => { + const settingsWithUnknownKey = { + unknownKey: 'some value', + }; + + const result = validateSettings(settingsWithUnknownKey); + expect(result.success).toBe(true); + // Unknown keys are allowed via .passthrough() for migration scenarios + }); + + it('should accept nested valid settings', () => { + const validSettings = { + ui: { + theme: 'dark', + hideWindowTitle: true, + footer: { + hideCWD: false, + hideModelInfo: true, + }, + }, + tools: { + sandbox: 'inherit', + autoAccept: false, + }, + }; + + const result = validateSettings(validSettings); + expect(result.success).toBe(true); + }); + + it('should validate array types correctly', () => { + const validSettings = { + tools: { + allowed: ['git', 'npm'], + exclude: ['dangerous-tool'], + }, + context: { + includeDirectories: ['/path/1', '/path/2'], + }, + }; + + const result = validateSettings(validSettings); + expect(result.success).toBe(true); + }); + + it('should reject invalid types in arrays', () => { + const invalidSettings = { + tools: { + allowed: ['git', 123], + }, + }; + + const result = validateSettings(invalidSettings); + expect(result.success).toBe(false); + }); + + it('should validate boolean fields correctly', () => { + const validSettings = { + general: { + vimMode: true, + disableAutoUpdate: false, + }, + }; + + const result = validateSettings(validSettings); + expect(result.success).toBe(true); + }); + + it('should reject non-boolean values for boolean fields', () => { + const invalidSettings = { + general: { + vimMode: 'yes', + }, + }; + + const result = validateSettings(invalidSettings); + expect(result.success).toBe(false); + }); + + it('should validate number fields correctly', () => { + const validSettings = { + model: { + maxSessionTurns: 50, + compressionThreshold: 0.2, + }, + }; + + const result = validateSettings(validSettings); + expect(result.success).toBe(true); + }); + + it('should validate complex nested mcpServers configuration', () => { + const invalidSettings = { + mcpServers: { + 'my-server': { + command: 123, // Should be string + args: ['arg1'], + env: { + VAR: 'value', + }, + }, + }, + }; + + const result = validateSettings(invalidSettings); + expect(result.success).toBe(false); + if (result.error) { + expect(result.error.issues.length).toBeGreaterThan(0); + // Path should be mcpServers.my-server.command + const issue = result.error.issues.find((i) => + i.path.includes('command'), + ); + expect(issue).toBeDefined(); + expect(issue?.code).toBe('invalid_type'); + } + }); + + it('should validate complex nested customThemes configuration', () => { + const invalidSettings = { + ui: { + customThemes: { + 'my-theme': { + type: 'custom', + // Missing 'name' property which is required + text: { + primary: '#ffffff', + }, + }, + }, + }, + }; + + const result = validateSettings(invalidSettings); + expect(result.success).toBe(false); + if (result.error) { + expect(result.error.issues.length).toBeGreaterThan(0); + // Should complain about missing 'name' + const issue = result.error.issues.find( + (i) => i.code === 'invalid_type' && i.message.includes('Required'), + ); + expect(issue).toBeDefined(); + } + }); + }); + + describe('formatValidationError', () => { + it('should format error with file path and helpful message for model.name', () => { + const invalidSettings = { + model: { + name: { + skipNextSpeakerCheck: true, + }, + }, + }; + + const result = validateSettings(invalidSettings); + expect(result.success).toBe(false); + + if (result.error) { + const formatted = formatValidationError( + result.error, + '/path/to/settings.json', + ); + + expect(formatted).toContain('/path/to/settings.json'); + expect(formatted).toContain('model.name'); + expect(formatted).toContain('Expected: string, but received: object'); + expect(formatted).toContain( + 'Please fix the configuration and try again.', + ); + expect(formatted).toContain( + 'https://github.com/google-gemini/gemini-cli', + ); + } + }); + + it('should format error for model.summarizeToolOutput', () => { + const invalidSettings = { + model: { + summarizeToolOutput: 'wrong type', + }, + }; + + const result = validateSettings(invalidSettings); + expect(result.success).toBe(false); + + if (result.error) { + const formatted = formatValidationError( + result.error, + '~/.gemini/settings.json', + ); + + expect(formatted).toContain('~/.gemini/settings.json'); + expect(formatted).toContain('model.summarizeToolOutput'); + } + }); + + it('should include link to documentation', () => { + const invalidSettings = { + model: { + name: { invalid: 'object' }, // model.name should be a string + }, + }; + + const result = validateSettings(invalidSettings); + expect(result.success).toBe(false); + + if (result.error) { + const formatted = formatValidationError(result.error, 'test.json'); + + expect(formatted).toContain( + 'https://github.com/google-gemini/gemini-cli', + ); + expect(formatted).toContain('configuration.md'); + } + }); + + it('should list all validation errors', () => { + const invalidSettings = { + model: { + name: { invalid: 'object' }, + maxSessionTurns: 'not a number', + }, + }; + + const result = validateSettings(invalidSettings); + expect(result.success).toBe(false); + + if (result.error) { + const formatted = formatValidationError(result.error, 'test.json'); + + // Should have multiple errors listed + expect(formatted.match(/Error in:/g)?.length).toBeGreaterThan(1); + } + }); + + it('should format array paths correctly (e.g. tools.allowed[0])', () => { + const invalidSettings = { + tools: { + allowed: ['git', 123], // 123 is invalid, expected string + }, + }; + + const result = validateSettings(invalidSettings); + expect(result.success).toBe(false); + + if (result.error) { + const formatted = formatValidationError(result.error, 'test.json'); + expect(formatted).toContain('tools.allowed[1]'); + } + }); + + it('should limit the number of displayed errors', () => { + const invalidSettings = { + tools: { + // Create 6 invalid items to trigger the limit + allowed: [1, 2, 3, 4, 5, 6], + }, + }; + + const result = validateSettings(invalidSettings); + expect(result.success).toBe(false); + + if (result.error) { + const formatted = formatValidationError(result.error, 'test.json'); + // Should see the first 5 + expect(formatted).toContain('tools.allowed[0]'); + expect(formatted).toContain('tools.allowed[4]'); + // Should NOT see the 6th + expect(formatted).not.toContain('tools.allowed[5]'); + // Should see the summary + expect(formatted).toContain('...and 1 more errors.'); + } + }); + }); + + describe('settingsZodSchema', () => { + it('should be a valid Zod object schema', () => { + expect(settingsZodSchema).toBeInstanceOf(z.ZodObject); + }); + + it('should have optional fields', () => { + // All top-level fields should be optional + const shape = settingsZodSchema.shape; + expect(shape['model']).toBeDefined(); + expect(shape['ui']).toBeDefined(); + expect(shape['tools']).toBeDefined(); + + // Test that empty object is valid (all fields optional) + const result = settingsZodSchema.safeParse({}); + expect(result.success).toBe(true); + }); + }); +}); diff --git a/packages/cli/src/config/settings-validation.ts b/packages/cli/src/config/settings-validation.ts new file mode 100644 index 0000000000..17b3351565 --- /dev/null +++ b/packages/cli/src/config/settings-validation.ts @@ -0,0 +1,331 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { z } from 'zod'; +import { + getSettingsSchema, + type SettingDefinition, + type SettingCollectionDefinition, + SETTINGS_SCHEMA_DEFINITIONS, +} from './settingsSchema.js'; + +// Helper to build Zod schema from the JSON-schema-like definitions +// eslint-disable-next-line @typescript-eslint/no-explicit-any +function buildZodSchemaFromJsonSchema(def: any): z.ZodTypeAny { + if (def.anyOf) { + return z.union( + // eslint-disable-next-line @typescript-eslint/no-explicit-any + def.anyOf.map((d: any) => buildZodSchemaFromJsonSchema(d)) as any, + ); + } + + if (def.type === 'string') { + if (def.enum) return z.enum(def.enum as [string, ...string[]]); + return z.string(); + } + if (def.type === 'number') return z.number(); + if (def.type === 'boolean') return z.boolean(); + + if (def.type === 'array') { + if (def.items) { + return z.array(buildZodSchemaFromJsonSchema(def.items)); + } + return z.array(z.unknown()); + } + + if (def.type === 'object') { + let schema; + if (def.properties) { + const shape: Record = {}; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + for (const [key, propDef] of Object.entries(def.properties) as any) { + let propSchema = buildZodSchemaFromJsonSchema(propDef); + if ( + def.required && + Array.isArray(def.required) && + def.required.includes(key) + ) { + // keep it required + } else { + propSchema = propSchema.optional(); + } + shape[key] = propSchema; + } + schema = z.object(shape).passthrough(); + } else { + schema = z.object({}).passthrough(); + } + + if (def.additionalProperties === false) { + schema = schema.strict(); + } else if (typeof def.additionalProperties === 'object') { + schema = schema.catchall( + buildZodSchemaFromJsonSchema(def.additionalProperties), + ); + } + + return schema; + } + + return z.unknown(); +} + +/** + * Builds a Zod enum schema from options array + */ +function buildEnumSchema( + options: ReadonlyArray<{ value: string | number | boolean; label: string }>, +): z.ZodTypeAny { + if (!options || options.length === 0) { + throw new Error( + `Enum type must have options defined. Check your settings schema definition.`, + ); + } + const values = options.map((opt) => opt.value); + if (values.every((v) => typeof v === 'string')) { + return z.enum(values as [string, ...string[]]); + } else if (values.every((v) => typeof v === 'number')) { + return z.union( + values.map((v) => z.literal(v)) as [ + z.ZodLiteral, + z.ZodLiteral, + ...Array>, + ], + ); + } else { + return z.union( + values.map((v) => z.literal(v)) as [ + z.ZodLiteral, + z.ZodLiteral, + ...Array>, + ], + ); + } +} + +/** + * Builds a Zod object shape from properties record + */ +function buildObjectShapeFromProperties( + properties: Record, +): Record { + const shape: Record = {}; + for (const [key, childDef] of Object.entries(properties)) { + shape[key] = buildZodSchemaFromDefinition(childDef); + } + return shape; +} + +/** + * Builds a Zod schema for primitive types (string, number, boolean) + */ +function buildPrimitiveSchema( + type: 'string' | 'number' | 'boolean', +): z.ZodTypeAny { + switch (type) { + case 'string': + return z.string(); + case 'number': + return z.number(); + case 'boolean': + return z.boolean(); + default: + return z.unknown(); + } +} + +const REF_SCHEMAS: Record = {}; + +// Initialize REF_SCHEMAS +for (const [name, def] of Object.entries(SETTINGS_SCHEMA_DEFINITIONS)) { + REF_SCHEMAS[name] = buildZodSchemaFromJsonSchema(def); +} + +/** + * Recursively builds a Zod schema from a SettingDefinition + */ +function buildZodSchemaFromDefinition( + definition: SettingDefinition, +): z.ZodTypeAny { + let baseSchema: z.ZodTypeAny; + + // Special handling for TelemetrySettings which can be boolean or object + if (definition.ref === 'TelemetrySettings') { + const objectSchema = REF_SCHEMAS['TelemetrySettings']; + if (objectSchema) { + return z.union([z.boolean(), objectSchema]).optional(); + } + } + + // Handle refs using registry + if (definition.ref && definition.ref in REF_SCHEMAS) { + return REF_SCHEMAS[definition.ref].optional(); + } + + switch (definition.type) { + case 'string': + case 'number': + case 'boolean': + baseSchema = buildPrimitiveSchema(definition.type); + break; + + case 'enum': { + baseSchema = buildEnumSchema(definition.options!); + break; + } + + case 'array': + if (definition.items) { + const itemSchema = buildZodSchemaFromCollection(definition.items); + baseSchema = z.array(itemSchema); + } else { + baseSchema = z.array(z.unknown()); + } + break; + + case 'object': + if (definition.properties) { + const shape = buildObjectShapeFromProperties(definition.properties); + baseSchema = z.object(shape).passthrough(); + + if (definition.additionalProperties) { + const additionalSchema = buildZodSchemaFromCollection( + definition.additionalProperties, + ); + baseSchema = z.object(shape).catchall(additionalSchema); + } + } else if (definition.additionalProperties) { + const valueSchema = buildZodSchemaFromCollection( + definition.additionalProperties, + ); + baseSchema = z.record(z.string(), valueSchema); + } else { + baseSchema = z.record(z.string(), z.unknown()); + } + break; + + default: + baseSchema = z.unknown(); + } + + // Make all fields optional since settings are partial + return baseSchema.optional(); +} + +/** + * Builds a Zod schema from a SettingCollectionDefinition + */ +function buildZodSchemaFromCollection( + collection: SettingCollectionDefinition, +): z.ZodTypeAny { + if (collection.ref && collection.ref in REF_SCHEMAS) { + return REF_SCHEMAS[collection.ref]; + } + + switch (collection.type) { + case 'string': + case 'number': + case 'boolean': + return buildPrimitiveSchema(collection.type); + + case 'enum': { + return buildEnumSchema(collection.options!); + } + + case 'array': + if (collection.properties) { + const shape = buildObjectShapeFromProperties(collection.properties); + return z.array(z.object(shape)); + } + return z.array(z.unknown()); + + case 'object': + if (collection.properties) { + const shape = buildObjectShapeFromProperties(collection.properties); + return z.object(shape).passthrough(); + } + return z.record(z.string(), z.unknown()); + + default: + return z.unknown(); + } +} + +/** + * Builds the complete Zod schema for Settings from SETTINGS_SCHEMA + */ +function buildSettingsZodSchema(): z.ZodObject> { + const schema = getSettingsSchema(); + const shape: Record = {}; + + for (const [key, definition] of Object.entries(schema)) { + shape[key] = buildZodSchemaFromDefinition(definition); + } + + return z.object(shape).passthrough(); +} + +export const settingsZodSchema = buildSettingsZodSchema(); + +/** + * Validates settings data against the Zod schema + */ +export function validateSettings(data: unknown): { + success: boolean; + data?: unknown; + error?: z.ZodError; +} { + const result = settingsZodSchema.safeParse(data); + return result; +} + +/** + * Format a Zod error into a helpful error message + */ +export function formatValidationError( + error: z.ZodError, + filePath: string, +): string { + const lines: string[] = []; + lines.push(`Invalid configuration in ${filePath}:`); + lines.push(''); + + const MAX_ERRORS_TO_DISPLAY = 5; + const displayedIssues = error.issues.slice(0, MAX_ERRORS_TO_DISPLAY); + + for (const issue of displayedIssues) { + const path = issue.path.reduce( + (acc, curr) => + typeof curr === 'number' + ? `${acc}[${curr}]` + : `${acc ? acc + '.' : ''}${curr}`, + '', + ); + lines.push(`Error in: ${path || '(root)'}`); + lines.push(` ${issue.message}`); + + if (issue.code === 'invalid_type') { + const expected = issue.expected; + const received = issue.received; + lines.push(`Expected: ${expected}, but received: ${received}`); + } + lines.push(''); + } + + if (error.issues.length > MAX_ERRORS_TO_DISPLAY) { + lines.push( + `...and ${error.issues.length - MAX_ERRORS_TO_DISPLAY} more errors.`, + ); + lines.push(''); + } + + lines.push('Please fix the configuration and try again.'); + lines.push( + 'See: https://github.com/google-gemini/gemini-cli/blob/main/docs/get-started/configuration.md', + ); + + return lines.join('\n'); +} diff --git a/packages/cli/src/config/settings.ts b/packages/cli/src/config/settings.ts index 6365e99991..1983642231 100644 --- a/packages/cli/src/config/settings.ts +++ b/packages/cli/src/config/settings.ts @@ -33,6 +33,10 @@ import { resolveEnvVarsInObject } from '../utils/envVarResolver.js'; import { customDeepMerge, type MergeableObject } from '../utils/deepMerge.js'; import { updateSettingsFilePreservingFormat } from '../utils/commentJson.js'; import type { ExtensionManager } from './extension-manager.js'; +import { + validateSettings, + formatValidationError, +} from './settings-validation.js'; import { SettingPaths } from './settingPaths.js'; function getMergeStrategyForPath(path: string[]): MergeStrategy | undefined { @@ -270,7 +274,7 @@ export function needsMigration(settings: Record): boolean { if (v1Key === v2Path || !(v1Key in settings)) { return false; } - // If a key exists that is both a V1 key and a V2 container (like 'model'), + // If a key exists that is a V1 key and a V2 container (like 'model'), // we need to check the type. If it's an object, it's a V2 container and not // a V1 key that needs migration. if ( @@ -670,9 +674,24 @@ export function loadSettings( settingsObject = migratedSettings; } } + + // Validate settings structure with Zod after migration + const validationResult = validateSettings(settingsObject); + if (!validationResult.success && validationResult.error) { + const errorMessage = formatValidationError( + validationResult.error, + filePath, + ); + throw new FatalConfigError(errorMessage); + } + return { settings: settingsObject as Settings, rawJson: content }; } } catch (error: unknown) { + // Preserve FatalConfigError with formatted validation messages + if (error instanceof FatalConfigError) { + throw error; + } settingsErrors.push({ message: getErrorMessage(error), path: filePath,