Commit 4329791e authored by Mark Chao's avatar Mark Chao

Merge branch 'sh-log-all-api-project-uploads' into 'master'

Log all API uploads that exceed max attachment size

See merge request gitlab-org/gitlab!59292
parents b8fcf994 5d6211e0
---
title: Log all API uploads that exceed max attachment size
merge_request: 59292
author:
type: changed
......@@ -71,10 +71,10 @@ module API
# This is to help determine which projects to use in https://gitlab.com/gitlab-org/gitlab/-/issues/325788
def log_if_upload_exceed_max_size(user_project, file)
return if file.size <= user_project.max_attachment_size
return if exempt_from_global_attachment_size?(user_project)
if file.size > user_project.max_attachment_size
Gitlab::AppLogger.info({ message: "File exceeds maximum size", file_bytes: file.size, project_id: user_project.id, project_path: user_project.full_path })
allowed = exempt_from_global_attachment_size?(user_project)
Gitlab::AppLogger.info({ message: "File exceeds maximum size", file_bytes: file.size, project_id: user_project.id, project_path: user_project.full_path, upload_allowed: allowed })
end
end
end
......
......@@ -1587,14 +1587,6 @@ RSpec.describe API::Projects do
expect(json_response['full_path']).to start_with("/#{project.namespace.path}/#{project.path}/uploads")
end
it "logs a warning if file exceeds attachment size" do
allow(Gitlab::CurrentSettings).to receive(:max_attachment_size).and_return(0)
expect(Gitlab::AppLogger).to receive(:info).with(hash_including(message: 'File exceeds maximum size')).and_call_original
post api("/projects/#{project.id}/uploads", user), params: { file: file }
end
it "does not leave the temporary file in place after uploading, even when the tempfile reaper does not run" do
stub_env('GITLAB_TEMPFILE_IMMEDIATE_UNLINK', '1')
tempfile = Tempfile.new('foo')
......@@ -1613,7 +1605,7 @@ RSpec.describe API::Projects do
expect(File.exist?(path)).to be(false)
end
shared_examples 'capped upload attachments' do
shared_examples 'capped upload attachments' do |upload_allowed|
it "limits the upload to 1 GB" do
expect_next_instance_of(UploadService) do |instance|
expect(instance).to receive(:override_max_attachment_size=).with(1.gigabyte).and_call_original
......@@ -1623,6 +1615,16 @@ RSpec.describe API::Projects do
expect(response).to have_gitlab_http_status(:created)
end
it "logs a warning if file exceeds attachment size" do
allow(Gitlab::CurrentSettings).to receive(:max_attachment_size).and_return(0)
expect(Gitlab::AppLogger).to receive(:info).with(
hash_including(message: 'File exceeds maximum size', upload_allowed: upload_allowed))
.and_call_original
post api("/projects/#{project.id}/uploads", user), params: { file: file }
end
end
context 'with exempted project' do
......@@ -1630,7 +1632,7 @@ RSpec.describe API::Projects do
stub_env('GITLAB_UPLOAD_API_ALLOWLIST', project.id)
end
it_behaves_like 'capped upload attachments'
it_behaves_like 'capped upload attachments', true
end
context 'with upload size enforcement disabled' do
......@@ -1638,7 +1640,7 @@ RSpec.describe API::Projects do
stub_feature_flags(enforce_max_attachment_size_upload_api: false)
end
it_behaves_like 'capped upload attachments'
it_behaves_like 'capped upload attachments', false
end
end
......
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