From 48b0945156262cab70397d6404ef69ddbc9c5e0d Mon Sep 17 00:00:00 2001 From: RahulGautamSingh Date: Tue, 27 Feb 2024 13:09:49 +0545 Subject: [PATCH] fix(config/validation): improve validation of `globalOnly` options (#27487) Co-authored-by: HonkingGoose <34918129+HonkingGoose@users.noreply.github.com> --- lib/config/validation.spec.ts | 326 +++++++++++++++++++++++++++++++--- lib/config/validation.ts | 200 ++++++++++++++++++--- 2 files changed, 470 insertions(+), 56 deletions(-) diff --git a/lib/config/validation.spec.ts b/lib/config/validation.spec.ts index 34726a0f029750..70d5233a9d745f 100644 --- a/lib/config/validation.spec.ts +++ b/lib/config/validation.spec.ts @@ -1,3 +1,4 @@ +import { configFileNames } from './app-strings'; import { GlobalConfig } from './global'; import type { RenovateConfig } from './types'; import * as configValidation from './validation'; @@ -38,16 +39,15 @@ describe('config/validation', () => { expect(warnings).toHaveLength(2); expect(warnings).toMatchObject([ { - message: `The "binarySource" option is a global option reserved only for bot's global configuration and cannot be configured within repository config file`, + message: `The "binarySource" option is a global option reserved only for Renovate's global configuration and cannot be configured within repository config file.`, }, { - message: `The "username" option is a global option reserved only for bot's global configuration and cannot be configured within repository config file`, + message: `The "username" option is a global option reserved only for Renovate's global configuration and cannot be configured within repository config file.`, }, ]); }); - // false globals are the options which have names same to the another globalOnly option - it('does warn for false globals in repo config', async () => { + it('only warns for actual globals in repo config', async () => { const config = { hostRules: [ { @@ -967,38 +967,19 @@ describe('config/validation', () => { expect(warnings).toHaveLength(1); }); - it('validates valid customEnvVariables objects', async () => { - const config = { - customEnvVariables: { - example1: 'abc', - example2: 'https://www.example2.com/example', - }, - }; - const { warnings, errors } = await configValidation.validateConfig( - true, - config, - ); - expect(warnings).toHaveLength(0); - expect(errors).toHaveLength(0); - }); - - it('errors on invalid customEnvVariables objects', async () => { + // adding this test explicitly because we used to validate the customEnvVariables inside repo config previously + it('warns if customEnvVariables are found in repo config', async () => { const config = { customEnvVariables: { example1: 'abc', example2: 123, }, }; - const { warnings, errors } = await configValidation.validateConfig( - true, - config, - ); - expect(warnings).toHaveLength(0); - expect(errors).toMatchObject([ + const { warnings } = await configValidation.validateConfig(false, config); + expect(warnings).toMatchObject([ { - message: - 'Invalid `customEnvVariables.customEnvVariables.example2` configuration: value is not a string', topic: 'Configuration Error', + message: `The "customEnvVariables" option is a global option reserved only for Renovate's global configuration and cannot be configured within repository config file.`, }, ]); }); @@ -1242,4 +1223,293 @@ describe('config/validation', () => { ]); }); }); + + describe('validate globalOptions()', () => { + describe('validates string type options', () => { + it('binarySource', async () => { + const config = { + binarySource: 'invalid' as never, + }; + const { warnings } = await configValidation.validateConfig( + true, + config, + ); + expect(warnings).toEqual([ + { + message: + 'Invalid value `invalid` for `binarySource`. The allowed values are docker, global, install, hermit.', + topic: 'Configuration Error', + }, + ]); + }); + + it('baseDir', async () => { + const config = { + baseDir: false as never, + }; + const { warnings } = await configValidation.validateConfig( + true, + config, + ); + expect(warnings).toEqual([ + { + message: 'Configuration option `baseDir` should be a string.', + topic: 'Configuration Error', + }, + ]); + }); + + it('requireConfig', async () => { + const config = { + requireConfig: 'invalid' as never, + }; + const { warnings } = await configValidation.validateConfig( + true, + config, + ); + expect(warnings).toEqual([ + { + message: + 'Invalid value `invalid` for `requireConfig`. The allowed values are required, optional, ignored.', + topic: 'Configuration Error', + }, + ]); + }); + + it('dryRun', async () => { + const config = { + dryRun: 'invalid' as never, + }; + const { warnings } = await configValidation.validateConfig( + true, + config, + ); + expect(warnings).toEqual([ + { + message: + 'Invalid value `invalid` for `dryRun`. The allowed values are extract, lookup, full.', + topic: 'Configuration Error', + }, + ]); + }); + + it('repositoryCache', async () => { + const config = { + repositoryCache: 'invalid' as never, + }; + const { warnings } = await configValidation.validateConfig( + true, + config, + ); + expect(warnings).toEqual([ + { + message: + 'Invalid value `invalid` for `repositoryCache`. The allowed values are enabled, disabled, reset.', + topic: 'Configuration Error', + }, + ]); + }); + + it('onboardingConfigFileName', async () => { + const config = { + onboardingConfigFileName: 'invalid' as never, + }; + const { warnings } = await configValidation.validateConfig( + true, + config, + ); + expect(warnings).toEqual([ + { + message: `Invalid value \`invalid\` for \`onboardingConfigFileName\`. The allowed values are ${configFileNames.join(', ')}.`, + topic: 'Configuration Error', + }, + ]); + }); + + it('onboardingConfig', async () => { + const config = { + onboardingConfig: { + extends: ['config:recommended'], + binarySource: 'global', // should not allow globalOnly options inside onboardingConfig + fileMatch: ['somefile'], // invalid at top level + }, + }; + const { warnings } = await configValidation.validateConfig( + true, + config, + ); + expect(warnings).toEqual([ + { + message: + '"fileMatch" may not be defined at the top level of a config and must instead be within a manager block', + topic: 'Config error', + }, + { + topic: 'Configuration Error', + message: `The "binarySource" option is a global option reserved only for Renovate's global configuration and cannot be configured within repository config file.`, + }, + ]); + }); + + it('force', async () => { + const config = { + force: { + extends: ['config:recommended'], + binarySource: 'global', + fileMatch: ['somefile'], // invalid at top level + constraints: { + python: '2.7', + }, + }, + }; + const { warnings } = await configValidation.validateConfig( + true, + config, + ); + expect(warnings).toEqual([ + { + message: + '"fileMatch" may not be defined at the top level of a config and must instead be within a manager block', + topic: 'Config error', + }, + ]); + }); + + it('gitUrl', async () => { + const config = { + gitUrl: 'invalid' as never, + }; + const { warnings } = await configValidation.validateConfig( + true, + config, + ); + expect(warnings).toEqual([ + { + message: + 'Invalid value `invalid` for `gitUrl`. The allowed values are default, ssh, endpoint.', + topic: 'Configuration Error', + }, + ]); + }); + }); + + it('validates boolean type options', async () => { + const config = { + unicodeEmoji: false, + detectGlobalManagerConfig: 'invalid-type', + }; + const { warnings } = await configValidation.validateConfig(true, config); + expect(warnings).toMatchObject([ + { + message: `Configuration option \`detectGlobalManagerConfig\` should be a boolean. Found: ${JSON.stringify( + 'invalid-type', + )} (string).`, + topic: 'Configuration Error', + }, + ]); + }); + + it('validates integer type options', async () => { + const config = { + prCommitsPerRunLimit: 2, + gitTimeout: 'invalid-type', + }; + const { warnings } = await configValidation.validateConfig(true, config); + expect(warnings).toMatchObject([ + { + message: `Configuration option \`gitTimeout\` should be an integer. Found: ${JSON.stringify( + 'invalid-type', + )} (string).`, + topic: 'Configuration Error', + }, + ]); + }); + + it('validates array type options', async () => { + const config = { + allowedPostUpgradeCommands: ['cmd'], + checkedBranches: 'invalid-type', + gitNoVerify: ['invalid'], + }; + const { warnings } = await configValidation.validateConfig( + true, + // @ts-expect-error: contains invalid values + config, + ); + expect(warnings).toMatchObject([ + { + message: + 'Configuration option `checkedBranches` should be a list (Array).', + topic: 'Configuration Error', + }, + { + message: + 'Invalid value for `gitNoVerify`. The allowed values are commit, push.', + topic: 'Configuration Error', + }, + ]); + }); + + it('validates object type options', async () => { + const config = { + productLinks: { + documentation: 'https://docs.renovatebot.com/', + help: 'https://github.com/renovatebot/renovate/discussions', + homepage: 'https://github.com/renovatebot/renovate', + }, + secrets: 'invalid-type', + cacheTtlOverride: { + someField: false, + }, + }; + const { warnings } = await configValidation.validateConfig( + true, + // @ts-expect-error: contains invalid values + config, + ); + expect(warnings).toMatchObject([ + { + message: 'Configuration option `secrets` should be a JSON object.', + topic: 'Configuration Error', + }, + { + topic: 'Configuration Error', + message: + 'Invalid `cacheTtlOverride.someField` configuration: value must be an integer.', + }, + ]); + }); + + it('warns on invalid customEnvVariables objects', async () => { + const config = { + customEnvVariables: { + example1: 'abc', + example2: 123, + }, + }; + const { warnings } = await configValidation.validateConfig(true, config); + expect(warnings).toMatchObject([ + { + message: + 'Invalid `customEnvVariables.example2` configuration: value must be a string.', + topic: 'Configuration Error', + }, + ]); + }); + + it('validates valid customEnvVariables objects', async () => { + const config = { + customEnvVariables: { + example1: 'abc', + example2: 'https://www.example2.com/example', + }, + }; + const { warnings, errors } = await configValidation.validateConfig( + true, + config, + ); + expect(warnings).toHaveLength(0); + expect(errors).toHaveLength(0); + }); + }); }); diff --git a/lib/config/validation.ts b/lib/config/validation.ts index d01ad16658f4ca..7b3ea7af6513e3 100644 --- a/lib/config/validation.ts +++ b/lib/config/validation.ts @@ -19,6 +19,7 @@ import { hasValidSchedule, hasValidTimezone, } from '../workers/repository/update/branch/schedule'; +import { configFileNames } from './app-strings'; import { GlobalConfig } from './global'; import { migrateConfig } from './migration'; import { getOptions } from './options'; @@ -97,6 +98,18 @@ function getDeprecationMessage(option: string): string | undefined { return deprecatedOptions[option]; } +function isGlobalOption(key: string): boolean { + if (!optionGlobals) { + optionGlobals = new Set(); + for (const option of options) { + if (option.globalOnly) { + optionGlobals.add(option.name); + } + } + } + return optionGlobals.has(key); +} + export function getParentName(parentPath: string | undefined): string { return parentPath ? parentPath @@ -127,7 +140,6 @@ export async function validateConfig( } }); } - let errors: ValidationMessage[] = []; let warnings: ValidationMessage[] = []; @@ -151,20 +163,21 @@ export async function validateConfig( message: `The "${key}" object can only be configured at the top level of a config but was found inside "${parentPath}"`, }); } - if (!isGlobalConfig) { - if (!optionGlobals) { - optionGlobals = new Set(); - for (const option of options) { - if (option.globalOnly) { - optionGlobals.add(option.name); - } - } - } - if (optionGlobals.has(key) && !isFalseGlobal(key, parentPath)) { + if (isGlobalConfig && isGlobalOption(key)) { + await validateGlobalConfig( + key, + val, + optionTypes[key], + warnings, + currentPath, + ); + continue; + } else { + if (isGlobalOption(key) && !isFalseGlobal(key, parentPath)) { warnings.push({ topic: 'Configuration Error', - message: `The "${key}" option is a global option reserved only for bot's global configuration and cannot be configured within repository config file`, + message: `The "${key}" option is a global option reserved only for Renovate's global configuration and cannot be configured within repository config file.`, }); continue; } @@ -686,22 +699,6 @@ export async function validateConfig( } } } - } else if ( - [ - 'customEnvVariables', - 'migratePresets', - 'productLinks', - 'secrets', - 'customizeDashboard', - ].includes(key) - ) { - const res = validatePlainObject(val); - if (res !== true) { - errors.push({ - topic: 'Configuration Error', - message: `Invalid \`${currentPath}.${key}.${res}\` configuration: value is not a string`, - }); - } } else { const ignoredObjects = options .filter((option) => option.freeChoice) @@ -811,6 +808,153 @@ function validateRegexManagerFields( } } +/** + * Basic validation for global config options + */ +async function validateGlobalConfig( + key: string, + val: unknown, + type: string, + warnings: ValidationMessage[], + currentPath: string | undefined, +): Promise { + if (type === 'string') { + if (is.string(val)) { + if ( + key === 'onboardingConfigFileName' && + !configFileNames.includes(val) + ) { + warnings.push({ + topic: 'Configuration Error', + message: `Invalid value \`${val}\` for \`${currentPath}\`. The allowed values are ${configFileNames.join(', ')}.`, + }); + } else if ( + key === 'repositoryCache' && + !['enabled', 'disabled', 'reset'].includes(val) + ) { + warnings.push({ + topic: 'Configuration Error', + message: `Invalid value \`${val}\` for \`${currentPath}\`. The allowed values are ${['enabled', 'disabled', 'reset'].join(', ')}.`, + }); + } else if ( + key === 'dryRun' && + !['extract', 'lookup', 'full'].includes(val) + ) { + warnings.push({ + topic: 'Configuration Error', + message: `Invalid value \`${val}\` for \`${currentPath}\`. The allowed values are ${['extract', 'lookup', 'full'].join(', ')}.`, + }); + } else if ( + key === 'binarySource' && + !['docker', 'global', 'install', 'hermit'].includes(val) + ) { + warnings.push({ + topic: 'Configuration Error', + message: `Invalid value \`${val}\` for \`${currentPath}\`. The allowed values are ${['docker', 'global', 'install', 'hermit'].join(', ')}.`, + }); + } else if ( + key === 'requireConfig' && + !['required', 'optional', 'ignored'].includes(val) + ) { + warnings.push({ + topic: 'Configuration Error', + message: `Invalid value \`${val}\` for \`${currentPath}\`. The allowed values are ${['required', 'optional', 'ignored'].join(', ')}.`, + }); + } else if ( + key === 'gitUrl' && + !['default', 'ssh', 'endpoint'].includes(val) + ) { + warnings.push({ + topic: 'Configuration Error', + message: `Invalid value \`${val}\` for \`${currentPath}\`. The allowed values are ${['default', 'ssh', 'endpoint'].join(', ')}.`, + }); + } + } else { + warnings.push({ + topic: 'Configuration Error', + message: `Configuration option \`${currentPath}\` should be a string.`, + }); + } + } else if (type === 'integer') { + if (!is.number(val)) { + warnings.push({ + topic: 'Configuration Error', + message: `Configuration option \`${currentPath}\` should be an integer. Found: ${JSON.stringify( + val, + )} (${typeof val}).`, + }); + } + } else if (type === 'boolean') { + if (val !== true && val !== false) { + warnings.push({ + topic: 'Configuration Error', + message: `Configuration option \`${currentPath}\` should be a boolean. Found: ${JSON.stringify( + val, + )} (${typeof val}).`, + }); + } + } else if (type === 'array') { + if (is.array(val)) { + if (key === 'gitNoVerify') { + const allowedValues = ['commit', 'push']; + for (const value of val as string[]) { + if (!allowedValues.includes(value)) { + warnings.push({ + topic: 'Configuration Error', + message: `Invalid value for \`${currentPath}\`. The allowed values are ${allowedValues.join(', ')}.`, + }); + } + } + } + } else { + warnings.push({ + topic: 'Configuration Error', + message: `Configuration option \`${currentPath}\` should be a list (Array).`, + }); + } + } else if (type === 'object') { + if (is.plainObject(val)) { + if (key === 'onboardingConfig') { + const subValidation = await validateConfig(false, val); + for (const warning of subValidation.warnings.concat( + subValidation.errors, + )) { + warnings.push(warning); + } + } else if (key === 'force') { + const subValidation = await validateConfig(true, val); + for (const warning of subValidation.warnings.concat( + subValidation.errors, + )) { + warnings.push(warning); + } + } else if (key === 'cacheTtlOverride') { + for (const [subKey, subValue] of Object.entries(val)) { + if (!is.number(subValue)) { + warnings.push({ + topic: 'Configuration Error', + message: `Invalid \`${currentPath}.${subKey}\` configuration: value must be an integer.`, + }); + } + } + } else { + const res = validatePlainObject(val); + if (res !== true) { + warnings.push({ + topic: 'Configuration Error', + message: `Invalid \`${currentPath}.${res}\` configuration: value must be a string.`, + }); + } + } + } else { + warnings.push({ + topic: 'Configuration Error', + message: `Configuration option \`${currentPath}\` should be a JSON object.`, + }); + } + } +} + /** An option is a false global if it has the same name as a global only option * but is actually just the field of a non global option or field an children of the non global option * eg. token: it's global option used as the bot's token as well and