mirror of
https://github.com/projectsend/projectsend.git
synced 2026-09-23 03:53:21 +00:00
Find the files a real upgrade leaves behind
"Scan existing files" sat disabled on an installation with a library of 144 of them, and the hourly backfill would have found none either. Both looked for `scan_note = 'before_scanning'`, and no file on any upgraded installation carries it: the migration gives `scan_status` its default and writes no note, and the v1 import inserts rows the same way. The feature was inert on exactly the libraries it exists for. A file with no reason beside its "not scanned" is now what it plainly is — one nothing has ever looked at — through File::neverScanned(), which the backfill, the counts and the badge all ask. Reported as "Uploaded before virus scanning was switched on" rather than as a bare "Not scanned", which is the one badge somebody would have had to come and ask about. Found on the real screen, not by a test. The tests now cover the shape an upgrade actually produces.
This commit is contained in:
@@ -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.");
|
||||
}
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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', [
|
||||
|
||||
@@ -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<File> $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<File> $query
|
||||
*/
|
||||
|
||||
@@ -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),
|
||||
);
|
||||
});
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user