From bab90c0ad8913a3590fa1e3806482bc2d2342ceb Mon Sep 17 00:00:00 2001 From: ignacionelson Date: Wed, 16 Sep 2026 14:23:56 -0300 Subject: [PATCH 01/20] Scan uploaded files for viruses, and withhold them until they are checked MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every upload now starts as "being checked" and is not served to anyone until a scanner has looked at it. Infected files are quarantined: kept on disk, unreachable, waiting for an administrator. The scanner is ClamAV, reached over a socket, streaming the file wherever it is stored — no temporary copy for an S3 or GCS disk. What the scanner answers is a fact; what it means for the file is this installation's setting, so ClamAvScanner knows nothing about settings and ScanPolicy knows nothing about sockets. Three of clamd's own alert options are what make a file it could not open come back as an answer rather than as "OK"; the client maps those to "too large" and "encrypted" instead of to a threat. Both policies default to letting files through, marked "not scanned", which is the product owner's decision: a scanner that cannot answer must not stop people working. Every such file is logged, and the screens that say so come with the rest of this work. Withholding is two rules. A file that is not available drops out of the scopes that answer "what may this person see" — recipients and the public listings, never the uploader's own copy. And every route that puts bytes on the wire asks FileAvailability first: download, thumbnail, preview, share link, the four public routes and both ends of a zip build. A share link minted before the scan finishes says the file is still being checked rather than 404ing. Not yet here, and coming next: the quarantine screen and its permission, the notifications, the settings screen, the hourly retry, the backfill for existing libraries, and the Docker service. --- app/Modules/Audit/Action.php | 16 + .../Files/Events/FileBecameAvailable.php | 25 ++ app/Modules/Files/FilesServiceProvider.php | 22 ++ .../Controllers/FileDownloadController.php | 7 + .../Controllers/FileThumbnailController.php | 9 + .../Controllers/PublicShareController.php | 19 ++ .../Controllers/ZipDownloadsController.php | 8 +- .../Files/Jobs/BuildZipDownloadJob.php | 11 +- app/Modules/Files/Jobs/ScanFileJob.php | 159 +++++++++ app/Modules/Files/Models/File.php | 47 ++- app/Modules/Files/Scanning/ClamAvScanner.php | 233 +++++++++++++ .../Files/Scanning/FileAvailability.php | 83 +++++ .../Files/Scanning/NotScannedReason.php | 37 ++ app/Modules/Files/Scanning/ScanOutcome.php | 14 + app/Modules/Files/Scanning/ScanPolicy.php | 169 +++++++++ app/Modules/Files/Scanning/ScanStatus.php | 80 +++++ app/Modules/Files/Scanning/ScanVerdict.php | 51 +++ app/Modules/Files/Scanning/ScannerStatus.php | 47 +++ app/Modules/Files/Scanning/ScanningConfig.php | 109 ++++++ app/Modules/Files/Scanning/VirusScanner.php | 35 ++ .../Files/Uploads/StoreUploadedFile.php | 13 + .../Controllers/PublicGroupsController.php | 6 + app/Modules/Platform/Settings/Setting.php | 51 +++ config/projectsend.php | 28 ++ ...090000_add_scan_columns_to_files_table.php | 49 +++ docs/api/openapi.json | 3 + resources/js/pages/share/show.tsx | 18 +- tests/Feature/Files/VirusScanningTest.php | 321 ++++++++++++++++++ tests/Support/FakeVirusScanner.php | 79 +++++ 29 files changed, 1738 insertions(+), 11 deletions(-) create mode 100644 app/Modules/Files/Events/FileBecameAvailable.php create mode 100644 app/Modules/Files/Jobs/ScanFileJob.php create mode 100644 app/Modules/Files/Scanning/ClamAvScanner.php create mode 100644 app/Modules/Files/Scanning/FileAvailability.php create mode 100644 app/Modules/Files/Scanning/NotScannedReason.php create mode 100644 app/Modules/Files/Scanning/ScanOutcome.php create mode 100644 app/Modules/Files/Scanning/ScanPolicy.php create mode 100644 app/Modules/Files/Scanning/ScanStatus.php create mode 100644 app/Modules/Files/Scanning/ScanVerdict.php create mode 100644 app/Modules/Files/Scanning/ScannerStatus.php create mode 100644 app/Modules/Files/Scanning/ScanningConfig.php create mode 100644 app/Modules/Files/Scanning/VirusScanner.php create mode 100644 database/migrations/2026_09_16_090000_add_scan_columns_to_files_table.php create mode 100644 tests/Feature/Files/VirusScanningTest.php create mode 100644 tests/Support/FakeVirusScanner.php diff --git a/app/Modules/Audit/Action.php b/app/Modules/Audit/Action.php index 530dc2ac..e1d6453b 100644 --- a/app/Modules/Audit/Action.php +++ b/app/Modules/Audit/Action.php @@ -71,6 +71,16 @@ enum Action: string case FolderMadePrivate = 'folder.made_private'; case UploadAborted = 'upload.aborted'; case FileImported = 'file.imported'; + + // Virus scanning. A clean result is not logged: it is the ordinary + // outcome of every upload, and a row per upload would bury the ones + // that matter. Only the three that need answering are. + case FileQuarantined = 'file.quarantined'; + case FileReleased = 'file.released'; + // Allowed through without being checked — because the scanner could + // not be reached, or the file was too large or encrypted and this + // installation allows those. The context says which. + case FileNotScanned = 'file.not_scanned'; case OrphanFileDeleted = 'orphan_file.deleted'; case OrphanFileAutoDeleted = 'orphan_file.auto_deleted'; case ExpiredFileDeleted = 'file.expired_deleted'; @@ -207,6 +217,9 @@ enum Action: string self::CommentDeleted => 'Deleted a comment on the file ":subject"', self::CommentApproved => 'Approved a comment on the file ":subject"', self::FileImported => 'Imported the orphan file ":subject"', + self::FileQuarantined => 'The file ":subject" was quarantined: :threat', + self::FileReleased => 'Released the quarantined file ":subject" (:reason)', + self::FileNotScanned => 'The file ":subject" was not scanned for viruses: :reason', self::OrphanFileDeleted => 'Deleted the orphan file ":name"', self::OrphanFileAutoDeleted => 'Deleted the orphan file ":name"', self::ExpiredFileDeleted => 'Deleted the expired file ":name"', @@ -310,6 +323,9 @@ enum Action: string self::CommentDeleted => 'A comment was deleted', self::CommentApproved => 'A comment was approved', self::FileImported => 'An orphan file was imported', + self::FileQuarantined => 'A file was quarantined by the virus scanner', + self::FileReleased => 'A quarantined file was released by an administrator', + self::FileNotScanned => 'A file was allowed through without being scanned', self::OrphanFileDeleted => 'An orphan file was deleted', self::OrphanFileAutoDeleted => 'An orphan file was automatically deleted after its retention grace period passed', self::ExpiredFileDeleted => 'An expired file was automatically deleted after its retention grace period passed', diff --git a/app/Modules/Files/Events/FileBecameAvailable.php b/app/Modules/Files/Events/FileBecameAvailable.php new file mode 100644 index 00000000..1c2e0b35 --- /dev/null +++ b/app/Modules/Files/Events/FileBecameAvailable.php @@ -0,0 +1,25 @@ +app->scoped(ClientIdentityScope::class); + + // One implementation ships, and the interface exists so the test + // suite can state a verdict instead of producing a file that + // provokes one — and so a commercial engine can be added later + // without touching the job or the policy. + $this->app->bind(VirusScanner::class, ClamAvScanner::class); } public function boot(): void @@ -97,6 +108,17 @@ class FilesServiceProvider extends ServiceProvider url: fn (array $data): string => route('my-files.index'), )); + // Every upload path converges on FileWasStored, so this is the + // one place a scan is started from. Dispatched rather than run + // inline: a 5 GB file takes minutes to read, and an upload must + // not wait for it — the file is already withheld until the + // verdict arrives. + Event::listen(FileWasStored::class, function (FileWasStored $event): void { + if ($event->file->scan_status === ScanStatus::Pending) { + ScanFileJob::dispatch($event->file->id); + } + }); + if ($this->app->runningInConsole()) { $this->commands([ Console\PurgeStaleUploadsCommand::class, diff --git a/app/Modules/Files/Http/Controllers/FileDownloadController.php b/app/Modules/Files/Http/Controllers/FileDownloadController.php index 1bae3a9e..6c57d982 100644 --- a/app/Modules/Files/Http/Controllers/FileDownloadController.php +++ b/app/Modules/Files/Http/Controllers/FileDownloadController.php @@ -10,6 +10,7 @@ use App\Modules\Audit\ActivityLogger; use App\Modules\Files\Access\DownloadAllowance; use App\Modules\Files\Delivery\StoredFileResponse; use App\Modules\Files\Models\File; +use App\Modules\Files\Scanning\FileAvailability; use Illuminate\Http\RedirectResponse; use Illuminate\Http\Request; use Illuminate\Support\Facades\Gate; @@ -29,12 +30,18 @@ class FileDownloadController extends Controller private readonly ActivityLogger $activity, private readonly DownloadAllowance $allowance, private readonly StoredFileResponse $bytes, + private readonly FileAvailability $availability, ) {} public function __invoke(Request $request, File $file): Response|RedirectResponse { Gate::authorize('view', $file); + // Before the download limit and before the log: a file the scanner + // has not cleared is not served to anybody, and a refusal here is + // not a download to count. + $this->availability->guardDelivery($file); + // Separate from the policy on purpose: a spent download limit is // not "you may not see this file" — the file stays listed, and // the same person may still open its details. It is only the diff --git a/app/Modules/Files/Http/Controllers/FileThumbnailController.php b/app/Modules/Files/Http/Controllers/FileThumbnailController.php index 2467db5d..9bd51a0d 100644 --- a/app/Modules/Files/Http/Controllers/FileThumbnailController.php +++ b/app/Modules/Files/Http/Controllers/FileThumbnailController.php @@ -10,6 +10,7 @@ use App\Modules\Files\Access\DownloadAllowance; use App\Modules\Files\Delivery\FileDelivery; use App\Modules\Files\Delivery\StoredFileResponse; use App\Modules\Files\Models\File; +use App\Modules\Files\Scanning\FileAvailability; use App\Modules\Files\Preview\PreviewKind; use App\Modules\Files\Preview\PreviewLog; use App\Modules\Files\Thumbnails\Events\ResolvingImageRendering; @@ -78,12 +79,18 @@ class FileThumbnailController extends Controller private readonly LocalSourceFile $source, private readonly Settings $settings, private readonly FileDelivery $delivery, + private readonly FileAvailability $availability, ) {} public function thumbnail(Request $request, File $file): Response { Gate::authorize('view', $file); + // A rendition is made by an image library reading the file, which + // is itself a way in — so an unchecked file is not rendered, not + // even as 300 pixels. + $this->availability->guardDelivery($file); + // This one route serves both the staff file manager and the client // portal — the same URL, told apart only by who is asking. A client // and a staff member looking at the same file get different cached @@ -119,6 +126,8 @@ class FileThumbnailController extends Controller { Gate::authorize('view', $file); + $this->availability->guardDelivery($file); + // The inline allowlist. See the class docblock and PreviewKind — // the stored mime type is sniffed from the bytes, so an allowed // extension is not evidence of a safe-to-render payload. diff --git a/app/Modules/Files/Http/Controllers/PublicShareController.php b/app/Modules/Files/Http/Controllers/PublicShareController.php index 16236f8f..4f41a96e 100644 --- a/app/Modules/Files/Http/Controllers/PublicShareController.php +++ b/app/Modules/Files/Http/Controllers/PublicShareController.php @@ -11,6 +11,8 @@ use App\Modules\Files\Access\DownloadAllowance; use App\Modules\Files\Delivery\StoredFileResponse; use App\Modules\Files\Models\Category; use App\Modules\Files\Models\File; +use App\Modules\Files\Scanning\FileAvailability; +use App\Modules\Files\Scanning\ScanStatus; use App\Modules\Files\Models\ShareLink; use Illuminate\Http\RedirectResponse; use Inertia\Inertia; @@ -29,6 +31,7 @@ class PublicShareController extends Controller private readonly ActivityLogger $activity, private readonly DownloadAllowance $allowance, private readonly StoredFileResponse $bytes, + private readonly FileAvailability $availability, ) {} public function show(string $token): InertiaResponse @@ -47,6 +50,16 @@ class PublicShareController extends Controller return Inertia::render('share/show', ['status' => 'expired']); } + // A link can be minted the moment a file is stored — the hosted + // free plan does exactly that — so the link routinely exists + // before the scanner has finished. It says so rather than 404ing: + // the visitor was sent a real link and it will work shortly. + if (! $this->availability->isAvailable($file)) { + return Inertia::render('share/show', [ + 'status' => $file->scan_status === ScanStatus::Pending ? 'checking' : 'unavailable', + ]); + } + // Two separate caps reach the same page: the link's own // max_downloads, and the file's. A visitor here has no account, // so the file's limit is measured against the whole file — see @@ -83,6 +96,12 @@ class PublicShareController extends Controller return redirect()->route('share.show', $token); } + // Same for a file still being checked, and for the same reason + // the limit is asked before the counter moves. + if (! $this->availability->isAvailable($file)) { + return redirect()->route('share.show', $token); + } + // Before the link's counter moves, not after: a download refused // by the file's own limit must not spend one of the link's. if (! $this->allowance->allows($file, null)) { diff --git a/app/Modules/Files/Http/Controllers/ZipDownloadsController.php b/app/Modules/Files/Http/Controllers/ZipDownloadsController.php index 191d2572..f6c619b3 100644 --- a/app/Modules/Files/Http/Controllers/ZipDownloadsController.php +++ b/app/Modules/Files/Http/Controllers/ZipDownloadsController.php @@ -13,6 +13,7 @@ use App\Modules\Files\Access\DownloadAllowance; use App\Modules\Files\Access\ViewableFileScope; use App\Modules\Files\Jobs\BuildZipDownloadJob; use App\Modules\Files\Models\File; +use App\Modules\Files\Scanning\FileAvailability; use App\Modules\Files\Models\Folder; use App\Modules\Files\Models\ZipDownload; use App\Modules\Files\Uploads\StoreUploadedFile; @@ -45,6 +46,7 @@ class ZipDownloadsController extends Controller private readonly ActivityLogger $activity, private readonly ViewableFileScope $viewable, private readonly DownloadAllowance $allowance, + private readonly FileAvailability $availability, private readonly Settings $settings, private readonly FileDelivery $delivery, ) {} @@ -102,7 +104,11 @@ class ZipDownloadsController extends Controller // as many times as they were meant to is the whole point of not // hiding exhausted files. $selected = $files->count(); - $files = $files->filter(fn (File $file): bool => $this->allowance->allows($file, $user)); + // A file still being checked, or quarantined, is left out of the + // selection the same way a spent allowance leaves one out: the zip + // is bytes leaving the server, and nothing unchecked goes into one. + $files = $files->filter(fn (File $file): bool => $this->availability->isAvailable($file) + && $this->allowance->allows($file, $user)); abort_if( $files->isEmpty() && $folders->isEmpty() && $selected > 0, diff --git a/app/Modules/Files/Jobs/BuildZipDownloadJob.php b/app/Modules/Files/Jobs/BuildZipDownloadJob.php index 049ddd8a..90d7f100 100644 --- a/app/Modules/Files/Jobs/BuildZipDownloadJob.php +++ b/app/Modules/Files/Jobs/BuildZipDownloadJob.php @@ -8,6 +8,7 @@ use App\Models\User; use App\Modules\Files\Access\DownloadAllowance; use App\Modules\Files\Access\ViewableFileScope; use App\Modules\Files\Models\File; +use App\Modules\Files\Scanning\FileAvailability; use App\Modules\Files\Models\Folder; use App\Modules\Files\Models\ZipDownload; use App\Modules\Platform\Settings\Setting; @@ -114,6 +115,7 @@ class BuildZipDownloadJob implements ShouldQueue $visible = app(ViewableFileScope::class)->for($requester); $allowance = app(DownloadAllowance::class); + $availability = app(FileAvailability::class); try { $relativePath = 'zips/'.$zipDownload->id.'.zip'; @@ -148,7 +150,11 @@ class BuildZipDownloadJob implements ShouldQueue // Re-checked here for the same reason visibility is: the // archive is built some time after it was asked for, and // the allowance may have been spent in between. - if (! $allowance->allows($file, $requester)) { + // Availability is re-checked here for a sharper reason + // than the allowance is: a file can be quarantined between + // the request and the build, and an archive is exactly how + // an infected file would leave anyway. + if (! $availability->isAvailable($file) || ! $allowance->allows($file, $requester)) { $skipped[] = ['id' => $file->id, 'name' => $file->name]; continue; @@ -382,6 +388,7 @@ class BuildZipDownloadJob implements ShouldQueue private function addFolder(ZipArchive $zip, Folder $folder, User $requester, array &$usedNames, array &$tempFiles, Builder $visible, array &$skipped, array &$added): int { $allowance = app(DownloadAllowance::class); + $availability = app(FileAvailability::class); $subtreeIds = $folder->subtreeFolderIds(); /** @var Collection $foldersById */ @@ -403,7 +410,7 @@ class BuildZipDownloadJob implements ShouldQueue // inside it whose own allowance is spent — same reason the // per-file visibility filter is re-derived rather than // inherited from the folder. - if (! $allowance->allows($file, $requester)) { + if (! $availability->isAvailable($file) || ! $allowance->allows($file, $requester)) { $skipped[] = ['id' => $file->id, 'name' => $file->name]; continue; diff --git a/app/Modules/Files/Jobs/ScanFileJob.php b/app/Modules/Files/Jobs/ScanFileJob.php new file mode 100644 index 00000000..cbff32e4 --- /dev/null +++ b/app/Modules/Files/Jobs/ScanFileJob.php @@ -0,0 +1,159 @@ +onQueue('scans'); + } + + /** + * A day. Long enough that an overnight outage is survived by a + * "hold" installation, short enough that a job for a file somebody + * deleted does not live forever. + */ + public function retryUntil(): \DateTimeInterface + { + return now()->addDay(); + } + + public function handle( + VirusScanner $scanner, + ScanPolicy $policy, + ScanningConfig $config, + ): void { + $file = File::query()->find($this->fileId); + + // Deleted while it waited, or already decided by an earlier run + // (this job is dispatched from an upload and from the hourly + // sweep, and both can land on the same file). + if ($file === null || $file->scan_status !== ScanStatus::Pending) { + return; + } + + if (! $config->enabled()) { + $policy->markNeverScanned($file); + + return; + } + + // A file identical to one already quarantined needs no second + // opinion, and asking for one would send the same malware past + // the scanner again. Checksums are already computed at upload. + $known = File::query() + ->where('checksum', $file->checksum) + ->where('scan_status', ScanStatus::Infected) + ->whereKeyNot($file->id) + ->first(); + + if ($known !== null) { + $policy->record($file, ScanVerdict::infected((string) $known->scan_note)); + + return; + } + + $verdict = $this->read($file, $scanner); + + if ($verdict->outcome === ScanOutcome::Unavailable && $this->keepWaiting($file, $config)) { + $file->forceFill(['scan_attempts' => $file->scan_attempts + 1])->save(); + + // 30 seconds, then a minute, then two, up to five. Long + // enough not to hammer a scanner that is starting up; short + // enough that a brief blip does not hold an upload for the + // whole patience window. + $this->release(min(300, 30 * (2 ** min(4, $file->scan_attempts)))); + + return; + } + + $policy->record($file, $verdict); + } + + /** + * Whether the file should wait rather than be decided now. + * + * "Hold" waits forever, by design. Otherwise the wait is measured + * from when the file was stored, not from this attempt: what the + * setting promises is that nobody's upload sits unavailable for + * longer than that, however many times the job has run. + */ + private function keepWaiting(File $file, ScanningConfig $config): bool + { + if ($config->holdsWhileUnavailable()) { + return true; + } + + $storedAt = $file->created_at ?? now(); + + return $storedAt->copy()->addMinutes($config->unavailableWaitMinutes())->isFuture(); + } + + private function read(File $file, VirusScanner $scanner): ScanVerdict + { + try { + $stream = Storage::disk($file->disk)->readStream($file->path); + } catch (Throwable $e) { + $stream = null; + Log::warning("Could not open file {$file->id} for scanning: ".$e->getMessage()); + } + + if ($stream === null) { + // Not the scanner's fault and not a verdict about the file: + // treated as "could not be checked", so the installation's + // own policy decides, rather than calling a file nobody read + // clean. + return ScanVerdict::unavailable(__('The file could not be read from storage.')); + } + + try { + return $scanner->scan($stream, $file->size); + } finally { + fclose($stream); + } + } +} diff --git a/app/Modules/Files/Models/File.php b/app/Modules/Files/Models/File.php index f080461e..878b0abf 100644 --- a/app/Modules/Files/Models/File.php +++ b/app/Modules/Files/Models/File.php @@ -10,6 +10,7 @@ use App\Modules\Audit\ActivityLog; use App\Modules\Files\Access\SharingIdentity; use App\Modules\Files\DownloadLimitScope; use App\Modules\Files\FileDiskCleanup; +use App\Modules\Files\Scanning\ScanStatus; use App\Modules\Files\Versions\FileVersions; use App\Modules\Groups\Models\Group; use App\Support\Concerns\HasUniqueSlug; @@ -40,6 +41,13 @@ use Illuminate\Support\Carbon; * @property string $mime_type * @property int $size * @property string $checksum + * @property ScanStatus $scan_status + * @property string|null $scan_note the threat name, or a NotScannedReason + * @property Carbon|null $scanned_at + * @property string|null $scan_engine + * @property int $scan_attempts + * @property int|null $released_by + * @property Carbon|null $released_at * @property bool $public * @property Carbon|null $expires_at * @property int|null $download_limit @@ -74,6 +82,13 @@ class File extends Model { return [ 'public' => 'boolean', + // Where this file stands with the virus scanner. Cast to the + // enum so nothing compares raw strings — see ScanStatus and + // FileAvailability. + 'scan_status' => ScanStatus::class, + 'scanned_at' => 'datetime', + 'released_at' => 'datetime', + 'scan_attempts' => 'integer', 'commentable' => 'boolean', 'expires_at' => 'datetime', 'download_limit' => 'integer', @@ -271,6 +286,29 @@ class File extends Model return $this->expires_at !== null && $this->expires_at->isPast(); } + /** + * Files the virus scanner has finished with, one way or another. + * + * Sits beside notExpired() in every scope that answers "what may this + * person be shown", and for the same reason: a file nobody has + * checked yet is not a file anybody may be handed. The uploader is + * the exception — their own upload stays on their screen while it is + * being checked, marked as such, because a file that vanishes for ten + * minutes after you send it reads as a failed upload. + * + * @param Builder $query + */ + public function scopeAvailable(Builder $query, ?User $viewer = null): void + { + $query->where(function (Builder $inner) use ($viewer): void { + $inner->whereIn('scan_status', ScanStatus::availableValues()); + + if ($viewer !== null) { + $inner->orWhere('uploaded_by', $viewer->id); + } + }); + } + /** * @param Builder $query */ @@ -352,7 +390,7 @@ class File extends Model $outer->orWhere('uploaded_by', $client->id); }); - $query->notExpired(); + $query->notExpired()->available($client); } /** @@ -393,7 +431,7 @@ class File extends Model $outer->orWhereIn('folder_id', $subtreeFolderIds); }); - $query->notExpired(); + $query->notExpired()->available(); } /** @@ -407,7 +445,7 @@ class File extends Model */ public function scopePubliclyVisibleForFolder(Builder $query, Folder $folder): void { - $query->whereIn('folder_id', $folder->subtreeFolderIds())->notExpired(); + $query->whereIn('folder_id', $folder->subtreeFolderIds())->notExpired()->available(); } /** @@ -458,6 +496,7 @@ class File extends Model ->where(function (Builder $folder) use ($publicFolderSubtreeIds): void { $folder->whereNull('folder_id')->orWhereNotIn('folder_id', $publicFolderSubtreeIds); }) - ->notExpired(); + ->notExpired() + ->available(); } } diff --git a/app/Modules/Files/Scanning/ClamAvScanner.php b/app/Modules/Files/Scanning/ClamAvScanner.php new file mode 100644 index 00000000..aa3bb7a0 --- /dev/null +++ b/app/Modules/Files/Scanning/ClamAvScanner.php @@ -0,0 +1,233 @@ +\0`, and + * INSTREAM is that followed by length-prefixed chunks and a zero-length + * chunk to finish. Taking a library for this would be more dependency + * than code. + * + * **Three of clamd's own settings decide whether this class can tell the + * truth**, and without them a file it could not open comes back as `OK`: + * `AlertExceedsMax`, `AlertEncrypted` and its two companions turn those + * cases into answers, which arrive here as `Heuristics.Limits.Exceeded.*` + * and `Heuristics.Encrypted.*` and are mapped below to tooLarge and + * encrypted rather than to a threat. An encrypted archive full of malware + * reported as clean is the failure this exists to prevent, so the + * shipped Docker configuration sets all of them and the documentation + * says so for manual installs. + * + * Nothing here throws for a scanner that is down, slow or misconfigured: + * the caller has a policy for that, and an exception would read as a bug + * in the job rather than as the state of somebody's server. + */ +class ClamAvScanner implements VirusScanner +{ + /** 64 KiB — clamd's own read buffer size, and small enough to stream 5 GB without holding it. */ + private const CHUNK = 65536; + + public function __construct( + private readonly ScanningConfig $config, + ) {} + + public function scan(mixed $stream, int $size): ScanVerdict + { + $max = $this->config->maxScanBytes(); + + // Asked before opening a socket: a file this installation has + // decided not to scan should not spend a connection, and clamd + // would refuse it anyway once it passed StreamMaxLength. + if ($max > 0 && $size > $max) { + return ScanVerdict::tooLarge(); + } + + $socket = $this->connect(); + + if ($socket === null) { + return ScanVerdict::unavailable(__('The scanner could not be reached at :address.', [ + 'address' => $this->config->address(), + ])); + } + + try { + fwrite($socket, "zINSTREAM\0"); + + while (! feof($stream)) { + $chunk = fread($stream, self::CHUNK); + + if ($chunk === false) { + return ScanVerdict::unavailable(__('The file could not be read for scanning.')); + } + + if ($chunk === '') { + continue; + } + + // Big-endian length, then the bytes. A short write here + // means clamd hung up mid-stream — usually its own size + // limit — and the reply below says which. + if (fwrite($socket, pack('N', strlen($chunk)).$chunk) === false) { + break; + } + } + + fwrite($socket, pack('N', 0)); + + $reply = $this->readReply($socket); + } catch (Throwable $e) { + return ScanVerdict::unavailable($e->getMessage()); + } finally { + fclose($socket); + } + + if ($reply === null) { + return ScanVerdict::unavailable(__('The scanner did not answer in time.')); + } + + return $this->verdictFor($reply); + } + + public function status(): ScannerStatus + { + $socket = $this->connect(); + + if ($socket === null) { + return ScannerStatus::unreachable(__('No answer from :address.', ['address' => $this->config->address()])); + } + + try { + fwrite($socket, "zVERSION\0"); + $reply = $this->readReply($socket); + } catch (Throwable $e) { + return ScannerStatus::unreachable($e->getMessage()); + } finally { + fclose($socket); + } + + if ($reply === null || $reply === '') { + return ScannerStatus::unreachable(__('The scanner did not answer in time.')); + } + + // "ClamAV 1.4.1/27412/Mon Sep 15 09:12:03 2026" — engine, + // signature database number, and when that database was built. + // Older builds answer with the engine alone, so every part after + // the first is optional rather than assumed. + $parts = explode('/', $reply); + $definitions = isset($parts[1]) && is_numeric(trim($parts[1])) ? (int) trim($parts[1]) : null; + $built = null; + + if (isset($parts[2])) { + try { + $built = Carbon::parse(trim($parts[2])); + } catch (Throwable) { + $built = null; + } + } + + return new ScannerStatus(true, trim($parts[0]), $definitions, $built); + } + + private function verdictFor(string $reply): ScanVerdict + { + $engine = null; + + if (str_ends_with($reply, 'OK')) { + return ScanVerdict::clean($engine); + } + + // "stream: Win.Test.EICAR_HDB-1 FOUND" + if (str_ends_with($reply, 'FOUND')) { + $threat = trim(str_replace(['stream:', 'FOUND'], '', $reply)); + + // Not threats: clamd's way of saying "I could not look + // inside". Which one it is decides the file's fate, and both + // are the installation's policy rather than a detection. + if (str_contains($threat, 'Heuristics.Encrypted')) { + return ScanVerdict::encrypted($engine); + } + + if (str_contains($threat, 'Heuristics.Limits.Exceeded')) { + return ScanVerdict::tooLarge($engine); + } + + return ScanVerdict::infected($threat === '' ? 'unknown' : $threat, $engine); + } + + // "INSTREAM size limit exceeded. ERROR" — the stream was longer + // than clamd's StreamMaxLength. Same meaning as the heuristic + // above, reached when this installation's own maximum is the + // larger of the two. + if (str_contains($reply, 'size limit exceeded')) { + return ScanVerdict::tooLarge($engine); + } + + return ScanVerdict::unavailable($reply); + } + + /** @return resource|null */ + private function connect(): mixed + { + $address = $this->config->address(); + + if ($address === '') { + return null; + } + + $socket = @stream_socket_client( + $address, + $code, + $message, + $this->config->connectTimeoutSeconds(), + STREAM_CLIENT_CONNECT, + ); + + if ($socket === false) { + return null; + } + + // Without this a scanner that accepts the connection and then + // stops answering holds the worker open indefinitely. + stream_set_timeout($socket, $this->config->replyTimeoutSeconds()); + + return $socket; + } + + /** + * clamd's replies end with a NUL in `z` mode. Returns null when the + * socket timed out rather than answered. + * + * @param resource $socket + */ + private function readReply(mixed $socket): ?string + { + $reply = ''; + + while (! feof($socket)) { + $byte = fread($socket, 1); + + if ($byte === false || $byte === '') { + break; + } + + if ($byte === "\0") { + break; + } + + $reply .= $byte; + } + + if (stream_get_meta_data($socket)['timed_out']) { + return null; + } + + return trim($reply); + } +} diff --git a/app/Modules/Files/Scanning/FileAvailability.php b/app/Modules/Files/Scanning/FileAvailability.php new file mode 100644 index 00000000..8a58637a --- /dev/null +++ b/app/Modules/Files/Scanning/FileAvailability.php @@ -0,0 +1,83 @@ +scan_status->isAvailable(); + } + + /** + * Refuse to serve a file's bytes unless it is available. + * + * Called by every route that puts bytes on the wire — the download, + * the thumbnail, the preview, the share link, the public listing and + * the zip builder. Not by the listings: a staff member's library shows + * a pending file with its state on it, and the uploader sees their own. + * What this governs is the bytes. + * + * It refuses everybody, including staff and the file's own uploader. + * A file the scanner has not cleared is not one this application + * hands out, and an administrator who wants it anyway has a way to say + * so on the record: release it from quarantine. + * + * 423 rather than 403: the refusal is about the file's state and it is + * temporary in the pending case, which is exactly what "Locked" means + * and what "Forbidden" does not. ProblemDetails renders it as JSON for + * the API, which shares these controllers. + */ + public function guardDelivery(File $file): void + { + if ($this->isAvailable($file)) { + return; + } + + abort(423, $file->scan_status === ScanStatus::Pending + ? __('This file is still being checked for viruses.') + : __('This file is not available.')); + } + + /** + * A file has finished being checked, one way or another. + * + * Three roads lead here and they are not interchangeable: the scan + * passed, the scanner could not be reached and this installation lets + * files through, or an administrator released it from quarantine. What + * they share is the only thing this announces — the file can now be + * had by the people it was shared with, which is when everything that + * was waiting on it (a share email, a new-version notice) is allowed + * to go out. + */ + public function markAvailable(File $file): void + { + if (! $this->isAvailable($file)) { + return; + } + + Event::dispatch(new FileBecameAvailable($file)); + } +} diff --git a/app/Modules/Files/Scanning/NotScannedReason.php b/app/Modules/Files/Scanning/NotScannedReason.php new file mode 100644 index 00000000..cc694466 --- /dev/null +++ b/app/Modules/Files/Scanning/NotScannedReason.php @@ -0,0 +1,37 @@ + 'Too large to scan', + self::Encrypted => 'Encrypted, so it could not be scanned', + self::ScannerUnavailable => 'The scanner could not be reached', + self::BeforeScanning => 'Uploaded before virus scanning was switched on', + }; + } +} diff --git a/app/Modules/Files/Scanning/ScanOutcome.php b/app/Modules/Files/Scanning/ScanOutcome.php new file mode 100644 index 00000000..68973fef --- /dev/null +++ b/app/Modules/Files/Scanning/ScanOutcome.php @@ -0,0 +1,14 @@ +outcome) { + ScanOutcome::Clean => $this->settle($file, ScanStatus::Clean, null, $verdict->engine), + ScanOutcome::Infected => $this->quarantine($file, $verdict->detail ?? 'unknown', $verdict->engine), + ScanOutcome::TooLarge => $this->unscannable($file, NotScannedReason::TooLarge, $verdict->engine), + ScanOutcome::Encrypted => $this->unscannable($file, NotScannedReason::Encrypted, $verdict->engine), + ScanOutcome::Unavailable => $this->unavailable($file, $verdict->detail), + }; + } + + /** + * The file existed before there was a scanner, or scanning is off. + * Not a verdict, so it is never logged: nothing happened to this + * file, it simply was never looked at. + */ + public function markNeverScanned(File $file): void + { + $file->forceFill([ + 'scan_status' => ScanStatus::NotScanned, + 'scan_note' => NotScannedReason::BeforeScanning->value, + ])->save(); + } + + /** + * A threat was found. The bytes stay — a scanner can be wrong, and an + * administrator may release it — but nothing may reach them, and the + * thumbnails already rendered from this file have to go: they are + * derived from the same bytes and are served by their own routes. + */ + private function quarantine(File $file, string $threat, ?string $engine): ScanStatus + { + $wasAvailable = $this->availability->isAvailable($file); + + $this->settle($file, ScanStatus::Infected, $threat, $engine); + $this->purgeRenditions($file); + + $this->activity->logSystem(Action::FileQuarantined, [ + 'id' => $file->id, + 'name' => $file->name, + 'threat' => $threat, + // Said out loud because it changes what an administrator has + // to do: a file that was downloadable while it waited for a + // scanner may already be on somebody's machine, and its + // download history is the only way to know. + 'was_available' => $wasAvailable, + ]); + + return ScanStatus::Infected; + } + + /** The scanner could not open the file: too large, or encrypted. */ + private function unscannable(File $file, NotScannedReason $reason, ?string $engine): ScanStatus + { + if ($this->config->blocksUnscannable()) { + $this->settle($file, ScanStatus::UnscannableBlocked, $reason->value, $engine); + $this->purgeRenditions($file); + + $this->activity->logSystem(Action::FileQuarantined, [ + 'id' => $file->id, + 'name' => $file->name, + 'threat' => $reason->label(), + 'was_available' => false, + ]); + + return ScanStatus::UnscannableBlocked; + } + + return $this->letThrough($file, $reason, $engine); + } + + /** The scanner never answered. Either wait for it, or let the file go. */ + private function unavailable(File $file, ?string $reason): ?ScanStatus + { + if ($this->config->holdsWhileUnavailable()) { + return null; + } + + return $this->letThrough($file, NotScannedReason::ScannerUnavailable, null); + } + + /** + * Allowed through without being checked. + * + * Always logged, even though it is the configured behaviour: this is + * the state where the installation looks protected and is not, and + * the log is what makes "we were unprotected between these two dates" + * answerable afterwards. + */ + private function letThrough(File $file, NotScannedReason $reason, ?string $engine): ScanStatus + { + $this->settle($file, ScanStatus::NotScanned, $reason->value, $engine); + + $this->activity->logSystem(Action::FileNotScanned, [ + 'id' => $file->id, + 'name' => $file->name, + 'reason' => $reason->value, + ]); + + return ScanStatus::NotScanned; + } + + private function settle(File $file, ScanStatus $status, ?string $note, ?string $engine): ScanStatus + { + $file->forceFill([ + 'scan_status' => $status, + 'scan_note' => $note, + 'scanned_at' => now(), + 'scan_engine' => $engine, + ])->save(); + + $this->availability->markAvailable($file); + + return $status; + } + + /** + * Thumbnails and previews are cached copies of the same bytes, served + * by routes of their own, so a quarantined file with a rendition + * already on disk would still be showing part of itself. + */ + private function purgeRenditions(File $file): void + { + foreach (ThumbnailGenerator::pathsFor($file->id, $file->mime_type) as $path) { + Storage::disk('files')->delete($path); + } + } +} diff --git a/app/Modules/Files/Scanning/ScanStatus.php b/app/Modules/Files/Scanning/ScanStatus.php new file mode 100644 index 00000000..84584b8e --- /dev/null +++ b/app/Modules/Files/Scanning/ScanStatus.php @@ -0,0 +1,80 @@ + true, + self::Pending, self::Infected, self::UnscannableBlocked => false, + }; + } + + /** + * The states a query may hand to somebody other than staff. + * + * @return list + */ + public static function availableValues(): array + { + return array_values(array_map( + fn (self $status): string => $status->value, + array_filter(self::cases(), fn (self $status): bool => $status->isAvailable()), + )); + } + + /** Whether this state is waiting on an administrator's decision. */ + public function isQuarantined(): bool + { + return $this === self::Infected || $this === self::UnscannableBlocked; + } + + /** + * English, and the translation key — what staff see on the file. + */ + public function label(): string + { + return match ($this) { + self::Pending => 'Checking for viruses', + self::Clean => 'Checked', + self::Infected => 'Quarantined', + self::Released => 'Released by an administrator', + self::NotScanned => 'Not scanned', + self::UnscannableBlocked => 'Blocked: could not be scanned', + }; + } +} diff --git a/app/Modules/Files/Scanning/ScanVerdict.php b/app/Modules/Files/Scanning/ScanVerdict.php new file mode 100644 index 00000000..ed6e2e2b --- /dev/null +++ b/app/Modules/Files/Scanning/ScanVerdict.php @@ -0,0 +1,51 @@ +definitionsDate === null) { + return null; + } + + // diffInHours() answers with a float; whole hours is what the + // warning threshold and the status document both speak in. + return (int) $this->definitionsDate->diffInHours(now()); + } +} diff --git a/app/Modules/Files/Scanning/ScanningConfig.php b/app/Modules/Files/Scanning/ScanningConfig.php new file mode 100644 index 00000000..16f6a845 --- /dev/null +++ b/app/Modules/Files/Scanning/ScanningConfig.php @@ -0,0 +1,109 @@ +isManaged() || $this->settings->get(Setting::VirusScanningEnabled) === true; + } + + public function isManaged(): bool + { + return $this->managedAddress() !== ''; + } + + public function address(): string + { + if ($this->isManaged()) { + return $this->managedAddress(); + } + + $stored = $this->settings->get(Setting::VirusScannerAddress); + + return is_string($stored) ? trim($stored) : ''; + } + + /** + * The largest file this installation sends to the scanner, in bytes. + * Zero means no limit of our own — clamd's StreamMaxLength still + * applies, and answers with tooLarge when it is reached. + */ + public function maxScanBytes(): int + { + return max(0, (int) $this->settings->get(Setting::VirusScanMaxSizeMb)) * 1024 * 1024; + } + + /** What happens to a file the scanner could not open. */ + public function blocksUnscannable(): bool + { + return $this->settings->get(Setting::VirusUnscannablePolicy) === 'block'; + } + + /** Whether uploads wait for a scanner that is not answering. */ + public function holdsWhileUnavailable(): bool + { + return $this->settings->get(Setting::VirusScannerDownPolicy) === 'hold'; + } + + /** How long a file waits for an unreachable scanner before the policy applies. */ + public function unavailableWaitMinutes(): int + { + return max(1, (int) $this->settings->get(Setting::VirusScannerWaitMinutes)); + } + + /** How many already-stored files an hour-long backfill may scan per minute. */ + public function existingScanRatePerMinute(): int + { + return max(1, (int) $this->settings->get(Setting::VirusScanExistingRatePerMinute)); + } + + public function connectTimeoutSeconds(): int + { + return max(1, (int) config('projectsend.scanning.connect_timeout', 5)); + } + + public function replyTimeoutSeconds(): int + { + return max(1, (int) config('projectsend.scanning.reply_timeout', 600)); + } + + private function managedAddress(): string + { + $address = config('projectsend.scanning.address', ''); + + return is_string($address) ? trim($address) : ''; + } +} diff --git a/app/Modules/Files/Scanning/VirusScanner.php b/app/Modules/Files/Scanning/VirusScanner.php new file mode 100644 index 00000000..7a221027 --- /dev/null +++ b/app/Modules/Files/Scanning/VirusScanner.php @@ -0,0 +1,35 @@ +scanning->enabled(); + $file = File::query()->create([ 'uploaded_by' => $uploader->id, 'folder_id' => $folderId, @@ -50,6 +56,13 @@ class StoreUploadedFile 'mime_type' => $mimeType, 'size' => $size, 'checksum' => $checksum, + // Decided in the same insert as the row rather than a moment + // later: a file is unavailable from the instant it exists, or + // there is a window in which it is neither scanned nor + // withheld. Every upload path arrives here, so this is the + // only place that has to be right. + 'scan_status' => $scanning ? ScanStatus::Pending : ScanStatus::NotScanned, + 'scan_note' => $scanning ? null : NotScannedReason::BeforeScanning->value, ]); $this->activity->log($action, $uploader, $file); diff --git a/app/Modules/Groups/Http/Controllers/PublicGroupsController.php b/app/Modules/Groups/Http/Controllers/PublicGroupsController.php index 642bedcc..5a6da881 100644 --- a/app/Modules/Groups/Http/Controllers/PublicGroupsController.php +++ b/app/Modules/Groups/Http/Controllers/PublicGroupsController.php @@ -13,6 +13,7 @@ use App\Modules\Files\Delivery\FileDelivery; use App\Modules\Files\Delivery\StoredFileResponse; use App\Modules\Files\Models\Category; use App\Modules\Files\Models\File; +use App\Modules\Files\Scanning\FileAvailability; use App\Modules\Files\Models\Folder; use App\Modules\Files\Preview\PreviewKind; use App\Modules\Files\Preview\PreviewLog; @@ -81,6 +82,7 @@ class PublicGroupsController extends Controller private readonly ActivityLogger $activity, private readonly PreviewLog $previews, private readonly DownloadAllowance $allowance, + private readonly FileAvailability $availability, private readonly ThumbnailGenerator $thumbnails, private readonly PublicThemeRegistry $themes, private readonly CapabilityRegistry $capabilities, @@ -192,6 +194,7 @@ class PublicGroupsController extends Controller $this->guardSlug($publicSlug); abort_unless($file->isEffectivelyPublic() && ! $file->isExpired(), 404); + abort_unless($this->availability->isAvailable($file), 404); $file->loadMissing('categories'); @@ -243,6 +246,7 @@ class PublicGroupsController extends Controller $this->guardSlug($publicSlug); abort_unless($file->isEffectivelyPublic() && ! $file->isExpired(), 404); + abort_unless($this->availability->isAvailable($file), 404); abort_unless(ThumbnailGenerator::supports($file->mime_type), 404); // Always the external variant — nobody reaching a public listing is @@ -300,6 +304,7 @@ class PublicGroupsController extends Controller $this->guardSlug($publicSlug); abort_unless($file->isEffectivelyPublic() && ! $file->isExpired(), 404); + abort_unless($this->availability->isAvailable($file), 404); abort_unless($this->settings->get(Setting::PublicListingPreviewEnabled) === true, 404); abort_if(PreviewKind::forMime($file->mime_type) === null, 404); @@ -346,6 +351,7 @@ class PublicGroupsController extends Controller $this->guardSlug($publicSlug); abort_unless($file->isEffectivelyPublic() && ! $file->isExpired(), 404); + abort_unless($this->availability->isAvailable($file), 404); // 403 rather than 404, unlike the checks above it: the file is // genuinely here and genuinely public, it has simply been taken diff --git a/app/Modules/Platform/Settings/Setting.php b/app/Modules/Platform/Settings/Setting.php index d3fec14e..c44a0e9b 100644 --- a/app/Modules/Platform/Settings/Setting.php +++ b/app/Modules/Platform/Settings/Setting.php @@ -175,6 +175,27 @@ enum Setting: string // of this stored value — whenever external storage is active; this // feature only ever operates on the local disk (see // PurgeOrphanFilesCommand and FileRetentionSettingsController). + // Virus scanning — see docs/feature-virus-scanning.md and + // App\Modules\Files\Scanning\ScanningConfig, which is what reads + // these (the address and the on/off switch can both be overruled by a + // managed configuration in the environment). + case VirusScanningEnabled = 'virus_scanning_enabled'; + case VirusScannerAddress = 'virus_scanner_address'; + case VirusScanMaxSizeMb = 'virus_scan_max_size_mb'; + // 'allow' or 'block' — what happens to a file the scanner cannot open + // (too large, or encrypted). Allowing marks it "not scanned" rather + // than calling it clean. + case VirusUnscannablePolicy = 'virus_unscannable_policy'; + // 'allow' or 'hold' — what happens to new uploads while the scanner + // is unreachable. + case VirusScannerDownPolicy = 'virus_scanner_down_policy'; + // How long a file waits for an unreachable scanner before the policy + // above is applied. + case VirusScannerWaitMinutes = 'virus_scanner_wait_minutes'; + // How fast "Scan existing files" works through a library that was + // uploaded before scanning was switched on. + case VirusScanExistingRatePerMinute = 'virus_scan_existing_rate_per_minute'; + case OrphanFilesAutoDeleteEnabled = 'orphan_files_auto_delete_enabled'; case OrphanFilesDeleteAfterDays = 'orphan_files_delete_after_days'; @@ -353,6 +374,9 @@ enum Setting: string self::ClientsCanSelectGroup, self::TwoFactorEnforcement, self::UploadTypeRestriction, + self::VirusScannerAddress, + self::VirusUnscannablePolicy, + self::VirusScannerDownPolicy, self::DownloadIpLogging, self::CommentsScope, self::CommentsAuthors, @@ -394,6 +418,7 @@ enum Setting: string self::CaptchaOnPasswordReset, self::CaptchaOnPublicComments, self::PasswordRejectBreached, + self::VirusScanningEnabled, self::GettingStartedPending => SettingType::Boolean, self::ClientsAutoGroup, @@ -410,6 +435,9 @@ enum Setting: string self::ExpiredFilesDeleteAfterDays, self::CommentsEditWindowMinutes, self::OrphanFilesDeleteAfterDays, + self::VirusScanMaxSizeMb, + self::VirusScannerWaitMinutes, + self::VirusScanExistingRatePerMinute, self::PasswordMinLength => SettingType::Integer, self::AdminNotificationEmails, @@ -449,6 +477,9 @@ enum Setting: string self::PublicListingEnabled, self::ExpiredFilesAutoDeleteEnabled, self::PublicCommentsEnabled, + // Off until somebody points it at a scanner, or a managed + // configuration names one — see ScanningConfig. + self::VirusScanningEnabled, self::OrphanFilesAutoDeleteEnabled => false, self::CheckForUpdates, @@ -482,6 +513,22 @@ enum Setting: string self::OrphanFilesDeleteAfterDays => 30, self::CommentsEditWindowMinutes => 15, + // Large enough for ordinary documents and archives, small + // enough that one upload does not hold the scanner for + // minutes. Must stay under clamd's own StreamMaxLength. + self::VirusScanMaxSizeMb => 512, + self::VirusScannerWaitMinutes => 10, + self::VirusScanExistingRatePerMinute => 60, + + // Both of these let files through, which is the product + // owner's decision (2026-09-14): a scanner that cannot answer + // must not stop people working. What pays for it is + // visibility — every file allowed through this way is marked + // "not scanned", and the dashboard and projectsend:status both + // say so while it is happening. + self::VirusUnscannablePolicy, + self::VirusScannerDownPolicy => 'allow', + self::AdminNotificationEmails => [], // English only out of the box: a fresh install offering @@ -501,6 +548,10 @@ enum Setting: string self::CommentsScope => 'all', self::CommentsAuthors => 'staff_and_clients', self::PublicListingSlug => 'public', + // Empty means no scanner configured here. A managed + // configuration in the environment beats this when present — + // ScanningConfig, not this default, is what a caller asks. + self::VirusScannerAddress => '', self::DefaultLocale => '', self::Timezone => '', self::Theme => 'default', diff --git a/config/projectsend.php b/config/projectsend.php index adea811c..985e9e93 100644 --- a/config/projectsend.php +++ b/config/projectsend.php @@ -180,6 +180,34 @@ return [ ], ], + /* + |-------------------------------------------------------------------------- + | Virus scanning + |-------------------------------------------------------------------------- + | + | Naming an address here makes scanning *managed*: that scanner is used, + | scanning cannot be switched off from the settings screen, and the + | connection fields are hidden. It is how a hosted fleet points every + | site at one scanning service, and a self-hosted operator who would + | rather configure this in the environment than in the database can use + | it too. Everything else — what to do with files that cannot be scanned, + | and what to do while the scanner is down — stays a setting either way, + | because those are the site's decisions rather than the platform's. + | + | Format: unix:///var/run/clamav/clamd.sock, or tcp://host:3310. + | + */ + + 'scanning' => [ + 'address' => env('PROJECTSEND_SCANNER_ADDRESS'), + + // How long to wait for the socket, and then for each reply. A scan + // streams the whole file before the reply comes, so the second one + // has to allow for the largest file this installation accepts. + 'connect_timeout' => (int) env('PROJECTSEND_SCANNER_CONNECT_TIMEOUT', 5), + 'reply_timeout' => (int) env('PROJECTSEND_SCANNER_REPLY_TIMEOUT', 600), + ], + /* |-------------------------------------------------------------------------- | Release identity diff --git a/database/migrations/2026_09_16_090000_add_scan_columns_to_files_table.php b/database/migrations/2026_09_16_090000_add_scan_columns_to_files_table.php new file mode 100644 index 00000000..d8202347 --- /dev/null +++ b/database/migrations/2026_09_16_090000_add_scan_columns_to_files_table.php @@ -0,0 +1,49 @@ +string('scan_status', 24)->default('not_scanned')->index()->after('checksum'); + + // The threat name when infected, or why it was not scanned. + // See ScanStatus and NotScannedReason for the two vocabularies + // this column carries. + $table->string('scan_note')->nullable()->after('scan_status'); + $table->timestamp('scanned_at')->nullable()->after('scan_note'); + + // Engine and definitions, as the scanner reported them at the + // time. Kept so a verdict can be read back against what knew + // it — definitions change daily. + $table->string('scan_engine')->nullable()->after('scanned_at'); + $table->unsignedInteger('scan_attempts')->default(0)->after('scan_engine'); + + // Who overruled a quarantine, and when. The reason they gave + // is in the activity log; this is what the file itself shows. + $table->foreignId('released_by')->nullable()->after('scan_attempts')->constrained('users')->nullOnDelete(); + $table->timestamp('released_at')->nullable()->after('released_by'); + }); + } + + public function down(): void + { + Schema::table('files', function (Blueprint $table) { + $table->dropConstrainedForeignId('released_by'); + $table->dropIndex(['scan_status']); + $table->dropColumn(['scan_status', 'scan_note', 'scanned_at', 'scan_engine', 'scan_attempts', 'released_at']); + }); + } +}; diff --git a/docs/api/openapi.json b/docs/api/openapi.json index 3d44f966..9898207f 100644 --- a/docs/api/openapi.json +++ b/docs/api/openapi.json @@ -3560,6 +3560,9 @@ "folder.made_private", "upload.aborted", "file.imported", + "file.quarantined", + "file.released", + "file.not_scanned", "orphan_file.deleted", "orphan_file.auto_deleted", "file.expired_deleted", diff --git a/resources/js/pages/share/show.tsx b/resources/js/pages/share/show.tsx index 21c473f8..c64f2e36 100644 --- a/resources/js/pages/share/show.tsx +++ b/resources/js/pages/share/show.tsx @@ -8,7 +8,7 @@ import AuthLayout from '@/layouts/auth-layout'; import { formatBytes } from '@/lib/format-bytes'; interface ShareShowProps { - status: 'active' | 'expired' | 'limit_reached' | 'not_found'; + status: 'active' | 'expired' | 'limit_reached' | 'not_found' | 'checking' | 'unavailable'; file?: { original_name: string; size: number; @@ -26,11 +26,21 @@ export default function ShareShow({ status, file, download_url }: ShareShowProps ? t('This link has expired.') : status === 'limit_reached' ? t('This link has reached its download limit.') - : t("This link doesn't exist or has been revoked."); + : // A link can exist before its file has been checked for + // viruses — on some installations one is created the + // moment a file is uploaded — so this is "come back in a + // minute", not "something is wrong". + status === 'checking' + ? t('This file is still being checked for viruses. Please try again in a few minutes.') + : status === 'unavailable' + ? t('This file is not available.') + : t("This link doesn't exist or has been revoked."); + + const title = status === 'checking' ? t('Almost ready') : t('Link unavailable'); return ( - - + + ); } diff --git a/tests/Feature/Files/VirusScanningTest.php b/tests/Feature/Files/VirusScanningTest.php new file mode 100644 index 00000000..db1da184 --- /dev/null +++ b/tests/Feature/Files/VirusScanningTest.php @@ -0,0 +1,321 @@ +admin = User::factory()->create(); + + // Settings survive RefreshDatabase's rollback in the cache, so every + // value this file depends on is stated rather than assumed. + app(Settings::class)->set(Setting::VirusScanningEnabled, true); + app(Settings::class)->set(Setting::VirusScannerAddress, 'tcp://scanner.test:3310'); + app(Settings::class)->set(Setting::VirusScanMaxSizeMb, 512); + app(Settings::class)->set(Setting::VirusUnscannablePolicy, 'allow'); + app(Settings::class)->set(Setting::VirusScannerDownPolicy, 'allow'); + app(Settings::class)->set(Setting::VirusScannerWaitMinutes, 10); +}); + +function fakeScanner(?ScanVerdict $verdict = null): FakeVirusScanner +{ + $scanner = new FakeVirusScanner($verdict); + app()->instance(VirusScanner::class, $scanner); + + return $scanner; +} + +/** A stored file with real bytes, as an upload would leave it. */ +function scannableFile(array $overrides = []): File +{ + $path = 'uploads/'.Str::uuid()->toString().'.pdf'; + Storage::disk('files')->put($path, 'some bytes'); + + return File::factory()->create(array_merge([ + 'path' => $path, + 'disk' => 'files', + 'size' => 10, + 'scan_status' => ScanStatus::Pending, + 'checksum' => hash('sha256', $path), + ], $overrides)); +} + +/** Run the job the way the queue would, with this test's fake scanner. */ +function runScan(File $file): void +{ + (new ScanFileJob($file->id))->handle( + app(VirusScanner::class), + app(App\Modules\Files\Scanning\ScanPolicy::class), + app(App\Modules\Files\Scanning\ScanningConfig::class), + ); +} + +/* +|-------------------------------------------------------------------------- +| A verdict, and what this installation does with it +|-------------------------------------------------------------------------- +*/ + +test('a clean file becomes available', function () { + fakeScanner(ScanVerdict::clean('FakeAV 1.0')); + $file = scannableFile(); + + runScan($file); + + $file->refresh(); + expect($file->scan_status)->toBe(ScanStatus::Clean) + ->and($file->scan_status->isAvailable())->toBeTrue() + ->and($file->scanned_at)->not->toBeNull() + ->and($file->scan_engine)->toBe('FakeAV 1.0'); +}); + +test('an infected file is quarantined and logged', function () { + fakeScanner(ScanVerdict::infected('Eicar-Test-Signature')); + $file = scannableFile(); + + runScan($file); + + $file->refresh(); + expect($file->scan_status)->toBe(ScanStatus::Infected) + ->and($file->scan_status->isAvailable())->toBeFalse() + ->and($file->scan_note)->toBe('Eicar-Test-Signature'); + + $entry = ActivityLog::query()->where('action', Action::FileQuarantined)->sole(); + expect($entry->context['threat'])->toBe('Eicar-Test-Signature') + ->and($entry->context['was_available'])->toBeFalse(); +}); + +test('a quarantined file loses the thumbnails already rendered from it', function () { + fakeScanner(ScanVerdict::infected('Some.Threat')); + $file = scannableFile(['mime_type' => 'image/png']); + + $paths = App\Modules\Files\Thumbnails\ThumbnailGenerator::pathsFor($file->id, 'image/png'); + expect($paths)->not->toBeEmpty(); + + foreach ($paths as $path) { + Storage::disk('files')->put($path, 'rendered'); + } + + runScan($file); + + foreach ($paths as $path) { + expect(Storage::disk('files')->exists($path))->toBeFalse(); + } +}); + +test('a file the scanner cannot open is allowed through, marked, and logged', function () { + fakeScanner(ScanVerdict::encrypted()); + $file = scannableFile(); + + runScan($file); + + $file->refresh(); + expect($file->scan_status)->toBe(ScanStatus::NotScanned) + ->and($file->scan_note)->toBe(NotScannedReason::Encrypted->value) + ->and($file->scan_status->isAvailable())->toBeTrue(); + + expect(ActivityLog::query()->where('action', Action::FileNotScanned)->count())->toBe(1); +}); + +test('the same file is blocked when this installation says to block', function () { + app(Settings::class)->set(Setting::VirusUnscannablePolicy, 'block'); + fakeScanner(ScanVerdict::encrypted()); + $file = scannableFile(); + + runScan($file); + + expect($file->refresh()->scan_status)->toBe(ScanStatus::UnscannableBlocked) + ->and($file->scan_status->isAvailable())->toBeFalse(); +}); + +test('a file larger than the maximum never reaches the scanner at all', function () { + app(Settings::class)->set(Setting::VirusScanMaxSizeMb, 1); + + // The real client, deliberately: refusing an oversized file before a + // socket is opened is its job, and a fake that answered anyway would + // hide the day that check moves or disappears. There is no scanner at + // the configured address, so anything but an early refusal here would + // come back as "unavailable" instead. + $stream = fopen('php://memory', 'r+'); + $verdict = app(App\Modules\Files\Scanning\ClamAvScanner::class)->scan($stream, 2 * 1024 * 1024); + fclose($stream); + + expect($verdict->outcome)->toBe(App\Modules\Files\Scanning\ScanOutcome::TooLarge); +}); + +/* +|-------------------------------------------------------------------------- +| A scanner that is not answering +|-------------------------------------------------------------------------- +*/ + +test('a file waits while the scanner is down, then goes through', function () { + fakeScanner(ScanVerdict::unavailable('connection refused')); + $file = scannableFile(); + + runScan($file); + expect($file->refresh()->scan_status)->toBe(ScanStatus::Pending); + + // Past the ten minutes this installation is willing to wait. + $this->travel(11)->minutes(); + + runScan($file); + + $file->refresh(); + expect($file->scan_status)->toBe(ScanStatus::NotScanned) + ->and($file->scan_note)->toBe(NotScannedReason::ScannerUnavailable->value); +}); + +test('an installation set to hold keeps waiting however long it takes', function () { + app(Settings::class)->set(Setting::VirusScannerDownPolicy, 'hold'); + fakeScanner(ScanVerdict::unavailable('connection refused')); + $file = scannableFile(); + + runScan($file); + $this->travel(3)->days(); + runScan($file); + + expect($file->refresh()->scan_status)->toBe(ScanStatus::Pending); +}); + +test('an upload identical to a quarantined file is quarantined without a scan', function () { + $scanner = fakeScanner(ScanVerdict::clean()); + + $known = scannableFile(['scan_status' => ScanStatus::Infected, 'scan_note' => 'Known.Threat']); + $copy = scannableFile(['checksum' => $known->checksum]); + + runScan($copy); + + expect($copy->refresh()->scan_status)->toBe(ScanStatus::Infected) + ->and($copy->scan_note)->toBe('Known.Threat') + ->and($scanner->scans)->toBe(0); +}); + +/* +|-------------------------------------------------------------------------- +| Uploads +|-------------------------------------------------------------------------- +*/ + +test('a new upload is pending while scanning is on', function () { + fakeScanner(); + + $file = app(App\Modules\Files\Uploads\StoreUploadedFile::class)->create( + uploader: $this->admin, + originalName: 'report.pdf', + path: 'uploads/report.pdf', + mimeType: 'application/pdf', + size: 10, + checksum: str_repeat('b', 64), + ); + + expect($file->scan_status)->toBe(ScanStatus::Pending); +}); + +test('a new upload is marked never scanned while scanning is off', function () { + app(Settings::class)->set(Setting::VirusScanningEnabled, false); + app(Settings::class)->set(Setting::VirusScannerAddress, ''); + + $file = app(App\Modules\Files\Uploads\StoreUploadedFile::class)->create( + uploader: $this->admin, + originalName: 'report.pdf', + path: 'uploads/report.pdf', + mimeType: 'application/pdf', + size: 10, + checksum: str_repeat('c', 64), + ); + + expect($file->scan_status)->toBe(ScanStatus::NotScanned) + ->and($file->scan_note)->toBe(NotScannedReason::BeforeScanning->value); +}); + +/* +|-------------------------------------------------------------------------- +| Nothing unchecked leaves the server +|-------------------------------------------------------------------------- +*/ + +test('no route serves the bytes of a file that is still being checked', function () { + $file = scannableFile(['uploaded_by' => $this->admin->id, 'mime_type' => 'image/png']); + + foreach ([ + "/files/{$file->id}/download", + "/files/{$file->id}/thumbnail", + "/files/{$file->id}/preview", + ] as $path) { + $this->actingAs($this->admin)->get($path)->assertStatus(423, "{$path} served a pending file"); + } +}); + +test('a quarantined file is refused for the same routes', function () { + $file = scannableFile(['uploaded_by' => $this->admin->id, 'scan_status' => ScanStatus::Infected, 'scan_note' => 'X']); + + $this->actingAs($this->admin)->get("/files/{$file->id}/download")->assertStatus(423); +}); + +test('a clean file downloads normally', function () { + $file = scannableFile(['uploaded_by' => $this->admin->id, 'scan_status' => ScanStatus::Clean]); + + $this->actingAs($this->admin)->get("/files/{$file->id}/download")->assertOk(); +}); + +test('a share link says the file is still being checked', function () { + $file = scannableFile(); + $link = App\Modules\Files\Models\ShareLink::query()->create([ + 'shareable_type' => $file->getMorphClass(), + 'shareable_id' => $file->id, + 'token' => Str::random(32), + 'created_by' => $this->admin->id, + ]); + + $this->get("/s/{$link->token}")->assertInertia( + fn (Inertia\Testing\AssertableInertia $page) => $page->component('share/show')->where('status', 'checking'), + ); + + $this->get("/s/{$link->token}/download")->assertRedirect(route('share.show', $link->token)); +}); + +test('a pending file is not visible to the client it was shared with', function () { + $client = User::factory()->client()->create(); + $file = scannableFile(['uploaded_by' => $this->admin->id]); + app(App\Modules\Files\Sharing\FileSharing::class)->assign($file, $client, $client->name); + + expect(File::query()->visibleToClient($client)->count())->toBe(0); + + $file->forceFill(['scan_status' => ScanStatus::Clean])->save(); + + expect(File::query()->visibleToClient($client)->count())->toBe(1); +}); + +test('a client still sees their own upload while it is being checked', function () { + $client = User::factory()->client()->create(); + $file = scannableFile(['uploaded_by' => $client->id]); + + expect(File::query()->visibleToClient($client)->pluck('id')->all())->toBe([$file->id]); +}); + +test('a pending file is left out of a zip', function () { + $pending = scannableFile(['uploaded_by' => $this->admin->id]); + $clean = scannableFile(['uploaded_by' => $this->admin->id, 'scan_status' => ScanStatus::Clean]); + + $this->actingAs($this->admin) + ->postJson('/zip-downloads', ['file_ids' => [$pending->id, $clean->id], 'folder_ids' => []]) + ->assertOk(); + + $zip = App\Modules\Files\Models\ZipDownload::query()->latest('id')->sole(); + expect($zip->file_ids)->toBe([$clean->id]); +}); diff --git a/tests/Support/FakeVirusScanner.php b/tests/Support/FakeVirusScanner.php new file mode 100644 index 00000000..6b290d77 --- /dev/null +++ b/tests/Support/FakeVirusScanner.php @@ -0,0 +1,79 @@ + */ + private array $verdicts = []; + + public int $scans = 0; + + /** @var list the sizes it was asked to read, in order */ + public array $sizes = []; + + private ScannerStatus $status; + + public function __construct(?ScanVerdict $verdict = null) + { + if ($verdict !== null) { + $this->verdicts[] = $verdict; + } + + $this->status = new ScannerStatus(true, 'FakeAV 1.0', 1, now()); + } + + /** Answer this next. Queued, so a test can say "down, then up". */ + public function willAnswer(ScanVerdict ...$verdicts): self + { + foreach ($verdicts as $verdict) { + $this->verdicts[] = $verdict; + } + + return $this; + } + + public function reports(ScannerStatus $status): self + { + $this->status = $status; + + return $this; + } + + public function scan(mixed $stream, int $size): ScanVerdict + { + $this->scans++; + $this->sizes[] = $size; + + // The last answer stands once the queue runs dry: a test that + // says "infected" once means it, however many times the job is + // retried. + return count($this->verdicts) > 1 + ? array_shift($this->verdicts) + : ($this->verdicts[0] ?? ScanVerdict::clean('FakeAV 1.0')); + } + + public function status(): ScannerStatus + { + return $this->status; + } +} From e9496dc3573e3c46884df730fc1b70d1d5effaf6 Mon Sep 17 00:00:00 2001 From: ignacionelson Date: Wed, 16 Sep 2026 14:29:41 -0300 Subject: [PATCH 02/20] Give quarantined files a screen, an owner, and somebody to tell MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An infected file now goes somewhere rather than nowhere. Staff holding the new release_quarantined_files permission get a Quarantine screen listing what was refused, who uploaded it, and what the scanner called it. They can delete it as they always could, or release it — which needs a written reason, a password confirmation on top of the permission, and lands in the activity log under their name. Only the administrator role holds that permission by default. Deciding a threat report is wrong is a different judgement from deciding a file is no longer needed, which is why it is not delete_files. Two notifications, two audiences: staff who can act on it, and the person who uploaded it — for whom this is how they learn their own machine has something on it. The people the file was shared with are deliberately not told about a file they never received. `projectsend:scan-files` runs hourly: it re-queues files still waiting, and re-scans the ones that went out unscanned while the scanner was unreachable, since it may be back. With --existing it also works through a library uploaded before scanning was switched on, paced by a setting so it does not starve today's uploads. A file that was downloadable before it was caught says so on the screen, with its download count, because that is the case where somebody may already have a copy. --- .../Files/Console/ScanFilesCommand.php | 94 ++++++++++ app/Modules/Files/FilesServiceProvider.php | 22 +++ .../Http/Controllers/QuarantineController.php | 114 ++++++++++++ app/Modules/Files/Models/File.php | 2 + .../Files/Scanning/QuarantineNotifier.php | 71 +++++++ app/Modules/Files/Scanning/ScanPolicy.php | 7 + .../Identity/Permissions/Permission.php | 9 + .../SchedulerMonitoringController.php | 1 + ...090000_add_scan_columns_to_files_table.php | 10 +- resources/js/components/app-sidebar.tsx | 28 +-- resources/js/pages/files/quarantine.tsx | 166 +++++++++++++++++ routes/console.php | 3 + routes/web.php | 11 ++ tests/Feature/Files/QuarantineTest.php | 173 ++++++++++++++++++ .../Platform/SchedulerMonitoringTest.php | 2 +- 15 files changed, 687 insertions(+), 26 deletions(-) create mode 100644 app/Modules/Files/Console/ScanFilesCommand.php create mode 100644 app/Modules/Files/Http/Controllers/QuarantineController.php create mode 100644 app/Modules/Files/Scanning/QuarantineNotifier.php create mode 100644 resources/js/pages/files/quarantine.tsx create mode 100644 tests/Feature/Files/QuarantineTest.php diff --git a/app/Modules/Files/Console/ScanFilesCommand.php b/app/Modules/Files/Console/ScanFilesCommand.php new file mode 100644 index 00000000..f05a5bc3 --- /dev/null +++ b/app/Modules/Files/Console/ScanFilesCommand.php @@ -0,0 +1,94 @@ +enabled()) { + $this->info('Virus scanning is switched off.'); + + return self::SUCCESS; + } + + $waiting = $this->dispatchFor(File::query()->where('scan_status', ScanStatus::Pending)); + + // Allowed through while the scanner was unreachable. Now that it + // may be back, they are asked again — a file found infected at + // this point is quarantined like any other, and its quarantine + // notice says it was available in the meantime. + $missed = $this->dispatchFor( + File::query() + ->where('scan_status', ScanStatus::NotScanned) + ->where('scan_note', NotScannedReason::ScannerUnavailable->value) + ); + + $this->info("Re-queued {$waiting} waiting file(s) and {$missed} that were missed while the scanner was down."); + + if ($this->option('existing')) { + // Paced, because this can be a whole library at once and the + // scanner is also serving today's uploads. An hour's worth per + // run, since that is how often this command runs. + $limit = $config->existingScanRatePerMinute() * 60; + + $old = $this->dispatchFor( + File::query() + ->where('scan_status', ScanStatus::NotScanned) + ->where('scan_note', NotScannedReason::BeforeScanning->value), + $limit, + ); + + $this->info("Queued {$old} file(s) that had never been scanned."); + } + + return self::SUCCESS; + } + + /** + * @param Builder $query + */ + private function dispatchFor(Builder $query, ?int $limit = null): int + { + if ($limit !== null) { + $query->limit($limit); + } + + $ids = $query->orderBy('id')->pluck('id'); + + foreach ($ids as $id) { + // Back to pending first: the job only acts on a pending file, + // which is what stops two runs of this command from scanning + // the same file twice. + File::query()->whereKey($id)->update(['scan_status' => ScanStatus::Pending->value, 'scan_note' => null]); + + ScanFileJob::dispatch((int) $id); + } + + return $ids->count(); + } +} diff --git a/app/Modules/Files/FilesServiceProvider.php b/app/Modules/Files/FilesServiceProvider.php index 559c47e5..bf63f2f5 100644 --- a/app/Modules/Files/FilesServiceProvider.php +++ b/app/Modules/Files/FilesServiceProvider.php @@ -108,6 +108,27 @@ class FilesServiceProvider extends ServiceProvider url: fn (array $data): string => route('my-files.index'), )); + // Two audiences, two types, because they need different words and + // different links. Staff get a queue to act on; the person who + // uploaded gets told their file did not go through. + $registry = $this->app->make(NotificationTypeRegistry::class); + + $registry->register(new NotificationTypeDefinition( + key: 'file_quarantined', + label: 'A file was quarantined by the virus scanner', + template: 'A virus was found in ":itemName", uploaded by :uploaderName', + url: fn (array $data): string => route('files.quarantine'), + )); + + $registry->register(new NotificationTypeDefinition( + key: 'upload_blocked', + label: 'One of your uploads was blocked', + template: 'Your file ":itemName" was blocked: :threat', + // Their own files list. Deliberately not the quarantine + // screen, which they cannot open. + url: fn (array $data): string => route('my-files.index'), + )); + // Every upload path converges on FileWasStored, so this is the // one place a scan is started from. Dispatched rather than run // inline: a 5 GB file takes minutes to read, and an upload must @@ -121,6 +142,7 @@ class FilesServiceProvider extends ServiceProvider if ($this->app->runningInConsole()) { $this->commands([ + Console\ScanFilesCommand::class, Console\PurgeStaleUploadsCommand::class, Console\PurgeZipDownloadsCommand::class, Console\PurgeExpiredFilesCommand::class, diff --git a/app/Modules/Files/Http/Controllers/QuarantineController.php b/app/Modules/Files/Http/Controllers/QuarantineController.php new file mode 100644 index 00000000..e45a98e8 --- /dev/null +++ b/app/Modules/Files/Http/Controllers/QuarantineController.php @@ -0,0 +1,114 @@ +whereIn('scan_status', [ScanStatus::Infected->value, ScanStatus::UnscannableBlocked->value]) + ->with('uploader') + ->orderByDesc('scanned_at') + ->paginate(25) + ->withQueryString(); + + $files->through(fn (File $file): array => [ + 'id' => $file->id, + 'name' => $file->name, + 'original_name' => $file->original_name, + 'size' => $file->size, + 'uploader' => $file->uploader?->name, + // The threat name, or — for a file nothing could open — what + // stopped it being read. + 'threat' => $file->scan_status === ScanStatus::UnscannableBlocked + ? __(NotScannedReason::tryFrom((string) $file->scan_note)?->label() ?? 'Could not be scanned') + : $file->scan_note, + 'status' => $file->scan_status->value, + 'scanned_at' => $file->scanned_at?->toIso8601String(), + // True only for a file that went out unscanned while the + // scanner was unreachable and was caught later — which is the + // one case where somebody may already have a copy. + 'was_available' => $file->scan_was_available, + 'downloads_count' => $file->downloads()->count(), + ]); + + return Inertia::render('files/quarantine', [ + 'files' => $files->items(), + 'pagination' => Pagination::meta($files), + ]); + } + + /** + * Overrule the scanner for one file. + * + * The reason is required and is recorded against the person who gave + * it. A release is not undone by a later scan: the file stays + * released until somebody deletes it, which is the point — an + * administrator who has decided a detection is wrong should not have + * to decide it again every hour. + */ + public function release(Request $request, File $file): RedirectResponse + { + abort_unless($file->scan_status->isQuarantined(), 404); + + $validated = $request->validate([ + 'reason' => ['required', 'string', 'max:500'], + ]); + + $actor = $request->user(); + assert($actor !== null); + + $file->forceFill([ + 'scan_status' => ScanStatus::Released, + 'released_by' => $actor->id, + 'released_at' => now(), + ])->save(); + + $this->activity->log(Action::FileReleased, subject: $file, context: [ + 'reason' => $validated['reason'], + 'threat' => $file->scan_note, + ]); + + // Everything that was waiting on this file — a share email, a new + // version notice — goes out now, exactly as it would have if the + // scan had passed. + $this->availability->markAvailable($file); + + return back()->with('success', __('The file has been released.')); + } +} diff --git a/app/Modules/Files/Models/File.php b/app/Modules/Files/Models/File.php index 878b0abf..9fb0caaf 100644 --- a/app/Modules/Files/Models/File.php +++ b/app/Modules/Files/Models/File.php @@ -46,6 +46,7 @@ use Illuminate\Support\Carbon; * @property Carbon|null $scanned_at * @property string|null $scan_engine * @property int $scan_attempts + * @property bool $scan_was_available * @property int|null $released_by * @property Carbon|null $released_at * @property bool $public @@ -89,6 +90,7 @@ class File extends Model 'scanned_at' => 'datetime', 'released_at' => 'datetime', 'scan_attempts' => 'integer', + 'scan_was_available' => 'boolean', 'commentable' => 'boolean', 'expires_at' => 'datetime', 'download_limit' => 'integer', diff --git a/app/Modules/Files/Scanning/QuarantineNotifier.php b/app/Modules/Files/Scanning/QuarantineNotifier.php new file mode 100644 index 00000000..a06ccc79 --- /dev/null +++ b/app/Modules/Files/Scanning/QuarantineNotifier.php @@ -0,0 +1,71 @@ +uploader; + + $this->notifier->send('file_quarantined', $this->staff(), subject: $file, data: [ + 'itemName' => $file->name, + 'uploaderName' => $uploader->name ?? __('a deleted account'), + 'threat' => $threat, + ]); + + // The uploader hears it once. Without this check a staff member + // who uploaded an infected file would get both messages, which + // read as two different files. + if ($uploader !== null && ! $this->staff()->contains(fn (User $staff): bool => $staff->is($uploader))) { + $this->notifier->send('upload_blocked', [$uploader], subject: $file, data: [ + 'itemName' => $file->name, + 'threat' => $threat, + ]); + } + } + + /** + * @return \Illuminate\Support\Collection + */ + private function staff(): \Illuminate\Support\Collection + { + return User::query() + ->where('type', UserType::Staff) + ->where('active', true) + ->get() + ->filter(fn (User $staff): bool => $this->permissions->allows($staff, Permission::ReleaseQuarantinedFiles)) + ->values(); + } +} diff --git a/app/Modules/Files/Scanning/ScanPolicy.php b/app/Modules/Files/Scanning/ScanPolicy.php index c7ecc4d1..1e542162 100644 --- a/app/Modules/Files/Scanning/ScanPolicy.php +++ b/app/Modules/Files/Scanning/ScanPolicy.php @@ -30,6 +30,7 @@ class ScanPolicy private readonly ScanningConfig $config, private readonly FileAvailability $availability, private readonly ActivityLogger $activity, + private readonly QuarantineNotifier $notifier, ) {} /** @@ -73,6 +74,8 @@ class ScanPolicy { $wasAvailable = $this->availability->isAvailable($file); + $file->forceFill(['scan_was_available' => $wasAvailable])->save(); + $this->settle($file, ScanStatus::Infected, $threat, $engine); $this->purgeRenditions($file); @@ -87,6 +90,8 @@ class ScanPolicy 'was_available' => $wasAvailable, ]); + $this->notifier->quarantined($file, $threat); + return ScanStatus::Infected; } @@ -104,6 +109,8 @@ class ScanPolicy 'was_available' => false, ]); + $this->notifier->quarantined($file, $reason->label()); + return ScanStatus::UnscannableBlocked; } diff --git a/app/Modules/Identity/Permissions/Permission.php b/app/Modules/Identity/Permissions/Permission.php index c70e6a30..60c60a44 100644 --- a/app/Modules/Identity/Permissions/Permission.php +++ b/app/Modules/Identity/Permissions/Permission.php @@ -35,6 +35,13 @@ enum Permission: string // rather than a key nobody can reach. case ModerateComments = 'moderate_comments'; + // Overrule the virus scanner: let a quarantined file out. Its own key + // rather than riding on delete_files, because deciding that a threat + // report is wrong is a different judgement from deciding a file is no + // longer needed — and only the administrator role holds it by + // default. See docs/feature-virus-scanning.md. + case ReleaseQuarantinedFiles = 'release_quarantined_files'; + // Categories case CreateCategories = 'create_categories'; case EditCategories = 'edit_categories'; @@ -95,6 +102,7 @@ enum Permission: string self::ImportOrphans => 'Import orphan files', self::LimitDownloads => 'Limit download counts', self::ModerateComments => 'Moderate comments', + self::ReleaseQuarantinedFiles => 'Release quarantined files', self::CreateCategories => 'Create categories', self::EditCategories => 'Edit categories', self::DeleteCategories => 'Delete categories', @@ -186,6 +194,7 @@ enum Permission: string self::ImportOrphans, self::LimitDownloads, self::ModerateComments => PermissionCategory::Files, + self::ReleaseQuarantinedFiles => PermissionCategory::Files, self::CreateCategories, self::EditCategories, diff --git a/app/Modules/Platform/Http/Controllers/SchedulerMonitoringController.php b/app/Modules/Platform/Http/Controllers/SchedulerMonitoringController.php index d603092a..fbc1d4d2 100644 --- a/app/Modules/Platform/Http/Controllers/SchedulerMonitoringController.php +++ b/app/Modules/Platform/Http/Controllers/SchedulerMonitoringController.php @@ -56,6 +56,7 @@ class SchedulerMonitoringController extends Controller 'projectsend:purge-zip-downloads' => (string) __('Purge zip downloads'), 'projectsend:check-for-updates' => (string) __('Check for updates'), 'projectsend:fetch-news' => (string) __('Fetch dashboard news'), + 'projectsend:scan-files' => (string) __('Scan files for viruses'), 'projectsend:purge-expired-files' => (string) __('Purge expired files'), 'projectsend:purge-orphan-files' => (string) __('Purge orphan files'), 'projectsend:purge-api-request-logs' => (string) __('Purge API request logs'), diff --git a/database/migrations/2026_09_16_090000_add_scan_columns_to_files_table.php b/database/migrations/2026_09_16_090000_add_scan_columns_to_files_table.php index d8202347..117dd2ec 100644 --- a/database/migrations/2026_09_16_090000_add_scan_columns_to_files_table.php +++ b/database/migrations/2026_09_16_090000_add_scan_columns_to_files_table.php @@ -31,6 +31,14 @@ return new class extends Migration $table->string('scan_engine')->nullable()->after('scanned_at'); $table->unsignedInteger('scan_attempts')->default(0)->after('scan_engine'); + // Whether this file could be downloaded before it was + // quarantined — true only for one that went out unscanned + // while the scanner was unreachable and was caught later. + // Recorded on the file because it changes what an + // administrator has to do, and because reconstructing it from + // the activity log afterwards means reading every entry. + $table->boolean('scan_was_available')->default(false)->after('scan_attempts'); + // Who overruled a quarantine, and when. The reason they gave // is in the activity log; this is what the file itself shows. $table->foreignId('released_by')->nullable()->after('scan_attempts')->constrained('users')->nullOnDelete(); @@ -43,7 +51,7 @@ return new class extends Migration Schema::table('files', function (Blueprint $table) { $table->dropConstrainedForeignId('released_by'); $table->dropIndex(['scan_status']); - $table->dropColumn(['scan_status', 'scan_note', 'scanned_at', 'scan_engine', 'scan_attempts', 'released_at']); + $table->dropColumn(['scan_status', 'scan_note', 'scanned_at', 'scan_engine', 'scan_attempts', 'scan_was_available', 'released_at']); }); } }; diff --git a/resources/js/components/app-sidebar.tsx b/resources/js/components/app-sidebar.tsx index a570cbe1..d769292d 100644 --- a/resources/js/components/app-sidebar.tsx +++ b/resources/js/components/app-sidebar.tsx @@ -5,30 +5,7 @@ import { Sidebar, SidebarContent, SidebarFooter, SidebarHeader, SidebarMenu, Sid import { useTranslation } from '@/hooks/use-translation'; import { type NavGroup, type SharedData } from '@/types'; import { Link, usePage } from '@inertiajs/react'; -import { - Activity, - ArrowLeftRight, - BookOpen, - Boxes, - Contact, - Download, - FileCode, - FileText, - FileWarning, - History, - KeyRound, - LayoutGrid, - ListChecks, - MailPlus, - MessageSquare, - Settings, - ShieldCheck, - Tags, - Upload, - UserCheck, - UserPlus, - Users, -} from 'lucide-react'; +import { Activity, ArrowLeftRight, BookOpen, Boxes, Contact, Download, FileCode, FileText, FileWarning, History, KeyRound, LayoutGrid, ListChecks, MailPlus, MessageSquare, Settings, ShieldAlert, ShieldCheck, Tags, Upload, UserCheck, UserPlus, Users } from 'lucide-react'; import AppLogo from './app-logo'; export function AppSidebar() { @@ -87,6 +64,9 @@ export function AppSidebar() { if (can('import_orphans')) { fileItems.push({ title: t('Import orphan files'), url: '/files/orphans', icon: FileWarning }); } + if (can('release_quarantined_files')) { + fileItems.push({ title: t('Quarantine'), url: '/files/quarantine', icon: ShieldAlert }); + } if (can('moderate_comments')) { // Just "Comments" — the old "Comments awaiting approval" wrapped // and pushed its own count badge out of the sidebar, and the screen diff --git a/resources/js/pages/files/quarantine.tsx b/resources/js/pages/files/quarantine.tsx new file mode 100644 index 00000000..4502cd71 --- /dev/null +++ b/resources/js/pages/files/quarantine.tsx @@ -0,0 +1,166 @@ +import { type BreadcrumbItem } from '@/types'; +import { Head, useForm } from '@inertiajs/react'; +import { ShieldAlert } from 'lucide-react'; +import { useState } from 'react'; + +import Heading from '@/components/heading'; +import InputError from '@/components/input-error'; +import { Pagination, PaginationMeta } from '@/components/pagination'; +import { TableShell } from '@/components/table-shell'; +import { Alert, AlertDescription } from '@/components/ui/alert'; +import { Badge } from '@/components/ui/badge'; +import { Button } from '@/components/ui/button'; +import { + Dialog, + DialogContent, + DialogDescription, + DialogFooter, + DialogHeader, + DialogTitle, + DialogTrigger, +} from '@/components/ui/dialog'; +import { Label } from '@/components/ui/label'; +import { Textarea } from '@/components/ui/textarea'; +import { useFormatDate } from '@/hooks/use-format-date'; +import { useTranslation } from '@/hooks/use-translation'; +import AppLayout from '@/layouts/app-layout'; +import { formatBytes } from '@/lib/format-bytes'; + +interface QuarantinedFile { + id: number; + name: string; + original_name: string; + size: number; + uploader: string | null; + threat: string | null; + status: string; + scanned_at: string | null; + /** It could be downloaded before it was flagged — so somebody may already have it. */ + was_available: boolean; + downloads_count: number; +} + +interface QuarantineProps { + files: QuarantinedFile[]; + pagination: PaginationMeta; +} + +function ReleaseDialog({ file }: { file: QuarantinedFile }) { + const { t } = useTranslation(); + const [open, setOpen] = useState(false); + const { data, setData, post, processing, errors, reset } = useForm({ reason: '' }); + + return ( + { + setOpen(next); + if (!next) reset(); + }} + > + + + + + + {t('Release ":name"?', { name: file.name })} + + {t( + 'The scanner reported a threat in this file. Releasing it makes it downloadable again for everyone it was shared with. Only do this if you are sure the report is wrong.', + )} + + + +
+ +