Merge branch 'security-11-4' into 'security-fix/security-group-user-removal-11-4'

# Conflicts:
#   app/services/members/destroy_service.rb

app/assets/javascripts/gfm_auto_complete.js

@@ -242,7 +242,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;
       },
@@ -309,7 +309,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;
         }
@@ -559,8 +559,11 @@ GfmAutoComplete.Members = {
   },
 };
 GfmAutoComplete.Labels = {
-  // eslint-disable-next-line no-template-curly-in-string
-  template: '<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 = {
@@ -570,8 +573,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: '<li style="pointer-events: none;"><i class="fa fa-spinner fa-spin"></i> Loading...</li>',

app/controllers/concerns/sends_blob.rb

@@ -8,7 +8,7 @@ module SendsBlob
     include SendFileUpload
   end
-  def send_blob(blob, params = {})
+  def send_blob(repository, blob, params = {})
     if blob
       headers['X-Content-Type-Options'] = 'nosniff'

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_secret_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/avatars_controller.rb

@@ -8,7 +8,7 @@ class Projects::AvatarsController < Projects::ApplicationController
   def show
     @blob = @repository.blob_at_branch(@repository.root_ref, @project.avatar_in_git)
-    send_blob(@blob)
+    send_blob(@repository, @blob)
   end
   def destroy

app/controllers/projects/raw_controller.rb

@@ -12,6 +12,6 @@ class Projects::RawController < Projects::ApplicationController
   def show
     @blob = @repository.blob_at(@commit.id, @path)
-    send_blob(@blob, inline: (params[:inline] != 'false'))
+    send_blob(@repository, @blob, inline: (params[:inline] != 'false'))
   end
 end

app/controllers/projects/wikis_controller.rb

@@ -2,6 +2,7 @@
 class Projects::WikisController < Projects::ApplicationController
   include PreviewMarkdown
+  include SendsBlob
   include Gitlab::Utils::StrongMemoize
   before_action :authorize_read_wiki!
@@ -26,16 +27,8 @@ class Projects::WikisController < Projects::ApplicationController
       set_encoding_error unless valid_encoding?
       render 'show'
-    elsif file = @project_wiki.find_file(params[:id], params[:version_id])
-      response.headers['Content-Security-Policy'] = "default-src 'none'"
-      response.headers['X-Content-Security-Policy'] = "default-src 'none'"
-
-      send_data(
-        file.raw_data,
-        type: file.mime_type,
-        disposition: 'inline',
-        filename: file.name
-      )
+    elsif file_blob
+      send_blob(@project_wiki.repository, file_blob)
     elsif can?(current_user, :create_wiki, @project) && view_param == 'create'
       @page = build_page(title: params[:id])
@@ -164,4 +157,14 @@ class Projects::WikisController < Projects::ApplicationController
   def set_encoding_error
     flash.now[:notice] = "The content of this page is not encoded in UTF-8. Edits can only be made via the Git repository."
   end
+
+  def file_blob
+    strong_memoize(:file_blob) do
+      commit = @project_wiki.repository.commit(@project_wiki.default_branch)
+
+      next unless commit
+
+      @project_wiki.repository.blob_at(commit.id, params[:id])
+    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/helpers/blob_helper.rb

@@ -150,7 +150,9 @@ module BlobHelper
   # example of Javascript) we tell the browser of the victim not to
   # execute untrusted data.
   def safe_content_type(blob)
-    if blob.text?
+    if blob.extension == 'svg'
+      blob.mime_type
+    elsif blob.text?
       'text/plain; charset=utf-8'
     elsif blob.image?
       blob.content_type
@@ -159,6 +161,12 @@ module BlobHelper
     end
   end
+  def content_disposition(blob, inline)
+    return 'attachment' if blob.extension == 'svg'
+
+    inline ? 'inline' : 'attachment'
+  end
+
   def ref_project
     @ref_project ||= @target_project || @project
   end

app/helpers/workhorse_helper.rb

@@ -6,7 +6,7 @@ module WorkhorseHelper
   # Send a Git blob through Workhorse
   def send_git_blob(repository, blob, inline: true)
     headers.store(*Gitlab::Workhorse.send_git_blob(repository, blob))
-    headers['Content-Disposition'] = inline ? 'inline' : 'attachment'
+    headers['Content-Disposition'] = content_disposition(blob, inline)
     headers['Content-Type'] = safe_content_type(blob)
     render plain: ""
   end

app/models/ci/build.rb

@@ -9,6 +9,7 @@ module Ci
     include Presentable
     include Importable
     include Gitlab::Utils::StrongMemoize
+    include HasRef
     belongs_to :project, inverse_of: :builds
     belongs_to :runner
@@ -153,6 +154,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
@@ -593,11 +598,11 @@ module Ci
     def secret_group_variables
       return [] unless project.group
-      project.group.secret_variables_for(ref, project)
+      project.group.secret_variables_for(git_ref, project)
     end
     def secret_project_variables(environment: persisted_environment)
-      project.secret_variables_for(ref: ref, environment: environment)
+      project.secret_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
@@ -355,10 +356,6 @@ module Ci
       @commit ||= Commit.lazy(project, sha)
     end
-    def branch?
-      !tag?
-    end
-
     def stuck?
       pending_builds.any?(&:stuck?)
     end
@@ -558,7 +555,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

app/models/concerns/has_ref.rb

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

app/models/project.rb

@@ -305,10 +305,10 @@ class Project < ActiveRecord::Base
   validates :namespace, presence: true
   validates :name, uniqueness: { scope: :namespace_id }
-  validates :import_url, url: { protocols: %w(http https ssh git),
-                                allow_localhost: false,
-                                enforce_user: true,
-                                ports: VALID_IMPORT_PORTS }, if: [:external_import?, :import_url_changed?]
+  validates :import_url, public_url: { protocols: %w(http https ssh git),
+                                       allow_localhost: false,
+                                       enforce_user: true,
+                                       ports: VALID_IMPORT_PORTS }, 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? }
@@ -1846,10 +1846,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.
   #
@@ -177,6 +178,18 @@ class Repository
     tags.find { |tag| tag.name == name }
   end
+  def ambiguous_ref?(ref)
+    tag_exists?(ref) && branch_exists?(ref)
+  end
+
+  def expand_ref(ref)
+    if tag_exists?(ref)
+      Gitlab::Git::TAG_REF_PREFIX + ref
+    elsif branch_exists?(ref)
+      Gitlab::Git::BRANCH_REF_PREFIX + ref
+    end
+  end
+
   def add_branch(user, branch_name, ref)
     branch = raw_repository.add_branch(branch_name, user: user, target: ref)

app/models/todo.rb

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

app/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

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