Merge pull request #106 from snyk/fix/peer-deps-scanned-only-on-npm7

fix: ensure peer deps only scanned on npm7
snyk · Apr 26, 2021 · 991a21d · 991a21d
2 parents 93bc28e + 05e1d1f
commit 991a21d
Show file tree

Hide file tree

Showing 9 changed files with 226 additions and 16 deletions.
diff --git a/lib/parsers/index.ts b/lib/parsers/index.ts
@@ -6,7 +6,6 @@ import { Yarn2Lock } from './yarn2-lock-parser';
 export interface Dep {
   name: string;
   version: string;
-  isPeerDep?: boolean;
   dev?: boolean;
 }
 
@@ -100,6 +99,7 @@ export function parseManifestFile(manifestFileContents: string): ManifestFile {
 export function getTopLevelDeps(
   targetFile: ManifestFile,
   includeDev: boolean,
+  lockfile: Lockfile,
 ): Dep[] {
   const dependencies: Dep[] = [];
 
@@ -120,19 +120,26 @@ export function getTopLevelDeps(
     });
   }
 
-  if (targetFile.peerDependencies) {
+  if (isNpm7(lockfile) && targetFile.peerDependencies) {
     for (const [name, version] of Object.entries(targetFile.peerDependencies)) {
       dependencies.push({
         name,
         version,
-        isPeerDep: true,
       });
     }
   }
-
   return dependencies;
 }
 
+// Only include peerDependencies if using npm and npm is at least
+// version 7 as npm v7 automatically installs peerDependencies
+function isNpm7(lockfile: Lockfile): boolean {
+  return (
+    lockfile.type === LockfileType.npm &&
+    (lockfile as PackageLock).lockfileVersion === 2
+  );
+}
+
 export function createDepTreeDepFromDep(dep: Dep): DepTreeDep {
   return {
     labels: {

diff --git a/lib/parsers/lock-parser-base.ts b/lib/parsers/lock-parser-base.ts
@@ -23,7 +23,6 @@ import {
   OutOfSyncError,
   TreeSizeLimitError,
 } from '../errors';
-import { PackageLock } from './package-lock-parser';
 
 export interface PackageLockDeps {
   [depName: string]: PackageLockDep;
@@ -125,7 +124,11 @@ export abstract class LockParserBase implements LockfileParser {
     );
 
     // get trees for dependencies from manifest file
-    const topLevelDeps: Dep[] = getTopLevelDeps(manifestFile, includeDev);
+    const topLevelDeps: Dep[] = getTopLevelDeps(
+      manifestFile,
+      includeDev,
+      lockfile,
+    );
 
     // number of dependencies including root one
     let treeSize = 1;
@@ -149,8 +152,6 @@ export abstract class LockParserBase implements LockfileParser {
       } else if (/^file:/.test(dep.version)) {
         depTree.dependencies[dep.name] = createDepTreeDepFromDep(dep);
         treeSize++;
-      } else if (isPeerDepInLockfileV1(lockfile, dep)) {
-        continue;
       } else {
         // TODO: also check the package version
         // for a stricter check
@@ -420,11 +421,3 @@ export abstract class LockParserBase implements LockfileParser {
     throw new Error('Not implemented');
   }
 }
-
-// Checks for the case of using npm version 6 or less
-// in conjunction with peer deps. This is needed as npm 6
-// does not auto install peerDeps so we dont need to worry
-// about them.
-function isPeerDepInLockfileV1(lockfile: Lockfile, dep: Dep) {
-  return (lockfile as PackageLock).lockfileVersion === 1 && dep.isPeerDep;
-}
diff --git a/test/lib/fixtures/peer-deps/yarn/expected-tree.json b/test/lib/fixtures/peer-deps/yarn/expected-tree.json
@@ -0,0 +1,34 @@
+{
+  "dependencies": {
+    "adm-zip": {
+      "labels": {
+        "scope": "prod"
+      },
+      "name": "adm-zip",
+      "version": "0.4.7"
+    },
+    "debug": {
+      "labels": {
+        "scope": "prod"
+      },
+      "name": "debug",
+      "version": "2.6.9",
+      "dependencies": {
+        "ms": {
+          "labels": {
+            "scope": "prod"
+          },
+          "name": "ms",
+          "version": "2.0.0"
+        }
+      }
+    }
+  },
+  "hasDevDependencies": false,
+  "name": "goof",
+  "size": 4,
+  "version": "0.0.3",
+  "meta": {
+    "packageManagerVersion": "1"
+  }
+}
diff --git a/test/lib/fixtures/peer-deps/yarn/package.json b/test/lib/fixtures/peer-deps/yarn/package.json
@@ -0,0 +1,13 @@
+{
+  "name": "goof",
+  "version": "0.0.3",
+  "dependencies": {
+    "adm-zip": "0.4.7"
+  },
+  "peerDependencies": {
+    "has": "^1.0.0"
+  },
+  "optionalDependencies": {
+    "debug": "^2.2.0"
+  }
+}
diff --git a/test/lib/fixtures/peer-deps/yarn/yarn.lock b/test/lib/fixtures/peer-deps/yarn/yarn.lock
@@ -0,0 +1,20 @@
+# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY.
+# yarn lockfile v1
+
+
+adm-zip@0.4.7:
+  version "0.4.7"
+  resolved "https://registry.yarnpkg.com/adm-zip/-/adm-zip-0.4.7.tgz#8606c2cbf1c426ce8c8ec00174447fd49b6eafc1"
+  integrity sha1-hgbCy/HEJs6MjsABdER/1Jtur8E=
+
+debug@^2.2.0:
+  version "2.6.9"
+  resolved "https://registry.yarnpkg.com/debug/-/debug-2.6.9.tgz#5d128515df134ff327e90a4c93f4e077a536341f"
+  integrity sha512-bC7ElrdJaJnPbAP+1EotYvqZsb3ecl5wi6Bfi6BJTUcNowp6cvspg0jXznRTKDjm/E7AdgFBVeAPVMNcKGsHMA==
+  dependencies:
+    ms "2.0.0"
+
+ms@2.0.0:
+  version "2.0.0"
+  resolved "https://registry.yarnpkg.com/ms/-/ms-2.0.0.tgz#5608aeadfc00be6c2901df5f9861788de0d597c8"
+  integrity sha1-VgiurfwAvmwpAd9fmGF4jeDVl8g=
diff --git a/test/lib/fixtures/peer-deps/yarn2/expected-tree.json b/test/lib/fixtures/peer-deps/yarn2/expected-tree.json
@@ -0,0 +1,34 @@
+{
+  "dependencies": {
+    "adm-zip": {
+      "labels": {
+        "scope": "prod"
+      },
+      "name": "adm-zip",
+      "version": "0.4.7"
+    },
+    "debug": {
+      "labels": {
+        "scope": "prod"
+      },
+      "name": "debug",
+      "version": "2.6.9",
+      "dependencies": {
+        "ms": {
+          "labels": {
+            "scope": "prod"
+          },
+          "name": "ms",
+          "version": "2.0.0"
+        }
+      }
+    }
+  },
+  "hasDevDependencies": false,
+  "name": "goof",
+  "size": 4,
+  "version": "0.0.3",
+  "meta": {
+    "packageManagerVersion": "2"
+  }
+}
diff --git a/test/lib/fixtures/peer-deps/yarn2/package.json b/test/lib/fixtures/peer-deps/yarn2/package.json
@@ -0,0 +1,13 @@
+{
+  "name": "goof",
+  "version": "0.0.3",
+  "dependencies": {
+    "adm-zip": "0.4.7"
+  },
+  "peerDependencies": {
+    "has": "^1.0.0"
+  },
+  "optionalDependencies": {
+    "debug": "^2.2.0"
+  }
+}
diff --git a/test/lib/fixtures/peer-deps/yarn2/yarn.lock b/test/lib/fixtures/peer-deps/yarn2/yarn.lock
@@ -0,0 +1,43 @@
+# This file is generated by running "yarn install" inside your project.
+# Manual changes might be lost - proceed with caution!
+
+__metadata:
+  version: 4
+  cacheKey: 7
+
+"adm-zip@npm:0.4.7":
+  version: 0.4.7
+  resolution: "adm-zip@npm:0.4.7"
+  checksum: 11a1c45712be29358e9e62b8b7111c2af6b13ee8e954443f64b5f6e7e869473219f4601f34537e960cf98968b9e7d46a376abb650c4095adcbe402b98a1c8af0
+  languageName: node
+  linkType: hard
+
+"debug@npm:^2.2.0":
+  version: 2.6.9
+  resolution: "debug@npm:2.6.9"
+  dependencies:
+    ms: 2.0.0
+  checksum: 559f44f98cf25e2ee489022aec173afbff746564cb108c4493becb95bc3c017a67bdaa25a0ff64801fd32c35051d00af0e56cc7f762ae2c3bc089496e5a1c31b
+  languageName: node
+  linkType: hard
+
+"goof@workspace:.":
+  version: 0.0.0-use.local
+  resolution: "goof@workspace:."
+  dependencies:
+    adm-zip: 0.4.7
+    debug: ^2.2.0
+  peerDependencies:
+    has: ^1.0.0
+  dependenciesMeta:
+    debug:
+      optional: true
+  languageName: unknown
+  linkType: soft
+
+"ms@npm:2.0.0":
+  version: 2.0.0
+  resolution: "ms@npm:2.0.0"
+  checksum: 1a230340cc7f322fbe916783d8c8d60455407c6b7fb7f901d6ee34eb272402302c5c7f070a97b8531245cbb4ca6a0a623f6a128d7e5a5440cefa2c669c0b35bb
+  languageName: node
+  linkType: hard
diff --git a/test/lib/package-lock-depTree.test.ts b/test/lib/package-lock-depTree.test.ts
@@ -265,6 +265,21 @@ test('`package.json` with peer and optionals (npm6)', async (t) => {
 
   t.deepEqual(depTree, expectedDepTree, 'Tree generated as expected');
 });
+
+test('`package.json` with peer and optionals (npm6), strict = false', async (t) => {
+  const expectedDepTree = load('peer-deps/npm6/expected-tree.json');
+
+  const depTree = await buildDepTreeFromFiles(
+    `${__dirname}/fixtures/peer-deps/npm6`,
+    'package.json',
+    'package-lock.json',
+    false,
+    false,
+  );
+
+  t.deepEqual(depTree, expectedDepTree, 'Tree generated as expected');
+});
+
 test('`package.json` with peer and optionals (npm7)', async (t) => {
   const expectedDepTree = load('peer-deps/npm7/expected-tree.json');
 
@@ -277,6 +292,44 @@ test('`package.json` with peer and optionals (npm7)', async (t) => {
   t.deepEqual(depTree, expectedDepTree, 'Tree generated as expected');
 });
 
+test('`package.json` with peer and optionals (npm7), strict = false', async (t) => {
+  const expectedDepTree = load('peer-deps/npm7/expected-tree.json');
+
+  const depTree = await buildDepTreeFromFiles(
+    `${__dirname}/fixtures/peer-deps/npm7`,
+    'package.json',
+    'package-lock.json',
+    false,
+    false,
+  );
+
+  t.deepEqual(depTree, expectedDepTree, 'Tree generated as expected');
+});
+
+test('`package.json` with peer and optionals (yarn)', async (t) => {
+  const expectedDepTree = load('peer-deps/yarn/expected-tree.json');
+
+  const depTree = await buildDepTreeFromFiles(
+    `${__dirname}/fixtures/peer-deps/yarn`,
+    'package.json',
+    'yarn.lock',
+  );
+
+  t.deepEqual(depTree, expectedDepTree, 'Tree generated as expected');
+});
+
+test('`package.json` with peer and optionals (yarn2)', async (t) => {
+  const expectedDepTree = load('peer-deps/yarn2/expected-tree.json');
+
+  const depTree = await buildDepTreeFromFiles(
+    `${__dirname}/fixtures/peer-deps/yarn2`,
+    'package.json',
+    'yarn.lock',
+  );
+
+  t.deepEqual(depTree, expectedDepTree, 'Tree generated as expected');
+});
+
 test('Npm Tree size exceeds the allowed limit of 500 dependencies.', async (t) => {
   config.NPM_TREE_SIZE_LIMIT = 500;
   t.rejects(