From 17790c127b4973738c9ad03240f151eaf7cd7d8c Mon Sep 17 00:00:00 2001 From: Fernando Rojo Date: Thu, 19 Dec 2024 14:59:12 -0800 Subject: [PATCH 1/2] Do not register local and link components in pnpm9 detector --- .../PnpmParsingUtilitiesBase.cs | 9 ++- .../PnpmV5ParsingUtilities.cs | 2 +- .../PnpmV6ParsingUtilities.cs | 8 ++- .../PnpmV9ParsingUtilities.cs | 19 ++++- .../pnpm/Pnpm9Detector.cs | 27 ++++---- .../PnpmDetectorTests.cs | 69 +++++++++++++++++++ 6 files changed, 117 insertions(+), 17 deletions(-) diff --git a/src/Microsoft.ComponentDetection.Detectors/pnpm/ParsingUtilities/PnpmParsingUtilitiesBase.cs b/src/Microsoft.ComponentDetection.Detectors/pnpm/ParsingUtilities/PnpmParsingUtilitiesBase.cs index fbda4d29..d25355c0 100644 --- a/src/Microsoft.ComponentDetection.Detectors/pnpm/ParsingUtilities/PnpmParsingUtilitiesBase.cs +++ b/src/Microsoft.ComponentDetection.Detectors/pnpm/ParsingUtilities/PnpmParsingUtilitiesBase.cs @@ -32,12 +32,19 @@ public bool IsLocalDependency(KeyValuePair dependency) } /// - /// 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". /// /// a pnpm path of the form "/package-name/version". /// Data parsed from path. public abstract DetectedComponent CreateDetectedComponentFromPnpmPath(string pnpmPackagePath); + /// + /// Parse a pnpm path of the form "/package-name/version into a packageName and Version. + /// + /// a pnpm path of the form "/package-name/version". + /// Data parsed from path. + public abstract (string FullPackageName, string PackageVersion) ExtractNameAndVersionFromPnpmPackagePath(string pnpmPackagePath); + public virtual string ReconstructPnpmDependencyPath(string dependencyName, string dependencyVersion) { if (dependencyVersion.StartsWith('/')) diff --git a/src/Microsoft.ComponentDetection.Detectors/pnpm/ParsingUtilities/PnpmV5ParsingUtilities.cs b/src/Microsoft.ComponentDetection.Detectors/pnpm/ParsingUtilities/PnpmV5ParsingUtilities.cs index 02015c28..c94c17cb 100644 --- a/src/Microsoft.ComponentDetection.Detectors/pnpm/ParsingUtilities/PnpmV5ParsingUtilities.cs +++ b/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); diff --git a/src/Microsoft.ComponentDetection.Detectors/pnpm/ParsingUtilities/PnpmV6ParsingUtilities.cs b/src/Microsoft.ComponentDetection.Detectors/pnpm/ParsingUtilities/PnpmV6ParsingUtilities.cs index cd83e408..d4cfe2e7 100644 --- a/src/Microsoft.ComponentDetection.Detectors/pnpm/ParsingUtilities/PnpmV6ParsingUtilities.cs +++ b/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: /webpack-cli@4.10.0(webpack-bundle-analyzer@4.10.1)(webpack-dev-server@4.6.0)(webpack@5.89.0) + 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); } } diff --git a/src/Microsoft.ComponentDetection.Detectors/pnpm/ParsingUtilities/PnpmV9ParsingUtilities.cs b/src/Microsoft.ComponentDetection.Detectors/pnpm/ParsingUtilities/PnpmV9ParsingUtilities.cs index 3270fdc0..5f2918a3 100644 --- a/src/Microsoft.ComponentDetection.Detectors/pnpm/ParsingUtilities/PnpmV9ParsingUtilities.cs +++ b/src/Microsoft.ComponentDetection.Detectors/pnpm/ParsingUtilities/PnpmV9ParsingUtilities.cs @@ -7,6 +7,23 @@ public class PnpmV9ParsingUtilities : PnpmParsingUtilitiesBase 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: /webpack-cli@4.10.0(webpack-bundle-analyzer@4.10.1)(webpack-dev-server@4.6.0)(webpack@5.89.0) + (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); } /// diff --git a/src/Microsoft.ComponentDetection.Detectors/pnpm/Pnpm9Detector.cs b/src/Microsoft.ComponentDetection.Detectors/pnpm/Pnpm9Detector.cs index 82e1093a..efb10589 100644 --- a/src/Microsoft.ComponentDetection.Detectors/pnpm/Pnpm9Detector.cs +++ b/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 = packageVersion.StartsWith(PnpmConstants.PnpmLinkDependencyPath) || packageVersion.StartsWith(PnpmConstants.PnpmFileDependencyPath); 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,20 +71,20 @@ private void ProcessDependencyList(ISingleFileComponentRecorder singleFileCompon { foreach (var (name, dep) in dependencies ?? Enumerable.Empty>()) { - // 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 = dep.Version.StartsWith(PnpmConstants.PnpmLinkDependencyPath) || dep.Version.StartsWith(PnpmConstants.PnpmFileDependencyPath); + if (!isFileOrLink) + { + singleFileComponentRecorder.RegisterUsage(component, isExplicitReferencedDependency: true, isDevelopmentDependency: isDevelopmentDependency); + } + var seenDependencies = new HashSet(); - this.ProcessIndirectDependencies(singleFileComponentRecorder, components, component.Component.Id, package.Dependencies, isDevelopmentDependency, seenDependencies); + this.ProcessIndirectDependencies(singleFileComponentRecorder, components, isFileOrLink ? null : component.Component.Id, package.Dependencies, isDevelopmentDependency, seenDependencies); } } diff --git a/test/Microsoft.ComponentDetection.Detectors.Tests/PnpmDetectorTests.cs b/test/Microsoft.ComponentDetection.Detectors.Tests/PnpmDetectorTests.cs index e4c60c07..01500d13 100644 --- a/test/Microsoft.ComponentDetection.Detectors.Tests/PnpmDetectorTests.cs +++ b/test/Microsoft.ComponentDetection.Detectors.Tests/PnpmDetectorTests.cs @@ -783,6 +783,75 @@ 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: + sampleDependency@1.1.1: + resolution: {integrity: sha512-zRpUiDwd/xk6ADqPMATG8vc9VPrkck7T07OIx0gnjmJAnHnTVXNQG3vfvWNuiZIkwu9KrKdA1iJKfsfTVxE6NA==} + sampleIndirectDependency2@2.2.2: + resolution: {integrity: sha512-FQN4MRfuJeHf7cBbBMJFXhKSDq+2kAArBlmRBvcvFE5BB1HZKXtSFASDhdlz9zOYwxh8lDdnvmMOe/+5cdoEdg==} + engines: {node: '>= 0.8'} + sampleIndirectDependency@3.3.3: + resolution: {integrity: sha512-ZySD7Nf91aLB0RxL4KGrKHBXl7Eds1DAmEdcoVawXnLD7SDhpNgtuII2aAkg7a7QS41jxPSZ17p4VdGnMHk3MQ==} + engines: {node: '>=0.4.0'} + +snapshots: + sampleDependency@1.1.1: + dependencies: + sampleIndirectDependency: 3.3.3 + sampleIndirectDependency2: 2.2.2 + 'file://../sampleFile': 'link:../\\' + sampleIndirectDependency2@2.2.2: {} + sampleIndirectDependency@3.3.3: {} + 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( + indirectDependencyComponent.Component.Id, + parentComponent => parentComponent.Name == "sampleDependency"); + componentRecorder.AssertAllExplicitlyReferencedComponents( + indirectDependencyComponent2.Component.Id, + parentComponent => parentComponent.Name == "sampleDependency"); + } + [TestMethod] public async Task TestPnpmDetector_V9_GoodLockVersion_MissingSnapshotsSuccess() { From 5e4b41ad47d736069b50ed8847a5d2898b75d93d Mon Sep 17 00:00:00 2001 From: Fernando Rojo Date: Thu, 19 Dec 2024 15:43:13 -0800 Subject: [PATCH 2/2] Add filters for http and https --- .../pnpm/IPnpmDetector.cs | 2 + .../pnpm/Pnpm9Detector.cs | 12 +++- .../PnpmDetectorTests.cs | 69 +++++++++++++++++++ 3 files changed, 81 insertions(+), 2 deletions(-) diff --git a/src/Microsoft.ComponentDetection.Detectors/pnpm/IPnpmDetector.cs b/src/Microsoft.ComponentDetection.Detectors/pnpm/IPnpmDetector.cs index 7557c76b..cc1b96f2 100644 --- a/src/Microsoft.ComponentDetection.Detectors/pnpm/IPnpmDetector.cs +++ b/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:"; } diff --git a/src/Microsoft.ComponentDetection.Detectors/pnpm/Pnpm9Detector.cs b/src/Microsoft.ComponentDetection.Detectors/pnpm/Pnpm9Detector.cs index efb10589..a5903ab6 100644 --- a/src/Microsoft.ComponentDetection.Detectors/pnpm/Pnpm9Detector.cs +++ b/src/Microsoft.ComponentDetection.Detectors/pnpm/Pnpm9Detector.cs @@ -31,7 +31,7 @@ public void RecordDependencyGraphFromFile(string yamlFileContent, ISingleFileCom // 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. var (packageName, packageVersion) = this.pnpmParsingUtilities.ExtractNameAndVersionFromPnpmPackagePath(pnpmDependencyPath); - var isFileOrLink = packageVersion.StartsWith(PnpmConstants.PnpmLinkDependencyPath) || packageVersion.StartsWith(PnpmConstants.PnpmFileDependencyPath); + var isFileOrLink = this.IsFileOrLink(packageVersion) || this.IsFileOrLink(pnpmDependencyPath); var dependencyPath = pnpmDependencyPath; if (pnpmDependencyPath.StartsWith('/')) @@ -77,7 +77,7 @@ private void ProcessDependencyList(ISingleFileComponentRecorder singleFileCompon // 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. // Ignore "file:" and "link:" as these are local packages. - var isFileOrLink = dep.Version.StartsWith(PnpmConstants.PnpmLinkDependencyPath) || dep.Version.StartsWith(PnpmConstants.PnpmFileDependencyPath); + var isFileOrLink = this.IsFileOrLink(dep.Version); if (!isFileOrLink) { singleFileComponentRecorder.RegisterUsage(component, isExplicitReferencedDependency: true, isDevelopmentDependency: isDevelopmentDependency); @@ -88,6 +88,14 @@ private void ProcessDependencyList(ISingleFileComponentRecorder singleFileCompon } } + 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 components, string parentComponentId, Dictionary dependencies, bool isDevDependency, HashSet seenDependencies) { // Now that the `components` dictionary is populated, make another pass of all components, registering all the dependency edges in the graph. diff --git a/test/Microsoft.ComponentDetection.Detectors.Tests/PnpmDetectorTests.cs b/test/Microsoft.ComponentDetection.Detectors.Tests/PnpmDetectorTests.cs index 01500d13..3f212e96 100644 --- a/test/Microsoft.ComponentDetection.Detectors.Tests/PnpmDetectorTests.cs +++ b/test/Microsoft.ComponentDetection.Detectors.Tests/PnpmDetectorTests.cs @@ -852,6 +852,75 @@ public async Task TestPnpmDetector_V9_GoodLockVersion_FileAndLinkDependenciesAre 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: + sampleDependency@1.1.1: + resolution: {integrity: sha512-zRpUiDwd/xk6ADqPMATG8vc9VPrkck7T07OIx0gnjmJAnHnTVXNQG3vfvWNuiZIkwu9KrKdA1iJKfsfTVxE6NA==} + sampleIndirectDependency2@2.2.2: + resolution: {integrity: sha512-FQN4MRfuJeHf7cBbBMJFXhKSDq+2kAArBlmRBvcvFE5BB1HZKXtSFASDhdlz9zOYwxh8lDdnvmMOe/+5cdoEdg==} + engines: {node: '>= 0.8'} + sampleIndirectDependency@3.3.3: + resolution: {integrity: sha512-ZySD7Nf91aLB0RxL4KGrKHBXl7Eds1DAmEdcoVawXnLD7SDhpNgtuII2aAkg7a7QS41jxPSZ17p4VdGnMHk3MQ==} + engines: {node: '>=0.4.0'} + +snapshots: + sampleDependency@1.1.1: + dependencies: + sampleIndirectDependency: 3.3.3 + sampleIndirectDependency2: 2.2.2 + 'file://../sampleFile': 'link:../\\' + sampleIndirectDependency2@2.2.2: {} + sampleIndirectDependency@3.3.3: {} + 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( + indirectDependencyComponent.Component.Id, + parentComponent => parentComponent.Name == "sampleDependency"); + componentRecorder.AssertAllExplicitlyReferencedComponents( + indirectDependencyComponent2.Component.Id, + parentComponent => parentComponent.Name == "sampleDependency"); + } + [TestMethod] public async Task TestPnpmDetector_V9_GoodLockVersion_MissingSnapshotsSuccess() {