diff --git a/ee/app/models/sca/license_compliance.rb b/ee/app/models/sca/license_compliance.rb index 33e5e347153d8bb12af61d16bd53a5bbaaa7f4b2..f65a8303d47b4e227f72bfe4c52478455217a068 100644 --- a/ee/app/models/sca/license_compliance.rb +++ b/ee/app/models/sca/license_compliance.rb @@ -92,7 +92,7 @@ def direct_license_policies end end - # Constructs license to policy map for policy with `match_on_inclusion` as false + # Constructs license to policy map for policy with `match_on_inclusion_license` as false # by setting the `approval_status` as denied for all licenses from report except # for the one mentioned in the policy. def denied_license_policies diff --git a/ee/app/models/security/scan_result_policy_read.rb b/ee/app/models/security/scan_result_policy_read.rb index fa1faba08e5e2a7d8a0c10e385bb45f75148d54a..05bd107dbc890265aebc8c9af6e7ae75fba807c0 100644 --- a/ee/app/models/security/scan_result_policy_read.rb +++ b/ee/app/models/security/scan_result_policy_read.rb @@ -6,6 +6,8 @@ class ScanResultPolicyRead < ApplicationRecord self.table_name = 'scan_result_policies' + alias_attribute :match_on_inclusion_license, :match_on_inclusion + enum age_operator: { greater_than: 0, less_than: 1 } enum age_interval: { day: 0, week: 1, month: 2, year: 3 } enum commits: { any: 0, unsigned: 1 }, _prefix: true @@ -17,7 +19,7 @@ class ScanResultPolicyRead < ApplicationRecord has_many :violations, foreign_key: 'scan_result_policy_id', class_name: 'Security::ScanResultPolicyViolation', inverse_of: :scan_result_policy_read - validates :match_on_inclusion, inclusion: { in: [true, false], message: 'must be a boolean value' } + validates :match_on_inclusion_license, inclusion: { in: [true, false], message: 'must be a boolean value' } validates :role_approvers, inclusion: { in: Gitlab::Access.all_values } validates :age_value, numericality: { only_integer: true, greater_than_or_equal_to: 0 }, allow_nil: true validates :vulnerability_attributes, json_schema: { filename: 'scan_result_policy_vulnerability_attributes' }, diff --git a/ee/app/models/software_license_policy.rb b/ee/app/models/software_license_policy.rb index d0bc56bc08b9fafc72a85a2c29b8d9f0736201d8..47a01e79cf44b5d8ddeb980b0b74401ca34536d4 100644 --- a/ee/app/models/software_license_policy.rb +++ b/ee/app/models/software_license_policy.rb @@ -46,7 +46,7 @@ class SoftwareLicensePolicy < ApplicationRecord scope :exclusion_allowed, -> do joins(:scan_result_policy_read) - .where(scan_result_policy_read: { match_on_inclusion: false }) + .where(scan_result_policy_read: { match_on_inclusion_license: false }) end scope :with_license_by_name, -> (license_name) do diff --git a/ee/app/services/security/security_orchestration_policies/process_scan_result_policy_service.rb b/ee/app/services/security/security_orchestration_policies/process_scan_result_policy_service.rb index f68030055ba6a28e7e60772346941861d52af53f..4e5346fd4d73d2aa705738e3f28b0b1f55f28b72 100644 --- a/ee/app/services/security/security_orchestration_policies/process_scan_result_policy_service.rb +++ b/ee/app/services/security/security_orchestration_policies/process_scan_result_policy_service.rb @@ -81,7 +81,7 @@ def create_software_license_params(rule, scan_result_policy_read) rule[:license_types].map do |license_type| { name: license_type, - approval_status: rule[:match_on_inclusion] ? 'denied' : 'allowed', + approval_status: rule[:match_on_inclusion_license] || rule[:match_on_inclusion] ? 'denied' : 'allowed', scan_result_policy_read: scan_result_policy_read } end @@ -92,7 +92,7 @@ def create_scan_result_policy(rule, action_info, project_approval_settings, proj orchestration_policy_idx: policy_index, rule_idx: rule_index, license_states: rule[:license_states], - match_on_inclusion: rule[:match_on_inclusion] || false, + match_on_inclusion: rule.fetch(:match_on_inclusion_license, rule[:match_on_inclusion]) || false, role_approvers: role_access_levels(action_info&.dig(:role_approvers)), vulnerability_attributes: rule[:vulnerability_attributes], project_id: project.id, diff --git a/ee/app/services/security/sync_license_scanning_rules_service.rb b/ee/app/services/security/sync_license_scanning_rules_service.rb index f05dc5a42ea03f31f18ddb10567efa5b37fed204..3d29484cbe20fdae2c758d0edc27d2b21153f180 100644 --- a/ee/app/services/security/sync_license_scanning_rules_service.rb +++ b/ee/app/services/security/sync_license_scanning_rules_service.rb @@ -64,11 +64,11 @@ def update_required_approvals(merge_request, violated_rules, unviolated_rules) ## Checks if a policy rule violates the following conditions: ## - If license_states has `newly_detected`, check for newly detected dependency ## with license type violating the policy. - ## - If match_on_inclusion is false, any detected licenses that does not match + ## - If match_on_inclusion_license is false, any detected licenses that does not match ## the licenses from `license_types` should require approval def violates_policy?(merge_request, rule) scan_result_policy_read = rule.scan_result_policy_read - check_denied_licenses = scan_result_policy_read.match_on_inclusion + check_denied_licenses = scan_result_policy_read.match_on_inclusion_license target_branch_report = target_branch_report(merge_request) license_ids, license_names = licenses_to_check(target_branch_report, scan_result_policy_read) @@ -82,7 +82,7 @@ def violates_policy?(merge_request, rule) denied_licenses = license_names_from_policy violates_license_policy = report.violates_for_licenses?(license_policies, license_ids, license_names) else - # when match_on_inclusion is false, only allowed licenses are mentioned in policy + # when match_on_inclusion_license is false, only allowed licenses are mentioned in policy denied_licenses = (license_names_from_report - license_names_from_policy).uniq license_names_from_ids = license_names_from_ids(license_ids, license_names) violates_license_policy = (license_names_from_ids - license_names_from_policy).present? diff --git a/ee/app/validators/json_schemas/security_orchestration_policy.json b/ee/app/validators/json_schemas/security_orchestration_policy.json index d5cf893a26fe9040b788090d202a2d7286824f01..a63c354090841706c2ea2eafcf2cd47b6c478dd8 100644 --- a/ee/app/validators/json_schemas/security_orchestration_policy.json +++ b/ee/app/validators/json_schemas/security_orchestration_policy.json @@ -652,6 +652,10 @@ "type": "boolean", "description": "Specifies whether to match licenses on inclusion or exclusion." }, + "match_on_inclusion_license": { + "type": "boolean", + "description": "Specifies whether to match licenses on inclusion or exclusion." + }, "license_types": { "type": "array", "description": "Specifies the licenses to match.", @@ -726,11 +730,23 @@ } }, "then": { - "required": [ - "type", - "match_on_inclusion", - "license_types", - "license_states" + "oneOf": [ + { + "required": [ + "type", + "match_on_inclusion", + "license_types", + "license_states" + ] + }, + { + "required": [ + "type", + "match_on_inclusion_license", + "license_types", + "license_states" + ] + } ] } }, diff --git a/ee/spec/factories/security/policies.rb b/ee/spec/factories/security/policies.rb index ef9e911c3de9b301a6cd497bdbc6678152c71b5f..8fb9feb61d74368d6e1ec4a4c8afbe422fdb1c63 100644 --- a/ee/spec/factories/security/policies.rb +++ b/ee/spec/factories/security/policies.rb @@ -101,6 +101,20 @@ policy_scope { {} } trait :license_finding do + rules do + [ + { + type: 'license_finding', + branches: branches, + match_on_inclusion_license: true, + license_types: %w[BSD MIT], + license_states: %w[newly_detected detected] + } + ] + end + end + + trait :license_finding_with_match_on_inclusion do rules do [ { diff --git a/ee/spec/factories/security/scan_result_policy_reads.rb b/ee/spec/factories/security/scan_result_policy_reads.rb index 2ca7314d48996b00931890ebfbd18450dc0ef1c6..9254ff182df142b88ed8ac2c8d2b13995fdbe082 100644 --- a/ee/spec/factories/security/scan_result_policy_reads.rb +++ b/ee/spec/factories/security/scan_result_policy_reads.rb @@ -6,7 +6,7 @@ project orchestration_policy_idx { 0 } - match_on_inclusion { true } + match_on_inclusion_license { true } sequence :rule_idx trait :blocking_protected_branches do diff --git a/ee/spec/models/sca/license_compliance_spec.rb b/ee/spec/models/sca/license_compliance_spec.rb index 41da36795505b7c01bb0f92d9dbbc23fc28572c7..ec1c30e511a98474172e9863c5100463a3e64912 100644 --- a/ee/spec/models/sca/license_compliance_spec.rb +++ b/ee/spec/models/sca/license_compliance_spec.rb @@ -96,7 +96,7 @@ allow(license_compliance).to receive(:license_scanning_report).and_return(report) input.each do |policy| - scan_result_policy_read = policy[:scan_result_policy] ? create(:scan_result_policy_read, match_on_inclusion: policy[:classification] == 'denied') : nil + scan_result_policy_read = policy[:scan_result_policy] ? create(:scan_result_policy_read, match_on_inclusion_license: policy[:classification] == 'denied') : nil create(:software_license_policy, policy[:classification], project: project, software_license: license_map[policy[:id]], @@ -447,8 +447,8 @@ def assert_matches(item, expected = {}) let(:base_report) { create(:ci_reports_license_scanning_report) } let(:report) { create(:ci_reports_license_scanning_report) } - let(:scan_result_policy_read_with_inclusion) { create(:scan_result_policy_read, match_on_inclusion: true) } - let(:scan_result_policy_read_without_inclusion) { create(:scan_result_policy_read, match_on_inclusion: false) } + let(:scan_result_policy_read_with_inclusion) { create(:scan_result_policy_read, match_on_inclusion_license: true) } + let(:scan_result_policy_read_without_inclusion) { create(:scan_result_policy_read, match_on_inclusion_license: false) } context 'when base_report has new denied licenses' do before do @@ -563,7 +563,7 @@ def assert_matches(item, expected = {}) end let(:scan_result_policy_read) do - create(:scan_result_policy_read, license_states: ['newly_detected'], match_on_inclusion: true, + create(:scan_result_policy_read, license_states: ['newly_detected'], match_on_inclusion_license: true, role_approvers: [Gitlab::Access::MAINTAINER], project: project) end diff --git a/ee/spec/models/security/orchestration_policy_configuration_spec.rb b/ee/spec/models/security/orchestration_policy_configuration_spec.rb index 12aa3efa51297a1e3f8fdd4f44794f5675e6774f..1cf21ece09ee83e41d9e4ac22f9dfff61715a091 100644 --- a/ee/spec/models/security/orchestration_policy_configuration_spec.rb +++ b/ee/spec/models/security/orchestration_policy_configuration_spec.rb @@ -1237,7 +1237,7 @@ { type: "license_finding", branches: %w[master], - match_on_inclusion: true, + match_on_inclusion_license: true, license_types: %w[BSD MIT], license_states: %w[newly_detected detected] } @@ -1245,9 +1245,60 @@ specify { expect(errors).to be_empty } - it_behaves_like "scan result policy", %i[match_on_inclusion license_types license_states] it_behaves_like 'rule has branches or branch_type' + shared_examples 'missing required_property on one_of' do |missing_property, alternative_property| + before do + rule.delete(missing_property) + end + + specify do + expect(errors).to include( + "property '/#{type}/0/rules/0' is missing required keys: #{alternative_property}, #{missing_property}", + "property '/#{type}/0/rules/0' is missing required keys: #{missing_property}" + ) + end + end + + context "without match_on_inclusion or match_on_inclusion_license" do + before do + rule.delete(:match_on_inclusion_license) + end + + specify do + expect(errors).to include( + "property '/#{type}/0/rules/0' is missing required keys: match_on_inclusion", + "property '/#{type}/0/rules/0' is missing required keys: match_on_inclusion_license" + ) + end + end + + context "with match_on_inclusion_license" do + it_behaves_like 'missing required_property on one_of', :license_types, :match_on_inclusion + it_behaves_like 'missing required_property on one_of', :license_states, :match_on_inclusion + end + + context "with match_on_inclusion" do + before do + rule[:match_on_inclusion] = true + end + + context "with match_on_inclusion_license" do + specify do + expect(errors).to include("property '/#{type}/0/rules/0' is invalid: error_type=oneOf") + end + end + + context "without match_on_inclusion_license" do + before do + rule.delete(:match_on_inclusion_license) + end + + it_behaves_like 'missing required_property on one_of', :license_types, :match_on_inclusion_license + it_behaves_like 'missing required_property on one_of', :license_states, :match_on_inclusion_license + end + end + describe "license_types" do before do rule[:license_types] = [""] diff --git a/ee/spec/models/security/scan_result_policy_read_spec.rb b/ee/spec/models/security/scan_result_policy_read_spec.rb index 1ecbfb90e76dac3fbb05247856a2a524b7c91992..e5c4b83ab65eddced1daeade87634fca12d0f8d6 100644 --- a/ee/spec/models/security/scan_result_policy_read_spec.rb +++ b/ee/spec/models/security/scan_result_policy_read_spec.rb @@ -13,6 +13,7 @@ subject { scan_result_policy_read } it { is_expected.to validate_inclusion_of(:match_on_inclusion).in_array([true, false]) } + it { is_expected.to validate_inclusion_of(:match_on_inclusion_license).in_array([true, false]) } it { is_expected.not_to allow_value(nil).for(:role_approvers) } it { is_expected.to(validate_inclusion_of(:role_approvers).in_array(Gitlab::Access.values)) } diff --git a/ee/spec/models/software_license_policy_spec.rb b/ee/spec/models/software_license_policy_spec.rb index 9d1d5411a2752c7706847e88dba1afa5290a28df..821ae70c0f92a49886ef6d52305e26a616ce48df 100644 --- a/ee/spec/models/software_license_policy_spec.rb +++ b/ee/spec/models/software_license_policy_spec.rb @@ -55,8 +55,8 @@ describe '.exclusion_allowed' do let_it_be(:mit) { create(:software_license, :mit) } - let_it_be(:scan_result_policy_read_with_inclusion) { create(:scan_result_policy_read, match_on_inclusion: true) } - let_it_be(:scan_result_policy_read_without_inclusion) { create(:scan_result_policy_read, match_on_inclusion: false) } + let_it_be(:scan_result_policy_read_with_inclusion) { create(:scan_result_policy_read, match_on_inclusion_license: true) } + let_it_be(:scan_result_policy_read_without_inclusion) { create(:scan_result_policy_read, match_on_inclusion_license: false) } let!(:mit_policy) { create(:software_license_policy, software_license: mit) } let!(:mit_policy_with_inclusion) { create(:software_license_policy, software_license: mit, scan_result_policy_read: scan_result_policy_read_with_inclusion) } let!(:mit_policy_without_inclusion) { create(:software_license_policy, software_license: mit, scan_result_policy_read: scan_result_policy_read_without_inclusion) } diff --git a/ee/spec/services/security/security_orchestration_policies/process_scan_result_policy_service_spec.rb b/ee/spec/services/security/security_orchestration_policies/process_scan_result_policy_service_spec.rb index b6d6b3e5e90495fc3342510060d7b0237b39f29b..affdea942efdb3de9099efad9d5ef7edc262cd9b 100644 --- a/ee/spec/services/security/security_orchestration_policies/process_scan_result_policy_service_spec.rb +++ b/ee/spec/services/security/security_orchestration_policies/process_scan_result_policy_service_spec.rb @@ -543,76 +543,88 @@ context 'with license_finding rule_type' do let(:policy) { build(:scan_result_policy, :license_finding) } - it 'creates scan_result_policy_read' do - subject - - scan_result_policy_read = project.approval_rules.first.scan_result_policy_read - expect(scan_result_policy_read).to eq(Security::ScanResultPolicyRead.first) - expect(scan_result_policy_read.match_on_inclusion).to be_truthy - expect(scan_result_policy_read.license_states).to match_array(%w[newly_detected detected]) - expect(scan_result_policy_read.rule_idx).to be(0) - end - - it 'creates software_license_policies' do - expect { subject }.to change { project.software_license_policies.count }.by(2) - end + shared_examples 'license_finding_rule_type' do + it 'creates scan_result_policy_read' do + subject - it 'creates approval_rules with valid params' do - subject + scan_result_policy_read = project.approval_rules.first.scan_result_policy_read + expect(scan_result_policy_read).to eq(Security::ScanResultPolicyRead.first) + expect(scan_result_policy_read.match_on_inclusion_license).to be_truthy + expect(scan_result_policy_read.license_states).to match_array(%w[newly_detected detected]) + expect(scan_result_policy_read.rule_idx).to be(0) + end - approval_rule = project.approval_rules.first + it 'creates software_license_policies' do + expect { subject }.to change { project.software_license_policies.count }.by(2) + end - expect(approval_rule.severity_levels).to be_empty - end - - it 'calls SoftwareLicensePolicies::BulkCreateScanResultPolicyService' do - expect(SoftwareLicensePolicies::BulkCreateScanResultPolicyService).to receive(:new).with( - project, - [ - { - name: 'BSD', - approval_status: 'denied', - scan_result_policy_read: anything - }, - { - name: 'MIT', - approval_status: 'denied', - scan_result_policy_read: anything - } - ] - ).and_call_original + it 'creates approval_rules with valid params' do + subject - subject - end + approval_rule = project.approval_rules.first - context 'with bulk_create_scan_result_policies feature flag disabled' do - before do - stub_feature_flags(bulk_create_scan_result_policies: false) + expect(approval_rule.severity_levels).to be_empty end - it 'calls SoftwareLicensePolicies::CreateService' do - expect(SoftwareLicensePolicies::CreateService).to receive(:new).with( + it 'calls SoftwareLicensePolicies::BulkCreateScanResultPolicyService' do + expect(SoftwareLicensePolicies::BulkCreateScanResultPolicyService).to receive(:new).with( project, - anything, - { - name: 'BSD', - approval_status: 'denied', - scan_result_policy_read: anything - } - ).and_call_original - - expect(SoftwareLicensePolicies::CreateService).to receive(:new).with( - project, - anything, - { - name: 'MIT', - approval_status: 'denied', - scan_result_policy_read: anything - } + [ + { + name: 'BSD', + approval_status: 'denied', + scan_result_policy_read: anything + }, + { + name: 'MIT', + approval_status: 'denied', + scan_result_policy_read: anything + } + ] ).and_call_original subject end + + context 'with bulk_create_scan_result_policies feature flag disabled' do + before do + stub_feature_flags(bulk_create_scan_result_policies: false) + end + + it 'calls SoftwareLicensePolicies::CreateService' do + expect(SoftwareLicensePolicies::CreateService).to receive(:new).with( + project, + anything, + { + name: 'BSD', + approval_status: 'denied', + scan_result_policy_read: anything + } + ).and_call_original + + expect(SoftwareLicensePolicies::CreateService).to receive(:new).with( + project, + anything, + { + name: 'MIT', + approval_status: 'denied', + scan_result_policy_read: anything + } + ).and_call_original + + subject + end + end + end + + context 'when the policy has the YAML has the match_on_inclusion_license attribute' do + it_behaves_like 'license_finding_rule_type' + end + + context 'when the policy has the YAML has the match_on_inclusion attribute' do + let(:policy) { build(:scan_result_policy, :license_finding_with_match_on_inclusion) } + + it_behaves_like 'license_finding_rule_type' end end diff --git a/ee/spec/services/security/sync_license_scanning_rules_service_spec.rb b/ee/spec/services/security/sync_license_scanning_rules_service_spec.rb index b9c387826a1b83106374556e19b342ddee3d1dbb..887ce8b7e94f9df2a08a520e44a3906a9a3ddbc2 100644 --- a/ee/spec/services/security/sync_license_scanning_rules_service_spec.rb +++ b/ee/spec/services/security/sync_license_scanning_rules_service_spec.rb @@ -62,11 +62,12 @@ context 'with license_finding security policy' do let(:license_states) { ['newly_detected'] } - let(:match_on_inclusion) { true } + let(:match_on_inclusion_license) { true } let(:approvals_required) { 1 } let(:scan_result_policy_read) do - create(:scan_result_policy_read, license_states: license_states, match_on_inclusion: match_on_inclusion) + create(:scan_result_policy_read, license_states: license_states, + match_on_inclusion_license: match_on_inclusion_license) end let!(:license_finding_rule) do @@ -211,7 +212,7 @@ end with_them do - let(:match_on_inclusion) { policy_state == :denied } + let(:match_on_inclusion_license) { policy_state == :denied } let(:target_branch_report) { create(:ci_reports_license_scanning_report) } let(:pipeline_report) { create(:ci_reports_license_scanning_report) } let(:license_states) { states } diff --git a/ee/spec/support/helpers/features/security_policy_helpers.rb b/ee/spec/support/helpers/features/security_policy_helpers.rb index b373ea4d8570a771e1940a55cb8da8a298ed473d..ac0b80ccbcac1ba522b6d81c83e5dd9c625eefe1 100644 --- a/ee/spec/support/helpers/features/security_policy_helpers.rb +++ b/ee/spec/support/helpers/features/security_policy_helpers.rb @@ -52,7 +52,7 @@ def policy_rule { type: 'license_finding', branches: policy_branch_names, - match_on_inclusion: true, + match_on_inclusion_license: true, license_types: [license_type], license_states: license_states }