[SOLVED] How can I avoid accepting broken images in ActiveStorage?

How can I avoid accepting broken images in ActiveStorage?

I am currently migrating a Rails application (that drives screenshots.debian.net) from Paperclip to ActiveStorage. It allows users to upload PNG images that are then shown to all users.

TL;DR: The upload form accepts broken images. Imagemagick "accepts" them. ActiveStorage does not instantly validate that. I end up with broken files on disk.

This is the model for the images. I am using the 'active_storage_validations' gem:

class Screenshot < ApplicationRecord
  has_one_attached :image
  validates :image, attached: true, content_type: [:png]

  def medium_image
    if self.image.attached?
      self.image.variant(
        resize_to_limit: [670, 600]
      ).processed
    end
  end
end

The controller logic that receives uploads from an HTML form with file field:

def upload_receive
  @package = Package.find_by!(name: params[:name])
  params[:file].each do |img|
    new_screenshot = @package.screenshots.new(image: img)
    
    if new_screenshot.valid?  
      new_screenshot.save
    end
  end
end

This works well. PNG files are accepted. JPG files are rejected. Nice.

However when I upload a deliberately broken file I run into trouble. I have created a file called "broken.png" that consists of 5 KB of zero bytes. ActiveStorage accepts that file so I end up with an invalid file.

Right after the upload the image is supposed to be shown. The Screenshot.medium_image method is called that tries to create a variant. ImageMagick tries to convert resize the broken image and fails:

convert /tmp/ActiveStorage-21670-… failed with error:
convert-im6.q16: improper image header `/tmp/ActiveStorage-21670-…' @ error/png.c/ReadPNGImage/4092.
convert-im6.q16: no images defined `/tmp/image_processing….png' @ error/convert.c/ConvertImageCommand/3258.

So once such a broken image was uploaded the application will fail every time it tries to display it. Quite an easy denial-of-service.

What I understand:

The file is accepted and then the ".save" method succeeds. The broken file has just been saved to disk.
Briefly after that ActiveStorage starts a background job to analyze the file: [ActiveJob] Enqueued ActiveStorage::AnalyzeJob (Job ID: 91055878-f516-4f2f-98ff-e39573980b45) to Async(active_storage_analysis) with arguments
The job runs and prints "Skipping image analysis because ImageMagick doesn't support the file" (Source: https://github.com/rails/rails/blob/fcb5f9035fd1307c300f4ab31fda353bd6365fc3/activestorage/lib/active_storage/analyzer/image_analyzer.rb#L39)
The job writes the metadata and flags the file as identified and analyzed. ActiveStorage::Blob Update (1.0ms) UPDATE "active_storage_blobs" SET "metadata" = $1 WHERE "active_storage_blobs"."id" = $2 [["metadata", "{\"identified\":true,\"analyzed\":true}"], ["id", 21670]]

What I actually wanted:

The if new_screenshot.valid? line runs validations on the file and finds that it is broken.
No ".save" will happen. An error message is added to the flash messages.

After hours of frustrations I would be so grateful for any hint. Is this a bug in ActiveStorage or the active_storage_validations gem? Thanks.

[I'm missing Paperclip a lot. ActiveStorage doesn't seem to offer the same functionality yet.]

Solution