From 37354d2dbce5e525b2a3f8c04423687d8262ecbe Mon Sep 17 00:00:00 2001 From: Advay Tandon Date: Tue, 25 Jun 2024 22:46:23 -0700 Subject: [PATCH 1/4] wip: most scenarios working, but nuget.exe func test suppressions are still failing --- .../NuGet.Build.Tasks/NuGet.targets | 1 - .../Audit/AuditCheckResult.cs | 6 + .../Audit/AuditChecker.cs | 71 ++++- .../GlobalSuppressions.cs | 2 +- .../NuGetRestoreCommandTest.cs | 295 ++++++++++++++++++ .../MsbuildRestoreTaskTests.cs | 127 ++++++++ .../AuditCheckerTests.cs | 180 ++++++++++- 7 files changed, 656 insertions(+), 26 deletions(-) diff --git a/src/NuGet.Core/NuGet.Build.Tasks/NuGet.targets b/src/NuGet.Core/NuGet.Build.Tasks/NuGet.targets index 5ce46081a56..82aa7b37d61 100644 --- a/src/NuGet.Core/NuGet.Build.Tasks/NuGet.targets +++ b/src/NuGet.Core/NuGet.Build.Tasks/NuGet.targets @@ -1118,7 +1118,6 @@ Copyright (c) .NET Foundation. All rights reserved. ? Packages { get; set; } internal double? DownloadDurationInSeconds { get; set; } internal double? CheckPackagesDurationInSeconds { get; set; } @@ -32,6 +34,8 @@ public record AuditCheckResult private const string AuditVulnerabilitiesSev2Count = "PackagesConfig.Audit.Vulnerability.Severity2.Count"; private const string AuditVulnerabilitiesSev3Count = "PackagesConfig.Audit.Vulnerability.Severity3.Count"; private const string AuditVulnerabilitiesInvalidSeverityCount = "PackagesConfig.Audit.Vulnerability.SeverityInvalid.Count"; + private const string AuditSuppressedAdvisoriesTotalWarningsSuppressedCount = "PackagesConfig.Audit.SuppressedAdvisories.TotalWarningsSuppressed.Count"; + private const string AuditSuppressedAdvisoriesDistinctAdvisoriesSuppressedCount = "PackagesConfig.Audit.SuppressedAdvisories.DistinctAdvisoriesSuppressed.Count"; private const string AuditDurationDownload = "PackagesConfig.Audit.Duration.Download"; private const string AuditDurationCheck = "PackagesConfig.Audit.Duration.Check"; private const string SourcesWithVulnerabilitiesCount = "PackagesConfig.Audit.DataSources.Count"; @@ -56,6 +60,8 @@ public void AddMetricsToTelemetry(TelemetryEvent telemetryEvent) telemetryEvent[AuditVulnerabilitiesSev2Count] = Severity2VulnerabilitiesFound; telemetryEvent[AuditVulnerabilitiesSev3Count] = Severity3VulnerabilitiesFound; telemetryEvent[AuditVulnerabilitiesInvalidSeverityCount] = InvalidSeverityVulnerabilitiesFound; + telemetryEvent[AuditSuppressedAdvisoriesTotalWarningsSuppressedCount] = TotalWarningsSuppressedCount; + telemetryEvent[AuditSuppressedAdvisoriesDistinctAdvisoriesSuppressedCount] = DistinctAdvisoriesSuppressedCount; telemetryEvent[AuditVulnerabilitiesCount] = Packages?.Count ?? 0; if (DownloadDurationInSeconds.HasValue) diff --git a/src/NuGet.Core/NuGet.PackageManagement/Audit/AuditChecker.cs b/src/NuGet.Core/NuGet.PackageManagement/Audit/AuditChecker.cs index 90d08d38a63..9563fc74bcd 100644 --- a/src/NuGet.Core/NuGet.PackageManagement/Audit/AuditChecker.cs +++ b/src/NuGet.Core/NuGet.PackageManagement/Audit/AuditChecker.cs @@ -37,12 +37,12 @@ public async Task CheckPackageVulnerabilitiesAsync(IEnumerable // Before fetching vulnerability data, check if any projects are enabled for audit // If there are no settings, then run the audit for all packages bool anyProjectsEnabledForAudit = restoreAuditProperties.Count == 0; - var auditSettings = new Dictionary(restoreAuditProperties.Count); + var auditSettings = new Dictionary(restoreAuditProperties.Count); foreach (var (projectPath, restoreAuditProperty) in restoreAuditProperties) { _ = restoreAuditProperty.TryParseEnableAudit(out bool isAuditEnabled); _ = restoreAuditProperty.TryParseAuditLevel(out PackageVulnerabilitySeverity minimumAuditSeverity); - auditSettings.Add(projectPath, (isAuditEnabled, minimumAuditSeverity)); + auditSettings.Add(projectPath, new ProjectAuditSettings(isAuditEnabled, minimumAuditSeverity, restoreAuditProperty.SuppressedAdvisories)); anyProjectsEnabledForAudit |= isAuditEnabled; } @@ -80,13 +80,14 @@ public async Task CheckPackageVulnerabilitiesAsync(IEnumerable stopwatch.Restart(); Dictionary? packagesWithKnownVulnerabilities = FindPackagesWithKnownVulnerabilities(allVulnerabilityData.KnownVulnerabilities!, packages); - int Sev0Matches = 0, Sev1Matches = 0, Sev2Matches = 0, Sev3Matches = 0, InvalidSevMatches = 0; + int Sev0Matches = 0, Sev1Matches = 0, Sev2Matches = 0, Sev3Matches = 0, InvalidSevMatches = 0, TotalWarningsSuppressedCount = 0, DistinctAdvisoriesSuppressedCount = 0; List packagesWithReportedAdvisories = new(packagesWithKnownVulnerabilities?.Count ?? 0); - IReadOnlyList warnings = packagesWithKnownVulnerabilities is not null ? - CreateWarnings(packagesWithKnownVulnerabilities, auditSettings, ref Sev0Matches, ref Sev1Matches, ref Sev2Matches, ref Sev3Matches, ref InvalidSevMatches, ref packagesWithReportedAdvisories) : - Array.Empty(); + IReadOnlyList warnings = packagesWithKnownVulnerabilities is not null + ? CreateWarnings(packagesWithKnownVulnerabilities, auditSettings, ref Sev0Matches, ref Sev1Matches, ref Sev2Matches, ref Sev3Matches, ref InvalidSevMatches, + ref TotalWarningsSuppressedCount, ref DistinctAdvisoriesSuppressedCount, ref packagesWithReportedAdvisories) + : Array.Empty(); foreach (var warning in warnings.NoAllocEnumerate()) { @@ -103,6 +104,8 @@ public async Task CheckPackageVulnerabilitiesAsync(IEnumerable Severity2VulnerabilitiesFound = Sev2Matches, Severity3VulnerabilitiesFound = Sev3Matches, InvalidSeverityVulnerabilitiesFound = InvalidSevMatches, + TotalWarningsSuppressedCount = TotalWarningsSuppressedCount, + DistinctAdvisoriesSuppressedCount = DistinctAdvisoriesSuppressedCount, Packages = packagesWithReportedAdvisories, DownloadDurationInSeconds = downloadDurationInSeconds, CheckPackagesDurationInSeconds = checkPackagesDurationInSeconds, @@ -195,12 +198,14 @@ static bool IsAnyVulnerabilityDataFound(IReadOnlyList CreateWarnings(Dictionary packagesWithKnownVulnerabilities, - Dictionary auditSettings, + Dictionary auditSettings, ref int Sev0Matches, ref int Sev1Matches, ref int Sev2Matches, ref int Sev3Matches, ref int InvalidSevMatches, + ref int TotalWarningsSuppressedCount, + ref int DistinctAdvisoriesSuppressedCount, ref List packagesWithReportedAdvisories) { var warnings = new List(); @@ -221,10 +226,15 @@ internal static List CreateWarnings(Dictionary= (int)auditSetting.MinimumSeverity) { + if (CheckIfAdvisoryHasBeenSuppressed(auditSetting?.SuppressedAdvisories, vulnerability.Url.OriginalString, ref TotalWarningsSuppressedCount, ref DistinctAdvisoriesSuppressedCount)) + { + continue; + } + isVulnerabilityReported = true; if (!counted) { @@ -336,6 +346,24 @@ internal static (string severityLabel, NuGetLogCode code) GetSeverityLabelAndCod } } + private static bool CheckIfAdvisoryHasBeenSuppressed(Dictionary? suppressedAdvisories, string advisoryUrl, ref int totalWarningsSuppressedCount, ref int distinctAdvisoriesSuppressedCount) + { + if (suppressedAdvisories?.TryGetValue(advisoryUrl, out bool advisoryUsed) == true) + { + totalWarningsSuppressedCount++; + + if (!advisoryUsed) + { + suppressedAdvisories[advisoryUrl] = true; + distinctAdvisoriesSuppressedCount++; + } + + return true; + } + + return false; + } + internal class PackageAuditInfo { public PackageIdentity Identity { get; } @@ -351,5 +379,32 @@ public PackageAuditInfo(PackageIdentity identity, IList projects) Projects = projects; } } + + internal class ProjectAuditSettings + { + public bool IsAuditEnabled { get; } + + public PackageVulnerabilitySeverity MinimumSeverity { get; } + + public Dictionary? SuppressedAdvisories { get; } + + public ProjectAuditSettings(bool enableAudit, + PackageVulnerabilitySeverity auditLevel, + HashSet? suppressedAdvisories) + { + IsAuditEnabled = enableAudit; + MinimumSeverity = auditLevel; + + if (suppressedAdvisories != null) + { + SuppressedAdvisories = new Dictionary(suppressedAdvisories.Count); + + foreach (string advisory in suppressedAdvisories) + { + SuppressedAdvisories.Add(advisory, false); + } + } + } + } } } diff --git a/src/NuGet.Core/NuGet.PackageManagement/GlobalSuppressions.cs b/src/NuGet.Core/NuGet.PackageManagement/GlobalSuppressions.cs index ca5526dc427..3720be6232d 100644 --- a/src/NuGet.Core/NuGet.PackageManagement/GlobalSuppressions.cs +++ b/src/NuGet.Core/NuGet.PackageManagement/GlobalSuppressions.cs @@ -129,6 +129,6 @@ [assembly: SuppressMessage("Performance", "CA1822:Mark members as static", Justification = "", Scope = "member", Target = "~M:NuGet.PackageManagement.PackagesConfigContentHashProvider.TryGetNupkgMetadata(System.String)~NuGet.PackageManagement.PackagesConfigContentHashProvider.Result")] [assembly: SuppressMessage("Performance", "CA1822:Mark members as static", Justification = "", Scope = "member", Target = "~M:NuGet.ProjectManagement.DefaultProjectServices.GetGlobalService``1~``0")] [assembly: SuppressMessage("Globalization", "CA1305:Specify IFormatProvider", Justification = "", Scope = "member", Target = "~M:NuGet.PackageManagement.AuditChecker.CheckPackageVulnerabilitiesAsync(System.Collections.Generic.IEnumerable{NuGet.PackageManagement.PackageRestoreData},System.Collections.Generic.Dictionary{System.String,NuGet.ProjectModel.RestoreAuditProperties},System.Threading.CancellationToken)~System.Threading.Tasks.Task{NuGet.PackageManagement.AuditCheckResult}")] -[assembly: SuppressMessage("Globalization", "CA1305:Specify IFormatProvider", Justification = "", Scope = "member", Target = "~M:NuGet.PackageManagement.AuditChecker.CreateWarnings(System.Collections.Generic.Dictionary{NuGet.Packaging.Core.PackageIdentity,NuGet.PackageManagement.AuditChecker.PackageAuditInfo},System.Collections.Generic.Dictionary{System.String,System.ValueTuple{System.Boolean,NuGet.Protocol.PackageVulnerabilitySeverity}},System.Int32@,System.Int32@,System.Int32@,System.Int32@,System.Int32@,System.Collections.Generic.List{NuGet.Packaging.Core.PackageIdentity}@)~System.Collections.Generic.List{NuGet.Common.LogMessage}")] +[assembly: SuppressMessage("Globalization", "CA1305:Specify IFormatProvider", Justification = "", Scope = "member", Target = "~M:NuGet.PackageManagement.AuditChecker.CreateWarnings(System.Collections.Generic.Dictionary{NuGet.Packaging.Core.PackageIdentity,NuGet.PackageManagement.AuditChecker.PackageAuditInfo},System.Collections.Generic.Dictionary{System.String,NuGet.PackageManagement.AuditChecker.ProjectAuditSettings},System.Int32@,System.Int32@,System.Int32@,System.Int32@,System.Int32@,System.Int32@,System.Int32@,System.Collections.Generic.List{NuGet.Packaging.Core.PackageIdentity}@)~System.Collections.Generic.List{NuGet.Common.LogMessage}")] [assembly: SuppressMessage("Globalization", "CA1305:Specify IFormatProvider", Justification = "", Scope = "member", Target = "~M:NuGet.PackageManagement.NuGetPackageManager.ExecuteNuGetProjectActionsAsync(NuGet.ProjectManagement.NuGetProject,System.Collections.Generic.IEnumerable{NuGet.PackageManagement.NuGetProjectAction},NuGet.ProjectManagement.INuGetProjectContext,NuGet.Protocol.Core.Types.PackageDownloadContext,System.Threading.CancellationToken)~System.Threading.Tasks.Task")] [assembly: SuppressMessage("Globalization", "CA1305:Specify IFormatProvider", Justification = "", Scope = "member", Target = "~M:NuGet.PackageManagement.ResolverGather.GatherAsync(System.Threading.CancellationToken)~System.Threading.Tasks.Task{System.Collections.Generic.HashSet{NuGet.Protocol.Core.Types.SourcePackageDependencyInfo}}")] diff --git a/test/NuGet.Clients.Tests/NuGet.CommandLine.Test/NuGetRestoreCommandTest.cs b/test/NuGet.Clients.Tests/NuGet.CommandLine.Test/NuGetRestoreCommandTest.cs index d697fdf335c..cfd9683201e 100644 --- a/test/NuGet.Clients.Tests/NuGet.CommandLine.Test/NuGetRestoreCommandTest.cs +++ b/test/NuGet.Clients.Tests/NuGet.CommandLine.Test/NuGetRestoreCommandTest.cs @@ -3646,6 +3646,301 @@ public void RestoreCommand_WithMultipleProjectsInSameDirectory_RaisesAppropriate r.AllOutput.Should().Contain($"a known critical severity vulnerability", Exactly.Twice()); } + // Not working as expected. Nothing is being suppressed + // add breakpoint, extract solution directory. Run nuget.exe restore on it separately and try to debug it that way to see what's wrong. + [SkipMono()] + public void RestoreCommand_WithPackagesConfigProject_PackageWithVulnerabilities_WithSuppressedAdvisories_SuppressesExpectedVulnerabilities() + { + // Arrange + var nugetexe = Util.GetNuGetExePath(); + using var pathContext = new SimpleTestPathContext(); + + string advisoryUrl1 = "https://contoso.com/advisories/1"; + string advisoryUrl2 = "https://contoso.com/advisories/2"; + + using var mockServer = new FileSystemBackedV3MockServer(pathContext.PackageSource, sourceReportsVulnerabilities: true); + + mockServer.Vulnerabilities.Add( + "packageA", + new List<(Uri, PackageVulnerabilitySeverity, VersionRange)> { + (new Uri(advisoryUrl1), PackageVulnerabilitySeverity.High, VersionRange.Parse("[1.0.0, 3.0.0)")) + }); + mockServer.Vulnerabilities.Add( + "packageB", + new List<(Uri, PackageVulnerabilitySeverity, VersionRange)> { + (new Uri(advisoryUrl2), PackageVulnerabilitySeverity.Critical, VersionRange.Parse("[1.0.0, 3.0.0)")) + }); + pathContext.Settings.RemoveSource("source"); + pathContext.Settings.AddSource("source", mockServer.ServiceIndexUri); + + Util.CreateTestPackage("packageA", "1.1.0", pathContext.PackageSource); + Util.CreateTestPackage("packageB", "2.1.0", pathContext.PackageSource); + + var projectA = new SimpleTestProjectContext( + "projectA", + ProjectStyle.PackagesConfig, + pathContext.SolutionRoot); + projectA.Properties.Add("NuGetAudit", "true"); + projectA.Properties.Add("NuGetAuditLevel", "low"); + + Util.CreateFile(Path.GetDirectoryName(projectA.ProjectPath), "packages.config", +@" + + +"); + + // suppress the vulnerability on package A for project A + var xmlA = projectA.GetXML(); + ProjectFileUtils.AddItem( + xmlA, + name: "NuGetAuditSuppress", + identity: advisoryUrl1, + framework: NuGetFramework.AnyFramework, + properties: new Dictionary(), + attributes: new Dictionary()); + xmlA.Save(projectA.ProjectPath); + + mockServer.Start(); + + // Act + var r = CommandRunner.Run( + nugetexe, + pathContext.WorkingDirectory, + $"restore {projectA.ProjectPath}"); + + mockServer.Stop(); + + // Assert + r.Success.Should().BeTrue(because: r.AllOutput); + + var packageFileA1 = Path.Combine(pathContext.SolutionRoot, "packages", "packageA.1.1.0", "packageA.1.1.0.nupkg"); + var packageFileB1 = Path.Combine(pathContext.SolutionRoot, "packages", "packageB.2.1.0", "packageB.2.1.0.nupkg"); + File.Exists(packageFileA1).Should().BeTrue(); + File.Exists(packageFileB1).Should().BeTrue(); + + r.AllOutput.Should().NotContain($"Package 'packageA' 1.1.0 has a known high severity vulnerability"); // suppressed + r.AllOutput.Should().Contain($"Package 'packageB' 2.1.0 has a known critical severity vulnerability"); + } + + [SkipMono()] + public void RestoreCommand_WithPackagesConfigProject_PackageWithVulnerabilities_WithSuppressedAdvisories_SuppressesExpectedVulnerabilities_TEST() + { + // Arrange + var nugetexe = Util.GetNuGetExePath(); + using var pathContext = new SimpleTestPathContext(); + + string advisoryUrl1 = "https://contoso.com/advisories/1"; + string advisoryUrl2 = "https://contoso.com/advisories/2"; + + using var mockServer = new FileSystemBackedV3MockServer(pathContext.PackageSource, sourceReportsVulnerabilities: true); + + mockServer.Vulnerabilities.Add( + "packageA", + new List<(Uri, PackageVulnerabilitySeverity, VersionRange)> { + (new Uri(advisoryUrl1), PackageVulnerabilitySeverity.High, VersionRange.Parse("[1.0.0, 3.0.0)")) + }); + mockServer.Vulnerabilities.Add( + "packageB", + new List<(Uri, PackageVulnerabilitySeverity, VersionRange)> { + (new Uri(advisoryUrl2), PackageVulnerabilitySeverity.Critical, VersionRange.Parse("[1.0.0, 3.0.0)")) + }); + pathContext.Settings.RemoveSource("source"); + pathContext.Settings.AddSource("source", mockServer.ServiceIndexUri); + + + + Util.CreateTestPackage("packageA", "1.1.0", pathContext.PackageSource); + Util.CreateTestPackage("packageB", "2.1.0", pathContext.PackageSource); + + var projectA = new SimpleTestProjectContext( + "projectA", + ProjectStyle.PackagesConfig, + pathContext.SolutionRoot); + projectA.Properties.Add("NuGetAudit", "true"); + projectA.Properties.Add("NuGetAuditLevel", "low"); + + Util.CreateFile(Path.GetDirectoryName(projectA.ProjectPath), "packages.config", +@" + + +"); + + // suppress the vulnerability on package A for project A + var xmlA = projectA.GetXML(); + ProjectFileUtils.AddItem( + xmlA, + name: "NuGetAuditSuppress", + identity: advisoryUrl1, + framework: NuGetFramework.AnyFramework, + properties: new Dictionary(), + attributes: new Dictionary()); + xmlA.Save(projectA.ProjectPath); + + mockServer.Start(); + + // Act + var r = CommandRunner.Run( + nugetexe, + pathContext.WorkingDirectory, + $"restore {projectA.ProjectPath}"); + + mockServer.Stop(); + + // Assert + r.Success.Should().BeTrue(because: r.AllOutput); + + var packageFileA1 = Path.Combine(pathContext.SolutionRoot, "packages", "packageA.1.1.0", "packageA.1.1.0.nupkg"); + var packageFileB1 = Path.Combine(pathContext.SolutionRoot, "packages", "packageB.2.1.0", "packageB.2.1.0.nupkg"); + File.Exists(packageFileA1).Should().BeTrue(); + File.Exists(packageFileB1).Should().BeTrue(); + + r.AllOutput.Should().NotContain($"Package 'packageA' 1.1.0 has a known high severity vulnerability"); // suppressed + r.AllOutput.Should().Contain($"Package 'packageB' 2.1.0 has a known critical severity vulnerability"); + } + + // Not working as expected. Nothing is being suppressed + [SkipMono()] + public async void RestoreCommand_WithSolutionWithPackagesConfigProjects_PackageWithVulnerabilities_WithSuppressedAdvisories_SuppressesExpectedVulnerabilities() + { + // Arrange + var nugetexe = Util.GetNuGetExePath(); + using var pathContext = new SimpleTestPathContext(); + + string advisoryUrl1 = "https://contoso.com/advisories/1"; + string advisoryUrl2 = "https://contoso.com/advisories/2"; + + using var mockServer = new FileSystemBackedV3MockServer(pathContext.PackageSource, sourceReportsVulnerabilities: true); + + mockServer.Vulnerabilities.Add( + "packageA", + new List<(Uri, PackageVulnerabilitySeverity, VersionRange)> { + (new Uri(advisoryUrl1), PackageVulnerabilitySeverity.High, VersionRange.Parse("[1.0.0, 3.0.0)")) + }); + mockServer.Vulnerabilities.Add( + "packageB", + new List<(Uri, PackageVulnerabilitySeverity, VersionRange)> { + (new Uri(advisoryUrl2), PackageVulnerabilitySeverity.Critical, VersionRange.Parse("[1.0.0, 3.0.0)")) + }); + pathContext.Settings.RemoveSource("source"); + pathContext.Settings.AddSource("source", mockServer.ServiceIndexUri); + + /* + Util.CreateTestPackage("packageA", "1.1.0", pathContext.PackageSource); + Util.CreateTestPackage("packageA", "1.2.0", pathContext.PackageSource); + Util.CreateTestPackage("packageB", "2.1.0", pathContext.PackageSource); + Util.CreateTestPackage("packageB", "2.2.0", pathContext.PackageSource); + */ + + var packageA1 = new SimpleTestPackageContext() + { + Id = "packageA", + Version = "1.1.0" + }; + var packageA2 = new SimpleTestPackageContext() + { + Id = "packageA", + Version = "1.2.0" + }; + var packageB1 = new SimpleTestPackageContext() + { + Id = "packageB", + Version = "2.1.0" + }; + var packageB2 = new SimpleTestPackageContext() + { + Id = "packageB", + Version = "2.2.0" + }; + + await SimpleTestPackageUtility.CreatePackagesAsync( + pathContext.PackageSource, + packageA1, + packageA2, + packageB1, + packageB2); + + var solution = new SimpleTestSolutionContext(pathContext.SolutionRoot); + + var projectA = new SimpleTestProjectContext( + "projectA", + ProjectStyle.PackagesConfig, + pathContext.SolutionRoot); + projectA.Properties.Add("NuGetAudit", "true"); + projectA.Properties.Add("NuGetAuditLevel", "low"); + + var projectB = new SimpleTestProjectContext( + "projectB", + ProjectStyle.PackagesConfig, + pathContext.SolutionRoot); + projectB.Properties.Add("NuGetAudit", "true"); + projectB.Properties.Add("NuGetAuditLevel", "low"); + + solution.Projects.Add(projectA); + solution.Projects.Add(projectB); + solution.Create(pathContext.SolutionRoot); + + Util.CreateFile(Path.GetDirectoryName(projectA.ProjectPath), "packages.config", +@" + + +"); + + Util.CreateFile(Path.GetDirectoryName(projectB.ProjectPath), "packages.config", +@" + + +"); + + // suppress the vulnerability on package A for project A + var xmlA = projectA.GetXML(); + ProjectFileUtils.AddItem( + xmlA, + name: "NuGetAuditSuppress", + identity: advisoryUrl1, + framework: NuGetFramework.AnyFramework, + properties: new Dictionary(), + attributes: new Dictionary()); + xmlA.Save(projectA.ProjectPath); + + // suppress the vulnerability on package B for project B + var xmlB = projectB.GetXML(); + ProjectFileUtils.AddItem( + xmlB, + name: "NuGetAuditSuppress", + identity: advisoryUrl2, + framework: NuGetFramework.AnyFramework, + properties: new Dictionary(), + attributes: new Dictionary()); + xmlB.Save(projectB.ProjectPath); + + mockServer.Start(); + + // Act + var r = CommandRunner.Run( + nugetexe, + pathContext.WorkingDirectory, + $"restore {solution.SolutionPath}"); + + mockServer.Stop(); + + // Assert + r.Success.Should().BeTrue(because: r.AllOutput); + + var packageFileA1 = Path.Combine(pathContext.SolutionRoot, "packages", "packageA.1.1.0", "packageA.1.1.0.nupkg"); + var packageFileA2 = Path.Combine(pathContext.SolutionRoot, "packages", "packageA.1.2.0", "packageA.1.2.0.nupkg"); + var packageFileB1 = Path.Combine(pathContext.SolutionRoot, "packages", "packageB.2.1.0", "packageB.2.1.0.nupkg"); + var packageFileB2 = Path.Combine(pathContext.SolutionRoot, "packages", "packageB.2.2.0", "packageB.2.2.0.nupkg"); + File.Exists(packageFileA1).Should().BeTrue(); + File.Exists(packageFileA2).Should().BeTrue(); + File.Exists(packageFileB1).Should().BeTrue(); + File.Exists(packageFileB2).Should().BeTrue(); + + r.AllOutput.Should().NotContain($"Package 'packageA' 1.1.0 has a known high severity vulnerability"); // suppressed + r.AllOutput.Should().Contain($"Package 'packageB' 2.1.0 has a known critical severity vulnerability"); + r.AllOutput.Should().Contain($"Package 'packageA' 1.2.0 has a known high severity vulnerability"); + r.AllOutput.Should().NotContain($"Package 'packageB' 2.2.0 has a known critical severity vulnerability"); // suppressed + } + private static byte[] GetResource(string name) { return ResourceTestUtility.GetResourceBytes( diff --git a/test/NuGet.Core.FuncTests/Msbuild.Integration.Test/MsbuildRestoreTaskTests.cs b/test/NuGet.Core.FuncTests/Msbuild.Integration.Test/MsbuildRestoreTaskTests.cs index 1f25fee3c7d..d116ce2c397 100644 --- a/test/NuGet.Core.FuncTests/Msbuild.Integration.Test/MsbuildRestoreTaskTests.cs +++ b/test/NuGet.Core.FuncTests/Msbuild.Integration.Test/MsbuildRestoreTaskTests.cs @@ -1628,5 +1628,132 @@ await SimpleTestPackageUtility.CreatePackagesAsync( replayedOutput.Should().Contain($"a known high severity vulnerability", Exactly.Twice()); replayedOutput.Should().Contain($"a known critical severity vulnerability", Exactly.Twice()); } + + [Fact] + public async Task MsbuildRestore_PackagesConfigProject_PackagesWithVulnerabilities_WithSuppressions_RaisesAppropriateWarnings() + { + // Arrange + var pathContext = new SimpleTestPathContext(); + var advisoryUrl1 = "https://contoso.com/advisories/12345"; + var advisoryUrl2 = "https://contoso.com/advisories/12346"; + + // set up vulnerability server + using var mockServer = new FileSystemBackedV3MockServer(pathContext.PackageSource, sourceReportsVulnerabilities: true); + + mockServer.Vulnerabilities.Add( + "packageA", + new List<(Uri, PackageVulnerabilitySeverity, VersionRange)> { + (new Uri(advisoryUrl1), PackageVulnerabilitySeverity.Critical, VersionRange.Parse("[1.0.0, 3.0.0)")) + }); + mockServer.Vulnerabilities.Add( + "packageB", + new List<(Uri, PackageVulnerabilitySeverity, VersionRange)> { + (new Uri(advisoryUrl2), PackageVulnerabilitySeverity.High, VersionRange.Parse("[1.0.0, 3.0.0)")) + }); + pathContext.Settings.RemoveSource("source"); + pathContext.Settings.AddSource("source", mockServer.ServiceIndexUri); + + // set up solution, projects and packages + var solution = new SimpleTestSolutionContext(pathContext.SolutionRoot); + + var projectA = new SimpleTestProjectContext( + "A", + ProjectStyle.PackagesConfig, + pathContext.SolutionRoot); + projectA.Frameworks.Add(new SimpleTestProjectFrameworkContext(CommonFrameworks.Net472)); + + var projectB = new SimpleTestProjectContext( + "B", + ProjectStyle.PackagesConfig, + pathContext.SolutionRoot); + projectB.Frameworks.Add(new SimpleTestProjectFrameworkContext(CommonFrameworks.Net472)); + + var packageA1 = new SimpleTestPackageContext() + { + Id = "packageA", + Version = "1.1.0" + }; + var packageA2 = new SimpleTestPackageContext() + { + Id = "packageA", + Version = "1.2.0" + }; + var packageB1 = new SimpleTestPackageContext() + { + Id = "packageB", + Version = "2.1.0" + }; + var packageB2 = new SimpleTestPackageContext() + { + Id = "packageB", + Version = "2.2.0" + }; + + solution.Projects.Add(projectA); + solution.Projects.Add(projectB); + solution.Create(pathContext.SolutionRoot); + + using (var writer = new StreamWriter(Path.Combine(Path.GetDirectoryName(projectA.ProjectPath), "packages.config"))) + { + writer.Write( +@" + + +"); + } + + using (var writer = new StreamWriter(Path.Combine(Path.GetDirectoryName(projectB.ProjectPath), "packages.config"))) + { + writer.Write( +@" + + +"); + } + + // suppress the vulnerability on package A for project A + var xmlA = projectA.GetXML(); + ProjectFileUtils.AddItem( + xmlA, + name: "NuGetAuditSuppress", + identity: advisoryUrl1, + framework: NuGetFramework.AnyFramework, + properties: new Dictionary(), + attributes: new Dictionary()); + xmlA.Save(projectA.ProjectPath); + + // suppress the vulnerability on package B for project B + var xmlB = projectB.GetXML(); + ProjectFileUtils.AddItem( + xmlB, + name: "NuGetAuditSuppress", + identity: advisoryUrl2, + framework: NuGetFramework.AnyFramework, + properties: new Dictionary(), + attributes: new Dictionary()); + xmlB.Save(projectB.ProjectPath); + + await SimpleTestPackageUtility.CreatePackagesAsync( + pathContext.PackageSource, + packageA1, + packageA2, + packageB1, + packageB2); + + // Act + mockServer.Start(); + + string args = $"/t:restore {pathContext.SolutionRoot} /p:RestorePackagesConfig=true"; + var result = _msbuildFixture.RunMsBuild(pathContext.WorkingDirectory, args, ignoreExitCode: true, testOutputHelper: _testOutputHelper); + + mockServer.Stop(); + + // Assert + Assert.True(result.ExitCode == 0, result.AllOutput); + Assert.DoesNotContain($"Package 'packageA' 1.1.0 has a known critical severity vulnerability", result.AllOutput); // suppressed + Assert.Contains($"Package 'packageB' 2.1.0 has a known high severity vulnerability", result.AllOutput); + Assert.Contains($"Package 'packageA' 1.2.0 has a known critical severity vulnerability", result.AllOutput); + Assert.DoesNotContain($"Package 'packageB' 2.2.0 has a known high severity vulnerability", result.AllOutput); // suppressed + } } } diff --git a/test/NuGet.Core.Tests/NuGet.PackageManagement.Test/AuditCheckerTests.cs b/test/NuGet.Core.Tests/NuGet.PackageManagement.Test/AuditCheckerTests.cs index 74d29aa45b7..5a4259fe295 100644 --- a/test/NuGet.Core.Tests/NuGet.PackageManagement.Test/AuditCheckerTests.cs +++ b/test/NuGet.Core.Tests/NuGet.PackageManagement.Test/AuditCheckerTests.cs @@ -535,8 +535,8 @@ public void CreateWarnings_WithoutAuditSettings_WithoutProjectsInformation_Retur {packageIdentity, packageAuditInfo } }; - var auditSettings = new Dictionary(); - int Sev0Matches = 0, Sev1Matches = 0, Sev2Matches = 0, Sev3Matches = 0, InvalidSevMatches = 0; + var auditSettings = new Dictionary(); + int Sev0Matches = 0, Sev1Matches = 0, Sev2Matches = 0, Sev3Matches = 0, InvalidSevMatches = 0, TotalWarningsSuppressedCount = 0, DistinctAdvisoriesSuppressedCount = 0; List packagesWithReportedAdvisories = new List(); List warnings = AuditChecker.CreateWarnings(result!, @@ -546,6 +546,8 @@ public void CreateWarnings_WithoutAuditSettings_WithoutProjectsInformation_Retur ref Sev2Matches, ref Sev3Matches, ref InvalidSevMatches, + ref TotalWarningsSuppressedCount, + ref DistinctAdvisoriesSuppressedCount, ref packagesWithReportedAdvisories); warnings.Should().BeEmpty(); @@ -575,8 +577,8 @@ public void CreateWarnings_WithoutAuditSettings_RaisesWarningsForAllProjects() {packageIdentity, packageAuditInfo } }; - var auditSettings = new Dictionary(); - int Sev0Matches = 0, Sev1Matches = 0, Sev2Matches = 0, Sev3Matches = 0, InvalidSevMatches = 0; + var auditSettings = new Dictionary(); + int Sev0Matches = 0, Sev1Matches = 0, Sev2Matches = 0, Sev3Matches = 0, InvalidSevMatches = 0, TotalWarningsSuppressedCount = 0, DistinctAdvisoriesSuppressedCount = 0; List packagesWithReportedAdvisories = new List(); List warnings = AuditChecker.CreateWarnings(result!, @@ -586,6 +588,8 @@ public void CreateWarnings_WithoutAuditSettings_RaisesWarningsForAllProjects() ref Sev2Matches, ref Sev3Matches, ref InvalidSevMatches, + ref TotalWarningsSuppressedCount, + ref DistinctAdvisoriesSuppressedCount, ref packagesWithReportedAdvisories); warnings.Should().HaveCount(2); @@ -631,8 +635,8 @@ public void CreateWarnings_WithoutAuditSettings_RaisesWarningsForAllSeverities() {packageIdentity, packageAuditInfo } }; - var auditSettings = new Dictionary(); - int Sev0Matches = 0, Sev1Matches = 0, Sev2Matches = 0, Sev3Matches = 0, InvalidSevMatches = 0; + var auditSettings = new Dictionary(); + int Sev0Matches = 0, Sev1Matches = 0, Sev2Matches = 0, Sev3Matches = 0, InvalidSevMatches = 0, TotalWarningsSuppressedCount = 0, DistinctAdvisoriesSuppressedCount = 0; List packagesWithReportedAdvisories = new List(); List warnings = AuditChecker.CreateWarnings(result!, @@ -642,6 +646,8 @@ public void CreateWarnings_WithoutAuditSettings_RaisesWarningsForAllSeverities() ref Sev2Matches, ref Sev3Matches, ref InvalidSevMatches, + ref TotalWarningsSuppressedCount, + ref DistinctAdvisoriesSuppressedCount, ref packagesWithReportedAdvisories); warnings.Should().HaveCount(1); @@ -694,8 +700,8 @@ public void CreateWarnings_WithVariousVulnerabilties_CountsSeverityMatchesCorrec {packageC, packageAuditInfoC }, }; - var auditSettings = new Dictionary(); - int Sev0Matches = 0, Sev1Matches = 0, Sev2Matches = 0, Sev3Matches = 0, InvalidSevMatches = 0; + var auditSettings = new Dictionary(); + int Sev0Matches = 0, Sev1Matches = 0, Sev2Matches = 0, Sev3Matches = 0, InvalidSevMatches = 0, TotalWarningsSuppressedCount = 0, DistinctAdvisoriesSuppressedCount = 0; List packagesWithReportedAdvisories = new List(); List warnings = AuditChecker.CreateWarnings(result!, @@ -705,6 +711,8 @@ public void CreateWarnings_WithVariousVulnerabilties_CountsSeverityMatchesCorrec ref Sev2Matches, ref Sev3Matches, ref InvalidSevMatches, + ref TotalWarningsSuppressedCount, + ref DistinctAdvisoriesSuppressedCount, ref packagesWithReportedAdvisories); warnings.Should().HaveCount(11); @@ -745,13 +753,13 @@ public void CreateWarnings_WithAuditSettings_RaisesWarningsForEnabledProjectsOnl {packageIdentity, packageAuditInfo } }; - var auditSettings = new Dictionary() + var auditSettings = new Dictionary() { - { projectPath1 , (true, PackageVulnerabilitySeverity.Moderate) }, - { projectPath2 , (false, PackageVulnerabilitySeverity.Moderate) } + { projectPath1 , new AuditChecker.ProjectAuditSettings(true, PackageVulnerabilitySeverity.Moderate, suppressedAdvisories: null) }, + { projectPath2 , new AuditChecker.ProjectAuditSettings(false, PackageVulnerabilitySeverity.Moderate, suppressedAdvisories: null) } }; - int Sev0Matches = 0, Sev1Matches = 0, Sev2Matches = 0, Sev3Matches = 0, InvalidSevMatches = 0; + int Sev0Matches = 0, Sev1Matches = 0, Sev2Matches = 0, Sev3Matches = 0, InvalidSevMatches = 0, TotalWarningsSuppressedCount = 0, DistinctAdvisoriesSuppressedCount = 0; List packagesWithReportedAdvisories = new List(); List warnings = AuditChecker.CreateWarnings(result!, @@ -761,6 +769,8 @@ public void CreateWarnings_WithAuditSettings_RaisesWarningsForEnabledProjectsOnl ref Sev2Matches, ref Sev3Matches, ref InvalidSevMatches, + ref TotalWarningsSuppressedCount, + ref DistinctAdvisoriesSuppressedCount, ref packagesWithReportedAdvisories); warnings.Should().HaveCount(1); @@ -799,13 +809,13 @@ public void CreateWarnings_WithAuditSettings_RaisesWarningsForProjectsWithMatchi {packageIdentity, packageAuditInfo } }; - var auditSettings = new Dictionary() + var auditSettings = new Dictionary() { - { projectPath1 , (true, PackageVulnerabilitySeverity.Moderate) }, - { projectPath2 , (true, PackageVulnerabilitySeverity.High) } + { projectPath1 , new AuditChecker.ProjectAuditSettings(true, PackageVulnerabilitySeverity.Moderate, suppressedAdvisories: null) }, + { projectPath2 , new AuditChecker.ProjectAuditSettings(true, PackageVulnerabilitySeverity.High, suppressedAdvisories: null) } }; - int Sev0Matches = 0, Sev1Matches = 0, Sev2Matches = 0, Sev3Matches = 0, InvalidSevMatches = 0; + int Sev0Matches = 0, Sev1Matches = 0, Sev2Matches = 0, Sev3Matches = 0, InvalidSevMatches = 0, TotalWarningsSuppressedCount = 0, DistinctAdvisoriesSuppressedCount = 0; List packagesWithReportedAdvisories = new List(); List warnings = AuditChecker.CreateWarnings(result!, @@ -815,6 +825,8 @@ public void CreateWarnings_WithAuditSettings_RaisesWarningsForProjectsWithMatchi ref Sev2Matches, ref Sev3Matches, ref InvalidSevMatches, + ref TotalWarningsSuppressedCount, + ref DistinctAdvisoriesSuppressedCount, ref packagesWithReportedAdvisories); warnings.Should().HaveCount(1); @@ -831,6 +843,8 @@ public void CreateWarnings_WithAuditSettings_RaisesWarningsForProjectsWithMatchi Sev2Matches.Should().Be(0); Sev3Matches.Should().Be(0); InvalidSevMatches.Should().Be(0); + TotalWarningsSuppressedCount.Should().Be(0); + DistinctAdvisoriesSuppressedCount.Should().Be(0); packagesWithReportedAdvisories.Should().HaveCount(1); packagesWithReportedAdvisories[0].Should().Be(packageIdentity); } @@ -927,6 +941,140 @@ public async Task CheckVulnerabiltiesAsync_WithSeverityHigherThanVulnerabilities result.InvalidSeverityVulnerabilitiesFound.Should().Be(0); } + [Fact] + public void CreateWarnings_WithAuditSettings_WithSuppressedAdvisories_SuppressesExpectedVulnerabilities() + { + string cveUrl1 = "https://cve.test/suppressed/1"; + string cveUrl2 = "https://cve.test/suppressed/2"; + + PackageIdentity packageIdentity = new("packageId", new NuGetVersion(2, 0, 0)); + var projectPath = "C:\\solution\\project\\project1.csproj"; + AuditChecker.PackageAuditInfo packageAuditInfo = new(packageIdentity, new string[] { projectPath }); + + packageAuditInfo.Vulnerabilities.Add(new PackageVulnerabilityInfo( + new Uri(cveUrl1), + PackageVulnerabilitySeverity.Moderate, + VersionRange.Parse("[1.0.0, 3.0.0)"))); + packageAuditInfo.Vulnerabilities.Add(new PackageVulnerabilityInfo( + new Uri(cveUrl2), + PackageVulnerabilitySeverity.Moderate, + VersionRange.Parse("[1.0.0, 3.0.0)"))); + + var suppressedAdvisories = new HashSet { cveUrl1 }; // suppress one of the two advisories + + Dictionary result = new() + { + {packageIdentity, packageAuditInfo } + }; + + var auditSettings = new Dictionary() + { + { projectPath , new AuditChecker.ProjectAuditSettings(true, PackageVulnerabilitySeverity.Moderate, suppressedAdvisories) } + }; + + int Sev0Matches = 0, Sev1Matches = 0, Sev2Matches = 0, Sev3Matches = 0, InvalidSevMatches = 0, TotalWarningsSuppressedCount = 0, DistinctAdvisoriesSuppressedCount = 0; + List packagesWithReportedAdvisories = new List(); + + List warnings = AuditChecker.CreateWarnings(result!, + auditSettings, + ref Sev0Matches, + ref Sev1Matches, + ref Sev2Matches, + ref Sev3Matches, + ref InvalidSevMatches, + ref TotalWarningsSuppressedCount, + ref DistinctAdvisoriesSuppressedCount, + ref packagesWithReportedAdvisories); + + warnings.Should().HaveCount(1); + warnings.Any(m => m.Message.Contains(cveUrl1)).Should().BeFalse(); // cveUrl1 should be suppressed + warnings.Any(m => m.Message.Contains(cveUrl2)).Should().BeTrue(); // cveUrl2 should not be suppressed + + Sev0Matches.Should().Be(0); + Sev1Matches.Should().Be(1); + Sev2Matches.Should().Be(0); + Sev3Matches.Should().Be(0); + InvalidSevMatches.Should().Be(0); + TotalWarningsSuppressedCount.Should().Be(1); + DistinctAdvisoriesSuppressedCount.Should().Be(1); + packagesWithReportedAdvisories.Should().HaveCount(1); + packagesWithReportedAdvisories[0].Should().Be(packageIdentity); + } + + [Fact] + public void CreateWarnings_WithAuditSettings_WithSuppressedAdvisories_CountsSuppressedAdvisoriesCorrectly() + { + string cveUrl1 = "https://cve.test/suppressed/1"; + string cveUrl2 = "https://cve.test/suppressed/2"; + string cveUrl3 = "https://cve.test/suppressed/3"; + + var projectPath = "C:\\solution\\project\\project1.csproj"; + + PackageIdentity packageIdentityA = new("packageA", new NuGetVersion(2, 0, 0)); + AuditChecker.PackageAuditInfo packageAuditInfoA = new(packageIdentityA, new string[] { projectPath }); + packageAuditInfoA.Vulnerabilities.Add(new PackageVulnerabilityInfo( + new Uri(cveUrl1), // this will be suppressed + PackageVulnerabilitySeverity.Moderate, + VersionRange.Parse("[1.0.0, 3.0.0)"))); + packageAuditInfoA.Vulnerabilities.Add(new PackageVulnerabilityInfo( + new Uri(cveUrl2), // this will be suppressed + PackageVulnerabilitySeverity.Moderate, + VersionRange.Parse("[1.0.0, 3.0.0)"))); + + PackageIdentity packageIdentityB = new("packageB", new NuGetVersion(2, 0, 0)); + AuditChecker.PackageAuditInfo packageAuditInfoB = new(packageIdentityB, new string[] { projectPath }); + packageAuditInfoB.Vulnerabilities.Add(new PackageVulnerabilityInfo( + new Uri(cveUrl1), // this will be suppressed + PackageVulnerabilitySeverity.Moderate, + VersionRange.Parse("[1.0.0, 3.0.0)"))); + packageAuditInfoB.Vulnerabilities.Add(new PackageVulnerabilityInfo( + new Uri(cveUrl3), // this will not be suppressed + PackageVulnerabilitySeverity.Moderate, + VersionRange.Parse("[1.0.0, 3.0.0)"))); + + var suppressedAdvisories = new HashSet { cveUrl1, cveUrl2 }; + + Dictionary result = new() + { + { packageIdentityA, packageAuditInfoA }, + { packageIdentityB, packageAuditInfoB } + }; + + var auditSettings = new Dictionary() + { + { projectPath , new AuditChecker.ProjectAuditSettings(true, PackageVulnerabilitySeverity.Moderate, suppressedAdvisories) } + }; + + int Sev0Matches = 0, Sev1Matches = 0, Sev2Matches = 0, Sev3Matches = 0, InvalidSevMatches = 0, TotalWarningsSuppressedCount = 0, DistinctAdvisoriesSuppressedCount = 0; + List packagesWithReportedAdvisories = new List(); + + List warnings = AuditChecker.CreateWarnings(result!, + auditSettings, + ref Sev0Matches, + ref Sev1Matches, + ref Sev2Matches, + ref Sev3Matches, + ref InvalidSevMatches, + ref TotalWarningsSuppressedCount, + ref DistinctAdvisoriesSuppressedCount, + ref packagesWithReportedAdvisories); + + warnings.Should().HaveCount(1); + warnings.Any(m => m.Message.Contains(cveUrl1)).Should().BeFalse(); + warnings.Any(m => m.Message.Contains(cveUrl2)).Should().BeFalse(); + warnings.Any(m => m.Message.Contains(cveUrl3)).Should().BeTrue(); + + Sev0Matches.Should().Be(0); + Sev1Matches.Should().Be(1); + Sev2Matches.Should().Be(0); + Sev3Matches.Should().Be(0); + InvalidSevMatches.Should().Be(0); + TotalWarningsSuppressedCount.Should().Be(3); + DistinctAdvisoriesSuppressedCount.Should().Be(2); + packagesWithReportedAdvisories.Should().HaveCount(1); + packagesWithReportedAdvisories[0].Should().Be(packageIdentityB); + } + // Setup a test bed with multiple sources, 1 source with vulnerabilities, 1 vulnerable package wih a moderate vulnerability. private static void SetupPrerequisites(out AuditChecker auditChecker, out string projectPath, out List packages) { From 6af6f928ba8a42f6138a742e5428b80f53ea6d3c Mon Sep 17 00:00:00 2001 From: Advay Tandon Date: Wed, 26 Jun 2024 16:11:54 -0700 Subject: [PATCH 2/4] fixed up test formatting --- .../Commands/RestoreCommandTests.cs | 98 ++++++++ .../NuGetRestoreCommandTest.cs | 219 +----------------- .../MsbuildRestoreTaskTests.cs | 53 +---- 3 files changed, 117 insertions(+), 253 deletions(-) diff --git a/test/NuGet.Clients.FuncTests/NuGet.CommandLine.FuncTest/Commands/RestoreCommandTests.cs b/test/NuGet.Clients.FuncTests/NuGet.CommandLine.FuncTest/Commands/RestoreCommandTests.cs index 2298483bd5c..a1a1387adcf 100644 --- a/test/NuGet.Clients.FuncTests/NuGet.CommandLine.FuncTest/Commands/RestoreCommandTests.cs +++ b/test/NuGet.Clients.FuncTests/NuGet.CommandLine.FuncTest/Commands/RestoreCommandTests.cs @@ -1,6 +1,7 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. +using System; using System.Collections.Generic; using System.IO; using System.Linq; @@ -16,6 +17,8 @@ using NuGet.ProjectModel; using NuGet.Protocol; using NuGet.Test.Utility; +using NuGet.Versioning; +using Test.Utility; using Xunit; using Xunit.Abstractions; @@ -1275,6 +1278,101 @@ public async Task Restore_WithHttpSourceAndFalseAllowInsecureConnections_WarnsCo } } + [Fact] + public async Task Restore_WithPackagesConfigProject_PackageWithVulnerabilities_WithSuppressedAdvisories_SuppressesExpectedVulnerabilities() + { + // Arrange + var pathContext = new SimpleTestPathContext(); + var advisoryUrl1 = "https://contoso.com/advisories/12345"; + var advisoryUrl2 = "https://contoso.com/advisories/12346"; + + // set up vulnerability server + using var mockServer = new FileSystemBackedV3MockServer(pathContext.PackageSource, sourceReportsVulnerabilities: true); + + mockServer.Vulnerabilities.Add( + "packageA", + new List<(Uri, PackageVulnerabilitySeverity, VersionRange)> { + (new Uri(advisoryUrl1), PackageVulnerabilitySeverity.Critical, VersionRange.Parse("[1.0.0, 3.0.0)")) + }); + mockServer.Vulnerabilities.Add( + "packageB", + new List<(Uri, PackageVulnerabilitySeverity, VersionRange)> { + (new Uri(advisoryUrl2), PackageVulnerabilitySeverity.High, VersionRange.Parse("[1.0.0, 3.0.0)")) + }); + pathContext.Settings.RemoveSource("source"); + pathContext.Settings.AddSource("source", mockServer.ServiceIndexUri); + + // set up solution, projects and packages + var solution = new SimpleTestSolutionContext(pathContext.SolutionRoot); + + var projectA = new SimpleTestProjectContext("A", ProjectStyle.PackagesConfig, pathContext.SolutionRoot); + var projectB = new SimpleTestProjectContext("B", ProjectStyle.PackagesConfig, pathContext.SolutionRoot); + + var packageA1 = new SimpleTestPackageContext() { Id = "packageA", Version = "1.1.0" }; + var packageA2 = new SimpleTestPackageContext() { Id = "packageA", Version = "1.2.0" }; + var packageB1 = new SimpleTestPackageContext() { Id = "packageB", Version = "2.1.0" }; + var packageB2 = new SimpleTestPackageContext() { Id = "packageB", Version = "2.2.0" }; + + solution.Projects.Add(projectA); + solution.Projects.Add(projectB); + solution.Create(pathContext.SolutionRoot); + + using (var writer = new StreamWriter(Path.Combine(Path.GetDirectoryName(projectA.ProjectPath), "packages.config"))) + { + writer.Write( +@" + + +"); + } + using (var writer = new StreamWriter(Path.Combine(Path.GetDirectoryName(projectB.ProjectPath), "packages.config"))) + { + writer.Write( +@" + + +"); + } + + // suppress the vulnerability on package A for project A + var xmlA = projectA.GetXML(); + ProjectFileUtils.AddItem( + xmlA, + name: "NuGetAuditSuppress", + identity: advisoryUrl1, + framework: NuGetFramework.AnyFramework, + properties: new Dictionary(), + attributes: new Dictionary()); + xmlA.Save(projectA.ProjectPath); + + // suppress the vulnerability on package B for project B + var xmlB = projectB.GetXML(); + ProjectFileUtils.AddItem( + xmlB, + name: "NuGetAuditSuppress", + identity: advisoryUrl2, + framework: NuGetFramework.AnyFramework, + properties: new Dictionary(), + attributes: new Dictionary()); + xmlB.Save(projectB.ProjectPath); + + await SimpleTestPackageUtility.CreatePackagesAsync(pathContext.PackageSource, packageA1, packageA2, packageB1, packageB2); + + // Act + mockServer.Start(); + + var result = RunRestore(pathContext, _successExitCode); + + mockServer.Stop(); + + // Assert + result.Success.Should().BeTrue(); + Assert.DoesNotContain($"Package 'packageA' 1.1.0 has a known critical severity vulnerability", result.Output); // suppressed + Assert.Contains($"Package 'packageB' 2.1.0 has a known high severity vulnerability", result.Output); + Assert.Contains($"Package 'packageA' 1.2.0 has a known critical severity vulnerability", result.Output); + Assert.DoesNotContain($"Package 'packageB' 2.2.0 has a known high severity vulnerability", result.Output); // suppressed + } + public static string GetResource(string name) { using (var reader = new StreamReader(Assembly.GetExecutingAssembly().GetManifestResourceStream(name))) diff --git a/test/NuGet.Clients.Tests/NuGet.CommandLine.Test/NuGetRestoreCommandTest.cs b/test/NuGet.Clients.Tests/NuGet.CommandLine.Test/NuGetRestoreCommandTest.cs index cfd9683201e..f45470aeb96 100644 --- a/test/NuGet.Clients.Tests/NuGet.CommandLine.Test/NuGetRestoreCommandTest.cs +++ b/test/NuGet.Clients.Tests/NuGet.CommandLine.Test/NuGetRestoreCommandTest.cs @@ -3646,10 +3646,8 @@ public void RestoreCommand_WithMultipleProjectsInSameDirectory_RaisesAppropriate r.AllOutput.Should().Contain($"a known critical severity vulnerability", Exactly.Twice()); } - // Not working as expected. Nothing is being suppressed - // add breakpoint, extract solution directory. Run nuget.exe restore on it separately and try to debug it that way to see what's wrong. [SkipMono()] - public void RestoreCommand_WithPackagesConfigProject_PackageWithVulnerabilities_WithSuppressedAdvisories_SuppressesExpectedVulnerabilities() + public async void RestoreCommand_WithPackagesConfigProject_PackageWithVulnerabilities_WithSuppressedAdvisories_SuppressesExpectedVulnerabilities() { // Arrange var nugetexe = Util.GetNuGetExePath(); @@ -3673,207 +3671,16 @@ public void RestoreCommand_WithPackagesConfigProject_PackageWithVulnerabilities_ pathContext.Settings.RemoveSource("source"); pathContext.Settings.AddSource("source", mockServer.ServiceIndexUri); - Util.CreateTestPackage("packageA", "1.1.0", pathContext.PackageSource); - Util.CreateTestPackage("packageB", "2.1.0", pathContext.PackageSource); - - var projectA = new SimpleTestProjectContext( - "projectA", - ProjectStyle.PackagesConfig, - pathContext.SolutionRoot); - projectA.Properties.Add("NuGetAudit", "true"); - projectA.Properties.Add("NuGetAuditLevel", "low"); - - Util.CreateFile(Path.GetDirectoryName(projectA.ProjectPath), "packages.config", -@" - - -"); - - // suppress the vulnerability on package A for project A - var xmlA = projectA.GetXML(); - ProjectFileUtils.AddItem( - xmlA, - name: "NuGetAuditSuppress", - identity: advisoryUrl1, - framework: NuGetFramework.AnyFramework, - properties: new Dictionary(), - attributes: new Dictionary()); - xmlA.Save(projectA.ProjectPath); - - mockServer.Start(); - - // Act - var r = CommandRunner.Run( - nugetexe, - pathContext.WorkingDirectory, - $"restore {projectA.ProjectPath}"); - - mockServer.Stop(); - - // Assert - r.Success.Should().BeTrue(because: r.AllOutput); - - var packageFileA1 = Path.Combine(pathContext.SolutionRoot, "packages", "packageA.1.1.0", "packageA.1.1.0.nupkg"); - var packageFileB1 = Path.Combine(pathContext.SolutionRoot, "packages", "packageB.2.1.0", "packageB.2.1.0.nupkg"); - File.Exists(packageFileA1).Should().BeTrue(); - File.Exists(packageFileB1).Should().BeTrue(); - - r.AllOutput.Should().NotContain($"Package 'packageA' 1.1.0 has a known high severity vulnerability"); // suppressed - r.AllOutput.Should().Contain($"Package 'packageB' 2.1.0 has a known critical severity vulnerability"); - } - - [SkipMono()] - public void RestoreCommand_WithPackagesConfigProject_PackageWithVulnerabilities_WithSuppressedAdvisories_SuppressesExpectedVulnerabilities_TEST() - { - // Arrange - var nugetexe = Util.GetNuGetExePath(); - using var pathContext = new SimpleTestPathContext(); - - string advisoryUrl1 = "https://contoso.com/advisories/1"; - string advisoryUrl2 = "https://contoso.com/advisories/2"; - - using var mockServer = new FileSystemBackedV3MockServer(pathContext.PackageSource, sourceReportsVulnerabilities: true); - - mockServer.Vulnerabilities.Add( - "packageA", - new List<(Uri, PackageVulnerabilitySeverity, VersionRange)> { - (new Uri(advisoryUrl1), PackageVulnerabilitySeverity.High, VersionRange.Parse("[1.0.0, 3.0.0)")) - }); - mockServer.Vulnerabilities.Add( - "packageB", - new List<(Uri, PackageVulnerabilitySeverity, VersionRange)> { - (new Uri(advisoryUrl2), PackageVulnerabilitySeverity.Critical, VersionRange.Parse("[1.0.0, 3.0.0)")) - }); - pathContext.Settings.RemoveSource("source"); - pathContext.Settings.AddSource("source", mockServer.ServiceIndexUri); - - - - Util.CreateTestPackage("packageA", "1.1.0", pathContext.PackageSource); - Util.CreateTestPackage("packageB", "2.1.0", pathContext.PackageSource); - - var projectA = new SimpleTestProjectContext( - "projectA", - ProjectStyle.PackagesConfig, - pathContext.SolutionRoot); - projectA.Properties.Add("NuGetAudit", "true"); - projectA.Properties.Add("NuGetAuditLevel", "low"); - - Util.CreateFile(Path.GetDirectoryName(projectA.ProjectPath), "packages.config", -@" - - -"); - - // suppress the vulnerability on package A for project A - var xmlA = projectA.GetXML(); - ProjectFileUtils.AddItem( - xmlA, - name: "NuGetAuditSuppress", - identity: advisoryUrl1, - framework: NuGetFramework.AnyFramework, - properties: new Dictionary(), - attributes: new Dictionary()); - xmlA.Save(projectA.ProjectPath); - - mockServer.Start(); - - // Act - var r = CommandRunner.Run( - nugetexe, - pathContext.WorkingDirectory, - $"restore {projectA.ProjectPath}"); - - mockServer.Stop(); + var packageA1 = new SimpleTestPackageContext() { Id = "packageA", Version = "1.1.0" }; + var packageA2 = new SimpleTestPackageContext() { Id = "packageA", Version = "1.2.0" }; + var packageB1 = new SimpleTestPackageContext() { Id = "packageB", Version = "2.1.0" }; + var packageB2 = new SimpleTestPackageContext() { Id = "packageB", Version = "2.2.0" }; - // Assert - r.Success.Should().BeTrue(because: r.AllOutput); - - var packageFileA1 = Path.Combine(pathContext.SolutionRoot, "packages", "packageA.1.1.0", "packageA.1.1.0.nupkg"); - var packageFileB1 = Path.Combine(pathContext.SolutionRoot, "packages", "packageB.2.1.0", "packageB.2.1.0.nupkg"); - File.Exists(packageFileA1).Should().BeTrue(); - File.Exists(packageFileB1).Should().BeTrue(); - - r.AllOutput.Should().NotContain($"Package 'packageA' 1.1.0 has a known high severity vulnerability"); // suppressed - r.AllOutput.Should().Contain($"Package 'packageB' 2.1.0 has a known critical severity vulnerability"); - } - - // Not working as expected. Nothing is being suppressed - [SkipMono()] - public async void RestoreCommand_WithSolutionWithPackagesConfigProjects_PackageWithVulnerabilities_WithSuppressedAdvisories_SuppressesExpectedVulnerabilities() - { - // Arrange - var nugetexe = Util.GetNuGetExePath(); - using var pathContext = new SimpleTestPathContext(); - - string advisoryUrl1 = "https://contoso.com/advisories/1"; - string advisoryUrl2 = "https://contoso.com/advisories/2"; - - using var mockServer = new FileSystemBackedV3MockServer(pathContext.PackageSource, sourceReportsVulnerabilities: true); - - mockServer.Vulnerabilities.Add( - "packageA", - new List<(Uri, PackageVulnerabilitySeverity, VersionRange)> { - (new Uri(advisoryUrl1), PackageVulnerabilitySeverity.High, VersionRange.Parse("[1.0.0, 3.0.0)")) - }); - mockServer.Vulnerabilities.Add( - "packageB", - new List<(Uri, PackageVulnerabilitySeverity, VersionRange)> { - (new Uri(advisoryUrl2), PackageVulnerabilitySeverity.Critical, VersionRange.Parse("[1.0.0, 3.0.0)")) - }); - pathContext.Settings.RemoveSource("source"); - pathContext.Settings.AddSource("source", mockServer.ServiceIndexUri); - - /* - Util.CreateTestPackage("packageA", "1.1.0", pathContext.PackageSource); - Util.CreateTestPackage("packageA", "1.2.0", pathContext.PackageSource); - Util.CreateTestPackage("packageB", "2.1.0", pathContext.PackageSource); - Util.CreateTestPackage("packageB", "2.2.0", pathContext.PackageSource); - */ - - var packageA1 = new SimpleTestPackageContext() - { - Id = "packageA", - Version = "1.1.0" - }; - var packageA2 = new SimpleTestPackageContext() - { - Id = "packageA", - Version = "1.2.0" - }; - var packageB1 = new SimpleTestPackageContext() - { - Id = "packageB", - Version = "2.1.0" - }; - var packageB2 = new SimpleTestPackageContext() - { - Id = "packageB", - Version = "2.2.0" - }; - - await SimpleTestPackageUtility.CreatePackagesAsync( - pathContext.PackageSource, - packageA1, - packageA2, - packageB1, - packageB2); + await SimpleTestPackageUtility.CreatePackagesAsync(pathContext.PackageSource, packageA1, packageA2, packageB1, packageB2); var solution = new SimpleTestSolutionContext(pathContext.SolutionRoot); - - var projectA = new SimpleTestProjectContext( - "projectA", - ProjectStyle.PackagesConfig, - pathContext.SolutionRoot); - projectA.Properties.Add("NuGetAudit", "true"); - projectA.Properties.Add("NuGetAuditLevel", "low"); - - var projectB = new SimpleTestProjectContext( - "projectB", - ProjectStyle.PackagesConfig, - pathContext.SolutionRoot); - projectB.Properties.Add("NuGetAudit", "true"); - projectB.Properties.Add("NuGetAuditLevel", "low"); + var projectA = new SimpleTestProjectContext("projectA", ProjectStyle.PackagesConfig, pathContext.SolutionRoot); + var projectB = new SimpleTestProjectContext("projectB", ProjectStyle.PackagesConfig, pathContext.SolutionRoot); solution.Projects.Add(projectA); solution.Projects.Add(projectB); @@ -3925,16 +3732,6 @@ await SimpleTestPackageUtility.CreatePackagesAsync( // Assert r.Success.Should().BeTrue(because: r.AllOutput); - - var packageFileA1 = Path.Combine(pathContext.SolutionRoot, "packages", "packageA.1.1.0", "packageA.1.1.0.nupkg"); - var packageFileA2 = Path.Combine(pathContext.SolutionRoot, "packages", "packageA.1.2.0", "packageA.1.2.0.nupkg"); - var packageFileB1 = Path.Combine(pathContext.SolutionRoot, "packages", "packageB.2.1.0", "packageB.2.1.0.nupkg"); - var packageFileB2 = Path.Combine(pathContext.SolutionRoot, "packages", "packageB.2.2.0", "packageB.2.2.0.nupkg"); - File.Exists(packageFileA1).Should().BeTrue(); - File.Exists(packageFileA2).Should().BeTrue(); - File.Exists(packageFileB1).Should().BeTrue(); - File.Exists(packageFileB2).Should().BeTrue(); - r.AllOutput.Should().NotContain($"Package 'packageA' 1.1.0 has a known high severity vulnerability"); // suppressed r.AllOutput.Should().Contain($"Package 'packageB' 2.1.0 has a known critical severity vulnerability"); r.AllOutput.Should().Contain($"Package 'packageA' 1.2.0 has a known high severity vulnerability"); diff --git a/test/NuGet.Core.FuncTests/Msbuild.Integration.Test/MsbuildRestoreTaskTests.cs b/test/NuGet.Core.FuncTests/Msbuild.Integration.Test/MsbuildRestoreTaskTests.cs index d116ce2c397..658d2e641c4 100644 --- a/test/NuGet.Core.FuncTests/Msbuild.Integration.Test/MsbuildRestoreTaskTests.cs +++ b/test/NuGet.Core.FuncTests/Msbuild.Integration.Test/MsbuildRestoreTaskTests.cs @@ -1630,7 +1630,7 @@ await SimpleTestPackageUtility.CreatePackagesAsync( } [Fact] - public async Task MsbuildRestore_PackagesConfigProject_PackagesWithVulnerabilities_WithSuppressions_RaisesAppropriateWarnings() + public async Task MsbuildRestore_WithPackagesConfigProject_PackageWithVulnerabilities_WithSuppressedAdvisories_SuppressesExpectedVulnerabilities() { // Arrange var pathContext = new SimpleTestPathContext(); @@ -1653,46 +1653,22 @@ public async Task MsbuildRestore_PackagesConfigProject_PackagesWithVulnerabiliti pathContext.Settings.RemoveSource("source"); pathContext.Settings.AddSource("source", mockServer.ServiceIndexUri); - // set up solution, projects and packages + // set up the solution, projects and packages var solution = new SimpleTestSolutionContext(pathContext.SolutionRoot); - - var projectA = new SimpleTestProjectContext( - "A", - ProjectStyle.PackagesConfig, - pathContext.SolutionRoot); - projectA.Frameworks.Add(new SimpleTestProjectFrameworkContext(CommonFrameworks.Net472)); - - var projectB = new SimpleTestProjectContext( - "B", - ProjectStyle.PackagesConfig, - pathContext.SolutionRoot); - projectB.Frameworks.Add(new SimpleTestProjectFrameworkContext(CommonFrameworks.Net472)); - - var packageA1 = new SimpleTestPackageContext() - { - Id = "packageA", - Version = "1.1.0" - }; - var packageA2 = new SimpleTestPackageContext() - { - Id = "packageA", - Version = "1.2.0" - }; - var packageB1 = new SimpleTestPackageContext() - { - Id = "packageB", - Version = "2.1.0" - }; - var packageB2 = new SimpleTestPackageContext() - { - Id = "packageB", - Version = "2.2.0" - }; + var projectA = new SimpleTestProjectContext("projectA", ProjectStyle.PackagesConfig, pathContext.SolutionRoot); + var projectB = new SimpleTestProjectContext("projectB", ProjectStyle.PackagesConfig, pathContext.SolutionRoot); solution.Projects.Add(projectA); solution.Projects.Add(projectB); solution.Create(pathContext.SolutionRoot); + var packageA1 = new SimpleTestPackageContext() { Id = "packageA", Version = "1.1.0" }; + var packageA2 = new SimpleTestPackageContext() { Id = "packageA", Version = "1.2.0" }; + var packageB1 = new SimpleTestPackageContext() { Id = "packageB", Version = "2.1.0" }; + var packageB2 = new SimpleTestPackageContext() { Id = "packageB", Version = "2.2.0" }; + + await SimpleTestPackageUtility.CreatePackagesAsync(pathContext.PackageSource, packageA1, packageA2, packageB1, packageB2); + using (var writer = new StreamWriter(Path.Combine(Path.GetDirectoryName(projectA.ProjectPath), "packages.config"))) { writer.Write( @@ -1733,13 +1709,6 @@ public async Task MsbuildRestore_PackagesConfigProject_PackagesWithVulnerabiliti attributes: new Dictionary()); xmlB.Save(projectB.ProjectPath); - await SimpleTestPackageUtility.CreatePackagesAsync( - pathContext.PackageSource, - packageA1, - packageA2, - packageB1, - packageB2); - // Act mockServer.Start(); From 6f860f9ee03b1b6d18b838355de499a7d70df01d Mon Sep 17 00:00:00 2001 From: Advay Tandon Date: Wed, 26 Jun 2024 16:52:22 -0700 Subject: [PATCH 3/4] nit: formatting --- src/NuGet.Core/NuGet.PackageManagement/Audit/AuditChecker.cs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/NuGet.Core/NuGet.PackageManagement/Audit/AuditChecker.cs b/src/NuGet.Core/NuGet.PackageManagement/Audit/AuditChecker.cs index 9563fc74bcd..b2073f743f1 100644 --- a/src/NuGet.Core/NuGet.PackageManagement/Audit/AuditChecker.cs +++ b/src/NuGet.Core/NuGet.PackageManagement/Audit/AuditChecker.cs @@ -388,9 +388,7 @@ internal class ProjectAuditSettings public Dictionary? SuppressedAdvisories { get; } - public ProjectAuditSettings(bool enableAudit, - PackageVulnerabilitySeverity auditLevel, - HashSet? suppressedAdvisories) + public ProjectAuditSettings(bool enableAudit, PackageVulnerabilitySeverity auditLevel, HashSet? suppressedAdvisories) { IsAuditEnabled = enableAudit; MinimumSeverity = auditLevel; From aa346a3f8fc05a289676fb6f8055230157974ac4 Mon Sep 17 00:00:00 2001 From: Advay Tandon Date: Wed, 26 Jun 2024 17:02:01 -0700 Subject: [PATCH 4/4] fix nullable error --- src/NuGet.Core/NuGet.PackageManagement/Audit/AuditChecker.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/NuGet.Core/NuGet.PackageManagement/Audit/AuditChecker.cs b/src/NuGet.Core/NuGet.PackageManagement/Audit/AuditChecker.cs index b2073f743f1..5637e3aae5e 100644 --- a/src/NuGet.Core/NuGet.PackageManagement/Audit/AuditChecker.cs +++ b/src/NuGet.Core/NuGet.PackageManagement/Audit/AuditChecker.cs @@ -226,7 +226,7 @@ internal static List CreateWarnings(Dictionary= (int)auditSetting.MinimumSeverity) {