diff --git a/doc/architecture/blueprints/security_policies_database_read_model/index.md b/doc/architecture/blueprints/security_policies_database_read_model/index.md index 1d6bb16f00824a173abb83656c5501e4bda7cdcb..948ab4242114cb8b6e0f71f91e5ece06f3ee7412 100644 --- a/doc/architecture/blueprints/security_policies_database_read_model/index.md +++ b/doc/architecture/blueprints/security_policies_database_read_model/index.md @@ -73,7 +73,7 @@ This worker is called whenever these events happen: - Protected branch is added/removed from a project - Policy is created/updated/deleted for a project -To avoid this, we have created the `scan_result_policies` table (`Security::ScanResultPolicyRead` model) which acts as a read model for merge request approval policies to avoid reading from policy project. But currently, we don't store all the required fields in the table, we only store `role_approvers` , `license_state` and `match_on_inclusion`. +To avoid this, we have created the `scan_result_policies` table (`Security::ScanResultPolicyRead` model) which acts as a read model for merge request approval policies to avoid reading from policy project. But currently, we don't store all the required fields in the table, we only store `role_approvers` , `license_state` and `match_on_inclusion` (previously `match_on_inclusion_license`). ```mermaid erDiagram diff --git a/doc/user/application_security/policies/scan-result-policies.md b/doc/user/application_security/policies/scan-result-policies.md index 3ead737a5f03fa218c687b1be8ae9cc7d7326fcf..5ab41bc203c96b61a296b14460297048e7fa4420 100644 --- a/doc/user/application_security/policies/scan-result-policies.md +++ b/doc/user/application_security/policies/scan-result-policies.md @@ -156,7 +156,7 @@ This rule enforces the defined actions based on license findings. | `branches` | `array` of `string` | true if `branch_type` field does not exist | `[]` or the branch's name | Applicable only to protected target branches. An empty array, `[]`, applies the rule to all protected target branches. Cannot be used with the `branch_type` field. | | `branch_type` | `string` | true if `branches` field does not exist | `default` or `protected` | The types of protected branches the given policy applies to. Cannot be used with the `branches` field. Default branches must also be `protected`. | | `branch_exceptions` | `array` of `string` | false | Names of branches | Branches to exclude from this rule. | -| `match_on_inclusion` | `boolean` | true | `true`, `false` | **{warning}** **[Deprecated](https://gitlab.com/gitlab-org/gitlab/-/issues/424513)** in GitLab 16.9. Whether the rule matches inclusion or exclusion of licenses listed in `license_types`. When `false`, any detected licenses excluded from `license_types` require approval. | +| `match_on_inclusion_license` | `boolean` | true | `true`, `false` | Whether the rule matches inclusion or exclusion of licenses listed in `license_types`. | | `license_types` | `array` of `string` | true | license types | [SPDX license names](https://spdx.org/licenses) to match on, for example `Affero General Public License v1.0` or `MIT License`. | | `license_states` | `array` of `string` | true | `newly_detected`, `detected` | Whether to match newly detected and/or previously detected licenses. The `newly_detected` state triggers approval when either a new package is introduced or when a new license for an existing package is detected. | 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 2c1b9a1905adf7314a56c5017ea4b1ae739f58f8..0c8922ce8949a0f7e026273f28eeeaef100285df 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 @@ -84,7 +84,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_license] || rule[:match_on_inclusion] ? 'denied' : 'allowed', + approval_status: rule[:match_on_inclusion_license] ? 'denied' : 'allowed', scan_result_policy_read: scan_result_policy_read } end @@ -95,7 +95,7 @@ def create_scan_result_policy(rule, approval_action, send_bot_message_action, pr orchestration_policy_idx: policy_index, rule_idx: rule_index, license_states: rule[:license_states], - match_on_inclusion: rule.fetch(:match_on_inclusion_license, rule[:match_on_inclusion]) || false, + match_on_inclusion_license: rule[:match_on_inclusion_license] || false, role_approvers: role_access_levels(approval_action&.dig(:role_approvers)), vulnerability_attributes: rule[:vulnerability_attributes], project_id: project.id, diff --git a/ee/app/validators/json_schemas/security_orchestration_policy.json b/ee/app/validators/json_schemas/security_orchestration_policy.json index a6d33d2e1ae1599a385d1882e261d548f28e84b4..e932954d38799f61c68512907ebaf1b809d130bc 100644 --- a/ee/app/validators/json_schemas/security_orchestration_policy.json +++ b/ee/app/validators/json_schemas/security_orchestration_policy.json @@ -661,10 +661,6 @@ ], "additionalProperties": false }, - "match_on_inclusion": { - "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." @@ -745,23 +741,11 @@ } }, "then": { - "oneOf": [ - { - "required": [ - "type", - "match_on_inclusion", - "license_types", - "license_states" - ] - }, - { - "required": [ - "type", - "match_on_inclusion_license", - "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 a7da86cfa2bf818f7f939df0f9e63b7533e3affc..e8820aaf998e6f32c62845dec1f30d6adfdcea59 100644 --- a/ee/spec/factories/security/policies.rb +++ b/ee/spec/factories/security/policies.rb @@ -169,20 +169,6 @@ end end - trait :license_finding_with_match_on_inclusion do - rules do - [ - { - type: 'license_finding', - branches: branches, - match_on_inclusion: true, - license_types: %w[BSD MIT], - license_states: %w[newly_detected detected] - } - ] - end - end - trait :any_merge_request do rules do [ diff --git a/ee/spec/models/security/orchestration_policy_configuration_spec.rb b/ee/spec/models/security/orchestration_policy_configuration_spec.rb index d53e3d4b0c2fabdb5981059cd38a7999b265ba1d..b4a69ee119138e803e9188fbffe0246acb7b0e4f 100644 --- a/ee/spec/models/security/orchestration_policy_configuration_spec.rb +++ b/ee/spec/models/security/orchestration_policy_configuration_spec.rb @@ -1363,58 +1363,18 @@ 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 + context "without 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 be56dfdd78bbcf716b4fd8875aecf5612eb2263b..d5ecc3f9ff4236f85603a8a4d9d2232dc888cd39 100644 --- a/ee/spec/models/security/scan_result_policy_read_spec.rb +++ b/ee/spec/models/security/scan_result_policy_read_spec.rb @@ -12,7 +12,6 @@ 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) } 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 9b69cc317c93f294930b980732cd37ce41914d36..4b6f2481459acd55fa7549b47cb4bf495380e341 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 @@ -658,12 +658,6 @@ 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 context 'with any_merge_request rule_type' do