Skip to content

Fix the has_remediations ingestion logic for vulnerability_reads

We introduced the ingestion logic to update vulnerability_reads.has_remediations in !129125 (diffs) but during related backend issue verification we observed that this logic has a bug and needs a fix.

Solution proposal:

  1. Use return_data from ingestion task as it is the source of truth for remediations present on DB.
diff --git a/ee/app/services/security/ingestion/tasks/ingest_remediations.rb b/ee/app/services/security/ingestion/tasks/ingest_remediations.rb
index 99b53a9ea624..aa201d9b7899 100644
--- a/ee/app/services/security/ingestion/tasks/ingest_remediations.rb
+++ b/ee/app/services/security/ingestion/tasks/ingest_remediations.rb
@@ -93,28 +93,36 @@ def dissassociate_unfound_remediations
           unfound_remediations.delete_all
         end

+        def finding_maps_vul_finding_remediations
+          @finding_maps_vul_finding_remediations ||= Vulnerabilities::FindingRemediation.by_finding_id(finding_maps.map(&:finding_id))
+        end
+
         # When a new pipeline is run and the new findings set doesn't include some of the older findings,
         # the remediation associated with the older findings should be corrected.
         def unfound_remediations
-          @unfound_remediations ||= Vulnerabilities::FindingRemediation.by_finding_id(finding_maps.map(&:finding_id))
-                                        .id_not_in(return_data.flatten)
+          @unfound_remediations ||= finding_maps_vul_finding_remediations.id_not_in(return_data.flatten)
         end

-        def update_vulnerability_reads
-          finding_ids = finding_maps.map(&:finding_id)
-          vulnerability_ids = Vulnerabilities::Finding.id_in(finding_ids).pluck_vulnerability_ids
+        def found_remediations
+          @found_remediations ||= finding_maps_vul_finding_remediations.id_in(return_data.flatten)
+        end

+        def update_vulnerability_reads
           if unfound_remediations.present?
             unfound_remediations_finding_ids = unfound_remediations.map(&:vulnerability_occurrence_id)
             unfound_remediations_vulnerability_ids = Vulnerabilities::Finding.id_in(unfound_remediations_finding_ids)
                                                       .pluck_vulnerability_ids

             Vulnerabilities::Read.by_vulnerabilities(unfound_remediations_vulnerability_ids).update_all(has_remediations: false)
-
-            vulnerability_ids -= unfound_remediations_vulnerability_ids
           end

-          Vulnerabilities::Read.by_vulnerabilities(vulnerability_ids).update_all(has_remediations: true)
+          return unless found_remediations.present?
+
+          found_remediations_finding_ids = found_remediations.map(&:vulnerability_occurrence_id)
+          found_remediations_vulnerability_ids = Vulnerabilities::Finding.id_in(found_remediations_finding_ids)
+                                                    .pluck_vulnerability_ids
+
+          Vulnerabilities::Read.by_vulnerabilities(found_remediations_vulnerability_ids).update_all(has_remediations: true)
         end

         def new_report_remediations
  1. After step-1 is merged requeue the modified backfill batched migration as diff below following the requeue guidelines
diff --git a/lib/gitlab/background_migration/backfill_has_remediations_of_vulnerability_reads.rb b/lib/gitlab/background_migration/backfill_has_remediations_of_vulnerability_reads.rb
index 83acd8a27f7b..53855da6ad41 100644
--- a/lib/gitlab/background_migration/backfill_has_remediations_of_vulnerability_reads.rb
+++ b/lib/gitlab/background_migration/backfill_has_remediations_of_vulnerability_reads.rb
@@ -20,6 +20,10 @@ class BackfillHasRemediationsOfVulnerabilityReads < BatchedMigrationJob

       def perform
         each_sub_batch do |sub_batch|
+          reset_vulnerability_reads_query = reset_vulnerability_reads_query(sub_batch)
+
+          connection.execute(reset_vulnerability_reads_query)
+
           update_query = update_query_for(sub_batch)

           connection.execute(update_query)
@@ -28,6 +32,10 @@ def perform

       private

+      def reset_vulnerability_reads_query(sub_batch)
+        sub_batch.update(has_remediations: false).to_sql
+      end
+
       def update_query_for(sub_batch)
         subquery = sub_batch.joins("
           INNER JOIN vulnerability_occurrences ON

Verification Steps

  1. Import demo project and run the pipeline for the main branch.
  2. The vulnerability_reads records created on the imported project should have two records with has_remediations set to true and false for each of the records corresponding to the records in vulnerability_findings_remediations. This can be confirmed using GraphQL API by toggling hasRemediations: true/false and the result should have one vulnerability for each of the hasRemediations true and false value.
query {
  project(fullPath: "[full-path-here]") {
    name
    vulnerabilities(hasRemediations: true) {
      nodes {
        id
        title
        description
        severity
	hasRemediations
      }
    }
  }
}
Edited by Bala Kumar