From 6afff3985f933f442fe7ae00bed4773457e45c86 Mon Sep 17 00:00:00 2001 From: RKS Date: Wed, 16 Sep 2026 23:53:01 -0400 Subject: [PATCH 01/15] Support multi-document pnpm lockfiles - Use YamlDotNet Parser to deserialize all documents from multi-document pnpm lockfiles - Validate lockfile version consistency across documents, failing clearly on conflicting versions - Record components and dependencies across all documents in Pnpm9, Pnpm6, and Pnpm5 detectors - Support packageManagerDependencies and configDependencies from environment lockfile documents - Bump PnpmComponentDetectorFactory version to 9 --- .../Contracts/V6/PnpmHasDependenciesV6.cs | 6 + .../Contracts/V9/PnpmHasDependenciesV9.cs | 6 + .../PnpmParsingUtilitiesBase.cs | 29 +++- .../PnpmParsingUtilitiesFactory.cs | 30 +++- .../pnpm/Pnpm5Detector.cs | 45 +++--- .../pnpm/Pnpm6Detector.cs | 72 +++++---- .../pnpm/Pnpm9Detector.cs | 77 +++++---- .../pnpm/PnpmComponentDetectorFactory.cs | 2 +- .../PnpmDetectorTests.cs | 148 ++++++++++++++++++ .../PnpmParsingUtilitiesTest.cs | 56 +++++++ 10 files changed, 390 insertions(+), 81 deletions(-) diff --git a/src/Microsoft.ComponentDetection.Detectors/pnpm/Contracts/V6/PnpmHasDependenciesV6.cs b/src/Microsoft.ComponentDetection.Detectors/pnpm/Contracts/V6/PnpmHasDependenciesV6.cs index ea88568dd..9db70bf9c 100644 --- a/src/Microsoft.ComponentDetection.Detectors/pnpm/Contracts/V6/PnpmHasDependenciesV6.cs +++ b/src/Microsoft.ComponentDetection.Detectors/pnpm/Contracts/V6/PnpmHasDependenciesV6.cs @@ -14,4 +14,10 @@ internal class PnpmHasDependenciesV6 : PnpmYaml [YamlMember(Alias = "optionalDependencies")] public Dictionary OptionalDependencies { get; set; } + + [YamlMember(Alias = "packageManagerDependencies")] + public Dictionary PackageManagerDependencies { get; set; } + + [YamlMember(Alias = "configDependencies")] + public Dictionary ConfigDependencies { get; set; } } diff --git a/src/Microsoft.ComponentDetection.Detectors/pnpm/Contracts/V9/PnpmHasDependenciesV9.cs b/src/Microsoft.ComponentDetection.Detectors/pnpm/Contracts/V9/PnpmHasDependenciesV9.cs index f0e266e91..5c3d70157 100644 --- a/src/Microsoft.ComponentDetection.Detectors/pnpm/Contracts/V9/PnpmHasDependenciesV9.cs +++ b/src/Microsoft.ComponentDetection.Detectors/pnpm/Contracts/V9/PnpmHasDependenciesV9.cs @@ -14,4 +14,10 @@ internal class PnpmHasDependenciesV9 : PnpmYaml [YamlMember(Alias = "optionalDependencies")] public Dictionary OptionalDependencies { get; set; } + + [YamlMember(Alias = "packageManagerDependencies")] + public Dictionary PackageManagerDependencies { get; set; } + + [YamlMember(Alias = "configDependencies")] + public Dictionary ConfigDependencies { get; set; } } diff --git a/src/Microsoft.ComponentDetection.Detectors/pnpm/ParsingUtilities/PnpmParsingUtilitiesBase.cs b/src/Microsoft.ComponentDetection.Detectors/pnpm/ParsingUtilities/PnpmParsingUtilitiesBase.cs index da1770e5f..812fc281c 100644 --- a/src/Microsoft.ComponentDetection.Detectors/pnpm/ParsingUtilities/PnpmParsingUtilitiesBase.cs +++ b/src/Microsoft.ComponentDetection.Detectors/pnpm/ParsingUtilities/PnpmParsingUtilitiesBase.cs @@ -4,18 +4,43 @@ namespace Microsoft.ComponentDetection.Detectors.Pnpm; using System; using System.Collections.Generic; using System.IO; +using System.Linq; using Microsoft.ComponentDetection.Contracts; +using YamlDotNet.Core; +using YamlDotNet.Core.Events; using YamlDotNet.Serialization; internal abstract class PnpmParsingUtilitiesBase where T : PnpmYaml { - public T DeserializePnpmYamlFile(string fileContent) + public virtual List DeserializePnpmYamlFileDocuments(string fileContent) { var deserializer = new DeserializerBuilder() .IgnoreUnmatchedProperties() .Build(); - return deserializer.Deserialize(new StringReader(fileContent)); + + var reader = new StringReader(fileContent); + var parser = new Parser(reader); + parser.Consume(); + + var documents = new List(); + while (parser.TryConsume(out _)) + { + var doc = deserializer.Deserialize(parser); + if (doc != null) + { + documents.Add(doc); + } + + parser.TryConsume(out _); + } + + return documents; + } + + public T DeserializePnpmYamlFile(string fileContent) + { + return this.DeserializePnpmYamlFileDocuments(fileContent).FirstOrDefault(); } public virtual bool IsPnpmPackageDevDependency(Package pnpmPackage) diff --git a/src/Microsoft.ComponentDetection.Detectors/pnpm/ParsingUtilities/PnpmParsingUtilitiesFactory.cs b/src/Microsoft.ComponentDetection.Detectors/pnpm/ParsingUtilities/PnpmParsingUtilitiesFactory.cs index a286c7110..00fa9c9c9 100644 --- a/src/Microsoft.ComponentDetection.Detectors/pnpm/ParsingUtilities/PnpmParsingUtilitiesFactory.cs +++ b/src/Microsoft.ComponentDetection.Detectors/pnpm/ParsingUtilities/PnpmParsingUtilitiesFactory.cs @@ -1,7 +1,12 @@ #nullable disable namespace Microsoft.ComponentDetection.Detectors.Pnpm; +using System; +using System.Collections.Generic; using System.IO; +using System.Linq; +using YamlDotNet.Core; +using YamlDotNet.Core.Events; using YamlDotNet.Serialization; internal static class PnpmParsingUtilitiesFactory @@ -23,6 +28,29 @@ public static string DeserializePnpmYamlFileVersion(string fileContent) var deserializer = new DeserializerBuilder() .IgnoreUnmatchedProperties() .Build(); - return deserializer.Deserialize(new StringReader(fileContent))?.LockfileVersion; + + var reader = new StringReader(fileContent); + var parser = new Parser(reader); + parser.Consume(); + + var versions = new List(); + while (parser.TryConsume(out _)) + { + var doc = deserializer.Deserialize(parser); + if (doc != null && !string.IsNullOrWhiteSpace(doc.LockfileVersion)) + { + versions.Add(doc.LockfileVersion); + } + + parser.TryConsume(out _); + } + + var distinctVersions = versions.Distinct().ToList(); + if (distinctVersions.Count > 1) + { + throw new InvalidOperationException($"Inconsistent lockfile versions found: {string.Join(", ", distinctVersions)}"); + } + + return distinctVersions.FirstOrDefault(); } } diff --git a/src/Microsoft.ComponentDetection.Detectors/pnpm/Pnpm5Detector.cs b/src/Microsoft.ComponentDetection.Detectors/pnpm/Pnpm5Detector.cs index 6b34bcdd1..71c340486 100644 --- a/src/Microsoft.ComponentDetection.Detectors/pnpm/Pnpm5Detector.cs +++ b/src/Microsoft.ComponentDetection.Detectors/pnpm/Pnpm5Detector.cs @@ -12,36 +12,39 @@ public class Pnpm5Detector : IPnpmDetector public void RecordDependencyGraphFromFile(string yamlFileContent, ISingleFileComponentRecorder singleFileComponentRecorder) { - var yaml = this.pnpmParsingUtilities.DeserializePnpmYamlFile(yamlFileContent); + var yamls = this.pnpmParsingUtilities.DeserializePnpmYamlFileDocuments(yamlFileContent); - foreach (var packageKeyValue in yaml?.Packages ?? Enumerable.Empty>()) + foreach (var yaml in yamls) { - // Ignore file: as these are local packages. - if (packageKeyValue.Key.StartsWith(PnpmConstants.PnpmFileDependencyPath)) + foreach (var packageKeyValue in yaml?.Packages ?? Enumerable.Empty>()) { - continue; - } + // Ignore file: as these are local packages. + if (packageKeyValue.Key.StartsWith(PnpmConstants.PnpmFileDependencyPath)) + { + continue; + } - var parentDetectedComponent = this.pnpmParsingUtilities.CreateDetectedComponentFromPnpmPath(pnpmPackagePath: packageKeyValue.Key); - var isDevDependency = packageKeyValue.Value != null && this.pnpmParsingUtilities.IsPnpmPackageDevDependency(packageKeyValue.Value); - singleFileComponentRecorder.RegisterUsage(parentDetectedComponent, isDevelopmentDependency: isDevDependency); - parentDetectedComponent = singleFileComponentRecorder.GetComponent(parentDetectedComponent.Component.Id); + var parentDetectedComponent = this.pnpmParsingUtilities.CreateDetectedComponentFromPnpmPath(pnpmPackagePath: packageKeyValue.Key); + var isDevDependency = packageKeyValue.Value != null && this.pnpmParsingUtilities.IsPnpmPackageDevDependency(packageKeyValue.Value); + singleFileComponentRecorder.RegisterUsage(parentDetectedComponent, isDevelopmentDependency: isDevDependency); + parentDetectedComponent = singleFileComponentRecorder.GetComponent(parentDetectedComponent.Component.Id); - if (packageKeyValue.Value.Dependencies != null) - { - foreach (var dependency in packageKeyValue.Value.Dependencies) + if (packageKeyValue.Value?.Dependencies != null) { - // Ignore local packages. - if (this.pnpmParsingUtilities.IsLocalDependency(dependency)) + foreach (var dependency in packageKeyValue.Value.Dependencies) { - continue; - } + // Ignore local packages. + if (this.pnpmParsingUtilities.IsLocalDependency(dependency)) + { + continue; + } - var childDetectedComponent = this.pnpmParsingUtilities.CreateDetectedComponentFromPnpmPath( - pnpmPackagePath: this.CreatePnpmPackagePathFromDependency(dependency.Key, dependency.Value)); + var childDetectedComponent = this.pnpmParsingUtilities.CreateDetectedComponentFromPnpmPath( + pnpmPackagePath: this.CreatePnpmPackagePathFromDependency(dependency.Key, dependency.Value)); - // Older code used the root's dev dependency value. We're leaving this null until we do a second pass to look at each components' top level referrers. - singleFileComponentRecorder.RegisterUsage(childDetectedComponent, parentComponentId: parentDetectedComponent.Component.Id, isDevelopmentDependency: null); + // Older code used the root's dev dependency value. We're leaving this null until we do a second pass to look at each components' top level referrers. + singleFileComponentRecorder.RegisterUsage(childDetectedComponent, parentComponentId: parentDetectedComponent.Component.Id, isDevelopmentDependency: null); + } } } } diff --git a/src/Microsoft.ComponentDetection.Detectors/pnpm/Pnpm6Detector.cs b/src/Microsoft.ComponentDetection.Detectors/pnpm/Pnpm6Detector.cs index d915c6dea..f0b1de7b3 100644 --- a/src/Microsoft.ComponentDetection.Detectors/pnpm/Pnpm6Detector.cs +++ b/src/Microsoft.ComponentDetection.Detectors/pnpm/Pnpm6Detector.cs @@ -12,7 +12,7 @@ public class Pnpm6Detector : IPnpmDetector public void RecordDependencyGraphFromFile(string yamlFileContent, ISingleFileComponentRecorder singleFileComponentRecorder) { - var yaml = this.pnpmParsingUtilities.DeserializePnpmYamlFile(yamlFileContent); + var yamls = this.pnpmParsingUtilities.DeserializePnpmYamlFileDocuments(yamlFileContent); // There may be multiple instance of the same package (even at the same version) in pnpm differentiated by other aspects of the pnpm dependency path. // Therefor all DetectedComponents are tracked by the same full string pnpm uses, the pnpm dependency path, which is used as the key in this dictionary. @@ -20,25 +20,31 @@ public void RecordDependencyGraphFromFile(string yamlFileContent, ISingleFileCom var components = new Dictionary(); // Create a component for every package referenced in the lock file. - // This includes all directly and transitively referenced dependencies. - foreach (var (pnpmDependencyPath, package) in yaml.Packages ?? Enumerable.Empty>()) + // This includes all directly and transitively referenced dependencies across all documents. + foreach (var yaml in yamls) { - // 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)) + foreach (var (pnpmDependencyPath, package) in yaml.Packages ?? Enumerable.Empty>()) { - continue; + // 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; + } + + if (!components.ContainsKey(pnpmDependencyPath)) + { + var parentDetectedComponent = this.pnpmParsingUtilities.CreateDetectedComponentFromPnpmPath(pnpmPackagePath: pnpmDependencyPath); + components.Add(pnpmDependencyPath, (parentDetectedComponent, package)); + + // Register the component. + // 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, isDevelopmentDependency: this.pnpmParsingUtilities.IsPnpmPackageDevDependency(package)); + } } - - var parentDetectedComponent = this.pnpmParsingUtilities.CreateDetectedComponentFromPnpmPath(pnpmPackagePath: pnpmDependencyPath); - components.Add(pnpmDependencyPath, (parentDetectedComponent, package)); - - // Register the component. - // 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, isDevelopmentDependency: this.pnpmParsingUtilities.IsPnpmPackageDevDependency(package)); } // Now that the `components` dictionary is populated, make a second pass registering all the dependency edges in the graph. @@ -49,21 +55,24 @@ public void RecordDependencyGraphFromFile(string yamlFileContent, ISingleFileCom var pnpmDependencyPath = this.pnpmParsingUtilities.ReconstructPnpmDependencyPath(name, version); // If this lookup fails, then pnpmDependencyPath was either parsed incorrectly or constructed incorrectly. - var (referenced, _) = components[pnpmDependencyPath]; - - singleFileComponentRecorder.RegisterUsage(referenced, parentComponentId: component.Component.Id, isExplicitReferencedDependency: false); + if (components.TryGetValue(pnpmDependencyPath, out var referencedTuple)) + { + singleFileComponentRecorder.RegisterUsage(referencedTuple.Item1, parentComponentId: component.Component.Id, isExplicitReferencedDependency: false); + } } } // Lastly, add all direct dependencies of the current file/project setting isExplicitReferencedDependency to true: - - // "dedicated shrinkwrap" (single package) case: - this.ProcessDependencySet(singleFileComponentRecorder, components, yaml); - - // "shared shrinkwrap" (workspace / mono-repos) case: - foreach (var (_, package) in yaml.Importers ?? Enumerable.Empty>()) + foreach (var yaml in yamls) { - this.ProcessDependencySet(singleFileComponentRecorder, components, package); + // "dedicated shrinkwrap" (single package) case: + this.ProcessDependencySet(singleFileComponentRecorder, components, yaml); + + // "shared shrinkwrap" (workspace / mono-repos) case: + foreach (var (_, package) in yaml.Importers ?? Enumerable.Empty>()) + { + this.ProcessDependencySet(singleFileComponentRecorder, components, package); + } } } @@ -72,6 +81,8 @@ private void ProcessDependencySet(ISingleFileComponentRecorder singleFileCompone this.ProcessDependencyList(singleFileComponentRecorder, components, item.Dependencies); this.ProcessDependencyList(singleFileComponentRecorder, components, item.DevDependencies); this.ProcessDependencyList(singleFileComponentRecorder, components, item.OptionalDependencies); + this.ProcessDependencyList(singleFileComponentRecorder, components, item.PackageManagerDependencies); + this.ProcessDependencyList(singleFileComponentRecorder, components, item.ConfigDependencies); } private void ProcessDependencyList(ISingleFileComponentRecorder singleFileComponentRecorder, Dictionary components, Dictionary dependencies) @@ -85,7 +96,12 @@ private void ProcessDependencyList(ISingleFileComponentRecorder singleFileCompon } var pnpmDependencyPath = this.pnpmParsingUtilities.ReconstructPnpmDependencyPath(name, dep.Version); - var (component, package) = components[pnpmDependencyPath]; + if (!components.TryGetValue(pnpmDependencyPath, out var componentAndPackage)) + { + continue; + } + + var (component, package) = componentAndPackage; // Determine isDevelopmentDependency using metadata on package from pnpm rather than from which dependency list this package is under. // This ensures that dependencies which are a direct dev dependency and an indirect non-dev dependency get listed as non-dev. diff --git a/src/Microsoft.ComponentDetection.Detectors/pnpm/Pnpm9Detector.cs b/src/Microsoft.ComponentDetection.Detectors/pnpm/Pnpm9Detector.cs index a275c6c3d..cbbaf465f 100644 --- a/src/Microsoft.ComponentDetection.Detectors/pnpm/Pnpm9Detector.cs +++ b/src/Microsoft.ComponentDetection.Detectors/pnpm/Pnpm9Detector.cs @@ -17,7 +17,7 @@ public class Pnpm9Detector : IPnpmDetector public void RecordDependencyGraphFromFile(string yamlFileContent, ISingleFileComponentRecorder singleFileComponentRecorder) { - var yaml = this.pnpmParsingUtilities.DeserializePnpmYamlFile(yamlFileContent); + var yamls = this.pnpmParsingUtilities.DeserializePnpmYamlFileDocuments(yamlFileContent); // There may be multiple instance of the same package (even at the same version) in pnpm differentiated by other aspects of the pnpm dependency path. // Therefor all DetectedComponents are tracked by the same full string pnpm uses, the pnpm dependency path, which is used as the key in this dictionary. @@ -25,39 +25,48 @@ public void RecordDependencyGraphFromFile(string yamlFileContent, ISingleFileCom var components = new Dictionary(); // Create a component for every package referenced in the lock file. - // This includes all directly and transitively referenced dependencies. - foreach (var (pnpmDependencyPath, package) in yaml.Snapshots ?? Enumerable.Empty>()) + // This includes all directly and transitively referenced dependencies across all documents. + foreach (var yaml in yamls) { - // 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. - var (packageName, packageVersion) = this.pnpmParsingUtilities.ExtractNameAndVersionFromPnpmPackagePath(pnpmDependencyPath); - var isFileOrLink = this.IsFileOrLink(packageVersion) || this.IsFileOrLink(pnpmDependencyPath); - - var dependencyPath = pnpmDependencyPath; - if (pnpmDependencyPath.StartsWith('/')) + foreach (var (pnpmDependencyPath, package) in yaml.Snapshots ?? Enumerable.Empty>()) { - dependencyPath = pnpmDependencyPath[1..]; - } - - var parentDetectedComponent = this.pnpmParsingUtilities.CreateDetectedComponentFromPnpmPath(pnpmPackagePath: dependencyPath); - components.Add(dependencyPath, (parentDetectedComponent, package)); - - // Register the component. - // 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. - if (!isFileOrLink) - { - singleFileComponentRecorder.RegisterUsage(parentDetectedComponent); + // 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. + var (packageName, packageVersion) = this.pnpmParsingUtilities.ExtractNameAndVersionFromPnpmPackagePath(pnpmDependencyPath); + var isFileOrLink = this.IsFileOrLink(packageVersion) || this.IsFileOrLink(pnpmDependencyPath); + + var dependencyPath = pnpmDependencyPath; + if (pnpmDependencyPath.StartsWith('/')) + { + dependencyPath = pnpmDependencyPath[1..]; + } + + if (!components.ContainsKey(dependencyPath)) + { + var parentDetectedComponent = this.pnpmParsingUtilities.CreateDetectedComponentFromPnpmPath(pnpmPackagePath: dependencyPath); + components.Add(dependencyPath, (parentDetectedComponent, package)); + + // Register the component. + // 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. + if (!isFileOrLink) + { + singleFileComponentRecorder.RegisterUsage(parentDetectedComponent); + } + } } } // now that the components dictionary is populated, add direct dependencies of the current file/project setting isExplicitReferencedDependency to true // during this step, recursively processes any indirect dependencies - foreach (var (_, package) in yaml.Importers ?? Enumerable.Empty>()) + foreach (var yaml in yamls) { - this.ProcessDependencySets(singleFileComponentRecorder, components, package); + foreach (var (_, package) in yaml.Importers ?? Enumerable.Empty>()) + { + this.ProcessDependencySets(singleFileComponentRecorder, components, package); + } } } @@ -66,6 +75,8 @@ private void ProcessDependencySets(ISingleFileComponentRecorder singleFileCompon this.ProcessDependencyList(singleFileComponentRecorder, components, item.Dependencies, isDevelopmentDependency: false); this.ProcessDependencyList(singleFileComponentRecorder, components, item.DevDependencies, isDevelopmentDependency: true); this.ProcessDependencyList(singleFileComponentRecorder, components, item.OptionalDependencies, false); + this.ProcessDependencyList(singleFileComponentRecorder, components, item.PackageManagerDependencies, false); + this.ProcessDependencyList(singleFileComponentRecorder, components, item.ConfigDependencies, false); } private void ProcessDependencyList(ISingleFileComponentRecorder singleFileComponentRecorder, Dictionary components, Dictionary dependencies, bool isDevelopmentDependency) @@ -82,7 +93,12 @@ private void ProcessDependencyList(ISingleFileComponentRecorder singleFileCompon continue; } - var (component, package) = components[pnpmDependencyPath]; + if (!components.TryGetValue(pnpmDependencyPath, out var componentAndPackage)) + { + continue; + } + + var (component, package) = componentAndPackage; // 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. @@ -123,7 +139,12 @@ private void ProcessIndirectDependencies(ISingleFileComponentRecorder singleFile } // If this lookup fails, then pnpmDependencyPath was either parsed incorrectly or constructed incorrectly. - var (component, package) = components[pnpmDependencyPath]; + if (!components.TryGetValue(pnpmDependencyPath, out var componentAndPackage)) + { + continue; + } + + var (component, package) = componentAndPackage; singleFileComponentRecorder.RegisterUsage(component, parentComponentId: parentComponentId, isExplicitReferencedDependency: false, isDevelopmentDependency: isDevDependency); seenDependencies.Add(pnpmDependencyPath); this.ProcessIndirectDependencies(singleFileComponentRecorder, components, component.Component.Id, package.Dependencies, isDevDependency, seenDependencies); diff --git a/src/Microsoft.ComponentDetection.Detectors/pnpm/PnpmComponentDetectorFactory.cs b/src/Microsoft.ComponentDetection.Detectors/pnpm/PnpmComponentDetectorFactory.cs index 796016b73..9e8937f86 100644 --- a/src/Microsoft.ComponentDetection.Detectors/pnpm/PnpmComponentDetectorFactory.cs +++ b/src/Microsoft.ComponentDetection.Detectors/pnpm/PnpmComponentDetectorFactory.cs @@ -42,7 +42,7 @@ public PnpmComponentDetectorFactory( public override IEnumerable SupportedComponentTypes { get; } = [ComponentType.Npm]; - public override int Version { get; } = 8; + public override int Version { get; } = 9; public override bool NeedsAutomaticRootDependencyCalculation => true; diff --git a/test/Microsoft.ComponentDetection.Detectors.Tests/PnpmDetectorTests.cs b/test/Microsoft.ComponentDetection.Detectors.Tests/PnpmDetectorTests.cs index de0f977e2..a0ee3bd1c 100644 --- a/test/Microsoft.ComponentDetection.Detectors.Tests/PnpmDetectorTests.cs +++ b/test/Microsoft.ComponentDetection.Detectors.Tests/PnpmDetectorTests.cs @@ -1048,4 +1048,152 @@ public async Task TestPnpmDetector_V9_GoodLockVersion_MissingSnapshotsSuccess() var detectedComponents = componentRecorder.GetDetectedComponents(); detectedComponents.Should().BeEmpty(); } + + [TestMethod] + public async Task TestPnpmDetector_V9_MultiDocumentLockfileAsync() + { + var yamlFile = @" +--- +lockfileVersion: '9.0' + +importers: + .: + configDependencies: {} + packageManagerDependencies: + pnpm: + specifier: 12.3.4 + version: 12.3.4 + +packages: + pnpm@12.3.4: + resolution: {integrity: sha512-placeholder} + +snapshots: + pnpm@12.3.4: {} + +--- +lockfileVersion: '9.0' + +settings: + autoInstallPeers: false + +importers: + .: + dependencies: + fast-uri: + specifier: 3.1.7 + version: 3.1.7 + +packages: + fast-uri@3.1.7: + resolution: {integrity: sha512-placeholder} + +snapshots: + fast-uri@3.1.7: {} +"; + + 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(2); + var npmComponents = detectedComponents.Select(x => new { Component = x.Component as NpmComponent, DetectedComponent = x }); + npmComponents.Should().Contain(x => x.Component.Name == "pnpm" && x.Component.Version == "12.3.4"); + npmComponents.Should().Contain(x => x.Component.Name == "fast-uri" && x.Component.Version == "3.1.7"); + } + + [TestMethod] + public async Task TestPnpmDetector_MultiDocumentLockfile_InconsistentVersions_FailsAsync() + { + var yamlFile = @" +--- +lockfileVersion: '9.0' + +importers: + .: + dependencies: + pnpm: + specifier: 12.3.4 + version: 12.3.4 + +packages: + pnpm@12.3.4: + resolution: {integrity: sha512-placeholder} + +snapshots: + pnpm@12.3.4: {} + +--- +lockfileVersion: '6.0' + +importers: + .: + dependencies: + fast-uri: + specifier: 3.1.7 + version: 3.1.7 + +packages: + /fast-uri@3.1.7: + resolution: {integrity: sha512-placeholder} +"; + + var (scanResult, componentRecorder) = await this.detectorTestUtility + .WithFile("pnpm-lock.yaml", yamlFile) + .ExecuteDetectorAsync(); + + scanResult.ResultCode.Should().Be(ProcessingResultCode.Success); + + var detectedComponents = componentRecorder.GetDetectedComponents(); + detectedComponents.Should().BeEmpty(); + } + + [TestMethod] + public async Task TestPnpmDetector_V6_MultiDocumentLockfileAsync() + { + var yamlFile = @" +--- +lockfileVersion: '6.0' + +importers: + .: + packageManagerDependencies: + pnpm: + specifier: 12.3.4 + version: 12.3.4 + +packages: + /pnpm@12.3.4: + resolution: {integrity: sha512-placeholder} + +--- +lockfileVersion: '6.0' + +importers: + .: + dependencies: + fast-uri: + specifier: 3.1.7 + version: 3.1.7 + +packages: + /fast-uri@3.1.7: + resolution: {integrity: sha512-placeholder} +"; + + 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(2); + var npmComponents = detectedComponents.Select(x => new { Component = x.Component as NpmComponent, DetectedComponent = x }); + npmComponents.Should().Contain(x => x.Component.Name == "pnpm" && x.Component.Version == "12.3.4"); + npmComponents.Should().Contain(x => x.Component.Name == "fast-uri" && x.Component.Version == "3.1.7"); + } } diff --git a/test/Microsoft.ComponentDetection.Detectors.Tests/PnpmParsingUtilitiesTest.cs b/test/Microsoft.ComponentDetection.Detectors.Tests/PnpmParsingUtilitiesTest.cs index 2965f857b..ec73bd583 100644 --- a/test/Microsoft.ComponentDetection.Detectors.Tests/PnpmParsingUtilitiesTest.cs +++ b/test/Microsoft.ComponentDetection.Detectors.Tests/PnpmParsingUtilitiesTest.cs @@ -186,4 +186,60 @@ public void ReconstructPnpmDependencyPathV6() // Absolute path: pnpmParsingUtilities.ReconstructPnpmDependencyPath("events_pkg", "/events@3.3.0").Should().BeEquivalentTo("/events@3.3.0"); } + + [TestMethod] + public void DeserializePnpmYamlFileVersion_MultiDocument_ConsistentVersions_ReturnsVersion() + { + var yamlFile = @" +--- +lockfileVersion: '9.0' +--- +lockfileVersion: '9.0' +"; + var version = PnpmParsingUtilitiesFactory.DeserializePnpmYamlFileVersion(yamlFile); + version.Should().Be("9.0"); + } + + [TestMethod] + public void DeserializePnpmYamlFileVersion_MultiDocument_InconsistentVersions_ThrowsInvalidOperationException() + { + var yamlFile = @" +--- +lockfileVersion: '9.0' +--- +lockfileVersion: '6.0' +"; + var action = () => PnpmParsingUtilitiesFactory.DeserializePnpmYamlFileVersion(yamlFile); + action.Should().Throw() + .WithMessage("*Inconsistent lockfile versions*"); + } + + [TestMethod] + public void DeserializePnpmYamlFileDocuments_ReturnsAllDocuments() + { + var yamlFile = @" +--- +lockfileVersion: '9.0' +importers: + .: + packageManagerDependencies: + pnpm: + specifier: 12.3.4 + version: 12.3.4 +--- +lockfileVersion: '9.0' +importers: + .: + dependencies: + fast-uri: + specifier: 3.1.7 + version: 3.1.7 +"; + var pnpmParsingUtilities = PnpmParsingUtilitiesFactory.Create(); + var documents = pnpmParsingUtilities.DeserializePnpmYamlFileDocuments(yamlFile); + + documents.Should().HaveCount(2); + documents[0].Importers["."].PackageManagerDependencies.Should().ContainKey("pnpm"); + documents[1].Importers["."].Dependencies.Should().ContainKey("fast-uri"); + } } From ba8e86830d87478f5e8d2cec6246469085e78ba5 Mon Sep 17 00:00:00 2001 From: RKS Date: Thu, 17 Sep 2026 07:35:42 -0400 Subject: [PATCH 02/15] test: add v5 multi-document test and dependency-edge assertions for v6 and v9 --- .../PnpmDetectorTests.cs | 128 ++++++++++++++++-- 1 file changed, 120 insertions(+), 8 deletions(-) diff --git a/test/Microsoft.ComponentDetection.Detectors.Tests/PnpmDetectorTests.cs b/test/Microsoft.ComponentDetection.Detectors.Tests/PnpmDetectorTests.cs index a0ee3bd1c..229b96922 100644 --- a/test/Microsoft.ComponentDetection.Detectors.Tests/PnpmDetectorTests.cs +++ b/test/Microsoft.ComponentDetection.Detectors.Tests/PnpmDetectorTests.cs @@ -1067,9 +1067,14 @@ public async Task TestPnpmDetector_V9_MultiDocumentLockfileAsync() packages: pnpm@12.3.4: resolution: {integrity: sha512-placeholder} + pnpm-dep@1.0.0: + resolution: {integrity: sha512-placeholder} snapshots: - pnpm@12.3.4: {} + pnpm@12.3.4: + dependencies: + pnpm-dep: 1.0.0 + pnpm-dep@1.0.0: {} --- lockfileVersion: '9.0' @@ -1087,9 +1092,14 @@ public async Task TestPnpmDetector_V9_MultiDocumentLockfileAsync() packages: fast-uri@3.1.7: resolution: {integrity: sha512-placeholder} + uri-dep@2.0.0: + resolution: {integrity: sha512-placeholder} snapshots: - fast-uri@3.1.7: {} + fast-uri@3.1.7: + dependencies: + uri-dep: 2.0.0 + uri-dep@2.0.0: {} "; var (scanResult, componentRecorder) = await this.detectorTestUtility @@ -1098,11 +1108,31 @@ public async Task TestPnpmDetector_V9_MultiDocumentLockfileAsync() scanResult.ResultCode.Should().Be(ProcessingResultCode.Success); - var detectedComponents = componentRecorder.GetDetectedComponents(); - detectedComponents.Should().HaveCount(2); - var npmComponents = detectedComponents.Select(x => new { Component = x.Component as NpmComponent, DetectedComponent = x }); + var detectedComponents = componentRecorder.GetDetectedComponents().ToList(); + detectedComponents.Should().HaveCount(4); + var npmComponents = detectedComponents.Select(x => new { Component = (NpmComponent)x.Component, DetectedComponent = x }).ToList(); npmComponents.Should().Contain(x => x.Component.Name == "pnpm" && x.Component.Version == "12.3.4"); + npmComponents.Should().Contain(x => x.Component.Name == "pnpm-dep" && x.Component.Version == "1.0.0"); npmComponents.Should().Contain(x => x.Component.Name == "fast-uri" && x.Component.Version == "3.1.7"); + npmComponents.Should().Contain(x => x.Component.Name == "uri-dep" && x.Component.Version == "2.0.0"); + + var pnpm = detectedComponents.Single(c => ((NpmComponent)c.Component).Name == "pnpm"); + var pnpmDep = detectedComponents.Single(c => ((NpmComponent)c.Component).Name == "pnpm-dep"); + var fastUri = detectedComponents.Single(c => ((NpmComponent)c.Component).Name == "fast-uri"); + var uriDep = detectedComponents.Single(c => ((NpmComponent)c.Component).Name == "uri-dep"); + + var graph = componentRecorder.GetDependencyGraphsByLocation().Values.First(); + graph.IsComponentExplicitlyReferenced(pnpm.Component.Id).Should().BeTrue(); + graph.IsComponentExplicitlyReferenced(fastUri.Component.Id).Should().BeTrue(); + graph.GetDependenciesForComponent(pnpm.Component.Id).Should().Contain(pnpmDep.Component.Id); + graph.GetDependenciesForComponent(fastUri.Component.Id).Should().Contain(uriDep.Component.Id); + + componentRecorder.AssertAllExplicitlyReferencedComponents( + pnpmDep.Component.Id, + parent => parent.Name == "pnpm" && parent.Version == "12.3.4"); + componentRecorder.AssertAllExplicitlyReferencedComponents( + uriDep.Component.Id, + parent => parent.Name == "fast-uri" && parent.Version == "3.1.7"); } [TestMethod] @@ -1168,6 +1198,10 @@ public async Task TestPnpmDetector_V6_MultiDocumentLockfileAsync() packages: /pnpm@12.3.4: resolution: {integrity: sha512-placeholder} + dependencies: + pnpm-dep: 1.0.0 + /pnpm-dep@1.0.0: + resolution: {integrity: sha512-placeholder} --- lockfileVersion: '6.0' @@ -1182,6 +1216,10 @@ public async Task TestPnpmDetector_V6_MultiDocumentLockfileAsync() packages: /fast-uri@3.1.7: resolution: {integrity: sha512-placeholder} + dependencies: + uri-dep: 2.0.0 + /uri-dep@2.0.0: + resolution: {integrity: sha512-placeholder} "; var (scanResult, componentRecorder) = await this.detectorTestUtility @@ -1190,10 +1228,84 @@ public async Task TestPnpmDetector_V6_MultiDocumentLockfileAsync() scanResult.ResultCode.Should().Be(ProcessingResultCode.Success); - var detectedComponents = componentRecorder.GetDetectedComponents(); - detectedComponents.Should().HaveCount(2); - var npmComponents = detectedComponents.Select(x => new { Component = x.Component as NpmComponent, DetectedComponent = x }); + var detectedComponents = componentRecorder.GetDetectedComponents().ToList(); + detectedComponents.Should().HaveCount(4); + var npmComponents = detectedComponents.Select(x => new { Component = (NpmComponent)x.Component, DetectedComponent = x }).ToList(); npmComponents.Should().Contain(x => x.Component.Name == "pnpm" && x.Component.Version == "12.3.4"); + npmComponents.Should().Contain(x => x.Component.Name == "pnpm-dep" && x.Component.Version == "1.0.0"); npmComponents.Should().Contain(x => x.Component.Name == "fast-uri" && x.Component.Version == "3.1.7"); + npmComponents.Should().Contain(x => x.Component.Name == "uri-dep" && x.Component.Version == "2.0.0"); + + var pnpm = detectedComponents.Single(c => ((NpmComponent)c.Component).Name == "pnpm"); + var pnpmDep = detectedComponents.Single(c => ((NpmComponent)c.Component).Name == "pnpm-dep"); + var fastUri = detectedComponents.Single(c => ((NpmComponent)c.Component).Name == "fast-uri"); + var uriDep = detectedComponents.Single(c => ((NpmComponent)c.Component).Name == "uri-dep"); + + var graph = componentRecorder.GetDependencyGraphsByLocation().Values.First(); + graph.IsComponentExplicitlyReferenced(pnpm.Component.Id).Should().BeTrue(); + graph.IsComponentExplicitlyReferenced(fastUri.Component.Id).Should().BeTrue(); + graph.GetDependenciesForComponent(pnpm.Component.Id).Should().Contain(pnpmDep.Component.Id); + graph.GetDependenciesForComponent(fastUri.Component.Id).Should().Contain(uriDep.Component.Id); + } + + [TestMethod] + public async Task TestPnpmDetector_V5_MultiDocumentLockfileAsync() + { + var yamlFile = @" +--- +dependencies: + root-a: 1.0.0 + +packages: + /root-a/1.0.0: + dependencies: + dep-a: 1.1.0 + dev: false + /dep-a/1.1.0: + dev: false + +--- +dependencies: + root-b: 2.0.0 + +packages: + /root-b/2.0.0: + dependencies: + dep-b: 2.2.0 + dev: true + /dep-b/2.2.0: + dev: true +"; + + var (scanResult, componentRecorder) = await this.detectorTestUtility + .WithFile("shrinkwrap.yaml", yamlFile) + .ExecuteDetectorAsync(); + + scanResult.ResultCode.Should().Be(ProcessingResultCode.Success); + + var detectedComponents = componentRecorder.GetDetectedComponents().ToList(); + detectedComponents.Should().HaveCount(4); + var npmComponents = detectedComponents.Select(x => new { Component = (NpmComponent)x.Component, DetectedComponent = x }).ToList(); + npmComponents.Should().Contain(x => x.Component.Name == "root-a" && x.Component.Version == "1.0.0"); + npmComponents.Should().Contain(x => x.Component.Name == "dep-a" && x.Component.Version == "1.1.0"); + npmComponents.Should().Contain(x => x.Component.Name == "root-b" && x.Component.Version == "2.0.0"); + npmComponents.Should().Contain(x => x.Component.Name == "dep-b" && x.Component.Version == "2.2.0"); + + var rootA = detectedComponents.Single(c => ((NpmComponent)c.Component).Name == "root-a"); + var depA = detectedComponents.Single(c => ((NpmComponent)c.Component).Name == "dep-a"); + var rootB = detectedComponents.Single(c => ((NpmComponent)c.Component).Name == "root-b"); + var depB = detectedComponents.Single(c => ((NpmComponent)c.Component).Name == "dep-b"); + + componentRecorder.AssertAllExplicitlyReferencedComponents( + depA.Component.Id, + parent => parent.Name == "root-a" && parent.Version == "1.0.0"); + componentRecorder.AssertAllExplicitlyReferencedComponents( + depB.Component.Id, + parent => parent.Name == "root-b" && parent.Version == "2.0.0"); + + componentRecorder.GetEffectiveDevDependencyValue(rootA.Component.Id).GetValueOrDefault(true).Should().BeFalse(); + componentRecorder.GetEffectiveDevDependencyValue(depA.Component.Id).GetValueOrDefault(true).Should().BeFalse(); + componentRecorder.GetEffectiveDevDependencyValue(rootB.Component.Id).GetValueOrDefault(false).Should().BeTrue(); + componentRecorder.GetEffectiveDevDependencyValue(depB.Component.Id).GetValueOrDefault(false).Should().BeTrue(); } } From 506b4fc93fa92baf0c4d2700f84db389fef11edd Mon Sep 17 00:00:00 2001 From: RKS Date: Thu, 17 Sep 2026 07:58:02 -0400 Subject: [PATCH 03/15] fix: merge cross-document package metadata and add non-empty configDependencies assertions - In Pnpm6Detector, merge package metadata across documents so production occurrences take precedence over development occurrences, and merge declared child dependencies across documents. - In Pnpm9Detector, merge snapshot dependencies across documents when the same snapshot path occurs across multiple documents. - In PnpmDetectorTests, exercise non-empty configDependencies for v6 and v9 multi-document scenarios and assert explicit reference status. - Add TestPnpmDetector_V6_MultiDocumentLockfile_DuplicatePackage_ProductionWinsAsync to verify dev vs prod resolution and merged dependency edges. --- .../pnpm/Pnpm6Detector.cs | 31 +++++- .../pnpm/Pnpm9Detector.cs | 19 +++- .../PnpmDetectorTests.cs | 96 ++++++++++++++++++- 3 files changed, 141 insertions(+), 5 deletions(-) diff --git a/src/Microsoft.ComponentDetection.Detectors/pnpm/Pnpm6Detector.cs b/src/Microsoft.ComponentDetection.Detectors/pnpm/Pnpm6Detector.cs index f0b1de7b3..09cec107d 100644 --- a/src/Microsoft.ComponentDetection.Detectors/pnpm/Pnpm6Detector.cs +++ b/src/Microsoft.ComponentDetection.Detectors/pnpm/Pnpm6Detector.cs @@ -33,7 +33,7 @@ public void RecordDependencyGraphFromFile(string yamlFileContent, ISingleFileCom continue; } - if (!components.ContainsKey(pnpmDependencyPath)) + if (!components.TryGetValue(pnpmDependencyPath, out var existing)) { var parentDetectedComponent = this.pnpmParsingUtilities.CreateDetectedComponentFromPnpmPath(pnpmPackagePath: pnpmDependencyPath); components.Add(pnpmDependencyPath, (parentDetectedComponent, package)); @@ -44,6 +44,35 @@ public void RecordDependencyGraphFromFile(string yamlFileContent, ISingleFileCom // like skipping local dependencies which might have transitively depended on this. singleFileComponentRecorder.RegisterUsage(parentDetectedComponent, isDevelopmentDependency: this.pnpmParsingUtilities.IsPnpmPackageDevDependency(package)); } + else + { + // If the same package path occurs across multiple documents (e.g. environment and project documents), + // merge package metadata and dependencies, letting a production occurrence win for development classification. + var existingIsDev = this.pnpmParsingUtilities.IsPnpmPackageDevDependency(existing.Item2); + var newIsDev = this.pnpmParsingUtilities.IsPnpmPackageDevDependency(package); + var effectiveIsDev = existingIsDev && newIsDev; + + if (existingIsDev && !effectiveIsDev) + { + existing.Item2.Dev = bool.FalseString; + singleFileComponentRecorder.RegisterUsage(existing.Item1, isDevelopmentDependency: false); + } + + if (package.Dependencies != null) + { + if (existing.Item2.Dependencies == null) + { + existing.Item2.Dependencies = new Dictionary(package.Dependencies); + } + else + { + foreach (var (name, version) in package.Dependencies) + { + existing.Item2.Dependencies.TryAdd(name, version); + } + } + } + } } } diff --git a/src/Microsoft.ComponentDetection.Detectors/pnpm/Pnpm9Detector.cs b/src/Microsoft.ComponentDetection.Detectors/pnpm/Pnpm9Detector.cs index cbbaf465f..f7e2b0c16 100644 --- a/src/Microsoft.ComponentDetection.Detectors/pnpm/Pnpm9Detector.cs +++ b/src/Microsoft.ComponentDetection.Detectors/pnpm/Pnpm9Detector.cs @@ -42,7 +42,7 @@ public void RecordDependencyGraphFromFile(string yamlFileContent, ISingleFileCom dependencyPath = pnpmDependencyPath[1..]; } - if (!components.ContainsKey(dependencyPath)) + if (!components.TryGetValue(dependencyPath, out var existing)) { var parentDetectedComponent = this.pnpmParsingUtilities.CreateDetectedComponentFromPnpmPath(pnpmPackagePath: dependencyPath); components.Add(dependencyPath, (parentDetectedComponent, package)); @@ -56,6 +56,23 @@ public void RecordDependencyGraphFromFile(string yamlFileContent, ISingleFileCom singleFileComponentRecorder.RegisterUsage(parentDetectedComponent); } } + else + { + if (package.Dependencies != null) + { + if (existing.Item2.Dependencies == null) + { + existing.Item2.Dependencies = new Dictionary(package.Dependencies); + } + else + { + foreach (var (name, version) in package.Dependencies) + { + existing.Item2.Dependencies.TryAdd(name, version); + } + } + } + } } } diff --git a/test/Microsoft.ComponentDetection.Detectors.Tests/PnpmDetectorTests.cs b/test/Microsoft.ComponentDetection.Detectors.Tests/PnpmDetectorTests.cs index 229b96922..9e9739cc3 100644 --- a/test/Microsoft.ComponentDetection.Detectors.Tests/PnpmDetectorTests.cs +++ b/test/Microsoft.ComponentDetection.Detectors.Tests/PnpmDetectorTests.cs @@ -1058,19 +1058,25 @@ public async Task TestPnpmDetector_V9_MultiDocumentLockfileAsync() importers: .: - configDependencies: {} + configDependencies: + config-pkg: + specifier: 1.0.0 + version: 1.0.0 packageManagerDependencies: pnpm: specifier: 12.3.4 version: 12.3.4 packages: + config-pkg@1.0.0: + resolution: {integrity: sha512-placeholder} pnpm@12.3.4: resolution: {integrity: sha512-placeholder} pnpm-dep@1.0.0: resolution: {integrity: sha512-placeholder} snapshots: + config-pkg@1.0.0: {} pnpm@12.3.4: dependencies: pnpm-dep: 1.0.0 @@ -1109,19 +1115,22 @@ public async Task TestPnpmDetector_V9_MultiDocumentLockfileAsync() scanResult.ResultCode.Should().Be(ProcessingResultCode.Success); var detectedComponents = componentRecorder.GetDetectedComponents().ToList(); - detectedComponents.Should().HaveCount(4); + detectedComponents.Should().HaveCount(5); var npmComponents = detectedComponents.Select(x => new { Component = (NpmComponent)x.Component, DetectedComponent = x }).ToList(); + npmComponents.Should().Contain(x => x.Component.Name == "config-pkg" && x.Component.Version == "1.0.0"); npmComponents.Should().Contain(x => x.Component.Name == "pnpm" && x.Component.Version == "12.3.4"); npmComponents.Should().Contain(x => x.Component.Name == "pnpm-dep" && x.Component.Version == "1.0.0"); npmComponents.Should().Contain(x => x.Component.Name == "fast-uri" && x.Component.Version == "3.1.7"); npmComponents.Should().Contain(x => x.Component.Name == "uri-dep" && x.Component.Version == "2.0.0"); + var configPkg = detectedComponents.Single(c => ((NpmComponent)c.Component).Name == "config-pkg"); var pnpm = detectedComponents.Single(c => ((NpmComponent)c.Component).Name == "pnpm"); var pnpmDep = detectedComponents.Single(c => ((NpmComponent)c.Component).Name == "pnpm-dep"); var fastUri = detectedComponents.Single(c => ((NpmComponent)c.Component).Name == "fast-uri"); var uriDep = detectedComponents.Single(c => ((NpmComponent)c.Component).Name == "uri-dep"); var graph = componentRecorder.GetDependencyGraphsByLocation().Values.First(); + graph.IsComponentExplicitlyReferenced(configPkg.Component.Id).Should().BeTrue(); graph.IsComponentExplicitlyReferenced(pnpm.Component.Id).Should().BeTrue(); graph.IsComponentExplicitlyReferenced(fastUri.Component.Id).Should().BeTrue(); graph.GetDependenciesForComponent(pnpm.Component.Id).Should().Contain(pnpmDep.Component.Id); @@ -1190,12 +1199,18 @@ public async Task TestPnpmDetector_V6_MultiDocumentLockfileAsync() importers: .: + configDependencies: + config-pkg: + specifier: 1.0.0 + version: 1.0.0 packageManagerDependencies: pnpm: specifier: 12.3.4 version: 12.3.4 packages: + /config-pkg@1.0.0: + resolution: {integrity: sha512-placeholder} /pnpm@12.3.4: resolution: {integrity: sha512-placeholder} dependencies: @@ -1229,25 +1244,100 @@ public async Task TestPnpmDetector_V6_MultiDocumentLockfileAsync() scanResult.ResultCode.Should().Be(ProcessingResultCode.Success); var detectedComponents = componentRecorder.GetDetectedComponents().ToList(); - detectedComponents.Should().HaveCount(4); + detectedComponents.Should().HaveCount(5); var npmComponents = detectedComponents.Select(x => new { Component = (NpmComponent)x.Component, DetectedComponent = x }).ToList(); + npmComponents.Should().Contain(x => x.Component.Name == "config-pkg" && x.Component.Version == "1.0.0"); npmComponents.Should().Contain(x => x.Component.Name == "pnpm" && x.Component.Version == "12.3.4"); npmComponents.Should().Contain(x => x.Component.Name == "pnpm-dep" && x.Component.Version == "1.0.0"); npmComponents.Should().Contain(x => x.Component.Name == "fast-uri" && x.Component.Version == "3.1.7"); npmComponents.Should().Contain(x => x.Component.Name == "uri-dep" && x.Component.Version == "2.0.0"); + var configPkg = detectedComponents.Single(c => ((NpmComponent)c.Component).Name == "config-pkg"); var pnpm = detectedComponents.Single(c => ((NpmComponent)c.Component).Name == "pnpm"); var pnpmDep = detectedComponents.Single(c => ((NpmComponent)c.Component).Name == "pnpm-dep"); var fastUri = detectedComponents.Single(c => ((NpmComponent)c.Component).Name == "fast-uri"); var uriDep = detectedComponents.Single(c => ((NpmComponent)c.Component).Name == "uri-dep"); var graph = componentRecorder.GetDependencyGraphsByLocation().Values.First(); + graph.IsComponentExplicitlyReferenced(configPkg.Component.Id).Should().BeTrue(); graph.IsComponentExplicitlyReferenced(pnpm.Component.Id).Should().BeTrue(); graph.IsComponentExplicitlyReferenced(fastUri.Component.Id).Should().BeTrue(); graph.GetDependenciesForComponent(pnpm.Component.Id).Should().Contain(pnpmDep.Component.Id); graph.GetDependenciesForComponent(fastUri.Component.Id).Should().Contain(uriDep.Component.Id); } + [TestMethod] + public async Task TestPnpmDetector_V6_MultiDocumentLockfile_DuplicatePackage_ProductionWinsAsync() + { + var yamlFile = @" +--- +lockfileVersion: '6.0' + +importers: + .: + devDependencies: + shared-pkg: + specifier: 1.0.0 + version: 1.0.0 + +packages: + /shared-pkg@1.0.0: + resolution: {integrity: sha512-placeholder} + dev: true + dependencies: + dep-from-doc1: 1.0.0 + /dep-from-doc1@1.0.0: + resolution: {integrity: sha512-placeholder} + dev: true + +--- +lockfileVersion: '6.0' + +importers: + .: + dependencies: + shared-pkg: + specifier: 1.0.0 + version: 1.0.0 + +packages: + /shared-pkg@1.0.0: + resolution: {integrity: sha512-placeholder} + dependencies: + dep-from-doc2: 2.0.0 + /dep-from-doc2@2.0.0: + resolution: {integrity: sha512-placeholder} +"; + + var (scanResult, componentRecorder) = await this.detectorTestUtility + .WithFile("pnpm-lock.yaml", yamlFile) + .ExecuteDetectorAsync(); + + scanResult.ResultCode.Should().Be(ProcessingResultCode.Success); + + var detectedComponents = componentRecorder.GetDetectedComponents().ToList(); + detectedComponents.Should().HaveCount(3); + var npmComponents = detectedComponents.Select(x => new { Component = (NpmComponent)x.Component, DetectedComponent = x }).ToList(); + npmComponents.Should().Contain(x => x.Component.Name == "shared-pkg" && x.Component.Version == "1.0.0"); + npmComponents.Should().Contain(x => x.Component.Name == "dep-from-doc1" && x.Component.Version == "1.0.0"); + npmComponents.Should().Contain(x => x.Component.Name == "dep-from-doc2" && x.Component.Version == "2.0.0"); + + var sharedPkg = detectedComponents.Single(c => ((NpmComponent)c.Component).Name == "shared-pkg"); + var dep1 = detectedComponents.Single(c => ((NpmComponent)c.Component).Name == "dep-from-doc1"); + var dep2 = detectedComponents.Single(c => ((NpmComponent)c.Component).Name == "dep-from-doc2"); + + // Production occurrence from document 2 wins over dev occurrence from document 1 + componentRecorder.GetEffectiveDevDependencyValue(sharedPkg.Component.Id).Should().BeFalse(); + + var graph = componentRecorder.GetDependencyGraphsByLocation().Values.First(); + graph.IsComponentExplicitlyReferenced(sharedPkg.Component.Id).Should().BeTrue(); + + // Dependencies merged from both documents + var dependencies = graph.GetDependenciesForComponent(sharedPkg.Component.Id).ToList(); + dependencies.Should().Contain(dep1.Component.Id); + dependencies.Should().Contain(dep2.Component.Id); + } + [TestMethod] public async Task TestPnpmDetector_V5_MultiDocumentLockfileAsync() { From 7ecd1db7c379fa13b8260cdcb26325edd4fc42e1 Mon Sep 17 00:00:00 2001 From: RKS Date: Thu, 17 Sep 2026 08:07:51 -0400 Subject: [PATCH 04/15] fix: preserve indexed lookup for non-local dependencies in pnpm v6 and v9 Retain direct indexed dictionary lookup components[pnpmDependencyPath] when walking non-local package and indirect dependencies in Pnpm6Detector and Pnpm9Detector so that unresolved references in malformed lockfiles continue to fail fast rather than silently producing an incomplete dependency graph. --- .../pnpm/Pnpm6Detector.cs | 14 ++++---------- .../pnpm/Pnpm9Detector.cs | 14 ++------------ 2 files changed, 6 insertions(+), 22 deletions(-) diff --git a/src/Microsoft.ComponentDetection.Detectors/pnpm/Pnpm6Detector.cs b/src/Microsoft.ComponentDetection.Detectors/pnpm/Pnpm6Detector.cs index 09cec107d..fb72d8a9d 100644 --- a/src/Microsoft.ComponentDetection.Detectors/pnpm/Pnpm6Detector.cs +++ b/src/Microsoft.ComponentDetection.Detectors/pnpm/Pnpm6Detector.cs @@ -84,10 +84,9 @@ public void RecordDependencyGraphFromFile(string yamlFileContent, ISingleFileCom var pnpmDependencyPath = this.pnpmParsingUtilities.ReconstructPnpmDependencyPath(name, version); // If this lookup fails, then pnpmDependencyPath was either parsed incorrectly or constructed incorrectly. - if (components.TryGetValue(pnpmDependencyPath, out var referencedTuple)) - { - singleFileComponentRecorder.RegisterUsage(referencedTuple.Item1, parentComponentId: component.Component.Id, isExplicitReferencedDependency: false); - } + var (referenced, _) = components[pnpmDependencyPath]; + + singleFileComponentRecorder.RegisterUsage(referenced, parentComponentId: component.Component.Id, isExplicitReferencedDependency: false); } } @@ -125,12 +124,7 @@ private void ProcessDependencyList(ISingleFileComponentRecorder singleFileCompon } var pnpmDependencyPath = this.pnpmParsingUtilities.ReconstructPnpmDependencyPath(name, dep.Version); - if (!components.TryGetValue(pnpmDependencyPath, out var componentAndPackage)) - { - continue; - } - - var (component, package) = componentAndPackage; + var (component, package) = components[pnpmDependencyPath]; // Determine isDevelopmentDependency using metadata on package from pnpm rather than from which dependency list this package is under. // This ensures that dependencies which are a direct dev dependency and an indirect non-dev dependency get listed as non-dev. diff --git a/src/Microsoft.ComponentDetection.Detectors/pnpm/Pnpm9Detector.cs b/src/Microsoft.ComponentDetection.Detectors/pnpm/Pnpm9Detector.cs index f7e2b0c16..1ee378222 100644 --- a/src/Microsoft.ComponentDetection.Detectors/pnpm/Pnpm9Detector.cs +++ b/src/Microsoft.ComponentDetection.Detectors/pnpm/Pnpm9Detector.cs @@ -110,12 +110,7 @@ private void ProcessDependencyList(ISingleFileComponentRecorder singleFileCompon continue; } - if (!components.TryGetValue(pnpmDependencyPath, out var componentAndPackage)) - { - continue; - } - - var (component, package) = componentAndPackage; + 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. @@ -156,12 +151,7 @@ private void ProcessIndirectDependencies(ISingleFileComponentRecorder singleFile } // If this lookup fails, then pnpmDependencyPath was either parsed incorrectly or constructed incorrectly. - if (!components.TryGetValue(pnpmDependencyPath, out var componentAndPackage)) - { - continue; - } - - var (component, package) = componentAndPackage; + var (component, package) = components[pnpmDependencyPath]; singleFileComponentRecorder.RegisterUsage(component, parentComponentId: parentComponentId, isExplicitReferencedDependency: false, isDevelopmentDependency: isDevDependency); seenDependencies.Add(pnpmDependencyPath); this.ProcessIndirectDependencies(singleFileComponentRecorder, components, component.Component.Id, package.Dependencies, isDevDependency, seenDependencies); From 48153df9ff55bf8320354df29f155ced1022dfb7 Mon Sep 17 00:00:00 2001 From: Ryan Brandenburg Date: Thu, 17 Sep 2026 10:24:08 -0700 Subject: [PATCH 05/15] Docker Ping retry delay (#1872) --- .../DockerServiceTests.cs | 6 +++++- .../Microsoft.ComponentDetection.Common.Tests.csproj | 1 + 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/test/Microsoft.ComponentDetection.Common.Tests/DockerServiceTests.cs b/test/Microsoft.ComponentDetection.Common.Tests/DockerServiceTests.cs index 23be8fc12..b8ae3c34a 100644 --- a/test/Microsoft.ComponentDetection.Common.Tests/DockerServiceTests.cs +++ b/test/Microsoft.ComponentDetection.Common.Tests/DockerServiceTests.cs @@ -8,6 +8,7 @@ namespace Microsoft.ComponentDetection.Common.Tests; using Microsoft.Extensions.Logging; using Microsoft.VisualStudio.TestTools.UnitTesting; using Moq; +using Polly; [TestClass] [TestCategory("Governance/All")] @@ -26,7 +27,10 @@ public class DockerServiceTests [TestMethod] public async Task DockerService_CanPingDockerAsync() { - var canPingDocker = await this.dockerService.CanPingDockerAsync(); + var canPingDocker = await Policy.HandleResult(result => !result) + .WaitAndRetryAsync(3, retryAttempt => TimeSpan.FromSeconds(5)) + .ExecuteAsync(() => this.dockerService.CanPingDockerAsync()); + canPingDocker.Should().BeTrue(); } diff --git a/test/Microsoft.ComponentDetection.Common.Tests/Microsoft.ComponentDetection.Common.Tests.csproj b/test/Microsoft.ComponentDetection.Common.Tests/Microsoft.ComponentDetection.Common.Tests.csproj index 496f5be8c..8d4ae758e 100644 --- a/test/Microsoft.ComponentDetection.Common.Tests/Microsoft.ComponentDetection.Common.Tests.csproj +++ b/test/Microsoft.ComponentDetection.Common.Tests/Microsoft.ComponentDetection.Common.Tests.csproj @@ -9,6 +9,7 @@ + From a1d339b0dfcdb21c489685042d7cd8328d9ac50e Mon Sep 17 00:00:00 2001 From: Greg Villicana <58237075+grvillic@users.noreply.github.com> Date: Thu, 17 Sep 2026 17:48:32 -0700 Subject: [PATCH 06/15] Promote MSBuildBinaryLog Detector to prod (#1873) * Promote MSBuildBinaryLog Detector to prod --- README.md | 2 +- docs/detectors/README.md | 9 +-- docs/detectors/dotnet.md | 48 ------------- docs/detectors/nuget.md | 71 +++++++++++------- .../dotnet/DotNetComponentDetector.cs | 72 ------------------- .../dotnet/DotNetProjectInfoProvider.cs | 7 +- .../nuget/LockFileUtilities.cs | 6 +- .../MSBuildBinaryLogComponentDetector.cs | 16 ++++- ...ectModelProjectCentricComponentDetector.cs | 66 ----------------- .../LinuxApplicationLayerExperiment.cs | 4 +- .../Configs/MSBuildBinaryLogExperiment.cs | 27 ------- .../Extensions/ServiceCollectionExtensions.cs | 6 -- .../Services/DetectorRestrictionService.cs | 38 +++++----- .../ComponentDetectorTests.cs | 10 +++ ...aryLogDotNetFallbackCompatibilityTests.cs} | 67 ++++++++--------- ...ildBinaryLogFallbackCompatibilityTests.cs} | 65 +++++++++++------ .../MSBuildBinaryLogComponentDetectorTests.cs | 63 ++++++++++++++-- .../LinuxApplicationLayerExperimentTests.cs | 21 ++++-- .../DetectorRestrictionServiceTests.cs | 33 ++++++++- .../ComponentDetectionIntegrationTests.cs | 8 +-- 20 files changed, 286 insertions(+), 353 deletions(-) delete mode 100644 docs/detectors/dotnet.md delete mode 100644 src/Microsoft.ComponentDetection.Detectors/dotnet/DotNetComponentDetector.cs delete mode 100644 src/Microsoft.ComponentDetection.Detectors/nuget/NuGetProjectModelProjectCentricComponentDetector.cs delete mode 100644 src/Microsoft.ComponentDetection.Orchestrator/Experiments/Configs/MSBuildBinaryLogExperiment.cs rename test/Microsoft.ComponentDetection.Detectors.Tests/{DotNetComponentDetectorTests.cs => MSBuildBinaryLogDotNetFallbackCompatibilityTests.cs} (95%) rename test/Microsoft.ComponentDetection.Detectors.Tests/{NuGetProjectModelProjectCentricComponentDetectorTests.cs => MSBuildBinaryLogFallbackCompatibilityTests.cs} (85%) diff --git a/README.md b/README.md index 2d9961ad9..e3b529899 100644 --- a/README.md +++ b/README.md @@ -42,7 +42,7 @@ Component Detection supports detecting libraries from the following ecosystems: | [Conda (Python)](docs/detectors/conda.md) | ✔ | ✔ | | [Docker Compose](docs/detectors/dockercompose.md) | ✔ | ❌ | | [Dockerfile](docs/detectors/dockerfile.md) | ✔ | ❌ | -| [DotNet SDK](docs/detectors/dotnet.md) | ✔ | ❌ | +| [DotNet SDK](docs/detectors/nuget.md#msbuildbinarylog) | ✔ | ❌ | | [Go](docs/detectors/go.md) | ✔ | ✔ (with Go 1.11+) | | [Gradle (lockfiles only)](docs/detectors/gradle.md) | ✔ | ❌ | | [Helm](docs/detectors/helm.md) | ✔ | ❌ | diff --git a/docs/detectors/README.md b/docs/detectors/README.md index 09828f3bd..e39cd1b26 100644 --- a/docs/detectors/README.md +++ b/docs/detectors/README.md @@ -30,12 +30,6 @@ | --------------------------- | ---------- | | DockerfileComponentDetector | Experimental | -- [DotNet](dotnet.md) - -| Detector | Status | -| ----------------------- | ------ | -| DotNetComponentDetector | Stable | - - [Go](go.md) | Detector | Status | @@ -86,8 +80,7 @@ | ------------------------------------------------ | ---------- | | NuGetComponentDetector | Stable | | NuGetPackagesConfigDetector | Stable | -| NuGetProjectModelProjectCentricComponentDetector | Stable | -| MSBuildBinaryLogComponentDetector | Experimental | +| MSBuildBinaryLogComponentDetector | Stable | - [Pip](pip.md) diff --git a/docs/detectors/dotnet.md b/docs/detectors/dotnet.md deleted file mode 100644 index fbdcdf199..000000000 --- a/docs/detectors/dotnet.md +++ /dev/null @@ -1,48 +0,0 @@ -# DotNet SDK Detection - -## Requirements - -DotNet SDK Detection depends on `project.assets.json` files in the project output / intermediates. - -## Detection Strategy - -The `project.assets.json` will be produced when a project is restored and built. From this we can locate -the project file that was built. From the project file location we probe for the .NET SDK version used. -We look up the directory path to find a `global.json`[1] then will run `dotnet --version` in that -directory to determine which version of the .NET SDK it will select. If no `global.json` is found, then -probing will stop when the detector encounters `SourceDirectory`, `SourceFileRoot`, or the root of the drive -and proceed to run `dotnet --version` in that directory. Repositories control the version of the .NET SDK -used to build their project by either preinstalling on their build machine or container, or acquiring during -the build pipeline. The .NET SDK version used is important as this version selects redistributable content -that becomes part of the application (the dotnet host, runtime for self-contained or AOT apps, build tools -which generate source, etc). - -In addition to recording the SDK version used, the detector will report the framework versions targeted by -the project as `TargetFramework` values as well as the type of project `application` or `library`. These -are important because applications may be built to target old framework versions which may be out of support -and have unreported vulnerabilities. `TargetFramework` is determined from the `project.assets.json` while -the type of the project is determined by locating the project's output assembly in a subdirectory of the -output path and reading the PE COFF header's characteristics for `IMAGE_FILE_EXECUTABLE_IMAGE`[2]. - -The `ProjectType` value is further qualified with a `-selfcontained` suffix (e.g. `application-selfcontained` -or `library-selfcontained`) when the project is detected as self-contained. A project is considered -self-contained when either: -- Its `project.assets.json` indicates that a framework reference (e.g. `Microsoft.NETCore.App`) has a - corresponding runtime package download (e.g. `Microsoft.NETCore.App.Runtime.*`) listed in the target - framework's `downloadDependencies`. This covers `SelfContained=true` scenarios. -- The target references `Microsoft.DotNet.ILCompiler`, which indicates native AOT compilation - (`PublishAot=true`) and therefore an implicitly self-contained deployment. - -Self-contained applications bundle the .NET runtime and are responsible for servicing it, so this -distinction is important for vulnerability tracking. - -[1]: https://learn.microsoft.com/en-us/dotnet/core/tools/global-json -[2]: https://learn.microsoft.com/en-us/windows/win32/debug/pe-format#characteristics - -## Known Limitations - -If the `dotnet` executable is not on the path the detector may fail to locate the version used to build the -project. The detector will fallback to parsing the `global.json` in this case if it is present. -Detection of the output type is done by locating the output assembly under the output path specified in -`project.assets.json`. Some build systems may place project intermediates in a different location. In this -case the project type will be reported as `unknown` and the `-selfcontained` suffix will not be appended. \ No newline at end of file diff --git a/docs/detectors/nuget.md b/docs/detectors/nuget.md index 62524b121..14f6f8d40 100644 --- a/docs/detectors/nuget.md +++ b/docs/detectors/nuget.md @@ -8,19 +8,27 @@ NuGet Detection depends on the following to successfully run: - The files each NuGet detector searches for: - [The `NuGet` detector looks for `*.nupkg`, `*.nuspec`, `nuget.config`, `paket.lock`][1] - [The `NuGetPackagesConfig` detector looks for `packages.config`][2] - - [The `NuGetProjectCentric` detector looks for `project.assets.json`][3] + - [The `MSBuildBinaryLog` detector looks for `project.assets.json` and optionally uses `*.binlog` files][3] [1]: https://github.com/microsoft/component-detection/blob/13f3e9f32c94bf6189fbd0bfbdf2e68cc60fccd9/src/Microsoft.ComponentDetection.Detectors/nuget/NuGetComponentDetector.cs#L40 [2]: https://github.com/microsoft/component-detection/blob/13f3e9f32c94bf6189fbd0bfbdf2e68cc60fccd9/src/Microsoft.ComponentDetection.Detectors/nuget/NuGetPackagesConfigDetector.cs#L25 -[3]: https://github.com/microsoft/component-detection/blob/13f3e9f32c94bf6189fbd0bfbdf2e68cc60fccd9/src/Microsoft.ComponentDetection.Detectors/nuget/NuGetProjectModelProjectCentricComponentDetector.cs#L205 +[3]: https://github.com/microsoft/component-detection/blob/main/src/Microsoft.ComponentDetection.Detectors/nuget/MSBuildBinaryLogComponentDetector.cs ## Detection Strategy NuGet Detection is performed by parsing any `*.nuspec`, `*.nupkg`, `*.packages.config`, or `*.project.assets` files found under the scan directory. By searching for all `*.nuspec,` `*.nupkg` files on disk the global NuGet cache gets searched which can include packages that are not included in the final build. -## NuGetProjectCentric +## MSBuildBinaryLog + +The default-on `MSBuildBinaryLog` detector replaces the former `NuGetProjectCentric` and `DotNet` +detectors. It raises NuGet components referenced by projects that use the latest NuGet (v3 or later) +and build-integrated `PackageReference` [items][4], and it records the corresponding DotNet SDK +components. NuGet components represent both direct dependencies and transitive dependencies brought +in by references from direct package and project references. Packages that contribute no assets to +the project or exclusively contribute [Compile assets][5] are treated as development dependencies. -The `NuGetProjectCentric` detector raises NuGet components referenced by projects that use the latest NuGet (v3 or later) and build-integrated `PackageReference` [items][4]. These components represent both direct dependencies and transitive dependencies brought in by references from direct package and project references. Packages that contribute no assets to the project or exclusively contribute [Compile assets][5] are treated as development dependencies. +It looks for `project.assets.json` files and separately discovers `*.binlog` files. The binlog provides +build-time context that isn't available from `project.assets.json` alone. The .NET SDK will perform conflict resolution for all packages during the build. This process will remove assets from packages that overlap with assets of the same name that come from the .NET framework that's used by the project. For example if a project references `System.Text.Json` version `6.0.0` and targets `net8.0` which includes a newer `System.Text.Json` the .NET SDK will ignore all the assets provided by the `System.Text.Json` package and only use those provided by the framework. Unfortunately the result of this process is not persisted in any build artifact. To approximate this we capture a list of packages per framework version that would lose to the framework assets. When examining packages referenced by a project for a given framework, if we find that its included in the list we'll mark it as a development dependency. @@ -30,20 +38,6 @@ Future versions of the .NET SDK have moved this framework conflict resolution in [5]: https://learn.microsoft.com/en-us/nuget/consume-packages/package-references-in-project-files#controlling-dependency-assets [6]: https://github.com/NuGet/Home/blob/451c27180d14214bca60483caee57f0dc737b8cf/accepted/2024/prune-package-reference.md -## NuGetPackagesConfig - -The `NuGetPackagesConfig` detector raises NuGet components referenced by projects or solutions that use the older NuGet (v2) `packages.config` [file][7]. - -[7]: https://learn.microsoft.com/en-us/nuget/reference/packages-config - -## MSBuildBinaryLog - -The `MSBuildBinaryLog` detector is an **Experimental** detector intended to eventually replace both the `NuGetProjectCentric` and `DotNet` detectors. It combines MSBuild binary log (binlog) information with `project.assets.json` to provide enhanced component detection with project-level classifications. - -As an experimental detector, it runs automatically whenever a scan is performed, but its results are not reported as part of the normal scan output. Instead, the results are compared against the existing `NuGetProjectCentric` and `DotNet` detectors and recorded as telemetry so maintainers can evaluate parity before promoting the detector to default. - -It looks for `project.assets.json` files and separately discovers `*.binlog` files. The binlog provides build-time context that isn't available from `project.assets.json` alone. - ### MSBuild Properties The detector extracts the following MSBuild properties from binlog data: @@ -51,7 +45,7 @@ The detector extracts the following MSBuild properties from binlog data: | Property | Usage | | --- | --- | | `NETCoreSdkVersion` | Registered as the SDK version for the DotNet component. More accurate than `dotnet --version`, which can differ due to `global.json` rollforward. | -| `OutputType` | Classifies projects as "application" (`Exe`, `WinExe`, `AppContainerExe`) or "library" (`Library`, `Module`). The `DotNet` detector uses PE header inspection, which requires compiled output. | +| `OutputType` | Classifies projects as "application" (`Exe`, `WinExe`, `AppContainerExe`) or "library" (`Library`, `Module`). | | `ProjectAssetsFile` | Maps binlog project info to the corresponding `project.assets.json` on disk. | | `TargetFramework` / `TargetFrameworks` | Identifies inner builds for multi-targeted projects and determines per-TFM properties. | | `IsTestProject` | When `true`, all dependencies of the project are marked as development dependencies. | @@ -75,16 +69,41 @@ For multi-targeted projects, the detector understands the MSBuild outer/inner bu ### Fallback Mode -When no binlog is available for a project, the detector falls back to standard NuGet detection behavior (equivalent to the `NuGetProjectCentric` detector). +When no binlog is available for a project, or a binlog cannot be parsed, the detector falls back to +the existing `project.assets.json`-based NuGet and DotNet detection behavior. A parse failure logs a +warning and emits `FailedParsingFile` telemetry containing the detector ID, binlog path, and exception +message. To enable the richer build context, place a `*.binlog` in the scan directory (for example, +build with `dotnet build -bl`). + +For DotNet SDK detection, fallback processing starts at the project location from +`project.assets.json` and searches parent directories for a [`global.json`][7]. It runs +`dotnet --version` from the selected directory to resolve the SDK version, stopping the upward search +at `SourceDirectory`, `SourceFileRoot`, or the filesystem root. If `dotnet` is unavailable, it uses +the SDK version declared in `global.json` when possible. + +The detector registers the target frameworks from `project.assets.json` and classifies each project +as an `application`, `library`, or `unknown`. Binlog processing uses `OutputType`; fallback processing +uses the output assembly's [PE COFF characteristics][8] when the assembly is available. Project type +is qualified with `-selfcontained` when either the binlog sets `SelfContained` or `PublishAot`, or the +lock file indicates a corresponding runtime package download or a `Microsoft.DotNet.ILCompiler` +reference. This distinction matters because self-contained applications bundle the .NET runtime and +must service it independently. + +[7]: https://learn.microsoft.com/en-us/dotnet/core/tools/global-json +[8]: https://learn.microsoft.com/en-us/windows/win32/debug/pe-format#characteristics -### Enabling the Detector +## NuGetPackagesConfig -Pass `--DetectorArgs MSBuildBinaryLog=EnableIfDefaultOff` and ensure a `*.binlog` file is present in the scan directory (e.g., by building with `dotnet build -bl`). +The `NuGetPackagesConfig` detector raises NuGet components referenced by projects or solutions that +use the older NuGet (v2) [`packages.config` file][9]. -## Known Limitations +[9]: https://learn.microsoft.com/en-us/nuget/reference/packages-config -- Any components that are only found in `*.nuspec` or `*.nupkg` files will not be detected with the latest NuGet Detector approach, because the NuGet detector that scans `*.nuspec` or `*.nupkg` files overreports. This is due to of NuGet's [restore behaviour][8] which downloads all possible dependencies before [resolving the final dependency graph][9]. +## Known Limitations -[8]: https://learn.microsoft.com/en-us/nuget/consume-packages/package-restore#package-restore-behavior -[9]: https://learn.microsoft.com/en-us/nuget/concepts/dependency-resolution +- Any components that are only found in `*.nuspec` or `*.nupkg` files will not be detected with the latest NuGet Detector approach, because the NuGet detector that scans `*.nuspec` or `*.nupkg` files overreports. This is due to NuGet's [restore behavior][10], which downloads all possible dependencies before [resolving the final dependency graph][11]. +- Without matching binlog data, project type can be `unknown` when the output assembly is unavailable or is stored outside the output path recorded in `project.assets.json`. +- Without matching binlog data, SDK version detection depends on `dotnet --version` or a usable `global.json`. +[10]: https://learn.microsoft.com/en-us/nuget/consume-packages/package-restore#package-restore-behavior +[11]: https://learn.microsoft.com/en-us/nuget/concepts/dependency-resolution diff --git a/src/Microsoft.ComponentDetection.Detectors/dotnet/DotNetComponentDetector.cs b/src/Microsoft.ComponentDetection.Detectors/dotnet/DotNetComponentDetector.cs deleted file mode 100644 index 6d92ae056..000000000 --- a/src/Microsoft.ComponentDetection.Detectors/dotnet/DotNetComponentDetector.cs +++ /dev/null @@ -1,72 +0,0 @@ -namespace Microsoft.ComponentDetection.Detectors.DotNet; - -using System.Collections.Generic; -using System.Threading; -using System.Threading.Tasks; -using global::NuGet.ProjectModel; -using Microsoft.ComponentDetection.Contracts; -using Microsoft.ComponentDetection.Contracts.Internal; -using Microsoft.ComponentDetection.Contracts.TypedComponent; -using Microsoft.Extensions.Logging; - -public class DotNetComponentDetector : FileComponentDetector -{ - private readonly DotNetProjectInfoProvider projectInfoProvider; - private readonly LockFileFormat lockFileFormat = new(); - - public DotNetComponentDetector( - IComponentStreamEnumerableFactory componentStreamEnumerableFactory, - ICommandLineInvocationService commandLineInvocationService, - IDirectoryUtilityService directoryUtilityService, - IFileUtilityService fileUtilityService, - IPathUtilityService pathUtilityService, - IObservableDirectoryWalkerFactory walkerFactory, - ILogger logger) - { - this.ComponentStreamEnumerableFactory = componentStreamEnumerableFactory; - this.Scanner = walkerFactory; - this.Logger = logger; - this.projectInfoProvider = new DotNetProjectInfoProvider( - commandLineInvocationService, - directoryUtilityService, - fileUtilityService, - pathUtilityService, - logger); - } - - public override string Id => "DotNet"; - - public override IList SearchPatterns { get; } = [LockFileFormat.AssetsFileName]; - - public override IEnumerable SupportedComponentTypes => [ComponentType.DotNet]; - - public override int Version { get; } = 1; - - public override IEnumerable Categories => ["DotNet"]; - - public override Task ExecuteDetectorAsync(ScanRequest request, CancellationToken cancellationToken = default) - { - this.projectInfoProvider.Initialize(request.SourceDirectory.FullName, request.SourceFileRoot?.FullName); - return base.ExecuteDetectorAsync(request, cancellationToken); - } - - protected override async Task OnFileFoundAsync(ProcessRequest processRequest, IDictionary detectorArgs, CancellationToken cancellationToken = default) - { - var lockFile = this.lockFileFormat.Read(processRequest.ComponentStream.Stream, processRequest.ComponentStream.Location); - - if (lockFile.PackageSpec is null || lockFile.PackageSpec.RestoreMetadata is null) - { - // The lock file is not valid, or does not contain a PackageSpec. - // This could be due to the lock file being generated by a different version of the SDK. - // We should not fail the detector, but we should log a warning. - this.Logger.LogWarning("Lock file {LockFilePath} does not contain project information.", processRequest.ComponentStream.Location); - return; - } - - await this.projectInfoProvider.RegisterDotNetComponentsAsync( - lockFile, - processRequest.ComponentStream.Location, - this.ComponentRecorder, - cancellationToken); - } -} diff --git a/src/Microsoft.ComponentDetection.Detectors/dotnet/DotNetProjectInfoProvider.cs b/src/Microsoft.ComponentDetection.Detectors/dotnet/DotNetProjectInfoProvider.cs index 784fb2362..b84463ef1 100644 --- a/src/Microsoft.ComponentDetection.Detectors/dotnet/DotNetProjectInfoProvider.cs +++ b/src/Microsoft.ComponentDetection.Detectors/dotnet/DotNetProjectInfoProvider.cs @@ -18,8 +18,7 @@ namespace Microsoft.ComponentDetection.Detectors.DotNet; /// /// Resolves DotNet project and SDK information from the environment. /// Handles SDK version resolution, project type detection, path rebasing, -/// and DotNet component registration. Used by both DotNetComponentDetector -/// and MSBuildBinaryLogComponentDetector. +/// and DotNet component registration for MSBuildBinaryLogComponentDetector fallback processing. /// internal class DotNetProjectInfoProvider { @@ -64,8 +63,8 @@ public void Initialize(string? sourceDirectory, string? sourceFileRootDirectory) /// /// Registers DotNet components from a lock file, determining SDK version and project type. - /// This is the complete DotNet component registration logic shared between DotNetComponentDetector - /// and MSBuildBinaryLogComponentDetector's fallback path. + /// This is the complete DotNet component registration logic for + /// MSBuildBinaryLogComponentDetector's fallback path. /// /// The lock file to analyze. /// The location of the project.assets.json file. diff --git a/src/Microsoft.ComponentDetection.Detectors/nuget/LockFileUtilities.cs b/src/Microsoft.ComponentDetection.Detectors/nuget/LockFileUtilities.cs index d22393c82..d71088b9d 100644 --- a/src/Microsoft.ComponentDetection.Detectors/nuget/LockFileUtilities.cs +++ b/src/Microsoft.ComponentDetection.Detectors/nuget/LockFileUtilities.cs @@ -12,12 +12,12 @@ namespace Microsoft.ComponentDetection.Detectors.NuGet; using Microsoft.ComponentDetection.Contracts.TypedComponent; using Microsoft.Extensions.Logging; -// LockFileUtilities also includes self-contained detection helpers shared by -// DotNetComponentDetector and MSBuildBinaryLogComponentDetector. +// LockFileUtilities also includes self-contained detection helpers used by +// MSBuildBinaryLogComponentDetector. /// /// Shared utility methods for processing NuGet lock files (project.assets.json). -/// Used by both NuGetProjectModelProjectCentricComponentDetector and MSBuildBinaryLogComponentDetector. +/// Used by MSBuildBinaryLogComponentDetector for enhanced and fallback processing. /// internal static class LockFileUtilities { diff --git a/src/Microsoft.ComponentDetection.Detectors/nuget/MSBuildBinaryLogComponentDetector.cs b/src/Microsoft.ComponentDetection.Detectors/nuget/MSBuildBinaryLogComponentDetector.cs index 1aadee6a0..f155b0bbe 100644 --- a/src/Microsoft.ComponentDetection.Detectors/nuget/MSBuildBinaryLogComponentDetector.cs +++ b/src/Microsoft.ComponentDetection.Detectors/nuget/MSBuildBinaryLogComponentDetector.cs @@ -10,6 +10,7 @@ namespace Microsoft.ComponentDetection.Detectors.NuGet; using global::NuGet.Frameworks; using global::NuGet.ProjectModel; using Microsoft.Build.Framework; +using Microsoft.ComponentDetection.Common.Telemetry.Records; using Microsoft.ComponentDetection.Contracts; using Microsoft.ComponentDetection.Contracts.Internal; using Microsoft.ComponentDetection.Contracts.TypedComponent; @@ -18,7 +19,7 @@ namespace Microsoft.ComponentDetection.Detectors.NuGet; using Task = System.Threading.Tasks.Task; /// -/// An experimental detector that combines MSBuild binlog information with NuGet project.assets.json +/// A detector that combines MSBuild binlog information with NuGet project.assets.json /// to provide enhanced component detection with project-level classifications. /// This detector is intended to replace both DotNetComponentDetector and NuGetProjectModelProjectCentricComponentDetector. /// @@ -48,7 +49,7 @@ namespace Microsoft.ComponentDetection.Detectors.NuGet; /// - Fallback mode: When no binlog info is available, falls back to standard NuGet detection. /// /// -public class MSBuildBinaryLogComponentDetector : FileComponentDetector, IExperimentalDetector +public class MSBuildBinaryLogComponentDetector : FileComponentDetector { private readonly IBinLogProcessor binLogProcessor; private readonly IFileUtilityService fileUtilityService; @@ -318,6 +319,14 @@ private void ProcessBinlogFile(string binlogPath, CancellationToken cancellation catch (Exception ex) { this.Logger.LogWarning(ex, "Failed to process binlog file: {BinlogPath}", binlogPath); + + using var failedParsingRecord = new FailedParsingFileRecord + { + DetectorId = this.Id, + FilePath = binlogPath, + ExceptionMessage = ex.Message, + StackTrace = ex.StackTrace, + }; } } @@ -610,7 +619,8 @@ private bool IsPackageDownloadDevDependency(string packageName, NuGetFramework? /// private async Task ProcessLockFileFallbackAsync(LockFile lockFile, string location, CancellationToken cancellationToken) { - var singleFileComponentRecorder = this.ComponentRecorder.CreateSingleFileComponentRecorder(location); + var recorderLocation = lockFile.PackageSpec?.RestoreMetadata?.ProjectPath ?? location; + var singleFileComponentRecorder = this.ComponentRecorder.CreateSingleFileComponentRecorder(recorderLocation); LockFileUtilities.ProcessLockFile(lockFile, singleFileComponentRecorder, this.Logger); // Register DotNet components (SDK version, target framework, project type) diff --git a/src/Microsoft.ComponentDetection.Detectors/nuget/NuGetProjectModelProjectCentricComponentDetector.cs b/src/Microsoft.ComponentDetection.Detectors/nuget/NuGetProjectModelProjectCentricComponentDetector.cs deleted file mode 100644 index 0d4e507b7..000000000 --- a/src/Microsoft.ComponentDetection.Detectors/nuget/NuGetProjectModelProjectCentricComponentDetector.cs +++ /dev/null @@ -1,66 +0,0 @@ -#nullable disable -namespace Microsoft.ComponentDetection.Detectors.NuGet; - -using System; -using System.Collections.Generic; -using System.Threading; -using System.Threading.Tasks; -using global::NuGet.ProjectModel; -using Microsoft.ComponentDetection.Contracts; -using Microsoft.ComponentDetection.Contracts.Internal; -using Microsoft.ComponentDetection.Contracts.TypedComponent; -using Microsoft.Extensions.Logging; - -public class NuGetProjectModelProjectCentricComponentDetector : FileComponentDetector -{ - private readonly IFileUtilityService fileUtilityService; - - public NuGetProjectModelProjectCentricComponentDetector( - IComponentStreamEnumerableFactory componentStreamEnumerableFactory, - IObservableDirectoryWalkerFactory walkerFactory, - IFileUtilityService fileUtilityService, - ILogger logger) - { - this.ComponentStreamEnumerableFactory = componentStreamEnumerableFactory; - this.Scanner = walkerFactory; - this.fileUtilityService = fileUtilityService; - this.Logger = logger; - } - - public override string Id { get; } = "NuGetProjectCentric"; - - public override IEnumerable Categories => [Enum.GetName(typeof(DetectorClass), DetectorClass.NuGet)]; - - public override IList SearchPatterns { get; } = ["project.assets.json"]; - - public override IEnumerable SupportedComponentTypes { get; } = [ComponentType.NuGet]; - - public override int Version { get; } = 2; - - protected override Task OnFileFoundAsync(ProcessRequest processRequest, IDictionary detectorArgs, CancellationToken cancellationToken = default) - { - try - { - var lockFile = new LockFileFormat().Read(processRequest.ComponentStream.Stream, processRequest.ComponentStream.Location); - - this.RecordLockfileVersion(lockFile.Version); - - if (lockFile.PackageSpec == null) - { - this.Logger.LogWarning("Lock file {LockFilePath} does not contain a PackageSpec.", processRequest.ComponentStream.Location); - return Task.CompletedTask; - } - - // Since we report projects as the location, we ignore the passed in single file recorder. - var singleFileComponentRecorder = this.ComponentRecorder.CreateSingleFileComponentRecorder(lockFile.PackageSpec.RestoreMetadata.ProjectPath); - LockFileUtilities.ProcessLockFile(lockFile, singleFileComponentRecorder, this.Logger); - } - catch (Exception e) - { - // If something went wrong, just ignore the package - this.Logger.LogError(e, "Failed to process NuGet lockfile {NuGetLockFile}", processRequest.ComponentStream.Location); - } - - return Task.CompletedTask; - } -} diff --git a/src/Microsoft.ComponentDetection.Orchestrator/Experiments/Configs/LinuxApplicationLayerExperiment.cs b/src/Microsoft.ComponentDetection.Orchestrator/Experiments/Configs/LinuxApplicationLayerExperiment.cs index a99fe66f5..27f06025c 100644 --- a/src/Microsoft.ComponentDetection.Orchestrator/Experiments/Configs/LinuxApplicationLayerExperiment.cs +++ b/src/Microsoft.ComponentDetection.Orchestrator/Experiments/Configs/LinuxApplicationLayerExperiment.cs @@ -28,7 +28,7 @@ or NpmComponentDetector or NpmLockfileDetectorBase or PipReportComponentDetector or NuGetComponentDetector - or NuGetProjectModelProjectCentricComponentDetector + or MSBuildBinaryLogComponentDetector or NuGetPackagesConfigDetector; /// @@ -39,7 +39,7 @@ or NpmComponentDetector or NpmLockfileDetectorBase or PipReportComponentDetector or NuGetComponentDetector - or NuGetProjectModelProjectCentricComponentDetector + or MSBuildBinaryLogComponentDetector or NuGetPackagesConfigDetector; /// diff --git a/src/Microsoft.ComponentDetection.Orchestrator/Experiments/Configs/MSBuildBinaryLogExperiment.cs b/src/Microsoft.ComponentDetection.Orchestrator/Experiments/Configs/MSBuildBinaryLogExperiment.cs deleted file mode 100644 index e5f19ba4e..000000000 --- a/src/Microsoft.ComponentDetection.Orchestrator/Experiments/Configs/MSBuildBinaryLogExperiment.cs +++ /dev/null @@ -1,27 +0,0 @@ -namespace Microsoft.ComponentDetection.Orchestrator.Experiments.Configs; - -using Microsoft.ComponentDetection.Contracts; -using Microsoft.ComponentDetection.Detectors.DotNet; -using Microsoft.ComponentDetection.Detectors.NuGet; - -/// -/// Experiment configuration for validating the -/// against the existing and -/// . -/// -public class MSBuildBinaryLogExperiment : IExperimentConfiguration -{ - /// - public string Name => "MSBuildBinaryLogDetector"; - - /// - public bool IsInControlGroup(IComponentDetector componentDetector) => - componentDetector is NuGetProjectModelProjectCentricComponentDetector or DotNetComponentDetector; - - /// - public bool IsInExperimentGroup(IComponentDetector componentDetector) => - componentDetector is MSBuildBinaryLogComponentDetector; - - /// - public bool ShouldRecord(IComponentDetector componentDetector, int numComponents) => true; -} diff --git a/src/Microsoft.ComponentDetection.Orchestrator/Extensions/ServiceCollectionExtensions.cs b/src/Microsoft.ComponentDetection.Orchestrator/Extensions/ServiceCollectionExtensions.cs index 95fe15b13..dd33c8a83 100644 --- a/src/Microsoft.ComponentDetection.Orchestrator/Extensions/ServiceCollectionExtensions.cs +++ b/src/Microsoft.ComponentDetection.Orchestrator/Extensions/ServiceCollectionExtensions.cs @@ -8,7 +8,6 @@ namespace Microsoft.ComponentDetection.Orchestrator.Extensions; using Microsoft.ComponentDetection.Detectors.CondaLock; using Microsoft.ComponentDetection.Detectors.DockerCompose; using Microsoft.ComponentDetection.Detectors.Dockerfile; -using Microsoft.ComponentDetection.Detectors.DotNet; using Microsoft.ComponentDetection.Detectors.Go; using Microsoft.ComponentDetection.Detectors.Gradle; using Microsoft.ComponentDetection.Detectors.Helm; @@ -75,7 +74,6 @@ public static IServiceCollection AddComponentDetection(this IServiceCollection s services.AddSingleton(); services.AddSingleton(); services.AddSingleton(); - services.AddSingleton(); // Detectors // CocoaPods @@ -93,9 +91,6 @@ public static IServiceCollection AddComponentDetection(this IServiceCollection s // Dockerfile services.AddSingleton(); - // DotNet - services.AddSingleton(); - // Go services.AddSingleton(); services.AddSingleton(); @@ -138,7 +133,6 @@ public static IServiceCollection AddComponentDetection(this IServiceCollection s // NuGet services.AddSingleton(); services.AddSingleton(); - services.AddSingleton(); services.AddSingleton(); // PIP diff --git a/src/Microsoft.ComponentDetection.Orchestrator/Services/DetectorRestrictionService.cs b/src/Microsoft.ComponentDetection.Orchestrator/Services/DetectorRestrictionService.cs index 14f4c1ac3..bb3d61cf8 100644 --- a/src/Microsoft.ComponentDetection.Orchestrator/Services/DetectorRestrictionService.cs +++ b/src/Microsoft.ComponentDetection.Orchestrator/Services/DetectorRestrictionService.cs @@ -9,8 +9,15 @@ namespace Microsoft.ComponentDetection.Orchestrator.Services; internal class DetectorRestrictionService : IDetectorRestrictionService { - private readonly IList oldDetectorIds = ["MSLicenseDevNpm", "MSLicenseDevNpmList", "MSLicenseNpm", "MSLicenseNpmList"]; - private readonly string newDetectorId = "NpmWithRoots"; + private static readonly IReadOnlyDictionary DetectorReplacements = new Dictionary(StringComparer.OrdinalIgnoreCase) + { + ["MSLicenseDevNpm"] = "NpmWithRoots", + ["MSLicenseDevNpmList"] = "NpmWithRoots", + ["MSLicenseNpm"] = "NpmWithRoots", + ["MSLicenseNpmList"] = "NpmWithRoots", + ["DotNet"] = "MSBuildBinaryLog", + ["NuGetProjectCentric"] = "MSBuildBinaryLog", + }; private readonly ILogger logger; @@ -25,14 +32,18 @@ public IEnumerable ApplyRestrictions(DetectorRestrictions re // If someone specifies an "allow list", use it, otherwise assume everything is allowed if (restrictions.AllowedDetectorIds != null && restrictions.AllowedDetectorIds.Any()) { - var allowedIds = restrictions.AllowedDetectorIds; - - // If we have retired detectors in the arg specified list and don't have the new detector, add the new detector - if (allowedIds.Any(a => this.oldDetectorIds.Contains(a, StringComparer.OrdinalIgnoreCase)) && !allowedIds.Contains(this.newDetectorId, StringComparer.OrdinalIgnoreCase)) + var allowedIds = new HashSet(StringComparer.OrdinalIgnoreCase); + foreach (var requestedId in restrictions.AllowedDetectorIds.Distinct(StringComparer.OrdinalIgnoreCase)) { - allowedIds = allowedIds.Concat([ - this.newDetectorId, - ]); + if (DetectorReplacements.TryGetValue(requestedId, out var replacementId)) + { + allowedIds.Add(replacementId); + this.logger.LogWarning("The detector '{OldId}' has been phased out, we will run the '{NewId}' detector which replaced its functionality.", requestedId, replacementId); + } + else + { + allowedIds.Add(requestedId); + } } detectors = detectors.Where(d => allowedIds.Contains(d.Id, StringComparer.OrdinalIgnoreCase)).ToList(); @@ -41,14 +52,7 @@ public IEnumerable ApplyRestrictions(DetectorRestrictions re { if (!detectors.Select(d => d.Id).Contains(id, StringComparer.OrdinalIgnoreCase)) { - if (!this.oldDetectorIds.Contains(id, StringComparer.OrdinalIgnoreCase)) - { - throw new InvalidDetectorFilterException($"Detector '{id}' was not found"); - } - else - { - this.logger.LogWarning("The detector '{OldId}' has been phased out, we will run the '{NewId}' detector which replaced its functionality.", id, this.newDetectorId); - } + throw new InvalidDetectorFilterException($"Detector '{id}' was not found"); } } } diff --git a/test/Microsoft.ComponentDetection.Detectors.Tests/ComponentDetectorTests.cs b/test/Microsoft.ComponentDetection.Detectors.Tests/ComponentDetectorTests.cs index abba423dd..66d812b43 100644 --- a/test/Microsoft.ComponentDetection.Detectors.Tests/ComponentDetectorTests.cs +++ b/test/Microsoft.ComponentDetection.Detectors.Tests/ComponentDetectorTests.cs @@ -81,4 +81,14 @@ public void CondaLockComponentDetector_ImplementsIExperimentalDetector() condaLockDetector.Should().NotBeNull("because CondaLockComponentDetector should be registered"); condaLockDetector.Should().BeAssignableTo("because CondaLockComponentDetector should implement IExperimentalDetector"); } + + [TestMethod] + public void MSBuildBinaryLogComponentDetector_IsDefaultOnAndSupersedesLegacyDetectors() + { + var detector = this.detectors.SingleOrDefault(d => d.Id == "MSBuildBinaryLog"); + + detector.Should().NotBeNull("because MSBuildBinaryLogComponentDetector should be registered"); + detector.Should().NotBeAssignableTo("because MSBuildBinaryLogComponentDetector should be default-on"); + this.detectors.Select(d => d.Id).Should().NotContain(["NuGetProjectCentric", "DotNet"]); + } } diff --git a/test/Microsoft.ComponentDetection.Detectors.Tests/DotNetComponentDetectorTests.cs b/test/Microsoft.ComponentDetection.Detectors.Tests/MSBuildBinaryLogDotNetFallbackCompatibilityTests.cs similarity index 95% rename from test/Microsoft.ComponentDetection.Detectors.Tests/DotNetComponentDetectorTests.cs rename to test/Microsoft.ComponentDetection.Detectors.Tests/MSBuildBinaryLogDotNetFallbackCompatibilityTests.cs index 20eac5430..a93a1bf35 100644 --- a/test/Microsoft.ComponentDetection.Detectors.Tests/DotNetComponentDetectorTests.cs +++ b/test/Microsoft.ComponentDetection.Detectors.Tests/MSBuildBinaryLogDotNetFallbackCompatibilityTests.cs @@ -20,7 +20,7 @@ namespace Microsoft.ComponentDetection.Detectors.Tests; using Microsoft.ComponentDetection.Common; using Microsoft.ComponentDetection.Contracts; using Microsoft.ComponentDetection.Contracts.TypedComponent; -using Microsoft.ComponentDetection.Detectors.DotNet; +using Microsoft.ComponentDetection.Detectors.NuGet; using Microsoft.ComponentDetection.TestsUtilities; using Microsoft.Extensions.Logging; using Microsoft.VisualStudio.TestTools.UnitTesting; @@ -29,7 +29,7 @@ namespace Microsoft.ComponentDetection.Detectors.Tests; [TestClass] [TestCategory("Governance/All")] [TestCategory("Governance/ComponentDetection")] -public class DotNetComponentDetectorTests +public class MSBuildBinaryLogDotNetFallbackCompatibilityTests { private static readonly string RootDir = RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? "C:" : @"/"; @@ -41,9 +41,9 @@ public class DotNetComponentDetectorTests Assembly.GetExecutingAssembly().GetCustomAttribute().FrameworkName) .GetShortFolderName(); - private readonly DetectorTestUtilityBuilder detectorTestUtility = new(); + private readonly DetectorTestUtilityBuilder detectorTestUtility = new(); - private readonly Mock> mockLogger = new(); + private readonly Mock> mockLogger = new(); // uses ExecuteCommandAsync private readonly Mock mockCommandLineInvocationService = new(); @@ -64,9 +64,9 @@ public class DotNetComponentDetectorTests private Func commandLineCallback; /// - /// Initializes a new instance of the class. + /// Initializes a new instance of the class. /// - public DotNetComponentDetectorTests() + public MSBuildBinaryLogDotNetFallbackCompatibilityTests() { this.detectorTestUtility.AddServiceMock(this.mockLogger) .AddServiceMock(this.mockCommandLineInvocationService) @@ -359,7 +359,7 @@ public async Task TestDotNetDetectorWithNoFiles_ReturnsSuccessfullyAsync() var (scanResult, componentRecorder) = await this.detectorTestUtility.ExecuteDetectorAsync(); scanResult.ResultCode.Should().Be(ProcessingResultCode.Success); - componentRecorder.GetDetectedComponents().Should().BeEmpty(); + GetDotNetComponents(componentRecorder).Should().BeEmpty(); } [TestMethod] @@ -378,7 +378,7 @@ public async Task TestDotNetDetectorGlobalJson_ReturnsSDKVersion() scanResult.ResultCode.Should().Be(ProcessingResultCode.Success); - var detectedComponents = componentRecorder.GetDetectedComponents(); + var detectedComponents = GetDotNetComponents(componentRecorder); detectedComponents.Should().HaveCount(2); var discoveredComponents = detectedComponents.ToArray(); @@ -412,7 +412,7 @@ public async Task TestDotNetDetectorGlobalJsonWithComments_ReturnsSDKVersion() scanResult.ResultCode.Should().Be(ProcessingResultCode.Success); - var detectedComponents = componentRecorder.GetDetectedComponents(); + var detectedComponents = GetDotNetComponents(componentRecorder); detectedComponents.Should().HaveCount(2); var discoveredComponents = detectedComponents.ToArray(); @@ -444,7 +444,7 @@ public async Task TestDotNetDetectorGlobalJsonWithTrailingCommas_ReturnsSDKVersi scanResult.ResultCode.Should().Be(ProcessingResultCode.Success); - var detectedComponents = componentRecorder.GetDetectedComponents(); + var detectedComponents = GetDotNetComponents(componentRecorder); detectedComponents.Should().HaveCount(2); var discoveredComponents = detectedComponents.ToArray(); @@ -475,7 +475,7 @@ public async Task TestDotNetDetectorGlobalJsonWithoutVersion() scanResult.ResultCode.Should().Be(ProcessingResultCode.Success); - var detectedComponents = componentRecorder.GetDetectedComponents(); + var detectedComponents = GetDotNetComponents(componentRecorder); detectedComponents.Should().ContainSingle(); var discoveredComponents = detectedComponents.ToArray(); @@ -498,7 +498,7 @@ public async Task TestDotNetDetectorGlobalJsonRollForward_ReturnsSDKVersion() scanResult.ResultCode.Should().Be(ProcessingResultCode.Success); - var detectedComponents = componentRecorder.GetDetectedComponents(); + var detectedComponents = GetDotNetComponents(componentRecorder); detectedComponents.Should().HaveCount(2); var discoveredComponents = detectedComponents.ToArray(); @@ -522,7 +522,7 @@ public async Task TestDotNetDetectorGlobalJsonDotNetVersionFails_ReturnsSDKVersi scanResult.ResultCode.Should().Be(ProcessingResultCode.Success); - var detectedComponents = componentRecorder.GetDetectedComponents(); + var detectedComponents = GetDotNetComponents(componentRecorder); detectedComponents.Should().HaveCount(2); var discoveredComponents = detectedComponents.ToArray(); @@ -546,7 +546,7 @@ public async Task TestDotNetDetectorGlobalJsonDotNetVersionThrows_ReturnsSDKVers scanResult.ResultCode.Should().Be(ProcessingResultCode.Success); - var detectedComponents = componentRecorder.GetDetectedComponents(); + var detectedComponents = GetDotNetComponents(componentRecorder); detectedComponents.Should().HaveCount(2); var discoveredComponents = detectedComponents.ToArray(); @@ -568,7 +568,7 @@ public async Task TestDotNetDetectorNoGlobalJson_ReturnsDotNetVersion() scanResult.ResultCode.Should().Be(ProcessingResultCode.Success); - var detectedComponents = componentRecorder.GetDetectedComponents(); + var detectedComponents = GetDotNetComponents(componentRecorder); detectedComponents.Should().ContainSingle(); var discoveredComponents = detectedComponents.ToArray(); @@ -589,7 +589,7 @@ public async Task TestDotNetDetectorNoGlobalJsonNoDotnet_ReturnsUnknownVersion() scanResult.ResultCode.Should().Be(ProcessingResultCode.Success); - var detectedComponents = componentRecorder.GetDetectedComponents(); + var detectedComponents = GetDotNetComponents(componentRecorder); detectedComponents.Should().ContainSingle(); var discoveredComponents = detectedComponents.ToArray(); @@ -612,7 +612,7 @@ public async Task TestDotNetDetectorInvalidProjectName_ReturnsUnknownVersion() scanResult.ResultCode.Should().Be(ProcessingResultCode.Success); - var detectedComponents = componentRecorder.GetDetectedComponents(); + var detectedComponents = GetDotNetComponents(componentRecorder); detectedComponents.Should().ContainSingle(); var discoveredComponents = detectedComponents.ToArray(); @@ -633,7 +633,7 @@ public async Task TestDotNetDetectorMultipleTargetFrameworks() scanResult.ResultCode.Should().Be(ProcessingResultCode.Success); - var detectedComponents = componentRecorder.GetDetectedComponents(); + var detectedComponents = GetDotNetComponents(componentRecorder); detectedComponents.Should().HaveCount(3); var discoveredComponents = detectedComponents.ToArray(); @@ -686,7 +686,7 @@ public async Task TestDotNetDetectorMultipleProjectsWithDifferentOutputTypeAndSd scanResult.ResultCode.Should().Be(ProcessingResultCode.Success); - var detectedComponents = componentRecorder.GetDetectedComponents(); + var detectedComponents = GetDotNetComponents(componentRecorder); detectedComponents.Should().HaveCount(6); var discoveredComponents = detectedComponents.ToArray(); @@ -724,7 +724,7 @@ public async Task TestDotNetDetectorExe() scanResult.ResultCode.Should().Be(ProcessingResultCode.Success); - var detectedComponents = componentRecorder.GetDetectedComponents(); + var detectedComponents = GetDotNetComponents(componentRecorder); detectedComponents.Should().HaveCount(2); var discoveredComponents = detectedComponents.ToArray(); @@ -761,7 +761,7 @@ public async Task TestDotNetDetectorInvalidOutputAssembly() scanResult.ResultCode.Should().Be(ProcessingResultCode.Success); - var detectedComponents = componentRecorder.GetDetectedComponents(); + var detectedComponents = GetDotNetComponents(componentRecorder); detectedComponents.Should().HaveCount(4); var discoveredComponents = detectedComponents.ToArray(); @@ -796,7 +796,7 @@ public async Task TestDotNetDetectorNoGlobalJsonSourceRoot() scanResult.ResultCode.Should().Be(ProcessingResultCode.Success); - var detectedComponents = componentRecorder.GetDetectedComponents(); + var detectedComponents = GetDotNetComponents(componentRecorder); detectedComponents.Should().ContainSingle(); var discoveredComponents = detectedComponents.ToArray(); @@ -852,7 +852,7 @@ public async Task TestDotNetDetectorRebasePaths(string additionalPathSegment) scanResult.ResultCode.Should().Be(ProcessingResultCode.Success); - var detectedComponents = componentRecorder.GetDetectedComponents(); + var detectedComponents = GetDotNetComponents(componentRecorder); detectedComponents.Should().HaveCount(4); var discoveredComponents = detectedComponents.ToArray(); @@ -923,7 +923,7 @@ public async Task TestDotNetDetectorSelfContainedWithSelfContainedProperty() scanResult.ResultCode.Should().Be(ProcessingResultCode.Success); - var discoveredComponents = componentRecorder.GetDetectedComponents().ToArray(); + var discoveredComponents = GetDotNetComponents(componentRecorder).ToArray(); // Both the plain TFM and RID-qualified targets map to the same framework. // The detector should report application-selfcontained for this framework. @@ -983,7 +983,7 @@ public async Task TestDotNetDetectorSelfContainedLibrary() scanResult.ResultCode.Should().Be(ProcessingResultCode.Success); - var discoveredComponents = componentRecorder.GetDetectedComponents().ToArray(); + var discoveredComponents = GetDotNetComponents(componentRecorder).ToArray(); discoveredComponents.Where(component => component.Component.Id == $"4.5.6 {CurrentTfm} library-selfcontained - DotNet").Should().ContainSingle(); } finally @@ -1044,7 +1044,7 @@ public async Task TestDotNetDetectorSelfContainedWithPublishAot() scanResult.ResultCode.Should().Be(ProcessingResultCode.Success); - var discoveredComponents = componentRecorder.GetDetectedComponents().ToArray(); + var discoveredComponents = GetDotNetComponents(componentRecorder).ToArray(); discoveredComponents.Where(component => component.Component.Id == $"4.5.6 {CurrentTfm} application-selfcontained - DotNet").Should().ContainSingle(); } finally @@ -1102,7 +1102,7 @@ public async Task TestDotNetDetectorNotSelfContained() scanResult.ResultCode.Should().Be(ProcessingResultCode.Success); - var discoveredComponents = componentRecorder.GetDetectedComponents().ToArray(); + var discoveredComponents = GetDotNetComponents(componentRecorder).ToArray(); discoveredComponents.Where(component => component.Component.Id == $"4.5.6 {CurrentTfm} application - DotNet").Should().ContainSingle(); } finally @@ -1139,7 +1139,7 @@ public async Task TestDotNetDetectorSyntheticSelfContainedApplication() scanResult.ResultCode.Should().Be(ProcessingResultCode.Success); - var discoveredComponents = componentRecorder.GetDetectedComponents().ToArray(); + var discoveredComponents = GetDotNetComponents(componentRecorder).ToArray(); discoveredComponents.Where(component => component.Component.Id == "4.5.6 net8.0 application-selfcontained - DotNet").Should().ContainSingle(); } @@ -1168,7 +1168,7 @@ public async Task TestDotNetDetectorSyntheticSelfContainedLibrary() scanResult.ResultCode.Should().Be(ProcessingResultCode.Success); - var discoveredComponents = componentRecorder.GetDetectedComponents().ToArray(); + var discoveredComponents = GetDotNetComponents(componentRecorder).ToArray(); discoveredComponents.Where(component => component.Component.Id == "4.5.6 net8.0 library-selfcontained - DotNet").Should().ContainSingle(); } @@ -1198,7 +1198,7 @@ public async Task TestDotNetDetectorSyntheticAotApplication() scanResult.ResultCode.Should().Be(ProcessingResultCode.Success); - var discoveredComponents = componentRecorder.GetDetectedComponents().ToArray(); + var discoveredComponents = GetDotNetComponents(componentRecorder).ToArray(); discoveredComponents.Where(component => component.Component.Id == "4.5.6 net8.0 application-selfcontained - DotNet").Should().ContainSingle(); } @@ -1228,7 +1228,7 @@ public async Task TestDotNetDetectorSyntheticNotSelfContained() scanResult.ResultCode.Should().Be(ProcessingResultCode.Success); - var discoveredComponents = componentRecorder.GetDetectedComponents().ToArray(); + var discoveredComponents = GetDotNetComponents(componentRecorder).ToArray(); discoveredComponents.Where(component => component.Component.Id == "4.5.6 net8.0 application - DotNet").Should().ContainSingle(); } @@ -1263,8 +1263,11 @@ public async Task TestDotNetDetectorMultiTargetWithMixedSelfContained() scanResult.ResultCode.Should().Be(ProcessingResultCode.Success); - var discoveredComponents = componentRecorder.GetDetectedComponents().ToArray(); + var discoveredComponents = GetDotNetComponents(componentRecorder).ToArray(); discoveredComponents.Where(component => component.Component.Id == "4.5.6 net8.0 application-selfcontained - DotNet").Should().ContainSingle(); discoveredComponents.Where(component => component.Component.Id == "4.5.6 net6.0 application - DotNet").Should().ContainSingle(); } + + private static List GetDotNetComponents(IComponentRecorder componentRecorder) => + componentRecorder.GetDetectedComponents().Where(component => component.Component is DotNetComponent).ToList(); } diff --git a/test/Microsoft.ComponentDetection.Detectors.Tests/NuGetProjectModelProjectCentricComponentDetectorTests.cs b/test/Microsoft.ComponentDetection.Detectors.Tests/MSBuildBinaryLogFallbackCompatibilityTests.cs similarity index 85% rename from test/Microsoft.ComponentDetection.Detectors.Tests/NuGetProjectModelProjectCentricComponentDetectorTests.cs rename to test/Microsoft.ComponentDetection.Detectors.Tests/MSBuildBinaryLogFallbackCompatibilityTests.cs index f7c097ddb..8f752bd3d 100644 --- a/test/Microsoft.ComponentDetection.Detectors.Tests/NuGetProjectModelProjectCentricComponentDetectorTests.cs +++ b/test/Microsoft.ComponentDetection.Detectors.Tests/MSBuildBinaryLogFallbackCompatibilityTests.cs @@ -2,8 +2,11 @@ namespace Microsoft.ComponentDetection.Detectors.Tests; using System; +using System.Collections.Generic; +using System.IO; using System.Linq; using System.Runtime.InteropServices; +using System.Threading; using System.Threading.Tasks; using AwesomeAssertions; using Microsoft.ComponentDetection.Contracts; @@ -18,19 +21,34 @@ namespace Microsoft.ComponentDetection.Detectors.Tests; [TestClass] [TestCategory("Governance/All")] [TestCategory("Governance/ComponentDetection")] -public class NuGetProjectModelProjectCentricComponentDetectorTests +public class MSBuildBinaryLogFallbackCompatibilityTests { - private readonly DetectorTestUtilityBuilder detectorTestUtility = new(); + private readonly DetectorTestUtilityBuilder detectorTestUtility = new(); private readonly string projectAssetsJsonFileName = "project.assets.json"; + private readonly Mock commandLineInvocationServiceMock; private readonly Mock fileUtilityServiceMock; + private readonly Mock pathUtilityServiceMock; - public NuGetProjectModelProjectCentricComponentDetectorTests() + public MSBuildBinaryLogFallbackCompatibilityTests() { + this.commandLineInvocationServiceMock = new Mock(); + this.commandLineInvocationServiceMock + .Setup(x => x.ExecuteCommandAsync(It.IsAny(), It.IsAny>(), It.IsAny(), It.IsAny(), It.IsAny())) + .ReturnsAsync(new CommandLineExecutionResult { ExitCode = 1 }); + this.fileUtilityServiceMock = new Mock(); this.fileUtilityServiceMock.Setup(x => x.Exists(It.IsAny())) .Returns(true); - this.detectorTestUtility.AddServiceMock(this.fileUtilityServiceMock); + + this.pathUtilityServiceMock = new Mock(); + this.pathUtilityServiceMock.Setup(x => x.NormalizePath(It.IsAny())).Returns(path => path); + this.pathUtilityServiceMock.Setup(x => x.GetParentDirectory(It.IsAny())).Returns(path => Path.GetDirectoryName(path) ?? string.Empty); + + this.detectorTestUtility + .AddServiceMock(this.commandLineInvocationServiceMock) + .AddServiceMock(this.fileUtilityServiceMock) + .AddServiceMock(this.pathUtilityServiceMock); } [TestMethod] @@ -41,7 +59,7 @@ public async Task ScanDirectoryAsync_Base_2_2_VerificationAsync() .WithFile(this.projectAssetsJsonFileName, osAgnostic) .ExecuteDetectorAsync(); - var detectedComponents = componentRecorder.GetDetectedComponents(); + var detectedComponents = GetNuGetComponents(componentRecorder); detectedComponents.Should().HaveCount(22); @@ -66,7 +84,7 @@ public async Task ScanDirectoryAsync_Base_2_2_additional_VerificationAsync() .WithFile(this.projectAssetsJsonFileName, osAgnostic) .ExecuteDetectorAsync(); - var detectedComponents = componentRecorder.GetDetectedComponents(); + var detectedComponents = GetNuGetComponents(componentRecorder); detectedComponents.Should().HaveCount(68); @@ -93,7 +111,7 @@ public async Task ScanDirectoryAsync_ExcludedFrameworkComponent_2_2_Verification .WithFile(this.projectAssetsJsonFileName, osAgnostic) .ExecuteDetectorAsync(); - var dependencies = componentRecorder.GetDetectedComponents(); + var dependencies = GetNuGetComponents(componentRecorder); var developmentDependencies = dependencies.Where(c => componentRecorder.GetEffectiveDevDependencyValue(c.Component.Id).GetValueOrDefault()); developmentDependencies.Should().HaveCount(19); developmentDependencies.Should().Contain(c => c.Component.Id.StartsWith("Microsoft.NETCore.Platforms "), "Microsoft.NETCore.Platforms should be treated as a development dependency."); @@ -114,7 +132,7 @@ public async Task ScanDirectoryAsync_DependencyGraph_2_2_additional_Verification "NuGet.DependencyResolver.Core 5.6.0 - NuGet", }; - var detectedComponents = componentRecorder.GetDetectedComponents(); + var detectedComponents = GetNuGetComponents(componentRecorder); var componentDetectionCommon = detectedComponents.First(x => x.Component.Id.Contains("NuGet.ProjectModel")); var dependencies = graph.GetDependenciesForComponent(componentDetectionCommon.Component.Id); foreach (var expectedId in expectedDependencyIdsForCompositionTypedParts) @@ -124,7 +142,7 @@ public async Task ScanDirectoryAsync_DependencyGraph_2_2_additional_Verification expectedDependencyIdsForCompositionTypedParts.Should().HaveSameCount(dependencies); - detectedComponents.Should().HaveSameCount(graph.GetComponents()); + detectedComponents.Should().AllSatisfy(component => graph.GetComponents().Should().Contain(component.Component.Id)); // Top level dependencies look like this: // (we expect all non-proj and non-framework to show up as explicit refs, so those will be absent from the check) @@ -173,7 +191,7 @@ public async Task ScanDirectoryAsync_DependencyGraph_2_2_additional_Verification "YamlDotNet", }; - foreach (var componentId in graph.GetComponents()) + foreach (var componentId in detectedComponents.Select(component => component.Component.Id)) { var component = detectedComponents.First(x => x.Component.Id == componentId); var expectedExplicitRefValue = expectedExplicitRefs.Contains(((NuGetComponent)component.Component).Name); @@ -190,7 +208,7 @@ public async Task ScanDirectoryAsync_Base_3_1_VerificationAsync() .ExecuteDetectorAsync(); // Number of unique nodes in ProjectAssetsJson - var detectedComponents = componentRecorder.GetDetectedComponents(); + var detectedComponents = GetNuGetComponents(componentRecorder); detectedComponents.Should().HaveCount(11); var nonDevComponents = detectedComponents.Where(c => !componentRecorder.GetEffectiveDevDependencyValue(c.Component.Id).GetValueOrDefault()); @@ -219,7 +237,7 @@ public async Task ScanDirectoryAsync_ExcludedFrameworkComponent_3_1_Verification .WithFile(this.projectAssetsJsonFileName, osAgnostic) .ExecuteDetectorAsync(); - var developmentDependencies = componentRecorder.GetDetectedComponents().Where(c => componentRecorder.GetEffectiveDevDependencyValue(c.Component.Id).GetValueOrDefault()); + var developmentDependencies = GetNuGetComponents(componentRecorder).Where(c => componentRecorder.GetEffectiveDevDependencyValue(c.Component.Id).GetValueOrDefault()); developmentDependencies.Should().HaveCount(10, "Omitted framework assemblies are missing."); developmentDependencies.Should().Contain(c => c.Component.Id.StartsWith("System.Reflection "), "System.Reflection should be treated as a development dependency."); } @@ -240,7 +258,7 @@ public async Task ScanDirectoryAsync_DependencyGraph_3_1_VerificationAsync() "System.Text.Json 4.6.0 - NuGet", }; - var detectedComponents = componentRecorder.GetDetectedComponents(); + var detectedComponents = GetNuGetComponents(componentRecorder); var componentDetectionCommon = detectedComponents.First(x => x.Component.Id.Contains("Microsoft.Extensions.DependencyModel")); var dependencies = graph.GetDependenciesForComponent(componentDetectionCommon.Component.Id); @@ -249,7 +267,7 @@ public async Task ScanDirectoryAsync_DependencyGraph_3_1_VerificationAsync() dependencies.Should().Contain(expectedId); } - detectedComponents.Should().HaveSameCount(graph.GetComponents()); + detectedComponents.Should().AllSatisfy(component => graph.GetComponents().Should().Contain(component.Component.Id)); // Top level dependencies look like this: // (we expect all non-proj and non-framework to show up as explicit refs, so those will be absent from the check) @@ -262,7 +280,7 @@ public async Task ScanDirectoryAsync_DependencyGraph_3_1_VerificationAsync() "System.Runtime.Loader", }; - foreach (var componentId in graph.GetComponents()) + foreach (var componentId in detectedComponents.Select(component => component.Component.Id)) { var component = detectedComponents.First(x => x.Component.Id == componentId); var expectedExplicitRefValue = expectedExplicitRefs.Contains(((NuGetComponent)component.Component).Name); @@ -285,7 +303,7 @@ public async Task ScanDirectoryAsync_ExcludedFrameworkComponent_1x_2x_Verificati .WithFile(this.projectAssetsJsonFileName, testResource) .ExecuteDetectorAsync(); - var detectedComponents = componentRecorder.GetDetectedComponents(); + var detectedComponents = GetNuGetComponents(componentRecorder); detectedComponents.Should().AllSatisfy(c => componentRecorder.GetEffectiveDevDependencyValue(c.Component.Id).Should().BeTrue($"{c.Component.Id} should be a dev dependency")); } @@ -298,7 +316,7 @@ public async Task ScanDirectoryAsync_ExcludedFrameworkComponent_1_1_web_Verifica .WithFile(this.projectAssetsJsonFileName, TestResources.project_assets_1_1_web) .ExecuteDetectorAsync(); - var detectedComponents = componentRecorder.GetDetectedComponents(); + var detectedComponents = GetNuGetComponents(componentRecorder); detectedComponents.Should().HaveCount(169, "Find expected dependencies."); var developmentDependencies = detectedComponents.Where(c => componentRecorder.GetEffectiveDevDependencyValue(c.Component.Id).GetValueOrDefault()); @@ -313,7 +331,7 @@ public async Task ScanDirectoryAsync_ExcludedFrameworkComponent_8_0_web_Verifica .WithFile(this.projectAssetsJsonFileName, osAgnostic) .ExecuteDetectorAsync(); - var detectedComponents = componentRecorder.GetDetectedComponents(); + var detectedComponents = GetNuGetComponents(componentRecorder); detectedComponents.Should().AllSatisfy(c => componentRecorder.GetEffectiveDevDependencyValue(c.Component.Id).Should().BeTrue(), "All should be development dependencies"); } @@ -326,7 +344,7 @@ public async Task ScanDirectoryAsync_ExcludedFrameworkComponent_42_15_web_Verifi .ExecuteDetectorAsync(); // net42.15 is not a known framework, but it can import framework packages from the closest known framework. - var detectedComponents = componentRecorder.GetDetectedComponents(); + var detectedComponents = GetNuGetComponents(componentRecorder); detectedComponents.Should().AllSatisfy(c => componentRecorder.GetEffectiveDevDependencyValue(c.Component.Id).Should().BeTrue(), "All should be development dependencies"); } @@ -338,7 +356,7 @@ public async Task ScanDirectoryAsync_ExcludedFrameworkComponent_8_0_multi_framew .WithFile(this.projectAssetsJsonFileName, osAgnostic) .ExecuteDetectorAsync(); - var developmentDependencies = componentRecorder.GetDetectedComponents().Where(c => componentRecorder.GetEffectiveDevDependencyValue(c.Component.Id).GetValueOrDefault()); + var developmentDependencies = GetNuGetComponents(componentRecorder).Where(c => componentRecorder.GetEffectiveDevDependencyValue(c.Component.Id).GetValueOrDefault()); developmentDependencies.Should().HaveCount(3, "Omitted framework assemblies are missing."); developmentDependencies.Should().Contain(c => c.Component.Id.StartsWith("Microsoft.Extensions.Primitives "), "Microsoft.Extensions.Primitives should be treated as a development dependency."); developmentDependencies.Should().Contain(c => c.Component.Id.StartsWith("System.IO.Packaging "), "System.IO.Packaging should be treated as a development dependency."); @@ -352,7 +370,7 @@ public async Task ScanDirectoryAsync_ExcludedFrameworkComponent_6_0_8_0_multi_fr .WithFile(this.projectAssetsJsonFileName, osAgnostic) .ExecuteDetectorAsync(); - var developmentDependencies = componentRecorder.GetDetectedComponents().Where(c => componentRecorder.GetEffectiveDevDependencyValue(c.Component.Id).GetValueOrDefault()); + var developmentDependencies = GetNuGetComponents(componentRecorder).Where(c => componentRecorder.GetEffectiveDevDependencyValue(c.Component.Id).GetValueOrDefault()); developmentDependencies.Should().HaveCount(2, "Omitted framework assemblies are missing."); developmentDependencies.Should().Contain(c => c.Component.Id.StartsWith("Microsoft.Extensions.Primitives "), "Microsoft.Extensions.Primitives should be treated as a development dependency."); developmentDependencies.Should().NotContain(c => c.Component.Id.StartsWith("System.IO.Packaging "), "System.IO.Packaging should not be treated as a development dependency."); @@ -365,7 +383,7 @@ public async Task ScanDirectoryAsync_PackageDownload_VerificationAsync() .WithFile(this.projectAssetsJsonFileName, TestResources.project_assets_packageDownload) .ExecuteDetectorAsync(); - var dependencies = componentRecorder.GetDetectedComponents(); + var dependencies = GetNuGetComponents(componentRecorder); dependencies.Should().HaveCount(3, "PackageDownload dependencies should exist."); dependencies.Should().AllSatisfy(c => componentRecorder.GetEffectiveDevDependencyValue(c.Component.Id).Should().BeTrue(), "All PackageDownloads should be development dependencies"); dependencies.Select(c => c.Component).Should().AllBeOfType(); @@ -394,6 +412,9 @@ public async Task ScanDirectory_NoPackageSpecAsync() dependencyGraphs.Should().BeEmpty(); } + private static List GetNuGetComponents(IComponentRecorder componentRecorder) => + componentRecorder.GetDetectedComponents().Where(component => component.Component is NuGetComponent).ToList(); + private string Convert22SampleToOSAgnostic(string project_assets) { if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) diff --git a/test/Microsoft.ComponentDetection.Detectors.Tests/nuget/MSBuildBinaryLogComponentDetectorTests.cs b/test/Microsoft.ComponentDetection.Detectors.Tests/nuget/MSBuildBinaryLogComponentDetectorTests.cs index b67d133e6..fbf82a9c1 100644 --- a/test/Microsoft.ComponentDetection.Detectors.Tests/nuget/MSBuildBinaryLogComponentDetectorTests.cs +++ b/test/Microsoft.ComponentDetection.Detectors.Tests/nuget/MSBuildBinaryLogComponentDetectorTests.cs @@ -12,6 +12,8 @@ namespace Microsoft.ComponentDetection.Detectors.Tests.NuGet; using AwesomeAssertions; using Microsoft.Build.Framework; using Microsoft.ComponentDetection.Common.DependencyGraph; +using Microsoft.ComponentDetection.Common.Telemetry; +using Microsoft.ComponentDetection.Common.Telemetry.Records; using Microsoft.ComponentDetection.Contracts; using Microsoft.ComponentDetection.Contracts.Internal; using Microsoft.ComponentDetection.Contracts.TypedComponent; @@ -158,6 +160,48 @@ public async Task Fallback_PackageDownload_RegisteredAsDevDependency() recorder.GetEffectiveDevDependencyValue(download.Component.Id).Should().BeTrue(); } + [TestMethod] + [DoNotParallelize] + public async Task BinlogParseFailure_WarnsEmitsTelemetryAndFallsBack() + { + var telemetryServiceMock = new Mock(); + FailedParsingFileRecord? telemetryRecord = null; + telemetryServiceMock + .Setup(x => x.PostRecord(It.IsAny())) + .Callback(record => telemetryRecord = record as FailedParsingFileRecord); + TelemetryRelay.Instance.Init([telemetryServiceMock.Object]); + + try + { + var loggerMock = new Mock>(); + var logOutput = new List(); + loggerMock.CaptureLogOutput(logOutput); + var exception = new InvalidOperationException("Invalid binary log"); + + var (result, recorder) = await ExecuteWithBinlogAsync( + [], + SimpleAssetsJson("Newtonsoft.Json", "13.0.1"), + loggerMock: loggerMock, + binlogException: exception); + + result.ResultCode.Should().Be(ProcessingResultCode.Success); + recorder.GetDetectedComponents() + .Where(component => component.Component is NuGetComponent) + .Select(component => (NuGetComponent)component.Component) + .Should().ContainSingle(nuget => nuget.Name == "Newtonsoft.Json"); + logOutput.Should().Contain(message => message.Contains($"Failed to process binlog file: {BinlogFilePath}")); + + telemetryRecord.Should().NotBeNull(); + telemetryRecord.DetectorId.Should().Be("MSBuildBinaryLog"); + telemetryRecord.FilePath.Should().Be(BinlogFilePath); + telemetryRecord.ExceptionMessage.Should().Be(exception.Message); + } + finally + { + TelemetryRelay.Instance.Init([]); + } + } + // ================================================================ // Binlog-enhanced tests – mock IBinLogProcessor injected via // the internal constructor (accessible through InternalsVisibleTo). @@ -669,14 +713,23 @@ private static ITaskItem CreateTaskItemMock(string itemSpec, bool isDevelopmentD IReadOnlyList projectInfos, string assetsJson, string? binlogPath = null, - string? assetsLocation = null) + string? assetsLocation = null, + Mock>? loggerMock = null, + Exception? binlogException = null) { binlogPath ??= BinlogFilePath; assetsLocation ??= AssetsFilePath; var binLogProcessorMock = new Mock(); - binLogProcessorMock - .Setup(x => x.ExtractProjectInfo(binlogPath, It.IsAny(), It.IsAny())) - .Returns(projectInfos); + var extractProjectInfoSetup = binLogProcessorMock + .Setup(x => x.ExtractProjectInfo(binlogPath, It.IsAny(), It.IsAny())); + if (binlogException == null) + { + extractProjectInfoSetup.Returns(projectInfos); + } + else + { + extractProjectInfoSetup.Throws(binlogException); + } var walkerMock = new Mock(); var streamFactoryMock = new Mock(); @@ -687,7 +740,7 @@ private static ITaskItem CreateTaskItemMock(string itemSpec, bool isDevelopmentD var pathUtilityMock = new Mock(); pathUtilityMock.Setup(x => x.NormalizePath(It.IsAny())).Returns(p => p); pathUtilityMock.Setup(x => x.GetParentDirectory(It.IsAny())).Returns(p => Path.GetDirectoryName(p) ?? string.Empty); - var loggerMock = new Mock>(); + loggerMock ??= new Mock>(); var detector = new MSBuildBinaryLogComponentDetector( streamFactoryMock.Object, diff --git a/test/Microsoft.ComponentDetection.Orchestrator.Tests/Experiments/LinuxApplicationLayerExperimentTests.cs b/test/Microsoft.ComponentDetection.Orchestrator.Tests/Experiments/LinuxApplicationLayerExperimentTests.cs index fdfc38443..f6d8c4677 100644 --- a/test/Microsoft.ComponentDetection.Orchestrator.Tests/Experiments/LinuxApplicationLayerExperimentTests.cs +++ b/test/Microsoft.ComponentDetection.Orchestrator.Tests/Experiments/LinuxApplicationLayerExperimentTests.cs @@ -48,13 +48,16 @@ public void IsInControlGroup_FileBasedDetectors_ReturnsTrue() var nuGetDetector = new NuGetComponentDetector(null!, null!, null!); this.experiment.IsInControlGroup(nuGetDetector).Should().BeTrue(); - var nuGetProjectCentricDetector = new NuGetProjectModelProjectCentricComponentDetector( + var msBuildBinaryLogDetector = new MSBuildBinaryLogComponentDetector( + null!, + null!, + null!, null!, null!, null!, null! ); - this.experiment.IsInControlGroup(nuGetProjectCentricDetector).Should().BeTrue(); + this.experiment.IsInControlGroup(msBuildBinaryLogDetector).Should().BeTrue(); var nuGetPackagesConfigDetector = new NuGetPackagesConfigDetector(null!, null!, null!); this.experiment.IsInControlGroup(nuGetPackagesConfigDetector).Should().BeTrue(); @@ -109,13 +112,16 @@ public void IsInExperimentGroup_FileBasedDetectors_ReturnsTrue() var nuGetDetector = new NuGetComponentDetector(null!, null!, null!); this.experiment.IsInExperimentGroup(nuGetDetector).Should().BeTrue(); - var nuGetProjectCentricDetector = new NuGetProjectModelProjectCentricComponentDetector( + var msBuildBinaryLogDetector = new MSBuildBinaryLogComponentDetector( + null!, + null!, + null!, null!, null!, null!, null! ); - this.experiment.IsInExperimentGroup(nuGetProjectCentricDetector).Should().BeTrue(); + this.experiment.IsInExperimentGroup(msBuildBinaryLogDetector).Should().BeTrue(); var nuGetPackagesConfigDetector = new NuGetPackagesConfigDetector(null!, null!, null!); this.experiment.IsInExperimentGroup(nuGetPackagesConfigDetector).Should().BeTrue(); @@ -171,13 +177,16 @@ public void ShouldRecord_FileBasedDetectors_AlwaysReturnsTrue() var nuGetDetector = new NuGetComponentDetector(null!, null!, null!); this.experiment.ShouldRecord(nuGetDetector, 0).Should().BeTrue(); - var nuGetProjectCentricDetector = new NuGetProjectModelProjectCentricComponentDetector( + var msBuildBinaryLogDetector = new MSBuildBinaryLogComponentDetector( + null!, + null!, + null!, null!, null!, null!, null! ); - this.experiment.ShouldRecord(nuGetProjectCentricDetector, 0).Should().BeTrue(); + this.experiment.ShouldRecord(msBuildBinaryLogDetector, 0).Should().BeTrue(); var nuGetPackagesConfigDetector = new NuGetPackagesConfigDetector(null!, null!, null!); this.experiment.ShouldRecord(nuGetPackagesConfigDetector, 0).Should().BeTrue(); diff --git a/test/Microsoft.ComponentDetection.Orchestrator.Tests/Services/DetectorRestrictionServiceTests.cs b/test/Microsoft.ComponentDetection.Orchestrator.Tests/Services/DetectorRestrictionServiceTests.cs index e8b601072..b2e5fba5b 100644 --- a/test/Microsoft.ComponentDetection.Orchestrator.Tests/Services/DetectorRestrictionServiceTests.cs +++ b/test/Microsoft.ComponentDetection.Orchestrator.Tests/Services/DetectorRestrictionServiceTests.cs @@ -22,6 +22,7 @@ public class DetectorRestrictionServiceTests private Mock thirdDetectorMock; private Mock retiredNpmDetector; private Mock newNpmDetector; + private Mock msBuildBinaryLogDetector; private IComponentDetector[] detectors; private DetectorRestrictionService serviceUnderTest; @@ -34,6 +35,7 @@ public void TestInitialize() this.thirdDetectorMock = this.GenerateDetector("ThirdDetector"); this.retiredNpmDetector = this.GenerateDetector("MSLicenseDevNpm"); this.newNpmDetector = this.GenerateDetector("NpmWithRoots"); + this.msBuildBinaryLogDetector = this.GenerateDetector("MSBuildBinaryLog"); this.detectors = [ @@ -41,8 +43,8 @@ public void TestInitialize() this.secondDetectorMock.Object, this.thirdDetectorMock.Object, this.retiredNpmDetector.Object, - this.newNpmDetector.Object, + this.msBuildBinaryLogDetector.Object, ]; this.serviceUnderTest = new DetectorRestrictionService(this.logger.Object); @@ -121,6 +123,35 @@ public void WithRestrictions_CorrectsRetiredDetector() .Should().OnlyContain(item => item == this.newNpmDetector.Object); } + [TestMethod] + [DataRow("DotNet")] + [DataRow("nugetprojectcentric")] + public void WithRestrictions_CorrectsRetiredDotNetDetector(string retiredDetectorId) + { + var restrictions = new DetectorRestrictions + { + AllowedDetectorIds = [retiredDetectorId], + }; + + var restrictedDetectors = this.serviceUnderTest.ApplyRestrictions(restrictions, this.detectors); + + restrictedDetectors.Should().ContainSingle() + .Which.Should().Be(this.msBuildBinaryLogDetector.Object); + } + + [TestMethod] + public void WithRestrictions_DeduplicatesRetiredDotNetDetectorReplacements() + { + var restrictions = new DetectorRestrictions + { + AllowedDetectorIds = ["DotNet", "NuGetProjectCentric", "MSBuildBinaryLog", "FirstDetector"], + }; + + var restrictedDetectors = this.serviceUnderTest.ApplyRestrictions(restrictions, this.detectors); + + restrictedDetectors.Should().BeEquivalentTo([this.msBuildBinaryLogDetector.Object, this.firstDetectorMock.Object]); + } + [TestMethod] public void WithRestrictions_FiltersBasedOnCategory() { diff --git a/test/Microsoft.ComponentDetection.VerificationTests/ComponentDetectionIntegrationTests.cs b/test/Microsoft.ComponentDetection.VerificationTests/ComponentDetectionIntegrationTests.cs index abea9e4e6..223f9b106 100644 --- a/test/Microsoft.ComponentDetection.VerificationTests/ComponentDetectionIntegrationTests.cs +++ b/test/Microsoft.ComponentDetection.VerificationTests/ComponentDetectionIntegrationTests.cs @@ -15,8 +15,8 @@ namespace Microsoft.ComponentDetection.VerificationTests; [TestClass] public class ComponentDetectionIntegrationTests { - // Detectors intentionally removed (e.g., promoted/merged into another detector). MavenWithFallback was promoted into MvnCli. - private static readonly HashSet IntentionallyRemovedDetectors = ["MavenWithFallback"]; + // Detectors intentionally removed after their behavior was promoted or merged into another detector. + private static readonly HashSet IntentionallyRemovedDetectors = ["MavenWithFallback", "NuGetProjectCentric", "DotNet"]; private string oldLogFileContents; private string newLogFileContents; @@ -58,8 +58,8 @@ public void CheckManifestFiles_ExcludingExperimentalDetectors() // if any are lost, error, new ones should come with a bumped detector version, which is checked during the detectors counts test. var experimentalDetectorsId = this.GetExperimentalDetectorsId(this.newScanResult.DetectorsInScan, this.oldScanResult.DetectorsInScan); - var newComponents = this.newScanResult.ComponentsFound.Where(c => !experimentalDetectorsId.Contains(c.DetectorId)); - var oldComponents = this.oldScanResult.ComponentsFound.Where(c => !experimentalDetectorsId.Contains(c.DetectorId)); + var newComponents = this.newScanResult.ComponentsFound.Where(c => !experimentalDetectorsId.Contains(c.DetectorId) && !IntentionallyRemovedDetectors.Contains(c.DetectorId)); + var oldComponents = this.oldScanResult.ComponentsFound.Where(c => !experimentalDetectorsId.Contains(c.DetectorId) && !IntentionallyRemovedDetectors.Contains(c.DetectorId)); var newComponentDictionary = this.GetComponentDictionary(newComponents); var oldComponentDictionary = this.GetComponentDictionary(oldComponents); From 6159482d102f7929c41eb0d3b5fbb533b8a45a32 Mon Sep 17 00:00:00 2001 From: Ryan Brandenburg Date: Fri, 18 Sep 2026 11:52:53 -0700 Subject: [PATCH 07/15] Make Swift Detector experimental and improve output (#1866) * Make Swift Detector experimental * Expand Package.resolved data reporting * Serialization constructor * Fix test constructors * Fix serialization issue * PR feedback * Emit SwiftComponent afterall * Fix test expectations * Make the SwiftComponent Id truely representative * Don't include extranious ID information --- docs/detectors/README.md | 6 +- .../TypedComponent/SwiftComponent.cs | 48 +++++--- .../swiftpm/SwiftResolvedComponentDetector.cs | 29 +++-- .../TypedComponentSerializationTests.cs | 17 +++ .../ComponentDetectorTests.cs | 10 ++ .../SwiftComponentTests.cs | 108 ++++++++++++++---- .../SwiftResolvedDetectorTests.cs | 66 ++++------- 7 files changed, 183 insertions(+), 101 deletions(-) diff --git a/docs/detectors/README.md b/docs/detectors/README.md index e39cd1b26..cdf250b60 100644 --- a/docs/detectors/README.md +++ b/docs/detectors/README.md @@ -122,9 +122,9 @@ - [Swift](swift.md) -| Detector | Status | -| ------------------------------- | ---------- | -| SwiftResolvedComponentDetector | DefaultOff | +| Detector | Status | +| ------------------------------ | ------------ | +| SwiftResolvedComponentDetector | Experimental | - [Uv](uv.md) diff --git a/src/Microsoft.ComponentDetection.Contracts/TypedComponent/SwiftComponent.cs b/src/Microsoft.ComponentDetection.Contracts/TypedComponent/SwiftComponent.cs index ef76dc0d5..ca516d022 100644 --- a/src/Microsoft.ComponentDetection.Contracts/TypedComponent/SwiftComponent.cs +++ b/src/Microsoft.ComponentDetection.Contracts/TypedComponent/SwiftComponent.cs @@ -11,31 +11,43 @@ namespace Microsoft.ComponentDetection.Contracts.TypedComponent; /// public class SwiftComponent : TypedComponent { - private readonly Uri packageUrl; - - private readonly string hash; - /// /// Initializes a new instance of the class. /// /// The name of the component. /// The version of the component. - /// The package URL of the component. - /// The hash of the component. - public SwiftComponent(string name, string version, string packageUrl, string hash) + /// The source repository URL of the component. + /// The Git commit hash of the component. + /// The Swift package kind. + public SwiftComponent(string name, string version, string repositoryUrl, string hash, string kind) { this.Name = this.ValidateRequiredInput(name, nameof(name), nameof(ComponentType.Swift)); this.Version = this.ValidateRequiredInput(version, nameof(version), nameof(ComponentType.Swift)); - this.ValidateRequiredInput(packageUrl, nameof(packageUrl), nameof(ComponentType.Swift)); - this.packageUrl = new Uri(packageUrl); - this.hash = this.ValidateRequiredInput(hash, nameof(hash), nameof(ComponentType.Swift)); + this.ValidateRequiredInput(repositoryUrl, nameof(repositoryUrl), nameof(ComponentType.Swift)); + this.RepositoryUrl = new Uri(repositoryUrl); + this.Kind = this.ValidateRequiredInput(kind, nameof(kind), nameof(ComponentType.Swift)); + this.CommitHash = this.ValidateRequiredInput(hash, nameof(hash), nameof(ComponentType.Swift)); + } + + public SwiftComponent() + { + /* Reserved for deserialization */ } [JsonPropertyName("name")] - public string Name { get; } + public string Name { get; set; } [JsonPropertyName("version")] - public string Version { get; } + public string Version { get; set; } + + [JsonPropertyName("kind")] + public string Kind { get; set; } + + [JsonPropertyName("commitHash")] + public string CommitHash { get; set; } + + [JsonPropertyName("repositoryUrl")] + public Uri RepositoryUrl { get; set; } [JsonIgnore] public override ComponentType Type => ComponentType.Swift; @@ -52,25 +64,25 @@ public SwiftComponent(string name, string version, string packageUrl, string has version: this.Version, qualifiers: new SortedDictionary { - { "repository_url", this.packageUrl.AbsoluteUri }, + { "repository_url", this.RepositoryUrl.AbsoluteUri }, }, subpath: null); - protected override string ComputeBaseId() => $"{this.Name} {this.Version} - {this.Type}"; + protected override string ComputeBaseId() => $"{this.RepositoryUrl.AbsoluteUri} {this.CommitHash} - {this.Type}"; private string GetNamespaceFromPackageUrl() { // In the case of github.com, the namespace should contain the user/organization // See https://github.com/package-url/purl-spec/blob/master/PURL-TYPES.rst#swift - var uppercaseHost = this.packageUrl.Host.ToUpperInvariant(); + var uppercaseHost = this.RepositoryUrl.Host.ToUpperInvariant(); if (uppercaseHost.Contains("GITHUB.COM")) { // The first segment of the URL will contain the user or organization for GitHub - var firstSegment = this.packageUrl.Segments[1].Trim('/'); - return $"{this.packageUrl.Host}/{firstSegment}"; + var firstSegment = this.RepositoryUrl.Segments[1].Trim('/'); + return $"{this.RepositoryUrl.Host}/{firstSegment}"; } // In the default case of a generic host, the namespace should be the just the host - return this.packageUrl.Host; + return this.RepositoryUrl.Host; } } diff --git a/src/Microsoft.ComponentDetection.Detectors/swiftpm/SwiftResolvedComponentDetector.cs b/src/Microsoft.ComponentDetection.Detectors/swiftpm/SwiftResolvedComponentDetector.cs index a5d4e6bc4..241b7cbb3 100644 --- a/src/Microsoft.ComponentDetection.Detectors/swiftpm/SwiftResolvedComponentDetector.cs +++ b/src/Microsoft.ComponentDetection.Detectors/swiftpm/SwiftResolvedComponentDetector.cs @@ -15,7 +15,7 @@ namespace Microsoft.ComponentDetection.Detectors.Swift; /// /// Detects Swift Package Manager components. /// -public class SwiftResolvedComponentDetector : FileComponentDetector, IDefaultOffComponentDetector +public class SwiftResolvedComponentDetector : FileComponentDetector, IExperimentalDetector { // We are only interested in packages coming from remote sources such as git // The Package Kind is not an enum because the Swift Package Manager contract does not specify the possible values. @@ -39,7 +39,7 @@ public SwiftResolvedComponentDetector( public override IEnumerable SupportedComponentTypes => [ComponentType.Swift]; - public override int Version => 1; + public override int Version => 2; protected override Task OnFileFoundAsync( ProcessRequest processRequest, @@ -74,25 +74,22 @@ private void ProcessPackageResolvedFile(ISingleFileComponentRecorder singleFileC { if (package.Kind == TargetSwiftPackageKind) { + ArgumentException.ThrowIfNullOrWhiteSpace(package.Identity); + // The version of the package is not always available. var version = package.State.Version ?? package.State.Branch ?? package.State.Revision; - var detectedSwiftComponent = new SwiftComponent( + var detectedComponent = new SwiftComponent( name: package.Identity, version: version, - packageUrl: package.Location, - hash: package.State.Revision); - var newDetectedSwiftComponent = new DetectedComponent(component: detectedSwiftComponent); - singleFileComponentRecorder.RegisterUsage(newDetectedSwiftComponent); - - // We also register a Git component for the same package so that the git URL is registered. - // Swift Package Manager directly downloads the package from the git URL. - var detectedGitComponent = new GitComponent( - repositoryUrl: new Uri(package.Location), - commitHash: package.State.Revision, - tag: version); - var newDetectedGitComponent = new DetectedComponent(component: detectedGitComponent); - singleFileComponentRecorder.RegisterUsage(newDetectedGitComponent); + repositoryUrl: package.Location, + hash: package.State.Revision, + kind: package.Kind); + singleFileComponentRecorder.RegisterUsage(new DetectedComponent(component: detectedComponent)); + } + else + { + this.Logger.LogWarning("Unsupported package kind: {Kind}", package.Kind); } } catch (Exception exception) diff --git a/test/Microsoft.ComponentDetection.Contracts.Tests/TypedComponentSerializationTests.cs b/test/Microsoft.ComponentDetection.Contracts.Tests/TypedComponentSerializationTests.cs index 7f9f5ba8d..a351bd674 100644 --- a/test/Microsoft.ComponentDetection.Contracts.Tests/TypedComponentSerializationTests.cs +++ b/test/Microsoft.ComponentDetection.Contracts.Tests/TypedComponentSerializationTests.cs @@ -172,6 +172,23 @@ public void TypedComponent_Serialization_Git() gitComponent.CommitHash.Should().Be("SomeHash"); } + [TestMethod] + public void TypedComponent_Serialization_Swift() + { + TypedComponent component = new SwiftComponent( + "alamofire", + "5.9.1", + "https://github.com/Alamofire/Alamofire", + "f455c2975872ccd2d9c81594c658af65716e9b9a", + "remoteSourceControl"); + + var json = JsonSerializer.Serialize(component); + var deserializedComponent = JsonSerializer.Deserialize(json); + + deserializedComponent.Should().BeOfType(); + deserializedComponent.Should().BeEquivalentTo(component); + } + [TestMethod] public void TypedComponent_Serialization_RubyGems() { diff --git a/test/Microsoft.ComponentDetection.Detectors.Tests/ComponentDetectorTests.cs b/test/Microsoft.ComponentDetection.Detectors.Tests/ComponentDetectorTests.cs index 66d812b43..63034fbc3 100644 --- a/test/Microsoft.ComponentDetection.Detectors.Tests/ComponentDetectorTests.cs +++ b/test/Microsoft.ComponentDetection.Detectors.Tests/ComponentDetectorTests.cs @@ -82,6 +82,16 @@ public void CondaLockComponentDetector_ImplementsIExperimentalDetector() condaLockDetector.Should().BeAssignableTo("because CondaLockComponentDetector should implement IExperimentalDetector"); } + [TestMethod] + public void SwiftResolvedComponentDetector_ImplementsIExperimentalDetector() + { + var swiftDetector = this.detectors.SingleOrDefault(d => d.Id == "Swift"); + + swiftDetector.Should().NotBeNull("because SwiftResolvedComponentDetector should be registered"); + swiftDetector.Should().BeAssignableTo("because SwiftResolvedComponentDetector should implement IExperimentalDetector"); + swiftDetector.Should().NotBeAssignableTo("because SwiftResolvedComponentDetector should be enabled by default"); + } + [TestMethod] public void MSBuildBinaryLogComponentDetector_IsDefaultOnAndSupersedesLegacyDetectors() { diff --git a/test/Microsoft.ComponentDetection.Detectors.Tests/SwiftComponentTests.cs b/test/Microsoft.ComponentDetection.Detectors.Tests/SwiftComponentTests.cs index 78165e6e9..d0d75f0bd 100644 --- a/test/Microsoft.ComponentDetection.Detectors.Tests/SwiftComponentTests.cs +++ b/test/Microsoft.ComponentDetection.Detectors.Tests/SwiftComponentTests.cs @@ -3,6 +3,7 @@ namespace Microsoft.ComponentDetection.Detectors.Tests.Swift; using System; using System.Collections.Generic; +using System.Text.Json; using AwesomeAssertions; using Microsoft.ComponentDetection.Contracts.TypedComponent; using Microsoft.VisualStudio.TestTools.UnitTesting; @@ -16,15 +17,77 @@ public void Constructor_ShouldInitializeProperties() { var name = "alamofire"; var version = "5.9.1"; - var packageUrl = "https://github.com/Alamofire/Alamofire"; - var hash = "f455c2975872ccd2d9c81594c658af65716e9b9a"; + var repositoryUrl = "https://github.com/Alamofire/Alamofire"; + var kind = "remoteSourceControl"; + var commitHash = "f455c2975872ccd2d9c81594c658af65716e9b9a"; - var component = new SwiftComponent(name, version, packageUrl, hash); + var component = new SwiftComponent(name, version, repositoryUrl, commitHash, kind); component.Name.Should().Be(name); component.Version.Should().Be(version); + component.Kind.Should().Be(kind); + component.CommitHash.Should().Be(commitHash); + component.RepositoryUrl.Should().Be(new Uri(repositoryUrl)); component.Type.Should().Be(ComponentType.Swift); - component.Id.Should().Be($"{name} {version} - {component.Type}"); + component.Id.Should().Be( + $"{repositoryUrl} {commitHash} - {component.Type}"); + } + + [TestMethod] + public void Id_ShouldDistinguishRepositoriesAndCommitHashes() + { + var component = new SwiftComponent( + "alamofire", + "5.9.1", + "https://github.com/Alamofire/Alamofire", + "f455c2975872ccd2d9c81594c658af65716e9b9a", + "remoteSourceControl"); + var differentRepository = new SwiftComponent( + "alamofire", + "5.9.1", + "https://github.com/example/Alamofire", + "f455c2975872ccd2d9c81594c658af65716e9b9a", + "remoteSourceControl"); + var differentCommit = new SwiftComponent( + "alamofire", + "5.9.1", + "https://github.com/Alamofire/Alamofire", + "63dfa86548c4e5d5c6fd6ed42f638e388cbce529", + "remoteSourceControl"); + + component.Id.Should().NotBe(differentRepository.Id); + component.Id.Should().NotBe(differentCommit.Id); + } + + [TestMethod] + public void Serialization_ShouldIncludeCommitHash() + { + var commitHash = "f455c2975872ccd2d9c81594c658af65716e9b9a"; + TypedComponent component = new SwiftComponent( + name: "alamofire", + version: "5.9.1", + repositoryUrl: "https://github.com/Alamofire/Alamofire", + hash: commitHash, + kind: "remoteSourceControl"); + + using var json = JsonDocument.Parse(JsonSerializer.Serialize(component)); + + json.RootElement.GetProperty("kind").GetString().Should().Be("remoteSourceControl"); + json.RootElement.GetProperty("commitHash").GetString().Should().Be(commitHash); + } + + [TestMethod] + public void Constructor_ShouldThrowException_WhenKindIsNull() + { + Action action = () => + new SwiftComponent( + "alamofire", + "5.9.1", + "https://github.com/Alamofire/Alamofire", + "f455c2975872ccd2d9c81594c658af65716e9b9a", + null + ); + action.Should().Throw().WithMessage("*kind*"); } [TestMethod] @@ -35,7 +98,8 @@ public void Constructor_ShouldThrowException_WhenNameIsNull() null, "5.9.1", "https://github.com/Alamofire/Alamofire", - "f455c2975872ccd2d9c81594c658af65716e9b9a" + "f455c2975872ccd2d9c81594c658af65716e9b9a", + "remoteSourceControl" ); action.Should().Throw().WithMessage("*name*"); } @@ -48,22 +112,24 @@ public void Constructor_ShouldThrowException_WhenVersionIsNull() "alamofire", null, "https://github.com/Alamofire/Alamofire", - "f455c2975872ccd2d9c81594c658af65716e9b9a" + "f455c2975872ccd2d9c81594c658af65716e9b9a", + "remoteSourceControl" ); action.Should().Throw().WithMessage("*version*"); } [TestMethod] - public void Constructor_ShouldThrowException_WhenPackageUrlIsNull() + public void Constructor_ShouldThrowException_WhenRepositoryUrlIsNull() { Action action = () => new SwiftComponent( "alamofire", "5.9.1", null, - "f455c2975872ccd2d9c81594c658af65716e9b9a" + "f455c2975872ccd2d9c81594c658af65716e9b9a", + "remoteSourceControl" ); - action.Should().Throw().WithMessage("*packageUrl*"); + action.Should().Throw().WithMessage("*repositoryUrl*"); } [TestMethod] @@ -74,20 +140,22 @@ public void Constructor_ShouldThrowException_WhenHashIsNull() "alamofire", "5.9.1", "https://github.com/Alamofire/Alamofire", - null + null, + "remoteSourceControl" ); action.Should().Throw().WithMessage("*hash*"); } [TestMethod] - public void Constructor_ShouldThrowException_WhenPackageUrlIsInvalid() + public void Constructor_ShouldThrowException_WhenRepositoryUrlIsInvalid() { Action action = () => new SwiftComponent( "alamofire", "5.9.1", "invalid-url", - "f455c2975872ccd2d9c81594c658af65716e9b9a" + "f455c2975872ccd2d9c81594c658af65716e9b9a", + "remoteSourceControl" ); action.Should().Throw(); } @@ -97,17 +165,17 @@ public void PackageURL_ShouldReturnCorrectPackageURL_GithubHostname() { var name = "alamofire"; var version = "5.9.1"; - var packageUrl = "https://github.com/Alamofire/Alamofire"; + var repositoryUrl = "https://github.com/Alamofire/Alamofire"; var hash = "f455c2975872ccd2d9c81594c658af65716e9b9a"; - var component = new SwiftComponent(name, version, packageUrl, hash); + var component = new SwiftComponent(name, version, repositoryUrl, hash, "remoteSourceControl"); var expectedPackageURL = new PackageURL( type: "swift", @namespace: "github.com/Alamofire", name: name, version: version, - qualifiers: new SortedDictionary { { "repository_url", packageUrl } }, + qualifiers: new SortedDictionary { { "repository_url", repositoryUrl } }, subpath: null ); @@ -119,10 +187,10 @@ public void PackageURL_ShouldReturnCorrectPackageURL_GithubHostname_Alternate() { var name = "alamofire"; var version = "5.9.1"; - var packageUrl = "https://giTHub.com/Alamofire/Alamofire"; + var repositoryUrl = "https://giTHub.com/Alamofire/Alamofire"; var hash = "f455c2975872ccd2d9c81594c658af65716e9b9a"; - var component = new SwiftComponent(name, version, packageUrl, hash); + var component = new SwiftComponent(name, version, repositoryUrl, hash, "remoteSourceControl"); var expectedPackageURL = new PackageURL( type: "swift", @@ -144,17 +212,17 @@ public void PackageURL_ShouldReturnCorrectPackageURL_OtherHostname() { var name = "alamofire"; var version = "5.9.1"; - var packageUrl = "https://otherhostname.com/Alamofire/Alamofire"; + var repositoryUrl = "https://otherhostname.com/Alamofire/Alamofire"; var hash = "f455c2975872ccd2d9c81594c658af65716e9b9a"; - var component = new SwiftComponent(name, version, packageUrl, hash); + var component = new SwiftComponent(name, version, repositoryUrl, hash, "remoteSourceControl"); var expectedPackageURL = new PackageURL( type: "swift", @namespace: "otherhostname.com", name: name, version: version, - qualifiers: new SortedDictionary { { "repository_url", packageUrl } }, + qualifiers: new SortedDictionary { { "repository_url", repositoryUrl } }, subpath: null ); diff --git a/test/Microsoft.ComponentDetection.Detectors.Tests/SwiftResolvedDetectorTests.cs b/test/Microsoft.ComponentDetection.Detectors.Tests/SwiftResolvedDetectorTests.cs index e70aaea30..e8f13b0eb 100644 --- a/test/Microsoft.ComponentDetection.Detectors.Tests/SwiftResolvedDetectorTests.cs +++ b/test/Microsoft.ComponentDetection.Detectors.Tests/SwiftResolvedDetectorTests.cs @@ -1,7 +1,6 @@ #nullable disable namespace Microsoft.ComponentDetection.Detectors.Tests.Swift; -using System; using System.Linq; using System.Threading.Tasks; using AwesomeAssertions; @@ -44,8 +43,7 @@ public async Task Test_GivenDetectorWithValidFile_WhenScan_ThenScanIsSuccessfulA var detectedComponents = componentRecorder.GetDetectedComponents(); - // Two components are detected because this detector registers a SwiftPM and Git component. - detectedComponents.Should().HaveCount(2); + detectedComponents.Should().ContainSingle(); var typedComponents = detectedComponents.Select(c => c.Component).ToList(); @@ -53,14 +51,9 @@ public async Task Test_GivenDetectorWithValidFile_WhenScan_ThenScanIsSuccessfulA new SwiftComponent( name: "alamofire", version: "5.9.1", - packageUrl: "https://github.com/Alamofire/Alamofire", - hash: "f455c2975872ccd2d9c81594c658af65716e9b9a")); - - typedComponents.Should().ContainEquivalentOf( - new GitComponent( - repositoryUrl: new Uri("https://github.com/Alamofire/Alamofire"), - commitHash: "f455c2975872ccd2d9c81594c658af65716e9b9a", - tag: "5.9.1")); + repositoryUrl: "https://github.com/Alamofire/Alamofire", + hash: "f455c2975872ccd2d9c81594c658af65716e9b9a", + kind: "remoteSourceControl")); } // Test for several packages @@ -78,8 +71,7 @@ public async Task Test_GivenDetectorWithValidFileWithMultiplePackages_WhenScan_T var detectedComponents = componentRecorder.GetDetectedComponents(); - // Two components are detected because this detector registers a SwiftPM and Git component. - detectedComponents.Should().HaveCount(6); + detectedComponents.Should().HaveCount(3); var typedComponents = detectedComponents.Select(c => c.Component).ToList(); @@ -87,20 +79,17 @@ public async Task Test_GivenDetectorWithValidFileWithMultiplePackages_WhenScan_T new SwiftComponent( name: "alamofire", version: "5.6.0", - packageUrl: "https://github.com/Alamofire/Alamofire", - hash: "63dfa86548c4e5d5c6fd6ed42f638e388cbce529")); + repositoryUrl: "https://github.com/Alamofire/Alamofire", + hash: "63dfa86548c4e5d5c6fd6ed42f638e388cbce529", + kind: "remoteSourceControl")); typedComponents.Should().ContainEquivalentOf( - new GitComponent( - repositoryUrl: new Uri("https://github.com/sideeffect-io/AsyncExtensions"), - commitHash: "3442d3d046800f1974bda096faaf0ac510b21154", - tag: "0.5.3")); - - typedComponents.Should().ContainEquivalentOf( - new GitComponent( - repositoryUrl: new Uri("https://github.com/devicekit/DeviceKit.git"), - commitHash: "d37e70cb2646666dcf276d7d3d4a9760a41ff8a6", - tag: "4.9.0")); + new SwiftComponent( + name: "asyncextensions", + version: "0.5.3", + repositoryUrl: "https://github.com/sideeffect-io/AsyncExtensions", + hash: "3442d3d046800f1974bda096faaf0ac510b21154", + kind: "remoteSourceControl")); } // Duplicate packages @@ -142,9 +131,8 @@ public async Task Test_GivenDetectorWithValidFileWithDuplicatePackages_WhenScan_ var detectedComponents = componentRecorder.GetDetectedComponents(); - // Two components are detected because this detector registers a SwiftPM and Git component. // The duplicate package is not registered. - detectedComponents.Should().HaveCount(2); + detectedComponents.Should().ContainSingle(); var typedComponents = detectedComponents.Select(c => c.Component).ToList(); @@ -152,14 +140,9 @@ public async Task Test_GivenDetectorWithValidFileWithDuplicatePackages_WhenScan_ new SwiftComponent( name: "alamofire", version: "5.9.1", - packageUrl: "https://github.com/Alamofire/Alamofire", - hash: "f455c2975872ccd2d9c81594c658af65716e9b9a")); - - typedComponents.Should().ContainEquivalentOf( - new GitComponent( - repositoryUrl: new Uri("https://github.com/Alamofire/Alamofire"), - commitHash: "f455c2975872ccd2d9c81594c658af65716e9b9a", - tag: "5.9.1")); + repositoryUrl: "https://github.com/Alamofire/Alamofire", + hash: "f455c2975872ccd2d9c81594c658af65716e9b9a", + kind: "remoteSourceControl")); } [TestMethod] @@ -409,7 +392,7 @@ public async Task Test_GivenResolvedPackageWithoutVersion_WhenScan_ThenScanIsSuc .ExecuteDetectorAsync(); scanResult.ResultCode.Should().Be(ProcessingResultCode.Success); - componentRecorder.GetDetectedComponents().Should().HaveCount(2); + componentRecorder.GetDetectedComponents().Should().ContainSingle(); var detectedComponents = componentRecorder.GetDetectedComponents(); var typedComponents = detectedComponents.Select(c => c.Component).ToList(); @@ -418,14 +401,9 @@ public async Task Test_GivenResolvedPackageWithoutVersion_WhenScan_ThenScanIsSuc new SwiftComponent( name: "alamofire", version: "f455c2975872ccd2d9c81594c658af65716e9b9a", - packageUrl: "https://github.com/Alamofire/Alamofire", - hash: "f455c2975872ccd2d9c81594c658af65716e9b9a")); - - typedComponents.Should().ContainEquivalentOf( - new GitComponent( - repositoryUrl: new Uri("https://github.com/Alamofire/Alamofire"), - commitHash: "f455c2975872ccd2d9c81594c658af65716e9b9a", - tag: "f455c2975872ccd2d9c81594c658af65716e9b9a")); + repositoryUrl: "https://github.com/Alamofire/Alamofire", + hash: "f455c2975872ccd2d9c81594c658af65716e9b9a", + kind: "remoteSourceControl")); } [System.Diagnostics.CodeAnalysis.SuppressMessage("StyleCop.CSharp.OrderingRules", "SA1201:Elements should appear in the correct order", Justification = "Test data that is better placed at the end of the file.")] From 9a7761e95f76e91a585f75616b7ee8a3997c7c87 Mon Sep 17 00:00:00 2001 From: Ryan Brandenburg Date: Fri, 18 Sep 2026 15:27:10 -0700 Subject: [PATCH 08/15] Skip docker ping on windows (#1874) --- .../DockerServiceTests.cs | 6 ++---- .../Microsoft.ComponentDetection.Common.Tests.csproj | 1 - 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/test/Microsoft.ComponentDetection.Common.Tests/DockerServiceTests.cs b/test/Microsoft.ComponentDetection.Common.Tests/DockerServiceTests.cs index b8ae3c34a..2f7e54162 100644 --- a/test/Microsoft.ComponentDetection.Common.Tests/DockerServiceTests.cs +++ b/test/Microsoft.ComponentDetection.Common.Tests/DockerServiceTests.cs @@ -8,7 +8,6 @@ namespace Microsoft.ComponentDetection.Common.Tests; using Microsoft.Extensions.Logging; using Microsoft.VisualStudio.TestTools.UnitTesting; using Moq; -using Polly; [TestClass] [TestCategory("Governance/All")] @@ -25,11 +24,10 @@ public class DockerServiceTests public DockerServiceTests() => this.dockerService = new DockerService(this.loggerMock.Object); [TestMethod] + [OSCondition(ConditionMode.Exclude, OperatingSystems.Windows)] public async Task DockerService_CanPingDockerAsync() { - var canPingDocker = await Policy.HandleResult(result => !result) - .WaitAndRetryAsync(3, retryAttempt => TimeSpan.FromSeconds(5)) - .ExecuteAsync(() => this.dockerService.CanPingDockerAsync()); + var canPingDocker = await this.dockerService.CanPingDockerAsync(); canPingDocker.Should().BeTrue(); } diff --git a/test/Microsoft.ComponentDetection.Common.Tests/Microsoft.ComponentDetection.Common.Tests.csproj b/test/Microsoft.ComponentDetection.Common.Tests/Microsoft.ComponentDetection.Common.Tests.csproj index 8d4ae758e..496f5be8c 100644 --- a/test/Microsoft.ComponentDetection.Common.Tests/Microsoft.ComponentDetection.Common.Tests.csproj +++ b/test/Microsoft.ComponentDetection.Common.Tests/Microsoft.ComponentDetection.Common.Tests.csproj @@ -9,7 +9,6 @@ - From a35446f34c87640d220e14111582b9250f8bc8f3 Mon Sep 17 00:00:00 2001 From: Aayush Maini Date: Mon, 21 Sep 2026 14:13:56 -0700 Subject: [PATCH 09/15] Fix Go direct dependency classification (#1877) * Fix Go direct dependency classification Mark go.mod requirements as explicitly referenced unless they carry the // indirect marker. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: b9f67746-b363-43ac-9c8c-764b1890c63c * Bump Go detector version Increment the detector version because explicit-reference metadata now changes for go.mod dependencies. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: b9f67746-b363-43ac-9c8c-764b1890c63c --------- Co-authored-by: Aayush Maini Copilot-Session: b9f67746-b363-43ac-9c8c-764b1890c63c --- .../go/GoComponentDetector.cs | 2 +- .../go/Parsers/GoModParser.cs | 7 ++- .../GoComponentDetectorTests.cs | 62 +++++++++++++++++++ 3 files changed, 67 insertions(+), 4 deletions(-) diff --git a/src/Microsoft.ComponentDetection.Detectors/go/GoComponentDetector.cs b/src/Microsoft.ComponentDetection.Detectors/go/GoComponentDetector.cs index 2625d2f56..12f3b1835 100644 --- a/src/Microsoft.ComponentDetection.Detectors/go/GoComponentDetector.cs +++ b/src/Microsoft.ComponentDetection.Detectors/go/GoComponentDetector.cs @@ -49,7 +49,7 @@ public GoComponentDetector( public override IEnumerable SupportedComponentTypes { get; } = [ComponentType.Go]; - public override int Version => 10; + public override int Version => 11; protected async override Task> OnPrepareDetectionAsync( IObservable processRequests, diff --git a/src/Microsoft.ComponentDetection.Detectors/go/Parsers/GoModParser.cs b/src/Microsoft.ComponentDetection.Detectors/go/Parsers/GoModParser.cs index f0972f755..2e1862186 100644 --- a/src/Microsoft.ComponentDetection.Detectors/go/Parsers/GoModParser.cs +++ b/src/Microsoft.ComponentDetection.Detectors/go/Parsers/GoModParser.cs @@ -95,8 +95,6 @@ public async Task ParseAsync( record.GoModVersion = line[3..].Trim(); } - // In go >= 1.17, direct dependencies are listed as "require x/y v1.2.3", and transitive dependencies - // are listed in the require () section if (line.StartsWith(StartString)) { this.TryRegisterDependencyFromModLine(file, line[StartString.Length..], singleFileComponentRecorder, replacePathDirectives, moduleReplacements); @@ -152,7 +150,10 @@ private void TryRegisterDependencyFromModLine(IComponentStream file, string line goComponent = new GoComponent(replacement.TargetPathOrModule, replacement.TargetVersion ?? goComponent.Version); } - singleFileComponentRecorder.RegisterUsage(new DetectedComponent(goComponent)); + var isExplicitReferencedDependency = !line.TrimEnd().EndsWith("// indirect", StringComparison.Ordinal); + singleFileComponentRecorder.RegisterUsage( + new DetectedComponent(goComponent), + isExplicitReferencedDependency: isExplicitReferencedDependency); } private bool TryToCreateGoComponentFromModLine(string line, out GoComponent goComponent) diff --git a/test/Microsoft.ComponentDetection.Detectors.Tests/GoComponentDetectorTests.cs b/test/Microsoft.ComponentDetection.Detectors.Tests/GoComponentDetectorTests.cs index 55388510e..2f8858199 100644 --- a/test/Microsoft.ComponentDetection.Detectors.Tests/GoComponentDetectorTests.cs +++ b/test/Microsoft.ComponentDetection.Detectors.Tests/GoComponentDetectorTests.cs @@ -110,6 +110,68 @@ public async Task TestGoModDetectorWithValidFile_ReturnsSuccessfullyAsync() discoveredComponents.Where(component => component.Component.Id == "github.com/kr/pretty v0.1.0 - Go").Should().ContainSingle(); } + [TestMethod] + [DataRow(false)] + [DataRow(true)] + public async Task GoModDetector_DirectRequirementsAreExplicitlyReferenced(bool isGoCliAvailable) + { + var goMod = + @"module example.com/main + +go 1.24.3 + +require github.com/direct/single v1.0.0 + +require ( + github.com/direct/block v2.0.0 + github.com/transitive/indirect v3.0.0 // indirect +)"; + + this.envVarService.Setup(x => x.IsEnvironmentVariableValueTrue("DisableGoCliScan")).Returns(false); + this.commandLineMock + .Setup(x => x.CanCommandBeLocatedAsync("go", null, null, It.IsAny())) + .ReturnsAsync(isGoCliAvailable); + this.commandLineMock + .Setup(x => x.ExecuteCommandAsync( + "go", + null, + null, + It.IsAny(), + It.IsAny())) + .ReturnsAsync(new CommandLineExecutionResult + { + ExitCode = 0, + StdOut = "go version go1.24.3 windows/amd64", + }); + this.commandLineMock + .Setup(x => x.ExecuteCommandAsync( + "go", + null, + It.IsAny(), + It.IsAny(), + It.IsAny())) + .ReturnsAsync(new CommandLineExecutionResult + { + ExitCode = 0, + StdOut = + "example.com/main github.com/direct/single@v1.0.0\n" + + "example.com/main github.com/direct/block@v2.0.0\n" + + "example.com/main github.com/transitive/indirect@v3.0.0", + }); + + this.SetupActualGoModParser(); + var (scanResult, componentRecorder) = await this.detectorTestUtility + .WithFile("go.mod", goMod) + .ExecuteDetectorAsync(); + + scanResult.ResultCode.Should().Be(ProcessingResultCode.Success); + + var dependencyGraph = componentRecorder.GetDependencyGraphsByLocation().Values.Should().ContainSingle().Which; + dependencyGraph.GetAllExplicitlyReferencedComponents().Should().BeEquivalentTo( + "github.com/direct/single v1.0.0 - Go", + "github.com/direct/block v2.0.0 - Go"); + } + [TestMethod] public async Task TestGoModDetector_CommentsOnFile_CommentsAreIgnoredAsync() { From d88864436a7152d80721c53b264ce9f364ac4180 Mon Sep 17 00:00:00 2001 From: Cameron Taggart Date: Fri, 25 Sep 2026 10:25:02 -0600 Subject: [PATCH 10/15] Fix Cargo.lock Git source dependency matching (#1881) * Fix Cargo.lock Git source dependency matching Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Skip fork-only detector reminder failure and bump Rust detector version Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../detector-version-bump-reminder.yml | 2 + .../rust/Parsers/RustCargoLockParser.cs | 10 +- .../rust/RustSbomDetector.cs | 2 +- .../RustCargoLockParserTests.cs | 100 ++++++++++++++++++ .../RustSbomDetectorTests.cs | 1 + 5 files changed, 113 insertions(+), 2 deletions(-) diff --git a/.github/workflows/detector-version-bump-reminder.yml b/.github/workflows/detector-version-bump-reminder.yml index 1370cce30..a2a9ae1db 100644 --- a/.github/workflows/detector-version-bump-reminder.yml +++ b/.github/workflows/detector-version-bump-reminder.yml @@ -9,6 +9,8 @@ permissions: jobs: comment: + # Fork pushes cannot find PRs opened against the upstream repository. + if: github.event.repository.fork == false runs-on: ubuntu-latest steps: - name: Harden Runner diff --git a/src/Microsoft.ComponentDetection.Detectors/rust/Parsers/RustCargoLockParser.cs b/src/Microsoft.ComponentDetection.Detectors/rust/Parsers/RustCargoLockParser.cs index 87eb1443d..906b078fd 100644 --- a/src/Microsoft.ComponentDetection.Detectors/rust/Parsers/RustCargoLockParser.cs +++ b/src/Microsoft.ComponentDetection.Detectors/rust/Parsers/RustCargoLockParser.cs @@ -40,6 +40,14 @@ internal class RustCargoLockParser : IRustCargoLockParser private static bool IsLocalPackage(CargoPackage package) => package.Source == null; + private static bool SourceMatches(string dependencySource, string packageSource) + { + // Cargo includes the commit in Git package sources but omits it in dependency references. + return string.Equals(dependencySource, packageSource, StringComparison.Ordinal) || + (dependencySource.StartsWith("git+", StringComparison.Ordinal) && + packageSource?.StartsWith(dependencySource + "#", StringComparison.Ordinal) == true); + } + /// /// Parses a Cargo.lock file and records components. /// @@ -185,7 +193,7 @@ private void ProcessDependency( continue; } - if (childSource != null && candidatePackage.Source != childSource) + if (childSource != null && !SourceMatches(childSource, candidatePackage.Source)) { // This does not have the requested source continue; diff --git a/src/Microsoft.ComponentDetection.Detectors/rust/RustSbomDetector.cs b/src/Microsoft.ComponentDetection.Detectors/rust/RustSbomDetector.cs index 38dd575bd..d33bb2baa 100644 --- a/src/Microsoft.ComponentDetection.Detectors/rust/RustSbomDetector.cs +++ b/src/Microsoft.ComponentDetection.Detectors/rust/RustSbomDetector.cs @@ -139,7 +139,7 @@ private enum FileKind public override IEnumerable SupportedComponentTypes => [ComponentType.Cargo]; /// - public override int Version => 2; + public override int Version => 3; /// public override IList SearchPatterns { get; } = [CargoTomlFileName, CargoLockFileName, "*.cargo-sbom.json"]; diff --git a/test/Microsoft.ComponentDetection.Detectors.Tests/RustCargoLockParserTests.cs b/test/Microsoft.ComponentDetection.Detectors.Tests/RustCargoLockParserTests.cs index 159e9dcab..a1249076f 100644 --- a/test/Microsoft.ComponentDetection.Detectors.Tests/RustCargoLockParserTests.cs +++ b/test/Microsoft.ComponentDetection.Detectors.Tests/RustCargoLockParserTests.cs @@ -316,6 +316,106 @@ public async Task DependencySourceMismatch_ParseFailure() failures.Should().Be(1); } + [TestMethod] + public async Task GitDependencyWithCommitFragment_ResolvesRegistryAndGitPackages() + { + var gitSource = "git+https://github.com/dtolnay/itoa?tag=1.0.17"; + var toml = $""" + version = 4 + + [[package]] + name = "cargo-source-repro" + version = "0.1.0" + dependencies = [ + "itoa 1.0.17 ({CratesIo})", + "itoa 1.0.17 ({gitSource})", + ] + + [[package]] + name = "itoa" + version = "1.0.17" + source = "{CratesIo}" + checksum = "92ecc6618181def0457392ccd0ee51198e065e016d1d527a7ac1b6dc7c1f09d2" + + [[package]] + name = "itoa" + version = "1.0.17" + source = "{gitSource}#21d610902fb79eb16b7c155d25574fb7376d9e97" + """; + + var recorder = new Mock(MockBehavior.Loose); + var version = await this.parser.ParseAsync(MakeStream("Cargo.lock", toml), recorder.Object); + + version.Should().Be(4); + var (usages, explicitRoots, edges, failures) = Analyze(recorder); + usages.Should().Be(4); + explicitRoots.Should().Be(2); + edges.Should().Be(0); + failures.Should().Be(0); + } + + [TestMethod] + public async Task RemoteParentDependsOnGitChildWithCommitFragment_RecordsEdge() + { + var gitSource = "git+https://github.com/dtolnay/itoa?tag=1.0.17"; + var toml = $""" + version = 4 + + [[package]] + name = "parent" + version = "0.1.0" + source = "git+https://github.com/example/parent?branch=main#0123456789abcdef0123456789abcdef01234567" + dependencies = [ + "itoa 1.0.17 ({gitSource})", + ] + + [[package]] + name = "itoa" + version = "1.0.17" + source = "{CratesIo}" + + [[package]] + name = "itoa" + version = "1.0.17" + source = "{gitSource}#21d610902fb79eb16b7c155d25574fb7376d9e97" + """; + + var recorder = new Mock(MockBehavior.Loose); + await this.parser.ParseAsync(MakeStream("Cargo.lock", toml), recorder.Object); + + var (usages, explicitRoots, edges, failures) = Analyze(recorder); + usages.Should().Be(6); + explicitRoots.Should().Be(2); + edges.Should().Be(1); + failures.Should().Be(0); + } + + [TestMethod] + public async Task GitDependencyWithDifferentRef_ParseFailure() + { + var toml = """ + [[package]] + name = "root" + version = "0.1.0" + dependencies = [ + "child 1.0.0 (git+https://github.com/example/child?branch=main)", + ] + + [[package]] + name = "child" + version = "1.0.0" + source = "git+https://github.com/example/child?branch=main-v2#0123456789abcdef0123456789abcdef01234567" + """; + + var recorder = new Mock(MockBehavior.Loose); + await this.parser.ParseAsync(MakeStream("Cargo.lock", toml), recorder.Object); + + var (_, explicitRoots, edges, failures) = Analyze(recorder); + explicitRoots.Should().Be(1); + edges.Should().Be(0); + failures.Should().Be(1); + } + [TestMethod] public async Task MalformedDependencyString_ParseFailure() { diff --git a/test/Microsoft.ComponentDetection.Detectors.Tests/RustSbomDetectorTests.cs b/test/Microsoft.ComponentDetection.Detectors.Tests/RustSbomDetectorTests.cs index 6c02aa17d..9a8f9dbd2 100644 --- a/test/Microsoft.ComponentDetection.Detectors.Tests/RustSbomDetectorTests.cs +++ b/test/Microsoft.ComponentDetection.Detectors.Tests/RustSbomDetectorTests.cs @@ -313,6 +313,7 @@ public void TestDetectorProperties() null); detector.Id.Should().Be("RustSbom"); + detector.Version.Should().Be(3); detector.Categories.Should().BeEquivalentTo(["Rust"]); detector.SupportedComponentTypes.Should().BeEquivalentTo([ComponentType.Cargo]); detector.SearchPatterns.Should().BeEquivalentTo(["Cargo.toml", "Cargo.lock", "*.cargo-sbom.json"]); From 6321ef1527eb8b94d4962bf7133f5ef9e032bfca Mon Sep 17 00:00:00 2001 From: Ryan Brandenburg Date: Fri, 25 Sep 2026 11:42:29 -0700 Subject: [PATCH 11/15] Don't report Pip for conda component (#1879) * Don't report Pip for conda component * Cleanup * Fix outdated comment --- .../conda/CondaDependencyResolver.cs | 16 ++-------------- .../conda/CondaLockComponentDetector.cs | 2 +- .../CondaLockComponentDetectorTests.cs | 3 ++- 3 files changed, 5 insertions(+), 16 deletions(-) diff --git a/src/Microsoft.ComponentDetection.Detectors/conda/CondaDependencyResolver.cs b/src/Microsoft.ComponentDetection.Detectors/conda/CondaDependencyResolver.cs index 07f8a80c1..729b4789d 100644 --- a/src/Microsoft.ComponentDetection.Detectors/conda/CondaDependencyResolver.cs +++ b/src/Microsoft.ComponentDetection.Detectors/conda/CondaDependencyResolver.cs @@ -116,7 +116,7 @@ private static List GetPackages(CondaLock condaLock) /// /// Converts a CondaPackage to a TypedComponent. - /// If the condapackage is a python package it will be converted to a + /// If the package has a manager of "pip" it will be converted to a /// PipComponent. Otherwise it will be converted to a CondaComponent. /// /// Conda package metadata is populated from the lock entry and, for older @@ -126,7 +126,7 @@ private static List GetPackages(CondaLock condaLock) /// The TypedComponent. private static TypedComponent CreateComponent(CondaPackage package) { - if (IsPythonPackage(package)) + if (package.Manager?.Equals("pip", StringComparison.OrdinalIgnoreCase) is true) { var pipComponent = new PipComponent(package.Name, package.Version); if (Uri.TryCreate(package.Url, UriKind.Absolute, out var downloadUrl)) @@ -202,16 +202,4 @@ private static (string Channel, string Subdir, string FileName) GetPackageUrlMet Uri.UnescapeDataString(segments[^2]), Uri.UnescapeDataString(segments[^1])); } - - /// - /// Checks if a package is a python package. - /// - /// If the package is either managed by pip, or if it depends on python - /// it is considered a python package. - /// - /// The CondaPackage. - /// True if the package is a python package. - private static bool IsPythonPackage(CondaPackage package) - => package.Manager?.Equals("pip", StringComparison.OrdinalIgnoreCase) == true || - package.Dependencies?.Keys.Any(dependency => dependency.Equals("python", StringComparison.OrdinalIgnoreCase)) == true; } diff --git a/src/Microsoft.ComponentDetection.Detectors/conda/CondaLockComponentDetector.cs b/src/Microsoft.ComponentDetection.Detectors/conda/CondaLockComponentDetector.cs index fff3118fe..bbc754eea 100644 --- a/src/Microsoft.ComponentDetection.Detectors/conda/CondaLockComponentDetector.cs +++ b/src/Microsoft.ComponentDetection.Detectors/conda/CondaLockComponentDetector.cs @@ -30,7 +30,7 @@ public CondaLockComponentDetector( public override IEnumerable SupportedComponentTypes => [ComponentType.Conda, ComponentType.Pip]; - public override int Version { get; } = 3; + public override int Version { get; } = 4; public override IEnumerable Categories => ["Python"]; diff --git a/test/Microsoft.ComponentDetection.Detectors.Tests/CondaLockComponentDetectorTests.cs b/test/Microsoft.ComponentDetection.Detectors.Tests/CondaLockComponentDetectorTests.cs index 255aca210..744f28c6d 100644 --- a/test/Microsoft.ComponentDetection.Detectors.Tests/CondaLockComponentDetectorTests.cs +++ b/test/Microsoft.ComponentDetection.Detectors.Tests/CondaLockComponentDetectorTests.cs @@ -68,6 +68,7 @@ public async Task CondaComponentDetector_TestCondaLockFileAsync() build: pyhd8ed1ab_0 dependencies: urllib3: '>=1.26.5,<2.0' + python: '>=3.' url: https://conda.anaconda.org/conda-forge/noarch/conda-lock-2.1.0-pyhd8ed1ab_0.conda hash: md5: 1e07afcf3d3e371fc3a3681fe9b78e90 @@ -337,7 +338,7 @@ public async Task CondaComponentDetector_CircularDependenciesDoNotOverflowAsync( var detectedComponents = componentRecorder.GetDetectedComponents(); var dependencyGraph = componentRecorder.GetDependencyGraphsByLocation().Values.First(); var pythonId = detectedComponents.Single(component => component.Component is CondaComponent { Name: "python" }).Component.Id; - var pipId = detectedComponents.Single(component => component.Component is PipComponent { Name: "pip" }).Component.Id; + var pipId = detectedComponents.Single(component => component.Component is CondaComponent { Name: "pip" }).Component.Id; scanResult.ResultCode.Should().Be(ProcessingResultCode.Success); detectedComponents.Should().HaveCount(2); From 2107109ac626f55ffd59e89dcc512c6cba88d51b Mon Sep 17 00:00:00 2001 From: Aayush Maini Date: Tue, 29 Sep 2026 13:03:28 -0700 Subject: [PATCH 12/15] Resolve Maven properties in component coordinates (#1882) * Resolve Maven properties in component coordinates Resolve Maven property references in group IDs and artifact IDs as well as versions when using static POM parsing. Skip components whose coordinates remain unresolved and bump the detector version for the output change. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Resolve nested Maven property references Recursively resolve coordinate properties with memoization and cycle detection. Add coverage for nested local and inherited properties and cyclic definitions. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Bound Maven property expansion depth Limit recursive coordinate property expansion to prevent deeply nested POM properties from exhausting the process stack. Leave over-depth coordinates unresolved so they are skipped safely. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Reject malformed Maven property references Treat any residual Maven interpolation marker as unresolved so malformed references are skipped rather than emitted as component coordinates. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Aayush Maini Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../maven/MvnCliComponentDetector.cs | 187 ++++++++++-------- .../maven/PendingComponent.cs | 4 +- .../MvnCliDetectorTests.cs | 186 ++++++++++++++++- 3 files changed, 291 insertions(+), 86 deletions(-) diff --git a/src/Microsoft.ComponentDetection.Detectors/maven/MvnCliComponentDetector.cs b/src/Microsoft.ComponentDetection.Detectors/maven/MvnCliComponentDetector.cs index f25d82fac..334cf304c 100644 --- a/src/Microsoft.ComponentDetection.Detectors/maven/MvnCliComponentDetector.cs +++ b/src/Microsoft.ComponentDetection.Detectors/maven/MvnCliComponentDetector.cs @@ -80,9 +80,10 @@ public class MvnCliComponentDetector : FileComponentDetector private const string GroupIdSelector = "groupId"; private const string ArtifactIdSelector = "artifactId"; private const string VersionSelector = "version"; + private const int MaxPropertyExpansionDepth = 100; - private static readonly Regex VersionRegex = new( - @"^\$\{(.*)\}$", + private static readonly Regex PropertyReferenceRegex = new( + @"\$\{([^{}]+)\}", RegexOptions.Compiled | RegexOptions.IgnoreCase); // Auth error patterns to detect in Maven error output @@ -162,7 +163,7 @@ public MvnCliComponentDetector( public override IEnumerable SupportedComponentTypes => [ComponentType.Maven]; - public override int Version => 5; + public override int Version => 6; public override IEnumerable Categories => [Enum.GetName(typeof(DetectorClass), DetectorClass.Maven)]; @@ -228,6 +229,60 @@ private static string NormalizeEndpointUrl(string rawUrl) return $"{uri.Scheme}://{uri.Host}{port}"; } + private static string ResolveProperties( + string template, + Func resolveProperty) + { + var resolvedProperties = new Dictionary(StringComparer.Ordinal); + var activeProperties = new HashSet(StringComparer.Ordinal); + + string ResolveTemplate(string value, int depth) => + depth >= MaxPropertyExpansionDepth + ? value + : PropertyReferenceRegex.Replace( + value, + match => + { + var variable = match.Groups[1].Value; + if (resolvedProperties.TryGetValue(variable, out var resolvedProperty)) + { + return resolvedProperty; + } + + if (!activeProperties.Add(variable)) + { + return match.Value; + } + + var propertyValue = resolveProperty(variable); + if (propertyValue == null) + { + activeProperties.Remove(variable); + return match.Value; + } + + resolvedProperty = ResolveTemplate(propertyValue, depth + 1); + activeProperties.Remove(variable); + resolvedProperties[variable] = resolvedProperty; + return resolvedProperty; + }); + + return ResolveTemplate(template, 0); + } + + private static string ResolvePropertiesFromLocalOnly( + string template, + Dictionary localVariables) + { + return ResolveProperties( + template, + variable => localVariables.TryGetValue(variable, out var localReplacement) + ? localReplacement + : null); + } + + private static bool ContainsPropertyReference(string value) => value.Contains("${", StringComparison.Ordinal); + private void LogDebugWithId(string message) => this.Logger.LogDebug("{DetectorId}: {Message}", this.Id, message); @@ -705,40 +760,28 @@ private void ProcessPomFileStatically(ProcessRequest processRequest) if (version != null && !version.InnerText.Contains(',')) { var versionRef = version.InnerText.Trim('[', ']'); + var resolvedGroupId = ResolvePropertiesFromLocalOnly(groupId, localVariables); + var resolvedArtifactId = ResolvePropertiesFromLocalOnly(artifactId, localVariables); + var resolvedVersion = ResolvePropertiesFromLocalOnly(versionRef, localVariables); - if (versionRef.StartsWith("${")) - { - // Only resolve immediately if local variable exists (highest priority) - // Otherwise, defer to second pass to ensure proper hierarchy-aware resolution - var resolvedVersion = this.ResolveVersionFromLocalOnly(versionRef, localVariables); - if (!resolvedVersion.StartsWith("${")) - { - // Local variable found - resolve immediately (highest priority) - var component = new MavenComponent(groupId, artifactId, resolvedVersion); - var detectedComponent = new DetectedComponent(component); - singleFileComponentRecorder.RegisterUsage(detectedComponent); - Interlocked.Increment(ref this.staticParserComponentCount); - } - else - { - // No local variable - defer to second pass for hierarchy-aware resolution - // This ensures we consider all variable definitions before resolving - this.pendingComponents.Enqueue(new PendingComponent( - groupId, - artifactId, - versionRef, - singleFileComponentRecorder, - filePath)); - } - } - else + if (!ContainsPropertyReference(resolvedGroupId) + && !ContainsPropertyReference(resolvedArtifactId) + && !ContainsPropertyReference(resolvedVersion)) { - // Direct version - register immediately - var component = new MavenComponent(groupId, artifactId, versionRef); + var component = new MavenComponent(resolvedGroupId, resolvedArtifactId, resolvedVersion); var detectedComponent = new DetectedComponent(component); singleFileComponentRecorder.RegisterUsage(detectedComponent); Interlocked.Increment(ref this.staticParserComponentCount); } + else + { + this.pendingComponents.Enqueue(new PendingComponent( + groupId, + artifactId, + versionRef, + singleFileComponentRecorder, + filePath)); + } } else { @@ -997,32 +1040,6 @@ private void TrackMavenProjectCoordinates(XmlDocument document, XmlNamespaceMana } } - /// - /// Resolves a version template using only local variables from the current file. - /// This ensures immediate resolution only when the variable is defined in the same file (highest priority). - /// - /// The version template with variables (e.g., "${revision}"). - /// Local variables from the current file. - /// The resolved version string, or the original template if local variable not found. - private string ResolveVersionFromLocalOnly(string versionTemplate, Dictionary localVariables) - { - var resolvedVersion = versionTemplate; - var match = VersionRegex.Match(versionTemplate); - - if (match.Success) - { - var variable = match.Groups[1].Captures[0].ToString(); - - // Only check local variables (same file priority) - if (localVariables.TryGetValue(variable, out var localReplacement)) - { - resolvedVersion = versionTemplate.Replace("${" + variable + "}", localReplacement); - } - } - - return resolvedVersion; - } - private IEnumerable ExtractFailedEndpoints(string errorMessage) { if (string.IsNullOrWhiteSpace(errorMessage)) @@ -1158,10 +1175,21 @@ private void ResolvePendingComponents() { try { - var resolvedVersion = this.ResolveVersionWithHierarchyAwareness(pendingComponent.VersionTemplate, pendingComponent.FilePath); - if (!resolvedVersion.StartsWith("${")) + var resolvedGroupId = this.ResolvePropertiesWithHierarchyAwareness( + pendingComponent.GroupIdTemplate, + pendingComponent.FilePath); + var resolvedArtifactId = this.ResolvePropertiesWithHierarchyAwareness( + pendingComponent.ArtifactIdTemplate, + pendingComponent.FilePath); + var resolvedVersion = this.ResolvePropertiesWithHierarchyAwareness( + pendingComponent.VersionTemplate, + pendingComponent.FilePath); + + if (!ContainsPropertyReference(resolvedGroupId) + && !ContainsPropertyReference(resolvedArtifactId) + && !ContainsPropertyReference(resolvedVersion)) { - var component = new MavenComponent(pendingComponent.GroupId, pendingComponent.ArtifactId, resolvedVersion); + var component = new MavenComponent(resolvedGroupId, resolvedArtifactId, resolvedVersion); var detectedComponent = new DetectedComponent(component); pendingComponent.Recorder.RegisterUsage(detectedComponent); Interlocked.Increment(ref this.staticParserComponentCount); @@ -1171,10 +1199,10 @@ private void ResolvePendingComponents() { skippedCount++; this.Logger.LogDebug( - "Version string {Version} for component {Group}/{Artifact} could not be resolved and a component will not be recorded. File: {File}", + "Coordinate {Group}/{Artifact}/{Version} could not be fully resolved and a component will not be recorded. File: {File}", + resolvedGroupId, + resolvedArtifactId, resolvedVersion, - pendingComponent.GroupId, - pendingComponent.ArtifactId, pendingComponent.FilePath); } } @@ -1184,8 +1212,8 @@ private void ResolvePendingComponents() this.Logger.LogError( e, "Failed to resolve pending component {Group}/{Artifact} from {File}", - pendingComponent.GroupId, - pendingComponent.ArtifactId, + pendingComponent.GroupIdTemplate, + pendingComponent.ArtifactIdTemplate, pendingComponent.FilePath); } } @@ -1194,32 +1222,29 @@ private void ResolvePendingComponents() } /// - /// Resolves a version template with hierarchy-aware precedence. + /// Resolves a coordinate template with hierarchy-aware precedence. /// When multiple variable definitions exist, picks the closest ancestor to the requesting file. /// This implements Maven's property inheritance rule: child properties take precedence over parent properties. /// - /// The version template with variables (e.g., "${revision}"). + /// The coordinate template containing Maven property references. /// The file path of the POM requesting the variable resolution. - /// The resolved version string, or the original template if variables cannot be resolved. - private string ResolveVersionWithHierarchyAwareness(string versionTemplate, string requestingFilePath) + /// The template with all hierarchy-defined properties resolved. + private string ResolvePropertiesWithHierarchyAwareness(string template, string requestingFilePath) { - var resolvedVersion = versionTemplate; - var match = VersionRegex.Match(versionTemplate); + var unresolvedVariables = new HashSet(StringComparer.Ordinal); - if (match.Success) + return ResolveProperties( + template, + variable => { - var variable = match.Groups[1].Captures[0].ToString(); - - // Use Maven-compliant hierarchy search: current → parent → grandparent var foundValue = this.FindVariableInMavenHierarchy(variable, requestingFilePath); if (foundValue != null) { - resolvedVersion = versionTemplate.Replace("${" + variable + "}", foundValue.Value.Value); + return foundValue.Value.Value; } - else + + if (unresolvedVariables.Add(variable)) { - // Variable not found in Maven hierarchy - log at debug level since unresolved - // properties are common (profiles, external parents, etc.) and aggregate count in telemetry Interlocked.Increment(ref this.unresolvedVariableCount); this.Logger.LogDebug( "{DetectorId}: Variable {Variable} not found in Maven hierarchy for {File}", @@ -1227,9 +1252,9 @@ private string ResolveVersionWithHierarchyAwareness(string versionTemplate, stri variable, Path.GetFileName(requestingFilePath)); } - } - return resolvedVersion; + return null; + }); } /// diff --git a/src/Microsoft.ComponentDetection.Detectors/maven/PendingComponent.cs b/src/Microsoft.ComponentDetection.Detectors/maven/PendingComponent.cs index 82c92b6fc..8ab65ec10 100644 --- a/src/Microsoft.ComponentDetection.Detectors/maven/PendingComponent.cs +++ b/src/Microsoft.ComponentDetection.Detectors/maven/PendingComponent.cs @@ -7,8 +7,8 @@ namespace Microsoft.ComponentDetection.Detectors.Maven; /// Represents a component with unresolved variables that needs second-pass processing. /// internal record PendingComponent( - string GroupId, - string ArtifactId, + string GroupIdTemplate, + string ArtifactIdTemplate, string VersionTemplate, ISingleFileComponentRecorder Recorder, string FilePath); diff --git a/test/Microsoft.ComponentDetection.Detectors.Tests/MvnCliDetectorTests.cs b/test/Microsoft.ComponentDetection.Detectors.Tests/MvnCliDetectorTests.cs index 3847b9ba0..b36647e30 100644 --- a/test/Microsoft.ComponentDetection.Detectors.Tests/MvnCliDetectorTests.cs +++ b/test/Microsoft.ComponentDetection.Detectors.Tests/MvnCliDetectorTests.cs @@ -86,6 +86,184 @@ public async Task WhenMavenCliNotAvailable_FallsBackToStaticParsing_Async() mavenComponent.Version.Should().Be("3.12.0"); } + [TestMethod] + public async Task StaticParser_ResolvesPropertiesInAllCoordinateFields_Async() + { + this.mavenCommandServiceMock.Setup(x => x.MavenCLIExistsAsync()) + .ReturnsAsync(false); + + var pomXmlContent = @" + + 4.0.0 + com.test + my-app + 1.0.0 + + org.apache.spark + spark + ${artifact.prefix}-core + 2.12 + 3.4.1 + + + + ${spark.group} + ${artifact.name}_${scala.version.major} + ${spark.version} + + +"; + + var (detectorResult, componentRecorder) = await this.DetectorTestUtility + .WithFile("pom.xml", pomXmlContent) + .ExecuteDetectorAsync(); + + detectorResult.ResultCode.Should().Be(ProcessingResultCode.Success); + + var detectedComponents = componentRecorder.GetDetectedComponents(); + detectedComponents.Should().ContainSingle(); + + var mavenComponent = detectedComponents.First().Component as MavenComponent; + mavenComponent.Should().NotBeNull(); + mavenComponent.GroupId.Should().Be("org.apache.spark"); + mavenComponent.ArtifactId.Should().Be("spark-core_2.12"); + mavenComponent.Version.Should().Be("3.4.1"); + } + + [TestMethod] + public async Task StaticParser_SkipsComponentWithUnresolvedCoordinateProperty_Async() + { + this.mavenCommandServiceMock.Setup(x => x.MavenCLIExistsAsync()) + .ReturnsAsync(false); + + var pomXmlContent = @" + + 4.0.0 + com.test + my-app + 1.0.0 + + + org.apache.spark + spark-core_${scala.version.major} + 3.4.1 + + +"; + + var (detectorResult, componentRecorder) = await this.DetectorTestUtility + .WithFile("pom.xml", pomXmlContent) + .ExecuteDetectorAsync(); + + detectorResult.ResultCode.Should().Be(ProcessingResultCode.Success); + componentRecorder.GetDetectedComponents().Should().BeEmpty(); + } + + [TestMethod] + public async Task StaticParser_SkipsComponentsWithMalformedPropertyReferences_Async() + { + this.mavenCommandServiceMock.Setup(x => x.MavenCLIExistsAsync()) + .ReturnsAsync(false); + + var pomXmlContent = @" + + 4.0.0 + com.test + my-app + 1.0.0 + + + org.apache.spark + spark-core + ${} + + + org.apache.spark + spark-sql + ${missing + + +"; + + var (detectorResult, componentRecorder) = await this.DetectorTestUtility + .WithFile("pom.xml", pomXmlContent) + .ExecuteDetectorAsync(); + + detectorResult.ResultCode.Should().Be(ProcessingResultCode.Success); + componentRecorder.GetDetectedComponents().Should().BeEmpty(); + } + + [TestMethod] + public async Task StaticParser_SkipsComponentWithCyclicCoordinateProperties_Async() + { + this.mavenCommandServiceMock.Setup(x => x.MavenCLIExistsAsync()) + .ReturnsAsync(false); + + var pomXmlContent = @" + + 4.0.0 + com.test + my-app + 1.0.0 + + ${artifact.alias} + ${artifact.name} + + + + org.apache.spark + ${artifact.name} + 3.4.1 + + +"; + + var (detectorResult, componentRecorder) = await this.DetectorTestUtility + .WithFile("pom.xml", pomXmlContent) + .ExecuteDetectorAsync(); + + detectorResult.ResultCode.Should().Be(ProcessingResultCode.Success); + componentRecorder.GetDetectedComponents().Should().BeEmpty(); + } + + [TestMethod] + public async Task StaticParser_SkipsComponentExceedingPropertyExpansionDepth_Async() + { + this.mavenCommandServiceMock.Setup(x => x.MavenCLIExistsAsync()) + .ReturnsAsync(false); + + var nestedProperties = string.Join( + "\n", + Enumerable.Range(0, 101) + .Select(index => $" ${{property{index + 1}}}")); + + var pomXmlContent = $@" + + 4.0.0 + com.test + my-app + 1.0.0 + +{nestedProperties} + spark-core + + + + org.apache.spark + ${{property0}} + 3.4.1 + + +"; + + var (detectorResult, componentRecorder) = await this.DetectorTestUtility + .WithFile("pom.xml", pomXmlContent) + .ExecuteDetectorAsync(); + + detectorResult.ResultCode.Should().Be(ProcessingResultCode.Success); + componentRecorder.GetDetectedComponents().Should().BeEmpty(); + } + [TestMethod] public async Task WhenMavenCliNotAvailable_DetectsMultipleDependencies_Async() { @@ -435,7 +613,7 @@ public async Task StaticParser_IgnoresDependenciesWithUnresolvablePropertyVersio } [TestMethod] - public async Task StaticParser_ResolvesVariableFromPreviousFile_Async() + public async Task StaticParser_ResolvesCoordinatePropertiesFromParent_Async() { // Arrange - Test case 1: Variable defined in parent POM, referenced in child POM // Uses Maven's standard parent inheritance mechanism @@ -455,6 +633,8 @@ public async Task StaticParser_ResolvesVariableFromPreviousFile_Async() pom 3.12.0 + 2.12 + ${scala.version.major} "; @@ -470,7 +650,7 @@ public async Task StaticParser_ResolvesVariableFromPreviousFile_Async() org.apache.commons - commons-lang3 + commons-lang3_${scala.binary.version} ${commons.version} @@ -491,7 +671,7 @@ public async Task StaticParser_ResolvesVariableFromPreviousFile_Async() var mavenComponent = detectedComponents.First().Component as MavenComponent; mavenComponent.Should().NotBeNull(); mavenComponent.GroupId.Should().Be("org.apache.commons"); - mavenComponent.ArtifactId.Should().Be("commons-lang3"); + mavenComponent.ArtifactId.Should().Be("commons-lang3_2.12"); mavenComponent.Version.Should().Be("3.12.0"); } From b44ed3760a0c5de56a0c8c304c5525181956000a Mon Sep 17 00:00:00 2001 From: Julian Date: Wed, 30 Sep 2026 17:13:09 -0400 Subject: [PATCH 13/15] Add exception handling for non-Dockerfile formats (#1883) * Add exception handling for non-Dockerfile formats and add tests for non-dockerfiles * Don't check for dockerfiles in `node_modules` or subfolders of it. * Add logging verification for Shiki language definition test --- .../dockerfile/DockerfileComponentDetector.cs | 24 +++++++++- .../DockerfileComponentDetectorTests.cs | 48 +++++++++++++++++++ 2 files changed, 71 insertions(+), 1 deletion(-) diff --git a/src/Microsoft.ComponentDetection.Detectors/dockerfile/DockerfileComponentDetector.cs b/src/Microsoft.ComponentDetection.Detectors/dockerfile/DockerfileComponentDetector.cs index 031358105..00760e336 100644 --- a/src/Microsoft.ComponentDetection.Detectors/dockerfile/DockerfileComponentDetector.cs +++ b/src/Microsoft.ComponentDetection.Detectors/dockerfile/DockerfileComponentDetector.cs @@ -3,6 +3,8 @@ namespace Microsoft.ComponentDetection.Detectors.Dockerfile; using System; using System.Collections.Generic; using System.IO; +using System.Linq; +using System.Reactive.Linq; using System.Threading; using System.Threading.Tasks; using Microsoft.ComponentDetection.Common; @@ -10,6 +12,7 @@ namespace Microsoft.ComponentDetection.Detectors.Dockerfile; using Microsoft.ComponentDetection.Contracts.Internal; using Microsoft.ComponentDetection.Contracts.TypedComponent; using Microsoft.Extensions.Logging; +using Sprache; using Valleysoft.DockerfileModel; public class DockerfileComponentDetector : FileComponentDetector, IExperimentalDetector @@ -41,6 +44,15 @@ public DockerfileComponentDetector( public override int Version => 1; + protected override IList SkippedFolders => ["node_modules"]; + + protected override Task> OnPrepareDetectionAsync( + IObservable processRequests, + IDictionary detectorArgs, + CancellationToken cancellationToken = default) => + Task.FromResult(processRequests.Where(processRequest => + !this.IsInSkippedFolder(processRequest.ComponentStream.Location))); + protected override async Task OnFileFoundAsync(ProcessRequest processRequest, IDictionary detectorArgs, CancellationToken cancellationToken = default) { var singleFileComponentRecorder = processRequest.SingleFileComponentRecorder; @@ -59,12 +71,22 @@ protected override async Task OnFileFoundAsync(ProcessRequest processRequest, ID var stageNameMap = new Dictionary(); var dockerFileComponent = this.ParseDockerFileAsync(contents, file.Location, singleFileComponentRecorder, stageNameMap); } + catch (ParseException e) + { + this.Logger.LogWarning(e, "Ignoring file that doesn't appear to be a Dockerfile: {Location}", filePath); + } catch (Exception e) { - this.Logger.LogError(e, "The file doesn't appear to be a Dockerfile: {Location}", filePath); + this.Logger.LogError(e, "Failed to process Dockerfile: {Location}", filePath); } } + private bool IsInSkippedFolder(string filePath) + { + var pathSegments = filePath.Split(['/', '\\'], StringSplitOptions.RemoveEmptyEntries); + return this.SkippedFolders.Any(skippedFolder => pathSegments.Contains(skippedFolder, StringComparer.OrdinalIgnoreCase)); + } + private Task ParseDockerFileAsync(string fileContents, string fileLocation, ISingleFileComponentRecorder singleFileComponentRecorder, Dictionary stageNameMap) { var dockerfileModel = Dockerfile.Parse(fileContents); diff --git a/test/Microsoft.ComponentDetection.Detectors.Tests/DockerfileComponentDetectorTests.cs b/test/Microsoft.ComponentDetection.Detectors.Tests/DockerfileComponentDetectorTests.cs index 62bf38b24..90d685172 100644 --- a/test/Microsoft.ComponentDetection.Detectors.Tests/DockerfileComponentDetectorTests.cs +++ b/test/Microsoft.ComponentDetection.Detectors.Tests/DockerfileComponentDetectorTests.cs @@ -1,6 +1,8 @@ #nullable enable namespace Microsoft.ComponentDetection.Detectors.Tests; +using System; +using System.IO; using System.Linq; using System.Threading.Tasks; using AwesomeAssertions; @@ -8,6 +10,7 @@ namespace Microsoft.ComponentDetection.Detectors.Tests; using Microsoft.ComponentDetection.Contracts.TypedComponent; using Microsoft.ComponentDetection.Detectors.Dockerfile; using Microsoft.ComponentDetection.TestsUtilities; +using Microsoft.Extensions.Logging; using Microsoft.VisualStudio.TestTools.UnitTesting; using Moq; @@ -210,6 +213,51 @@ public async Task TestDockerfile_PrefixedFilenameAsync() componentRecorder.GetDetectedComponents().Should().ContainSingle(); } + [TestMethod] + [DataRow("dockerfile.mjs")] + [DataRow("dockerfile.d.mts")] + public async Task TestDockerfile_ShikiLanguageDefinitionIsIgnoredAsync(string fileName) + { + var loggerMock = new Mock>(); + + var (scanResult, componentRecorder) = await this.DetectorTestUtility + .WithFile(fileName, "export default Object.freeze({ name: 'dockerfile' });") + .AddServiceMock(loggerMock) + .ExecuteDetectorAsync(); + + scanResult.ResultCode.Should().Be(ProcessingResultCode.Success); + componentRecorder.GetDetectedComponents().Should().BeEmpty(); + loggerMock.Verify( + logger => logger.Log( + LogLevel.Warning, + It.IsAny(), + It.IsAny(), + It.IsAny(), + It.IsAny>()), + Times.Once); + loggerMock.Verify( + logger => logger.Log( + LogLevel.Error, + It.IsAny(), + It.IsAny(), + It.IsAny(), + It.IsAny>()), + Times.Never); + } + + [TestMethod] + public async Task TestDockerfile_UnderNodeModulesIsIgnoredAsync() + { + var fileLocation = Path.Combine(Path.GetTempPath(), "node_modules", "shiki", "dist", "langs", "dockerfile.mjs"); + + var (scanResult, componentRecorder) = await this.DetectorTestUtility + .WithFile("dockerfile.mjs", "FROM redis:7-alpine", fileLocation: fileLocation) + .ExecuteDetectorAsync(); + + scanResult.ResultCode.Should().Be(ProcessingResultCode.Success); + componentRecorder.GetDetectedComponents().Should().BeEmpty(); + } + [TestMethod] public async Task TestDockerfile_NoFromInstructionsAsync() { From 2f00ab9b7def197310deec187f87ad0c52825f57 Mon Sep 17 00:00:00 2001 From: Ryan Brandenburg Date: Thu, 1 Oct 2026 09:46:33 -0700 Subject: [PATCH 14/15] Swift relies on Version, not hash (#1885) * Swift relies on Version, not hash * PR feedback --- .../TypedComponent/SwiftComponent.cs | 25 +-- .../swiftpm/SwiftResolvedComponentDetector.cs | 24 ++- .../TypedComponentSerializationTests.cs | 14 +- .../SwiftComponentTests.cs | 173 ++++++------------ .../SwiftResolvedDetectorTests.cs | 76 ++++---- 5 files changed, 118 insertions(+), 194 deletions(-) diff --git a/src/Microsoft.ComponentDetection.Contracts/TypedComponent/SwiftComponent.cs b/src/Microsoft.ComponentDetection.Contracts/TypedComponent/SwiftComponent.cs index ca516d022..de64905e1 100644 --- a/src/Microsoft.ComponentDetection.Contracts/TypedComponent/SwiftComponent.cs +++ b/src/Microsoft.ComponentDetection.Contracts/TypedComponent/SwiftComponent.cs @@ -11,29 +11,6 @@ namespace Microsoft.ComponentDetection.Contracts.TypedComponent; /// public class SwiftComponent : TypedComponent { - /// - /// Initializes a new instance of the class. - /// - /// The name of the component. - /// The version of the component. - /// The source repository URL of the component. - /// The Git commit hash of the component. - /// The Swift package kind. - public SwiftComponent(string name, string version, string repositoryUrl, string hash, string kind) - { - this.Name = this.ValidateRequiredInput(name, nameof(name), nameof(ComponentType.Swift)); - this.Version = this.ValidateRequiredInput(version, nameof(version), nameof(ComponentType.Swift)); - this.ValidateRequiredInput(repositoryUrl, nameof(repositoryUrl), nameof(ComponentType.Swift)); - this.RepositoryUrl = new Uri(repositoryUrl); - this.Kind = this.ValidateRequiredInput(kind, nameof(kind), nameof(ComponentType.Swift)); - this.CommitHash = this.ValidateRequiredInput(hash, nameof(hash), nameof(ComponentType.Swift)); - } - - public SwiftComponent() - { - /* Reserved for deserialization */ - } - [JsonPropertyName("name")] public string Name { get; set; } @@ -68,7 +45,7 @@ public SwiftComponent() }, subpath: null); - protected override string ComputeBaseId() => $"{this.RepositoryUrl.AbsoluteUri} {this.CommitHash} - {this.Type}"; + protected override string ComputeBaseId() => $"{this.RepositoryUrl.AbsoluteUri} {this.Version} - {this.Type}"; private string GetNamespaceFromPackageUrl() { diff --git a/src/Microsoft.ComponentDetection.Detectors/swiftpm/SwiftResolvedComponentDetector.cs b/src/Microsoft.ComponentDetection.Detectors/swiftpm/SwiftResolvedComponentDetector.cs index 241b7cbb3..d6cff8635 100644 --- a/src/Microsoft.ComponentDetection.Detectors/swiftpm/SwiftResolvedComponentDetector.cs +++ b/src/Microsoft.ComponentDetection.Detectors/swiftpm/SwiftResolvedComponentDetector.cs @@ -39,7 +39,7 @@ public SwiftResolvedComponentDetector( public override IEnumerable SupportedComponentTypes => [ComponentType.Swift]; - public override int Version => 2; + public override int Version => 3; protected override Task OnFileFoundAsync( ProcessRequest processRequest, @@ -77,15 +77,19 @@ private void ProcessPackageResolvedFile(ISingleFileComponentRecorder singleFileC ArgumentException.ThrowIfNullOrWhiteSpace(package.Identity); // The version of the package is not always available. - var version = package.State.Version ?? package.State.Branch ?? package.State.Revision; - - var detectedComponent = new SwiftComponent( - name: package.Identity, - version: version, - repositoryUrl: package.Location, - hash: package.State.Revision, - kind: package.Kind); - singleFileComponentRecorder.RegisterUsage(new DetectedComponent(component: detectedComponent)); + // For now when there's no version we don't report. + if (!string.IsNullOrWhiteSpace(package.State.Version)) + { + var detectedComponent = new SwiftComponent + { + Name = package.Identity, + Version = package.State.Version, + RepositoryUrl = new Uri(package.Location), + CommitHash = package.State.Revision, + Kind = package.Kind, + }; + singleFileComponentRecorder.RegisterUsage(new DetectedComponent(component: detectedComponent)); + } } else { diff --git a/test/Microsoft.ComponentDetection.Contracts.Tests/TypedComponentSerializationTests.cs b/test/Microsoft.ComponentDetection.Contracts.Tests/TypedComponentSerializationTests.cs index a351bd674..600fa1c8e 100644 --- a/test/Microsoft.ComponentDetection.Contracts.Tests/TypedComponentSerializationTests.cs +++ b/test/Microsoft.ComponentDetection.Contracts.Tests/TypedComponentSerializationTests.cs @@ -175,12 +175,14 @@ public void TypedComponent_Serialization_Git() [TestMethod] public void TypedComponent_Serialization_Swift() { - TypedComponent component = new SwiftComponent( - "alamofire", - "5.9.1", - "https://github.com/Alamofire/Alamofire", - "f455c2975872ccd2d9c81594c658af65716e9b9a", - "remoteSourceControl"); + TypedComponent component = new SwiftComponent + { + Name = "alamofire", + Version = "5.9.1", + RepositoryUrl = new Uri("https://github.com/Alamofire/Alamofire"), + CommitHash = "f455c2975872ccd2d9c81594c658af65716e9b9a", + Kind = "remoteSourceControl", + }; var json = JsonSerializer.Serialize(component); var deserializedComponent = JsonSerializer.Deserialize(json); diff --git a/test/Microsoft.ComponentDetection.Detectors.Tests/SwiftComponentTests.cs b/test/Microsoft.ComponentDetection.Detectors.Tests/SwiftComponentTests.cs index d0d75f0bd..085501ed1 100644 --- a/test/Microsoft.ComponentDetection.Detectors.Tests/SwiftComponentTests.cs +++ b/test/Microsoft.ComponentDetection.Detectors.Tests/SwiftComponentTests.cs @@ -21,7 +21,14 @@ public void Constructor_ShouldInitializeProperties() var kind = "remoteSourceControl"; var commitHash = "f455c2975872ccd2d9c81594c658af65716e9b9a"; - var component = new SwiftComponent(name, version, repositoryUrl, commitHash, kind); + var component = new SwiftComponent + { + Name = name, + Version = version, + RepositoryUrl = new Uri(repositoryUrl), + CommitHash = commitHash, + Kind = kind, + }; component.Name.Should().Be(name); component.Version.Should().Be(version); @@ -30,45 +37,44 @@ public void Constructor_ShouldInitializeProperties() component.RepositoryUrl.Should().Be(new Uri(repositoryUrl)); component.Type.Should().Be(ComponentType.Swift); component.Id.Should().Be( - $"{repositoryUrl} {commitHash} - {component.Type}"); + $"{repositoryUrl} {version} - {component.Type}"); } [TestMethod] - public void Id_ShouldDistinguishRepositoriesAndCommitHashes() + public void Id_ShouldDistinguishRepositories() { - var component = new SwiftComponent( - "alamofire", - "5.9.1", - "https://github.com/Alamofire/Alamofire", - "f455c2975872ccd2d9c81594c658af65716e9b9a", - "remoteSourceControl"); - var differentRepository = new SwiftComponent( - "alamofire", - "5.9.1", - "https://github.com/example/Alamofire", - "f455c2975872ccd2d9c81594c658af65716e9b9a", - "remoteSourceControl"); - var differentCommit = new SwiftComponent( - "alamofire", - "5.9.1", - "https://github.com/Alamofire/Alamofire", - "63dfa86548c4e5d5c6fd6ed42f638e388cbce529", - "remoteSourceControl"); + var component = new SwiftComponent + { + Name = "alamofire", + Version = "5.9.1", + RepositoryUrl = new Uri("https://github.com/Alamofire/Alamofire"), + CommitHash = "f455c2975872ccd2d9c81594c658af65716e9b9a", + Kind = "remoteSourceControl", + }; + var differentRepository = new SwiftComponent + { + Name = "alamofire", + Version = "5.9.1", + RepositoryUrl = new Uri("https://github.com/example/Alamofire"), + CommitHash = "f455c2975872ccd2d9c81594c658af65716e9b9a", + Kind = "remoteSourceControl", + }; component.Id.Should().NotBe(differentRepository.Id); - component.Id.Should().NotBe(differentCommit.Id); } [TestMethod] public void Serialization_ShouldIncludeCommitHash() { var commitHash = "f455c2975872ccd2d9c81594c658af65716e9b9a"; - TypedComponent component = new SwiftComponent( - name: "alamofire", - version: "5.9.1", - repositoryUrl: "https://github.com/Alamofire/Alamofire", - hash: commitHash, - kind: "remoteSourceControl"); + TypedComponent component = new SwiftComponent + { + Name = "alamofire", + Version = "5.9.1", + RepositoryUrl = new Uri("https://github.com/Alamofire/Alamofire"), + CommitHash = commitHash, + Kind = "remoteSourceControl", + }; using var json = JsonDocument.Parse(JsonSerializer.Serialize(component)); @@ -76,90 +82,6 @@ public void Serialization_ShouldIncludeCommitHash() json.RootElement.GetProperty("commitHash").GetString().Should().Be(commitHash); } - [TestMethod] - public void Constructor_ShouldThrowException_WhenKindIsNull() - { - Action action = () => - new SwiftComponent( - "alamofire", - "5.9.1", - "https://github.com/Alamofire/Alamofire", - "f455c2975872ccd2d9c81594c658af65716e9b9a", - null - ); - action.Should().Throw().WithMessage("*kind*"); - } - - [TestMethod] - public void Constructor_ShouldThrowException_WhenNameIsNull() - { - Action action = () => - new SwiftComponent( - null, - "5.9.1", - "https://github.com/Alamofire/Alamofire", - "f455c2975872ccd2d9c81594c658af65716e9b9a", - "remoteSourceControl" - ); - action.Should().Throw().WithMessage("*name*"); - } - - [TestMethod] - public void Constructor_ShouldThrowException_WhenVersionIsNull() - { - Action action = () => - new SwiftComponent( - "alamofire", - null, - "https://github.com/Alamofire/Alamofire", - "f455c2975872ccd2d9c81594c658af65716e9b9a", - "remoteSourceControl" - ); - action.Should().Throw().WithMessage("*version*"); - } - - [TestMethod] - public void Constructor_ShouldThrowException_WhenRepositoryUrlIsNull() - { - Action action = () => - new SwiftComponent( - "alamofire", - "5.9.1", - null, - "f455c2975872ccd2d9c81594c658af65716e9b9a", - "remoteSourceControl" - ); - action.Should().Throw().WithMessage("*repositoryUrl*"); - } - - [TestMethod] - public void Constructor_ShouldThrowException_WhenHashIsNull() - { - Action action = () => - new SwiftComponent( - "alamofire", - "5.9.1", - "https://github.com/Alamofire/Alamofire", - null, - "remoteSourceControl" - ); - action.Should().Throw().WithMessage("*hash*"); - } - - [TestMethod] - public void Constructor_ShouldThrowException_WhenRepositoryUrlIsInvalid() - { - Action action = () => - new SwiftComponent( - "alamofire", - "5.9.1", - "invalid-url", - "f455c2975872ccd2d9c81594c658af65716e9b9a", - "remoteSourceControl" - ); - action.Should().Throw(); - } - [TestMethod] public void PackageURL_ShouldReturnCorrectPackageURL_GithubHostname() { @@ -168,7 +90,14 @@ public void PackageURL_ShouldReturnCorrectPackageURL_GithubHostname() var repositoryUrl = "https://github.com/Alamofire/Alamofire"; var hash = "f455c2975872ccd2d9c81594c658af65716e9b9a"; - var component = new SwiftComponent(name, version, repositoryUrl, hash, "remoteSourceControl"); + var component = new SwiftComponent + { + Name = name, + Version = version, + RepositoryUrl = new Uri(repositoryUrl), + CommitHash = hash, + Kind = "remoteSourceControl", + }; var expectedPackageURL = new PackageURL( type: "swift", @@ -190,7 +119,14 @@ public void PackageURL_ShouldReturnCorrectPackageURL_GithubHostname_Alternate() var repositoryUrl = "https://giTHub.com/Alamofire/Alamofire"; var hash = "f455c2975872ccd2d9c81594c658af65716e9b9a"; - var component = new SwiftComponent(name, version, repositoryUrl, hash, "remoteSourceControl"); + var component = new SwiftComponent + { + Name = name, + Version = version, + RepositoryUrl = new Uri(repositoryUrl), + CommitHash = hash, + Kind = "remoteSourceControl", + }; var expectedPackageURL = new PackageURL( type: "swift", @@ -215,7 +151,14 @@ public void PackageURL_ShouldReturnCorrectPackageURL_OtherHostname() var repositoryUrl = "https://otherhostname.com/Alamofire/Alamofire"; var hash = "f455c2975872ccd2d9c81594c658af65716e9b9a"; - var component = new SwiftComponent(name, version, repositoryUrl, hash, "remoteSourceControl"); + var component = new SwiftComponent + { + Name = name, + Version = version, + RepositoryUrl = new Uri(repositoryUrl), + CommitHash = hash, + Kind = "remoteSourceControl", + }; var expectedPackageURL = new PackageURL( type: "swift", diff --git a/test/Microsoft.ComponentDetection.Detectors.Tests/SwiftResolvedDetectorTests.cs b/test/Microsoft.ComponentDetection.Detectors.Tests/SwiftResolvedDetectorTests.cs index e8f13b0eb..25322b00a 100644 --- a/test/Microsoft.ComponentDetection.Detectors.Tests/SwiftResolvedDetectorTests.cs +++ b/test/Microsoft.ComponentDetection.Detectors.Tests/SwiftResolvedDetectorTests.cs @@ -1,6 +1,7 @@ #nullable disable namespace Microsoft.ComponentDetection.Detectors.Tests.Swift; +using System; using System.Linq; using System.Threading.Tasks; using AwesomeAssertions; @@ -48,12 +49,14 @@ public async Task Test_GivenDetectorWithValidFile_WhenScan_ThenScanIsSuccessfulA var typedComponents = detectedComponents.Select(c => c.Component).ToList(); typedComponents.Should().ContainEquivalentOf( - new SwiftComponent( - name: "alamofire", - version: "5.9.1", - repositoryUrl: "https://github.com/Alamofire/Alamofire", - hash: "f455c2975872ccd2d9c81594c658af65716e9b9a", - kind: "remoteSourceControl")); + new SwiftComponent + { + Name = "alamofire", + Version = "5.9.1", + RepositoryUrl = new Uri("https://github.com/Alamofire/Alamofire"), + CommitHash = "f455c2975872ccd2d9c81594c658af65716e9b9a", + Kind = "remoteSourceControl", + }); } // Test for several packages @@ -76,20 +79,24 @@ public async Task Test_GivenDetectorWithValidFileWithMultiplePackages_WhenScan_T var typedComponents = detectedComponents.Select(c => c.Component).ToList(); typedComponents.Should().ContainEquivalentOf( - new SwiftComponent( - name: "alamofire", - version: "5.6.0", - repositoryUrl: "https://github.com/Alamofire/Alamofire", - hash: "63dfa86548c4e5d5c6fd6ed42f638e388cbce529", - kind: "remoteSourceControl")); + new SwiftComponent + { + Name = "alamofire", + Version = "5.6.0", + RepositoryUrl = new Uri("https://github.com/Alamofire/Alamofire"), + CommitHash = "63dfa86548c4e5d5c6fd6ed42f638e388cbce529", + Kind = "remoteSourceControl", + }); typedComponents.Should().ContainEquivalentOf( - new SwiftComponent( - name: "asyncextensions", - version: "0.5.3", - repositoryUrl: "https://github.com/sideeffect-io/AsyncExtensions", - hash: "3442d3d046800f1974bda096faaf0ac510b21154", - kind: "remoteSourceControl")); + new SwiftComponent + { + Name = "asyncextensions", + Version = "0.5.3", + RepositoryUrl = new Uri("https://github.com/sideeffect-io/AsyncExtensions"), + CommitHash = "3442d3d046800f1974bda096faaf0ac510b21154", + Kind = "remoteSourceControl", + }); } // Duplicate packages @@ -137,12 +144,14 @@ public async Task Test_GivenDetectorWithValidFileWithDuplicatePackages_WhenScan_ var typedComponents = detectedComponents.Select(c => c.Component).ToList(); typedComponents.Should().ContainEquivalentOf( - new SwiftComponent( - name: "alamofire", - version: "5.9.1", - repositoryUrl: "https://github.com/Alamofire/Alamofire", - hash: "f455c2975872ccd2d9c81594c658af65716e9b9a", - kind: "remoteSourceControl")); + new SwiftComponent + { + Name = "alamofire", + Version = "5.9.1", + RepositoryUrl = new Uri("https://github.com/Alamofire/Alamofire"), + CommitHash = "f455c2975872ccd2d9c81594c658af65716e9b9a", + Kind = "remoteSourceControl", + }); } [TestMethod] @@ -340,7 +349,7 @@ public async Task Test_GivenResolvedPackageWithEmptyState_WhenScan_ThenScanIsSuc } [TestMethod] - public async Task Test_GivenResolvedPackageWithoutRevision_WhenScan_ThenScanIsSuccessfulAndNoComponentsRegistered() + public async Task Test_GivenResolvedPackageWithoutRevision_WhenScan_ThenScanIsSuccessfulAndComponentsRegistered() { var resolvedPackageWithoutRevision = """ { @@ -364,11 +373,11 @@ public async Task Test_GivenResolvedPackageWithoutRevision_WhenScan_ThenScanIsSu .ExecuteDetectorAsync(); scanResult.ResultCode.Should().Be(ProcessingResultCode.Success); - componentRecorder.GetDetectedComponents().Should().BeEmpty(); + componentRecorder.GetDetectedComponents().Should().HaveCount(1); } [TestMethod] - public async Task Test_GivenResolvedPackageWithoutVersion_WhenScan_ThenScanIsSuccessfulAndComponentRegisteredWithRevisionHashAsVersion() + public async Task Test_GivenResolvedPackageWithoutVersion_WhenScan_ThenScanIsSuccessfulAndNoComponentRegistered() { var resolvedPackageWithoutVersion = """ { @@ -392,18 +401,7 @@ public async Task Test_GivenResolvedPackageWithoutVersion_WhenScan_ThenScanIsSuc .ExecuteDetectorAsync(); scanResult.ResultCode.Should().Be(ProcessingResultCode.Success); - componentRecorder.GetDetectedComponents().Should().ContainSingle(); - - var detectedComponents = componentRecorder.GetDetectedComponents(); - var typedComponents = detectedComponents.Select(c => c.Component).ToList(); - - typedComponents.Should().ContainEquivalentOf( - new SwiftComponent( - name: "alamofire", - version: "f455c2975872ccd2d9c81594c658af65716e9b9a", - repositoryUrl: "https://github.com/Alamofire/Alamofire", - hash: "f455c2975872ccd2d9c81594c658af65716e9b9a", - kind: "remoteSourceControl")); + componentRecorder.GetDetectedComponents().Should().BeEmpty(); } [System.Diagnostics.CodeAnalysis.SuppressMessage("StyleCop.CSharp.OrderingRules", "SA1201:Elements should appear in the correct order", Justification = "Test data that is better placed at the end of the file.")] From 7102ad7a8c8f3525fc3c5941b34a54d28e199365 Mon Sep 17 00:00:00 2001 From: RKS Date: Tue, 6 Oct 2026 13:24:49 -0400 Subject: [PATCH 15/15] fix: validate pnpm lockfile document structure --- docs/detectors/pnpm.md | 6 + .../PnpmParsingUtilitiesBase.cs | 26 ++-- .../PnpmParsingUtilitiesFactory.cs | 32 +---- .../PnpmDetectorTests.cs | 129 ++++++++++++++++++ .../PnpmParsingUtilitiesTest.cs | 35 ++++- 5 files changed, 189 insertions(+), 39 deletions(-) diff --git a/docs/detectors/pnpm.md b/docs/detectors/pnpm.md index 6c8c4780f..d6ee5aa0a 100644 --- a/docs/detectors/pnpm.md +++ b/docs/detectors/pnpm.md @@ -9,6 +9,12 @@ Pnpm detection relies on the presence of lockfiles generated by the pnpm package The detector supports lockfile versions 5, 6, and 9, with version 9 being the maximum supported version. +Lockfiles can contain one or two YAML documents. When an environment document is +present, it comes before the project document. Both documents are scanned so that +config and package-manager dependencies are included alongside project dependencies. +Empty documents, more than two documents, and inconsistent declared lockfile versions +are rejected as invalid lockfiles. + ## Detection strategy Pnpm detection is performed by parsing lockfiles found in the scan directory. The `PnpmComponentDetectorFactory` acts as a version-aware factory that: diff --git a/src/Microsoft.ComponentDetection.Detectors/pnpm/ParsingUtilities/PnpmParsingUtilitiesBase.cs b/src/Microsoft.ComponentDetection.Detectors/pnpm/ParsingUtilities/PnpmParsingUtilitiesBase.cs index 812fc281c..9ba8f8d63 100644 --- a/src/Microsoft.ComponentDetection.Detectors/pnpm/ParsingUtilities/PnpmParsingUtilitiesBase.cs +++ b/src/Microsoft.ComponentDetection.Detectors/pnpm/ParsingUtilities/PnpmParsingUtilitiesBase.cs @@ -4,7 +4,6 @@ namespace Microsoft.ComponentDetection.Detectors.Pnpm; using System; using System.Collections.Generic; using System.IO; -using System.Linq; using Microsoft.ComponentDetection.Contracts; using YamlDotNet.Core; using YamlDotNet.Core.Events; @@ -13,34 +12,39 @@ namespace Microsoft.ComponentDetection.Detectors.Pnpm; internal abstract class PnpmParsingUtilitiesBase where T : PnpmYaml { - public virtual List DeserializePnpmYamlFileDocuments(string fileContent) + public virtual IReadOnlyList DeserializePnpmYamlFileDocuments(string fileContent) { var deserializer = new DeserializerBuilder() .IgnoreUnmatchedProperties() .Build(); - var reader = new StringReader(fileContent); + using var reader = new StringReader(fileContent); var parser = new Parser(reader); parser.Consume(); var documents = new List(); + var documentCount = 0; while (parser.TryConsume(out _)) { - var doc = deserializer.Deserialize(parser); - if (doc != null) + if (++documentCount > 2) { - documents.Add(doc); + throw new InvalidOperationException("A pnpm lockfile must contain one or two YAML documents."); } + var doc = deserializer.Deserialize(parser) + ?? throw new InvalidOperationException("A pnpm lockfile must not contain empty YAML documents."); + + documents.Add(doc); parser.TryConsume(out _); } - return documents; - } + parser.Consume(); + if (documentCount == 0) + { + throw new InvalidOperationException("A pnpm lockfile must contain one or two YAML documents."); + } - public T DeserializePnpmYamlFile(string fileContent) - { - return this.DeserializePnpmYamlFileDocuments(fileContent).FirstOrDefault(); + return documents.AsReadOnly(); } public virtual bool IsPnpmPackageDevDependency(Package pnpmPackage) diff --git a/src/Microsoft.ComponentDetection.Detectors/pnpm/ParsingUtilities/PnpmParsingUtilitiesFactory.cs b/src/Microsoft.ComponentDetection.Detectors/pnpm/ParsingUtilities/PnpmParsingUtilitiesFactory.cs index 00fa9c9c9..3fd879cc2 100644 --- a/src/Microsoft.ComponentDetection.Detectors/pnpm/ParsingUtilities/PnpmParsingUtilitiesFactory.cs +++ b/src/Microsoft.ComponentDetection.Detectors/pnpm/ParsingUtilities/PnpmParsingUtilitiesFactory.cs @@ -2,12 +2,7 @@ namespace Microsoft.ComponentDetection.Detectors.Pnpm; using System; -using System.Collections.Generic; -using System.IO; using System.Linq; -using YamlDotNet.Core; -using YamlDotNet.Core.Events; -using YamlDotNet.Serialization; internal static class PnpmParsingUtilitiesFactory { @@ -25,27 +20,12 @@ public static PnpmParsingUtilitiesBase Create() public static string DeserializePnpmYamlFileVersion(string fileContent) { - var deserializer = new DeserializerBuilder() - .IgnoreUnmatchedProperties() - .Build(); - - var reader = new StringReader(fileContent); - var parser = new Parser(reader); - parser.Consume(); - - var versions = new List(); - while (parser.TryConsume(out _)) - { - var doc = deserializer.Deserialize(parser); - if (doc != null && !string.IsNullOrWhiteSpace(doc.LockfileVersion)) - { - versions.Add(doc.LockfileVersion); - } - - parser.TryConsume(out _); - } - - var distinctVersions = versions.Distinct().ToList(); + var documents = Create().DeserializePnpmYamlFileDocuments(fileContent); + var distinctVersions = documents + .Select(doc => doc.LockfileVersion) + .Where(version => !string.IsNullOrWhiteSpace(version)) + .Distinct() + .ToList(); if (distinctVersions.Count > 1) { throw new InvalidOperationException($"Inconsistent lockfile versions found: {string.Join(", ", distinctVersions)}"); diff --git a/test/Microsoft.ComponentDetection.Detectors.Tests/PnpmDetectorTests.cs b/test/Microsoft.ComponentDetection.Detectors.Tests/PnpmDetectorTests.cs index 9e9739cc3..5dfee6a20 100644 --- a/test/Microsoft.ComponentDetection.Detectors.Tests/PnpmDetectorTests.cs +++ b/test/Microsoft.ComponentDetection.Detectors.Tests/PnpmDetectorTests.cs @@ -1398,4 +1398,133 @@ public async Task TestPnpmDetector_V5_MultiDocumentLockfileAsync() componentRecorder.GetEffectiveDevDependencyValue(rootB.Component.Id).GetValueOrDefault(false).Should().BeTrue(); componentRecorder.GetEffectiveDevDependencyValue(depB.Component.Id).GetValueOrDefault(false).Should().BeTrue(); } + + [TestMethod] + [DataRow("5.0", "/present/1.0.0", "packages")] + [DataRow("6.0", "/present@1.0.0", "packages")] + [DataRow("9.0", "present@1.0.0", "snapshots")] + public async Task TestPnpmDetector_ThreeDocuments_DetectsNoComponentsAsync(string version, string packagePath, string packageSection) + { + var yamlFile = $@" +lockfileVersion: '{version}' +{packageSection}: + {packagePath}: {{}} +--- +lockfileVersion: '{version}' +--- +lockfileVersion: '{version}' +"; + + var (_, componentRecorder) = await this.detectorTestUtility + .WithFile("pnpm-lock.yaml", yamlFile) + .ExecuteDetectorAsync(); + + componentRecorder.GetDetectedComponents().Should().BeEmpty(); + } + + [TestMethod] + [DataRow(false)] + [DataRow(true)] + public async Task TestPnpmDetector_V5_MultiDocumentLockfile_DuplicatePackage_ProductionWinsAsync(bool productionFirst) + { + var developmentDocument = @" +lockfileVersion: '5.0' +dependencies: + shared-pkg: 1.0.0 +packages: + /shared-pkg/1.0.0: + dev: true + dependencies: + dev-document-dep: 1.0.0 + /dev-document-dep/1.0.0: + dev: true +"; + var productionDocument = @" +lockfileVersion: '5.0' +dependencies: + shared-pkg: 1.0.0 +packages: + /shared-pkg/1.0.0: + dev: false + dependencies: + production-document-dep: 2.0.0 + /production-document-dep/2.0.0: + dev: false +"; + var yamlFile = productionFirst + ? productionDocument + "\n---\n" + developmentDocument + : developmentDocument + "\n---\n" + productionDocument; + + var (scanResult, componentRecorder) = await this.detectorTestUtility + .WithFile("pnpm-lock.yaml", yamlFile) + .ExecuteDetectorAsync(); + + scanResult.ResultCode.Should().Be(ProcessingResultCode.Success); + var components = componentRecorder.GetDetectedComponents().ToList(); + components.Should().HaveCount(3); + var shared = components.Single(c => ((NpmComponent)c.Component).Name == "shared-pkg"); + var developmentDependency = components.Single(c => ((NpmComponent)c.Component).Name == "dev-document-dep"); + var productionDependency = components.Single(c => ((NpmComponent)c.Component).Name == "production-document-dep"); + + componentRecorder.GetEffectiveDevDependencyValue(shared.Component.Id).Should().BeFalse(); + var graph = componentRecorder.GetDependencyGraphsByLocation().Values.First(); + graph.IsComponentExplicitlyReferenced(shared.Component.Id).Should().BeTrue(); + graph.GetDependenciesForComponent(shared.Component.Id).Should().BeEquivalentTo([developmentDependency.Component.Id, productionDependency.Component.Id]); + } + + [TestMethod] + [DataRow(false)] + [DataRow(true)] + public async Task TestPnpmDetector_V9_MultiDocumentLockfile_DuplicatePackage_ProductionWinsAsync(bool productionFirst) + { + var developmentDocument = @" +lockfileVersion: '9.0' +importers: + .: + devDependencies: + shared-pkg: + specifier: 1.0.0 + version: 1.0.0 +snapshots: + shared-pkg@1.0.0: + dependencies: + dev-document-dep: 1.0.0 + dev-document-dep@1.0.0: {} +"; + var productionDocument = @" +lockfileVersion: '9.0' +importers: + .: + dependencies: + shared-pkg: + specifier: 1.0.0 + version: 1.0.0 +snapshots: + shared-pkg@1.0.0: + dependencies: + production-document-dep: 2.0.0 + production-document-dep@2.0.0: {} +"; + var yamlFile = productionFirst + ? productionDocument + "\n---\n" + developmentDocument + : developmentDocument + "\n---\n" + productionDocument; + + var (scanResult, componentRecorder) = await this.detectorTestUtility + .WithFile("pnpm-lock.yaml", yamlFile) + .ExecuteDetectorAsync(); + + scanResult.ResultCode.Should().Be(ProcessingResultCode.Success); + var components = componentRecorder.GetDetectedComponents().ToList(); + components.Should().HaveCount(3); + var shared = components.Single(c => ((NpmComponent)c.Component).Name == "shared-pkg"); + var developmentDependency = components.Single(c => ((NpmComponent)c.Component).Name == "dev-document-dep"); + var productionDependency = components.Single(c => ((NpmComponent)c.Component).Name == "production-document-dep"); + + componentRecorder.GetEffectiveDevDependencyValue(shared.Component.Id).Should().BeFalse(); + var graph = componentRecorder.GetDependencyGraphsByLocation().Values.First(); + graph.IsComponentExplicitlyReferenced(shared.Component.Id).Should().BeTrue(); + graph.GetDependenciesForComponent(shared.Component.Id).Should().BeEquivalentTo([developmentDependency.Component.Id, productionDependency.Component.Id]); + componentRecorder.GetEffectiveDevDependencyValue(developmentDependency.Component.Id).Should().BeFalse(); + componentRecorder.GetEffectiveDevDependencyValue(productionDependency.Component.Id).Should().BeFalse(); + } } diff --git a/test/Microsoft.ComponentDetection.Detectors.Tests/PnpmParsingUtilitiesTest.cs b/test/Microsoft.ComponentDetection.Detectors.Tests/PnpmParsingUtilitiesTest.cs index ec73bd583..b5f111ac1 100644 --- a/test/Microsoft.ComponentDetection.Detectors.Tests/PnpmParsingUtilitiesTest.cs +++ b/test/Microsoft.ComponentDetection.Detectors.Tests/PnpmParsingUtilitiesTest.cs @@ -2,6 +2,8 @@ namespace Microsoft.ComponentDetection.Detectors.Tests; using System; +using System.Collections.Generic; +using System.Linq; using AwesomeAssertions; using Microsoft.ComponentDetection.Contracts.TypedComponent; using Microsoft.ComponentDetection.Detectors.Pnpm; @@ -39,7 +41,7 @@ public void DeserializePnpmYamlFileV3() var pnpmParsingUtilities = PnpmParsingUtilitiesFactory.Create(); var version = PnpmParsingUtilitiesFactory.DeserializePnpmYamlFileVersion(yamlFile); version.Should().BeNull(); // Versions older than 5 report null as they don't use the same version field. - var parsedYaml = pnpmParsingUtilities.DeserializePnpmYamlFile(yamlFile); + var parsedYaml = pnpmParsingUtilities.DeserializePnpmYamlFileDocuments(yamlFile).Single(); parsedYaml.Packages.Should().HaveCount(2); parsedYaml.Packages.Should().ContainKey("/query-string/4.3.4"); @@ -130,7 +132,7 @@ public void DeserializePnpmYamlFileV6() var pnpmParsingUtilities = PnpmParsingUtilitiesFactory.Create(); var version = PnpmParsingUtilitiesFactory.DeserializePnpmYamlFileVersion(yamlFile); version.Should().Be("6.0"); - var parsedYaml = pnpmParsingUtilities.DeserializePnpmYamlFile(yamlFile); + var parsedYaml = pnpmParsingUtilities.DeserializePnpmYamlFileDocuments(yamlFile).Single(); parsedYaml.Packages.Should().ContainSingle(); parsedYaml.Packages.Should().ContainKey("/minimist@1.2.8"); @@ -242,4 +244,33 @@ public void DeserializePnpmYamlFileDocuments_ReturnsAllDocuments() documents[0].Importers["."].PackageManagerDependencies.Should().ContainKey("pnpm"); documents[1].Importers["."].Dependencies.Should().ContainKey("fast-uri"); } + + [TestMethod] + [DataRow("")] + [DataRow(" \n")] + [DataRow("---\n")] + [DataRow("---\n---\nlockfileVersion: '9.0'\n")] + [DataRow("lockfileVersion: '9.0'\n---\n")] + [DataRow("lockfileVersion: '9.0'\n---\nlockfileVersion: '9.0'\n---\nlockfileVersion: '9.0'\n")] + [DataRow("lockfileVersion: '9.0'\n---\n---\nlockfileVersion: '9.0'\n")] + public void DeserializePnpmYamlFileDocuments_InvalidDocumentCountOrEmptyDocument_Throws(string yamlFile) + { + var pnpmParsingUtilities = PnpmParsingUtilitiesFactory.Create(); + Action parseDocuments = () => pnpmParsingUtilities.DeserializePnpmYamlFileDocuments(yamlFile); + Action parseVersion = () => PnpmParsingUtilitiesFactory.DeserializePnpmYamlFileVersion(yamlFile); + + parseDocuments.Should().Throw(); + parseVersion.Should().Throw(); + } + + [TestMethod] + public void DeserializePnpmYamlFileDocuments_ReturnedCollectionCannotBeModified() + { + var pnpmParsingUtilities = PnpmParsingUtilitiesFactory.Create(); + var documents = pnpmParsingUtilities.DeserializePnpmYamlFileDocuments("lockfileVersion: '9.0'\n---\nlockfileVersion: '9.0'\n"); + Action removeDocument = () => ((IList)documents).RemoveAt(0); + + removeDocument.Should().Throw(); + documents.Should().HaveCount(2); + } }