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.1.2
+
+### Security (1 change)
+
+- Ensure the Insights configuration project is part of the group and is accessible to the current user.
+
+### Security (6 changes)
+
+- Don't override approval rules if not allowed.
+- Grant admin note permissions in epics for maintainers and owners.
+- Queries for Upload should be scoped by model.
+- Fix bypass email verification when SCIM user is created via API.
+- Prevent an XSS vector in the add approver email.
+- Make vulnerability feedback invisible if limited access to repo.
+
+
 ## 12.1.1
 
 ### Fixed (1 change)
@@ -150,6 +166,17 @@ Please view this file on the master branch, on stable branches it's out of date.
 - Remove commit count from storage quotass.
 
 
+## 11.11.7
+
+### Security (5 changes)
+
+- Don't override approval rules if not allowed.
+- Grant admin note permissions in epics for maintainers and owners.
+- Prevent an XSS vector in the add approver email.
+- Ensure the Insights configuration project is part of the group and is accessible to the current user.
+- Make vulnerability feedback invisible if limited access to repo.
+
+
 ## 11.11.4 (2019-06-26)
 
 ### Fixed (1 change)

CHANGELOG.md

@@ -2,6 +2,25 @@
 documentation](doc/development/changelog.md) for instructions on adding your own
 entry.
 
+## 12.1.2
+
+### Security (1 change)
+
+- Use source project as permissions reference for MergeRequestsController#pipelines.
+
+### Security (9 changes)
+
+- Restrict slash commands to users who can log in.
+- Patch XSS issue in wiki links.
+- Queries for Upload should be scoped by model.
+- Filter merge request params on the new merge request page.
+- Fix Server Side Request Forgery mitigation bypass.
+- Show badges if pipelines are public otherwise default to project permissions.
+- Do not allow localhost url redirection in GitHub Integration.
+- Do not show moved issue id for users that cannot read issue.
+- Drop feature to take ownership of trigger token.
+
+
 ## 12.1.1
 
 - No changes.
@@ -624,6 +643,21 @@ entry.
 - Moves the table pagination shared component.
 
 
+## 11.11.7
+
+### Security (9 changes)
+
+- Restrict slash commands to users who can log in.
+- Patch XSS issue in wiki links.
+- Filter merge request params on the new merge request page.
+- Fix Server Side Request Forgery mitigation bypass.
+- Show badges if pipelines are public otherwise default to project permissions.
+- Do not allow localhost url redirection in GitHub Integration.
+- Do not show moved issue id for users that cannot read issue.
+- Use source project as permissions reference for MergeRequestsController#pipelines.
+- Drop feature to take ownership of trigger token.
+
+
 ## 11.11.4 (2019-06-26)
 
 ### Fixed (3 changes)

app/controllers/concerns/uploads_actions.rb

@@ -90,7 +90,7 @@ module UploadsActions
     return unless uploader = build_uploader
 
     upload_paths = uploader.upload_paths(params[:filename])
-    upload = Upload.find_by(uploader: uploader_class.to_s, path: upload_paths)
+    upload = Upload.find_by(model: model, uploader: uploader_class.to_s, path: upload_paths)
     upload&.build_uploader
   end
   # rubocop: enable CodeReuse/ActiveRecord

app/controllers/projects/badges_controller.rb

@@ -3,7 +3,8 @@
 class Projects::BadgesController < Projects::ApplicationController
   layout 'project_settings'
   before_action :authorize_admin_project!, only: [:index]
-  before_action :no_cache_headers, except: [:index]
+  before_action :no_cache_headers, only: [:pipeline, :coverage]
+  before_action :authorize_read_build!, only: [:pipeline, :coverage]
 
   def pipeline
     pipeline_status = Gitlab::Badge::Pipeline::Status

app/controllers/projects/merge_requests/application_controller.rb

@@ -45,7 +45,7 @@ class Projects::MergeRequests::ApplicationController < Projects::ApplicationCont
 
   def set_pipeline_variables
     @pipelines =
-      if can?(current_user, :read_pipeline, @project)
+      if can?(current_user, :read_pipeline, @merge_request.source_project)
         @merge_request.all_pipelines
       else
         Ci::Pipeline.none

app/controllers/projects/merge_requests_controller.rb

@@ -82,7 +82,8 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
   end
 
   def pipelines
-    @pipelines = @merge_request.all_pipelines.page(params[:page]).per(30)
+    set_pipeline_variables
+    @pipelines = @pipelines.page(params[:page]).per(30)
 
     Gitlab::PollingInterval.set_header(response, interval: 10_000)
 

app/controllers/projects/triggers_controller.rb

@@ -4,7 +4,7 @@ class Projects::TriggersController < Projects::ApplicationController
   before_action :authorize_admin_build!
   before_action :authorize_manage_trigger!, except: [:index, :create]
   before_action :authorize_admin_trigger!, only: [:edit, :update]
-  before_action :trigger, only: [:take_ownership, :edit, :update, :destroy]
+  before_action :trigger, only: [:edit, :update, :destroy]
 
   layout 'project_settings'
 
@@ -24,16 +24,6 @@ class Projects::TriggersController < Projects::ApplicationController
     redirect_to project_settings_ci_cd_path(@project, anchor: 'js-pipeline-triggers')
   end
 
-  def take_ownership
-    if trigger.update(owner: current_user)
-      flash[:notice] = _('Trigger was re-assigned.')
-    else
-      flash[:alert] = _('You could not take ownership of trigger.')
-    end
-
-    redirect_to project_settings_ci_cd_path(@project, anchor: 'js-pipeline-triggers')
-  end
-
   def edit
   end
 

app/policies/group_policy.rb

@@ -74,6 +74,7 @@ class GroupPolicy < BasePolicy
   end
 
   rule { maintainer }.policy do
+    enable :maintainer_access
     enable :create_projects
     enable :admin_pipeline
     enable :admin_build
@@ -85,6 +86,7 @@ class GroupPolicy < BasePolicy
   end
 
   rule { owner }.policy do
+    enable :owner_access
     enable :admin_group
     enable :admin_namespace
     enable :admin_group_member

app/serializers/issue_entity.rb

@@ -17,9 +17,14 @@ class IssueEntity < IssuableEntity
   expose :discussion_locked
   expose :assignees, using: API::Entities::UserBasic
   expose :due_date
-  expose :moved_to_id
   expose :project_id
 
+  expose :moved_to_id do |issue|
+    if issue.moved_to_id.present? && can?(request.current_user, :read_issue, issue.moved_to)
+      issue.moved_to_id
+    end
+  end
+
   expose :web_url do |issue|
     project_issue_path(issue.project, issue)
   end

app/services/merge_requests/build_service.rb

@@ -11,15 +11,18 @@ module MergeRequests
       # https://gitlab.com/gitlab-org/gitlab-ce/issues/53658
       merge_quick_actions_into_params!(merge_request, only: [:target_branch])
       merge_request.merge_params['force_remove_source_branch'] = params.delete(:force_remove_source_branch) if params.has_key?(:force_remove_source_branch)
-      merge_request.assign_attributes(params)
 
+      # Assign the projects first so we can use policies for `filter_params`
       merge_request.author = current_user
+      merge_request.source_project = find_source_project
+      merge_request.target_project = find_target_project
+
+      filter_params(merge_request)
+      merge_request.assign_attributes(params.to_h.compact)
+
       merge_request.compare_commits = []
-      merge_request.source_project  = find_source_project
-      merge_request.target_project  = find_target_project
-      merge_request.target_branch   = find_target_branch
-      merge_request.can_be_created  = projects_and_branches_valid?
-      ensure_milestone_available(merge_request)
+      merge_request.target_branch = find_target_branch
+      merge_request.can_be_created = projects_and_branches_valid?
 
       # compare branches only if branches are valid, otherwise
       # compare_branches may raise an error
@@ -50,12 +53,14 @@ module MergeRequests
              to: :merge_request
 
     def find_source_project
+      source_project = project_from_params(:source_project)
       return source_project if source_project.present? && can?(current_user, :create_merge_request_from, source_project)
 
       project
     end
 
     def find_target_project
+      target_project = project_from_params(:target_project)
       return target_project if target_project.present? && can?(current_user, :create_merge_request_in, target_project)
 
       target_project = project.default_merge_request_target
@@ -65,6 +70,17 @@ module MergeRequests
       project
     end
 
+    def project_from_params(param_name)
+      project_from_params = params.delete(param_name)
+
+      id_param_name = :"#{param_name}_id"
+      if project_from_params.nil? && params[id_param_name]
+        project_from_params = Project.find_by_id(params.delete(id_param_name))
+      end
+
+      project_from_params
+    end
+
     def find_target_branch
       target_branch || target_project.default_branch
     end

app/uploaders/records_uploads.rb

@@ -27,7 +27,7 @@ module RecordsUploads
     end
 
     def readd_upload
-      uploads.where(path: upload_path).delete_all
+      uploads.where(model: model, path: upload_path).delete_all
       upload.delete if upload
 
       self.upload = build_upload.tap(&:save!)

app/views/projects/triggers/_trigger.html.haml

@@ -33,10 +33,7 @@
       Never
 
   %td.text-right.trigger-actions
-    - take_ownership_confirmation = "By taking ownership you will bind this trigger to your user account. With this the trigger will have access to all your projects as if it was you. Are you sure?"
     - revoke_trigger_confirmation = "By revoking a trigger you will break any processes making use of it. Are you sure?"
-    - if trigger.owner != current_user && can?(current_user, :manage_trigger, trigger)
-      = link_to 'Take ownership', take_ownership_project_trigger_path(@project, trigger), data: { confirm: take_ownership_confirmation }, method: :post, class: "btn btn-default btn-sm btn-trigger-take-ownership"
     - if can?(current_user, :admin_trigger, trigger)
       = link_to edit_project_trigger_path(@project, trigger), method: :get, title: "Edit", class: "btn btn-default btn-sm" do
         %i.fa.fa-pencil

config/initializers/octokit.rb

+Octokit.middleware.insert_after Octokit::Middleware::FollowRedirects, Gitlab::Octokit::Middleware

config/routes/project.rb

@@ -339,11 +339,7 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
 
       resource :variables, only: [:show, :update]
 
-      resources :triggers, only: [:index, :create, :edit, :update, :destroy] do
-        member do
-          post :take_ownership
-        end
-      end
+      resources :triggers, only: [:index, :create, :edit, :update, :destroy]
 
       resource :mirror, only: [:show, :update] do
         member do

doc/api/pipeline_triggers.md

@@ -120,35 +120,6 @@ curl --request PUT --header "PRIVATE-TOKEN: <your_access_token>" --form descript
 }
 ```
 
-## Take ownership of a project trigger
-
-Update an owner of a project trigger.
-
-```
-POST /projects/:id/triggers/:trigger_id/take_ownership
-```
-
-| Attribute     | Type    | required | Description              |
-|---------------|---------|----------|--------------------------|
-| `id`          | integer/string | yes      | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user      |
-| `trigger_id`  | integer | yes      | The trigger id           |
-
-```
-curl --request POST --header "PRIVATE-TOKEN: <your_access_token>" "https://gitlab.example.com/api/v4/projects/1/triggers/10/take_ownership"
-```
-
-```json
-{
-    "id": 10,
-    "description": "my trigger",
-    "created_at": "2016-01-07T09:53:58.235Z",
-    "last_used": null,
-    "token": "6d056f63e50fe6f8c5f8f4aa10edb7",
-    "updated_at": "2016-01-07T09:53:58.235Z",
-    "owner": null
-}
-```
-
 ## Remove a project trigger
 
 Remove a project's build trigger.

doc/ci/triggers/README.md

@@ -97,17 +97,6 @@ overview of the time the triggers were last used.
 
 ![Triggers page overview](img/triggers_page.png)
 
-## Taking ownership of a trigger
-
-> **Note**:
-GitLab 9.0 introduced a trigger ownership to solve permission problems.
-
-Each created trigger when run will impersonate their associated user including
-their access to projects and their project permissions.
-
-You can take ownership of existing triggers by clicking *Take ownership*.
-From now on the trigger will be run as you.
-
 ## Revoking a trigger
 
 You can revoke a trigger any time by going at your project's
@@ -282,8 +271,7 @@ Old triggers, created before GitLab 9.0 will be marked as legacy.
 
 Triggers with the legacy label do not have an associated user and only have
 access to the current project. They are considered deprecated and will be
-removed with one of the future versions of GitLab. You are advised to
-[take ownership](#taking-ownership-of-a-trigger) of any legacy triggers.
+removed with one of the future versions of GitLab. 
 
 [ee-2017]: https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/2017
 [ee-2346]: https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/2346

doc/user/project/new_ci_build_permissions_model.md

@@ -90,8 +90,7 @@ to steal the tokens of other jobs.
 
 Since 9.0 [pipeline triggers][triggers] do support the new permission model.
 The new triggers do impersonate their associated user including their access
-to projects and their project permissions. To migrate trigger to use new permission
-model use **Take ownership**.
+to projects and their project permissions. 
 
 ## Before GitLab 8.12
 

ee/app/controllers/concerns/insights_actions.rb

@@ -44,7 +44,7 @@ module InsightsActions
   end
 
   def query_param
-    @query_param ||= params[:query]
+    @query_param ||= params[:query] || {}
   end
 
   def period_param

ee/app/controllers/groups/insights_controller.rb

@@ -4,6 +4,7 @@ class Groups::InsightsController < Groups::ApplicationController
   include InsightsActions
 
   before_action :authorize_read_group!
+  before_action :authorize_read_insights_config_project!
 
   private
 
@@ -11,6 +12,12 @@ class Groups::InsightsController < Groups::ApplicationController
     render_404 unless can?(current_user, :read_group, group)
   end
 
+  def authorize_read_insights_config_project!
+    insights_config_project = group.insights_config_project
+
+    render_404 if insights_config_project && !can?(current_user, :read_project, insights_config_project)
+  end
+
   def insights_entity
     group
   end

ee/app/models/concerns/insights_feature.rb

@@ -10,32 +10,32 @@ module InsightsFeature
     feature_available?(:insights)
   end
 
-  def insights_config(follow_group: true)
-    case self
-    when Group
-      # When there's a config file, we use it regardless it's valid or not
-      if insight&.project&.project_insights_config_yaml
-        insight.project.insights_config(follow_group: false)
-      else # When there's nothing, then we use the default
-        default_insights_config
+  def insights_config_project
+    strong_memoize(:insights_config_project) do
+      case self
+      when Group
+        insight&.project
+      when Project
+        if insights_config_yaml
+          self
+        else
+          group&.insights_config_project
+        end
       end
-    when Project
-      yaml = project_insights_config_yaml
+    end
+  end
+
+  def insights_config
+    strong_memoize(:insights_config) do
+      yaml = insights_config_project&.insights_config_yaml
 
-      # When there's a config file, we use it regardless it's valid or not
       if yaml
-        strong_memoize(:insights_config) do
-          ::Gitlab::Config::Loader::Yaml.new(yaml).load!
-        rescue Gitlab::Config::Loader::FormatError
-          nil
-        end
-      # When we're following the group and there's a group then we use it
-      elsif follow_group && group
-        group.insights_config
-      # A project might not have a group, then we just use the default
+        ::Gitlab::Config::Loader::Yaml.new(yaml).load!
       else
         default_insights_config
       end
+    rescue Gitlab::Config::Loader::FormatError
+      nil
     end
   end
 
@@ -50,8 +50,10 @@ module InsightsFeature
 
   protected
 
-  def project_insights_config_yaml
-    strong_memoize(:project_insights_config_yaml) do
+  def insights_config_yaml
+    raise NotImplementedError unless is_a?(Project)
+
+    strong_memoize(:insights_config_yaml) do
       next if repository.empty?
 
       repository.insights_config_for(repository.root_ref)

ee/app/policies/ee/project_policy.rb

@@ -147,7 +147,7 @@ module EE
         prevent :read_project_security_dashboard
       end
 
-      rule { can?(:read_project) }.enable :read_vulnerability_feedback
+      rule { can?(:read_project) & (can?(:read_merge_request) | can?(:read_build)) }.enable :read_vulnerability_feedback
 
       rule { license_management_enabled & can?(:read_project) }.enable :read_software_license_policy
 

ee/app/policies/epic_policy.rb

@@ -13,4 +13,6 @@ class EpicPolicy < BasePolicy
   end
 
   rule { can?(:create_note) }.enable :award_emoji
+
+  rule { can?(:owner_access) | can?(:maintainer_access) }.enable :admin_note
 end

ee/app/services/approval_rules/params_filtering_service.rb

@@ -21,6 +21,8 @@ module ApprovalRules
     end
 
     def execute
+      params.delete(:approval_rules_attributes) unless current_user.can?(:update_approvers, target)
+
       return params unless params.key?(:approval_rules_attributes)
 
       params[:approval_rules_attributes].each do |rule_attributes|

ee/app/services/ee/groups/update_service.rb

@@ -35,6 +35,12 @@ module EE
           params.delete(:extra_shared_runners_minutes_limit)
         end
 
+        insight_project_id = params.dig(:insight_attributes, :project_id)
+        if insight_project_id
+          group_projects = GroupProjectsFinder.new(group: group, current_user: current_user, options: { only_owned: true, include_subgroups: true }).execute
+          params.delete(:insight_attributes) unless group_projects.exists?(insight_project_id) # rubocop:disable CodeReuse/ActiveRecord
+        end
+
         super
       end
 

ee/app/views/notify/add_merge_request_approver_email.html.haml

@@ -2,7 +2,7 @@
   %div
     #{link_to @updated_by.name, user_url(@updated_by)} added you as an approver for:
 %p.details
-  != merge_path_description(@merge_request, '→')
+  = merge_path_description(@merge_request, '→')
 
 - if @merge_request.assignees.any?
   %p

ee/changelogs/unreleased/security-373-public-group-owners-maintainers-can-not-delete-comments-posted-on-epics.yml

+---
+title: Grant admin note permissions in epics for maintainers and owners
+merge_request:
+author:
+type: security

ee/changelogs/unreleased/security-60551-fix-upload-scope-ee.yml

+---
+title: Queries for Upload should be scoped by model
+merge_request:
+author:
+type: security

ee/changelogs/unreleased/security-fix-bypass-email-verification-on-scim-user-creation-via-api-ee.yml

+---
+title: Fix bypass email verification when SCIM user is created via API
+merge_request:
+author:
+type: security

ee/lib/ee/gitlab/scim/provisioning_service.rb

@@ -8,7 +8,7 @@ module EE
 
         IDENTITY_PROVIDER = 'group_saml'
         PASSWORD_AUTOMATICALLY_SET = true
-        SKIP_EMAIL_CONFIRMATION = true
+        SKIP_EMAIL_CONFIRMATION = false
         DEFAULT_ACCESS = :guest
 
         def initialize(group, parsed_hash)

ee/lib/gitlab/insights/finders/issuable_finder.rb

@@ -54,7 +54,7 @@ module Gitlab
         attr_reader :entity, :current_user, :opts
 
         def finder
-          issuable_type = opts[:issuable_type].to_sym
+          issuable_type = opts[:issuable_type]&.to_sym
 
           FINDERS[issuable_type] ||
             raise(InvalidIssuableTypeError, "Invalid `:issuable_type` option: `#{opts[:issuable_type]}`. Allowed values are #{FINDERS.keys}!")

ee/spec/controllers/groups/insights_controller_spec.rb

+require 'spec_helper'
+
+describe Groups::InsightsController do
+  set(:parent_group) { create(:group, :private) }
+  set(:nested_group) { create(:group, :private, parent: parent_group) }
+  set(:project) { create(:project, :private) }
+  set(:insight) { create(:insight, group: parent_group, project: project) }
+  set(:user) { create(:user) }
+  let(:query_params) { { chart_type: 'bar', query: { issuable_type: 'issue', collection_labels: ['bug'] } } }
+
+  before do
+    stub_licensed_features(insights: true)
+    sign_in(user)
+    parent_group.add_developer(user)
+    nested_group.add_developer(user)
+  end
+
+  shared_examples '404 status' do
+    it 'returns 404 status' do
+      subject
+
+      expect(response).to have_gitlab_http_status(404)
+    end
+  end
+
+  shared_examples '200 status' do
+    it 'returns 200 status' do
+      subject
+
+      expect(response).to have_gitlab_http_status(200)
+    end
+  end
+
+  context 'when insights configuration project cannot be read by current user' do
+    context 'when visiting the parent group' do
+      describe 'GET #show.html' do
+        subject { get :show, params: { group_id: parent_group.to_param } }
+
+        it_behaves_like '404 status'
+      end
+
+      describe 'GET #show.json' do
+        subject { get :show, params: { group_id: parent_group.to_param }, format: :json }
+
+        it_behaves_like '404 status'
+      end
+
+      describe 'POST #query' do
+        subject { post :query, params: query_params.merge(group_id: parent_group.to_param) }
+
+        it_behaves_like '404 status'
+      end
+    end
+
+    context 'when visiting a nested group' do
+      describe 'GET #show.html' do
+        subject { get :show, params: { group_id: nested_group.to_param } }
+
+        # The following expectation should be changed to
+        # it_behaves_like '404 status'
+        # once https://gitlab.com/gitlab-org/gitlab-ee/issues/11340 is implemented.
+        it_behaves_like '200 status'
+      end
+
+      describe 'GET #show.json' do
+        subject { get :show, params: { group_id: nested_group.to_param }, format: :json }
+
+        # The following expectation should be changed to
+        # it_behaves_like '404 status'
+        # once https://gitlab.com/gitlab-org/gitlab-ee/issues/11340 is implemented.
+        it_behaves_like '200 status'
+
+        # The following expectation should be removed
+        # once https://gitlab.com/gitlab-org/gitlab-ee/issues/11340 is implemented.
+        it 'does return the default config' do
+          subject
+
+          expect(response.parsed_body).to eq(parent_group.default_insights_config.to_json)
+        end
+      end
+
+      describe 'POST #query.json' do
+        subject { post :query, params: query_params.merge(group_id: nested_group.to_param), format: :json }
+
+        # The following expectation should be changed to
+        # it_behaves_like '404 status'
+        # once https://gitlab.com/gitlab-org/gitlab-ee/issues/11340 is implemented.
+        it_behaves_like '200 status'
+      end
+    end
+  end
+
+  context 'when insights configuration project can be read by current user' do
+    before do
+      project.add_reporter(user)
+    end
+
+    context 'when the configuration is attached to the current group' do
+      describe 'GET #show.html' do
+        subject { get :show, params: { group_id: parent_group.to_param } }
+
+        it_behaves_like '200 status'
+      end
+
+      describe 'GET #show.sjon' do
+        subject { get :show, params: { group_id: parent_group.to_param }, format: :json }
+
+        it_behaves_like '200 status'
+      end
+
+      describe 'POST #query.json' do
+        subject { post :query, params: query_params.merge(group_id: parent_group.to_param), format: :json }
+
+        it_behaves_like '200 status'
+      end
+    end
+
+    context 'when the configuration is attached to a nested group' do
+      describe 'GET #show.html' do
+        subject { get :show, params: { group_id: nested_group.to_param } }
+
+        it_behaves_like '200 status'
+      end
+
+      describe 'GET #show.json' do
+        subject { get :show, params: { group_id: nested_group.to_param }, format: :json }
+
+        it_behaves_like '200 status'
+      end
+
+      describe 'POST #query.json' do
+        subject { post :query, params: query_params.merge(group_id: nested_group.to_param), format: :json }
+
+        it_behaves_like '200 status'
+      end
+    end
+  end
+end

ee/spec/controllers/projects/merge_requests/creations_controller_spec.rb

@@ -11,6 +11,8 @@ describe Projects::MergeRequests::CreationsController do
   end
 
   describe 'POST #create' do
+    let(:created_merge_request) { assigns(:merge_request) }
+
     def create_merge_request(overrides = {})
       params = {
         namespace_id: project.namespace.to_param,
@@ -27,8 +29,6 @@ describe Projects::MergeRequests::CreationsController do
     end
 
     context 'the approvals_before_merge param' do
-      let(:created_merge_request) { assigns(:merge_request) }
-
       before do
         project.update(approvals_before_merge: 2)
       end
@@ -97,5 +97,45 @@ describe Projects::MergeRequests::CreationsController do
         end
       end
     end
+
+    context 'overriding approvers per MR' do
+      let(:new_approver) { create(:user) }
+
+      before do
+        project.add_developer(new_approver)
+        project.update(disable_overriding_approvers_per_merge_request: disable_overriding_approvers_per_merge_request)
+
+        create_merge_request(
+          approval_rules_attributes: [
+            {
+              name: 'Test',
+              user_ids: [new_approver.id],
+              approvals_required: 1
+            }
+          ]
+        )
+      end
+
+      context 'enabled' do
+        let(:disable_overriding_approvers_per_merge_request) { false }
+
+        it 'does create approval rules' do
+          approval_rules = created_merge_request.reload.approval_rules
+
+          expect(approval_rules.count).to eq(1)
+          expect(approval_rules.first.name).to eq('Test')
+          expect(approval_rules.first.user_ids).to eq([new_approver.id])
+          expect(approval_rules.first.approvals_required).to eq(1)
+        end
+      end
+
+      context 'disabled' do
+        let(:disable_overriding_approvers_per_merge_request) { true }
+
+        it 'does not create approval rules' do
+          expect(created_merge_request.reload.approval_rules).to be_empty
+        end
+      end
+    end
   end
 end

ee/spec/controllers/projects/merge_requests_controller_spec.rb

@@ -190,6 +190,20 @@ describe Projects::MergeRequestsController do
 
           expect(merge_request.reload.approver_group_ids).to be_empty
         end
+
+        it 'does not create approval rules' do
+          update_merge_request(
+            approval_rules_attributes: [
+              {
+                name: 'Test',
+                user_ids: [new_approver.id],
+                approvals_required: 1
+              }
+            ]
+          )
+
+          expect(merge_request.reload.approval_rules).to be_empty
+        end
       end
     end
 

ee/spec/factories/insights.rb

 # frozen_string_literal: true
 
 FactoryBot.define do
+  factory :insight do
+    group
+    project
+  end
+
   factory :insights_issues_by_team, class: Hash do
     initialize_with do
       {

ee/spec/lib/ee/gitlab/scim/provisioning_service_spec.rb

@@ -44,6 +44,15 @@ describe ::EE::Gitlab::Scim::ProvisioningService do
         expect(User.find_by(service_params.except(:extern_uid))).to be_a(User)
       end
 
+      it 'user record requires confirmation' do
+        service.execute
+
+        user = User.find_by(email: service_params[:email])
+
+        expect(user).to be_present
+        expect(user).not_to be_confirmed
+      end
+
       context 'existing user' do
         before do
           create(:user, email: 'work@example.com')

ee/spec/models/group_spec.rb

@@ -425,10 +425,26 @@ describe Group do
       context 'with an invalid config file' do
         let(:insights_file_content) { ': foo bar' }
 
-        it 'returns the insights config data' do
+        it 'returns nil' do
           expect(group.insights_config).to be_nil
         end
       end
     end
+
+    context 'when group has an Insights project configured which is in a nested group' do
+      before do
+        nested_group = create(:group, parent: group)
+        project = create(:project, :custom_repo, group: nested_group, files: { ::Gitlab::Insights::CONFIG_FILE_PATH => insights_file_content })
+        group.create_insight!(project: project)
+      end
+
+      let(:insights_file_content) { 'key: monthlyBugsCreated' }
+
+      it 'returns the insights config data' do
+        insights_config = group.insights_config
+
+        expect(insights_config).to eq(key: 'monthlyBugsCreated')
+      end
+    end
   end
 end

ee/spec/models/project_spec.rb

@@ -1747,7 +1747,7 @@ describe Project do
 
         context 'when the group has an Insights config from another project' do
           let(:config_project) do
-            create(:project, :custom_repo, files: { ::Gitlab::Insights::CONFIG_FILE_PATH => insights_file_content })
+            create(:project, :custom_repo, group: group, files: { ::Gitlab::Insights::CONFIG_FILE_PATH => insights_file_content })
           end
 
           before do
@@ -1761,6 +1761,18 @@ describe Project do
               expect(project.insights_config).to eq(config_project.insights_config)
               expect(project.insights_config).to eq(group.insights_config)
             end
+
+            context 'when the project is inside a nested group' do
+              let(:nested_group) { create(:group, parent: group) }
+              let(:project) { create(:project, group: nested_group) }
+
+              # The following expectaction should be changed to
+              # expect(project.insights_config).to eq(config_project.insights_config)
+              # once https://gitlab.com/gitlab-org/gitlab-ee/issues/11340 is implemented.
+              it 'returns the project default config' do
+                expect(project.insights_config).to eq(project.default_insights_config)
+              end
+            end
           end
 
           context 'with an invalid config file' do
@@ -1783,25 +1795,22 @@ describe Project do
         let(:insights_file_content) { 'key: monthlyBugsCreated' }
 
         it 'returns the insights config data' do
-          insights_config = project.insights_config
-
-          expect(insights_config).to eq(key: 'monthlyBugsCreated')
+          expect(project.insights_config).to eq(key: 'monthlyBugsCreated')
         end
 
         context 'when the project is inside a group having another config' do
+          let(:group) { create(:group) }
           let(:config_project) do
-            create(:project, :custom_repo, files: { ::Gitlab::Insights::CONFIG_FILE_PATH => ': foo bar' })
+            create(:project, :custom_repo, group: group, files: { ::Gitlab::Insights::CONFIG_FILE_PATH => ': foo bar' })
           end
 
           before do
-            project.group = create(:group)
+            project.group = group
             project.group.create_insight!(project: config_project)
           end
 
           it 'returns the project insights config data' do
-            insights_config = project.insights_config
-
-            expect(insights_config).to eq(key: 'monthlyBugsCreated')
+            expect(project.insights_config).to eq(key: 'monthlyBugsCreated')
           end
         end
       end
@@ -1814,12 +1823,13 @@ describe Project do
         end
 
         context 'when the project is inside a group having another config' do
+          let(:group) { create(:group) }
           let(:config_project) do
-            create(:project, :custom_repo, files: { ::Gitlab::Insights::CONFIG_FILE_PATH => 'key: monthlyBugsCreated' })
+            create(:project, :custom_repo, group: group, files: { ::Gitlab::Insights::CONFIG_FILE_PATH => 'key: monthlyBugsCreated' })
           end
 
           before do
-            project.group = create(:group)
+            project.group = group
             project.group.create_insight!(project: config_project)
           end
 

ee/spec/policies/epic_policy_spec.rb

@@ -16,6 +16,14 @@ describe EpicPolicy do
     it { is_expected.to be_disallowed(:create_note, :award_emoji) }
   end
 
+  shared_examples 'can edit epic comments' do
+    it { is_expected.to be_allowed(:admin_note) }
+  end
+
+  shared_examples 'cannot edit epic comments' do
+    it { is_expected.to be_disallowed(:admin_note) }
+  end
+
   shared_examples 'can only read epics' do
     it do
       is_expected.to be_allowed(:read_epic, :read_epic_iid)
@@ -39,6 +47,7 @@ describe EpicPolicy do
 
       it_behaves_like 'can only read epics'
       it_behaves_like 'can comment on epics'
+      it_behaves_like 'cannot edit epic comments'
     end
 
     context 'reporter group member' do
@@ -48,6 +57,21 @@ describe EpicPolicy do
 
       it_behaves_like 'can manage epics'
       it_behaves_like 'can comment on epics'
+      it_behaves_like 'cannot edit epic comments'
+
+      it 'cannot destroy epics' do
+        is_expected.to be_disallowed(:destroy_epic)
+      end
+    end
+
+    context 'group maintainer' do
+      before do
+        group.add_maintainer(user)
+      end
+
+      it_behaves_like 'can manage epics'
+      it_behaves_like 'can comment on epics'
+      it_behaves_like 'can edit epic comments'
 
       it 'cannot destroy epics' do
         is_expected.to be_disallowed(:destroy_epic)
@@ -61,6 +85,7 @@ describe EpicPolicy do
 
       it_behaves_like 'can manage epics'
       it_behaves_like 'can comment on epics'
+      it_behaves_like 'can edit epic comments'
 
       it 'can destroy epics' do
         is_expected.to be_allowed(:destroy_epic)

ee/spec/policies/project_policy_spec.rb

@@ -25,7 +25,7 @@ describe ProjectPolicy do
     include_context 'ProjectPolicy context'
 
     let(:additional_guest_permissions) do
-      %i[read_issue_link read_vulnerability_feedback read_software_license_policy]
+      %i[read_issue_link read_software_license_policy]
     end
     let(:additional_reporter_permissions) { [:admin_issue_link] }
     let(:additional_developer_permissions) { %i[admin_vulnerability_feedback read_project_security_dashboard read_feature_flag] }
@@ -361,6 +361,36 @@ describe ProjectPolicy do
         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 'vulnerability feedback permissions' do

ee/spec/services/approval_rules/params_filtering_service_spec.rb

@@ -18,20 +18,39 @@ describe ApprovalRules::ParamsFilteringService do
         project.add_reporter(project_member)
 
         accessible_group.add_developer(user)
+
+        allow(Ability).to receive(:allowed?).and_call_original
+
+        allow(Ability)
+          .to receive(:allowed?)
+          .with(user, :update_approvers, merge_request)
+          .and_return(can_update_approvers?)
       end
 
-      it 'only assigns eligible users and groups' do
-        params = service.execute
+      context 'user can update approvers' do
+        let(:can_update_approvers?) { true }
+
+        it 'only assigns eligible users and groups' do
+          params = service.execute
+
+          rule1 = params[:approval_rules_attributes].first
 
-        rule1 = params[:approval_rules_attributes].first
+          expect(rule1[:user_ids]).to contain_exactly(project_member.id)
 
-        expect(rule1[:user_ids]).to contain_exactly(project_member.id)
+          rule2 = params[:approval_rules_attributes].last
+          expected_group_ids = expected_groups.map(&:id)
 
-        rule2 = params[:approval_rules_attributes].last
-        expected_group_ids = expected_groups.map(&:id)
+          expect(rule2[:user_ids]).to be_empty
+          expect(rule2[:group_ids]).to contain_exactly(*expected_group_ids)
+        end
+      end
 
-        expect(rule2[:user_ids]).to be_empty
-        expect(rule2[:group_ids]).to contain_exactly(*expected_group_ids)
+      context 'user cannot update approvers' do
+        let(:can_update_approvers?) { false }
+
+        it 'deletes the approval_rules_attributes from params' do
+          expect(service.execute).not_to have_key(:approval_rules_attributes)
+        end
       end
     end
 

ee/spec/services/groups/update_service_spec.rb

@@ -204,6 +204,58 @@ describe Groups::UpdateService, '#execute' do
     end
   end
 
+  context 'updating insight_attributes.project_id param' do
+    let(:attrs) { { insight_attributes: { project_id: private_project.id } } }
+
+    shared_examples 'successful update of the Insights project' do
+      it 'updates the Insights project' do
+        update_group(group, user, attrs)
+
+        expect(group.insight.project).to eq(private_project)
+      end
+    end
+
+    shared_examples 'ignorance of the Insights project ID' do
+      it 'ignores the Insights project ID' do
+        update_group(group, user, attrs)
+
+        expect(group.insight).to be_nil
+      end
+    end
+
+    context 'when project is not in the group' do
+      let(:private_project) { create(:project, :private) }
+
+      context 'when user can read the project' do
+        before do
+          private_project.add_maintainer(user)
+        end
+
+        it_behaves_like 'ignorance of the Insights project ID'
+      end
+
+      context 'when user cannot read the project' do
+        it_behaves_like 'ignorance of the Insights project ID'
+      end
+    end
+
+    context 'when project is in the group' do
+      let(:private_project) { create(:project, :private, group: group) }
+
+      context 'when user can read the project' do
+        before do
+          private_project.add_maintainer(user)
+        end
+
+        it_behaves_like 'successful update of the Insights project'
+      end
+
+      context 'when user cannot read the project' do
+        it_behaves_like 'ignorance of the Insights project ID'
+      end
+    end
+  end
+
   def update_group(group, user, opts)
     Groups::UpdateService.new(group, user, opts).execute
   end

lib/api/triggers.rb

@@ -112,27 +112,6 @@ module API
         end
       end
 
-      desc 'Take ownership of trigger' do
-        success Entities::Trigger
-      end
-      params do
-        requires :trigger_id, type: Integer, desc: 'The trigger ID'
-      end
-      post ':id/triggers/:trigger_id/take_ownership' do
-        authenticate!
-        authorize! :admin_build, user_project
-
-        trigger = user_project.triggers.find(params.delete(:trigger_id))
-        break not_found!('Trigger') unless trigger
-
-        if trigger.update(owner: current_user)
-          status :ok
-          present trigger, with: Entities::Trigger, current_user: current_user
-        else
-          render_validation_error!(trigger)
-        end
-      end
-
       desc 'Delete a trigger' do
         success Entities::Trigger
       end

lib/banzai/filter/autolink_filter.rb

@@ -18,6 +18,7 @@ module Banzai
     #
     class AutolinkFilter < HTML::Pipeline::Filter
       include ActionView::Helpers::TagHelper
+      include Gitlab::Utils::SanitizeNodeLink
 
       # Pattern to match text that should be autolinked.
       #
@@ -72,19 +73,11 @@ module Banzai
 
       private
 
-      # Return true if any of the UNSAFE_PROTOCOLS strings are included in the URI scheme
-      def contains_unsafe?(scheme)
-        return false unless scheme
-
-        scheme = scheme.strip.downcase
-        Banzai::Filter::SanitizationFilter::UNSAFE_PROTOCOLS.any? { |protocol| scheme.include?(protocol) }
-      end
-
       def autolink_match(match)
         # start by stripping out dangerous links
         begin
           uri = Addressable::URI.parse(match)
-          return match if contains_unsafe?(uri.scheme)
+          return match unless safe_protocol?(uri.scheme)
         rescue Addressable::URI::InvalidURIError
           return match
         end

lib/banzai/filter/base_sanitization_filter.rb

@@ -11,6 +11,7 @@ module Banzai
     # Extends HTML::Pipeline::SanitizationFilter with common rules.
     class BaseSanitizationFilter < HTML::Pipeline::SanitizationFilter
       include Gitlab::Utils::StrongMemoize
+      extend Gitlab::Utils::SanitizeNodeLink
 
       UNSAFE_PROTOCOLS = %w(data javascript vbscript).freeze
 
@@ -40,7 +41,7 @@ module Banzai
           # Allow any protocol in `a` elements
           # and then remove links with unsafe protocols
           whitelist[:protocols].delete('a')
-          whitelist[:transformers].push(self.class.remove_unsafe_links)
+          whitelist[:transformers].push(self.class.method(:remove_unsafe_links))
 
           # Remove `rel` attribute from `a` elements
           whitelist[:transformers].push(self.class.remove_rel)
@@ -54,35 +55,6 @@ module Banzai
       end
 
       class << self
-        def remove_unsafe_links
-          lambda do |env|
-            node = env[:node]
-
-            return unless node.name == 'a'
-            return unless node.has_attribute?('href')
-
-            begin
-              node['href'] = node['href'].strip
-              uri = Addressable::URI.parse(node['href'])
-
-              return unless uri.scheme
-
-              # Remove all invalid scheme characters before checking against the
-              # list of unsafe protocols.
-              #
-              # See https://tools.ietf.org/html/rfc3986#section-3.1
-              scheme = uri.scheme
-                           .strip
-                           .downcase
-                           .gsub(/[^A-Za-z0-9\+\.\-]+/, '')
-
-              node.remove_attribute('href') if UNSAFE_PROTOCOLS.include?(scheme)
-            rescue Addressable::URI::InvalidURIError
-              node.remove_attribute('href')
-            end
-          end
-        end
-
         def remove_rel
           lambda do |env|
             if env[:node_name] == 'a'

lib/banzai/filter/wiki_link_filter.rb

@@ -8,15 +8,19 @@ module Banzai
     # Context options:
     #   :project_wiki
     class WikiLinkFilter < HTML::Pipeline::Filter
+      include Gitlab::Utils::SanitizeNodeLink
+
       def call
         return doc unless project_wiki?
 
-        doc.search('a:not(.gfm)').each { |el| process_link_attr(el.attribute('href')) }
-        doc.search('video').each { |el| process_link_attr(el.attribute('src')) }
+        doc.search('a:not(.gfm)').each { |el| process_link(el.attribute('href'), el) }
+
+        doc.search('video').each { |el| process_link(el.attribute('src'), el) }
+
         doc.search('img').each do |el|
           attr = el.attribute('data-src') || el.attribute('src')
 
-          process_link_attr(attr)
+          process_link(attr, el)
         end
 
         doc
@@ -24,6 +28,11 @@ module Banzai
 
       protected
 
+      def process_link(link_attr, node)
+        process_link_attr(link_attr)
+        remove_unsafe_links({ node: node }, remove_invalid_links: false)
+      end
+
       def project_wiki?
         !context[:project_wiki].nil?
       end

lib/banzai/filter/wiki_link_filter/rewriter.rb

@@ -4,8 +4,6 @@ module Banzai
   module Filter
     class WikiLinkFilter < HTML::Pipeline::Filter
       class Rewriter
-        UNSAFE_SLUG_REGEXES = [/\Ajavascript:/i].freeze
-
         def initialize(link_string, wiki:, slug:)
           @uri = Addressable::URI.parse(link_string)
           @wiki_base_path = wiki && wiki.wiki_base_path
@@ -37,8 +35,6 @@ module Banzai
 
         # Of the form `./link`, `../link`, or similar
         def apply_hierarchical_link_rules!
-          return if slug_considered_unsafe?
-
           @uri = Addressable::URI.join(@slug, @uri) if @uri.to_s[0] == '.'
         end
 
@@ -58,10 +54,6 @@ module Banzai
         def repository_upload?
           @uri.relative? && @uri.path.starts_with?(Wikis::CreateAttachmentService::ATTACHMENT_PATH)
         end
-
-        def slug_considered_unsafe?
-          !!UNSAFE_SLUG_REGEXES.detect { |r| r.match?(@slug) }
-        end
       end
     end
   end

lib/gitlab/github_import/client.rb

@@ -40,7 +40,7 @@ module Gitlab
       #            otherwise hitting the rate limit will result in a thread
       #            being blocked in a `sleep()` call for up to an hour.
       def initialize(token, per_page: 100, parallel: true)
-        @octokit = Octokit::Client.new(
+        @octokit = ::Octokit::Client.new(
           access_token: token,
           per_page: per_page,
           api_endpoint: api_endpoint
@@ -139,7 +139,7 @@ module Gitlab
 
         begin
           yield
-        rescue Octokit::TooManyRequests
+        rescue ::Octokit::TooManyRequests
           raise_or_wait_for_rate_limit
 
           # This retry will only happen when running in sequential mode as we'll

lib/gitlab/legacy_github_import/client.rb

@@ -101,7 +101,7 @@ module Gitlab
       # GitHub Rate Limit API returns 404 when the rate limit is
       # disabled. In this case we just want to return gracefully
       # instead of spitting out an error.
-      rescue Octokit::NotFound
+      rescue ::Octokit::NotFound
         nil
       end
 

lib/gitlab/octokit/middleware.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Octokit
+    class Middleware
+      def initialize(app)
+        @app = app
+      end
+
+      def call(env)
+        Gitlab::UrlBlocker.validate!(env[:url], { allow_localhost: allow_local_requests?, allow_local_network: allow_local_requests? })
+
+        @app.call(env)
+      end
+
+      private
+
+      def allow_local_requests?
+        Gitlab::CurrentSettings.allow_local_requests_from_hooks_and_services?
+      end
+    end
+  end
+end

lib/gitlab/url_blocker.rb

@@ -115,6 +115,15 @@ module Gitlab
           addr.ipv6_v4mapped? ? addr.ipv6_to_ipv4 : addr
         end
       rescue SocketError
+        # In the test suite we use a lot of mocked urls that are either invalid or
+        # don't exist. In order to avoid modifying a ton of tests and factories
+        # we allow invalid urls unless the environment variable RSPEC_ALLOW_INVALID_URLS
+        # is not true
+        return if Rails.env.test? && ENV['RSPEC_ALLOW_INVALID_URLS'] == 'true'
+
+        # If the addr can't be resolved or the url is invalid (i.e http://1.1.1.1.1)
+        # we block the url
+        raise BlockedUrlError, "Host cannot be resolved or invalid"
       end
 
       def validate_local_request(

lib/gitlab/utils/sanitize_node_link.rb

+# frozen_string_literal: true
+
+require_dependency 'gitlab/utils'
+
+module Gitlab
+  module Utils
+    module SanitizeNodeLink
+      UNSAFE_PROTOCOLS  = %w(data javascript vbscript).freeze
+      ATTRS_TO_SANITIZE = %w(href src data-src).freeze
+
+      def remove_unsafe_links(env, remove_invalid_links: true)
+        node = env[:node]
+
+        sanitize_node(node: node, remove_invalid_links: remove_invalid_links)
+
+        # HTML entities such as <video></video> have scannable attrs in
+        #   children elements, which also need to be sanitized.
+        #
+        node.children.each do |child_node|
+          sanitize_node(node: child_node, remove_invalid_links: remove_invalid_links)
+        end
+      end
+
+      # Remove all invalid scheme characters before checking against the
+      # list of unsafe protocols.
+      #
+      # See https://tools.ietf.org/html/rfc3986#section-3.1
+      #
+      def safe_protocol?(scheme)
+        return false unless scheme
+
+        scheme = scheme
+          .strip
+          .downcase
+          .gsub(/[^A-Za-z\+\.\-]+/, '')
+
+        UNSAFE_PROTOCOLS.none?(scheme)
+      end
+
+      private
+
+      def sanitize_node(node:, remove_invalid_links: true)
+        ATTRS_TO_SANITIZE.each do |attr|
+          next unless node.has_attribute?(attr)
+
+          begin
+            node[attr] = node[attr].strip
+            uri = Addressable::URI.parse(node[attr])
+
+            next unless uri.scheme
+            next if safe_protocol?(uri.scheme)
+
+            node.remove_attribute(attr)
+          rescue Addressable::URI::InvalidURIError
+            node.remove_attribute(attr) if remove_invalid_links
+          end
+        end
+      end
+    end
+  end
+end

locale/gitlab.pot

@@ -15054,9 +15054,6 @@ msgstr ""
 msgid "Trigger was created successfully."
 msgstr ""
 
-msgid "Trigger was re-assigned."
-msgstr ""
-
 msgid "Trigger was successfully updated."
 msgstr ""
 
@@ -16442,9 +16439,6 @@ msgstr ""
 msgid "You could not create a new trigger."
 msgstr ""
 
-msgid "You could not take ownership of trigger."
-msgstr ""
-
 msgid "You do not have any subscriptions yet"
 msgstr ""
 

spec/controllers/groups/uploads_controller_spec.rb

@@ -10,6 +10,11 @@ describe Groups::UploadsController do
     { group_id: model }
   end
 
+  let(:other_model) { create(:group, :public) }
+  let(:other_params) do
+    { group_id: other_model }
+  end
+
   it_behaves_like 'handle uploads' do
     let(:uploader_class) { NamespaceFileUploader }
   end

spec/controllers/projects/badges_controller_spec.rb

@@ -7,51 +7,115 @@ describe Projects::BadgesController do
   let!(:pipeline) { create(:ci_empty_pipeline) }
   let(:user) { create(:user) }
 
-  before do
-    project.add_maintainer(user)
-    sign_in(user)
-  end
+  shared_examples 'a badge resource' do |badge_type|
+    context 'when pipelines are public' do
+      before do
+        project.update!(public_builds: true)
+      end
+
+      context 'when project is public' do
+        before do
+          project.update!(visibility_level: Gitlab::VisibilityLevel::PUBLIC)
+        end
+
+        it "returns the #{badge_type} badge to unauthenticated users" do
+          get_badge(badge_type)
+
+          expect(response).to have_gitlab_http_status(:ok)
+        end
+      end
+
+      context 'when project is restricted' do
+        before do
+          project.update!(visibility_level: Gitlab::VisibilityLevel::INTERNAL)
+          project.add_guest(user)
+          sign_in(user)
+        end
+
+        it "returns the #{badge_type} badge to guest users" do
+          get_badge(badge_type)
+
+          expect(response).to have_gitlab_http_status(:ok)
+        end
+      end
+    end
 
-  it 'requests the pipeline badge successfully' do
-    get_badge(:pipeline)
+    context 'format' do
+      before do
+        project.add_maintainer(user)
+        sign_in(user)
+      end
 
-    expect(response).to have_gitlab_http_status(:ok)
-  end
+      it 'renders the `flat` badge layout by default' do
+        get_badge(badge_type)
 
-  it 'requests the coverage badge successfully' do
-    get_badge(:coverage)
+        expect(response).to render_template('projects/badges/badge')
+      end
 
-    expect(response).to have_gitlab_http_status(:ok)
-  end
+      context 'when style param is set to `flat`' do
+        it 'renders the `flat` badge layout' do
+          get_badge(badge_type, 'flat')
 
-  it 'renders the `flat` badge layout by default' do
-    get_badge(:coverage)
+          expect(response).to render_template('projects/badges/badge')
+        end
+      end
 
-    expect(response).to render_template('projects/badges/badge')
-  end
+      context 'when style param is set to an invalid type' do
+        it 'renders the `flat` (default) badge layout' do
+          get_badge(badge_type, 'xxx')
+
+          expect(response).to render_template('projects/badges/badge')
+        end
+      end
 
-  context 'when style param is set to `flat`' do
-    it 'renders the `flat` badge layout' do
-      get_badge(:coverage, 'flat')
+      context 'when style param is set to `flat-square`' do
+        it 'renders the `flat-square` badge layout' do
+          get_badge(badge_type, 'flat-square')
 
-      expect(response).to render_template('projects/badges/badge')
+          expect(response).to render_template('projects/badges/badge_flat-square')
+        end
+      end
     end
-  end
 
-  context 'when style param is set to an invalid type' do
-    it 'renders the `flat` (default) badge layout' do
-      get_badge(:coverage, 'xxx')
+    context 'when pipelines are not public' do
+      before do
+        project.update!(public_builds: false)
+      end
 
-      expect(response).to render_template('projects/badges/badge')
+      context 'when project is public' do
+        before do
+          project.update!(visibility_level: Gitlab::VisibilityLevel::PUBLIC)
+        end
+
+        it 'returns 404 to unauthenticated users' do
+          get_badge(badge_type)
+
+          expect(response).to have_gitlab_http_status(:not_found)
+        end
+      end
+
+      context 'when project is restricted to the user' do
+        before do
+          project.update!(visibility_level: Gitlab::VisibilityLevel::INTERNAL)
+          project.add_guest(user)
+          sign_in(user)
+        end
+
+        it 'defaults to project permissions' do
+          get_badge(:coverage)
+
+          expect(response).to have_gitlab_http_status(:not_found)
+        end
+      end
     end
   end
 
-  context 'when style param is set to `flat-square`' do
-    it 'renders the `flat-square` badge layout' do
-      get_badge(:coverage, 'flat-square')
+  describe '#pipeline' do
+    it_behaves_like 'a badge resource', :pipeline
+  end
 
-      expect(response).to render_template('projects/badges/badge_flat-square')
-    end
+  describe '#coverage' do
+    it_behaves_like 'a badge resource', :coverage
   end
 
   def get_badge(badge, style = nil)

spec/controllers/projects/merge_requests_controller_spec.rb

@@ -621,10 +621,100 @@ describe Projects::MergeRequestsController do
           format: :json
     end
 
-    it 'responds with serialized pipelines' do
-      expect(json_response['pipelines']).not_to be_empty
-      expect(json_response['count']['all']).to eq 1
-      expect(response).to include_pagination_headers
+    context 'with "enabled" builds on a public project' do
+      let(:project) { create(:project, :repository, :public) }
+
+      context 'for a project owner' do
+        it 'responds with serialized pipelines' do
+          expect(json_response['pipelines']).to be_present
+          expect(json_response['count']['all']).to eq(1)
+          expect(response).to include_pagination_headers
+        end
+      end
+
+      context 'for an unassociated user' do
+        let(:user) { create :user }
+
+        it 'responds with no pipelines' do
+          expect(json_response['pipelines']).to be_present
+          expect(json_response['count']['all']).to eq(1)
+          expect(response).to include_pagination_headers
+        end
+      end
+    end
+
+    context 'with private builds on a public project' do
+      let(:project) { create(:project, :repository, :public, :builds_private) }
+
+      context 'for a project owner' do
+        it 'responds with serialized pipelines' do
+          expect(json_response['pipelines']).to be_present
+          expect(json_response['count']['all']).to eq(1)
+          expect(response).to include_pagination_headers
+        end
+      end
+
+      context 'for an unassociated user' do
+        let(:user) { create :user }
+
+        it 'responds with no pipelines' do
+          expect(json_response['pipelines']).to be_empty
+          expect(json_response['count']['all']).to eq(0)
+          expect(response).to include_pagination_headers
+        end
+      end
+
+      context 'from a project fork' do
+        let(:fork_user)      { create :user }
+        let(:forked_project) { fork_project(project, fork_user, repository: true) } # Forked project carries over :builds_private
+        let(:merge_request)  { create(:merge_request_with_diffs, target_project: project, source_project: forked_project) }
+
+        context 'with private builds' do
+          context 'for the target project member' do
+            it 'does not respond with serialized pipelines' do
+              expect(json_response['pipelines']).to be_empty
+              expect(json_response['count']['all']).to eq(0)
+              expect(response).to include_pagination_headers
+            end
+          end
+
+          context 'for the source project member' do
+            let(:user) { fork_user }
+
+            it 'responds with serialized pipelines' do
+              expect(json_response['pipelines']).to be_present
+              expect(json_response['count']['all']).to eq(1)
+              expect(response).to include_pagination_headers
+            end
+          end
+        end
+
+        context 'with public builds' do
+          let(:forked_project) do
+            fork_project(project, fork_user, repository: true).tap do |new_project|
+              new_project.project_feature.update(builds_access_level: ProjectFeature::ENABLED)
+            end
+          end
+
+          context 'for the target project member' do
+            it 'does not respond with serialized pipelines' do
+              expect(json_response['pipelines']).to be_present
+              expect(json_response['count']['all']).to eq(1)
+              expect(response).to include_pagination_headers
+            end
+          end
+
+          context 'for the source project member' do
+            let(:user) { fork_user }
+
+            it 'responds with serialized pipelines' do
+              expect(json_response['pipelines']).to be_present
+              expect(json_response['count']['all']).to eq(1)
+              expect(response).to include_pagination_headers
+            end
+          end
+        end
+      end
     end
   end
 

spec/controllers/projects/uploads_controller_spec.rb

@@ -10,6 +10,11 @@ describe Projects::UploadsController do
     { namespace_id: model.namespace.to_param, project_id: model }
   end
 
+  let(:other_model) { create(:project, :public) }
+  let(:other_params) do
+    { namespace_id: other_model.namespace.to_param, project_id: other_model }
+  end
+
   it_behaves_like 'handle uploads'
 
   context 'when the URL the old style, without /-/system' do

spec/features/merge_request/user_tries_to_access_private_repository_through_new_mr_spec.rb → spec/features/merge_request/user_tries_to_access_private_project_info_through_new_mr_spec.rb

@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-describe 'Merge Request > Tries to access private repo of public project' do
+describe 'Merge Request > User tries to access private project information through the new mr page' do
   let(:current_user) { create(:user) }
   let(:private_project) do
     create(:project, :public, :repository,
@@ -35,5 +35,22 @@ describe 'Merge Request > Tries to access private repo of public project' do
     it "does not mention the project the user can't see the repo of" do
       expect(page).not_to have_content('nothing-to-see-here')
     end
+
+    context 'when the user enters label information from the private project in the querystring' do
+      let(:inaccessible_label) { create(:label, project: private_project) }
+      let(:mr_path) do
+        project_new_merge_request_path(
+          owned_project,
+          merge_request: {
+            label_ids: [inaccessible_label.id],
+            source_branch: 'feature'
+          }
+        )
+      end
+
+      it 'does not expose the label name' do
+        expect(page).not_to have_content(inaccessible_label.name)
+      end
+    end
   end
 end

spec/features/triggers_spec.rb

@@ -83,29 +83,6 @@ describe 'Triggers', :js do
     end
   end
 
-  describe 'trigger "Take ownership" workflow' do
-    before do
-      create(:ci_trigger, owner: user2, project: @project, description: trigger_title)
-      visit project_settings_ci_cd_path(@project)
-    end
-
-    it 'button "Take ownership" has correct alert' do
-      expected_alert = 'By taking ownership you will bind this trigger to your user account. With this the trigger will have access to all your projects as if it was you. Are you sure?'
-      expect(page.find('a.btn-trigger-take-ownership')['data-confirm']).to eq expected_alert
-    end
-
-    it 'take trigger ownership' do
-      # See if "Take ownership" on trigger works post trigger creation
-      page.accept_confirm do
-        first(:link, "Take ownership").send_keys(:return)
-      end
-
-      expect(page.find('.flash-notice')).to have_content 'Trigger was re-assigned.'
-      expect(page.find('.triggers-list')).to have_content trigger_title
-      expect(page.find('.triggers-list .trigger-owner')).to have_content user.name
-    end
-  end
-
   describe 'trigger "Revoke" workflow' do
     before do
       create(:ci_trigger, owner: user2, project: @project, description: trigger_title)

spec/lib/banzai/filter/wiki_link_filter_spec.rb

@@ -72,47 +72,5 @@ describe Banzai::Filter::WikiLinkFilter do
         expect(filtered_link.attribute('href').value).to eq(invalid_link)
       end
     end
-
-    context "when the slug is deemed unsafe or invalid" do
-      let(:link) { "alert(1);" }
-
-      invalid_slugs = [
-        "javascript:",
-        "JaVaScRiPt:",
-        "\u0001java\u0003script:",
-        "javascript    :",
-        "javascript:    ",
-        "javascript    :   ",
-        ":javascript:",
-        "javascript:",
-        "javascript:",
-        "javascript:",
-        "javascript:",
-        "java\0script:",
-        "   javascript:"
-      ]
-
-      invalid_slugs.each do |slug|
-        context "with the slug #{slug}" do
-          it "doesn't rewrite a (.) relative link" do
-            filtered_link = filter(
-              "<a href='.#{link}'>Link</a>",
-              project_wiki: wiki,
-              page_slug: slug).children[0]
-
-            expect(filtered_link.attribute('href').value).not_to include(slug)
-          end
-
-          it "doesn't rewrite a (..) relative link" do
-            filtered_link = filter(
-              "<a href='..#{link}'>Link</a>",
-              project_wiki: wiki,
-              page_slug: slug).children[0]
-
-            expect(filtered_link.attribute('href').value).not_to include(slug)
-          end
-        end
-      end
-    end
   end
 end

spec/lib/banzai/pipeline/wiki_pipeline_spec.rb

@@ -179,6 +179,85 @@ describe Banzai::Pipeline::WikiPipeline do
         end
       end
     end
+
+    describe "checking slug validity when assembling links" do
+      context "with a valid slug" do
+        let(:valid_slug) { "http://example.com" }
+
+        it "includes the slug in a (.) relative link" do
+          output = described_class.to_html(
+            "[Link](./alert(1);)",
+            project: project,
+            project_wiki: project_wiki,
+            page_slug: valid_slug
+          )
+
+          expect(output).to include(valid_slug)
+        end
+
+        it "includeds the slug in a (..) relative link" do
+          output = described_class.to_html(
+            "[Link](../alert(1);)",
+            project: project,
+            project_wiki: project_wiki,
+            page_slug: valid_slug
+          )
+
+          expect(output).to include(valid_slug)
+        end
+      end
+
+      context "when the slug is deemed unsafe or invalid" do
+        invalid_slugs = [
+          "javascript:",
+          "JaVaScRiPt:",
+          "\u0001java\u0003script:",
+          "javascript    :",
+          "javascript:    ",
+          "javascript    :   ",
+          ":javascript:",
+          "javascript:",
+          "javascript:",
+          "javascript:",
+          "javascript:",
+          "java\0script:",
+          "   javascript:"
+        ]
+
+        invalid_js_links = [
+          "alert(1);",
+          "alert(document.location);"
+        ]
+
+        invalid_slugs.each do |slug|
+          context "with the invalid slug #{slug}" do
+            invalid_js_links.each do |link|
+              it "doesn't include a prohibited slug in a (.) relative link '#{link}'" do
+                output = described_class.to_html(
+                  "[Link](./#{link})",
+                  project: project,
+                  project_wiki: project_wiki,
+                  page_slug: slug
+                )
+
+                expect(output).not_to include(slug)
+              end
+
+              it "doesn't include a prohibited slug in a (..) relative link '#{link}'" do
+                output = described_class.to_html(
+                  "[Link](../#{link})",
+                  project: project,
+                  project_wiki: project_wiki,
+                  page_slug: slug
+                )
+
+                expect(output).not_to include(slug)
+              end
+            end
+          end
+        end
+      end
+    end
   end
 
   describe 'videos' do

spec/lib/gitlab/octokit/middleware_spec.rb

+require 'spec_helper'
+
+describe Gitlab::Octokit::Middleware do
+  let(:app) { double(:app) }
+  let(:middleware) { described_class.new(app) }
+
+  shared_examples 'Public URL' do
+    it 'does not raise an error' do
+      expect(app).to receive(:call).with(env)
+
+      expect { middleware.call(env) }.not_to raise_error
+    end
+  end
+
+  shared_examples 'Local URL' do
+    it 'raises an error' do
+      expect { middleware.call(env) }.to raise_error(Gitlab::UrlBlocker::BlockedUrlError)
+    end
+  end
+
+  describe '#call' do
+    context 'when the URL is a public URL' do
+      let(:env) { { url: 'https://public-url.com' } }
+
+      it_behaves_like 'Public URL'
+    end
+
+    context 'when the URL is a localhost adresss' do
+      let(:env) { { url: 'http://127.0.0.1' } }
+
+      context 'when localhost requests are not allowed' do
+        before do
+          stub_application_setting(allow_local_requests_from_hooks_and_services: false)
+        end
+
+        it_behaves_like 'Local URL'
+      end
+
+      context 'when localhost requests are allowed' do
+        before do
+          stub_application_setting(allow_local_requests_from_hooks_and_services: true)
+        end
+
+        it_behaves_like 'Public URL'
+      end
+    end
+
+    context 'when the URL is a local network address' do
+      let(:env) { { url: 'http://172.16.0.0' } }
+
+      context 'when local network requests are not allowed' do
+        before do
+          stub_application_setting(allow_local_requests_from_hooks_and_services: false)
+        end
+
+        it_behaves_like 'Local URL'
+      end
+
+      context 'when local network requests are allowed' do
+        before do
+          stub_application_setting(allow_local_requests_from_hooks_and_services: true)
+        end
+
+        it_behaves_like 'Public URL'
+      end
+    end
+  end
+end

spec/lib/gitlab/url_blocker_spec.rb

@@ -68,6 +68,16 @@ describe Gitlab::UrlBlocker do
           expect(uri).to eq(Addressable::URI.parse('https://example.org'))
           expect(hostname).to eq(nil)
         end
+
+        context 'when it cannot be resolved' do
+          let(:import_url) { 'http://foobar.x' }
+
+          it 'raises error' do
+            stub_env('RSPEC_ALLOW_INVALID_URLS', 'false')
+
+            expect { described_class.validate!(import_url) }.to raise_error(described_class::BlockedUrlError)
+          end
+        end
       end
 
       context 'when the URL hostname is an IP address' do
@@ -79,6 +89,16 @@ describe Gitlab::UrlBlocker do
           expect(uri).to eq(Addressable::URI.parse('https://93.184.216.34'))
           expect(hostname).to be(nil)
         end
+
+        context 'when it is invalid' do
+          let(:import_url) { 'http://1.1.1.1.1' }
+
+          it 'raises an error' do
+            stub_env('RSPEC_ALLOW_INVALID_URLS', 'false')
+
+            expect { described_class.validate!(import_url) }.to raise_error(described_class::BlockedUrlError)
+          end
+        end
       end
     end
   end
@@ -180,8 +200,6 @@ describe Gitlab::UrlBlocker do
     end
 
     it 'returns true for a non-alphanumeric hostname' do
-      stub_resolv
-
       aggregate_failures do
         expect(described_class).to be_blocked_url('ssh://-oProxyCommand=whoami/a')
 
@@ -454,10 +472,6 @@ describe Gitlab::UrlBlocker do
     end
 
     context 'when enforce_user is' do
-      before do
-        stub_resolv
-      end
-
       context 'false (default)' do
         it 'does not block urls with a non-alphanumeric username' do
           expect(described_class).not_to be_blocked_url('ssh://-oProxyCommand=whoami@example.com/a')
@@ -505,6 +519,18 @@ describe Gitlab::UrlBlocker do
         expect(described_class.blocked_url?('https://git‌lab.com/foo/foo.bar', ascii_only: true)).to be true
       end
     end
+
+    it 'blocks urls with invalid ip address' do
+      stub_env('RSPEC_ALLOW_INVALID_URLS', 'false')
+
+      expect(described_class).to be_blocked_url('http://8.8.8.8.8')
+    end
+
+    it 'blocks urls whose hostname cannot be resolved' do
+      stub_env('RSPEC_ALLOW_INVALID_URLS', 'false')
+
+      expect(described_class).to be_blocked_url('http://foobar.x')
+    end
   end
 
   describe '#validate_hostname' do
@@ -536,10 +562,4 @@ describe Gitlab::UrlBlocker do
       end
     end
   end
-
-  # Resolv does not support resolving UTF-8 domain names
-  # See https://bugs.ruby-lang.org/issues/4270
-  def stub_resolv
-    allow(Resolv).to receive(:getaddresses).and_return([])
-  end
 end

spec/lib/gitlab/utils/sanitize_node_link_spec.rb

+require 'spec_helper'
+
+describe Gitlab::Utils::SanitizeNodeLink do
+  let(:klass) do
+    struct = Struct.new(:value)
+    struct.include(described_class)
+
+    struct
+  end
+
+  subject(:object) { klass.new(:value) }
+
+  invalid_schemes = [
+    "javascript:",
+    "JaVaScRiPt:",
+    "\u0001java\u0003script:",
+    "javascript    :",
+    "javascript:    ",
+    "javascript    :   ",
+    ":javascript:",
+    "javascript:",
+    "javascript:",
+    "   javascript:"
+  ]
+
+  invalid_schemes.each do |scheme|
+    context "with the scheme: #{scheme}" do
+      describe "#remove_unsafe_links" do
+        tags = {
+          a: {
+            doc: HTML::Pipeline.parse("<a href='#{scheme}alert(1);'>foo</a>"),
+            attr: "href",
+            node_to_check: -> (doc) { doc.children.first }
+          },
+          img: {
+            doc: HTML::Pipeline.parse("<img src='#{scheme}alert(1);'>"),
+            attr: "src",
+            node_to_check: -> (doc) { doc.children.first }
+          },
+          video: {
+            doc: HTML::Pipeline.parse("<video><source src='#{scheme}alert(1);'></video>"),
+            attr: "src",
+            node_to_check: -> (doc) { doc.children.first.children.filter("source").first }
+          }
+        }
+
+        tags.each do |tag, opts|
+          context "<#{tag}> tags" do
+            it "removes the unsafe link" do
+              node = opts[:node_to_check].call(opts[:doc])
+
+              expect { object.remove_unsafe_links({ node: node }, remove_invalid_links: true) }
+                .to change { node[opts[:attr]] }
+
+              expect(node[opts[:attr]]).to be_blank
+            end
+          end
+        end
+      end
+
+      describe "#safe_protocol?" do
+        let(:doc) { HTML::Pipeline.parse("<a href='#{scheme}alert(1);'>foo</a>") }
+        let(:node) { doc.children.first }
+        let(:uri) { Addressable::URI.parse(node['href'])}
+
+        it "returns false" do
+          expect(object.safe_protocol?(scheme)).to be_falsy
+        end
+      end
+    end
+  end
+end

spec/requests/api/triggers_spec.rb

@@ -270,34 +270,6 @@ describe API::Triggers do
     end
   end
 
-  describe 'POST /projects/:id/triggers/:trigger_id/take_ownership' do
-    context 'authenticated user with valid permissions' do
-      it 'updates owner' do
-        post api("/projects/#{project.id}/triggers/#{trigger.id}/take_ownership", user)
-
-        expect(response).to have_gitlab_http_status(200)
-        expect(json_response).to include('owner')
-        expect(trigger.reload.owner).to eq(user)
-      end
-    end
-
-    context 'authenticated user with invalid permissions' do
-      it 'does not update owner' do
-        post api("/projects/#{project.id}/triggers/#{trigger.id}/take_ownership", user2)
-
-        expect(response).to have_gitlab_http_status(403)
-      end
-    end
-
-    context 'unauthenticated user' do
-      it 'does not update owner' do
-        post api("/projects/#{project.id}/triggers/#{trigger.id}/take_ownership")
-
-        expect(response).to have_gitlab_http_status(401)
-      end
-    end
-  end
-
   describe 'DELETE /projects/:id/triggers/:trigger_id' do
     context 'authenticated user with valid permissions' do
       it 'deletes trigger' do

spec/serializers/issue_entity_spec.rb

@@ -17,4 +17,37 @@ describe IssueEntity do
   it 'has time estimation attributes' do
     expect(subject).to include(:time_estimate, :total_time_spent, :human_time_estimate, :human_total_time_spent)
   end
+
+  context 'when issue got moved' do
+    let(:public_project) { create(:project, :public) }
+    let(:member) { create(:user) }
+    let(:non_member) { create(:user) }
+    let(:issue) { create(:issue, project: public_project) }
+
+    before do
+      project.add_developer(member)
+      public_project.add_developer(member)
+      Issues::MoveService.new(public_project, member).execute(issue, project)
+    end
+
+    context 'when user cannot read target project' do
+      it 'does not return moved_to_id' do
+        request = double('request', current_user: non_member)
+
+        response = described_class.new(issue, request: request).as_json
+
+        expect(response[:moved_to_id]).to be_nil
+      end
+    end
+
+    context 'when user can read target project' do
+      it 'returns moved moved_to_id' do
+        request = double('request', current_user: member)
+
+        response = described_class.new(issue, request: request).as_json
+
+        expect(response[:moved_to_id]).to eq(issue.moved_to_id)
+      end
+    end
+  end
 end

spec/services/merge_requests/build_service_spec.rb

 # frozen_string_literal: true
-
 require 'spec_helper'
 
 describe MergeRequests::BuildService do
@@ -225,6 +224,11 @@ describe MergeRequests::BuildService do
             let(:label_ids) { [label2.id] }
             let(:milestone_id) { milestone2.id }
 
+            before do
+              # Guests are not able to assign labels or milestones to an issue
+              project.add_developer(user)
+            end
+
             it 'assigns milestone_id and label_ids instead of issue labels and milestone' do
               expect(merge_request.milestone).to eq(milestone2)
               expect(merge_request.labels).to match_array([label2])
@@ -479,4 +483,35 @@ describe MergeRequests::BuildService do
       end
     end
   end
+
+  context 'when assigning labels' do
+    let(:label_ids) { [create(:label, project: project).id] }
+
+    context 'for members with less than developer access' do
+      it 'is not allowed' do
+        expect(merge_request.label_ids).to be_empty
+      end
+    end
+
+    context 'for users allowed to assign labels' do
+      before do
+        project.add_developer(user)
+      end
+
+      context 'for labels in the project' do
+        it 'is allowed for developers' do
+          expect(merge_request.label_ids).to contain_exactly(*label_ids)
+        end
+      end
+
+      context 'for unrelated labels' do
+        let(:project_label) { create(:label, project: project) }
+        let(:label_ids) { [create(:label).id, project_label.id] }
+
+        it 'only assigns related labels' do
+          expect(merge_request.label_ids).to contain_exactly(project_label.id)
+        end
+      end
+    end
+  end
 end

spec/spec_helper.rb

@@ -3,6 +3,7 @@ SimpleCovEnv.start!
 
 ENV["RAILS_ENV"] = 'test'
 ENV["IN_MEMORY_APPLICATION_SETTINGS"] = 'true'
+ENV["RSPEC_ALLOW_INVALID_URLS"] = 'true'
 
 require File.expand_path('../config/environment', __dir__)
 require 'rspec/rails'

spec/support/shared_examples/controllers/uploads_actions_shared_examples.rb

@@ -76,6 +76,16 @@ shared_examples 'handle uploads' do
       UploadService.new(model, jpg, uploader_class).execute
     end
 
+    context 'when accessing a specific upload via different model' do
+      it 'responds with status 404' do
+        params.merge!(other_params)
+
+        show_upload
+
+        expect(response).to have_gitlab_http_status(404)
+      end
+    end
+
     context "when the model is public" do
       before do
         model.update_attribute(:visibility_level, Gitlab::VisibilityLevel::PUBLIC)

spec/uploaders/records_uploads_spec.rb

@@ -85,6 +85,27 @@ describe RecordsUploads do
       expect { existing.reload }.to raise_error(ActiveRecord::RecordNotFound)
       expect(Upload.count).to eq(1)
     end
+
+    it 'does not affect other uploads with different model but the same path' do
+      project = create(:project)
+      other_project = create(:project)
+
+      uploader = RecordsUploadsExampleUploader.new(other_project)
+
+      upload_for_project = Upload.create!(
+        path: File.join('uploads', 'rails_sample.jpg'),
+        size: 512.kilobytes,
+        model: project,
+        uploader: uploader.class.to_s
+      )
+
+      uploader.store!(upload_fixture('rails_sample.jpg'))
+
+      upload_for_project_fresh = Upload.find(upload_for_project.id)
+
+      expect(upload_for_project).to eq(upload_for_project_fresh)
+      expect(Upload.count).to eq(2)
+    end
   end
 
   describe '#destroy_upload callback' do