diff --git a/app/Modules/Files/Http/Controllers/FilesController.php b/app/Modules/Files/Http/Controllers/FilesController.php index ac344c23..108aed0c 100644 --- a/app/Modules/Files/Http/Controllers/FilesController.php +++ b/app/Modules/Files/Http/Controllers/FilesController.php @@ -177,6 +177,18 @@ class FilesController extends Controller // 12th reopens showing the 11th. 'expires_at' => $this->expiry->asShown($file, $request->user()), 'expired' => $file->isExpired(), + // Said on the one screen that still shows a quarantined or + // missing file, since the library no longer lists it: a + // staff member who followed a link from Quarantine should + // not have to work out why the download refuses. + 'scan_status' => $file->scan_status->value, + 'scan_note' => $file->scan_note, + // Decided here rather than by the page comparing six + // states: whether there are bytes to hand over at all. + // Every button that would produce them is hidden when + // there are not — a download that answers 423 is not an + // affordance, it is a trap. + 'scan_available' => $file->scan_status->isAvailable(), 'download_limit' => $file->download_limit, 'download_limit_scope' => ($file->download_limit_scope ?? DownloadLimitScope::Total)->value, // The file's total downloads, so the editor can see what diff --git a/app/Modules/Files/Http/Controllers/FoldersController.php b/app/Modules/Files/Http/Controllers/FoldersController.php index e43c71a0..918abca6 100644 --- a/app/Modules/Files/Http/Controllers/FoldersController.php +++ b/app/Modules/Files/Http/Controllers/FoldersController.php @@ -108,7 +108,18 @@ class FoldersController extends Controller // something on this install is actually limited. $fileQuery = $this->allowance->withOwnCount( $this->scope->files($user)->with('uploader.role', 'categories', 'folder') - ->withCount(['assignments', 'downloads']), + ->withCount(['assignments', 'downloads']) + // A file the scanner refused, or one whose bytes are gone, + // is not a file anybody can work with: every button on its + // row leads somewhere that refuses, and the download leads + // to an error page. They are listed on the two screens + // that exist to act on them — Quarantine, and Files + // missing from storage — and left out here. + ->whereNotIn('scan_status', [ + ScanStatus::Infected->value, + ScanStatus::UnscannableBlocked->value, + ScanStatus::Missing->value, + ]), $user, ); diff --git a/resources/js/pages/files/edit.tsx b/resources/js/pages/files/edit.tsx index ce3816bc..ec8cea30 100644 --- a/resources/js/pages/files/edit.tsx +++ b/resources/js/pages/files/edit.tsx @@ -1,10 +1,11 @@ import { type BreadcrumbItem } from '@/types'; -import { Head, router, useForm, usePage } from '@inertiajs/react'; -import { Check, Copy, Download, Eye, File as FileIcon, Loader2, X } from 'lucide-react'; +import { Head, Link, router, useForm, usePage } from '@inertiajs/react'; +import { Check, Copy, Download, Eye, File as FileIcon, Loader2, ShieldAlert, X } from 'lucide-react'; import { FormEventHandler, useEffect, useState } from 'react'; import { CommentThread } from '@/components/comments/comment-thread'; import { ConfirmDialog } from '@/components/confirm-dialog'; +import { Alert, AlertDescription, AlertTitle } from '@/components/ui/alert'; import { FilePreviewDialog } from '@/components/file-preview-dialog'; import { FileVersionField, type ChainEntry, type VersionLink } from '@/components/files/file-version-field'; import { InheritedSharingNotice, type SharingRoot } from '@/components/files/inherited-sharing-notice'; @@ -93,6 +94,11 @@ interface FilesEditProps { slug: string; expires_at: string | null; expired: boolean; + /** What the virus scanner made of it — see the notice at the top of this page. */ + scan_status?: string; + scan_note?: string | null; + /** Whether there are bytes to hand over: false hides everything that would ask for them. */ + scan_available?: boolean; download_limit: number | null; download_limit_scope: string | null; downloads_used: number; @@ -304,9 +310,41 @@ export default function FilesEdit({
+ {/* The library no longer lists a quarantined or missing + file, so this is where somebody arrives from Quarantine + or from Files missing from storage — and the page is + full of buttons that will refuse. Say why, once, at the + top. */} + {(file.scan_status === 'infected' || file.scan_status === 'unscannable_blocked') && ( + + + {t('This file is in quarantine')} + + {t('The virus scanner reported: :threat. Nobody can download it, and it is not listed in the library.', { + threat: file.scan_note ?? t('a threat'), + })} + + {t('Quarantine')} + + + + )} + {file.scan_status === 'missing' && ( + + + {t('This file is missing from storage')} + + {t('The record is here and the file itself is not, so nothing can be downloaded. It is not listed in the library.')} + + {t('Files missing from storage')} + + + + )} +
- {isPreviewable(file.mime_type) && ( + {file.scan_available !== false && isPreviewable(file.mime_type) && (
- + {/* Offered only when there are bytes to hand over. + The notice above says why, so a missing button + is not a mystery. */} + {file.scan_available !== false && ( + + )} {can_delete && ( ( -
{file.name}
+ {/* The file itself, for somebody deciding + whether the scanner is right: who it was + shared with, where it came from. It is no + longer listed in the library. */} + + {file.name} +
{file.original_name} · {formatBytes(file.size)}
diff --git a/tests/Feature/Files/VirusScanningTest.php b/tests/Feature/Files/VirusScanningTest.php index 469abda2..25005d50 100644 --- a/tests/Feature/Files/VirusScanningTest.php +++ b/tests/Feature/Files/VirusScanningTest.php @@ -577,3 +577,47 @@ test('a rescan leaves a file that is waiting for its first verdict alone', funct expect($scanner->scans)->toBe(0) ->and($file->refresh()->scan_status)->toBe(ScanStatus::Pending); }); + +test('the library does not list a file nobody can use', function () { + // Every button on such a row leads somewhere that refuses, and the + // download leads to an error page. They live on the two screens that + // exist to act on them. + $clean = scannableFile(['uploaded_by' => $this->admin->id, 'name' => 'Usable', 'scan_status' => ScanStatus::Clean]); + $quarantined = scannableFile(['uploaded_by' => $this->admin->id, 'name' => 'Infectado', 'scan_status' => ScanStatus::Infected, 'scan_note' => 'X']); + $gone = scannableFile(['uploaded_by' => $this->admin->id, 'name' => 'Sin bytes', 'scan_status' => ScanStatus::Missing]); + $waiting = scannableFile(['uploaded_by' => $this->admin->id, 'name' => 'Esperando', 'scan_status' => ScanStatus::Pending]); + + $names = collect($this->actingAs($this->admin)->get('/files')->viewData('page')['props']['files'])->pluck('name'); + + expect($names)->toContain('Usable') + // Still listed: it is about to be usable, and its uploader should + // see where it went. + ->toContain('Esperando') + ->not->toContain('Infectado') + ->not->toContain('Sin bytes'); + + expect([$clean->id, $quarantined->id, $gone->id, $waiting->id])->toHaveCount(4); +}); + +test('the file editor says why a quarantined file refuses everything', function () { + $file = scannableFile(['uploaded_by' => $this->admin->id, 'scan_status' => ScanStatus::Infected, 'scan_note' => 'Eicar-Test-Signature']); + + $this->actingAs($this->admin)->get("/files/{$file->id}")->assertInertia( + fn (Inertia\Testing\AssertableInertia $page) => $page + ->where('file.scan_status', 'infected') + ->where('file.scan_note', 'Eicar-Test-Signature'), + ); +}); + +test('the editor offers no download for a file it cannot produce', function () { + $quarantined = scannableFile(['uploaded_by' => $this->admin->id, 'scan_status' => ScanStatus::Infected, 'scan_note' => 'X']); + $clean = scannableFile(['uploaded_by' => $this->admin->id, 'scan_status' => ScanStatus::Clean]); + + $this->actingAs($this->admin)->get("/files/{$quarantined->id}")->assertInertia( + fn (Inertia\Testing\AssertableInertia $page) => $page->where('file.scan_available', false), + ); + + $this->actingAs($this->admin)->get("/files/{$clean->id}")->assertInertia( + fn (Inertia\Testing\AssertableInertia $page) => $page->where('file.scan_available', true), + ); +});