FIX: Skip upload extension validation when changing security (#16498)

When changing upload security using `Upload#update_secure_status`,
we may not have the context of how an upload is being created, because
this code path can be run through scheduled jobs. When calling
update_secure_status, the normal ActiveRecord validations are run,
and ours include validating extensions. In some cases the upload
is created in an automated way, such as user export zips, and the
security is applied later, with the extension prohibited from
use when normally uploading.

This caused the upload to fail validation on `update_secure_status`,
causing the security change to silently fail. This fixes the issue
by skipping the file extension validation when the upload security
is being changed.
This commit is contained in:
Martin Brennan
2022-04-20 14:11:39 +10:00
committed by GitHub
parent 5a76a3669b
commit 154afa60eb
4 changed files with 89 additions and 8 deletions

View File

@ -33,6 +33,8 @@ class UploadValidator < ActiveModel::Validator
return true
end
return true if changing_upload_security?(upload)
if is_authorized?(upload, extension)
if FileHelper.is_supported_image?(upload.original_filename)
authorized_image_extension(upload, extension)
@ -44,6 +46,16 @@ class UploadValidator < ActiveModel::Validator
end
end
# this should only be run on existing records, and covers cases of
# upload.update_secure_status being run outside of the creation flow,
# where some cases e.g. have exemptions on the extension enforcement
def changing_upload_security?(upload)
!upload.new_record? && \
upload.changed_attributes.keys.all? do |attribute|
%w(secure security_last_changed_at security_last_changed_reason).include?(attribute)
end
end
def is_authorized?(upload, extension)
extension_authorized?(upload, extension, authorized_extensions(upload))
end