diff --git a/.env.example b/.env.example index 98b74657..73f5b20b 100644 --- a/.env.example +++ b/.env.example @@ -20,6 +20,13 @@ PROJECTSEND_EDITION=community # stream deliberately. The dashboard's System panel shows which is in use. # PROJECTSEND_FILE_DELIVERY=auto +# Optional: the virus scanner every upload is checked against, as +# tcp://host:3310 or unix:///path/to/clamd.sock. Naming it here makes +# scanning managed: it is used, it cannot be switched off from the settings +# screen, and the address does not appear there. Leave it unset to +# configure scanning in Settings instead, which is the ordinary way. +# PROJECTSEND_SCANNER_ADDRESS=tcp://clamav:3310 + # Optional: uid/gid the app/web containers' internal user runs as, so the # bind-mounted repo needs no permission fixes. Defaults to 1000; override # if your host user's `id -u`/`id -g` differ. diff --git a/app/Modules/Audit/Http/Controllers/DashboardController.php b/app/Modules/Audit/Http/Controllers/DashboardController.php index a620debe..92ca259e 100644 --- a/app/Modules/Audit/Http/Controllers/DashboardController.php +++ b/app/Modules/Audit/Http/Controllers/DashboardController.php @@ -17,6 +17,9 @@ use App\Modules\Clients\ClientStorageUsage; use App\Modules\Files\Access\StaffLibraryScope; use App\Modules\Files\Delivery\FileDelivery; use App\Modules\Files\Models\File; +use App\Modules\Files\Scanning\ScanningConfig; +use App\Modules\Files\Scanning\ScanStatus; +use App\Modules\Files\Scanning\VirusScanner; use App\Modules\Groups\Models\Group; use App\Modules\Identity\UserType; use App\Modules\Platform\Capabilities\Capability; @@ -480,7 +483,7 @@ class DashboardController extends Controller } /** - * @return array|bool|int|string|null> + * @return array|bool|int|string|null> */ private function systemInfo(): array { @@ -511,6 +514,60 @@ class DashboardController extends Controller // able to confirm at a glance, not only worth warning about // when it is false — the same reasoning as storage_durability. 'file_delivery' => $this->fileDelivery->describe(), + // Null when scanning is off, so the card says nothing about a + // feature this installation does not use. When it is on, this + // is the only place an administrator finds out that the + // scanner stopped answering — every other screen looks exactly + // as it did, because files keep flowing by design. + 'scanning' => $this->scanningWarning(), + ]; + } + + /** + * What is wrong with virus scanning right now, or null. + * + * Deliberately only the bad news. The settings screen reports the + * healthy state; the dashboard exists here to interrupt somebody who + * was not looking for it. + * + * @return array{reachable: bool, engine: string|null, definitions_age_hours: int|null, let_through_24h: int, pending: int}|null + */ + private function scanningWarning(): ?array + { + $config = app(ScanningConfig::class); + + if (! $config->enabled()) { + return null; + } + + $scanner = app(VirusScanner::class)->status(); + + $letThrough = ActivityLog::query() + ->where('action', Action::FileNotScanned) + ->where('created_at', '>=', now()->subDay()) + ->count(); + + $pending = File::query() + ->where('scan_status', ScanStatus::Pending) + ->where('created_at', '<=', now()->subHour()) + ->count(); + + $stale = $scanner->definitionsAgeHours(); + + // Nothing to say when the scanner is there, current, and nothing + // has gone out unchecked. + if ($scanner->reachable && $letThrough === 0 && $pending === 0 && ($stale === null || $stale < 72)) { + return null; + } + + return [ + 'reachable' => $scanner->reachable, + 'engine' => $scanner->engine, + 'definitions_age_hours' => $stale, + 'let_through_24h' => $letThrough, + // Files that have been waiting more than an hour: on an + // installation set to hold, this is what an outage looks like. + 'pending' => $pending, ]; } diff --git a/app/Modules/Platform/Installation/Console/StatusCommand.php b/app/Modules/Platform/Installation/Console/StatusCommand.php index 5b723772..852454b8 100644 --- a/app/Modules/Platform/Installation/Console/StatusCommand.php +++ b/app/Modules/Platform/Installation/Console/StatusCommand.php @@ -8,6 +8,9 @@ use App\Modules\Audit\Action; use App\Modules\Audit\ActivityLog; use App\Modules\Clients\ClientStorageUsage; use App\Modules\Files\Models\File; +use App\Modules\Files\Scanning\ScanningConfig; +use App\Modules\Files\Scanning\ScanStatus; +use App\Modules\Files\Scanning\VirusScanner; use App\Modules\Identity\TwoFactor\TwoFactorEnforcement; use App\Modules\Identity\UserType; use App\Modules\Platform\Capabilities\CapabilityRegistry; @@ -269,6 +272,7 @@ class StatusCommand extends Command ], 'storage' => $this->storage(), 'usage' => $this->usage(), + 'scanning' => $this->scanning(), 'health' => $this->health(), 'settings' => [ // Echoed back rather than assumed: an operator writes the @@ -333,6 +337,7 @@ class StatusCommand extends Command $this->line('Health: '.$status['health']['pending_migrations'].' migrations pending, ' .$status['health']['failed_jobs'].' failed jobs, ' .array_sum(array_filter($status['health']['queues'], 'is_int')).' queued'); + $this->line('Scanning: '.$this->scanningSummary($status['scanning'])); $this->line('Scheduler: '.($status['health']['scheduler']['last_run_at'] ?? 'never run') .' ('.$status['health']['scheduler']['failing'].' failing)'); $this->line('Last '.self::USAGE_WINDOW_DAYS.'d: ' @@ -392,6 +397,82 @@ class StatusCommand extends Command /** * @return array{pending_migrations: int, failed_jobs: int, failed_jobs_latest_at: string|null, queues: array, scheduler: array{last_run_at: string|null, failing: int}} */ + /** + * Whether this installation is actually checking what it accepts. + * + * The block exists because the honest answer to "are you protected?" + * is not a boolean. Scanning can be on, the scanner unreachable, and + * every upload sailing through marked "not scanned" — which is the + * configured behaviour and looks, from every screen a customer sees, + * exactly like a working installation. `let_through_24h` is the + * number that gives that away, and `reachable` is measured now rather + * than remembered. + * + * Absent, null and zero stay distinct here as everywhere else in this + * document: `enabled: false` is a decision, `reachable: null` is + * "nothing to reach because it is off", and `reachable: false` is a + * scanner that should be answering and is not. + * + * @return array + */ + private function scanning(): array + { + $config = app(ScanningConfig::class); + + if (! $config->enabled()) { + return [ + 'enabled' => false, + 'managed' => $config->isManaged(), + 'reachable' => null, + 'engine' => null, + 'definitions_age_hours' => null, + 'pending' => 0, + 'quarantined' => 0, + 'let_through_24h' => 0, + ]; + } + + $scanner = app(VirusScanner::class)->status(); + + return [ + 'enabled' => true, + 'managed' => $config->isManaged(), + 'reachable' => $scanner->reachable, + 'engine' => $scanner->engine, + 'definitions_age_hours' => $scanner->definitionsAgeHours(), + 'pending' => File::query()->where('scan_status', ScanStatus::Pending)->count(), + 'quarantined' => File::query()->whereIn('scan_status', [ + ScanStatus::Infected->value, + ScanStatus::UnscannableBlocked->value, + ])->count(), + // Files that went out unchecked in the last day. Zero is the + // only number that means "protected"; anything else is a + // scanner that was down, or files nobody could open. + 'let_through_24h' => ActivityLog::query() + ->where('action', Action::FileNotScanned) + ->where('created_at', '>=', now()->subDay()) + ->count(), + ]; + } + + /** + * @param array $scanning + */ + private function scanningSummary(array $scanning): string + { + if ($scanning['enabled'] !== true) { + return 'off'; + } + + $reachable = $scanning['reachable'] === true ? 'reachable' : 'UNREACHABLE'; + + return "{$reachable}, {$scanning['pending']} waiting, {$scanning['quarantined']} quarantined, " + ."{$scanning['let_through_24h']} let through in 24h"; + } + + /** + * @return array + */ private function health(): array { return [ @@ -405,6 +486,7 @@ class StatusCommand extends Command 'queues' => [ 'default' => $this->queueDepth('default'), 'zips' => $this->queueDepth('zips'), + 'scans' => $this->queueDepth('scans'), ], 'scheduler' => $this->scheduler(), ]; diff --git a/compose.yaml b/compose.yaml index 8fae83e5..129c6389 100644 --- a/compose.yaml +++ b/compose.yaml @@ -91,6 +91,27 @@ services: redis: condition: service_started + # Scans get their own worker for the reason zips do: reading a 5 GB file + # to the scanner takes minutes, and on the default queue it would sit in + # front of every notification email. + worker-scans: + build: + context: . + dockerfile: docker/app/Dockerfile + args: + WWWUSER: ${WWWUSER:-1000} + WWWGROUP: ${WWWGROUP:-1000} + command: php artisan queue:work --queue=scans --tries=1 + volumes: + - .:/var/www/html + - ../packages:/var/www/packages + restart: unless-stopped + depends_on: + db: + condition: service_healthy + redis: + condition: service_started + scheduler: build: context: . @@ -169,6 +190,27 @@ services: profiles: - dev + # The virus scanner, off unless you ask for it: + # docker compose --profile scanner up -d + # then point Settings → Virus scanning at tcp://clamav:3310. + # + # It costs about 1-1.5 GB of memory, because the virus definitions are + # held in memory, and the first start downloads them before it answers. + # That is why this is a profile rather than a service everybody runs. + clamav: + image: clamav/clamav:stable + # No ports. clamd has no authentication and no encryption of any kind, + # so anything that can reach it can use it, and file contents cross + # that connection in the clear. It is reachable from the application + # on this network and from nowhere else. + volumes: + - clamav-data:/var/lib/clamav + - ./docker/clamav/clamd.conf:/etc/clamav/clamd.conf:ro + restart: unless-stopped + profiles: + - scanner + volumes: db-data: redis-data: + clamav-data: diff --git a/docker/clamav/clamd.conf b/docker/clamav/clamd.conf new file mode 100644 index 00000000..7f4a838a --- /dev/null +++ b/docker/clamav/clamd.conf @@ -0,0 +1,34 @@ +# clamd settings ProjectSend depends on. +# +# The three Alert* options are not cosmetic. Without them clamd answers +# "OK" for a file it could not actually look inside — an encrypted archive, +# or one that hit a size limit — and ProjectSend would record a clean scan +# for a file nobody scanned. With them, those come back as +# Heuristics.Encrypted.* and Heuristics.Limits.Exceeded.*, which the client +# maps to "encrypted" and "too large" rather than to a threat, and this +# installation's own policy decides what happens to the file. + +LogTime yes +Foreground yes + +# Listen for the application on this network only. There is no +# authentication in this protocol; see the note in compose.yaml. +TCPSocket 3310 +TCPAddr 0.0.0.0 + +# The largest stream clamd will accept. ProjectSend refuses anything over +# its own "largest file to scan" setting before it gets here, and that +# setting must not exceed this number. +StreamMaxLength 512M +MaxFileSize 512M +MaxScanSize 1024M + +# Archive limits, which is where zip bombs live. Reaching one is reported +# rather than passed, thanks to AlertExceedsMax below. +MaxRecursion 16 +MaxFiles 10000 + +AlertExceedsMax yes +AlertEncrypted yes +AlertEncryptedArchive yes +AlertEncryptedDoc yes diff --git a/docker/production/compose.example.yaml b/docker/production/compose.example.yaml index c1abb213..4b1bb123 100644 --- a/docker/production/compose.example.yaml +++ b/docker/production/compose.example.yaml @@ -116,7 +116,26 @@ services: volumes: - redis-data:/data + # Virus scanning, off unless you ask for it: + # docker compose --profile scanner up -d + # then point Settings → Virus scanning at tcp://clamav:3310. + # + # Budget about 1-1.5 GB of memory: the virus definitions are held in + # memory. The first start downloads them and does not answer until it + # has, which the Test button on that screen reports plainly. + clamav: + image: clamav/clamav:stable + restart: unless-stopped + # Deliberately no ports. clamd has no authentication and no + # encryption, so anything that can reach it can use it, and files + # cross that connection in the clear. + volumes: + - clamav-data:/var/lib/clamav + profiles: + - scanner + volumes: storage: db-data: redis-data: + clamav-data: diff --git a/docker/production/supervisord.conf b/docker/production/supervisord.conf index 62f56106..36c257b4 100644 --- a/docker/production/supervisord.conf +++ b/docker/production/supervisord.conf @@ -61,6 +61,21 @@ stdout_logfile_maxbytes=0 stderr_logfile=/dev/stderr stderr_logfile_maxbytes=0 +; Virus scans get their own worker, for the reason zips do: reading a 5 GB +; file to the scanner takes minutes, and on the shared queue it would sit +; in front of every notification email. Harmless on an installation with no +; scanner configured — the queue is simply empty. +[program:queue-scans] +command=su-exec www-data php /var/www/html/artisan queue:work --queue=scans --max-time=3600 --tries=1 +autostart=true +autorestart=true +priority=32 +stopwaitsecs=3630 +stdout_logfile=/dev/stdout +stdout_logfile_maxbytes=0 +stderr_logfile=/dev/stderr +stderr_logfile_maxbytes=0 + ; schedule:work is the long-running equivalent of a per-minute cron entry, ; which is what a container should use — there is no crond here. [program:scheduler] diff --git a/resources/js/components/dashboard-widgets/system-widget.tsx b/resources/js/components/dashboard-widgets/system-widget.tsx index 08a404a9..7093125f 100644 --- a/resources/js/components/dashboard-widgets/system-widget.tsx +++ b/resources/js/components/dashboard-widgets/system-widget.tsx @@ -1,6 +1,6 @@ import { type SharedData } from '@/types'; -import { usePage } from '@inertiajs/react'; -import { AlertTriangle, ArrowUpCircle, HardDrive } from 'lucide-react'; +import { Link, usePage } from '@inertiajs/react'; +import { AlertTriangle, ArrowUpCircle, HardDrive, ShieldAlert } from 'lucide-react'; import { useState } from 'react'; import { Alert, AlertDescription, AlertTitle } from '@/components/ui/alert'; @@ -33,6 +33,20 @@ export interface SystemInfo { install_kind: InstallKind; /** How downloads leave the server — see FileDeliveryDialog. */ file_delivery: FileDelivery; + /** + * Only ever present when something is wrong with virus scanning, and + * null when scanning is off. A scanner that has stopped answering + * looks, from every other screen, exactly like one that is working: + * uploads keep arriving and downloads keep working, because that is + * the configured behaviour. This is where that gets said out loud. + */ + scanning: { + reachable: boolean; + engine: string | null; + definitions_age_hours: number | null; + let_through_24h: number; + pending: number; + } | null; } /** @@ -115,6 +129,37 @@ export function SystemWidget({ system, onViewReleaseNotes }: { system: SystemInf {/* Before the update notice on purpose: losing the files outranks being a version behind. */} {durability && } + {system.scanning && ( + + + + {system.scanning.reachable ? t('Files are going out unscanned') : t('The virus scanner is not answering')} + + +
    + {!system.scanning.reachable &&
  • {t('Uploads cannot be checked until it is back.')}
    • } + {system.scanning.let_through_24h > 0 && ( +
  • + {t(':count files were allowed through without being scanned in the last 24 hours.', { + count: system.scanning.let_through_24h, + })} +
    • + )} + {system.scanning.pending > 0 && ( +
  • {t(':count files have been waiting to be checked for over an hour.', { count: system.scanning.pending })}
    • + )} + {system.scanning.definitions_age_hours !== null && system.scanning.definitions_age_hours >= 72 && ( +
  • + {t('The virus definitions are :hours hours old.', { hours: system.scanning.definitions_age_hours })} +
    • + )} +
+ + {t('Virus scanning settings')} + + + + )} {system.update_available && ( diff --git a/tests/Feature/Files/VirusScanningSettingsTest.php b/tests/Feature/Files/VirusScanningSettingsTest.php index e51c8342..a9eb7323 100644 --- a/tests/Feature/Files/VirusScanningSettingsTest.php +++ b/tests/Feature/Files/VirusScanningSettingsTest.php @@ -23,6 +23,13 @@ beforeEach(function () { app(Settings::class)->set(Setting::VirusScannerDownPolicy, 'allow'); config()->set('projectsend.scanning.address', null); + + // The dashboard test below lands on the greeting instead of the + // dashboard otherwise — and settings survive RefreshDatabase's + // rollback in the cache, so both markers are stated rather than + // assumed. + app(Settings::class)->set(Setting::GettingStartedPending, false); + app(Settings::class)->set(Setting::UpdateWelcomeTo, ''); }); test('the screen shows what is configured and what is outstanding', function () { @@ -143,3 +150,67 @@ test('only somebody who can edit settings may test or save', function () { $this->actingAs($staff)->get('/system/settings/virus-scanning')->assertForbidden(); $this->actingAs($staff)->post('/system/settings/virus-scanning/test')->assertForbidden(); }); + +/* +|-------------------------------------------------------------------------- +| Saying so where nobody is looking +|-------------------------------------------------------------------------- +*/ + +/** The scanning block of the status document, as the fleet probe reads it. */ +function scanningStatus(): array +{ + Illuminate\Support\Facades\Artisan::call('projectsend:status', ['--json' => true]); + + /** @var array $document */ + $document = json_decode(Illuminate\Support\Facades\Artisan::output(), true); + + return $document['scanning']; +} + +test('the status command reports scanning as off when it is off', function () { + $this->artisan('projectsend:status')->assertSuccessful(); + + // statusJson() lives in StatusCommandTest — Pest loads every test + // file into one process, so a second copy here would be a redeclare. + $status = scanningStatus(); + + expect($status['enabled'])->toBeFalse() + // Null, not false: there is nothing to reach. A watcher must be + // able to tell that from a scanner that should answer and does not. + ->and($status['reachable'])->toBeNull(); +}); + +test('the status command reports an unreachable scanner and what got through', function () { + app(Settings::class)->set(Setting::VirusScanningEnabled, true); + app(Settings::class)->set(Setting::VirusScannerAddress, 'tcp://nowhere.test:3310'); + app()->instance(VirusScanner::class, (new FakeVirusScanner)->reports(ScannerStatus::unreachable('no answer'))); + + app(App\Modules\Audit\ActivityLogger::class)->logSystem(App\Modules\Audit\Action::FileNotScanned, [ + 'id' => 1, 'name' => 'x', 'reason' => 'scanner_unavailable', + ]); + + // statusJson() lives in StatusCommandTest — Pest loads every test + // file into one process, so a second copy here would be a redeclare. + $status = scanningStatus(); + + expect($status['enabled'])->toBeTrue() + ->and($status['reachable'])->toBeFalse() + ->and($status['let_through_24h'])->toBe(1); +}); + +test('the dashboard says nothing while scanning is healthy, and speaks up when it is not', function () { + app(Settings::class)->set(Setting::VirusScanningEnabled, true); + app(Settings::class)->set(Setting::VirusScannerAddress, 'tcp://scanner.test:3310'); + app()->instance(VirusScanner::class, new FakeVirusScanner); + + $this->actingAs($this->admin)->get('/dashboard')->assertInertia( + fn (AssertableInertia $page) => $page->where('system.scanning', null), + ); + + app()->instance(VirusScanner::class, (new FakeVirusScanner)->reports(ScannerStatus::unreachable('no answer'))); + + $this->actingAs($this->admin)->get('/dashboard')->assertInertia( + fn (AssertableInertia $page) => $page->where('system.scanning.reachable', false), + ); +});