Allow custom hooks errors to appear in GitLab UI

Error messages from custom pre-receive hooks now appear in the GitLab
UI.

This is re-enabling a feature that had been disabled in merge request
https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/18646

The feature had been disabled due to security concerns that information
which was not intended to be public (like stack traces) would leak into
public view.

PreReceiveErrors (from pre-receive, post-receive and update custom
hooks) are now filtered for messages that have been prefixed in a
particular way.

Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/48132

7 files changed, 23 insertions, 13 deletions

diff --git a/spec/features/tags/master_deletes_tag_spec.rb b/spec/features/tags/master_deletes_tag_spec.rb
index 8d567e925ef..bdbbe645779 100644
--- a/spec/features/tags/master_deletes_tag_spec.rb
+++ b/spec/features/tags/master_deletes_tag_spec.rb
@@ -37,7 +37,7 @@ describe 'Maintainer deletes tag' do
   context 'when pre-receive hook fails', :js do
     before do
       allow_any_instance_of(Gitlab::GitalyClient::OperationService).to receive(:rm_tag)
-        .and_raise(Gitlab::Git::PreReceiveError, 'Do not delete tags')
+        .and_raise(Gitlab::Git::PreReceiveError, 'GitLab: Do not delete tags')
     end
 
     it 'shows the error message' do
diff --git a/spec/javascripts/vue_mr_widget/components/states/mr_widget_failed_to_merge_spec.js b/spec/javascripts/vue_mr_widget/components/states/mr_widget_failed_to_merge_spec.js
index 3229ddd5e27..780bed1bf69 100644
--- a/spec/javascripts/vue_mr_widget/components/states/mr_widget_failed_to_merge_spec.js
+++ b/spec/javascripts/vue_mr_widget/components/states/mr_widget_failed_to_merge_spec.js
@@ -120,7 +120,7 @@ describe('MRWidgetFailedToMerge', () => {
 
     it('renders given error', () => {
       expect(vm.$el.querySelector('.has-error-message').textContent.trim()).toEqual(
-        'Merge error happened.',
+        'Merge error happened',
       );
     });
 
diff --git a/spec/lib/gitlab/git/pre_receive_error_spec.rb b/spec/lib/gitlab/git/pre_receive_error_spec.rb
index 1b8be62dec6..cb030e38032 100644
--- a/spec/lib/gitlab/git/pre_receive_error_spec.rb
+++ b/spec/lib/gitlab/git/pre_receive_error_spec.rb
@@ -1,9 +1,19 @@
 require 'spec_helper'
 
 describe Gitlab::Git::PreReceiveError do
-  it 'makes its message HTML-friendly' do
-    ex = described_class.new("hello\nworld\n")
+  Gitlab::Git::PreReceiveError::SAFE_MESSAGE_PREFIXES.each do |prefix|
+    context "error messages prefixed with #{prefix}" do
+      it 'accepts only errors lines with the prefix' do
+        ex = described_class.new("#{prefix} Hello,\nworld!")
 
-    expect(ex.message).to eq('hello<br>world<br>')
+        expect(ex.message).to eq('Hello,')
+      end
+
+      it 'makes its message HTML-friendly' do
+        ex = described_class.new("#{prefix} Hello,\n#{prefix} world!\n")
+
+        expect(ex.message).to eq('Hello,<br>world!')
+      end
+    end
   end
 end
diff --git a/spec/lib/gitlab/gitaly_client/operation_service_spec.rb b/spec/lib/gitlab/gitaly_client/operation_service_spec.rb
index b37fe2686b6..7579a6577b9 100644
--- a/spec/lib/gitlab/gitaly_client/operation_service_spec.rb
+++ b/spec/lib/gitlab/gitaly_client/operation_service_spec.rb
@@ -39,7 +39,7 @@ describe Gitlab::GitalyClient::OperationService do
 
     context "when pre_receive_error is present" do
       let(:response) do
-        Gitaly::UserCreateBranchResponse.new(pre_receive_error: "something failed")
+        Gitaly::UserCreateBranchResponse.new(pre_receive_error: "GitLab: something failed")
       end
 
       it "throws a PreReceive exception" do
@@ -80,7 +80,7 @@ describe Gitlab::GitalyClient::OperationService do
 
     context "when pre_receive_error is present" do
       let(:response) do
-        Gitaly::UserUpdateBranchResponse.new(pre_receive_error: "something failed")
+        Gitaly::UserUpdateBranchResponse.new(pre_receive_error: "GitLab: something failed")
       end
 
       it "throws a PreReceive exception" do
@@ -117,7 +117,7 @@ describe Gitlab::GitalyClient::OperationService do
 
     context "when pre_receive_error is present" do
       let(:response) do
-        Gitaly::UserDeleteBranchResponse.new(pre_receive_error: "something failed")
+        Gitaly::UserDeleteBranchResponse.new(pre_receive_error: "GitLab: something failed")
       end
 
       it "throws a PreReceive exception" do
@@ -175,7 +175,7 @@ describe Gitlab::GitalyClient::OperationService do
 
   shared_examples 'cherry pick and revert errors' do
     context 'when a pre_receive_error is present' do
-      let(:response) { response_class.new(pre_receive_error: "something failed") }
+      let(:response) { response_class.new(pre_receive_error: "GitLab: something failed") }
 
       it 'raises a PreReceiveError' do
         expect { subject }.to raise_error(Gitlab::Git::PreReceiveError, "something failed")
@@ -313,7 +313,7 @@ describe Gitlab::GitalyClient::OperationService do
       end
 
       context 'when a pre_receive_error is present' do
-        let(:response) { Gitaly::UserCommitFilesResponse.new(pre_receive_error: "something failed") }
+        let(:response) { Gitaly::UserCommitFilesResponse.new(pre_receive_error: "GitLab: something failed") }
 
         it 'raises a PreReceiveError' do
           expect { subject }.to raise_error(Gitlab::Git::PreReceiveError, "something failed")
diff --git a/spec/services/merge_requests/ff_merge_service_spec.rb b/spec/services/merge_requests/ff_merge_service_spec.rb
index fe673de46aa..1430e12a07e 100644
--- a/spec/services/merge_requests/ff_merge_service_spec.rb
+++ b/spec/services/merge_requests/ff_merge_service_spec.rb
@@ -72,7 +72,7 @@ describe MergeRequests::FfMergeService do
       it 'logs and saves error if there is an PreReceiveError exception' do
         error_message = 'error message'
 
-        allow(service).to receive(:repository).and_raise(Gitlab::Git::PreReceiveError, error_message)
+        allow(service).to receive(:repository).and_raise(Gitlab::Git::PreReceiveError, "GitLab: #{error_message}")
         allow(service).to receive(:execute_hooks)
 
         service.execute(merge_request)
diff --git a/spec/services/merge_requests/merge_service_spec.rb b/spec/services/merge_requests/merge_service_spec.rb
index ede79b87bcc..887ec17171e 100644
--- a/spec/services/merge_requests/merge_service_spec.rb
+++ b/spec/services/merge_requests/merge_service_spec.rb
@@ -239,7 +239,7 @@ describe MergeRequests::MergeService do
       it 'logs and saves error if there is an PreReceiveError exception' do
         error_message = 'error message'
 
-        allow(service).to receive(:repository).and_raise(Gitlab::Git::PreReceiveError, error_message)
+        allow(service).to receive(:repository).and_raise(Gitlab::Git::PreReceiveError, "GitLab: #{error_message}")
         allow(service).to receive(:execute_hooks)
 
         service.execute(merge_request)
diff --git a/spec/services/tags/create_service_spec.rb b/spec/services/tags/create_service_spec.rb
index 0cbe57352be..e112cdc8881 100644
--- a/spec/services/tags/create_service_spec.rb
+++ b/spec/services/tags/create_service_spec.rb
@@ -41,7 +41,7 @@ describe Tags::CreateService do
       it 'returns an error' do
         expect(repository).to receive(:add_tag)
           .with(user, 'v1.1.0', 'master', 'Foo')
-          .and_raise(Gitlab::Git::PreReceiveError, 'something went wrong')
+          .and_raise(Gitlab::Git::PreReceiveError, 'GitLab: something went wrong')
 
         response = service.execute('v1.1.0', 'master', 'Foo')