Do not register local and link components in pnpm9 detector (#1331)

* Do not register local and link components in pnpm9 detector

* Add filters for http and https

src/Microsoft.ComponentDetection.Detectors/pnpm/IPnpmDetector.cs

@@ -23,4 +23,6 @@ public static class PnpmConstants
     public const string PnpmFileDependencyPath = "file:";
 
     public const string PnpmLinkDependencyPath = "link:";
+    public const string PnpmHttpDependencyPath = "http:";
+    public const string PnpmHttpsDependencyPath = "https:";
 }

src/Microsoft.ComponentDetection.Detectors/pnpm/ParsingUtilities/PnpmParsingUtilitiesBase.cs

@@ -32,12 +32,19 @@ public bool IsLocalDependency(KeyValuePair<string, string> dependency)
     }
 
     /// <summary>
-    /// Parse a pnpm path of the form "/package-name/version".
+    /// Parse a pnpm path of the form "/package-name/version and create an npm component".
     /// </summary>
     /// <param name="pnpmPackagePath">a pnpm path of the form "/package-name/version".</param>
     /// <returns>Data parsed from path.</returns>
     public abstract DetectedComponent CreateDetectedComponentFromPnpmPath(string pnpmPackagePath);
 
+    /// <summary>
+    /// Parse a pnpm path of the form "/package-name/version into a packageName and Version.
+    /// </summary>
+    /// <param name="pnpmPackagePath">a pnpm path of the form "/package-name/version".</param>
+    /// <returns>Data parsed from path.</returns>
+    public abstract (string FullPackageName, string PackageVersion) ExtractNameAndVersionFromPnpmPackagePath(string pnpmPackagePath);
+
     public virtual string ReconstructPnpmDependencyPath(string dependencyName, string dependencyVersion)
     {
         if (dependencyVersion.StartsWith('/'))

src/Microsoft.ComponentDetection.Detectors/pnpm/ParsingUtilities/PnpmV5ParsingUtilities.cs

@@ -14,7 +14,7 @@ public override DetectedComponent CreateDetectedComponentFromPnpmPath(string pnp
         return new DetectedComponent(new NpmComponent(parentName, parentVersion));
     }
 
-    private (string Name, string Version) ExtractNameAndVersionFromPnpmPackagePath(string pnpmPackagePath)
+    public override (string FullPackageName, string PackageVersion) ExtractNameAndVersionFromPnpmPackagePath(string pnpmPackagePath)
     {
         var pnpmComponentDefSections = pnpmPackagePath.Trim('/').Split('/');
         (var packageVersion, var indexVersionIsAt) = this.GetPackageVersion(pnpmComponentDefSections);

src/Microsoft.ComponentDetection.Detectors/pnpm/ParsingUtilities/PnpmV6ParsingUtilities.cs

@@ -17,6 +17,12 @@ public override DetectedComponent CreateDetectedComponentFromPnpmPath(string pnp
 
         // Strip parenthesized suffices from package. These hold peed dep related information that is unneeded here.
         // An example of a dependency path with these: /[email protected]([email protected])([email protected])([email protected])
+        var (normalizedPackageName, packageVersion) = this.ExtractNameAndVersionFromPnpmPackagePath(pnpmPackagePath);
+        return new DetectedComponent(new NpmComponent(normalizedPackageName, packageVersion));
+    }
+
+    public override (string FullPackageName, string PackageVersion) ExtractNameAndVersionFromPnpmPackagePath(string pnpmPackagePath)
+    {
         var fullPackageNameAndVersion = pnpmPackagePath.Split("(")[0];
 
         var packageNameParts = fullPackageNameAndVersion.Split("@");
@@ -37,6 +43,6 @@ public override DetectedComponent CreateDetectedComponentFromPnpmPath(string pnp
         // It is unclear if real packages could have a name starting with `/`, so avoid `TrimStart` that just in case.
         var normalizedPackageName = fullPackageName[1..];
 
-        return new DetectedComponent(new NpmComponent(normalizedPackageName, packageVersion));
+        return (normalizedPackageName, packageVersion);
     }
 }

src/Microsoft.ComponentDetection.Detectors/pnpm/ParsingUtilities/PnpmV9ParsingUtilities.cs

@@ -7,6 +7,23 @@ public class PnpmV9ParsingUtilities<T> : PnpmParsingUtilitiesBase<T>
 where T : PnpmYaml
 {
     public override DetectedComponent CreateDetectedComponentFromPnpmPath(string pnpmPackagePath)
+    {
+        /*
+         * The format is documented at https://github.com/pnpm/spec/blob/master/dependency-path.md.
+         * At the writing it does not seem to reflect changes which were made in lock file format v9:
+         * See https://github.com/pnpm/spec/issues/5.
+         * In general, the spec sheet for the v9 lockfile is not published, so parsing of this lockfile was emperically determined.
+         * see https://github.com/pnpm/spec/issues/6
+         */
+
+        // Strip parenthesized suffices from package. These hold peed dep related information that is unneeded here.
+        // An example of a dependency path with these: /[email protected]([email protected])([email protected])([email protected])
+        (var fullPackageName, var packageVersion) = this.ExtractNameAndVersionFromPnpmPackagePath(pnpmPackagePath);
+
+        return new DetectedComponent(new NpmComponent(fullPackageName, packageVersion));
+    }
+
+    public override (string FullPackageName, string PackageVersion) ExtractNameAndVersionFromPnpmPackagePath(string pnpmPackagePath)
     {
         /*
          * The format is documented at https://github.com/pnpm/spec/blob/master/dependency-path.md.
@@ -28,7 +45,7 @@ public override DetectedComponent CreateDetectedComponentFromPnpmPath(string pnp
         // Version is section after last `@`.
         var packageVersion = packageNameParts[^1];
 
-        return new DetectedComponent(new NpmComponent(fullPackageName, packageVersion));
+        return (fullPackageName, packageVersion);
     }
 
     /// <summary>

src/Microsoft.ComponentDetection.Detectors/pnpm/Pnpm9Detector.cs

@@ -30,10 +30,8 @@ public void RecordDependencyGraphFromFile(string yamlFileContent, ISingleFileCom
             // Ignore "file:" as these are local packages.
             // Such local packages should only be referenced at the top level (via ProcessDependencyList) which also skips them or from other local packages (which this skips).
             // There should be no cases where a non-local package references a local package, so skipping them here should not result in failed lookups below when adding all the graph references.
-            if (pnpmDependencyPath.StartsWith(PnpmConstants.PnpmFileDependencyPath))
-            {
-                continue;
-            }
+            var (packageName, packageVersion) = this.pnpmParsingUtilities.ExtractNameAndVersionFromPnpmPackagePath(pnpmDependencyPath);
+            var isFileOrLink = this.IsFileOrLink(packageVersion) || this.IsFileOrLink(pnpmDependencyPath);
 
             var dependencyPath = pnpmDependencyPath;
             if (pnpmDependencyPath.StartsWith('/'))
@@ -48,7 +46,10 @@ public void RecordDependencyGraphFromFile(string yamlFileContent, ISingleFileCom
             // It should get registered again with with additional information (what depended on it) later,
             // but registering it now ensures nothing is missed due to a limitation in dependency traversal
             // like skipping local dependencies which might have transitively depended on this.
-            singleFileComponentRecorder.RegisterUsage(parentDetectedComponent);
+            if (!isFileOrLink)
+            {
+                singleFileComponentRecorder.RegisterUsage(parentDetectedComponent);
+            }
         }
 
         // now that the components dictionary is populated, add direct dependencies of the current file/project setting isExplicitReferencedDependency to true
@@ -70,23 +71,31 @@ private void ProcessDependencyList(ISingleFileComponentRecorder singleFileCompon
     {
         foreach (var (name, dep) in dependencies ?? Enumerable.Empty<KeyValuePair<string, PnpmYamlV9Dependency>>())
         {
-            // Ignore "file:" and "link:" as these are local packages.
-            if (dep.Version.StartsWith(PnpmConstants.PnpmLinkDependencyPath) || dep.Version.StartsWith(PnpmConstants.PnpmFileDependencyPath))
-            {
-                continue;
-            }
-
             var pnpmDependencyPath = this.pnpmParsingUtilities.ReconstructPnpmDependencyPath(name, dep.Version);
             var (component, package) = components[pnpmDependencyPath];
 
             // Lockfile v9 apparently removed the tagging of dev dependencies in the lockfile, so we revert to using the dependency tree to establish dev dependency state.
             // At this point, the root dependencies are marked according to which dependency group they are declared in the lockfile itself.
-            singleFileComponentRecorder.RegisterUsage(component, isExplicitReferencedDependency: true, isDevelopmentDependency: isDevelopmentDependency);
+            // Ignore "file:" and "link:" as these are local packages.
+            var isFileOrLink = this.IsFileOrLink(dep.Version);
+            if (!isFileOrLink)
+            {
+                singleFileComponentRecorder.RegisterUsage(component, isExplicitReferencedDependency: true, isDevelopmentDependency: isDevelopmentDependency);
+            }
+
             var seenDependencies = new HashSet<string>();
-            this.ProcessIndirectDependencies(singleFileComponentRecorder, components, component.Component.Id, package.Dependencies, isDevelopmentDependency, seenDependencies);
+            this.ProcessIndirectDependencies(singleFileComponentRecorder, components, isFileOrLink ? null : component.Component.Id, package.Dependencies, isDevelopmentDependency, seenDependencies);
         }
     }
 
+    private bool IsFileOrLink(string packagePath)
+    {
+        return packagePath.StartsWith(PnpmConstants.PnpmLinkDependencyPath) ||
+               packagePath.StartsWith(PnpmConstants.PnpmFileDependencyPath) ||
+               packagePath.StartsWith(PnpmConstants.PnpmHttpDependencyPath) ||
+               packagePath.StartsWith(PnpmConstants.PnpmHttpsDependencyPath);
+    }
+
     private void ProcessIndirectDependencies(ISingleFileComponentRecorder singleFileComponentRecorder, Dictionary<string, (DetectedComponent C, Package P)> components, string parentComponentId, Dictionary<string, string> dependencies, bool isDevDependency, HashSet<string> seenDependencies)
     {
         // Now that the `components` dictionary is populated, make another pass of all components, registering all the dependency edges in the graph.

test/Microsoft.ComponentDetection.Detectors.Tests/PnpmDetectorTests.cs

@@ -783,6 +783,144 @@ public async Task TestPnpmDetector_V9_GoodLockVersion_SkipsFileAndLinkDependenci
             parentComponent => parentComponent.Name == "sampleDependency");
     }
 
+    [TestMethod]
+    public async Task TestPnpmDetector_V9_GoodLockVersion_FileAndLinkDependenciesAreNotRegistered()
+    {
+        var yamlFile = @"
+lockfileVersion: '9.0'
+settings:
+  autoInstallPeers: true
+  excludeLinksFromLockfile: false
+importers:
+  .:
+    dependencies:
+      sampleDependency:
+        specifier: ^1.1.1
+        version: 1.1.1
+      sampleFileDependency:
+        specifier: file:../test
+        version: file:../test
+      SampleLinkDependency:
+        specifier: workspace:*
+        version: link:SampleLinkDependency
+packages:
+  [email protected]:
+    resolution: {integrity: sha512-zRpUiDwd/xk6ADqPMATG8vc9VPrkck7T07OIx0gnjmJAnHnTVXNQG3vfvWNuiZIkwu9KrKdA1iJKfsfTVxE6NA==}
+  [email protected]:
+    resolution: {integrity: sha512-FQN4MRfuJeHf7cBbBMJFXhKSDq+2kAArBlmRBvcvFE5BB1HZKXtSFASDhdlz9zOYwxh8lDdnvmMOe/+5cdoEdg==}
+    engines: {node: '>= 0.8'}
+  [email protected]:
+    resolution: {integrity: sha512-ZySD7Nf91aLB0RxL4KGrKHBXl7Eds1DAmEdcoVawXnLD7SDhpNgtuII2aAkg7a7QS41jxPSZ17p4VdGnMHk3MQ==}
+    engines: {node: '>=0.4.0'}
+
+snapshots:
+  [email protected]:
+    dependencies:
+      sampleIndirectDependency: 3.3.3
+      sampleIndirectDependency2: 2.2.2
+      'file://../sampleFile':  'link:../\\'
+  [email protected]: {}
+  [email protected]: {}
+  sampleFileDependency@file:../test': {}
+";
+
+        var (scanResult, componentRecorder) = await this.DetectorTestUtility
+            .WithFile("pnpm-lock.yaml", yamlFile)
+            .ExecuteDetectorAsync();
+
+        scanResult.ResultCode.Should().Be(ProcessingResultCode.Success);
+
+        var detectedComponents = componentRecorder.GetDetectedComponents();
+        detectedComponents.Should().HaveCount(3);
+        var npmComponents = detectedComponents.Select(x => new { Component = x.Component as NpmComponent, DetectedComponent = x });
+        npmComponents.Should().Contain(x => x.Component.Name == "sampleDependency" && x.Component.Version == "1.1.1");
+        npmComponents.Should().Contain(x => x.Component.Name == "sampleIndirectDependency2" && x.Component.Version == "2.2.2");
+        npmComponents.Should().Contain(x => x.Component.Name == "sampleIndirectDependency" && x.Component.Version == "3.3.3");
+
+        var noDevDependencyComponent = npmComponents.First(x => x.Component.Name == "sampleDependency");
+        var indirectDependencyComponent2 = npmComponents.First(x => x.Component.Name == "sampleIndirectDependency2");
+        var indirectDependencyComponent = npmComponents.First(x => x.Component.Name == "sampleIndirectDependency");
+
+        componentRecorder.GetEffectiveDevDependencyValue(noDevDependencyComponent.Component.Id).Should().BeFalse();
+        componentRecorder.GetEffectiveDevDependencyValue(indirectDependencyComponent2.Component.Id).Should().BeFalse();
+        componentRecorder.GetEffectiveDevDependencyValue(indirectDependencyComponent.Component.Id).Should().BeFalse();
+        componentRecorder.AssertAllExplicitlyReferencedComponents<NpmComponent>(
+            indirectDependencyComponent.Component.Id,
+            parentComponent => parentComponent.Name == "sampleDependency");
+        componentRecorder.AssertAllExplicitlyReferencedComponents<NpmComponent>(
+            indirectDependencyComponent2.Component.Id,
+            parentComponent => parentComponent.Name == "sampleDependency");
+    }
+
+    [TestMethod]
+    public async Task TestPnpmDetector_V9_GoodLockVersion_HttpDependenciesAreNotRegistered()
+    {
+        var yamlFile = @"
+lockfileVersion: '9.0'
+settings:
+  autoInstallPeers: true
+  excludeLinksFromLockfile: false
+importers:
+  .:
+    dependencies:
+      sampleDependency:
+        specifier: ^1.1.1
+        version: 1.1.1
+      sampleHttpDependency:
+        specifier: https://samplePackage/tar.gz/32f550d3b3bdb1b781aabe100683311cd982c98e
+        version: sample@https://samplePackage/tar.gz/32f550d3b3bdb1b781aabe100683311cd982c98e
+      SampleLinkDependency:
+        specifier: workspace:*
+        version: link:SampleLinkDependency
+packages:
+  [email protected]:
+    resolution: {integrity: sha512-zRpUiDwd/xk6ADqPMATG8vc9VPrkck7T07OIx0gnjmJAnHnTVXNQG3vfvWNuiZIkwu9KrKdA1iJKfsfTVxE6NA==}
+  [email protected]:
+    resolution: {integrity: sha512-FQN4MRfuJeHf7cBbBMJFXhKSDq+2kAArBlmRBvcvFE5BB1HZKXtSFASDhdlz9zOYwxh8lDdnvmMOe/+5cdoEdg==}
+    engines: {node: '>= 0.8'}
+  [email protected]:
+    resolution: {integrity: sha512-ZySD7Nf91aLB0RxL4KGrKHBXl7Eds1DAmEdcoVawXnLD7SDhpNgtuII2aAkg7a7QS41jxPSZ17p4VdGnMHk3MQ==}
+    engines: {node: '>=0.4.0'}
+
+snapshots:
+  [email protected]:
+    dependencies:
+      sampleIndirectDependency: 3.3.3
+      sampleIndirectDependency2: 2.2.2
+      'file://../sampleFile':  'link:../\\'
+  [email protected]: {}
+  [email protected]: {}
+  sampleHttpDependency@https://samplePackage/tar.gz/32f550d3b3bdb1b781aabe100683311cd982c98e': {}
+";
+
+        var (scanResult, componentRecorder) = await this.DetectorTestUtility
+            .WithFile("pnpm-lock.yaml", yamlFile)
+            .ExecuteDetectorAsync();
+
+        scanResult.ResultCode.Should().Be(ProcessingResultCode.Success);
+
+        var detectedComponents = componentRecorder.GetDetectedComponents();
+        detectedComponents.Should().HaveCount(3);
+        var npmComponents = detectedComponents.Select(x => new { Component = x.Component as NpmComponent, DetectedComponent = x });
+        npmComponents.Should().Contain(x => x.Component.Name == "sampleDependency" && x.Component.Version == "1.1.1");
+        npmComponents.Should().Contain(x => x.Component.Name == "sampleIndirectDependency2" && x.Component.Version == "2.2.2");
+        npmComponents.Should().Contain(x => x.Component.Name == "sampleIndirectDependency" && x.Component.Version == "3.3.3");
+
+        var noDevDependencyComponent = npmComponents.First(x => x.Component.Name == "sampleDependency");
+        var indirectDependencyComponent2 = npmComponents.First(x => x.Component.Name == "sampleIndirectDependency2");
+        var indirectDependencyComponent = npmComponents.First(x => x.Component.Name == "sampleIndirectDependency");
+
+        componentRecorder.GetEffectiveDevDependencyValue(noDevDependencyComponent.Component.Id).Should().BeFalse();
+        componentRecorder.GetEffectiveDevDependencyValue(indirectDependencyComponent2.Component.Id).Should().BeFalse();
+        componentRecorder.GetEffectiveDevDependencyValue(indirectDependencyComponent.Component.Id).Should().BeFalse();
+        componentRecorder.AssertAllExplicitlyReferencedComponents<NpmComponent>(
+            indirectDependencyComponent.Component.Id,
+            parentComponent => parentComponent.Name == "sampleDependency");
+        componentRecorder.AssertAllExplicitlyReferencedComponents<NpmComponent>(
+            indirectDependencyComponent2.Component.Id,
+            parentComponent => parentComponent.Name == "sampleDependency");
+    }
+
     [TestMethod]
     public async Task TestPnpmDetector_V9_GoodLockVersion_MissingSnapshotsSuccess()
     {