Rename PackageVulnerability's ReferenceUrl to AdvisoryUrl and make it required

src/GitHubVulnerabilities2Db/Collector/AdvisoryQueryBuilder.cs

@@ -24,9 +24,6 @@ public string CreateSecurityAdvisoriesQuery(DateTimeOffset? updatedSince = null,
         ghsaId
         severity
         updatedAt
-        references {
-          url
-        }
         " + CreateVulnerabilitiesConnectionQuery() + @"
       }
     }
@@ -37,9 +34,6 @@ public string CreateSecurityAdvisoryQuery(SecurityAdvisory advisory)
             => @"
 {
   securityAdvisory(ghsaId: " + advisory.GhsaId + @") {
-    references {
-      url
-    }
     severity
     updatedAt
     identifiers {

src/GitHubVulnerabilities2Db/GitHubVulnerabilities2Db.csproj

@@ -61,6 +61,7 @@
     <Compile Include="GraphQL\SecurityAdvisory.cs" />
     <Compile Include="GraphQL\QueryResponse.cs" />
     <Compile Include="GraphQL\QueryService.cs" />
+    <Compile Include="GraphQL\SecurityAdvisoryExtensions.cs" />
     <Compile Include="GraphQL\SecurityVulnerability.cs" />
     <Compile Include="Ingest\AdvisoryIngestor.cs" />
     <Compile Include="Ingest\GitHubVersionRangeParser.cs" />

src/GitHubVulnerabilities2Db/GraphQL/SecurityAdvisory.cs

@@ -2,7 +2,6 @@
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System;
-using System.Collections.Generic;
 
 namespace GitHubVulnerabilities2Db.GraphQL
 {
@@ -12,19 +11,15 @@ namespace GitHubVulnerabilities2Db.GraphQL
     public class SecurityAdvisory : INode
     {
         public int DatabaseId { get; set; }
+
+        /// <summary>
+        /// The GitHub SecurityAdvisory ID in the format 'GHSA-xxxx-xxxx-xxxx'.
+        /// </summary>
         public string GhsaId { get; set; }
-        public IEnumerable<SecurityAdvisoryReference> References { get; set; }
+
         public string Severity { get; set; }
         public DateTimeOffset UpdatedAt { get; set; }
         public DateTimeOffset? WithdrawnAt { get; set; }
         public ConnectionResponseData<SecurityVulnerability> Vulnerabilities { get; set; }
     }
-
-    /// <summary>
-    /// https://developer.github.com/v4/object/securityadvisoryreference/
-    /// </summary>
-    public class SecurityAdvisoryReference
-    {
-        public string Url { get; set; }
-    }
 }

src/GitHubVulnerabilities2Db/GraphQL/SecurityAdvisoryExtensions.cs

@@ -0,0 +1,35 @@
+// 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;
+
+namespace GitHubVulnerabilities2Db.GraphQL
+{
+    public static class SecurityAdvisoryExtensions
+    {
+        /// <summary>
+        /// Gets the permalink for the security advisory.
+        /// </summary>
+        /// <param name="advisory"></param>
+        public static string GetPermalink(this SecurityAdvisory advisory)
+        {
+            if (advisory == null)
+            {
+                throw new ArgumentNullException(nameof(advisory));
+            }
+
+            if (string.IsNullOrEmpty(advisory.GhsaId))
+            {
+                throw new ArgumentException("Cannot create a permalink for security advisory without GHSA ID.", nameof(advisory.GhsaId));
+            }
+
+            // This is the permalink format used by GitHub for security advisory pages.
+            // Note that the GHSA ID part of the URL is case-sensitive, so we pass in the GHSA ID as-is.
+
+            // Todo: remove this hard-coded work-around when the "permalink" field becomes available on the GH API.
+            // See: https://developer.github.com/v4/object/securityadvisory/
+
+            return $"https://github.com/advisories/{advisory.GhsaId}";
+        }
+    }
+}

src/GitHubVulnerabilities2Db/Ingest/AdvisoryIngestor.cs

@@ -41,7 +41,7 @@ private Tuple<PackageVulnerability, bool> FromAdvisory(SecurityAdvisory advisory
             {
                 GitHubDatabaseKey = advisory.DatabaseId,
                 Severity = (PackageVulnerabilitySeverity)Enum.Parse(typeof(PackageVulnerabilitySeverity), advisory.Severity, ignoreCase: true),
-                ReferenceUrl = advisory.References.FirstOrDefault()?.Url
+                AdvisoryUrl = advisory.GetPermalink()
             };
 
             foreach (var securityVulnerability in advisory.Vulnerabilities?.Edges?.Select(e => e.Node) ?? Enumerable.Empty<SecurityVulnerability>())

src/NuGet.Services.Entities/PackageVulnerability.cs

@@ -28,9 +28,11 @@ public PackageVulnerability()
         public int GitHubDatabaseKey { get; set; }
 
         /// <summary>
-        /// A URL with more metadata about the vulnerability.
+        /// A URL with more metadata about security advisory linked to this vulnerability.
+        /// 
+        /// Required.
         /// </summary>
-        public string ReferenceUrl { get; set; }
+        public string AdvisoryUrl { get; set; }
 
         public PackageVulnerabilitySeverity Severity { get; set; }
 

src/NuGetGallery.Core/Entities/EntitiesContext.cs

@@ -462,6 +462,10 @@ protected override void OnModelCreating(DbModelBuilder modelBuilder)
                 .HasIndex(v => v.GitHubDatabaseKey)
                 .IsUnique();
 
+            modelBuilder.Entity<PackageVulnerability>()
+                .Property(pv => pv.AdvisoryUrl)
+                .IsRequired();
+
             modelBuilder.Entity<VulnerablePackageVersionRange>()
                 .HasKey(pv => pv.Key)
                 .HasMany(pv => pv.Packages)

src/NuGetGallery.Services/PackageManagement/PackageVulnerabilityService.cs

@@ -185,9 +185,9 @@ private bool UpdatePackageVulnerabilityMetadata(PackageVulnerability vulnerabili
                 wasUpdated = true;
             }
 
-            if (vulnerability.ReferenceUrl != existingVulnerability.ReferenceUrl)
+            if (vulnerability.AdvisoryUrl != existingVulnerability.AdvisoryUrl)
             {
-                existingVulnerability.ReferenceUrl = vulnerability.ReferenceUrl;
+                existingVulnerability.AdvisoryUrl = vulnerability.AdvisoryUrl;
                 wasUpdated = true;
             }
 

src/NuGetGallery/Migrations/201912051135094_PackageVulnerabilityAdvisoryUrlRenameAndRequired.cs

@@ -0,0 +1,20 @@
+namespace NuGetGallery.Migrations
+{
+    using System;
+    using System.Data.Entity.Migrations;
+
+    public partial class PackageVulnerabilityAdvisoryUrlRenameAndRequired : DbMigration
+    {
+        public override void Up()
+        {
+            RenameColumn("dbo.PackageVulnerabilities", "ReferenceUrl", "AdvisoryUrl");
+            AlterColumn("dbo.PackageVulnerabilities", "AdvisoryUrl", c => c.String(nullable: false));
+        }
+
+        public override void Down()
+        {
+            AlterColumn("dbo.PackageVulnerabilities", "AdvisoryUrl", c => c.String());
+            RenameColumn("dbo.PackageVulnerabilities", "AdvisoryUrl", "ReferenceUrl");
+        }
+    }
+}

src/NuGetGallery/NuGetGallery.csproj

@@ -282,6 +282,10 @@
     <Compile Include="Migrations\201911160032363_AddCredentialRevocationSourceKeyColumn.designer.cs">
       <DependentUpon>201911160032363_AddCredentialRevocationSourceKeyColumn.cs</DependentUpon>
     </Compile>
+    <Compile Include="Migrations\201912051135094_PackageVulnerabilityAdvisoryUrlRenameAndRequired.cs" />
+    <Compile Include="Migrations\201912051135094_PackageVulnerabilityAdvisoryUrlRenameAndRequired.designer.cs">
+      <DependentUpon>201912051135094_PackageVulnerabilityAdvisoryUrlRenameAndRequired.cs</DependentUpon>
+    </Compile>
     <Compile Include="Services\ConfigurationIconFileProvider.cs" />
     <Compile Include="Services\IconUrlDeprecationValidationMessage.cs" />
     <Compile Include="Services\GravatarProxyResult.cs" />
@@ -1652,6 +1656,9 @@
     <EmbeddedResource Include="Migrations\201911160032363_AddCredentialRevocationSourceKeyColumn.resx">
       <DependentUpon>201911160032363_AddCredentialRevocationSourceKeyColumn.cs</DependentUpon>
     </EmbeddedResource>
+    <EmbeddedResource Include="Migrations\201912051135094_PackageVulnerabilityAdvisoryUrlRenameAndRequired.resx">
+      <DependentUpon>201912051135094_PackageVulnerabilityAdvisoryUrlRenameAndRequired.cs</DependentUpon>
+    </EmbeddedResource>
     <EmbeddedResource Include="OData\QueryAllowed\Data\apiv1packages.json" />
     <EmbeddedResource Include="OData\QueryAllowed\Data\apiv1search.json" />
     <EmbeddedResource Include="OData\QueryAllowed\Data\apiv2getupdates.json" />

tests/GitHubVulnerabilities2Db.Facts/AdvisoryIngestorFacts.cs

@@ -42,7 +42,6 @@ public async Task IngestsAdvisoryWithoutVulnerability(bool withdrawn)
                     DatabaseId = 1,
                     GhsaId = "ghsa",
                     Severity = "MODERATE",
-                    References = new[] { new SecurityAdvisoryReference { Url = "https://vulnerable" } },
                     WithdrawnAt = withdrawn ? new DateTimeOffset() : (DateTimeOffset?)null
                 };
 
@@ -52,7 +51,7 @@ public async Task IngestsAdvisoryWithoutVulnerability(bool withdrawn)
                     {
                         Assert.Equal(advisory.DatabaseId, vulnerability.GitHubDatabaseKey);
                         Assert.Equal(PackageVulnerabilitySeverity.Moderate, vulnerability.Severity);
-                        Assert.Equal(advisory.References.Single().Url, vulnerability.ReferenceUrl);
+                        Assert.Equal(advisory.GetPermalink(), vulnerability.AdvisoryUrl);
                     })
                     .Returns(Task.CompletedTask)
                     .Verifiable();
@@ -85,7 +84,6 @@ public async Task IngestsAdvisory(bool withdrawn, bool vulnerabilityHasFirstPatc
                     DatabaseId = 1,
                     GhsaId = "ghsa",
                     Severity = "CRITICAL",
-                    References = new[] { new SecurityAdvisoryReference { Url = "https://vulnerable" } },
                     WithdrawnAt = withdrawn ? new DateTimeOffset() : (DateTimeOffset?)null,
                     Vulnerabilities = new ConnectionResponseData<SecurityVulnerability>
                     {
@@ -112,7 +110,7 @@ public async Task IngestsAdvisory(bool withdrawn, bool vulnerabilityHasFirstPatc
                     {
                         Assert.Equal(advisory.DatabaseId, vulnerability.GitHubDatabaseKey);
                         Assert.Equal(PackageVulnerabilitySeverity.Critical, vulnerability.Severity);
-                        Assert.Equal(advisory.References.Single().Url, vulnerability.ReferenceUrl);
+                        Assert.Equal(advisory.GetPermalink(), vulnerability.AdvisoryUrl);
 
                         var range = vulnerability.AffectedRanges.Single();
                         Assert.Equal(securityVulnerability.Package.Name, range.PackageId);

tests/NuGetGallery.Facts/Services/PackageVulnerabilityServiceFacts.cs

@@ -292,14 +292,14 @@ public async Task WithExistingVulnerability_NotWithdrawn_UpdatesPackages(
                 {
                     GitHubDatabaseKey = key,
                     Severity = wasUpdated ? newSeverity : severity,
-                    ReferenceUrl = wasUpdated ? newUrl : url
+                    AdvisoryUrl = wasUpdated ? newUrl : url
                 };
 
                 var existingVulnerability = new PackageVulnerability
                 {
                     GitHubDatabaseKey = key,
                     Severity = severity,
-                    ReferenceUrl = url
+                    AdvisoryUrl = url
                 };
 
                 Context.Vulnerabilities.Add(existingVulnerability);
@@ -450,7 +450,7 @@ public async Task WithExistingVulnerability_NotWithdrawn_UpdatesPackages(
                 Assert.Contains(range, Context.VulnerableRanges);
                 Assert.Equal(existingVulnerability, range.Vulnerability);
                 Assert.Equal(wasUpdated ? newSeverity : severity, existingVulnerability.Severity);
-                Assert.Equal(wasUpdated ? newUrl : url, existingVulnerability.ReferenceUrl);
+                Assert.Equal(wasUpdated ? newUrl : url, existingVulnerability.AdvisoryUrl);
                 Assert.Equal(
                     expectedVulnerablePackages.OrderBy(p => p.NormalizedVersion),
                     Context.VulnerableRanges.SelectMany(pv => pv.Packages).OrderBy(p => p.NormalizedVersion));