diff --git a/app/models/repository.rb b/app/models/repository.rb index 0b120ca15b20818d615a29818812f8f223ff7108..09bfcbdb7149a797c5acb2733527c59e1eba745e 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -179,8 +179,8 @@ def commits_between(from, to, limit: nil) end # Returns a list of commits that are not present in any reference - def new_commits(newrev) - commits = raw.new_commits(newrev) + def new_commits(newrev, timeout = nil) + commits = raw.new_commits(newrev, timeout) ::Commit.decorate(commits, container) end diff --git a/ee/lib/gitlab/checks/secret_push_protection/payload_processor.rb b/ee/lib/gitlab/checks/secret_push_protection/payload_processor.rb index d0a7efa7b105e712b5b18ca1c6db5471ab2fd5e5..e1c17094e71b08b1cc55476506c1f321fc0a5078 100644 --- a/ee/lib/gitlab/checks/secret_push_protection/payload_processor.rb +++ b/ee/lib/gitlab/checks/secret_push_protection/payload_processor.rb @@ -18,10 +18,20 @@ class PayloadProcessor < ::Gitlab::Checks::SecretPushProtection::Base DIFF_CONTEXT_LINE = ' ' END_OF_DIFF = '\\' + # Note: this shouldn't be required, as GITLAB_RAILS_RACK_TIMEOUT is 60s and INTERNAL_TIMEOUT is 50s + TIME_LEFT_BUFF = 10 # extra time to reduce chance of timeouts + MAX_TIMEOUT = 20 + LOG_MESSAGES = { invalid_encoding: "Could not convert data to UTF-8 from %{encoding}" }.freeze + def initialize(project:, changes_access:, timed_logger:) + super(project: project, changes_access: changes_access) + + @timed_logger = timed_logger + end + # The `standardize_payloads` method gets payloads containing git diffs # and converts them into a standardized format. Each payload is processed to include # its `id`, `data`, and `offset` (used to calculate the line number that a secret is on). @@ -189,7 +199,8 @@ def diff_blobs(paths_slice) right_blob: path.new_blob_id ) end - project.repository.diff_blobs(blob_pair_ids, patch_bytes_limit: PAYLOAD_BYTES_LIMIT).to_a + project.repository.diff_blobs(blob_pair_ids, patch_bytes_limit: PAYLOAD_BYTES_LIMIT, + timeout: gitaly_request_timeout).to_a end def diff_blobs_with_raw_info(paths_slice) @@ -213,7 +224,8 @@ def diff_blobs_with_raw_info(paths_slice) begin project.repository.diff_blobs_with_raw_info( raw_info_data, - patch_bytes_limit: PAYLOAD_BYTES_LIMIT + patch_bytes_limit: PAYLOAD_BYTES_LIMIT, + timeout: gitaly_request_timeout ).to_a rescue StandardError secret_detection_logger.error( @@ -226,7 +238,8 @@ def diff_blobs_with_raw_info(paths_slice) def get_diffs diffs = [] # Get new commits - commits = project.repository.new_commits(revisions) + pp "project.repository.new_commits #{gitaly_request_timeout}" + commits = project.repository.new_commits(revisions, gitaly_request_timeout) diff_filters = [ :DIFF_STATUS_ADDED, @@ -237,8 +250,9 @@ def get_diffs ] # Get changed paths + pp "project.repository.find_changed_paths #{gitaly_request_timeout}" paths = project.repository.find_changed_paths(commits, merge_commit_diff_mode: :all_parents, - find_renames: true, diff_filters: diff_filters) + find_renames: true, diff_filters: diff_filters, timeout: gitaly_request_timeout) # Reject diff blob objects from paths that are excluded # -- TODO: pass changed paths with diff blob objects and move this exclusion process into the gem. @@ -276,6 +290,17 @@ def exclusions_manager changes_access: changes_access ) end + + def gitaly_request_timeout + timeout = @timed_logger.time_left - TIME_LEFT_BUFF + timeout = MAX_TIMEOUT if timeout > MAX_TIMEOUT + if timeout <= 0 + raise Gitlab::Git::CommandTimedOut, + "Secret push protection has exceeded its time limit as timeout is #{timeout}" + end + + timeout + end end end end diff --git a/ee/lib/gitlab/checks/secret_push_protection/secrets_check.rb b/ee/lib/gitlab/checks/secret_push_protection/secrets_check.rb index 70b2fb036d50dd73791efdbaa69ddd338551b81a..496ec2d07eed29afc70dc34f13baa85a20352880 100644 --- a/ee/lib/gitlab/checks/secret_push_protection/secrets_check.rb +++ b/ee/lib/gitlab/checks/secret_push_protection/secrets_check.rb @@ -45,12 +45,16 @@ def public_project_without_spp? # Only runs for public projects that don't have SPP enabled or licensed. # See: https://gitlab.com/gitlab-org/gitlab/-/issues/551932 def run_validation_dark_launch! + pp "SHOULD BE HERE!!!" audit_logger.track_spp_scan_executed('dark-launch') logger.log_timed(LOG_MESSAGES[:secrets_check]) do + pp "SHOULD BE HERE!!!1" payloads = payload_processor.standardize_payloads + pp "SHOULD BE HERE!!!2" break unless payloads + pp "SHOULD BE HERE!!!3" extra_headers = { 'x-correlation-id': correlation_id, 'x-request-type': 'dark-launch' @@ -58,8 +62,14 @@ def run_validation_dark_launch! sds_client.send_request_to_sds(payloads, exclusions: exclusions_manager.active_exclusions, extra_headers: extra_headers) + + pp "GOT TO HERE!!!!" + pp logger.time_left + pp "GOT TO HERE!!!!/" end rescue StandardError => e + pp "SHOULD BE HERE ERROR #{e}" + pp e.backtrace ::Gitlab::ErrorTracking.track_exception(e) # Log the error but don't re-raise to prevent blocking pushes secret_detection_logger.error( @@ -69,6 +79,7 @@ def run_validation_dark_launch! error_class: e.class.name ) ) + raise e # re-raise to help with testing end def run_validation! @@ -171,7 +182,8 @@ def eligibility_checker def payload_processor @payload_processor = Gitlab::Checks::SecretPushProtection::PayloadProcessor.new( project: project, - changes_access: changes_access + changes_access: changes_access, + timed_logger: logger ) end diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index 7df6a542fd6be04c325ba09f76abd9617f9bff17..13b60963ddedbf61a6ac0f4608d70b931353f99a 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -397,9 +397,9 @@ def log(options) end end - def new_commits(newrevs) + def new_commits(newrevs, timeout = nil) wrapped_gitaly_errors do - gitaly_commit_client.list_new_commits(Array.wrap(newrevs)) + gitaly_commit_client.list_new_commits(Array.wrap(newrevs), timeout) end end @@ -548,13 +548,13 @@ def diff_stats(left_id, right_id) empty_diff_stats end - def find_changed_paths(treeish_objects, merge_commit_diff_mode: nil, find_renames: false, diff_filters: nil) + def find_changed_paths(treeish_objects, merge_commit_diff_mode: nil, find_renames: false, diff_filters: nil, timeout: nil) processed_objects = treeish_objects.compact return [] if processed_objects.empty? wrapped_gitaly_errors do - gitaly_commit_client.find_changed_paths(processed_objects, merge_commit_diff_mode: merge_commit_diff_mode, find_renames: find_renames, diff_filters: diff_filters) + gitaly_commit_client.find_changed_paths(processed_objects, merge_commit_diff_mode: merge_commit_diff_mode, find_renames: find_renames, diff_filters: diff_filters, timeout: timeout) end rescue CommandError, TypeError, NoRepository [] diff --git a/lib/gitlab/gitaly_client/commit_service.rb b/lib/gitlab/gitaly_client/commit_service.rb index be44a80e984b74df94afab1485dd041ebdbc7a50..1b762e9b26035372428392899aaf636331e2971c 100644 --- a/lib/gitlab/gitaly_client/commit_service.rb +++ b/lib/gitlab/gitaly_client/commit_service.rb @@ -251,12 +251,12 @@ def diff_stats(left_commit_sha, right_commit_sha) # else # defaults to :include_merges behavior # ['foo_bar.rb', 'bar_baz.rb'], # - def find_changed_paths(objects, merge_commit_diff_mode: nil, find_renames: false, diff_filters: nil) + def find_changed_paths(objects, merge_commit_diff_mode: nil, find_renames: false, diff_filters: nil, timeout: nil) request = find_changed_paths_request(objects, merge_commit_diff_mode, find_renames, diff_filters) return [] if request.nil? - response = gitaly_client_call(@repository.storage, :diff_service, :find_changed_paths, request, timeout: GitalyClient.medium_timeout) + response = gitaly_client_call(@repository.storage, :diff_service, :find_changed_paths, request, timeout: timeout || GitalyClient.medium_timeout) response.flat_map do |msg| msg.paths.map do |path| Gitlab::Git::ChangedPath.new( @@ -320,7 +320,7 @@ def list_commits(revisions, params = {}) end # List all commits which are new in the repository. If commits have been pushed into the repo - def list_new_commits(revisions) + def list_new_commits(revisions, timeout = nil) git_env = Gitlab::Git::HookEnv.all(@gitaly_repo.gl_repository) if git_env['GIT_OBJECT_DIRECTORY_RELATIVE'].present? # If we have a quarantine environment, then we can optimize the check @@ -337,7 +337,7 @@ def list_new_commits(revisions) repository: quarantined_repo ) - response = gitaly_client_call(@repository.storage, :commit_service, :list_all_commits, request, timeout: GitalyClient.medium_timeout) + response = gitaly_client_call(@repository.storage, :commit_service, :list_all_commits, request, timeout: timeout || GitalyClient.medium_timeout) quarantined_commits = consume_commits_response(response) quarantined_commit_ids = quarantined_commits.map(&:id) diff --git a/lib/gitlab/gitaly_client/diff_service.rb b/lib/gitlab/gitaly_client/diff_service.rb index ec93bd9364a7bf16a535b799d5e29f58adaf09fd..7e13c1944c77153eb8c635b52393d45c999f5fe9 100644 --- a/lib/gitlab/gitaly_client/diff_service.rb +++ b/lib/gitlab/gitaly_client/diff_service.rb @@ -32,7 +32,7 @@ def initialize(repository) # @return [GitalyClient::DiffBlobsStitcher] Streamed diff responses from Gitaly def diff_blobs( blob_pairs, diff_mode: DIFF_MODES[:unspecified], whitespace_changes: WHITESPACE_CHANGES[:unspecified], - patch_bytes_limit: 0 + patch_bytes_limit: 0, timeout: nil ) request = Gitaly::DiffBlobsRequest.new( repository: @gitaly_repo, @@ -43,7 +43,7 @@ def diff_blobs( ) response = gitaly_client_call(@storage, :diff_service, :diff_blobs, request, - timeout: GitalyClient.medium_timeout) + timeout: timeout || GitalyClient.medium_timeout) GitalyClient::DiffBlobsStitcher.new(response) end @@ -60,7 +60,8 @@ def diff_blobs_with_raw_info( raw_info, diff_mode: DIFF_MODES[:unspecified], whitespace_changes: WHITESPACE_CHANGES[:unspecified], - patch_bytes_limit: 0 + patch_bytes_limit: 0, + timeout: nil ) request = Gitaly::DiffBlobsRequest.new( repository: @gitaly_repo, @@ -71,7 +72,7 @@ def diff_blobs_with_raw_info( ) response = gitaly_client_call(@storage, :diff_service, :diff_blobs, request, - timeout: GitalyClient.medium_timeout) + timeout: timeout || GitalyClient.medium_timeout) GitalyClient::DiffBlobsStitcher.new(response) end