Stop offering a file nobody can have

A quarantined file was listed in the library with every button a working
file has, and Download answered with an error page. Three changes, all
the same idea: do not offer what cannot be done.

The library no longer lists a file that is quarantined or missing from
storage. Those two live on the screens that exist to act on them —
Quarantine, and Files missing from storage — and both now link each row
to the file itself, which is where somebody deciding needs to look.

That page says why, at the top, in the colour the state deserves: red
for a threat, amber for bytes that are gone. And it stops offering the
download and the preview, because a button that answers 423 is not an
affordance.

A file still being checked stays in the library. It is about to be
usable, and its uploader should be able to see where it went.

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

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,
         );
 

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({
             <Head title={file.name} />
 
             <div className="px-4 py-6">
+                {/* 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') && (
+                    <Alert variant="destructive" className="mb-4">
+                        <ShieldAlert className="size-4" />
+                        <AlertTitle>{t('This file is in quarantine')}</AlertTitle>
+                        <AlertDescription>
+                            {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'),
+                            })}
+                            <Link href="/files/quarantine" className="mt-1 inline-block underline hover:no-underline">
+                                {t('Quarantine')}
+                            </Link>
+                        </AlertDescription>
+                    </Alert>
+                )}
+                {file.scan_status === 'missing' && (
+                    <Alert variant="warning" className="mb-4">
+                        <ShieldAlert className="size-4" />
+                        <AlertTitle>{t('This file is missing from storage')}</AlertTitle>
+                        <AlertDescription>
+                            {t('The record is here and the file itself is not, so nothing can be downloaded. It is not listed in the library.')}
+                            <Link href="/files/orphans?tab=missing" className="mt-1 inline-block underline hover:no-underline">
+                                {t('Files missing from storage')}
+                            </Link>
+                        </AlertDescription>
+                    </Alert>
+                )}
+
                 <div className="flex items-start justify-between">
                     <div className="flex items-start gap-4">
-                        {isPreviewable(file.mime_type) && (
+                        {file.scan_available !== false && isPreviewable(file.mime_type) && (
                             <FilePreviewDialog
                                 previewUrl={route('files.preview', file.id)}
                                 mimeType={file.mime_type}
@@ -335,12 +373,17 @@ export default function FilesEdit({
                         />
                     </div>
                     <div className="flex gap-2">
-                        <Button variant="outline" asChild>
-                            <a href={route('files.download', file.id)}>
-                                <Download className="size-4" />
-                                {t('Download')}
-                            </a>
-                        </Button>
+                        {/* 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 && (
+                            <Button variant="outline" asChild>
+                                <a href={route('files.download', file.id)}>
+                                    <Download className="size-4" />
+                                    {t('Download')}
+                                </a>
+                            </Button>
+                        )}
                         {can_delete && (
                             <ConfirmDialog
                                 trigger={

resources/js/pages/files/quarantine.tsx

@@ -148,7 +148,13 @@ export default function Quarantine({ files, pagination }: QuarantineProps) {
                     {files.map((file) => (
                         <tr key={file.id} className="border-b last:border-0">
                             <td className="px-4 py-2.5">
-                                <div className="font-medium">{file.name}</div>
+                                {/* 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. */}
+                                <Link href={route('files.edit', file.id)} className="font-medium hover:underline">
+                                    {file.name}
+                                </Link>
                                 <div className="text-muted-foreground text-xs">
                                     {file.original_name} · {formatBytes(file.size)}
                                 </div>

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),
+    );
+});