mirror of
https://github.com/projectsend/projectsend.git
synced 2026-09-20 02:23:20 +00:00
ignacionelson
c15c9c48f8
Run against the dev stack with real ClamAV and queue workers, and a code review looking for ways around the scanner. Quarantine now stays quarantined until somebody releases the file. A rescan only touches files people can download, and changes nothing when the scanner cannot answer or scanning is off. Before, an old infected file rescanned while clamd restarted went through the "allow" policy and became downloadable. The daily missing-files check leaves quarantined files alone, so a storage outage no longer brings one back as a fresh upload. A file longer than clamd's StreamMaxLength is "too large" again. clamd answers and hangs up; the next write raised a warning that became an exception before the answer was read, so the file was recorded as "scanner down" and retried past the unscannable policy. The production compose example gives clamd the settings it needs. On its own defaults an encrypted zip comes back clean. The Test button now sends a password-protected zip and fails when it is called clean, and says when an address answers but is not ClamAV. Saving the settings restarts the queue workers, which kept the old values in memory. New scan runs --all, as its name says, and is refused while scans are queued. A retry scheduled for later no longer counts as a scan in progress. Also: quarantine respects client scope for listing, release and notifications; a zip built before a file was quarantined is refused; public comments and version links skip unavailable files; a client no longer sees their own quarantined or missing upload; a file whose bytes return is scanned at once; clamd listens on IPv6 too, so its container health check passes.
83 lines
3.0 KiB
PHP
83 lines
3.0 KiB
PHP
<?php
|
|
|
|
declare(strict_types=1);
|
|
|
|
namespace App\Modules\Files\Scanning;
|
|
|
|
use App\Models\User;
|
|
use App\Modules\Files\Access\StaffLibraryScope;
|
|
use App\Modules\Files\Models\File;
|
|
use App\Modules\Identity\Permissions\Permission;
|
|
use App\Modules\Identity\Permissions\PermissionChecker;
|
|
use App\Modules\Identity\UserType;
|
|
use App\Modules\Notifications\Notifier;
|
|
|
|
/**
|
|
* Who hears about a quarantined file.
|
|
*
|
|
* Two audiences, deliberately not three. Staff who can do something about
|
|
* it are told, because a file sitting in quarantine that nobody looks at
|
|
* is the same as a file silently lost. The person who uploaded it is
|
|
* told, because on an honest account this is how they find out their own
|
|
* machine has something on it — and because otherwise their file simply
|
|
* never arrives and they have no idea why.
|
|
*
|
|
* The people the file was shared with are **not** told. They never
|
|
* received it, and a message about a virus in a file they never saw
|
|
* would alarm without informing.
|
|
*
|
|
* Recipients are resolved here rather than inside Notifier, which
|
|
* authorizes nothing by design — see its security contract.
|
|
*/
|
|
class QuarantineNotifier
|
|
{
|
|
public function __construct(
|
|
private readonly Notifier $notifier,
|
|
private readonly PermissionChecker $permissions,
|
|
private readonly StaffLibraryScope $scope,
|
|
) {}
|
|
|
|
public function quarantined(File $file, string $threat): void
|
|
{
|
|
$uploader = $file->uploader;
|
|
$staff = $this->staff($file);
|
|
|
|
$this->notifier->send('file_quarantined', $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 && ! $staff->contains(fn (User $member): bool => $member->is($uploader))) {
|
|
$this->notifier->send('upload_blocked', [$uploader], subject: $file, data: [
|
|
'itemName' => $file->name,
|
|
'threat' => $threat,
|
|
]);
|
|
}
|
|
}
|
|
|
|
/**
|
|
* Staff who can release this file — the permission, and a client
|
|
* scope that reaches its uploader (see QuarantineController).
|
|
*
|
|
* @return \Illuminate\Support\Collection<int, User>
|
|
*/
|
|
private function staff(File $file): \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))
|
|
->filter(function (User $staff) use ($file): bool {
|
|
$uploaders = $this->scope->uploaderIds($staff);
|
|
|
|
return $uploaders === null || in_array($file->uploaded_by, $uploaders, true);
|
|
})
|
|
->values();
|
|
}
|
|
}
|