Fix bug causing reduced vulnerabilities version ranges to not flow into V3 (#9229)

Entity Framework clears dependent collections when the parent entity is removed from the DB context. This means it can't be used to do further cleanup. The database was properly updated (based on cascading deletion logic from EF) but the packages did not get their LastEdited timestamp updated meaning Db2Catalog did not flow the latest (no longer vulnerable) state into V3.
Progress on NuGet/Engineering#4566

After running VerifyGitHubVulnerabilities and reflowing any affected packages, DEV, INT, and PROD environments are now fully consistent with the GitHub advisory database.

Author: joelverhagen

src/GitHubVulnerabilities2Db/Ingest/AdvisoryIngestor.cs

@@ -6,6 +6,7 @@
 using System.Linq;
 using System.Threading.Tasks;
 using GitHubVulnerabilities2Db.GraphQL;
+using Microsoft.Extensions.Logging;
 using NuGet.Services.Entities;
 using NuGetGallery;
 
@@ -15,19 +16,24 @@ public class AdvisoryIngestor : IAdvisoryIngestor
     {
         private readonly IPackageVulnerabilitiesManagementService _packageVulnerabilityService;
         private readonly IGitHubVersionRangeParser _gitHubVersionRangeParser;
+        private readonly ILogger<AdvisoryIngestor> _logger;
 
         public AdvisoryIngestor(
             IPackageVulnerabilitiesManagementService packageVulnerabilityService,
-            IGitHubVersionRangeParser gitHubVersionRangeParser)
+            IGitHubVersionRangeParser gitHubVersionRangeParser,
+            ILogger<AdvisoryIngestor> logger)
         {
             _packageVulnerabilityService = packageVulnerabilityService ?? throw new ArgumentNullException(nameof(packageVulnerabilityService));
             _gitHubVersionRangeParser = gitHubVersionRangeParser ?? throw new ArgumentNullException(nameof(gitHubVersionRangeParser));
+            _logger = logger ?? throw new ArgumentNullException(nameof(logger));
         }
 
         public async Task IngestAsync(IReadOnlyList<SecurityAdvisory> advisories)
         {
-            foreach (var advisory in advisories)
+            for (int i = 0; i < advisories.Count; i++)
             {
+                _logger.LogInformation("Processing advisory {Current} of {Total}...", i + 1, advisories.Count);
+                SecurityAdvisory advisory = advisories[i];
                 var vulnerabilityTuple = FromAdvisory(advisory);
                 var vulnerability = vulnerabilityTuple.Item1;
                 var wasWithdrawn = vulnerabilityTuple.Item2;

src/NuGetGallery.Services/PackageManagement/PackageVulnerabilitiesManagementService.cs

@@ -212,9 +212,9 @@ private void UpdateRangesOfPackageVulnerability(PackageVulnerability vulnerabili
                         existingRange.PackageVersionRange,
                         vulnerability.GitHubDatabaseKey);
 
+                    packagesToUpdate.UnionWith(existingRange.Packages);
                     _entitiesContext.VulnerableRanges.Remove(existingRange);
                     existingVulnerability.AffectedRanges.Remove(existingRange);
-                    packagesToUpdate.UnionWith(existingRange.Packages);
                 }
                 else
                 {

src/VerifyGitHubVulnerabilities/Job.cs

@@ -2,6 +2,7 @@
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System;
+using System.Linq;
 using System.Net.Http;
 using System.Threading;
 using System.Threading.Tasks;
@@ -28,14 +29,14 @@ class Job : JsonConfigurationJob
 
         public override async Task Run()
         {
-            Console.Write("Fetching vulnerabilities from GitHub...");
+            Console.WriteLine("Fetching vulnerabilities from GitHub...");
             var advisoryQueryService = _serviceProvider.GetRequiredService<IAdvisoryQueryService>();
             var advisories = await advisoryQueryService.GetAdvisoriesSinceAsync(DateTimeOffset.MinValue, CancellationToken.None);
             Console.WriteLine($" FOUND {advisories.Count} advisories.");
 
             Console.WriteLine("Fetching vulnerabilities from DB...");
             var ingestor = _serviceProvider.GetRequiredService<IAdvisoryIngestor>();
-            await ingestor.IngestAsync(advisories);
+            await ingestor.IngestAsync(advisories.OrderBy(x => x.DatabaseId).ToList());
 
             var verifier = _serviceProvider.GetRequiredService<IPackageVulnerabilitiesVerifier>();
             Console.WriteLine(verifier.HasErrors ? 
@@ -82,7 +83,7 @@ protected void ConfigureGalleryServices(ContainerBuilder containerBuilder)
                 .Register(ctx =>
                 {
                     var connection = CreateSqlConnection<GalleryDbConfiguration>();
-                    return new EntitiesContext(connection, false);
+                    return new EntitiesContext(connection, readOnly: true);
                 })
                 .As<IEntitiesContext>();
 
@@ -97,10 +98,6 @@ protected void ConfigureQueryServices(ContainerBuilder containerBuilder)
                 .RegisterInstance(_client)
                 .As<HttpClient>();
 
-            containerBuilder
-                .RegisterGeneric(typeof(NullLogger<>))
-                .As(typeof(ILogger<>));
-
             containerBuilder
                 .RegisterType<VerifyGitHubVulnerabilities.GraphQL.QueryService>()
                 .As<IQueryService>();

src/VerifyGitHubVulnerabilities/README.md

@@ -20,7 +20,9 @@ The easiest way to run the tool if you are on the nuget.org team is to use the D
 1. Install the certificate used to authenticate as our client AAD app registration into your `CurrentUser` certificate store.
 1. Clone our internal [`NuGetDeployment`](https://nuget.visualstudio.com/DefaultCollection/NuGetMicrosoft/_git/NuGetDeploymentp) repository.
 1. Take a copy of the [DEV GitHubVulnerabilities2Db appsettings.json](https://nuget.visualstudio.com/NuGetMicrosoft/_git/NuGetDeployment?path=%2Fsrc%2FJobs%2FNuGet.Jobs.Cloud%2FJobs%2FGitHubVulnerabilities2Db%2FDEV%2Fnorthcentralus%2Fappsettings.json) file and place it in the same directory as the `verifygithubvulnerabilities.exe`. This will use our secrets to authenticate to the SQL server (this file also contains a reference to the secret used for the access token to GitHub).
-1. Run as per above.
+1. Set the following property, in the `appsettings.json`, under the `"Initialization"` object: `"NuGetV3Index": "https://apidev.nugettest.org/v3/index.json"`.
+1. Overwrite the Gallery DB `"ConnectionString"` property to be the connection string from [DEV USSC (read-only) gallery config](https://nuget.visualstudio.com/NuGetMicrosoft/_git/NuGetDeployment?path=/src/Gallery/ExpressV2/ServiceSpecs/Parameters.AS/DEV/USSC/NuGet.Gallery.Parameters.json&version=GBmaster). This provides an additional protection to ensure the job runs as read-only.
+2. Run as per above.
 
 ## Algorithm
 

src/VerifyGitHubVulnerabilities/Verify/PackageVulnerabilitiesVerifier.cs

@@ -258,7 +258,7 @@ private async Task VerifyVulnerabilityForRangeAsync(
                     {
                         Console.Error.WriteLine(
                             $@"[Metadata] Vulnerability advisory {advisoryDatabaseKey
-                                }, version {versionMetadata} of package {packageId} is marked vulnerable and is not in a vulnerable range!");
+                                }, version {versionMetadata.Identity.Version} of package {packageId} is marked vulnerable and is not in a vulnerable range!");
                         HasErrors = true;
                     }
                 }

tests/GitHubVulnerabilities2Db.Facts/AdvisoryIngestorFacts.cs

@@ -6,6 +6,7 @@
 using System.Threading.Tasks;
 using GitHubVulnerabilities2Db.GraphQL;
 using GitHubVulnerabilities2Db.Ingest;
+using Microsoft.Extensions.Logging;
 using Moq;
 using NuGet.Services.Entities;
 using NuGet.Versioning;
@@ -137,7 +138,8 @@ public MethodFacts()
                 GitHubVersionRangeParserMock = new Mock<IGitHubVersionRangeParser>();
                 Ingestor = new AdvisoryIngestor(
                     PackageVulnerabilityServiceMock.Object,
-                    GitHubVersionRangeParserMock.Object);
+                    GitHubVersionRangeParserMock.Object,
+                    Mock.Of<ILogger<AdvisoryIngestor>>());
             }
 
             public Mock<IPackageVulnerabilitiesManagementService> PackageVulnerabilityServiceMock { get; }

tests/NuGetGallery.Facts/Services/PackageVulnerabilitiesManagementServiceFacts.cs

@@ -481,6 +481,14 @@ public MethodFacts()
                     .Verifiable();
 
                 Context.SetupDatabase(_databaseMock.Object);
+
+                // Entity Framework clears the collection. We use the Packages collection to know which packages to
+                // mark as updated so it's important to use the collection prior to removing the range.
+                // Related to: https://github.com/NuGet/Engineering/issues/4566
+                Mock
+                    .Get(Context.VulnerableRanges)
+                    .Setup(x => x.Remove(It.IsAny<VulnerablePackageVersionRange>()))
+                    .Callback<VulnerablePackageVersionRange>(x => x.Packages.Clear());
             }
 
             private Mock<IDbContextTransaction> _transactionMock { get; }