From 8c13fea2efbcc6ac6546650db9130aa2d4ca0456 Mon Sep 17 00:00:00 2001 From: Andy Zivkovic Date: Fri, 10 Nov 2023 10:39:07 +0100 Subject: [PATCH] Set NuGetAudit property defaults in MSBuild (#5469) --- .../NuGet.Build.Tasks/NuGet.targets | 13 +++++ .../RestoreCommand/RestoreCommand.cs | 11 ++--- .../RestoreCommand/Utility/AuditUtility.cs | 43 ++++------------- test/EndToEnd/tests/NetCoreProjectTest.ps1 | 4 ++ .../RestoreCommandTests.cs | 2 +- .../Utility/AuditUtilityTests.cs | 47 +++++++------------ 6 files changed, 50 insertions(+), 70 deletions(-) diff --git a/src/NuGet.Core/NuGet.Build.Tasks/NuGet.targets b/src/NuGet.Core/NuGet.Build.Tasks/NuGet.targets index bad1d971606..d676d1bdabc 100644 --- a/src/NuGet.Core/NuGet.Build.Tasks/NuGet.targets +++ b/src/NuGet.Core/NuGet.Build.Tasks/NuGet.targets @@ -63,6 +63,19 @@ Copyright (c) .NET Foundation. All rights reserved. <_CentralPackageVersionsEnabled Condition="'$(ManagePackageVersionsCentrally)' == 'true' AND '$(CentralPackageVersionsFileImported)' == 'true'">true + + + + true + + low + + direct + + <_GenerateRestoreGraphProjectEntryInputProperties>ExcludeRestorePackageImports=true diff --git a/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs b/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs index e11cf58c0e2..0f8088157af 100644 --- a/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs +++ b/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs @@ -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(); @@ -477,11 +477,10 @@ await _logger.LogAsync(RestoreLogMessage.CreateWarning(NuGetLogCode.NU1803, } } - private async Task PerformAuditAsync(AuditUtility.EnabledValue enableAudit, IEnumerable graphs, TelemetryActivity telemetry, CancellationToken token) + private async Task PerformAuditAsync(IEnumerable graphs, TelemetryActivity telemetry, CancellationToken token) { telemetry.StartIntervalMeasure(); var audit = new AuditUtility( - enableAudit, _request.Project.RestoreMetadata.RestoreAuditProperties, _request.Project.FilePath, graphs, diff --git a/src/NuGet.Core/NuGet.Commands/RestoreCommand/Utility/AuditUtility.cs b/src/NuGet.Core/NuGet.Commands/RestoreCommand/Utility/AuditUtility.cs index 126082dd32f..fe1fa43464e 100644 --- a/src/NuGet.Core/NuGet.Commands/RestoreCommand/Utility/AuditUtility.cs +++ b/src/NuGet.Core/NuGet.Commands/RestoreCommand/Utility/AuditUtility.cs @@ -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 _targetGraphs; @@ -48,14 +47,12 @@ internal class AuditUtility internal int SourcesWithVulnerabilityData { get; private set; } public AuditUtility( - EnabledValue auditEnabled, ProjectModel.RestoreAuditProperties? restoreAuditProperties, string projectFullPath, IEnumerable graphs, IReadOnlyList vulnerabilityInformationProviders, ILogger logger) { - _auditEnabled = auditEnabled; _restoreAuditProperties = restoreAuditProperties; _projectFullPath = projectFullPath; _targetGraphs = graphs; @@ -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. diff --git a/test/EndToEnd/tests/NetCoreProjectTest.ps1 b/test/EndToEnd/tests/NetCoreProjectTest.ps1 index 40fb06e9786..18deb355efb 100644 --- a/test/EndToEnd/tests/NetCoreProjectTest.ps1 +++ b/test/EndToEnd/tests/NetCoreProjectTest.ps1 @@ -139,6 +139,8 @@ function Test-NetCoreConsoleAppRebuildDoesNotDeleteCacheFile { } function Test-NetCoreVSandMSBuildNoOp { + [SkipTest('https://github.com/NuGet/Home/issues/13003')] + param () # Arrange $project = New-NetCoreConsoleApp ConsoleApp @@ -163,6 +165,8 @@ function Test-NetCoreVSandMSBuildNoOp { } function Test-NetCoreTargetFrameworksVSandMSBuildNoOp { + [SkipTest('https://github.com/NuGet/Home/issues/13003')] + param () # Arrange $project = New-NetCoreConsoleTargetFrameworksApp ConsoleApp diff --git a/test/NuGet.Core.Tests/NuGet.Commands.Test/RestoreCommandTests/RestoreCommandTests.cs b/test/NuGet.Core.Tests/NuGet.Commands.Test/RestoreCommandTests/RestoreCommandTests.cs index 5ac1176ee18..38e723d5b52 100644 --- a/test/NuGet.Core.Tests/NuGet.Commands.Test/RestoreCommandTests/RestoreCommandTests.cs +++ b/test/NuGet.Core.Tests/NuGet.Commands.Test/RestoreCommandTests/RestoreCommandTests.cs @@ -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), diff --git a/test/NuGet.Core.Tests/NuGet.Commands.Test/RestoreCommandTests/Utility/AuditUtilityTests.cs b/test/NuGet.Core.Tests/NuGet.Commands.Test/RestoreCommandTests/Utility/AuditUtilityTests.cs index 6d04ed7a188..f14ff2c3911 100644 --- a/test/NuGet.Core.Tests/NuGet.Commands.Test/RestoreCommandTests/Utility/AuditUtilityTests.cs +++ b/test/NuGet.Core.Tests/NuGet.Commands.Test/RestoreCommandTests/Utility/AuditUtilityTests.cs @@ -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().Single(); - message.Code.Should().Be(NuGetLogCode.NU1014); - message.Level.Should().Be(LogLevel.Error); - } + actual.Should().Be(expected); + logger.LogMessages.Cast().Count(m => m.Code == NuGetLogCode.NU1014).Should().Be(expectLog ? 1 : 0); } [Fact] @@ -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, @@ -473,8 +462,8 @@ public VulnerabilityProviderTestContext WithVulnerabilityProvider() public async Task 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."); } @@ -495,7 +484,7 @@ public async Task 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;