From 42836855e3d0b523ef84ec7d73e1aeab8d48d7d3 Mon Sep 17 00:00:00 2001 From: Ahmed Hemdan Date: Thu, 26 Oct 2023 17:50:36 +0200 Subject: [PATCH 1/3] Add a migration for pre_receive_secret_detection_enabled setting --- ...e_secret_detection_enabled_to_application_settings.rb | 9 +++++++++ db/schema_migrations/20231025191217 | 1 + db/structure.sql | 1 + 3 files changed, 11 insertions(+) create mode 100644 db/migrate/20231025191217_add_pre_receive_secret_detection_enabled_to_application_settings.rb create mode 100644 db/schema_migrations/20231025191217 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 00000000000000..a447183a6d2c7c --- /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.6' + + 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 00000000000000..c0458feed62be9 --- /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 1055e902056f7d..e5bcd33f19861c 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)), -- GitLab From 4db4b4b10acb31cab2f62c895fc01bf89dc7e996 Mon Sep 17 00:00:00 2001 From: Ahmed Hemdan Date: Thu, 26 Oct 2023 17:50:36 +0200 Subject: [PATCH 2/3] Update push check to honor application setting --- .../gitlab/checks/push_rules/secrets_check.rb | 16 ++-- .../checks/push_rules/secrets_check_spec.rb | 74 +++++++++++++++---- 2 files changed, 70 insertions(+), 20 deletions(-) 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 6361e45925b1fe..c0fa9d868dbe09 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 cab206795bbcbb..585cb7c0d553ad 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 -- GitLab From 4d631d3dec2fad88204d3697ed9ca3737f2f5851 Mon Sep 17 00:00:00 2001 From: Ahmed Hemdan Date: Mon, 13 Nov 2023 22:05:31 +0100 Subject: [PATCH 3/3] Update milestone in migration --- ..._receive_secret_detection_enabled_to_application_settings.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 index a447183a6d2c7c..db37fc5e8f1574 100644 --- 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 @@ -1,7 +1,7 @@ # frozen_string_literal: true class AddPreReceiveSecretDetectionEnabledToApplicationSettings < Gitlab::Database::Migration[2.2] - milestone '16.6' + milestone '16.7' def change add_column :application_settings, :pre_receive_secret_detection_enabled, :boolean, null: false, default: false -- GitLab