Merge branch '119020-fix-project-security-status-categorization' into 'master'

Fix: incorrect security status counts

See merge request gitlab-org/gitlab!22650

ee/app/models/vulnerabilities/occurrence.rb

@@ -165,9 +165,14 @@ module Vulnerabilities
         project_ids = items.map { |i| i[:project_id] }.uniq
         severities = items.map { |i| i[:severity] }.uniq
 
-        counts = undismissed
+        latest_pipelines = Ci::Pipeline
+          .where(project_id: project_ids)
+          .with_vulnerabilities
+          .latest_successful_ids_per_project
+
+        counts = for_pipelines(latest_pipelines)
+          .undismissed
           .by_severities(severities)
-          .by_projects(project_ids)
           .group(:project_id, :severity)
           .count
 

ee/changelogs/unreleased/119020-fix-project-security-status-categorization.yml

+---
+title: 'Fix incorrect security status counts'
+merge_request: 22650
+author:
+type: fixed

ee/spec/controllers/groups/security/vulnerable_projects_controller_spec.rb

@@ -25,7 +25,14 @@ describe Groups::Security::VulnerableProjectsController do
       _ungrouped_project = create(:project)
       _safe_project = create(:project, namespace: group)
       vulnerable_project = create(:project, namespace: group)
-      create_list(:vulnerabilities_occurrence, 2, project: vulnerable_project, severity: :critical)
+      pipeline = create(:ci_pipeline, :success, project: vulnerable_project)
+      create_list(
+        :vulnerabilities_occurrence,
+        2,
+        pipelines: [pipeline],
+        project: vulnerable_project,
+        severity: :critical
+      )
 
       subject
 
@@ -39,8 +46,10 @@ describe Groups::Security::VulnerableProjectsController do
     it 'does not include archived or deleted projects' do
       archived_project = create(:project, :archived, namespace: group)
       deleted_project = create(:project, namespace: group, pending_delete: true)
-      create(:vulnerabilities_occurrence, project: archived_project)
-      create(:vulnerabilities_occurrence, project: deleted_project)
+      archived_pipeline = create(:ci_pipeline, :success, project: archived_project)
+      deleted_pipeline = create(:ci_pipeline, :success, project: deleted_project)
+      create(:vulnerabilities_occurrence, pipelines: [archived_pipeline], project: archived_project)
+      create(:vulnerabilities_occurrence, pipelines: [deleted_pipeline], project: deleted_project)
 
       subject
 

ee/spec/models/vulnerabilities/occurrence_spec.rb

@@ -319,16 +319,40 @@ describe Vulnerabilities::Occurrence do
   end
 
   describe '.batch_count_by_project_and_severity' do
+    let(:pipeline) { create(:ci_pipeline, :success, project: project) }
     let(:project) { create(:project) }
 
     it 'fetches a vulnerability count for the given project and severity' do
-      create(:vulnerabilities_occurrence, project: project, severity: :high)
+      create(:vulnerabilities_occurrence, pipelines: [pipeline], project: project, severity: :high)
 
       count = described_class.batch_count_by_project_and_severity(project.id, 'high')
 
       expect(count).to be(1)
     end
 
+    it 'only returns vulnerabilities from the latest successful pipeline' do
+      old_pipeline = create(:ci_pipeline, :success, project: project)
+      latest_pipeline = create(:ci_pipeline, :success, project: project)
+      latest_failed_pipeline = create(:ci_pipeline, :failed, project: project)
+      create(:vulnerabilities_occurrence, pipelines: [old_pipeline], project: project, severity: :critical)
+      create(
+        :vulnerabilities_occurrence,
+        pipelines: [latest_failed_pipeline],
+        project: project,
+        severity: :critical
+      )
+      create_list(
+        :vulnerabilities_occurrence, 2,
+        pipelines: [latest_pipeline],
+        project: project,
+        severity: :critical
+      )
+
+      count = described_class.batch_count_by_project_and_severity(project.id, 'critical')
+
+      expect(count).to be(2)
+    end
+
     it 'returns 0 when there are no vulnerabilities for that severity level' do
       count = described_class.batch_count_by_project_and_severity(project.id, 'high')
 
@@ -339,8 +363,10 @@ describe Vulnerabilities::Occurrence do
       projects = create_list(:project, 2)
 
       projects.each do |project|
-        create(:vulnerabilities_occurrence, project: project, severity: :high)
-        create(:vulnerabilities_occurrence, project: project, severity: :low)
+        pipeline = create(:ci_pipeline, :success, project: project)
+
+        create(:vulnerabilities_occurrence, pipelines: [pipeline], project: project, severity: :high)
+        create(:vulnerabilities_occurrence, pipelines: [pipeline], project: project, severity: :low)
       end
 
       projects_and_severities = [
@@ -358,8 +384,8 @@ describe Vulnerabilities::Occurrence do
     end
 
     it 'does not include dismissed vulnerabilities in the counts' do
-      create(:vulnerabilities_occurrence, project: project, severity: :high)
-      dismissed_vulnerability = create(:vulnerabilities_occurrence, project: project, severity: :high)
+      create(:vulnerabilities_occurrence, pipelines: [pipeline], project: project, severity: :high)
+      dismissed_vulnerability = create(:vulnerabilities_occurrence, pipelines: [pipeline], project: project, severity: :high)
       create(
         :vulnerability_feedback,
         project: project,
@@ -371,6 +397,25 @@ describe Vulnerabilities::Occurrence do
 
       expect(count).to be(1)
     end
+
+    it "does not overwrite one project's counts with another's" do
+      project1 = create(:project)
+      project2 = create(:project)
+      pipeline1 = create(:ci_pipeline, :success, project: project1)
+      pipeline2 = create(:ci_pipeline, :success, project: project2)
+      create(:vulnerabilities_occurrence, pipelines: [pipeline1], project: project1, severity: :critical)
+      create(:vulnerabilities_occurrence, pipelines: [pipeline2], project: project2, severity: :high)
+
+      project1_critical_count = described_class.batch_count_by_project_and_severity(project1.id, 'critical')
+      project1_high_count = described_class.batch_count_by_project_and_severity(project1.id, 'high')
+      project2_critical_count = described_class.batch_count_by_project_and_severity(project2.id, 'critical')
+      project2_high_count = described_class.batch_count_by_project_and_severity(project2.id, 'high')
+
+      expect(project1_critical_count).to be(1)
+      expect(project1_high_count).to be(0)
+      expect(project2_critical_count).to be(0)
+      expect(project2_high_count).to be(1)
+    end
   end
 
   describe 'feedback' do