gitlab Debian release 11.5.6+dfsg-1

-----BEGIN PGP SIGNATURE-----
 
 iQIzBAABCAAdFiEEKnl0ri/BUtd4Z9pKzh+cZ0USwioFAlwt4YYACgkQzh+cZ0US
 wirP9w//XswmG+b9/mylMXdcCleRYbfZhHCMKqm/iha+REWc3pXgkRw6vE9uvSRX
 ghjL18sjTnPRVJMVkkSrdPUtWbtnh/xOLveBDa9/yIavDVhhYP6DWh4gx0NzT6e1
 eyDuwxGmDiubVQacq9URbFBYfDlgoP72o+kFMbxR68rahctHbOThKB2yK5Oww8gw
 /vdehTXTuG8IOVPaAB8zBeZNGa8dSky+s+hFLC/4zotjOr3if+zegPwoGnd58KwH
 UNWosrs9zQ6oAobuygsy+Cw1EwfgCoO0KBKiKUkgo4zpgOmtSEf7VyJrPuKt0/oy
 5JOGp9Pvedbfdmu4jKxJC3NkN56iBWgE3LPwdkVyEphJAyVJCyU/rwcT9f74Y1PK
 SYJBpGUwCTNt67vL1ctEbDTQsi2pREOeTFMj6htHavDaN/HB7MTMqTlUl54QZ4cQ
 SEeiukARyDkuskpWWzW0FH6f5AenESySN5dPBM07ep9+UfIG2kMlMEXx1QUw+Bbp
 luJgFHzMIbfZ0NmQ0md/P7XWBQXm0jp4frEqw/4YR8zp6MoOpHn+OkA9s2/j99E0
 xG8mpk/gVXLY6mCpZf6jqQCIVRJ/uJ0871K4q4Wbg5A/q5B1TebY+3WNbewJLMgw
 cQcjyOutdcvO2EDk9g4ZYK0DjhPDVg/x6p3R1wAWxngT/LIHqX8=
 =ZIKa
 -----END PGP SIGNATURE-----

Merge tag 'debian/11.5.6+dfsg-1' into stretch-backports

gitlab Debian release 11.5.6+dfsg-1

.gitlab-ci.yml

@@ -614,7 +614,8 @@ docs lint:
     # Build HTML from Markdown
     - bundle exec nanoc
     # Check the internal links
-    - bundle exec nanoc check internal_links
+    # Disabled until https://gitlab.com/gitlab-com/gitlab-docs/issues/305 is resolved
+    # - bundle exec nanoc check internal_links
 
 downtime_check:
   <<: *rake-exec

CHANGELOG.md

@@ -2,6 +2,33 @@
 documentation](doc/development/changelog.md) for instructions on adding your own
 entry.
 
+## 11.5.6 (2018-12-28)
+
+### Security (17 changes)
+
+- Escape label and milestone titles to prevent XSS in GFM autocomplete. !2741
+- Validate LFS hrefs before downloading them.
+- Ensure that build token is only used when running.
+- Add subresources removal to member destroy service.
+- Prevent a path traversal attack on global file templates.
+- Allow changing group CI/CD settings only for owners.
+- Authorize before reading job information via API.
+- Prevent leaking protected variables for ambiguous refs.
+- Escape html entities in LabelReferenceFilter when no label found.
+- Prevent private snippets from being embeddable.
+- Issuable no longer is visible to users when project can't be viewed.
+- Don't expose cross project repositories through diffs when creating merge reqeusts.
+- Fix SSRF with import_url and remote mirror url.
+- Fix persistent symlink in project import.
+- Set URL rel attribute for broken URLs.
+- Project guests no longer are able to see refs page.
+- Delete confidential todos for user when downgraded to Guest.
+
+### Other (1 change)
+
+- Fix due date test. !23845
+
+
 ## 11.5.5 (2018-12-20)
 
 ### Security (1 change)

VERSION

@@ -1 +1 @@
-11.5.5
+11.5.6

app/assets/javascripts/gfm_auto_complete.js

@@ -244,7 +244,7 @@ class GfmAutoComplete {
       displayTpl(value) {
         let tmpl = GfmAutoComplete.Loading.template;
         if (value.title != null) {
-          tmpl = GfmAutoComplete.Milestones.template;
+          tmpl = GfmAutoComplete.Milestones.templateFunction(value.title);
         }
         return tmpl;
       },
@@ -311,7 +311,7 @@ class GfmAutoComplete {
       searchKey: 'search',
       data: GfmAutoComplete.defaultLoadingData,
       displayTpl(value) {
-        let tmpl = GfmAutoComplete.Labels.template;
+        let tmpl = GfmAutoComplete.Labels.templateFunction(value.color, value.title);
         if (GfmAutoComplete.isLoading(value)) {
           tmpl = GfmAutoComplete.Loading.template;
         }
@@ -576,9 +576,11 @@ GfmAutoComplete.Members = {
   },
 };
 GfmAutoComplete.Labels = {
-  template:
-    // eslint-disable-next-line no-template-curly-in-string
-    '<li><span class="dropdown-label-box" style="background: ${color}"></span> ${title}</li>',
+  templateFunction(color, title) {
+    return `<li><span class="dropdown-label-box" style="background: ${_.escape(
+      color,
+    )}"></span> ${_.escape(title)}</li>`;
+  },
 };
 // Issues, MergeRequests and Snippets
 GfmAutoComplete.Issues = {
@@ -588,8 +590,9 @@ GfmAutoComplete.Issues = {
 };
 // Milestones
 GfmAutoComplete.Milestones = {
-  // eslint-disable-next-line no-template-curly-in-string
-  template: '<li>${title}</li>',
+  templateFunction(title) {
+    return `<li>${_.escape(title)}</li>`;
+  },
 };
 GfmAutoComplete.Loading = {
   template:

app/controllers/concerns/membership_actions.rb

@@ -35,7 +35,9 @@ module MembershipActions
 
     respond_to do |format|
       format.html do
-        message = "User was successfully removed from #{source_type}."
+        source = source_type == 'group' ? 'group and any subresources' : source_type
+
+        message = "User was successfully removed from #{source}."
         redirect_to members_page_url, notice: message
       end
 

app/controllers/groups/settings/ci_cd_controller.rb

@@ -4,7 +4,7 @@ module Groups
   module Settings
     class CiCdController < Groups::ApplicationController
       skip_cross_project_access_check :show
-      before_action :authorize_admin_pipeline!
+      before_action :authorize_admin_group!
 
       def show
         define_ci_variables
@@ -26,8 +26,8 @@ module Groups
           .map { |variable| variable.present(current_user: current_user) }
       end
 
-      def authorize_admin_pipeline!
-        return render_404 unless can?(current_user, :admin_pipeline, group)
+      def authorize_admin_group!
+        return render_404 unless can?(current_user, :admin_group, group)
       end
     end
   end

app/controllers/projects/snippets_controller.rb

@@ -75,7 +75,14 @@ class Projects::SnippetsController < Projects::ApplicationController
       format.json do
         render_blob_json(blob)
       end
-      format.js { render 'shared/snippets/show'}
+
+      format.js do
+        if @snippet.embeddable?
+          render 'shared/snippets/show'
+        else
+          head :not_found
+        end
+      end
     end
   end
 

app/controllers/projects_controller.rb

@@ -19,6 +19,7 @@ class ProjectsController < Projects::ApplicationController
   before_action :lfs_blob_ids, only: [:show], if: [:repo_exists?, :project_view_files?]
   before_action :project_export_enabled, only: [:export, :download_export, :remove_export, :generate_new_export]
   before_action :present_project, only: [:edit]
+  before_action :authorize_download_code!, only: [:refs]
 
   # Authorize
   before_action :authorize_admin_project!, only: [:edit, :update, :housekeeping, :download_export, :export, :remove_export, :generate_new_export]

app/controllers/snippets_controller.rb

@@ -80,7 +80,13 @@ class SnippetsController < ApplicationController
         render_blob_json(blob)
       end
 
-      format.js { render 'shared/snippets/show' }
+      format.js do
+        if @snippet.embeddable?
+          render 'shared/snippets/show'
+        else
+          head :not_found
+        end
+      end
     end
   end
 

app/helpers/members_helper.rb

@@ -18,12 +18,13 @@ module MembersHelper
         "remove #{member.user.name} from"
       end
 
-    "#{text} #{action} the #{member.source.human_name} #{member.real_source_type.humanize(capitalize: false)}?"
+    "#{text} #{action} the #{member.source.human_name} #{source_text(member)}?"
   end
 
   def remove_member_title(member)
     action = member.request? ? 'Deny access request' : 'Remove user'
-    "#{action} from #{member.real_source_type.humanize(capitalize: false)}"
+
+    "#{action} from #{source_text(member)}"
   end
 
   def leave_confirmation_message(member_source)
@@ -35,4 +36,14 @@ module MembersHelper
     options = params.slice(:search, :sort).merge(options)
     "#{request.path}?#{options.to_param}"
   end
+
+  private
+
+  def source_text(member)
+    type = member.real_source_type.humanize(capitalize: false)
+
+    return type if member.request? || member.invite? || type != 'group'
+
+    'group and any subresources'
+  end
 end

app/helpers/snippets_helper.rb

@@ -130,12 +130,4 @@ module SnippetsHelper
 
     link_to external_snippet_icon('download'), download_url, class: 'btn', target: '_blank', title: 'Download', rel: 'noopener noreferrer'
   end
-
-  def public_snippet?
-    if @snippet.project_id?
-      can?(nil, :read_project_snippet, @snippet)
-    else
-      can?(nil, :read_personal_snippet, @snippet)
-    end
-  end
 end

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
@@ -152,6 +153,10 @@ module Ci
           .execute(build)
         # rubocop: enable CodeReuse/ServiceClass
       end
+
+      def find_running_by_token(token)
+        running.find_by_token(token)
+      end
     end
 
     state_machine :status do
@@ -638,11 +643,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: :pipelines
     belongs_to :user
@@ -374,10 +375,6 @@ module Ci
       @commit ||= Commit.lazy(project, sha)
     end
 
-    def branch?
-      !tag?
-    end
-
     def stuck?
       pending_builds.any?(&:stuck?)
     end
@@ -577,7 +574,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
@@ -697,16 +694,6 @@ module Ci
       end
     end
 
-    def git_ref
-      if branch?
-        Gitlab::Git::BRANCH_REF_PREFIX + ref.to_s
-      elsif tag?
-        Gitlab::Git::TAG_REF_PREFIX + ref.to_s
-      else
-        raise ArgumentError, 'Invalid pipeline type!'
-      end
-    end
-
     def latest_builds_status
       return 'failed' unless yaml_errors.blank?
 

app/models/concerns/has_ref.rb

@@ -0,0 +1,17 @@
+# 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/member.rb

@@ -76,6 +76,7 @@ class Member < ActiveRecord::Base
   scope :owners,  -> { active.where(access_level: OWNER) }
   scope :owners_and_maintainers,  -> { active.where(access_level: [OWNER, MAINTAINER]) }
   scope :owners_and_masters,  -> { owners_and_maintainers } # @deprecated
+  scope :with_user, -> (user) { where(user: user) }
 
   scope :order_name_asc, -> { left_join_users.reorder(Gitlab::Database.nulls_last_order('users.name', 'ASC')) }
   scope :order_name_desc, -> { left_join_users.reorder(Gitlab::Database.nulls_last_order('users.name', 'DESC')) }

app/models/members/group_member.rb

@@ -12,6 +12,8 @@ class GroupMember < Member
   validates :source_type, format: { with: /\ANamespace\z/ }
   default_scope { where(source_type: SOURCE_TYPE) }
 
+  scope :in_groups, ->(groups) { where(source_id: groups.select(:id)) }
+
   after_create :update_two_factor_requirement, unless: :invite?
   after_destroy :update_two_factor_requirement, unless: :invite?
 

app/models/members/project_member.rb

@@ -14,6 +14,10 @@ class ProjectMember < Member
   default_scope { where(source_type: SOURCE_TYPE) }
 
   scope :in_project, ->(project) { where(source_id: project.id) }
+  scope :in_namespaces, ->(groups) do
+    joins('INNER JOIN projects ON projects.id = members.source_id')
+      .where('projects.namespace_id in (?)', groups.select(:id))
+  end
 
   class << self
     # Add users to projects with passed access option

app/models/project.rb

@@ -300,10 +300,9 @@ class Project < ActiveRecord::Base
 
   validates :namespace, presence: true
   validates :name, uniqueness: { scope: :namespace_id }
-  validates :import_url, url: { protocols: ->(project) { project.persisted? ? VALID_MIRROR_PROTOCOLS : VALID_IMPORT_PROTOCOLS },
-                                ports: ->(project) { project.persisted? ? VALID_MIRROR_PORTS : VALID_IMPORT_PORTS },
-                                allow_localhost: false,
-                                enforce_user: true }, if: [:external_import?, :import_url_changed?]
+  validates :import_url, public_url: { protocols: ->(project) { project.persisted? ? VALID_MIRROR_PROTOCOLS : VALID_IMPORT_PROTOCOLS },
+                                       ports: ->(project) { project.persisted? ? VALID_MIRROR_PORTS : VALID_IMPORT_PORTS },
+                                       enforce_user: true }, if: [:external_import?, :import_url_changed?]
   validates :star_count, numericality: { greater_than_or_equal_to: 0 }
   validate :check_limit, on: :create
   validate :check_repository_path_availability, on: :update, if: ->(project) { project.renamed? }
@@ -1818,10 +1817,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/remote_mirror.rb

@@ -18,7 +18,7 @@ class RemoteMirror < ActiveRecord::Base
 
   belongs_to :project, inverse_of: :remote_mirrors
 
-  validates :url, presence: true, url: { protocols: %w(ssh git http https), allow_blank: true, enforce_user: true }
+  validates :url, presence: true, public_url: { protocols: %w(ssh git http https), allow_blank: true, enforce_user: true }
 
   before_save :set_new_remote_name, if: :mirror_url_changed?
 

app/models/repository.rb

@@ -26,6 +26,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.
   #
@@ -176,6 +177,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/snippet.rb

@@ -175,6 +175,12 @@ class Snippet < ActiveRecord::Base
     :visibility_level
   end
 
+  def embeddable?
+    ability = project_id? ? :read_project_snippet : :read_personal_snippet
+
+    Ability.allowed?(nil, ability, self)
+  end
+
   def notes_with_associations
     notes.includes(:author)
   end

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/policies/issuable_policy.rb

@@ -11,7 +11,7 @@ class IssuablePolicy < BasePolicy
     @user && @subject.assignee_or_author?(@user)
   end
 
-  rule { assignee_or_author }.policy do
+  rule { can?(:guest_access) & assignee_or_author }.policy do
     enable :read_issue
     enable :update_issue
     enable :reopen_issue

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

@@ -38,7 +38,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

@@ -2,9 +2,11 @@
 
 module Members
   class DestroyService < Members::BaseService
-    def execute(member, skip_authorization: false)
+    def execute(member, skip_authorization: false, skip_subresources: false)
       raise Gitlab::Access::AccessDeniedError unless skip_authorization || can_destroy_member?(member)
 
+      @skip_auth = skip_authorization
+
       return member if member.is_a?(GroupMember) && member.source.last_owner?(member.user)
 
       member.destroy
@@ -15,7 +17,8 @@ module Members
         notification_service.decline_access_request(member)
       end
 
-      enqeue_delete_todos(member)
+      delete_subresources(member) unless skip_subresources
+      enqueue_delete_todos(member)
 
       after_execute(member: member)
 
@@ -24,7 +27,30 @@ module Members
 
     private
 
-    def enqeue_delete_todos(member)
+    def delete_subresources(member)
+      return unless member.is_a?(GroupMember) && member.user && member.group
+
+      delete_project_members(member)
+      delete_subgroup_members(member) if Group.supports_nested_groups?
+    end
+
+    def delete_project_members(member)
+      groups = member.group.self_and_descendants
+
+      ProjectMember.in_namespaces(groups).with_user(member.user).each do |project_member|
+        self.class.new(current_user).execute(project_member, skip_authorization: @skip_auth)
+      end
+    end
+
+    def delete_subgroup_members(member)
+      groups = member.group.descendants
+
+      GroupMember.in_groups(groups).with_user(member.user).each do |group_member|
+        self.class.new(current_user).execute(group_member, skip_authorization: @skip_auth, skip_subresources: true)
+      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(1.hour, member.user_id, member.source_id, type)

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/merge_requests/build_service.rb

@@ -17,7 +17,7 @@ module MergeRequests
       merge_request.source_project  = find_source_project
       merge_request.target_project  = find_target_project
       merge_request.target_branch   = find_target_branch
-      merge_request.can_be_created  = branches_valid?
+      merge_request.can_be_created  = projects_and_branches_valid?
 
       # compare branches only if branches are valid, otherwise
       # compare_branches may raise an error
@@ -48,15 +48,19 @@ module MergeRequests
              to: :merge_request
 
     def find_source_project
-      return source_project if source_project.present? && can?(current_user, :read_project, source_project)
+      return source_project if source_project.present? && can?(current_user, :create_merge_request_from, source_project)
 
       project
     end
 
     def find_target_project
-      return target_project if target_project.present? && can?(current_user, :read_project, target_project)
+      return target_project if target_project.present? && can?(current_user, :create_merge_request_in, target_project)
 
-      project.default_merge_request_target
+      target_project = project.default_merge_request_target
+
+      return target_project if target_project.present? && can?(current_user, :create_merge_request_in, target_project)
+
+      project
     end
 
     def find_target_branch
@@ -71,10 +75,11 @@ module MergeRequests
       params[:target_branch].present?
     end
 
-    def branches_valid?
+    def projects_and_branches_valid?
+      return false if source_project.nil? || target_project.nil?
       return false unless source_branch_specified? || target_branch_specified?
 
-      validate_branches
+      validate_projects_and_branches
       errors.blank?
     end
 
@@ -93,7 +98,12 @@ module MergeRequests
       end
     end
 
-    def validate_branches
+    def validate_projects_and_branches
+      merge_request.validate_target_project
+      merge_request.validate_fork
+
+      return if errors.any?
+
       add_error('You must select source and target branch') unless branches_present?
       add_error('You must select different branches') if same_source_and_target?
       add_error("Source branch \"#{source_branch}\" does not exist") unless source_branch_exists?

app/services/projects/lfs_pointers/lfs_download_service.rb

@@ -12,28 +12,43 @@ module Projects
 
         return if LfsObject.exists?(oid: oid)
 
-        sanitized_uri = Gitlab::UrlSanitizer.new(url)
-        Gitlab::UrlBlocker.validate!(sanitized_uri.sanitized_url, protocols: VALID_PROTOCOLS)
+        sanitized_uri = sanitize_url!(url)
 
         with_tmp_file(oid) do |file|
-          size = download_and_save_file(file, sanitized_uri)
-          lfs_object = LfsObject.new(oid: oid, size: size, file: file)
+          download_and_save_file(file, sanitized_uri)
+          lfs_object = LfsObject.new(oid: oid, size: file.size, file: file)
 
           project.all_lfs_objects << lfs_object
         end
+      rescue Gitlab::UrlBlocker::BlockedUrlError => e
+        Rails.logger.error("LFS file with oid #{oid} couldn't be downloaded: #{e.message}")
       rescue StandardError => e
-        Rails.logger.error("LFS file with oid #{oid} could't be downloaded from #{sanitized_uri.sanitized_url}: #{e.message}")
+        Rails.logger.error("LFS file with oid #{oid} couldn't be downloaded from #{sanitized_uri.sanitized_url}: #{e.message}")
       end
       # rubocop: enable CodeReuse/ActiveRecord
 
       private
 
+      def sanitize_url!(url)
+        Gitlab::UrlSanitizer.new(url).tap do |sanitized_uri|
+          # Just validate that HTTP/HTTPS protocols are used. The
+          # subsequent Gitlab::HTTP.get call will do network checks
+          # based on the settings.
+          Gitlab::UrlBlocker.validate!(sanitized_uri.sanitized_url,
+                                       protocols: VALID_PROTOCOLS)
+        end
+      end
+
       def download_and_save_file(file, sanitized_uri)
-        IO.copy_stream(open(sanitized_uri.sanitized_url, headers(sanitized_uri)), file) # rubocop:disable Security/Open
+        response = Gitlab::HTTP.get(sanitized_uri.sanitized_url, headers(sanitized_uri)) do |fragment|
+          file.write(fragment)
+        end
+
+        raise StandardError, "Received error code #{response.code}" unless response.success?
       end
 
       def headers(sanitized_uri)
-        {}.tap do |headers|
+        query_options.tap do |headers|
           credentials = sanitized_uri.credentials
 
           if credentials[:user].present? || credentials[:password].present?
@@ -43,10 +58,14 @@ module Projects
         end
       end
 
+      def query_options
+        { stream_body: true }
+      end
+
       def with_tmp_file(oid)
         create_tmp_storage_dir
 
-        File.open(File.join(tmp_storage_dir, oid), 'w') { |file| yield file }
+        File.open(File.join(tmp_storage_dir, oid), 'wb') { |file| yield file }
       end
 
       def create_tmp_storage_dir

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')

app/views/shared/snippets/_header.html.haml

@@ -30,7 +30,7 @@
   - if @snippet.updated_at != @snippet.created_at
     = edited_time_ago_with_tooltip(@snippet, placement: 'bottom', html_class: 'snippet-edited-ago', exclude_author: true)
 
-  - if public_snippet?
+  - if @snippet.embeddable?
     .embed-snippet
       .input-group
         .input-group-prepend

debian/changelog

@@ -1,3 +1,14 @@
+gitlab (11.5.6+dfsg-1) unstable; urgency=high
+
+  * New upstream version 11.5.6+dfsg (Closes: #918086) (Fixes: CVE-2018-20488,
+    CVE-2018-20489, CVE-2018-20490, CVE-2018-20491, CVE-2018-20492,
+    CVE-2018-20493, CVE-2018-20494, CVE-2018-20495, CVE-2018-20496,
+    CVE-2018-20497, CVE-2018-20498, CVE-2018-20499, CVE-2018-20500,
+    CVE-2018-20501, CVE-2018-20507)
+  * Bump Standards-Version to 4.3.0
+
+ -- Sruthi Chandran <srud@disroot.org>  Thu, 03 Jan 2019 12:56:20 +0530
+
 gitlab (11.5.5+dfsg-1~bpo9+1) stretch-backports; urgency=medium
 
   * Rebuild for stretch-backports.

debian/control

@@ -7,7 +7,7 @@ Uploaders: Cédric Boutillier <boutil@debian.org>,
  Balasankar C <balasankarc@autistici.org>,
  Sruthi Chandran <srud@disroot.org>
 Build-Depends: debhelper (>= 10~), gem2deb, bc
-Standards-Version: 4.2.1
+Standards-Version: 4.3.0
 Vcs-Git: https://salsa.debian.org/ruby-team/gitlab.git
 Vcs-Browser: https://salsa.debian.org/ruby-team/gitlab
 Homepage: https://about.gitlab.com/

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/api/entities.rb

@@ -1346,7 +1346,17 @@ module API
       end
 
       class Dependency < Grape::Entity
-        expose :id, :name, :token
+        expose :id, :name
+        expose :token do |dependency, options|
+          # overrides the job's dependency authorization token
+          # with the token of the job that is being run
+          # this way we use the parent job auth token
+          #
+          # ideally we would change the runner implementation to
+          # use different token, but this would require upgrade of
+          # all runners which is impossible
+          options[:auth_token]
+        end
         expose :artifacts_file, using: JobArtifactFile, if: ->(job, _) { job.artifacts? }
       end
 
@@ -1374,7 +1384,10 @@ module API
         expose :artifacts, using: Artifacts
         expose :cache, using: Cache
         expose :credentials, using: Credentials
-        expose :dependencies, using: Dependency
+        expose :dependencies do |model|
+          Dependency.represent(model.dependencies,
+            options.merge(auth_token: model.token))
+        end
         expose :features
       end
     end

lib/api/helpers/runner.rb

@@ -36,26 +36,32 @@ module API
       def validate_job!(job)
         not_found! unless job
 
-        yield if block_given?
-
         project = job.project
-        forbidden!('Project has been deleted!') if project.nil? || project.pending_delete?
-        forbidden!('Job has been erased!') if job.erased?
+        job_forbidden!(job, 'Project has been deleted!') if project.nil? || project.pending_delete?
+        job_forbidden!(job, 'Job has been erased!') if job.erased?
+        job_forbidden!(job, 'Not running!') unless job.running?
       end
 
-      def authenticate_job!
-        job = Ci::Build.find_by_id(params[:id])
-
-        validate_job!(job) do
-          forbidden! unless job_token_valid?(job)
-        end
-
-        job
-      end
-
-      def job_token_valid?(job)
+      def authenticate_job_by_token!
         token = (params[JOB_TOKEN_PARAM] || env[JOB_TOKEN_HEADER]).to_s
-        token && job.valid_token?(token)
+
+        Ci::Build.find_by_token(token).tap do |job|
+          validate_job!(job)
+        end
+      end
+
+      # we look for a job that has ID and token matching
+      def authenticate_job!
+        authenticate_job_by_token!.tap do |job|
+          job_forbidden!(job, 'Invalid Job ID!') unless job.id == params[:id]
+        end
+      end
+
+      # we look for a job that has been shared via pipeline using the ID
+      def authenticate_pipeline_job!
+        job = authenticate_job_by_token!
+
+        job.pipeline.builds.find(params[:id])
       end
 
       def max_artifacts_size

lib/api/jobs.rb

@@ -38,6 +38,8 @@ module API
       end
       # rubocop: disable CodeReuse/ActiveRecord
       get ':id/jobs' do
+        authorize_read_builds!
+
         builds = user_project.builds.order('id DESC')
         builds = filter_builds(builds, params[:scope])
 
@@ -56,7 +58,10 @@ module API
       end
       # rubocop: disable CodeReuse/ActiveRecord
       get ':id/pipelines/:pipeline_id/jobs' do
+        authorize!(:read_pipeline, user_project)
         pipeline = user_project.pipelines.find(params[:pipeline_id])
+        authorize!(:read_build, pipeline)
+
         builds = pipeline.builds
         builds = filter_builds(builds, params[:scope])
         builds = builds.preload(:job_artifacts_archive, :job_artifacts, project: [:namespace])

lib/api/runner.rb

@@ -146,7 +146,6 @@ module API
       end
       put '/:id' do
         job = authenticate_job!
-        job_forbidden!(job, 'Job is not running') unless job.running?
 
         job.trace.set(params[:trace]) if params[:trace]
 
@@ -174,7 +173,6 @@ module API
       end
       patch '/:id/trace' do
         job = authenticate_job!
-        job_forbidden!(job, 'Job is not running') unless job.running?
 
         error!('400 Missing header Content-Range', 400) unless request.headers.key?('Content-Range')
         content_range = request.headers['Content-Range']
@@ -217,8 +215,7 @@ module API
         require_gitlab_workhorse!
         Gitlab::Workhorse.verify_api_request!(headers)
 
-        job = authenticate_job!
-        forbidden!('Job is not running') unless job.running?
+        authenticate_job!
 
         if params[:filesize]
           file_size = params[:filesize].to_i
@@ -261,7 +258,6 @@ module API
         require_gitlab_workhorse!
 
         job = authenticate_job!
-        forbidden!('Job is not running!') unless job.running?
 
         artifacts = UploadedFile.from_params(params, :file, JobArtifactUploader.workhorse_local_upload_path)
         metadata = UploadedFile.from_params(params, :metadata, JobArtifactUploader.workhorse_local_upload_path)
@@ -308,7 +304,7 @@ module API
         optional :direct_download, default: false, type: Boolean, desc: %q(Perform direct download from remote storage instead of proxying artifacts)
       end
       get '/:id/artifacts' do
-        job = authenticate_job!
+        job = authenticate_pipeline_job!
 
         present_carrierwave_file!(job.artifacts_file, supports_direct_download: params[:direct_download])
       end

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/auth.rb

@@ -296,7 +296,7 @@ module Gitlab
       private
 
       def find_build_by_token(token)
-        ::Ci::Build.running.find_by_token(token)
+        ::Ci::Build.find_running_by_token(token)
       end
     end
   end

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/controllers/groups/group_members_controller_spec.rb

@@ -118,7 +118,7 @@ describe Groups::GroupMembersController do
         it '[HTML] removes user from members' do
           delete :destroy, group_id: group, id: member
 
-          expect(response).to set_flash.to 'User was successfully removed from group.'
+          expect(response).to set_flash.to 'User was successfully removed from group and any subresources.'
           expect(response).to redirect_to(group_group_members_path(group))
           expect(group.members).not_to include member
         end

spec/controllers/groups/settings/ci_cd_controller_spec.rb

@@ -5,30 +5,65 @@ describe Groups::Settings::CiCdController do
   let(:user) { create(:user) }
 
   before do
-    group.add_maintainer(user)
     sign_in(user)
   end
 
   describe 'GET #show' do
-    it 'renders show with 200 status code' do
-      get :show, group_id: group
+    context 'when user is owner' do
+      before do
+        group.add_owner(user)
+      end
 
-      expect(response).to have_gitlab_http_status(200)
-      expect(response).to render_template(:show)
+      it 'renders show with 200 status code' do
+        get :show, group_id: group
+
+        expect(response).to have_gitlab_http_status(200)
+        expect(response).to render_template(:show)
+      end
+    end
+
+    context 'when user is not owner' do
+      before do
+        group.add_maintainer(user)
+      end
+
+      it 'renders a 404' do
+        get :show, group_id: group
+
+        expect(response).to have_gitlab_http_status(404)
+      end
     end
   end
 
   describe 'PUT #reset_registration_token' do
     subject { put :reset_registration_token, group_id: group }
 
-    it 'resets runner registration token' do
-      expect { subject }.to change { group.reload.runners_token }
+    context 'when user is owner' do
+      before do
+        group.add_owner(user)
+      end
+
+      it 'resets runner registration token' do
+        expect { subject }.to change { group.reload.runners_token }
+      end
+
+      it 'redirects the user to admin runners page' do
+        subject
+
+        expect(response).to redirect_to(group_settings_ci_cd_path)
+      end
     end
 
-    it 'redirects the user to admin runners page' do
-      subject
+    context 'when user is not owner' do
+      before do
+        group.add_maintainer(user)
+      end
 
-      expect(response).to redirect_to(group_settings_ci_cd_path)
+      it 'renders a 404' do
+        subject
+
+        expect(response).to have_gitlab_http_status(404)
+      end
     end
   end
 end

spec/controllers/projects/snippets_controller_spec.rb

@@ -371,6 +371,46 @@ describe Projects::SnippetsController do
     end
   end
 
+  describe "GET #show for embeddable content" do
+    let(:project_snippet) { create(:project_snippet, snippet_permission, project: project, author: user) }
+
+    before do
+      sign_in(user)
+
+      get :show, namespace_id: project.namespace, project_id: project, id: project_snippet.to_param, format: :js
+    end
+
+    context 'when snippet is private' do
+      let(:snippet_permission) { :private }
+
+      it 'responds with status 404' do
+        expect(response).to have_gitlab_http_status(404)
+      end
+    end
+
+    context 'when snippet is public' do
+      let(:snippet_permission) { :public }
+
+      it 'responds with status 200' do
+        expect(assigns(:snippet)).to eq(project_snippet)
+        expect(response).to have_gitlab_http_status(200)
+      end
+    end
+
+    context 'when the project is private' do
+      let(:project) { create(:project_empty_repo, :private) }
+
+      context 'when snippet is public' do
+        let(:project_snippet) { create(:project_snippet, :public, project: project, author: user) }
+
+        it 'responds with status 404' do
+          expect(assigns(:snippet)).to eq(project_snippet)
+          expect(response).to have_gitlab_http_status(404)
+        end
+      end
+    end
+  end
+
   describe 'GET #raw' do
     let(:project_snippet) do
       create(

spec/controllers/projects_controller_spec.rb

@@ -590,10 +590,10 @@ describe ProjectsController do
   end
 
   describe "GET refs" do
-    let(:public_project) { create(:project, :public, :repository) }
+    let(:project) { create(:project, :public, :repository) }
 
     it 'gets a list of branches and tags' do
-      get :refs, namespace_id: public_project.namespace, id: public_project, sort: 'updated_desc'
+      get :refs, namespace_id: project.namespace, id: project, sort: 'updated_desc'
 
       parsed_body = JSON.parse(response.body)
       expect(parsed_body['Branches']).to include('master')
@@ -603,7 +603,7 @@ describe ProjectsController do
     end
 
     it "gets a list of branches, tags and commits" do
-      get :refs, namespace_id: public_project.namespace, id: public_project, ref: "123456"
+      get :refs, namespace_id: project.namespace, id: project, ref: "123456"
 
       parsed_body = JSON.parse(response.body)
       expect(parsed_body["Branches"]).to include("master")
@@ -618,7 +618,7 @@ describe ProjectsController do
       end
 
       it "gets a list of branches, tags and commits" do
-        get :refs, namespace_id: public_project.namespace, id: public_project, ref: "123456"
+        get :refs, namespace_id: project.namespace, id: project, ref: "123456"
 
         parsed_body = JSON.parse(response.body)
         expect(parsed_body["Branches"]).to include("master")
@@ -626,6 +626,22 @@ describe ProjectsController do
         expect(parsed_body["Commits"]).to include("123456")
       end
     end
+
+    context 'when private project' do
+      let(:project) { create(:project, :repository) }
+
+      context 'as a guest' do
+        it 'renders forbidden' do
+          user = create(:user)
+          project.add_guest(user)
+
+          sign_in(user)
+          get :refs, namespace_id: project.namespace, id: project
+
+          expect(response).to have_gitlab_http_status(404)
+        end
+      end
+    end
   end
 
   describe 'POST #preview_markdown' do

spec/controllers/snippets_controller_spec.rb

@@ -80,6 +80,12 @@ describe SnippetsController do
             expect(assigns(:snippet)).to eq(personal_snippet)
             expect(response).to have_gitlab_http_status(200)
           end
+
+          it 'responds with status 404 when embeddable content is requested' do
+            get :show, id: personal_snippet.to_param, format: :js
+
+            expect(response).to have_gitlab_http_status(404)
+          end
         end
       end
 
@@ -106,6 +112,12 @@ describe SnippetsController do
           expect(assigns(:snippet)).to eq(personal_snippet)
           expect(response).to have_gitlab_http_status(200)
         end
+
+        it 'responds with status 404 when embeddable content is requested' do
+          get :show, id: personal_snippet.to_param, format: :js
+
+          expect(response).to have_gitlab_http_status(404)
+        end
       end
 
       context 'when not signed in' do
@@ -131,6 +143,13 @@ describe SnippetsController do
           expect(assigns(:snippet)).to eq(personal_snippet)
           expect(response).to have_gitlab_http_status(200)
         end
+
+        it 'responds with status 200 when embeddable content is requested' do
+          get :show, id: personal_snippet.to_param, format: :js
+
+          expect(assigns(:snippet)).to eq(personal_snippet)
+          expect(response).to have_gitlab_http_status(200)
+        end
       end
 
       context 'when not signed in' do

spec/features/group_variables_spec.rb

@@ -7,7 +7,7 @@ describe 'Group variables', :js do
   let(:page_path) { group_settings_ci_cd_path(group) }
 
   before do
-    group.add_maintainer(user)
+    group.add_owner(user)
     gitlab_sign_in(user)
 
     visit page_path

spec/features/issues/gfm_autocomplete_spec.rb

@@ -3,6 +3,8 @@ require 'rails_helper'
 describe 'GFM autocomplete', :js do
   let(:issue_xss_title) { 'This will execute alert<img src=x onerror=alert(2)&lt;img src=x onerror=alert(1)&gt;' }
   let(:user_xss_title) { 'eve <img src=x onerror=alert(2)&lt;img src=x onerror=alert(1)&gt;' }
+  let(:label_xss_title) { 'alert label &lt;img src=x onerror="alert(\'Hello xss\');" a'}
+  let(:milestone_xss_title) { 'alert milestone &lt;img src=x onerror="alert(\'Hello xss\');" a' }
 
   let(:user_xss) { create(:user, name: user_xss_title, username: 'xss.user') }
   let(:user)    { create(:user, name: '💃speciąl someone💃', username: 'someone.special') }
@@ -26,10 +28,14 @@ describe 'GFM autocomplete', :js do
 
     simulate_input('#issue-description', "@#{user.name[0...3]}")
 
+    wait_for_requests
+
     find('.atwho-view .cur').click
 
     click_button 'Save changes'
 
+    wait_for_requests
+
     expect(find('.description')).to have_content(user.to_reference)
   end
 
@@ -48,6 +54,8 @@ describe 'GFM autocomplete', :js do
       find('#note-body').native.send_keys('#')
     end
 
+    wait_for_requests
+
     expect(page).to have_selector('.atwho-container')
 
     page.within '.atwho-container #at-view-issues' do
@@ -60,6 +68,8 @@ describe 'GFM autocomplete', :js do
       find('#note-body').native.send_keys('@ev')
     end
 
+    wait_for_requests
+
     expect(page).to have_selector('.atwho-container')
 
     page.within '.atwho-container #at-view-users' do
@@ -67,6 +77,22 @@ describe 'GFM autocomplete', :js do
     end
   end
 
+  it 'opens autocomplete menu for Milestone when field starts with text with item escaping HTML characters' do
+    create(:milestone, project: project, title: milestone_xss_title)
+
+    page.within '.timeline-content-form' do
+      find('#note-body').native.send_keys('%')
+    end
+
+    wait_for_requests
+
+    expect(page).to have_selector('.atwho-container')
+
+    page.within '.atwho-container #at-view-milestones' do
+      expect(find('li').text).to have_content('alert milestone')
+    end
+  end
+
   it 'doesnt open autocomplete menu character is prefixed with text' do
     page.within '.timeline-content-form' do
       find('#note-body').native.send_keys('testing')
@@ -259,6 +285,21 @@ describe 'GFM autocomplete', :js do
     let!(:bug)              { create(:label, project: project, title: 'bug') }
     let!(:feature_proposal) { create(:label, project: project, title: 'feature proposal') }
 
+    it 'opens autocomplete menu for Labels when field starts with text with item escaping HTML characters' do
+      create(:label, project: project, title: label_xss_title)
+
+      note = find('#note-body')
+
+      # It should show all the labels on "~".
+      type(note, '~')
+
+      wait_for_requests
+
+      page.within '.atwho-container #at-view-labels' do
+        expect(find('.atwho-view-ul').text).to have_content('alert label')
+      end
+    end
+
     context 'when no labels are assigned' do
       it 'shows labels' do
         note = find('#note-body')

spec/features/merge_request/user_tries_to_access_private_repository_through_new_mr_spec.rb

@@ -0,0 +1,37 @@
+require 'spec_helper'
+
+describe 'Merge Request > Tries to access private repo of public project' do
+  let(:current_user) { create(:user) }
+  let(:private_project) do
+    create(:project, :public, :repository,
+           path: 'nothing-to-see-here',
+           name: 'nothing to see here',
+           repository_access_level: ProjectFeature::PRIVATE)
+  end
+  let(:owned_project) do
+    create(:project, :public, :repository,
+           namespace: current_user.namespace,
+           creator: current_user)
+  end
+
+  context 'when the user enters the querystring info for the other project' do
+    let(:mr_path) do
+      project_new_merge_request_diffs_path(
+        owned_project,
+        merge_request: {
+          source_project_id: private_project.id,
+          source_branch: 'feature'
+        }
+      )
+    end
+
+    before do
+      sign_in current_user
+      visit mr_path
+    end
+
+    it "does not mention the project the user can't see the repo of" do
+      expect(page).not_to have_content('nothing-to-see-here')
+    end
+  end
+end

spec/features/runners_spec.rb

@@ -259,8 +259,9 @@ describe 'Runners' do
 
   context 'group runners in group settings' do
     let(:group) { create(:group) }
+
     before do
-      group.add_maintainer(user)
+      group.add_owner(user)
     end
 
     context 'group with no runners' do

spec/helpers/members_helper_spec.rb

@@ -16,7 +16,7 @@ describe MembersHelper do
     it { expect(remove_member_message(project_member_invite)).to eq "Are you sure you want to revoke the invitation for #{project_member_invite.invite_email} to join the #{project.full_name} project?" }
     it { expect(remove_member_message(project_member_request)).to eq "Are you sure you want to deny #{requester.name}'s request to join the #{project.full_name} project?" }
     it { expect(remove_member_message(project_member_request, user: requester)).to eq "Are you sure you want to withdraw your access request for the #{project.full_name} project?" }
-    it { expect(remove_member_message(group_member)).to eq "Are you sure you want to remove #{group_member.user.name} from the #{group.name} group?" }
+    it { expect(remove_member_message(group_member)).to eq "Are you sure you want to remove #{group_member.user.name} from the #{group.name} group and any subresources?" }
     it { expect(remove_member_message(group_member_invite)).to eq "Are you sure you want to revoke the invitation for #{group_member_invite.invite_email} to join the #{group.name} group?" }
     it { expect(remove_member_message(group_member_request)).to eq "Are you sure you want to deny #{requester.name}'s request to join the #{group.name} group?" }
     it { expect(remove_member_message(group_member_request, user: requester)).to eq "Are you sure you want to withdraw your access request for the #{group.name} group?" }
@@ -33,7 +33,7 @@ describe MembersHelper do
 
     it { expect(remove_member_title(project_member)).to eq 'Remove user from project' }
     it { expect(remove_member_title(project_member_request)).to eq 'Deny access request from project' }
-    it { expect(remove_member_title(group_member)).to eq 'Remove user from group' }
+    it { expect(remove_member_title(group_member)).to eq 'Remove user from group and any subresources' }
     it { expect(remove_member_title(group_member_request)).to eq 'Deny access request from group' }
   end
 

spec/javascripts/boards/components/issue_due_date_spec.js

@@ -49,10 +49,11 @@ describe('Issue Due Date component', () => {
   it('should render month and day for other dates', () => {
     date.setDate(date.getDate() + 17);
     vm = createComponent(date);
+    const today = new Date();
+    const isDueInCurrentYear = today.getFullYear() === date.getFullYear();
+    const format = isDueInCurrentYear ? 'mmm d' : 'mmm d, yyyy';
 
-    expect(vm.$el.querySelector('time').textContent.trim()).toEqual(
-      dateFormat(date, 'mmm d', true),
-    );
+    expect(vm.$el.querySelector('time').textContent.trim()).toEqual(dateFormat(date, format, true));
   });
 
   it('should contain the correct `.text-danger` css class for overdue issue', () => {

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

@@ -1,7 +1,8 @@
 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

@@ -1,7 +1,8 @@
 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

@@ -2275,6 +2275,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
@@ -2287,7 +2289,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) }
@@ -2295,7 +2297,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) }
@@ -2320,6 +2322,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
@@ -2332,7 +2336,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) }
@@ -2340,7 +2344,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) }
@@ -2615,7 +2619,7 @@ describe Ci::Build do
 
           allow_any_instance_of(Project)
             .to receive(:ci_variables_for)
-            .with(ref: 'master', environment: nil) do
+            .with(ref: 'refs/heads/master', environment: nil) do
             [create(:ci_variable, key: 'secret', value: 'value')]
           end
 

spec/models/ci/pipeline_spec.rb

@@ -227,6 +227,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

@@ -0,0 +1,59 @@
+# 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/event_spec.rb

@@ -243,6 +243,20 @@ describe Event do
           expect(event.visible_to_user?(admin)).to eq true
         end
       end
+
+      context 'private project' do
+        let(:project) { create(:project, :private) }
+        let(:target) { note_on_issue }
+
+        it do
+          expect(event.visible_to_user?(non_member)).to eq false
+          expect(event.visible_to_user?(author)).to eq false
+          expect(event.visible_to_user?(assignee)).to eq false
+          expect(event.visible_to_user?(member)).to eq true
+          expect(event.visible_to_user?(guest)).to eq true
+          expect(event.visible_to_user?(admin)).to eq true
+        end
+      end
     end
 
     context 'merge request diff note event' do
@@ -265,8 +279,8 @@ describe Event do
 
         it do
           expect(event.visible_to_user?(non_member)).to eq false
-          expect(event.visible_to_user?(author)).to eq true
-          expect(event.visible_to_user?(assignee)).to eq true
+          expect(event.visible_to_user?(author)).to eq false
+          expect(event.visible_to_user?(assignee)).to eq false
           expect(event.visible_to_user?(member)).to eq true
           expect(event.visible_to_user?(guest)).to eq false
           expect(event.visible_to_user?(admin)).to eq true

spec/models/project_spec.rb

@@ -268,6 +268,13 @@ describe Project do
         expect(project.errors[:import_url].first).to include('Requests to localhost are not allowed')
       end
 
+      it 'does not allow import_url pointing to the local network' do
+        project = build(:project, import_url: 'https://192.168.1.1')
+
+        expect(project).to be_invalid
+        expect(project.errors[:import_url].first).to include('Requests to the local network are not allowed')
+      end
+
       it "does not allow import_url with invalid ports for new projects" do
         project = build(:project, import_url: 'http://github.com:25/t.git')
 
@@ -2475,6 +2482,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
@@ -2514,42 +2525,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
@@ -2758,7 +2866,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/remote_mirror_spec.rb

@@ -24,6 +24,20 @@ describe RemoteMirror do
         expect(remote_mirror).to be_invalid
         expect(remote_mirror.errors[:url].first).to include('Username needs to start with an alphanumeric character')
       end
+
+      it 'does not allow url pointing to localhost' do
+        remote_mirror = build(:remote_mirror, url: 'http://127.0.0.2/t.git')
+
+        expect(remote_mirror).to be_invalid
+        expect(remote_mirror.errors[:url].first).to include('Requests to loopback addresses are not allowed')
+      end
+
+      it 'does not allow url pointing to the local network' do
+        remote_mirror = build(:remote_mirror, url: 'https://192.168.1.1')
+
+        expect(remote_mirror).to be_invalid
+        expect(remote_mirror.errors[:url].first).to include('Requests to the local network are not allowed')
+      end
     end
   end
 

spec/models/repository_spec.rb

@@ -1066,6 +1066,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/models/snippet_spec.rb

@@ -423,4 +423,41 @@ describe Snippet do
       expect(blob.data).to eq(snippet.content)
     end
   end
+
+  describe '#embeddable?' do
+    context 'project snippet' do
+      [
+        { project: :public,   snippet: :public,   embeddable: true },
+        { project: :internal, snippet: :public,   embeddable: false },
+        { project: :private,  snippet: :public,   embeddable: false },
+        { project: :public,   snippet: :internal, embeddable: false },
+        { project: :internal, snippet: :internal, embeddable: false },
+        { project: :private,  snippet: :internal, embeddable: false },
+        { project: :public,   snippet: :private,  embeddable: false },
+        { project: :internal, snippet: :private,  embeddable: false },
+        { project: :private,  snippet: :private,  embeddable: false }
+      ].each do |combination|
+        it 'only returns true when both project and snippet are public' do
+          project = create(:project, combination[:project])
+          snippet = create(:project_snippet, combination[:snippet], project: project)
+
+          expect(snippet.embeddable?).to eq(combination[:embeddable])
+        end
+      end
+    end
+
+    context 'personal snippet' do
+      [
+        { snippet: :public,   embeddable: true },
+        { snippet: :internal, embeddable: false },
+        { snippet: :private,  embeddable: false }
+      ].each do |combination|
+        it 'only returns true when snippet is public' do
+          snippet = create(:personal_snippet, combination[:snippet])
+
+          expect(snippet.embeddable?).to eq(combination[:embeddable])
+        end
+      end
+    end
+  end
 end

spec/policies/issuable_policy_spec.rb

@@ -7,6 +7,33 @@ describe IssuablePolicy, models: true do
   let(:policies) { described_class.new(user, issue) }
 
   describe '#rules' do
+    context 'when user is author of issuable' do
+      let(:merge_request) { create(:merge_request, source_project: project, author: user) }
+      let(:policies) { described_class.new(user, merge_request) }
+
+      context 'when user is able to read project' do
+        it 'enables user to read and update issuables' do
+          expect(policies).to be_allowed(:read_issue, :update_issue, :reopen_issue, :read_merge_request, :update_merge_request)
+        end
+      end
+
+      context 'when project is private' do
+        let(:project) { create(:project, :private) }
+
+        context 'when user belongs to the projects team' do
+          it 'enables user to read and update issuables' do
+            project.add_maintainer(user)
+
+            expect(policies).to be_allowed(:read_issue, :update_issue, :reopen_issue, :read_merge_request, :update_merge_request)
+          end
+        end
+
+        it 'disallows user from reading and updating issuables from that project' do
+          expect(policies).to be_disallowed(:read_issue, :update_issue, :reopen_issue, :read_merge_request, :update_merge_request)
+        end
+      end
+    end
+
     context 'when discussion is locked for the issuable' do
       let(:issue) { create(:issue, project: project, discussion_locked: true) }
 

spec/requests/api/jobs_spec.rb

@@ -142,10 +142,20 @@ describe API::Jobs do
     end
 
     context 'unauthorized user' do
-      let(:api_user) { nil }
+      context 'when user is not logged in' do
+        let(:api_user) { nil }
 
-      it 'does not return project jobs' do
-        expect(response).to have_gitlab_http_status(401)
+        it 'does not return project jobs' do
+          expect(response).to have_gitlab_http_status(401)
+        end
+      end
+
+      context 'when user is guest' do
+        let(:api_user) { guest }
+
+        it 'does not return project jobs' do
+          expect(response).to have_gitlab_http_status(403)
+        end
       end
     end
 
@@ -241,10 +251,20 @@ describe API::Jobs do
     end
 
     context 'unauthorized user' do
-      let(:api_user) { nil }
+      context 'when user is not logged in' do
+        let(:api_user) { nil }
 
-      it 'does not return jobs' do
-        expect(response).to have_gitlab_http_status(401)
+        it 'does not return jobs' do
+          expect(response).to have_gitlab_http_status(401)
+        end
+      end
+
+      context 'when user is guest' do
+        let(:api_user) { guest }
+
+        it 'does not return jobs' do
+          expect(response).to have_gitlab_http_status(403)
+        end
       end
     end
   end

spec/requests/api/runner_spec.rb

@@ -441,9 +441,11 @@ describe API::Runner, :clean_gitlab_redis_shared_state do
           it 'picks a job' do
             request_job info: { platform: :darwin }
 
+            runner.reload
+
             expect(response).to have_gitlab_http_status(201)
             expect(response.headers).not_to have_key('X-GitLab-Last-Update')
-            expect(runner.reload.platform).to eq('darwin')
+            expect(runner.platform).to eq('darwin')
             expect(json_response['id']).to eq(job.id)
             expect(json_response['token']).to eq(job.token)
             expect(json_response['job_info']).to eq(expected_job_info)
@@ -537,8 +539,8 @@ describe API::Runner, :clean_gitlab_redis_shared_state do
               expect(json_response['id']).to eq(test_job.id)
               expect(json_response['dependencies'].count).to eq(2)
               expect(json_response['dependencies']).to include(
-                { 'id' => job.id, 'name' => job.name, 'token' => job.token },
-                { 'id' => job2.id, 'name' => job2.name, 'token' => job2.token })
+                { 'id' => job.id, 'name' => job.name, 'token' => test_job.token },
+                { 'id' => job2.id, 'name' => job2.name, 'token' => test_job.token })
             end
           end
 
@@ -557,7 +559,7 @@ describe API::Runner, :clean_gitlab_redis_shared_state do
               expect(json_response['id']).to eq(test_job.id)
               expect(json_response['dependencies'].count).to eq(1)
               expect(json_response['dependencies']).to include(
-                { 'id' => job.id, 'name' => job.name, 'token' => job.token,
+                { 'id' => job.id, 'name' => job.name, 'token' => test_job.token,
                   'artifacts_file' => { 'filename' => 'ci_build_artifacts.zip', 'size' => 106365 } })
             end
           end
@@ -582,7 +584,8 @@ describe API::Runner, :clean_gitlab_redis_shared_state do
               expect(response).to have_gitlab_http_status(201)
               expect(json_response['id']).to eq(test_job.id)
               expect(json_response['dependencies'].count).to eq(1)
-              expect(json_response['dependencies'][0]).to include('id' => job2.id, 'name' => job2.name, 'token' => job2.token)
+              expect(json_response['dependencies'][0]).to include(
+                'id' => job2.id, 'name' => job2.name, 'token' => test_job.token)
             end
           end
 
@@ -983,7 +986,7 @@ describe API::Runner, :clean_gitlab_redis_shared_state do
                 patch_the_trace
               end
 
-              it 'returns Forbidden ' do
+              it 'returns Forbidden' do
                 expect(response.status).to eq(403)
               end
             end
@@ -1024,11 +1027,12 @@ describe API::Runner, :clean_gitlab_redis_shared_state do
 
         context 'when the job is canceled' do
           before do
-            job.cancel
+            job.cancel!
             patch_the_trace
           end
 
-          it 'receives status in header' do
+          it 'responds with forbidden and status in header' do
+            expect(response).to have_gitlab_http_status(403)
             expect(response.header['Job-Status']).to eq 'canceled'
           end
         end
@@ -1199,7 +1203,7 @@ describe API::Runner, :clean_gitlab_redis_shared_state do
           it 'fails to authorize artifacts posting' do
             authorize_artifacts(token: job.project.runners_token)
 
-            expect(response).to have_gitlab_http_status(403)
+            expect(response).to have_gitlab_http_status(404)
           end
         end
 
@@ -1212,10 +1216,10 @@ describe API::Runner, :clean_gitlab_redis_shared_state do
         end
 
         context 'authorization token is invalid' do
-          it 'responds with forbidden' do
+          it 'responds with not found' do
             authorize_artifacts(token: 'invalid', filesize: 100 )
 
-            expect(response).to have_gitlab_http_status(403)
+            expect(response).to have_gitlab_http_status(404)
           end
         end
 
@@ -1248,12 +1252,24 @@ describe API::Runner, :clean_gitlab_redis_shared_state do
             end
 
             it 'responds with forbidden' do
-              upload_artifacts(file_upload, headers_with_token)
-
               expect(response).to have_gitlab_http_status(403)
             end
           end
 
+          context 'when job has been canceled' do
+            let(:job) { create(:ci_build) }
+
+            before do
+              job.cancel!
+              upload_artifacts(file_upload, headers_with_token)
+            end
+
+            it 'responds with forbidden' do
+              expect(response).to have_gitlab_http_status(403)
+              expect(response.header['Job-Status']).to eq('canceled')
+            end
+          end
+
           context 'when job is running' do
             shared_examples 'successful artifacts upload' do
               it 'updates successfully' do
@@ -1303,10 +1319,10 @@ describe API::Runner, :clean_gitlab_redis_shared_state do
             end
 
             context 'when using runners token' do
-              it 'responds with forbidden' do
+              it 'responds with not found' do
                 upload_artifacts(file_upload, headers.merge(API::Helpers::Runner::JOB_TOKEN_HEADER => job.project.runners_token))
 
-                expect(response).to have_gitlab_http_status(403)
+                expect(response).to have_gitlab_http_status(404)
               end
             end
           end
@@ -1526,10 +1542,13 @@ describe API::Runner, :clean_gitlab_redis_shared_state do
       end
 
       describe 'GET /api/v4/jobs/:id/artifacts' do
-        let(:token) { job.token }
+        let(:project) { create(:project) }
+        let(:pipeline) { create(:ci_empty_pipeline, project: project) }
+        let(:running_job) { create(:ci_build, :running, pipeline: pipeline) }
+        let(:token) { running_job.token }
 
         context 'when job has artifacts' do
-          let(:job) { create(:ci_build) }
+          let(:job) { create(:ci_build, pipeline: pipeline) }
           let(:store) { JobArtifactUploader::Store::LOCAL }
 
           before do
@@ -1555,7 +1574,6 @@ describe API::Runner, :clean_gitlab_redis_shared_state do
 
             context 'when artifacts are stored remotely' do
               let(:store) { JobArtifactUploader::Store::REMOTE }
-              let!(:job) { create(:ci_build) }
 
               context 'when proxy download is being used' do
                 before do
@@ -1582,6 +1600,30 @@ describe API::Runner, :clean_gitlab_redis_shared_state do
             end
           end
 
+          context 'when using running token from another pipeline' do
+            let(:running_job) { create(:ci_build, :running, project: project) }
+
+            before do
+              download_artifact
+            end
+
+            it 'responds with not found' do
+              expect(response).to have_gitlab_http_status(404)
+            end
+          end
+
+          context 'when using running token from another project' do
+            let(:running_job) { create(:ci_build, :running) }
+
+            before do
+              download_artifact
+            end
+
+            it 'responds with not found' do
+              expect(response).to have_gitlab_http_status(404)
+            end
+          end
+
           context 'when using runnners token' do
             let(:token) { job.project.runners_token }
 
@@ -1589,8 +1631,8 @@ describe API::Runner, :clean_gitlab_redis_shared_state do
               download_artifact
             end
 
-            it 'responds with forbidden' do
-              expect(response).to have_gitlab_http_status(403)
+            it 'responds with not found' do
+              expect(response).to have_gitlab_http_status(404)
             end
           end
         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/issuable/bulk_update_service_spec.rb

@@ -28,6 +28,33 @@ describe Issuable::BulkUpdateService do
       expect(project.issues.opened).to be_empty
       expect(project.issues.closed).not_to be_empty
     end
+
+    context 'when issue for a different project is created' do
+      let(:private_project) { create(:project, :private) }
+      let(:issue) { create(:issue, project: private_project, author: user) }
+
+      context 'when user has access to the project' do
+        it 'closes all issues passed' do
+          private_project.add_maintainer(user)
+
+          bulk_update(issues + [issue], state_event: 'close')
+
+          expect(project.issues.opened).to be_empty
+          expect(project.issues.closed).not_to be_empty
+          expect(private_project.issues.closed).not_to be_empty
+        end
+      end
+
+      context 'when user does not have access to project' do
+        it 'only closes all issues that the user has access to' do
+          bulk_update(issues + [issue], state_event: 'close')
+
+          expect(project.issues.opened).to be_empty
+          expect(project.issues.closed).not_to be_empty
+          expect(private_project.issues.closed).to be_empty
+        end
+      end
+    end
   end
 
   describe 'reopen issues' 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
@@ -69,14 +69,14 @@ describe Members::DestroyService do
     it 'calls Member#after_decline_request' do
       expect_any_instance_of(NotificationService).to receive(:decline_access_request).with(member)
 
-      described_class.new(current_user).execute(member)
+      described_class.new(current_user).execute(member, opts)
     end
 
     context 'when current user is the member' do
       it 'does not call Member#after_decline_request' do
         expect_any_instance_of(NotificationService).not_to receive(:decline_access_request).with(member)
 
-        described_class.new(member_user).execute(member)
+        described_class.new(member_user).execute(member, opts)
       end
     end
   end
@@ -159,7 +159,7 @@ describe Members::DestroyService do
       end
 
       it_behaves_like 'a service destroying a member' do
-        let(:opts) { { skip_authorization: true } }
+        let(:opts) { { skip_authorization: true, skip_subresources: true } }
         let(:member) { group_project.requesters.find_by(user_id: member_user.id) }
       end
 
@@ -168,12 +168,14 @@ describe Members::DestroyService do
       end
 
       it_behaves_like 'a service destroying a member' do
-        let(:opts) { { skip_authorization: true } }
+        let(:opts) { { skip_authorization: true, skip_subresources: true  } }
         let(:member) { group.requesters.find_by(user_id: member_user.id) }
       end
     end
 
     context 'when current user can destroy the given access requester' do
+      let(:opts) { { skip_subresources: true } }
+
       before do
         group_project.add_maintainer(current_user)
         group.add_owner(current_user)
@@ -229,4 +231,54 @@ describe Members::DestroyService do
       end
     end
   end
+
+  context 'subresources' do
+    let(:user) { create(:user) }
+    let(:member_user) { create(:user) }
+    let(:opts) { {} }
+
+    let(:group) { create(:group, :public) }
+    let(:subgroup) { create(:group, parent: group) }
+    let(:subsubgroup) { create(:group, parent: subgroup) }
+    let(:subsubproject) { create(:project, group: subsubgroup) }
+
+    let(:group_project) { create(:project, :public, group: group) }
+    let(:control_project) { create(:project, group: subsubgroup) }
+
+    before do
+      create(:group_member, :developer, group: subsubgroup, user: member_user)
+
+      subsubproject.add_developer(member_user)
+      control_project.add_maintainer(user)
+      group.add_owner(user)
+
+      group_member = create(:group_member, :developer, group: group, user: member_user)
+
+      described_class.new(user).execute(group_member, opts)
+    end
+
+    it 'removes the project membership' do
+      expect(group_project.members.map(&:user)).not_to include(member_user)
+    end
+
+    it 'removes the group membership' do
+      expect(group.members.map(&:user)).not_to include(member_user)
+    end
+
+    it 'removes the subgroup membership', :postgresql do
+      expect(subgroup.members.map(&:user)).not_to include(member_user)
+    end
+
+    it 'removes the subsubgroup membership', :postgresql do
+      expect(subsubgroup.members.map(&:user)).not_to include(member_user)
+    end
+
+    it 'removes the subsubproject membership', :postgresql do
+      expect(subsubproject.members.map(&:user)).not_to include(member_user)
+    end
+
+    it 'does not remove the user from the control project' do
+      expect(control_project.members.map(&:user)).to include(user)
+    end
+  end
 end

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/merge_requests/build_service_spec.rb

@@ -3,6 +3,7 @@ require 'spec_helper'
 describe MergeRequests::BuildService do
   using RSpec::Parameterized::TableSyntax
   include RepoHelpers
+  include ProjectForksHelper
 
   let(:project) { create(:project, :repository) }
   let(:source_project) { nil }
@@ -44,7 +45,7 @@ describe MergeRequests::BuildService do
 
   describe '#execute' do
     it 'calls the compare service with the correct arguments' do
-      allow_any_instance_of(described_class).to receive(:branches_valid?).and_return(true)
+      allow_any_instance_of(described_class).to receive(:projects_and_branches_valid?).and_return(true)
       expect(CompareService).to receive(:new)
                                   .with(project, Gitlab::Git::BRANCH_REF_PREFIX + source_branch)
                                   .and_call_original
@@ -375,11 +376,27 @@ describe MergeRequests::BuildService do
       end
     end
 
+    context 'target_project is set but repo is not accessible by current_user' do
+      let(:target_project) do
+        create(:project, :public, :repository, repository_access_level: ProjectFeature::PRIVATE)
+      end
+
+      it 'sets target project correctly' do
+        expect(merge_request.target_project).to eq(project)
+      end
+    end
+
     context 'source_project is set and accessible by current_user' do
       let(:source_project) { create(:project, :public, :repository)}
       let(:commits) { Commit.decorate([commit_1], project) }
 
-      it 'sets target project correctly' do
+      before do
+        # To create merge requests _from_ a project the user needs at least
+        # developer access
+        source_project.add_developer(user)
+      end
+
+      it 'sets source project correctly' do
         expect(merge_request.source_project).to eq(source_project)
       end
     end
@@ -388,11 +405,43 @@ describe MergeRequests::BuildService do
       let(:source_project) { create(:project, :private, :repository)}
       let(:commits) { Commit.decorate([commit_1], project) }
 
-      it 'sets target project correctly' do
+      it 'sets source project correctly' do
         expect(merge_request.source_project).to eq(project)
       end
     end
 
+    context 'source_project is set but the user cannot create merge requests from the project' do
+      let(:source_project) do
+        create(:project, :public, :repository, merge_requests_access_level: ProjectFeature::PRIVATE)
+      end
+
+      it 'sets the source_project correctly' do
+        expect(merge_request.source_project).to eq(project)
+      end
+    end
+
+    context 'target_project is not in the fork network of source_project' do
+      let(:target_project) { create(:project, :public, :repository) }
+
+      it 'adds an error to the merge request' do
+        expect(merge_request.errors[:validate_fork]).to contain_exactly('Source project is not a fork of the target project')
+      end
+    end
+
+    context 'target_project is in the fork network of source project but no longer accessible' do
+      let!(:project) { fork_project(target_project, user, namespace: user.namespace, repository: true) }
+      let(:source_project) { project }
+      let(:target_project) { create(:project, :public, :repository) }
+
+      before do
+        target_project.update(visibility_level: Gitlab::VisibilityLevel::PRIVATE)
+      end
+
+      it 'sets the target_project correctly' do
+        expect(merge_request.target_project).to eq(project)
+      end
+    end
+
     context 'when specifying target branch in the description' do
       let(:description) { "A merge request targeting another branch\n\n/target_branch with-codeowners" }
 

spec/services/projects/lfs_pointers/lfs_download_service_spec.rb

@@ -4,17 +4,15 @@ describe Projects::LfsPointers::LfsDownloadService do
   let(:project) { create(:project) }
   let(:oid) { '9e548e25631dd9ce6b43afd6359ab76da2819d6a5b474e66118c7819e1d8b3e8' }
   let(:download_link) { "http://gitlab.com/#{oid}" }
-  let(:lfs_content) do
-    <<~HEREDOC
-    whatever
-    HEREDOC
-  end
+  let(:lfs_content) { SecureRandom.random_bytes(10) }
 
   subject { described_class.new(project) }
 
   before do
     allow(project).to receive(:lfs_enabled?).and_return(true)
     WebMock.stub_request(:get, download_link).to_return(body: lfs_content)
+
+    allow(Gitlab::CurrentSettings).to receive(:allow_local_requests_from_hooks_and_services?).and_return(false)
   end
 
   describe '#execute' do
@@ -32,7 +30,7 @@ describe Projects::LfsPointers::LfsDownloadService do
       it 'stores the content' do
         subject.execute(oid, download_link)
 
-        expect(File.read(LfsObject.first.file.file.file)).to eq lfs_content
+        expect(File.binread(LfsObject.first.file.file.file)).to eq lfs_content
       end
     end
 
@@ -54,18 +52,61 @@ describe Projects::LfsPointers::LfsDownloadService do
       end
     end
 
+    context 'when localhost requests are allowed' do
+      let(:download_link) { 'http://192.168.2.120' }
+
+      before do
+        allow(Gitlab::CurrentSettings).to receive(:allow_local_requests_from_hooks_and_services?).and_return(true)
+      end
+
+      it 'downloads the file' do
+        expect(subject).to receive(:download_and_save_file).and_call_original
+
+        expect { subject.execute(oid, download_link) }.to change { LfsObject.count }.by(1)
+      end
+    end
+
     context 'when a bad URL is used' do
-      where(download_link: ['/etc/passwd', 'ftp://example.com', 'http://127.0.0.2'])
+      where(download_link: ['/etc/passwd', 'ftp://example.com', 'http://127.0.0.2', 'http://192.168.2.120'])
 
       with_them do
         it 'does not download the file' do
-          expect(subject).not_to receive(:download_and_save_file)
-
           expect { subject.execute(oid, download_link) }.not_to change { LfsObject.count }
         end
       end
     end
 
+    context 'when the URL points to a redirected URL' do
+      context 'that is blocked' do
+        where(redirect_link: ['ftp://example.com', 'http://127.0.0.2', 'http://192.168.2.120'])
+
+        with_them do
+          before do
+            WebMock.stub_request(:get, download_link).to_return(status: 301, headers: { 'Location' => redirect_link })
+          end
+
+          it 'does not follow the redirection' do
+            expect(Rails.logger).to receive(:error).with(/LFS file with oid #{oid} couldn't be downloaded/)
+
+            expect { subject.execute(oid, download_link) }.not_to change { LfsObject.count }
+          end
+        end
+      end
+
+      context 'that is valid' do
+        let(:redirect_link) { "http://example.com/"}
+
+        before do
+          WebMock.stub_request(:get, download_link).to_return(status: 301, headers: { 'Location' => redirect_link })
+          WebMock.stub_request(:get, redirect_link).to_return(body: lfs_content)
+        end
+
+        it 'follows the redirection' do
+          expect { subject.execute(oid, download_link) }.to change { LfsObject.count }.from(0).to(1)
+        end
+      end
+    end
+
     context 'when an lfs object with the same oid already exists'  do
       before do
         create(:lfs_object, oid: 'oid')

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 }

spec/services/todo_service_spec.rb

@@ -19,6 +19,7 @@ describe TodoService do
   before do
     project.add_guest(guest)
     project.add_developer(author)
+    project.add_developer(assignee)
     project.add_developer(member)
     project.add_developer(john_doe)
     project.add_developer(skipped)