From 9c11a02e7483a97e3a5a38dca572383d5f02a813 Mon Sep 17 00:00:00 2001 From: Ahmed Hemdan Date: Mon, 11 Sep 2023 17:43:27 +0200 Subject: [PATCH 1/2] POC for secrets detection push check --- ee/lib/ee/gitlab/checks/push_rule_check.rb | 13 ++++++ .../gitlab/checks/push_rules/secrets_check.rb | 41 +++++++++++++++++++ .../ee/gitlab/checks/push_rule_check_spec.rb | 2 + 3 files changed, 56 insertions(+) create mode 100644 ee/lib/ee/gitlab/checks/push_rules/secrets_check.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..4ec3b459f5c49c --- /dev/null +++ b/ee/lib/ee/gitlab/checks/push_rules/secrets_check.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true + +module EE + module Gitlab + module Checks + module PushRules + class SecretsCheck < ::Gitlab::Checks::BaseBulkChecker + ERROR_MESSAGE = "Secrets check failed." + LOG_MESSAGE = "Checking if any files contain secrets..." + + def validate! + return unless push_rule + + # Log that we're running secrets check + logger.log_timed(LOG_MESSAGE) do + # Maybe move all code below into its own check class? (similar to FileSizeCheck::AnyOversizedBlobs) + all_blobs = project.repository.list_all_blobs( + # Limit blobs size to 10MB, see https://gitlab.com/gitlab-org/gitlab/-/issues/422574#note_1524129330 + # we could also check if the blob is a binary through `blob.binary` but we would have to fetch + # all blobs first, so it might be better to limit the blob size early on here instead. + bytes_limit: 1_000_000_0, + dynamic_timeout: logger.time_left + ) + + # Enumerate through blobs and check secrets + all_blobs.each do |_blob| + # Check blob for secrets here, since this is still + # to be done, we sleep for one second instead. + sleep(1) + end + + # For the sake of displaying an error message, + # we raise on an always true if statement check. + raise ::Gitlab::GitAccess::ForbiddenError, ERROR_MESSAGE if [].empty? + end + 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) -- GitLab From 9bb0d59ecdb843acb27be056352d76eb948cc703 Mon Sep 17 00:00:00 2001 From: Ahmed Hemdan Date: Wed, 13 Sep 2023 02:01:03 +0200 Subject: [PATCH 2/2] Skip secrets scanning if flag is present in commit message --- .../ee/gitlab/checks/push_rules/secrets_check.rb | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 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 4ec3b459f5c49c..7ef04ce22c683d 100644 --- a/ee/lib/ee/gitlab/checks/push_rules/secrets_check.rb +++ b/ee/lib/ee/gitlab/checks/push_rules/secrets_check.rb @@ -8,9 +8,16 @@ class SecretsCheck < ::Gitlab::Checks::BaseBulkChecker ERROR_MESSAGE = "Secrets check failed." LOG_MESSAGE = "Checking if any files contain secrets..." + SKIP_PATTERN = /\[(secret[ _-]detection[ _-]skip|skip[ _-]secret[ _-]detection)\]/i + def validate! return unless push_rule + # To show how a [skip secret detection] flag might work, + # we will go over the commit message and see if we can + # match it with the regex pattern above + return if skip_secret_detection? + # Log that we're running secrets check logger.log_timed(LOG_MESSAGE) do # Maybe move all code below into its own check class? (similar to FileSizeCheck::AnyOversizedBlobs) @@ -29,11 +36,15 @@ def validate! sleep(1) end - # For the sake of displaying an error message, - # we raise on an always true if statement check. raise ::Gitlab::GitAccess::ForbiddenError, ERROR_MESSAGE if [].empty? end end + + private + + def skip_secret_detection? + changes_access.commits.any? { |commit| commit.safe_message =~ SKIP_PATTERN } + end end end end -- GitLab