Merge branch 'rs-pick-bvl-security' into 'master'

Pick a released security fix into master

See merge request !2895

lib/gitlab/checks/change_access.rb

@@ -14,7 +14,8 @@ module Gitlab
         change_existing_tags: 'You are not allowed to change existing tags on this project.',
         update_protected_tag: 'Protected tags cannot be updated.',
         delete_protected_tag: 'Protected tags cannot be deleted.',
-        create_protected_tag: 'You are not allowed to create this tag as it is protected.'
+        create_protected_tag: 'You are not allowed to create this tag as it is protected.',
+        push_rule_branch_name: "Branch name does not follow the pattern '%{branch_name_regex}'"
       }.freeze
 
       # protocol is currently used only in EE
@@ -152,8 +153,12 @@ module Gitlab
             raise GitAccess::UnauthorizedError, 'You cannot delete a tag'
           end
         else
-          commit_validation = push_rule.try(:commit_validation?)
+          unless branch_name_allowed_by_push_rule?(push_rule)
+            message = ERROR_MESSAGES[:push_rule_branch_name] % { branch_name_regex: push_rule.branch_name_regex }
+            raise GitAccess::UnauthorizedError.new(message)
+          end
 
+          commit_validation = push_rule.try(:commit_validation?)
           # if newrev is blank, the branch was deleted
           return if deletion? || !(commit_validation || validate_path_locks?)
 
@@ -170,6 +175,13 @@ module Gitlab
         end
       end
 
+      def branch_name_allowed_by_push_rule?(push_rule)
+        return true unless push_rule
+        return true if @branch_name.empty?
+
+        push_rule.branch_name_allowed?(@branch_name)
+      end
+
       def tag_deletion_denied_by_push_rule?(push_rule)
         push_rule.try(:deny_delete_tag) &&
           protocol != 'web' &&
@@ -185,10 +197,6 @@ module Gitlab
           return "Commit message does not follow the pattern '#{push_rule.commit_message_regex}'"
         end
 
-        if @branch_name && !push_rule.branch_name_allowed?(@branch_name)
-          return "Branch name does not follow the pattern '#{push_rule.branch_name_regex}'"
-        end
-
         unless push_rule.author_email_allowed?(commit.committer_email)
           return "Committer's email '#{commit.committer_email}' does not follow the pattern '#{push_rule.author_email_regex}'"
         end

spec/lib/gitlab/checks/change_access_spec.rb

@@ -239,6 +239,27 @@ describe Gitlab::Checks::ChangeAccess do
         end
       end
 
+      context 'branch name rules' do
+        let(:push_rule) { create(:push_rule, branch_name_regex: '^(w*)$') }
+        let(:ref) { 'refs/heads/a-branch-that-is-not-allowed' }
+
+        it_behaves_like 'check ignored when push rule unlicensed'
+
+        it 'rejects the branch that is not allowed' do
+          expect { subject }.to raise_error(Gitlab::GitAccess::UnauthorizedError, "Branch name does not follow the pattern '^(w*)$'")
+        end
+
+        context 'when no commits are present' do
+          before do
+            allow(project.repository).to receive(:new_commits) { [] }
+          end
+
+          it 'rejects the branch that is not allowed' do
+            expect { subject }.to raise_error(Gitlab::GitAccess::UnauthorizedError, "Branch name does not follow the pattern '^(w*)$'")
+          end
+        end
+      end
+
       context 'existing member rules' do
         let(:push_rule) { create(:push_rule, member_check: true) }