Automatically rescan attachments

app/controllers/admin/settings/virus_scanning_settings_controller.rb

@@ -31,8 +31,7 @@ module Admin::Settings
     menu_item :settings_attachments
 
     before_action :require_ee
-    before_action :check_clamav, only: %i[update], if: -> { scan_will_enable? }
-    before_action :mark_unscanned_attachments, if: -> { scan_will_enable? }
+    before_action :check_clamav, only: %i[update], if: -> { scan_enabled? }
 
     def default_breadcrumb
       t('settings.antivirus.title')
@@ -73,19 +72,34 @@ module Admin::Settings
       redirect_to action: :show
     end
 
-    def scan_will_enable?
-      Setting.antivirus_scan_mode == :disabled && params.dig(:settings, :antivirus_scan_mode) != 'disabled'
+    def scan_enabled?
+      Setting.antivirus_scan_mode != :disabled || params.dig(:settings, :antivirus_scan_mode) != 'disabled'
     end
 
     def success_callback(_call)
       if Setting.antivirus_scan_mode == :disabled && Attachment.status_quarantined.any?
-        flash[:info] = t('settings.antivirus.remaining_quarantined_files_html',
-                         link: helpers.link_to(t('antivirus_scan.quarantined_attachments.title'), admin_quarantined_attachments_path),
-                         file_count: t(:label_x_files, count: Attachment.status_quarantined.count))
-        redirect_to action: :show
+        remaining_quarantine_warning
+      elsif scan_enabled?
+        rescan_files
       else
         super
       end
     end
+
+    def rescan_files
+      flash[:info] = t('settings.antivirus.remaining_rescanned_files',
+                       file_count: t(:label_x_files, count: Attachment.status_uploaded.count))
+      Attachment.status_uploaded.update_all(status: :rescan)
+
+      job = Attachments::VirusRescanJob.perform_later
+      redirect_to job_status_path(job.job_id)
+    end
+
+    def remaining_quarantine_warning
+      flash[:info] = t('settings.antivirus.remaining_quarantined_files_html',
+                       link: helpers.link_to(t('antivirus_scan.quarantined_attachments.title'), admin_quarantined_attachments_path),
+                       file_count: t(:label_x_files, count: Attachment.status_quarantined.count))
+      redirect_to action: :show
+    end
   end
 end

app/models/attachment.rb

@@ -33,7 +33,8 @@ class Attachment < ApplicationRecord
     uploaded: 0,
     prepared: 1,
     scanned: 2,
-    quarantined: 3
+    quarantined: 3,
+    rescan: 4
   }.freeze, _prefix: true
 
   belongs_to :container, polymorphic: true

app/views/admin/settings/virus_scanning_settings/_unscanned_dialog.html.erb

@@ -1,8 +0,0 @@
-<%=
-  render(Primer::Alpha::Dialog.new(title: t('settings.antivirus.unscanned.title'))) do |d|
-    d.with_header(variant: :large)
-    d.with_body do
-      "You have not scanned #{attachments.pluck(:filename)}"
-    end
-  end
-%>

app/views/admin/settings/virus_scanning_settings/show.html.erb

@@ -28,10 +28,6 @@ See COPYRIGHT and LICENSE files for more details.
 ++#%>
 <%= toolbar title: t(:'settings.antivirus.title') %>
 
-<% if true || (Setting::VirusScanning.enabled? && @unscanned_atachments&.exists?) %>
-  <%= render partial: 'unscanned_dialog', locals: { attachments: Attachment.status_uploaded } %>
-<% end %>
-
 <%= styled_form_tag(
       admin_settings_virus_scanning_path,
       method: :patch,

app/workers/attachments/virus_rescan_job.rb

@@ -0,0 +1,68 @@
+#-- copyright
+# OpenProject is an open source project management software.
+# Copyright (C) 2012-2024 the OpenProject GmbH
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License version 3.
+#
+# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
+# Copyright (C) 2006-2013 Jean-Philippe Lang
+# Copyright (C) 2010-2013 the ChiliProject Team
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+#
+# See COPYRIGHT and LICENSE files for more details.
+#++
+
+module Attachments
+  class VirusRescanJob < VirusScanJob
+    queue_with_priority :low
+
+    def perform
+      return unless Setting::VirusScanning.enabled?
+
+      User.execute_as(User.system) do
+        OpenProject::Mutex.with_advisory_lock(Attachment, 'virus_rescan') do
+          Attachment.status_rescan.find_each do |attachment|
+            scan_attachment(attachment)
+          rescue StandardError => e
+            Rails.logger.error "Failed to rescan #{attachment.id} for viruses: #{e.message}. Attachment will remain accessible."
+          end
+        end
+      end
+      redirect_status
+    end
+
+    def redirect_status
+      path = ApplicationController.helpers.admin_quarantined_attachments_path
+      payload = redirect_payload(path)
+      html = I18n.t('settings.antivirus.remaining_scan_complete_html',
+                    file_count: I18n.t(:label_x_files, count: Attachment.status_quarantined.count))
+
+      upsert_status(
+        status: :success,
+        payload: payload.merge(html:)
+      )
+    end
+
+    def store_status?
+      true
+    end
+
+    def updates_own_status?
+      true
+    end
+  end
+end

app/workers/attachments/virus_scan_job.rb

@@ -52,6 +52,7 @@ module Attachments
     private
 
     def scan_attachment(attachment)
+      Rails.logger.debug { "Scanning file #{attachment.id} for viruses." }
       service = Attachments::ClamAVService.new
       response = service.scan(attachment)
       case response

config/locales/en.yml

@@ -3001,14 +3001,17 @@ en:
     antivirus:
       title: "Virus scanning"
       clamav_ping_failed: "Failed to connect the the ClamAV daemon. Double-check the configuration and try again."
-      unscanned:
-        title: 'Unscanned attachments'
+      remaining_quarantined_files_html: >
+        Virus scanning has been disbled. %{file_count} remain in quarantine.
+        To review quarantined files, please visit this link: %{link}
+      remaining_scan_complete_html: >
+        Remaining files have been scanned. There are %{file_count} in quarantine.
+        You are being redirected to the quarantine page. Use this page to delete or override quarantined files.
+      remaining_rescanned_files: >
+        With virus scanning now active, there are %{file_count} that need to be rescanned. 
+        This process has been scheduled in the background. The files will remain accessible during the scan.
       upsale:
         description: "Ensure uploaded files in OpenProject are scanned for viruses before being accessible by other users."
-      remaining_quarantined_files_html: >
-        You have successfully disabled virus scanning. 
-        There are however still %{file_count} in quarantine. 
-        Visit %{link} to clean them up."
       actions:
         delete: 'Delete the file'
         quarantine: 'Quarantine the file'

spec/jobs/attachments/virus_rescan_job_spec.rb

@@ -0,0 +1,35 @@
+require 'rails_helper'
+
+RSpec.describe Attachments::VirusRescanJob,
+               with_ee: %i[virus_scanning],
+               with_settings: { antivirus_scan_mode: :clamav_socket } do
+  let!(:attachment1) { create(:attachment, status: :uploaded) }
+  let!(:attachment2) { create(:attachment, status: :rescan) }
+  let!(:attachment3) { create(:attachment, status: :rescan) }
+
+  let(:client_double) { instance_double(ClamAV::Client) }
+
+  subject { described_class.perform_now }
+
+  before do
+    allow(ClamAV::Client).to receive(:new).and_return(client_double)
+  end
+
+  describe '#perform' do
+    let(:response) { ClamAV::SuccessResponse.new('wat') }
+
+    before do
+      allow(client_double)
+        .to receive(:execute).with(instance_of(ClamAV::Commands::InstreamCommand))
+                             .and_return(response)
+    end
+
+    it 'updates the attachments' do
+      subject
+
+      expect(attachment1.reload).to be_status_uploaded
+      expect(attachment2.reload).to be_status_scanned
+      expect(attachment3.reload).to be_status_scanned
+    end
+  end
+end

spec/jobs/attachments/virus_scan_job_spec.rb

@@ -1,6 +1,7 @@
 require 'rails_helper'
 
 RSpec.describe Attachments::VirusScanJob,
+               with_ee: %i[virus_scanning],
                with_settings: { antivirus_scan_mode: :clamav_socket } do
   let(:attachment_status) { :uploaded }
   let(:attachment) { build_stubbed(:attachment, status: attachment_status) }

spec/requests/admin/attachments/virus_scanning_settings_spec.rb

@@ -0,0 +1,140 @@
+#-- copyright
+# OpenProject is an open source project management software.
+# Copyright (C) 2012-2024 the OpenProject GmbH
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License version 3.
+#
+# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
+# Copyright (C) 2006-2013 Jean-Philippe Lang
+# Copyright (C) 2010-2013 the ChiliProject Team
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+#
+# See COPYRIGHT and LICENSE files for more details.
+#++
+
+require 'spec_helper'
+
+RSpec.describe 'Attachments virus scanning',
+               :skip_csrf,
+               type: :rails_request do
+
+  shared_let(:admin) { create(:admin) }
+  let(:service) { instance_double(Attachments::ClamAVService) }
+
+  before do
+    login_as admin
+    allow(::Attachments::ClamAVService)
+      .to receive(:new)
+      .and_return(service)
+
+    allow(service).to receive(:ping)
+  end
+
+  describe 'enabling virus scanning',
+           with_ee: %i[virus_scanning] do
+
+    subject do
+      patch '/admin/settings/virus_scanning',
+            params: {
+              settings: {
+                antivirus_scan_mode: 'clamav_socket'
+              }
+            }
+      response
+    end
+
+    it 'shows an error if ClamAV cannot be reached' do
+      allow(service).to receive(:ping).and_raise(Errno::ECONNREFUSED)
+
+      expect(subject).to be_redirect
+      follow_redirect!
+      expect(response.body).to have_text I18n.t('settings.antivirus.clamav_ping_failed')
+      expect(Setting.antivirus_scan_mode).to eq(:disabled)
+    end
+
+    it 'shows no error if ClamAV can be reached' do
+      expect(subject).to be_redirect
+      follow_redirect!
+      expect(response.body).not_to have_text I18n.t('settings.antivirus.clamav_ping_failed')
+      expect(Setting.antivirus_scan_mode).to eq(:clamav_socket)
+    end
+  end
+
+  describe 'rescanning uploaded files',
+           with_ee: %i[virus_scanning] do
+    shared_let(:attachment) { create(:attachment, status: :uploaded) }
+
+    it 'triggers rescanning of the uploaded files' do
+      patch '/admin/settings/virus_scanning',
+            params: {
+              settings: {
+                antivirus_scan_mode: 'clamav_socket'
+              }
+            }
+
+      expect(response).to be_redirect
+      follow_redirect!
+      expect(response.body).to have_text 'This process has been scheduled in the background'
+      expect(Setting.antivirus_scan_mode).to eq(:clamav_socket)
+
+      expect(attachment.reload).to be_status_rescan
+      expect(Attachments::VirusRescanJob)
+        .to have_been_enqueued
+    end
+  end
+
+  describe 'disabling virus scanning',
+           with_ee: %i[virus_scanning] do
+    shared_let(:attachment) { create(:attachment, status: :quarantined) }
+
+    it 'shows no warning if there are no quarantined files' do
+      attachment.destroy!
+      patch '/admin/settings/virus_scanning',
+            params: {
+              settings: {
+                antivirus_scan_mode: 'disabled'
+              }
+            }
+
+      expect(response).to be_redirect
+      follow_redirect!
+      expect(response.body).not_to have_text 'remain in quarantine.'
+      expect(Setting.antivirus_scan_mode).to eq(:disabled)
+    end
+
+    it 'shows a warning if there are still quarantined files' do
+      patch '/admin/settings/virus_scanning',
+            params: {
+              settings: {
+                antivirus_scan_mode: 'disabled'
+              }
+            }
+
+      expect(response).to be_redirect
+      follow_redirect!
+      expect(response.body).to have_text '1 file remain in quarantine.'
+      expect(Setting.antivirus_scan_mode).to eq(:disabled)
+    end
+  end
+
+  describe 'without ee' do
+    it 'redirects to upsale' do
+      get '/admin/settings/virus_scanning'
+      expect(response.body).to have_text 'Virus scanning is an Enterprise add-on', normalize_ws: true
+    end
+  end
+end

spec/support/rspec_request_specs.rb

@@ -13,4 +13,5 @@ RSpec.configure do |config|
   # If desired, we can use the Rails IntegrationTest request spec
   # (more like a feature spec) with this type.
   config.include RSpec::Rails::RequestExampleGroup, type: :rails_request
+  config.include Capybara::RSpecMatchers, type: :rails_request
 end