Merge branch 'mc/bug/38984-wildcard-protected-tags-10-3' into 'security-10-3'

Fix using wildcards in protected tags to expose protected variables - 10.3

See merge request gitlab/gitlabhq!2307

7 files changed, 40 insertions, 9 deletions

diff --git a/app/models/project.rb b/app/models/project.rb
index 3fcb4abe7ac..eddd2c827cc 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -1557,8 +1557,11 @@ class Project < ActiveRecord::Base
   end
 
   def protected_for?(ref)
-    ProtectedBranch.protected?(self, ref) ||
+    if repository.branch_exists?(ref)
+      ProtectedBranch.protected?(self, ref)
+    elsif repository.tag_exists?(ref)
       ProtectedTag.protected?(self, ref)
+    end
   end
 
   def deployment_variables
diff --git a/app/models/repository.rb b/app/models/repository.rb
index 929ba97b4a1..fd483949f13 100644
--- a/app/models/repository.rb
+++ b/app/models/repository.rb
@@ -224,6 +224,12 @@ class Repository
     branch_names.include?(branch_name)
   end
 
+  def tag_exists?(tag_name)
+    return false unless raw_repository
+
+    tag_names.include?(tag_name)
+  end
+
   def ref_exists?(ref)
     !!raw_repository&.ref_exists?(ref)
   rescue ArgumentError
diff --git a/changelogs/unreleased/mc-bug-38984-wildcard-protected-tags.yml b/changelogs/unreleased/mc-bug-38984-wildcard-protected-tags.yml
new file mode 100644
index 00000000000..27219b096af
--- /dev/null
+++ b/changelogs/unreleased/mc-bug-38984-wildcard-protected-tags.yml
@@ -0,0 +1,5 @@
+---
+title: Fix wilcard protected tags protecting all branches
+merge_request:
+author:
+type: security
diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb
index 917a5aac501..1f19626bf4d 100644
--- a/spec/models/ci/build_spec.rb
+++ b/spec/models/ci/build_spec.rb
@@ -1556,7 +1556,7 @@ describe Ci::Build do
 
       context 'when the branch is protected' do
         before do
-          create(:protected_branch, project: build.project, name: build.ref)
+          allow(build.project).to receive(:protected_for?).with(build.ref).and_return(true)
         end
 
         it { is_expected.to include(protected_variable) }
@@ -1564,7 +1564,7 @@ describe Ci::Build do
 
       context 'when the tag is protected' do
         before do
-          create(:protected_tag, project: build.project, name: build.ref)
+          allow(build.project).to receive(:protected_for?).with(build.ref).and_return(true)
         end
 
         it { is_expected.to include(protected_variable) }
@@ -1601,7 +1601,7 @@ describe Ci::Build do
 
       context 'when the branch is protected' do
         before do
-          create(:protected_branch, project: build.project, name: build.ref)
+          allow(build.project).to receive(:protected_for?).with(build.ref).and_return(true)
         end
 
         it { is_expected.to include(protected_variable) }
@@ -1609,7 +1609,7 @@ describe Ci::Build do
 
       context 'when the tag is protected' do
         before do
-          create(:protected_tag, project: build.project, name: build.ref)
+          allow(build.project).to receive(:protected_for?).with(build.ref).and_return(true)
         end
 
         it { is_expected.to include(protected_variable) }
diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb
index 5e82a2988ce..1335bc04921 100644
--- a/spec/models/group_spec.rb
+++ b/spec/models/group_spec.rb
@@ -550,7 +550,7 @@ describe Group do
 
     context 'when the ref is a protected branch' do
       before do
-        create(:protected_branch, name: 'ref', project: project)
+        allow(project).to receive(:protected_for?).with('ref').and_return(true)
       end
 
       it_behaves_like 'ref is protected'
@@ -558,7 +558,7 @@ describe Group do
 
     context 'when the ref is a protected tag' do
       before do
-        create(:protected_tag, name: 'ref', project: project)
+        allow(project).to receive(:protected_for?).with('ref').and_return(true)
       end
 
       it_behaves_like 'ref is protected'
@@ -572,6 +572,10 @@ describe Group do
       let(:variable_child_2) { create(:ci_group_variable, group: group_child_2) }
       let(:variable_child_3) { create(:ci_group_variable, group: group_child_3) }
 
+      before do
+        allow(project).to receive(:protected_for?).with('ref').and_return(true)
+      end
+
       it 'returns all variables belong to the group and parent groups' do
         expected_array1 = [protected_variable, secret_variable]
         expected_array2 = [variable_child, variable_child_2, variable_child_3]
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index f4699fd243d..e493b100802 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -2083,7 +2083,7 @@ describe Project do
 
     context 'when the ref is a protected branch' do
       before do
-        create(:protected_branch, name: 'ref', project: project)
+        allow(project).to receive(:protected_for?).with('ref').and_return(true)
       end
 
       it_behaves_like 'ref is protected'
@@ -2091,7 +2091,7 @@ describe Project do
 
     context 'when the ref is a protected tag' do
       before do
-        create(:protected_tag, name: 'ref', project: project)
+        allow(project).to receive(:protected_for?).with('ref').and_return(true)
       end
 
       it_behaves_like 'ref is protected'
@@ -2116,6 +2116,8 @@ describe Project do
 
     context 'when the ref is a protected branch' do
       before do
+        allow(project).to receive(:repository).and_call_original
+        allow(project).to receive_message_chain(:repository, :branch_exists?).and_return(true)
         create(:protected_branch, name: 'ref', project: project)
       end
 
@@ -2126,6 +2128,8 @@ describe Project do
 
     context 'when the ref is a protected tag' do
       before do
+        allow(project).to receive_message_chain(:repository, :branch_exists?).and_return(false)
+        allow(project).to receive_message_chain(:repository, :tag_exists?).and_return(true)
         create(:protected_tag, name: 'ref', project: project)
       end
 
diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb
index 90415b34aeb..d0f0f09c29f 100644
--- a/spec/models/repository_spec.rb
+++ b/spec/models/repository_spec.rb
@@ -1179,6 +1179,15 @@ describe Repository do
     end
   end
 
+  describe '#tag_exists?' do
+    it 'uses tag_names' do
+      allow(repository).to receive(:tag_names).and_return(['foobar'])
+
+      expect(repository.tag_exists?('foobar')).to eq(true)
+      expect(repository.tag_exists?('master')).to eq(false)
+    end
+  end
+
   describe '#branch_names', :use_clean_rails_memory_store_caching do
     let(:fake_branch_names) { ['foobar'] }