From 48af0da981193aaa9e6eb254fcb976a765042b34 Mon Sep 17 00:00:00 2001
From: Michael Kriese <michael.kriese@visualon.de>
Date: Thu, 20 May 2021 12:25:22 +0200
Subject: [PATCH] fix: validate `customEnvVariables` (#10070)

---
 .../__snapshots__/validation.spec.ts.snap     |  20 ---
 lib/config/definitions.ts                     |   2 +-
 lib/config/types.ts                           |   4 +
 lib/config/validation.spec.ts                 |  52 +++++++-
 lib/config/validation.ts                      | 118 ++++++++++--------
 5 files changed, 118 insertions(+), 78 deletions(-)

diff --git a/lib/config/__snapshots__/validation.spec.ts.snap b/lib/config/__snapshots__/validation.spec.ts.snap
index 6ae432f8ec..27285c4322 100644
--- a/lib/config/__snapshots__/validation.spec.ts.snap
+++ b/lib/config/__snapshots__/validation.spec.ts.snap
@@ -101,24 +101,6 @@ Array [
 ]
 `;
 
-exports[`config/validation validateConfig(config) errors if aliases depth is more than 1 1`] = `
-Array [
-  Object {
-    "message": "Invalid alias object configuration",
-    "topic": "Configuration Error",
-  },
-]
-`;
-
-exports[`config/validation validateConfig(config) errors if aliases have invalid url 1`] = `
-Array [
-  Object {
-    "message": "Invalid alias object configuration",
-    "topic": "Configuration Error",
-  },
-]
-`;
-
 exports[`config/validation validateConfig(config) errors if fileMatch has wrong parent 1`] = `
 Array [
   Object {
@@ -258,8 +240,6 @@ Array [
 ]
 `;
 
-exports[`config/validation validateConfig(config) validates valid alias objects 1`] = `Array []`;
-
 exports[`config/validation validateConfig(config) warns if hostType has the wrong parent 1`] = `
 Array [
   Object {
diff --git a/lib/config/definitions.ts b/lib/config/definitions.ts
index 6994ac05e0..70d2a78d2c 100644
--- a/lib/config/definitions.ts
+++ b/lib/config/definitions.ts
@@ -252,7 +252,7 @@ const options: RenovateOptions[] = [
       'Custom environment variables for child processes and sidecar Docker containers.',
     admin: true,
     type: 'object',
-    default: false,
+    default: {},
   },
   {
     name: 'dockerChildPrefix',
diff --git a/lib/config/types.ts b/lib/config/types.ts
index d17c840129..5b00e51439 100644
--- a/lib/config/types.ts
+++ b/lib/config/types.ts
@@ -270,7 +270,11 @@ export interface RenovateOptionBase {
 
   env?: false | string;
 
+  /**
+   * Do not validate object children
+   */
   freeChoice?: boolean;
+
   mergeable?: boolean;
 
   autogenerated?: boolean;
diff --git a/lib/config/validation.spec.ts b/lib/config/validation.spec.ts
index 5d337956ff..eb7eb914b4 100644
--- a/lib/config/validation.spec.ts
+++ b/lib/config/validation.spec.ts
@@ -460,7 +460,6 @@ describe(getName(), () => {
       );
       expect(warnings).toHaveLength(0);
       expect(errors).toHaveLength(0);
-      expect(errors).toMatchSnapshot();
     });
 
     it('errors if aliases depth is more than 1', async () => {
@@ -475,8 +474,13 @@ describe(getName(), () => {
         config
       );
       expect(warnings).toHaveLength(0);
-      expect(errors).toHaveLength(1);
-      expect(errors).toMatchSnapshot();
+      expect(errors).toMatchObject([
+        {
+          message:
+            'Invalid `aliases.aliases.sample` configuration: value is not a url',
+          topic: 'Configuration Error',
+        },
+      ]);
     });
 
     it('errors if aliases have invalid url', async () => {
@@ -490,8 +494,13 @@ describe(getName(), () => {
         config
       );
       expect(warnings).toHaveLength(0);
-      expect(errors).toHaveLength(1);
-      expect(errors).toMatchSnapshot();
+      expect(errors).toMatchObject([
+        {
+          message:
+            'Invalid `aliases.aliases.example1` configuration: value is not a url',
+          topic: 'Configuration Error',
+        },
+      ]);
     });
 
     it('errors if fileMatch has wrong parent', async () => {
@@ -619,5 +628,38 @@ describe(getName(), () => {
       expect(errors).toHaveLength(0);
       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(
+        config
+      );
+      expect(warnings).toHaveLength(0);
+      expect(errors).toHaveLength(0);
+    });
+    it('errors on invalid customEnvVariables objects', async () => {
+      const config = {
+        customEnvVariables: {
+          example1: 'abc',
+          example2: 123,
+        },
+      };
+      const { warnings, errors } = await configValidation.validateConfig(
+        config
+      );
+      expect(warnings).toHaveLength(0);
+      expect(errors).toMatchObject([
+        {
+          message:
+            'Invalid `customEnvVariables.customEnvVariables.example2` configuration: value is not a string',
+          topic: 'Configuration Error',
+        },
+      ]);
+    });
   });
 });
diff --git a/lib/config/validation.ts b/lib/config/validation.ts
index 5923659cd4..4893046903 100644
--- a/lib/config/validation.ts
+++ b/lib/config/validation.ts
@@ -21,6 +21,22 @@ let optionParents: Record<string, RenovateOptions['parent']>;
 
 const managerList = getManagerList();
 
+const topLevelObjects = getLanguageList().concat(getManagerList());
+
+const ignoredNodes = [
+  '$schema',
+  'depType',
+  'npmToken',
+  'packageFile',
+  'forkToken',
+  'repository',
+  'vulnerabilityAlertsOnly',
+  'vulnerabilityAlert',
+  'isVulnerabilityAlert',
+  'copyLocalLibs', // deprecated - functionality is now enabled by default
+  'prBody', // deprecated
+];
+
 function isManagerPath(parentPath: string): boolean {
   return (
     /^regexManagers\[[0-9]+]$/.test(parentPath) ||
@@ -28,6 +44,45 @@ function isManagerPath(parentPath: string): boolean {
   );
 }
 
+function isIgnored(key: string): boolean {
+  return ignoredNodes.includes(key);
+}
+
+function validateAliasObject(val: Record<string, unknown>): true | string {
+  for (const [key, value] of Object.entries(val)) {
+    if (!is.urlString(value)) {
+      return key;
+    }
+  }
+  return true;
+}
+
+function validatePlainObject(val: Record<string, unknown>): true | string {
+  for (const [key, value] of Object.entries(val)) {
+    if (!is.string(value)) {
+      return key;
+    }
+  }
+  return true;
+}
+
+function getUnsupportedEnabledManagers(enabledManagers: string[]): string[] {
+  return enabledManagers.filter(
+    (manager) => !getManagerList().includes(manager)
+  );
+}
+
+function getDeprecationMessage(option: string): string {
+  const deprecatedOptions = {
+    branchName: `Direct editing of branchName is now deprecated. Please edit branchPrefix, additionalBranchPrefix, or branchTopic instead`,
+    commitMessage: `Direct editing of commitMessage is now deprecated. Please edit commitMessage's subcomponents instead.`,
+    prTitle: `Direct editing of prTitle is now deprecated. Please edit commitMessage subcomponents instead as they will be passed through to prTitle.`,
+    yarnrc:
+      'Use of `yarnrc` in config is deprecated. Please commit it to your repository instead.',
+  };
+  return deprecatedOptions[option];
+}
+
 export function getParentName(parentPath: string): string {
   return parentPath
     ? parentPath
@@ -38,8 +93,6 @@ export function getParentName(parentPath: string): string {
     : '.';
 }
 
-const topLevelObjects = getLanguageList().concat(getManagerList());
-
 export async function validateConfig(
   config: RenovateConfig,
   isPreset?: boolean,
@@ -62,54 +115,6 @@ export async function validateConfig(
   let errors: ValidationMessage[] = [];
   let warnings: ValidationMessage[] = [];
 
-  function getDeprecationMessage(option: string): string {
-    const deprecatedOptions = {
-      branchName: `Direct editing of branchName is now deprecated. Please edit branchPrefix, additionalBranchPrefix, or branchTopic instead`,
-      commitMessage: `Direct editing of commitMessage is now deprecated. Please edit commitMessage's subcomponents instead.`,
-      prTitle: `Direct editing of prTitle is now deprecated. Please edit commitMessage subcomponents instead as they will be passed through to prTitle.`,
-      yarnrc:
-        'Use of `yarnrc` in config is deprecated. Please commit it to your repository instead.',
-    };
-    return deprecatedOptions[option];
-  }
-
-  function isIgnored(key: string): boolean {
-    const ignoredNodes = [
-      '$schema',
-      'depType',
-      'npmToken',
-      'packageFile',
-      'forkToken',
-      'repository',
-      'vulnerabilityAlertsOnly',
-      'vulnerabilityAlert',
-      'isVulnerabilityAlert',
-      'copyLocalLibs', // deprecated - functionality is now enabled by default
-      'prBody', // deprecated
-    ];
-    return ignoredNodes.includes(key);
-  }
-
-  function validateAliasObject(
-    key: string,
-    val: Record<string, unknown>
-  ): boolean {
-    if (key === 'aliases') {
-      for (const value of Object.values(val)) {
-        if (!is.urlString(value)) {
-          return false;
-        }
-      }
-    }
-    return true;
-  }
-
-  function getUnsupportedEnabledManagers(enabledManagers: string[]): string[] {
-    return enabledManagers.filter(
-      (manager) => !getManagerList().includes(manager)
-    );
-  }
-
   for (const [key, val] of Object.entries(config)) {
     const currentPath = parentPath ? `${parentPath}.${key}` : key;
     // istanbul ignore if
@@ -502,10 +507,19 @@ export async function validateConfig(
         ) {
           if (is.plainObject(val)) {
             if (key === 'aliases') {
-              if (!validateAliasObject(key, val)) {
+              const res = validateAliasObject(val);
+              if (res !== true) {
+                errors.push({
+                  topic: 'Configuration Error',
+                  message: `Invalid \`${currentPath}.${key}.${res}\` configuration: value is not a url`,
+                });
+              }
+            } else if (key === 'customEnvVariables') {
+              const res = validatePlainObject(val);
+              if (res !== true) {
                 errors.push({
                   topic: 'Configuration Error',
-                  message: `Invalid alias object configuration`,
+                  message: `Invalid \`${currentPath}.${key}.${res}\` configuration: value is not a string`,
                 });
               }
             } else {
-- 
GitLab