Commit fbb848e4 authored by Zamir Martins Filho's avatar Zamir Martins Filho Committed by Alex Kalderimis

Sync vulnerability rules

based on selected security scanners.

Changelog: added
EE: true
parent 40c1b8ad
...@@ -22,6 +22,9 @@ class ApprovalProjectRule < ApplicationRecord ...@@ -22,6 +22,9 @@ class ApprovalProjectRule < ApplicationRecord
validates :name, uniqueness: { scope: [:project_id, :rule_type] } validates :name, uniqueness: { scope: [:project_id, :rule_type] }
validates :rule_type, uniqueness: { scope: :project_id, message: proc { _('any-approver for the project already exists') } }, if: :any_approver? validates :rule_type, uniqueness: { scope: :project_id, message: proc { _('any-approver for the project already exists') } }, if: :any_approver?
validates :scanners, inclusion: { in: ::Ci::JobArtifact::SECURITY_REPORT_FILE_TYPES }
default_value_for :scanners, allows_nil: false, value: ::Ci::JobArtifact::SECURITY_REPORT_FILE_TYPES
def applies_to_branch?(branch) def applies_to_branch?(branch)
return true if protected_branches.empty? return true if protected_branches.empty?
......
...@@ -29,6 +29,7 @@ module ApprovalRuleLike ...@@ -29,6 +29,7 @@ module ApprovalRuleLike
scope :with_users, -> { preload(:users, :group_users) } scope :with_users, -> { preload(:users, :group_users) }
scope :regular_or_any_approver, -> { where(rule_type: [:regular, :any_approver]) } scope :regular_or_any_approver, -> { where(rule_type: [:regular, :any_approver]) }
scope :for_groups, -> (groups) { joins(:groups).where(approval_project_rules_groups: { group_id: groups }) } scope :for_groups, -> (groups) { joins(:groups).where(approval_project_rules_groups: { group_id: groups }) }
scope :vulnerability_reports, -> { where(rule_type: :report_approver, name: ApprovalRuleLike::DEFAULT_NAME_FOR_VULNERABILITY_REPORT) }
end end
def audit_add def audit_add
......
...@@ -9,13 +9,14 @@ module EE ...@@ -9,13 +9,14 @@ module EE
include ::Gitlab::Utils::StrongMemoize include ::Gitlab::Utils::StrongMemoize
extend ActiveSupport::Concern extend ActiveSupport::Concern
SECURITY_REPORT_FILE_TYPES = %w[sast secret_detection dependency_scanning container_scanning cluster_image_scanning dast coverage_fuzzing api_fuzzing].freeze
prepended do prepended do
# After destroy callbacks are often skipped because of FastDestroyAll. # After destroy callbacks are often skipped because of FastDestroyAll.
# All destroy callbacks should be implemented in `Ci::JobArtifacts::DestroyBatchService` # All destroy callbacks should be implemented in `Ci::JobArtifacts::DestroyBatchService`
# See https://gitlab.com/gitlab-org/gitlab/-/issues/297472 # See https://gitlab.com/gitlab-org/gitlab/-/issues/297472
after_destroy :log_geo_deleted_event after_destroy :log_geo_deleted_event
SECURITY_REPORT_FILE_TYPES = %w[sast secret_detection dependency_scanning container_scanning cluster_image_scanning dast coverage_fuzzing api_fuzzing].freeze
LICENSE_SCANNING_REPORT_FILE_TYPES = %w[license_scanning].freeze LICENSE_SCANNING_REPORT_FILE_TYPES = %w[license_scanning].freeze
DEPENDENCY_LIST_REPORT_FILE_TYPES = %w[dependency_scanning].freeze DEPENDENCY_LIST_REPORT_FILE_TYPES = %w[dependency_scanning].freeze
METRICS_REPORT_FILE_TYPES = %w[metrics].freeze METRICS_REPORT_FILE_TYPES = %w[metrics].freeze
......
...@@ -2,6 +2,8 @@ ...@@ -2,6 +2,8 @@
module Ci module Ci
class SyncReportsToApprovalRulesService < ::BaseService class SyncReportsToApprovalRulesService < ::BaseService
include Gitlab::Utils::StrongMemoize
def initialize(pipeline) def initialize(pipeline)
@pipeline = pipeline @pipeline = pipeline
end end
...@@ -52,7 +54,15 @@ module Ci ...@@ -52,7 +54,15 @@ module Ci
end end
def reports def reports
@reports ||= pipeline.security_reports strong_memoize(:reports) do
project_vulnerability_rules ? pipeline.security_reports(report_types: project_vulnerability_rules) : []
end
end
def project_vulnerability_rules
strong_memoize(:project_vulnerability_rules) do
pipeline.project.approval_rules.vulnerability_reports.first&.scanners
end
end end
def merge_requests_approved_coverage def merge_requests_approved_coverage
...@@ -66,7 +76,7 @@ module Ci ...@@ -66,7 +76,7 @@ module Ci
def merge_requests_approved_security_reports def merge_requests_approved_security_reports
pipeline.merge_requests_as_head_pipeline.reject do |merge_request| pipeline.merge_requests_as_head_pipeline.reject do |merge_request|
reports.violates_default_policy_against?(merge_request.base_pipeline&.security_reports) reports.present? && reports.violates_default_policy_against?(merge_request.base_pipeline&.security_reports)
end end
end end
......
...@@ -47,6 +47,26 @@ RSpec.describe ApprovalProjectRule do ...@@ -47,6 +47,26 @@ RSpec.describe ApprovalProjectRule do
end end
end end
describe '.vulnerability_reports scope' do
subject { described_class.vulnerability_reports }
context 'with vulnerability reports' do
before do
create(:approval_project_rule, :vulnerability_report)
end
it { is_expected.to be_present }
end
context 'without vulnerability reports' do
before do
create(:approval_project_rule)
end
it { is_expected.to be_empty }
end
end
describe '#regular?' do describe '#regular?' do
let(:vulnerability_approver_rule) { build(:approval_project_rule, :vulnerability_report) } let(:vulnerability_approver_rule) { build(:approval_project_rule, :vulnerability_report) }
...@@ -139,9 +159,22 @@ RSpec.describe ApprovalProjectRule do ...@@ -139,9 +159,22 @@ RSpec.describe ApprovalProjectRule do
end end
context "with a `Vulnerability-Check` rule" do context "with a `Vulnerability-Check` rule" do
subject { vulnerability_check_rule } using RSpec::Parameterized::TableSyntax
where(:is_valid, :scanners) do
true | []
true | %w(dast)
true | %w(dast sast)
true | %w(dast dast)
false | %w(dast unknown_scanner)
false | %w(unknown_scanner)
end
with_them do
let(:vulnerability_check_rule) { build(:approval_project_rule, :vulnerability, scanners: scanners) }
specify { expect(subject).to be_valid } specify { expect(vulnerability_check_rule.valid?).to be(is_valid) }
end
end end
end end
end end
......
...@@ -19,6 +19,11 @@ RSpec.describe Ci::SyncReportsToApprovalRulesService, '#execute' do ...@@ -19,6 +19,11 @@ RSpec.describe Ci::SyncReportsToApprovalRulesService, '#execute' do
context 'with security rules' do context 'with security rules' do
let(:report_approver_rule) { create(:report_approver_rule, merge_request: merge_request, approvals_required: 2) } let(:report_approver_rule) { create(:report_approver_rule, merge_request: merge_request, approvals_required: 2) }
let(:scanners) { %w[dependency_scanning] }
before do
create(:approval_project_rule, :vulnerability, project: project, approvals_required: 2, scanners: scanners)
end
context 'when there are security reports' do context 'when there are security reports' do
context 'when pipeline passes' do context 'when pipeline passes' do
...@@ -44,6 +49,15 @@ RSpec.describe Ci::SyncReportsToApprovalRulesService, '#execute' do ...@@ -44,6 +49,15 @@ RSpec.describe Ci::SyncReportsToApprovalRulesService, '#execute' do
.not_to change { report_approver_rule.reload.approvals_required } .not_to change { report_approver_rule.reload.approvals_required }
end end
end end
context 'without any scanners related to the security reports' do
let(:scanners) { %w[sast] }
it 'lowers approvals_required count to zero' do
expect { subject }
.to change { report_approver_rule.reload.approvals_required }.from(2).to(0)
end
end
end end
context 'when only low-severity vulnerabilities are present' do context 'when only low-severity vulnerabilities are present' do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment