Merge branch 'security-fix-tag-ref-detection' into 'master'

Fix tag ref detection for pipelines

See merge request gitlab-org/security/gitlab!1496

lib/gitlab/ci/pipeline/chain/command.rb

@@ -26,13 +26,13 @@ module Gitlab
 
           def branch_exists?
             strong_memoize(:is_branch) do
-              project.repository.branch_exists?(ref)
+              branch_ref? && project.repository.branch_exists?(ref)
             end
           end
 
           def tag_exists?
             strong_memoize(:is_tag) do
-              project.repository.tag_exists?(ref)
+              tag_ref? && project.repository.tag_exists?(ref)
             end
           end
 
@@ -110,6 +110,32 @@ module Gitlab
           def dangling_build?
             %i[ondemand_dast_scan webide].include?(source)
           end
+
+          private
+
+          # Verifies that origin_ref is a fully qualified tag reference (refs/tags/<tag-name>)
+          #
+          # Fallbacks to `true` for backward compatibility reasons
+          # if origin_ref is a short ref
+          def tag_ref?
+            return true if full_git_ref_name_unavailable?
+
+            Gitlab::Git.tag_ref?(origin_ref).present?
+          end
+
+          # Verifies that origin_ref is a fully qualified branch reference (refs/heads/<branch-name>)
+          #
+          # Fallbacks to `true` for backward compatibility reasons
+          # if origin_ref is a short ref
+          def branch_ref?
+            return true if full_git_ref_name_unavailable?
+
+            Gitlab::Git.branch_ref?(origin_ref).present?
+          end
+
+          def full_git_ref_name_unavailable?
+            ref == origin_ref
+          end
         end
       end
     end

spec/lib/gitlab/ci/pipeline/chain/build_spec.rb

@@ -136,7 +136,7 @@ RSpec.describe Gitlab::Ci::Pipeline::Chain::Build do
     let(:command) do
       Gitlab::Ci::Pipeline::Chain::Command.new(
         source: :push,
-        origin_ref: 'mytag',
+        origin_ref: origin_ref,
         checkout_sha: project.commit.id,
         after_sha: nil,
         before_sha: nil,
@@ -147,6 +147,8 @@ RSpec.describe Gitlab::Ci::Pipeline::Chain::Build do
         current_user: user)
     end
 
+    let(:origin_ref) { 'mytag' }
+
     before do
       allow_any_instance_of(Repository).to receive(:tag_exists?).with('mytag').and_return(true)
 
@@ -156,6 +158,14 @@ RSpec.describe Gitlab::Ci::Pipeline::Chain::Build do
     it 'correctly indicated that this is a tagged pipeline' do
       expect(pipeline).to be_tag
     end
+
+    context 'when origin_ref is branch but tag ref with the same name exists' do
+      let(:origin_ref) { 'refs/heads/mytag' }
+
+      it 'correctly indicated that a pipeline is not tagged' do
+        expect(pipeline).not_to be_tag
+      end
+    end
   end
 
   context 'when pipeline is running for a merge request' do

spec/lib/gitlab/ci/pipeline/chain/command_spec.rb

@@ -27,6 +27,18 @@ RSpec.describe Gitlab::Ci::Pipeline::Chain::Command do
         it { is_expected.to eq(true) }
       end
 
+      context 'for fully described tag ref' do
+        let(:origin_ref) { 'refs/tags/master' }
+
+        it { is_expected.to eq(false) }
+      end
+
+      context 'for fully described branch ref' do
+        let(:origin_ref) { 'refs/heads/master' }
+
+        it { is_expected.to eq(true) }
+      end
+
       context 'for invalid branch' do
         let(:origin_ref) { 'something' }
 
@@ -43,6 +55,18 @@ RSpec.describe Gitlab::Ci::Pipeline::Chain::Command do
         it { is_expected.to eq(true) }
       end
 
+      context 'for fully described tag ref' do
+        let(:origin_ref) { 'refs/tags/v1.0.0' }
+
+        it { is_expected.to eq(true) }
+      end
+
+      context 'for fully described branch ref' do
+        let(:origin_ref) { 'refs/heads/v1.0.0' }
+
+        it { is_expected.to eq(false) }
+      end
+
       context 'for invalid ref' do
         let(:origin_ref) { 'something' }
 

spec/services/ci/create_pipeline_service_spec.rb

@@ -1209,6 +1209,73 @@ RSpec.describe Ci::CreatePipelineService do
       end
     end
 
+    context 'when pipeline is running for a nonexistant-branch' do
+      let(:gitlab_ci_yaml) { YAML.dump(test: { script: 'test' }) }
+
+      let(:ref_name) { 'refs/heads/nonexistant-branch' }
+
+      let(:pipeline) { execute_service.payload }
+
+      it 'does not create the pipeline' do
+        expect(pipeline).not_to be_created_successfully
+        expect(pipeline.errors[:base]).to eq(['Reference not found'])
+      end
+
+      context 'when there is a tag with that nonexistant-branch' do
+        # v1.0.0 is on the test repo as a tag
+        let(:ref_name) { 'refs/heads/v1.0.0' }
+
+        it 'does not create the pipeline' do
+          expect(pipeline).not_to be_created_successfully
+          expect(pipeline.errors[:base]).to eq(['Reference not found'])
+        end
+      end
+    end
+
+    context 'when pipeline is running for a branch with the name of both a branch and a tag' do
+      let(:gitlab_ci_yaml) { YAML.dump(test: { script: 'test' }) }
+
+      # v1.1.0 is on the test repo as branch and tag
+      let(:ref_name) { 'refs/heads/v1.1.0' }
+
+      let(:pipeline) { execute_service.payload }
+
+      it 'creates the pipeline for the branch' do
+        expect(pipeline).to be_created_successfully
+        expect(pipeline.branch?).to be true
+        expect(pipeline.tag?).to be false
+      end
+    end
+
+    context 'when pipeline is running for a tag with the name of both a branch and a tag' do
+      let(:gitlab_ci_yaml) { YAML.dump(test: { script: 'test' }) }
+
+      # v1.1.0 is on the test repo as branch and tag
+      let(:ref_name) { 'refs/tags/v1.1.0' }
+
+      let(:pipeline) { execute_service.payload }
+
+      it 'creates the pipeline for the tag' do
+        expect(pipeline).to be_created_successfully
+        expect(pipeline.branch?).to be false
+        expect(pipeline.tag?).to be true
+      end
+    end
+
+    context 'when pipeline is running for an ambiguous ref' do
+      let(:gitlab_ci_yaml) { YAML.dump(test: { script: 'test' }) }
+
+      # v1.1.0 is on the test repo as branch and tag
+      let(:ref_name) { 'v1.1.0' }
+
+      let(:pipeline) { execute_service.payload }
+
+      it 'does not create the pipeline' do
+        expect(pipeline).not_to be_created_successfully
+        expect(pipeline.errors[:base]).to eq(['Ref is ambiguous'])
+      end
+    end
+
     context 'when pipeline variables are specified' do
       let(:variables_attributes) do
         [{ key: 'first', secret_value: 'world' },

spec/services/git/process_ref_changes_service_spec.rb

@@ -109,9 +109,13 @@ RSpec.describe Git::ProcessRefChangesService do
             .to receive(:commit)
             .and_return(project.commit)
 
-          allow_any_instance_of(Repository)
-            .to receive(:branch_exists?)
-            .and_return(true)
+          if changes_method == :branch_changes
+            allow_any_instance_of(Repository).to receive(:branch_exists?) { true }
+          end
+
+          if changes_method == :tag_changes
+            allow_any_instance_of(Repository).to receive(:tag_exists?) { true }
+          end
         end
 
         context 'when git_push_create_all_pipelines is disabled' do