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

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

See merge request gitlab/gitlabhq!2464

2 files changed, 43 insertions, 8 deletions

diff --git a/lib/gitlab/import_export/uploads_manager.rb b/lib/gitlab/import_export/uploads_manager.rb
index 07875ebb56a..e0d4235e65b 100644
--- a/lib/gitlab/import_export/uploads_manager.rb
+++ b/lib/gitlab/import_export/uploads_manager.rb
@@ -13,13 +13,11 @@ module Gitlab
       end
 
       def save
-        copy_files(@from, uploads_export_path) if File.directory?(@from)
-
         if File.file?(@from) && @relative_export_path == 'avatar'
           copy_files(@from, File.join(uploads_export_path, @project.avatar.filename))
         end
 
-        copy_from_object_storage
+        copy_project_uploads
 
         true
       rescue => e
@@ -48,14 +46,19 @@ module Gitlab
         UploadService.new(@project, File.open(upload, 'r'), FileUploader, uploader_context).execute
       end
 
-      def copy_from_object_storage
-        return unless Gitlab::ImportExport.object_storage?
-
+      def copy_project_uploads
         each_uploader do |uploader|
           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
 
diff --git a/spec/lib/gitlab/import_export/uploads_manager_spec.rb b/spec/lib/gitlab/import_export/uploads_manager_spec.rb
index 9c3870a0af8..f799de18cd0 100644
--- a/spec/lib/gitlab/import_export/uploads_manager_spec.rb
+++ b/spec/lib/gitlab/import_export/uploads_manager_spec.rb
@@ -36,6 +36,38 @@ describe Gitlab::ImportExport::UploadsManager do
 
         expect(File).to exist(exported_file_path)
       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
 
     context 'using object storage' do