From cdaf142991f7b856f3f3ea5134b20d01af54d769 Mon Sep 17 00:00:00 2001 From: Ahmed Hemdan Date: Tue, 24 Oct 2023 19:17:07 +0200 Subject: [PATCH 1/2] Add placeholder push check for secrets detection This commit creates a new placeholder push check that will be used for pre-receive secret detection. The check is also added to EE::Gitlab::Checks::PushRuleCheck to ensure it runs with other similar checks. --- ee/lib/ee/gitlab/checks/push_rule_check.rb | 13 +++++++++++++ .../ee/gitlab/checks/push_rules/secrets_check.rb | 15 +++++++++++++++ .../lib/ee/gitlab/checks/push_rule_check_spec.rb | 2 ++ .../checks/push_rules/secrets_check_spec.rb | 11 +++++++++++ 4 files changed, 41 insertions(+) create mode 100644 ee/lib/ee/gitlab/checks/push_rules/secrets_check.rb create mode 100644 ee/spec/lib/ee/gitlab/checks/push_rules/secrets_check_spec.rb diff --git a/ee/lib/ee/gitlab/checks/push_rule_check.rb b/ee/lib/ee/gitlab/checks/push_rule_check.rb index 10cff97a08a20e..67a7c9b9dff236 100644 --- a/ee/lib/ee/gitlab/checks/push_rule_check.rb +++ b/ee/lib/ee/gitlab/checks/push_rule_check.rb @@ -38,6 +38,14 @@ def check_file_size! nil end + # @return [Nil] returns nil unless an error is raised + # @raise [Gitlab::GitAccess::ForbiddenError] if check fails + def check_secrets! + PushRules::SecretsCheck.new(changes_access).validate! + + nil + end + # Run the checks one after the other. # # @return [Nil] returns nil unless an error is raised @@ -45,6 +53,7 @@ def check_file_size! def run_checks_in_sequence! check_tag_or_branch! check_file_size! + check_secrets! end # Run the checks in separate threads for performance benefits. @@ -67,6 +76,10 @@ def run_checks_in_parallel! check_file_size! end + parallelize(git_env) do + check_secrets! + end + # Block whilst waiting for threads, however if one errors # it will exit early and raise the error immediately as # we set `abort_on_exception` to true. diff --git a/ee/lib/ee/gitlab/checks/push_rules/secrets_check.rb b/ee/lib/ee/gitlab/checks/push_rules/secrets_check.rb new file mode 100644 index 00000000000000..bcdcf450b523b6 --- /dev/null +++ b/ee/lib/ee/gitlab/checks/push_rules/secrets_check.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +module EE + module Gitlab + module Checks + module PushRules + class SecretsCheck < ::Gitlab::Checks::BaseBulkChecker + def validate! + return unless push_rule + end + end + end + end + end +end diff --git a/ee/spec/lib/ee/gitlab/checks/push_rule_check_spec.rb b/ee/spec/lib/ee/gitlab/checks/push_rule_check_spec.rb index 92b1adf5afc4f1..776638fa526276 100644 --- a/ee/spec/lib/ee/gitlab/checks/push_rule_check_spec.rb +++ b/ee/spec/lib/ee/gitlab/checks/push_rule_check_spec.rb @@ -16,6 +16,8 @@ shared_examples "push checks" do before do + allow_any_instance_of(EE::Gitlab::Checks::PushRules::SecretsCheck) + .to receive(:validate!).and_return(nil) allow_any_instance_of(EE::Gitlab::Checks::PushRules::FileSizeCheck) .to receive(:validate!).and_return(nil) allow_any_instance_of(EE::Gitlab::Checks::PushRules::TagCheck) 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 new file mode 100644 index 00000000000000..7e481e909a1416 --- /dev/null +++ b/ee/spec/lib/ee/gitlab/checks/push_rules/secrets_check_spec.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe EE::Gitlab::Checks::PushRules::SecretsCheck, feature_category: :secret_detection do + include_context 'push rules checks context' + + describe '#validate!' do + xit 'spec examples will go here' + end +end -- GitLab From 49ce545daf06e5e5032336d9c5d84077bc6fdc58 Mon Sep 17 00:00:00 2001 From: Ahmed Hemdan Date: Tue, 24 Oct 2023 20:05:49 +0200 Subject: [PATCH 2/2] Add feature flag and put push check behind it --- .../pre_receive_secret_detection_push_check.yml | 8 ++++++++ .../ee/gitlab/checks/push_rules/secrets_check.rb | 5 ++++- .../checks/push_rules/secrets_check_spec.rb | 15 ++++++++++++++- 3 files changed, 26 insertions(+), 2 deletions(-) create mode 100644 ee/config/feature_flags/development/pre_receive_secret_detection_push_check.yml diff --git a/ee/config/feature_flags/development/pre_receive_secret_detection_push_check.yml b/ee/config/feature_flags/development/pre_receive_secret_detection_push_check.yml new file mode 100644 index 00000000000000..0005304987c6ea --- /dev/null +++ b/ee/config/feature_flags/development/pre_receive_secret_detection_push_check.yml @@ -0,0 +1,8 @@ +--- +name: pre_receive_secret_detection_push_check +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/135032 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/429287 +milestone: '16.6' +type: development +group: group::static analysis +default_enabled: false 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 bcdcf450b523b6..1367887ef0142a 100644 --- a/ee/lib/ee/gitlab/checks/push_rules/secrets_check.rb +++ b/ee/lib/ee/gitlab/checks/push_rules/secrets_check.rb @@ -6,7 +6,10 @@ module Checks module PushRules class SecretsCheck < ::Gitlab::Checks::BaseBulkChecker def validate! - return unless push_rule + # Return early and not perform the check if: + # 1. no push rule exist + # 2. feature flag is disabled + return unless push_rule && ::Feature.enabled?(:pre_receive_secret_detection_push_check, push_rule.project) 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 7e481e909a1416..fe005f3fa4501e 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 @@ -5,7 +5,20 @@ RSpec.describe EE::Gitlab::Checks::PushRules::SecretsCheck, feature_category: :secret_detection do include_context 'push rules checks context' + let_it_be(:push_rule) { create(:push_rule) } + describe '#validate!' do - xit 'spec examples will go here' + it_behaves_like 'check ignored when push rule unlicensed' + 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) + end + + it 'skips the check' do + expect(subject.validate!).to be_nil + end + end end end -- GitLab