Merge branch '329345-finding-mismatch-on-store-report-service' into 'master'

fix: Propagate tracking feature flag to CI report parsers

See merge request gitlab-org/gitlab!60872

ee/app/models/ee/ci/build.rb

@@ -176,7 +176,8 @@ module EE
 
       def parse_security_artifact_blob(security_report, blob)
         report_clone = security_report.clone_as_blank
-        ::Gitlab::Ci::Parsers.fabricate!(security_report.type, blob, report_clone).parse!
+        signatures_enabled = ::Feature.enabled?(:vulnerability_finding_tracking_signatures, project) && project.licensed_feature_available?(:vulnerability_finding_signatures)
+        ::Gitlab::Ci::Parsers.fabricate!(security_report.type, blob, report_clone, signatures_enabled).parse!
         security_report.merge!(report_clone)
       end
 

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

@@ -96,7 +96,7 @@ module Gitlab
             links = create_links(data['links'])
             location = create_location(data['location'] || {})
             remediations = create_remediations(data['remediations'])
-            signatures = create_signatures(location, tracking_data(data))
+            signatures = create_signatures(tracking_data(data))
 
             if @vulnerability_finding_signatures_enabled && !signatures.empty?
               # NOT the signature_sha - the compare key is hashed
@@ -129,7 +129,7 @@ module Gitlab
                 vulnerability_finding_signatures_enabled: @vulnerability_finding_signatures_enabled))
           end
 
-          def create_signatures(location, tracking)
+          def create_signatures(tracking)
             tracking ||= { 'items' => [] }
 
             signature_algorithms = Hash.new { |hash, key| hash[key] = [] }

ee/spec/models/ci/build_spec.rb

@@ -262,6 +262,31 @@ RSpec.describe Ci::Build do
           expect(security_reports.get_report('sast', artifact)).to be_errored
         end
       end
+
+      context 'vulnerability_finding_tracking_signatures' do
+        let!(:artifact) { create(:ee_ci_job_artifact, :sast, job: job, project: job.project) }
+
+        where(vulnerability_finding_signatures_enabled: [true, false])
+        with_them do
+          it 'parses the report' do
+            stub_licensed_features(
+              sast: true,
+              vulnerability_finding_signatures: vulnerability_finding_signatures_enabled
+            )
+            stub_feature_flags(
+              vulnerability_finding_tracking_signatures: vulnerability_finding_signatures_enabled
+            )
+
+            expect(::Gitlab::Ci::Parsers::Security::Sast).to receive(:new).with(
+              artifact.file.read,
+              kind_of(::Gitlab::Ci::Reports::Security::Report),
+              vulnerability_finding_signatures_enabled
+            )
+
+            subject
+          end
+        end
+      end
     end
 
     context 'when there is unsupported file type' do

ee/spec/services/security/store_report_service_spec.rb

@@ -603,4 +603,96 @@ RSpec.describe Security::StoreReportService, '#execute' do
       end
     end
   end
+
+  context 'vulnerability tracking' do
+    let!(:artifact) { create(:ee_ci_job_artifact, :sast_minimal) }
+
+    def generate_new_pipeline
+      pipeline = create(:ci_pipeline, :success, project: project)
+      build = create(:ci_build, :success, pipeline: pipeline, project: project)
+      artifact = create(:ee_ci_job_artifact, :sast_minimal, job: build, project: project)
+
+      [
+        pipeline,
+        pipeline.security_reports.get_report('sast', artifact)
+      ]
+    end
+
+    before do
+      project.add_developer(user)
+      allow(pipeline).to receive(:user).and_return(user)
+    end
+
+    # This spec runs three pipelines, ensuring findings are tracked as expected:
+    #  1. pipeline creates initial findings without tracking signatures
+    #  2. pipeline creates identical findings with tracking signatures
+    #  3. pipeline updates previous findings using tracking signatures
+    it 'remaps findings across pipeline executions', :aggregate_failures do
+      stub_licensed_features(
+        sast: true,
+        security_dashboard: true,
+        vulnerability_finding_signatures: false
+      )
+      stub_feature_flags(
+        vulnerability_finding_tracking_signatures: false
+      )
+      stub_feature_flags(optimize_sql_query_for_security_report: true)
+
+      expect do
+        expect do
+          described_class.new(pipeline, report).execute
+        end.not_to(raise_error)
+      end.to change { Vulnerabilities::FindingPipeline.count }.by(1)
+        .and change { Vulnerability.count }.by(1)
+        .and change { Vulnerabilities::Finding.count }.by(1)
+        .and change { Vulnerabilities::FindingSignature.count }.by(0)
+
+      stub_licensed_features(
+        sast: true,
+        security_dashboard: true,
+        vulnerability_finding_signatures: true
+      )
+      stub_feature_flags(vulnerability_finding_tracking_signatures: true)
+
+      pipeline, report = generate_new_pipeline
+
+      allow(pipeline).to receive(:user).and_return(user)
+
+      expect do
+        expect do
+          described_class.new(pipeline, report).execute
+        end.not_to(raise_error)
+      end.to change { Vulnerabilities::FindingPipeline.count }.by(1)
+        .and change { Vulnerability.count }.by(1)
+        .and change { Vulnerabilities::Finding.count }.by(1)
+        .and change { Vulnerabilities::FindingSignature.count }.by(2)
+
+      pipeline, report = generate_new_pipeline
+
+      # Update the location of the finding to trigger persistence of signatures
+      finding = report.findings.first
+      location_data = finding.location.as_json.symbolize_keys.tap { |h| h.delete(:fingerprint) }
+      location_data[:start_line] += 1
+      location_data[:end_line] += 1
+
+      allow(finding).to receive(:location).and_return(
+        Gitlab::Ci::Reports::Security::Locations::Sast.new(**location_data)
+      )
+      allow(finding).to receive(:raw_metadata).and_return(
+        Gitlab::Json.parse(finding.raw_metadata).merge("location" => location_data).to_json
+      )
+      allow(pipeline).to receive(:user).and_return(user)
+
+      expect do
+        expect do
+          described_class.new(pipeline, report).execute
+        end.not_to(raise_error)
+      end.to change { Vulnerabilities::FindingPipeline.count }.by(1)
+        .and change { Vulnerability.count }.by(0)
+        .and change { Vulnerabilities::Finding.count }.by(0)
+        .and change { Vulnerabilities::FindingSignature.count }.by(0)
+        .and change { Vulnerabilities::Finding.last.location['start_line'] }.from(29).to(30)
+        .and change { Vulnerabilities::Finding.last.location['end_line'] }.from(29).to(30)
+    end
+  end
 end

spec/factories/ci/job_artifacts.rb

@@ -277,6 +277,16 @@ FactoryBot.define do
       end
     end
 
+    trait :sast_minimal do
+      file_type { :sast }
+      file_format { :raw }
+
+      after(:build) do |artifact, _|
+        artifact.file = fixture_file_upload(
+          Rails.root.join('spec/fixtures/security_reports/master/gl-sast-report-minimal.json'), 'application/json')
+      end
+    end
+
     trait :secret_detection do
       file_type { :secret_detection }
       file_format { :raw }

spec/fixtures/security_reports/master/gl-sast-report-minimal.json

+{
+  "version": "14.0.0",
+  "vulnerabilities": [
+    {
+      "category": "sast",
+      "name": "Cipher with no integrity",
+      "message": "Cipher with no integrity",
+      "cve": "groovy/src/main/java/com/gitlab/security_products/tests/App.groovy:29:CIPHER_INTEGRITY",
+      "severity": "Medium",
+      "confidence": "High",
+      "scanner": {
+        "id": "find_sec_bugs",
+        "name": "Find Security Bugs"
+      },
+      "location": {
+        "file": "groovy/src/main/java/com/gitlab/security_products/tests/App.groovy",
+        "start_line": 29,
+        "end_line": 29,
+        "class": "com.gitlab.security_products.tests.App",
+        "method": "insecureCypher"
+      },
+      "identifiers": [
+        {
+          "type": "find_sec_bugs_type",
+          "name": "Find Security Bugs-CIPHER_INTEGRITY",
+          "value": "CIPHER_INTEGRITY",
+          "url": "https://find-sec-bugs.github.io/bugs.htm#CIPHER_INTEGRITY"
+        }
+      ],
+      "tracking": {
+        "type": "source",
+        "items": [
+          {
+            "file": "groovy/src/main/java/com/gitlab/security_products/tests/App.groovy",
+            "start_line": 29,
+            "end_line": 29,
+            "signatures": [
+              {
+                "algorithm": "hash",
+                "value": "HASHVALUE"
+              },
+              {
+                "algorithm": "scope_offset",
+                "value": "groovy/src/main/java/com/gitlab/security_products/tests/App.groovy:App[0]:insecureCypher[0]:2"
+              }
+            ]
+          }
+        ]
+      }
+    }
+  ],
+  "remediations": [],
+  "scan": {
+    "scanner": {
+      "id": "find_sec_bugs",
+      "name": "Find Security Bugs",
+      "url": "https://spotbugs.github.io",
+      "vendor": {
+        "name": "GitLab"
+      },
+      "version": "4.0.2"
+    },
+    "type": "sast",
+    "status": "success",
+    "start_time": "placeholder-value",
+    "end_time": "placeholder-value"
+  }
+}

spec/fixtures/security_reports/master/gl-sast-report.json

@@ -154,8 +154,8 @@
         "items": [
           {
             "file": "groovy/src/main/java/com/gitlab/security_products/tests/App.groovy",
-            "start_line": 47,
-            "end_line": 47,
+            "start_line": 29,
+            "end_line": 29,
             "signatures": [
               {
                 "algorithm": "hash",