Commit 666130a4 authored by Markus Koller's avatar Markus Koller Committed by GitLab Release Tools Bot

Ignore empty remote_id params from Workhorse

In https://gitlab.com/gitlab-org/security/gitlab-workhorse/-/merge_requests/3
we're changing Workhorse to always send empty values for unused fields,
to avoid any injected client parameters overriding them.

This causes an error in Rails because we're not checking for empty
strings in `remote_id` and attempting to store a remote file:

```
ObjectStorage::RemoteStoreError - Bad file path:
  app/uploaders/object_storage.rb:353:in `cache_remote_file!'
```
parent 75f8d42b
......@@ -318,7 +318,7 @@ module ObjectStorage
def cache!(new_file = sanitized_file)
# We intercept ::UploadedFile which might be stored on remote storage
# We use that for "accelerated" uploads, where we store result on remote storage
if new_file.is_a?(::UploadedFile) && new_file.remote_id
if new_file.is_a?(::UploadedFile) && new_file.remote_id.present?
return cache_remote_file!(new_file.remote_id, new_file.original_filename)
end
......
---
title: Ignore empty remote_id params from Workhorse accelerated uploads
merge_request:
author:
type: security
......@@ -76,7 +76,7 @@ module WorkhorseHelpers
"#{key}.size" => file.size
}.tap do |params|
params["#{key}.path"] = file.path if file.path
params["#{key}.remote_id"] = file.remote_id if file.respond_to?(:remote_id) && file.remote_id
params["#{key}.remote_id"] = file.remote_id if file.respond_to?(:remote_id) && file.remote_id.present?
end
end
......
......@@ -714,6 +714,19 @@ describe ObjectStorage do
end
end
context 'when empty remote_id is specified' do
let(:uploaded_file) do
UploadedFile.new(temp_file.path, remote_id: '')
end
it 'uses local storage' do
subject
expect(uploader).to be_file_storage
expect(uploader.object_store).to eq(described_class::Store::LOCAL)
end
end
context 'when valid file is specified' do
let(:uploaded_file) do
UploadedFile.new(temp_file.path, filename: "my_file.txt", remote_id: "test/123123")
......
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