Merge branch 'master' of dev.gitlab.org:gitlab/gitlabhq

app/models/ci/build.rb

@@ -10,6 +10,7 @@ module Ci
     include Importable
     include Gitlab::Utils::StrongMemoize
     include Deployable
+    include HasRef
 
     belongs_to :project, inverse_of: :builds
     belongs_to :runner
@@ -640,11 +641,11 @@ module Ci
     def secret_group_variables
       return [] unless project.group
 
-      project.group.ci_variables_for(ref, project)
+      project.group.ci_variables_for(git_ref, project)
     end
 
     def secret_project_variables(environment: persisted_environment)
-      project.ci_variables_for(ref: ref, environment: environment)
+      project.ci_variables_for(ref: git_ref, environment: environment)
     end
 
     def steps

app/models/ci/pipeline.rb

@@ -11,6 +11,7 @@ module Ci
     include Gitlab::Utils::StrongMemoize
     include AtomicInternalId
     include EnumWithNil
+    include HasRef
 
     belongs_to :project, inverse_of: :all_pipelines
     belongs_to :user
@@ -380,7 +381,7 @@ module Ci
     end
 
     def branch?
-      !tag? && !merge_request?
+      super && !merge_request?
     end
 
     def stuck?
@@ -580,7 +581,7 @@ module Ci
     end
 
     def protected_ref?
-      strong_memoize(:protected_ref) { project.protected_for?(ref) }
+      strong_memoize(:protected_ref) { project.protected_for?(git_ref) }
     end
 
     def legacy_trigger
@@ -712,14 +713,10 @@ module Ci
     end
 
     def git_ref
-      if branch?
+      if merge_request?
         Gitlab::Git::BRANCH_REF_PREFIX + ref.to_s
-      elsif merge_request?
-        Gitlab::Git::BRANCH_REF_PREFIX + ref.to_s
-      elsif tag?
-        Gitlab::Git::TAG_REF_PREFIX + ref.to_s
       else
-        raise ArgumentError, 'Invalid pipeline type!'
+        super
       end
     end
 

app/models/concerns/has_ref.rb

+# frozen_string_literal: true
+
+module HasRef
+  extend ActiveSupport::Concern
+
+  def branch?
+    !tag?
+  end
+
+  def git_ref
+    if branch?
+      Gitlab::Git::BRANCH_REF_PREFIX + ref.to_s
+    elsif tag?
+      Gitlab::Git::TAG_REF_PREFIX + ref.to_s
+    end
+  end
+end

app/models/project.rb

@@ -1733,10 +1733,21 @@ class Project < ActiveRecord::Base
   end
 
   def protected_for?(ref)
-    if repository.branch_exists?(ref)
-      ProtectedBranch.protected?(self, ref)
-    elsif repository.tag_exists?(ref)
-      ProtectedTag.protected?(self, ref)
+    raise Repository::AmbiguousRefError if repository.ambiguous_ref?(ref)
+
+    resolved_ref = repository.expand_ref(ref) || ref
+    return false unless Gitlab::Git.tag_ref?(resolved_ref) || Gitlab::Git.branch_ref?(resolved_ref)
+
+    ref_name = if resolved_ref == ref
+                 Gitlab::Git.ref_name(resolved_ref)
+               else
+                 ref
+               end
+
+    if Gitlab::Git.branch_ref?(resolved_ref)
+      ProtectedBranch.protected?(self, ref_name)
+    elsif Gitlab::Git.tag_ref?(resolved_ref)
+      ProtectedTag.protected?(self, ref_name)
     end
   end
 

app/models/repository.rb

@@ -25,6 +25,7 @@ class Repository
   delegate :bundle_to_disk, to: :raw_repository
 
   CreateTreeError = Class.new(StandardError)
+  AmbiguousRefError = Class.new(StandardError)
 
   # Methods that cache data from the Git repository.
   #
@@ -181,6 +182,18 @@ class Repository
     tags.find { |tag| tag.name == name }
   end
 
+  def ambiguous_ref?(ref)
+    tag_exists?(ref) && branch_exists?(ref)
+  end
+
+  def expand_ref(ref)
+    if tag_exists?(ref)
+      Gitlab::Git::TAG_REF_PREFIX + ref
+    elsif branch_exists?(ref)
+      Gitlab::Git::BRANCH_REF_PREFIX + ref
+    end
+  end
+
   def add_branch(user, branch_name, ref)
     branch = raw_repository.add_branch(branch_name, user: user, target: ref)
 

app/models/todo.rb

@@ -4,6 +4,11 @@ class Todo < ActiveRecord::Base
   include Sortable
   include FromUnion
 
+  # Time to wait for todos being removed when not visible for user anymore.
+  # Prevents TODOs being removed by mistake, for example, removing access from a user
+  # and giving it back again.
+  WAIT_FOR_DELETE    = 1.hour
+
   ASSIGNED           = 1
   MENTIONED          = 2
   BUILD_FAILED       = 3

app/services/groups/update_service.rb

@@ -31,7 +31,7 @@ module Groups
     def after_update
       if group.previous_changes.include?(:visibility_level) && group.private?
         # don't enqueue immediately to prevent todos removal in case of a mistake
-        TodosDestroyer::GroupPrivateWorker.perform_in(1.hour, group.id)
+        TodosDestroyer::GroupPrivateWorker.perform_in(Todo::WAIT_FOR_DELETE, group.id)
       end
     end
 

app/services/issues/update_service.rb

@@ -44,7 +44,7 @@ module Issues
 
       if issue.previous_changes.include?('confidential')
         # don't enqueue immediately to prevent todos removal in case of a mistake
-        TodosDestroyer::ConfidentialIssueWorker.perform_in(1.hour, issue.id) if issue.confidential?
+        TodosDestroyer::ConfidentialIssueWorker.perform_in(Todo::WAIT_FOR_DELETE, issue.id) if issue.confidential?
         create_confidentiality_note(issue)
       end
 

app/services/members/base_service.rb

@@ -47,5 +47,11 @@ module Members
         raise "Unknown action '#{action}' on #{member}!"
       end
     end
+
+    def enqueue_delete_todos(member)
+      type = member.is_a?(GroupMember) ? 'Group' : 'Project'
+      # don't enqueue immediately to prevent todos removal in case of a mistake
+      TodosDestroyer::EntityLeaveWorker.perform_in(Todo::WAIT_FOR_DELETE, member.user_id, member.source_id, type)
+    end
   end
 end

app/services/members/destroy_service.rb

@@ -15,7 +15,7 @@ module Members
         notification_service.decline_access_request(member)
       end
 
-      enqeue_delete_todos(member)
+      enqueue_delete_todos(member)
 
       after_execute(member: member)
 
@@ -24,12 +24,6 @@ module Members
 
     private
 
-    def enqeue_delete_todos(member)
-      type = member.is_a?(GroupMember) ? 'Group' : 'Project'
-      # don't enqueue immediately to prevent todos removal in case of a mistake
-      TodosDestroyer::EntityLeaveWorker.perform_in(1.hour, member.user_id, member.source_id, type)
-    end
-
     def can_destroy_member?(member)
       can?(current_user, destroy_member_permission(member), member)
     end

app/services/members/update_service.rb

@@ -10,9 +10,18 @@ module Members
 
       if member.update(params)
         after_execute(action: permission, old_access_level: old_access_level, member: member)
+
+        # Deletes only confidential issues todos for guests
+        enqueue_delete_todos(member) if downgrading_to_guest?
       end
 
       member
     end
+
+    private
+
+    def downgrading_to_guest?
+      params[:access_level] == Gitlab::Access::GUEST
+    end
   end
 end

app/services/projects/update_service.rb

@@ -61,9 +61,9 @@ module Projects
 
       if project.previous_changes.include?(:visibility_level) && project.private?
         # don't enqueue immediately to prevent todos removal in case of a mistake
-        TodosDestroyer::ProjectPrivateWorker.perform_in(1.hour, project.id)
+        TodosDestroyer::ProjectPrivateWorker.perform_in(Todo::WAIT_FOR_DELETE, project.id)
       elsif (project_changed_feature_keys & todos_features_changes).present?
-        TodosDestroyer::PrivateFeaturesWorker.perform_in(1.hour, project.id)
+        TodosDestroyer::PrivateFeaturesWorker.perform_in(Todo::WAIT_FOR_DELETE, project.id)
       end
 
       if project.previous_changes.include?('path')

changelogs/unreleased/54427-label-xss.yml

+---
+title: Escape html entities in LabelReferenceFilter when no label found
+merge_request:
+author:
+type: security

changelogs/unreleased/security-master-secret-ci-variables-exposed.yml

+---
+title: Prevent leaking protected variables for ambiguous refs.
+merge_request:
+author:
+type: security

changelogs/unreleased/security-master-url-rel.yml

+---
+title: Set URL rel attribute for broken URLs.
+merge_request:
+author:
+type: security

changelogs/unreleased/security-todos_not_redacted_for_guests.yml

+---
+title: Delete confidential todos for user when downgraded to Guest
+merge_request:
+author:
+type: security

doc/workflow/todos.md

@@ -35,6 +35,9 @@ A Todo appears in your Todos dashboard when:
   - the author, or
   - have set it to automatically merge once pipeline succeeds.
 
+NOTE: **Note:**
+When an user no longer has access to a resource related to a Todo like an issue, merge request, project or group the related Todos, for security reasons, gets deleted within the next hour. The delete is delayed to prevent data loss in case user got their access revoked by mistake.
+
 ### Directly addressed Todos
 
 > [Introduced][ce-7926] in GitLab 9.0.

lib/banzai/filter/external_link_filter.rb

@@ -9,11 +9,10 @@ module Banzai
       def call
         links.each do |node|
           uri = uri(node['href'].to_s)
-          next unless uri
 
-          node.set_attribute('href', uri.to_s)
+          node.set_attribute('href', uri.to_s) if uri
 
-          if SCHEMES.include?(uri.scheme) && external_url?(uri)
+          if SCHEMES.include?(uri&.scheme) && !internal_url?(uri)
             node.set_attribute('rel', 'nofollow noreferrer noopener')
             node.set_attribute('target', '_blank')
           end
@@ -35,11 +34,12 @@ module Banzai
         doc.xpath(query)
       end
 
-      def external_url?(uri)
+      def internal_url?(uri)
+        return false if uri.nil?
         # Relative URLs miss a hostname
-        return false unless uri.hostname
+        return true unless uri.hostname
 
-        uri.hostname != internal_url.hostname
+        uri.hostname == internal_url.hostname
       end
 
       def internal_url

lib/banzai/filter/label_reference_filter.rb

@@ -29,7 +29,7 @@ module Banzai
           if label
             yield match, label.id, project, namespace, $~
           else
-            match
+            escape_html_entities(match)
           end
         end
       end
@@ -102,6 +102,10 @@ module Banzai
         CGI.unescapeHTML(text.to_s)
       end
 
+      def escape_html_entities(text)
+        CGI.escapeHTML(text.to_s)
+      end
+
       def object_link_title(object, matches)
         # use title of wrapped element instead
         nil

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

@@ -54,7 +54,13 @@ module Gitlab
 
           def protected_ref?
             strong_memoize(:protected_ref) do
-              project.protected_for?(ref)
+              project.protected_for?(origin_ref)
+            end
+          end
+
+          def ambiguous_ref?
+            strong_memoize(:ambiguous_ref) do
+              project.repository.ambiguous_ref?(origin_ref)
             end
           end
         end

lib/gitlab/ci/pipeline/chain/validate/repository.rb

@@ -16,6 +16,10 @@ module Gitlab
               unless @command.sha
                 return error('Commit not found')
               end
+
+              if @command.ambiguous_ref?
+                return error('Ref is ambiguous')
+              end
             end
 
             def break?

lib/gitlab/git.rb

@@ -54,11 +54,11 @@ module Gitlab
       end
 
       def tag_ref?(ref)
-        ref.start_with?(TAG_REF_PREFIX)
+        ref =~ /^#{TAG_REF_PREFIX}.+/
       end
 
       def branch_ref?(ref)
-        ref.start_with?(BRANCH_REF_PREFIX)
+        ref =~ /^#{BRANCH_REF_PREFIX}.+/
       end
 
       def blank_ref?(ref)

spec/lib/banzai/filter/external_link_filter_spec.rb

@@ -49,16 +49,16 @@ describe Banzai::Filter::ExternalLinkFilter do
   end
 
   context 'for invalid urls' do
-    it 'skips broken hrefs' do
+    it 'adds rel and target attributes to broken hrefs' do
       doc = filter %q(<p><a href="don't crash on broken urls">Google</a></p>)
-      expected = %q(<p><a href="don't%20crash%20on%20broken%20urls">Google</a></p>)
+      expected = %q(<p><a href="don't%20crash%20on%20broken%20urls" rel="nofollow noreferrer noopener" target="_blank">Google</a></p>)
 
       expect(doc.to_html).to eq(expected)
     end
 
-    it 'skips improperly formatted mailtos' do
+    it 'adds rel and target to improperly formatted mailtos' do
       doc = filter %q(<p><a href="mailto://jblogs@example.com">Email</a></p>)
-      expected = %q(<p><a href="mailto://jblogs@example.com">Email</a></p>)
+      expected = %q(<p><a href="mailto://jblogs@example.com" rel="nofollow noreferrer noopener" target="_blank">Email</a></p>)
 
       expect(doc.to_html).to eq(expected)
     end

spec/lib/banzai/filter/label_reference_filter_spec.rb

@@ -236,6 +236,24 @@ describe Banzai::Filter::LabelReferenceFilter do
     end
   end
 
+  context 'References with html entities' do
+    let!(:label)     { create(:label, name: '<html>', project: project) }
+
+    it 'links to a valid reference' do
+      doc = reference_filter('See ~"<html>"')
+
+      expect(doc.css('a').first.attr('href')).to eq urls
+        .project_issues_url(project, label_name: label.name)
+      expect(doc.text).to eq 'See <html>'
+    end
+
+    it 'ignores invalid label names and escapes entities' do
+      act = %(Label #{Label.reference_prefix}"&lt;non valid&gt;")
+
+      expect(reference_filter(act).to_html).to eq act
+    end
+  end
+
   describe 'consecutive references' do
     let(:bug) { create(:label, name: 'bug', project: project) }
     let(:feature_proposal) { create(:label, name: 'feature proposal', project: project) }

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

@@ -182,4 +182,24 @@ describe Gitlab::Ci::Pipeline::Chain::Command do
       it { is_expected.to eq(false) }
     end
   end
+
+  describe '#ambiguous_ref' do
+    let(:project) { create(:project, :repository) }
+    let(:command) { described_class.new(project: project, origin_ref: 'ref') }
+
+    subject { command.ambiguous_ref? }
+
+    context 'when ref is not ambiguous' do
+      it { is_expected. to eq(false) }
+    end
+
+    context 'when ref is ambiguous' do
+      before do
+        project.repository.add_tag(project.creator, 'ref', 'master')
+        project.repository.add_branch(project.creator, 'ref', 'master')
+      end
+
+      it { is_expected. to eq(true) }
+    end
+  end
 end

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
 

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
+      create(:project, :repository).tap do |proj|
+        proj.repository.add_tag(user, 'master', 'master')
+      end
+    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(

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

 require 'spec_helper'
 
 describe Gitlab::Ci::Pipeline::Seed::Build do
-  let(:pipeline) { create(:ci_empty_pipeline) }
+  let(:project) { create(:project, :repository) }
+  let(:pipeline) { create(:ci_empty_pipeline, project: project) }
 
   let(:attributes) do
     { name: 'rspec',

spec/lib/gitlab/ci/pipeline/seed/stage_spec.rb

 require 'spec_helper'
 
 describe Gitlab::Ci::Pipeline::Seed::Stage do
-  let(:pipeline) { create(:ci_empty_pipeline) }
+  let(:project) { create(:project, :repository) }
+  let(:pipeline) { create(:ci_empty_pipeline, project: project) }
 
   let(:attributes) do
     { name: 'test',

spec/models/ci/build_spec.rb

@@ -2386,6 +2386,8 @@ describe Ci::Build do
     end
 
     context 'when protected variable is defined' do
+      let(:ref) { Gitlab::Git::BRANCH_REF_PREFIX + build.ref }
+
       let(:protected_variable) do
         { key: 'PROTECTED_KEY', value: 'protected_value', public: false }
       end
@@ -2398,7 +2400,7 @@ describe Ci::Build do
 
       context 'when the branch is protected' do
         before do
-          allow(build.project).to receive(:protected_for?).with(build.ref).and_return(true)
+          allow(build.project).to receive(:protected_for?).with(ref).and_return(true)
         end
 
         it { is_expected.to include(protected_variable) }
@@ -2406,7 +2408,7 @@ describe Ci::Build do
 
       context 'when the tag is protected' do
         before do
-          allow(build.project).to receive(:protected_for?).with(build.ref).and_return(true)
+          allow(build.project).to receive(:protected_for?).with(ref).and_return(true)
         end
 
         it { is_expected.to include(protected_variable) }
@@ -2431,6 +2433,8 @@ describe Ci::Build do
     end
 
     context 'when group protected variable is defined' do
+      let(:ref) { Gitlab::Git::BRANCH_REF_PREFIX + build.ref }
+
       let(:protected_variable) do
         { key: 'PROTECTED_KEY', value: 'protected_value', public: false }
       end
@@ -2443,7 +2447,7 @@ describe Ci::Build do
 
       context 'when the branch is protected' do
         before do
-          allow(build.project).to receive(:protected_for?).with(build.ref).and_return(true)
+          allow(build.project).to receive(:protected_for?).with(ref).and_return(true)
         end
 
         it { is_expected.to include(protected_variable) }
@@ -2451,7 +2455,7 @@ describe Ci::Build do
 
       context 'when the tag is protected' do
         before do
-          allow(build.project).to receive(:protected_for?).with(build.ref).and_return(true)
+          allow(build.project).to receive(:protected_for?).with(ref).and_return(true)
         end
 
         it { is_expected.to include(protected_variable) }

spec/models/ci/pipeline_spec.rb

@@ -397,6 +397,10 @@ describe Ci::Pipeline, :mailer do
   end
 
   describe '#protected_ref?' do
+    before do
+      pipeline.project = create(:project, :repository)
+    end
+
     it 'delegates method to project' do
       expect(pipeline).not_to be_protected_ref
     end

spec/models/concerns/has_ref_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe HasRef do
+  describe '#branch?' do
+    let(:build) { create(:ci_build) }
+
+    subject { build.branch? }
+
+    context 'is not a tag' do
+      before do
+        build.tag = false
+      end
+
+      it 'return true when tag is set to false' do
+        is_expected.to be_truthy
+      end
+    end
+
+    context 'is not a tag' do
+      before do
+        build.tag = true
+      end
+
+      it 'return false when tag is set to true' do
+        is_expected.to be_falsey
+      end
+    end
+  end
+
+  describe '#git_ref' do
+    subject { build.git_ref }
+
+    context 'when tag is true' do
+      let(:build) { create(:ci_build, tag: true) }
+
+      it 'returns a tag ref' do
+        is_expected.to start_with(Gitlab::Git::TAG_REF_PREFIX)
+      end
+    end
+
+    context 'when tag is false' do
+      let(:build) { create(:ci_build, tag: false) }
+
+      it 'returns a branch ref' do
+        is_expected.to start_with(Gitlab::Git::BRANCH_REF_PREFIX)
+      end
+    end
+
+    context 'when tag is nil' do
+      let(:build) { create(:ci_build, tag: nil) }
+
+      it 'returns a branch ref' do
+        is_expected.to start_with(Gitlab::Git::BRANCH_REF_PREFIX)
+      end
+    end
+  end
+end

spec/models/project_spec.rb

@@ -2550,6 +2550,10 @@ describe Project do
     end
 
     context 'when the ref is not protected' do
+      before do
+        allow(project).to receive(:protected_for?).with('ref').and_return(false)
+      end
+
       it 'contains only the CI variables' do
         is_expected.to contain_exactly(ci_variable)
       end
@@ -2589,42 +2593,139 @@ describe Project do
   end
 
   describe '#protected_for?' do
-    let(:project) { create(:project) }
+    let(:project) { create(:project, :repository) }
 
-    subject { project.protected_for?('ref') }
+    subject { project.protected_for?(ref) }
 
-    context 'when the ref is not protected' do
+    shared_examples 'ref is not protected' do
       before do
         stub_application_setting(
           default_branch_protection: Gitlab::Access::PROTECTION_NONE)
       end
 
       it 'returns false' do
-        is_expected.to be_falsey
+        is_expected.to be false
       end
     end
 
-    context 'when the ref is a protected branch' do
+    shared_examples 'ref is 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)
+        create(:protected_branch, name: 'master', project: project)
       end
 
       it 'returns true' do
-        is_expected.to be_truthy
+        is_expected.to be true
       end
     end
 
-    context 'when the ref is a protected tag' do
+    shared_examples 'ref is 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)
+        create(:protected_tag, name: 'v1.0.0', project: project)
       end
 
       it 'returns true' do
-        is_expected.to be_truthy
+        is_expected.to be true
+      end
+    end
+
+    context 'when ref is nil' do
+      let(:ref) { nil }
+
+      it 'returns false' do
+        is_expected.to be false
+      end
+    end
+
+    context 'when ref is ref name' do
+      context 'when ref is ambiguous' do
+        let(:ref) { 'ref' }
+
+        before do
+          project.repository.add_branch(project.creator, 'ref', 'master')
+          project.repository.add_tag(project.creator, 'ref', 'master')
+        end
+
+        it 'raises an error' do
+          expect { subject }.to raise_error(Repository::AmbiguousRefError)
+        end
+      end
+
+      context 'when the ref is not protected' do
+        let(:ref) { 'master' }
+
+        it_behaves_like 'ref is not protected'
+      end
+
+      context 'when the ref is a protected branch' do
+        let(:ref) { 'master' }
+
+        it_behaves_like 'ref is protected branch'
+      end
+
+      context 'when the ref is a protected tag' do
+        let(:ref) { 'v1.0.0' }
+
+        it_behaves_like 'ref is protected tag'
+      end
+
+      context 'when ref does not exist' do
+        let(:ref) { 'something' }
+
+        it 'returns false' do
+          is_expected.to be false
+        end
+      end
+    end
+
+    context 'when ref is full ref' do
+      context 'when the ref is not protected' do
+        let(:ref) { 'refs/heads/master' }
+
+        it_behaves_like 'ref is not protected'
+      end
+
+      context 'when the ref is a protected branch' do
+        let(:ref) { 'refs/heads/master' }
+
+        it_behaves_like 'ref is protected branch'
+      end
+
+      context 'when the ref is a protected tag' do
+        let(:ref) { 'refs/tags/v1.0.0' }
+
+        it_behaves_like 'ref is protected tag'
+      end
+
+      context 'when branch ref name is a full tag ref' do
+        let(:ref) { 'refs/tags/something' }
+
+        before do
+          project.repository.add_branch(project.creator, ref, 'master')
+        end
+
+        context 'when ref is not protected' do
+          it 'returns false' do
+            is_expected.to be false
+          end
+        end
+
+        context 'when ref is a protected branch' do
+          before do
+            create(:protected_branch, name: 'refs/tags/something', project: project)
+          end
+
+          it 'returns true' do
+            is_expected.to be true
+          end
+        end
+      end
+
+      context 'when ref does not exist' do
+        let(:ref) { 'refs/heads/something' }
+
+        it 'returns false' do
+          is_expected.to be false
+        end
       end
     end
   end
@@ -2844,7 +2945,7 @@ describe Project do
 
     it 'shows full error updating an invalid MR' do
       error_message = 'Failed to replace merge_requests because one or more of the new records could not be saved.'\
-                      ' Validate fork Source project is not a fork of the target project'
+        ' Validate fork Source project is not a fork of the target project'
 
       expect { project.append_or_update_attribute(:merge_requests, [create(:merge_request)]) }
         .to raise_error(ActiveRecord::RecordNotSaved, error_message)

spec/models/repository_spec.rb

@@ -1005,6 +1005,67 @@ describe Repository do
     end
   end
 
+  describe '#ambiguous_ref?' do
+    let(:ref) { 'ref' }
+
+    subject { repository.ambiguous_ref?(ref) }
+
+    context 'when ref is ambiguous' do
+      before do
+        repository.add_tag(project.creator, ref, 'master')
+        repository.add_branch(project.creator, ref, 'master')
+      end
+
+      it 'should be true' do
+        is_expected.to eq(true)
+      end
+    end
+
+    context 'when ref is not ambiguous' do
+      before do
+        repository.add_tag(project.creator, ref, 'master')
+      end
+
+      it 'should be false' do
+        is_expected.to eq(false)
+      end
+    end
+  end
+
+  describe '#expand_ref' do
+    let(:ref) { 'ref' }
+
+    subject { repository.expand_ref(ref) }
+
+    context 'when ref is not tag or branch name' do
+      let(:ref) { 'refs/heads/master' }
+
+      it 'returns nil' do
+        is_expected.to eq(nil)
+      end
+    end
+
+    context 'when ref is tag name' do
+      before do
+        repository.add_tag(project.creator, ref, 'master')
+      end
+
+      it 'returns the tag ref' do
+        is_expected.to eq("refs/tags/#{ref}")
+      end
+    end
+
+    context 'when ref is branch name' do
+      before do
+        repository.add_branch(project.creator, ref, 'master')
+      end
+
+      it 'returns the branch ref' do
+        is_expected.to eq("refs/heads/#{ref}")
+      end
+    end
+  end
+
   describe '#add_branch' do
     let(:branch_name) { 'new_feature' }
     let(:target) { 'master' }

spec/services/ci/create_pipeline_service_spec.rb

@@ -667,7 +667,7 @@ describe Ci::CreatePipelineService do
         stub_ci_pipeline_yaml_file(YAML.dump(config))
       end
 
-      let(:ref_name) { 'feature' }
+      let(:ref_name) { 'refs/heads/feature' }
 
       context 'when source is merge request' do
         let(:source) { :merge_request }
@@ -696,7 +696,7 @@ describe Ci::CreatePipelineService do
             let(:merge_request) do
               create(:merge_request,
                 source_project: project,
-                source_branch: ref_name,
+                source_branch: Gitlab::Git.ref_name(ref_name),
                 target_project: project,
                 target_branch: 'master')
             end
@@ -709,7 +709,7 @@ describe Ci::CreatePipelineService do
             end
 
             context 'when ref is tag' do
-              let(:ref_name) { 'v1.1.0' }
+              let(:ref_name) { 'refs/tags/v1.1.0' }
 
               it 'does not create a merge request pipeline' do
                 expect(pipeline).not_to be_persisted
@@ -721,7 +721,7 @@ describe Ci::CreatePipelineService do
               let(:merge_request) do
                 create(:merge_request,
                   source_project: project,
-                  source_branch: ref_name,
+                  source_branch: Gitlab::Git.ref_name(ref_name),
                   target_project: target_project,
                   target_branch: 'master')
               end
@@ -786,7 +786,7 @@ describe Ci::CreatePipelineService do
             let(:merge_request) do
               create(:merge_request,
                 source_project: project,
-                source_branch: ref_name,
+                source_branch: Gitlab::Git.ref_name(ref_name),
                 target_project: project,
                 target_branch: 'master')
             end
@@ -928,7 +928,7 @@ describe Ci::CreatePipelineService do
             let(:merge_request) do
               create(:merge_request,
                 source_project: project,
-                source_branch: ref_name,
+                source_branch: Gitlab::Git.ref_name(ref_name),
                 target_project: project,
                 target_branch: 'master')
             end

spec/services/groups/update_service_spec.rb

@@ -56,7 +56,7 @@ describe Groups::UpdateService do
           create(:project, :private, group: internal_group)
 
           expect(TodosDestroyer::GroupPrivateWorker).to receive(:perform_in)
-            .with(1.hour, internal_group.id)
+            .with(Todo::WAIT_FOR_DELETE, internal_group.id)
         end
 
         it "changes permission level to private" do

spec/services/issues/update_service_spec.rb

@@ -77,7 +77,7 @@ describe Issues::UpdateService, :mailer do
       end
 
       it 'enqueues ConfidentialIssueWorker when an issue is made confidential' do
-        expect(TodosDestroyer::ConfidentialIssueWorker).to receive(:perform_in).with(1.hour, issue.id)
+        expect(TodosDestroyer::ConfidentialIssueWorker).to receive(:perform_in).with(Todo::WAIT_FOR_DELETE, issue.id)
 
         update_issue(confidential: true)
       end

spec/services/members/destroy_service_spec.rb

@@ -22,7 +22,7 @@ describe Members::DestroyService do
   shared_examples 'a service destroying a member' do
     before do
       type = member.is_a?(GroupMember) ? 'Group' : 'Project'
-      expect(TodosDestroyer::EntityLeaveWorker).to receive(:perform_in).with(1.hour, member.user_id, member.source_id, type)
+      expect(TodosDestroyer::EntityLeaveWorker).to receive(:perform_in).with(Todo::WAIT_FOR_DELETE, member.user_id, member.source_id, type)
     end
 
     it 'destroys the member' do

spec/services/members/update_service_spec.rb

@@ -20,11 +20,28 @@ describe Members::UpdateService do
 
   shared_examples 'a service updating a member' do
     it 'updates the member' do
+      expect(TodosDestroyer::EntityLeaveWorker).not_to receive(:perform_in).with(Todo::WAIT_FOR_DELETE, member.user_id, member.source_id, source.class.name)
+
       updated_member = described_class.new(current_user, params).execute(member, permission: permission)
 
       expect(updated_member).to be_valid
       expect(updated_member.access_level).to eq(Gitlab::Access::MAINTAINER)
     end
+
+    context 'when member is downgraded to guest' do
+      let(:params) do
+        { access_level: Gitlab::Access::GUEST }
+      end
+
+      it 'schedules to delete confidential todos' do
+        expect(TodosDestroyer::EntityLeaveWorker).to receive(:perform_in).with(Todo::WAIT_FOR_DELETE, member.user_id, member.source_id, source.class.name).once
+
+        updated_member = described_class.new(current_user, params).execute(member, permission: permission)
+
+        expect(updated_member).to be_valid
+        expect(updated_member.access_level).to eq(Gitlab::Access::GUEST)
+      end
+    end
   end
 
   before do

spec/services/projects/update_service_spec.rb

@@ -41,7 +41,7 @@ describe Projects::UpdateService do
         end
 
         it 'updates the project to private' do
-          expect(TodosDestroyer::ProjectPrivateWorker).to receive(:perform_in).with(1.hour, project.id)
+          expect(TodosDestroyer::ProjectPrivateWorker).to receive(:perform_in).with(Todo::WAIT_FOR_DELETE, project.id)
 
           result = update_project(project, user, visibility_level: Gitlab::VisibilityLevel::PRIVATE)
 
@@ -191,7 +191,7 @@ describe Projects::UpdateService do
     context 'when changing feature visibility to private' do
       it 'updates the visibility correctly' do
         expect(TodosDestroyer::PrivateFeaturesWorker)
-          .to receive(:perform_in).with(1.hour, project.id)
+          .to receive(:perform_in).with(Todo::WAIT_FOR_DELETE, project.id)
 
         result = update_project(project, user, project_feature_attributes:
                                  { issues_access_level: ProjectFeature::PRIVATE }