Merge branch '285593-generate-finding-name-from-data-if-message-is-missing' into 'master'

Generate finding name from report data if message is missing

See merge request gitlab-org/gitlab!48279

ee/changelogs/unreleased/285593-generate-finding-name-from-data-if-message-is-missing.yml

+---
+title: Generate finding name from report data if message is missing
+merge_request: 48279
+author:
+type: fixed

ee/lib/gitlab/ci/parsers/security/common.rb

@@ -56,13 +56,15 @@ module Gitlab
           def create_vulnerability(report, data, version)
             identifiers = create_identifiers(report, data['identifiers'])
             links = create_links(report, data['links'])
+            location = create_location(data['location'] || {})
+
             report.add_finding(
               ::Gitlab::Ci::Reports::Security::Finding.new(
                 uuid: SecureRandom.uuid,
                 report_type: report.type,
-                name: data['message'],
+                name: finding_name(data, identifiers, location),
                 compare_key: data['cve'] || '',
-                location: create_location(data['location'] || {}),
+                location: location,
                 severity: parse_severity_level(data['severity']&.downcase),
                 confidence: parse_confidence_level(data['confidence']&.downcase),
                 scanner: create_scanner(report, data['scanner']),
@@ -139,6 +141,16 @@ module Gitlab
           def create_location(location_data)
             raise NotImplementedError
           end
+
+          private
+
+          def finding_name(data, identifiers, location)
+            return data['message'] if data['message'].present?
+            return data['name'] if data['name'].present?
+
+            identifier = identifiers.find(&:cve?) || identifiers.find(&:cwe?) || identifiers.first
+            "#{identifier.name} in #{location&.fingerprint_path}"
+          end
         end
       end
     end

ee/lib/gitlab/ci/reports/security/concerns/fingerprint_path_from_file.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Ci
+    module Reports
+      module Security
+        module Concerns
+          module FingerprintPathFromFile
+            extend ActiveSupport::Concern
+
+            def fingerprint_path
+              File.basename(file_path.to_s)
+            end
+          end
+        end
+      end
+    end
+  end
+end

ee/lib/gitlab/ci/reports/security/identifier.rb

@@ -41,6 +41,14 @@ module Gitlab
               other.external_id == external_id
           end
 
+          def cve?
+            external_type.to_s.casecmp('cve') == 0
+          end
+
+          def cwe?
+            external_type.to_s.casecmp('cwe') == 0
+          end
+
           private
 
           def generate_fingerprint

ee/lib/gitlab/ci/reports/security/locations/base.rb

@@ -24,6 +24,10 @@ module Gitlab
               super
             end
 
+            def fingerprint_path
+              fingerprint_data
+            end
+
             private
 
             def fingerprint_data

ee/lib/gitlab/ci/reports/security/locations/dast.rb

@@ -18,6 +18,8 @@ module Gitlab
               @path = path
             end
 
+            alias_method :fingerprint_path, :path
+
             private
 
             def fingerprint_data

ee/lib/gitlab/ci/reports/security/locations/dependency_scanning.rb

@@ -6,6 +6,8 @@ module Gitlab
       module Security
         module Locations
           class DependencyScanning < Base
+            include Security::Concerns::FingerprintPathFromFile
+
             attr_reader :file_path
             attr_reader :package_name
             attr_reader :package_version

ee/lib/gitlab/ci/reports/security/locations/sast.rb

@@ -6,6 +6,8 @@ module Gitlab
       module Security
         module Locations
           class Sast < Base
+            include Security::Concerns::FingerprintPathFromFile
+
             attr_reader :class_name
             attr_reader :end_line
             attr_reader :file_path

ee/lib/gitlab/ci/reports/security/locations/secret_detection.rb

@@ -6,6 +6,8 @@ module Gitlab
       module Security
         module Locations
           class SecretDetection < Base
+            include Security::Concerns::FingerprintPathFromFile
+
             attr_reader :class_name
             attr_reader :end_line
             attr_reader :file_path

ee/spec/factories/ci/job_artifacts.rb

@@ -321,6 +321,16 @@ FactoryBot.define do
       end
     end
 
+    trait :common_security_report_with_blank_names do
+      file_format { :raw }
+      file_type { :dependency_scanning }
+
+      after(:build) do |artifact, _|
+        artifact.file = fixture_file_upload(
+          Rails.root.join('ee/spec/fixtures/security_reports/master/gl-common-scanning-report-names.json'), 'application/json')
+      end
+    end
+
     trait :container_scanning_feature_branch do
       file_format { :raw }
       file_type { :container_scanning }

ee/spec/fixtures/security_reports/master/gl-common-scanning-report-names.json

+{
+  "vulnerabilities": [
+    {
+      "category": "dependency_scanning",
+      "name": "Vulnerabilities in libxml2",
+      "message": "Vulnerabilities in libxml2 in nokogiri",
+      "description": "",
+      "cve": "CVE-1020",
+      "severity": "High",
+      "solution": "Upgrade to latest version.",
+      "scanner": {
+        "id": "gemnasium",
+        "name": "Gemnasium"
+      },
+      "location": {},
+      "identifiers": [],
+      "links": [
+        {
+          "url": "https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2020-1020"
+        }
+      ]
+    },
+    {
+      "id": "bb2fbeb1b71ea360ce3f86f001d4e84823c3ffe1a1f7d41ba7466b14cfa953d3",
+      "category": "dependency_scanning",
+      "name": "Regular Expression Denial of Service",
+      "message": "",
+      "description": "",
+      "cve": "CVE-1030",
+      "severity": "Unknown",
+      "solution": "Upgrade to latest versions.",
+      "scanner": {
+        "id": "gemnasium",
+        "name": "Gemnasium"
+      },
+      "location": {},
+      "identifiers": [],
+      "links": [
+        {
+          "name": "CVE-1030",
+          "url": "https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2020-1030"
+        }
+      ]
+    },
+    {
+      "category": "dependency_scanning",
+      "name": "",
+      "message": "",
+      "description": "",
+      "cve": "CVE-2017-11429",
+      "severity": "Unknown",
+      "solution": "Upgrade to fixed version.\r\n",
+      "scanner": {
+        "id": "gemnasium",
+        "name": "Gemnasium"
+      },
+      "location": {
+        "file": "yarn/yarn.lock",
+        "dependency": {
+          "package": {
+            "name": "io.netty/netty"
+          },
+          "version": "3.9.1.Final"
+        }
+      },
+      "identifiers": [
+        {
+          "value": "2017-11429",
+          "type": "cwe",
+          "name": "CWE-2017-11429",
+          "url": "https://cve.mitre.org/cgi-bin/cwename.cgi?name=CWE-2017-11429"
+        },
+        {
+          "value": "2017-11429",
+          "type": "cve",
+          "name": "CVE-2017-11429",
+          "url": "https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2017-11429"
+        }
+      ],
+      "links": []
+    },
+    {
+      "category": "dependency_scanning",
+      "name": "",
+      "message": "",
+      "description": "",
+      "cve": "CWE-2017-11429",
+      "severity": "Unknown",
+      "solution": "Upgrade to fixed version.\r\n",
+      "scanner": {
+        "id": "gemnasium",
+        "name": "Gemnasium"
+      },
+      "location": {
+        "file": "yarn/yarn.lock",
+        "dependency": {
+          "package": {
+            "name": "io.netty/netty"
+          },
+          "version": "3.9.1.Final"
+        }
+      },
+      "identifiers": [
+        {
+          "value": "2017-11429",
+          "type": "cwe",
+          "name": "CwE-2017-11429",
+          "url": "https://cwe.mitre.org/cgi-bin/cwename.cgi?name=CWE-2017-11429"
+        },
+        {
+          "value": "2017-11429",
+          "type": "other",
+          "name": "other-2017-11429",
+          "url": "https://other.mitre.org/cgi-bin/othername.cgi?name=other-2017-11429"
+        }
+      ],
+      "links": []
+    },
+    {
+      "category": "dependency_scanning",
+      "name": "",
+      "message": "",
+      "description": "",
+      "cve": "OTHER-2017-11429",
+      "severity": "Unknown",
+      "solution": "Upgrade to fixed version.\r\n",
+      "scanner": {
+        "id": "gemnasium",
+        "name": "Gemnasium"
+      },
+      "location": {
+        "file": "yarn/yarn.lock",
+        "dependency": {
+          "package": {
+            "name": "io.netty/netty"
+          },
+          "version": "3.9.1.Final"
+        }
+      },
+      "identifiers": [
+        {
+          "value": "2017-11429",
+          "type": "other",
+          "name": "other-2017-11429",
+          "url": "https://other.mitre.org/cgi-bin/othername.cgi?name=other-2017-11429"
+        }
+      ],
+      "links": []
+    }
+  ],
+  "remediations": [],
+  "dependency_files": [],
+  "scan": {
+    "scanner": {
+      "id": "gemnasium",
+      "name": "Gemnasium",
+      "url": "https://gitlab.com/gitlab-org/security-products/analyzers/gemnasium-maven",
+      "vendor": {
+        "name": "GitLab"
+      },
+      "version": "2.18.0"
+    },
+    "type": "dependency_scanning",
+    "start_time": "placeholder-value",
+    "end_time": "placeholder-value",
+    "status": "success"
+  }
+}

ee/spec/lib/gitlab/ci/parsers/security/common_spec.rb

@@ -9,14 +9,62 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Common do
     let(:artifact) { build(:ee_ci_job_artifact, :common_security_report) }
     let(:report) { Gitlab::Ci::Reports::Security::Report.new(artifact.file_type, pipeline, 2.weeks.ago) }
     let(:parser) { described_class.new }
+    let(:location) { ::Gitlab::Ci::Reports::Security::Locations::DependencyScanning.new(file_path: 'yarn/yarn.lock', package_version: 'v2', package_name: 'saml2') }
 
     before do
-      allow(parser).to receive(:create_location).and_return(nil)
+      allow(parser).to receive(:create_location).and_return(location)
       artifact.each_blob do |blob|
         parser.parse!(blob, report)
       end
     end
 
+    context 'parsing finding.name' do
+      let(:artifact) { build(:ee_ci_job_artifact, :common_security_report_with_blank_names) }
+
+      context 'when message is provided' do
+        it 'sets message from the report as a finding name' do
+          vulnerability = report.findings.find { |x| x.compare_key == 'CVE-1020' }
+          expected_name = Gitlab::Json.parse(vulnerability.raw_metadata)['message']
+
+          expect(vulnerability.name).to eq(expected_name)
+        end
+      end
+
+      context 'when message is not provided' do
+        context 'and name is provided' do
+          it 'sets name from the report as a name' do
+            vulnerability = report.findings.find { |x| x.compare_key == 'CVE-1030' }
+            expected_name = Gitlab::Json.parse(vulnerability.raw_metadata)['name']
+
+            expect(vulnerability.name).to eq(expected_name)
+          end
+        end
+
+        context 'and name is not provided' do
+          context 'when CVE identifier exists' do
+            it 'combines identifier with location to create name' do
+              vulnerability = report.findings.find { |x| x.compare_key == 'CVE-2017-11429' }
+              expect(vulnerability.name).to eq("CVE-2017-11429 in yarn.lock")
+            end
+          end
+
+          context 'when CWE identifier exists' do
+            it 'combines identifier with location to create name' do
+              vulnerability = report.findings.find { |x| x.compare_key == 'CWE-2017-11429' }
+              expect(vulnerability.name).to eq("CWE-2017-11429 in yarn.lock")
+            end
+          end
+
+          context 'when neither CVE nor CWE identifier exist' do
+            it 'combines identifier with location to create name' do
+              vulnerability = report.findings.find { |x| x.compare_key == 'OTHER-2017-11429' }
+              expect(vulnerability.name).to eq("other-2017-11429 in yarn.lock")
+            end
+          end
+        end
+      end
+    end
+
     context 'parsing remediations' do
       it 'finds remediation with same cve' do
         vulnerability = report.findings.find { |x| x.compare_key == "CVE-1020" }

ee/spec/lib/gitlab/ci/reports/security/identifier_spec.rb

@@ -52,6 +52,42 @@ RSpec.describe Gitlab::Ci::Reports::Security::Identifier do
     end
   end
 
+  describe '#cve?' do
+    let(:identifier) { create(:ci_reports_security_identifier, external_type: external_type) }
+
+    subject { identifier.cve? }
+
+    context 'when has cve as external type' do
+      let(:external_type) { 'Cve' }
+
+      it { is_expected.to eq(true) }
+    end
+
+    context 'when does not have cve as external type' do
+      let(:external_type) { 'Cwe' }
+
+      it { is_expected.to eq(false) }
+    end
+  end
+
+  describe '#cwe?' do
+    let(:identifier) { create(:ci_reports_security_identifier, external_type: external_type) }
+
+    subject { identifier.cwe? }
+
+    context 'when has cwe as external type' do
+      let(:external_type) { 'Cwe' }
+
+      it { is_expected.to eq(true) }
+    end
+
+    context 'when does not have cwe as external type' do
+      let(:external_type) { 'Cve' }
+
+      it { is_expected.to eq(false) }
+    end
+  end
+
   describe '#to_hash' do
     let(:identifier) { create(:ci_reports_security_identifier) }
 

ee/spec/lib/gitlab/ci/reports/security/locations/container_scanning_spec.rb

@@ -14,6 +14,7 @@ RSpec.describe Gitlab::Ci::Reports::Security::Locations::ContainerScanning do
 
   let(:mandatory_params) { %i[image operating_system] }
   let(:expected_fingerprint) { Digest::SHA1.hexdigest('registry.gitlab.com/my/project:glibc') }
+  let(:expected_fingerprint_path) { 'registry.gitlab.com/my/project:glibc' }
 
   it_behaves_like 'vulnerability location'
 

ee/spec/lib/gitlab/ci/reports/security/locations/dast_spec.rb

@@ -14,6 +14,7 @@ RSpec.describe Gitlab::Ci::Reports::Security::Locations::Dast do
 
   let(:mandatory_params) { %i[path method_name] }
   let(:expected_fingerprint) { Digest::SHA1.hexdigest('/some/path:GET:X-Content-Type-Options') }
+  let(:expected_fingerprint_path) { '/some/path' }
 
   it_behaves_like 'vulnerability location'
 end

ee/spec/lib/gitlab/ci/reports/security/locations/dependency_scanning_spec.rb

@@ -13,6 +13,7 @@ RSpec.describe Gitlab::Ci::Reports::Security::Locations::DependencyScanning do
 
   let(:mandatory_params) { %i[file_path package_name] }
   let(:expected_fingerprint) { Digest::SHA1.hexdigest('app/pom.xml:io.netty/netty') }
+  let(:expected_fingerprint_path) { 'pom.xml' }
 
   it_behaves_like 'vulnerability location'
 end

ee/spec/lib/gitlab/ci/reports/security/locations/sast_spec.rb

@@ -15,6 +15,7 @@ RSpec.describe Gitlab::Ci::Reports::Security::Locations::Sast do
 
   let(:mandatory_params) { %i[file_path start_line] }
   let(:expected_fingerprint) { Digest::SHA1.hexdigest('src/main/App.java:29:31') }
+  let(:expected_fingerprint_path) { 'App.java' }
 
   it_behaves_like 'vulnerability location'
 end

ee/spec/lib/gitlab/ci/reports/security/locations/secret_detection_spec.rb

@@ -15,6 +15,7 @@ RSpec.describe Gitlab::Ci::Reports::Security::Locations::SecretDetection do
 
   let(:mandatory_params) { %i[file_path start_line] }
   let(:expected_fingerprint) { Digest::SHA1.hexdigest('src/main/App.java:29:31') }
+  let(:expected_fingerprint_path) { 'App.java' }
 
   it_behaves_like 'vulnerability location'
 end

ee/spec/support/shared_examples/lib/gitlab/ci/reports/security/locations/locations_shared_examples.rb

@@ -37,6 +37,14 @@ RSpec.shared_examples 'vulnerability location' do
     end
   end
 
+  describe '#fingerprint_path' do
+    subject { described_class.new(**params).fingerprint_path }
+
+    it "generates expected fingerprint" do
+      expect(subject).to eq(expected_fingerprint_path)
+    end
+  end
+
   describe '#==' do
     let(:location_1) { create(:ci_reports_security_locations_sast) }
     let(:location_2) { create(:ci_reports_security_locations_sast) }