Skip git hooks commit validation when pushing new tag.

CHANGELOG-EE

-v 7.11.1
+v 7.11.0
+  - Skip git hooks commit validation when pushing new tag.
+
+v 7.10.1
   - Check if comment exists in Jira before sending a reference
 
 v 7.10.0

lib/gitlab/git_access.rb

@@ -150,14 +150,14 @@ module Gitlab
 
       # Return build_status_object(true) if all git hook checks passed successfully
       # or build_status_object(false) if any hook fails
-      pass_git_hooks?(user, project, ref, oldrev, newrev)
+      git_hook_check(user, project, ref, oldrev, newrev)
     end
 
     def forced_push?(oldrev, newrev)
       Gitlab::ForcePushCheck.force_push?(project, oldrev, newrev)
     end
 
-    def pass_git_hooks?(user, project, ref, oldrev, newrev)
+    def git_hook_check(user, project, ref, oldrev, newrev)
       return build_status_object(true) unless project.git_hook
 
       return build_status_object(true) unless newrev && oldrev
@@ -165,51 +165,53 @@ module Gitlab
       git_hook = project.git_hook
 
       # Prevent tag removal
-      if git_hook.deny_delete_tag
-        if project.repository.tag_names.include?(ref) && newrev =~ /0000000/
+      if Gitlab::Git.tag_ref?(ref)
+        if git_hook.deny_delete_tag && protected_tag?(tag_name(ref)) && Gitlab::Git.blank_ref?(newrev)
           return build_status_object(false, "You can not delete tag")
         end
-      end
-
-      # Check commit messages unless its branch removal
-      if git_hook.commit_validation? && newrev !~ /00000000/
-        if oldrev == Gitlab::Git::BLANK_SHA
-          oldrev = project.default_branch
-        end
-
-        commits = project.repository.commits_between(oldrev, newrev)
-        commits.each do |commit|
-          if git_hook.commit_message_regex.present?
-            unless commit.safe_message =~ Regexp.new(git_hook.commit_message_regex)
-              return build_status_object(false, "Commit message does not follow the pattern")
-            end
+      else
+        # Check commit messages unless its branch removal
+        if git_hook.commit_validation? && !Gitlab::Git.blank_ref?(newrev)
+          if Gitlab::Git.blank_ref?(oldrev)
+            oldrev = project.default_branch
           end
 
-          if git_hook.author_email_regex.present?
-            unless commit.committer_email =~ Regexp.new(git_hook.author_email_regex)
-              return build_status_object(false, "Commiter's email does not follow the pattern")
-            end
-            unless commit.author_email =~ Regexp.new(git_hook.author_email_regex)
-              return build_status_object(false, "Author's email does not follow the pattern")
+          commits = project.repository.commits_between(oldrev, newrev)
+          commits.each do |commit|
+            if git_hook.commit_message_regex.present?
+              unless commit.safe_message =~ Regexp.new(git_hook.commit_message_regex)
+                return build_status_object(false, "Commit message does not follow the pattern")
+              end
             end
-          end
 
-          # Check whether author is a GitLab member
-          if git_hook.member_check
-            unless User.existing_member?(commit.author_email)
-              return build_status_object(false, "Author is not a member of team")
+            if git_hook.author_email_regex.present?
+              unless commit.committer_email =~ Regexp.new(git_hook.author_email_regex)
+                return build_status_object(false, "Commiter's email does not follow the pattern")
+              end
+
+              unless commit.author_email =~ Regexp.new(git_hook.author_email_regex)
+                return build_status_object(false, "Author's email does not follow the pattern")
+              end
             end
-            if commit.author_email != commit.committer_email
-              unless User.existing_member?(commit.committer_email)
-                return build_status_object(false, "Commiter is not a member of team")
+
+            # Check whether author is a GitLab member
+            if git_hook.member_check
+              unless User.existing_member?(commit.author_email)
+                return build_status_object(false, "Author is not a member of team")
+              end
+
+              if commit.author_email != commit.committer_email
+                unless User.existing_member?(commit.committer_email)
+                  return build_status_object(false, "Commiter is not a member of team")
+                end
               end
             end
-          end
 
-          if git_hook.file_name_regex.present?
-            commit.diffs.each do |diff|
-              if (diff.renamed_file || diff.new_file) && diff.new_path =~ Regexp.new(git_hook.file_name_regex)
-                return build_status_object(false, "File name #{diff.new_path.inspect} does not follow the pattern")
+            if git_hook.file_name_regex.present?
+              commit.diffs.each do |diff|
+                if (diff.renamed_file || diff.new_file) && diff.new_path =~ Regexp.new(git_hook.file_name_regex)
+                  return build_status_object(false, "File name #{diff.new_path.inspect} does not follow the pattern")
+                end
               end
             end
           end

spec/lib/gitlab/git_access_spec.rb

@@ -233,16 +233,22 @@ describe Gitlab::GitAccess do
     end
   end
 
-  describe "pass_git_hooks?" do
+  describe "git_hook_check" do
     describe "author email check" do
       it 'returns true' do
-        access.pass_git_hooks?(user, project, 'refs/heads/master', '6f6d7e7ed', '570e7b2ab').should be_truthy
+        access.git_hook_check(user, project, 'refs/heads/master', '6f6d7e7ed', '570e7b2ab').should be_truthy
       end
 
       it 'returns false' do
         project.create_git_hook
         project.git_hook.update(commit_message_regex: "@only.com")
-        access.pass_git_hooks?(user, project, 'refs/heads/master', '6f6d7e7ed', '570e7b2ab').allowed?.should be_falsey
+        access.git_hook_check(user, project, 'refs/heads/master', '6f6d7e7ed', '570e7b2ab').allowed?.should be_falsey
+      end
+
+      it 'returns true for tags' do
+        project.create_git_hook
+        project.git_hook.update(commit_message_regex: "@only.com")
+        access.git_hook_check(user, project, 'refs/tags/v1', '6f6d7e7ed', '570e7b2ab').allowed?.should be_truthy
       end
     end
 
@@ -253,12 +259,12 @@ describe Gitlab::GitAccess do
       end
 
       it 'returns false for non-member user' do
-        access.pass_git_hooks?(user, project, 'refs/heads/master', '6f6d7e7ed', '570e7b2ab').allowed?.should be_falsey
+        access.git_hook_check(user, project, 'refs/heads/master', '6f6d7e7ed', '570e7b2ab').allowed?.should be_falsey
       end
 
       it 'returns true if committer is a gitlab member' do
         create(:user, email: 'dmitriy.zaporozhets@gmail.com')
-        access.pass_git_hooks?(user, project, 'refs/heads/master', '6f6d7e7ed', '570e7b2ab').allowed?.should be_truthy
+        access.git_hook_check(user, project, 'refs/heads/master', '6f6d7e7ed', '570e7b2ab').allowed?.should be_truthy
       end
     end
 
@@ -266,13 +272,13 @@ describe Gitlab::GitAccess do
       it 'returns false when filename is prohibited' do
         project.create_git_hook
         project.git_hook.update(file_name_regex: "jpg$")
-        access.pass_git_hooks?(user, project, 'refs/heads/master', '913c66a37', '33f3729a4').allowed?.should be_falsey
+        access.git_hook_check(user, project, 'refs/heads/master', '913c66a37', '33f3729a4').allowed?.should be_falsey
       end
 
       it 'returns true if file name is allowed' do
         project.create_git_hook
         project.git_hook.update(file_name_regex: "exe$")
-        access.pass_git_hooks?(user, project, 'refs/heads/master', '913c66a37', '33f3729a4').allowed?.should be_truthy
+        access.git_hook_check(user, project, 'refs/heads/master', '913c66a37', '33f3729a4').allowed?.should be_truthy
       end
     end
   end