LeadMagnet/leadchat
1
0
Fork 0
Code Issues Pull Requests Actions 3 Packages Projects Releases Wiki Activity

fix: Validate blob before attaching it to a record (#13115)

Browse Source 
Previously, attachments relied only on blob_id, which made it possible
to attach blobs across accounts by enumerating IDs. We now require both
blob_id and blob_key, add cross-account validation to prevent blob
reuse, and centralize the logic in a shared BlobOwnershipValidation
concern.

It also fixes a frontend bug where mixed-type action params (number +
string) were incorrectly dropped, causing attachment uploads to fail.
This commit is contained in:
Pranav
2025-12-19 19:02:21 -08:00
committed by GitHub
parent 86da3f7c06
commit 2adc040a8f
9 changed files with 278 additions and 90 deletions
Download Patch File Download Diff File Expand all files Collapse all files

43
app/controllers/api/v1/accounts/automation_rules_controller.rb
View File

@@ -1,4 +1,6 @@
class Api::V1::Accounts::AutomationRulesController < Api::V1::Accounts::BaseController
include AttachmentConcern
before_action :check_authorization
before_action :fetch_automation_rule, only: [:show, :update, :destroy, :clone]
@@ -9,25 +11,32 @@ class Api::V1::Accounts::AutomationRulesController < Api::V1::Accounts::BaseCont
def show; end
def create
blobs, actions, error = validate_and_prepare_attachments(params[:actions])
return render_could_not_create_error(error) if error
@automation_rule = Current.account.automation_rules.new(automation_rules_permit)
@automation_rule.actions = params[:actions]
@automation_rule.actions = actions
@automation_rule.conditions = params[:conditions]
render json: { error: @automation_rule.errors.messages }, status: :unprocessable_entity and return unless @automation_rule.valid?
return render_could_not_create_error(@automation_rule.errors.messages) unless @automation_rule.valid?
@automation_rule.save!
process_attachments
@automation_rule
blobs.each { |blob| @automation_rule.files.attach(blob) }
end
def update
ActiveRecord::Base.transaction do
automation_rule_update
process_attachments
blobs, actions, error = validate_and_prepare_attachments(params[:actions], @automation_rule)
return render_could_not_create_error(error) if error
ActiveRecord::Base.transaction do
@automation_rule.assign_attributes(automation_rules_permit)
@automation_rule.actions = actions if params[:actions]
@automation_rule.conditions = params[:conditions] if params[:conditions]
@automation_rule.save!
blobs.each { |blob| @automation_rule.files.attach(blob) }
rescue StandardError => e
Rails.logger.error e
render json: { error: @automation_rule.errors.messages }.to_json, status: :unprocessable_entity
render_could_not_create_error(@automation_rule.errors.messages)
end
end
@@ -43,26 +52,8 @@ class Api::V1::Accounts::AutomationRulesController < Api::V1::Accounts::BaseCont
@automation_rule = new_rule
end
def process_attachments
actions = @automation_rule.actions.filter_map { |k, _v| k if k['action_name'] == 'send_attachment' }
return if actions.blank?
actions.each do |action|
blob_id = action['action_params']
blob = ActiveStorage::Blob.find_by(id: blob_id)
@automation_rule.files.attach(blob)
end
end
private
def automation_rule_update
@automation_rule.update!(automation_rules_permit)
@automation_rule.actions = params[:actions] if params[:actions]
@automation_rule.conditions = params[:conditions] if params[:conditions]
@automation_rule.save!
end
def automation_rules_permit
params.permit(
:name, :description, :event_name, :active,

33
app/controllers/api/v1/accounts/macros_controller.rb
View File

@@ -1,4 +1,6 @@
class Api::V1::Accounts::MacrosController < Api::V1::Accounts::BaseController
include AttachmentConcern
before_action :fetch_macro, only: [:show, :update, :destroy, :execute]
before_action :check_authorization, only: [:show, :update, :destroy, :execute]
@@ -11,26 +13,32 @@ class Api::V1::Accounts::MacrosController < Api::V1::Accounts::BaseController
end
def create
blobs, actions, error = validate_and_prepare_attachments(params[:actions])
return render_could_not_create_error(error) if error
@macro = Current.account.macros.new(macros_with_user.merge(created_by_id: current_user.id))
@macro.set_visibility(current_user, permitted_params)
@macro.actions = params[:actions]
@macro.actions = actions
render json: { error: @macro.errors.messages }, status: :unprocessable_entity and return unless @macro.valid?
return render_could_not_create_error(@macro.errors.messages) unless @macro.valid?
@macro.save!
process_attachments
@macro
blobs.each { |blob| @macro.files.attach(blob) }
end
def update
blobs, actions, error = validate_and_prepare_attachments(params[:actions], @macro)
return render_could_not_create_error(error) if error
ActiveRecord::Base.transaction do
@macro.update!(macros_with_user)
@macro.assign_attributes(macros_with_user)
@macro.set_visibility(current_user, permitted_params)
process_attachments
@macro.actions = actions if params[:actions]
@macro.save!
blobs.each { |blob| @macro.files.attach(blob) }
rescue StandardError => e
Rails.logger.error e
render json: { error: @macro.errors.messages }.to_json, status: :unprocessable_entity
render_could_not_create_error(@macro.errors.messages)
end
end
@@ -47,17 +55,6 @@ class Api::V1::Accounts::MacrosController < Api::V1::Accounts::BaseController
private
def process_attachments
actions = @macro.actions.filter_map { |k, _v| k if k['action_name'] == 'send_attachment' }
return if actions.blank?
actions.each do |action|
blob_id = action['action_params']
blob = ActiveStorage::Blob.find_by(id: blob_id)
@macro.files.attach(blob)
end
end
def permitted_params
params.permit(
:name, :visibility,

2
app/controllers/api/v1/accounts/portals_controller.rb
View File

@@ -62,7 +62,7 @@ class Api::V1::Accounts::PortalsController < Api::V1::Accounts::BaseController
def process_attached_logo
blob_id = params[:blob_id]
blob = ActiveStorage::Blob.find_by(id: blob_id)
blob = ActiveStorage::Blob.find_signed(blob_id)
@portal.logo.attach(blob)
end

2
app/controllers/api/v1/accounts/upload_controller.rb
View File

@@ -59,7 +59,7 @@ class Api::V1::Accounts::UploadController < Api::V1::Accounts::BaseController
end
def render_success(file_blob)
render json: { file_url: url_for(file_blob), blob_key: file_blob.key, blob_id: file_blob.id }
render json: { file_url: url_for(file_blob), blob_id: file_blob.signed_id }
end
def render_error(message, status)

35
app/controllers/concerns/attachment_concern.rb Normal file
View File

@@ -0,0 +1,35 @@
module AttachmentConcern
extend ActiveSupport::Concern
def validate_and_prepare_attachments(actions, record = nil)
blobs = []
return [blobs, actions, nil] if actions.blank?
sanitized = actions.map do |action|
next action unless action[:action_name] == 'send_attachment'
result = process_attachment_action(action, record, blobs)
return [nil, nil, I18n.t('errors.attachments.invalid')] unless result
result
end
[blobs, sanitized, nil]
end
private
def process_attachment_action(action, record, blobs)
blob_id = action[:action_params].first
blob = ActiveStorage::Blob.find_signed(blob_id.to_s)
return action.merge(action_params: [blob.id]).tap { blobs << blob } if blob.present?
return action if blob_already_attached?(record, blob_id)
nil
end
def blob_already_attached?(record, blob_id)
record&.files&.any? { |f| f.blob_id == blob_id.to_i }
end
end

2
config/locales/en.yml
View File

@@ -62,6 +62,8 @@ en:
failed: Signup failed
assignment_policy:
not_found: Assignment policy not found
attachments:
invalid: Invalid attachment
saml:
feature_not_enabled: SAML feature not enabled for this account
sso_not_enabled: SAML SSO is not enabled for this installation

149
spec/controllers/api/v1/accounts/automation_rules_controller_spec.rb
View File

@@ -141,21 +141,14 @@ RSpec.describe 'Api::V1::Accounts::AutomationRulesController', type: :request do
end
it 'Saves file in the automation actions to send an attachments' do
file = fixture_file_upload(Rails.root.join('spec/assets/avatar.png'), 'image/png')
blob = ActiveStorage::Blob.create_and_upload!(
io: Rails.root.join('spec/assets/avatar.png').open,
filename: 'avatar.png',
content_type: 'image/png'
)
expect(account.automation_rules.count).to eq(0)
post "/api/v1/accounts/#{account.id}/upload/",
headers: administrator.create_new_auth_token,
params: { attachment: file }
expect(response).to have_http_status(:success)
blob = response.parsed_body
expect(blob['blob_key']).to be_present
expect(blob['blob_id']).to be_present
params[:actions] = [
{
'action_name': :send_message,
@@ -163,7 +156,7 @@ RSpec.describe 'Api::V1::Accounts::AutomationRulesController', type: :request do
},
{
'action_name': :send_attachment,
'action_params': [blob['blob_id']]
'action_params': [blob.signed_id]
}
]
@@ -177,29 +170,25 @@ RSpec.describe 'Api::V1::Accounts::AutomationRulesController', type: :request do
end
it 'Saves files in the automation actions to send multiple attachments' do
file_1 = fixture_file_upload(Rails.root.join('spec/assets/avatar.png'), 'image/png')
file_2 = fixture_file_upload(Rails.root.join('spec/assets/sample.png'), 'image/png')
post "/api/v1/accounts/#{account.id}/upload/",
headers: administrator.create_new_auth_token,
params: { attachment: file_1 }
blob_1 = response.parsed_body
post "/api/v1/accounts/#{account.id}/upload/",
headers: administrator.create_new_auth_token,
params: { attachment: file_2 }
blob_2 = response.parsed_body
blob_1 = ActiveStorage::Blob.create_and_upload!(
io: Rails.root.join('spec/assets/avatar.png').open,
filename: 'avatar.png',
content_type: 'image/png'
)
blob_2 = ActiveStorage::Blob.create_and_upload!(
io: Rails.root.join('spec/assets/sample.png').open,
filename: 'sample.png',
content_type: 'image/png'
)
params[:actions] = [
{
'action_name': :send_attachment,
'action_params': [blob_1['blob_id']]
'action_params': [blob_1.signed_id]
},
{
'action_name': :send_attachment,
'action_params': [blob_2['blob_id']]
'action_params': [blob_2.signed_id]
}
]
@@ -210,6 +199,46 @@ RSpec.describe 'Api::V1::Accounts::AutomationRulesController', type: :request do
automation_rule = account.automation_rules.first
expect(automation_rule.files.count).to eq(2)
end
it 'returns error for invalid attachment blob_id' do
params[:actions] = [
{
'action_name': :send_attachment,
'action_params': ['invalid_blob_id']
}
]
post "/api/v1/accounts/#{account.id}/automation_rules",
headers: administrator.create_new_auth_token,
params: params
expect(response).to have_http_status(:unprocessable_entity)
expect(response.parsed_body['error']).to eq(I18n.t('errors.attachments.invalid'))
end
it 'stores the original blob_id in action_params after create' do
blob = ActiveStorage::Blob.create_and_upload!(
io: Rails.root.join('spec/assets/avatar.png').open,
filename: 'avatar.png',
content_type: 'image/png'
)
params[:actions] = [
{
'action_name': :send_attachment,
'action_params': [blob.signed_id]
}
]
post "/api/v1/accounts/#{account.id}/automation_rules",
headers: administrator.create_new_auth_token,
params: params
automation_rule = account.automation_rules.first
attachment_action = automation_rule.actions.find { |a| a['action_name'] == 'send_attachment' }
expect(attachment_action['action_params'].first).to be_a(Integer)
expect(attachment_action['action_params'].first).to eq(automation_rule.files.first.blob_id)
end
end
end
@@ -328,6 +357,68 @@ RSpec.describe 'Api::V1::Accounts::AutomationRulesController', type: :request do
expect(body[:payload][:active]).to be(false)
expect(automation_rule.reload.active).to be(false)
end
it 'allows update with existing blob_id' do
blob = ActiveStorage::Blob.create_and_upload!(
io: Rails.root.join('spec/assets/avatar.png').open,
filename: 'avatar.png',
content_type: 'image/png'
)
automation_rule.update!(actions: [{ 'action_name' => 'send_attachment', 'action_params' => [blob.id] }])
automation_rule.files.attach(blob)
update_params[:actions] = [
{
'action_name': :send_attachment,
'action_params': [blob.id]
}
]
patch "/api/v1/accounts/#{account.id}/automation_rules/#{automation_rule.id}",
headers: administrator.create_new_auth_token,
params: update_params
expect(response).to have_http_status(:success)
end
it 'returns error for invalid blob_id on update' do
update_params[:actions] = [
{
'action_name': :send_attachment,
'action_params': [999_999]
}
]
patch "/api/v1/accounts/#{account.id}/automation_rules/#{automation_rule.id}",
headers: administrator.create_new_auth_token,
params: update_params
expect(response).to have_http_status(:unprocessable_entity)
expect(response.parsed_body['error']).to eq(I18n.t('errors.attachments.invalid'))
end
it 'allows adding new attachment on update with signed blob_id' do
blob = ActiveStorage::Blob.create_and_upload!(
io: Rails.root.join('spec/assets/avatar.png').open,
filename: 'avatar.png',
content_type: 'image/png'
)
update_params[:actions] = [
{
'action_name': :send_attachment,
'action_params': [blob.signed_id]
}
]
patch "/api/v1/accounts/#{account.id}/automation_rules/#{automation_rule.id}",
headers: administrator.create_new_auth_token,
params: update_params
expect(response).to have_http_status(:success)
expect(automation_rule.reload.files.count).to eq(1)
end
end
end

100
spec/controllers/api/v1/accounts/macros_controller_spec.rb
View File

@@ -127,18 +127,11 @@ RSpec.describe 'Api::V1::Accounts::MacrosController', type: :request do
end
it 'Saves file in the macros actions to send an attachments' do
file = fixture_file_upload(Rails.root.join('spec/assets/avatar.png'), 'image/png')
post "/api/v1/accounts/#{account.id}/upload/",
headers: administrator.create_new_auth_token,
params: { attachment: file }
expect(response).to have_http_status(:success)
blob = response.parsed_body
expect(blob['blob_key']).to be_present
expect(blob['blob_id']).to be_present
blob = ActiveStorage::Blob.create_and_upload!(
io: Rails.root.join('spec/assets/avatar.png').open,
filename: 'avatar.png',
content_type: 'image/png'
)
params[:actions] = [
{
@@ -147,7 +140,7 @@ RSpec.describe 'Api::V1::Accounts::MacrosController', type: :request do
},
{
'action_name': :send_attachment,
'action_params': [blob['blob_id']]
'action_params': [blob.signed_id]
}
]
@@ -159,6 +152,46 @@ RSpec.describe 'Api::V1::Accounts::MacrosController', type: :request do
expect(macro.files.presence).to be_truthy
expect(macro.files.count).to eq(1)
end
it 'returns error for invalid attachment blob_id' do
params[:actions] = [
{
'action_name': :send_attachment,
'action_params': ['invalid_blob_id']
}
]
post "/api/v1/accounts/#{account.id}/macros",
headers: administrator.create_new_auth_token,
params: params
expect(response).to have_http_status(:unprocessable_entity)
expect(response.parsed_body['error']).to eq(I18n.t('errors.attachments.invalid'))
end
it 'stores the original blob_id in action_params after create' do
blob = ActiveStorage::Blob.create_and_upload!(
io: Rails.root.join('spec/assets/avatar.png').open,
filename: 'avatar.png',
content_type: 'image/png'
)
params[:actions] = [
{
'action_name': :send_attachment,
'action_params': [blob.signed_id]
}
]
post "/api/v1/accounts/#{account.id}/macros",
headers: administrator.create_new_auth_token,
params: params
macro = account.macros.last
attachment_action = macro.actions.find { |a| a['action_name'] == 'send_attachment' }
expect(attachment_action['action_params'].first).to be_a(Integer)
expect(attachment_action['action_params'].first).to eq(macro.files.first.blob_id)
end
end
end
@@ -202,6 +235,47 @@ RSpec.describe 'Api::V1::Accounts::MacrosController', type: :request do
expect(response).to have_http_status(:unauthorized)
expect(json_response['error']).to eq('You are not authorized to do this action')
end
it 'allows update with existing blob_id' do
blob = ActiveStorage::Blob.create_and_upload!(
io: Rails.root.join('spec/assets/avatar.png').open,
filename: 'avatar.png',
content_type: 'image/png'
)
macro.update!(actions: [{ 'action_name' => 'send_attachment', 'action_params' => [blob.id] }])
macro.files.attach(blob)
put "/api/v1/accounts/#{account.id}/macros/#{macro.id}",
params: { actions: [{ 'action_name': :send_attachment, 'action_params': [blob.id] }] },
headers: administrator.create_new_auth_token
expect(response).to have_http_status(:success)
end
it 'returns error for invalid blob_id on update' do
put "/api/v1/accounts/#{account.id}/macros/#{macro.id}",
params: { actions: [{ 'action_name': :send_attachment, 'action_params': [999_999] }] },
headers: administrator.create_new_auth_token
expect(response).to have_http_status(:unprocessable_entity)
expect(response.parsed_body['error']).to eq(I18n.t('errors.attachments.invalid'))
end
it 'allows adding new attachment on update with signed blob_id' do
blob = ActiveStorage::Blob.create_and_upload!(
io: Rails.root.join('spec/assets/avatar.png').open,
filename: 'avatar.png',
content_type: 'image/png'
)
put "/api/v1/accounts/#{account.id}/macros/#{macro.id}",
params: { actions: [{ 'action_name': :send_attachment, 'action_params': [blob.signed_id] }] },
headers: administrator.create_new_auth_token
expect(response).to have_http_status(:success)
expect(macro.reload.files.count).to eq(1)
end
end
end

2
spec/controllers/api/v1/upload_controller_spec.rb
View File

@@ -18,7 +18,6 @@ RSpec.describe 'Api::V1::Accounts::UploadController', type: :request do
blob = response.parsed_body
expect(blob['errors']).to be_nil
expect(blob['file_url']).to be_present
expect(blob['blob_key']).to be_present
expect(blob['blob_id']).to be_present
end
@@ -53,7 +52,6 @@ RSpec.describe 'Api::V1::Accounts::UploadController', type: :request do
blob = response.parsed_body
expect(blob['error']).to be_nil
expect(blob['file_url']).to be_present
expect(blob['blob_key']).to be_present
expect(blob['blob_id']).to be_present
end