From 135fdd29ebf98a560cab75c488d5121d25ccc1b2 Mon Sep 17 00:00:00 2001 From: "Brett V. Forsgren" Date: Thu, 19 Sep 2024 11:58:22 -0600 Subject: [PATCH 01/10] add MSBuild binary log (.binlog) component detector --- Directory.Packages.props | 3 + ...rosoft.ComponentDetection.Detectors.csproj | 3 + .../NuGetMSBuildBinaryLogComponentDetector.cs | 212 +++++++++++ .../Extensions/ServiceCollectionExtensions.cs | 1 + .../MSBuildTestUtilities.cs | 335 ++++++++++++++++++ ...tMSBuildBinaryLogComponentDetectorTests.cs | 123 +++++++ .../NugetTestUtilities.cs | 22 +- 7 files changed, 694 insertions(+), 5 deletions(-) create mode 100644 src/Microsoft.ComponentDetection.Detectors/nuget/NuGetMSBuildBinaryLogComponentDetector.cs create mode 100644 test/Microsoft.ComponentDetection.Detectors.Tests/MSBuildTestUtilities.cs create mode 100644 test/Microsoft.ComponentDetection.Detectors.Tests/NuGetMSBuildBinaryLogComponentDetectorTests.cs diff --git a/Directory.Packages.props b/Directory.Packages.props index 8beeef793..adc0944eb 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -21,9 +21,12 @@ + + + diff --git a/src/Microsoft.ComponentDetection.Detectors/Microsoft.ComponentDetection.Detectors.csproj b/src/Microsoft.ComponentDetection.Detectors/Microsoft.ComponentDetection.Detectors.csproj index f50084046..8f8736076 100644 --- a/src/Microsoft.ComponentDetection.Detectors/Microsoft.ComponentDetection.Detectors.csproj +++ b/src/Microsoft.ComponentDetection.Detectors/Microsoft.ComponentDetection.Detectors.csproj @@ -9,7 +9,10 @@ + + + diff --git a/src/Microsoft.ComponentDetection.Detectors/nuget/NuGetMSBuildBinaryLogComponentDetector.cs b/src/Microsoft.ComponentDetection.Detectors/nuget/NuGetMSBuildBinaryLogComponentDetector.cs new file mode 100644 index 000000000..d62d6979d --- /dev/null +++ b/src/Microsoft.ComponentDetection.Detectors/nuget/NuGetMSBuildBinaryLogComponentDetector.cs @@ -0,0 +1,212 @@ +namespace Microsoft.ComponentDetection.Detectors.NuGet; + +using System; +using System.Collections.Generic; +using System.Linq; +using System.Threading; +using Microsoft.Build.Locator; +using Microsoft.Build.Logging.StructuredLogger; +using Microsoft.ComponentDetection.Contracts; +using Microsoft.ComponentDetection.Contracts.Internal; +using Microsoft.ComponentDetection.Contracts.TypedComponent; +using Microsoft.Extensions.Logging; + +using Task = System.Threading.Tasks.Task; + +public class NuGetMSBuildBinaryLogComponentDetector : FileComponentDetector +{ + private static readonly HashSet TopLevelPackageItemNames = new HashSet(StringComparer.OrdinalIgnoreCase) + { + "PackageReference", + }; + + // the items listed below represent collection names that NuGet will resolve a package into, along with the metadata value names to get the package name and version + private static readonly Dictionary ResolvedPackageItemNames = new Dictionary(StringComparer.OrdinalIgnoreCase) + { + ["NativeCopyLocalItems"] = ("NuGetPackageId", "NuGetPackageVersion"), + ["ResourceCopyLocalItems"] = ("NuGetPackageId", "NuGetPackageVersion"), + ["RuntimeCopyLocalItems"] = ("NuGetPackageId", "NuGetPackageVersion"), + ["ResolvedAnalyzers"] = ("NuGetPackageId", "NuGetPackageVersion"), + ["_PackageDependenciesDesignTime"] = ("Name", "Version"), + }; + + private static bool isMSBuildRegistered; + + public NuGetMSBuildBinaryLogComponentDetector( + IObservableDirectoryWalkerFactory walkerFactory, + ILogger logger) + { + this.Scanner = walkerFactory; + this.Logger = logger; + } + + public override string Id { get; } = "NuGetMSBuildBinaryLog"; + + public override IEnumerable Categories => new[] { Enum.GetName(typeof(DetectorClass), DetectorClass.NuGet) }; + + public override IList SearchPatterns { get; } = new List { "*.binlog" }; + + public override IEnumerable SupportedComponentTypes { get; } = new[] { ComponentType.NuGet }; + + public override int Version { get; } = 1; + + private static void ProcessResolvedPackageReference(Dictionary> topLevelDependencies, Dictionary> projectResolvedDependencies, NamedNode node) + { + var doRemoveOperation = node is RemoveItem; + var doAddOperation = node is AddItem; + if (TopLevelPackageItemNames.Contains(node.Name)) + { + var projectEvaluation = node.GetNearestParent(); + if (projectEvaluation is not null) + { + foreach (var child in node.Children.OfType()) + { + var packageName = child.Name; + if (!topLevelDependencies.TryGetValue(projectEvaluation.ProjectFile, out var topLevel)) + { + topLevel = new(StringComparer.OrdinalIgnoreCase); + topLevelDependencies[projectEvaluation.ProjectFile] = topLevel; + } + + if (doRemoveOperation) + { + topLevel.Remove(packageName); + } + + if (doAddOperation) + { + topLevel.Add(packageName); + } + } + } + } + else if (ResolvedPackageItemNames.TryGetValue(node.Name, out var metadataNames)) + { + var nameMetadata = metadataNames.NameMetadata; + var versionMetadata = metadataNames.VersionMetadata; + var originalProject = node.GetNearestParent(); + if (originalProject is not null) + { + foreach (var child in node.Children.OfType()) + { + var packageName = GetChildMetadataValue(child, nameMetadata); + var packageVersion = GetChildMetadataValue(child, versionMetadata); + if (packageName is not null && packageVersion is not null) + { + var project = originalProject; + while (project is not null) + { + if (!projectResolvedDependencies.TryGetValue(project.ProjectFile, out var projectDependencies)) + { + projectDependencies = new(StringComparer.OrdinalIgnoreCase); + projectResolvedDependencies[project.ProjectFile] = projectDependencies; + } + + if (doRemoveOperation) + { + projectDependencies.Remove(packageName); + } + + if (doAddOperation) + { + projectDependencies[packageName] = packageVersion; + } + + project = project.GetNearestParent(); + } + } + } + } + } + } + + private static string GetChildMetadataValue(TreeNode node, string metadataItemName) + { + var metadata = node.Children.OfType(); + var metadataValue = metadata.FirstOrDefault(m => m.Name.Equals(metadataItemName, StringComparison.OrdinalIgnoreCase))?.Value; + return metadataValue; + } + + protected override Task OnFileFoundAsync(ProcessRequest processRequest, IDictionary detectorArgs, CancellationToken cancellationToken = default) + { + try + { + if (!isMSBuildRegistered) + { + // this must happen once per process, and never again + var defaultInstance = MSBuildLocator.QueryVisualStudioInstances().First(); + MSBuildLocator.RegisterInstance(defaultInstance); + isMSBuildRegistered = true; + } + + var singleFileComponentRecorder = this.ComponentRecorder.CreateSingleFileComponentRecorder(processRequest.ComponentStream.Location); + var buildRoot = BinaryLog.ReadBuild(processRequest.ComponentStream.Stream); + this.RecordLockfileVersion(buildRoot.FileFormatVersion); + this.ProcessBinLog(buildRoot, singleFileComponentRecorder); + } + catch (Exception e) + { + // If something went wrong, just ignore the package + this.Logger.LogError(e, "Failed to process MSBuild binary log {BinLogFile}", processRequest.ComponentStream.Location); + } + + return Task.CompletedTask; + } + + protected override Task OnDetectionFinishedAsync() + { + return Task.CompletedTask; + } + + private void ProcessBinLog(Build buildRoot, ISingleFileComponentRecorder componentRecorder) + { + // maps a project path to a set of resolved dependencies + var projectTopLevelDependencies = new Dictionary>(StringComparer.OrdinalIgnoreCase); + var projectResolvedDependencies = new Dictionary>(StringComparer.OrdinalIgnoreCase); + buildRoot.VisitAllChildren(node => + { + switch (node) + { + case NamedNode namedNode when namedNode is AddItem or RemoveItem: + ProcessResolvedPackageReference(projectTopLevelDependencies, projectResolvedDependencies, namedNode); + break; + default: + break; + } + }); + + // dependencies were resolved per project, we need to re-arrange them to be per package/version + var projectsPerPackage = new Dictionary>(StringComparer.OrdinalIgnoreCase); + foreach (var projectPath in projectResolvedDependencies.Keys) + { + var projectDependencies = projectResolvedDependencies[projectPath]; + foreach (var (packageName, packageVersion) in projectDependencies) + { + var key = $"{packageName}/{packageVersion}"; + if (!projectsPerPackage.TryGetValue(key, out var projectPaths)) + { + projectPaths = new HashSet(StringComparer.OrdinalIgnoreCase); + projectsPerPackage[key] = projectPaths; + } + + projectPaths.Add(projectPath); + } + } + + // report it all + foreach (var (packageNameAndVersion, projectPaths) in projectsPerPackage) + { + var parts = packageNameAndVersion.Split('/', 2); + var packageName = parts[0]; + var packageVersion = parts[1]; + var component = new NuGetComponent(packageName, packageVersion); + var libraryComponent = new DetectedComponent(component); + foreach (var projectPath in projectPaths) + { + libraryComponent.FilePaths.Add(projectPath); + } + + componentRecorder.RegisterUsage(libraryComponent); + } + } +} diff --git a/src/Microsoft.ComponentDetection.Orchestrator/Extensions/ServiceCollectionExtensions.cs b/src/Microsoft.ComponentDetection.Orchestrator/Extensions/ServiceCollectionExtensions.cs index 41d16ccc5..4da100fe3 100644 --- a/src/Microsoft.ComponentDetection.Orchestrator/Extensions/ServiceCollectionExtensions.cs +++ b/src/Microsoft.ComponentDetection.Orchestrator/Extensions/ServiceCollectionExtensions.cs @@ -105,6 +105,7 @@ public static IServiceCollection AddComponentDetection(this IServiceCollection s services.AddSingleton(); services.AddSingleton(); services.AddSingleton(); + services.AddSingleton(); // PIP services.AddSingleton(); diff --git a/test/Microsoft.ComponentDetection.Detectors.Tests/MSBuildTestUtilities.cs b/test/Microsoft.ComponentDetection.Detectors.Tests/MSBuildTestUtilities.cs new file mode 100644 index 000000000..7e93c5bc2 --- /dev/null +++ b/test/Microsoft.ComponentDetection.Detectors.Tests/MSBuildTestUtilities.cs @@ -0,0 +1,335 @@ +namespace Microsoft.ComponentDetection.Detectors.Tests; + +using System; +using System.Collections.Generic; +using System.Diagnostics; +using System.IO; +using System.Linq; +using System.Security.Cryptography; +using System.Text; +using System.Text.RegularExpressions; +using System.Threading; +using System.Threading.Tasks; +using System.Xml.Linq; +using System.Xml.XPath; +using FluentAssertions; + +public static class MSBuildTestUtilities +{ + public const int TestTargetFrameworkVersion = 6; + public static readonly string TestTargetFramework = $"net{TestTargetFrameworkVersion}.0"; + + // we need to find the file `Microsoft.NETCoreSdk.BundledVersions.props` in the SDK directory + private static readonly Lazy BundledVersionsPropsPath = new(static () => + { + // get the sdk version + using var tempDir = new TemporaryProjectDirectory(); + var projectContents = @" + + + + + + "; + var projectPath = Path.Combine(tempDir.DirectoryPath, "project.csproj"); + File.WriteAllText(projectPath, projectContents); + var (exitCode, stdout, stderr) = RunProcessAsync("dotnet", $"msbuild {projectPath} /t:_ReportCurrentSdkVersion").Result; + if (exitCode != 0) + { + throw new NotSupportedException($"Failed to report the current SDK version:\n{stdout}\n{stderr}"); + } + + var matches = Regex.Matches(stdout, "_CurrentSdkVersion=(?.*)$", RegexOptions.Multiline); + if (matches.Count == 0) + { + throw new NotSupportedException($"Failed to find the current SDK version in the output:\n{stdout}"); + } + + var sdkVersionString = matches.First().Groups["SdkVersion"].Value.Trim(); + + // find the actual SDK directory + var privateCoreLibPath = typeof(object).Assembly.Location; // e.g., C:\Program Files\dotnet\shared\Microsoft.NETCore.App\8.0.4\System.Private.CoreLib.dll + var sdkDirectory = Path.Combine(Path.GetDirectoryName(privateCoreLibPath), "..", "..", "..", "sdk", sdkVersionString); // e.g., C:\Program Files\dotnet\sdk\8.0.204 + var bundledVersionsPropsPath = Path.Combine(sdkDirectory, "Microsoft.NETCoreSdk.BundledVersions.props"); + var normalizedPath = new FileInfo(bundledVersionsPropsPath); + return normalizedPath.FullName; + }); + + public static async Task GetBinLogStreamFromFileContentsAsync( + string projectContents, + (string FileName, string Contents)[] additionalFiles = null, + (string Name, string Version, string TargetFramework, string AdditionalMetadataXml)[] mockedPackages = null) + { + // write all files + using var tempDir = new TemporaryProjectDirectory(); + var fullProjectPath = Path.Combine(tempDir.DirectoryPath, "project.csproj"); + await File.WriteAllTextAsync(fullProjectPath, projectContents); + if (additionalFiles is not null) + { + foreach (var (fileName, contents) in additionalFiles) + { + var fullFilePath = Path.Combine(tempDir.DirectoryPath, fileName); + var fullFileDirectory = Path.GetDirectoryName(fullFilePath); + Directory.CreateDirectory(fullFileDirectory); + await File.WriteAllTextAsync(fullFilePath, contents); + } + } + + await MockNuGetPackagesInDirectoryAsync(tempDir, mockedPackages); + + // generate the binlog + var (exitCode, stdOut, stdErr) = await RunProcessAsync("dotnet", $"build \"{fullProjectPath}\" /t:GenerateBuildDependencyFile /bl:msbuild.binlog", workingDirectory: tempDir.DirectoryPath); + exitCode.Should().Be(0, $"STDOUT:\n{stdOut}\n\nSTDERR:\n{stdErr}"); + + // copy it to memory so the temporary directory can be cleaned up + var fullBinLogPath = Path.Combine(tempDir.DirectoryPath, "msbuild.binlog"); + using var binLogStream = File.OpenRead(fullBinLogPath); + var inMemoryStream = new MemoryStream(); + await binLogStream.CopyToAsync(inMemoryStream); + inMemoryStream.Position = 0; + return inMemoryStream; + } + + private static async Task MockNuGetPackagesInDirectoryAsync( + TemporaryProjectDirectory tempDir, + (string Name, string Version, string TargetFramework, string AdditionalMetadataXml)[] mockedPackages) + { + if (mockedPackages is not null) + { + var nugetConfig = @" + + + + + + + "; + await File.WriteAllTextAsync(Path.Combine(tempDir.DirectoryPath, "NuGet.Config"), nugetConfig); + var packagesPath = Path.Combine(tempDir.DirectoryPath, "local-packages"); + Directory.CreateDirectory(packagesPath); + + var mockedPackagesWithFiles = mockedPackages.Select(p => + { + return ( + p.Name, + p.Version, + p.TargetFramework, + p.AdditionalMetadataXml, + Files: new[] { ($"lib/{p.TargetFramework}/{p.Name}.dll", Array.Empty()) }); + }); + + var allPackages = mockedPackagesWithFiles.Concat(GetCommonPackages()); + + using var sha512 = SHA512.Create(); // this is used to compute the hash of each package below + foreach (var package in allPackages) + { + var nuspec = NugetTestUtilities.GetValidNuspec(package.Name, package.Version, Array.Empty()); + if (package.AdditionalMetadataXml is not null) + { + // augment the nuspec + var doc = XDocument.Parse(nuspec); + var additionalMetadata = XElement.Parse(package.AdditionalMetadataXml); + additionalMetadata = WithNamespace(additionalMetadata, doc.Root.Name.Namespace); + + var metadataElement = doc.Root.Descendants().First(e => e.Name.LocalName == "metadata"); + metadataElement.Add(additionalMetadata); + nuspec = doc.ToString(); + } + + var nupkg = await NugetTestUtilities.ZipNupkgComponentAsync(package.Name, nuspec, additionalFiles: package.Files); + + // to create a local nuget package source, we need a directory structure like this: + // local-packages/// + var packagePath = Path.Combine(packagesPath, package.Name.ToLower(), package.Version.ToLower()); + Directory.CreateDirectory(packagePath); + + // and we need the following files: + // 1. the package + var nupkgPath = Path.Combine(packagePath, $"{package.Name}.{package.Version}.nupkg".ToLower()); + using (var nupkgFileStream = File.OpenWrite(nupkgPath)) + { + await nupkg.CopyToAsync(nupkgFileStream); + } + + // 2. the nuspec + var nuspecPath = Path.Combine(packagePath, $"{package.Name}.nuspec".ToLower()); + await File.WriteAllTextAsync(nuspecPath, nuspec); + + // 3. SHA512 hash of the package + var hash = sha512.ComputeHash(File.ReadAllBytes(nupkgPath)); + var hashString = Convert.ToBase64String(hash); + var hashPath = $"{nupkgPath}.sha512"; + await File.WriteAllTextAsync(hashPath, hashString); + + // 4. a JSON metadata file + var metadata = $@"{{""version"": 2, ""contentHash"": ""{hashString}"", ""source"": null}}"; + var metadataPath = Path.Combine(packagePath, ".nupkg.metadata"); + await File.WriteAllTextAsync(metadataPath, metadata); + } + } + } + + private static XElement WithNamespace(XElement element, XNamespace ns) + { + return new XElement( + ns + element.Name.LocalName, + element.Attributes(), + element.Elements().Select(e => WithNamespace(e, ns)), + element.Value); + } + + private static IEnumerable<(string Name, string Version, string TargetFramework, string AdditionalMetadataXml, (string Path, byte[] Content)[] Files)> GetCommonPackages() + { + // to allow the tests to not require the network, we need to mock some common packages + yield return MakeWellKnownReferencePackage("Microsoft.AspNetCore.App", null); + yield return MakeWellKnownReferencePackage("Microsoft.WindowsDesktop.App", null); + + var frameworksXml = $@" + + + "; + yield return MakeWellKnownReferencePackage("Microsoft.NETCore.App", new[] { ("data/FrameworkList.xml", Encoding.UTF8.GetBytes(frameworksXml)) }); + } + + private static (string Name, string Version, string TargetFramework, string AdditionalMetadataXml, (string Path, byte[] Content)[] Files) MakeWellKnownReferencePackage(string packageName, (string Path, byte[] Content)[] files) + { + var propsDocument = XDocument.Load(BundledVersionsPropsPath.Value); + var xpathQuery = $@" + /Project/ItemGroup/KnownFrameworkReference + [ + @Include='{packageName}' and + @TargetingPackName='{packageName}.Ref' and + @TargetFramework='{TestTargetFramework}' + ] + "; + var matchingFrameworkElement = propsDocument.XPathSelectElement(xpathQuery); + if (matchingFrameworkElement is null) + { + throw new NotSupportedException($"Unable to find {packageName}.Ref"); + } + + var expectedVersion = matchingFrameworkElement.Attribute("TargetingPackVersion").Value; + return ( + $"{packageName}.Ref", + expectedVersion, + TestTargetFramework, + "", + files); + } + + public static Task<(int ExitCode, string Output, string Error)> RunProcessAsync(string fileName, string arguments = "", string workingDirectory = null) + { + var tcs = new TaskCompletionSource<(int, string, string)>(); + + var redirectInitiated = new ManualResetEventSlim(); + var process = new Process + { + StartInfo = + { + FileName = fileName, + Arguments = arguments, + UseShellExecute = false, // required to redirect output + RedirectStandardOutput = true, + RedirectStandardError = true, + }, + EnableRaisingEvents = true, + }; + + if (workingDirectory is not null) + { + process.StartInfo.WorkingDirectory = workingDirectory; + } + + var stdout = new StringBuilder(); + var stderr = new StringBuilder(); + + process.OutputDataReceived += (_, e) => stdout.AppendLine(e.Data); + process.ErrorDataReceived += (_, e) => stderr.AppendLine(e.Data); + + process.Exited += (sender, args) => + { + // It is necessary to wait until we have invoked 'BeginXReadLine' for our redirected IO. Then, + // we must call WaitForExit to make sure we've received all OutputDataReceived/ErrorDataReceived calls + // or else we'll be returning a list we're still modifying. For paranoia, we'll start a task here rather + // than enter right back into the Process type and start a wait which isn't guaranteed to be safe. + var unused = Task.Run(() => + { + redirectInitiated.Wait(); + redirectInitiated.Dispose(); + redirectInitiated = null; + + process.WaitForExit(); + + tcs.TrySetResult((process.ExitCode, stdout.ToString(), stderr.ToString())); + process.Dispose(); + }); + }; + + if (!process.Start()) + { + throw new InvalidOperationException("Process failed to start"); + } + + process.BeginOutputReadLine(); + process.BeginErrorReadLine(); + + redirectInitiated.Set(); + + return tcs.Task; + } + + private class TemporaryProjectDirectory : IDisposable + { + private const string DirectoryBuildPropsContents = @" + + + false + + + "; + + private readonly Dictionary originalEnvironment = new(); + + public TemporaryProjectDirectory() + { + var testDataPath = Path.Combine(Path.GetDirectoryName(this.GetType().Assembly.Location), "test-data"); + Directory.CreateDirectory(testDataPath); + + // ensure tests don't crawl the directory tree + File.WriteAllText(Path.Combine(testDataPath, "Directory.Build.props"), DirectoryBuildPropsContents); + File.WriteAllText(Path.Combine(testDataPath, "Directory.Build.targets"), ""); + File.WriteAllText(Path.Combine(testDataPath, "Directory.Packages.props"), ""); + + // create temporary project directory + this.DirectoryPath = Path.Combine(testDataPath, Guid.NewGuid().ToString("d")); + Directory.CreateDirectory(this.DirectoryPath); + + // ensure each project gets a fresh package cache + foreach (var envName in new[] { "NUGET_PACKAGES", "NUGET_HTTP_CACHE_PATH", "NUGET_SCRATCH", "NUGET_PLUGINS_CACHE_PATH" }) + { + this.originalEnvironment[envName] = Environment.GetEnvironmentVariable(envName); + var dir = Path.Join(this.DirectoryPath, envName); + Directory.CreateDirectory(dir); + Environment.SetEnvironmentVariable(envName, dir); + } + } + + public string DirectoryPath { get; } + + public void Dispose() + { + foreach (var (key, value) in this.originalEnvironment) + { + Environment.SetEnvironmentVariable(key, value); + } + + try + { + Directory.Delete(this.DirectoryPath, recursive: true); + } + catch + { + } + } + } +} diff --git a/test/Microsoft.ComponentDetection.Detectors.Tests/NuGetMSBuildBinaryLogComponentDetectorTests.cs b/test/Microsoft.ComponentDetection.Detectors.Tests/NuGetMSBuildBinaryLogComponentDetectorTests.cs new file mode 100644 index 000000000..e7b729230 --- /dev/null +++ b/test/Microsoft.ComponentDetection.Detectors.Tests/NuGetMSBuildBinaryLogComponentDetectorTests.cs @@ -0,0 +1,123 @@ +namespace Microsoft.ComponentDetection.Detectors.Tests; + +using System.Linq; +using System.Threading.Tasks; +using FluentAssertions; +using Microsoft.ComponentDetection.Contracts.TypedComponent; +using Microsoft.ComponentDetection.Detectors.NuGet; +using Microsoft.ComponentDetection.TestsUtilities; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +[TestClass] +[TestCategory("Governance/All")] +[TestCategory("Governance/ComponentDetection")] +public class NuGetMSBuildBinaryLogComponentDetectorTests : BaseDetectorTest +{ + [TestMethod] + public async Task DependenciesAreReportedForEachProjectFile() + { + // the contents of `projectContents` are the root entrypoint to the detector, but MSBuild will crawl to the other project file + var (scanResult, componentRecorder) = await this.ExecuteDetectorAndGetBinLogAsync( + projectContents: $@" + + + {MSBuildTestUtilities.TestTargetFramework} + + + + + + ", + additionalFiles: new[] + { + ("other-project/other-project.csproj", $@" + + + {MSBuildTestUtilities.TestTargetFramework} + + + + + + "), + }, + mockedPackages: new[] + { + ("Some.Package", "1.2.3", MSBuildTestUtilities.TestTargetFramework, ""), + ("Transitive.Dependency", "4.5.6", MSBuildTestUtilities.TestTargetFramework, null), + }); + + var detectedComponents = componentRecorder.GetDetectedComponents(); + + // components are reported for each project file + var originalFileComponents = detectedComponents + .Where(d => d.FilePaths.Any(p => p.Replace("\\", "/").EndsWith("/project.csproj"))) + .Select(d => d.Component) + .Cast() + .OrderBy(c => c.Name) + .Select(c => $"{c.Name}/{c.Version}"); + originalFileComponents.Should().Equal("Some.Package/1.2.3", "Transitive.Dependency/4.5.6"); + + var referencedFileComponents = detectedComponents + .Where(d => d.FilePaths.Any(p => p.Replace("\\", "/").EndsWith("/other-project/other-project.csproj"))) + .Select(d => d.Component) + .Cast() + .OrderBy(c => c.Name) + .Select(c => $"{c.Name}/{c.Version}"); + referencedFileComponents.Should().Equal("Some.Package/1.2.3", "Transitive.Dependency/4.5.6"); + } + + [TestMethod] + public async Task RemovedPackagesAreNotReported() + { + // This is a very specific scenario that should be tested, but we don't want any changes to this repo's SDK to + // change the outcome of the test, so we're doing it manually. The scenario is the SDK knowingly replacing an + // assembly from an outdated transitive package. One example common in the wild is the package + // `Microsoft.Extensions.Configuration.Json/6.0.0` which contains a transitive dependency on + // `System.Text.Json/6.0.0`, but the SDK version 6.0.424 or later pulls this reference out of the dependency + // set and replaces the .dll with a local updated copy. The end result is the `package.assets.json` file + // reports that `System.Text.Json/6.0.0` is referenced by a project, but after build and at runtime, this isn't + // the case and can lead to false positives when reporting dependencies. The way the SDK accomplishes this is + // by removing `System.Text.Json.dll` from the group `@(RuntimeCopyLocalItems)`. To accomplish this in the + // test, we're inserting a custom target that does this same action. + var (scanResult, componentRecorder) = await this.ExecuteDetectorAndGetBinLogAsync( + projectContents: $@" + + + {MSBuildTestUtilities.TestTargetFramework} + + + + + + + + + + + + ", + mockedPackages: new[] + { + ("Some.Package", "1.2.3", MSBuildTestUtilities.TestTargetFramework, ""), + ("Transitive.Dependency", "4.5.6", MSBuildTestUtilities.TestTargetFramework, null), + }); + + var detectedComponents = componentRecorder.GetDetectedComponents(); + + var packages = detectedComponents.Select(d => d.Component).Cast().OrderBy(c => c.Name).Select(c => $"{c.Name}/{c.Version}"); + packages.Should().Equal("Some.Package/1.2.3"); + } + + private async Task<(Contracts.IndividualDetectorScanResult ScanResult, Contracts.IComponentRecorder ComponentRecorder)> ExecuteDetectorAndGetBinLogAsync( + string projectContents, + (string FileName, string Content)[] additionalFiles = null, + (string Name, string Version, string TargetFramework, string DependenciesXml)[] mockedPackages = null) + { + using var binLogStream = await MSBuildTestUtilities.GetBinLogStreamFromFileContentsAsync(projectContents, additionalFiles, mockedPackages); + var (scanResult, componentRecorder) = await this.DetectorTestUtility + .WithFile("msbuild.binlog", binLogStream) + .ExecuteDetectorAsync(); + return (scanResult, componentRecorder); + } +} diff --git a/test/Microsoft.ComponentDetection.Detectors.Tests/NugetTestUtilities.cs b/test/Microsoft.ComponentDetection.Detectors.Tests/NugetTestUtilities.cs index 37fa45ce4..ea576713c 100644 --- a/test/Microsoft.ComponentDetection.Detectors.Tests/NugetTestUtilities.cs +++ b/test/Microsoft.ComponentDetection.Detectors.Tests/NugetTestUtilities.cs @@ -3,6 +3,7 @@ using System.IO; using System.IO.Compression; using System.Text; +using System.Threading; using System.Threading.Tasks; using Microsoft.ComponentDetection.Contracts; using Microsoft.ComponentDetection.TestsUtilities; @@ -60,7 +61,7 @@ public static string GetValidNuspec(string componentName, string version, string return GetTemplatedNuspec(componentName, version, authors); } - public static async Task ZipNupkgComponentAsync(string filename, string content) + public static async Task ZipNupkgComponentAsync(string filename, string content, (string Path, byte[] Contents)[] additionalFiles = null) { var stream = new MemoryStream(); @@ -68,10 +69,21 @@ public static async Task ZipNupkgComponentAsync(string filename, string { var entry = archive.CreateEntry($"{filename}.nuspec"); - using var entryStream = entry.Open(); - - var templateBytes = Encoding.UTF8.GetBytes(content); - await entryStream.WriteAsync(templateBytes); + using (var entryStream = entry.Open()) + { + var templateBytes = Encoding.UTF8.GetBytes(content); + await entryStream.WriteAsync(templateBytes); + } + + if (additionalFiles is not null) + { + foreach (var file in additionalFiles) + { + var additionalEntry = archive.CreateEntry(file.Path); + using var additionalEntryStream = additionalEntry.Open(); + await additionalEntryStream.WriteAsync(file.Contents, CancellationToken.None); + } + } } stream.Seek(0, SeekOrigin.Begin); From 8b4aa9c19fb1d016e924f7a6b4b59fb2ec9a63ff Mon Sep 17 00:00:00 2001 From: "Brett V. Forsgren" Date: Thu, 19 Sep 2024 16:29:54 -0600 Subject: [PATCH 02/10] add lock around MSBuild registration --- .../NuGetMSBuildBinaryLogComponentDetector.cs | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/Microsoft.ComponentDetection.Detectors/nuget/NuGetMSBuildBinaryLogComponentDetector.cs b/src/Microsoft.ComponentDetection.Detectors/nuget/NuGetMSBuildBinaryLogComponentDetector.cs index d62d6979d..5fb9291e6 100644 --- a/src/Microsoft.ComponentDetection.Detectors/nuget/NuGetMSBuildBinaryLogComponentDetector.cs +++ b/src/Microsoft.ComponentDetection.Detectors/nuget/NuGetMSBuildBinaryLogComponentDetector.cs @@ -30,6 +30,7 @@ public class NuGetMSBuildBinaryLogComponentDetector : FileComponentDetector ["_PackageDependenciesDesignTime"] = ("Name", "Version"), }; + private static readonly object MSBuildRegistrationGate = new(); private static bool isMSBuildRegistered; public NuGetMSBuildBinaryLogComponentDetector( @@ -131,12 +132,15 @@ protected override Task OnFileFoundAsync(ProcessRequest processRequest, IDiction { try { - if (!isMSBuildRegistered) + lock (MSBuildRegistrationGate) { - // this must happen once per process, and never again - var defaultInstance = MSBuildLocator.QueryVisualStudioInstances().First(); - MSBuildLocator.RegisterInstance(defaultInstance); - isMSBuildRegistered = true; + if (!isMSBuildRegistered) + { + // this must happen once per process, and never again + var defaultInstance = MSBuildLocator.QueryVisualStudioInstances().First(); + MSBuildLocator.RegisterInstance(defaultInstance); + isMSBuildRegistered = true; + } } var singleFileComponentRecorder = this.ComponentRecorder.CreateSingleFileComponentRecorder(processRequest.ComponentStream.Location); From 910caa48691ec98467a1d5dd03cac5f8188a4bbe Mon Sep 17 00:00:00 2001 From: "Brett V. Forsgren" Date: Thu, 19 Sep 2024 16:30:44 -0600 Subject: [PATCH 03/10] add test for `.sln` with two unrelated projects --- .../MSBuildTestUtilities.cs | 9 +- ...tMSBuildBinaryLogComponentDetectorTests.cs | 93 ++++++++++++++++++- 2 files changed, 96 insertions(+), 6 deletions(-) diff --git a/test/Microsoft.ComponentDetection.Detectors.Tests/MSBuildTestUtilities.cs b/test/Microsoft.ComponentDetection.Detectors.Tests/MSBuildTestUtilities.cs index 7e93c5bc2..97f51c5d3 100644 --- a/test/Microsoft.ComponentDetection.Detectors.Tests/MSBuildTestUtilities.cs +++ b/test/Microsoft.ComponentDetection.Detectors.Tests/MSBuildTestUtilities.cs @@ -56,14 +56,15 @@ public static class MSBuildTestUtilities }); public static async Task GetBinLogStreamFromFileContentsAsync( - string projectContents, + string defaultFilePath, + string defaultFileContents, (string FileName, string Contents)[] additionalFiles = null, (string Name, string Version, string TargetFramework, string AdditionalMetadataXml)[] mockedPackages = null) { // write all files using var tempDir = new TemporaryProjectDirectory(); - var fullProjectPath = Path.Combine(tempDir.DirectoryPath, "project.csproj"); - await File.WriteAllTextAsync(fullProjectPath, projectContents); + var fullDefaultFilePath = Path.Combine(tempDir.DirectoryPath, defaultFilePath); + await File.WriteAllTextAsync(fullDefaultFilePath, defaultFileContents); if (additionalFiles is not null) { foreach (var (fileName, contents) in additionalFiles) @@ -78,7 +79,7 @@ public static async Task GetBinLogStreamFromFileContentsAsync( await MockNuGetPackagesInDirectoryAsync(tempDir, mockedPackages); // generate the binlog - var (exitCode, stdOut, stdErr) = await RunProcessAsync("dotnet", $"build \"{fullProjectPath}\" /t:GenerateBuildDependencyFile /bl:msbuild.binlog", workingDirectory: tempDir.DirectoryPath); + var (exitCode, stdOut, stdErr) = await RunProcessAsync("dotnet", $"build \"{fullDefaultFilePath}\" /t:GenerateBuildDependencyFile /bl:msbuild.binlog", workingDirectory: tempDir.DirectoryPath); exitCode.Should().Be(0, $"STDOUT:\n{stdOut}\n\nSTDERR:\n{stdErr}"); // copy it to memory so the temporary directory can be cleaned up diff --git a/test/Microsoft.ComponentDetection.Detectors.Tests/NuGetMSBuildBinaryLogComponentDetectorTests.cs b/test/Microsoft.ComponentDetection.Detectors.Tests/NuGetMSBuildBinaryLogComponentDetectorTests.cs index e7b729230..c3dffce51 100644 --- a/test/Microsoft.ComponentDetection.Detectors.Tests/NuGetMSBuildBinaryLogComponentDetectorTests.cs +++ b/test/Microsoft.ComponentDetection.Detectors.Tests/NuGetMSBuildBinaryLogComponentDetectorTests.cs @@ -3,6 +3,7 @@ using System.Linq; using System.Threading.Tasks; using FluentAssertions; +using Microsoft.ComponentDetection.Contracts; using Microsoft.ComponentDetection.Contracts.TypedComponent; using Microsoft.ComponentDetection.Detectors.NuGet; using Microsoft.ComponentDetection.TestsUtilities; @@ -109,12 +110,100 @@ public async Task RemovedPackagesAreNotReported() packages.Should().Equal("Some.Package/1.2.3"); } - private async Task<(Contracts.IndividualDetectorScanResult ScanResult, Contracts.IComponentRecorder ComponentRecorder)> ExecuteDetectorAndGetBinLogAsync( + [TestMethod] + public async Task PackagesReportedFromSeparateProjectsDoNotOverlap() + { + // In this test, a top-level solution file references two projects which have no relationship between them. + // The result should be that each project only reports its own dependencies. + var slnContents = @" +Microsoft Visual Studio Solution File, Format Version 12.00 +# Visual Studio 17 +VisualStudioVersion = 17.0.31808.319 +MinimumVisualStudioVersion = 15.0.26124.0 +Project(""{9A19103F-16F7-4668-BE54-9A1E7A4F7556}"") = ""project1"", ""project1\project1.csproj"", ""{782E0C0A-10D3-444D-9640-263D03D2B20C}"" +EndProject +Project(""{9A19103F-16F7-4668-BE54-9A1E7A4F7556}"") = ""project2"", ""project2\project2.csproj"", ""{CBA73BF8-C922-4DD7-A41D-88CD22914356}"" +EndProject +Global + GlobalSection(SolutionConfigurationPlatforms) = preSolution + Debug|Any CPU = Debug|Any CPU + Release|Any CPU = Release|Any CPU + EndGlobalSection + GlobalSection(ProjectConfigurationPlatforms) = postSolution + {782E0C0A-10D3-444D-9640-263D03D2B20C}.Debug|Any CPU.ActiveCfg = Debug|Any CPU + {782E0C0A-10D3-444D-9640-263D03D2B20C}.Debug|Any CPU.Build.0 = Debug|Any CPU + {782E0C0A-10D3-444D-9640-263D03D2B20C}.Release|Any CPU.ActiveCfg = Release|Any CPU + {782E0C0A-10D3-444D-9640-263D03D2B20C}.Release|Any CPU.Build.0 = Release|Any CPU + {CBA73BF8-C922-4DD7-A41D-88CD22914356}.Debug|Any CPU.ActiveCfg = Debug|Any CPU + {CBA73BF8-C922-4DD7-A41D-88CD22914356}.Debug|Any CPU.Build.0 = Debug|Any CPU + {CBA73BF8-C922-4DD7-A41D-88CD22914356}.Release|Any CPU.ActiveCfg = Release|Any CPU + {CBA73BF8-C922-4DD7-A41D-88CD22914356}.Release|Any CPU.Build.0 = Release|Any CPU + EndGlobalSection + GlobalSection(SolutionProperties) = preSolution + HideSolutionNode = FALSE + EndGlobalSection +EndGlobal +"; + using var binLogStream = await MSBuildTestUtilities.GetBinLogStreamFromFileContentsAsync( + defaultFilePath: "solution.sln", + defaultFileContents: slnContents, + additionalFiles: new[] + { + ("project1/project1.csproj", $@" + + + {MSBuildTestUtilities.TestTargetFramework} + + + + + + "), + ("project2/project2.csproj", $@" + + + {MSBuildTestUtilities.TestTargetFramework} + + + + + + "), + }, + mockedPackages: new[] + { + ("Package.A", "1.2.3", MSBuildTestUtilities.TestTargetFramework, ""), + ("Package.B", "4.5.6", MSBuildTestUtilities.TestTargetFramework, ""), + }); + var (scanResult, componentRecorder) = await this.DetectorTestUtility + .WithFile("msbuild.binlog", binLogStream) + .ExecuteDetectorAsync(); + + var detectedComponents = componentRecorder.GetDetectedComponents(); + + var project1Components = detectedComponents + .Where(d => d.FilePaths.Any(p => p.Replace("\\", "/").EndsWith("/project1.csproj"))) + .Select(d => d.Component) + .Cast() + .OrderBy(c => c.Name) + .Select(c => $"{c.Name}/{c.Version}"); + project1Components.Should().Equal("Package.A/1.2.3"); + + var project2Components = detectedComponents + .Where(d => d.FilePaths.Any(p => p.Replace("\\", "/").EndsWith("/project2.csproj"))) + .Select(d => d.Component) + .Cast() + .OrderBy(c => c.Name) + .Select(c => $"{c.Name}/{c.Version}"); + project2Components.Should().Equal("Package.B/4.5.6"); + } + + private async Task<(IndividualDetectorScanResult ScanResult, IComponentRecorder ComponentRecorder)> ExecuteDetectorAndGetBinLogAsync( string projectContents, (string FileName, string Content)[] additionalFiles = null, (string Name, string Version, string TargetFramework, string DependenciesXml)[] mockedPackages = null) { - using var binLogStream = await MSBuildTestUtilities.GetBinLogStreamFromFileContentsAsync(projectContents, additionalFiles, mockedPackages); + using var binLogStream = await MSBuildTestUtilities.GetBinLogStreamFromFileContentsAsync("project.csproj", projectContents, additionalFiles, mockedPackages); var (scanResult, componentRecorder) = await this.DetectorTestUtility .WithFile("msbuild.binlog", binLogStream) .ExecuteDetectorAsync(); From eee567f4adf65bd54dce369dea4dcff713c12c48 Mon Sep 17 00:00:00 2001 From: "Brett V. Forsgren" Date: Thu, 19 Sep 2024 17:02:07 -0600 Subject: [PATCH 04/10] don't report dependencies from `.sln` files --- .../nuget/NuGetMSBuildBinaryLogComponentDetector.cs | 10 +++++++++- .../NuGetMSBuildBinaryLogComponentDetectorTests.cs | 8 ++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/src/Microsoft.ComponentDetection.Detectors/nuget/NuGetMSBuildBinaryLogComponentDetector.cs b/src/Microsoft.ComponentDetection.Detectors/nuget/NuGetMSBuildBinaryLogComponentDetector.cs index 5fb9291e6..2743c8b5a 100644 --- a/src/Microsoft.ComponentDetection.Detectors/nuget/NuGetMSBuildBinaryLogComponentDetector.cs +++ b/src/Microsoft.ComponentDetection.Detectors/nuget/NuGetMSBuildBinaryLogComponentDetector.cs @@ -2,6 +2,7 @@ using System; using System.Collections.Generic; +using System.IO; using System.Linq; using System.Threading; using Microsoft.Build.Locator; @@ -183,6 +184,12 @@ private void ProcessBinLog(Build buildRoot, ISingleFileComponentRecorder compone var projectsPerPackage = new Dictionary>(StringComparer.OrdinalIgnoreCase); foreach (var projectPath in projectResolvedDependencies.Keys) { + if (Path.GetExtension(projectPath).Equals(".sln", StringComparison.OrdinalIgnoreCase)) + { + // don't report solution files + continue; + } + var projectDependencies = projectResolvedDependencies[projectPath]; foreach (var (packageName, packageVersion) in projectDependencies) { @@ -198,8 +205,9 @@ private void ProcessBinLog(Build buildRoot, ISingleFileComponentRecorder compone } // report it all - foreach (var (packageNameAndVersion, projectPaths) in projectsPerPackage) + foreach (var packageNameAndVersion in projectsPerPackage.Keys.OrderBy(p => p)) { + var projectPaths = projectsPerPackage[packageNameAndVersion]; var parts = packageNameAndVersion.Split('/', 2); var packageName = parts[0]; var packageVersion = parts[1]; diff --git a/test/Microsoft.ComponentDetection.Detectors.Tests/NuGetMSBuildBinaryLogComponentDetectorTests.cs b/test/Microsoft.ComponentDetection.Detectors.Tests/NuGetMSBuildBinaryLogComponentDetectorTests.cs index c3dffce51..71b79eae8 100644 --- a/test/Microsoft.ComponentDetection.Detectors.Tests/NuGetMSBuildBinaryLogComponentDetectorTests.cs +++ b/test/Microsoft.ComponentDetection.Detectors.Tests/NuGetMSBuildBinaryLogComponentDetectorTests.cs @@ -196,6 +196,14 @@ public async Task PackagesReportedFromSeparateProjectsDoNotOverlap() .OrderBy(c => c.Name) .Select(c => $"{c.Name}/{c.Version}"); project2Components.Should().Equal("Package.B/4.5.6"); + + var solutionComponents = detectedComponents + .Where(d => d.FilePaths.Any(p => p.Replace("\\", "/").EndsWith("/solution.sln"))) + .Select(d => d.Component) + .Cast() + .OrderBy(c => c.Name) + .Select(c => $"{c.Name}/{c.Version}"); + solutionComponents.Should().BeEmpty(); } private async Task<(IndividualDetectorScanResult ScanResult, IComponentRecorder ComponentRecorder)> ExecuteDetectorAndGetBinLogAsync( From b8370879ee7df335f79a98b5a8606fd65088cc03 Mon Sep 17 00:00:00 2001 From: "Brett V. Forsgren" Date: Fri, 20 Sep 2024 15:43:04 -0600 Subject: [PATCH 05/10] report apphost packages from publish --- ...rosoft.ComponentDetection.Detectors.csproj | 4 + .../NuGetMSBuildBinaryLogComponentDetector.cs | 42 +++- ....ComponentDetection.Detectors.Tests.csproj | 3 + ...tMSBuildBinaryLogComponentDetectorTests.cs | 222 +++++++++++++++++- 4 files changed, 258 insertions(+), 13 deletions(-) diff --git a/src/Microsoft.ComponentDetection.Detectors/Microsoft.ComponentDetection.Detectors.csproj b/src/Microsoft.ComponentDetection.Detectors/Microsoft.ComponentDetection.Detectors.csproj index 8f8736076..8398c1710 100644 --- a/src/Microsoft.ComponentDetection.Detectors/Microsoft.ComponentDetection.Detectors.csproj +++ b/src/Microsoft.ComponentDetection.Detectors/Microsoft.ComponentDetection.Detectors.csproj @@ -36,4 +36,8 @@ + + + + diff --git a/src/Microsoft.ComponentDetection.Detectors/nuget/NuGetMSBuildBinaryLogComponentDetector.cs b/src/Microsoft.ComponentDetection.Detectors/nuget/NuGetMSBuildBinaryLogComponentDetector.cs index 2743c8b5a..d58f437f4 100644 --- a/src/Microsoft.ComponentDetection.Detectors/nuget/NuGetMSBuildBinaryLogComponentDetector.cs +++ b/src/Microsoft.ComponentDetection.Detectors/nuget/NuGetMSBuildBinaryLogComponentDetector.cs @@ -24,11 +24,18 @@ public class NuGetMSBuildBinaryLogComponentDetector : FileComponentDetector // the items listed below represent collection names that NuGet will resolve a package into, along with the metadata value names to get the package name and version private static readonly Dictionary ResolvedPackageItemNames = new Dictionary(StringComparer.OrdinalIgnoreCase) { + // regular restore operations ["NativeCopyLocalItems"] = ("NuGetPackageId", "NuGetPackageVersion"), ["ResourceCopyLocalItems"] = ("NuGetPackageId", "NuGetPackageVersion"), ["RuntimeCopyLocalItems"] = ("NuGetPackageId", "NuGetPackageVersion"), ["ResolvedAnalyzers"] = ("NuGetPackageId", "NuGetPackageVersion"), ["_PackageDependenciesDesignTime"] = ("Name", "Version"), + + // implicitly added by the SDK during a publish operation + ["ResolvedAppHostPack"] = ("NuGetPackageId", "NuGetPackageVersion"), + ["ResolvedSingleFileHostPack"] = ("NuGetPackageId", "NuGetPackageVersion"), + ["ResolvedComHostPack"] = ("NuGetPackageId", "NuGetPackageVersion"), + ["ResolvedIjwHostPack"] = ("NuGetPackageId", "NuGetPackageVersion"), }; private static readonly object MSBuildRegistrationGate = new(); @@ -46,7 +53,7 @@ public NuGetMSBuildBinaryLogComponentDetector( public override IEnumerable Categories => new[] { Enum.GetName(typeof(DetectorClass), DetectorClass.NuGet) }; - public override IList SearchPatterns { get; } = new List { "*.binlog" }; + public override IList SearchPatterns { get; } = new List { "*.binlog", "*.buildlog" }; public override IEnumerable SupportedComponentTypes { get; } = new[] { ComponentType.NuGet }; @@ -133,19 +140,16 @@ protected override Task OnFileFoundAsync(ProcessRequest processRequest, IDiction { try { - lock (MSBuildRegistrationGate) - { - if (!isMSBuildRegistered) - { - // this must happen once per process, and never again - var defaultInstance = MSBuildLocator.QueryVisualStudioInstances().First(); - MSBuildLocator.RegisterInstance(defaultInstance); - isMSBuildRegistered = true; - } - } + EnsureMSBuildIsRegistered(); var singleFileComponentRecorder = this.ComponentRecorder.CreateSingleFileComponentRecorder(processRequest.ComponentStream.Location); - var buildRoot = BinaryLog.ReadBuild(processRequest.ComponentStream.Stream); + var extension = Path.GetExtension(processRequest.ComponentStream.Location); + var buildRoot = extension.ToLower() switch + { + ".binlog" => BinaryLog.ReadBuild(processRequest.ComponentStream.Stream), + ".buildlog" => BuildLogReader.Read(processRequest.ComponentStream.Stream), + _ => throw new NotSupportedException($"Unexpected log file extension: {extension}"), + }; this.RecordLockfileVersion(buildRoot.FileFormatVersion); this.ProcessBinLog(buildRoot, singleFileComponentRecorder); } @@ -158,6 +162,20 @@ protected override Task OnFileFoundAsync(ProcessRequest processRequest, IDiction return Task.CompletedTask; } + internal static void EnsureMSBuildIsRegistered() + { + lock (MSBuildRegistrationGate) + { + if (!isMSBuildRegistered) + { + // this must happen once per process, and never again + var defaultInstance = MSBuildLocator.QueryVisualStudioInstances().First(); + MSBuildLocator.RegisterInstance(defaultInstance); + isMSBuildRegistered = true; + } + } + } + protected override Task OnDetectionFinishedAsync() { return Task.CompletedTask; diff --git a/test/Microsoft.ComponentDetection.Detectors.Tests/Microsoft.ComponentDetection.Detectors.Tests.csproj b/test/Microsoft.ComponentDetection.Detectors.Tests/Microsoft.ComponentDetection.Detectors.Tests.csproj index efcd77335..470a9f9af 100644 --- a/test/Microsoft.ComponentDetection.Detectors.Tests/Microsoft.ComponentDetection.Detectors.Tests.csproj +++ b/test/Microsoft.ComponentDetection.Detectors.Tests/Microsoft.ComponentDetection.Detectors.Tests.csproj @@ -7,7 +7,10 @@ + + + diff --git a/test/Microsoft.ComponentDetection.Detectors.Tests/NuGetMSBuildBinaryLogComponentDetectorTests.cs b/test/Microsoft.ComponentDetection.Detectors.Tests/NuGetMSBuildBinaryLogComponentDetectorTests.cs index 71b79eae8..47d02d83a 100644 --- a/test/Microsoft.ComponentDetection.Detectors.Tests/NuGetMSBuildBinaryLogComponentDetectorTests.cs +++ b/test/Microsoft.ComponentDetection.Detectors.Tests/NuGetMSBuildBinaryLogComponentDetectorTests.cs @@ -1,19 +1,27 @@ namespace Microsoft.ComponentDetection.Detectors.Tests; +using System; +using System.IO; using System.Linq; using System.Threading.Tasks; using FluentAssertions; +using Microsoft.Build.Logging.StructuredLogger; using Microsoft.ComponentDetection.Contracts; using Microsoft.ComponentDetection.Contracts.TypedComponent; using Microsoft.ComponentDetection.Detectors.NuGet; using Microsoft.ComponentDetection.TestsUtilities; using Microsoft.VisualStudio.TestTools.UnitTesting; +using MSBuildTask = Microsoft.Build.Logging.StructuredLogger.Task; +using Task = System.Threading.Tasks.Task; + [TestClass] [TestCategory("Governance/All")] [TestCategory("Governance/ComponentDetection")] public class NuGetMSBuildBinaryLogComponentDetectorTests : BaseDetectorTest { + static NuGetMSBuildBinaryLogComponentDetectorTests() => NuGetMSBuildBinaryLogComponentDetector.EnsureMSBuildIsRegistered(); + [TestMethod] public async Task DependenciesAreReportedForEachProjectFile() { @@ -206,12 +214,224 @@ public async Task PackagesReportedFromSeparateProjectsDoNotOverlap() solutionComponents.Should().BeEmpty(); } + [TestMethod] + public async Task PackagesImplicitlyAddedBySdkDuringPublishAreAdded() + { + // When a project is published, the SDK will add references to some AppHost specific packages. Doing an actual + // publish operation here would be too slow, so a mock in-memory binary log is used that has the same shape + // (although trimmed down) of a real publish log. + var binlog = new Build() + { + Succeeded = true, + Children = + { + new Project() + { + ProjectFile = "project.csproj", + Children = + { + new Target() + { + Name = "ResolveFrameworkReferences", + Children = + { + // ResolvedAppHostPack + new MSBuildTask() + { + Name = "GetPackageDirectory", + Children = + { + new Folder() + { + Name = "OutputItems", + Children = + { + new AddItem() + { + Name = "ResolvedAppHostPack", + Children = + { + new Item() + { + Name = "AppHost", + Children = + { + new Metadata() + { + Name = "NuGetPackageId", + Value = "Microsoft.NETCore.App.Host.win-x64", + }, + new Metadata() + { + Name = "NuGetPackageVersion", + Value = "6.0.33", + }, + }, + }, + }, + }, + }, + }, + }, + }, + + // ResolvedSingleFileHostPack + new MSBuildTask() + { + Name = "GetPackageDirectory", + Children = + { + new Folder() + { + Name = "OutputItems", + Children = + { + new AddItem() + { + Name = "ResolvedSingleFileHostPack", + Children = + { + new Item() + { + Name = "SingleFileHost", + Children = + { + new Metadata() + { + Name = "NuGetPackageId", + Value = "Microsoft.NETCore.App.Host.win-x64", + }, + new Metadata() + { + Name = "NuGetPackageVersion", + Value = "6.0.33", + }, + }, + }, + }, + }, + }, + }, + }, + }, + + // ResolvedComHostPack + new MSBuildTask() + { + Name = "GetPackageDirectory", + Children = + { + new Folder() + { + Name = "OutputItems", + Children = + { + new AddItem() + { + Name = "ResolvedComHostPack", + Children = + { + new Item() + { + Name = "ComHost", + Children = + { + new Metadata() + { + Name = "NuGetPackageId", + Value = "Microsoft.NETCore.App.Host.win-x64", + }, + new Metadata() + { + Name = "NuGetPackageVersion", + Value = "6.0.33", + }, + }, + }, + }, + }, + }, + }, + }, + }, + + // ResolvedIjwHostPack + new MSBuildTask() + { + Name = "GetPackageDirectory", + Children = + { + new Folder() + { + Name = "OutputItems", + Children = + { + new AddItem() + { + Name = "ResolvedIjwHostPack", + Children = + { + new Item() + { + Name = "IjwHost", + Children = + { + new Metadata() + { + Name = "NuGetPackageId", + Value = "Microsoft.NETCore.App.Host.win-x64", + }, + new Metadata() + { + Name = "NuGetPackageVersion", + Value = "6.0.33", + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }; + + // in-memory logs need to be `.buildlog` + var tempFile = Path.Combine(Path.GetTempPath(), $"{Guid.NewGuid():d}.buildlog"); + try + { + Serialization.Write(binlog, tempFile); + using var binLogStream = File.OpenRead(tempFile); + + var (scanResult, componentRecorder) = await this.DetectorTestUtility + .WithFile(tempFile, binLogStream) + .ExecuteDetectorAsync(); + var detectedComponents = componentRecorder.GetDetectedComponents(); + + var components = detectedComponents + .Select(d => d.Component) + .Cast() + .OrderBy(c => c.Name) + .Select(c => $"{c.Name}/{c.Version}"); + components.Should().Equal("Microsoft.NETCore.App.Host.win-x64/6.0.33"); + } + finally + { + File.Delete(tempFile); + } + } + private async Task<(IndividualDetectorScanResult ScanResult, IComponentRecorder ComponentRecorder)> ExecuteDetectorAndGetBinLogAsync( string projectContents, (string FileName, string Content)[] additionalFiles = null, (string Name, string Version, string TargetFramework, string DependenciesXml)[] mockedPackages = null) { - using var binLogStream = await MSBuildTestUtilities.GetBinLogStreamFromFileContentsAsync("project.csproj", projectContents, additionalFiles, mockedPackages); + using var binLogStream = await MSBuildTestUtilities.GetBinLogStreamFromFileContentsAsync("project.csproj", projectContents, additionalFiles: additionalFiles, mockedPackages: mockedPackages); var (scanResult, componentRecorder) = await this.DetectorTestUtility .WithFile("msbuild.binlog", binLogStream) .ExecuteDetectorAsync(); From aa9d767b422aa6cf2dd8ad6431196e898f714534 Mon Sep 17 00:00:00 2001 From: "Brett V. Forsgren" Date: Fri, 20 Sep 2024 15:44:34 -0600 Subject: [PATCH 06/10] use built-in registration check --- .../nuget/NuGetMSBuildBinaryLogComponentDetector.cs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/Microsoft.ComponentDetection.Detectors/nuget/NuGetMSBuildBinaryLogComponentDetector.cs b/src/Microsoft.ComponentDetection.Detectors/nuget/NuGetMSBuildBinaryLogComponentDetector.cs index d58f437f4..6939fed21 100644 --- a/src/Microsoft.ComponentDetection.Detectors/nuget/NuGetMSBuildBinaryLogComponentDetector.cs +++ b/src/Microsoft.ComponentDetection.Detectors/nuget/NuGetMSBuildBinaryLogComponentDetector.cs @@ -39,7 +39,6 @@ public class NuGetMSBuildBinaryLogComponentDetector : FileComponentDetector }; private static readonly object MSBuildRegistrationGate = new(); - private static bool isMSBuildRegistered; public NuGetMSBuildBinaryLogComponentDetector( IObservableDirectoryWalkerFactory walkerFactory, @@ -166,12 +165,11 @@ internal static void EnsureMSBuildIsRegistered() { lock (MSBuildRegistrationGate) { - if (!isMSBuildRegistered) + if (!MSBuildLocator.IsRegistered) { // this must happen once per process, and never again var defaultInstance = MSBuildLocator.QueryVisualStudioInstances().First(); MSBuildLocator.RegisterInstance(defaultInstance); - isMSBuildRegistered = true; } } } From ffc2ece1dac8b1e9b4a09c1b94a3bc1d03ab309c Mon Sep 17 00:00:00 2001 From: "Brett V. Forsgren" Date: Mon, 23 Sep 2024 11:20:43 -0600 Subject: [PATCH 07/10] allow the same package with multiple versions This corresponds to a project with multiple TFMs where the same package is imported in each case, but with a different version each time. --- .../NuGetMSBuildBinaryLogComponentDetector.cs | 33 ++++++++----- .../MSBuildTestUtilities.cs | 4 +- ...tMSBuildBinaryLogComponentDetectorTests.cs | 46 ++++++++++++++++++- 3 files changed, 69 insertions(+), 14 deletions(-) diff --git a/src/Microsoft.ComponentDetection.Detectors/nuget/NuGetMSBuildBinaryLogComponentDetector.cs b/src/Microsoft.ComponentDetection.Detectors/nuget/NuGetMSBuildBinaryLogComponentDetector.cs index 6939fed21..2530673ea 100644 --- a/src/Microsoft.ComponentDetection.Detectors/nuget/NuGetMSBuildBinaryLogComponentDetector.cs +++ b/src/Microsoft.ComponentDetection.Detectors/nuget/NuGetMSBuildBinaryLogComponentDetector.cs @@ -58,7 +58,7 @@ public NuGetMSBuildBinaryLogComponentDetector( public override int Version { get; } = 1; - private static void ProcessResolvedPackageReference(Dictionary> topLevelDependencies, Dictionary> projectResolvedDependencies, NamedNode node) + private static void ProcessResolvedPackageReference(Dictionary> topLevelDependencies, Dictionary>> projectResolvedDependencies, NamedNode node) { var doRemoveOperation = node is RemoveItem; var doAddOperation = node is AddItem; @@ -110,14 +110,20 @@ private static void ProcessResolvedPackageReference(Dictionary(); @@ -183,7 +189,7 @@ private void ProcessBinLog(Build buildRoot, ISingleFileComponentRecorder compone { // maps a project path to a set of resolved dependencies var projectTopLevelDependencies = new Dictionary>(StringComparer.OrdinalIgnoreCase); - var projectResolvedDependencies = new Dictionary>(StringComparer.OrdinalIgnoreCase); + var projectResolvedDependencies = new Dictionary>>(StringComparer.OrdinalIgnoreCase); buildRoot.VisitAllChildren(node => { switch (node) @@ -198,7 +204,7 @@ private void ProcessBinLog(Build buildRoot, ISingleFileComponentRecorder compone // dependencies were resolved per project, we need to re-arrange them to be per package/version var projectsPerPackage = new Dictionary>(StringComparer.OrdinalIgnoreCase); - foreach (var projectPath in projectResolvedDependencies.Keys) + foreach (var projectPath in projectResolvedDependencies.Keys.OrderBy(p => p)) { if (Path.GetExtension(projectPath).Equals(".sln", StringComparison.OrdinalIgnoreCase)) { @@ -207,16 +213,19 @@ private void ProcessBinLog(Build buildRoot, ISingleFileComponentRecorder compone } var projectDependencies = projectResolvedDependencies[projectPath]; - foreach (var (packageName, packageVersion) in projectDependencies) + foreach (var (packageName, packageVersions) in projectDependencies.OrderBy(p => p.Key)) { - var key = $"{packageName}/{packageVersion}"; - if (!projectsPerPackage.TryGetValue(key, out var projectPaths)) + foreach (var packageVersion in packageVersions.OrderBy(v => v)) { - projectPaths = new HashSet(StringComparer.OrdinalIgnoreCase); - projectsPerPackage[key] = projectPaths; - } + var key = $"{packageName}/{packageVersion}"; + if (!projectsPerPackage.TryGetValue(key, out var projectPaths)) + { + projectPaths = new HashSet(StringComparer.OrdinalIgnoreCase); + projectsPerPackage[key] = projectPaths; + } - projectPaths.Add(projectPath); + projectPaths.Add(projectPath); + } } } diff --git a/test/Microsoft.ComponentDetection.Detectors.Tests/MSBuildTestUtilities.cs b/test/Microsoft.ComponentDetection.Detectors.Tests/MSBuildTestUtilities.cs index 97f51c5d3..2ca0553ac 100644 --- a/test/Microsoft.ComponentDetection.Detectors.Tests/MSBuildTestUtilities.cs +++ b/test/Microsoft.ComponentDetection.Detectors.Tests/MSBuildTestUtilities.cs @@ -58,6 +58,7 @@ public static class MSBuildTestUtilities public static async Task GetBinLogStreamFromFileContentsAsync( string defaultFilePath, string defaultFileContents, + string targetName = null, (string FileName, string Contents)[] additionalFiles = null, (string Name, string Version, string TargetFramework, string AdditionalMetadataXml)[] mockedPackages = null) { @@ -79,7 +80,8 @@ public static async Task GetBinLogStreamFromFileContentsAsync( await MockNuGetPackagesInDirectoryAsync(tempDir, mockedPackages); // generate the binlog - var (exitCode, stdOut, stdErr) = await RunProcessAsync("dotnet", $"build \"{fullDefaultFilePath}\" /t:GenerateBuildDependencyFile /bl:msbuild.binlog", workingDirectory: tempDir.DirectoryPath); + targetName ??= "GenerateBuildDependencyFile"; + var (exitCode, stdOut, stdErr) = await RunProcessAsync("dotnet", $"build \"{fullDefaultFilePath}\" /t:{targetName} /bl:msbuild.binlog", workingDirectory: tempDir.DirectoryPath); exitCode.Should().Be(0, $"STDOUT:\n{stdOut}\n\nSTDERR:\n{stdErr}"); // copy it to memory so the temporary directory can be cleaned up diff --git a/test/Microsoft.ComponentDetection.Detectors.Tests/NuGetMSBuildBinaryLogComponentDetectorTests.cs b/test/Microsoft.ComponentDetection.Detectors.Tests/NuGetMSBuildBinaryLogComponentDetectorTests.cs index 47d02d83a..be703b59e 100644 --- a/test/Microsoft.ComponentDetection.Detectors.Tests/NuGetMSBuildBinaryLogComponentDetectorTests.cs +++ b/test/Microsoft.ComponentDetection.Detectors.Tests/NuGetMSBuildBinaryLogComponentDetectorTests.cs @@ -214,6 +214,49 @@ public async Task PackagesReportedFromSeparateProjectsDoNotOverlap() solutionComponents.Should().BeEmpty(); } + [TestMethod] + public async Task PackagesAreReportedWhenConditionedOnTargetFramework() + { + // This test simulates a project with multiple TFMs where the same package is imported in each, but with a + // different version. To avoid building the entire project, we can fake what MSBuild does by resolving + // packages with each TFM. I manually verified that a "real" invocation of MSBuild with the "Build" target + // produces the same shape of the binlog as this test generates. + + // The end result is that _all_ packages are reported, regardless of the TFM invokation they came from, and in + // this case that is good, because we really only care about what packages were used in the build and what + // project file they came from. + var (scanResult, componentRecorder) = await this.ExecuteDetectorAndGetBinLogAsync( + projectContents: $@" + + + netstandard2.0;{MSBuildTestUtilities.TestTargetFramework} + + + + + + + + + + + ", + targetName: "TEST_GenerateBuildDependencyFileForTargetFrameworks", + mockedPackages: new[] + { + ("NETStandard.Library", "2.0.3", "netstandard2.0", ""), + ("Some.Package", "1.2.3", "netstandard2.0", null), + ("Some.Package", "4.5.6", MSBuildTestUtilities.TestTargetFramework, null), + }); + + var detectedComponents = componentRecorder.GetDetectedComponents(); + + var packages = detectedComponents + .Where(d => d.FilePaths.Any(p => p.Replace("\\", "/").EndsWith("/project.csproj"))) + .Select(d => d.Component).Cast().OrderBy(c => c.Name).ThenBy(c => c.Version).Select(c => $"{c.Name}/{c.Version}"); + packages.Should().Equal("NETStandard.Library/2.0.3", "Some.Package/1.2.3", "Some.Package/4.5.6"); + } + [TestMethod] public async Task PackagesImplicitlyAddedBySdkDuringPublishAreAdded() { @@ -428,10 +471,11 @@ public async Task PackagesImplicitlyAddedBySdkDuringPublishAreAdded() private async Task<(IndividualDetectorScanResult ScanResult, IComponentRecorder ComponentRecorder)> ExecuteDetectorAndGetBinLogAsync( string projectContents, + string targetName = null, (string FileName, string Content)[] additionalFiles = null, (string Name, string Version, string TargetFramework, string DependenciesXml)[] mockedPackages = null) { - using var binLogStream = await MSBuildTestUtilities.GetBinLogStreamFromFileContentsAsync("project.csproj", projectContents, additionalFiles: additionalFiles, mockedPackages: mockedPackages); + using var binLogStream = await MSBuildTestUtilities.GetBinLogStreamFromFileContentsAsync("project.csproj", projectContents, targetName: targetName, additionalFiles: additionalFiles, mockedPackages: mockedPackages); var (scanResult, componentRecorder) = await this.DetectorTestUtility .WithFile("msbuild.binlog", binLogStream) .ExecuteDetectorAsync(); From d4ddd71bfbae7f9ddd9fdf1273122eab207bf8c5 Mon Sep 17 00:00:00 2001 From: "Brett V. Forsgren" Date: Tue, 24 Sep 2024 10:48:39 -0600 Subject: [PATCH 08/10] report the exact version of the SDK used to build the project --- .../NuGetMSBuildBinaryLogComponentDetector.cs | 47 ++++++++ ...tMSBuildBinaryLogComponentDetectorTests.cs | 100 ++++++++++++++---- .../Utilities/TemporaryFile.cs | 41 +++++++ 3 files changed, 165 insertions(+), 23 deletions(-) create mode 100644 test/Microsoft.ComponentDetection.Detectors.Tests/Utilities/TemporaryFile.cs diff --git a/src/Microsoft.ComponentDetection.Detectors/nuget/NuGetMSBuildBinaryLogComponentDetector.cs b/src/Microsoft.ComponentDetection.Detectors/nuget/NuGetMSBuildBinaryLogComponentDetector.cs index 2530673ea..0bccb1f16 100644 --- a/src/Microsoft.ComponentDetection.Detectors/nuget/NuGetMSBuildBinaryLogComponentDetector.cs +++ b/src/Microsoft.ComponentDetection.Detectors/nuget/NuGetMSBuildBinaryLogComponentDetector.cs @@ -38,6 +38,12 @@ public class NuGetMSBuildBinaryLogComponentDetector : FileComponentDetector ["ResolvedIjwHostPack"] = ("NuGetPackageId", "NuGetPackageVersion"), }; + // the items listed below represent top-level property names that correspond to well-known packages + private static readonly Dictionary PropertyPackageNames = new Dictionary(StringComparer.OrdinalIgnoreCase) + { + ["NETCoreSdkVersion"] = ".NET SDK", + }; + private static readonly object MSBuildRegistrationGate = new(); public NuGetMSBuildBinaryLogComponentDetector( @@ -134,6 +140,44 @@ private static void ProcessResolvedPackageReference(Dictionary>> projectResolvedDependencies, Property node) + { + if (PropertyPackageNames.TryGetValue(node.Name, out var packageName)) + { + string projectFile; + var projectEvaluation = node.GetNearestParent(); + if (projectEvaluation is not null) + { + // `.binlog` files store properties in a `ProjectEvaluation` + projectFile = projectEvaluation.ProjectFile; + } + else + { + // `.buildlog` files store proeprties in `Project` + var project = node.GetNearestParent(); + projectFile = project?.ProjectFile; + } + + if (projectFile is not null) + { + var packageVersion = node.Value; + if (!projectResolvedDependencies.TryGetValue(projectFile, out var projectDependencies)) + { + projectDependencies = new(StringComparer.OrdinalIgnoreCase); + projectResolvedDependencies[projectFile] = projectDependencies; + } + + if (!projectDependencies.TryGetValue(packageName, out var packageVersions)) + { + packageVersions = new(StringComparer.OrdinalIgnoreCase); + projectDependencies[packageName] = packageVersions; + } + + packageVersions.Add(packageVersion); + } + } + } + private static string GetChildMetadataValue(TreeNode node, string metadataItemName) { var metadata = node.Children.OfType(); @@ -197,6 +241,9 @@ private void ProcessBinLog(Build buildRoot, ISingleFileComponentRecorder compone case NamedNode namedNode when namedNode is AddItem or RemoveItem: ProcessResolvedPackageReference(projectTopLevelDependencies, projectResolvedDependencies, namedNode); break; + case Property property when property.Parent is Folder folder && folder.Name == "Properties": + ProcessProjectProperty(projectResolvedDependencies, property); + break; default: break; } diff --git a/test/Microsoft.ComponentDetection.Detectors.Tests/NuGetMSBuildBinaryLogComponentDetectorTests.cs b/test/Microsoft.ComponentDetection.Detectors.Tests/NuGetMSBuildBinaryLogComponentDetectorTests.cs index be703b59e..5da3dbfe7 100644 --- a/test/Microsoft.ComponentDetection.Detectors.Tests/NuGetMSBuildBinaryLogComponentDetectorTests.cs +++ b/test/Microsoft.ComponentDetection.Detectors.Tests/NuGetMSBuildBinaryLogComponentDetectorTests.cs @@ -9,6 +9,7 @@ using Microsoft.ComponentDetection.Contracts; using Microsoft.ComponentDetection.Contracts.TypedComponent; using Microsoft.ComponentDetection.Detectors.NuGet; +using Microsoft.ComponentDetection.Detectors.Tests.Utilities; using Microsoft.ComponentDetection.TestsUtilities; using Microsoft.VisualStudio.TestTools.UnitTesting; @@ -63,6 +64,7 @@ public async Task DependenciesAreReportedForEachProjectFile() .Where(d => d.FilePaths.Any(p => p.Replace("\\", "/").EndsWith("/project.csproj"))) .Select(d => d.Component) .Cast() + .Where(c => c.Name != ".NET SDK") // dealt with in another test because the SDK version will change regularly .OrderBy(c => c.Name) .Select(c => $"{c.Name}/{c.Version}"); originalFileComponents.Should().Equal("Some.Package/1.2.3", "Transitive.Dependency/4.5.6"); @@ -71,6 +73,7 @@ public async Task DependenciesAreReportedForEachProjectFile() .Where(d => d.FilePaths.Any(p => p.Replace("\\", "/").EndsWith("/other-project/other-project.csproj"))) .Select(d => d.Component) .Cast() + .Where(c => c.Name != ".NET SDK") // dealt with in another test because the SDK version will change regularly .OrderBy(c => c.Name) .Select(c => $"{c.Name}/{c.Version}"); referencedFileComponents.Should().Equal("Some.Package/1.2.3", "Transitive.Dependency/4.5.6"); @@ -114,7 +117,12 @@ public async Task RemovedPackagesAreNotReported() var detectedComponents = componentRecorder.GetDetectedComponents(); - var packages = detectedComponents.Select(d => d.Component).Cast().OrderBy(c => c.Name).Select(c => $"{c.Name}/{c.Version}"); + var packages = detectedComponents + .Select(d => d.Component) + .Cast() + .Where(c => c.Name != ".NET SDK") // dealt with in another test because the SDK version will change regularly + .OrderBy(c => c.Name) + .Select(c => $"{c.Name}/{c.Version}"); packages.Should().Equal("Some.Package/1.2.3"); } @@ -193,6 +201,7 @@ public async Task PackagesReportedFromSeparateProjectsDoNotOverlap() .Where(d => d.FilePaths.Any(p => p.Replace("\\", "/").EndsWith("/project1.csproj"))) .Select(d => d.Component) .Cast() + .Where(c => c.Name != ".NET SDK") // dealt with in another test because the SDK version will change regularly .OrderBy(c => c.Name) .Select(c => $"{c.Name}/{c.Version}"); project1Components.Should().Equal("Package.A/1.2.3"); @@ -201,6 +210,7 @@ public async Task PackagesReportedFromSeparateProjectsDoNotOverlap() .Where(d => d.FilePaths.Any(p => p.Replace("\\", "/").EndsWith("/project2.csproj"))) .Select(d => d.Component) .Cast() + .Where(c => c.Name != ".NET SDK") // dealt with in another test because the SDK version will change regularly .OrderBy(c => c.Name) .Select(c => $"{c.Name}/{c.Version}"); project2Components.Should().Equal("Package.B/4.5.6"); @@ -253,7 +263,11 @@ public async Task PackagesAreReportedWhenConditionedOnTargetFramework() var packages = detectedComponents .Where(d => d.FilePaths.Any(p => p.Replace("\\", "/").EndsWith("/project.csproj"))) - .Select(d => d.Component).Cast().OrderBy(c => c.Name).ThenBy(c => c.Version).Select(c => $"{c.Name}/{c.Version}"); + .Select(d => d.Component) + .Cast() + .Where(c => c.Name != ".NET SDK") // dealt with in another test because the SDK version will change regularly + .OrderBy(c => c.Name).ThenBy(c => c.Version) + .Select(c => $"{c.Name}/{c.Version}"); packages.Should().Equal("NETStandard.Library/2.0.3", "Some.Package/1.2.3", "Some.Package/4.5.6"); } @@ -445,28 +459,68 @@ public async Task PackagesImplicitlyAddedBySdkDuringPublishAreAdded() }; // in-memory logs need to be `.buildlog` - var tempFile = Path.Combine(Path.GetTempPath(), $"{Guid.NewGuid():d}.buildlog"); - try - { - Serialization.Write(binlog, tempFile); - using var binLogStream = File.OpenRead(tempFile); - - var (scanResult, componentRecorder) = await this.DetectorTestUtility - .WithFile(tempFile, binLogStream) - .ExecuteDetectorAsync(); - var detectedComponents = componentRecorder.GetDetectedComponents(); - - var components = detectedComponents - .Select(d => d.Component) - .Cast() - .OrderBy(c => c.Name) - .Select(c => $"{c.Name}/{c.Version}"); - components.Should().Equal("Microsoft.NETCore.App.Host.win-x64/6.0.33"); - } - finally + using var tempFile = new TemporaryFile(".buildlog"); + Serialization.Write(binlog, tempFile.FilePath); + using var binLogStream = File.OpenRead(tempFile.FilePath); + + var (scanResult, componentRecorder) = await this.DetectorTestUtility + .WithFile(tempFile.FilePath, binLogStream) + .ExecuteDetectorAsync(); + var detectedComponents = componentRecorder.GetDetectedComponents(); + + var components = detectedComponents + .Select(d => d.Component) + .Cast() + .OrderBy(c => c.Name) + .Select(c => $"{c.Name}/{c.Version}"); + components.Should().Equal("Microsoft.NETCore.App.Host.win-x64/6.0.33"); + } + + [TestMethod] + public async Task DotNetSDKVersionIsReported() + { + var binlog = new Build() { - File.Delete(tempFile); - } + Succeeded = true, + }; + + binlog.EvaluationFolder.Children.Add( + new Project() + { + ProjectFile = "project.csproj", + Children = + { + new Folder() + { + Name = "Properties", + Children = + { + new Property() + { + Name = "NETCoreSdkVersion", + Value = "6.0.789", + }, + }, + }, + }, + }); + + // in-memory logs need to be `.buildlog` + using var tempFile = new TemporaryFile(".buildlog"); + Serialization.Write(binlog, tempFile.FilePath); + using var binLogStream = File.OpenRead(tempFile.FilePath); + + var (scanResult, componentRecorder) = await this.DetectorTestUtility + .WithFile(tempFile.FilePath, binLogStream) + .ExecuteDetectorAsync(); + var detectedComponents = componentRecorder.GetDetectedComponents(); + + var components = detectedComponents + .Select(d => d.Component) + .Cast() + .OrderBy(c => c.Name) + .Select(c => $"{c.Name}/{c.Version}"); + components.Should().Equal(".NET SDK/6.0.789"); } private async Task<(IndividualDetectorScanResult ScanResult, IComponentRecorder ComponentRecorder)> ExecuteDetectorAndGetBinLogAsync( diff --git a/test/Microsoft.ComponentDetection.Detectors.Tests/Utilities/TemporaryFile.cs b/test/Microsoft.ComponentDetection.Detectors.Tests/Utilities/TemporaryFile.cs new file mode 100644 index 000000000..f19776cba --- /dev/null +++ b/test/Microsoft.ComponentDetection.Detectors.Tests/Utilities/TemporaryFile.cs @@ -0,0 +1,41 @@ +namespace Microsoft.ComponentDetection.Detectors.Tests.Utilities; + +using System; +using System.IO; + +public sealed class TemporaryFile : IDisposable +{ + static TemporaryFile() + { + TemporaryDirectory = Path.Combine(Path.GetDirectoryName(typeof(TemporaryFile).Assembly.Location), "temporary-files"); + Directory.CreateDirectory(TemporaryDirectory); + } + + // Creates a temporary file in the test directory with the optional given file extension. The test/debug directory + // is used to avoid polluting the user's temp directory and so that a `git clean` operation will remove any + // remaining files. + public TemporaryFile(string extension = null) + { + if (extension is not null && !extension.StartsWith(".")) + { + throw new ArgumentException("Extension must start with a period.", nameof(extension)); + } + + this.FilePath = Path.Combine(TemporaryDirectory, $"{Guid.NewGuid():d}{extension}"); + } + + private static string TemporaryDirectory { get; } + + public string FilePath { get; } + + public void Dispose() + { + try + { + File.Delete(this.FilePath); + } + catch + { + } + } +} From 3ff2a8fe64cd82fb64fe693087011ca1a5b510d2 Mon Sep 17 00:00:00 2001 From: "Brett V. Forsgren" Date: Tue, 24 Sep 2024 13:57:56 -0600 Subject: [PATCH 09/10] track project dependencies by evaluation id --- .../NuGetMSBuildBinaryLogComponentDetector.cs | 48 +++++++++----- ...tMSBuildBinaryLogComponentDetectorTests.cs | 66 +++++++++++++++++++ 2 files changed, 99 insertions(+), 15 deletions(-) diff --git a/src/Microsoft.ComponentDetection.Detectors/nuget/NuGetMSBuildBinaryLogComponentDetector.cs b/src/Microsoft.ComponentDetection.Detectors/nuget/NuGetMSBuildBinaryLogComponentDetector.cs index 0bccb1f16..d54504274 100644 --- a/src/Microsoft.ComponentDetection.Detectors/nuget/NuGetMSBuildBinaryLogComponentDetector.cs +++ b/src/Microsoft.ComponentDetection.Detectors/nuget/NuGetMSBuildBinaryLogComponentDetector.cs @@ -64,7 +64,7 @@ public NuGetMSBuildBinaryLogComponentDetector( public override int Version { get; } = 1; - private static void ProcessResolvedPackageReference(Dictionary> topLevelDependencies, Dictionary>> projectResolvedDependencies, NamedNode node) + private static void ProcessResolvedPackageReference(Dictionary> topLevelDependencies, Dictionary>>> projectResolvedDependencies, NamedNode node) { var doRemoveOperation = node is RemoveItem; var doAddOperation = node is AddItem; @@ -116,10 +116,16 @@ private static void ProcessResolvedPackageReference(Dictionary>> projectResolvedDependencies, Property node) + private static void ProcessProjectProperty(Dictionary>>> projectResolvedDependencies, Property node) { if (PropertyPackageNames.TryGetValue(node.Name, out var packageName)) { string projectFile; + int evaluationId; var projectEvaluation = node.GetNearestParent(); if (projectEvaluation is not null) { // `.binlog` files store properties in a `ProjectEvaluation` projectFile = projectEvaluation.ProjectFile; + evaluationId = projectEvaluation.Id; } else { // `.buildlog` files store proeprties in `Project` var project = node.GetNearestParent(); projectFile = project?.ProjectFile; + evaluationId = project?.EvaluationId ?? 0; } if (projectFile is not null) @@ -167,10 +176,16 @@ private static void ProcessProjectProperty(Dictionary>(StringComparer.OrdinalIgnoreCase); - var projectResolvedDependencies = new Dictionary>>(StringComparer.OrdinalIgnoreCase); + var projectResolvedDependencies = new Dictionary>>>(StringComparer.OrdinalIgnoreCase); buildRoot.VisitAllChildren(node => { switch (node) @@ -260,18 +275,21 @@ private void ProcessBinLog(Build buildRoot, ISingleFileComponentRecorder compone } var projectDependencies = projectResolvedDependencies[projectPath]; - foreach (var (packageName, packageVersions) in projectDependencies.OrderBy(p => p.Key)) + foreach (var (packageName, packageVersionsPerEvaluationid) in projectDependencies.OrderBy(p => p.Key)) { - foreach (var packageVersion in packageVersions.OrderBy(v => v)) + foreach (var packageVersions in packageVersionsPerEvaluationid.OrderBy(p => p.Key).Select(kvp => kvp.Value)) { - var key = $"{packageName}/{packageVersion}"; - if (!projectsPerPackage.TryGetValue(key, out var projectPaths)) + foreach (var packageVersion in packageVersions.OrderBy(v => v)) { - projectPaths = new HashSet(StringComparer.OrdinalIgnoreCase); - projectsPerPackage[key] = projectPaths; - } + var key = $"{packageName}/{packageVersion}"; + if (!projectsPerPackage.TryGetValue(key, out var projectPaths)) + { + projectPaths = new HashSet(StringComparer.OrdinalIgnoreCase); + projectsPerPackage[key] = projectPaths; + } - projectPaths.Add(projectPath); + projectPaths.Add(projectPath); + } } } } diff --git a/test/Microsoft.ComponentDetection.Detectors.Tests/NuGetMSBuildBinaryLogComponentDetectorTests.cs b/test/Microsoft.ComponentDetection.Detectors.Tests/NuGetMSBuildBinaryLogComponentDetectorTests.cs index 5da3dbfe7..00696f9f4 100644 --- a/test/Microsoft.ComponentDetection.Detectors.Tests/NuGetMSBuildBinaryLogComponentDetectorTests.cs +++ b/test/Microsoft.ComponentDetection.Detectors.Tests/NuGetMSBuildBinaryLogComponentDetectorTests.cs @@ -271,6 +271,72 @@ public async Task PackagesAreReportedWhenConditionedOnTargetFramework() packages.Should().Equal("NETStandard.Library/2.0.3", "Some.Package/1.2.3", "Some.Package/4.5.6"); } + [TestMethod] + public async Task PackagesAreReportedWhenConflictResolutionRemovesPackages() + { + // This comes from a real-world scenario where the project file looks like this: + // + // + // + // net472;net8.0 + // + // + // + // + // + // + // During a build, the project is evantually evaluated twice: once for `net472` and once for `net8.0`. In the + // `net472` scenario, `System.Text.Json/6.0.0` is added to the output, but in the `net8.0` scenario, that + // package is added but then removed by the conflict resolution logic in the SDK. We need to ensure that the + // `RemoveItem` from the `net8.0` evaluation doesn't affect the `net472` evaluation. The end result is that we + // _should_ report the package because it was used in at least one project evaluation. + // + // To make this easy to test, custom targets have been added to the test project below that do exactly what the + // SDK does, just without an expernsive build invocation; remove an item from the item group, but only for one + // of the target frameworks. The end result is a binary log that mimics the shape of the real-world example + // given above. + var (scanResult, componentRecorder) = await this.ExecuteDetectorAndGetBinLogAsync( + projectContents: $@" + + + netstandard2.0;{MSBuildTestUtilities.TestTargetFramework} + + + + + + + + + + + + + + + ", + targetName: "TEST_GenerateBuildDependencyFileForTargetFrameworks", + mockedPackages: new[] + { + ("NETStandard.Library", "2.0.3", "netstandard2.0", ""), + ("Some.Package", "1.2.3", "netstandard2.0", null), + }); + + var detectedComponents = componentRecorder.GetDetectedComponents(); + + var packages = detectedComponents + .Where(d => d.FilePaths.Any(p => p.Replace("\\", "/").EndsWith("/project.csproj"))) + .Select(d => d.Component) + .Cast() + .Where(c => c.Name != ".NET SDK") // dealt with in another test because the SDK version will change regularly + .OrderBy(c => c.Name).ThenBy(c => c.Version) + .Select(c => $"{c.Name}/{c.Version}"); + packages.Should().Equal("NETStandard.Library/2.0.3", "Some.Package/1.2.3"); + } + [TestMethod] public async Task PackagesImplicitlyAddedBySdkDuringPublishAreAdded() { From 0a38a99a55a209d780aa4d4eab80294d3a0b2bcf Mon Sep 17 00:00:00 2001 From: "Brett V. Forsgren" Date: Tue, 24 Sep 2024 14:23:51 -0600 Subject: [PATCH 10/10] introduce types to make the code easier to read --- .../NuGetMSBuildBinaryLogComponentDetector.cs | 185 +++++++++++------- 1 file changed, 112 insertions(+), 73 deletions(-) diff --git a/src/Microsoft.ComponentDetection.Detectors/nuget/NuGetMSBuildBinaryLogComponentDetector.cs b/src/Microsoft.ComponentDetection.Detectors/nuget/NuGetMSBuildBinaryLogComponentDetector.cs index d54504274..1f49a11be 100644 --- a/src/Microsoft.ComponentDetection.Detectors/nuget/NuGetMSBuildBinaryLogComponentDetector.cs +++ b/src/Microsoft.ComponentDetection.Detectors/nuget/NuGetMSBuildBinaryLogComponentDetector.cs @@ -38,8 +38,8 @@ public class NuGetMSBuildBinaryLogComponentDetector : FileComponentDetector ["ResolvedIjwHostPack"] = ("NuGetPackageId", "NuGetPackageVersion"), }; - // the items listed below represent top-level property names that correspond to well-known packages - private static readonly Dictionary PropertyPackageNames = new Dictionary(StringComparer.OrdinalIgnoreCase) + // the items listed below represent top-level property names that correspond to well-known components + private static readonly Dictionary ComponentPropertyNames = new Dictionary(StringComparer.OrdinalIgnoreCase) { ["NETCoreSdkVersion"] = ".NET SDK", }; @@ -64,7 +64,7 @@ public NuGetMSBuildBinaryLogComponentDetector( public override int Version { get; } = 1; - private static void ProcessResolvedPackageReference(Dictionary> topLevelDependencies, Dictionary>>> projectResolvedDependencies, NamedNode node) + private static void ProcessResolvedComponentReference(ProjectPathToTopLevelComponents topLevelComponents, ProjectPathToComponents projectResolvedComponents, NamedNode node) { var doRemoveOperation = node is RemoveItem; var doAddOperation = node is AddItem; @@ -75,21 +75,17 @@ private static void ProcessResolvedPackageReference(Dictionary()) { - var packageName = child.Name; - if (!topLevelDependencies.TryGetValue(projectEvaluation.ProjectFile, out var topLevel)) - { - topLevel = new(StringComparer.OrdinalIgnoreCase); - topLevelDependencies[projectEvaluation.ProjectFile] = topLevel; - } + var componentName = child.Name; + var topLevel = topLevelComponents.GetComponentNames(projectEvaluation.ProjectFile); if (doRemoveOperation) { - topLevel.Remove(packageName); + topLevel.Remove(componentName); } if (doAddOperation) { - topLevel.Add(packageName); + topLevel.Add(componentName); } } } @@ -103,39 +99,25 @@ private static void ProcessResolvedPackageReference(Dictionary()) { - var packageName = GetChildMetadataValue(child, nameMetadata); - var packageVersion = GetChildMetadataValue(child, versionMetadata); - if (packageName is not null && packageVersion is not null) + var componentName = GetChildMetadataValue(child, nameMetadata); + var componentVersion = GetChildMetadataValue(child, versionMetadata); + if (componentName is not null && componentVersion is not null) { var project = originalProject; while (project is not null) { - if (!projectResolvedDependencies.TryGetValue(project.ProjectFile, out var projectDependencies)) - { - projectDependencies = new(StringComparer.OrdinalIgnoreCase); - projectResolvedDependencies[project.ProjectFile] = projectDependencies; - } - - if (!projectDependencies.TryGetValue(packageName, out var packageVersionsPerEvaluation)) - { - packageVersionsPerEvaluation = new(); - projectDependencies[packageName] = packageVersionsPerEvaluation; - } - - if (!packageVersionsPerEvaluation.TryGetValue(project.EvaluationId, out var packageVersions)) - { - packageVersions = new(StringComparer.OrdinalIgnoreCase); - packageVersionsPerEvaluation[project.EvaluationId] = packageVersions; - } + var components = projectResolvedComponents.GetComponents(project.ProjectFile); + var evaluatedVersions = components.GetEvaluatedVersions(componentName); + var componentVersions = evaluatedVersions.GetComponentVersions(project.EvaluationId); if (doRemoveOperation) { - packageVersions.Remove(packageVersion); + componentVersions.Remove(componentVersion); } if (doAddOperation) { - packageVersions.Add(packageVersion); + componentVersions.Add(componentVersion); } project = project.GetNearestParent(); @@ -146,9 +128,9 @@ private static void ProcessResolvedPackageReference(Dictionary>>> projectResolvedDependencies, Property node) + private static void ProcessProjectProperty(ProjectPathToComponents projectResolvedComponents, Property node) { - if (PropertyPackageNames.TryGetValue(node.Name, out var packageName)) + if (ComponentPropertyNames.TryGetValue(node.Name, out var packageName)) { string projectFile; int evaluationId; @@ -169,26 +151,12 @@ private static void ProcessProjectProperty(Dictionary>(StringComparer.OrdinalIgnoreCase); - var projectResolvedDependencies = new Dictionary>>>(StringComparer.OrdinalIgnoreCase); + var projectTopLevelComponents = new ProjectPathToTopLevelComponents(); + var projectResolvedComponents = new ProjectPathToComponents(); buildRoot.VisitAllChildren(node => { switch (node) { case NamedNode namedNode when namedNode is AddItem or RemoveItem: - ProcessResolvedPackageReference(projectTopLevelDependencies, projectResolvedDependencies, namedNode); + ProcessResolvedComponentReference(projectTopLevelComponents, projectResolvedComponents, namedNode); break; case Property property when property.Parent is Folder folder && folder.Name == "Properties": - ProcessProjectProperty(projectResolvedDependencies, property); + ProcessProjectProperty(projectResolvedComponents, property); break; default: break; @@ -265,8 +233,8 @@ private void ProcessBinLog(Build buildRoot, ISingleFileComponentRecorder compone }); // dependencies were resolved per project, we need to re-arrange them to be per package/version - var projectsPerPackage = new Dictionary>(StringComparer.OrdinalIgnoreCase); - foreach (var projectPath in projectResolvedDependencies.Keys.OrderBy(p => p)) + var projectsPerComponent = new Dictionary>(StringComparer.OrdinalIgnoreCase); + foreach (var projectPath in projectResolvedComponents.Keys.OrderBy(p => p)) { if (Path.GetExtension(projectPath).Equals(".sln", StringComparison.OrdinalIgnoreCase)) { @@ -274,18 +242,18 @@ private void ProcessBinLog(Build buildRoot, ISingleFileComponentRecorder compone continue; } - var projectDependencies = projectResolvedDependencies[projectPath]; - foreach (var (packageName, packageVersionsPerEvaluationid) in projectDependencies.OrderBy(p => p.Key)) + var projectComponents = projectResolvedComponents[projectPath]; + foreach (var (componentName, componentVersionsPerEvaluationid) in projectComponents.OrderBy(p => p.Key)) { - foreach (var packageVersions in packageVersionsPerEvaluationid.OrderBy(p => p.Key).Select(kvp => kvp.Value)) + foreach (var componentVersions in componentVersionsPerEvaluationid.OrderBy(p => p.Key).Select(kvp => kvp.Value)) { - foreach (var packageVersion in packageVersions.OrderBy(v => v)) + foreach (var componentVersion in componentVersions.OrderBy(v => v)) { - var key = $"{packageName}/{packageVersion}"; - if (!projectsPerPackage.TryGetValue(key, out var projectPaths)) + var key = $"{componentName}/{componentVersion}"; + if (!projectsPerComponent.TryGetValue(key, out var projectPaths)) { projectPaths = new HashSet(StringComparer.OrdinalIgnoreCase); - projectsPerPackage[key] = projectPaths; + projectsPerComponent[key] = projectPaths; } projectPaths.Add(projectPath); @@ -295,13 +263,13 @@ private void ProcessBinLog(Build buildRoot, ISingleFileComponentRecorder compone } // report it all - foreach (var packageNameAndVersion in projectsPerPackage.Keys.OrderBy(p => p)) + foreach (var componentNameAndVersion in projectsPerComponent.Keys.OrderBy(p => p)) { - var projectPaths = projectsPerPackage[packageNameAndVersion]; - var parts = packageNameAndVersion.Split('/', 2); - var packageName = parts[0]; - var packageVersion = parts[1]; - var component = new NuGetComponent(packageName, packageVersion); + var projectPaths = projectsPerComponent[componentNameAndVersion]; + var parts = componentNameAndVersion.Split('/', 2); + var componentName = parts[0]; + var componentVersion = parts[1]; + var component = new NuGetComponent(componentName, componentVersion); var libraryComponent = new DetectedComponent(component); foreach (var projectPath in projectPaths) { @@ -311,4 +279,75 @@ private void ProcessBinLog(Build buildRoot, ISingleFileComponentRecorder compone componentRecorder.RegisterUsage(libraryComponent); } } + + // To make the above code easier to read, some helper types are added here. Without these, the code above would contain a type of: + // Dictionary>>> + // which isn't very descriptive. + private abstract class KeyedCollection : Dictionary + where TKey : notnull + { + protected KeyedCollection() + : base() + { + } + + protected KeyedCollection(IEqualityComparer comparer) + : base(comparer) + { + } + + protected TValue GetOrAdd(TKey key, Func valueFactory) + { + if (!this.TryGetValue(key, out var value)) + { + value = valueFactory(); + this[key] = value; + } + + return value; + } + } + + // Represents a collection of top-level components for a given project path. + private class ProjectPathToTopLevelComponents : KeyedCollection> + { + public HashSet GetComponentNames(string projectPath) => this.GetOrAdd(projectPath, () => new(StringComparer.OrdinalIgnoreCase)); + } + + // Represents a collection of evaluated components for a given project path. + private class ProjectPathToComponents : KeyedCollection + { + public ProjectPathToComponents() + : base(StringComparer.OrdinalIgnoreCase) + { + } + + public ComponentNameToEvaluatedVersions GetComponents(string projectPath) => this.GetOrAdd(projectPath, () => new()); + } + + // Represents a collection of evaluated components for a given component name. + private class ComponentNameToEvaluatedVersions : KeyedCollection + { + public ComponentNameToEvaluatedVersions() + : base(StringComparer.OrdinalIgnoreCase) + { + } + + public EvaluationIdToComponentVersions GetEvaluatedVersions(string componentName) => this.GetOrAdd(componentName, () => new()); + } + + // Represents a collection of component versions for a given evaluation id. + private class EvaluationIdToComponentVersions : KeyedCollection + { + public ComponentVersions GetComponentVersions(int evaluationId) => this.GetOrAdd(evaluationId, () => new()); + } + + // Represents a collection of version strings. + private class ComponentVersions : HashSet + { + public ComponentVersions() + : base(StringComparer.OrdinalIgnoreCase) + { + } + } }