From 15a7c4e5c52a7adccf3cd6f8e616d9dacf6e3347 Mon Sep 17 00:00:00 2001 From: Fernando Rojo Date: Thu, 26 Feb 2026 16:33:42 -0800 Subject: [PATCH] Update npm lockfile 3 ancestor traversal to properly detect dependency roots --- .../npm/NpmLockfile3Detector.cs | 31 ++++---- .../NpmLockfile3DetectorTests.cs | 76 +++++++++++++++++++ 2 files changed, 89 insertions(+), 18 deletions(-) diff --git a/src/Microsoft.ComponentDetection.Detectors/npm/NpmLockfile3Detector.cs b/src/Microsoft.ComponentDetection.Detectors/npm/NpmLockfile3Detector.cs index 5fbcdf0fd..0c02143f8 100644 --- a/src/Microsoft.ComponentDetection.Detectors/npm/NpmLockfile3Detector.cs +++ b/src/Microsoft.ComponentDetection.Detectors/npm/NpmLockfile3Detector.cs @@ -1,5 +1,6 @@ namespace Microsoft.ComponentDetection.Detectors.Npm; +using System; using System.Collections.Generic; using System.Linq; using System.Text.Json; @@ -102,7 +103,7 @@ protected override void ProcessLockfile( this.RecordComponent(singleFileComponentRecorder, component, lockPackage.Dev ?? false, component); // Enqueue nested dependencies - this.EnqueueNestedDependencies(subQueue, path, lockPackage, packageLookup, singleFileComponentRecorder, component); + this.EnqueueNestedDependencies(subQueue, path, lockPackage, packageLookup, component); // Process sub-dependencies while (subQueue.Count > 0) @@ -127,7 +128,7 @@ protected override void ProcessLockfile( this.RecordComponent(singleFileComponentRecorder, subComponent, subPackage.Dev ?? false, component, parentComponent.Id); - this.EnqueueNestedDependencies(subQueue, subPath, subPackage, packageLookup, singleFileComponentRecorder, subComponent); + this.EnqueueNestedDependencies(subQueue, subPath, subPackage, packageLookup, subComponent); } } } @@ -157,7 +158,6 @@ private void EnqueueNestedDependencies( string currentPath, PackageLockV3Package package, Dictionary packageLookup, - ISingleFileComponentRecorder componentRecorder, TypedComponent parent) { if (package.Dependencies is null) @@ -167,31 +167,26 @@ private void EnqueueNestedDependencies( foreach (var dep in package.Dependencies) { - // First, check if there is an entry in the lockfile for this dependency nested in its ancestors - var ancestors = componentRecorder.DependencyGraph.GetAncestors(parent.Id); - ancestors.Add(parent.Id); - - // Remove version information from ancestor IDs - ancestors = ancestors.Select(x => x.Split(' ')[0]).ToList(); - var found = false; - // Depth-first search through ancestors - for (var i = 0; i < ancestors.Count && !found; i++) + // Resolve using lockfile paths: current package path first, then each ancestor path, then top-level. + var currentAncestorPath = currentPath; + while (!string.IsNullOrEmpty(currentAncestorPath) && !found) { - var possiblePath = ancestors.Skip(i).ToList(); - var ancestorNodeModulesPath = string.Format( - "{0}/{1}/{0}/{2}", - NodeModules, - string.Join($"/{NodeModules}/", possiblePath), - dep.Key); + var ancestorNodeModulesPath = $"{currentAncestorPath}/{NodeModules}/{dep.Key}"; if (packageLookup.TryGetValue(ancestorNodeModulesPath, out var nestedPkg)) { this.Logger.LogDebug("Found nested dependency {Dependency} in {AncestorNodeModulesPath}", dep.Key, ancestorNodeModulesPath); queue.Enqueue((nestedPkg.Path, nestedPkg.Package, parent)); found = true; + continue; } + + var parentNodeModulesIndex = currentAncestorPath.LastIndexOf($"/{NodeModules}/", StringComparison.Ordinal); + currentAncestorPath = parentNodeModulesIndex >= 0 + ? currentAncestorPath[..parentNodeModulesIndex] + : null; } if (found) diff --git a/test/Microsoft.ComponentDetection.Detectors.Tests/NpmLockfile3DetectorTests.cs b/test/Microsoft.ComponentDetection.Detectors.Tests/NpmLockfile3DetectorTests.cs index 45d6c0524..a966dc1f1 100644 --- a/test/Microsoft.ComponentDetection.Detectors.Tests/NpmLockfile3DetectorTests.cs +++ b/test/Microsoft.ComponentDetection.Detectors.Tests/NpmLockfile3DetectorTests.cs @@ -286,6 +286,82 @@ public async Task TestNpmDetector_NestedNodeModulesV3Async() dependencyGraph.GetDependenciesForComponent(componentBId).Should().BeEmpty(); } + [TestMethod] + public async Task TestNpmDetector_ResolvesDependencyFromAncestorPathV3Async() + { + var componentA = (Name: "componentA", Version: "1.0.0"); + var componentB = (Name: "componentB", Version: "1.0.0"); + var componentC = (Name: "componentC", Version: "1.0.0"); + + var packageLockJson = @"{{ + ""name"": ""test"", + ""version"": ""0.0.0"", + ""lockfileVersion"": 3, + ""requires"": true, + ""packages"": {{ + """": {{ + ""name"": ""test"", + ""version"": ""0.0.0"", + ""dependencies"": {{ + ""{0}"": ""{1}"" + }} + }}, + ""node_modules/{0}"": {{ + ""version"": ""{1}"", + ""resolved"": ""https://registry.npmjs.org/{0}/-/{0}-{1}.tgz"", + ""integrity"": ""sha512-AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA="", + ""dependencies"": {{ + ""{2}"": ""{3}"" + }} + }}, + ""node_modules/{0}/node_modules/{2}"": {{ + ""version"": ""{3}"", + ""resolved"": ""https://registry.npmjs.org/{2}/-/{2}-{3}.tgz"", + ""integrity"": ""sha512-BBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB="", + ""dependencies"": {{ + ""{4}"": ""{5}"" + }} + }}, + ""node_modules/{0}/node_modules/{4}"": {{ + ""version"": ""{5}"", + ""resolved"": ""https://registry.npmjs.org/{4}/-/{4}-{5}.tgz"", + ""integrity"": ""sha512-CCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCC="" + }} + }} + }}"; + + var packageLockTemplate = string.Format(packageLockJson, componentA.Name, componentA.Version, componentB.Name, componentB.Version, componentC.Name, componentC.Version); + + var packagejson = @"{{ + ""name"": ""test"", + ""version"": ""0.0.0"", + ""dependencies"": {{ + ""{0}"": ""{1}"" + }} + }}"; + + var packageJsonTemplate = string.Format(packagejson, componentA.Name, componentA.Version); + + var (scanResult, componentRecorder) = await this.DetectorTestUtility + .WithFile(this.packageLockJsonFileName, packageLockTemplate, this.packageLockJsonSearchPatterns) + .WithFile(this.packageJsonFileName, packageJsonTemplate, this.packageJsonSearchPattern) + .ExecuteDetectorAsync(); + + scanResult.ResultCode.Should().Be(ProcessingResultCode.Success); + + var detectedComponents = componentRecorder.GetDetectedComponents(); + detectedComponents.Should().HaveCount(3); + + var componentAId = detectedComponents.First(c => ((NpmComponent)c.Component).Name.Equals(componentA.Name)).Component.Id; + var componentBId = detectedComponents.First(c => ((NpmComponent)c.Component).Name.Equals(componentB.Name)).Component.Id; + var componentCId = detectedComponents.First(c => ((NpmComponent)c.Component).Name.Equals(componentC.Name)).Component.Id; + + var dependencyGraph = componentRecorder.GetDependencyGraphsByLocation().Values.First(); + + dependencyGraph.GetDependenciesForComponent(componentAId).Should().Contain(componentBId); + dependencyGraph.GetDependenciesForComponent(componentBId).Should().Contain(componentCId); + } + [TestMethod] public async Task TestNpmDetector_PackageLockWithoutPackagesObject_ShouldHandleGracefully() {