Merge branch '5528-enrich_container_scanning_report' into 'master'

Improve Container Scanning parser

See merge request gitlab-org/gitlab-ee!9641

ee/changelogs/unreleased/5528-enrich_container_scanning_report.yml

+---
+title: Enrich container scanning report
+merge_request: 9641
+author:
+type: added

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

@@ -33,22 +33,23 @@ module Gitlab
               # We only report unapproved vulnerabilities
               next unless unapproved.include?(vulnerability['vulnerability'])
 
-              results.append(format_vulnerability(vulnerability))
+              results.append(format_vulnerability(vulnerability, data['image']))
             end
 
             results
           end
 
-          def format_vulnerability(vulnerability)
+          def format_vulnerability(vulnerability, image)
             {
               'category' => 'container_scanning',
-              'message' => name(vulnerability),
-              'description' => vulnerability['description'],
+              'message' => message(vulnerability),
+              'description' => description(vulnerability),
               'cve' => vulnerability['vulnerability'],
               'severity' => translate_severity(vulnerability['severity']),
               'solution' => solution(vulnerability),
               'confidence' => 'Medium',
               'location' => {
+                'image' => image,
                 'operating_system' => vulnerability["namespace"],
                 'dependency' => {
                   'package' => {
@@ -87,15 +88,50 @@ module Gitlab
             end
           end
 
+          def message(vulnerability)
+            format(
+              vulnerability,
+              %w[vulnerability featurename] =>
+                '%{vulnerability} in %{featurename}',
+              'vulnerability' =>
+                '%{vulnerability}'
+            )
+          end
+
+          def description(vulnerability)
+            format(
+              vulnerability,
+              'description' =>
+                '%{description}',
+              %w[featurename featureversion] =>
+                '%{featurename}:%{featureversion} is affected by %{vulnerability}',
+              'featurename' =>
+                '%{featurename} is affected by %{vulnerability}',
+              'namespace' =>
+                '%{namespace} is affected by %{vulnerability}'
+            )
+          end
+
           def solution(vulnerability)
-            if vulnerability['fixedby'].present?
-              "Upgrade to version #{vulnerability['fixedby']}"
+            format(
+              vulnerability,
+              %w[fixedby featurename featureversion] =>
+                'Upgrade %{featurename} from %{featureversion} to %{fixedby}',
+              %w[fixedby featurename] =>
+                'Upgrade %{featurename} to %{fixedby}',
+              'fixedby' =>
+                'Upgrade to %{fixedby}'
+            )
+          end
+
+          def format(vulnerability, definitions)
+            definitions.each do |keys, value|
+              if vulnerability.values_at(*Array(keys)).all?(&:present?)
+                return value % vulnerability.with_indifferent_access
               end
             end
 
-          def name(vulnerability)
-            # Name is package name and the CVE is is affected by.
-            "#{vulnerability['featurename']} - #{vulnerability['vulnerability']}"
+            nil
           end
 
           def generate_location_fingerprint(location)

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

@@ -5,7 +5,7 @@ require 'spec_helper'
 describe Gitlab::Ci::Parsers::Security::ContainerScanning do
   let(:parser) { described_class.new }
 
-  let(:zap_vulnerabilities) do
+  let(:clair_vulnerabilities) do
     JSON.parse!(
       File.read(
         Rails.root.join('spec/fixtures/security-reports/master/gl-container-scanning-report.json')
@@ -40,13 +40,18 @@ describe Gitlab::Ci::Parsers::Security::ContainerScanning do
     it "generates expected metadata_version" do
       expect(report.occurrences.first[:metadata_version]).to eq('1.3')
     end
+
+    it "adds report image's name to raw_metadata" do
+      expect(JSON.parse(report.occurrences.first[:raw_metadata]).dig('location', 'image'))
+        .to eq('registry.gitlab.com/groulot/container-scanning-test/master:5f21de6956aee99ddb68ae49498662d9872f50ff')
+    end
   end
 
   describe '#format_vulnerability' do
     it 'format ZAP vulnerability into the 1.3 format' do
-      expect(parser.send(:format_vulnerability, zap_vulnerabilities[0])).to eq( {
+      expect(parser.send(:format_vulnerability, clair_vulnerabilities[0], 'image_name')).to eq( {
         'category' => 'container_scanning',
-        'message' => 'glibc - CVE-2017-18269',
+        'message' => 'CVE-2017-18269 in glibc',
         'confidence' => 'Medium',
         'cve' => 'CVE-2017-18269',
         'identifiers' => [
@@ -58,6 +63,7 @@ describe Gitlab::Ci::Parsers::Security::ContainerScanning do
           }
         ],
         'location' => {
+          'image' => 'image_name',
           'operating_system' => 'debian:9',
           'dependency' => {
             'package' => {
@@ -71,7 +77,7 @@ describe Gitlab::Ci::Parsers::Security::ContainerScanning do
         'priority' => 'Unknown',
         'scanner' => { 'id' => 'clair', 'name' => 'Clair' },
         'severity' => 'critical',
-        'solution' => 'Upgrade to version 2.24-11+deb9u4',
+        'solution' => 'Upgrade glibc from 2.24-11+deb9u3 to 2.24-11+deb9u4',
         'tool' => 'clair',
         'url' => 'https://security-tracker.debian.org/tracker/CVE-2017-18269'
       } )
@@ -109,16 +115,145 @@ describe Gitlab::Ci::Parsers::Security::ContainerScanning do
     end
   end
 
+  describe '#message' do
+    let(:input) do
+      {
+        'featurename' => 'foo',
+        'featureversion' => '',
+        'vulnerability' => 'CVE-2018-777',
+        'namespace' => 'debian:9',
+        'description' => 'CVE-2018-777 is affecting your system',
+        'link' => 'https://security-tracker.debian.org/tracker/CVE-2018-777',
+        'severity' => 'Unknown',
+        'fixedby' => '1.4'
+      }
+    end
+
+    subject { parser.send(:message, input)}
+
+    context 'when there is a featurename' do
+      it 'formats message using the featurename' do
+        is_expected.to eq('CVE-2018-777 in foo')
+      end
+    end
+
+    context 'when there is no featurename' do
+      before do
+        input['featurename'] = ''
+      end
+
+      it 'formats message using the vulnerability only' do
+        is_expected.to eq('CVE-2018-777')
+      end
+    end
+  end
+
+  describe '#description' do
+    let(:input) do
+      {
+        'featurename' => 'foo',
+        'featureversion' => '1.2.3',
+        'vulnerability' => 'CVE-2018-777',
+        'namespace' => 'debian:9',
+        'description' => 'SSE2-optimized memmove implementation problem.',
+        'link' => 'https://security-tracker.debian.org/tracker/CVE-2018-777',
+        'severity' => 'Unknown',
+        'fixedby' => '1.4'
+      }
+    end
+
+    subject { parser.send(:description, input) }
+
+    context 'when there is a description' do
+      it 'returns the provided description' do
+        is_expected.to eq('SSE2-optimized memmove implementation problem.')
+      end
+    end
+
+    context 'when there is no description' do
+      before do
+        input['description'] = ''
+      end
+
+      context 'when there is no featurename' do
+        before do
+          input['featurename'] = ''
+        end
+
+        it 'formats description using the namespace' do
+          is_expected.to eq('debian:9 is affected by CVE-2018-777')
+        end
+      end
+
+      context 'when there is no featureversion' do
+        before do
+          input['featureversion'] = ''
+        end
+
+        it 'formats description using the featurename only' do
+          is_expected.to eq('foo is affected by CVE-2018-777')
+        end
+      end
+
+      context 'when featurename and featureversion are present' do
+        it 'formats description using featurename and featureversion' do
+          is_expected.to eq('foo:1.2.3 is affected by CVE-2018-777')
+        end
+      end
+    end
+  end
+
   describe '#solution' do
-    context 'without a fixedby value' do
+    let(:input) do
+      {
+        'featurename' => 'foo',
+        'featureversion' => '1.2.3',
+        'vulnerability' => 'CVE-2018-777',
+        'namespace' => 'debian:9',
+        'description' => 'SSE2-optimized memmove implementation problem.',
+        'link' => 'https://security-tracker.debian.org/tracker/CVE-2018-777',
+        'severity' => 'Unknown',
+        'fixedby' => '1.4'
+      }
+    end
+
+    subject { parser.send(:solution, input) }
+
+    context 'when there is no fixedby value' do
+      before do
+        input['fixedby'] = ''
+      end
+
       it 'returns nil' do
-        expect(parser.send(:solution, zap_vulnerabilities[1])).to be_nil
+        is_expected.to be_nil
       end
     end
 
-    context 'with a fixedby value' do
-      it 'returns a solution' do
-        expect(parser.send(:solution, zap_vulnerabilities[0])).to eq('Upgrade to version 2.24-11+deb9u4')
+    context 'when there is a fixedby' do
+      context 'when there is no featurename' do
+        before do
+          input['featurename'] = ''
+        end
+
+        it 'formats solution using the fixedby only' do
+          is_expected.to eq('Upgrade to 1.4')
+        end
+      end
+
+      context 'when there is no featureversion' do
+        before do
+          input['featureversion'] = ''
+        end
+
+        it 'formats solution using the featurename only' do
+          is_expected.to eq('Upgrade foo to 1.4')
+        end
+      end
+
+      context 'when featurename and featureversion are present' do
+        it 'formats solution using featurename and featureversion' do
+          is_expected.to eq('Upgrade foo from 1.2.3 to 1.4')
+        end
       end
     end
   end