Add coverage for replacing SAST analyzers

This adds more thorough coverage in preparation of upcoming changes to
replace SAST analyzers.

ee/spec/factories/ci/builds.rb

@@ -22,6 +22,16 @@ FactoryBot.define do
       end
 
       trait report_type do
+        options do
+          {
+            artifacts: {
+              reports: {
+                report_type => Ci::JobArtifact::DEFAULT_FILE_NAMES[report_type]
+              }
+            }
+          }
+        end
+
         after(:build) do |build|
           build.job_artifacts << build(:ee_ci_job_artifact, report_type, job: build)
         end

ee/spec/workers/store_security_reports_worker_spec.rb

@@ -27,7 +27,7 @@ RSpec.describe StoreSecurityReportsWorker do
     end
 
     with_them do
-      let(:pipeline) { build(:ci_pipeline, project: build(:project, :repository, visibility)) if visibility }
+      let(:pipeline) { build(:ee_ci_pipeline, project: build(:project, :repository, visibility)) if visibility }
       let(:expected_result) { [visibility, token_revocation_enabled, secret_detection_vulnerability_found] == [:public, true, true] }
 
       before do
@@ -45,11 +45,9 @@ RSpec.describe StoreSecurityReportsWorker do
   describe '#perform' do
     let(:group)   { create(:group) }
     let(:project) { create(:project, namespace: group) }
-    let(:pipeline) { create(:ci_pipeline, ref: 'master', project: project) }
+    let(:pipeline) { create(:ee_ci_pipeline, ref: 'master', project: project) }
 
     before do
-      allow(Ci::Pipeline).to receive(:find).with(pipeline.id) { pipeline }
-
       allow(::ScanSecurityReportSecretsWorker).to receive(:perform_async).and_return(nil)
       allow_next_instance_of(described_class) do |store_scans_worker|
         allow(store_scans_worker).to receive(:revoke_secret_detection_token?) { true }
@@ -78,6 +76,95 @@ RSpec.describe StoreSecurityReportsWorker do
       end
     end
 
+    context 'when running SAST analyzers that produce duplicate vulnerabilities' do
+      where(vulnerability_finding_signatures_enabled: [true, false])
+      with_them do
+        context 'and properly dedupe vulnerabilities' do
+          let(:artifact_bandit1) { create(:ee_ci_job_artifact, :sast_bandit, job: bandit1_build) }
+          let(:bandit1_build) { create(:ci_build, :sast, :success, user: project.creator, pipeline: pipeline, project: project) }
+
+          let(:artifact_bandit2) { create(:ee_ci_job_artifact, :sast_bandit, job: bandit2_build) }
+          let(:artifact_semgrep) { create(:ee_ci_job_artifact, :sast_semgrep_for_bandit, job: semgrep_build) }
+          let(:pipeline2) { create(:ee_ci_pipeline, ref: 'master', project: project) }
+          let(:bandit2_build) { create(:ci_build, :sast, :success, user: project.creator, pipeline: pipeline2, project: project) }
+          let(:semgrep_build) { create(:ci_build, :sast, :success, user: project.creator, pipeline: pipeline2, project: project) }
+
+          before do
+            stub_licensed_features(
+              sast: true,
+              vulnerability_finding_signatures: vulnerability_finding_signatures_enabled
+            )
+            pipeline.update!(user: bandit1_build.user)
+            pipeline2.update!(user: bandit2_build.user)
+          end
+
+          it 'does not duplicate vulnerabilities' do
+            expect do
+              Security::StoreGroupedScansService.execute([artifact_bandit1])
+            end.to change { Security::Finding.count }.by(1)
+              .and change { Security::Scan.count }.by(1)
+
+            expect do
+              described_class.new.perform(pipeline.id)
+            end.to change { Vulnerabilities::Finding.count }.by(1)
+              .and change { Vulnerability.count }.by(1)
+
+            expect do
+              Security::StoreGroupedScansService.execute([artifact_bandit2, artifact_semgrep])
+            end.to change { Security::Finding.count }.by(2)
+              .and change { Security::Scan.count }.by(2)
+
+            expect do
+              described_class.new.perform(pipeline2.id)
+            end.to change { Vulnerabilities::Finding.count }.by(0)
+              .and change { Vulnerability.count }.by(0)
+          end
+        end
+
+        context 'and improperly dedupe vulnerabilities' do
+          let(:artifact_gosec1) { create(:ee_ci_job_artifact, :sast_gosec, job: gosec1_build) }
+          let(:gosec1_build) { create(:ci_build, :sast, :success, user: project.creator, pipeline: pipeline, project: project) }
+
+          let(:artifact_gosec2) { create(:ee_ci_job_artifact, :sast_gosec, job: gosec2_build) }
+          let(:artifact_semgrep) { create(:ee_ci_job_artifact, :sast_semgrep_for_gosec, job: semgrep_build) }
+          let(:pipeline2) { create(:ee_ci_pipeline, ref: 'master', project: project) }
+          let(:gosec2_build) { create(:ci_build, :sast, :success, user: project.creator, pipeline: pipeline2, project: project) }
+          let(:semgrep_build) { create(:ci_build, :sast, :success, user: project.creator, pipeline: pipeline2, project: project) }
+
+          before do
+            stub_licensed_features(
+              sast: true,
+              vulnerability_finding_signatures: vulnerability_finding_signatures_enabled
+            )
+            pipeline.update!(user: gosec1_build.user)
+            pipeline2.update!(user: gosec2_build.user)
+          end
+
+          it 'duplicates vulnerabilities' do
+            expect do
+              Security::StoreGroupedScansService.execute([artifact_gosec1])
+            end.to change { Security::Finding.count }.by(1)
+              .and change { Security::Scan.count }.by(1)
+
+            expect do
+              described_class.new.perform(pipeline.id)
+            end.to change { Vulnerabilities::Finding.count }.by(1)
+              .and change { Vulnerability.count }.by(1)
+
+            expect do
+              Security::StoreGroupedScansService.execute([artifact_gosec2, artifact_semgrep])
+            end.to change { Security::Finding.count }.by(2)
+              .and change { Security::Scan.count }.by(2)
+
+            expect do
+              described_class.new.perform(pipeline2.id)
+            end.to change { Vulnerabilities::Finding.count }.by(1)
+              .and change { Vulnerability.count }.by(1)
+          end
+        end
+      end
+    end
+
     context "when security reports feature is not available" do
       let(:default_branch) { pipeline.ref }
 

spec/factories/ci/job_artifacts.rb

@@ -302,6 +302,56 @@ FactoryBot.define do
       end
     end
 
+    # Bandit reports are correctly de-duplicated when ran in the same pipeline
+    # as a corresponding semgrep report.
+    # This report does not include signature tracking.
+    trait :sast_bandit 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-bandit.json'), 'application/json')
+      end
+    end
+
+    # Equivalent Semgrep report for :sast_bandit report.
+    # This report includes signature tracking.
+    trait :sast_semgrep_for_bandit 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-semgrep-for-bandit.json'), 'application/json')
+      end
+    end
+
+    # Gosec reports are not correctly de-duplicated when ran in the same pipeline
+    # as a corresponding semgrep report.
+    # This report includes signature tracking.
+    trait :sast_gosec 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-gosec.json'), 'application/json')
+      end
+    end
+
+    # Equivalent Semgrep report for :sast_gosec report.
+    # This report includes signature tracking.
+    trait :sast_semgrep_for_gosec 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-semgrep-for-gosec.json'), 'application/json')
+      end
+    end
+
     trait :common_security_report do
       file_format { :raw }
       file_type { :dependency_scanning }

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

+{
+  "version": "14.0.4",
+  "vulnerabilities": [
+    {
+      "id": "985a5666dcae22adef5ac12f8a8a2dacf9b9b481ae5d87cd0ac1712b0fd64864",
+      "category": "sast",
+      "message": "Deserialization of Untrusted Data",
+      "description": "Avoid using `load()`. `PyYAML.load` can create arbitrary Python\nobjects. A malicious actor could exploit this to run arbitrary\ncode. Use `safe_load()` instead.\n",
+      "cve": "",
+      "severity": "Critical",
+      "scanner": {
+        "id": "bandit",
+        "name": "Bandit"
+      },
+      "location": {
+        "file": "app/app.py",
+        "start_line": 39
+      },
+      "identifiers": [
+        {
+          "type": "bandit_test_id",
+          "name": "Bandit Test ID B506",
+          "value": "B506"
+        }
+      ]
+    }
+  ],
+  "scan": {
+    "scanner": {
+      "id": "bandit",
+      "name": "Bandit",
+      "url": "https://github.com/PyCQA/bandit",
+      "vendor": {
+        "name": "GitLab"
+      },
+      "version": "1.7.1"
+    },
+    "type": "sast",
+    "start_time": "2022-03-11T00:21:49",
+    "end_time": "2022-03-11T00:21:50",
+    "status": "success"
+  }
+}

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

+{
+  "version": "14.0.4",
+  "vulnerabilities": [
+    {
+      "id": "2e5656ff30e2e7cc93c36b4845c8a689ddc47fdbccf45d834c67442fbaa89be0",
+      "category": "sast",
+      "name": "Key Exchange without Entity Authentication",
+      "message": "Use of ssh InsecureIgnoreHostKey should be audited",
+      "description": "The software performs a key exchange with an actor without verifying the identity of that actor.",
+      "cve": "og.go:8:7: func foo() {\n8: \t_ = ssh.InsecureIgnoreHostKey()\n9: }\n:CWE-322",
+      "severity": "Medium",
+      "confidence": "High",
+      "raw_source_code_extract": "7: func foo() {\n8: \t_ = ssh.InsecureIgnoreHostKey()\n9: }\n",
+      "scanner": {
+        "id": "gosec",
+        "name": "Gosec"
+      },
+      "location": {
+        "file": "og.go",
+        "start_line": 8
+      },
+      "identifiers": [
+        {
+          "type": "gosec_rule_id",
+          "name": "Gosec Rule ID G106",
+          "value": "G106"
+        },
+        {
+          "type": "CWE",
+          "name": "CWE-322",
+          "value": "322",
+          "url": "https://cwe.mitre.org/data/definitions/322.html"
+        }
+      ],
+      "tracking": {
+        "type": "source",
+        "items": [
+          {
+            "file": "og.go",
+            "line_start": 8,
+            "line_end": 8,
+            "signatures": [
+              {
+                "algorithm": "scope_offset",
+                "value": "og.go|foo[0]:1"
+              }
+            ]
+          }
+        ]
+      }
+    }
+  ],
+  "scan": {
+    "scanner": {
+      "id": "gosec",
+      "name": "Gosec",
+      "url": "https://github.com/securego/gosec",
+      "vendor": {
+        "name": "GitLab"
+      },
+      "version": "2.10.0"
+    },
+    "type": "sast",
+    "start_time": "2022-03-15T20:33:12",
+    "end_time": "2022-03-15T20:33:17",
+    "status": "success"
+  }
+}

spec/fixtures/security_reports/master/gl-sast-report-semgrep-for-bandit.json

+{
+  "version": "14.0.4",
+  "vulnerabilities": [
+    {
+      "id": "985a5666dcae22adef5ac12f8a8a2dacf9b9b481ae5d87cd0ac1712b0fd64864",
+      "category": "sast",
+      "message": "Deserialization of Untrusted Data",
+      "description": "Avoid using `load()`. `PyYAML.load` can create arbitrary Python\nobjects. A malicious actor could exploit this to run arbitrary\ncode. Use `safe_load()` instead.\n",
+      "cve": "",
+      "severity": "Critical",
+      "scanner": {
+        "id": "semgrep",
+        "name": "Semgrep"
+      },
+      "location": {
+        "file": "app/app.py",
+        "start_line": 39
+      },
+      "identifiers": [
+        {
+          "type": "semgrep_id",
+          "name": "bandit.B506",
+          "value": "bandit.B506",
+          "url": "https://semgrep.dev/r/gitlab.bandit.B506"
+        },
+        {
+          "type": "cwe",
+          "name": "CWE-502",
+          "value": "502",
+          "url": "https://cwe.mitre.org/data/definitions/502.html"
+        },
+        {
+          "type": "bandit_test_id",
+          "name": "Bandit Test ID B506",
+          "value": "B506"
+        }
+      ],
+      "tracking": {
+        "type": "source",
+        "items": [
+          {
+            "file": "app/app.py",
+            "line_start": 39,
+            "line_end": 39,
+            "signatures": [
+              {
+                "algorithm": "scope_offset",
+                "value": "app/app.py|yaml_hammer[0]:13"
+              }
+            ]
+          }
+        ]
+      }
+    }
+  ],
+  "scan": {
+    "scanner": {
+      "id": "semgrep",
+      "name": "Semgrep",
+      "url": "https://github.com/returntocorp/semgrep",
+      "vendor": {
+        "name": "GitLab"
+      },
+      "version": "0.82.0"
+    },
+    "type": "sast",
+    "start_time": "2022-03-11T18:48:16",
+    "end_time": "2022-03-11T18:48:22",
+    "status": "success"
+  }
+}

spec/fixtures/security_reports/master/gl-sast-report-semgrep-for-gosec.json

+{
+  "version": "14.0.4",
+  "vulnerabilities": [
+    {
+      "id": "79f6537b7ec83c7717f5bd1a4f12645916caafefe2e4359148d889855505aa67",
+      "category": "sast",
+      "message": "Key Exchange without Entity Authentication",
+      "description": "Audit the use of ssh.InsecureIgnoreHostKey\n",
+      "cve": "",
+      "severity": "Medium",
+      "scanner": {
+        "id": "semgrep",
+        "name": "Semgrep"
+      },
+      "location": {
+        "file": "og.go",
+        "start_line": 8
+      },
+      "identifiers": [
+        {
+          "type": "semgrep_id",
+          "name": "gosec.G106-1",
+          "value": "gosec.G106-1"
+        },
+        {
+          "type": "cwe",
+          "name": "CWE-322",
+          "value": "322",
+          "url": "https://cwe.mitre.org/data/definitions/322.html"
+        },
+        {
+          "type": "gosec_rule_id",
+          "name": "Gosec Rule ID G106",
+          "value": "G106"
+        }
+      ],
+      "tracking": {
+        "type": "source",
+        "items": [
+          {
+            "file": "og.go",
+            "line_start": 8,
+            "line_end": 8,
+            "signatures": [
+              {
+                "algorithm": "scope_offset",
+                "value": "og.go|foo[0]:1"
+              }
+            ]
+          }
+        ]
+      }
+    }
+  ],
+  "scan": {
+    "scanner": {
+      "id": "semgrep",
+      "name": "Semgrep",
+      "url": "https://github.com/returntocorp/semgrep",
+      "vendor": {
+        "name": "GitLab"
+      },
+      "version": "0.82.0"
+    },
+    "type": "sast",
+    "start_time": "2022-03-15T20:36:58",
+    "end_time": "2022-03-15T20:37:05",
+    "status": "success"
+  }
+}