Merge branch '36010-api-v4-allows-setting-a-branch-that-doesn-t-exist-as-the-default-one' into 'master'

Resolve "API v4 allows setting a branch that doesn't exist as the default one"

Closes #36010

See merge request !13359

5 files changed, 29 insertions, 8 deletions

diff --git a/app/models/project.rb b/app/models/project.rb
index e7baba2ef08..1575f38b2ea 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -1046,13 +1046,18 @@ class Project < ActiveRecord::Base
   end
 
   def change_head(branch)
-    repository.before_change_head
-    repository.rugged.references.create('HEAD',
-                                        "refs/heads/#{branch}",
-                                        force: true)
-    repository.copy_gitattributes(branch)
-    repository.after_change_head
-    reload_default_branch
+    if repository.branch_exists?(branch)
+      repository.before_change_head
+      repository.rugged.references.create('HEAD',
+                                          "refs/heads/#{branch}",
+                                          force: true)
+      repository.copy_gitattributes(branch)
+      repository.after_change_head
+      reload_default_branch
+    else
+      errors.add(:base, "Could not change HEAD: branch '#{branch}' does not exist")
+      false
+    end
   end
 
   def forked_from?(project)
diff --git a/app/services/projects/update_service.rb b/app/services/projects/update_service.rb
index d81035e4eba..cf69007bc3b 100644
--- a/app/services/projects/update_service.rb
+++ b/app/services/projects/update_service.rb
@@ -10,7 +10,7 @@ module Projects
       end
 
       if changing_default_branch?
-        project.change_head(params[:default_branch])
+        return error("Could not set the default branch") unless project.change_head(params[:default_branch])
       end
 
       if project.update_attributes(params.except(:default_branch))
diff --git a/changelogs/unreleased/36010-api-v4-allows-setting-a-branch-that-doesn-t-exist-as-the-default-one.yml b/changelogs/unreleased/36010-api-v4-allows-setting-a-branch-that-doesn-t-exist-as-the-default-one.yml
new file mode 100644
index 00000000000..04791e09b84
--- /dev/null
+++ b/changelogs/unreleased/36010-api-v4-allows-setting-a-branch-that-doesn-t-exist-as-the-default-one.yml
@@ -0,0 +1,4 @@
+---
+title: Add checks for branch existence before changing HEAD
+merge_request: 13359
+author: Vitaliy @blackst0ne Klachkov
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index 8f951605954..581fea65838 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -1832,6 +1832,11 @@ describe Project do
   describe '#change_head' do
     let(:project) { create(:project, :repository) }
 
+    it 'returns error if branch does not exist' do
+      expect(project.change_head('unexisted-branch')).to be false
+      expect(project.errors.size).to eq(1)
+    end
+
     it 'calls the before_change_head and after_change_head methods' do
       expect(project.repository).to receive(:before_change_head)
       expect(project.repository).to receive(:after_change_head)
diff --git a/spec/services/projects/update_service_spec.rb b/spec/services/projects/update_service_spec.rb
index d945e0aa1f0..1b282e82187 100644
--- a/spec/services/projects/update_service_spec.rb
+++ b/spec/services/projects/update_service_spec.rb
@@ -101,6 +101,13 @@ describe Projects::UpdateService, '#execute' do
 
       expect(Project.find(project.id).default_branch).to eq 'feature'
     end
+
+    it 'does not change a default branch' do
+      # The branch 'unexisted-branch' does not exist.
+      update_project(project, admin, default_branch: 'unexisted-branch')
+
+      expect(Project.find(project.id).default_branch).to eq 'master'
+    end
   end
 
   context 'when updating a project that contains container images' do