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

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.
@@ -625,6 +644,21 @@ entry.
 - Moves snowplow to CE repo.
 
 
+## 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/serializers/issue_entity.rb

@@ -16,9 +16,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.compare_commits = []
       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.target_branch = find_target_branch
       merge_request.can_be_created = projects_and_branches_valid?
-      ensure_milestone_available(merge_request)
 
       # 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

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

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

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
 

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.any? { |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

@@ -86,8 +86,11 @@ module Gitlab
       #
       # The original hostname is used to validate the SSL, given in that scenario
       # we'll be making the request to the IP address, instead of using the hostname.
-      def enforce_uri_hostname(ip_address, uri, hostname, dns_rebind_protection)
-        return [uri, nil] unless dns_rebind_protection && ip_address && ip_address != hostname
+      def enforce_uri_hostname(addrs_info, uri, hostname, dns_rebind_protection)
+        address = addrs_info.first
+        ip_address = address.ip_address
+
+        return [uri, nil] unless dns_rebind_protection && ip_address != hostname
 
         uri = uri.dup
         uri.hostname = ip_address
@@ -115,6 +118,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

@@ -11627,9 +11627,6 @@ msgstr ""
 msgid "Trigger was created successfully."
 msgstr ""
 
-msgid "Trigger was re-assigned."
-msgstr ""
-
 msgid "Trigger was successfully updated."
 msgstr ""
 
@@ -12583,9 +12580,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,32 +7,54 @@ describe Projects::BadgesController do
   let!(:pipeline) { create(:ci_empty_pipeline) }
   let(:user) { create(:user) }
 
+  shared_examples 'a badge resource' do |badge_type|
+    context 'when pipelines are public' do
       before do
-    project.add_maintainer(user)
-    sign_in(user)
+        project.update!(public_builds: true)
+      end
+
+      context 'when project is public' do
+        before do
+          project.update!(visibility_level: Gitlab::VisibilityLevel::PUBLIC)
         end
 
-  it 'requests the pipeline badge successfully' do
-    get_badge(:pipeline)
+        it "returns the #{badge_type} badge to unauthenticated users" do
+          get_badge(badge_type)
 
           expect(response).to have_gitlab_http_status(:ok)
         end
+      end
 
-  it 'requests the coverage badge successfully' do
-    get_badge(:coverage)
+      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
+
+    context 'format' do
+      before do
+        project.add_maintainer(user)
+        sign_in(user)
+      end
 
       it 'renders the `flat` badge layout by default' do
-    get_badge(:coverage)
+        get_badge(badge_type)
 
         expect(response).to render_template('projects/badges/badge')
       end
 
       context 'when style param is set to `flat`' do
         it 'renders the `flat` badge layout' do
-      get_badge(:coverage, 'flat')
+          get_badge(badge_type, 'flat')
 
           expect(response).to render_template('projects/badges/badge')
         end
@@ -40,7 +62,7 @@ describe Projects::BadgesController do
 
       context 'when style param is set to an invalid type' do
         it 'renders the `flat` (default) badge layout' do
-      get_badge(:coverage, 'xxx')
+          get_badge(badge_type, 'xxx')
 
           expect(response).to render_template('projects/badges/badge')
         end
@@ -48,11 +70,53 @@ describe Projects::BadgesController do
 
       context 'when style param is set to `flat-square`' do
         it 'renders the `flat-square` badge layout' do
-      get_badge(:coverage, 'flat-square')
+          get_badge(badge_type, 'flat-square')
 
           expect(response).to render_template('projects/badges/badge_flat-square')
         end
       end
+    end
+
+    context 'when pipelines are not public' do
+      before do
+        project.update!(public_builds: false)
+      end
+
+      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
+
+  describe '#pipeline' do
+    it_behaves_like 'a badge resource', :pipeline
+  end
+
+  describe '#coverage' do
+    it_behaves_like 'a badge resource', :coverage
+  end
 
   def get_badge(badge, style = nil)
     params = {

spec/controllers/projects/merge_requests_controller_spec.rb

@@ -621,13 +621,103 @@ describe Projects::MergeRequestsController do
           format: :json
     end
 
+    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']).not_to be_empty
-      expect(json_response['count']['all']).to eq 1
+          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
+
   describe 'GET test_reports' do
     subject do
       get :test_reports,

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