Merge pull request #258 from snyk/fix/OSM-2329/handle-peer-deps-in-dep-path

fix: [OSM-2329] handle peers deps in dep path for git protocol deps

Author: gemaxim

lib/dep-graph-builders/pnpm/lockfile-parser/lockfile-parser.ts

@@ -50,6 +50,11 @@ export abstract class PnpmLockfileParser {
         // name and version are optional in version data - if they don't show up in version data, they can be deducted from the dependency path
         const { name, version } = versionData;
         let parsedPath: ParsedDepPath = {};
+
+        // Exclude transitive peer deps from depPath
+        // e.g. '/[email protected]([email protected])([email protected])' -> [email protected]
+        depPath = this.excludeTransPeerDepsVersions(depPath);
+
         if (!(version && name)) {
           parsedPath = this.parseDepPath(depPath);
         }
@@ -123,7 +128,7 @@ export abstract class PnpmLockfileParser {
       ] as PnpmProject;
       this.extractedPackages[`${name}@${version}`] = {
         id: `${name}@${version}`,
-        name: version,
+        name: name,
         version: version,
         dependencies: this.topLevelDepsToNormalizedPkgs(prodDeps),
         devDependencies: this.topLevelDepsToNormalizedPkgs(devDeps),

lib/dep-graph-builders/pnpm/lockfile-parser/lockfile-v5.ts

@@ -69,7 +69,17 @@ export class LockfileV5Parser extends PnpmLockfileParser {
   // '/@babel/preset-typescript/7.12.13_@[email protected]'
   // https://github.com/pnpm/spec/blob/master/dependency-path.md
   public excludeTransPeerDepsVersions(fullVersionStr: string): string {
-    return fullVersionStr.split('_')[0];
+    if (!fullVersionStr.includes('/')) {
+      return fullVersionStr.split('_')[0];
+    }
+    // When dealing with dependency paths, the dependency name can include '_'
+    // so we need to make sure the '_' we split by is after '/'
+    const splitSlashes = fullVersionStr.split('/');
+    const stringAfterLastSlash = splitSlashes[splitSlashes.length - 1];
+    const stringBeforeLastSlash = fullVersionStr.split(stringAfterLastSlash)[0];
+    const stringBetweenLastSlashAndUnderscore =
+      stringAfterLastSlash.split('_')[0];
+    return `${stringBeforeLastSlash}${stringBetweenLastSlashAndUnderscore}`;
   }
 
   // eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types

lib/dep-graph-builders/pnpm/lockfile-parser/lockfile-v6.ts

@@ -11,24 +11,31 @@ export class LockfileV6Parser extends PnpmLockfileParser {
   }
 
   public parseDepPath(depPath: string): ParsedDepPath {
-    // Exclude transitive peer deps from depPath
-    // e.g. '/[email protected]([email protected])([email protected])' -> [email protected]
-    depPath = this.excludeTransPeerDepsVersions(depPath);
-
     // Check if path is absolute (doesn't start with '/')
     // If it's not absolute, omit first '/'
     depPath = LockfileV6Parser.isAbsoluteDepenencyPath(depPath)
       ? depPath
       : depPath.substring(1);
 
-    // Next, get version based on the last occurence of '@' separator
-    // e.g. @babel/[email protected] -> name: @babel/code-frame and version: 7.24.2
-    const sepIndex = depPath.lastIndexOf('@');
-    if (sepIndex === -1) {
-      return {};
+    // This is the case for scoped packages, get version based on the last occurence of '@' separator
+    // to include the '@' in the name
+    if (depPath.startsWith('@')) {
+      // e.g. @babel/[email protected] -> name: @babel/code-frame and version: 7.24.2
+      const sepIndex = depPath.lastIndexOf('@');
+      if (sepIndex === -1) {
+        return {};
+      }
+      const name = depPath.substring(0, sepIndex);
+      const version = depPath.substring(sepIndex + 1);
+      return {
+        name,
+        version,
+      };
     }
-    const name = depPath.substring(0, sepIndex);
-    const version = depPath.substring(sepIndex + 1);
+
+    // For non-scoped packages, the dependency specifier should contain only
+    // one '@', separating the name and version
+    const [name, version] = depPath.split('@');
     return {
       name,
       version,

lib/dep-graph-builders/pnpm/utils.ts

@@ -4,7 +4,6 @@ import { LockfileType } from '../..';
 import { getGraphDependencies } from '../util';
 import { PnpmLockfileParser } from './lockfile-parser/lockfile-parser';
 import { NormalisedPnpmPkgs, PnpmNode } from './types';
-import { valid } from 'semver';
 import { OpenSourceEcosystems } from '@snyk/error-catalog-nodejs-public';
 import {
   INSTALL_COMMAND,
@@ -23,18 +22,14 @@ export const getPnpmChildNode = (
   includeDevDeps: boolean,
   lockfileParser: PnpmLockfileParser,
 ): PnpmNode => {
-  let resolvedVersion =
-    valid(depInfo.version) || depInfo.version === undefined
-      ? depInfo.version
-      : lockfileParser.excludeTransPeerDepsVersions(depInfo.version);
+  const resolvedVersion = lockfileParser.excludeTransPeerDepsVersions(
+    depInfo.version,
+  );
   let childNodeKey = `${name}@${resolvedVersion}`;
   // For aliases, the version is the dependency path that
   // shows up in the packages section of lockfiles
-  if (lockfileParser.resolvedPackages[depInfo.version]) {
-    childNodeKey = lockfileParser.resolvedPackages[depInfo.version];
-    const pkgData = pkgs[childNodeKey];
-    name = pkgData.name;
-    resolvedVersion = pkgData.version;
+  if (lockfileParser.resolvedPackages[resolvedVersion]) {
+    childNodeKey = lockfileParser.resolvedPackages[resolvedVersion];
   }
   if (!pkgs[childNodeKey]) {
     if (strictOutOfSync && !/^file:/.test(depInfo.version)) {
@@ -70,8 +65,8 @@ export const getPnpmChildNode = (
       ? getGraphDependencies(depData.optionalDependencies || {}, depInfo.isDev)
       : {};
     return {
-      id: `${name}@${depData.version}`,
-      name: name,
+      id: `${depData.name}@${depData.version}`,
+      name: depData.name,
       version: depData.version || UNDEFINED_VERSION,
       dependencies: {
         ...dependencies,