From d11bda094b2552dc2bde0649d696f35f962d6be9 Mon Sep 17 00:00:00 2001 From: ignacionelson Date: Wed, 16 Sep 2026 14:39:06 -0300 Subject: [PATCH] Say out loud when scanning has quietly stopped protecting anything MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The defaults let files through when the scanner cannot answer, so an installation whose scanner died looks, from every screen anybody uses, exactly like one that is working. Three places now say otherwise. `projectsend:status` gains a `scanning` block: whether it is on, whether it is managed, whether the scanner answers right now, the engine and how old its definitions are, what is waiting, what is quarantined, and how many files went out unscanned in the last 24 hours. Absent, null and zero stay distinct — `reachable: null` means there is nothing to reach, `false` means it should be answering and is not. The scans queue is reported beside the other two. The dashboard's System card carries the same warning for whoever is actually looking at a screen, and says nothing at all while scanning is healthy or switched off. Docker gets the scanner as an opt-in profile — `--profile scanner` — in both the development compose file and the published example, with a clamd.conf whose Alert* options are what make an encrypted archive come back as "could not scan" instead of "OK". No published ports: clamd has no authentication and the file crosses that socket in the clear. Both images also run a worker for the scans queue. The dashboard test caught a 500 before it shipped: a nullable return written as `array`. --- .env.example | 7 ++ .../Http/Controllers/DashboardController.php | 59 ++++++++++++- .../Installation/Console/StatusCommand.php | 82 +++++++++++++++++++ compose.yaml | 42 ++++++++++ docker/clamav/clamd.conf | 34 ++++++++ docker/production/compose.example.yaml | 19 +++++ docker/production/supervisord.conf | 15 ++++ .../dashboard-widgets/system-widget.tsx | 49 ++++++++++- .../Files/VirusScanningSettingsTest.php | 71 ++++++++++++++++ 9 files changed, 375 insertions(+), 3 deletions(-) create mode 100644 docker/clamav/clamd.conf 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), + ); +});