Merge branch 'security-330561-apollo-upload-server-update' into 'master'

Update apollo_upload_server dependency

See merge request gitlab-org/security/gitlab!1649

Gemfile

@@ -101,7 +101,7 @@ gem 'graphql', '~> 1.11.8'
 # TODO: remove app/views/graphiql/rails/editors/show.html.erb when https://github.com/rmosolgo/graphiql-rails/pull/71 is released:
 # https://gitlab.com/gitlab-org/gitlab/issues/31747
 gem 'graphiql-rails', '~> 1.4.10'
-gem 'apollo_upload_server', '~> 2.0.2'
+gem 'apollo_upload_server', '~> 2.1.0'
 gem 'graphql-docs', '~> 1.6.0', group: [:development, :test]
 gem 'graphlient', '~> 0.4.0' # Used by BulkImport feature (group::import)
 

Gemfile.lock

@@ -80,9 +80,9 @@ GEM
     aes_key_wrap (1.1.0)
     akismet (3.0.0)
     android_key_attestation (0.3.0)
-    apollo_upload_server (2.0.2)
+    apollo_upload_server (2.1.0)
+      actionpack (>= 4.2)
       graphql (>= 1.8)
-      rails (>= 4.2)
     asana (0.10.3)
       faraday (~> 1.0)
       faraday_middleware (~> 1.0)
@@ -1391,7 +1391,7 @@ DEPENDENCIES
   acts-as-taggable-on (~> 7.0)
   addressable (~> 2.8)
   akismet (~> 3.0)
-  apollo_upload_server (~> 2.0.2)
+  apollo_upload_server (~> 2.1.0)
   asana (~> 0.10.3)
   asciidoctor (~> 2.0.10)
   asciidoctor-include-ext (~> 0.3.1)

config/initializers/apollo_upload_server.rb

+# frozen_string_literal: true
+
+require 'apollo_upload_server'
+
+ApolloUploadServer::Middleware.strict_mode = true

lib/gitlab/middleware/multipart.rb

@@ -176,7 +176,7 @@ module Gitlab
         ::Gitlab::Middleware::Multipart::Handler.new(env, message).with_open_files do
           @app.call(env)
         end
-      rescue UploadedFile::InvalidPathError => e
+      rescue UploadedFile::InvalidPathError, ApolloUploadServer::GraphQLDataBuilder::OutOfBounds => e
         [400, { 'Content-Type' => 'text/plain' }, [e.message]]
       end
     end

spec/features/file_uploads/graphql_add_design_spec.rb

@@ -19,8 +19,18 @@ RSpec.describe 'Upload a design through graphQL', :js do
   let_it_be(:user) { create(:user, :admin) }
   let_it_be(:personal_access_token) { create(:personal_access_token, user: user) }
   let_it_be(:design) { create(:design) }
-  let_it_be(:operations) { { "operationName": "uploadDesign", "variables": { "files": [], "projectPath": design.project.full_path, "iid": design.issue.iid }, "query": query }.to_json }
   let_it_be(:map) { { "1": ["variables.files.0"] }.to_json }
+  let_it_be(:operations) do
+    {
+      "operationName": "uploadDesign",
+      "variables": {
+        "files": [nil],
+        "projectPath": design.project.full_path,
+        "iid": design.issue.iid
+      },
+      "query": query
+    }.to_json
+  end
 
   let(:url) { capybara_url("/api/graphql?private_token=#{personal_access_token.token}") }
   let(:file) { fixture_file_upload('spec/fixtures/dk.png') }

spec/requests/api/graphql/mutations/design_management/upload_spec.rb

@@ -11,6 +11,7 @@ RSpec.describe "uploading designs" do
   let(:project) { issue.project }
   let(:files) { [fixture_file_upload("spec/fixtures/dk.png")] }
   let(:variables) { {} }
+  let(:mutation_response) { graphql_mutation_response(:design_management_upload) }
 
   def mutation
     input = {
@@ -21,14 +22,32 @@ RSpec.describe "uploading designs" do
     graphql_mutation(:design_management_upload, input)
   end
 
-  let(:mutation_response) { graphql_mutation_response(:design_management_upload) }
-
   before do
     enable_design_management
 
     project.add_developer(current_user)
   end
 
+  context 'when the input does not include a null value for each mapped file' do
+    let(:operations) { { query: mutation.query, variables: mutation.variables.merge(files: []) } }
+    let(:mapping) { { '1' => ['variables.files.0'] } }
+    let(:params) do
+      { '1' => files.first, operations: operations.to_json, map: mapping.to_json }
+    end
+
+    it 'returns an error' do
+      workhorse_post_with_file(api('/', current_user, version: 'graphql'),
+                               params: params,
+                               file_key: '1'
+                              )
+
+      expect(response).to have_attributes(
+        code: eq('400'),
+        body: include('out-of-bounds')
+      )
+    end
+  end
+
   it "returns an error if the user is not allowed to upload designs" do
     post_graphql_mutation_with_uploads(mutation, current_user: create(:user))