diff --git a/app/uploaders/gitlab_uploader.rb b/app/uploaders/gitlab_uploader.rb
index 29091eb93ef1a2f40ff37247fedd104c1e3dc7e4..62024bff4c080d12936117b3d79de2b675c40bb5 100644
--- a/app/uploaders/gitlab_uploader.rb
+++ b/app/uploaders/gitlab_uploader.rb
@@ -115,6 +115,15 @@ def open
end
end
+ def multi_read(offsets)
+ open do |stream|
+ offsets.map do |start_offset, end_offset|
+ stream.seek(start_offset)
+ stream.read(end_offset - start_offset + 1)
+ end
+ end
+ end
+
# Used to replace an existing upload with another +file+ without modifying stored metadata
# Use this method only to repair/replace an existing upload, or to upload to a Geo secondary node
#
diff --git a/ee/app/finders/security/findings_finder.rb b/ee/app/finders/security/findings_finder.rb
index 19f127fa01f7a64f00295b9afc5d8d9acee4300c..7846422a8b5c431b7d92983ce4953d8c714c14ff 100644
--- a/ee/app/finders/security/findings_finder.rb
+++ b/ee/app/finders/security/findings_finder.rb
@@ -19,7 +19,8 @@ class FindingsFinder
include ::VulnerabilityFindingHelpers
ResultSet = Struct.new(:relation, :findings) do
- delegate :current_page, :limit_value, :total_pages, :total_count, :next_page, :prev_page, to: :relation
+ delegate :current_page, :limit_value, :total_pages, :total_count,
+ :next_page, :prev_page, :page, :empty?, to: :relation
end
DEFAULT_PAGE = 1
@@ -31,7 +32,7 @@ def initialize(pipeline, params: {})
end
def execute
- return unless has_security_findings?
+ return ResultSet.new(Security::Finding.none.page(DEFAULT_PAGE), []) unless has_security_findings?
ResultSet.new(security_findings, findings)
end
diff --git a/ee/app/finders/security/pure_findings_finder.rb b/ee/app/finders/security/pure_findings_finder.rb
new file mode 100644
index 0000000000000000000000000000000000000000..b123a0b4a06c7c317122c7f6e3e47f8dde1b159e
--- /dev/null
+++ b/ee/app/finders/security/pure_findings_finder.rb
@@ -0,0 +1,39 @@
+# frozen_string_literal: true
+
+# Security::PureFindingsFinder
+#
+# This finder returns Active Record relation of `Security::Finding` model
+# which is different than the other finders maintained by threat insights.
+# It's called pure because it does not depend on report artifacts(mostly)
+# and uses the data stored in the `Security::Finding` model.
+#
+# Arguments:
+# pipeline - object to filter findings
+# params:
+# severity: Array<String>
+# confidence: Array<String>
+# report_type: Array<String>
+# scope: String
+# page: Int
+# per_page: Int
+
+module Security
+ class PureFindingsFinder < FindingsFinder
+ def execute
+ security_findings.tap do |findings|
+ findings.each(&:remediations) # initiates Batchloader
+ end
+ end
+
+ def available?
+ pipeline.security_findings.first&.finding_data.present?
+ end
+
+ private
+
+ def security_findings
+ super.with_feedbacks
+ .with_vulnerability
+ end
+ end
+end
diff --git a/ee/app/models/security/finding.rb b/ee/app/models/security/finding.rb
index e12f1d68f6547015b3617c61085f75589a7bd780..e61b4855f8819c44f10e2dc87a83c077ade126a1 100644
--- a/ee/app/models/security/finding.rb
+++ b/ee/app/models/security/finding.rb
@@ -10,9 +10,12 @@
module Security
class Finding < ApplicationRecord
include EachBatch
+ include Presentable
include PartitionedTable
MAX_PARTITION_SIZE = 10.gigabyte
+ ATTRIBUTES_DELEGATED_TO_FINDING_DATA = %i[name description solution location identifiers links false_positive?
+ assets evidence details remediation_byte_offsets].freeze
self.table_name = 'security_findings'
self.primary_key = :id # As ActiveRecord does not support compound PKs
@@ -26,8 +29,15 @@ class Finding < ApplicationRecord
belongs_to :scan, inverse_of: :findings, optional: false
belongs_to :scanner, class_name: 'Vulnerabilities::Scanner', inverse_of: :security_findings, optional: false
+ belongs_to :vulnerability_read,
+ class_name: 'Vulnerabilities::Read',
+ primary_key: :uuid,
+ foreign_key: :uuid,
+ inverse_of: :security_findings
has_one :build, through: :scan, disable_joins: true
+ has_one :vulnerability, through: :vulnerability_read
+
has_many :feedbacks,
class_name: 'Vulnerabilities::Feedback',
inverse_of: :security_finding,
@@ -62,15 +72,17 @@ class Finding < ApplicationRecord
.where('vulnerability_feedback.finding_uuid = security_findings.uuid'))
end
scope :latest, -> { joins(:scan).merge(Security::Scan.latest_successful) }
- scope :ordered, -> { order(severity: :desc, confidence: :desc, id: :asc) }
+ scope :ordered, -> { order(severity: :desc, id: :asc) }
scope :with_pipeline_entities, -> { preload(build: [:job_artifacts, :pipeline]) }
- scope :with_scan, -> { includes(:scan) }
+ scope :with_scan, -> { preload(:scan) }
scope :with_scanner, -> { includes(:scanner) }
+ scope :with_feedbacks, -> { includes(:feedbacks) }
+ scope :with_vulnerability, -> { includes(:vulnerability) }
scope :deduplicated, -> { where(deduplicated: true) }
scope :grouped_by_scan_type, -> { joins(:scan).group('security_scans.scan_type') }
- delegate :scan_type, :pipeline, to: :scan, allow_nil: true
- delegate :project, to: :pipeline
+ delegate :scan_type, :pipeline, :remediations_proxy, to: :scan, allow_nil: true
+ delegate :project, :sha, to: :pipeline
class << self
def count_by_scan_type
@@ -103,5 +115,56 @@ def last_finding_in_partition(partition_number)
where(partition_number: partition_number).last
end
end
+
+ # Following alias attributes as used by `Vulnerabilities::FindingEntity`
+ alias_attribute :raw_metadata, :finding_data
+ alias_attribute :report_type, :scan_type
+
+ def dismissal_feedback
+ feedbacks.find(&:for_dismissal?)
+ end
+
+ def issue_feedback
+ feedbacks.find(&:for_issue?)
+ end
+
+ def merge_request_feedback
+ feedbacks.find(&:for_merge_request?)
+ end
+
+ def state
+ return vulnerability.state if vulnerability
+
+ dismissal_feedback ? 'dismissed' : 'detected'
+ end
+
+ # Symbolizing the hash keys is important as Grape entity
+ # works with symbolized keys only.
+ # See https://github.com/ruby-grape/grape-entity/issues/223
+ def symbolized_finding_data
+ @symbolized_finding_data ||= finding_data.deep_symbolize_keys
+ end
+
+ def finding_data=(value)
+ super
+ ensure
+ @symbolized_finding_data = nil
+ end
+
+ # Defines methods for the keys exist in `finding_data` to support the same
+ # interface with `Vulnerabilities::Finding` model as these methods are used
+ # by `Vulnerabilities::FindingEntity`.
+ ATTRIBUTES_DELEGATED_TO_FINDING_DATA.each do |delegated_attribute|
+ define_method(delegated_attribute) do
+ symbolized_finding_data.fetch(delegated_attribute)
+ end
+ end
+
+ def remediations
+ return [] unless symbolized_finding_data[:remediation_byte_offsets]
+
+ symbolized_finding_data[:remediation_byte_offsets].map { |offset| offset.values_at(:start_byte, :end_byte) }
+ .then { remediations_proxy.by_byte_offsets(_1) }
+ end
end
end
diff --git a/ee/app/models/security/remediations_proxy.rb b/ee/app/models/security/remediations_proxy.rb
new file mode 100644
index 0000000000000000000000000000000000000000..0c5f12726401470df976083ad19b9aa46ec67416
--- /dev/null
+++ b/ee/app/models/security/remediations_proxy.rb
@@ -0,0 +1,30 @@
+# frozen_string_literal: true
+
+module Security
+ class RemediationsProxy
+ attr_reader :file
+
+ def initialize(file)
+ @file = file
+ end
+
+ def by_byte_offsets(byte_offsets)
+ byte_offsets.map do |offsets|
+ batch_load(offsets)
+ end
+ end
+
+ private
+
+ def batch_load(offsets)
+ BatchLoader.for(offsets).batch(key: object_id) do |byte_offsets, loader|
+ byte_offsets.uniq
+ .sort
+ .then { |offsets| file.multi_read(offsets) }
+ .map { |data| Gitlab::Json.parse(data) }
+ .zip(byte_offsets)
+ .each { |remediation, offsets| loader.call(offsets, remediation) }
+ end
+ end
+ end
+end
diff --git a/ee/app/models/security/scan.rb b/ee/app/models/security/scan.rb
index 88a9417c6db98476df4131dd345eca8a4fc3d37b..756573f56a2fcce27c543cd159284b20c35332c9 100644
--- a/ee/app/models/security/scan.rb
+++ b/ee/app/models/security/scan.rb
@@ -107,6 +107,10 @@ def report_primary_identifiers
@report_primary_identifiers ||= security_report&.primary_identifiers
end
+ def remediations_proxy
+ @remediations_proxy ||= RemediationsProxy.new(job_artifact.file)
+ end
+
private
def security_report
diff --git a/ee/app/models/vulnerabilities/finding.rb b/ee/app/models/vulnerabilities/finding.rb
index 9f97eaff4a53c46ff084eaf3547650ead730e90c..fccbbf8bc5165d611ff107ba2afd036369f69d8b 100644
--- a/ee/app/models/vulnerabilities/finding.rb
+++ b/ee/app/models/vulnerabilities/finding.rb
@@ -380,7 +380,7 @@ def pipeline_branch
end
def false_positive?
- vulnerability_flags.false_positive.any?
+ vulnerability_flags.any?(&:false_positive?)
end
def first_finding_pipeline
diff --git a/ee/app/models/vulnerabilities/read.rb b/ee/app/models/vulnerabilities/read.rb
index 5a244bde35b4f38d47ebfeb3936b5cfc6d79d798..f4299fcb56d4b7adbe7bb3de57cfebc11132c3c5 100644
--- a/ee/app/models/vulnerabilities/read.rb
+++ b/ee/app/models/vulnerabilities/read.rb
@@ -12,6 +12,12 @@ class Read < ApplicationRecord
belongs_to :project
belongs_to :scanner, class_name: 'Vulnerabilities::Scanner'
+ has_many :security_findings,
+ class_name: 'Security::Finding',
+ primary_key: :uuid,
+ foreign_key: :uuid,
+ inverse_of: :vulnerability_read
+
validates :vulnerability_id, uniqueness: true, presence: true
validates :project_id, presence: true
validates :scanner_id, presence: true
diff --git a/ee/app/serializers/vulnerabilities/finding_entity.rb b/ee/app/serializers/vulnerabilities/finding_entity.rb
index f878020ed1e9719e0ed84064348e293d60fca831..ae5ec53908efc83ebbe815c961adbad728bb4186 100644
--- a/ee/app/serializers/vulnerabilities/finding_entity.rb
+++ b/ee/app/serializers/vulnerabilities/finding_entity.rb
@@ -13,9 +13,7 @@ class Vulnerabilities::FindingEntity < Grape::Entity
expose :create_jira_issue_url do |occurrence|
create_jira_issue_url_for(occurrence)
end
- expose :false_positive, if: -> (_, _) { expose_false_positive? } do |occurrence|
- occurrence.vulnerability_flags.any?(&:false_positive?)
- end
+ expose :false_positive?, as: :false_positive, if: -> (_, _) { expose_false_positive? }
expose :create_vulnerability_feedback_issue_path do |occurrence|
create_vulnerability_feedback_issue_path(occurrence.project)
end
@@ -53,7 +51,7 @@ class Vulnerabilities::FindingEntity < Grape::Entity
expose :scan
expose :blob_path do |occurrence|
- occurrence.present.blob_path
+ occurrence.present(presenter_class: Vulnerabilities::FindingPresenter).blob_path
end
alias_method :occurrence, :object
diff --git a/ee/config/feature_flags/development/utilize_finding_data.yml b/ee/config/feature_flags/development/utilize_finding_data.yml
new file mode 100644
index 0000000000000000000000000000000000000000..336bdce5ad097293b95336200e468d1ba428100d
--- /dev/null
+++ b/ee/config/feature_flags/development/utilize_finding_data.yml
@@ -0,0 +1,8 @@
+---
+name: utilize_finding_data
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/103652
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/383430
+milestone: '15.7'
+type: development
+group: group::threat insights
+default_enabled: false
diff --git a/ee/lib/api/vulnerability_findings.rb b/ee/lib/api/vulnerability_findings.rb
index 38f49165c42b5b427e06b68276eaa85b387b64bb..67d99891743724e07e05e85d9585fab4d03dccf2 100644
--- a/ee/lib/api/vulnerability_findings.rb
+++ b/ee/lib/api/vulnerability_findings.rb
@@ -23,31 +23,26 @@ def vulnerability_findings
@vulnerability_findings ||= begin
return paginate(Kaminari.paginate_array([])) unless pipeline
- with_adaptive_finder || with_vulnerabilities_finder
+ with_pure_finder || with_adaptive_finder
end
end
- # We are using this fallback to provide an early response to the users
- # until the `security_findings` are stored.
- # We will remove this fallback by https://gitlab.com/gitlab-org/gitlab/-/issues/334488
- def with_vulnerabilities_finder
- aggregated_report = Security::PipelineVulnerabilitiesFinder.new(pipeline: pipeline, params: declared_params).execute
-
- # We might have to add rubocop:disable annotation here in case
- # https://gitlab.com/gitlab-org/gitlab/issues/32763 happens, using
- # Kaminari.paginate_array here is correct
- # See https://gitlab.com/gitlab-org/gitlab/issues/33588#note_291849433
- # for discussion
- paginate(Kaminari.paginate_array(aggregated_report.findings))
- rescue Security::PipelineVulnerabilitiesFinder::ParseError
- paginate(Kaminari.paginate_array([]))
+ def with_pure_finder
+ pure_finder = Security::PureFindingsFinder.new(pipeline, params: declared_params)
+ return unless Feature.enabled?(:utilize_finding_data, pipeline.project) && pure_finder.available?
+
+ paginate(pure_finder.execute)
end
def with_adaptive_finder
result = Security::FindingsFinder.new(pipeline, params: declared_params).execute
- return unless result
+ paginate(result)
+
+ if Feature.disabled?(:deprecate_vulnerabilities_feedback, user_project)
+ Gitlab::Vulnerabilities::FindingsPreloader.preload_feedback!(result.findings)
+ end
- paginate(result).findings
+ result.findings
end
end
@@ -101,10 +96,6 @@ def with_adaptive_finder
get ':id/vulnerability_findings' do
authorize! :read_security_resource, user_project
- if Feature.disabled?(:deprecate_vulnerabilities_feedback, user_project)
- Gitlab::Vulnerabilities::FindingsPreloader.preload_feedback!(vulnerability_findings)
- end
-
present vulnerability_findings,
with: ::Vulnerabilities::FindingEntity,
request: GrapeRequestProxy.new(request, current_user)
diff --git a/ee/spec/factories/security_scans.rb b/ee/spec/factories/security_scans.rb
index 08d5a741a330bfcf9c46d7273a3d7f9621c205a9..2362ca73ec3672e9f09c6a569c8b30b267031b83 100644
--- a/ee/spec/factories/security_scans.rb
+++ b/ee/spec/factories/security_scans.rb
@@ -23,5 +23,26 @@
trait :purged do
status { :purged }
end
+
+ trait :with_findings do
+ # rubocop:disable RSpec/FactoryBot/StrategyInCallback
+ after(:create) do |scan|
+ artifact = create(:ee_ci_job_artifact, scan.scan_type, job: scan.build, project: scan.project)
+ report = create(:ci_reports_security_report, pipeline: scan.pipeline, type: scan.scan_type)
+
+ Gitlab::Ci::Parsers.fabricate!(scan.scan_type, artifact.file.read, report).parse!
+ report = Security::MergeReportsService.new(report).execute
+
+ report.findings.each do |finding|
+ create(:security_finding,
+ severity: finding.severity,
+ confidence: finding.confidence,
+ uuid: finding.uuid,
+ deduplicated: true,
+ scan: scan)
+ end
+ end
+ # rubocop:enable RSpec/FactoryBot/StrategyInCallback
+ end
end
end
diff --git a/ee/spec/finders/security/findings_finder_spec.rb b/ee/spec/finders/security/findings_finder_spec.rb
index 2310652e48de083a2917d25fb10616c8296141dc..dbf94aeb4c1f584d544922fe879c7be459cbb468 100644
--- a/ee/spec/finders/security/findings_finder_spec.rb
+++ b/ee/spec/finders/security/findings_finder_spec.rb
@@ -2,377 +2,56 @@
require 'spec_helper'
-RSpec.describe Security::FindingsFinder do
- let_it_be(:pipeline) { create(:ci_pipeline) }
- let_it_be(:build_1) { create(:ci_build, :success, name: 'dependency_scanning', pipeline: pipeline) }
- let_it_be(:build_2) { create(:ci_build, :success, name: 'sast', pipeline: pipeline) }
- let_it_be(:artifact_ds) { create(:ee_ci_job_artifact, :dependency_scanning, job: build_1) }
- let_it_be(:artifact_sast) { create(:ee_ci_job_artifact, :sast, job: build_2) }
- let_it_be(:report_ds) { create(:ci_reports_security_report, pipeline: pipeline, type: :dependency_scanning) }
- let_it_be(:report_sast) { create(:ci_reports_security_report, pipeline: pipeline, type: :sast) }
+RSpec.describe Security::FindingsFinder, feature_category: :vulnerability_management do
+ it_behaves_like 'security findings finder' do
+ let(:findings) { service_object.execute.findings }
+ let(:query_limit) { 11 }
- let(:severity_levels) { nil }
- let(:confidence_levels) { nil }
- let(:report_types) { nil }
- let(:scope) { nil }
- let(:page) { nil }
- let(:per_page) { nil }
- let(:service_object) { described_class.new(pipeline, params: params) }
- let(:params) do
- {
- severity: severity_levels,
- confidence: confidence_levels,
- report_type: report_types,
- scope: scope,
- page: page,
- per_page: per_page
- }
- end
-
- describe '#execute' do
- context 'when the pipeline does not have security findings' do
- subject { service_object.execute }
-
- it { is_expected.to be_nil }
- end
-
- context 'when the pipeline has security findings' do
- let(:finder_result) { service_object.execute }
-
- before(:all) do
- ds_content = File.read(artifact_ds.file.path)
- Gitlab::Ci::Parsers::Security::DependencyScanning.parse!(ds_content, report_ds)
- report_ds.merge!(report_ds)
- sast_content = File.read(artifact_sast.file.path)
- Gitlab::Ci::Parsers::Security::Sast.parse!(sast_content, report_sast)
- report_sast.merge!(report_sast)
-
- findings = { artifact_ds => report_ds, artifact_sast => report_sast }.collect do |artifact, report|
- scan = create(:security_scan, :latest_successful, scan_type: artifact.job.name, build: artifact.job)
-
- report.findings.collect do |finding, index|
- create(:security_finding,
- severity: finding.severity,
- confidence: finding.confidence,
- uuid: finding.uuid,
- deduplicated: true,
- scan: scan)
- end
- end.flatten
-
- findings.second.update!(deduplicated: false)
- end
-
- before do
- stub_licensed_features(sast: true, dependency_scanning: true)
- end
-
- context 'N+1 queries' do
- it 'does not cause N+1 queries' do
- expect { finder_result }.not_to exceed_query_limit(11)
- end
+ context 'when the `security_findings` records have `overridden_uuid`s' do
+ let(:security_findings) { Security::Finding.by_build_ids(build_1) }
+ let(:expected_uuids) do
+ Security::Finding.where(overridden_uuid: nil).pluck(:uuid)
+ .concat(Security::Finding.where.not(overridden_uuid: nil).pluck(:overridden_uuid)) -
+ [Security::Finding.second[:overridden_uuid]]
end
- describe '#current_page' do
- subject { finder_result.current_page }
-
- context 'when the page is not provided' do
- it { is_expected.to be(1) }
- end
-
- context 'when the page is provided' do
- let(:page) { 2 }
-
- it { is_expected.to be(2) }
- end
- end
-
- describe '#limit_value' do
- subject { finder_result.limit_value }
-
- context 'when the per_page is not provided' do
- it { is_expected.to be(20) }
- end
-
- context 'when the per_page is provided' do
- let(:per_page) { 100 }
-
- it { is_expected.to be(100) }
- end
- end
-
- describe '#total_pages' do
- subject { finder_result.total_pages }
-
- context 'when the per_page is not provided' do
- it { is_expected.to be(1) }
- end
+ subject { findings.map(&:uuid) }
- context 'when the per_page is provided' do
- let(:per_page) { 3 }
-
- it { is_expected.to be(3) }
- end
- end
-
- describe '#total_count' do
- subject { finder_result.total_count }
-
- context 'when the scope is not provided' do
- let(:finding_to_dismiss) { Security::Finding.first }
-
- before do
- stub_feature_flags(deprecate_vulnerabilities_feedback: deprecate_vulnerabilities_feedback?)
- end
-
- context 'when the `deprecate_vulnerabilities_feedback` feature is enabled' do
- let(:deprecate_vulnerabilities_feedback?) { true }
-
- before do
- create(:vulnerabilities_finding, :dismissed, uuid: finding_to_dismiss.uuid)
- end
-
- it { is_expected.to be(7) }
- end
-
- context 'when the `deprecate_vulnerabilities_feedback` feature is disabled' do
- let(:deprecate_vulnerabilities_feedback?) { false }
-
- before do
- create(:vulnerability_feedback,
- :dismissal,
- project: pipeline.project,
- category: :dependency_scanning,
- finding_uuid: finding_to_dismiss.uuid)
- end
-
- it { is_expected.to be(7) }
- end
- end
-
- context 'when the scope is provided as `all`' do
- let(:scope) { 'all' }
-
- it { is_expected.to be(8) }
+ before do
+ security_findings.each do |security_finding|
+ security_finding.update!(overridden_uuid: security_finding.uuid, uuid: SecureRandom.uuid)
end
end
- describe '#next_page' do
- subject { finder_result.next_page }
-
- context 'when the page is not provided' do
- # Limit per_page to force pagination on smaller dataset
- let(:per_page) { 2 }
-
- it { is_expected.to be(2) }
- end
-
- context 'when the page is provided' do
- let(:page) { 2 }
+ it { is_expected.to match_array(expected_uuids) }
+ end
- it { is_expected.to be_nil }
- end
+ describe '#vulnerability_flags' do
+ before do
+ stub_licensed_features(sast_fp_reduction: true)
end
- describe '#prev_page' do
- subject { finder_result.prev_page }
-
- context 'when the page is not provided' do
- it { is_expected.to be_nil }
- end
-
- context 'when the page is provided' do
- let(:page) { 2 }
- # Limit per_page to force pagination on smaller dataset
- let(:per_page) { 2 }
-
- it { is_expected.to be(1) }
+ context 'with no vulnerability flags present' do
+ it 'does not have any vulnerability flag' do
+ expect(findings).to all(have_attributes(vulnerability_flags: be_empty))
end
end
- describe '#vulnerability_flags' do
+ context 'with some vulnerability flags present' do
before do
- stub_licensed_features(sast_fp_reduction: true)
- end
-
- context 'with no vulnerability flags present' do
- it 'does not have any vulnerability flag' do
- expect(finder_result.findings).to all(have_attributes(vulnerability_flags: be_empty))
- end
- end
-
- context 'with some vulnerability flags present' do
- before do
- allow_next_instance_of(Gitlab::Ci::Reports::Security::Finding) do |finding|
- allow(finding).to receive(:flags).and_return([create(:ci_reports_security_flag)]) if finding.report_type == 'sast'
- end
- end
-
- it 'has some vulnerability_findings with vulnerability flag' do
- expect(finder_result.findings).to include(have_attributes(vulnerability_flags: be_present))
- end
-
- it 'does not have any vulnerability_flag if license is not available' do
- stub_licensed_features(sast_fp_reduction: false)
-
- expect(finder_result.findings).to all(have_attributes(vulnerability_flags: be_empty))
- end
- end
- end
-
- describe '#findings' do
- subject { finder_result.findings.map(&:uuid) }
-
- context 'with the default parameters' do
- let(:expected_uuids) { Security::Finding.pluck(:uuid) - [Security::Finding.second[:uuid]] }
-
- it { is_expected.to match_array(expected_uuids) }
- end
-
- context 'when the uuid is provided' do
- let(:uuid) { Security::Finding.pick(:uuid) }
- let(:params) do
- {
- uuid: uuid
- }
- end
-
- it { is_expected.to match_array([uuid]) }
- end
-
- context 'when the page is provided' do
- let(:page) { 2 }
- # Limit per_page to force pagination on smaller dataset
- let(:per_page) { 2 }
- let(:expected_uuids) { Security::Finding.pluck(:uuid)[4..5] }
-
- it { is_expected.to match_array(expected_uuids) }
- end
-
- context 'when the per_page is provided' do
- let(:per_page) { 1 }
- let(:expected_uuids) { [Security::Finding.pick(:uuid)] }
-
- it { is_expected.to match_array(expected_uuids) }
- end
-
- context 'when the `severity_levels` is provided' do
- let(:severity_levels) { [:medium] }
- let(:expected_uuids) { Security::Finding.where(severity: 'medium').pluck(:uuid) }
-
- it { is_expected.to match_array(expected_uuids) }
- end
-
- context 'when the `confidence_levels` is provided' do
- let(:confidence_levels) { [:low] }
- let(:expected_uuids) { Security::Finding.where(confidence: 'low' ).pluck(:uuid) }
-
- it { is_expected.to match_array(expected_uuids) }
- end
-
- context 'when the `report_types` is provided' do
- let(:report_types) { :dependency_scanning }
- let(:expected_uuids) do
- Security::Finding.by_scan(Security::Scan.find_by(scan_type: 'dependency_scanning')).pluck(:uuid) -
- [Security::Finding.second[:uuid]]
+ allow_next_instance_of(Gitlab::Ci::Reports::Security::Finding) do |finding|
+ allow(finding).to receive(:flags).and_return([create(:ci_reports_security_flag)]) if finding.report_type == 'sast'
end
-
- it { is_expected.to match_array(expected_uuids) }
- end
-
- context 'when the `scope` is provided as `all`' do
- let(:scope) { 'all' }
-
- let(:expected_uuids) { Security::Finding.pluck(:uuid) - [Security::Finding.second[:uuid]] }
-
- it { is_expected.to match_array(expected_uuids) }
end
- context 'when there is a retried build' do
- let(:retried_build) { create(:ci_build, :success, :retried, name: 'dependency_scanning', pipeline: pipeline) }
- let(:artifact) { create(:ee_ci_job_artifact, :dependency_scanning, job: retried_build) }
- let(:report) { create(:ci_reports_security_report, pipeline: pipeline, type: :dependency_scanning) }
- let(:report_types) { :dependency_scanning }
- let(:expected_uuids) do
- Security::Finding.by_scan(Security::Scan.find_by(scan_type: 'dependency_scanning')).pluck(:uuid) -
- [Security::Finding.second[:uuid]]
- end
-
- before do
- retried_content = File.read(artifact.file.path)
- Gitlab::Ci::Parsers::Security::DependencyScanning.parse!(retried_content, report)
- report.merge!(report)
-
- scan = create(:security_scan, scan_type: retried_build.name, build: retried_build, latest: false)
-
- report.findings.each_with_index do |finding, index|
- create(:security_finding,
- severity: finding.severity,
- confidence: finding.confidence,
- uuid: finding.uuid,
- deduplicated: true,
- scan: scan)
- end
- end
-
- it { is_expected.to match_array(expected_uuids) }
+ it 'has some vulnerability_findings with vulnerability flag' do
+ expect(findings).to include(have_attributes(vulnerability_flags: be_present))
end
- context 'when the `security_findings` records have `overridden_uuid`s' do
- let(:security_findings) { Security::Finding.by_build_ids(build_1) }
-
- let(:expected_uuids) do
- Security::Finding.where(overridden_uuid: nil).pluck(:uuid)
- .concat(Security::Finding.where.not(overridden_uuid: nil).pluck(:overridden_uuid)) -
- [Security::Finding.second[:overridden_uuid]]
- end
+ it 'does not have any vulnerability_flag if license is not available' do
+ stub_licensed_features(sast_fp_reduction: false)
- before do
- security_findings.each do |security_finding|
- security_finding.update!(overridden_uuid: security_finding.uuid, uuid: SecureRandom.uuid)
- end
- end
-
- it { is_expected.to match_array(expected_uuids) }
- end
-
- context 'when a build has more than one security report artifacts' do
- let(:report_types) { :secret_detection }
- let(:secret_detection_report) { create(:ci_reports_security_report, pipeline: pipeline, type: :secret_detection) }
- let(:expected_uuids) { secret_detection_report.findings.map(&:uuid) }
-
- before do
- scan = create(:security_scan, :latest_successful, scan_type: :secret_detection, build: build_2)
- artifact = create(:ee_ci_job_artifact, :secret_detection, job: build_2)
- report_content = File.read(artifact.file.path)
-
- Gitlab::Ci::Parsers::Security::SecretDetection.parse!(report_content, secret_detection_report)
-
- secret_detection_report.findings.each_with_index do |finding, index|
- create(:security_finding,
- severity: finding.severity,
- confidence: finding.confidence,
- uuid: finding.uuid,
- deduplicated: true,
- scan: scan)
- end
- end
-
- it { is_expected.to match_array(expected_uuids) }
- end
-
- context 'when a vulnerability already exist for a security finding' do
- let(:vulnerability) { create(:vulnerability, :with_finding, project: pipeline.project) }
-
- subject { finder_result.findings.map(&:vulnerability).first }
-
- before do
- vulnerability.finding.update_attribute(:uuid, Security::Finding.first.uuid)
- end
-
- describe 'the vulnerability is included in results' do
- it { is_expected.to eq(vulnerability) }
- end
+ expect(findings).to all(have_attributes(vulnerability_flags: be_empty))
end
end
end
diff --git a/ee/spec/finders/security/pure_findings_finder_spec.rb b/ee/spec/finders/security/pure_findings_finder_spec.rb
new file mode 100644
index 0000000000000000000000000000000000000000..c0228596584d300b9c791da15d4a18c1696e0251
--- /dev/null
+++ b/ee/spec/finders/security/pure_findings_finder_spec.rb
@@ -0,0 +1,22 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Security::PureFindingsFinder, feature_category: :vulnerability_management do
+ it_behaves_like 'security findings finder' do
+ let(:findings) { finder_result.to_a }
+ let(:query_limit) { 8 }
+
+ describe 'parsing artifacts' do
+ before do
+ allow(::Gitlab::Ci::Parsers).to receive(:fabricate!)
+ end
+
+ it 'does not parse artifacts' do
+ service_object.execute
+
+ expect(::Gitlab::Ci::Parsers).not_to have_received(:fabricate!)
+ end
+ end
+ end
+end
diff --git a/ee/spec/models/security/finding_spec.rb b/ee/spec/models/security/finding_spec.rb
index 90fbe64923dff3da766b3388445c685a31787322..60a12d287582e5334b71fc428229143c158f4b1a 100644
--- a/ee/spec/models/security/finding_spec.rb
+++ b/ee/spec/models/security/finding_spec.rb
@@ -2,16 +2,18 @@
require 'spec_helper'
-RSpec.describe Security::Finding do
+RSpec.describe Security::Finding, feature_category: :vulnerability_management do
let_it_be(:scan_1) { create(:security_scan, :latest_successful, scan_type: :sast) }
let_it_be(:scan_2) { create(:security_scan, :latest_successful, scan_type: :dast) }
- let_it_be(:finding_1) { create(:security_finding, scan: scan_1) }
- let_it_be(:finding_2) { create(:security_finding, scan: scan_2) }
+ let_it_be(:finding_1, refind: true) { create(:security_finding, scan: scan_1) }
+ let_it_be(:finding_2, refind: true) { create(:security_finding, scan: scan_2) }
describe 'associations' do
it { is_expected.to belong_to(:scan).required }
it { is_expected.to belong_to(:scanner).required }
+ it { is_expected.to belong_to(:vulnerability_read).class_name('Vulnerabilities::Read') }
it { is_expected.to have_one(:build).through(:scan) }
+ it { is_expected.to have_one(:vulnerability).through(:vulnerability_read) }
it {
is_expected.to have_many(:feedbacks)
@@ -148,16 +150,16 @@
end
describe '.ordered' do
- let_it_be(:finding_3) { create(:security_finding, severity: :critical, confidence: :confirmed) }
- let_it_be(:finding_4) { create(:security_finding, severity: :critical, confidence: :high) }
+ let_it_be(:finding_3) { create(:security_finding, severity: :critical) }
+ let_it_be(:finding_4) { create(:security_finding, severity: :critical) }
let(:expected_findings) { [finding_3, finding_4, finding_1, finding_2] }
subject { described_class.ordered }
before do
- finding_1.update!(severity: :high, confidence: :unknown)
- finding_2.update!(severity: :low, confidence: :confirmed)
+ finding_1.update!(severity: :high)
+ finding_2.update!(severity: :low)
end
it { is_expected.to eq(expected_findings) }
@@ -287,4 +289,126 @@
it { is_expected.to match(1) }
end
end
+
+ describe '#state' do
+ subject { finding_1.state }
+
+ context 'when there is no associated vulnerability' do
+ context 'when there is no associated dismissal feedback' do
+ it { is_expected.to eq('detected') }
+ end
+
+ context 'when there is an associated dismissal feedback' do
+ before do
+ create(:vulnerability_feedback, :dismissal, finding_uuid: finding_1.uuid)
+ end
+
+ it { is_expected.to eq('dismissed') }
+ end
+ end
+
+ context 'when there is an associated vulnerability' do
+ where(:state) { %i[detected confirmed dismissed resolved] }
+
+ before do
+ create(:vulnerabilities_finding, state, uuid: finding_1.uuid)
+ end
+
+ with_them do
+ it { is_expected.to eq(state.to_s) }
+ end
+ end
+ end
+
+ describe 'feedback accessors' do
+ shared_examples_for 'has feedback method for' do |type|
+ context 'when there is no associated dismissal feedback' do
+ it { is_expected.to be_nil }
+ end
+
+ context 'when there is an associated dismissal feedback' do
+ let!(:feedback) { create(:vulnerability_feedback, type, finding_uuid: finding_1.uuid) }
+
+ it { is_expected.to eq(feedback) }
+ end
+ end
+
+ describe '#dismissal_feedback' do
+ it_behaves_like 'has feedback method for', :dismissal do
+ subject { finding_1.dismissal_feedback }
+ end
+ end
+
+ describe '#issue_feedback' do
+ it_behaves_like 'has feedback method for', :issue do
+ subject { finding_1.issue_feedback }
+ end
+ end
+
+ describe '#merge_request_feedback' do
+ it_behaves_like 'has feedback method for', :merge_request do
+ subject { finding_1.merge_request_feedback }
+ end
+ end
+ end
+
+ describe 'attributes delegated to `finding_data`' do
+ using RSpec::Parameterized::TableSyntax
+
+ where(:attribute, :expected_value) do
+ :name | 'Test finding'
+ :description | 'Test description'
+ :solution | 'Test solution'
+ :location | 'Test location'
+ :identifiers | ['Test identifier']
+ :links | ['Test link']
+ :false_positive? | false
+ :assets | ['Test asset']
+ :evidence | {}
+ :details | []
+ :remediation_byte_offsets | { start_byte: 0, end_byte: 1 }
+ end
+
+ with_them do
+ let(:finding) { build(:security_finding) }
+
+ subject { finding.send(attribute) }
+
+ before do
+ finding.finding_data[attribute] = expected_value
+ end
+
+ it { is_expected.to eq(expected_value) }
+ end
+ end
+
+ describe '#remediations', :aggregate_failures do
+ let(:finding) { create(:security_finding, finding_data: finding_data) }
+ let(:mock_remediations) { [Object.new] }
+ let(:mock_proxy) { instance_double(Security::RemediationsProxy, by_byte_offsets: mock_remediations) }
+
+ subject(:remediations) { finding.remediations }
+
+ before do
+ allow(finding.scan).to receive(:remediations_proxy).and_return(mock_proxy)
+ end
+
+ context 'when the remediation byte offsets do not exist' do
+ let(:finding_data) { {} }
+
+ it 'does not call the proxy and returns an empty array' do
+ expect(remediations).to be_empty
+ expect(mock_proxy).not_to have_received(:by_byte_offsets)
+ end
+ end
+
+ context 'when the remediation byte offsets exist' do
+ let(:finding_data) { { remediation_byte_offsets: [{ start_byte: 0, end_byte: 10 }] } }
+
+ it 'delegates the call to the proxy' do
+ expect(remediations).to eq(mock_remediations)
+ expect(mock_proxy).to have_received(:by_byte_offsets)
+ end
+ end
+ end
end
diff --git a/ee/spec/models/security/remediations_proxy_spec.rb b/ee/spec/models/security/remediations_proxy_spec.rb
new file mode 100644
index 0000000000000000000000000000000000000000..97984c661f7b66035a05aa76cd61a3c2aa48cc63
--- /dev/null
+++ b/ee/spec/models/security/remediations_proxy_spec.rb
@@ -0,0 +1,55 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Security::RemediationsProxy, feature_category: :vulnerability_management do
+ let(:dast_artifact) { create(:ci_job_artifact, :common_security_report) }
+ let(:file) { dast_artifact.file }
+ let(:model) { described_class.new(file) }
+
+ describe '#by_byte_offsets' do
+ # The following byte offsets are collected by parsing the related artifact with Oj::Introspect.
+ # If the specs fail due to a change in the related artifact, you can collect them again by parsing
+ # the artifact again and checking the `:__oj_introspect` keys for remediations.
+ let(:remediation_1_byte_offsets) { [11842, 12008] }
+ let(:remediation_2_byte_offsets) { [12015, 12181] }
+ let(:remediation_3_byte_offsets) { remediation_2_byte_offsets }
+
+ subject(:data_fragments) do
+ model.by_byte_offsets([remediation_1_byte_offsets, remediation_2_byte_offsets, remediation_2_byte_offsets])
+ .map(&:deep_symbolize_keys)
+ end
+
+ before do
+ allow(file).to receive(:multi_read).and_call_original
+ end
+
+ it 'returns remediations by given byte offsets' do
+ expect(data_fragments).to eq(
+ [
+ {
+ diff: 'dG90YWxseSBsZWdpdCBkaWZm',
+ summary: 'this remediates CVE-2137',
+ fixes: [{ cve: 'CVE-2137' }]
+ },
+ {
+ diff: 'dG90YWxseSBsZWdpdCBkaWZm',
+ summary: 'this remediates CVE-2138',
+ fixes: [{ cve: 'CVE-2138' }]
+ },
+ {
+ diff: 'dG90YWxseSBsZWdpdCBkaWZm',
+ summary: 'this remediates CVE-2138',
+ fixes: [{ cve: 'CVE-2138' }]
+ }
+ ]
+ )
+ end
+
+ it 'delegates the call to GitlabUploader#multi_read with unique offsets' do
+ data_fragments
+
+ expect(file).to have_received(:multi_read).once.with([remediation_1_byte_offsets, remediation_2_byte_offsets])
+ end
+ end
+end
diff --git a/ee/spec/models/security/scan_spec.rb b/ee/spec/models/security/scan_spec.rb
index 46fc5ff8012980bd895b5a4818724972bd1b55ef..ed8688698be67bdfdeabccd788206d66a5bca264 100644
--- a/ee/spec/models/security/scan_spec.rb
+++ b/ee/spec/models/security/scan_spec.rb
@@ -2,7 +2,7 @@
require 'spec_helper'
-RSpec.describe Security::Scan do
+RSpec.describe Security::Scan, feature_category: :vulnerability_management do
it_behaves_like 'cleanup by a loose foreign key' do
let!(:model) { create(:security_scan) }
let(:parent) { model.build }
@@ -399,6 +399,21 @@
end
end
+ describe '#remediations_proxy' do
+ let(:mock_file) { instance_double(JobArtifactUploader) }
+ let(:scan) { create(:security_scan, :with_findings) }
+
+ subject { scan.remediations_proxy }
+
+ before do
+ allow_next_found_instance_of(Ci::JobArtifact) do |artifact|
+ allow(artifact).to receive(:file).and_return(mock_file)
+ end
+ end
+
+ it { is_expected.to be_an_instance_of(Security::RemediationsProxy).and have_attributes(file: mock_file) }
+ end
+
it_behaves_like 'having unique enum values'
it 'sets `project_id` and `pipeline_id` before save' do
diff --git a/ee/spec/models/vulnerabilities/read_spec.rb b/ee/spec/models/vulnerabilities/read_spec.rb
index 4817b082b0cd0e9246e618cb20f024653750c9dc..55c400b3fa7a836f78281f92736292e3d48807f2 100644
--- a/ee/spec/models/vulnerabilities/read_spec.rb
+++ b/ee/spec/models/vulnerabilities/read_spec.rb
@@ -2,13 +2,14 @@
require 'spec_helper'
-RSpec.describe Vulnerabilities::Read, type: :model do
+RSpec.describe Vulnerabilities::Read, type: :model, feature_category: :vulnerability_management do
let_it_be(:project) { create(:project) }
describe 'associations' do
it { is_expected.to belong_to(:vulnerability) }
it { is_expected.to belong_to(:project) }
it { is_expected.to belong_to(:scanner).class_name('Vulnerabilities::Scanner') }
+ it { is_expected.to have_many(:security_findings).class_name('Security::Finding') }
end
describe 'validations' do
diff --git a/ee/spec/requests/api/vulnerability_findings_spec.rb b/ee/spec/requests/api/vulnerability_findings_spec.rb
index 1fc73e7889dcf240c26dbfb5dda93289e9a15dc5..8c377633f898e97ae54301576f20fc374e0b0380 100644
--- a/ee/spec/requests/api/vulnerability_findings_spec.rb
+++ b/ee/spec/requests/api/vulnerability_findings_spec.rb
@@ -2,7 +2,7 @@
require 'spec_helper'
-RSpec.describe API::VulnerabilityFindings do
+RSpec.describe API::VulnerabilityFindings, feature_category: :vulnerability_management do
include AccessMatchersForRequest
let_it_be(:project) { create(:project, :public) }
@@ -11,24 +11,23 @@
describe 'GET /projects/:id/vulnerability_findings' do
let(:project_vulnerability_findings_path) { "/projects/#{project.id}/vulnerability_findings" }
- before do
- stub_licensed_features(security_dashboard: true, sast: true, dependency_scanning: true, container_scanning: true)
-
- create(:ee_ci_job_artifact, :dependency_scanning, job: build_ds, project: project)
- create(:ee_ci_job_artifact, :sast, job: build_sast, project: project)
- dismissal
- end
-
- let(:pipeline) { create(:ci_empty_pipeline, status: :created, project: project) }
- let(:pipeline_without_vulnerabilities) { create(:ci_pipeline, status: :created, project: project) }
+ let_it_be(:pipeline) { create(:ci_empty_pipeline, status: :created, project: project) }
+ let_it_be(:pipeline_without_vulnerabilities) { create(:ci_pipeline, status: :created, project: project) }
- let(:build_ds) { create(:ci_build, :success, name: 'ds_job', pipeline: pipeline, project: project) }
- let(:build_sast) { create(:ci_build, :success, name: 'sast_job', pipeline: pipeline, project: project) }
+ let_it_be(:build_ds) { create(:ci_build, :success, name: 'ds_job', pipeline: pipeline, project: project) }
+ let_it_be(:build_sast) { create(:ci_build, :success, name: 'sast_job', pipeline: pipeline, project: project) }
let(:ds_report) { pipeline.security_reports.reports['dependency_scanning'] }
let(:sast_report) { pipeline.security_reports.reports['sast'] }
- let(:dismissal) do
+ before(:all) do
+ create(:security_scan, :latest_successful, :with_findings, scan_type: :dependency_scanning, build: build_ds)
+ create(:security_scan, :latest_successful, :with_findings, scan_type: :sast, build: build_sast)
+ end
+
+ before do
+ stub_licensed_features(security_dashboard: true, sast: true, dependency_scanning: true, container_scanning: true)
+
create(:vulnerability_feedback, :dismissal, :sast,
project: project,
pipeline: pipeline,
@@ -36,6 +35,8 @@
vulnerability_data: sast_report.findings.first.raw_metadata,
finding_uuid: sast_report.findings.first.uuid
)
+
+ create(:vulnerabilities_finding, :dismissed, uuid: sast_report.findings.first.uuid)
end
context 'with an authorized user with proper permissions' do
@@ -80,45 +81,58 @@
end
describe 'using different finders' do
+ let(:utilize_finding_data?) { true }
+ let(:mock_findings_finder) { instance_double(Security::FindingsFinder, execute: []) }
+ let(:mock_pure_findings_finder) do
+ instance_double(Security::PureFindingsFinder,
+ execute: [], available?: pure_finder_available?)
+ end
+
before do
- allow(Security::PipelineVulnerabilitiesFinder).to receive(:new).and_call_original
+ stub_feature_flags(utilize_finding_data: utilize_finding_data?)
- allow_next_instance_of(Security::FindingsFinder) do |finder|
- allow(finder).to receive(:execute).and_return(mock_result)
- end
- end
+ allow(Security::FindingsFinder).to receive(:new).and_return(mock_findings_finder)
+ allow(Security::PureFindingsFinder).to receive(:new).and_return(mock_pure_findings_finder)
- context 'when the project uses `security_findings`' do
- let(:finding) { create(:vulnerability_finding) }
- let(:mock_result) { double(findings: [finding]) }
+ get api(project_vulnerability_findings_path, user), params: pagination
+ end
- it 'does not use `Security::PipelineVulnerabilitiesFinder`' do
- get api(project_vulnerability_findings_path, user), params: pagination
+ context 'when the `Security::PureFindingsFinder` is not available' do
+ let(:pure_finder_available?) { false }
- expect(Security::PipelineVulnerabilitiesFinder).not_to have_received(:new)
+ it 'uses the `Security::FindingsFinder`' do
+ expect(mock_pure_findings_finder).not_to have_received(:execute)
+ expect(mock_findings_finder).to have_received(:execute)
end
end
- context 'when the project does not use `security_findings`' do
- let(:mock_result) { nil }
+ context 'when the `Security::PureFindingsFinder` is available' do
+ let(:pure_finder_available?) { true }
- it 'fallsback to `Security::PipelineVulnerabilitiesFinder`' do
- get api(project_vulnerability_findings_path, user), params: pagination
-
- expect(Security::PipelineVulnerabilitiesFinder).to have_received(:new)
+ it 'uses the `Security::FindingsFinder`' do
+ expect(mock_pure_findings_finder).to have_received(:execute)
+ expect(mock_findings_finder).not_to have_received(:execute)
end
+ end
+
+ context 'when the `utilize_finding_data` feature flag is turned off' do
+ let(:utilize_finding_data?) { false }
- context 'when the `Security::PipelineVulnerabilitiesFinder` raises exception' do
- before do
- allow_next_instance_of(Security::PipelineVulnerabilitiesFinder) do |finder|
- allow(finder).to receive(:execute).and_raise(Security::PipelineVulnerabilitiesFinder::ParseError.new('failed'))
- end
+ context 'when the `Security::PureFindingsFinder` is not available' do
+ let(:pure_finder_available?) { false }
+
+ it 'uses the `Security::FindingsFinder`' do
+ expect(mock_pure_findings_finder).not_to have_received(:execute)
+ expect(mock_findings_finder).to have_received(:execute)
end
+ end
- it 'does not propagate the error to the client' do
- get api(project_vulnerability_findings_path, user), params: pagination
+ context 'when the `Security::PureFindingsFinder` is available' do
+ let(:pure_finder_available?) { true }
- expect(response).to have_gitlab_http_status(:ok)
+ it 'uses the `Security::FindingsFinder`' do
+ expect(mock_pure_findings_finder).not_to have_received(:execute)
+ expect(mock_findings_finder).to have_received(:execute)
end
end
end
diff --git a/ee/spec/support/shared_examples/finders/security/findings_finder_shared_examples.rb b/ee/spec/support/shared_examples/finders/security/findings_finder_shared_examples.rb
new file mode 100644
index 0000000000000000000000000000000000000000..c126c5748e7ef73f2f698a226be1ccf1eb7041f5
--- /dev/null
+++ b/ee/spec/support/shared_examples/finders/security/findings_finder_shared_examples.rb
@@ -0,0 +1,343 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.shared_examples 'security findings finder' do
+ let_it_be(:pipeline) { create(:ci_pipeline) }
+ let_it_be(:build_1) { create(:ci_build, :success, name: 'dependency_scanning', pipeline: pipeline) }
+ let_it_be(:build_2) { create(:ci_build, :success, name: 'sast', pipeline: pipeline) }
+ let_it_be(:artifact_ds) { create(:ee_ci_job_artifact, :dependency_scanning, job: build_1) }
+ let_it_be(:artifact_sast) { create(:ee_ci_job_artifact, :sast, job: build_2) }
+ let_it_be(:report_ds) { create(:ci_reports_security_report, pipeline: pipeline, type: :dependency_scanning) }
+ let_it_be(:report_sast) { create(:ci_reports_security_report, pipeline: pipeline, type: :sast) }
+
+ let(:severity_levels) { nil }
+ let(:confidence_levels) { nil }
+ let(:report_types) { nil }
+ let(:scope) { nil }
+ let(:page) { nil }
+ let(:per_page) { nil }
+ let(:service_object) { described_class.new(pipeline, params: params) }
+ let(:params) do
+ {
+ severity: severity_levels,
+ confidence: confidence_levels,
+ report_type: report_types,
+ scope: scope,
+ page: page,
+ per_page: per_page
+ }
+ end
+
+ describe '#execute' do
+ context 'when the pipeline does not have security findings' do
+ subject { service_object.execute }
+
+ it { is_expected.to be_empty }
+ end
+
+ context 'when the pipeline has security findings' do
+ let(:finder_result) { service_object.execute }
+
+ before(:all) do
+ ds_content = File.read(artifact_ds.file.path)
+ Gitlab::Ci::Parsers::Security::DependencyScanning.parse!(ds_content, report_ds)
+ report_ds.merge!(report_ds)
+ sast_content = File.read(artifact_sast.file.path)
+ Gitlab::Ci::Parsers::Security::Sast.parse!(sast_content, report_sast)
+ report_sast.merge!(report_sast)
+
+ findings = { artifact_ds => report_ds, artifact_sast => report_sast }.collect do |artifact, report|
+ scan = create(:security_scan, :latest_successful, scan_type: artifact.job.name, build: artifact.job)
+
+ report.findings.collect do |finding, index|
+ create(:security_finding,
+ severity: finding.severity,
+ confidence: finding.confidence,
+ uuid: finding.uuid,
+ deduplicated: true,
+ scan: scan)
+ end
+ end.flatten
+
+ findings.second.update!(deduplicated: false)
+
+ create(:vulnerability_feedback,
+ :dismissal,
+ project: pipeline.project,
+ category: :sast,
+ finding_uuid: findings.first.uuid)
+ end
+
+ before do
+ stub_licensed_features(sast: true, dependency_scanning: true)
+ end
+
+ describe 'N+1 queries' do
+ it 'does not cause N+1 queries' do
+ expect { finder_result }.not_to exceed_query_limit(query_limit)
+ end
+ end
+
+ describe '#current_page' do
+ subject { finder_result.current_page }
+
+ context 'when the page is not provided' do
+ it { is_expected.to be(1) }
+ end
+
+ context 'when the page is provided' do
+ let(:page) { 2 }
+
+ it { is_expected.to be(2) }
+ end
+ end
+
+ describe '#limit_value' do
+ subject { finder_result.limit_value }
+
+ context 'when the per_page is not provided' do
+ it { is_expected.to be(20) }
+ end
+
+ context 'when the per_page is provided' do
+ let(:per_page) { 100 }
+
+ it { is_expected.to be(100) }
+ end
+ end
+
+ describe '#total_pages' do
+ subject { finder_result.total_pages }
+
+ context 'when the per_page is not provided' do
+ it { is_expected.to be(1) }
+ end
+
+ context 'when the per_page is provided' do
+ let(:per_page) { 3 }
+
+ it { is_expected.to be(3) }
+ end
+ end
+
+ describe '#total_count' do
+ subject { finder_result.total_count }
+
+ context 'when the scope is not provided' do
+ let(:finding_to_dismiss) { Security::Finding.first }
+
+ before do
+ stub_feature_flags(deprecate_vulnerabilities_feedback: deprecate_vulnerabilities_feedback?)
+ end
+
+ context 'when the `deprecate_vulnerabilities_feedback` feature is enabled' do
+ let(:deprecate_vulnerabilities_feedback?) { true }
+
+ before do
+ create(:vulnerabilities_finding, :dismissed, uuid: finding_to_dismiss.uuid)
+ end
+
+ it { is_expected.to be(7) }
+ end
+
+ context 'when the `deprecate_vulnerabilities_feedback` feature is disabled' do
+ let(:deprecate_vulnerabilities_feedback?) { false }
+
+ before do
+ create(:vulnerability_feedback,
+ :dismissal,
+ project: pipeline.project,
+ category: :dependency_scanning,
+ finding_uuid: finding_to_dismiss.uuid)
+ end
+
+ it { is_expected.to be(7) }
+ end
+ end
+
+ context 'when the scope is provided as `all`' do
+ let(:scope) { 'all' }
+
+ it { is_expected.to be(8) }
+ end
+ end
+
+ describe '#next_page' do
+ subject { finder_result.next_page }
+
+ context 'when the page is not provided' do
+ # Limit per_page to force pagination on smaller dataset
+ let(:per_page) { 2 }
+
+ it { is_expected.to be(2) }
+ end
+
+ context 'when the page is provided' do
+ let(:page) { 2 }
+
+ it { is_expected.to be_nil }
+ end
+ end
+
+ describe '#prev_page' do
+ subject { finder_result.prev_page }
+
+ context 'when the page is not provided' do
+ it { is_expected.to be_nil }
+ end
+
+ context 'when the page is provided' do
+ let(:page) { 2 }
+ # Limit per_page to force pagination on smaller dataset
+ let(:per_page) { 2 }
+
+ it { is_expected.to be(1) }
+ end
+ end
+
+ describe '#findings' do
+ subject { findings.map(&:uuid) }
+
+ context 'with the default parameters' do
+ let(:expected_uuids) { Security::Finding.pluck(:uuid) - [Security::Finding.second[:uuid]] }
+
+ it { is_expected.to match_array(expected_uuids) }
+ end
+
+ context 'when the uuid is provided' do
+ let(:uuid) { Security::Finding.pick(:uuid) }
+ let(:params) do
+ {
+ uuid: uuid
+ }
+ end
+
+ it { is_expected.to match_array([uuid]) }
+ end
+
+ context 'when the page is provided' do
+ let(:page) { 2 }
+ # Limit per_page to force pagination on smaller dataset
+ let(:per_page) { 2 }
+ let(:expected_uuids) { Security::Finding.pluck(:uuid)[4..5] }
+
+ it { is_expected.to match_array(expected_uuids) }
+ end
+
+ context 'when the per_page is provided' do
+ let(:per_page) { 1 }
+ let(:expected_uuids) { [Security::Finding.pick(:uuid)] }
+
+ it { is_expected.to match_array(expected_uuids) }
+ end
+
+ context 'when the `severity_levels` is provided' do
+ let(:severity_levels) { [:medium] }
+ let(:expected_uuids) { Security::Finding.where(severity: 'medium').pluck(:uuid) }
+
+ it { is_expected.to match_array(expected_uuids) }
+ end
+
+ context 'when the `confidence_levels` is provided' do
+ let(:confidence_levels) { [:low] }
+ let(:expected_uuids) { Security::Finding.where(confidence: 'low').pluck(:uuid) }
+
+ it { is_expected.to match_array(expected_uuids) }
+ end
+
+ context 'when the `report_types` is provided' do
+ let(:report_types) { :dependency_scanning }
+ let(:expected_uuids) do
+ Security::Finding.by_scan(Security::Scan.find_by(scan_type: 'dependency_scanning')).pluck(:uuid) -
+ [Security::Finding.second[:uuid]]
+ end
+
+ it { is_expected.to match_array(expected_uuids) }
+ end
+
+ context 'when the `scope` is provided as `all`' do
+ let(:scope) { 'all' }
+
+ let(:expected_uuids) { Security::Finding.pluck(:uuid) - [Security::Finding.second[:uuid]] }
+
+ it { is_expected.to match_array(expected_uuids) }
+ end
+
+ context 'when there is a retried build' do
+ let(:retried_build) { create(:ci_build, :success, :retried, name: 'dependency_scanning', pipeline: pipeline) }
+ let(:artifact) { create(:ee_ci_job_artifact, :dependency_scanning, job: retried_build) }
+ let(:report) { create(:ci_reports_security_report, pipeline: pipeline, type: :dependency_scanning) }
+ let(:report_types) { :dependency_scanning }
+ let(:expected_uuids) do
+ Security::Finding.by_scan(Security::Scan.find_by(scan_type: 'dependency_scanning')).pluck(:uuid) -
+ [Security::Finding.second[:uuid]]
+ end
+
+ before do
+ retried_content = File.read(artifact.file.path)
+ Gitlab::Ci::Parsers::Security::DependencyScanning.parse!(retried_content, report)
+ report.merge!(report)
+
+ scan = create(:security_scan, scan_type: retried_build.name, build: retried_build, latest: false)
+
+ report.findings.each_with_index do |finding, index|
+ create(:security_finding,
+ severity: finding.severity,
+ confidence: finding.confidence,
+ uuid: finding.uuid,
+ deduplicated: true,
+ scan: scan)
+ end
+ end
+
+ it { is_expected.to match_array(expected_uuids) }
+ end
+
+ context 'when a build has more than one security report artifacts' do
+ let(:report_types) { :secret_detection }
+ let(:expected_uuids) { secret_detection_report.findings.map(&:uuid) }
+ let(:secret_detection_report) do
+ create(:ci_reports_security_report,
+ pipeline: pipeline,
+ type: :secret_detection)
+ end
+
+ before do
+ scan = create(:security_scan, :latest_successful, scan_type: :secret_detection, build: build_2)
+ artifact = create(:ee_ci_job_artifact, :secret_detection, job: build_2)
+ report_content = File.read(artifact.file.path)
+
+ Gitlab::Ci::Parsers::Security::SecretDetection.parse!(report_content, secret_detection_report)
+
+ secret_detection_report.findings.each_with_index do |finding, index|
+ create(:security_finding,
+ severity: finding.severity,
+ confidence: finding.confidence,
+ uuid: finding.uuid,
+ deduplicated: true,
+ scan: scan)
+ end
+ end
+
+ it { is_expected.to match_array(expected_uuids) }
+ end
+
+ context 'when a vulnerability already exist for a security finding' do
+ let!(:vulnerability_finding) do
+ create(:vulnerabilities_finding,
+ :detected,
+ uuid: Security::Finding.first.uuid,
+ project: pipeline.project)
+ end
+
+ subject { findings.map(&:vulnerability).first }
+
+ describe 'the vulnerability is included in results' do
+ it { is_expected.to eq(vulnerability_finding.vulnerability) }
+ end
+ end
+ end
+ end
+ end
+end
diff --git a/spec/uploaders/gitlab_uploader_spec.rb b/spec/uploaders/gitlab_uploader_spec.rb
index db70441aaf56c274509e3cd9f6dda1f08f0a5e9e..f62ab7266319f281dc04984d77c890e6355841c8 100644
--- a/spec/uploaders/gitlab_uploader_spec.rb
+++ b/spec/uploaders/gitlab_uploader_spec.rb
@@ -6,7 +6,7 @@
RSpec.describe GitlabUploader do
let(:uploader_class) { Class.new(described_class) }
- subject { uploader_class.new(double) }
+ subject(:uploader) { uploader_class.new(double) }
describe '#file_storage?' do
context 'when file storage is used' do
@@ -161,6 +161,19 @@
end
end
+ describe '#multi_read' do
+ let(:file) { fixture_file_upload('spec/fixtures/trace/sample_trace', 'text/plain') }
+ let(:byte_offsets) { [[4, 10], [17, 29]] }
+
+ subject { uploader.multi_read(byte_offsets) }
+
+ before do
+ uploader.store!(file)
+ end
+
+ it { is_expected.to eq(%w[Running gitlab-runner]) }
+ end
+
describe '.version' do
subject { uploader_class.version }