Skip to content

Commit f54a77e

Browse files
committed
Fix path traversal in CamaleonCmsAwsUploader and add regression coverage
1 parent df576a2 commit f54a77e

File tree

3 files changed

+123
-1
lines changed

3 files changed

+123
-1
lines changed

.github/copilot-instructions.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
When running rspec tests, please ensure that the RAILS_ENV environment variable is always set to test. The command should be executed as RAILS_ENV=test bundle exec rspec.

app/uploaders/camaleon_cms_aws_uploader.rb

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,8 @@ def objects(prefix = '/', sort = 'created_at')
3737
end
3838

3939
def fetch_file(file_name)
40+
return { error: 'Invalid file path' } unless valid_folder_path?(file_name)
41+
4042
return file_name if file_exists?(file_name)
4143

4244
return file_name if bucket.object(file_name).download_file(file_name) && file_exists?(file_name)
@@ -84,8 +86,9 @@ def file_parse(s3_file)
8486
# - same_name: false => avoid to overwrite an existent file with same key and search for an available key
8587
# - is_thumb: true => if this file is a thumbnail of an uploaded file
8688
def add_file(uploaded_io_or_file_path, key, args = {})
89+
return { error: 'Invalid file path' } unless valid_folder_path?(key)
90+
8791
args = { same_name: false, is_thumb: false }.merge(args)
88-
res = nil
8992
key = "#{@aws_settings['inner_folder']}/#{key}" if @aws_settings['inner_folder'].present? && !args[:is_thumb]
9093
key = key.cama_fix_media_key
9194
key = search_new_key(key) unless args[:same_name]
@@ -117,6 +120,8 @@ def add_folder(key)
117120

118121
# delete a folder in AWS with :key
119122
def delete_folder(key)
123+
return { error: 'Invalid folder path' } unless valid_folder_path?(key)
124+
120125
key = "#{@aws_settings['inner_folder']}/#{key}" if @aws_settings['inner_folder'].present?
121126
key = key.cama_fix_media_key
122127
bucket.objects(prefix: key.slice(1..-1) << '/').delete
@@ -125,6 +130,8 @@ def delete_folder(key)
125130

126131
# delete a file in AWS with :key
127132
def delete_file(key)
133+
return { error: 'Invalid file path' } unless valid_folder_path?(key)
134+
128135
key = "#{@aws_settings['inner_folder']}/#{key}" if @aws_settings['inner_folder'].present?
129136
key = key.cama_fix_media_key
130137
begin
Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
1+
# frozen_string_literal: true
2+
3+
require 'rails_helper'
4+
5+
RSpec.describe CamaleonCmsAwsUploader do
6+
init_site
7+
8+
let(:current_site) { Cama::Site.first.decorate }
9+
let(:hook_instance) { instance_double('UploaderHookInstance', hooks_run: nil) }
10+
let(:uploader) { described_class.new({ current_site: current_site, aws_settings: {} }, hook_instance) }
11+
let(:bucket) { instance_double('Aws::S3::Bucket') }
12+
13+
before { allow(uploader).to receive(:bucket).and_return(bucket) }
14+
15+
context 'with an invalid path containing path traversal characters' do
16+
describe '#add_file' do
17+
it 'returns an error' do
18+
expect(bucket).not_to receive(:object)
19+
20+
expect(uploader.add_file('/tmp/test.png', '../tmp/test.png')).to eql(error: 'Invalid file path')
21+
end
22+
end
23+
24+
describe '#delete_folder' do
25+
it 'returns an error' do
26+
expect(bucket).not_to receive(:objects)
27+
28+
expect(uploader.delete_folder('../tmp')).to eql(error: 'Invalid folder path')
29+
end
30+
end
31+
32+
describe '#delete_file' do
33+
it 'returns an error' do
34+
expect(bucket).not_to receive(:object)
35+
expect(hook_instance).not_to receive(:hooks_run)
36+
37+
expect(uploader.delete_file('../tmp/test.png')).to eql(error: 'Invalid file path')
38+
end
39+
end
40+
end
41+
42+
context 'with an invalid URI-like path' do
43+
describe '#add_file' do
44+
it 'returns an error' do
45+
expect(bucket).not_to receive(:object)
46+
47+
expect(uploader.add_file('/tmp/test.png', 'file:///tmp/test.png')).to eql(error: 'Invalid file path')
48+
end
49+
end
50+
51+
describe '#delete_folder' do
52+
it 'returns an error' do
53+
expect(bucket).not_to receive(:objects)
54+
55+
expect(uploader.delete_folder('s3://bucket/folder')).to eql(error: 'Invalid folder path')
56+
end
57+
end
58+
59+
describe '#delete_file' do
60+
it 'returns an error' do
61+
expect(bucket).not_to receive(:object)
62+
expect(hook_instance).not_to receive(:hooks_run)
63+
64+
expect(uploader.delete_file('https://example.com/file.txt')).to eql(error: 'Invalid file path')
65+
end
66+
end
67+
end
68+
69+
context 'with a valid file path' do
70+
describe '#add_file' do
71+
let(:s3_file) { instance_double('Aws::S3::Object') }
72+
let(:parsed_file) do
73+
{
74+
'name' => 'test.png',
75+
'folder_path' => '/safe',
76+
'url' => 'https://cdn.example.com/safe/test.png',
77+
'is_folder' => false,
78+
'file_size' => 123.45,
79+
'thumb' => '/safe/thumb/test-png.png',
80+
'file_type' => 'image',
81+
'created_at' => '2026-03-09T00:00:00Z',
82+
'dimension' => '100x100',
83+
'key' => '/safe/test.png'
84+
}
85+
end
86+
87+
before do
88+
allow(bucket).to receive(:object).and_return(s3_file)
89+
allow(s3_file).to receive(:upload_file).and_return(true)
90+
allow(uploader).to receive(:search_new_key).and_return('/safe/test.png')
91+
allow(uploader).to receive(:file_parse).with(s3_file).and_return(parsed_file)
92+
allow(uploader).to receive(:cache_item).with(parsed_file).and_return(parsed_file)
93+
end
94+
95+
it 'uploads the file and returns cached metadata' do
96+
file_path = "#{CAMALEON_CMS_ROOT}/spec/support/fixtures/rails.png"
97+
expect(hook_instance).to receive(:hooks_run).with(
98+
'uploader_aws_before_upload',
99+
hash_including(
100+
file: file_path, key: '/safe/test.png', args: hash_including(same_name: false, is_thumb: false)
101+
)
102+
)
103+
104+
result = uploader.add_file(file_path, 'safe/test.png')
105+
106+
expect(bucket).to have_received(:object).with('safe/test.png')
107+
expect(s3_file).to have_received(:upload_file).with(
108+
file_path, { acl: 'public-read' }
109+
)
110+
expect(result).to eql(parsed_file)
111+
end
112+
end
113+
end
114+
end

0 commit comments

Comments
 (0)