diff --git a/src/NuGet.Core/NuGet.Build.Tasks.Console/MSBuildStaticGraphRestore.cs b/src/NuGet.Core/NuGet.Build.Tasks.Console/MSBuildStaticGraphRestore.cs index 13319b122d9..3dccf59d12d 100644 --- a/src/NuGet.Core/NuGet.Build.Tasks.Console/MSBuildStaticGraphRestore.cs +++ b/src/NuGet.Core/NuGet.Build.Tasks.Console/MSBuildStaticGraphRestore.cs @@ -138,7 +138,9 @@ static bool HasProjectToRestore(DependencyGraphSpec dgSpec, bool restorePackages try { - bool result = await BuildTasksUtility.RestoreAsync( + // todo: need to return Restore task output properties, like in NuGet.targets + // https://github.com/NuGet/Home/issues/13828 + List restoreSummaries = await BuildTasksUtility.RestoreAsync( dependencyGraphSpec: dependencyGraphSpec, interactive, recursive: IsOptionTrue(nameof(RestoreTaskEx.Recursive), options), @@ -152,6 +154,7 @@ static bool HasProjectToRestore(DependencyGraphSpec dgSpec, bool restorePackages cleanupAssetsForUnsupportedProjects: IsOptionTrue(nameof(RestoreTaskEx.CleanupAssetsForUnsupportedProjects), options), log: MSBuildLogger, cancellationToken: CancellationToken.None); + bool result = restoreSummaries.All(rs => rs.Success); LogFilesToEmbedInBinlog(dependencyGraphSpec, options); diff --git a/src/NuGet.Core/NuGet.Build.Tasks/BuildTasksUtility.cs b/src/NuGet.Core/NuGet.Build.Tasks/BuildTasksUtility.cs index f89fdfa09b3..62286b2b486 100644 --- a/src/NuGet.Core/NuGet.Build.Tasks/BuildTasksUtility.cs +++ b/src/NuGet.Core/NuGet.Build.Tasks/BuildTasksUtility.cs @@ -114,7 +114,7 @@ public static void AddPropertyIfExists(IDictionary properties, s ProjectStyle.ProjectJson }; - public static Task RestoreAsync( + public static Task> RestoreAsync( DependencyGraphSpec dependencyGraphSpec, bool interactive, bool recursive, @@ -131,7 +131,7 @@ public static Task RestoreAsync( return RestoreAsync(dependencyGraphSpec, interactive, recursive, noCache, ignoreFailedSources, disableParallel, force, forceEvaluate, hideWarningsAndErrors, restorePC, cleanupAssetsForUnsupportedProjects: false, log, cancellationToken); } - public static async Task RestoreAsync( + public static async Task> RestoreAsync( DependencyGraphSpec dependencyGraphSpec, bool interactive, bool recursive, @@ -289,7 +289,7 @@ public static async Task RestoreAsync( { RestoreSummary.Log(log, restoreSummaries); } - return restoreSummaries.All(x => x.Success); + return restoreSummaries; } finally { diff --git a/src/NuGet.Core/NuGet.Build.Tasks/NuGet.targets b/src/NuGet.Core/NuGet.Build.Tasks/NuGet.targets index cba5374be15..ea427c30e29 100644 --- a/src/NuGet.Core/NuGet.Build.Tasks/NuGet.targets +++ b/src/NuGet.Core/NuGet.Build.Tasks/NuGet.targets @@ -191,6 +191,9 @@ Copyright (c) .NET Foundation. All rights reserved. RestorePackagesConfig="$(RestorePackagesConfig)" EmbedFilesInBinlog="$(RestoreEmbedFilesInBinlog)"> + + + diff --git a/src/NuGet.Core/NuGet.Build.Tasks/RestoreTask.cs b/src/NuGet.Core/NuGet.Build.Tasks/RestoreTask.cs index 4cc74fc8730..5d71c1af81c 100644 --- a/src/NuGet.Core/NuGet.Build.Tasks/RestoreTask.cs +++ b/src/NuGet.Core/NuGet.Build.Tasks/RestoreTask.cs @@ -88,6 +88,30 @@ public class RestoreTask : Microsoft.Build.Utilities.Task, ICancelableTask, IDis [Output] public ITaskItem[] EmbedInBinlog { get; set; } + /// + /// Gets or sets the number of projects that were considered for this restore operation. + /// + /// + /// Projects that no-op (were already up to date) are included. + /// + [Output] + public int ProjectsRestored { get; set; } + + /// + /// Gets or sets the number of projects that were already up to date. + /// + [Output] + public int ProjectsAlreadyUpToDate { get; set; } + + /// + /// Gets or sets the number of projects that NuGetAudit scanned. + /// + /// + /// NuGetAudit does not run on no-op restores, or when no vulnerability database can be downloaded. + /// + [Output] + public int ProjectsAudited { get; set; } + /// /// Gets or sets a value indicating whether to embed files produced by restore in the MSBuild binary logger. /// 0 = Nothing @@ -148,7 +172,7 @@ private async Task ExecuteAsync(Common.ILogger log) log.LogInformation(Strings.Log_RestoreNoCacheInformation); } - return await BuildTasksUtility.RestoreAsync( + var restoreSummaries = await BuildTasksUtility.RestoreAsync( dependencyGraphSpec: dgFile, interactive: Interactive, recursive: RestoreRecursive, @@ -161,6 +185,20 @@ private async Task ExecuteAsync(Common.ILogger log) restorePC: RestorePackagesConfig, log: log, cancellationToken: _cts.Token); + + int upToDate = 0; + int audited = 0; + foreach (var summary in restoreSummaries) + { + if (summary.NoOpRestore) { upToDate++; } + if (summary.AuditRan) { audited++; } + } + + ProjectsRestored = restoreSummaries.Count; + ProjectsAlreadyUpToDate = upToDate; + ProjectsAudited = audited; + + return restoreSummaries.All(s => s.Success); } public void Cancel() diff --git a/src/NuGet.Core/NuGet.Commands/PublicAPI/net472/PublicAPI.Unshipped.txt b/src/NuGet.Core/NuGet.Commands/PublicAPI/net472/PublicAPI.Unshipped.txt index 7dc5c58110b..4c2791b2d81 100644 --- a/src/NuGet.Core/NuGet.Commands/PublicAPI/net472/PublicAPI.Unshipped.txt +++ b/src/NuGet.Core/NuGet.Commands/PublicAPI/net472/PublicAPI.Unshipped.txt @@ -1 +1,2 @@ #nullable enable +NuGet.Commands.RestoreSummary.AuditRan.get -> bool diff --git a/src/NuGet.Core/NuGet.Commands/PublicAPI/netcoreapp5.0/PublicAPI.Unshipped.txt b/src/NuGet.Core/NuGet.Commands/PublicAPI/netcoreapp5.0/PublicAPI.Unshipped.txt index 7dc5c58110b..4c2791b2d81 100644 --- a/src/NuGet.Core/NuGet.Commands/PublicAPI/netcoreapp5.0/PublicAPI.Unshipped.txt +++ b/src/NuGet.Core/NuGet.Commands/PublicAPI/netcoreapp5.0/PublicAPI.Unshipped.txt @@ -1 +1,2 @@ #nullable enable +NuGet.Commands.RestoreSummary.AuditRan.get -> bool diff --git a/src/NuGet.Core/NuGet.Commands/PublicAPI/netstandard2.0/PublicAPI.Unshipped.txt b/src/NuGet.Core/NuGet.Commands/PublicAPI/netstandard2.0/PublicAPI.Unshipped.txt index 7dc5c58110b..4c2791b2d81 100644 --- a/src/NuGet.Core/NuGet.Commands/PublicAPI/netstandard2.0/PublicAPI.Unshipped.txt +++ b/src/NuGet.Core/NuGet.Commands/PublicAPI/netstandard2.0/PublicAPI.Unshipped.txt @@ -1 +1,2 @@ #nullable enable +NuGet.Commands.RestoreSummary.AuditRan.get -> bool diff --git a/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs b/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs index 173d179821e..3052cce2942 100644 --- a/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs +++ b/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs @@ -345,9 +345,10 @@ await _logger.LogAsync(RestoreLogMessage.CreateWarning(NuGetLogCode.NU1803, _request.Project.FilePath, _logger); telemetry.TelemetryEvent[AuditEnabled] = auditEnabled ? "enabled" : "disabled"; + bool auditRan = false; if (auditEnabled) { - await PerformAuditAsync(graphs, telemetry, token); + auditRan = await PerformAuditAsync(graphs, telemetry, token); } telemetry.StartIntervalMeasure(); @@ -511,11 +512,19 @@ await _logger.LogAsync(RestoreLogMessage.CreateWarning(NuGetLogCode.NU1803, dependencyGraphSpecFilePath: NoOpRestoreUtilities.GetPersistedDGSpecFilePath(_request), dependencyGraphSpec: _request.DependencyGraphSpec, _request.ProjectStyle, - restoreTime.Elapsed); + restoreTime.Elapsed) + { + AuditRan = auditRan + }; } } - private async Task PerformAuditAsync(IEnumerable graphs, TelemetryActivity telemetry, CancellationToken token) + /// Run NuGetAudit on the project's resolved restore graphs, and log messages and telemetry with the results. + /// The resolved package graphs, one for each project target framework. + /// The to log NuGetAudit telemetry to. + /// A to cancel obtaining a vulnerability database. Once the database is downloaded, audit is quick to complete. + /// False if no vulnerability database could be found (so packages were not scanned for vulnerabilities), true otherwise. + private async Task PerformAuditAsync(IEnumerable graphs, TelemetryActivity telemetry, CancellationToken token) { telemetry.StartIntervalMeasure(); var audit = new AuditUtility( @@ -524,7 +533,7 @@ private async Task PerformAuditAsync(IEnumerable graphs, Tel graphs, _request.DependencyProviders.VulnerabilityInfoProviders, _logger); - await audit.CheckPackageVulnerabilitiesAsync(token); + bool auditRan = await audit.CheckPackageVulnerabilitiesAsync(token); telemetry.TelemetryEvent[AuditLevel] = (int)audit.MinSeverity; telemetry.TelemetryEvent[AuditMode] = AuditUtility.GetString(audit.AuditMode); @@ -554,6 +563,8 @@ private async Task PerformAuditAsync(IEnumerable graphs, Tel if (audit.GenerateOutputDurationSeconds.HasValue) { telemetry.TelemetryEvent[AuditDurationOutput] = audit.GenerateOutputDurationSeconds.Value; } telemetry.EndIntervalMeasure(AuditDurationTotal); + return auditRan; + void AddPackagesList(TelemetryActivity telemetry, string eventName, List packages) { List result = new List(packages.Count); diff --git a/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreResult.cs b/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreResult.cs index e66c9501f0e..7a332bd02e5 100644 --- a/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreResult.cs +++ b/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreResult.cs @@ -82,6 +82,9 @@ public class RestoreResult : IRestoreResult /// internal PackagesLockFile _newPackagesLockFile { get; } + /// + internal bool AuditRan { get; init; } + private readonly string _dependencyGraphSpecFilePath; diff --git a/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreSummary.cs b/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreSummary.cs index 1c181620f46..b3a65177484 100644 --- a/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreSummary.cs +++ b/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreSummary.cs @@ -26,6 +26,18 @@ public class RestoreSummary public int InstallCount { get; } + /// + /// A boolean that specifies if NuGetAudit verified packages for known vulnerabilities + /// + /// This could be false if NuGetAudit is disabled, if + /// + /// NuGetAudit is disabled + /// Project is already up to date (no-op restore) + /// No sources provided a vulnerability database + /// + /// + public bool AuditRan { get; } + /// /// All the warnings and errors that were produced as a result of the restore. /// @@ -59,6 +71,7 @@ public RestoreSummary( .AsReadOnly(); InstallCount = result.GetAllInstalled().Count; Errors = errors.ToArray(); + AuditRan = result.AuditRan; } public RestoreSummary( diff --git a/src/NuGet.Core/NuGet.Commands/RestoreCommand/Utility/AuditUtility.cs b/src/NuGet.Core/NuGet.Commands/RestoreCommand/Utility/AuditUtility.cs index d80fb0d3d17..7ea699a5d27 100644 --- a/src/NuGet.Core/NuGet.Commands/RestoreCommand/Utility/AuditUtility.cs +++ b/src/NuGet.Core/NuGet.Commands/RestoreCommand/Utility/AuditUtility.cs @@ -78,12 +78,13 @@ public AuditUtility( } } - public async Task CheckPackageVulnerabilitiesAsync(CancellationToken cancellationToken) + public async Task CheckPackageVulnerabilitiesAsync(CancellationToken cancellationToken) { // Performance: Early exit if restore graph does not contain any packages. if (!HasPackages()) { - return; + // No packages means we've validated there are none with known vulnerabilities. + return true; } Stopwatch stopwatch = Stopwatch.StartNew(); @@ -94,10 +95,11 @@ public async Task CheckPackageVulnerabilitiesAsync(CancellationToken cancellatio // Performance: Early exit if there's no vulnerability data to check packages against. if (allVulnerabilityData is null || !AnyVulnerabilityDataFound(allVulnerabilityData)) { - return; + return false; } CheckPackageVulnerabilities(allVulnerabilityData); + return true; bool HasPackages() { diff --git a/test/NuGet.Core.FuncTests/Dotnet.Integration.Test/DotnetRestoreTests.cs b/test/NuGet.Core.FuncTests/Dotnet.Integration.Test/DotnetRestoreTests.cs index 7749ac72036..cfb6cad518b 100644 --- a/test/NuGet.Core.FuncTests/Dotnet.Integration.Test/DotnetRestoreTests.cs +++ b/test/NuGet.Core.FuncTests/Dotnet.Integration.Test/DotnetRestoreTests.cs @@ -13,7 +13,10 @@ using NuGet.Packaging; using NuGet.Packaging.Signing; using NuGet.ProjectModel; +using NuGet.Protocol; using NuGet.Test.Utility; +using NuGet.Versioning; +using Test.Utility; using Test.Utility.Signing; using Xunit; using Xunit.Abstractions; @@ -2693,6 +2696,107 @@ public async Task DotnetRestore_WithMultiTargettingProject_WhenTargetFrameworkIs allTargets.Should().Contain(condition); } + // https://github.com/NuGet/Home/issues/13829 + [PlatformFact(SkipPlatform = Platform.Linux)] + public async Task DotnetRestore_WithServerProvidingAuditData_RestoreTaskReturnsCountProperties() + { + // Arrange + using var pathContext = new SimpleTestPathContext(); + using var mockServer = new FileSystemBackedV3MockServer(pathContext.PackageSource, sourceReportsVulnerabilities: true); + + mockServer.Vulnerabilities.Add( + "Insecure.Package", + new List<(Uri, PackageVulnerabilitySeverity, VersionRange)> { + (new Uri("https://contoso.test/advisories/12346"), PackageVulnerabilitySeverity.Critical, VersionRange.Parse("[1.0.0, 2.0.0)")) + }); + pathContext.Settings.RemoveSource("source"); + pathContext.Settings.AddSource("source", mockServer.ServiceIndexUri, allowInsecureConnectionsValue: "true"); + + var packageA1 = new SimpleTestPackageContext() + { + Id = "packageA", + Version = "1.0.0" + }; + + await SimpleTestPackageUtility.CreatePackagesAsync( + pathContext.PackageSource, + packageA1); + + var targetFramework = Constants.DefaultTargetFramework.GetShortFolderName(); + string csprojContents = GetProjectFileForRestoreTaskOutputTests(targetFramework); + var csprojPath = Path.Combine(pathContext.SolutionRoot, "test.csproj"); + File.WriteAllText(csprojPath, csprojContents); + + mockServer.Start(); + + // Act & Assert + + // First restore is fresh, so should not be skipped (no-op), and should run audit + _dotnetFixture.RunDotnetExpectSuccess(pathContext.SolutionRoot, "restore -p:ExpectedRestoreProjectCount=1 -p:ExpectedRestoreSkippedCount=0 -p:ExpectedRestoreProjectsAuditedCount=1"); + + // Second restore is no-op, so should be skipped, and audit not run + _dotnetFixture.RunDotnetExpectSuccess(pathContext.SolutionRoot, "restore -p:ExpectedRestoreProjectCount=1 -p:ExpectedRestoreSkippedCount=1 -p:ExpectedRestoreProjectsAuditedCount=0"); + + mockServer.Stop(); + } + + [Fact] + public async Task DotnetRestore_WithServerWithoutAuditData_RestoreTaskReturnsCountProperties() + { + // Arrange + using var pathContext = new SimpleTestPathContext(); + + var packageA1 = new SimpleTestPackageContext() + { + Id = "packageA", + Version = "1.0.0" + }; + + await SimpleTestPackageUtility.CreatePackagesAsync( + pathContext.PackageSource, + packageA1); + + var targetFramework = Constants.DefaultTargetFramework.GetShortFolderName(); + string csprojContents = GetProjectFileForRestoreTaskOutputTests(targetFramework); + var csprojPath = Path.Combine(pathContext.SolutionRoot, "test.csproj"); + File.WriteAllText(csprojPath, csprojContents); + + // Act & Assert + + // First restore is fresh, so should not be skipped (no-op), and should run audit + _dotnetFixture.RunDotnetExpectSuccess(pathContext.SolutionRoot, "restore -p:ExpectedRestoreProjectCount=1 -p:ExpectedRestoreSkippedCount=0 -p:ExpectedRestoreProjectsAuditedCount=0"); + + // Second restore is no-op, so should be skipped, and audit not run + _dotnetFixture.RunDotnetExpectSuccess(pathContext.SolutionRoot, "restore -p:ExpectedRestoreProjectCount=1 -p:ExpectedRestoreSkippedCount=1 -p:ExpectedRestoreProjectsAuditedCount=0"); + } + + private string GetProjectFileForRestoreTaskOutputTests(string targetFramework) + { + string csprojContents = @$" + + {targetFramework} + + + + + + + + + + + +"; + return csprojContents; + } + private void AssertRelatedProperty(IList items, string path, string related) { var item = items.Single(i => i.Path.Equals(path)); diff --git a/test/TestUtilities/Test.Utility/FileSystemBackedV3MockServer.cs b/test/TestUtilities/Test.Utility/FileSystemBackedV3MockServer.cs index 0e185ebffab..83a80aaba20 100644 --- a/test/TestUtilities/Test.Utility/FileSystemBackedV3MockServer.cs +++ b/test/TestUtilities/Test.Utility/FileSystemBackedV3MockServer.cs @@ -158,7 +158,7 @@ private Action ServerHandlerV3(HttpListenerRequest request return new Action(response => { response.ContentType = "application/json"; - var vulnerabilityJson = FeedUtilities.CreateVulnerabilitiesJson(Uri + "/vulnerability/vulnerability.json"); + var vulnerabilityJson = FeedUtilities.CreateVulnerabilitiesJson(Uri + "vulnerability/vulnerability.json"); SetResponseContent(response, vulnerabilityJson.ToString()); }); } diff --git a/test/TestUtilities/Test.Utility/SimpleTestSetup/SimpleTestSettingsContext.cs b/test/TestUtilities/Test.Utility/SimpleTestSetup/SimpleTestSettingsContext.cs index d4028cd4b67..d17ced16f80 100644 --- a/test/TestUtilities/Test.Utility/SimpleTestSetup/SimpleTestSettingsContext.cs +++ b/test/TestUtilities/Test.Utility/SimpleTestSetup/SimpleTestSettingsContext.cs @@ -112,11 +112,13 @@ private static XDocument GetEmptyConfig() var config = GetOrAddSection(doc, "config"); var packageSources = GetOrAddSection(doc, "packageSources"); + var auditSources = GetOrAddSection(doc, "auditSources"); var disabledSources = GetOrAddSection(doc, "disabledPackageSources"); var fallbackFolders = GetOrAddSection(doc, "fallbackPackageFolders"); var packageSourceMapping = GetOrAddSection(doc, "packageSourceMapping"); packageSources.Add(new XElement(XName.Get("clear"))); + auditSources.Add(new XElement(XName.Get("clear"))); disabledSources.Add(new XElement(XName.Get("clear"))); packageSourceMapping.Add(new XElement(XName.Get("clear")));