Commit 27580720 authored by José Iván Vargas López's avatar José Iván Vargas López

Merge branch 'security-mk-exclude-orphaned-upload-files-from-export' into 'master'

[master] Resolve "Orphaned upload files are accessible via project exports"

Closes #2695

See merge request gitlab/gitlabhq!2453
parents 365217eb 029d5eeb
...@@ -13,13 +13,11 @@ module Gitlab ...@@ -13,13 +13,11 @@ module Gitlab
end end
def save def save
copy_files(@from, uploads_export_path) if File.directory?(@from)
if File.file?(@from) && @relative_export_path == 'avatar' if File.file?(@from) && @relative_export_path == 'avatar'
copy_files(@from, File.join(uploads_export_path, @project.avatar.filename)) copy_files(@from, File.join(uploads_export_path, @project.avatar.filename))
end end
copy_from_object_storage copy_project_uploads
true true
rescue => e rescue => e
...@@ -48,14 +46,19 @@ module Gitlab ...@@ -48,14 +46,19 @@ module Gitlab
UploadService.new(@project, File.open(upload, 'r'), FileUploader, uploader_context).execute UploadService.new(@project, File.open(upload, 'r'), FileUploader, uploader_context).execute
end end
def copy_from_object_storage def copy_project_uploads
return unless Gitlab::ImportExport.object_storage?
each_uploader do |uploader| each_uploader do |uploader|
next unless uploader.file next unless uploader.file
next if uploader.upload.local? # Already copied, using the old method
download_and_copy(uploader) if uploader.upload.local?
next unless uploader.upload.exist?
copy_files(uploader.absolute_path, File.join(uploads_export_path, uploader.upload.path))
else
next unless Gitlab::ImportExport.object_storage?
download_and_copy(uploader)
end
end end
end end
......
...@@ -36,6 +36,38 @@ describe Gitlab::ImportExport::UploadsManager do ...@@ -36,6 +36,38 @@ describe Gitlab::ImportExport::UploadsManager do
expect(File).to exist(exported_file_path) expect(File).to exist(exported_file_path)
end end
context 'with orphaned project upload files' do
let(:orphan_path) { File.join(FileUploader.absolute_base_dir(project), 'f93f088ddf492ffd950cf059002cbbb6', 'orphan.jpg') }
let(:exported_orphan_path) { "#{shared.export_path}/uploads/f93f088ddf492ffd950cf059002cbbb6/orphan.jpg" }
before do
FileUtils.mkdir_p(File.dirname(orphan_path))
FileUtils.touch(orphan_path)
end
after do
File.delete(orphan_path) if File.exist?(orphan_path)
end
it 'excludes orphaned upload files' do
manager.save
expect(File).not_to exist(exported_orphan_path)
end
end
context 'with an upload missing its file' do
before do
File.delete(upload.absolute_path)
end
it 'does not cause errors' do
manager.save
expect(shared.errors).to be_empty
end
end
end end
context 'using object storage' do context 'using object storage' do
......
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