gsadmin/projectsend
1
0
Fork 0
mirror of https://github.com/projectsend/projectsend.git synced 2026-09-17 17:15:08 +00:00
Code Issues Packages Projects Releases Wiki Activity

Put virus scanning on the System card as a line, not only as a warning

Browse Source 
"Uploads checked by: ClamAV 1.5.4" now sits beside "Downloads sent by"
and "Files stored on", and is always there. Same reasoning those two
already carry: being able to confirm at a glance that uploads are
checked is worth as much as being told when they are not.

Four states in one row. A working scanner is named. One that is not
answering says so. One letting files through is amber. No scanner at
all reads "Nothing", amber, and links to the screen that sets it up —
which is where the "turn it on" link now lives, so the big alert above
is left to the cases where a configured scanner is misbehaving. Absent
entirely where the scanner is not this installation's to connect.

Also fixes a line the dashboard itself exposed: the activity log read
'The file "" was quarantined'. The scan job has no actor and attaches no
subject, so those two templates have to take the name from their
context, not from :subject. There is a test now, which there was not
before, because a real screen caught it and a green suite did not.
This commit is contained in:
ignacionelson
2026-09-16 15:41:47 -03:00
parent 85b1650ef0
commit da79969435
5 changed files with 162 additions and 70 deletions
Download Patch File Download Diff File Expand all files Collapse all files
app/Modules/Audit/Action.php
+6 -2
View File
@@ -217,9 +217,13 @@ enum Action: string
self::CommentDeleted => 'Deleted a comment on the file ":subject"',
self::CommentApproved => 'Approved a comment on the file ":subject"',
self::FileImported => 'Imported the orphan file ":subject"',
self::FileQuarantined => 'The file ":subject" was quarantined: :threat',
// :name rather than :subject, unlike the file actions above
// it: these two are written by the scan job, which has no
// actor and attaches no subject, so the name has to travel in
// the context or the line reads 'The file "" was quarantined'.
self::FileQuarantined => 'The file ":name" was quarantined: :threat',
self::FileReleased => 'Released the quarantined file ":subject" (:reason)',
self::FileNotScanned => 'The file ":subject" was not scanned for viruses: :reason',
self::FileNotScanned => 'The file ":name" was not scanned for viruses: :reason',
self::OrphanFileDeleted => 'Deleted the orphan file ":name"',
self::OrphanFileAutoDeleted => 'Deleted the orphan file ":name"',
self::ExpiredFileDeleted => 'Deleted the expired file ":name"',
app/Modules/Audit/Http/Controllers/DashboardController.php
+43 -51
View File
@@ -514,77 +514,69 @@ 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(),
// Always stated, like delivery and storage above it: "my
// uploads are checked by ClamAV" is worth confirming at a
// glance, not only worth mentioning when it is false. Null
// only where this installation does not connect its own
// scanner at all.
'scanning' => $this->scanningState(),
];
}
/**
* What is wrong with virus scanning right now, or null.
* Where this installation stands with virus scanning.
*
* 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.
* Reported whether or not anything is wrong: the System card states
* how downloads leave and where files are stored for the same reason,
* and "nothing is checking my uploads" is exactly the fact an
* administrator will not go looking for.
*
* Null only when this installation does not connect its own scanner —
* on a hosted one that is the platform's infrastructure, and a tenant
* reading about it could neither confirm nor fix it. See
* Capability::VirusScanningConnect.
*
* @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
private function scanningState(): ?array
{
if (! $this->capabilities->has(Capability::VirusScanningConnect)) {
return null;
}
$config = app(ScanningConfig::class);
if (! $config->enabled()) {
// 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;
return [
'configured' => false,
'reachable' => false,
'engine' => null,
'definitions_age_hours' => null,
'let_through_24h' => 0,
'pending' => 0,
];
}
$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 [
'configured' => true,
'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,
'definitions_age_hours' => $scanner->definitionsAgeHours(),
// 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(),
// Waiting more than an hour: on an installation set to hold,
// this is what an outage looks like.
'pending' => File::query()
->where('scan_status', ScanStatus::Pending)
->where('created_at', '<=', now()->subHour())
->count(),
];
}
resources/js/components/dashboard-widgets/system-widget.tsx
+82 -13
View File
@@ -117,6 +117,43 @@ function StorageDurabilityNotice({ durability }: { durability: StorageDurability
return null;
}
/**
* What the scanning row says, and whether it is a warning.
*
* Four states in one line, because the row is always there: no scanner at
* all, one that is not answering, one letting files through, and one
* quietly working — which is the common case and the only one that is not
* a warning.
*/
function scanningRow(
scanning: NonNullable<SystemInfo['scanning']>,
t: (key: string, replacements?: Record<string, string | number>) => string,
): { value: string; warning: boolean; title: string } {
if (!scanning.configured) {
return {
value: t('Nothing'),
warning: true,
title: t('Uploads are passed on without being checked for viruses.'),
};
}
const engine = scanning.engine ?? t('A virus scanner');
if (!scanning.reachable) {
return { value: t(':engine (not answering)', { engine }), warning: true, title: t('The scanner could not be reached.') };
}
if (scanning.let_through_24h > 0 || scanning.pending > 0) {
return {
value: engine,
warning: true,
title: t('Some files were not checked. Open the virus scanning settings for the detail.'),
};
}
return { value: engine, warning: false, title: '' };
}
export function SystemWidget({ system, onViewReleaseNotes }: { system: SystemInfo; onViewReleaseNotes: () => void }) {
const { t } = useTranslation();
const { update_notice } = usePage<SharedData>().props;
@@ -125,30 +162,26 @@ export function SystemWidget({ system, onViewReleaseNotes }: { system: SystemInf
// Only PHP is worth flagging. The other two are the file being handed
// to the web server, which is the outcome this is watching for.
const deliveryNeedsAttention = system.file_delivery.method === 'php';
const scanning = system.scanning ? scanningRow(system.scanning, t) : null;
return (
<div>
{/* Before the update notice on purpose: losing the files outranks
being a version behind. */}
{durability && <StorageDurabilityNotice durability={durability} />}
{system.scanning && (
{/* Only for a scanner that is configured and misbehaving. An
installation with no scanner at all says so on its own row
below, with the same link — two warnings for one fact would
make the card noisier without saying more. */}
{system.scanning?.configured && scanning?.warning && (
<Alert variant="warning" className="mb-3">
<ShieldAlert className="size-4" />
<AlertTitle>
{!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('Files are going out unscanned') : t('The virus scanner is not answering')}
</AlertTitle>
<AlertDescription>
<ul className="list-inside list-disc">
{!system.scanning.configured && (
<li>{t('Anything uploaded here — by staff, by clients, or through an upload link — is passed on unchecked.')}</li>
)}
{system.scanning.configured && !system.scanning.reachable && (
<li>{t('Uploads cannot be checked until it is back.')}</li>
)}
{!system.scanning.reachable && <li>{t('Uploads cannot be checked until it is back.')}</li>}
{system.scanning.let_through_24h > 0 && (
<li>
{t(':count files were allowed through without being scanned in the last 24 hours.', {
@@ -166,7 +199,7 @@ export function SystemWidget({ system, onViewReleaseNotes }: { system: SystemInf
)}
</ul>
<Link href="/system/settings/virus-scanning" className="mt-1 inline-block underline hover:no-underline">
{system.scanning.configured ? t('Virus scanning settings') : t('Set up virus scanning')}
{t('Virus scanning settings')}
</Link>
</AlertDescription>
</Alert>
@@ -270,6 +303,42 @@ export function SystemWidget({ system, onViewReleaseNotes }: { system: SystemInf
)}
</dd>
</div>
{/* Same rule as the row above, and the reason this one is
never hidden: an installation checking nothing looks
exactly like one that is. Absent only where the scanner
is not this installation's to connect. */}
{scanning && (
<div className="flex justify-between gap-2">
<dt>
{scanning.warning ? (
<Link
href="/system/settings/virus-scanning"
className="font-medium text-amber-600 underline underline-offset-2 hover:no-underline dark:text-amber-500"
title={scanning.title}
>
{t('Uploads checked by')}
</Link>
) : (
<span className="text-muted-foreground">{t('Uploads checked by')}</span>
)}
</dt>
<dd>
{scanning.warning ? (
<Link
href="/system/settings/virus-scanning"
className="flex items-center gap-1.5 font-medium text-amber-600 hover:underline dark:text-amber-500"
aria-label={scanning.title}
title={scanning.title}
>
{scanning.value}
<AlertTriangle className="size-4" />
</Link>
) : (
<span>{scanning.value}</span>
)}
</dd>
</div>
)}
{/* Stated even when everything is correct: "my files are on a
host directory" is worth being able to confirm at a glance,
not only worth warning about when it is false. */}
tests/Feature/Files/VirusScanningSettingsTest.php
+11 -4
View File
@@ -228,13 +228,13 @@ test('the status command reports an unreachable scanner and what got through', f
->and($status['let_through_24h'])->toBe(1);
});
test('the dashboard says nothing while scanning is healthy, and speaks up when it is not', function () {
test('the dashboard reports a healthy scanner, and says so when it stops answering', 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),
fn (AssertableInertia $page) => $page->where('system.scanning.reachable', true),
);
app()->instance(VirusScanner::class, (new FakeVirusScanner)->reports(ScannerStatus::unreachable('no answer')));
@@ -273,13 +273,20 @@ test('a hosted installation is not told: the scanner is not its job', function (
);
});
test('the notice goes away once a scanner is configured', function () {
test('a working scanner is still reported, by name', function () {
// The row is always there, like the delivery and storage rows beside
// it: "my uploads are checked by ClamAV" is worth confirming at a
// glance, not only worth saying when it is false.
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),
fn (AssertableInertia $page) => $page
->where('system.scanning.configured', true)
->where('system.scanning.reachable', true)
->where('system.scanning.engine', 'FakeAV 1.0')
->where('system.scanning.let_through_24h', 0),
);
});
tests/Feature/Files/VirusScanningTest.php
+20
View File
@@ -421,3 +421,23 @@ test('a released file is announced then, not before', function () {
expect(App\Modules\Notifications\InAppNotification::query()->where('user_id', $client->id)->where('type', 'file_shared')->count())->toBe(1);
});
test('the activity log names the file it quarantined', function () {
// The scan job has no actor and attaches no subject, so a template
// written with :subject renders 'The file "" was quarantined'. Caught
// on a real dashboard, not by a test, which is why there is one now.
fakeScanner(ScanVerdict::infected('Eicar-Test-Signature'));
$file = scannableFile(['name' => 'Contrato firmado']);
runScan($file);
$entry = ActivityLog::query()->where('action', Action::FileQuarantined)->sole();
$presented = app(App\Modules\Audit\ActivityPresenter::class)->present($entry);
$line = strtr($presented['template'], collect($presented['replacements'])
->mapWithKeys(fn (string $value, string $key): array => [":{$key}" => $value])
->all());
expect($line)->toContain('Contrato firmado')
->toContain('Eicar-Test-Signature');
});