Commit 6e57c487 authored by Zamir Martins's avatar Zamir Martins Committed by James Fargher

Add vulnerabilities_allowed into the sync worker

parent 81493714
......@@ -25,6 +25,9 @@ class ApprovalProjectRule < ApplicationRecord
validates :scanners, inclusion: { in: ::Ci::JobArtifact::SECURITY_REPORT_FILE_TYPES }
default_value_for :scanners, allows_nil: false, value: ::Ci::JobArtifact::SECURITY_REPORT_FILE_TYPES
validates :vulnerabilities_allowed, numericality: { only_integer: true }
default_value_for :vulnerabilities_allowed, allows_nil: false, value: 0
def applies_to_branch?(branch)
return true if protected_branches.empty?
......
......@@ -804,6 +804,10 @@ module EE
namespace.created_at >= PUBLIC_COST_FACTOR_RELEASE_DAY
end
def vulnerability_report_rule
approval_rules.vulnerability_reports.first
end
private
def github_integration_enabled?
......
......@@ -22,6 +22,10 @@ module Ci
backtrace: error.backtrace
)
error("Failed to update approval rules")
ensure
[:project_rule_vulnerabilities_allowed, :project_vulnerability_rule, :reports].each do |memoization|
clear_memoization(memoization)
end
end
private
......@@ -55,13 +59,19 @@ module Ci
def reports
strong_memoize(:reports) do
project_vulnerability_rules ? pipeline.security_reports(report_types: project_vulnerability_rules) : []
project_vulnerability_rule ? pipeline.security_reports(report_types: project_vulnerability_rule) : []
end
end
def project_vulnerability_rule
strong_memoize(:project_vulnerability_rule) do
pipeline.project.vulnerability_report_rule&.scanners
end
end
def project_vulnerability_rules
strong_memoize(:project_vulnerability_rules) do
pipeline.project.approval_rules.vulnerability_reports.first&.scanners
def project_rule_vulnerabilities_allowed
strong_memoize(:project_rule_vulnerabilities_allowed) do
pipeline.project.vulnerability_report_rule&.vulnerabilities_allowed
end
end
......@@ -76,7 +86,7 @@ module Ci
def merge_requests_approved_security_reports
pipeline.merge_requests_as_head_pipeline.reject do |merge_request|
reports.present? && reports.violates_default_policy_against?(merge_request.base_pipeline&.security_reports)
reports.present? && reports.violates_default_policy_against?(merge_request.base_pipeline&.security_reports, project_rule_vulnerabilities_allowed)
end
end
......
......@@ -22,8 +22,8 @@ module Gitlab
reports.values.flat_map(&:findings)
end
def violates_default_policy_against?(target_reports)
findings_diff(target_reports).any?(&:unsafe?)
def violates_default_policy_against?(target_reports, vulnerabilities_allowed)
unsafe_findings_count(target_reports) > vulnerabilities_allowed
end
private
......@@ -31,6 +31,10 @@ module Gitlab
def findings_diff(target_reports)
findings - target_reports&.findings.to_a
end
def unsafe_findings_count(target_reports)
findings_diff(target_reports).count(&:unsafe?)
end
end
end
end
......
......@@ -56,8 +56,9 @@ RSpec.describe Gitlab::Ci::Reports::Security::Reports do
describe "#violates_default_policy_against?" do
let(:low_severity_sast) { build(:ci_reports_security_finding, severity: 'low', report_type: :sast) }
let(:high_severity_dast) { build(:ci_reports_security_finding, severity: 'high', report_type: :dast) }
let(:vulnerabilities_allowed) { 0 }
subject { security_reports.violates_default_policy_against?(target_reports) }
subject { security_reports.violates_default_policy_against?(target_reports, vulnerabilities_allowed) }
context 'when the target_reports is `nil`' do
let(:target_reports) { nil }
......@@ -90,6 +91,12 @@ RSpec.describe Gitlab::Ci::Reports::Security::Reports do
end
it { is_expected.to be(true) }
context 'with vulnerabilities_allowed higher than the number of new vulnerabilities' do
let(:vulnerabilities_allowed) { 10000 }
it { is_expected.to be(false) }
end
end
context "when none of the reports have a new unsafe vulnerability" do
......
......@@ -161,17 +161,19 @@ RSpec.describe ApprovalProjectRule do
context "with a `Vulnerability-Check` rule" do
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)
where(:is_valid, :scanners, :vulnerabilities_allowed) do
true | [] | 0
true | %w(dast) | 1
true | %w(dast sast) | 10
true | %w(dast dast) | 100
false | %w(dast unknown_scanner) | 100
false | %w(unknown_scanner) | 100
false | %w(dast sast) | 1.1
false | %w(dast sast) | 'one'
end
with_them do
let(:vulnerability_check_rule) { build(:approval_project_rule, :vulnerability, scanners: scanners) }
let(:vulnerability_check_rule) { build(:approval_project_rule, :vulnerability, scanners: scanners, vulnerabilities_allowed: vulnerabilities_allowed) }
specify { expect(vulnerability_check_rule.valid?).to be(is_valid) }
end
......
......@@ -2921,4 +2921,27 @@ RSpec.describe Project do
end
end
end
describe '#vulnerability_report_rule' do
subject { project.vulnerability_report_rule }
context 'with vulnerability report rule' do
let!(:rule) { create(:approval_project_rule, :vulnerability_report, project: project) }
it { is_expected.to eql(rule) }
end
context 'without vulnerability report rule' do
let!(:rule) { create(:approval_project_rule, project: project) }
it { is_expected.to be_nil }
end
context 'with multiple rules' do
let!(:rule) { create(:approval_project_rule, project: project) }
let!(:vuln_rule) { create(:approval_project_rule, :vulnerability_report, project: project) }
it { is_expected.to eql(vuln_rule) }
end
end
end
......@@ -20,9 +20,10 @@ RSpec.describe Ci::SyncReportsToApprovalRulesService, '#execute' do
context 'with security rules' 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 }
before do
create(:approval_project_rule, :vulnerability, project: project, approvals_required: 2, scanners: scanners)
create(:approval_project_rule, :vulnerability, project: project, approvals_required: 2, scanners: scanners, vulnerabilities_allowed: vulnerabilities_allowed)
end
context 'when there are security reports' do
......@@ -58,6 +59,15 @@ RSpec.describe Ci::SyncReportsToApprovalRulesService, '#execute' do
.to change { report_approver_rule.reload.approvals_required }.from(2).to(0)
end
end
context 'with the minimum number of vulnerabilities allowed greater than the amount from the security reports' do
let(:vulnerabilities_allowed) { 10000 }
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
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