ActiveRecord::RecordNotUnique in MigrateVulnerabilitiesFeedbackToVulnerabilitiesState background migration

Related to https://gitlab.zendesk.com/agent/tickets/412248 and https://gitlab.zendesk.com/agent/tickets/418801 tickets

The problematic path

  1. The migration violates index_vulnerability_occurrences_on_uuid constraint
  2. The only path in the migration where we would try to insert records there is handle_security_findings_only_scenario
  3. An insert would happen because we call Vulnerabilities::FindOrCreateFromSecurityFindingService which, in turn calls Vulnerabilities::Findings::FindOrCreateFromSecurityFindingService which mean we would try to create a row in vulnerability_occurrences table.

Why does it happen?

Based on the data provided by the customer (internal link) I am fairly confident that the scenario happens when:

  1. The Security::Finding has uuid and an overridden_uuid
  2. The Vulnerabilities::Feedback record references the security_finding.uuid not the security_finding.overridden_uuid
  3. There is a Vulnerabilities::Finding record for the security_finding.overridden_uuid already

I made a test case which appears to verify my theory

Test case
diff --git a/ee/spec/lib/ee/gitlab/background_migration/migrate_vulnerabilities_feedback_to_vulnerabilities_state_transition_spec.rb b/ee/spec/lib/ee/gitlab/background_migration/migrate_vulnerabilities_feedback_to_vulnerabilities_state_transition_spec.rb
index c2a1afaea65e..a1d0c71b5c7d 100644
--- a/ee/spec/lib/ee/gitlab/background_migration/migrate_vulnerabilities_feedback_to_vulnerabilities_state_transition_spec.rb
+++ b/ee/spec/lib/ee/gitlab/background_migration/migrate_vulnerabilities_feedback_to_vulnerabilities_state_transition_spec.rb
@@ -90,6 +90,38 @@
 
     subject { described_class.new(**migration_attrs).perform }
 
+    context "when there's a Security::Finding with an overriden UUID" do
+      before do
+        sast_category = 0
+        sast_scan_type = 1
+
+        nonexistent_project_fingerprint = SecureRandom.hex(20)
+        known_uuid = "7099388a-d37c-5940-9a44-4e3645f2fd23"
+        # This UUID would be calculated by our pipeline ingestion process
+        overridden_uuid = "429005aa-8b32-58a9-b2ea-bc8ae80b0963"
+        ci_pipeline = create_ci_pipeline(project_id: project.id)
+        ci_build = create_ci_build(
+          project_id: project.id,
+          status: "success",
+          commit_id: ci_pipeline.id
+        )
+        # rubocop:disable RSpec/FactoriesInMigrationSpecs
+        create(:ee_ci_job_artifact, :sast_with_signatures_and_vulnerability_flags, job_id: ci_build.id)
+        # rubocop:enable RSpec/FactoriesInMigrationSpecs
+        security_scan = create_security_scan(ci_build, sast_scan_type, project_id: project.id)
+        @security_finding = create_security_finding(security_scan, scanner, uuid: known_uuid, overridden_uuid: overridden_uuid)
+        @vulnerability_feedback = create_feedback(
+          project, user, finding.report_type, feedback_types[:dismissal], finding.project_fingerprint, known_uuid,
+          comment: "this feedback uses the non-overridden UUID"
+        )
+        finding.update(uuid: overridden_uuid)
+      end
+
+      it "fails with ActiveRecord::RecordNotUnique" do
+        expect { subject }.to raise_error(ActiveRecord::RecordNotUnique)
+      end
+    end
+
     context "when a Finding has no Vulnerability" do
       before do
         create_feedback(

Better test case: !124469 (diffs)

Edited by Michał Zając