Merge pull request #6061 from betagouv/integrity_error

Marque les fichiers qui ont un pb d'intégrité comme corrompus et prévient l'utilisateur

app/jobs/virus_scanner_job.rb

@@ -11,17 +11,32 @@ class VirusScannerJob < ApplicationJob
   # If the file is not analyzed yet, retry later (to avoid clobbering metadata)
   retry_on FileNotAnalyzedYetError, wait: :exponentially_longer, attempts: 10
   # If for some reason the file appears invalid, retry for a while
-  retry_on ActiveStorage::IntegrityError, attempts: 10, wait: 5.seconds
+  retry_on(ActiveStorage::IntegrityError, attempts: 5, wait: 5.seconds) do |job, _error|
+    blob = job.arguments.first
+
+    metadata = {
+      virus_scan_result: ActiveStorage::VirusScanner::INTEGRITY_ERROR,
+      scanned_at: Time.zone.now
+    }
+
+    merge_and_update_metadata(blob, metadata)
+  end
 
   def perform(blob)
     if !blob.analyzed? then raise FileNotAnalyzedYetError end
     if blob.virus_scanner.done? then return end
 
     metadata = extract_metadata_via_virus_scanner(blob)
-    blob.update!(metadata: blob.metadata.merge(metadata))
+    VirusScannerJob.merge_and_update_metadata(blob, metadata)
   end
 
   def extract_metadata_via_virus_scanner(blob)
     ActiveStorage::VirusScanner.new(blob).metadata
   end
+
+  private
+
+  def self.merge_and_update_metadata(blob, metadata)
+    blob.update!(metadata: blob.metadata.merge(metadata))
+  end
 end

app/lib/active_storage/virus_scanner.rb

@@ -8,6 +8,7 @@ class ActiveStorage::VirusScanner
   PENDING = 'pending'
   INFECTED = 'infected'
   SAFE = 'safe'
+  INTEGRITY_ERROR = 'integrity_error'
 
   def pending?
     blob.metadata[:virus_scan_result] == PENDING
@@ -21,6 +22,10 @@ class ActiveStorage::VirusScanner
     blob.metadata[:virus_scan_result] == SAFE
   end
 
+  def corrupt?
+    blob.metadata[:virus_scan_result] == INTEGRITY_ERROR
+  end
+
   def done?
     started? && blob.metadata[:virus_scan_result] != PENDING
   end

app/views/shared/attachment/_show.html.haml

@@ -28,3 +28,9 @@
         (virus détecté, merci d’envoyer un autre fichier)
       - else
         (virus détecté, le téléchargement de ce fichier est bloqué)
+    - elsif attachment.virus_scanner.corrupt?
+      - if user_can_upload
+        (le fichier est corrompu, merci d’envoyer un autre fichier)
+      - else
+        (le fichier est corrompu, le téléchargement est bloqué)
+

spec/jobs/virus_scanner_job_spec.rb

@@ -18,6 +18,20 @@ describe VirusScannerJob, type: :job do
       blob.analyze
     end
 
+    context "when there is an integrity error" do
+      before do
+        blob.update_column('checksum', 'integrity error')
+
+        assert_performed_jobs(5) do
+          VirusScannerJob.perform_later(blob)
+        end
+      end
+
+      it do
+        expect(blob.reload.virus_scanner.corrupt?).to be_truthy
+      end
+    end
+
     context "when no virus is found" do
       before do
         allow(ClamavService).to receive(:safe_file?).and_return(true)

spec/views/shared/attachment/_show.html.haml_spec.rb

@@ -55,4 +55,14 @@ describe 'shared/attachment/_show.html.haml', type: :view do
       expect(subject).to have_text('virus détecté')
     end
   end
+
+  context 'when the file is corrupted' do
+    let(:virus_scan_result) { ActiveStorage::VirusScanner::INTEGRITY_ERROR }
+
+    it 'displays the filename, but doesn’t allow to download the file' do
+      expect(subject).to have_text(champ.piece_justificative_file.filename.to_s)
+      expect(subject).not_to have_link(champ.piece_justificative_file.filename.to_s)
+      expect(subject).to have_text('corrompu')
+    end
+  end
 end