Skip to content

Commit

Permalink
Set NuGetAudit property defaults in MSBuild (#5469)
Browse files Browse the repository at this point in the history
  • Loading branch information
@zivkan @Nigusu-Allehu
zivkan authored and Nigusu-Allehu committed Nov 21, 2023
1 parent b2ba468 commit 8c13fea
Show file tree
Hide file tree
Showing 6 changed files with 50 additions and 70 deletions.
13 changes: 13 additions & 0 deletions src/NuGet.Core/NuGet.Build.Tasks/NuGet.targets
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,19 @@ Copyright (c) .NET Foundation. All rights reserved.
<_CentralPackageVersionsEnabled Condition="'$(ManagePackageVersionsCentrally)' == 'true' AND '$(CentralPackageVersionsFileImported)' == 'true'">true</_CentralPackageVersionsEnabled>
</PropertyGroup>

<!--
Visual Studio's project property page requires defaults to be set to inform customers what the default values are.
Project-system uses DefaultValueSourceLocation.AfterContext to detect when a customer's project changes the value, so these defaults must be set here in the targets file.
-->
<PropertyGroup>
<!-- Enable NuGetAudit by default -->
<NuGetAudit Condition=" '$(NuGetAudit)' == '' ">true</NuGetAudit>
<!-- Report on low severity vulnerabilities, and higher. Allowed values are: low, moderate, high, critical -->
<NuGetAuditLevel Condition=" '$(NuGetAuditLevel)' == '' ">low</NuGetAuditLevel>
<!-- Report known vulnerabilities on direct dependencies only. Allowed values are: direct, all -->
<NuGetAuditMode Condition=" '$(NuGetAuditMode)' == '' ">direct</NuGetAuditMode>
</PropertyGroup>

<PropertyGroup>
<!-- Exclude packages from changing restore inputs. -->
<_GenerateRestoreGraphProjectEntryInputProperties>ExcludeRestorePackageImports=true</_GenerateRestoreGraphProjectEntryInputProperties>
Expand Down
11 changes: 5 additions & 6 deletions src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -304,14 +304,14 @@ await _logger.LogAsync(RestoreLogMessage.CreateWarning(NuGetLogCode.NU1803,
});
}

AuditUtility.EnabledValue enableAudit = AuditUtility.ParseEnableValue(
bool auditEnabled = AuditUtility.ParseEnableValue(
_request.Project.RestoreMetadata?.RestoreAuditProperties?.EnableAudit,
_request.Project.FilePath,
_logger);
telemetry.TelemetryEvent[AuditEnabled] = AuditUtility.GetString(enableAudit);
if (enableAudit != AuditUtility.EnabledValue.ExplicitOptOut)
telemetry.TelemetryEvent[AuditEnabled] = auditEnabled ? "enabled" : "disabled";
if (auditEnabled)
{
await PerformAuditAsync(enableAudit, graphs, telemetry, token);
await PerformAuditAsync(graphs, telemetry, token);
}

telemetry.StartIntervalMeasure();
Expand Down Expand Up @@ -477,11 +477,10 @@ await _logger.LogAsync(RestoreLogMessage.CreateWarning(NuGetLogCode.NU1803,
}
}

private async Task PerformAuditAsync(AuditUtility.EnabledValue enableAudit, IEnumerable<RestoreTargetGraph> graphs, TelemetryActivity telemetry, CancellationToken token)
private async Task PerformAuditAsync(IEnumerable<RestoreTargetGraph> graphs, TelemetryActivity telemetry, CancellationToken token)
{
telemetry.StartIntervalMeasure();
var audit = new AuditUtility(
enableAudit,
_request.Project.RestoreMetadata.RestoreAuditProperties,
_request.Project.FilePath,
graphs,
Expand Down
43 changes: 9 additions & 34 deletions src/NuGet.Core/NuGet.Commands/RestoreCommand/Utility/AuditUtility.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ namespace NuGet.Commands.Restore.Utility
{
internal class AuditUtility
{
private readonly EnabledValue _auditEnabled;
private readonly ProjectModel.RestoreAuditProperties? _restoreAuditProperties;
private readonly string _projectFullPath;
private readonly IEnumerable<RestoreTargetGraph> _targetGraphs;
Expand All @@ -48,14 +47,12 @@ internal class AuditUtility
internal int SourcesWithVulnerabilityData { get; private set; }

public AuditUtility(
EnabledValue auditEnabled,
ProjectModel.RestoreAuditProperties? restoreAuditProperties,
string projectFullPath,
IEnumerable<RestoreTargetGraph> graphs,
IReadOnlyList<IVulnerabilityInformationProvider> vulnerabilityInformationProviders,
ILogger logger)
{
_auditEnabled = auditEnabled;
_restoreAuditProperties = restoreAuditProperties;
_projectFullPath = projectFullPath;
_targetGraphs = graphs;
Expand Down Expand Up @@ -401,50 +398,28 @@ private NuGetAuditMode ParseAuditMode()
return NuGetAuditMode.Unknown;
}

internal enum EnabledValue
{
Invalid,
ImplicitOptIn,
ExplicitOptIn,
ExplicitOptOut
}

// Enum parsing and ToString are a magnitude of times slower than a naive implementation.
public static EnabledValue ParseEnableValue(string? value, string projectFullPath, ILogger logger)
public static bool ParseEnableValue(string? value, string projectFullPath, ILogger logger)
{
if (string.IsNullOrEmpty(value) || string.Equals(value, "default", StringComparison.OrdinalIgnoreCase))
// Earlier versions allowed "enable" and "default" to opt-in
if (string.IsNullOrEmpty(value)
|| string.Equals(value, bool.TrueString, StringComparison.OrdinalIgnoreCase)
|| string.Equals(value, "enable", StringComparison.OrdinalIgnoreCase)
|| string.Equals(value, "default", StringComparison.OrdinalIgnoreCase))
{
return EnabledValue.ImplicitOptIn;
}
if (string.Equals(value, bool.TrueString, StringComparison.OrdinalIgnoreCase)
|| string.Equals(value, "enable", StringComparison.OrdinalIgnoreCase))
{
return EnabledValue.ExplicitOptIn;
return true;
}
if (string.Equals(value, bool.FalseString, StringComparison.OrdinalIgnoreCase)
|| string.Equals(value, "disable", StringComparison.OrdinalIgnoreCase))
{
return EnabledValue.ExplicitOptOut;
return false;
}

string messageText = string.Format(Strings.Error_InvalidNuGetAuditValue, value, "true, false");
RestoreLogMessage message = RestoreLogMessage.CreateError(NuGetLogCode.NU1014, messageText);
message.ProjectPath = projectFullPath;
logger.Log(message);
return EnabledValue.Invalid;
}

// Enum parsing and ToString are a magnitude of times slower than a naive implementation.
internal static string GetString(EnabledValue enableAudit)
{
return enableAudit switch
{
EnabledValue.Invalid => nameof(EnabledValue.Invalid),
EnabledValue.ExplicitOptIn => nameof(EnabledValue.ExplicitOptIn),
EnabledValue.ExplicitOptOut => nameof(EnabledValue.ExplicitOptOut),
EnabledValue.ImplicitOptIn => nameof(EnabledValue.ImplicitOptIn),
_ => enableAudit.ToString()
};
return true;
}

// Enum parsing and ToString are a magnitude of times slower than a naive implementation.
Expand Down
4 changes: 4 additions & 0 deletions test/EndToEnd/tests/NetCoreProjectTest.ps1
View file
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,8 @@ function Test-NetCoreConsoleAppRebuildDoesNotDeleteCacheFile {
}

function Test-NetCoreVSandMSBuildNoOp {
[SkipTest('https://github.com/NuGet/Home/issues/13003')]
param ()

# Arrange
$project = New-NetCoreConsoleApp ConsoleApp
Expand All @@ -163,6 +165,8 @@ function Test-NetCoreVSandMSBuildNoOp {
}

function Test-NetCoreTargetFrameworksVSandMSBuildNoOp {
[SkipTest('https://github.com/NuGet/Home/issues/13003')]
param ()

# Arrange
$project = New-NetCoreConsoleTargetFrameworksApp ConsoleApp
Expand Down
2 changes: 1 addition & 1 deletion test/NuGet.Core.Tests/NuGet.Commands.Test/RestoreCommandTests/RestoreCommandTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2870,7 +2870,7 @@ await SimpleTestPackageUtility.CreateFolderFeedV3Async(
["HttpSourcesCount"] = value => value.Should().Be(0),
["LocalSourcesCount"] = value => value.Should().Be(1),
["FallbackFoldersCount"] = value => value.Should().Be(0),
["Audit.Enabled"] = value => value.Should().Be("ExplicitOptIn"),
["Audit.Enabled"] = value => value.Should().Be("enabled"),
["Audit.Level"] = value => value.Should().Be(0),
["Audit.Mode"] = value => value.Should().Be("Unknown"),
["Audit.Vulnerability.Direct.Count"] = value => value.Should().Be(0),
Expand Down
47 changes: 18 additions & 29 deletions test/NuGet.Core.Tests/NuGet.Commands.Test/RestoreCommandTests/Utility/AuditUtilityTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,38 +32,28 @@ public class AuditUtilityTests
private static VersionRange UpToV2 = new VersionRange(maxVersion: new NuGetVersion(2, 0, 0), includeMaxVersion: false);

[Theory]
[InlineData(null, nameof(AuditUtility.EnabledValue.ImplicitOptIn))]
[InlineData("", nameof(AuditUtility.EnabledValue.ImplicitOptIn))]
[InlineData("default", nameof(AuditUtility.EnabledValue.ImplicitOptIn))]
[InlineData("true", nameof(AuditUtility.EnabledValue.ExplicitOptIn))]
[InlineData("enable", nameof(AuditUtility.EnabledValue.ExplicitOptIn))]
[InlineData("TRUE", nameof(AuditUtility.EnabledValue.ExplicitOptIn))]
[InlineData("false", nameof(AuditUtility.EnabledValue.ExplicitOptOut))]
[InlineData("disable", nameof(AuditUtility.EnabledValue.ExplicitOptOut))]
[InlineData("FALSE", nameof(AuditUtility.EnabledValue.ExplicitOptOut))]
[InlineData("invalid", nameof(AuditUtility.EnabledValue.Invalid))]
public void ParseEnableValue_WithValue_ReturnsExpectedEnum(string input, string expected)
[InlineData(null, true)]
[InlineData("", true)]
[InlineData("default", true)]
[InlineData("true", true)]
[InlineData("enable", true)]
[InlineData("TRUE", true)]
[InlineData("false", false)]
[InlineData("disable", false)]
[InlineData("FALSE", false)]
[InlineData("invalid", true, true)]
public void ParseEnableValue_WithValue_ReturnsExpected(string input, bool expected, bool expectLog = false)
{
// Arrange
AuditUtility.EnabledValue expectedValue = (AuditUtility.EnabledValue)Enum.Parse(typeof(AuditUtility.EnabledValue), expected);
string projectPath = "my.csproj";
TestLogger? logger = expectedValue == AuditUtility.EnabledValue.Invalid
? new TestLogger()
: null;
TestLogger logger = new TestLogger();

// Act
AuditUtility.EnabledValue actual = AuditUtility.ParseEnableValue(input, projectPath, logger ?? NullLogger.Instance);
bool actual = AuditUtility.ParseEnableValue(input, projectPath, logger);

// Assert
actual.Should().Be(expectedValue);

if (logger is not null)
{
logger.Errors.Should().Be(1);
RestoreLogMessage message = logger.LogMessages.Cast<RestoreLogMessage>().Single();
message.Code.Should().Be(NuGetLogCode.NU1014);
message.Level.Should().Be(LogLevel.Error);
}
actual.Should().Be(expected);
logger.LogMessages.Cast<RestoreLogMessage>().Count(m => m.Code == NuGetLogCode.NU1014).Should().Be(expectLog ? 1 : 0);
}

[Fact]
Expand Down Expand Up @@ -388,7 +378,6 @@ await createGraphTasks[1]

// Act
var audit = new AuditUtility(
AuditUtility.ParseEnableValue(null, "/path/proj.csproj", log),
restoreAuditProperties: null,
"/path/proj.csproj",
graphs,
Expand Down Expand Up @@ -473,8 +462,8 @@ public VulnerabilityProviderTestContext WithVulnerabilityProvider()

public async Task<AuditUtility> CheckPackageVulnerabilitiesAsync(CancellationToken cancellationToken)
{
AuditUtility.EnabledValue enabled = AuditUtility.ParseEnableValue(Enabled, ProjectFullPath, Log);
if (enabled == AuditUtility.EnabledValue.ExplicitOptOut)
bool enabled = AuditUtility.ParseEnableValue(Enabled, ProjectFullPath, Log);
if (!enabled)
{
throw new InvalidOperationException($"{nameof(Enabled)} must have a value that does not disable NuGetAudit.");
}
Expand All @@ -495,7 +484,7 @@ public async Task<AuditUtility> CheckPackageVulnerabilitiesAsync(CancellationTok

var vulnProviders = CreateVulnerabilityInformationProviders(_vulnerabilityProviders);

var audit = new AuditUtility(enabled, restoreAuditProperties, ProjectFullPath, graphs, vulnProviders, Log);
var audit = new AuditUtility(restoreAuditProperties, ProjectFullPath, graphs, vulnProviders, Log);
await audit.CheckPackageVulnerabilitiesAsync(CancellationToken.None);

return audit;
Expand Down

0 comments on commit 8c13fea

Please sign in to comment.