diff --git a/.env.example b/.env.example index 73f5b20b..8207967c 100644 --- a/.env.example +++ b/.env.example @@ -27,6 +27,13 @@ PROJECTSEND_EDITION=community # configure scanning in Settings instead, which is the ordinary way. # PROJECTSEND_SCANNER_ADDRESS=tcp://clamav:3310 +# Optional: the scanner a fresh installation starts out pointed at, written +# into the settings on first boot and ignored on every later one. Unlike the +# variable above it leaves both the address and the switch on the settings +# screen, which is what a self-hosted install wants: configured out of the +# box, and still yours. +# PROJECTSEND_SCANNER_DEFAULT_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/DOCKER.md b/DOCKER.md index d808b90d..0830f725 100644 --- a/DOCKER.md +++ b/DOCKER.md @@ -372,7 +372,10 @@ docker compose --profile scanner up -d ``` Then go to **System → Settings → Virus scanning**, switch it on, and use `tcp://clamav:3310` as the -address. **Test scanner** sends a harmless standard test file and tells you whether it was actually +address. On a brand-new installation you can skip that step: uncomment +`PROJECTSEND_SCANNER_DEFAULT_ADDRESS` in the compose file before the first start and the site comes +up already pointed at the scanner. It is a starting value, not a lock — the address and the switch +stay on that screen. **Test scanner** sends a harmless standard test file and tells you whether it was actually detected. Two things to know before you turn it on. It needs about **1–1.5 GB of memory**, because the virus diff --git a/app/Modules/Audit/Http/Controllers/DashboardController.php b/app/Modules/Audit/Http/Controllers/DashboardController.php index 92ca259e..494eb726 100644 --- a/app/Modules/Audit/Http/Controllers/DashboardController.php +++ b/app/Modules/Audit/Http/Controllers/DashboardController.php @@ -530,14 +530,30 @@ class DashboardController extends Controller * 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 + * @return array{configured: bool, 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; + // Nothing is checking what this installation accepts. Said + // only where somebody could act on it: an installation that + // connects its own scanner (community — see + // Capability::VirusScanningConnect). On a hosted one the + // scanner is the platform's to run, and a tenant reading + // "not configured" would be reading about somebody else's + // job. + return $this->capabilities->has(Capability::VirusScanningConnect) + ? [ + 'configured' => false, + 'reachable' => false, + 'engine' => null, + 'definitions_age_hours' => null, + 'let_through_24h' => 0, + 'pending' => 0, + ] + : null; } $scanner = app(VirusScanner::class)->status(); @@ -561,6 +577,7 @@ class DashboardController extends Controller } return [ + 'configured' => true, 'reachable' => $scanner->reachable, 'engine' => $scanner->engine, 'definitions_age_hours' => $stale, diff --git a/app/Modules/Files/Http/Controllers/VirusScanningSettingsController.php b/app/Modules/Files/Http/Controllers/VirusScanningSettingsController.php index 6979807a..b0ff7397 100644 --- a/app/Modules/Files/Http/Controllers/VirusScanningSettingsController.php +++ b/app/Modules/Files/Http/Controllers/VirusScanningSettingsController.php @@ -13,6 +13,8 @@ use App\Modules\Files\Scanning\ScanningConfig; use App\Modules\Files\Scanning\ScanOutcome; use App\Modules\Files\Scanning\ScanStatus; use App\Modules\Files\Scanning\VirusScanner; +use App\Modules\Platform\Capabilities\Capability; +use App\Modules\Platform\Capabilities\CapabilityRegistry; use App\Modules\Platform\Settings\Setting; use App\Modules\Platform\Settings\Settings; use Illuminate\Http\RedirectResponse; @@ -43,6 +45,7 @@ class VirusScanningSettingsController extends Controller private readonly Settings $settings, private readonly ScanningConfig $config, private readonly ActivityLogger $activity, + private readonly CapabilityRegistry $capabilities, ) {} public function edit(Request $request): Response @@ -60,7 +63,12 @@ class VirusScanningSettingsController extends Controller // nothing at all. Same shape the CAPTCHA screen uses. 'test_result' => $request->session()->get('scanner_test_result'), 'enabled' => $this->config->enabled(), - 'managed' => $this->config->isManaged(), + // Two different reasons the connection is not this screen's to + // change: a managed configuration names the scanner, or this + // edition does not connect scanners at all. The screen says + // the same thing for both, since to the person reading it + // they are the same fact. + 'managed' => $this->config->isManaged() || ! $this->canConnect(), 'address' => $this->config->isManaged() ? '' : $this->settings->get(Setting::VirusScannerAddress), 'max_size_mb' => $this->settings->get(Setting::VirusScanMaxSizeMb), 'unscannable_policy' => $this->settings->get(Setting::VirusUnscannablePolicy), @@ -86,7 +94,7 @@ class VirusScanningSettingsController extends Controller // A managed installation may still choose its policies. The // connection and the switch are not on the screen there, and a // request that sends them anyway changes nothing. - if (! $this->config->isManaged()) { + if (! $this->config->isManaged() && $this->canConnect()) { $address = trim((string) ($validated['address'] ?? '')); // Refused rather than saved and quietly inert: switching this @@ -123,6 +131,10 @@ class VirusScanningSettingsController extends Controller */ public function test(VirusScanner $scanner): RedirectResponse { + // Nothing to test where the connection is not this installation's + // to make. + abort_unless($this->canConnect(), 403); + $status = $scanner->status(); if (! $status->reachable) { @@ -177,6 +189,17 @@ class VirusScanningSettingsController extends Controller return back()->with('success', __('Scanning existing files has started. It runs in the background.')); } + /** + * Whether this installation connects its own scanner. + * + * Community only, through the registry rather than an edition check — + * see Capability::VirusScanningConnect for the division. + */ + private function canConnect(): bool + { + return $this->capabilities->has(Capability::VirusScanningConnect); + } + /** * @return array */ diff --git a/app/Modules/Platform/Capabilities/Capability.php b/app/Modules/Platform/Capabilities/Capability.php index f15d1d78..bcbde230 100644 --- a/app/Modules/Platform/Capabilities/Capability.php +++ b/app/Modules/Platform/Capabilities/Capability.php @@ -29,6 +29,16 @@ enum Capability: string // the bucket is provisioned, what goes in it is not. case UsersManage = 'users.manage'; case StorageConfigure = 'storage.configure'; + + // Connecting this installation to a virus scanner, and being told on + // the dashboard when it has none. Community only, and the division is + // the one managed storage already draws: on a hosted installation the + // scanner is infrastructure the platform runs, so its address is not a + // tenant's to set and its absence is not a tenant's to fix. What stays + // on both editions is what to *do* with a file nobody could scan — + // that is a decision about somebody's own files, not about + // infrastructure. See docs/feature-virus-scanning.md. + case VirusScanningConnect = 'scanning.connect'; case EmailTransportConfigure = 'email.transport.configure'; case SystemUpdates = 'system.updates'; @@ -166,6 +176,7 @@ enum Capability: string { return match ($this) { self::StorageConfigure, + self::VirusScanningConnect, self::EmailTransportConfigure, self::SystemUpdates, self::NewsConfigure, diff --git a/app/Modules/Platform/Settings/Console/SeedSettingsCommand.php b/app/Modules/Platform/Settings/Console/SeedSettingsCommand.php index 69e5244d..838fc99c 100644 --- a/app/Modules/Platform/Settings/Console/SeedSettingsCommand.php +++ b/app/Modules/Platform/Settings/Console/SeedSettingsCommand.php @@ -60,9 +60,43 @@ class SeedSettingsCommand extends Command $this->seedTwoFactorEnforcement($settings, $enforcement); } + $scanner = config('projectsend.scanning.default_address'); + + if (is_string($scanner) && trim($scanner) !== '') { + $this->seedScanner($settings, trim($scanner)); + } + return self::SUCCESS; } + /** + * Point a fresh installation at its scanner, and switch scanning on. + * + * For the operator who brings up the optional scanner container beside + * the application: without this they would have to find the settings + * screen and type an address the compose file already knows. Unlike + * PROJECTSEND_SCANNER_ADDRESS this leaves both the address and the + * switch editable afterwards — it is a starting value, not a policy. + * + * Both are seeded together or neither: an address with scanning off + * would look configured and check nothing, and scanning on with no + * address would hold every upload. + */ + private function seedScanner(Settings $settings, string $address): void + { + // The address, asked of the table for the reason given below: its + // default is the empty string, so get() cannot tell "never set" + // from "deliberately cleared". + if (StoredSetting::query()->where('key', Setting::VirusScannerAddress->value)->exists()) { + return; + } + + $settings->set(Setting::VirusScannerAddress, $address); + $settings->set(Setting::VirusScanningEnabled, true); + + $this->info("Virus scanning seeded to '{$address}' and switched on (first boot)."); + } + private function seedTwoFactorEnforcement(Settings $settings, string $value): void { if (TwoFactorEnforcement::tryFrom($value) === null) { diff --git a/config/projectsend.php b/config/projectsend.php index 985e9e93..d1f63018 100644 --- a/config/projectsend.php +++ b/config/projectsend.php @@ -201,6 +201,16 @@ return [ 'scanning' => [ 'address' => env('PROJECTSEND_SCANNER_ADDRESS'), + // The other way to point an installation at a scanner, and the + // opposite of the one above: written into the settings table on + // first boot and then owned by whoever administers the + // installation, who can change it or switch scanning off like any + // other setting. It is what a self-hosted operator who brings up + // the optional scanner container wants — the site arrives + // configured, without the platform taking the switch away. Ignored + // on any boot where the setting already has a value. + 'default_address' => env('PROJECTSEND_SCANNER_DEFAULT_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. diff --git a/docker/production/compose.example.yaml b/docker/production/compose.example.yaml index 4b1bb123..1d8b9a07 100644 --- a/docker/production/compose.example.yaml +++ b/docker/production/compose.example.yaml @@ -56,6 +56,13 @@ services: SESSION_DRIVER: redis QUEUE_CONNECTION: redis + # Uncomment together with the clamav service at the bottom of this + # file. It is written into the settings once, on first boot, so the + # site arrives configured — and it stays yours afterwards: the + # address and the switch are both on System → Settings → Virus + # scanning, and this line is ignored on every later boot. + # PROJECTSEND_SCANNER_DEFAULT_ADDRESS: tcp://clamav:3310 + # Mail is easier to configure from System → Settings → Email once you # are logged in — it has a "send test" button. These are the fallback # until then. diff --git a/resources/js/components/dashboard-widgets/system-widget.tsx b/resources/js/components/dashboard-widgets/system-widget.tsx index 7093125f..20cc57aa 100644 --- a/resources/js/components/dashboard-widgets/system-widget.tsx +++ b/resources/js/components/dashboard-widgets/system-widget.tsx @@ -41,6 +41,8 @@ export interface SystemInfo { * the configured behaviour. This is where that gets said out loud. */ scanning: { + /** False means no scanner is configured at all. */ + configured: boolean; reachable: boolean; engine: string | null; definitions_age_hours: number | null; @@ -133,11 +135,20 @@ export function SystemWidget({ system, onViewReleaseNotes }: { system: SystemInf - {system.scanning.reachable ? t('Files are going out unscanned') : t('The virus scanner is not answering')} + {!system.scanning.configured + ? t('Uploads are not being checked for viruses') + : 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.configured && ( +
  • {t('Anything uploaded here — by staff, by clients, or through an upload link — is passed on unchecked.')}
    • + )} + {system.scanning.configured && !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.', { @@ -155,7 +166,7 @@ export function SystemWidget({ system, onViewReleaseNotes }: { system: SystemInf )}
- {t('Virus scanning settings')} + {system.scanning.configured ? t('Virus scanning settings') : t('Set up virus scanning')} diff --git a/tests/Feature/Files/VirusScanningSettingsTest.php b/tests/Feature/Files/VirusScanningSettingsTest.php index 0a3e649d..9e43dace 100644 --- a/tests/Feature/Files/VirusScanningSettingsTest.php +++ b/tests/Feature/Files/VirusScanningSettingsTest.php @@ -243,3 +243,66 @@ test('the dashboard says nothing while scanning is healthy, and speaks up when i fn (AssertableInertia $page) => $page->where('system.scanning.reachable', false), ); }); + +/* +|-------------------------------------------------------------------------- +| Nothing is checking what this installation accepts +|-------------------------------------------------------------------------- +*/ + +test('an installation with no scanner is told so on the dashboard', function () { + $this->actingAs($this->admin)->get('/dashboard')->assertInertia( + fn (AssertableInertia $page) => $page + ->where('system.scanning.configured', false) + ->where('system.scanning.reachable', false), + ); +}); + +test('a hosted installation is not told: the scanner is not its job', function () { + // The capability, not an edition check — see + // Capability::VirusScanningConnect. The System card is community-only + // in its own right, so on a hosted installation the whole card is + // absent and the notice with it; the assertion below is about the + // card, and the one on the settings screen (further down) is what + // pins the capability itself. + config(['projectsend.edition' => App\Modules\Platform\Capabilities\Edition::Cloud]); + forgetRequestState(); + + $this->actingAs($this->admin)->get('/dashboard')->assertInertia( + fn (AssertableInertia $page) => $page->where('system', null), + ); +}); + +test('the notice goes away once a scanner is configured', 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), + ); +}); + +test('a hosted installation cannot connect a scanner of its own, but keeps its policies', function () { + config(['projectsend.edition' => App\Modules\Platform\Capabilities\Edition::Cloud]); + forgetRequestState(); + + $this->actingAs($this->admin)->get('/system/settings/virus-scanning')->assertInertia( + fn (AssertableInertia $page) => $page->where('managed', true), + ); + + $this->actingAs($this->admin)->post('/system/settings/virus-scanning/test')->assertForbidden(); + + $this->actingAs($this->admin)->patch('/system/settings/virus-scanning', [ + 'enabled' => true, + 'address' => 'tcp://mine:3310', + 'max_size_mb' => 256, + 'unscannable_policy' => 'block', + 'scanner_down_policy' => 'hold', + 'wait_minutes' => 10, + 'existing_rate_per_minute' => 60, + ])->assertSessionHasNoErrors(); + + expect(app(Settings::class)->get(Setting::VirusScannerAddress))->toBe('') + ->and(app(Settings::class)->get(Setting::VirusUnscannablePolicy))->toBe('block'); +}); diff --git a/tests/Feature/Platform/SeedSettingsTest.php b/tests/Feature/Platform/SeedSettingsTest.php index a368a88c..ef5907dd 100644 --- a/tests/Feature/Platform/SeedSettingsTest.php +++ b/tests/Feature/Platform/SeedSettingsTest.php @@ -95,3 +95,50 @@ test('the seeded policy is in force for the first account the same boot creates' expect(app(Settings::class)->get(Setting::TwoFactorEnforcement))->toBe('staff') ->and($admin->hasTwoFactorEnabled())->toBeFalse(); }); + +/* +|-------------------------------------------------------------------------- +| The virus scanner +|-------------------------------------------------------------------------- +| +| The opposite of PROJECTSEND_SCANNER_ADDRESS, which is a policy the +| platform keeps. This is a starting value for an operator who brought up +| the optional scanner container beside the application: it arrives +| configured, and stays theirs to change. +| +*/ + +test('a first boot points the installation at the scanner named in its environment', function () { + config(['projectsend.scanning.default_address' => 'tcp://clamav:3310']); + + $this->artisan('projectsend:seed-settings')->assertSuccessful(); + + $settings = app(App\Modules\Platform\Settings\Settings::class); + + expect($settings->get(App\Modules\Platform\Settings\Setting::VirusScannerAddress))->toBe('tcp://clamav:3310') + // Both together: an address with scanning off would look + // configured and check nothing. + ->and($settings->get(App\Modules\Platform\Settings\Setting::VirusScanningEnabled))->toBeTrue(); +}); + +test('it never argues with an administrator who has already chosen', function () { + $settings = app(App\Modules\Platform\Settings\Settings::class); + $settings->set(App\Modules\Platform\Settings\Setting::VirusScannerAddress, ''); + $settings->set(App\Modules\Platform\Settings\Setting::VirusScanningEnabled, false); + + config(['projectsend.scanning.default_address' => 'tcp://clamav:3310']); + + $this->artisan('projectsend:seed-settings')->assertSuccessful(); + + // Cleared on purpose is a decision, and a restart must not undo it. + expect($settings->get(App\Modules\Platform\Settings\Setting::VirusScannerAddress))->toBe('') + ->and($settings->get(App\Modules\Platform\Settings\Setting::VirusScanningEnabled))->toBeFalse(); +}); + +test('an installation with no scanner named in its environment is left alone', function () { + config(['projectsend.scanning.default_address' => null]); + + $this->artisan('projectsend:seed-settings')->assertSuccessful(); + + expect(app(App\Modules\Platform\Settings\Settings::class)->get(App\Modules\Platform\Settings\Setting::VirusScanningEnabled))->toBeFalse(); +});