Skip to content

Commit 10c85b6

Browse files
FernandoRojoFernandoRojo
authored
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
1 parent 8a744bf commit 10c85b6

File tree

7 files changed

+196
-17
lines changed

7 files changed

+196
-17
lines changed

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,4 +23,6 @@ public static class PnpmConstants
2323
public const string PnpmFileDependencyPath = "file:";
2424

2525
public const string PnpmLinkDependencyPath = "link:";
26+
public const string PnpmHttpDependencyPath = "http:";
27+
public const string PnpmHttpsDependencyPath = "https:";
2628
}

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

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,12 +32,19 @@ public bool IsLocalDependency(KeyValuePair<string, string> dependency)
3232
}
3333

3434
/// <summary>
35-
/// Parse a pnpm path of the form "/package-name/version".
35+
/// Parse a pnpm path of the form "/package-name/version and create an npm component".
3636
/// </summary>
3737
/// <param name="pnpmPackagePath">a pnpm path of the form "/package-name/version".</param>
3838
/// <returns>Data parsed from path.</returns>
3939
public abstract DetectedComponent CreateDetectedComponentFromPnpmPath(string pnpmPackagePath);
4040

41+
/// <summary>
42+
/// Parse a pnpm path of the form "/package-name/version into a packageName and Version.
43+
/// </summary>
44+
/// <param name="pnpmPackagePath">a pnpm path of the form "/package-name/version".</param>
45+
/// <returns>Data parsed from path.</returns>
46+
public abstract (string FullPackageName, string PackageVersion) ExtractNameAndVersionFromPnpmPackagePath(string pnpmPackagePath);
47+
4148
public virtual string ReconstructPnpmDependencyPath(string dependencyName, string dependencyVersion)
4249
{
4350
if (dependencyVersion.StartsWith('/'))

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ public override DetectedComponent CreateDetectedComponentFromPnpmPath(string pnp
1414
return new DetectedComponent(new NpmComponent(parentName, parentVersion));
1515
}
1616

17-
private (string Name, string Version) ExtractNameAndVersionFromPnpmPackagePath(string pnpmPackagePath)
17+
public override (string FullPackageName, string PackageVersion) ExtractNameAndVersionFromPnpmPackagePath(string pnpmPackagePath)
1818
{
1919
var pnpmComponentDefSections = pnpmPackagePath.Trim('/').Split('/');
2020
(var packageVersion, var indexVersionIsAt) = this.GetPackageVersion(pnpmComponentDefSections);

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

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,12 @@ public override DetectedComponent CreateDetectedComponentFromPnpmPath(string pnp
1717

1818
// Strip parenthesized suffices from package. These hold peed dep related information that is unneeded here.
1919
// An example of a dependency path with these: /[email protected]([email protected])([email protected])([email protected])
20+
var (normalizedPackageName, packageVersion) = this.ExtractNameAndVersionFromPnpmPackagePath(pnpmPackagePath);
21+
return new DetectedComponent(new NpmComponent(normalizedPackageName, packageVersion));
22+
}
23+
24+
public override (string FullPackageName, string PackageVersion) ExtractNameAndVersionFromPnpmPackagePath(string pnpmPackagePath)
25+
{
2026
var fullPackageNameAndVersion = pnpmPackagePath.Split("(")[0];
2127

2228
var packageNameParts = fullPackageNameAndVersion.Split("@");
@@ -37,6 +43,6 @@ public override DetectedComponent CreateDetectedComponentFromPnpmPath(string pnp
3743
// It is unclear if real packages could have a name starting with `/`, so avoid `TrimStart` that just in case.
3844
var normalizedPackageName = fullPackageName[1..];
3945

40-
return new DetectedComponent(new NpmComponent(normalizedPackageName, packageVersion));
46+
return (normalizedPackageName, packageVersion);
4147
}
4248
}

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

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,23 @@ public class PnpmV9ParsingUtilities<T> : PnpmParsingUtilitiesBase<T>
77
where T : PnpmYaml
88
{
99
public override DetectedComponent CreateDetectedComponentFromPnpmPath(string pnpmPackagePath)
10+
{
11+
/*
12+
* The format is documented at https://github.com/pnpm/spec/blob/master/dependency-path.md.
13+
* At the writing it does not seem to reflect changes which were made in lock file format v9:
14+
* See https://github.com/pnpm/spec/issues/5.
15+
* In general, the spec sheet for the v9 lockfile is not published, so parsing of this lockfile was emperically determined.
16+
* see https://github.com/pnpm/spec/issues/6
17+
*/
18+
19+
// Strip parenthesized suffices from package. These hold peed dep related information that is unneeded here.
20+
// An example of a dependency path with these: /[email protected]([email protected])([email protected])([email protected])
21+
(var fullPackageName, var packageVersion) = this.ExtractNameAndVersionFromPnpmPackagePath(pnpmPackagePath);
22+
23+
return new DetectedComponent(new NpmComponent(fullPackageName, packageVersion));
24+
}
25+
26+
public override (string FullPackageName, string PackageVersion) ExtractNameAndVersionFromPnpmPackagePath(string pnpmPackagePath)
1027
{
1128
/*
1229
* 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
2845
// Version is section after last `@`.
2946
var packageVersion = packageNameParts[^1];
3047

31-
return new DetectedComponent(new NpmComponent(fullPackageName, packageVersion));
48+
return (fullPackageName, packageVersion);
3249
}
3350

3451
/// <summary>

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

Lines changed: 22 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,8 @@ public void RecordDependencyGraphFromFile(string yamlFileContent, ISingleFileCom
3030
// Ignore "file:" as these are local packages.
3131
// 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).
3232
// 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.
33-
if (pnpmDependencyPath.StartsWith(PnpmConstants.PnpmFileDependencyPath))
34-
{
35-
continue;
36-
}
33+
var (packageName, packageVersion) = this.pnpmParsingUtilities.ExtractNameAndVersionFromPnpmPackagePath(pnpmDependencyPath);
34+
var isFileOrLink = this.IsFileOrLink(packageVersion) || this.IsFileOrLink(pnpmDependencyPath);
3735

3836
var dependencyPath = pnpmDependencyPath;
3937
if (pnpmDependencyPath.StartsWith('/'))
@@ -48,7 +46,10 @@ public void RecordDependencyGraphFromFile(string yamlFileContent, ISingleFileCom
4846
// It should get registered again with with additional information (what depended on it) later,
4947
// but registering it now ensures nothing is missed due to a limitation in dependency traversal
5048
// like skipping local dependencies which might have transitively depended on this.
51-
singleFileComponentRecorder.RegisterUsage(parentDetectedComponent);
49+
if (!isFileOrLink)
50+
{
51+
singleFileComponentRecorder.RegisterUsage(parentDetectedComponent);
52+
}
5253
}
5354

5455
// 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
7071
{
7172
foreach (var (name, dep) in dependencies ?? Enumerable.Empty<KeyValuePair<string, PnpmYamlV9Dependency>>())
7273
{
73-
// Ignore "file:" and "link:" as these are local packages.
74-
if (dep.Version.StartsWith(PnpmConstants.PnpmLinkDependencyPath) || dep.Version.StartsWith(PnpmConstants.PnpmFileDependencyPath))
75-
{
76-
continue;
77-
}
78-
7974
var pnpmDependencyPath = this.pnpmParsingUtilities.ReconstructPnpmDependencyPath(name, dep.Version);
8075
var (component, package) = components[pnpmDependencyPath];
8176

8277
// 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.
8378
// At this point, the root dependencies are marked according to which dependency group they are declared in the lockfile itself.
84-
singleFileComponentRecorder.RegisterUsage(component, isExplicitReferencedDependency: true, isDevelopmentDependency: isDevelopmentDependency);
79+
// Ignore "file:" and "link:" as these are local packages.
80+
var isFileOrLink = this.IsFileOrLink(dep.Version);
81+
if (!isFileOrLink)
82+
{
83+
singleFileComponentRecorder.RegisterUsage(component, isExplicitReferencedDependency: true, isDevelopmentDependency: isDevelopmentDependency);
84+
}
85+
8586
var seenDependencies = new HashSet<string>();
86-
this.ProcessIndirectDependencies(singleFileComponentRecorder, components, component.Component.Id, package.Dependencies, isDevelopmentDependency, seenDependencies);
87+
this.ProcessIndirectDependencies(singleFileComponentRecorder, components, isFileOrLink ? null : component.Component.Id, package.Dependencies, isDevelopmentDependency, seenDependencies);
8788
}
8889
}
8990

91+
private bool IsFileOrLink(string packagePath)
92+
{
93+
return packagePath.StartsWith(PnpmConstants.PnpmLinkDependencyPath) ||
94+
packagePath.StartsWith(PnpmConstants.PnpmFileDependencyPath) ||
95+
packagePath.StartsWith(PnpmConstants.PnpmHttpDependencyPath) ||
96+
packagePath.StartsWith(PnpmConstants.PnpmHttpsDependencyPath);
97+
}
98+
9099
private void ProcessIndirectDependencies(ISingleFileComponentRecorder singleFileComponentRecorder, Dictionary<string, (DetectedComponent C, Package P)> components, string parentComponentId, Dictionary<string, string> dependencies, bool isDevDependency, HashSet<string> seenDependencies)
91100
{
92101
// 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‎

Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -783,6 +783,144 @@ public async Task TestPnpmDetector_V9_GoodLockVersion_SkipsFileAndLinkDependenci
783783
parentComponent => parentComponent.Name == "sampleDependency");
784784
}
785785

786+
[TestMethod]
787+
public async Task TestPnpmDetector_V9_GoodLockVersion_FileAndLinkDependenciesAreNotRegistered()
788+
{
789+
var yamlFile = @"
790+
lockfileVersion: '9.0'
791+
settings:
792+
autoInstallPeers: true
793+
excludeLinksFromLockfile: false
794+
importers:
795+
.:
796+
dependencies:
797+
sampleDependency:
798+
specifier: ^1.1.1
799+
version: 1.1.1
800+
sampleFileDependency:
801+
specifier: file:../test
802+
version: file:../test
803+
SampleLinkDependency:
804+
specifier: workspace:*
805+
version: link:SampleLinkDependency
806+
packages:
807+
[email protected]:
808+
resolution: {integrity: sha512-zRpUiDwd/xk6ADqPMATG8vc9VPrkck7T07OIx0gnjmJAnHnTVXNQG3vfvWNuiZIkwu9KrKdA1iJKfsfTVxE6NA==}
809+
[email protected]:
810+
resolution: {integrity: sha512-FQN4MRfuJeHf7cBbBMJFXhKSDq+2kAArBlmRBvcvFE5BB1HZKXtSFASDhdlz9zOYwxh8lDdnvmMOe/+5cdoEdg==}
811+
engines: {node: '>= 0.8'}
812+
[email protected]:
813+
resolution: {integrity: sha512-ZySD7Nf91aLB0RxL4KGrKHBXl7Eds1DAmEdcoVawXnLD7SDhpNgtuII2aAkg7a7QS41jxPSZ17p4VdGnMHk3MQ==}
814+
engines: {node: '>=0.4.0'}
815+
816+
snapshots:
817+
[email protected]:
818+
dependencies:
819+
sampleIndirectDependency: 3.3.3
820+
sampleIndirectDependency2: 2.2.2
821+
'file://../sampleFile': 'link:../\\'
822+
[email protected]: {}
823+
[email protected]: {}
824+
sampleFileDependency@file:../test': {}
825+
";
826+
827+
var (scanResult, componentRecorder) = await this.DetectorTestUtility
828+
.WithFile("pnpm-lock.yaml", yamlFile)
829+
.ExecuteDetectorAsync();
830+
831+
scanResult.ResultCode.Should().Be(ProcessingResultCode.Success);
832+
833+
var detectedComponents = componentRecorder.GetDetectedComponents();
834+
detectedComponents.Should().HaveCount(3);
835+
var npmComponents = detectedComponents.Select(x => new { Component = x.Component as NpmComponent, DetectedComponent = x });
836+
npmComponents.Should().Contain(x => x.Component.Name == "sampleDependency" && x.Component.Version == "1.1.1");
837+
npmComponents.Should().Contain(x => x.Component.Name == "sampleIndirectDependency2" && x.Component.Version == "2.2.2");
838+
npmComponents.Should().Contain(x => x.Component.Name == "sampleIndirectDependency" && x.Component.Version == "3.3.3");
839+
840+
var noDevDependencyComponent = npmComponents.First(x => x.Component.Name == "sampleDependency");
841+
var indirectDependencyComponent2 = npmComponents.First(x => x.Component.Name == "sampleIndirectDependency2");
842+
var indirectDependencyComponent = npmComponents.First(x => x.Component.Name == "sampleIndirectDependency");
843+
844+
componentRecorder.GetEffectiveDevDependencyValue(noDevDependencyComponent.Component.Id).Should().BeFalse();
845+
componentRecorder.GetEffectiveDevDependencyValue(indirectDependencyComponent2.Component.Id).Should().BeFalse();
846+
componentRecorder.GetEffectiveDevDependencyValue(indirectDependencyComponent.Component.Id).Should().BeFalse();
847+
componentRecorder.AssertAllExplicitlyReferencedComponents<NpmComponent>(
848+
indirectDependencyComponent.Component.Id,
849+
parentComponent => parentComponent.Name == "sampleDependency");
850+
componentRecorder.AssertAllExplicitlyReferencedComponents<NpmComponent>(
851+
indirectDependencyComponent2.Component.Id,
852+
parentComponent => parentComponent.Name == "sampleDependency");
853+
}
854+
855+
[TestMethod]
856+
public async Task TestPnpmDetector_V9_GoodLockVersion_HttpDependenciesAreNotRegistered()
857+
{
858+
var yamlFile = @"
859+
lockfileVersion: '9.0'
860+
settings:
861+
autoInstallPeers: true
862+
excludeLinksFromLockfile: false
863+
importers:
864+
.:
865+
dependencies:
866+
sampleDependency:
867+
specifier: ^1.1.1
868+
version: 1.1.1
869+
sampleHttpDependency:
870+
specifier: https://samplePackage/tar.gz/32f550d3b3bdb1b781aabe100683311cd982c98e
871+
version: sample@https://samplePackage/tar.gz/32f550d3b3bdb1b781aabe100683311cd982c98e
872+
SampleLinkDependency:
873+
specifier: workspace:*
874+
version: link:SampleLinkDependency
875+
packages:
876+
[email protected]:
877+
resolution: {integrity: sha512-zRpUiDwd/xk6ADqPMATG8vc9VPrkck7T07OIx0gnjmJAnHnTVXNQG3vfvWNuiZIkwu9KrKdA1iJKfsfTVxE6NA==}
878+
[email protected]:
879+
resolution: {integrity: sha512-FQN4MRfuJeHf7cBbBMJFXhKSDq+2kAArBlmRBvcvFE5BB1HZKXtSFASDhdlz9zOYwxh8lDdnvmMOe/+5cdoEdg==}
880+
engines: {node: '>= 0.8'}
881+
[email protected]:
882+
resolution: {integrity: sha512-ZySD7Nf91aLB0RxL4KGrKHBXl7Eds1DAmEdcoVawXnLD7SDhpNgtuII2aAkg7a7QS41jxPSZ17p4VdGnMHk3MQ==}
883+
engines: {node: '>=0.4.0'}
884+
885+
snapshots:
886+
[email protected]:
887+
dependencies:
888+
sampleIndirectDependency: 3.3.3
889+
sampleIndirectDependency2: 2.2.2
890+
'file://../sampleFile': 'link:../\\'
891+
[email protected]: {}
892+
[email protected]: {}
893+
sampleHttpDependency@https://samplePackage/tar.gz/32f550d3b3bdb1b781aabe100683311cd982c98e': {}
894+
";
895+
896+
var (scanResult, componentRecorder) = await this.DetectorTestUtility
897+
.WithFile("pnpm-lock.yaml", yamlFile)
898+
.ExecuteDetectorAsync();
899+
900+
scanResult.ResultCode.Should().Be(ProcessingResultCode.Success);
901+
902+
var detectedComponents = componentRecorder.GetDetectedComponents();
903+
detectedComponents.Should().HaveCount(3);
904+
var npmComponents = detectedComponents.Select(x => new { Component = x.Component as NpmComponent, DetectedComponent = x });
905+
npmComponents.Should().Contain(x => x.Component.Name == "sampleDependency" && x.Component.Version == "1.1.1");
906+
npmComponents.Should().Contain(x => x.Component.Name == "sampleIndirectDependency2" && x.Component.Version == "2.2.2");
907+
npmComponents.Should().Contain(x => x.Component.Name == "sampleIndirectDependency" && x.Component.Version == "3.3.3");
908+
909+
var noDevDependencyComponent = npmComponents.First(x => x.Component.Name == "sampleDependency");
910+
var indirectDependencyComponent2 = npmComponents.First(x => x.Component.Name == "sampleIndirectDependency2");
911+
var indirectDependencyComponent = npmComponents.First(x => x.Component.Name == "sampleIndirectDependency");
912+
913+
componentRecorder.GetEffectiveDevDependencyValue(noDevDependencyComponent.Component.Id).Should().BeFalse();
914+
componentRecorder.GetEffectiveDevDependencyValue(indirectDependencyComponent2.Component.Id).Should().BeFalse();
915+
componentRecorder.GetEffectiveDevDependencyValue(indirectDependencyComponent.Component.Id).Should().BeFalse();
916+
componentRecorder.AssertAllExplicitlyReferencedComponents<NpmComponent>(
917+
indirectDependencyComponent.Component.Id,
918+
parentComponent => parentComponent.Name == "sampleDependency");
919+
componentRecorder.AssertAllExplicitlyReferencedComponents<NpmComponent>(
920+
indirectDependencyComponent2.Component.Id,
921+
parentComponent => parentComponent.Name == "sampleDependency");
922+
}
923+
786924
[TestMethod]
787925
public async Task TestPnpmDetector_V9_GoodLockVersion_MissingSnapshotsSuccess()
788926
{

0 commit comments

Comments
 (0)