Merge branch 'fix_git_hooks_for_web_ui_commits' into 'master'

Fix git hooks when commit goes from web UI

Fixes https://gitlab.com/gitlab-org/gitlab-ee/issues/587

See merge request !487

CHANGELOG-EE

@@ -15,6 +15,7 @@ v 8.9.0 (unreleased)
   - Remove explicit Gitlab::Metrics.action assignments, are already automatic.
   - [Elastic] Project members with guest role can't access confidential issues
   - Ability to lock file or folder in the repository
+  - Fix: Git hooks don't fire when committing from the UI
 
 v 8.8.5
   - Make sure OAuth routes that we generate for Geo matches with the ones in Rails routes !444

lib/gitlab/git_access.rb

@@ -420,7 +420,8 @@ module Gitlab
     end
 
     def old_commit?(commit)
-      commit.refs(project.repository).any?
+      # We skip refs/tmp ref because we use it for Web UI commiting
+      commit.refs(project.repository).reject { |ref| ref.name.start_with?('refs/tmp') }.any?
     end
   end
 end

spec/lib/gitlab/git_access_spec.rb

@@ -382,7 +382,8 @@ describe Gitlab::GitAccess, lib: true do
 
       it 'allows githook for new branch with an old bad commit' do
         bad_commit = double("Commit", safe_message: 'Some change').as_null_object
-        allow(bad_commit).to receive(:refs).and_return(['heads/master'])
+        ref_object = double(name: 'heads/master')
+        allow(bad_commit).to receive(:refs).and_return([ref_object])
         allow_any_instance_of(Repository).to receive(:commits_between).and_return([bad_commit])
 
         project.create_git_hook
@@ -394,7 +395,8 @@ describe Gitlab::GitAccess, lib: true do
 
       it 'allows githook for any change with an old bad commit' do
         bad_commit = double("Commit", safe_message: 'Some change').as_null_object
-        allow(bad_commit).to receive(:refs).and_return(['heads/master'])
+        ref_object = double(name: 'heads/master')
+        allow(bad_commit).to receive(:refs).and_return([ref_object])
         allow_any_instance_of(Repository).to receive(:commits_between).and_return([bad_commit])
 
         project.create_git_hook
@@ -403,6 +405,20 @@ describe Gitlab::GitAccess, lib: true do
         # push to new branch, so use a blank old rev and new ref
         expect(access.git_hook_check(user, project, 'refs/heads/master', '6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9', '570e7b2abdd848b95f2f578043fc23bd6f6fd24d')).to be_allowed
       end
+
+      it 'does not allow any change from Web UI with bad commit' do
+        bad_commit = double("Commit", safe_message: 'Some change').as_null_object
+        # We use tmp ref a a temporary for Web UI commiting
+        ref_object = double(name: 'refs/tmp')
+        allow(bad_commit).to receive(:refs).and_return([ref_object])
+        allow_any_instance_of(Repository).to receive(:commits_between).and_return([bad_commit])
+
+        project.create_git_hook
+        project.git_hook.update(commit_message_regex: "Change some files")
+
+        # push to new branch, so use a blank old rev and new ref
+        expect(access.git_hook_check(user, project, 'refs/heads/master', '6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9', '570e7b2abdd848b95f2f578043fc23bd6f6fd24d')).not_to be_allowed
+      end
     end
 
     describe "member_check" do