Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable NuGetAuditSuppress for packages.config CLI restore #5875

Merged
merged 5 commits into from
Jul 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion src/NuGet.Core/NuGet.Build.Tasks/NuGet.targets
Original file line number Diff line number Diff line change
Expand Up @@ -1118,7 +1118,6 @@ Copyright (c) .NET Foundation. All rights reserved.

<!-- Write out advisory suppressions-->
<GetRestoreNuGetAuditSuppressionsTask
Condition=" '$(PackageReferenceCompatibleProjectStyle)' == 'true' "
ProjectUniqueName="$(MSBuildProjectFullPath)"
NuGetAuditSuppressions="@(NuGetAuditSuppress)"
TargetFrameworks="$(TargetFramework)"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ public record AuditCheckResult
internal int Severity2VulnerabilitiesFound { get; set; }
internal int Severity3VulnerabilitiesFound { get; set; }
internal int InvalidSeverityVulnerabilitiesFound { get; set; }
internal int TotalWarningsSuppressedCount { get; set; }
internal int DistinctAdvisoriesSuppressedCount { get; set; }
internal List<PackageIdentity>? Packages { get; set; }
internal double? DownloadDurationInSeconds { get; set; }
internal double? CheckPackagesDurationInSeconds { get; set; }
Expand All @@ -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";
Expand All @@ -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)
Expand Down
69 changes: 61 additions & 8 deletions src/NuGet.Core/NuGet.PackageManagement/Audit/AuditChecker.cs
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,12 @@ public async Task<AuditCheckResult> 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<string, (bool, PackageVulnerabilitySeverity)>(restoreAuditProperties.Count);
var auditSettings = new Dictionary<string, ProjectAuditSettings>(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;
}

Expand Down Expand Up @@ -80,13 +80,14 @@ public async Task<AuditCheckResult> CheckPackageVulnerabilitiesAsync(IEnumerable

stopwatch.Restart();
Dictionary<PackageIdentity, PackageAuditInfo>? 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<PackageIdentity> packagesWithReportedAdvisories = new(packagesWithKnownVulnerabilities?.Count ?? 0);

IReadOnlyList<ILogMessage> warnings = packagesWithKnownVulnerabilities is not null ?
CreateWarnings(packagesWithKnownVulnerabilities, auditSettings, ref Sev0Matches, ref Sev1Matches, ref Sev2Matches, ref Sev3Matches, ref InvalidSevMatches, ref packagesWithReportedAdvisories) :
Array.Empty<ILogMessage>();
IReadOnlyList<ILogMessage> 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<ILogMessage>();

foreach (var warning in warnings.NoAllocEnumerate())
{
Expand All @@ -103,6 +104,8 @@ public async Task<AuditCheckResult> CheckPackageVulnerabilitiesAsync(IEnumerable
Severity2VulnerabilitiesFound = Sev2Matches,
Severity3VulnerabilitiesFound = Sev3Matches,
InvalidSeverityVulnerabilitiesFound = InvalidSevMatches,
TotalWarningsSuppressedCount = TotalWarningsSuppressedCount,
DistinctAdvisoriesSuppressedCount = DistinctAdvisoriesSuppressedCount,
Packages = packagesWithReportedAdvisories,
DownloadDurationInSeconds = downloadDurationInSeconds,
CheckPackagesDurationInSeconds = checkPackagesDurationInSeconds,
Expand Down Expand Up @@ -195,12 +198,14 @@ static bool IsAnyVulnerabilityDataFound(IReadOnlyList<IReadOnlyDictionary<string
}

internal static List<LogMessage> CreateWarnings(Dictionary<PackageIdentity, PackageAuditInfo> packagesWithKnownVulnerabilities,
Dictionary<string, (bool, PackageVulnerabilitySeverity)> auditSettings,
Dictionary<string, ProjectAuditSettings> auditSettings,
ref int Sev0Matches,
ref int Sev1Matches,
ref int Sev2Matches,
ref int Sev3Matches,
ref int InvalidSevMatches,
ref int TotalWarningsSuppressedCount,
ref int DistinctAdvisoriesSuppressedCount,
ref List<PackageIdentity> packagesWithReportedAdvisories)
{
var warnings = new List<LogMessage>();
Expand All @@ -221,10 +226,15 @@ internal static List<LogMessage> CreateWarnings(Dictionary<PackageIdentity, Pack
for (int i = 0; i < auditInfo.Projects.Count; i++)
{
string projectPath = auditInfo.Projects[i];
auditSettings.TryGetValue(projectPath, out (bool IsAuditEnabled, PackageVulnerabilitySeverity MinimumSeverity) auditSetting);
auditSettings.TryGetValue(projectPath, out ProjectAuditSettings? auditSetting);

if (auditSetting == default || auditSetting.IsAuditEnabled && (int)vulnerability.Severity >= (int)auditSetting.MinimumSeverity)
{
if (CheckIfAdvisoryHasBeenSuppressed(auditSetting?.SuppressedAdvisories, vulnerability.Url.OriginalString, ref TotalWarningsSuppressedCount, ref DistinctAdvisoriesSuppressedCount))
{
continue;
}

isVulnerabilityReported = true;
if (!counted)
{
Expand Down Expand Up @@ -336,6 +346,24 @@ internal static (string severityLabel, NuGetLogCode code) GetSeverityLabelAndCod
}
}

private static bool CheckIfAdvisoryHasBeenSuppressed(Dictionary<string, bool>? 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; }
Expand All @@ -351,5 +379,30 @@ public PackageAuditInfo(PackageIdentity identity, IList<string> projects)
Projects = projects;
}
}

internal class ProjectAuditSettings
{
public bool IsAuditEnabled { get; }

public PackageVulnerabilitySeverity MinimumSeverity { get; }

public Dictionary<string, bool>? SuppressedAdvisories { get; }

public ProjectAuditSettings(bool enableAudit, PackageVulnerabilitySeverity auditLevel, HashSet<string>? suppressedAdvisories)
{
IsAuditEnabled = enableAudit;
MinimumSeverity = auditLevel;

if (suppressedAdvisories != null)
{
SuppressedAdvisories = new Dictionary<string, bool>(suppressedAdvisories.Count);

foreach (string advisory in suppressedAdvisories)
{
SuppressedAdvisories.Add(advisory, false);
}
}
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,6 @@
[assembly: SuppressMessage("Performance", "CA1822:Mark members as static", Justification = "<Pending>", Scope = "member", Target = "~M:NuGet.PackageManagement.PackagesConfigContentHashProvider.TryGetNupkgMetadata(System.String)~NuGet.PackageManagement.PackagesConfigContentHashProvider.Result")]
[assembly: SuppressMessage("Performance", "CA1822:Mark members as static", Justification = "<Pending>", Scope = "member", Target = "~M:NuGet.ProjectManagement.DefaultProjectServices.GetGlobalService``1~``0")]
[assembly: SuppressMessage("Globalization", "CA1305:Specify IFormatProvider", Justification = "<Pending>", 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 = "<Pending>", 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 = "<Pending>", 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 = "<Pending>", 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 = "<Pending>", Scope = "member", Target = "~M:NuGet.PackageManagement.ResolverGather.GatherAsync(System.Threading.CancellationToken)~System.Threading.Tasks.Task{System.Collections.Generic.HashSet{NuGet.Protocol.Core.Types.SourcePackageDependencyInfo}}")]
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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;

Expand Down Expand Up @@ -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(
@"<packages>
<package id=""packageA"" version=""1.1.0"" />
<package id=""packageB"" version=""2.1.0"" />
</packages>");
}
using (var writer = new StreamWriter(Path.Combine(Path.GetDirectoryName(projectB.ProjectPath), "packages.config")))
{
writer.Write(
@"<packages>
<package id=""packageA"" version=""1.2.0"" />
<package id=""packageB"" version=""2.2.0"" />
</packages>");
}

// 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<string, string>(),
attributes: new Dictionary<string, string>());
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<string, string>(),
attributes: new Dictionary<string, string>());
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)))
Expand Down
Loading
Loading