Merge branch 'master' of dev.gitlab.org:gitlab/gitlab-ee

CHANGELOG-EE.md

 Please view this file on the master branch, on stable branches it's out of date.
 
+## 12.9.1 (2020-03-26)
+
+### Security (1 change)
+
+- Add NPM package versions SemVer validation.
+
+
 ## 12.9.0 (2020-03-22)
 
 ### Removed (1 change)

CHANGELOG.md

@@ -2,6 +2,32 @@
 documentation](doc/development/changelog.md) for instructions on adding your own
 entry.
 
+## 12.9.1 (2020-03-26)
+
+### Security (16 changes)
+
+- Add permission check for pipeline status of MR.
+- Ignore empty remote_id params from Workhorse accelerated uploads.
+- External user can not create personal snippet through API.
+- Prevent malicious entry for group name.
+- Restrict mirroring changes to admins only when mirroring is disabled.
+- Reject all container registry requests from blocked users.
+- Deny localhost requests on fogbugz importer.
+- Redact notes in moved confidential issues.
+- Fix UploadRewriter Path Traversal vulnerability.
+- Block hotlinking to repository archives.
+- Restrict access to project pipeline metrics reports.
+- vulnerability_feedback records should be restricted to a dev role and above.
+- Exclude Carrierwave remote URL methods from import.
+- Update Nokogiri to fix CVE-2020-7595.
+- Prevent updating trigger by other maintainers.
+- Fix XSS vulnerability in `admin/email` "Recipient Group" dropdown.
+
+### Fixed (1 change)
+
+- Fix updating the authorized_keys file. !27798
+
+
 ## 12.9.0 (2020-03-22)
 
 ### Security (1 change)

GITLAB_WORKHORSE_VERSION

-8.27.0
+8.28.0

Gemfile.lock

@@ -652,7 +652,7 @@ GEM
     netrc (0.11.0)
     nio4r (2.5.2)
     no_proxy_fix (0.1.2)
-    nokogiri (1.10.7)
+    nokogiri (1.10.8)
       mini_portile2 (~> 2.4.0)
     nokogumbo (1.5.0)
       nokogiri

app/assets/javascripts/frequent_items/utils.js

@@ -45,8 +45,19 @@ export const updateExistingFrequentItem = (frequentItem, item) => {
   };
 };
 
-export const sanitizeItem = item => ({
-  ...item,
-  name: sanitize(item.name.toString(), { allowedTags: [] }),
-  namespace: sanitize(item.namespace.toString(), { allowedTags: [] }),
-});
+export const sanitizeItem = item => {
+  // Only sanitize if the key exists on the item
+  const maybeSanitize = key => {
+    if (!Object.prototype.hasOwnProperty.call(item, key)) {
+      return {};
+    }
+
+    return { [key]: sanitize(item[key].toString(), { allowedTags: [] }) };
+  };
+
+  return {
+    ...item,
+    ...maybeSanitize('name'),
+    ...maybeSanitize('namespace'),
+  };
+};

app/controllers/concerns/hotlink_interceptor.rb

+# frozen_string_literal: true
+
+module HotlinkInterceptor
+  extend ActiveSupport::Concern
+
+  def intercept_hotlinking!
+    return render_406 if Gitlab::HotlinkingDetector.intercept_hotlinking?(request)
+  end
+
+  private
+
+  def render_406
+    head :not_acceptable
+  end
+end

app/controllers/import/fogbugz_controller.rb

@@ -3,6 +3,7 @@
 class Import::FogbugzController < Import::BaseController
   before_action :verify_fogbugz_import_enabled
   before_action :user_map, only: [:new_user_map, :create_user_map]
+  before_action :verify_blocked_uri, only: :callback
 
   rescue_from Fogbugz::AuthenticationException, with: :fogbugz_unauthorized
 
@@ -106,4 +107,21 @@ class Import::FogbugzController < Import::BaseController
   def verify_fogbugz_import_enabled
     render_404 unless fogbugz_import_enabled?
   end
+
+  def verify_blocked_uri
+    Gitlab::UrlBlocker.validate!(
+      params[:uri],
+      {
+        allow_localhost: allow_local_requests?,
+        allow_local_network: allow_local_requests?,
+        schemes: %w(http https)
+      }
+    )
+  rescue Gitlab::UrlBlocker::BlockedUrlError => e
+    redirect_to new_import_fogbugz_url, alert: _('Specified URL cannot be used: "%{reason}"') % { reason: e.message }
+  end
+
+  def allow_local_requests?
+    Gitlab::CurrentSettings.allow_local_requests_from_web_hooks_and_services?
+  end
 end

app/controllers/projects/mirrors_controller.rb

@@ -67,7 +67,7 @@ class Projects::MirrorsController < Projects::ApplicationController
   end
 
   def check_mirror_available!
-    Gitlab::CurrentSettings.current_application_settings.mirror_available || current_user&.admin?
+    render_404 unless can?(current_user, :admin_remote_mirror, project)
   end
 
   def mirror_params_attributes

app/controllers/projects/repositories_controller.rb

@@ -4,12 +4,14 @@ class Projects::RepositoriesController < Projects::ApplicationController
   include ExtractsPath
   include StaticObjectExternalStorage
   include Gitlab::RateLimitHelpers
+  include HotlinkInterceptor
 
   prepend_before_action(only: [:archive]) { authenticate_sessionless_user!(:archive) }
 
   # Authorize
   before_action :require_non_empty_project, except: :create
   before_action :archive_rate_limit!, only: :archive
+  before_action :intercept_hotlinking!, only: :archive
   before_action :assign_archive_vars, only: :archive
   before_action :assign_append_sha, only: :archive
   before_action :authorize_download_code!

app/models/group.rb

@@ -70,6 +70,9 @@ class Group < Namespace
   validates :variables, variable_duplicates: true
 
   validates :two_factor_grace_period, presence: true, numericality: { greater_than_or_equal_to: 0 }
+  validates :name,
+    format: { with: Gitlab::Regex.group_name_regex,
+              message: Gitlab::Regex.group_name_regex_message }
 
   add_authentication_token_field :runners_token, encrypted: -> { Feature.enabled?(:groups_tokens_optional_encryption, default_enabled: true) ? :optional : :required }
 

app/models/issue.rb

@@ -326,10 +326,8 @@ class Issue < ApplicationRecord
       true
     elsif project.owner == user
       true
-    elsif confidential?
-      author == user ||
-        assignees.include?(user) ||
-        project.team.member?(user, Gitlab::Access::REPORTER)
+    elsif confidential? && !assignee_or_author?(user)
+      project.team.member?(user, Gitlab::Access::REPORTER)
     else
       project.public? ||
         project.internal? && !user.external? ||

app/serializers/merge_request_poll_widget_entity.rb

@@ -53,7 +53,7 @@ class MergeRequestPollWidgetEntity < Grape::Entity
 
   # CI related
   expose :has_ci?, as: :has_ci
-  expose :ci_status do |merge_request|
+  expose :ci_status, if: -> (mr, _) { presenter(mr).can_read_pipeline? } do |merge_request|
     presenter(merge_request).ci_status
   end
 

app/uploaders/object_storage.rb

@@ -318,7 +318,7 @@ module ObjectStorage
     def cache!(new_file = sanitized_file)
       # We intercept ::UploadedFile which might be stored on remote storage
       # We use that for "accelerated" uploads, where we store result on remote storage
-      if new_file.is_a?(::UploadedFile) && new_file.remote_id
+      if new_file.is_a?(::UploadedFile) && new_file.remote_id.present?
         return cache_remote_file!(new_file.remote_id, new_file.original_filename)
       end
 

app/views/projects/mirrors/_mirror_repos.html.haml

 - expanded = expanded_by_default?
 - protocols = Gitlab::UrlSanitizer::ALLOWED_SCHEMES.join('|')
+- mirror_settings_enabled = can?(current_user, :admin_remote_mirror, @project)
+- mirror_settings_class = "#{'expanded' if expanded} #{'js-mirror-settings' if mirror_settings_enabled}".strip
 
-%section.settings.project-mirror-settings.js-mirror-settings.no-animate#js-push-remote-settings{ class: ('expanded' if expanded), data: { qa_selector: 'mirroring_repositories_settings_section' } }
+%section.settings.project-mirror-settings.no-animate#js-push-remote-settings{ class: mirror_settings_class, data: { qa_selector: 'mirroring_repositories_settings_section' } }
   .settings-header
     %h4= _('Mirroring repositories')
     %button.btn.js-settings-toggle
@@ -11,26 +13,32 @@
       = link_to _('Read more'), help_page_path('workflow/repository_mirroring'), target: '_blank'
 
   .settings-content
-    = form_for @project, url: project_mirror_path(@project), html: { class: 'gl-show-field-errors js-mirror-form', autocomplete: 'new-password', data: mirrors_form_data_attributes } do |f|
-      .panel.panel-default
-        .panel-body
-          %div= form_errors(@project)
+    - if mirror_settings_enabled
+      = form_for @project, url: project_mirror_path(@project), html: { class: 'gl-show-field-errors js-mirror-form', autocomplete: 'new-password', data: mirrors_form_data_attributes } do |f|
+        .panel.panel-default
+          .panel-body
+            %div= form_errors(@project)
 
-          .form-group.has-feedback
-            = label_tag :url, _('Git repository URL'), class: 'label-light'
-            = text_field_tag :url, nil, class: 'form-control js-mirror-url js-repo-url qa-mirror-repository-url-input', placeholder: _('Input your repository URL'), required: true, pattern: "(#{protocols}):\/\/.+", autocomplete: 'new-password'
+            .form-group.has-feedback
+              = label_tag :url, _('Git repository URL'), class: 'label-light'
+              = text_field_tag :url, nil, class: 'form-control js-mirror-url js-repo-url qa-mirror-repository-url-input', placeholder: _('Input your repository URL'), required: true, pattern: "(#{protocols}):\/\/.+", autocomplete: 'new-password'
 
-          = render 'projects/mirrors/instructions'
+            = render 'projects/mirrors/instructions'
 
-          = render 'projects/mirrors/mirror_repos_form', f: f
+            = render 'projects/mirrors/mirror_repos_form', f: f
 
-          .form-check.append-bottom-10
-            = check_box_tag :only_protected_branches, '1', false, class: 'js-mirror-protected form-check-input'
-            = label_tag :only_protected_branches, _('Only mirror protected branches'), class: 'form-check-label'
-            = link_to icon('question-circle'), help_page_path('user/project/protected_branches'), target: '_blank'
+            .form-check.append-bottom-10
+              = check_box_tag :only_protected_branches, '1', false, class: 'js-mirror-protected form-check-input'
+              = label_tag :only_protected_branches, _('Only mirror protected branches'), class: 'form-check-label'
+              = link_to icon('question-circle'), help_page_path('user/project/protected_branches'), target: '_blank'
 
-        .panel-footer
-          = f.submit _('Mirror repository'), class: 'btn btn-success js-mirror-submit qa-mirror-repository-button', name: :update_remote_mirror
+          .panel-footer
+            = f.submit _('Mirror repository'), class: 'btn btn-success js-mirror-submit qa-mirror-repository-button', name: :update_remote_mirror
+    - else
+      .gl-alert.gl-alert-info{ role: 'alert' }
+        = sprite_icon('information-o', size: 16, css_class: 'gl-icon gl-alert-icon gl-alert-icon-no-title')
+        .gl-alert-body
+          = _('Mirror settings are only available to GitLab administrators.')
 
     .panel.panel-default
       .table-responsive
@@ -61,8 +69,10 @@
                   - if mirror.last_error.present?
                     .badge.mirror-error-badge{ data: { toggle: 'tooltip', html: 'true', qa_selector: 'mirror_error_badge' }, title: html_escape(mirror.last_error.try(:strip)) }= _('Error')
                 %td
-                  .btn-group.mirror-actions-group.pull-right{ role: 'group' }
-                    - if mirror.ssh_key_auth?
-                      = clipboard_button(text: mirror.ssh_public_key, class: 'btn btn-default', title: _('Copy SSH public key'), qa_selector: 'copy_public_key_button')
-                    = render 'shared/remote_mirror_update_button', remote_mirror: mirror
+                  - if mirror_settings_enabled
+                    .btn-group.mirror-actions-group.pull-right{ role: 'group' }
+                      - if mirror.ssh_key_auth?
+                        = clipboard_button(text: mirror.ssh_public_key, class: 'btn btn-default', title: _('Copy SSH public key'), qa_selector: 'copy_public_key_button')
+                      = render 'shared/remote_mirror_update_button', remote_mirror: mirror
                     %button.js-delete-mirror.qa-delete-mirror.rspec-delete-mirror.btn.btn-danger{ type: 'button', data: { mirror_id: mirror.id, toggle: 'tooltip', container: 'body' }, title: _('Remove') }= icon('trash-o')
+

changelogs/unreleased/212178-fix-authorized-keys-worker.yml

----
-title: Fix updating the authorized_keys file
-merge_request: 27798
-author:
-type: fixed

ee/app/assets/javascripts/pages/admin/emails/show/admin_email_select.js

 import $ from 'jquery';
 import Api from '~/api';
 import { sprintf, __ } from '~/locale';
+import { sanitizeItem } from '~/frequent_items/utils';
 
 const formatResult = selectedItem => {
   if (selectedItem.path_with_namespace) {
@@ -38,7 +39,7 @@ const AdminEmailSelect = () => {
           const all = {
             id: 'all',
           };
-          const data = [all].concat(groups, projects.data);
+          const data = [all].concat(groups, projects.data).map(sanitizeItem);
           return query.callback({
             results: data,
           });

ee/app/controllers/ee/projects/merge_requests_controller.rb

@@ -16,7 +16,8 @@ module EE
 
         before_action :whitelist_query_limiting_ee_merge, only: [:merge]
         before_action :whitelist_query_limiting_ee_show, only: [:show]
-        before_action :authorize_read_pipeline!, only: [:container_scanning_reports, :dependency_scanning_reports, :sast_reports, :dast_reports]
+        before_action :authorize_read_pipeline!, only: [:container_scanning_reports, :dependency_scanning_reports,
+                                                        :sast_reports, :dast_reports, :metrics_reports]
       end
 
       def approve

ee/app/models/packages/package.rb

@@ -30,6 +30,7 @@ class Packages::Package < ApplicationRecord
   validate :valid_conan_package_recipe, if: :conan?
   validate :valid_npm_package_name, if: :npm?
   validate :package_already_taken, if: :npm?
+  validates :version, format: { with: Gitlab::Regex.semver_regex }, if: :npm?
 
   enum package_type: { maven: 1, npm: 2, conan: 3, nuget: 4, pypi: 5 }
 

ee/app/policies/ee/project_policy.rb

@@ -170,6 +170,7 @@ module EE
 
       rule { can?(:developer_access) }.policy do
         enable :admin_board
+        enable :read_vulnerability_feedback
         enable :create_vulnerability_feedback
         enable :destroy_vulnerability_feedback
         enable :update_vulnerability_feedback
@@ -185,8 +186,6 @@ module EE
 
       rule { can?(:public_access) }.enable :read_package
 
-      rule { can?(:read_build) & can?(:download_code) }.enable :read_security_findings
-
       rule { security_dashboard_enabled & can?(:developer_access) }.enable :read_vulnerability
 
       rule { can?(:read_merge_request) & can?(:read_pipeline) }.enable :read_merge_train
@@ -201,8 +200,6 @@ module EE
 
       rule { threat_monitoring_enabled & (auditor | can?(:developer_access)) }.enable :read_threat_monitoring
 
-      rule { can?(:read_security_findings) }.enable :read_vulnerability_feedback
-
       rule { dependency_scanning_enabled & can?(:download_code) }.enable :read_dependencies
 
       rule { license_scanning_enabled & can?(:download_code) }.enable :read_licenses

ee/lib/ee/gitlab/regex.rb

@@ -43,6 +43,11 @@ module EE
           maven_app_name_regex
         end
 
+        def semver_regex
+          # see the official regex: https://semver.org/#is-there-a-suggested-regular-expression-regex-to-check-a-semver-string
+          @semver_regex ||= %r{\A(0|[1-9]\d*)\.(0|[1-9]\d*)\.(0|[1-9]\d*)(?:-((?:0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*)(?:\.(?:0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*))*))?(?:\+([0-9a-zA-Z-]+(?:\.[0-9a-zA-Z-]+)*))?\z}.freeze
+        end
+
         def feature_flag_regex
           /\A[a-z]([-_a-z0-9]*[a-z0-9])?\z/
         end

ee/spec/controllers/projects/merge_requests_controller_spec.rb

@@ -90,6 +90,29 @@ shared_examples 'approvals' do
   end
 end
 
+shared_examples 'authorize read pipeline' do
+  context 'public project with private builds' do
+    let(:comparison_status) { {} }
+    let(:project) { create(:project, :public, :builds_private) }
+
+    it 'restricts access to signed out users' do
+      sign_out user
+
+      subject
+
+      expect(response).to have_gitlab_http_status(:not_found)
+    end
+
+    it 'restricts access to other users' do
+      sign_in create(:user)
+
+      subject
+
+      expect(response).to have_gitlab_http_status(:not_found)
+    end
+  end
+end
+
 describe Projects::MergeRequestsController do
   include ProjectForksHelper
 
@@ -462,20 +485,7 @@ describe Projects::MergeRequestsController do
       end
     end
 
-    context 'public project with private builds' do
-      let(:comparison_status) { {} }
-      let(:project) { create(:project, :public, :builds_private) }
-
-      before do
-        sign_out user
-      end
-
-      it 'restricts unauthorized access' do
-        subject
-
-        expect(response).to have_gitlab_http_status(:not_found)
-      end
-    end
+    it_behaves_like 'authorize read pipeline'
   end
 
   describe 'GET #container_scanning_reports' do
@@ -545,20 +555,7 @@ describe Projects::MergeRequestsController do
       end
     end
 
-    context 'public project with private builds' do
-      let(:comparison_status) { {} }
-      let(:project) { create(:project, :public, :builds_private) }
-
-      before do
-        sign_out user
-      end
-
-      it 'restricts unauthorized access' do
-        subject
-
-        expect(response).to have_gitlab_http_status(:not_found)
-      end
-    end
+    it_behaves_like 'authorize read pipeline'
   end
 
   describe 'GET #sast_reports' do
@@ -628,20 +625,7 @@ describe Projects::MergeRequestsController do
       end
     end
 
-    context 'public project with private builds' do
-      let(:comparison_status) { {} }
-      let(:project) { create(:project, :public, :builds_private) }
-
-      before do
-        sign_out user
-      end
-
-      it 'restricts unauthorized access' do
-        subject
-
-        expect(response).to have_gitlab_http_status(:not_found)
-      end
-    end
+    it_behaves_like 'authorize read pipeline'
   end
 
   describe 'GET #dast_reports' do
@@ -711,26 +695,7 @@ describe Projects::MergeRequestsController do
       end
     end
 
-    context 'public project with private builds' do
-      let(:comparison_status) { {} }
-      let(:project) { create(:project, :public, :builds_private) }
-
-      it 'restricts access to signed out users' do
-        sign_out user
-
-        subject
-
-        expect(response).to have_gitlab_http_status(:not_found)
-      end
-
-      it 'restricts access to other users' do
-        sign_in create(:user)
-
-        subject
-
-        expect(response).to have_gitlab_http_status(:not_found)
-      end
-    end
+    it_behaves_like 'authorize read pipeline'
   end
 
   describe 'GET #license_management_reports' do
@@ -868,6 +833,8 @@ describe Projects::MergeRequestsController do
         expect(json_response).to eq({ 'status_reason' => 'Failed to parse test reports' })
       end
     end
+
+    it_behaves_like 'authorize read pipeline'
   end
 
   it_behaves_like DescriptionDiffActions do

ee/spec/controllers/projects/vulnerability_feedback_controller_spec.rb

@@ -25,6 +25,10 @@ describe Projects::VulnerabilityFeedbackController do
     let!(:vuln_feedback_5) { create(:vulnerability_feedback, :merge_request, :dependency_scanning, project: project, author: user, pipeline: pipeline_1, merge_request: merge_request) }
 
     context '@vulnerability_feedback' do
+      before do
+        sign_in(user)
+      end
+
       it 'returns a successful 200 response' do
         list_feedbacks
 

ee/spec/controllers/subscriptions_controller_spec.rb

@@ -202,7 +202,7 @@ describe SubscriptionsController do
           group.save
           subject
 
-          expect(response.body).to eq({ name: ["can't be blank"] }.to_json)
+          expect(response.body).to include({ name: ["can't be blank", Gitlab::Regex.group_name_regex_message] }.to_json)
         end
       end
 

ee/spec/lib/gitlab/regex_spec.rb

@@ -103,4 +103,17 @@ describe Gitlab::Regex do
     it { is_expected.not_to match('my package name') }
     it { is_expected.not_to match('!!()()') }
   end
+
+  describe '.semver_regex' do
+    subject { described_class.semver_regex }
+
+    it { is_expected.to match('1.2.3') }
+    it { is_expected.to match('1.2.3-beta') }
+    it { is_expected.to match('1.2.3-alpha.3') }
+    it { is_expected.not_to match('1') }
+    it { is_expected.not_to match('1.2') }
+    it { is_expected.not_to match('1./2.3') }
+    it { is_expected.not_to match('../../../../../1.2.3') }
+    it { is_expected.not_to match('%2e%2e%2f1.2.3') }
+  end
 end

ee/spec/models/packages/package_spec.rb

@@ -80,6 +80,21 @@ RSpec.describe Packages::Package, type: :model do
       it { is_expected.not_to allow_value("my(dom$$$ain)com.my-app").for(:name) }
     end
 
+    describe '#version' do
+      context 'npm package' do
+        subject { create(:npm_package) }
+
+        it { is_expected.to allow_value('1.2.3').for(:version) }
+        it { is_expected.to allow_value('1.2.3-beta').for(:version) }
+        it { is_expected.to allow_value('1.2.3-alpha.3').for(:version) }
+        it { is_expected.not_to allow_value('1').for(:version) }
+        it { is_expected.not_to allow_value('1.2').for(:version) }
+        it { is_expected.not_to allow_value('1./2.3').for(:version) }
+        it { is_expected.not_to allow_value('../../../../../1.2.3').for(:version) }
+        it { is_expected.not_to allow_value('%2e%2e%2f1.2.3').for(:version) }
+      end
+    end
+
     describe '#package_already_taken' do
       context 'npm package' do
         let!(:package) { create(:npm_package) }
@@ -173,7 +188,7 @@ RSpec.describe Packages::Package, type: :model do
     end
 
     describe '.has_version' do
-      let!(:package4) { create(:npm_package, version: nil) }
+      let!(:package4) { create(:nuget_package, version: nil) }
 
       subject { described_class.has_version }
 

ee/spec/policies/project_policy_spec.rb

@@ -51,7 +51,7 @@ describe ProjectPolicy do
         read_environment read_deployment read_merge_request read_pages
         create_merge_request_in award_emoji
         read_project_security_dashboard read_vulnerability
-        read_vulnerability_feedback read_security_findings read_software_license_policy
+        read_software_license_policy
         read_threat_monitoring read_merge_train
       ]
     end
@@ -331,121 +331,11 @@ describe ProjectPolicy do
     end
   end
 
-  describe 'read_vulnerability_feedback' do
-    context 'with private project' do
-      let(:current_user) { admin }
-      let(:project) { create(:project, :private, namespace: owner.namespace) }
-
-      where(role: %w[admin owner maintainer developer reporter])
-
-      with_them do
-        let(:current_user) { public_send(role) }
-
-        it { is_expected.to be_allowed(:read_vulnerability_feedback) }
-      end
-
-      context 'with guest' do
-        let(:current_user) { guest }
-
-        it { is_expected.to be_disallowed(:read_vulnerability_feedback) }
-      end
-
-      context 'with non member' do
-        let(:current_user) { create(:user) }
-
-        it { is_expected.to be_disallowed(:read_vulnerability_feedback) }
-      end
-
-      context 'with anonymous' do
-        let(:current_user) { nil }
-
-        it { is_expected.to be_disallowed(:read_vulnerability_feedback) }
-      end
-    end
-
-    context 'with public project' do
-      let(:current_user) { create(:user) }
-
-      context 'with limited access to both builds and merge requests' do
-        context 'when builds enabled for project members' do
-          let(:project) { create(:project, :public, :merge_requests_private, :builds_private) }
-
-          it { is_expected.not_to be_allowed(:read_vulnerability_feedback) }
-        end
-
-        context 'when public builds disabled' do
-          let(:project) { create(:project, :public, :merge_requests_private, public_builds: false) }
-
-          it { is_expected.not_to be_allowed(:read_vulnerability_feedback) }
-        end
-      end
-
-      context 'with limited access to merge requests' do
-        let(:project) { create(:project, :public, :merge_requests_private) }
-
-        it { is_expected.to be_allowed(:read_vulnerability_feedback) }
-      end
-
-      context 'with public access to repository' do
-        let(:project) { create(:project, :public) }
-
-        it { is_expected.to be_allowed(:read_vulnerability_feedback) }
-      end
-    end
-  end
-
-  describe 'read_security_findings' do
-    context 'with private project' do
-      let(:project) { create(:project, :private, namespace: owner.namespace) }
-
-      context 'with reporter or above' do
-        let(:current_user) { reporter }
-
-        it { is_expected.to be_allowed(:read_security_findings) }
-      end
-
-      context 'with non member' do
-        let(:current_user) { create(:user) }
-
-        it { is_expected.to be_disallowed(:read_security_findings) }
-      end
-
-      context 'with anonymous' do
-        let(:current_user) { nil }
-
-        it { is_expected.to be_disallowed(:read_security_findings) }
-      end
-    end
-
-    context 'with public project' do
-      let(:current_user) { create(:user) }
-
-      context 'with limited access to builds' do
-        context 'when builds enabled only for project members' do
-          let(:project) { create(:project, :public, :builds_private) }
-
-          it { is_expected.not_to be_allowed(:read_security_findings) }
-        end
-
-        context 'when public builds disabled' do
-          let(:project) { create(:project, :public, public_builds: false) }
-
-          it { is_expected.not_to be_allowed(:read_security_findings) }
-        end
-      end
-
-      context 'with public access to repository' do
-        let(:project) { create(:project, :public) }
-
-        it { is_expected.to be_allowed(:read_security_findings) }
-      end
-    end
-  end
-
   describe 'vulnerability feedback permissions' do
     subject { described_class.new(current_user, project) }
 
     where(permission: %i[
+      read_vulnerability_feedback
       create_vulnerability_feedback
       update_vulnerability_feedback
       destroy_vulnerability_feedback

ee/spec/requests/api/npm_packages_spec.rb

@@ -230,21 +230,25 @@ describe API::NpmPackages do
   end
 
   describe 'PUT /api/v4/projects/:id/packages/npm/:package_name' do
+    RSpec.shared_examples 'handling invalid record with 400 error' do
+      it 'handles an ActiveRecord::RecordInvalid exception with 400 error' do
+        expect { upload_package_with_token(package_name, params) }
+          .not_to change { project.packages.count }
+
+        expect(response).to have_gitlab_http_status(:bad_request)
+      end
+    end
+
     context 'when params are correct' do
       context 'invalid package record' do
         context 'unscoped package' do
           let(:package_name) { 'my_unscoped_package' }
-          let(:params) { upload_params(package_name) }
+          let(:params) { upload_params(package_name: package_name) }
 
-          it 'handles an ActiveRecord::RecordInvalid exception with 400 error' do
-            expect { upload_package_with_token(package_name, params) }
-              .not_to change { project.packages.count }
-
-            expect(response).to have_gitlab_http_status(:bad_request)
-          end
+          it_behaves_like 'handling invalid record with 400 error'
 
           context 'with empty versions' do
-            let(:params) { upload_params(package_name).merge!(versions: {}) }
+            let(:params) { upload_params(package_name: package_name).merge!(versions: {}) }
 
             it 'throws a 400 error' do
               expect { upload_package_with_token(package_name, params) }
@@ -257,20 +261,37 @@ describe API::NpmPackages do
 
         context 'invalid package name' do
           let(:package_name) { "@#{group.path}/my_inv@@lid_package_name" }
-          let(:params) { upload_params(package_name) }
+          let(:params) { upload_params(package_name: package_name) }
 
-          it 'handles an ActiveRecord::RecordInvalid exception with 400 error' do
-            expect { upload_package_with_token(package_name, params) }
-              .not_to change { project.packages.count }
+          it_behaves_like 'handling invalid record with 400 error'
+        end
+
+        context 'invalid package version' do
+          using RSpec::Parameterized::TableSyntax
+
+          let(:package_name) { "@#{group.path}/my_package_name" }
+
+          where(:version) do
+            [
+              '1',
+              '1.2',
+              '1./2.3',
+              '../../../../../1.2.3',
+              '%2e%2e%2f1.2.3'
+            ]
+          end
+
+          with_them do
+            let(:params) { upload_params(package_name: package_name, package_version: version) }
 
-            expect(response).to have_gitlab_http_status(:bad_request)
+            it_behaves_like 'handling invalid record with 400 error'
           end
         end
       end
 
       context 'scoped package' do
         let(:package_name) { "@#{group.path}/my_package_name" }
-        let(:params) { upload_params(package_name) }
+        let(:params) { upload_params(package_name: package_name) }
 
         context 'with access token' do
           subject { upload_package_with_token(package_name, params) }
@@ -319,7 +340,7 @@ describe API::NpmPackages do
 
       context 'package creation fails' do
         let(:package_name) { "@#{group.path}/my_package_name" }
-        let(:params) { upload_params(package_name) }
+        let(:params) { upload_params(package_name: package_name) }
 
         it 'returns an error if the package already exists' do
           create(:npm_package, project: project, version: '1.0.1', name: "@#{group.path}/my_package_name")
@@ -332,7 +353,7 @@ describe API::NpmPackages do
 
       context 'with dependencies' do
         let(:package_name) { "@#{group.path}/my_package_name" }
-        let(:params) { upload_params(package_name, 'npm/payload_with_duplicated_packages.json') }
+        let(:params) { upload_params(package_name: package_name, file: 'npm/payload_with_duplicated_packages.json') }
 
         it 'creates npm package with file and dependencies' do
           expect { upload_package_with_token(package_name, params) }
@@ -347,7 +368,7 @@ describe API::NpmPackages do
         context 'with existing dependencies' do
           before do
             name = "@#{group.path}/existing_package"
-            upload_package_with_token(name, upload_params(name, 'npm/payload_with_duplicated_packages.json'))
+            upload_package_with_token(name, upload_params(package_name: name, file: 'npm/payload_with_duplicated_packages.json'))
           end
 
           it 'reuses them' do
@@ -373,10 +394,11 @@ describe API::NpmPackages do
       upload_package(package_name, params.merge(job_token: job.token))
     end
 
-    def upload_params(package_name, file = 'npm/payload.json')
+    def upload_params(package_name:, package_version: '1.0.1', file: 'npm/payload.json')
       JSON.parse(
         fixture_file(file, dir: 'ee')
-          .gsub('@root/npm-test', package_name))
+          .gsub('@root/npm-test', package_name)
+          .gsub('1.0.1', package_version))
     end
   end
 

ee/spec/services/packages/npm/create_package_service_spec.rb

@@ -5,7 +5,7 @@ describe Packages::Npm::CreatePackageService do
   let(:namespace) {create(:namespace)}
   let(:project) { create(:project, namespace: namespace) }
   let(:user) { create(:user) }
-  let(:version) { '1.0.1'.freeze }
+  let(:version) { '1.0.1' }
 
   let(:params) do
     JSON.parse(
@@ -37,7 +37,6 @@ describe Packages::Npm::CreatePackageService do
       expect(package.version).to eq(version)
     end
 
-    it { is_expected.to be_valid }
     it { expect(subject.name).to eq(package_name) }
     it { expect(subject.version).to eq(version) }
   end
@@ -77,5 +76,23 @@ describe Packages::Npm::CreatePackageService do
       it { expect(subject[:http_status]).to eq 400 }
       it { expect(subject[:message]).to eq 'Version is empty.' }
     end
+
+    context 'with invalid versions' do
+      using RSpec::Parameterized::TableSyntax
+
+      where(:version) do
+        [
+          '1',
+          '1.2',
+          '1./2.3',
+          '../../../../../1.2.3',
+          '%2e%2e%2f1.2.3'
+        ]
+      end
+
+      with_them do
+        it { expect { subject }.to raise_error(ActiveRecord::RecordInvalid, 'Validation failed: Version is invalid') }
+      end
+    end
   end
 end

ee/spec/services/packages/npm/create_tag_service_spec.rb

@@ -37,7 +37,7 @@ describe Packages::Npm::CreateTagService do
       end
 
       context 'on same package with different version' do
-        let!(:package2) { create(:npm_package, project: package.project, name: package.name, version: '5.0.0testing') }
+        let!(:package2) { create(:npm_package, project: package.project, name: package.name, version: '5.0.0-testing') }
 
         it { expect { subject }.to not_change { Packages::Tag.count } }
         it { expect(subject.name).to eq(tag_name) }

lib/api/helpers.rb

@@ -367,6 +367,10 @@ module API
       render_api_error!('405 Method Not Allowed', 405)
     end
 
+    def not_acceptable!
+      render_api_error!('406 Not Acceptable', 406)
+    end
+
     def service_unavailable!
       render_api_error!('503 Service Unavailable', 503)
     end

lib/api/repositories.rb

@@ -95,6 +95,8 @@ module API
           render_api_error!({ error: ::Gitlab::RateLimitHelpers::ARCHIVE_RATE_LIMIT_REACHED_MESSAGE }, 429)
         end
 
+        not_acceptable! if Gitlab::HotlinkingDetector.intercept_hotlinking?(request)
+
         send_git_archive user_project.repository, ref: params[:sha], format: params[:format], append_sha: true
       rescue
         not_found!('File')

lib/api/snippets.rb

@@ -74,6 +74,8 @@ module API
                               desc: 'The visibility of the snippet'
       end
       post do
+        authorize! :create_snippet
+
         attrs = declared_params(include_missing: false).merge(request: request, api: true)
         service_response = ::Snippets::CreateService.new(nil, current_user, attrs).execute
         snippet = service_response.payload[:snippet]

lib/api/triggers.rb

@@ -109,6 +109,8 @@ module API
         trigger = user_project.triggers.find(params.delete(:trigger_id))
         break not_found!('Trigger') unless trigger
 
+        authorize! :admin_trigger, trigger
+
         if trigger.update(declared_params(include_missing: false))
           present trigger, with: Entities::Trigger, current_user: current_user
         else

lib/gitlab/auth.rb

@@ -171,6 +171,8 @@ module Gitlab
 
           if valid_oauth_token?(token)
             user = User.find_by(id: token.resource_owner_id)
+            return unless user.can?(:log_in)
+
             Gitlab::Auth::Result.new(user, nil, :oauth, full_authentication_abilities)
           end
         end
@@ -182,7 +184,7 @@ module Gitlab
 
         token = PersonalAccessTokensFinder.new(state: 'active').find_by_token(password)
 
-        if token && valid_scoped_token?(token, all_available_scopes)
+        if token && valid_scoped_token?(token, all_available_scopes) && token.user.can?(:log_in)
           Gitlab::Auth::Result.new(token.user, nil, :personal_access_token, abilities_for_scopes(token.scopes))
         end
       end
@@ -260,6 +262,8 @@ module Gitlab
         return unless build.project.builds_enabled?
 
         if build.user
+          return unless build.user.can?(:log_in)
+
           # If user is assigned to build, use restricted credentials of user
           Gitlab::Auth::Result.new(build.user, build.project, :build, build_authentication_abilities)
         else

lib/gitlab/gfm/uploads_rewriter.rb

@@ -22,6 +22,8 @@ module Gitlab
         return @text unless needs_rewrite?
 
         @text.gsub(@pattern) do |markdown|
+          Gitlab::Utils.check_path_traversal!($~[:file])
+
           file = find_file(@source_project, $~[:secret], $~[:file])
           break markdown unless file.try(:exists?)
 

lib/gitlab/hotlinking_detector.rb

+# frozen_string_literal: true
+
+module Gitlab
+  class HotlinkingDetector
+    IMAGE_FORMATS = %w(image/jpeg image/apng image/png image/webp image/svg+xml image/*).freeze
+    MEDIA_FORMATS = %w(video/webm video/ogg video/* application/ogg audio/webm audio/ogg audio/wav audio/*).freeze
+    CSS_FORMATS = %w(text/css).freeze
+    INVALID_FORMATS = (IMAGE_FORMATS + MEDIA_FORMATS + CSS_FORMATS).freeze
+    INVALID_FETCH_MODES = %w(cors no-cors websocket).freeze
+
+    class << self
+      def intercept_hotlinking?(request)
+        request_accepts = parse_request_accepts(request)
+
+        return false unless Feature.enabled?(:repository_archive_hotlinking_interception, default_enabled: true)
+
+        # Block attempts to embed as JS
+        return true if sec_fetch_invalid?(request)
+
+        # If no Accept header was set, skip the rest
+        return false if request_accepts.empty?
+
+        # Workaround for IE8 weirdness
+        return false if IMAGE_FORMATS.include?(request_accepts.first) && request_accepts.include?("application/x-ms-application")
+
+        # Block all other media requests if the first format is a media type
+        return true if INVALID_FORMATS.include?(request_accepts.first)
+
+        false
+      end
+
+      private
+
+      def sec_fetch_invalid?(request)
+        fetch_mode = request.headers["Sec-Fetch-Mode"]
+
+        return if fetch_mode.blank?
+        return true if INVALID_FETCH_MODES.include?(fetch_mode)
+      end
+
+      def parse_request_accepts(request)
+        # Rails will already have parsed the Accept header
+        return request.accepts if request.respond_to?(:accepts)
+
+        # Grape doesn't parse it, so we can use the Rails system for this
+        return Mime::Type.parse(request.headers["Accept"]) if request.respond_to?(:headers) && request.headers["Accept"].present?
+
+        []
+      end
+    end
+  end
+end

lib/gitlab/import_export/attribute_cleaner.rb

@@ -11,7 +11,14 @@ module Gitlab
         'discussion_id',
         'custom_attributes'
       ].freeze
-      PROHIBITED_REFERENCES = Regexp.union(/\Acached_markdown_version\Z/, /_id\Z/, /_ids\Z/, /_html\Z/, /attributes/).freeze
+      PROHIBITED_REFERENCES = Regexp.union(
+        /\Acached_markdown_version\Z/,
+        /_id\Z/,
+        /_ids\Z/,
+        /_html\Z/,
+        /attributes/,
+        /\Aremote_\w+_(url|urls|request_header)\Z/ # carrierwave automatically creates these attribute methods for uploads
+      ).freeze
 
       def self.clean(*args)
         new(*args).clean

lib/gitlab/regex.rb

@@ -16,6 +16,14 @@ module Gitlab
       "It must start with letter, digit, emoji or '_'."
     end
 
+    def group_name_regex
+      project_name_regex
+    end
+
+    def group_name_regex_message
+      project_name_regex_message
+    end
+
     ##
     # Docker Distribution Registry repository / tag name rules
     #

locale/gitlab.pot

@@ -12860,6 +12860,9 @@ msgstr ""
 msgid "Mirror repository"
 msgstr ""
 
+msgid "Mirror settings are only available to GitLab administrators."
+msgstr ""
+
 msgid "Mirror user"
 msgstr ""
 
@@ -18862,6 +18865,9 @@ msgstr ""
 msgid "Specified URL cannot be used."
 msgstr ""
 
+msgid "Specified URL cannot be used: \"%{reason}\""
+msgstr ""
+
 msgid "Specify an e-mail address regex pattern to identify default internal users."
 msgstr ""
 

spec/controllers/groups_controller_spec.rb

@@ -258,6 +258,18 @@ describe GroupsController do
         end
       end
     end
+
+    context "malicious group name" do
+      subject { post :create, params: { group: { name: "<script>alert('Mayday!');</script>", path: "invalid_group_url" } } }
+
+      before do
+        sign_in(user)
+      end
+
+      it { expect { subject }.not_to change { Group.count } }
+
+      it { expect(subject).to render_template(:new) }
+    end
   end
 
   describe 'GET #index' do
@@ -836,6 +848,16 @@ describe GroupsController do
             put :update, params: { id: group.to_param, group: { name: 'world' } }
           end.to change { group.reload.name }
         end
+
+        context "malicious group name" do
+          subject { put :update, params: { id: group.to_param, group: { name: "<script>alert('Attack!');</script>" } } }
+
+          it { is_expected.to render_template(:edit) }
+
+          it 'does not update name' do
+            expect { subject }.not_to change { group.reload.name }
+          end
+        end
       end
 
       describe 'DELETE #destroy' do

spec/controllers/import/fogbugz_controller_spec.rb

@@ -25,6 +25,35 @@ describe Import::FogbugzController do
       expect(session[:fogbugz_uri]).to eq(uri)
       expect(response).to redirect_to(new_user_map_import_fogbugz_path)
     end
+
+    context 'verify url' do
+      shared_examples 'denies local request' do |reason|
+        it 'does not allow requests' do
+          post :callback, params: { uri: uri, email: 'test@example.com', password: 'mypassword' }
+
+          expect(response).to redirect_to(new_import_fogbugz_url)
+          expect(flash[:alert]).to eq("Specified URL cannot be used: \"#{reason}\"")
+        end
+      end
+
+      context 'when host is localhost' do
+        let(:uri) { 'https://localhost:3000' }
+
+        include_examples 'denies local request', 'Requests to localhost are not allowed'
+      end
+
+      context 'when host is on local network' do
+        let(:uri) { 'http://192.168.0.1/' }
+
+        include_examples 'denies local request', 'Requests to the local network are not allowed'
+      end
+
+      context 'when host is ftp protocol' do
+        let(:uri) { 'ftp://testing' }
+
+        include_examples 'denies local request', 'Only allowed schemes are http, https'
+      end
+    end
   end
 
   describe 'POST #create_user_map' do

spec/controllers/projects/mirrors_controller_spec.rb

@@ -5,6 +5,72 @@ require 'spec_helper'
 describe Projects::MirrorsController do
   include ReactiveCachingHelpers
 
+  shared_examples 'only admin is allowed when mirroring is disabled' do
+    let(:subject_action) { raise 'subject_action is required' }
+    let(:user) { project.owner }
+    let(:project_settings_path) { project_settings_repository_path(project, anchor: 'js-push-remote-settings') }
+
+    context 'when project mirroring is enabled' do
+      it 'allows requests from a maintainer' do
+        sign_in(user)
+
+        subject_action
+        expect(response).to redirect_to(project_settings_path)
+      end
+
+      it 'allows requests from an admin user' do
+        user.update!(admin: true)
+        sign_in(user)
+
+        subject_action
+        expect(response).to redirect_to(project_settings_path)
+      end
+    end
+
+    context 'when project mirroring is disabled' do
+      before do
+        stub_application_setting(mirror_available: false)
+      end
+
+      it 'disallows requests from a maintainer' do
+        sign_in(user)
+
+        subject_action
+        expect(response).to have_gitlab_http_status(:not_found)
+      end
+
+      it 'allows requests from an admin user' do
+        user.update!(admin: true)
+        sign_in(user)
+
+        subject_action
+        expect(response).to redirect_to(project_settings_path)
+      end
+    end
+  end
+
+  describe 'Access control' do
+    let(:project) { create(:project, :repository) }
+
+    describe '#update' do
+      include_examples 'only admin is allowed when mirroring is disabled' do
+        let(:subject_action) do
+          do_put(project, remote_mirrors_attributes: { '0' => { 'enabled' => 1, 'url' => 'http://foo.com' } })
+        end
+      end
+    end
+
+    describe '#update_now' do
+      include_examples 'only admin is allowed when mirroring is disabled' do
+        let(:options) { { namespace_id: project.namespace, project_id: project } }
+
+        let(:subject_action) do
+          get :update_now, params: options.merge(sync_remote: true)
+        end
+      end
+    end
+  end
+
   describe 'setting up a remote mirror' do
     let_it_be(:project) { create(:project, :repository) }
 

spec/controllers/projects/repositories_controller_spec.rb

@@ -28,6 +28,12 @@ describe Projects::RepositoriesController do
         sign_in(user)
       end
 
+      it_behaves_like "hotlink interceptor" do
+        let(:http_request) do
+          get :archive, params: { namespace_id: project.namespace, project_id: project, id: "master" }, format: "zip"
+        end
+      end
+
       it "uses Gitlab::Workhorse" do
         get :archive, params: { namespace_id: project.namespace, project_id: project, id: "master" }, format: "zip"
 

spec/features/projects/settings/repository_settings_spec.rb

@@ -26,11 +26,7 @@ describe 'Projects > Settings > Repository settings' do
     let(:role) { :maintainer }
 
     context 'remote mirror settings' do
-      let(:user2) { create(:user) }
-
       before do
-        project.add_maintainer(user2)
-
         visit project_settings_repository_path(project)
       end
 
@@ -90,6 +86,18 @@ describe 'Projects > Settings > Repository settings' do
         expect(page).to have_selector('[title="Copy SSH public key"]')
       end
 
+      context 'when project mirroring is disabled' do
+        before do
+          stub_application_setting(mirror_available: false)
+          visit project_settings_repository_path(project)
+        end
+
+        it 'hides remote mirror settings' do
+          expect(page.find('.project-mirror-settings')).not_to have_selector('form')
+          expect(page).to have_content('Mirror settings are only available to GitLab administrators.')
+        end
+      end
+
       def select_direction(direction = 'push')
         direction_select = find('#mirror_direction')
 
@@ -154,4 +162,31 @@ describe 'Projects > Settings > Repository settings' do
       expect(mirror).not_to have_selector('.rspec-update-now-button')
     end
   end
+
+  context 'for admin' do
+    shared_examples_for 'shows mirror settings' do
+      it 'shows mirror settings' do
+        expect(page.find('.project-mirror-settings')).to have_selector('form')
+        expect(page).not_to have_content('Changing mirroring setting is disabled for non-admin users.')
+      end
+    end
+
+    before do
+      stub_application_setting(mirror_available: mirror_available)
+      user.update!(admin: true)
+      visit project_settings_repository_path(project)
+    end
+
+    context 'when project mirroring is enabled' do
+      let(:mirror_available) { true }
+
+      include_examples 'shows mirror settings'
+    end
+
+    context 'when project mirroring is disabled' do
+      let(:mirror_available) { false }
+
+      include_examples 'shows mirror settings'
+    end
+  end
 end

spec/javascripts/frequent_items/utils_spec.js

@@ -108,5 +108,23 @@ describe('Frequent Items utils spec', () => {
 
       expect(sanitizeItem(input)).toEqual({ name: 'test', namespace: 'test', id: 1 });
     });
+
+    it("skips `name` key if it doesn't exist on the item", () => {
+      const input = {
+        namespace: '<br>test',
+        id: 1,
+      };
+
+      expect(sanitizeItem(input)).toEqual({ namespace: 'test', id: 1 });
+    });
+
+    it("skips `namespace` key if it doesn't exist on the item", () => {
+      const input = {
+        name: '<br><b>test</b>',
+        id: 1,
+      };
+
+      expect(sanitizeItem(input)).toEqual({ name: 'test', id: 1 });
+    });
   });
 });

spec/lib/banzai/filter/label_reference_filter_spec.rb

@@ -523,7 +523,12 @@ describe Banzai::Filter::LabelReferenceFilter do
     end
 
     context 'when group name has HTML entities' do
-      let(:another_group) { create(:group, name: '<img src=x onerror=alert(1)>', path: 'another_group') }
+      let(:another_group) { create(:group, name: 'random', path: 'another_group') }
+
+      before do
+        another_group.name = "<img src=x onerror=alert(1)>"
+        another_group.save!(validate: false)
+      end
 
       it 'escapes the HTML entities' do
         expect(result.text)

spec/lib/banzai/filter/reference_redactor_filter_spec.rb

@@ -20,8 +20,8 @@ describe Banzai::Filter::ReferenceRedactorFilter do
   it 'skips when the skip_redaction flag is set' do
     user = create(:user)
     project = create(:project)
-
     link = reference_link(project: project.id, reference_type: 'test')
+
     doc = filter(link, current_user: user, skip_redaction: true)
 
     expect(doc.css('a').length).to eq 1
@@ -51,8 +51,8 @@ describe Banzai::Filter::ReferenceRedactorFilter do
         user = create(:user)
         project = create(:project)
         project.add_maintainer(user)
-
         link = reference_link(project: project.id, reference_type: 'test')
+
         doc = filter(link, current_user: user)
 
         expect(doc.css('a').length).to eq 1
@@ -69,8 +69,8 @@ describe Banzai::Filter::ReferenceRedactorFilter do
       it 'removes unpermitted references' do
         user = create(:user)
         project = create(:project)
-
         link = reference_link(project: project.id, reference_type: 'test')
+
         doc = filter(link, current_user: user)
 
         expect(doc.css('a').length).to eq 0
@@ -90,8 +90,8 @@ describe Banzai::Filter::ReferenceRedactorFilter do
         non_member = create(:user)
         project = create(:project, :public)
         issue = create(:issue, :confidential, project: project)
-
         link = reference_link(project: project.id, issue: issue.id, reference_type: 'issue')
+
         doc = filter(link, current_user: non_member)
 
         expect(doc.css('a').length).to eq 0
@@ -124,8 +124,8 @@ describe Banzai::Filter::ReferenceRedactorFilter do
         assignee = create(:user)
         project = create(:project, :public)
         issue = create(:issue, :confidential, project: project, assignees: [assignee])
-
         link = reference_link(project: project.id, issue: issue.id, reference_type: 'issue')
+
         doc = filter(link, current_user: assignee)
 
         expect(doc.css('a').length).to eq 1
@@ -136,8 +136,8 @@ describe Banzai::Filter::ReferenceRedactorFilter do
         project = create(:project, :public)
         project.add_developer(member)
         issue = create(:issue, :confidential, project: project)
-
         link = reference_link(project: project.id, issue: issue.id, reference_type: 'issue')
+
         doc = filter(link, current_user: member)
 
         expect(doc.css('a').length).to eq 1
@@ -147,20 +147,62 @@ describe Banzai::Filter::ReferenceRedactorFilter do
         admin = create(:admin)
         project = create(:project, :public)
         issue = create(:issue, :confidential, project: project)
-
         link = reference_link(project: project.id, issue: issue.id, reference_type: 'issue')
+
         doc = filter(link, current_user: admin)
 
         expect(doc.css('a').length).to eq 1
       end
+
+      context "when a confidential issue is moved from a public project to a private one" do
+        let(:public_project) { create(:project, :public) }
+        let(:private_project) { create(:project, :private) }
+
+        it 'removes references for author' do
+          author = create(:user)
+          issue = create(:issue, :confidential, project: public_project, author: author)
+          issue.update!(project: private_project) # move issue to private project
+          link = reference_link(project: private_project.id, issue: issue.id, reference_type: 'issue')
+
+          doc = filter(link, current_user: author)
+
+          expect(doc.css('a').length).to eq 0
+        end
+
+        it 'removes references for assignee' do
+          assignee = create(:user)
+          issue = create(:issue, :confidential, project: public_project, assignees: [assignee])
+          issue.update!(project: private_project) # move issue to private project
+          link = reference_link(project: private_project.id, issue: issue.id, reference_type: 'issue')
+
+          doc = filter(link, current_user: assignee)
+
+          expect(doc.css('a').length).to eq 0
+        end
+
+        it 'allows references for project members' do
+          member = create(:user)
+          project = create(:project, :public)
+          project_2 = create(:project, :private)
+          project.add_developer(member)
+          project_2.add_developer(member)
+          issue = create(:issue, :confidential, project: project)
+          issue.update!(project: project_2) # move issue to private project
+          link = reference_link(project: project_2.id, issue: issue.id, reference_type: 'issue')
+
+          doc = filter(link, current_user: member)
+
+          expect(doc.css('a').length).to eq 1
+        end
+      end
     end
 
     it 'allows references for non confidential issues' do
       user = create(:user)
       project = create(:project, :public)
       issue = create(:issue, project: project)
-
       link = reference_link(project: project.id, issue: issue.id, reference_type: 'issue')
+
       doc = filter(link, current_user: user)
 
       expect(doc.css('a').length).to eq 1
@@ -172,8 +214,8 @@ describe Banzai::Filter::ReferenceRedactorFilter do
       it 'removes unpermitted Group references' do
         user = create(:user)
         group = create(:group, :private)
-
         link = reference_link(group: group.id, reference_type: 'user')
+
         doc = filter(link, current_user: user)
 
         expect(doc.css('a').length).to eq 0
@@ -183,8 +225,8 @@ describe Banzai::Filter::ReferenceRedactorFilter do
         user = create(:user)
         group = create(:group, :private)
         group.add_developer(user)
-
         link = reference_link(group: group.id, reference_type: 'user')
+
         doc = filter(link, current_user: user)
 
         expect(doc.css('a').length).to eq 1
@@ -200,8 +242,8 @@ describe Banzai::Filter::ReferenceRedactorFilter do
     context 'with data-user' do
       it 'allows any User reference' do
         user = create(:user)
-
         link = reference_link(user: user.id, reference_type: 'user')
+
         doc = filter(link)
 
         expect(doc.css('a').length).to eq 1

spec/lib/gitlab/auth_spec.rb

@@ -164,6 +164,12 @@ describe Gitlab::Auth, :use_clean_rails_memory_store_caching do
 
           expect(subject).to eq(Gitlab::Auth::Result.new(build.user, build.project, :build, described_class.build_authentication_abilities))
         end
+
+        it 'fails with blocked user token' do
+          build.update(user: create(:user, :blocked))
+
+          expect(subject).to eq(Gitlab::Auth::Result.new(nil, nil, nil, nil))
+        end
       end
 
       (HasStatus::AVAILABLE_STATUSES - ['running']).each do |build_status|
@@ -259,6 +265,15 @@ describe Gitlab::Auth, :use_clean_rails_memory_store_caching do
 
         gl_auth.find_for_git_client("oauth2", token_w_api_scope.token, project: nil, ip: 'ip')
       end
+
+      context 'blocked user' do
+        let(:user) { create(:user, :blocked) }
+
+        it 'fails' do
+          expect(gl_auth.find_for_git_client("oauth2", token_w_api_scope.token, project: nil, ip: 'ip'))
+            .to eq(Gitlab::Auth::Result.new(nil, nil, nil, nil))
+        end
+      end
     end
 
     context 'while using personal access tokens as passwords' do
@@ -307,9 +322,35 @@ describe Gitlab::Auth, :use_clean_rails_memory_store_caching do
       it 'fails if password is nil' do
         expect_results_with_abilities(nil, nil, false)
       end
+
+      context 'when user is blocked' do
+        let(:user) { create(:user, :blocked) }
+        let(:personal_access_token) { create(:personal_access_token, scopes: ['read_registry'], user: user) }
+
+        before do
+          stub_container_registry_config(enabled: true)
+        end
+
+        it 'fails if user is blocked' do
+          expect(gl_auth.find_for_git_client('', personal_access_token.token, project: nil, ip: 'ip'))
+          .to eq(Gitlab::Auth::Result.new(nil, nil, nil, nil))
+        end
+      end
     end
 
     context 'while using regular user and password' do
+      it 'fails for a blocked user' do
+        user = create(
+          :user,
+          :blocked,
+          username: 'normal_user',
+          password: 'my-secret'
+        )
+
+        expect(gl_auth.find_for_git_client(user.username, user.password, project: nil, ip: 'ip'))
+          .to eq(Gitlab::Auth::Result.new(nil, nil, nil, nil))
+      end
+
       it 'goes through lfs authentication' do
         user = create(
           :user,

spec/lib/gitlab/gfm/uploads_rewriter_spec.rb

@@ -68,6 +68,16 @@ describe Gitlab::Gfm::UploadsRewriter do
       expect(moved_text.scan(/\A\[.*?\]/).count).to eq(1)
     end
 
+    context 'path traversal in file name' do
+      let(:text) do
+        "![a](/uploads/11111111111111111111111111111111/../../../../../../../../../../../../../../etc/passwd)"
+      end
+
+      it 'throw an error' do
+        expect { rewriter.rewrite(new_project) }.to raise_error(an_instance_of(StandardError).and having_attributes(message: "Invalid path"))
+      end
+    end
+
     context "file are stored locally" do
       include_examples "files are accessible"
     end

spec/lib/gitlab/hotlinking_detector_spec.rb

+# frozen_string_literal: true
+
+require "spec_helper"
+
+RSpec.describe Gitlab::HotlinkingDetector do
+  describe ".intercept_hotlinking?" do
+    using RSpec::Parameterized::TableSyntax
+
+    subject { described_class.intercept_hotlinking?(request) }
+
+    let(:request) { double("request", headers: headers) }
+    let(:headers) { {} }
+
+    context "hotlinked as media" do
+      where(:return_value, :accept_header) do
+        # These are default formats in modern browsers, and IE
+        false | "*/*"
+        false | "text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8"
+        false | "text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,*/*;q=0.8"
+        false | "text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8"
+        false | "text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,image/apng,*/*;q=0.8"
+        false | "image/jpeg, application/x-ms-application, image/gif, application/xaml+xml, image/pjpeg, application/x-ms-xbap, application/x-shockwave-flash, application/msword, */*"
+        false | "text/html, application/xhtml+xml, image/jxr, */*"
+        false | "text/html, application/xml;q=0.9, application/xhtml+xml, image/png, image/webp, image/jpeg, image/gif, image/x-xbitmap, */*;q=0.1"
+
+        # These are image request formats
+        true | "image/webp,*/*"
+        true | "image/png,image/*;q=0.8,*/*;q=0.5"
+        true | "image/webp,image/apng,image/*,*/*;q=0.8"
+        true | "image/png,image/svg+xml,image/*;q=0.8, */*;q=0.5"
+
+        # Video request formats
+        true | "video/webm,video/ogg,video/*;q=0.9,application/ogg;q=0.7,audio/*;q=0.6,*/*;q=0.5"
+
+        # Audio request formats
+        true | "audio/webm,audio/ogg,audio/wav,audio/*;q=0.9,application/ogg;q=0.7,video/*;q=0.6,*/*;q=0.5"
+
+        # CSS request formats
+        true | "text/css,*/*;q=0.1"
+        true | "text/css"
+        true | "text/css,*/*;q=0.1"
+      end
+
+      with_them do
+        let(:headers) do
+          { "Accept" => accept_header }
+        end
+
+        it { is_expected.to be(return_value) }
+      end
+    end
+
+    context "hotlinked as a script" do
+      where(:return_value, :fetch_mode) do
+        # Standard navigation fetch modes
+        false | "navigate"
+        false | "nested-navigate"
+        false | "same-origin"
+
+        # Fetch modes when linking as JS
+        true | "cors"
+        true | "no-cors"
+        true | "websocket"
+      end
+
+      with_them do
+        let(:headers) do
+          { "Sec-Fetch-Mode" => fetch_mode }
+        end
+
+        it { is_expected.to be(return_value) }
+      end
+    end
+  end
+end

spec/lib/gitlab/import_export/attribute_cleaner_spec.rb

@@ -32,6 +32,9 @@ describe Gitlab::ImportExport::AttributeCleaner do
       'issue_ids' => [1, 2, 3],
       'merge_request_ids' => [1, 2, 3],
       'note_ids' => [1, 2, 3],
+      'remote_attachment_url' => 'http://something.dodgy',
+      'remote_attachment_request_header' => 'bad value',
+      'remote_attachment_urls' => %w(http://something.dodgy http://something.okay),
       'attributes' => {
         'issue_ids' => [1, 2, 3],
         'merge_request_ids' => [1, 2, 3],

spec/lib/gitlab/regex_spec.rb

@@ -3,9 +3,7 @@
 require 'spec_helper'
 
 describe Gitlab::Regex do
-  describe '.project_name_regex' do
-    subject { described_class.project_name_regex }
-
+  shared_examples_for 'project/group name regex' do
     it { is_expected.to match('gitlab-ce') }
     it { is_expected.to match('GitLab CE') }
     it { is_expected.to match('100 lines') }
@@ -15,6 +13,34 @@ describe Gitlab::Regex do
     it { is_expected.not_to match('?gitlab') }
   end
 
+  shared_examples_for 'project/group name error message' do
+    it { is_expected.to eq("can contain only letters, digits, emojis, '_', '.', dash, space. It must start with letter, digit, emoji or '_'.") }
+  end
+
+  describe '.project_name_regex' do
+    subject { described_class.project_name_regex }
+
+    it_behaves_like 'project/group name regex'
+  end
+
+  describe '.group_name_regex' do
+    subject { described_class.group_name_regex }
+
+    it_behaves_like 'project/group name regex'
+  end
+
+  describe '.project_name_regex_message' do
+    subject { described_class.project_name_regex_message }
+
+    it_behaves_like 'project/group name error message'
+  end
+
+  describe '.group_name_regex_message' do
+    subject { described_class.group_name_regex_message }
+
+    it_behaves_like 'project/group name error message'
+  end
+
   describe '.environment_name_regex' do
     subject { described_class.environment_name_regex }
 

spec/models/group_spec.rb

@@ -48,6 +48,9 @@ describe Group do
 
   describe 'validations' do
     it { is_expected.to validate_presence_of :name }
+    it { is_expected.to allow_value('group test_4').for(:name) }
+    it { is_expected.not_to allow_value('test/../foo').for(:name) }
+    it { is_expected.not_to allow_value('<script>alert("Attack!")</script>').for(:name) }
     it { is_expected.to validate_presence_of :path }
     it { is_expected.not_to validate_presence_of :owner }
     it { is_expected.to validate_presence_of :two_factor_grace_period }

spec/models/issue_spec.rb

@@ -529,222 +529,258 @@ describe Issue do
   end
 
   describe '#visible_to_user?' do
+    let(:project) { build(:project) }
+    let(:issue)   { build(:issue, project: project) }
+    let(:user)    { create(:user) }
+
+    subject { issue.visible_to_user?(user) }
+
+    context 'with a project' do
+      it 'returns false when feature is disabled' do
+        project.project_feature.update_attribute(:issues_access_level, ProjectFeature::DISABLED)
+
+        is_expected.to eq(false)
+      end
+
+      it 'returns false when restricted for members' do
+        project.project_feature.update_attribute(:issues_access_level, ProjectFeature::PRIVATE)
+
+        is_expected.to eq(false)
+      end
+    end
+
     context 'without a user' do
-      let(:issue) { build(:issue) }
+      let(:user) { nil }
 
       it 'returns true when the issue is publicly visible' do
         expect(issue).to receive(:publicly_visible?).and_return(true)
 
-        expect(issue.visible_to_user?).to eq(true)
+        is_expected.to eq(true)
       end
 
       it 'returns false when the issue is not publicly visible' do
         expect(issue).to receive(:publicly_visible?).and_return(false)
 
-        expect(issue.visible_to_user?).to eq(false)
+        is_expected.to eq(false)
       end
     end
 
     context 'with a user' do
-      let(:user) { create(:user) }
-      let(:issue) { build(:issue) }
-
-      it 'returns true when the issue is readable' do
-        expect(issue).to receive(:readable_by?).with(user).and_return(true)
-
-        expect(issue.visible_to_user?(user)).to eq(true)
+      shared_examples 'issue readable by user' do
+        it { is_expected.to eq(true) }
       end
 
-      it 'returns false when the issue is not readable' do
-        expect(issue).to receive(:readable_by?).with(user).and_return(false)
-
-        expect(issue.visible_to_user?(user)).to eq(false)
+      shared_examples 'issue not readable by user' do
+        it { is_expected.to eq(false) }
       end
 
-      it 'returns false when feature is disabled' do
-        expect(issue).not_to receive(:readable_by?)
-
-        issue.project.project_feature.update_attribute(:issues_access_level, ProjectFeature::DISABLED)
+      shared_examples 'confidential issue readable by user' do
+        specify do
+          issue.confidential = true
 
-        expect(issue.visible_to_user?(user)).to eq(false)
+          is_expected.to eq(true)
+        end
       end
 
-      it 'returns false when restricted for members' do
-        expect(issue).not_to receive(:readable_by?)
-
-        issue.project.project_feature.update_attribute(:issues_access_level, ProjectFeature::PRIVATE)
+      shared_examples 'confidential issue not readable by user' do
+        specify do
+          issue.confidential = true
 
-        expect(issue.visible_to_user?(user)).to eq(false)
+          is_expected.to eq(false)
+        end
       end
-    end
-
-    describe 'with a regular user that is not a team member' do
-      let(:user) { create(:user) }
 
-      context 'using a public project' do
-        let(:project) { create(:project, :public) }
+      context 'with an admin user' do
+        let(:user) { build(:admin) }
 
-        it 'returns true for a regular issue' do
-          issue = build(:issue, project: project)
+        it_behaves_like 'issue readable by user'
+        it_behaves_like 'confidential issue readable by user'
+      end
 
-          expect(issue.visible_to_user?(user)).to eq(true)
+      context 'with an owner' do
+        before do
+          project.add_maintainer(user)
         end
 
-        it 'returns false for a confidential issue' do
-          issue = build(:issue, project: project, confidential: true)
+        it_behaves_like 'issue readable by user'
+        it_behaves_like 'confidential issue readable by user'
+      end
 
-          expect(issue.visible_to_user?(user)).to eq(false)
+      context 'with a reporter user' do
+        before do
+          project.add_reporter(user)
         end
+
+        it_behaves_like 'issue readable by user'
+        it_behaves_like 'confidential issue readable by user'
       end
 
-      context 'using an internal project' do
-        let(:project) { create(:project, :internal) }
+      context 'with a guest user' do
+        before do
+          project.add_guest(user)
+        end
 
-        context 'using an internal user' do
-          it 'returns true for a regular issue' do
-            issue = build(:issue, project: project)
+        it_behaves_like 'issue readable by user'
+        it_behaves_like 'confidential issue not readable by user'
 
-            expect(issue.visible_to_user?(user)).to eq(true)
+        context 'when user is an assignee' do
+          before do
+            issue.update!(assignees: [user])
           end
 
-          it 'returns false for a confidential issue' do
-            issue = build(:issue, :confidential, project: project)
-
-            expect(issue.visible_to_user?(user)).to eq(false)
-          end
+          it_behaves_like 'issue readable by user'
+          it_behaves_like 'confidential issue readable by user'
         end
 
-        context 'using an external user' do
+        context 'when user is the author' do
           before do
-            allow(user).to receive(:external?).and_return(true)
-          end
-
-          it 'returns false for a regular issue' do
-            issue = build(:issue, project: project)
-
-            expect(issue.visible_to_user?(user)).to eq(false)
+            issue.update!(author: user)
           end
 
-          it 'returns false for a confidential issue' do
-            issue = build(:issue, :confidential, project: project)
-
-            expect(issue.visible_to_user?(user)).to eq(false)
-          end
+          it_behaves_like 'issue readable by user'
+          it_behaves_like 'confidential issue readable by user'
         end
       end
 
-      context 'using a private project' do
-        let(:project) { create(:project, :private) }
-
-        it 'returns false for a regular issue' do
-          issue = build(:issue, project: project)
+      context 'with a user that is not a member' do
+        context 'using a public project' do
+          let(:project) { build(:project, :public) }
 
-          expect(issue.visible_to_user?(user)).to eq(false)
+          it_behaves_like 'issue readable by user'
+          it_behaves_like 'confidential issue not readable by user'
         end
 
-        it 'returns false for a confidential issue' do
-          issue = build(:issue, :confidential, project: project)
+        context 'using an internal project' do
+          let(:project) { build(:project, :internal) }
 
-          expect(issue.visible_to_user?(user)).to eq(false)
-        end
+          context 'using an internal user' do
+            before do
+              allow(user).to receive(:external?).and_return(false)
+            end
 
-        context 'when the user is the project owner' do
-          before do
-            project.add_maintainer(user)
+            it_behaves_like 'issue readable by user'
+            it_behaves_like 'confidential issue not readable by user'
           end
 
-          it 'returns true for a regular issue' do
-            issue = build(:issue, project: project)
+          context 'using an external user' do
+            before do
+              allow(user).to receive(:external?).and_return(true)
+            end
 
-            expect(issue.visible_to_user?(user)).to eq(true)
+            it_behaves_like 'issue not readable by user'
+            it_behaves_like 'confidential issue not readable by user'
           end
+        end
 
-          it 'returns true for a confidential issue' do
-            issue = build(:issue, :confidential, project: project)
-
-            expect(issue.visible_to_user?(user)).to eq(true)
+        context 'using an external user' do
+          before do
+            allow(user).to receive(:external?).and_return(true)
           end
+
+          it_behaves_like 'issue not readable by user'
+          it_behaves_like 'confidential issue not readable by user'
         end
       end
-    end
-
-    context 'with a regular user that is a team member' do
-      let(:user) { create(:user) }
-      let(:project) { create(:project, :public) }
 
-      context 'using a public project' do
+      context 'with an external authentication service' do
         before do
-          project.add_developer(user)
+          enable_external_authorization_service_check
         end
 
-        it 'returns true for a regular issue' do
-          issue = build(:issue, project: project)
+        it 'is `false` when an external authorization service is enabled' do
+          issue = build(:issue, project: build(:project, :public))
 
-          expect(issue.visible_to_user?(user)).to eq(true)
+          expect(issue).not_to be_visible_to_user
         end
 
-        it 'returns true for a confidential issue' do
-          issue = build(:issue, :confidential, project: project)
-
-          expect(issue.visible_to_user?(user)).to eq(true)
-        end
-      end
-
-      context 'using an internal project' do
-        let(:project) { create(:project, :internal) }
+        it 'checks the external service to determine if an issue is readable by a user' do
+          project = build(:project, :public,
+                          external_authorization_classification_label: 'a-label')
+          issue = build(:issue, project: project)
+          user = build(:user)
 
-        before do
-          project.add_developer(user)
+          expect(::Gitlab::ExternalAuthorization).to receive(:access_allowed?).with(user, 'a-label') { false }
+          expect(issue.visible_to_user?(user)).to be_falsy
         end
 
-        it 'returns true for a regular issue' do
+        it 'does not check the external service if a user does not have access to the project' do
+          project = build(:project, :private,
+                          external_authorization_classification_label: 'a-label')
           issue = build(:issue, project: project)
+          user = build(:user)
 
-          expect(issue.visible_to_user?(user)).to eq(true)
+          expect(::Gitlab::ExternalAuthorization).not_to receive(:access_allowed?)
+          expect(issue.visible_to_user?(user)).to be_falsy
         end
 
-        it 'returns true for a confidential issue' do
-          issue = build(:issue, :confidential, project: project)
+        it 'does not check the external webservice for admins' do
+          issue = build(:issue)
+          user = build(:admin)
 
-          expect(issue.visible_to_user?(user)).to eq(true)
+          expect(::Gitlab::ExternalAuthorization).not_to receive(:access_allowed?)
+
+          issue.visible_to_user?(user)
         end
       end
 
-      context 'using a private project' do
-        let(:project) { create(:project, :private) }
+      context 'when issue is moved to a private project' do
+        let(:private_project) { build(:project, :private)}
 
         before do
-          project.add_developer(user)
+          issue.update(project: private_project) # move issue to private project
         end
 
-        it 'returns true for a regular issue' do
-          issue = build(:issue, project: project)
+        shared_examples 'issue visible if user has guest access' do
+          context 'when user is not a member' do
+            it_behaves_like 'issue not readable by user'
+            it_behaves_like 'confidential issue not readable by user'
+          end
 
-          expect(issue.visible_to_user?(user)).to eq(true)
+          context 'when user is a guest' do
+            before do
+              private_project.add_guest(user)
+            end
+
+            it_behaves_like 'issue readable by user'
+            it_behaves_like 'confidential issue readable by user'
+          end
         end
 
-        it 'returns true for a confidential issue' do
-          issue = build(:issue, :confidential, project: project)
+        context 'when user is the author of the original issue' do
+          before do
+            issue.update!(author: user)
+          end
 
-          expect(issue.visible_to_user?(user)).to eq(true)
+          it_behaves_like 'issue visible if user has guest access'
         end
-      end
-    end
 
-    context 'with an admin user' do
-      let(:project) { create(:project) }
-      let(:user) { create(:admin) }
+        context 'when user is an assignee in the original issue' do
+          before do
+            issue.update!(assignees: [user])
+          end
 
-      it 'returns true for a regular issue' do
-        issue = build(:issue, project: project)
+          it_behaves_like 'issue visible if user has guest access'
+        end
 
-        expect(issue.visible_to_user?(user)).to eq(true)
-      end
+        context 'when user is not the author or an assignee in original issue' do
+          context 'when user is a guest' do
+            before do
+              private_project.add_guest(user)
+            end
 
-      it 'returns true for a confidential issue' do
-        issue = build(:issue, :confidential, project: project)
+            it_behaves_like 'issue readable by user'
+            it_behaves_like 'confidential issue not readable by user'
+          end
 
-        expect(issue.visible_to_user?(user)).to eq(true)
+          context 'when user is a reporter' do
+            before do
+              private_project.add_reporter(user)
+            end
+
+            it_behaves_like 'issue readable by user'
+            it_behaves_like 'confidential issue readable by user'
+          end
+        end
       end
     end
   end
@@ -868,49 +904,6 @@ describe Issue do
     subject { create(:issue, updated_at: 1.hour.ago) }
   end
 
-  context 'when an external authentication service' do
-    before do
-      enable_external_authorization_service_check
-    end
-
-    describe '#visible_to_user?' do
-      it 'is `false` when an external authorization service is enabled' do
-        issue = build(:issue, project: build(:project, :public))
-
-        expect(issue).not_to be_visible_to_user
-      end
-
-      it 'checks the external service to determine if an issue is readable by a user' do
-        project = build(:project, :public,
-                        external_authorization_classification_label: 'a-label')
-        issue = build(:issue, project: project)
-        user = build(:user)
-
-        expect(::Gitlab::ExternalAuthorization).to receive(:access_allowed?).with(user, 'a-label') { false }
-        expect(issue.visible_to_user?(user)).to be_falsy
-      end
-
-      it 'does not check the external service if a user does not have access to the project' do
-        project = build(:project, :private,
-                        external_authorization_classification_label: 'a-label')
-        issue = build(:issue, project: project)
-        user = build(:user)
-
-        expect(::Gitlab::ExternalAuthorization).not_to receive(:access_allowed?)
-        expect(issue.visible_to_user?(user)).to be_falsy
-      end
-
-      it 'does not check the external webservice for admins' do
-        issue = build(:issue)
-        user = build(:admin)
-
-        expect(::Gitlab::ExternalAuthorization).not_to receive(:access_allowed?)
-
-        issue.visible_to_user?(user)
-      end
-    end
-  end
-
   describe "#labels_hook_attrs" do
     let(:label) { create(:label) }
     let(:issue) { create(:labeled_issue, labels: [label]) }

spec/policies/issue_policy_spec.rb

@@ -103,12 +103,24 @@ describe IssuePolicy do
         expect(permissions(author, confidential_issue_no_assignee)).to be_disallowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue)
       end
 
+      it 'does not allow issue author to read or update confidential issue moved to an private project' do
+        confidential_issue.project = build(:project, :private)
+
+        expect(permissions(author, confidential_issue)).to be_disallowed(:read_issue, :read_issue_iid, :update_issue)
+      end
+
       it 'allows issue assignees to read and update their confidential issues' do
         expect(permissions(assignee, confidential_issue)).to be_allowed(:read_issue, :read_issue_iid, :update_issue)
         expect(permissions(assignee, confidential_issue)).to be_disallowed(:admin_issue)
 
         expect(permissions(assignee, confidential_issue_no_assignee)).to be_disallowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue)
       end
+
+      it 'does not allow issue assignees to read or update confidential issue moved to an private project' do
+        confidential_issue.project = build(:project, :private)
+
+        expect(permissions(assignee, confidential_issue)).to be_disallowed(:read_issue, :read_issue_iid, :update_issue)
+      end
     end
   end
 

spec/requests/api/groups_spec.rb

@@ -642,6 +642,20 @@ describe API::Groups do
         expect(json_response['default_branch_protection']).to eq(::Gitlab::Access::MAINTAINER_PROJECT_ACCESS)
       end
 
+      context 'malicious group name' do
+        subject { put api("/groups/#{group1.id}", user1), params: { name: "<SCRIPT>alert('DOUBLE-ATTACK!')</SCRIPT>" } }
+
+        it 'returns bad request' do
+          subject
+
+          expect(response).to have_gitlab_http_status(:bad_request)
+        end
+
+        it 'does not update group name' do
+          expect { subject }.not_to change { group1.reload.name }
+        end
+      end
+
       it 'returns 404 for a non existing group' do
         put api('/groups/1328', user1), params: { name: new_group_name }
 
@@ -1083,6 +1097,20 @@ describe API::Groups do
         expect(json_response["parent_id"]).to eq(parent.id)
       end
 
+      context 'malicious group name' do
+        subject { post api("/groups", user3), params: group_params }
+
+        let(:group_params) { attributes_for_group_api name: "<SCRIPT>alert('ATTACKED!')</SCRIPT>", path: "unique-url" }
+
+        it 'returns bad request' do
+          subject
+
+          expect(response).to have_gitlab_http_status(:bad_request)
+        end
+
+        it { expect { subject }.not_to change { Group.count } }
+      end
+
       it "does not create group, duplicate" do
         post api("/groups", user3), params: { name: 'Duplicate Test', path: group2.path }
 

spec/requests/api/project_snippets_spec.rb

@@ -164,6 +164,30 @@ describe API::ProjectSnippets do
       end
     end
 
+    context 'with an external user' do
+      let(:user) { create(:user, :external) }
+
+      context 'that belongs to the project' do
+        before do
+          project.add_developer(user)
+        end
+
+        it 'creates a new snippet' do
+          post api("/projects/#{project.id}/snippets/", user), params: params
+
+          expect(response).to have_gitlab_http_status(:created)
+        end
+      end
+
+      context 'that does not belong to the project' do
+        it 'does not create a new snippet' do
+          post api("/projects/#{project.id}/snippets/", user), params: params
+
+          expect(response).to have_gitlab_http_status(:forbidden)
+        end
+      end
+    end
+
     context 'with a regular user' do
       let(:user) { create(:user) }
 

spec/requests/api/repositories_spec.rb

@@ -275,6 +275,18 @@ describe API::Repositories do
 
         expect(response).to have_gitlab_http_status(:too_many_requests)
       end
+
+      context "when hotlinking detection is enabled" do
+        before do
+          Feature.enable(:repository_archive_hotlinking_interception)
+        end
+
+        it_behaves_like "hotlink interceptor" do
+          let(:http_request) do
+            get api(route, current_user), headers: headers
+          end
+        end
+      end
     end
 
     context 'when unauthenticated', 'and project is public' do

spec/requests/api/snippets_spec.rb

@@ -266,6 +266,16 @@ describe API::Snippets do
 
     it_behaves_like 'snippet creation'
 
+    context 'with an external user' do
+      let(:user) { create(:user, :external) }
+
+      it 'does not create a new snippet' do
+        post api("/snippets/", user), params: params
+
+        expect(response).to have_gitlab_http_status(:forbidden)
+      end
+    end
+
     it 'returns 400 for missing parameters' do
       params.delete(:title)
 

spec/requests/api/triggers_spec.rb

@@ -238,24 +238,44 @@ describe API::Triggers do
   end
 
   describe 'PUT /projects/:id/triggers/:trigger_id' do
-    context 'authenticated user with valid permissions' do
-      let(:new_description) { 'new description' }
+    context 'user is maintainer of the project' do
+      context 'the trigger belongs to user' do
+        let(:new_description) { 'new description' }
 
-      it 'updates description' do
-        put api("/projects/#{project.id}/triggers/#{trigger.id}", user),
-          params: { description: new_description }
+        it 'updates description' do
+          put api("/projects/#{project.id}/triggers/#{trigger.id}", user),
+            params: { description: new_description }
 
-        expect(response).to have_gitlab_http_status(:ok)
-        expect(json_response).to include('description' => new_description)
-        expect(trigger.reload.description).to eq(new_description)
+          expect(response).to have_gitlab_http_status(:ok)
+          expect(json_response).to include('description' => new_description)
+          expect(trigger.reload.description).to eq(new_description)
+        end
+      end
+
+      context 'the trigger does not belong to user' do
+        it 'does not update trigger' do
+          put api("/projects/#{project.id}/triggers/#{trigger2.id}", user)
+
+          expect(response).to have_gitlab_http_status(:forbidden)
+        end
       end
     end
 
-    context 'authenticated user with invalid permissions' do
-      it 'does not update trigger' do
-        put api("/projects/#{project.id}/triggers/#{trigger.id}", user2)
+    context 'user is developer of the project' do
+      context 'the trigger belongs to user' do
+        it 'does not update trigger' do
+          put api("/projects/#{project.id}/triggers/#{trigger2.id}", user2)
 
-        expect(response).to have_gitlab_http_status(:forbidden)
+          expect(response).to have_gitlab_http_status(:forbidden)
+        end
+      end
+
+      context 'the trigger does not belong to user' do
+        it 'does not update trigger' do
+          put api("/projects/#{project.id}/triggers/#{trigger.id}", user2)
+
+          expect(response).to have_gitlab_http_status(:forbidden)
+        end
       end
     end
 

spec/requests/jwt_controller_spec.rb

@@ -25,6 +25,17 @@ describe JwtController do
   end
 
   context 'when using authenticated request' do
+    shared_examples 'rejecting a blocked user' do
+      context 'with blocked user' do
+        let(:user) { create(:user, :blocked) }
+
+        it 'rejects the request as unauthorized' do
+          expect(response).to have_gitlab_http_status(:unauthorized)
+          expect(response.body).to include('HTTP Basic: Access denied')
+        end
+      end
+    end
+
     context 'using CI token' do
       let(:build) { create(:ci_build, :running) }
       let(:project) { build.project }
@@ -61,6 +72,8 @@ describe JwtController do
           expect(response).to have_gitlab_http_status(:ok)
           expect(service_class).to have_received(:new).with(nil, user, ActionController::Parameters.new(parameters).permit!)
         end
+
+        it_behaves_like 'rejecting a blocked user'
       end
     end
 
@@ -72,6 +85,8 @@ describe JwtController do
 
       it { expect(service_class).to have_received(:new).with(nil, user, ActionController::Parameters.new(parameters).permit!) }
 
+      it_behaves_like 'rejecting a blocked user'
+
       context 'when passing a flat array of scopes' do
         # We use this trick to make rails to generate a query_string:
         # scope=scope1&scope=scope2

spec/serializers/merge_request_poll_widget_entity_spec.rb

@@ -138,7 +138,7 @@ describe MergeRequestPollWidgetEntity do
   end
 
   describe 'pipeline' do
-    let(:pipeline) { create(:ci_empty_pipeline, project: project, ref: resource.source_branch, sha: resource.source_branch_sha, head_pipeline_of: resource) }
+    let!(:pipeline) { create(:ci_empty_pipeline, project: project, ref: resource.source_branch, sha: resource.source_branch_sha, head_pipeline_of: resource) }
 
     before do
       allow_any_instance_of(MergeRequestPresenter).to receive(:can?).and_call_original
@@ -158,6 +158,10 @@ describe MergeRequestPollWidgetEntity do
 
           expect(subject[:pipeline]).to eq(pipeline_payload)
         end
+
+        it 'returns ci_status' do
+          expect(subject[:ci_status]).to eq('pending')
+        end
       end
 
       context 'when is not up to date' do
@@ -171,10 +175,15 @@ describe MergeRequestPollWidgetEntity do
 
     context 'when user does not have access to pipelines' do
       let(:result) { false }
+      let(:req) { double('request', current_user: user, project: project) }
 
       it 'does not have pipeline' do
         expect(subject[:pipeline]).to eq(nil)
       end
+
+      it 'does not return ci_status' do
+        expect(subject[:ci_status]).to eq(nil)
+      end
     end
   end
 end

spec/support/helpers/workhorse_helpers.rb

@@ -76,7 +76,7 @@ module WorkhorseHelpers
       "#{key}.size" => file.size
     }.tap do |params|
       params["#{key}.path"] = file.path if file.path
-      params["#{key}.remote_id"] = file.remote_id if file.respond_to?(:remote_id) && file.remote_id
+      params["#{key}.remote_id"] = file.remote_id if file.respond_to?(:remote_id) && file.remote_id.present?
     end
   end
 

spec/support/shared_examples/controllers/hotlink_interceptor_shared_examples.rb

+# frozen_string_literal: true
+
+RSpec.shared_examples "hotlink interceptor" do
+  let(:http_request) { nil }
+  let(:headers) { nil }
+
+  describe "DDOS prevention" do
+    using RSpec::Parameterized::TableSyntax
+
+    context "hotlinked as media" do
+      where(:response_status, :accept_header) do
+        # These are default formats in modern browsers, and IE
+        :ok | "*/*"
+        :ok | "text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8"
+        :ok | "text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,*/*;q=0.8"
+        :ok | "text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8"
+        :ok | "text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,image/apng,*/*;q=0.8"
+        :ok | "image/jpeg, application/x-ms-application, image/gif, application/xaml+xml, image/pjpeg, application/x-ms-xbap, application/x-shockwave-flash, application/msword, */*"
+        :ok | "text/html, application/xhtml+xml, image/jxr, */*"
+        :ok | "text/html, application/xml;q=0.9, application/xhtml+xml, image/png, image/webp, image/jpeg, image/gif, image/x-xbitmap, */*;q=0.1"
+
+        # These are image request formats
+        :not_acceptable | "image/webp,*/*"
+        :not_acceptable | "image/png,image/*;q=0.8,*/*;q=0.5"
+        :not_acceptable | "image/webp,image/apng,image/*,*/*;q=0.8"
+        :not_acceptable | "image/png,image/svg+xml,image/*;q=0.8, */*;q=0.5"
+
+        # Video request formats
+        :not_acceptable | "video/webm,video/ogg,video/*;q=0.9,application/ogg;q=0.7,audio/*;q=0.6,*/*;q=0.5"
+
+        # Audio request formats
+        :not_acceptable | "audio/webm,audio/ogg,audio/wav,audio/*;q=0.9,application/ogg;q=0.7,video/*;q=0.6,*/*;q=0.5"
+
+        # CSS request formats
+        :not_acceptable | "text/css,*/*;q=0.1"
+        :not_acceptable | "text/css"
+        :not_acceptable | "text/css,*/*;q=0.1"
+      end
+
+      with_them do
+        let(:headers) do
+          { "Accept" => accept_header }
+        end
+
+        before do
+          request.headers.merge!(headers) if request.present?
+        end
+
+        it "renders the response" do
+          http_request
+
+          expect(response).to have_gitlab_http_status(response_status)
+        end
+      end
+    end
+
+    context "hotlinked as a script" do
+      where(:response_status, :fetch_mode) do
+        # Standard navigation fetch modes
+        :ok | "navigate"
+        :ok | "nested-navigate"
+        :ok | "same-origin"
+
+        # Fetch modes when linking as JS
+        :not_acceptable | "cors"
+        :not_acceptable | "no-cors"
+        :not_acceptable | "websocket"
+      end
+
+      with_them do
+        let(:headers) do
+          { "Sec-Fetch-Mode" => fetch_mode }
+        end
+
+        before do
+          request.headers.merge!(headers) if request.present?
+        end
+
+        it "renders the response" do
+          http_request
+
+          expect(response).to have_gitlab_http_status(response_status)
+        end
+      end
+    end
+  end
+end

spec/uploaders/object_storage_spec.rb

@@ -714,6 +714,19 @@ describe ObjectStorage do
           end
         end
 
+        context 'when empty remote_id is specified' do
+          let(:uploaded_file) do
+            UploadedFile.new(temp_file.path, remote_id: '')
+          end
+
+          it 'uses local storage' do
+            subject
+
+            expect(uploader).to be_file_storage
+            expect(uploader.object_store).to eq(described_class::Store::LOCAL)
+          end
+        end
+
         context 'when valid file is specified' do
           let(:uploaded_file) do
             UploadedFile.new(temp_file.path, filename: "my_file.txt", remote_id: "test/123123")