From a930de7889cc41410f7b67edbe7f5f11003fa214 Mon Sep 17 00:00:00 2001
From: Can Eldem <celdem@gitlab.com>
Date: Wed, 23 Oct 2019 09:44:46 +0000
Subject: [PATCH] Sort vulnerabilities for pipeline dashboard
Consider enum values rather than string
Added further test
---
...vulnerabilities-for-pipeline-dashboard.yml | 5 ++++
.../pipeline_vulnerabilities_finder.rb | 2 +-
ee/app/models/vulnerabilities/occurrence.rb | 8 +++++++
.../pipeline_vulnerabilities_finder_spec.rb | 23 +++++++++++++------
.../api/vulnerabilities_shared_examples.rb | 2 +-
5 files changed, 31 insertions(+), 9 deletions(-)
create mode 100644 changelogs/unreleased/sort-vulnerabilities-for-pipeline-dashboard.yml
diff --git a/changelogs/unreleased/sort-vulnerabilities-for-pipeline-dashboard.yml b/changelogs/unreleased/sort-vulnerabilities-for-pipeline-dashboard.yml
new file mode 100644
index 0000000000000..ffbfc652b81a9
--- /dev/null
+++ b/changelogs/unreleased/sort-vulnerabilities-for-pipeline-dashboard.yml
@@ -0,0 +1,5 @@
+---
+title: Pipeline vulnerability dashboard sort vulnerabilities by severity then confidence
+merge_request: 18863
+author:
+type: fixed
diff --git a/ee/app/finders/security/pipeline_vulnerabilities_finder.rb b/ee/app/finders/security/pipeline_vulnerabilities_finder.rb
index 4818d21737e39..77ffe6e2821f8 100644
--- a/ee/app/finders/security/pipeline_vulnerabilities_finder.rb
+++ b/ee/app/finders/security/pipeline_vulnerabilities_finder.rb
@@ -41,7 +41,7 @@ def execute
occurrences.concat(filtered_occurrences)
end
- occurrences.sort_by { |x| [x.severity, x.confidence] }
+ occurrences.sort_by { |x| [-x.severity_value, -x.confidence_value] }
end
private
diff --git a/ee/app/models/vulnerabilities/occurrence.rb b/ee/app/models/vulnerabilities/occurrence.rb
index 0b015562ba9ba..fdb5c1ad3b8ff 100644
--- a/ee/app/models/vulnerabilities/occurrence.rb
+++ b/ee/app/models/vulnerabilities/occurrence.rb
@@ -221,6 +221,14 @@ def hash
report_type.hash ^ location.hash ^ first_fingerprint.hash
end
+ def severity_value
+ self.class.severities[self.severity]
+ end
+
+ def confidence_value
+ self.class.confidences[self.confidence]
+ end
+
protected
def first_fingerprint
diff --git a/ee/spec/finders/security/pipeline_vulnerabilities_finder_spec.rb b/ee/spec/finders/security/pipeline_vulnerabilities_finder_spec.rb
index cd19809e270c7..811bfdd46a66e 100644
--- a/ee/spec/finders/security/pipeline_vulnerabilities_finder_spec.rb
+++ b/ee/spec/finders/security/pipeline_vulnerabilities_finder_spec.rb
@@ -54,15 +54,24 @@ def disable_deduplication
context 'by order' do
let(:params) { { report_type: %w[sast] } }
- let!(:occurrence1) { build(:vulnerabilities_occurrence, confidence: Vulnerabilities::Occurrence::CONFIDENCE_LEVELS[:high], severity: Vulnerabilities::Occurrence::SEVERITY_LEVELS[:high]) }
- let!(:occurrence2) { build(:vulnerabilities_occurrence, confidence: Vulnerabilities::Occurrence::CONFIDENCE_LEVELS[:medium], severity: Vulnerabilities::Occurrence::SEVERITY_LEVELS[:critical]) }
- let!(:occurrence3) { build(:vulnerabilities_occurrence, confidence: Vulnerabilities::Occurrence::CONFIDENCE_LEVELS[:high], severity: Vulnerabilities::Occurrence::SEVERITY_LEVELS[:critical]) }
- let!(:res) { [occurrence3, occurrence2, occurrence1] }
+ let!(:high_high) { build(:vulnerabilities_occurrence, confidence: :high, severity: :high) }
+ let!(:critical_medium) { build(:vulnerabilities_occurrence, confidence: :medium, severity: :critical) }
+ let!(:critical_high) { build(:vulnerabilities_occurrence, confidence: :high, severity: :critical) }
+ let!(:unknown_high) { build(:vulnerabilities_occurrence, confidence: :high, severity: :unknown) }
+ let!(:unknown_medium) { build(:vulnerabilities_occurrence, confidence: :medium, severity: :unknown) }
+ let!(:unknown_low) { build(:vulnerabilities_occurrence, confidence: :low, severity: :unknown) }
it 'orders by severity and confidence' do
- allow_any_instance_of(described_class).to receive(:filter).and_return(res)
-
- expect(subject).to eq([occurrence3, occurrence2, occurrence1])
+ allow_any_instance_of(described_class).to receive(:filter).and_return([
+ unknown_low,
+ unknown_medium,
+ critical_high,
+ unknown_high,
+ critical_medium,
+ high_high
+ ])
+
+ expect(subject).to eq([critical_high, critical_medium, high_high, unknown_high, unknown_medium, unknown_low])
end
end
diff --git a/ee/spec/support/shared_examples/requests/api/vulnerabilities_shared_examples.rb b/ee/spec/support/shared_examples/requests/api/vulnerabilities_shared_examples.rb
index 1f9282e365ff2..ee6f057db2ff6 100644
--- a/ee/spec/support/shared_examples/requests/api/vulnerabilities_shared_examples.rb
+++ b/ee/spec/support/shared_examples/requests/api/vulnerabilities_shared_examples.rb
@@ -127,7 +127,7 @@
# occurrences are implicitly sorted by Security::MergeReportsService,
# occurrences order differs from what is present in fixture file
- expect(json_response.first['name']).to eq 'Consider possible security implications associated with Popen module.'
+ expect(json_response.first['name']).to eq 'ECB mode is insecure'
end
it 'returns vulnerabilities with dependency_scanning report_type' do
--
GitLab