Commit a360142b authored by Alex Kalderimis's avatar Alex Kalderimis

Update apollo_upload_server dependency

This enables us to make use of `strict_mode`, preventing denial of
service attacks.

Changelog: security
parent e60fa82f
......@@ -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)
......
......@@ -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)
......
# frozen_string_literal: true
require 'apollo_upload_server'
ApolloUploadServer::Middleware.strict_mode = true
......@@ -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
......
......@@ -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') }
......
......@@ -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))
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment