Merge branch '292017-fj-fix-bug-when-cloning-snippet-different-from-master' into 'master'

Fix bug when creating snippet and default branch not master

See merge request gitlab-org/gitlab!50366

app/models/snippet.rb

@@ -20,6 +20,7 @@ class Snippet < ApplicationRecord
   extend ::Gitlab::Utils::Override
 
   MAX_FILE_COUNT = 10
+  MASTER_BRANCH = 'master'
 
   cache_markdown_field :title, pipeline: :single_line
   cache_markdown_field :description
@@ -311,13 +312,27 @@ class Snippet < ApplicationRecord
 
   override :default_branch
   def default_branch
-    super || 'master'
+    super || MASTER_BRANCH
   end
 
   def repository_storage
     snippet_repository&.shard_name || self.class.pick_repository_storage
   end
 
+  # Repositories are created by default with the `master` branch.
+  # This method changes the `HEAD` file to point to the existing
+  # default branch in case it's not master.
+  def change_head_to_default_branch
+    return unless repository.exists?
+    return if default_branch == MASTER_BRANCH
+    # All snippets must have at least 1 file. Therefore, if
+    # `HEAD` is empty is because it's pointing to the wrong
+    # default branch
+    return unless repository.empty? || list_files('HEAD').empty?
+
+    repository.raw_repository.write_ref('HEAD', "refs/heads/#{default_branch}")
+  end
+
   def create_repository
     return if repository_exists? && snippet_repository
 

changelogs/unreleased/292017-fj-fix-bug-when-cloning-snippet-different-from-master.yml

+---
+title: Fix bug with snippets in HEAD when default branch is not master
+merge_request: 50366
+author:
+type: fixed

lib/gitlab/git_access_snippet.rb

@@ -30,7 +30,10 @@ module Gitlab
     def check(cmd, changes)
       check_snippet_accessibility!
 
-      super
+      super.tap do |_|
+        # Ensure HEAD points to the default branch in case it is not master
+        snippet.change_head_to_default_branch
+      end
     end
 
     override :download_ability

spec/lib/gitlab/git_access_snippet_spec.rb

@@ -390,6 +390,38 @@ RSpec.describe Gitlab::GitAccessSnippet do
     end
   end
 
+  describe 'HEAD realignment' do
+    let_it_be(:snippet) { create(:project_snippet, :private, :repository, project: project) }
+
+    shared_examples 'HEAD is updated to the snippet default branch' do
+      let(:actor) { snippet.author }
+
+      specify do
+        expect(snippet).to receive(:change_head_to_default_branch).and_call_original
+
+        subject
+      end
+
+      context 'when an error is raised' do
+        let(:actor) { nil }
+
+        it 'does not realign HEAD' do
+          expect(snippet).not_to receive(:change_head_to_default_branch).and_call_original
+
+          expect { subject }.to raise_error(described_class::ForbiddenError)
+        end
+      end
+    end
+
+    it_behaves_like 'HEAD is updated to the snippet default branch' do
+      subject { push_access_check }
+    end
+
+    it_behaves_like 'HEAD is updated to the snippet default branch' do
+      subject { pull_access_check }
+    end
+  end
+
   private
 
   def raise_snippet_not_found

spec/models/snippet_spec.rb

@@ -796,4 +796,90 @@ RSpec.describe Snippet do
   it_behaves_like 'can move repository storage' do
     let_it_be(:container) { create(:snippet, :repository) }
   end
+
+  describe '#change_head_to_default_branch' do
+    let(:head_path) { Rails.root.join(TestEnv.repos_path, "#{snippet.disk_path}.git", 'HEAD') }
+
+    subject { snippet.change_head_to_default_branch }
+
+    context 'when repository does not exist' do
+      let(:snippet) { create(:snippet) }
+
+      it 'does nothing' do
+        expect(snippet.repository_exists?).to eq false
+        expect(snippet.repository.raw_repository).not_to receive(:write_ref)
+
+        subject
+      end
+    end
+
+    context 'when repository is empty' do
+      let(:snippet) { create(:snippet, :empty_repo) }
+
+      before do
+        allow(Gitlab::CurrentSettings).to receive(:default_branch_name).and_return(default_branch)
+      end
+
+      context 'when default branch in settings is "master"' do
+        let(:default_branch) { 'master' }
+
+        it 'does nothing' do
+          expect(File.read(head_path).squish).to eq 'ref: refs/heads/master'
+
+          expect(snippet.repository.raw_repository).not_to receive(:write_ref)
+
+          subject
+        end
+      end
+
+      context 'when default branch in settings is different from "master"' do
+        let(:default_branch) { 'main' }
+
+        it 'changes the HEAD reference to the default branch' do
+          expect(File.read(head_path).squish).to eq 'ref: refs/heads/master'
+
+          subject
+
+          expect(File.read(head_path).squish).to eq "ref: refs/heads/#{default_branch}"
+        end
+      end
+    end
+
+    context 'when repository is not empty' do
+      let(:snippet) { create(:snippet, :empty_repo) }
+
+      before do
+        populate_snippet_repo
+      end
+
+      context 'when HEAD branch is empty' do
+        it 'changes HEAD to default branch' do
+          File.write(head_path, 'ref: refs/heads/non_existen_branch')
+          expect(File.read(head_path).squish).to eq 'ref: refs/heads/non_existen_branch'
+
+          subject
+
+          expect(File.read(head_path).squish).to eq 'ref: refs/heads/main'
+          expect(snippet.list_files('HEAD')).not_to be_empty
+        end
+      end
+
+      context 'when HEAD branch is not empty' do
+        it 'does nothing' do
+          File.write(head_path, 'ref: refs/heads/main')
+
+          expect(snippet.repository.raw_repository).not_to receive(:write_ref)
+
+          subject
+        end
+      end
+
+      def populate_snippet_repo
+        allow(Gitlab::CurrentSettings).to receive(:default_branch_name).and_return('main')
+
+        data = [{ file_path: 'new_file_test', content: 'bar' }]
+        snippet.snippet_repository.multi_files_action(snippet.author, data, branch_name: 'main', message: 'foo')
+      end
+    end
+  end
 end