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