Merge branch 'kassio/import-validate-url-before-downloading' into 'master'

ImportExport: Validate URL before downloading

See merge request gitlab-org/gitlab!60388

lib/gitlab/import_export/command_line_util.rb

@@ -30,6 +30,8 @@ module Gitlab
       end
 
       def download(url, upload_path)
+        validate_url!(url)
+
         File.open(upload_path, 'w') do |file|
           # Download (stream) file from the uploader's location
           IO.copy_stream(URI.parse(url).open, file)
@@ -63,6 +65,19 @@ module Gitlab
         FileUtils.copy_entry(source, destination)
         true
       end
+
+      def validate_url!(url)
+        ::Gitlab::UrlBlocker.validate!(
+          url,
+          allow_localhost: allow_local_requests?,
+          allow_local_network: allow_local_requests?,
+          schemes: %w(http https)
+        )
+      end
+
+      def allow_local_requests?
+        ::Gitlab::CurrentSettings.allow_local_requests_from_web_hooks_and_services?
+      end
     end
   end
 end

spec/lib/gitlab/import_export/command_line_util_spec.rb

@@ -16,6 +16,10 @@ RSpec.describe Gitlab::ImportExport::CommandLineUtil do
       def initialize
         @shared = Gitlab::ImportExport::Shared.new(nil)
       end
+
+      def execute_download(url)
+        download(url, 'path')
+      end
     end.new
   end
 
@@ -35,4 +39,29 @@ RSpec.describe Gitlab::ImportExport::CommandLineUtil do
   it 'has the right mask for uploads' do
     expect(file_permissions("#{path}/uploads")).to eq(0755) # originally 555
   end
+
+  context 'validates the URL before executing the download' do
+    before do
+      stub_application_setting(allow_local_requests_from_web_hooks_and_services: false)
+    end
+
+    it 'raises error when the given URL is blocked' do
+      expect { subject.execute_download('http://localhost:3000/file') }
+        .to raise_error(Gitlab::UrlBlocker::BlockedUrlError, 'Requests to localhost are not allowed')
+    end
+
+    it 'executes the download when the URL is allowed' do
+      expect_next_instance_of(URI::HTTP) do |uri|
+        expect(uri)
+          .to receive(:open)
+          .and_return('file content')
+      end
+
+      expect(IO)
+        .to receive(:copy_stream)
+        .with('file content', instance_of(File))
+
+      subject.execute_download('http://some.url.remote/file')
+    end
+  end
 end