From 24d943fee00a06e5f558c999d6789ab74630b897 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Thu, 12 Jun 2025 10:53:02 +0200 Subject: [PATCH] Change media attachments in moderated posts to not be accessible (#34872) --- app/controllers/backups_controller.rb | 13 +---- app/controllers/media_proxy_controller.rb | 52 ++++++++++++++++--- app/helpers/media_component_helper.rb | 4 +- app/helpers/routing_helper.rb | 15 ++++++ app/models/media_attachment.rb | 4 ++ app/models/status_edit.rb | 2 +- app/policies/media_attachment_policy.rb | 13 +++++ .../rest/media_attachment_serializer.rb | 4 +- app/services/remove_status_service.rb | 8 ++- app/services/suspend_account_service.rb | 37 +------------ app/services/unsuspend_account_service.rb | 37 +------------ ...e_media_attachments_permissions_service.rb | 47 +++++++++++++++++ .../reports/_media_attachments.html.haml | 15 +----- 13 files changed, 140 insertions(+), 111 deletions(-) create mode 100644 app/policies/media_attachment_policy.rb create mode 100644 app/services/update_media_attachments_permissions_service.rb diff --git a/app/controllers/backups_controller.rb b/app/controllers/backups_controller.rb index 076d19874..7580acd49 100644 --- a/app/controllers/backups_controller.rb +++ b/app/controllers/backups_controller.rb @@ -12,18 +12,7 @@ class BackupsController < ApplicationController BACKUP_LINK_TIMEOUT = 1.hour.freeze def download - case Paperclip::Attachment.default_options[:storage] - when :s3, :azure - redirect_to @backup.dump.expiring_url(BACKUP_LINK_TIMEOUT.to_i), allow_other_host: true - when :fog - if Paperclip::Attachment.default_options.dig(:fog_credentials, :openstack_temp_url_key).present? - redirect_to @backup.dump.expiring_url(BACKUP_LINK_TIMEOUT.from_now), allow_other_host: true - else - redirect_to full_asset_url(@backup.dump.url), allow_other_host: true - end - when :filesystem - redirect_to full_asset_url(@backup.dump.url), allow_other_host: true - end + redirect_to expiring_asset_url(@backup.dump, BACKUP_LINK_TIMEOUT), allow_other_host: true end private diff --git a/app/controllers/media_proxy_controller.rb b/app/controllers/media_proxy_controller.rb index f68d85e44..d55b90ad8 100644 --- a/app/controllers/media_proxy_controller.rb +++ b/app/controllers/media_proxy_controller.rb @@ -9,6 +9,7 @@ class MediaProxyController < ApplicationController skip_before_action :require_functional! before_action :authenticate_user!, if: :limited_federation_mode? + before_action :set_media_attachment rescue_from ActiveRecord::RecordInvalid, with: :not_found rescue_from Mastodon::UnexpectedResponseError, with: :not_found @@ -16,25 +17,36 @@ class MediaProxyController < ApplicationController rescue_from(*Mastodon::HTTP_CONNECTION_ERRORS, with: :internal_server_error) def show - with_redis_lock("media_download:#{params[:id]}") do - @media_attachment = MediaAttachment.remote.attached.find(params[:id]) - authorize @media_attachment.status, :show? - redownload! if @media_attachment.needs_redownload? && !reject_media? + if @media_attachment.needs_redownload? && !reject_media? + with_redis_lock("media_download:#{params[:id]}") do + @media_attachment.reload # Reload once we have acquired a lock, in case the file was downloaded in the meantime + redownload! if @media_attachment.needs_redownload? + end end - redirect_to full_asset_url(@media_attachment.file.url(version)), allow_other_host: true + if requires_file_streaming? + send_file(media_attachment_file.path, type: media_attachment_file.instance_read(:content_type), disposition: 'inline') + else + redirect_to media_attachment_file_path, allow_other_host: true + end end private + def set_media_attachment + @media_attachment = MediaAttachment.attached.find(params[:id]) + authorize @media_attachment, :download? + end + def redownload! @media_attachment.download_file! + @media_attachment.download_thumbnail! @media_attachment.created_at = Time.now.utc @media_attachment.save! end - def version - if request.path.end_with?('/small') + def attachment_style + if @media_attachment.thumbnail.blank? && preview_requested? :small else :original @@ -42,6 +54,30 @@ class MediaProxyController < ApplicationController end def reject_media? - DomainBlock.reject_media?(@media_attachment.account.domain) + @media_attachment.account.remote? && DomainBlock.reject_media?(@media_attachment.account.domain) + end + + def media_attachment_file_path + if @media_attachment.discarded? + expiring_asset_url(media_attachment_file, 10.minutes) + else + full_asset_url(media_attachment_file.url(attachment_style)) + end + end + + def media_attachment_file + if @media_attachment.thumbnail.present? && preview_requested? + @media_attachment.thumbnail + else + @media_attachment.file + end + end + + def preview_requested? + request.path.end_with?('/small') + end + + def requires_file_streaming? + Paperclip::Attachment.default_options[:storage] == :filesystem && @media_attachment.discarded? end end diff --git a/app/helpers/media_component_helper.rb b/app/helpers/media_component_helper.rb index 269566528..194a19a45 100644 --- a/app/helpers/media_component_helper.rb +++ b/app/helpers/media_component_helper.rb @@ -11,9 +11,9 @@ module MediaComponentHelper src: full_asset_url(video.file.url(:original)), preview: full_asset_url(video.thumbnail.present? ? video.thumbnail.url : video.file.url(:small)), alt: video.description, + lang: status.language, blurhash: video.blurhash, frameRate: meta.dig('original', 'frame_rate'), - inline: true, aspectRatio: "#{meta.dig('original', 'width')} / #{meta.dig('original', 'height')}", media: [ serialize_media_attachment(video), @@ -34,6 +34,8 @@ module MediaComponentHelper src: full_asset_url(audio.file.url(:original)), poster: full_asset_url(audio.thumbnail.present? ? audio.thumbnail.url : status.account.avatar_static_url), alt: audio.description, + lang: status.language, + blurhash: audio.blurhash, backgroundColor: meta.dig('colors', 'background'), foregroundColor: meta.dig('colors', 'foreground'), accentColor: meta.dig('colors', 'accent'), diff --git a/app/helpers/routing_helper.rb b/app/helpers/routing_helper.rb index 1ff8b992c..b5d090db4 100644 --- a/app/helpers/routing_helper.rb +++ b/app/helpers/routing_helper.rb @@ -20,6 +20,21 @@ module RoutingHelper URI.join(asset_host, source).to_s end + def expiring_asset_url(attachment, expires_in) + case Paperclip::Attachment.default_options[:storage] + when :s3, :azure + attachment.expiring_url(expires_in.to_i) + when :fog + if Paperclip::Attachment.default_options.dig(:fog_credentials, :openstack_temp_url_key).present? + attachment.expiring_url(expires_in.from_now) + else + full_asset_url(attachment.url) + end + when :filesystem + full_asset_url(attachment.url) + end + end + def asset_host Rails.configuration.action_controller.asset_host || root_url end diff --git a/app/models/media_attachment.rb b/app/models/media_attachment.rb index 2f51da025..2e60f732b 100644 --- a/app/models/media_attachment.rb +++ b/app/models/media_attachment.rb @@ -228,6 +228,10 @@ class MediaAttachment < ApplicationRecord file.blank? && remote_url.present? end + def discarded? + status&.discarded? || (status_id.present? && status.nil?) + end + def significantly_changed? description_previously_changed? || thumbnail_updated_at_previously_changed? || file_meta_previously_changed? end diff --git a/app/models/status_edit.rb b/app/models/status_edit.rb index a64ef3490..25e222887 100644 --- a/app/models/status_edit.rb +++ b/app/models/status_edit.rb @@ -32,7 +32,7 @@ class StatusEdit < ApplicationRecord :preview_remote_url, :text_url, :meta, :blurhash, :not_processed?, :needs_redownload?, :local?, :file, :thumbnail, :thumbnail_remote_url, - :shortcode, :video?, :audio?, to: :media_attachment + :shortcode, :video?, :audio?, :discarded?, to: :media_attachment end rate_limit by: :account, family: :statuses diff --git a/app/policies/media_attachment_policy.rb b/app/policies/media_attachment_policy.rb new file mode 100644 index 000000000..bf108ae09 --- /dev/null +++ b/app/policies/media_attachment_policy.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +class MediaAttachmentPolicy < ApplicationPolicy + def download? + (record.discarded? && role.can?(:manage_reports)) || show_status? + end + + private + + def show_status? + record.status && StatusPolicy.new(current_account, record.status).show? + end +end diff --git a/app/serializers/rest/media_attachment_serializer.rb b/app/serializers/rest/media_attachment_serializer.rb index 90a2a9727..28fa0205f 100644 --- a/app/serializers/rest/media_attachment_serializer.rb +++ b/app/serializers/rest/media_attachment_serializer.rb @@ -16,7 +16,7 @@ class REST::MediaAttachmentSerializer < ActiveModel::Serializer def url if object.not_processed? nil - elsif object.needs_redownload? + elsif object.needs_redownload? || object.discarded? media_proxy_url(object.id, :original) else full_asset_url(object.file.url(:original)) @@ -28,7 +28,7 @@ class REST::MediaAttachmentSerializer < ActiveModel::Serializer end def preview_url - if object.needs_redownload? + if object.needs_redownload? || object.discarded? media_proxy_url(object.id, :small) elsif object.thumbnail.present? full_asset_url(object.thumbnail.url(:original)) diff --git a/app/services/remove_status_service.rb b/app/services/remove_status_service.rb index 522437aea..2adb8c1ed 100644 --- a/app/services/remove_status_service.rb +++ b/app/services/remove_status_service.rb @@ -147,9 +147,13 @@ class RemoveStatusService < BaseService end def remove_media - return if @options[:redraft] || !permanently? + return if @options[:redraft] - @status.media_attachments.destroy_all + if permanently? + @status.media_attachments.destroy_all + else + UpdateMediaAttachmentsPermissionsService.new.call(@status.media_attachments, :private) + end end def permanently? diff --git a/app/services/suspend_account_service.rb b/app/services/suspend_account_service.rb index 1b9d051b3..7ddf1a553 100644 --- a/app/services/suspend_account_service.rb +++ b/app/services/suspend_account_service.rb @@ -64,42 +64,7 @@ class SuspendAccountService < BaseService end def privatize_media_attachments! - attachment_names = MediaAttachment.attachment_definitions.keys - - @account.media_attachments.find_each do |media_attachment| - attachment_names.each do |attachment_name| - attachment = media_attachment.public_send(attachment_name) - styles = MediaAttachment::DEFAULT_STYLES | attachment.styles.keys - - next if attachment.blank? - - styles.each do |style| - case Paperclip::Attachment.default_options[:storage] - when :s3 - # Prevent useless S3 calls if ACLs are disabled - next if ENV['S3_PERMISSION'] == '' - - begin - attachment.s3_object(style).acl.put(acl: 'private') - rescue Aws::S3::Errors::NoSuchKey - Rails.logger.warn "Tried to change acl on non-existent key #{attachment.s3_object(style).key}" - rescue Aws::S3::Errors::NotImplemented => e - Rails.logger.error "Error trying to change ACL on #{attachment.s3_object(style).key}: #{e.message}" - end - when :fog, :azure - # Not supported - when :filesystem - begin - FileUtils.chmod(0o600 & ~File.umask, attachment.path(style)) unless attachment.path(style).nil? - rescue Errno::ENOENT - Rails.logger.warn "Tried to change permission on non-existent file #{attachment.path(style)}" - end - end - - CacheBusterWorker.perform_async(attachment.url(style)) if Rails.configuration.x.cache_buster.enabled - end - end - end + UpdateMediaAttachmentsPermissionsService.new.call(@account.media_attachments, :private) end def remove_from_trends! diff --git a/app/services/unsuspend_account_service.rb b/app/services/unsuspend_account_service.rb index 8ad01737a..180b2cd3f 100644 --- a/app/services/unsuspend_account_service.rb +++ b/app/services/unsuspend_account_service.rb @@ -59,42 +59,7 @@ class UnsuspendAccountService < BaseService end def publish_media_attachments! - attachment_names = MediaAttachment.attachment_definitions.keys - - @account.media_attachments.find_each do |media_attachment| - attachment_names.each do |attachment_name| - attachment = media_attachment.public_send(attachment_name) - styles = MediaAttachment::DEFAULT_STYLES | attachment.styles.keys - - next if attachment.blank? - - styles.each do |style| - case Paperclip::Attachment.default_options[:storage] - when :s3 - # Prevent useless S3 calls if ACLs are disabled - next if ENV['S3_PERMISSION'] == '' - - begin - attachment.s3_object(style).acl.put(acl: Paperclip::Attachment.default_options[:s3_permissions]) - rescue Aws::S3::Errors::NoSuchKey - Rails.logger.warn "Tried to change acl on non-existent key #{attachment.s3_object(style).key}" - rescue Aws::S3::Errors::NotImplemented => e - Rails.logger.error "Error trying to change ACL on #{attachment.s3_object(style).key}: #{e.message}" - end - when :fog, :azure - # Not supported - when :filesystem - begin - FileUtils.chmod(0o666 & ~File.umask, attachment.path(style)) unless attachment.path(style).nil? - rescue Errno::ENOENT - Rails.logger.warn "Tried to change permission on non-existent file #{attachment.path(style)}" - end - end - - CacheBusterWorker.perform_async(attachment.url(style)) if Rails.configuration.x.cache_buster.enabled - end - end - end + UpdateMediaAttachmentsPermissionsService.new.call(@account.media_attachments, :public) end def signed_activity_json diff --git a/app/services/update_media_attachments_permissions_service.rb b/app/services/update_media_attachments_permissions_service.rb new file mode 100644 index 000000000..311473afa --- /dev/null +++ b/app/services/update_media_attachments_permissions_service.rb @@ -0,0 +1,47 @@ +# frozen_string_literal: true + +class UpdateMediaAttachmentsPermissionsService < BaseService + def call(media_attachments_scope, direction) + # Only s3 and filesystem storage systems support modifying permissions + return unless %i(s3 filesystem).include?(Paperclip::Attachment.default_options[:storage]) + + # Prevent useless S3 calls if ACLs are disabled + return if Paperclip::Attachment.default_options[:storage] == :s3 && ENV['S3_PERMISSION'] == '' + + attachment_names = MediaAttachment.attachment_definitions.keys + + media_attachments_scope.find_each do |media_attachment| + attachment_names.each do |attachment_name| + attachment = media_attachment.public_send(attachment_name) + styles = MediaAttachment::DEFAULT_STYLES | attachment.styles.keys + + next if attachment.blank? + + styles.each do |style| + case Paperclip::Attachment.default_options[:storage] + when :s3 + acl = direction == :public ? Paperclip::Attachment.default_options[:s3_permissions] : 'private' + + begin + attachment.s3_object(style).acl.put(acl: acl) + rescue Aws::S3::Errors::NoSuchKey + Rails.logger.warn "Tried to change acl on non-existent key #{attachment.s3_object(style).key}" + rescue Aws::S3::Errors::NotImplemented => e + Rails.logger.error "Error trying to change ACL on #{attachment.s3_object(style).key}: #{e.message}" + end + when :filesystem + mask = direction == :public ? 0o666 : 0o600 + + begin + FileUtils.chmod(mask & ~File.umask, attachment.path(style)) unless attachment.path(style).nil? + rescue Errno::ENOENT + Rails.logger.warn "Tried to change permission on non-existent file #{attachment.path(style)}" + end + end + + CacheBusterWorker.perform_async(attachment.url(style)) if Rails.configuration.x.cache_buster.enabled + end + end + end + end +end diff --git a/app/views/admin/reports/_media_attachments.html.haml b/app/views/admin/reports/_media_attachments.html.haml index 45cc4c5aa..aa82ec09a 100644 --- a/app/views/admin/reports/_media_attachments.html.haml +++ b/app/views/admin/reports/_media_attachments.html.haml @@ -1,17 +1,6 @@ - if status.ordered_media_attachments.first.video? = render_video_component(status, visible: false) - elsif status.ordered_media_attachments.first.audio? - - audio = status.ordered_media_attachments.first - = react_component :audio, - alt: audio.description, - duration: audio.file.meta.dig(:original, :duration), - height: 110, - lang: status.language, - src: audio.file.url(:original) + = render_audio_component(status) - else - = react_component :media_gallery, - height: 343, - lang: status.language, - media: serialized_media_attachments(status.ordered_media_attachments), - sensitive: status.sensitive?, - visible: false + = render_media_gallery_component(status, visible: false)