diff --git a/app/Modules/Files/Console/ScanFilesCommand.php b/app/Modules/Files/Console/ScanFilesCommand.php index c3798a24..38b5066b 100644 --- a/app/Modules/Files/Console/ScanFilesCommand.php +++ b/app/Modules/Files/Console/ScanFilesCommand.php @@ -57,13 +57,7 @@ class ScanFilesCommand extends Command // run, since that is how often this command runs. $limit = $config->existingScanRatePerMinute() * 60; - $old = $this->dispatchFor( - File::query() - ->where('scan_status', ScanStatus::NotScanned) - ->where('scan_note', NotScannedReason::BeforeScanning->value), - $limit, - rescan: true, - ); + $old = $this->dispatchFor(File::query()->neverScanned(), $limit, rescan: true); $this->info("Queued {$old} file(s) that had never been scanned."); } diff --git a/app/Modules/Files/Http/Controllers/FoldersController.php b/app/Modules/Files/Http/Controllers/FoldersController.php index 881266be..e43c71a0 100644 --- a/app/Modules/Files/Http/Controllers/FoldersController.php +++ b/app/Modules/Files/Http/Controllers/FoldersController.php @@ -220,7 +220,12 @@ class FoldersController extends Controller return null; } - $note = $file->scan_note; + // A file from before the scanner existed carries no reason — see + // File::scopeNeverScanned — and "Not scanned" with no explanation + // is the one badge somebody would have to come and ask about. + $note = $file->scan_note ?? ($file->scan_status === ScanStatus::NotScanned + ? NotScannedReason::BeforeScanning->value + : null); return [ 'status' => $file->scan_status->value, diff --git a/app/Modules/Files/Http/Controllers/VirusScanningSettingsController.php b/app/Modules/Files/Http/Controllers/VirusScanningSettingsController.php index b0ff7397..bcd845fd 100644 --- a/app/Modules/Files/Http/Controllers/VirusScanningSettingsController.php +++ b/app/Modules/Files/Http/Controllers/VirusScanningSettingsController.php @@ -211,10 +211,7 @@ class VirusScanningSettingsController extends Controller ScanStatus::Infected->value, ScanStatus::UnscannableBlocked->value, ])->count(), - 'never_scanned' => File::query() - ->where('scan_status', ScanStatus::NotScanned) - ->where('scan_note', NotScannedReason::BeforeScanning->value) - ->count(), + 'never_scanned' => File::query()->neverScanned()->count(), 'let_through' => File::query() ->where('scan_status', ScanStatus::NotScanned) ->whereIn('scan_note', [ diff --git a/app/Modules/Files/Models/File.php b/app/Modules/Files/Models/File.php index 9fb0caaf..5fc31d7c 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\NotScannedReason; use App\Modules\Files\Scanning\ScanStatus; use App\Modules\Files\Versions\FileVersions; use App\Modules\Groups\Models\Group; @@ -311,6 +312,27 @@ class File extends Model }); } + /** + * Files nothing has ever looked at. + * + * Two ways to be one, and the second is the common one: a file stored + * while scanning was off carries the reason, and a file that predates + * the scanner entirely carries none at all — the migration gives the + * column its default and writes no note, and the v1 import inserts + * rows the same way. Reading only the reason missed every file on + * every real installation, which is exactly the set "Scan existing + * files" exists for. + * + * @param Builder $query + */ + public function scopeNeverScanned(Builder $query): void + { + $query->where('scan_status', ScanStatus::NotScanned) + ->where(fn (Builder $inner) => $inner + ->whereNull('scan_note') + ->orWhere('scan_note', NotScannedReason::BeforeScanning->value)); + } + /** * @param Builder $query */ diff --git a/tests/Feature/Files/VirusScanningSettingsTest.php b/tests/Feature/Files/VirusScanningSettingsTest.php index d5377cd4..279ccfa9 100644 --- a/tests/Feature/Files/VirusScanningSettingsTest.php +++ b/tests/Feature/Files/VirusScanningSettingsTest.php @@ -35,6 +35,11 @@ beforeEach(function () { test('the screen shows what is configured and what is outstanding', function () { File::factory()->create(['scan_status' => ScanStatus::NotScanned, 'scan_note' => NotScannedReason::ScannerUnavailable->value]); File::factory()->create(['scan_status' => ScanStatus::NotScanned, 'scan_note' => NotScannedReason::BeforeScanning->value]); + // The shape every upgraded installation is actually in: the status + // from the column default, and no reason beside it. Counted, or the + // "Scan existing files" button sits disabled on a library of + // thousands. + File::factory()->create(['scan_status' => ScanStatus::NotScanned, 'scan_note' => null]); File::factory()->create(['scan_status' => ScanStatus::Infected, 'scan_note' => 'X']); $this->actingAs($this->admin)->get('/system/settings/virus-scanning')->assertInertia( @@ -42,7 +47,7 @@ test('the screen shows what is configured and what is outstanding', function () ->component('system/settings/virus-scanning') ->where('managed', false) ->where('counts.let_through', 1) - ->where('counts.never_scanned', 1) + ->where('counts.never_scanned', 2) ->where('counts.quarantined', 1), ); }); diff --git a/tests/Feature/Files/VirusScanningTest.php b/tests/Feature/Files/VirusScanningTest.php index 6aec0e08..8c4da1b6 100644 --- a/tests/Feature/Files/VirusScanningTest.php +++ b/tests/Feature/Files/VirusScanningTest.php @@ -441,3 +441,38 @@ test('the activity log names the file it quarantined', function () { expect($line)->toContain('Contrato firmado') ->toContain('Eicar-Test-Signature'); }); + +test('a library from before the scanner is what "scan existing files" actually finds', function () { + // The migration gives the column its default and writes no reason, so + // every file on every upgraded installation has scan_note = null. A + // backfill that looked for the reason found none of them — the whole + // feature was inert on exactly the libraries it exists for. + $old = File::factory()->create(['scan_status' => ScanStatus::NotScanned, 'scan_note' => null]); + $stated = File::factory()->create([ + 'scan_status' => ScanStatus::NotScanned, + 'scan_note' => NotScannedReason::BeforeScanning->value, + ]); + $letThrough = File::factory()->create([ + 'scan_status' => ScanStatus::NotScanned, + 'scan_note' => NotScannedReason::ScannerUnavailable->value, + ]); + + $found = File::query()->neverScanned()->pluck('id')->all(); + + expect($found)->toContain($old->id) + ->toContain($stated->id) + // Not this one: it was offered to a scanner that could not answer, + // and the hourly sweep already re-scans those. + ->not->toContain($letThrough->id); +}); + +test('the backfill queues those files', function () { + fakeScanner(ScanVerdict::clean()); + $old = scannableFile(['scan_status' => ScanStatus::NotScanned, 'scan_note' => null]); + + Illuminate\Support\Facades\Queue::fake(); + + $this->artisan('projectsend:scan-files', ['--existing' => true])->assertSuccessful(); + + Illuminate\Support\Facades\Queue::assertPushed(ScanFileJob::class, fn (ScanFileJob $job): bool => $job->fileId === $old->id); +});