Merge branch '12771-enable-SVGs-for-design-management' into 'master'

Enable uploading of SVG images for Design Management

Closes #12771

See merge request gitlab-org/gitlab!16160

app/models/blob.rb

@@ -176,7 +176,7 @@ class Blob < SimpleDelegator
   end
 
   def video?
-    UploaderHelper::VIDEO_EXT.include?(extension)
+    UploaderHelper::SAFE_VIDEO_EXT.include?(extension)
   end
 
   def readable_text?

app/models/blob_viewer/image.rb

@@ -6,7 +6,7 @@ module BlobViewer
     include ClientSide
 
     self.partial_name = 'image'
-    self.extensions = UploaderHelper::IMAGE_EXT
+    self.extensions = UploaderHelper::SAFE_IMAGE_EXT
     self.binary = true
     self.switcher_icon = 'picture-o'
     self.switcher_title = 'image'

app/models/blob_viewer/video.rb

@@ -6,7 +6,7 @@ module BlobViewer
     include ClientSide
 
     self.partial_name = 'video'
-    self.extensions = UploaderHelper::VIDEO_EXT
+    self.extensions = UploaderHelper::SAFE_VIDEO_EXT
     self.binary = true
     self.switcher_icon = 'film'
     self.switcher_title = 'video'

app/models/concerns/avatarable.rb

@@ -38,7 +38,7 @@ module Avatarable
 
   def avatar_type
     unless self.avatar.image?
-      errors.add :avatar, "file format is not supported. Please try one of the following supported formats: #{AvatarUploader::IMAGE_EXT.join(', ')}"
+      errors.add :avatar, "file format is not supported. Please try one of the following supported formats: #{AvatarUploader::SAFE_IMAGE_EXT.join(', ')}"
     end
   end
 

app/models/diff_viewer/image.rb

@@ -6,7 +6,7 @@ module DiffViewer
     include ClientSide
 
     self.partial_name = 'image'
-    self.extensions = UploaderHelper::IMAGE_EXT
+    self.extensions = UploaderHelper::SAFE_IMAGE_EXT
     self.binary = true
     self.switcher_icon = 'picture-o'
     self.switcher_title = _('image diff')

ee/app/controllers/projects/designs_controller.rb

@@ -8,7 +8,7 @@ class Projects::DesignsController < Projects::ApplicationController
   def show
     blob = design_repository.blob_at(ref, design.full_path)
 
-    send_blob(design_repository, blob, inline: (params[:inline] != 'false'))
+    send_blob(design_repository, blob, { inline: false })
   end
 
   private

ee/app/models/design_management/design.rb

@@ -130,10 +130,18 @@ module DesignManagement
       strong_memoize(:head_sha) { versions.ordered.first }
     end
 
+    def allow_dangerous_images?
+      Feature.enabled?(:design_management_allow_dangerous_images, project)
+    end
+
+    def valid_file_extensions
+      allow_dangerous_images? ? (SAFE_IMAGE_EXT + DANGEROUS_IMAGE_EXT) : SAFE_IMAGE_EXT
+    end
+
     def validate_file_is_image
-      unless image?
+      unless image? || (dangerous_image? && allow_dangerous_images?)
         message = _("Only these extensions are supported: %{extension_list}") % {
-          extension_list: Gitlab::FileTypeDetection::IMAGE_EXT.join(", ")
+          extension_list: valid_file_extensions.to_sentence
         }
         errors.add(:filename, message)
       end

ee/changelogs/unreleased/12771-enable-SVGs-for-design-management.yml

+---
+title: Allow files with .svg extensions to be uploaded as designs for Design Management
+merge_request: 16160
+author:
+type: changed

ee/spec/controllers/projects/designs_controller_spec.rb

@@ -5,9 +5,12 @@ require 'spec_helper'
 describe Projects::DesignsController do
   include DesignManagementTestHelpers
 
-  let(:project) { create(:project, :public) }
-  let(:issue) { create(:issue, project: project) }
-  let(:design) { create(:design, :with_file, issue: issue) }
+  set(:project) { create(:project, :public) }
+  set(:issue) { create(:issue, project: project) }
+  let(:file) { fixture_file_upload('spec/fixtures/dk.png', '`/png') }
+  let(:lfs_pointer) { Gitlab::Git::LfsPointerFile.new(file.read) }
+  let(:design) { create(:design, :with_lfs_file, file: lfs_pointer.pointer, issue: issue) }
+  let(:filename) { design.filename }
 
   before do
     enable_design_management
@@ -24,13 +27,35 @@ describe Projects::DesignsController do
       })
     end
 
-    it 'serves the file using workhorse' do
-      subject
+    # For security, .svg images should only ever be served with Content-Disposition: attachment.
+    # If these specs ever fail we must assess whether we should be serving svg images.
+    # See https://gitlab.com/gitlab-org/gitlab/issues/12771
+    describe 'Response headers' do
+      it 'serves LFS files with `Content-Disposition: attachment`' do
+        lfs_object = create(:lfs_object, file: file, oid: lfs_pointer.sha256, size: lfs_pointer.size)
+        create(:lfs_objects_project, project: project, lfs_object: lfs_object, repository_type: :design)
 
-      expect(response).to have_gitlab_http_status(200)
-      expect(response.header['Content-Disposition']).to eq('inline')
-      expect(response.header[Gitlab::Workhorse::DETECT_HEADER]).to eq "true"
-      expect(response.header[Gitlab::Workhorse::SEND_DATA_HEADER]).to start_with('git-blob:')
+        subject
+
+        expect(response.header['Content-Disposition']).to eq(%Q(attachment; filename*=UTF-8''#{filename}; filename=\"#{filename}\"))
+      end
+
+      context 'when the design is not an LFS file' do
+        let(:design) { create(:design, :with_file, issue: issue) }
+
+        it 'serves files with `Content-Disposition: attachment`' do
+          subject
+
+          expect(response.header['Content-Disposition']).to eq('attachment')
+        end
+
+        it 'serves files with Workhorse' do
+          subject
+
+          expect(response.header[Gitlab::Workhorse::DETECT_HEADER]).to eq "true"
+          expect(response.header[Gitlab::Workhorse::SEND_DATA_HEADER]).to start_with('git-blob:')
+        end
+      end
     end
 
     # Pass `skip_lfs_disabled_tests: true` to this shared example to disable
@@ -41,9 +66,7 @@ describe Projects::DesignsController do
     # controller will never authorize the user. Therefore #show will return a 403 and
     # we cannot test the data that it serves.
     it_behaves_like 'a controller that can serve LFS files', skip_lfs_disabled_tests: true do
-      let(:design) { create(:design, :with_lfs_file, issue: issue) }
       let(:lfs_oid) { project.design_repository.blob_at('HEAD', design.full_path).lfs_oid }
-      let(:filename) { design.filename }
       let(:filepath) { design.full_path }
     end
   end

ee/spec/features/projects/issues/design_management/user_uploads_designs_spec.rb

@@ -3,9 +3,9 @@ require 'spec_helper'
 describe 'User uploads new design', :js do
   include DesignManagementTestHelpers
 
-  let(:user) { project.owner }
-  let(:project) { create(:project_empty_repo, :public) }
-  let(:issue) { create(:issue, project: project) }
+  set(:project) { create(:project_empty_repo, :public) }
+  set(:user) { project.owner }
+  set(:issue) { create(:issue, project: project) }
 
   before do
     sign_in(user)

ee/spec/features/projects/issues/design_management/user_views_design_images_spec.rb

@@ -5,9 +5,9 @@ require 'spec_helper'
 describe 'Users views raw design image files' do
   include DesignManagementTestHelpers
 
-  let(:project) { create(:project, :public) }
-  let(:issue) { create(:issue, project: project) }
-  let(:design) { create(:design, :with_file, issue: issue, versions_count: 2) }
+  set(:project) { create(:project, :public) }
+  set(:issue) { create(:issue, project: project) }
+  set(:design) { create(:design, :with_file, issue: issue, versions_count: 2) }
   let(:newest_version) { design.versions.ordered.first }
   let(:oldest_version) { design.versions.ordered.last }
 

ee/spec/features/projects/issues/design_management/user_views_design_spec.rb

@@ -3,8 +3,8 @@ require 'spec_helper'
 describe 'User views issue designs', :js do
   include DesignManagementTestHelpers
 
-  let(:project) { create(:project_empty_repo, :public) }
-  let(:issue) { create(:issue, project: project) }
+  set(:project) { create(:project_empty_repo, :public) }
+  set(:issue) { create(:issue, project: project) }
 
   before do
     enable_design_management

ee/spec/features/projects/issues/design_management/user_views_designs_spec.rb

@@ -3,8 +3,8 @@ require 'spec_helper'
 describe 'User views issue designs', :js do
   include DesignManagementTestHelpers
 
-  let(:project) { create(:project_empty_repo, :public) }
-  let(:issue) { create(:issue, project: project) }
+  set(:project) { create(:project_empty_repo, :public) }
+  set(:issue) { create(:issue, project: project) }
 
   before do
     enable_design_management

ee/spec/features/projects/issues/design_management/user_views_designs_with_svg_xss_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe 'User views an SVG design that contains XSS', :js do
+  include DesignManagementTestHelpers
+
+  let(:project) { create(:project_empty_repo, :public) }
+  let(:issue) { create(:issue, project: project) }
+  let(:file) { Rails.root.join('spec', 'fixtures', 'logo_sample.svg') }
+  let(:design) { create(:design, :with_file, filename: 'xss.svg', file: file, issue: issue) }
+
+  before do
+    enable_design_management
+
+    visit designs_project_issue_path(
+      project,
+      issue,
+      { vueroute: design.filename }
+    )
+
+    wait_for_requests
+  end
+
+  it 'has XSS within the SVG file' do
+    file_content = File.read(file)
+
+    expect(file_content).to include("<script>alert('FAIL')</script>")
+  end
+
+  it 'displays the SVG' do
+    expect(page).to have_selector("img.design-img[alt='xss.svg']", count: 1)
+  end
+
+  it 'does not execute the JavaScript within the SVG' do
+    # The expectation is that we can call the capybara `page.dismiss_prompt`
+    # method to close a JavaScript alert prompt without a `Capybara::ModalNotFound`
+    # being raised.
+    run_expectation = -> {
+      page.dismiss_prompt(wait: 1)
+    }
+
+    # With the page loaded, there should be no alert modal
+    expect(run_expectation).to raise_error(
+      Capybara::ModalNotFound,
+      'Unable to find modal dialog'
+    )
+
+    # Perform a negative control test of the above expectation.
+    # With an alert modal displaying, the modal should be dismissable.
+    execute_script('alert(true)')
+
+    expect(run_expectation).not_to raise_error
+  end
+end

ee/spec/models/design_management/design_spec.rb

@@ -28,12 +28,35 @@ describe DesignManagement::Design do
     it { is_expected.to validate_presence_of(:filename) }
     it { is_expected.to validate_uniqueness_of(:filename).scoped_to(:issue_id) }
 
-    it "validates that the file is an image" do
+    it "validates that the extension is an image" do
       design.filename = "thing.txt"
+      extensions = described_class::SAFE_IMAGE_EXT + described_class::DANGEROUS_IMAGE_EXT
 
       expect(design).not_to be_valid
-      expect(design.errors[:filename].first)
-        .to match %r/Only these extensions are supported/
+      expect(design.errors[:filename].first).to eq(
+        "Only these extensions are supported: #{extensions.to_sentence}"
+      )
+    end
+
+    describe 'validating files with .svg extension' do
+      before do
+        design.filename = "thing.svg"
+      end
+
+      it "allows .svg files when feature flag is enabled" do
+        stub_feature_flags(design_management_allow_dangerous_images: true)
+
+        expect(design).to be_valid
+      end
+
+      it "does not allow .svg files when feature flag is disabled" do
+        stub_feature_flags(design_management_allow_dangerous_images: false)
+
+        expect(design).not_to be_valid
+        expect(design.errors[:filename].first).to eq(
+          "Only these extensions are supported: #{described_class::SAFE_IMAGE_EXT.to_sentence}"
+        )
+      end
     end
   end
 

lib/banzai/filter/video_link_filter.rb

@@ -19,13 +19,13 @@ module Banzai
 
       def query
         @query ||= begin
-          src_query = UploaderHelper::VIDEO_EXT.map do |ext|
+          src_query = UploaderHelper::SAFE_VIDEO_EXT.map do |ext|
             "'.#{ext}' = substring(@src, string-length(@src) - #{ext.size})"
           end
 
           if context[:asset_proxy_enabled].present?
             src_query.concat(
-              UploaderHelper::VIDEO_EXT.map do |ext|
+              UploaderHelper::SAFE_VIDEO_EXT.map do |ext|
                 "'.#{ext}' = substring(@data-canonical-src, string-length(@data-canonical-src) - #{ext.size})"
               end
             )

lib/gitlab/file_markdown_link_builder.rb

@@ -10,7 +10,7 @@ module Gitlab
       return unless name = markdown_name
 
       markdown = "[#{name.gsub(']', '\\]')}](#{secure_url})"
-      markdown = "!#{markdown}" if image_or_video? || dangerous?
+      markdown = "!#{markdown}" if image_or_video? || dangerous_image_or_video?
       markdown
     end
 

lib/gitlab/file_type_detection.rb

 # frozen_string_literal: true
 
-# File helpers methods.
-# It needs the method filename to be defined.
+# The method `filename` must be defined in classes that use this module.
+#
+# This module is intended to be used as a helper and not a security gate
+# to validate that a file is safe, as it identifies files only by the
+# file extension and not its actual contents.
+#
+# An example useage of this module is in `FileMarkdownLinkBuilder` that
+# renders markdown depending on a file name.
+#
+# We use Workhorse to detect the real extension when we serve files with
+# the `SendsBlob` helper methods, and ask Workhorse to set the content
+# type when it serves the file:
+# https://gitlab.com/gitlab-org/gitlab-ce/blob/33e5955/app/helpers/workhorse_helper.rb#L48.
+#
+# Because Workhorse has access to the content when it is downloaded, if
+# the type/extension doesn't match the real type, we adjust the
+# `Content-Type` and `Content-Disposition` to the one we get from the detection.
 module Gitlab
   module FileTypeDetection
-    IMAGE_EXT = %w[png jpg jpeg gif bmp tiff ico].freeze
+    SAFE_IMAGE_EXT = %w[png jpg jpeg gif bmp tiff ico].freeze
     # We recommend using the .mp4 format over .mov. Videos in .mov format can
     # still be used but you really need to make sure they are served with the
     # proper MIME type video/mp4 and not video/quicktime or your videos won't play
     # on IE >= 9.
     # http://archive.sublimevideo.info/20150912/docs.sublimevideo.net/troubleshooting.html
-    VIDEO_EXT = %w[mp4 m4v mov webm ogv].freeze
+    SAFE_VIDEO_EXT = %w[mp4 m4v mov webm ogv].freeze
+
     # These extension types can contain dangerous code and should only be embedded inline with
     # proper filtering. They should always be tagged as "Content-Disposition: attachment", not "inline".
-    DANGEROUS_EXT = %w[svg].freeze
+    DANGEROUS_IMAGE_EXT = %w[svg].freeze
+    DANGEROUS_VIDEO_EXT = [].freeze # None, yet
 
     def image?
-      extension_match?(IMAGE_EXT)
+      extension_match?(SAFE_IMAGE_EXT)
     end
 
     def video?
-      extension_match?(VIDEO_EXT)
+      extension_match?(SAFE_VIDEO_EXT)
     end
 
     def image_or_video?
       image? || video?
     end
 
-    def dangerous?
-      extension_match?(DANGEROUS_EXT)
+    def dangerous_image?
+      extension_match?(DANGEROUS_IMAGE_EXT)
+    end
+
+    def dangerous_video?
+      extension_match?(DANGEROUS_VIDEO_EXT)
+    end
+
+    def dangerous_image_or_video?
+      dangerous_image? || dangerous_video?
     end
 
     private

spec/lib/banzai/filter/video_link_filter_spec.rb

@@ -18,7 +18,7 @@ describe Banzai::Filter::VideoLinkFilter do
   let(:project) { create(:project, :repository) }
 
   context 'when the element src has a video extension' do
-    UploaderHelper::VIDEO_EXT.each do |ext|
+    UploaderHelper::SAFE_VIDEO_EXT.each do |ext|
       it "replaces the image tag 'path/video.#{ext}' with a video tag" do
         container = filter(link_to_image("/path/video.#{ext}")).children.first
 

spec/lib/gitlab/file_type_detection_spec.rb

@@ -2,38 +2,103 @@
 require 'spec_helper'
 
 describe Gitlab::FileTypeDetection do
-  def upload_fixture(filename)
-    fixture_file_upload(File.join('spec', 'fixtures', filename))
-  end
+  context 'when class is an uploader' do
+    shared_examples '#image? for an uploader' do
+      it 'returns true for an image file' do
+        uploader.store!(upload_fixture('dk.png'))
 
-  describe '#image_or_video?' do
-    context 'when class is an uploader' do
-      let(:uploader) do
-        example_uploader = Class.new(CarrierWave::Uploader::Base) do
-          include Gitlab::FileTypeDetection
+        expect(uploader).to be_image
+      end
 
-          storage :file
-        end
+      it 'returns false if filename has a dangerous image extension' do
+        uploader.store!(upload_fixture('unsanitized.svg'))
 
-        example_uploader.new
+        expect(uploader).to be_dangerous_image
+        expect(uploader).not_to be_image
       end
 
-      it 'returns true for an image file' do
+      it 'returns false for a video file' do
+        uploader.store!(upload_fixture('video_sample.mp4'))
+
+        expect(uploader).not_to be_image
+      end
+
+      it 'returns false if filename is blank' do
         uploader.store!(upload_fixture('dk.png'))
 
-        expect(uploader).to be_image_or_video
+        allow(uploader).to receive(:filename).and_return(nil)
+
+        expect(uploader).not_to be_image
       end
+    end
 
+    shared_examples '#video? for an uploader' do
       it 'returns true for a video file' do
         uploader.store!(upload_fixture('video_sample.mp4'))
 
-        expect(uploader).to be_image_or_video
+        expect(uploader).to be_video
+      end
+
+      it 'returns false for an image file' do
+        uploader.store!(upload_fixture('dk.png'))
+
+        expect(uploader).not_to be_video
+      end
+
+      it 'returns false if filename is blank' do
+        uploader.store!(upload_fixture('dk.png'))
+
+        allow(uploader).to receive(:filename).and_return(nil)
+
+        expect(uploader).not_to be_video
+      end
+    end
+
+    shared_examples '#dangerous_image? for an uploader' do
+      it 'returns true if filename has a dangerous extension' do
+        uploader.store!(upload_fixture('unsanitized.svg'))
+
+        expect(uploader).to be_dangerous_image
+      end
+
+      it 'returns false for an image file' do
+        uploader.store!(upload_fixture('dk.png'))
+
+        expect(uploader).not_to be_dangerous_image
+      end
+
+      it 'returns false for a video file' do
+        uploader.store!(upload_fixture('video_sample.mp4'))
+
+        expect(uploader).not_to be_dangerous_image
+      end
+
+      it 'returns false if filename is blank' do
+        uploader.store!(upload_fixture('dk.png'))
+
+        allow(uploader).to receive(:filename).and_return(nil)
+
+        expect(uploader).not_to be_dangerous_image
+      end
+    end
+
+    shared_examples '#dangerous_video? for an uploader' do
+      it 'returns false for a safe video file' do
+        uploader.store!(upload_fixture('video_sample.mp4'))
+
+        expect(uploader).not_to be_dangerous_video
+      end
+
+      it 'returns false if filename is a dangerous image extension' do
+        uploader.store!(upload_fixture('unsanitized.svg'))
+
+        expect(uploader).not_to be_dangerous_video
       end
 
-      it 'returns false for other extensions' do
-        uploader.store!(upload_fixture('doc_sample.txt'))
+      it 'returns false for an image file' do
+        uploader.store!(upload_fixture('dk.png'))
 
-        expect(uploader).not_to be_image_or_video
+        expect(uploader).not_to be_dangerous_video
       end
 
       it 'returns false if filename is blank' do
@@ -41,42 +106,190 @@ describe Gitlab::FileTypeDetection do
 
         allow(uploader).to receive(:filename).and_return(nil)
 
-        expect(uploader).not_to be_image_or_video
+        expect(uploader).not_to be_dangerous_video
       end
     end
 
-    context 'when class is a regular class' do
-      let(:custom_class) do
-        custom_class = Class.new do
-          include Gitlab::FileTypeDetection
-        end
+    let(:uploader) do
+      example_uploader = Class.new(CarrierWave::Uploader::Base) do
+        include Gitlab::FileTypeDetection
 
-        custom_class.new
+        storage :file
       end
 
+      example_uploader.new
+    end
+
+    def upload_fixture(filename)
+      fixture_file_upload(File.join('spec', 'fixtures', filename))
+    end
+
+    describe '#image?' do
+      include_examples '#image? for an uploader'
+    end
+
+    describe '#video?' do
+      include_examples '#video? for an uploader'
+    end
+
+    describe '#image_or_video?' do
+      include_examples '#image? for an uploader'
+      include_examples '#video? for an uploader'
+    end
+
+    describe '#dangerous_image?' do
+      include_examples '#dangerous_image? for an uploader'
+    end
+
+    describe '#dangerous_video?' do
+      include_examples '#dangerous_video? for an uploader'
+    end
+
+    describe '#dangerous_image_or_video?' do
+      include_examples '#dangerous_image? for an uploader'
+      include_examples '#dangerous_video? for an uploader'
+    end
+  end
+
+  context 'when class is a regular class' do
+    shared_examples '#image? for a regular class' do
       it 'returns true for an image file' do
         allow(custom_class).to receive(:filename).and_return('dk.png')
 
-        expect(custom_class).to be_image_or_video
+        expect(custom_class).to be_image
       end
 
+      it 'returns false if file has a dangerous image extension' do
+        allow(custom_class).to receive(:filename).and_return('unsanitized.svg')
+
+        expect(custom_class).to be_dangerous_image
+        expect(custom_class).not_to be_image
+      end
+
+      it 'returns false for any non image file' do
+        allow(custom_class).to receive(:filename).and_return('video_sample.mp4')
+
+        expect(custom_class).not_to be_image
+      end
+
+      it 'returns false if filename is blank' do
+        allow(custom_class).to receive(:filename).and_return(nil)
+
+        expect(custom_class).not_to be_image
+      end
+    end
+
+    shared_examples '#video? for a regular class' do
       it 'returns true for a video file' do
         allow(custom_class).to receive(:filename).and_return('video_sample.mp4')
 
-        expect(custom_class).to be_image_or_video
+        expect(custom_class).to be_video
+      end
+
+      it 'returns false for any non-video file' do
+        allow(custom_class).to receive(:filename).and_return('dk.png')
+
+        expect(custom_class).not_to be_video
+      end
+
+      it 'returns false if file has a dangerous image extension' do
+        allow(custom_class).to receive(:filename).and_return('unsanitized.svg')
+
+        expect(custom_class).to be_dangerous_image
+        expect(custom_class).not_to be_video
+      end
+
+      it 'returns false if filename is blank' do
+        allow(custom_class).to receive(:filename).and_return(nil)
+
+        expect(custom_class).not_to be_video
+      end
+    end
+
+    shared_examples '#dangerous_image? for a regular class' do
+      it 'returns true if file has a dangerous image extension' do
+        allow(custom_class).to receive(:filename).and_return('unsanitized.svg')
+
+        expect(custom_class).to be_dangerous_image
+      end
+
+      it 'returns false for an image file' do
+        allow(custom_class).to receive(:filename).and_return('dk.png')
+
+        expect(custom_class).not_to be_dangerous_image
+      end
+
+      it 'returns false for any non image file' do
+        allow(custom_class).to receive(:filename).and_return('video_sample.mp4')
+
+        expect(custom_class).not_to be_dangerous_image
+      end
+
+      it 'returns false if filename is blank' do
+        allow(custom_class).to receive(:filename).and_return(nil)
+
+        expect(custom_class).not_to be_dangerous_image
+      end
+    end
+
+    shared_examples '#dangerous_video? for a regular class' do
+      it 'returns false for a safe video file' do
+        allow(custom_class).to receive(:filename).and_return('video_sample.mp4')
+
+        expect(custom_class).not_to be_dangerous_video
+      end
+
+      it 'returns false for an image file' do
+        allow(custom_class).to receive(:filename).and_return('dk.png')
+
+        expect(custom_class).not_to be_dangerous_video
       end
 
-      it 'returns false for other extensions' do
-        allow(custom_class).to receive(:filename).and_return('doc_sample.txt')
+      it 'returns false if file has a dangerous image extension' do
+        allow(custom_class).to receive(:filename).and_return('unsanitized.svg')
 
-        expect(custom_class).not_to be_image_or_video
+        expect(custom_class).not_to be_dangerous_video
       end
 
       it 'returns false if filename is blank' do
         allow(custom_class).to receive(:filename).and_return(nil)
 
-        expect(custom_class).not_to be_image_or_video
+        expect(custom_class).not_to be_dangerous_video
       end
     end
+
+    let(:custom_class) do
+      custom_class = Class.new do
+        include Gitlab::FileTypeDetection
+      end
+
+      custom_class.new
+    end
+
+    describe '#image?' do
+      include_examples '#image? for a regular class'
+    end
+
+    describe '#video?' do
+      include_examples '#video? for a regular class'
+    end
+
+    describe '#image_or_video?' do
+      include_examples '#image? for a regular class'
+      include_examples '#video? for a regular class'
+    end
+
+    describe '#dangerous_image?' do
+      include_examples '#dangerous_image? for a regular class'
+    end
+
+    describe '#dangerous_video?' do
+      include_examples '#dangerous_video? for a regular class'
+    end
+
+    describe '#dangerous_image_or_video?' do
+      include_examples '#dangerous_image? for a regular class'
+      include_examples '#dangerous_video? for a regular class'
+    end
   end
 end