chore(workers/repository): split malicious dependency checks (renovatebot#42508)

* chore(workers/repository): report malicious packages before `packageFiles with updates`

As part of future changes, we may modify the `skipReason`s before the
update phase, which means that we should ensure these are correct before
logging out `packageFiles with updates`.

* chore(workers/repository): split malicious dependency checks

We will likely run into two specific cases of malicious dependencies -
you're currently using one, or you're receiving an update to one.

We should make these separate, explicit, cases so users are aware what
`malicious` actually means, and what the path forward is.

Author: jamietanna

lib/types/skip-reason.ts

@@ -47,9 +47,17 @@ export type SkipReason =
   | 'github-token-required'
   | 'inherited-dependency'
   /**
-   * The dependency has been detected as explicitly malicious, and should not be updated.
+   * The dependency has been detected as explicitly malicious.
+   *
+   * This reason should be removed before the update phase, so updates can be determined.
    */
-  | 'malicious';
+  | 'malicious-version-in-use'
+  /**
+   * The dependency has a new dependency version available which has been marked as malicious.
+   *
+   * Renovate will not propose any updates, and leave you on the version you are currently on, which is currently known as safe.
+   */
+  | 'malicious-update-proposed';
 
 export type StageName =
   | 'current-timestamp'

lib/workers/repository/process/extract-update.spec.ts

@@ -159,23 +159,143 @@ describe('workers/repository/process/extract-update', () => {
       expect(createVulnerabilitiesMock).toHaveBeenCalledExactlyOnceWith();
     });
 
-    it('logs a warning for each skipReason=malicious', async () => {
+    describe('when skipReason=malicious-version-in-use', () => {
+      it('logs a warning', async () => {
+        const packageFiles: Record<string, PackageFile[]> = {
+          npm: [
+            {
+              deps: [
+                {
+                  depType: 'devDependencies',
+                  depName: 'axios',
+                  currentValue: '1.14.1',
+                  datasource: 'npm',
+                  prettyDepType: 'devDependency',
+                  lockedVersion: '1.14.1',
+                  updates: [],
+                  packageName: 'axios',
+
+                  // most importantly
+                  skipReason: 'malicious-version-in-use',
+                },
+                // not malicious
+                {
+                  depType: 'devDependencies',
+                  depName: 'axios',
+                  currentValue: '1.14.0',
+                  datasource: 'npm',
+                  prettyDepType: 'devDependency',
+                  lockedVersion: '1.14.0',
+                  updates: [],
+                  packageName: 'axios',
+                },
+              ],
+              packageFile: 'package.json',
+            },
+          ],
+        };
+
+        const config = {
+          repoIsOnboarded: true,
+          baseBranch: 'main',
+        };
+
+        await lookup(config, packageFiles);
+
+        expect(logger.logger.warn).toHaveBeenCalledWith(
+          {
+            packageFile: 'package.json',
+            depName: 'axios',
+            packageName: 'axios',
+            manager: 'npm',
+            datasource: 'npm',
+          },
+          'Dependency axios is currently using a malicious version',
+        );
+      });
+
+      it('deletes the skipReason and skipStage, to allow the update phase to continue updating', async () => {
+        const packageFiles: Record<string, PackageFile[]> = {
+          npm: [
+            {
+              deps: [
+                {
+                  depType: 'devDependencies',
+                  depName: 'axios',
+                  currentValue: '1.14.1',
+                  datasource: 'npm',
+                  prettyDepType: 'devDependency',
+                  lockedVersion: '1.14.1',
+                  updates: [],
+                  packageName: 'axios',
+
+                  // most importantly
+                  skipReason: 'malicious-version-in-use',
+                  skipStage: 'lookup',
+                },
+                // not malicious
+                {
+                  depType: 'devDependencies',
+                  depName: 'axios',
+                  currentValue: '1.14.0',
+                  datasource: 'npm',
+                  prettyDepType: 'devDependency',
+                  lockedVersion: '1.14.0',
+                  updates: [],
+                  packageName: 'axios',
+                },
+              ],
+              packageFile: 'package.json',
+            },
+          ],
+        };
+
+        const config = {
+          repoIsOnboarded: true,
+          baseBranch: 'main',
+        };
+
+        await lookup(config, packageFiles);
+
+        expect(packageFiles.npm[0].deps[0].skipReason).toBeUndefined();
+        expect(packageFiles.npm[0].deps[0].skipStage).toBeUndefined();
+      });
+    });
+
+    it('when skipReason=malicious-version-in-use, it logs a warning for each skipReason', async () => {
       const packageFiles: Record<string, PackageFile[]> = {
         npm: [
           {
             deps: [
               {
                 depType: 'devDependencies',
                 depName: 'axios',
-                currentValue: '1.14.1',
+                currentValue: '1.14.0',
                 datasource: 'npm',
                 prettyDepType: 'devDependency',
-                lockedVersion: '1.14.1',
-                updates: [],
+                lockedVersion: '1.14.0',
+                updates: [
+                  {
+                    newVersion: '1.14.1',
+                  },
+                  {
+                    // unrelated, using newValue
+                    newValue: '1.14.2',
+                  },
+                  {
+                    // unrelated
+                    newVersion: '2.0.0',
+                  },
+                  {
+                    // doesn't have a newVersion or newValue
+                    updateType: 'digest',
+                    newDigest: '1234',
+                  },
+                ],
                 packageName: 'axios',
 
                 // most importantly
-                skipReason: 'malicious',
+                skipReason: 'malicious-update-proposed',
               },
               // not malicious
               {
@@ -208,8 +328,9 @@ describe('workers/repository/process/extract-update', () => {
           packageName: 'axios',
           manager: 'npm',
           datasource: 'npm',
+          newVersions: ['1.14.1', '1.14.2', '2.0.0'],
         },
-        'Dependency axios will not be updated as it malicious',
+        'Dependency axios has update(s) proposed which would update you to a malicious version - skipping',
       );
     });
   });

lib/workers/repository/process/extract-update.ts

@@ -5,6 +5,7 @@ import { logger } from '../../../logger/index.ts';
 import { hashMap } from '../../../modules/manager/index.ts';
 import type { PackageFile } from '../../../modules/manager/types.ts';
 import { scm } from '../../../modules/platform/scm.ts';
+import { isNotNullOrUndefined } from '../../../util/array.ts';
 import * as memCache from '../../../util/cache/memory/index.ts';
 import { getCache } from '../../../util/cache/repository/index.ts';
 import type { BaseBranchCache } from '../../../util/cache/repository/types.ts';
@@ -236,12 +237,12 @@ export async function lookup(
     config,
     packageFiles,
   );
+  reportMaliciousSkippedDependencies(packageFiles);
   logger.debug(
     { baseBranch: config.baseBranch, config: packageFiles },
     'packageFiles with updates',
   );
   sortBranches(branches);
-  reportMaliciousSkippedDependencies(packageFiles);
   return { branches, branchList, packageFiles };
 }
 
@@ -255,16 +256,35 @@ export function reportMaliciousSkippedDependencies(
   for (const [manager, packageFiles] of Object.entries(allPackageFiles)) {
     for (const packageFile of packageFiles) {
       for (const dep of packageFile.deps) {
-        if (dep.skipReason === 'malicious') {
+        if (dep.skipReason === 'malicious-version-in-use') {
+          logger.warn(
+            {
+              packageFile: packageFile.packageFile,
+              depName: dep.depName,
+              packageName: dep.packageName,
+              manager: manager,
+              datasource: dep.datasource,
+            },
+            `Dependency ${dep.depName} is currently using a malicious version`,
+          );
+
+          // and make sure that it then gets updates proposed in the update phase
+          delete dep.skipReason;
+          delete dep.skipStage;
+        } else if (dep.skipReason === 'malicious-update-proposed') {
+          const newVersions = dep.updates
+            ?.map((u) => u.newVersion ?? u.newValue)
+            .filter(isNotNullOrUndefined);
           logger.warn(
             {
               packageFile: packageFile.packageFile,
               depName: dep.depName,
               packageName: dep.packageName,
               manager: manager,
               datasource: dep.datasource,
+              newVersions,
             },
-            `Dependency ${dep.depName} will not be updated as it malicious`,
+            `Dependency ${dep.depName} has update(s) proposed which would update you to a malicious version - skipping`,
           );
         }
       }