diff --git a/app/services/merge_requests/merge_service.rb b/app/services/merge_requests/merge_service.rb index cedfcb50e09..2209a60a840 100644 --- a/app/services/merge_requests/merge_service.rb +++ b/app/services/merge_requests/merge_service.rb @@ -50,21 +50,30 @@ module MergeRequests end def commit + log_info("Git merge started on JID #{merge_jid}") + commit_id = try_merge + + if commit_id + log_info("Git merge finished on JID #{merge_jid} commit #{commit_id}") + else + raise MergeError, 'Conflicts detected during merge' + end + + merge_request.update!(merge_commit_sha: commit_id) + end + + def try_merge message = params[:commit_message] || merge_request.merge_commit_message - log_info("Git merge started on JID #{merge_jid}") - commit_id = repository.merge(current_user, source, merge_request, message) - log_info("Git merge finished on JID #{merge_jid} commit #{commit_id}") - - raise MergeError, 'Conflicts detected during merge' unless commit_id - - merge_request.update(merge_commit_sha: commit_id) + repository.merge(current_user, source, merge_request, message) rescue Gitlab::Git::HooksService::PreReceiveError => e - raise MergeError, e.message - rescue StandardError => e - raise MergeError, "Something went wrong during merge: #{e.message}" + handle_merge_error(log_message: e.message) + raise MergeError, 'Something went wrong during merge pre-receive hook' + rescue => e + handle_merge_error(log_message: e.message) + raise MergeError, 'Something went wrong during merge' ensure - merge_request.update(in_progress_merge_commit_sha: nil) + merge_request.update!(in_progress_merge_commit_sha: nil) end def after_merge diff --git a/changelogs/unreleased/jprovazn-generic-error.yml b/changelogs/unreleased/jprovazn-generic-error.yml new file mode 100644 index 00000000000..ced3b84fe02 --- /dev/null +++ b/changelogs/unreleased/jprovazn-generic-error.yml @@ -0,0 +1,6 @@ +--- +title: Display only generic message on merge error to avoid exposing any potentially + sensitive or user unfriendly backend messages. +merge_request: +author: +type: fixed diff --git a/spec/services/merge_requests/merge_service_spec.rb b/spec/services/merge_requests/merge_service_spec.rb index c38ddf4612b..e8568bf8bb3 100644 --- a/spec/services/merge_requests/merge_service_spec.rb +++ b/spec/services/merge_requests/merge_service_spec.rb @@ -219,7 +219,7 @@ describe MergeRequests::MergeService do service.execute(merge_request) - expect(merge_request.merge_error).to include(error_message) + expect(merge_request.merge_error).to include('Something went wrong during merge') expect(Rails.logger).to have_received(:error).with(a_string_matching(error_message)) end @@ -231,7 +231,7 @@ describe MergeRequests::MergeService do service.execute(merge_request) - expect(merge_request.merge_error).to include(error_message) + expect(merge_request.merge_error).to include('Something went wrong during merge pre-receive hook') expect(Rails.logger).to have_received(:error).with(a_string_matching(error_message)) end