diff --git a/db/migrate/20231025191217_add_pre_receive_secret_detection_enabled_to_application_settings.rb b/db/migrate/20231025191217_add_pre_receive_secret_detection_enabled_to_application_settings.rb new file mode 100644 index 0000000000000000000000000000000000000000..db37fc5e8f1574ddde2b81ec4f23e49cbd6c83a6 --- /dev/null +++ b/db/migrate/20231025191217_add_pre_receive_secret_detection_enabled_to_application_settings.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +class AddPreReceiveSecretDetectionEnabledToApplicationSettings < Gitlab::Database::Migration[2.2] + milestone '16.7' + + def change + add_column :application_settings, :pre_receive_secret_detection_enabled, :boolean, null: false, default: false + end +end diff --git a/db/schema_migrations/20231025191217 b/db/schema_migrations/20231025191217 new file mode 100644 index 0000000000000000000000000000000000000000..c0458feed62be9c4d8f201fe7ef2b9bb84354672 --- /dev/null +++ b/db/schema_migrations/20231025191217 @@ -0,0 +1 @@ +057503cc1306afe9dea3a3d01a2fd8eeb240c33d292a6e3f2bd8ba52b38cfa62 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 1055e902056f7dfb68a81dfb62dc696e6563d031..e5bcd33f19861ca269009aa85469b75f4a3eab28 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -12122,6 +12122,7 @@ CREATE TABLE application_settings ( enable_artifact_external_redirect_warning_page boolean DEFAULT true NOT NULL, allow_project_creation_for_guest_and_below boolean DEFAULT true NOT NULL, update_namespace_name_rate_limit smallint DEFAULT 120 NOT NULL, + pre_receive_secret_detection_enabled boolean DEFAULT false NOT NULL, CONSTRAINT app_settings_container_reg_cleanup_tags_max_list_size_positive CHECK ((container_registry_cleanup_tags_service_max_list_size >= 0)), CONSTRAINT app_settings_container_registry_pre_import_tags_rate_positive CHECK ((container_registry_pre_import_tags_rate >= (0)::numeric)), CONSTRAINT app_settings_dep_proxy_ttl_policies_worker_capacity_positive CHECK ((dependency_proxy_ttl_group_policy_worker_capacity >= 0)), diff --git a/ee/lib/ee/gitlab/checks/push_rules/secrets_check.rb b/ee/lib/ee/gitlab/checks/push_rules/secrets_check.rb index 6361e45925b1fe905b72cac3337ce8890daebe8b..c0fa9d868dbe099242171b5f8bfdbab280731de8 100644 --- a/ee/lib/ee/gitlab/checks/push_rules/secrets_check.rb +++ b/ee/lib/ee/gitlab/checks/push_rules/secrets_check.rb @@ -7,12 +7,16 @@ module PushRules class SecretsCheck < ::Gitlab::Checks::BaseBulkChecker def validate! # Return early and not perform the check if: - # 1. no push rule exist - # 2. and license is not ultimate - # 3. and feature flag is disabled - return unless push_rule && - push_rule.project.licensed_feature_available?(:pre_receive_secret_detection) && - ::Feature.enabled?(:pre_receive_secret_detection_push_check, push_rule.project) + # 1. unless application setting is enabled (regardless of whether it's a gitlab dedicated instance or not) + # 2. feature flag is disabled for this project (when instance type is not gitlab dedicated) + # 3. no push rule exist + # 4. license is not ultimate + return unless ::Gitlab::CurrentSettings.pre_receive_secret_detection_enabled + + return if ::Gitlab::CurrentSettings.gitlab_dedicated_instance != true && + ::Feature.disabled?(:pre_receive_secret_detection_push_check, push_rule.project) + + return unless push_rule && push_rule.project.licensed_feature_available?(:pre_receive_secret_detection) end end end diff --git a/ee/spec/lib/ee/gitlab/checks/push_rules/secrets_check_spec.rb b/ee/spec/lib/ee/gitlab/checks/push_rules/secrets_check_spec.rb index cab206795bbcbb1a0691b61712fe4701953483d9..585cb7c0d553adf05cde3002e10ed2e20a51987f 100644 --- a/ee/spec/lib/ee/gitlab/checks/push_rules/secrets_check_spec.rb +++ b/ee/spec/lib/ee/gitlab/checks/push_rules/secrets_check_spec.rb @@ -9,32 +9,78 @@ describe '#validate!' do it_behaves_like 'check ignored when push rule unlicensed' - it_behaves_like 'use predefined push rules' - context 'when license is not ultimate' do + context 'when application settings is disabled' do + before do + Gitlab::CurrentSettings.update!(pre_receive_secret_detection_enabled: false) + end + it 'skips the check' do expect(subject.validate!).to be_nil end end - context 'when license is ultimate' do + context 'when application settings is enabled' do before do - stub_licensed_features(pre_receive_secret_detection: true) + Gitlab::CurrentSettings.update!(pre_receive_secret_detection_enabled: true) end - it 'returns without raising errors' do - # Since the check does nothing at the moment, it just execute without raising errors - expect { subject.validate! }.not_to raise_error - end - end + it_behaves_like 'use predefined push rules' - context 'when feature flag is disabled' do - before do - stub_feature_flags(pre_receive_secret_detection_push_check: false) + context 'when instance is dedicated' do + before do + Gitlab::CurrentSettings.update!(gitlab_dedicated_instance: true) + end + + context 'when license is not ultimate' do + it 'skips the check' do + expect(subject.validate!).to be_nil + end + end + + context 'when license is ultimate' do + before do + stub_licensed_features(pre_receive_secret_detection: true) + end + + it 'returns without raising errors' do + # Since the check does nothing at the moment, it just execute without raising errors + expect { subject.validate! }.not_to raise_error + end + end end - it 'skips the check' do - expect(subject.validate!).to be_nil + context 'when instance is not dedicated' do + before do + Gitlab::CurrentSettings.update!(gitlab_dedicated_instance: false) + end + + context 'when license is not ultimate' do + it 'skips the check' do + expect(subject.validate!).to be_nil + end + end + + context 'when license is ultimate' do + before do + stub_licensed_features(pre_receive_secret_detection: true) + end + + it 'returns without raising errors' do + # Since the check does nothing at the moment, it just execute without raising errors + expect { subject.validate! }.not_to raise_error + end + end + + context 'when feature flag is disabled' do + before do + stub_feature_flags(pre_receive_secret_detection_push_check: false) + end + + it 'skips the check' do + expect(subject.validate!).to be_nil + end + end end end end