From 5f7e3089ebe50b98d47d256487fb93252a63a76a Mon Sep 17 00:00:00 2001
From: ignacionelson
Date: Wed, 16 Sep 2026 23:51:33 -0300
Subject: [PATCH] Stop offering a file nobody can have
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
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.
---
.../Http/Controllers/FilesController.php | 12 ++++
.../Http/Controllers/FoldersController.php | 13 +++-
resources/js/pages/files/edit.tsx | 61 +++++++++++++++---
resources/js/pages/files/orphans.tsx | Bin 19940 -> 20080 bytes
resources/js/pages/files/quarantine.tsx | 8 ++-
tests/Feature/Files/VirusScanningTest.php | 44 +++++++++++++
6 files changed, 127 insertions(+), 11 deletions(-)
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')}
+
+
+
+ )}
+
-
+ {/* 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),
+ );
+});