Change conan api to use proper workhorse validation

Removes custom code to handle workhorse params
and relies on the newer
::API::Validations::Types::WorkhorseFile validator.

changelogs/unreleased/security-fix-conan-workhorse-params.yml

+---
+title: Change conan api to use proper workhorse validation
+merge_request:
+author:
+type: security

lib/api/conan_packages.rb

@@ -26,6 +26,8 @@ module API
     PACKAGE_COMPONENT_REGEX = Gitlab::Regex.conan_recipe_component_regex
     CONAN_REVISION_REGEX = Gitlab::Regex.conan_revision_regex
 
+    CONAN_FILES = (Gitlab::Regex::Packages::CONAN_RECIPE_FILES + Gitlab::Regex::Packages::CONAN_PACKAGE_FILES).freeze
+
     before do
       require_packages_enabled!
 
@@ -259,7 +261,7 @@ module API
         end
 
         params do
-          requires :file_name, type: String, desc: 'Package file name', regexp: Gitlab::Regex.conan_file_name_regex
+          requires :file_name, type: String, desc: 'Package file name', values: CONAN_FILES
         end
         namespace 'export/:file_name', requirements: FILE_NAME_REQUIREMENTS do
           desc 'Download recipe files' do
@@ -277,7 +279,7 @@ module API
           end
 
           params do
-            use :workhorse_upload_params
+            requires :file, type: ::API::Validations::Types::WorkhorseFile, desc: 'The package file to be published (generated by Multipart middleware)'
           end
 
           route_setting :authentication, job_token_allowed: true, basic_auth_personal_access_token: true
@@ -300,7 +302,7 @@ module API
         params do
           requires :conan_package_reference, type: String, desc: 'Conan Package ID'
           requires :package_revision, type: String, desc: 'Conan Package Revision'
-          requires :file_name, type: String, desc: 'Package file name', regexp: Gitlab::Regex.conan_file_name_regex
+          requires :file_name, type: String, desc: 'Package file name', values: CONAN_FILES
         end
         namespace 'package/:conan_package_reference/:package_revision/:file_name', requirements: FILE_NAME_REQUIREMENTS do
           desc 'Download package files' do
@@ -328,7 +330,7 @@ module API
           end
 
           params do
-            use :workhorse_upload_params
+            requires :file, type: ::API::Validations::Types::WorkhorseFile, desc: 'The package file to be published (generated by Multipart middleware)'
           end
 
           route_setting :authentication, job_token_allowed: true, basic_auth_personal_access_token: true

lib/api/helpers/packages/conan/api_helpers.rb

@@ -133,7 +133,7 @@ module API
           end
 
           def track_push_package_event
-            if params[:file_name] == ::Packages::Conan::FileMetadatum::PACKAGE_BINARY && params['file.size'] > 0
+            if params[:file_name] == ::Packages::Conan::FileMetadatum::PACKAGE_BINARY && params[:file].size > 0 # rubocop: disable Style/ZeroLengthPredicate
               track_event('push_package')
             end
           end
@@ -147,9 +147,9 @@ module API
           end
 
           def create_package_file_with_type(file_type, current_package)
-            unless params['file.size'] == 0
+            unless params[:file].size == 0 # rubocop: disable Style/ZeroLengthPredicate
               # conan sends two upload requests, the first has no file, so we skip record creation if file.size == 0
-              ::Packages::Conan::CreatePackageFileService.new(current_package, uploaded_package_file, params.merge(conan_file_type: file_type)).execute
+              ::Packages::Conan::CreatePackageFileService.new(current_package, params[:file], params.merge(conan_file_type: file_type)).execute
             end
           end
 

lib/gitlab/regex.rb

@@ -6,11 +6,6 @@ module Gitlab
       CONAN_RECIPE_FILES = %w[conanfile.py conanmanifest.txt conan_sources.tgz conan_export.tgz].freeze
       CONAN_PACKAGE_FILES = %w[conaninfo.txt conanmanifest.txt conan_package.tgz].freeze
 
-      def conan_file_name_regex
-        @conan_file_name_regex ||=
-          %r{\A#{(CONAN_RECIPE_FILES + CONAN_PACKAGE_FILES).join("|")}\z}.freeze
-      end
-
       def conan_package_reference_regex
         @conan_package_reference_regex ||= %r{\A[A-Za-z0-9]+\z}.freeze
       end

spec/lib/gitlab/regex_spec.rb

@@ -180,15 +180,6 @@ RSpec.describe Gitlab::Regex do
     it { is_expected.not_to match('foo/bar') }
   end
 
-  describe '.conan_file_name_regex' do
-    subject { described_class.conan_file_name_regex }
-
-    it { is_expected.to match('conanfile.py') }
-    it { is_expected.to match('conan_package.tgz') }
-    it { is_expected.not_to match('foo.txt') }
-    it { is_expected.not_to match('!!()()') }
-  end
-
   describe '.conan_package_reference_regex' do
     subject { described_class.conan_package_reference_regex }
 

spec/requests/api/conan_packages_spec.rb

@@ -233,6 +233,18 @@ RSpec.describe API::ConanPackages do
     end
   end
 
+  shared_examples 'rejects invalid file_name' do |invalid_file_name|
+    let(:file_name) { invalid_file_name }
+
+    context 'with invalid file_name' do
+      it 'returns 400' do
+        subject
+
+        expect(response).to have_gitlab_http_status(:bad_request)
+      end
+    end
+  end
+
   shared_examples 'rejects recipe for invalid project' do
     context 'with invalid recipe path' do
       let(:recipe_path) { 'aa/bb/not-existing-project/ccc' }
@@ -697,8 +709,6 @@ RSpec.describe API::ConanPackages do
         context 'without a file from workhorse' do
           let(:params) { { file: nil } }
 
-          it_behaves_like 'package workhorse uploads'
-
           it 'rejects the request' do
             subject
 
@@ -706,6 +716,10 @@ RSpec.describe API::ConanPackages do
           end
         end
 
+        context 'with a file' do
+          it_behaves_like 'package workhorse uploads'
+        end
+
         context 'without a token' do
           it 'rejects request without a token' do
             headers_with_token.delete('HTTP_AUTHORIZATION')
@@ -864,16 +878,22 @@ RSpec.describe API::ConanPackages do
     end
 
     describe 'PUT /api/v4/packages/conan/v1/files/:package_name/package_version/:package_username/:package_channel/:recipe_revision/export/:file_name/authorize' do
-      subject { put api("/packages/conan/v1/files/#{recipe_path}/0/export/conanfile.py/authorize"), headers: headers_with_token }
+      let(:file_name) { 'conanfile.py' }
+
+      subject { put api("/packages/conan/v1/files/#{recipe_path}/0/export/#{file_name}/authorize"), headers: headers_with_token }
 
       it_behaves_like 'rejects invalid recipe'
+      it_behaves_like 'rejects invalid file_name', 'conanfile.py.git%2fgit-upload-pack'
       it_behaves_like 'workhorse authorization'
     end
 
     describe 'PUT /api/v4/packages/conan/v1/files/:package_name/package_version/:package_username/:package_channel/:recipe_revision/export/:conan_package_reference/:package_revision/:file_name/authorize' do
-      subject { put api("/packages/conan/v1/files/#{recipe_path}/0/package/123456789/0/conaninfo.txt/authorize"), headers: headers_with_token }
+      let(:file_name) { 'conaninfo.txt' }
+
+      subject { put api("/packages/conan/v1/files/#{recipe_path}/0/package/123456789/0/#{file_name}/authorize"), headers: headers_with_token }
 
       it_behaves_like 'rejects invalid recipe'
+      it_behaves_like 'rejects invalid file_name', 'conaninfo.txttest'
       it_behaves_like 'workhorse authorization'
     end
 
@@ -887,11 +907,13 @@ RSpec.describe API::ConanPackages do
           method: :put,
           file_key: :file,
           params: params,
+          send_rewritten_field: true,
           headers: headers_with_token
         )
       end
 
       it_behaves_like 'rejects invalid recipe'
+      it_behaves_like 'rejects invalid file_name', 'conanfile.py.git%2fgit-upload-pack'
       it_behaves_like 'uploads a package file'
     end
 
@@ -905,12 +927,15 @@ RSpec.describe API::ConanPackages do
           method: :put,
           file_key: :file,
           params: params,
-          headers: headers_with_token
+          headers: headers_with_token,
+          send_rewritten_field: true
         )
       end
 
       it_behaves_like 'rejects invalid recipe'
+      it_behaves_like 'rejects invalid file_name', 'conaninfo.txttest'
       it_behaves_like 'uploads a package file'
+
       context 'tracking the conan_package.tgz upload' do
         let(:file_name) { ::Packages::Conan::FileMetadatum::PACKAGE_BINARY }