Merge branch '218012-vulnerabilities-are-incorrectly-marked-as-resolved-in-master' into 'master'

Use latest successful pipeline for default branch which ran security jobs

See merge request gitlab-org/gitlab!37937

ee/app/models/vulnerability.rb

@@ -142,9 +142,25 @@ class Vulnerability < ApplicationRecord
   def resolved_on_default_branch
     return false unless findings.any?
 
-    latest_successful_pipeline_for_default_branch = project.latest_successful_pipeline_for_default_branch
-    latest_pipeline_with_vulnerability = finding.pipelines.order(created_at: :desc).first
-    latest_pipeline_with_vulnerability != latest_successful_pipeline_for_default_branch
+    project = Project
+      .includes(ci_pipelines: :builds)
+      .find(self.project_id)
+
+    # We can't just use project.latest_successful_pipeline_for_default_branch
+    # because there's no guarantee that it actually ran the security jobs
+    # See https://gitlab.com/gitlab-org/gitlab/-/issues/218012
+    latest_successful_pipeline = project
+      .ci_pipelines
+      .success
+      .newest_first(ref: project.default_branch, limit: 10)
+      .find { |pipeline| pipeline.builds.any? { |build| build.name == self.report_type } }
+
+    # Technically this shouldn't ever happen.
+    # If an vulnerability was discovered, then we must have ran a scan of the
+    # appropriate type at least once.
+    return false unless latest_successful_pipeline
+
+    finding.pipelines.exclude?(latest_successful_pipeline)
   end
 
   def user_notes_count

ee/changelogs/unreleased/218012-vulnerabilities-are-incorrectly-marked-as-resolved-in-master.yml

+---
+title: Mark Vulnerabilities as resolved only if the latest pipeline for default branch
+  ran respective security job
+merge_request: 37937
+author:
+type: fixed

ee/spec/models/vulnerability_spec.rb

@@ -267,7 +267,7 @@ RSpec.describe Vulnerability do
 
   describe '#resolved_on_default_branch' do
     let_it_be(:project) { create(:project, :repository, :with_vulnerability) }
-    let_it_be(:pipeline_with_vulnerability) { create(:ci_pipeline, :success, project: project, sha: project.commit.id) }
+    let_it_be(:pipeline_with_vulnerability) { create(:ci_pipeline, :success, :with_sast_build, project: project, sha: project.commit.id) }
     let_it_be(:vulnerability) { project.vulnerabilities.first }
     let_it_be(:finding1) { create(:vulnerabilities_occurrence, vulnerability: vulnerability, pipelines: [pipeline_with_vulnerability]) }
     let_it_be(:finding2) { create(:vulnerabilities_occurrence, vulnerability: vulnerability, pipelines: [pipeline_with_vulnerability]) }
@@ -279,11 +279,7 @@ RSpec.describe Vulnerability do
     end
 
     context 'Vulnerability::Finding is not present on the pipeline for default branch' do
-      before do
-        project.instance_variable_set(:@latest_successful_pipeline_for_default_branch, pipeline_without_vulnerability)
-      end
-
-      let_it_be(:pipeline_without_vulnerability) { create(:ci_pipeline, :success, project: project, sha: project.commit.id) }
+      let_it_be(:pipeline_without_vulnerability) { create(:ci_pipeline, :success, :with_sast_build, project: project, sha: project.commit.id) }
 
       it { is_expected.to eq(true) }
     end

spec/factories/ci/builds.rb

@@ -420,6 +420,8 @@ FactoryBot.define do
     end
 
     trait :sast do
+      name { "sast" }
+
       options do
         {
             artifacts: { reports: { sast: 'gl-sast-report.json' } }

spec/factories/ci/pipelines.rb

@@ -130,6 +130,14 @@ FactoryBot.define do
         end
       end
 
+      trait :with_sast_build do
+        status { :success }
+
+        after(:build) do |pipeline, evaluator|
+          pipeline.builds << build(:ci_build, :sast, pipeline: pipeline, project: pipeline.project)
+        end
+      end
+
       trait :with_exposed_artifacts do
         status { :success }