Commit 6db1f49e authored by Zamir Martins's avatar Zamir Martins Committed by Mayra Cabrera

Add project rules based on scan result policies

parent d0140af7
...@@ -128,6 +128,10 @@ class ApprovalMergeRequestRule < ApplicationRecord ...@@ -128,6 +128,10 @@ class ApprovalMergeRequestRule < ApplicationRecord
refresh_license_scanning_approvals(project_approval_rule) if license_scanning? refresh_license_scanning_approvals(project_approval_rule) if license_scanning?
end end
def self.remove_required_approved(approval_rules)
where(id: approval_rules).update_all(approvals_required: 0)
end
private private
def compare_with_project_rule def compare_with_project_rule
......
...@@ -24,6 +24,7 @@ class ApprovalProjectRule < ApplicationRecord ...@@ -24,6 +24,7 @@ class ApprovalProjectRule < ApplicationRecord
} }
scope :report_approver_without_scan_finding, -> { report_approver.where.not(report_type: :scan_finding) } scope :report_approver_without_scan_finding, -> { report_approver.where.not(report_type: :scan_finding) }
scope :distinct_scanners, -> { scan_finding.select(:scanners).distinct }
alias_method :code_owner, :code_owner? alias_method :code_owner, :code_owner?
validate :validate_default_license_report_name, on: :update, if: :report_approver? validate :validate_default_license_report_name, on: :update, if: :report_approver?
......
...@@ -22,6 +22,13 @@ module Security ...@@ -22,6 +22,13 @@ module Security
def scan_result_policies def scan_result_policies
policy_by_type(:scan_result_policy) policy_by_type(:scan_result_policy)
end end
def uniq_scanners
distinct_scanners = approval_rules.distinct_scanners
return [] if distinct_scanners.none?
distinct_scanners.pluck(:scanners).flatten.uniq
end
end end
end end
end end
...@@ -4,6 +4,18 @@ module Ci ...@@ -4,6 +4,18 @@ module Ci
class SyncReportsToApprovalRulesService < ::BaseService class SyncReportsToApprovalRulesService < ::BaseService
include Gitlab::Utils::StrongMemoize include Gitlab::Utils::StrongMemoize
MEMOIZATIONS = %i(
policy_configuration
policy_rule_reports
policy_rule_scanners
project_rule_scanners
project_rule_severity_levels
project_rule_vulnerabilities_allowed
project_rule_vulnerability_states
project_vulnerability_report
reports
).freeze
def initialize(pipeline) def initialize(pipeline)
@pipeline = pipeline @pipeline = pipeline
end end
...@@ -12,6 +24,7 @@ module Ci ...@@ -12,6 +24,7 @@ module Ci
sync_license_scanning_rules sync_license_scanning_rules
sync_vulnerability_rules sync_vulnerability_rules
sync_coverage_rules sync_coverage_rules
sync_scan_finding
success success
rescue StandardError => error rescue StandardError => error
payload = { payload = {
...@@ -23,7 +36,7 @@ module Ci ...@@ -23,7 +36,7 @@ module Ci
log_error(payload) log_error(payload)
error("Failed to update approval rules") error("Failed to update approval rules")
ensure ensure
[:project_rule_vulnerabilities_allowed, :project_rule_scanners, :project_rule_severity_levels, :project_vulnerability_report, :reports, :project_rule_vulnerability_states].each do |memoization| MEMOIZATIONS.each do |memoization|
clear_memoization(memoization) clear_memoization(memoization)
end end
end end
...@@ -57,18 +70,37 @@ module Ci ...@@ -57,18 +70,37 @@ module Ci
remove_required_approvals_for(ApprovalMergeRequestRule.code_coverage, merge_requests_approved_coverage) remove_required_approvals_for(ApprovalMergeRequestRule.code_coverage, merge_requests_approved_coverage)
end end
def sync_scan_finding
return if ::Feature.disabled?(:scan_result_policy, pipeline.project)
return if policy_rule_reports.empty? && !pipeline.complete?
remove_required_approvals_for_scan_finding(pipeline.merge_requests_as_head_pipeline.opened)
end
def reports def reports
strong_memoize(:reports) do strong_memoize(:reports) do
project_rule_scanners ? pipeline.security_reports(report_types: project_rule_scanners) : [] project_rule_scanners ? pipeline.security_reports(report_types: project_rule_scanners) : []
end end
end end
def policy_rule_reports
strong_memoize(:policy_rule_reports) do
policy_rule_scanners ? pipeline.security_reports(report_types: policy_rule_scanners) : []
end
end
def project_rule_scanners def project_rule_scanners
strong_memoize(:project_rule_scanners) do strong_memoize(:project_rule_scanners) do
project_vulnerability_report&.scanners project_vulnerability_report&.scanners
end end
end end
def policy_rule_scanners
strong_memoize(:policy_rule_scanners) do
policy_configuration&.uniq_scanners
end
end
def project_rule_vulnerabilities_allowed def project_rule_vulnerabilities_allowed
strong_memoize(:project_rule_vulnerabilities_allowed) do strong_memoize(:project_rule_vulnerabilities_allowed) do
project_vulnerability_report&.vulnerabilities_allowed project_vulnerability_report&.vulnerabilities_allowed
...@@ -96,6 +128,17 @@ module Ci ...@@ -96,6 +128,17 @@ module Ci
.update_all(approvals_required: 0) .update_all(approvals_required: 0)
end end
def remove_required_approvals_for_scan_finding(merge_requests)
merge_requests.each do |merge_request|
base_reports = merge_request.base_pipeline&.security_reports
scan_finding_rules = merge_request.approval_rules.scan_finding
selected_rules = scan_finding_rules.reject do |rule|
violates_default_policy?(rule.source_rule, base_reports)
end
scan_finding_rules.remove_required_approved(selected_rules)
end
end
def project_rule_severity_levels def project_rule_severity_levels
strong_memoize(:project_rule_severity_levels) do strong_memoize(:project_rule_severity_levels) do
project_vulnerability_report&.severity_levels project_vulnerability_report&.severity_levels
...@@ -113,5 +156,15 @@ module Ci ...@@ -113,5 +156,15 @@ module Ci
pipeline.project.vulnerability_report_rule pipeline.project.vulnerability_report_rule
end end
end end
def policy_configuration
strong_memoize(:policy_configuration) do
pipeline.project.security_orchestration_policy_configuration
end
end
def violates_default_policy?(source_rule, base_reports)
policy_rule_reports.violates_default_policy_against?(base_reports, source_rule.vulnerabilities_allowed, source_rule.severity_levels, source_rule.vulnerability_states_for_branch, source_rule.scanners)
end
end end
end end
...@@ -11,11 +11,11 @@ module EE ...@@ -11,11 +11,11 @@ module EE
private private
override :unsafe_findings_count override :unsafe_findings_count
def unsafe_findings_count(target_reports, severity_levels, vulnerability_states) def unsafe_findings_count(target_reports, severity_levels, vulnerability_states, report_types)
pipeline_uuids = unsafe_findings_uuids(severity_levels) pipeline_uuids = unsafe_findings_uuids(severity_levels, report_types)
pipeline_count = count_by_uuid(pipeline_uuids, vulnerability_states) pipeline_count = count_by_uuid(pipeline_uuids, vulnerability_states)
new_uuids = pipeline_uuids - target_reports&.unsafe_findings_uuids(severity_levels).to_a new_uuids = pipeline_uuids - target_reports&.unsafe_findings_uuids(severity_levels, report_types).to_a
if vulnerability_states.include?(ApprovalProjectRule::NEWLY_DETECTED) if vulnerability_states.include?(ApprovalProjectRule::NEWLY_DETECTED)
pipeline_count += new_uuids.count pipeline_count += new_uuids.count
......
...@@ -42,6 +42,11 @@ FactoryBot.define do ...@@ -42,6 +42,11 @@ FactoryBot.define do
name { ApprovalRuleLike::DEFAULT_NAME_FOR_COVERAGE } name { ApprovalRuleLike::DEFAULT_NAME_FOR_COVERAGE }
report_type { :code_coverage } report_type { :code_coverage }
end end
trait :scan_finding do
sequence(:name) { |n| "Scan finding #{n}" }
report_type { :scan_finding }
end
end end
factory :any_approver_rule, parent: :approval_merge_request_rule do factory :any_approver_rule, parent: :approval_merge_request_rule do
......
...@@ -185,19 +185,20 @@ RSpec.describe Gitlab::Ci::Reports::Security::Finding do ...@@ -185,19 +185,20 @@ RSpec.describe Gitlab::Ci::Reports::Security::Finding do
end end
describe '#unsafe?' do describe '#unsafe?' do
where(:severity, :levels, :unsafe?) do where(:severity, :levels, :report_types, :unsafe?) do
'critical' | %w(critical high) | true 'critical' | %w(critical high) | %w(dast) | true
'high' | %w(critical high) | true 'high' | %w(critical high) | %w(dast sast) | true
'medium' | %w(critical high) | false 'high' | %w(critical high) | %w(container_scanning) | false
'low' | %w(critical high) | false 'medium' | %w(critical high) | %w(dast) | false
'info' | %w(critical high) | false 'low' | %w(critical high) | %w(dast) | false
'unknown' | [] | false 'info' | %w(critical high) | %w(dast) | false
'unknown' | [] | %w(dast) | false
end end
with_them do with_them do
let(:finding) { create(:ci_reports_security_finding, severity: severity) } let(:finding) { create(:ci_reports_security_finding, severity: severity, report_type: 'dast') }
subject { finding.unsafe?(levels) } subject { finding.unsafe?(levels, report_types) }
it { is_expected.to be(unsafe?) } it { is_expected.to be(unsafe?) }
end end
......
...@@ -303,4 +303,44 @@ RSpec.describe ApprovalProjectRule do ...@@ -303,4 +303,44 @@ RSpec.describe ApprovalProjectRule do
end end
end end
end end
describe '.distinct_scanners scope' do
subject { described_class.distinct_scanners }
before do
create(:approval_project_rule, type, scanners: ['dast'])
end
context 'with scan_finding approval rules' do
let(:type) { :scan_finding }
it { is_expected.to be_present }
context 'with duplicated scanners' do
before do
create(:approval_project_rule, :scan_finding, scanners: ['dast'])
end
it 'returns only one record' do
expect(subject.count).to be 1
end
end
context 'without duplicated scanners' do
before do
create(:approval_project_rule, :scan_finding, scanners: ['sast'])
end
it 'returns both records' do
expect(subject.count).to be 2
end
end
end
context 'without scan_finding approval rules' do
let(:type) { :vulnerability }
it { is_expected.to be_empty }
end
end
end end
...@@ -380,4 +380,23 @@ RSpec.describe Security::OrchestrationPolicyConfiguration do ...@@ -380,4 +380,23 @@ RSpec.describe Security::OrchestrationPolicyConfiguration do
expect(scan_result_policies.pluck(:enabled)).to contain_exactly(true, true, false, true, true, true, true, true) expect(scan_result_policies.pluck(:enabled)).to contain_exactly(true, true, false, true, true, true, true, true)
end end
end end
describe '#uniq_scanners' do
let(:project) { security_orchestration_policy_configuration.project }
subject { security_orchestration_policy_configuration.uniq_scanners }
context 'with approval rules' do
before do
create(:approval_project_rule, :scan_finding, scanners: %w(dast sast), project: project)
create(:approval_project_rule, :scan_finding, scanners: %w(dast container_scanning), project: project)
end
it { is_expected.to contain_exactly('dast', 'sast', 'container_scanning') }
end
context 'without approval rules' do
it { is_expected.to be_empty }
end
end
end end
...@@ -8,6 +8,10 @@ RSpec.describe Ci::SyncReportsToApprovalRulesService, '#execute' do ...@@ -8,6 +8,10 @@ RSpec.describe Ci::SyncReportsToApprovalRulesService, '#execute' do
let(:merge_request) { create(:merge_request, source_project: project) } let(:merge_request) { create(:merge_request, source_project: project) }
let(:pipeline) { create(:ee_ci_pipeline, :success, project: project, merge_requests_as_head_pipeline: [merge_request]) } let(:pipeline) { create(:ee_ci_pipeline, :success, project: project, merge_requests_as_head_pipeline: [merge_request]) }
let(:base_pipeline) { create(:ee_ci_pipeline, :success, project: project, ref: merge_request.target_branch, sha: merge_request.diff_base_sha) } let(:base_pipeline) { create(:ee_ci_pipeline, :success, project: project, ref: merge_request.target_branch, sha: merge_request.diff_base_sha) }
let(:scanners) { %w[dependency_scanning] }
let(:vulnerabilities_allowed) { 0 }
let(:severity_levels) { %w[high unknown] }
let(:vulnerability_states) { %w(newly_detected) }
subject(:sync_rules) { described_class.new(pipeline).execute } subject(:sync_rules) { described_class.new(pipeline).execute }
...@@ -17,27 +21,17 @@ RSpec.describe Ci::SyncReportsToApprovalRulesService, '#execute' do ...@@ -17,27 +21,17 @@ RSpec.describe Ci::SyncReportsToApprovalRulesService, '#execute' do
stub_licensed_features(dependency_scanning: true, dast: true, license_scanning: true) stub_licensed_features(dependency_scanning: true, dast: true, license_scanning: true)
end end
context 'with security rules' do shared_context 'security reports with vulnerabilities' do
let(:report_approver_rule) { create(:report_approver_rule, merge_request: merge_request, approvals_required: 2) }
let(:scanners) { %w[dependency_scanning] }
let(:vulnerabilities_allowed) { 0 }
let(:severity_levels) { %w[high unknown] }
let(:vulnerability_states) { %w(newly_detected) }
before do
create(:approval_project_rule, :vulnerability, project: project, approvals_required: 2, scanners: scanners, vulnerabilities_allowed: vulnerabilities_allowed, severity_levels: severity_levels, vulnerability_states: vulnerability_states)
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
context 'when high-severity vulnerabilities are present' do context 'when high-severity vulnerabilities are present' do
before do before do
create(:ee_ci_build, :success, :dependency_scanning, name: 'ds_job', pipeline: pipeline, project: project) create(:ee_ci_build, :success, :dependency_scanning, :coverage, name: 'ds_job', pipeline: pipeline, project: project)
end end
context 'when high-severity vulnerabilities already present in target branch pipeline' do context 'when high-severity vulnerabilities already present in target branch pipeline' do
before do before do
create(:ee_ci_build, :success, :dependency_scanning, name: 'ds_job', pipeline: base_pipeline, project: project) create(:ee_ci_build, :success, :dependency_scanning, :coverage, name: 'ds_job', pipeline: base_pipeline, project: project)
end end
it 'lowers approvals_required count to zero' do it 'lowers approvals_required count to zero' do
...@@ -161,12 +155,12 @@ RSpec.describe Ci::SyncReportsToApprovalRulesService, '#execute' do ...@@ -161,12 +155,12 @@ RSpec.describe Ci::SyncReportsToApprovalRulesService, '#execute' do
context 'when high-severity vulnerabilities are present' do context 'when high-severity vulnerabilities are present' do
before do before do
create(:ee_ci_build, :success, :dependency_scanning, name: 'ds_job', pipeline: pipeline, project: project) create(:ee_ci_build, :success, :dependency_scanning, :coverage, name: 'ds_job', pipeline: pipeline, project: project)
end end
context 'when high-severity vulnerabilities already present in target branch pipeline' do context 'when high-severity vulnerabilities already present in target branch pipeline' do
before do before do
create(:ee_ci_build, :success, :dependency_scanning, name: 'ds_job', pipeline: base_pipeline, project: project) create(:ee_ci_build, :success, :dependency_scanning, :coverage, name: 'ds_job', pipeline: base_pipeline, project: project)
end end
it 'lowers approvals_required count to zero' do it 'lowers approvals_required count to zero' do
...@@ -215,6 +209,16 @@ RSpec.describe Ci::SyncReportsToApprovalRulesService, '#execute' do ...@@ -215,6 +209,16 @@ RSpec.describe Ci::SyncReportsToApprovalRulesService, '#execute' do
end end
end end
context 'with security rules' do
let(:report_approver_rule) { create(:report_approver_rule, merge_request: merge_request, approvals_required: 2) }
before do
create(:approval_project_rule, :vulnerability, project: project, approvals_required: 2, scanners: scanners, vulnerabilities_allowed: vulnerabilities_allowed, severity_levels: severity_levels, vulnerability_states: vulnerability_states)
end
include_context 'security reports with vulnerabilities'
end
context 'with code coverage rules' do context 'with code coverage rules' do
let!(:head_pipeline_builds) do let!(:head_pipeline_builds) do
[ [
...@@ -315,4 +319,43 @@ RSpec.describe Ci::SyncReportsToApprovalRulesService, '#execute' do ...@@ -315,4 +319,43 @@ RSpec.describe Ci::SyncReportsToApprovalRulesService, '#execute' do
end end
end end
end end
context 'with security orchestration rules' do
let(:report_approver_rule) { create(:report_approver_rule, :scan_finding, merge_request: merge_request, approvals_required: 2) }
let(:approval_project_rule) { create(:approval_project_rule, :scan_finding, project: project, approvals_required: 2, scanners: scanners, vulnerabilities_allowed: vulnerabilities_allowed, severity_levels: severity_levels, vulnerability_states: vulnerability_states) }
let!(:security_orchestration_policy_configuration) { create(:security_orchestration_policy_configuration, project: project) }
before do
create(:approval_merge_request_rule_source, approval_merge_request_rule: report_approver_rule, approval_project_rule: approval_project_rule)
end
context 'when there are security reports' do
context 'when pipeline passes' do
context 'when new vulnerabilities are present' do
before do
create(:ee_ci_build, :success, :dependency_scanning, name: 'ds_job', pipeline: pipeline, project: project)
end
context 'when only existing vulnerabilities are present' do
before do
create(:ee_ci_build, :success, :dependency_scanning, name: 'ds_job', pipeline: base_pipeline, project: project)
end
context 'with feature flag disabled' do
before do
stub_feature_flags(scan_result_policy: false)
end
it "won't change approval_required count" do
expect { subject }
.not_to change { report_approver_rule.reload.approvals_required }
end
end
end
end
end
end
include_context 'security reports with vulnerabilities'
end
end end
...@@ -88,8 +88,8 @@ module Gitlab ...@@ -88,8 +88,8 @@ module Gitlab
@location = new_location @location = new_location
end end
def unsafe?(severity_levels) def unsafe?(severity_levels, report_types)
severity.in?(severity_levels) severity.to_s.in?(severity_levels) && (report_types.blank? || report_type.to_s.in?(report_types) )
end end
def eql?(other) def eql?(other)
......
...@@ -22,18 +22,18 @@ module Gitlab ...@@ -22,18 +22,18 @@ module Gitlab
reports.values.flat_map(&:findings) reports.values.flat_map(&:findings)
end end
def violates_default_policy_against?(target_reports, vulnerabilities_allowed, severity_levels, vulnerability_states) def violates_default_policy_against?(target_reports, vulnerabilities_allowed, severity_levels, vulnerability_states, report_types = [])
unsafe_findings_count(target_reports, severity_levels, vulnerability_states) > vulnerabilities_allowed unsafe_findings_count(target_reports, severity_levels, vulnerability_states, report_types) > vulnerabilities_allowed
end end
def unsafe_findings_uuids(severity_levels) def unsafe_findings_uuids(severity_levels, report_types)
findings.select { |finding| finding.unsafe?(severity_levels) }.map(&:uuid) findings.select { |finding| finding.unsafe?(severity_levels, report_types) }.map(&:uuid)
end end
private private
def unsafe_findings_count(target_reports, severity_levels, vulnerability_states) def unsafe_findings_count(target_reports, severity_levels, vulnerability_states, report_types)
new_uuids = unsafe_findings_uuids(severity_levels) - target_reports&.unsafe_findings_uuids(severity_levels).to_a new_uuids = unsafe_findings_uuids(severity_levels, report_types) - target_reports&.unsafe_findings_uuids(severity_levels, report_types).to_a
new_uuids.count new_uuids.count
end end
end end
......
...@@ -54,7 +54,7 @@ RSpec.describe Gitlab::Ci::Reports::Security::Reports do ...@@ -54,7 +54,7 @@ RSpec.describe Gitlab::Ci::Reports::Security::Reports do
end end
describe "#violates_default_policy_against?" do describe "#violates_default_policy_against?" do
let(:high_severity_dast) { build(:ci_reports_security_finding, severity: 'high', report_type: :dast) } let(:high_severity_dast) { build(:ci_reports_security_finding, severity: 'high', report_type: 'dast') }
let(:vulnerabilities_allowed) { 0 } let(:vulnerabilities_allowed) { 0 }
let(:severity_levels) { %w(critical high) } let(:severity_levels) { %w(critical high) }
let(:vulnerability_states) { %w(newly_detected)} let(:vulnerability_states) { %w(newly_detected)}
...@@ -109,6 +109,22 @@ RSpec.describe Gitlab::Ci::Reports::Security::Reports do ...@@ -109,6 +109,22 @@ RSpec.describe Gitlab::Ci::Reports::Security::Reports do
it { is_expected.to be(false) } it { is_expected.to be(false) }
end end
context 'with related report_types' do
let(:report_types) { %w(dast sast) }
subject { security_reports.violates_default_policy_against?(target_reports, vulnerabilities_allowed, severity_levels, vulnerability_states, report_types) }
it { is_expected.to be(true) }
end
context 'with unrelated report_types' do
let(:report_types) { %w(dependency_scanning sast) }
subject { security_reports.violates_default_policy_against?(target_reports, vulnerabilities_allowed, severity_levels, vulnerability_states, report_types) }
it { is_expected.to be(false) }
end
end end
end end
end end
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