Merge branch '213289-cop-banning-uploaded-file-from-params-calls' into 'master'

New rubocop rule to avoid using `UploadedFile.from_params` directly

See merge request gitlab-org/gitlab!39353

.rubocop.yml

@@ -246,6 +246,13 @@ Gitlab/Json:
     - 'lib/quality/**/*'
     - 'lib/gitlab/danger/**/*'
 
+Gitlab/AvoidUploadedFileFromParams:
+  Enabled: true
+  Exclude:
+    - 'lib/gitlab/middleware/multipart.rb'
+    - 'spec/**/*'
+    - 'ee/spec/**/*'
+
 GitlabSecurity/PublicSend:
   Enabled: true
   Exclude:

rubocop/cop/gitlab/avoid_uploaded_file_from_params.rb

+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    module Gitlab
+      # This cop checks for `UploadedFile.from_params` usage.
+      # See https://docs.gitlab.com/ee/development/uploads.html#how-to-add-a-new-upload-route
+      #
+      # @example
+      #
+      #   # bad
+      #   class MyAwfulApi < Grape::API::Instance
+      #     params do
+      #       optional 'file.path', type: String
+      #       optional 'file.name', type: String
+      #       optional 'file.type', type: String
+      #       optional 'file.size', type: Integer
+      #       optional 'file.md5', type: String
+      #       optional 'file.sha1', type: String
+      #       optional 'file.sha256', type: String
+      #     end
+      #     put '/files' do
+      #       uploaded_file = UploadedFile.from_params(params, :file, FileUploader.workhorse_local_upload_path)
+      #     end
+      #   end
+      #
+      #   # good
+      #   class MyMuchBetterApi < Grape::API::Instance
+      #     params do
+      #       requires :file, type: ::API::Validations::Types::WorkhorseFile
+      #     end
+      #     put '/files' do
+      #       uploaded_file = declared_params[:file]
+      #     end
+      #   end
+      class AvoidUploadedFileFromParams < RuboCop::Cop::Cop
+        MSG = 'Use the `UploadedFile` set by `multipart.rb` instead of calling `UploadedFile.from_params` directly. See https://docs.gitlab.com/ee/development/uploads.html#how-to-add-a-new-upload-route'
+
+        def_node_matcher :calling_uploaded_file_from_params?, <<~PATTERN
+          (send (const nil? :UploadedFile) :from_params ...)
+        PATTERN
+
+        def on_send(node)
+          return unless calling_uploaded_file_from_params?(node)
+
+          add_offense(node, location: :expression)
+        end
+      end
+    end
+  end
+end

spec/rubocop/cop/gitlab/avoid_uploaded_file_from_params_spec.rb

+# frozen_string_literal: true
+
+require 'fast_spec_helper'
+require 'rubocop'
+require 'rubocop/rspec/support'
+require_relative '../../../../rubocop/cop/gitlab/avoid_uploaded_file_from_params'
+
+RSpec.describe RuboCop::Cop::Gitlab::AvoidUploadedFileFromParams, type: :rubocop do
+  include CopHelper
+
+  subject(:cop) { described_class.new }
+
+  context 'UploadedFile.from_params' do
+    it 'flags its call' do
+      expect_offense(<<~SOURCE)
+      UploadedFile.from_params(params)
+      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use the `UploadedFile` set by `multipart.rb` instead of calling `UploadedFile.from_params` directly. See https://docs.gitlab.com/ee/development/uploads.html#how-to-add-a-new-upload-route
+      SOURCE
+    end
+  end
+end