Respect security scanner schema

As outlined in https://gitlab.com/gitlab-org/gitlab/-/issues/339507 our
GraphQL mutation deviated from the security scanner schema is some
places. This attempts to collect the same dataset but falls back to some
hardcoded values if the required ones are not provided. To do this we
have to:

* use externalId and externalType in Vulnerabilites::Identifier creation
* collect additional data for Vulnerabilites::Scanner creation

Changelog: changed
EE: true

doc/api/graphql/reference/index.md

@@ -4677,13 +4677,13 @@ Input type: `VulnerabilityCreateInput`
 | <a id="mutationvulnerabilitycreatedismissedat"></a>`dismissedAt` | [`Time`](#time) | Timestamp of when the vulnerability state changed to dismissed (defaults to creation time if status is `dismissed`). |
 | <a id="mutationvulnerabilitycreateidentifiers"></a>`identifiers` | [`[VulnerabilityIdentifierInput!]!`](#vulnerabilityidentifierinput) | Array of CVE or CWE identifiers for the vulnerability. |
 | <a id="mutationvulnerabilitycreatemessage"></a>`message` | [`String`](#string) | Additional information about the vulnerability. |
+| <a id="mutationvulnerabilitycreatename"></a>`name` | [`String!`](#string) | Name of the vulnerability. |
 | <a id="mutationvulnerabilitycreateproject"></a>`project` | [`ProjectID!`](#projectid) | ID of the project to attach the vulnerability to. |
 | <a id="mutationvulnerabilitycreateresolvedat"></a>`resolvedAt` | [`Time`](#time) | Timestamp of when the vulnerability state changed to resolved (defaults to creation time if status is `resolved`). |
-| <a id="mutationvulnerabilitycreatescannername"></a>`scannerName` | [`String!`](#string) | Name of the security scanner used to discover the vulnerability. |
+| <a id="mutationvulnerabilitycreatescanner"></a>`scanner` | [`VulnerabilityScannerInput!`](#vulnerabilityscannerinput) | Information about the scanner used to discover the vulnerability. |
 | <a id="mutationvulnerabilitycreateseverity"></a>`severity` | [`VulnerabilitySeverity`](#vulnerabilityseverity) | Severity of the vulnerability (defaults to `unknown`). |
 | <a id="mutationvulnerabilitycreatesolution"></a>`solution` | [`String`](#string) | How to fix this vulnerability. |
 | <a id="mutationvulnerabilitycreatestate"></a>`state` | [`VulnerabilityState`](#vulnerabilitystate) | State of the vulnerability (defaults to `detected`). |
-| <a id="mutationvulnerabilitycreatetitle"></a>`title` | [`String!`](#string) | Title of the vulnerability. |
 
 #### Fields
 
@@ -18265,3 +18265,23 @@ A time-frame defined as a closed inclusive range of two dates.
 | <a id="vulnerabilityidentifierinputexternaltype"></a>`externalType` | [`String`](#string) | External type of the vulnerability identifier. |
 | <a id="vulnerabilityidentifierinputname"></a>`name` | [`String!`](#string) | Name of the vulnerability identifier. |
 | <a id="vulnerabilityidentifierinputurl"></a>`url` | [`String!`](#string) | URL of the vulnerability identifier. |
+
+### `VulnerabilityScannerInput`
+
+#### Arguments
+
+| Name | Type | Description |
+| ---- | ---- | ----------- |
+| <a id="vulnerabilityscannerinputid"></a>`id` | [`String!`](#string) | Unique ID that identifies the scanner. |
+| <a id="vulnerabilityscannerinputname"></a>`name` | [`String!`](#string) | Human readable value that identifies the analyzer, not required to be unique. |
+| <a id="vulnerabilityscannerinputurl"></a>`url` | [`String!`](#string) | Link to more information about the analyzer. |
+| <a id="vulnerabilityscannerinputvendor"></a>`vendor` | [`VulnerabilityScannerVendorInput`](#vulnerabilityscannervendorinput) | Information about vendor/maintainer of the scanner. |
+| <a id="vulnerabilityscannerinputversion"></a>`version` | [`String!`](#string) | Version of the scanner. |
+
+### `VulnerabilityScannerVendorInput`
+
+#### Arguments
+
+| Name | Type | Description |
+| ---- | ---- | ----------- |
+| <a id="vulnerabilityscannervendorinputname"></a>`name` | [`String!`](#string) | Name of the vendor/maintainer. |

ee/app/graphql/mutations/vulnerabilities/create.rb

@@ -11,17 +11,17 @@ module Mutations
         required: true,
         description: 'ID of the project to attach the vulnerability to.'
 
-      argument :title, GraphQL::Types::String,
+      argument :name, GraphQL::Types::String,
         required: true,
-        description: 'Title of the vulnerability.'
+        description: 'Name of the vulnerability.'
 
       argument :description, GraphQL::Types::String,
         required: true,
         description: 'Description of the vulnerability.'
 
-      argument :scanner_name, GraphQL::Types::String,
+      argument :scanner, Types::VulnerabilityScannerInputType,
         required: true,
-        description: 'Name of the security scanner used to discover the vulnerability.'
+        description: 'Information about the scanner used to discover the vulnerability.'
 
       argument :identifiers, [Types::VulnerabilityIdentifierInputType],
         required: true,
@@ -100,7 +100,7 @@ module Mutations
 
       def build_vulnerability_params(params)
         vulnerability_params = params.slice(*%i[
-          title
+          name
           state
           severity
           confidence
@@ -111,15 +111,11 @@ module Mutations
           resolved_at
           dismissed_at
           identifiers
+          scanner
         ])
 
-        scanner_params = {
-          name: params.fetch(:scanner_name)
-        }
-
         {
           vulnerability: vulnerability_params
-            .merge(scanner: scanner_params)
         }
       end
     end

ee/app/graphql/types/vulnerability_scanner_input_type.rb

+# frozen_string_literal: true
+module Types
+  class VulnerabilityScannerInputType < BaseInputObject
+    argument :id, GraphQL::Types::String,
+          description: 'Unique ID that identifies the scanner.',
+          required: true
+
+    argument :name, GraphQL::Types::String,
+          description: 'Human readable value that identifies the analyzer, not required to be unique.',
+          required: true
+
+    argument :url, GraphQL::Types::String,
+          description: 'Link to more information about the analyzer.',
+          required: true
+
+    argument :vendor, Types::VulnerabilityScannerVendorInputType,
+          description: 'Information about vendor/maintainer of the scanner.',
+          required: false
+
+    argument :version, GraphQL::Types::String,
+          description: 'Version of the scanner.',
+          required: true
+  end
+end

ee/app/graphql/types/vulnerability_scanner_vendor_input_type.rb

+# frozen_string_literal: true
+module Types
+  class VulnerabilityScannerVendorInputType < BaseInputObject
+    argument :name, GraphQL::Types::String,
+          description: 'Name of the vendor/maintainer.',
+          required: true
+  end
+end

ee/app/services/vulnerabilities/create_service_base.rb

@@ -44,7 +44,12 @@ module Vulnerabilities
         .merge(
           project: @project,
           author: @author,
-          title: vulnerability_hash[:title]&.truncate(::Issuable::TITLE_LENGTH_MAX),
+          # Our security report schema has name
+          # https://gitlab.com/gitlab-org/security-products/security-report-schemas/-/blob/master/src/security-report-format.json#L369
+          # Our database has title
+          # https://gitlab.com/gitlab-org/gitlab/blob/master/db/structure.sql#L20164
+          # We want the GraphQL mutation arguments to reflect the security scanner schema
+          title: vulnerability_hash[:name]&.truncate(::Issuable::TITLE_LENGTH_MAX),
           report_type: report_type
         )
 
@@ -61,8 +66,8 @@ module Vulnerabilities
     def initialize_identifiers(identifier_hashes)
       identifier_hashes.map do |identifier|
         name = identifier[:name]
-        external_type = map_external_type_from_name(name)
-        external_id = name
+        external_type = identifier[:external_type] || map_external_type_from_name(name)
+        external_id = identifier[:external_id] || name
         fingerprint = Digest::SHA1.hexdigest("#{external_type}:#{external_id}")
         url = identifier[:url]
 

ee/app/services/vulnerabilities/manually_create_service.rb

@@ -66,11 +66,14 @@ module Vulnerabilities
 
     # rubocop: disable CodeReuse/ActiveRecord
     def initialize_scanner(scanner_hash)
-      name = scanner_hash[:name]
-
-      Vulnerabilities::Scanner.find_or_initialize_by(name: name) do |s|
+      # In database Vulnerabilities::Scanner#id is autoincrementing primary key
+      # In the GraphQL mutation mutation arguments we want to respect the security scanner schema:
+      # https://gitlab.com/gitlab-org/security-products/security-report-schemas/-/blob/master/src/security-report-format.json#L339
+      # So the id provided to the mutation is actually external_id in our database
+      Vulnerabilities::Scanner.find_or_initialize_by(external_id: scanner_hash[:id]) do |s|
+        s.name = scanner_hash[:name]
+        s.vendor = scanner_hash.dig(:vendor, :name).to_s
         s.project = @project
-        s.external_id = Gitlab::Utils.slugify(name)
       end
     end
     # rubocop: enable CodeReuse/ActiveRecord

ee/spec/graphql/mutations/vulnerabilities/create_spec.rb

@@ -15,33 +15,49 @@ RSpec.describe Mutations::Vulnerabilities::Create do
   describe '#resolve' do
     using RSpec::Parameterized::TableSyntax
 
+    let(:identifier_attributes) do
+      {
+        name: "Test identifier",
+        url: "https://vulnerabilities.com/test"
+      }
+    end
+
+    let(:scanner_attributes) do
+      {
+        id: "my-custom-scanner",
+        name: "My Custom Scanner",
+        url: "https://superscanner.com",
+        vendor: vendor_attributes,
+        version: "21.37.00"
+      }
+    end
+
+    let(:vendor_attributes) do
+      {
+        name: "Custom Scanner Vendor"
+      }
+    end
+
+    let(:attributes) do
+      {
+        project: project_gid,
+        name: "Test vulnerability",
+        description: "Test vulnerability created via GraphQL",
+        scanner: scanner_attributes,
+        identifiers: [identifier_attributes],
+        state: "detected",
+        severity: "unknown",
+        confidence: "unknown",
+        solution: "rm -rf --no-preserve-root /",
+        message: "You can't fix this"
+      }
+    end
+
     context 'when a vulnerability with the same identifier already exists' do
       subject { resolve(described_class, args: attributes, ctx: { current_user: user }) }
 
       let(:project_gid) { GitlabSchema.id_from_object(project) }
 
-      let(:identifier_attributes) do
-        {
-          name: "Test identifier",
-          url: "https://vulnerabilities.com/test"
-        }
-      end
-
-      let(:attributes) do
-        {
-          project: project_gid,
-          title: "Test vulnerability",
-          description: "Test vulnerability created via GraphQL",
-          scanner_name: "My custom DAST scanner",
-          identifiers: [identifier_attributes],
-          state: "detected",
-          severity: "unknown",
-          confidence: "unknown",
-          solution: "rm -rf --no-preserve-root /",
-          message: "You can't fix this"
-        }
-      end
-
       before do
         project.add_developer(user)
         resolve(described_class, args: attributes, ctx: { current_user: user })
@@ -61,28 +77,6 @@ RSpec.describe Mutations::Vulnerabilities::Create do
 
       let(:project_gid) { GitlabSchema.id_from_object(project) }
 
-      let(:identifier_attributes) do
-        {
-          name: "Test identifier",
-          url: "https://vulnerabilities.com/test"
-        }
-      end
-
-      let(:attributes) do
-        {
-          project: project_gid,
-          title: "Test vulnerability",
-          description: "Test vulnerability created via GraphQL",
-          scanner_name: "My custom DAST scanner",
-          identifiers: [identifier_attributes],
-          state: "detected",
-          severity: "unknown",
-          confidence: "unknown",
-          solution: "rm -rf --no-preserve-root /",
-          message: "You can't fix this"
-        }
-      end
-
       context 'when feature flag is disabled' do
         before do
           stub_feature_flags(create_vulnerabilities_via_api: false)
@@ -118,9 +112,9 @@ RSpec.describe Mutations::Vulnerabilities::Create do
             let(:attributes) do
               {
                 project: project_gid,
-                title: "Test vulnerability",
+                name: "Test vulnerability",
                 description: "Test vulnerability created via GraphQL",
-                scanner_name: "My custom DAST scanner",
+                scanner: scanner_attributes,
                 identifiers: [identifier_attributes],
                 state: state,
                 severity: "unknown",

ee/spec/graphql/types/vulnerability_scanner_input_type_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Types::VulnerabilityScannerInputType do
+  specify { expect(described_class.graphql_name).to eq('VulnerabilityScannerInput') }
+
+  it 'has the correct arguments' do
+    expect(described_class.arguments.keys).to match_array(%w[id name url vendor version])
+  end
+end

ee/spec/graphql/types/vulnerability_scanner_vendor_input_type_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Types::VulnerabilityScannerVendorInputType do
+  specify { expect(described_class.graphql_name).to eq('VulnerabilityScannerVendorInput') }
+
+  it 'has the correct arguments' do
+    expect(described_class.arguments.keys).to match_array(%w[name])
+  end
+end

ee/spec/services/vulnerabilities/manually_create_service_spec.rb

@@ -23,13 +23,23 @@ RSpec.describe Vulnerabilities::ManuallyCreateService do
         stub_feature_flags(create_vulnerabilities_via_api: false)
       end
 
-      let(:scanner_params) do
+      let(:scanner_attributes) do
         {
-          name: "My manual scanner"
+          id: "my-custom-scanner",
+          name: "My Custom Scanner",
+          url: "https://superscanner.com",
+          vendor: vendor_attributes,
+          version: "21.37.00"
         }
       end
 
-      let(:identifier_params) do
+      let(:vendor_attributes) do
+        {
+          name: "Custom Scanner Vendor"
+        }
+      end
+
+      let(:identifier_attributes) do
         {
           name: "Test identifier 1",
           url: "https://test.com"
@@ -39,12 +49,12 @@ RSpec.describe Vulnerabilities::ManuallyCreateService do
       let(:params) do
         {
           vulnerability: {
-            title: "Test vulnerability",
+            name: "Test vulnerability",
             state: "detected",
             severity: "unknown",
             confidence: "unknown",
-            identifiers: [identifier_params],
-            scanner: scanner_params
+            identifiers: [identifier_attributes],
+            scanner: scanner_attributes
           }
         }
       end
@@ -62,34 +72,70 @@ RSpec.describe Vulnerabilities::ManuallyCreateService do
       end
 
       context 'with valid parameters' do
-        let(:scanner_params) do
+        let(:scanner_attributes) do
+          {
+            id: "my-custom-scanner",
+            name: "My Custom Scanner",
+            url: "https://superscanner.com",
+            vendor: vendor_attributes,
+            version: "21.37.00"
+          }
+        end
+
+        let(:vendor_attributes) do
           {
-            name: "My manual scanner"
+            name: "Custom Scanner Vendor"
           }
         end
 
-        let(:identifier_params) do
+        let(:identifier_attributes) do
           {
             name: "Test identifier 1",
             url: "https://test.com"
           }
         end
 
+        let(:identifier_fingerprint) do
+          Digest::SHA1.hexdigest("other:#{identifier_attributes[:name]}")
+        end
+
         let(:params) do
           {
             vulnerability: {
-              title: "Test vulnerability",
+              name: "Test vulnerability",
               state: "detected",
               severity: "unknown",
               confidence: "unknown",
-              identifiers: [identifier_params],
-              scanner: scanner_params
+              identifiers: [identifier_attributes],
+              scanner: scanner_attributes
             }
           }
         end
 
         let(:vulnerability) { subject.payload[:vulnerability] }
 
+        context 'with custom external_type and external_id' do
+          let(:identifier_attributes) do
+            {
+              name: "Test identifier 1",
+              url: "https://test.com",
+              external_id: "my external id",
+              external_type: "my external type"
+            }
+          end
+
+          let(:identifier_fingerprint) do
+            Digest::SHA1.hexdigest("#{identifier_attributes[:external_type]}:#{identifier_attributes[:external_id]}")
+          end
+
+          it 'uses them to create a Vulnerabilities::Identifier' do
+            primary_identifier = vulnerability.finding.primary_identifier
+            expect(primary_identifier.external_id).to eq(identifier_attributes.dig(:external_id))
+            expect(primary_identifier.external_type).to eq(identifier_attributes.dig(:external_type))
+            expect(primary_identifier.fingerprint).to eq(identifier_fingerprint)
+          end
+        end
+
         it 'does not exceed query limit' do
           expect { subject }.not_to exceed_query_limit(20)
         end
@@ -112,7 +158,7 @@ RSpec.describe Vulnerabilities::ManuallyCreateService do
         end
 
         context 'when Scanner already exists' do
-          let!(:scanner) { create(:vulnerabilities_scanner, name: scanner_params[:name]) }
+          let!(:scanner) { create(:vulnerabilities_scanner, external_id: scanner_attributes[:id]) }
 
           it 'does not create a new Scanner' do
             expect { subject }.to change(Vulnerabilities::Scanner, :count).by(0)
@@ -120,7 +166,7 @@ RSpec.describe Vulnerabilities::ManuallyCreateService do
         end
 
         context 'when Identifier already exists' do
-          let!(:identifier) { create(:vulnerabilities_identifier, name: identifier_params[:name]) }
+          let!(:identifier) { create(:vulnerabilities_identifier, name: identifier_attributes[:name]) }
 
           it 'does not create a new Identifier' do
             expect { subject }.not_to change(Vulnerabilities::Identifier, :count)
@@ -128,7 +174,7 @@ RSpec.describe Vulnerabilities::ManuallyCreateService do
         end
 
         it 'creates all objects with correct attributes' do
-          expect(vulnerability.title).to eq(params.dig(:vulnerability, :title))
+          expect(vulnerability.title).to eq(params.dig(:vulnerability, :name))
           expect(vulnerability.report_type).to eq("generic")
           expect(vulnerability.state).to eq(params.dig(:vulnerability, :state))
           expect(vulnerability.severity).to eq(params.dig(:vulnerability, :severity))
@@ -146,19 +192,23 @@ RSpec.describe Vulnerabilities::ManuallyCreateService do
 
           primary_identifier = finding.primary_identifier
           expect(primary_identifier.name).to eq(params.dig(:vulnerability, :identifiers, 0, :name))
+          expect(primary_identifier.url).to eq(params.dig(:vulnerability, :identifiers, 0, :url))
+          expect(primary_identifier.external_id).to eq(params.dig(:vulnerability, :identifiers, 0, :name))
+          expect(primary_identifier.external_type).to eq("other")
+          expect(primary_identifier.fingerprint).to eq(identifier_fingerprint)
         end
 
         context "when state fields match state" do
           let(:params) do
             {
               vulnerability: {
-                title: "Test vulnerability",
+                name: "Test vulnerability",
                 state: "confirmed",
                 severity: "unknown",
                 confidence: "unknown",
                 confirmed_at: Time.now.iso8601,
-                identifiers: [identifier_params],
-                scanner: scanner_params
+                identifiers: [identifier_attributes],
+                scanner: scanner_attributes
               }
             }
           end
@@ -176,13 +226,13 @@ RSpec.describe Vulnerabilities::ManuallyCreateService do
           let(:params) do
             {
               vulnerability: {
-                title: "Test vulnerability",
+                name: "Test vulnerability",
                 state: "detected",
                 severity: "unknown",
                 confidence: "unknown",
                 confirmed_at: Time.now.iso8601,
-                identifiers: [identifier_params],
-                scanner: scanner_params
+                identifiers: [identifier_attributes],
+                scanner: scanner_attributes
               }
             }
           end