Prevent creating pipelines with ambiguous refs

8 files changed, 35 insertions, 30 deletions

diff --git a/lib/gitlab/ci/pipeline/chain/build.rb b/lib/gitlab/ci/pipeline/chain/build.rb
index d33d1edfe35..41632211374 100644
--- a/lib/gitlab/ci/pipeline/chain/build.rb
+++ b/lib/gitlab/ci/pipeline/chain/build.rb
@@ -17,7 +17,6 @@ module Gitlab
               user: @command.current_user,
               pipeline_schedule: @command.schedule,
               merge_request: @command.merge_request,
-              protected: @command.protected_ref?,
               variables_attributes: Array(@command.variables_attributes)
             )
 
diff --git a/lib/gitlab/ci/pipeline/chain/command.rb b/lib/gitlab/ci/pipeline/chain/command.rb
index 316c283d90b..ee5022e47c4 100644
--- a/lib/gitlab/ci/pipeline/chain/command.rb
+++ b/lib/gitlab/ci/pipeline/chain/command.rb
@@ -51,12 +51,6 @@ module Gitlab
           def before_sha
             self[:before_sha] || checkout_sha || Gitlab::Git::BLANK_SHA
           end
-
-          def protected_ref?
-            strong_memoize(:protected_ref) do
-              project.protected_for?(origin_ref)
-            end
-          end
         end
       end
     end
diff --git a/lib/gitlab/ci/pipeline/chain/populate.rb b/lib/gitlab/ci/pipeline/chain/populate.rb
index 633d3cd4f6b..45b4393ecf3 100644
--- a/lib/gitlab/ci/pipeline/chain/populate.rb
+++ b/lib/gitlab/ci/pipeline/chain/populate.rb
@@ -19,6 +19,11 @@ module Gitlab
             @command.seeds_block&.call(pipeline)
 
             ##
+            # Populate pipeline protected status
+            #
+            pipeline.protected = @command.project.protected_for?(@command.origin_ref)
+
+            ##
             # Populate pipeline with all stages, and stages with builds.
             #
             pipeline.stage_seeds.each do |stage|
diff --git a/lib/gitlab/ci/pipeline/chain/validate/abilities.rb b/lib/gitlab/ci/pipeline/chain/validate/abilities.rb
index ebd7e6e8289..e4979102fd9 100644
--- a/lib/gitlab/ci/pipeline/chain/validate/abilities.rb
+++ b/lib/gitlab/ci/pipeline/chain/validate/abilities.rb
@@ -31,7 +31,7 @@ module Gitlab
               if current_user
                 allowed_to_create?
               else # legacy triggers don't have a corresponding user
-                !@command.protected_ref?
+                !@command.project.protected_for?(@command.origin_ref)
               end
             end
 
diff --git a/lib/gitlab/ci/pipeline/chain/validate/repository.rb b/lib/gitlab/ci/pipeline/chain/validate/repository.rb
index d88851d8245..3cec55cdb71 100644
--- a/lib/gitlab/ci/pipeline/chain/validate/repository.rb
+++ b/lib/gitlab/ci/pipeline/chain/validate/repository.rb
@@ -16,6 +16,12 @@ module Gitlab
               unless @command.sha
                 return error('Commit not found')
               end
+
+              begin
+                @command.project.resolve_ref(@command.origin_ref)
+              rescue Project::AmbiguousRef
+                return error('Ref is ambiguous')
+              end
             end
 
             def break?
diff --git a/spec/lib/gitlab/ci/pipeline/chain/command_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/command_spec.rb
index 75a177d2d1f..14b4fbd1f5a 100644
--- a/spec/lib/gitlab/ci/pipeline/chain/command_spec.rb
+++ b/spec/lib/gitlab/ci/pipeline/chain/command_spec.rb
@@ -160,26 +160,4 @@ describe Gitlab::Ci::Pipeline::Chain::Command do
       end
     end
   end
-
-  describe '#protected_ref?' do
-    let(:command) { described_class.new(project: project, origin_ref: 'my-branch') }
-
-    subject { command.protected_ref? }
-
-    context 'when a ref is protected' do
-      before do
-        expect_any_instance_of(Project).to receive(:protected_for?).with('my-branch').and_return(true)
-      end
-
-      it { is_expected.to eq(true) }
-    end
-
-    context 'when a ref is unprotected' do
-      before do
-        expect_any_instance_of(Project).to receive(:protected_for?).with('my-branch').and_return(false)
-      end
-
-      it { is_expected.to eq(false) }
-    end
-  end
 end
diff --git a/spec/lib/gitlab/ci/pipeline/chain/populate_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/populate_spec.rb
index 284aed91e29..1b014ecfaa4 100644
--- a/spec/lib/gitlab/ci/pipeline/chain/populate_spec.rb
+++ b/spec/lib/gitlab/ci/pipeline/chain/populate_spec.rb
@@ -14,6 +14,7 @@ describe Gitlab::Ci::Pipeline::Chain::Populate do
     Gitlab::Ci::Pipeline::Chain::Command.new(
       project: project,
       current_user: user,
+      origin_ref: 'master',
       seeds_block: nil)
   end
 
@@ -106,6 +107,7 @@ describe Gitlab::Ci::Pipeline::Chain::Populate do
       Gitlab::Ci::Pipeline::Chain::Command.new(
         project: project,
         current_user: user,
+        origin_ref: 'master',
         seeds_block: seeds_block)
     end
 
diff --git a/spec/lib/gitlab/ci/pipeline/chain/validate/repository_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/validate/repository_spec.rb
index fb1b53fc55c..7b30a8381e9 100644
--- a/spec/lib/gitlab/ci/pipeline/chain/validate/repository_spec.rb
+++ b/spec/lib/gitlab/ci/pipeline/chain/validate/repository_spec.rb
@@ -42,6 +42,27 @@ describe Gitlab::Ci::Pipeline::Chain::Validate::Repository do
     end
   end
 
+  context 'when ref is ambiguous' do
+    let(:project) do
+      p = create(:project, :repository)
+      p.repository.add_tag(user, 'master', 'master')
+      p
+    end
+    let(:command) do
+      Gitlab::Ci::Pipeline::Chain::Command.new(
+        project: project, current_user: user, origin_ref: 'master')
+    end
+
+    it 'breaks the chain' do
+      expect(step.break?).to be true
+    end
+
+    it 'adds an error about missing ref' do
+      expect(pipeline.errors.to_a)
+        .to include 'Ref is ambiguous'
+    end
+  end
+
   context 'when does not have existing SHA set' do
     let(:command) do
       Gitlab::Ci::Pipeline::Chain::Command.new(