From 290a03da76ab1fc21e05ae60fde5d2772b1648df Mon Sep 17 00:00:00 2001 From: Frank Karlitschek Date: Mon, 19 Oct 2026 16:01:28 -0400 Subject: [PATCH 01/16] feat(os): report operating system concerns where admins look Three setup checks on Settings > Overview, a notification to admins when a concern escalates (withdrawn once it is resolved), an os block with counts and flags only in the monitoring API, and a Server health dashboard widget. Assisted-by: ClaudeCode:claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 Signed-off-by: Frank Karlitschek --- README.md | 33 ++- img/status-error.svg | 5 + img/status-info.svg | 5 + img/status-ok.svg | 5 + img/status-warning.svg | 5 + lib/AppInfo/Application.php | 10 + lib/Commands/ShowOsStatus.php | 3 + lib/Config/ConfigLexicon.php | 16 ++ lib/Controller/ApiController.php | 13 +- lib/Dashboard/ServerHealthWidget.php | 161 +++++++++++++ lib/Jobs/UpdateOsStatus.php | 8 +- .../Notifications/OsNotificationSender.php | 124 ++++++++++ lib/OsStatus/Notifications/OsNotifier.php | 85 +++++++ lib/OsStatus/OsStatusService.php | 54 +++++ lib/OsStatus/OsStatusStore.php | 18 ++ lib/OsStatus/Verdict/Concern.php | 7 + lib/OsStatus/Verdict/ConcernTexts.php | 219 ++++++++++++++++++ lib/OsStatus/Verdict/Verdict.php | 15 ++ lib/SetupChecks/AbstractOsCheck.php | 104 +++++++++ lib/SetupChecks/OsRebootCheck.php | 27 +++ lib/SetupChecks/OsSupportCheck.php | 27 +++ lib/SetupChecks/OsUpdatesCheck.php | 27 +++ tests/lib/ApiControllerTest.php | 23 +- tests/lib/Commands/ShowOsStatusTest.php | 3 +- tests/lib/Jobs/UpdateOsStatusTest.php | 8 +- tests/lib/OsStatus/ConcernTextsTest.php | 98 ++++++++ tests/lib/OsStatus/EntryPointsTest.php | 128 ++++++++++ tests/lib/OsStatus/NotificationsTest.php | 176 ++++++++++++++ tests/lib/OsStatus/OsSetupChecksTest.php | 115 +++++++++ tests/lib/OsStatus/OsStatusServiceTest.php | 55 +++++ tests/lib/OsStatus/OsStatusStoreTest.php | 6 + tests/lib/OsStatus/OsVerdictTest.php | 2 + 32 files changed, 1575 insertions(+), 10 deletions(-) create mode 100644 img/status-error.svg create mode 100644 img/status-info.svg create mode 100644 img/status-ok.svg create mode 100644 img/status-warning.svg create mode 100644 lib/Dashboard/ServerHealthWidget.php create mode 100644 lib/OsStatus/Notifications/OsNotificationSender.php create mode 100644 lib/OsStatus/Notifications/OsNotifier.php create mode 100644 lib/OsStatus/Verdict/ConcernTexts.php create mode 100644 lib/SetupChecks/AbstractOsCheck.php create mode 100644 lib/SetupChecks/OsRebootCheck.php create mode 100644 lib/SetupChecks/OsSupportCheck.php create mode 100644 lib/SetupChecks/OsUpdatesCheck.php create mode 100644 tests/lib/OsStatus/ConcernTextsTest.php create mode 100644 tests/lib/OsStatus/EntryPointsTest.php create mode 100644 tests/lib/OsStatus/NotificationsTest.php create mode 100644 tests/lib/OsStatus/OsSetupChecksTest.php diff --git a/README.md b/README.md index 3adc39499..6a40bfdc7 100644 --- a/README.md +++ b/README.md @@ -70,7 +70,9 @@ occ serverinfo:os-status [--refresh] [--output=json] The command shows how old the status is and exits with 0, 1, 2 or 3 for ok, warning, error and unknown, so it can be used as a monitoring check. Unknown means the status has not been collected yet, or nothing is wrong but the pending updates could not be -determined. When something counts as a warning or an error is set by +determined. The same verdict appears as setup checks on Settings → Overview, in +a notification to admins when a concern escalates, and in the **Server health** +dashboard widget. When something counts as a warning or an error is set by the `os_*_days` app config values, see `lib/Config/ConfigLexicon.php`. Release support dates are bundled with the app (see @@ -141,6 +143,35 @@ https:///ocs/v2.php/apps/serverinfo/api/v1/info - To request the information in JSON append the url parameter `format=json` - Use the url parameter `skipUpdate=true` to omit server updates. - Use the url parameter `skipApps=true` to omit app updates (including available app updates will send an external request to the app store). +- Use the url parameter `skipOs=true` to omit the operating system status. + +The operating system status in `server.os` holds counts and flags only, never +package names or advisories, so a leaked token does not reveal what is +unpatched. Values that could not be determined are `null`: + +```json +"os": { + "state": "ok", + "level": "warning", + "collectedAt": 1792411200, + "outdated": false, + "name": "Ubuntu 24.04.3 LTS", + "kernel": "6.8.0-45-generic", + "environment": "host", + "container": null, + "updates": { "classified": true, "security": 7, "total": 23, "pendingDays": 5, "held": 0 }, + "rebootRequired": true, + "rebootPendingDays": 2, + "phpRestartRequired": false, + "servicesNeedRestart": null, + "support": { "state": "supported", "ends": "2029-05-31", "daysLeft": 954 }, + "automaticUpdates": "enabled", + "listsAgeHours": 4, + "timeSynchronized": true, + "failedUnits": 0, + "helper": "absent" +} +``` ### Example XML output: ``` diff --git a/img/status-error.svg b/img/status-error.svg new file mode 100644 index 000000000..0a29570cc --- /dev/null +++ b/img/status-error.svg @@ -0,0 +1,5 @@ + + diff --git a/img/status-info.svg b/img/status-info.svg new file mode 100644 index 000000000..bfc2b8474 --- /dev/null +++ b/img/status-info.svg @@ -0,0 +1,5 @@ + + diff --git a/img/status-ok.svg b/img/status-ok.svg new file mode 100644 index 000000000..6a5efed18 --- /dev/null +++ b/img/status-ok.svg @@ -0,0 +1,5 @@ + + diff --git a/img/status-warning.svg b/img/status-warning.svg new file mode 100644 index 000000000..3e181b17b --- /dev/null +++ b/img/status-warning.svg @@ -0,0 +1,5 @@ + + diff --git a/lib/AppInfo/Application.php b/lib/AppInfo/Application.php index b0507a46d..36d2a0759 100644 --- a/lib/AppInfo/Application.php +++ b/lib/AppInfo/Application.php @@ -10,9 +10,14 @@ namespace OCA\ServerInfo\AppInfo; use OCA\ServerInfo\Config\ConfigLexicon; +use OCA\ServerInfo\Dashboard\ServerHealthWidget; use OCA\ServerInfo\OsStatus\Host\IHost; use OCA\ServerInfo\OsStatus\Host\LocalHost; +use OCA\ServerInfo\OsStatus\Notifications\OsNotifier; use OCA\ServerInfo\SetupChecks\DatabaseChecks; +use OCA\ServerInfo\SetupChecks\OsRebootCheck; +use OCA\ServerInfo\SetupChecks\OsSupportCheck; +use OCA\ServerInfo\SetupChecks\OsUpdatesCheck; use OCP\AppFramework\App; use OCP\AppFramework\Bootstrap\IBootContext; use OCP\AppFramework\Bootstrap\IBootstrap; @@ -32,7 +37,12 @@ public function __construct() { public function register(IRegistrationContext $context): void { $context->registerConfigLexicon(ConfigLexicon::class); $context->registerSetupCheck(DatabaseChecks::class); + $context->registerSetupCheck(OsUpdatesCheck::class); + $context->registerSetupCheck(OsRebootCheck::class); + $context->registerSetupCheck(OsSupportCheck::class); $context->registerServiceAlias(IHost::class, LocalHost::class); + $context->registerNotifierService(OsNotifier::class); + $context->registerDashboardWidget(ServerHealthWidget::class); } #[\Override] diff --git a/lib/Commands/ShowOsStatus.php b/lib/Commands/ShowOsStatus.php index c73f3da28..bc0257b5b 100644 --- a/lib/Commands/ShowOsStatus.php +++ b/lib/Commands/ShowOsStatus.php @@ -10,6 +10,7 @@ namespace OCA\ServerInfo\Commands; use OC\Core\Command\Base; +use OCA\ServerInfo\OsStatus\Notifications\OsNotificationSender; use OCA\ServerInfo\OsStatus\OsStatusCollector; use OCA\ServerInfo\OsStatus\OsStatusService; use OCA\ServerInfo\OsStatus\Verdict\Concern; @@ -30,6 +31,7 @@ class ShowOsStatus extends Base { public function __construct( private OsStatusCollector $collector, private OsStatusService $service, + private OsNotificationSender $notifications, ) { parent::__construct(); } @@ -49,6 +51,7 @@ public function execute(InputInterface $input, OutputInterface $output): int { return self::EXIT_UNKNOWN; } + $this->notifications->sync(); } $status = $this->service->getStatus(); diff --git a/lib/Config/ConfigLexicon.php b/lib/Config/ConfigLexicon.php index 7c018135a..b9f8643e7 100644 --- a/lib/Config/ConfigLexicon.php +++ b/lib/Config/ConfigLexicon.php @@ -31,8 +31,10 @@ class ConfigLexicon implements ILexicon { public const CACHED_OS_FIRST_SEEN = 'cached_os_first_seen'; public const CACHED_OS_PHP_PROCESS = 'cached_os_php_process'; public const CACHED_OS_PHP_OUTDATED_SINCE = 'cached_os_php_outdated_since'; + public const OS_NOTIFIED_LEVELS = 'os_notified_levels'; public const JOB_INTERVAL_OS_STATUS = 'job_interval_os_status'; public const OS_HELPER_REPORT_PATH = 'os_helper_report_path'; + public const OS_NOTIFICATIONS = 'os_notifications'; public const OS_SECURITY_WARNING_DAYS = 'os_security_warning_days'; public const OS_SECURITY_ERROR_DAYS = 'os_security_error_days'; public const OS_REBOOT_WARNING_DAYS = 'os_reboot_warning_days'; @@ -107,6 +109,13 @@ public function getAppConfigs(): array { 'When web requests first saw their PHP process running replaced libraries, 0 when they do not', lazy: true, ), + new Entry( + self::OS_NOTIFIED_LEVELS, + ValueType::ARRAY, + [], + 'Alert level each operating system concern was last notified at, so admins are told once per escalation', + lazy: true, + ), new Entry( self::JOB_INTERVAL_OS_STATUS, ValueType::INT, @@ -120,6 +129,13 @@ public function getAppConfigs(): array { 'Where the optional root helper bin/serverinfo-os-report writes its report; an absolute path ending in .json', lazy: true, ), + new Entry( + self::OS_NOTIFICATIONS, + ValueType::BOOL, + true, + 'Whether admins are notified when an operating system concern escalates', + lazy: true, + ), new Entry(self::OS_SECURITY_WARNING_DAYS, ValueType::INT, 3, 'Days a security update may be pending before a warning', lazy: true), new Entry(self::OS_SECURITY_ERROR_DAYS, ValueType::INT, 14, 'Days a security update may be pending before an error', lazy: true), new Entry(self::OS_REBOOT_WARNING_DAYS, ValueType::INT, 7, 'Days a reboot may be pending before a warning', lazy: true), diff --git a/lib/Controller/ApiController.php b/lib/Controller/ApiController.php index 5fd32d0a8..75ec31c9a 100644 --- a/lib/Controller/ApiController.php +++ b/lib/Controller/ApiController.php @@ -12,6 +12,7 @@ use OCA\ServerInfo\DatabaseStatistics; use OCA\ServerInfo\FpmStatistics; use OCA\ServerInfo\Os; +use OCA\ServerInfo\OsStatus\OsStatusService; use OCA\ServerInfo\PhpStatistics; use OCA\ServerInfo\SessionStatistics; use OCA\ServerInfo\ShareStatistics; @@ -42,6 +43,7 @@ public function __construct( private ShareStatistics $shareStatistics, private SessionStatistics $sessionStatistics, private UptimeFormatter $uptimeFormatter, + private OsStatusService $osStatus, ) { parent::__construct($appName, $request); } @@ -79,14 +81,14 @@ private function checkAuthorized(): bool { * @PublicPage * @BruteForceProtection(action=serverinfo) */ - public function info(bool $skipApps = true, bool $skipUpdate = true): DataResponse { + public function info(bool $skipApps = true, bool $skipUpdate = true, bool $skipOs = false): DataResponse { if (!$this->checkAuthorized()) { $response = new DataResponse(['message' => 'Unauthorized']); $response->throttle(); $response->setStatus(Http::STATUS_UNAUTHORIZED); return $response; } - return new DataResponse([ + $data = [ 'nextcloud' => [ 'system' => $this->systemStatistics->getSystemStatistics($skipApps, $skipUpdate), 'storage' => $this->storageStatistics->getStorageStatistics(), @@ -99,7 +101,12 @@ public function info(bool $skipApps = true, bool $skipUpdate = true): DataRespon 'database' => $this->databaseStatistics->getDatabaseStatistics() ], 'activeUsers' => $this->sessionStatistics->getSessionStatistics() - ]); + ]; + if (!$skipOs) { + $data['server']['os'] = $this->osStatus->monitoringSummary(); + } + + return new DataResponse($data); } public function BasicData(): DataResponse { diff --git a/lib/Dashboard/ServerHealthWidget.php b/lib/Dashboard/ServerHealthWidget.php new file mode 100644 index 000000000..08d021191 --- /dev/null +++ b/lib/Dashboard/ServerHealthWidget.php @@ -0,0 +1,161 @@ +l10n->t('Server health'); + } + + #[\Override] + public function getOrder(): int { + return 50; + } + + #[\Override] + public function getIconClass(): string { + return ''; + } + + #[\Override] + public function getIconUrl(): string { + return $this->urlGenerator->getAbsoluteURL($this->urlGenerator->imagePath(Application::APP_ID, 'app-dark.svg')); + } + + #[\Override] + public function getUrl(): ?string { + return $this->pageUrl(); + } + + #[\Override] + public function load(): void { + } + + #[\Override] + public function isEnabled(): bool { + $user = $this->userSession->getUser(); + + return $user !== null && $this->groupManager->isAdmin($user->getUID()); + } + + #[\Override] + public function getReloadInterval(): int { + return 600; + } + + #[\Override] + public function getWidgetButtons(string $userId): array { + return [new WidgetButton(WidgetButton::TYPE_MORE, $this->pageUrl(), $this->l10n->t('Open Monitoring'))]; + } + + #[\Override] + public function getItemsV2(string $userId, ?string $since = null, int $limit = 7): WidgetItems { + if (!$this->groupManager->isAdmin($userId)) { + return new WidgetItems(); + } + + $status = $this->service->getStatus(); + if ($status === null || $status->state !== OsStatus::STATE_OK) { + return new WidgetItems([], $this->l10n->t('The operating system has not been checked yet.')); + } + + $verdict = $this->service->evaluate($status); + $items = []; + foreach ($verdict?->atLeast(Concern::LEVEL_INFO) ?? [] as $concern) { + if (!in_array($concern->id, self::SHOWN, true)) { + continue; + } + $items[] = $this->item($concern, $status); + } + + if ($items === []) { + $support = $verdict?->get(Concern::SUPPORT); + $items[] = new WidgetItem( + $this->l10n->t('The system is up to date'), + $support !== null && $support->level !== Concern::LEVEL_UNKNOWN ? $this->texts->title($support, $status, $this->l10n) : '', + $this->pageUrl(), + $this->icon(Concern::LEVEL_OK), + ); + } + + return new WidgetItems(array_slice($items, 0, max(1, $limit))); + } + + private function item(Concern $concern, OsStatus $status): WidgetItem { + return new WidgetItem( + $this->texts->title($concern, $status, $this->l10n), + $this->texts->detail($concern, $this->l10n), + $this->pageUrl(), + $this->icon($concern->level), + $concern->id, + ); + } + + private function icon(string $level): string { + $name = match ($level) { + Concern::LEVEL_ERROR => 'status-error.svg', + Concern::LEVEL_WARNING => 'status-warning.svg', + Concern::LEVEL_INFO => 'status-info.svg', + default => 'status-ok.svg', + }; + + return $this->urlGenerator->getAbsoluteURL($this->urlGenerator->imagePath(Application::APP_ID, $name)); + } + + private function pageUrl(): string { + return $this->urlGenerator->linkToRouteAbsolute('settings.AdminSettings.index', ['section' => Application::APP_ID]) . '#os'; + } +} diff --git a/lib/Jobs/UpdateOsStatus.php b/lib/Jobs/UpdateOsStatus.php index 57ba63493..07fdbc7ff 100644 --- a/lib/Jobs/UpdateOsStatus.php +++ b/lib/Jobs/UpdateOsStatus.php @@ -11,13 +11,14 @@ use OCA\ServerInfo\AppInfo\Application; use OCA\ServerInfo\Config\ConfigLexicon; +use OCA\ServerInfo\OsStatus\Notifications\OsNotificationSender; use OCA\ServerInfo\OsStatus\OsStatusCollector; use OCP\AppFramework\Utility\ITimeFactory; use OCP\BackgroundJob\TimedJob; use OCP\IAppConfig; /** - * Collects the operating system status. + * Collects the operating system status and notifies admins of escalations. * * Time sensitive on purpose: a maintenance window would hold a pending * security update back from the admin for most of a day. @@ -33,6 +34,7 @@ class UpdateOsStatus extends TimedJob { public function __construct( ITimeFactory $time, private OsStatusCollector $collector, + private OsNotificationSender $notifications, private IAppConfig $appConfig, ) { parent::__construct($time); @@ -45,7 +47,9 @@ protected function run($argument): void { return; } - $this->collector->collectAndStore(); + if ($this->collector->collectAndStore() !== null) { + $this->notifications->sync(); + } } /** diff --git a/lib/OsStatus/Notifications/OsNotificationSender.php b/lib/OsStatus/Notifications/OsNotificationSender.php new file mode 100644 index 000000000..f8ca290e7 --- /dev/null +++ b/lib/OsStatus/Notifications/OsNotificationSender.php @@ -0,0 +1,124 @@ +appConfig->getAppValueBool(ConfigLexicon::OS_NOTIFICATIONS, true); + $verdict = $enabled ? $this->service->evaluate() : null; + $notified = $this->store->notifiedLevels(); + $changed = false; + + foreach (Concern::NOTIFIABLE as $id) { + $concern = $verdict?->get($id); + $previous = $notified[$id] ?? null; + + if ($concern === null || !$concern->isProblem()) { + // Unknown is not resolved: a probe failing once must not + // withdraw a standing warning. + if ($previous !== null && ($verdict === null || $concern === null || $concern->level !== Concern::LEVEL_UNKNOWN)) { + $this->withdraw($id); + unset($notified[$id]); + $changed = true; + } + continue; + } + + $level = $concern->level; + if ($previous === null || Concern::rank($level) > Concern::rank($previous)) { + $this->withdraw($id); + $this->notifyAdmins($concern); + $notified[$id] = $level; + $changed = true; + } elseif (Concern::rank($level) < Concern::rank($previous)) { + // Remember the lower level so a later escalation is announced again. + $notified[$id] = $level; + $changed = true; + } + } + + if ($changed) { + $this->store->setNotifiedLevels($notified); + } + } + + private function notifyAdmins(Concern $concern): void { + $admins = $this->groupManager->get('admin'); + if ($admins === null) { + return; + } + + $time = $this->timeFactory->getDateTime(); + foreach ($admins->getUsers() as $user) { + if (!$user->isEnabled()) { + continue; + } + + try { + $notification = $this->notificationManager->createNotification() + ->setApp(Application::APP_ID) + ->setUser($user->getUID()) + ->setDateTime($time) + ->setObject(self::OBJECT_TYPE, $concern->id) + ->setSubject($concern->id, ['level' => $concern->level]); + $this->notificationManager->notify($notification); + } catch (\Throwable $e) { + $this->logger->warning('Could not notify an admin about the operating system status', ['exception' => $e]); + } + } + } + + private function withdraw(string $id): void { + $notification = $this->notificationManager->createNotification() + ->setApp(Application::APP_ID) + ->setObject(self::OBJECT_TYPE, $id); + $this->notificationManager->markProcessed($notification); + } + + /** + * For the notifier: whether a notification still describes a live concern. + */ + public function isCurrent(INotification $notification): bool { + return isset($this->store->notifiedLevels()[$notification->getObjectId()]); + } +} diff --git a/lib/OsStatus/Notifications/OsNotifier.php b/lib/OsStatus/Notifications/OsNotifier.php new file mode 100644 index 000000000..ac722ec9c --- /dev/null +++ b/lib/OsStatus/Notifications/OsNotifier.php @@ -0,0 +1,85 @@ +l10nFactory->get(Application::APP_ID)->t('Operating system'); + } + + #[\Override] + public function prepare(INotification $notification, string $languageCode): INotification { + if ($notification->getApp() !== Application::APP_ID || $notification->getObjectType() !== OsNotificationSender::OBJECT_TYPE) { + throw new UnknownNotificationException(); + } + + if (!$this->sender->isCurrent($notification)) { + throw new AlreadyProcessedException(); + } + + $status = $this->service->getStatus(); + $concern = $status !== null ? $this->service->evaluate($status)?->get($notification->getObjectId()) : null; + if ($status === null || $concern === null || !$concern->isProblem()) { + throw new AlreadyProcessedException(); + } + + $l = $this->l10nFactory->get(Application::APP_ID, $languageCode); + $link = $this->urlGenerator->linkToRouteAbsolute('settings.AdminSettings.index', ['section' => Application::APP_ID]) . '#os'; + + $notification + ->setParsedSubject($this->texts->title($concern, $status, $l)) + ->setLink($link) + ->setIcon($this->urlGenerator->getAbsoluteURL($this->urlGenerator->imagePath(Application::APP_ID, 'app-dark.svg'))); + + $detail = $this->texts->detail($concern, $l); + if ($detail !== '') { + $notification->setParsedMessage($detail); + } + + $action = $notification->createAction(); + $action->setParsedLabel($l->t('View')) + ->setLink($link, 'WEB') + ->setPrimary(true); + $notification->addParsedAction($action); + + return $notification; + } +} diff --git a/lib/OsStatus/OsStatusService.php b/lib/OsStatus/OsStatusService.php index a5c7120d9..dcd91305a 100644 --- a/lib/OsStatus/OsStatusService.php +++ b/lib/OsStatus/OsStatusService.php @@ -13,6 +13,7 @@ use OCA\ServerInfo\OsStatus\Host\IHost; use OCA\ServerInfo\OsStatus\Restart\PhpProcessProbe; use OCA\ServerInfo\OsStatus\Restart\PhpProcessSample; +use OCA\ServerInfo\OsStatus\Verdict\Concern; use OCA\ServerInfo\OsStatus\Verdict\OsVerdict; use OCA\ServerInfo\OsStatus\Verdict\Thresholds; use OCA\ServerInfo\OsStatus\Verdict\Verdict; @@ -95,4 +96,57 @@ public function isOutdated(OsStatus $status): bool { return $changedAt !== null && $changedAt > $status->collectedAt; } + + /** + * Counts and flags for the monitoring API. Deliberately no package names, + * versions or advisories: a leaked monitoring token must not become a list + * of unpatched software. Unknown values are null, never 0 or false, so a + * monitoring system cannot mistake "unknown" for "healthy". + */ + public function monitoringSummary(): array { + $status = $this->getStatus(); + if ($status === null) { + return ['state' => 'pending', 'level' => Concern::LEVEL_UNKNOWN]; + } + if ($status->state !== OsStatus::STATE_OK) { + return ['state' => $status->state, 'level' => Concern::LEVEL_UNKNOWN, 'collectedAt' => $status->collectedAt]; + } + + $now = $this->timeFactory->getTime(); + $verdict = $this->evaluate($status); + $facts = static fn (string $id, string $fact): mixed => $verdict?->get($id)?->facts[$fact] ?? null; + $php = $this->getPhpSample(); + + return [ + 'state' => $status->state, + 'level' => $verdict?->level() ?? Concern::LEVEL_UNKNOWN, + 'collectedAt' => $status->collectedAt, + 'outdated' => $this->isOutdated($status), + 'name' => $status->release?->prettyName, + 'kernel' => $status->kernel, + 'environment' => $status->environment->kind, + 'container' => $status->environment->container, + 'updates' => [ + 'classified' => $status->securityClassified, + 'security' => $status->updates === null || !$status->securityClassified ? null : count($status->securityUpdates() ?? []), + 'total' => $status->updates === null ? null : count($status->updates) + $status->updatesOmitted, + 'pendingDays' => $facts(Concern::UPDATES, 'days'), + 'held' => $status->held === null ? null : count($status->held), + ], + 'rebootRequired' => $status->rebootReasons === null ? null : $status->rebootReasons !== [], + 'rebootPendingDays' => $facts(Concern::REBOOT, 'days'), + 'phpRestartRequired' => $php?->isOutdated(), + 'servicesNeedRestart' => $status->restartServices === null ? null : count($status->restartServices), + 'support' => [ + 'state' => $status->support->state($now), + 'ends' => $status->support->effectiveEnd(), + 'daysLeft' => $status->support->daysLeft($now), + ], + 'automaticUpdates' => $status->automaticUpdates->state, + 'listsAgeHours' => $status->listsRefreshedAt === null ? null : intdiv(max(0, $now - $status->listsRefreshedAt), 3600), + 'timeSynchronized' => $status->timeSynchronized, + 'failedUnits' => $status->failedUnits === null ? null : count($status->failedUnits), + 'helper' => $status->helper->state, + ]; + } } diff --git a/lib/OsStatus/OsStatusStore.php b/lib/OsStatus/OsStatusStore.php index 873036cf5..59864b439 100644 --- a/lib/OsStatus/OsStatusStore.php +++ b/lib/OsStatus/OsStatusStore.php @@ -213,6 +213,24 @@ public function firstSeen(array $current, int $now, ?array $scope = null): array return $found; } + /** + * @return array concern id => level last notified + */ + public function notifiedLevels(): array { + return array_filter( + $this->appConfig->getAppValueArray(ConfigLexicon::OS_NOTIFIED_LEVELS), + static fn (mixed $level, mixed $key): bool => is_string($key) && is_string($level), + ARRAY_FILTER_USE_BOTH, + ); + } + + /** + * @param array $levels + */ + public function setNotifiedLevels(array $levels): void { + $this->appConfig->setAppValueArray(ConfigLexicon::OS_NOTIFIED_LEVELS, $levels); + } + /** * @param list|null $scope */ diff --git a/lib/OsStatus/Verdict/Concern.php b/lib/OsStatus/Verdict/Concern.php index 50c556b73..68cb49d07 100644 --- a/lib/OsStatus/Verdict/Concern.php +++ b/lib/OsStatus/Verdict/Concern.php @@ -32,6 +32,9 @@ final class Concern { public const FRESHNESS = 'freshness'; public const HELPER = 'helper'; + /** Concerns worth a notification when they escalate */ + public const NOTIFIABLE = [self::UPDATES, self::REBOOT, self::RESTART, self::SUPPORT]; + /** * @param self::LEVEL_* $level * @param array|null> $facts what the level is based on, for display @@ -65,4 +68,8 @@ public static function lower(string $level): string { default => $level, }; } + + public function isProblem(): bool { + return self::rank($this->level) >= self::rank(self::LEVEL_WARNING); + } } diff --git a/lib/OsStatus/Verdict/ConcernTexts.php b/lib/OsStatus/Verdict/ConcernTexts.php new file mode 100644 index 000000000..06757fac3 --- /dev/null +++ b/lib/OsStatus/Verdict/ConcernTexts.php @@ -0,0 +1,219 @@ + 'Ubuntu Pro', + 'debian-elts' => 'Debian Extended LTS', + 'rhel-els' => 'Red Hat Extended Life Cycle Support', + 'suse-ltss' => 'SUSE Long Term Service Pack Support', + 'oracle-extended' => 'Oracle Linux Extended Support', + ]; + + public function __construct( + private IDateTimeFormatter $dateTimeFormatter, + ) { + } + + /** + * A short headline, e.g. "7 security updates pending". + */ + public function title(Concern $concern, OsStatus $status, IL10N $l): string { + $facts = $concern->facts; + + return match ($concern->id) { + Concern::UPDATES => $this->updatesTitle($concern, $l), + Concern::REBOOT => $concern->level === Concern::LEVEL_OK + ? $l->t('No reboot required') + : $l->t('Reboot required'), + Concern::RESTART => match (true) { + $concern->level === Concern::LEVEL_OK => $l->t('No service restart required'), + ($facts['php'] ?? false) === true => $l->t('PHP is running outdated libraries'), + default => $l->n('%n service needs a restart', '%n services need a restart', self::int($concern, 'services') ?? 0), + }, + Concern::SUPPORT => $this->supportTitle($concern, $status, $l), + Concern::PACKAGE_LISTS => $l->n( + 'Package lists were last refreshed %n day ago', + 'Package lists were last refreshed %n days ago', + self::int($concern, 'days') ?? 0, + ), + Concern::AUTOMATIC_UPDATES => $concern->level === Concern::LEVEL_OK + ? $l->t('Automatic updates are enabled') + : $l->t('Automatic updates are off'), + Concern::FAILED_UNITS => $l->n('%n system service has failed', '%n system services have failed', count(self::strings($concern, 'units'))), + Concern::TIME_SYNC => $concern->level === Concern::LEVEL_OK + ? $l->t('The system clock is synchronized') + : $l->t('The system clock is not synchronized'), + Concern::HELPER => $l->t('The root helper report needs attention'), + default => $concern->id, + }; + } + + /** + * A sentence explaining the headline; empty when the headline says it all. + */ + public function detail(Concern $concern, IL10N $l): string { + $facts = $concern->facts; + $days = self::int($concern, 'days'); + + switch ($concern->id) { + case Concern::UPDATES: + $sentences = []; + $stack = self::int($concern, 'stack') ?? 0; + if ($stack > 0) { + $sentences[] = $l->n('%n of them updates software Nextcloud runs on, such as PHP or the database.', '%n of them update software Nextcloud runs on, such as PHP or the database.', $stack); + } + if ($days !== null && $days >= 1) { + $sentences[] = $l->n('The oldest has been waiting for %n day.', 'The oldest has been waiting for %n days.', $days); + } + if (($facts['listsStale'] ?? false) === true) { + $sentences[] = $l->t('The package lists are outdated, so newer updates may be missing from this count.'); + } + + return implode(' ', $sentences); + case Concern::REBOOT: + if ($concern->level === Concern::LEVEL_OK) { + return ''; + } + $sentences = []; + foreach (self::strings($concern, 'kinds') as $kind) { + $sentences[] = match ($kind) { + RebootReason::KIND_KERNEL => $l->t('A newer kernel is installed.'), + RebootReason::KIND_KERNEL_REMOVED => $l->t('The running kernel was removed by an upgrade.'), + RebootReason::KIND_CORE_LIBRARY => $l->t('Core system libraries were updated since the last boot.'), + RebootReason::KIND_MICROCODE => $l->t('Newer processor microcode is installed.'), + default => $l->t('The distribution asks for a reboot.'), + }; + } + if ($days !== null && $days >= 1) { + $sentences[] = $l->n('Pending for %n day.', 'Pending for %n days.', $days); + } + if (($facts['livePatched'] ?? false) === true) { + $sentences[] = $l->t('A live kernel patch is active, which covers the kernel until the reboot.'); + } + + return implode(' ', array_unique($sentences)); + case Concern::RESTART: + if ($concern->level === Concern::LEVEL_OK) { + return ''; + } + $services = self::int($concern, 'services') ?? 0; + $sentences = []; + if (($facts['php'] ?? false) === true && $services > 0) { + $sentences[] = $l->n('Another %n service needs a restart too.', 'Another %n services need a restart too.', $services); + } + if ($days !== null && $days >= 1) { + $sentences[] = $l->n('Pending for %n day.', 'Pending for %n days.', $days); + } + + return implode(' ', $sentences); + case Concern::FAILED_UNITS: + $units = array_slice(self::strings($concern, 'units'), 0, 5); + + return $units === [] ? '' : implode(', ', $units); + case Concern::TIME_SYNC: + return $concern->level === Concern::LEVEL_OK ? '' : $l->t('Two-factor codes, federation and object storage requests fail when the clock drifts.'); + default: + return ''; + } + } + + private function updatesTitle(Concern $concern, IL10N $l): string { + $classified = ($concern->facts['classified'] ?? false) === true; + $security = self::int($concern, 'security') ?? 0; + $other = self::int($concern, 'other') ?? 0; + + if ($concern->level === Concern::LEVEL_UNKNOWN) { + return $l->t('Pending updates could not be determined'); + } + if ($classified) { + return $security > 0 + ? $l->n('%n security update pending', '%n security updates pending', $security) + : $l->t('No security updates pending'); + } + + return $other > 0 + ? $l->n('%n update pending', '%n updates pending', $other) + : $l->t('The system is up to date'); + } + + private function supportTitle(Concern $concern, OsStatus $status, IL10N $l): string { + $name = $status->release?->prettyName ?? $l->t('This release'); + $ends = $this->date(self::string($concern, 'ends'), $l); + $programme = self::PROGRAMMES[self::string($concern, 'extended') ?? ''] ?? null; + $extendedEnds = $this->date(self::string($concern, 'extendedEnds'), $l); + + return match (self::string($concern, 'state')) { + SupportStatus::STATE_ROLLING => $l->t('{name} is a rolling release', ['name' => $name]), + SupportStatus::STATE_ENDED => $l->t('{name} no longer receives security updates since {date}', ['name' => $name, 'date' => $ends]), + SupportStatus::STATE_EXTENDED_AVAILABLE => $l->t('Standard support for {name} has ended. Security updates continue only with {programme} until {date}', [ + 'name' => $name, + 'programme' => $programme ?? '', + 'date' => $extendedEnds, + ]), + SupportStatus::STATE_EXTENDED => $l->t('{name} receives security updates through {programme} until {date}', [ + 'name' => $name, + 'programme' => $programme ?? '', + 'date' => $ends, + ]), + SupportStatus::STATE_SUPPORTED => $concern->level === Concern::LEVEL_OK + ? $l->t('{name} receives security updates until {date}', ['name' => $name, 'date' => $ends]) + : $l->t('Security updates for {name} end on {date}', ['name' => $name, 'date' => $ends]), + default => $l->t('The support status of {name} is unknown', ['name' => $name]), + }; + } + + private static function int(Concern $concern, string $fact): ?int { + $value = $concern->facts[$fact] ?? null; + + return is_int($value) ? $value : null; + } + + private static function string(Concern $concern, string $fact): ?string { + $value = $concern->facts[$fact] ?? null; + + return is_string($value) ? $value : null; + } + + /** + * @return list + */ + private static function strings(Concern $concern, string $fact): array { + $value = $concern->facts[$fact] ?? null; + + return is_array($value) ? $value : []; + } + + private function date(?string $date, IL10N $l): string { + if ($date === null) { + return ''; + } + $time = strtotime($date . ' 12:00:00 UTC'); + if ($time === false) { + return $date; + } + + return $this->dateTimeFormatter->formatDate($time, 'long', null, $l); + } +} diff --git a/lib/OsStatus/Verdict/Verdict.php b/lib/OsStatus/Verdict/Verdict.php index 331bf5116..181afcfbc 100644 --- a/lib/OsStatus/Verdict/Verdict.php +++ b/lib/OsStatus/Verdict/Verdict.php @@ -56,4 +56,19 @@ public function get(string $id): ?Concern { return null; } + + /** + * Concerns at or above the given level, most severe first. + * + * @return list + */ + public function atLeast(string $level): array { + $matching = array_values(array_filter( + $this->concerns, + static fn (Concern $concern): bool => Concern::rank($concern->level) >= Concern::rank($level) && $concern->level !== Concern::LEVEL_UNKNOWN, + )); + usort($matching, static fn (Concern $a, Concern $b): int => Concern::rank($b->level) <=> Concern::rank($a->level)); + + return $matching; + } } diff --git a/lib/SetupChecks/AbstractOsCheck.php b/lib/SetupChecks/AbstractOsCheck.php new file mode 100644 index 000000000..50c5a7ab3 --- /dev/null +++ b/lib/SetupChecks/AbstractOsCheck.php @@ -0,0 +1,104 @@ + concern ids this check reports, most important first + */ + abstract protected function concernIds(): array; + + #[\Override] + public function getCategory(): string { + return 'security'; + } + + #[\Override] + public function run(): SetupResult { + $this->service->samplePhpProcess(); + + $status = $this->service->getStatus(); + if ($status === null) { + return SetupResult::info( + $this->l10n->t('The operating system has not been checked yet. It is checked by the background jobs when they run from system cron, or with occ serverinfo:os-status --refresh.'), + ); + } + if ($status->state === OsStatus::STATE_RESTRICTED) { + return SetupResult::success($this->l10n->t('Not checked, because the Monitoring app runs in restricted mode.')); + } + if ($status->state === OsStatus::STATE_UNSUPPORTED) { + return SetupResult::success($this->l10n->t('Only Linux systems are checked.')); + } + + $verdict = $this->service->evaluate($status); + $concerns = array_values(array_filter( + array_map(static fn (string $id): ?Concern => $verdict?->get($id), $this->concernIds()), + static fn (?Concern $concern): bool => $concern !== null && $concern->level !== Concern::LEVEL_UNKNOWN, + )); + if ($concerns === []) { + return SetupResult::info($this->l10n->t('This could not be checked on this system. {link}'), null, $this->linkParameter()); + } + + usort($concerns, static fn (Concern $a, Concern $b): int => Concern::rank($b->level) <=> Concern::rank($a->level)); + $worst = $concerns[0]; + + $text = trim($this->texts->title($worst, $status, $this->l10n) . '. ' . $this->texts->detail($worst, $this->l10n)); + $description = $text . ' {link}'; + + return match ($worst->level) { + Concern::LEVEL_ERROR => SetupResult::error($description, null, $this->linkParameter()), + Concern::LEVEL_WARNING => SetupResult::warning($description, null, $this->linkParameter()), + Concern::LEVEL_INFO => SetupResult::info($description, null, $this->linkParameter()), + default => SetupResult::success($text), + }; + } + + /** + * The Overview only renders a link when it comes as a rich object parameter. + * + * @return array> + */ + private function linkParameter(): array { + return [ + 'link' => [ + 'type' => 'highlight', + 'id' => 'serverinfo-os', + 'name' => $this->l10n->t('Show the operating system status…'), + 'link' => $this->urlGenerator->linkToRoute('settings.AdminSettings.index', ['section' => Application::APP_ID]) . '#os', + ], + ]; + } +} diff --git a/lib/SetupChecks/OsRebootCheck.php b/lib/SetupChecks/OsRebootCheck.php new file mode 100644 index 000000000..f91aedcf0 --- /dev/null +++ b/lib/SetupChecks/OsRebootCheck.php @@ -0,0 +1,27 @@ +l10n->t('Operating system reboot and restarts'); + } + + #[\Override] + protected function concernIds(): array { + return [Concern::REBOOT, Concern::RESTART]; + } +} diff --git a/lib/SetupChecks/OsSupportCheck.php b/lib/SetupChecks/OsSupportCheck.php new file mode 100644 index 000000000..ef9a75fe7 --- /dev/null +++ b/lib/SetupChecks/OsSupportCheck.php @@ -0,0 +1,27 @@ +l10n->t('Operating system support'); + } + + #[\Override] + protected function concernIds(): array { + return [Concern::SUPPORT]; + } +} diff --git a/lib/SetupChecks/OsUpdatesCheck.php b/lib/SetupChecks/OsUpdatesCheck.php new file mode 100644 index 000000000..c21aee378 --- /dev/null +++ b/lib/SetupChecks/OsUpdatesCheck.php @@ -0,0 +1,27 @@ +l10n->t('Operating system updates'); + } + + #[\Override] + protected function concernIds(): array { + return [Concern::UPDATES, Concern::PACKAGE_LISTS]; + } +} diff --git a/tests/lib/ApiControllerTest.php b/tests/lib/ApiControllerTest.php index fc1cff699..64d06ed6e 100644 --- a/tests/lib/ApiControllerTest.php +++ b/tests/lib/ApiControllerTest.php @@ -13,6 +13,7 @@ use OCA\ServerInfo\DatabaseStatistics; use OCA\ServerInfo\FpmStatistics; use OCA\ServerInfo\Os; +use OCA\ServerInfo\OsStatus\OsStatusService; use OCA\ServerInfo\PhpStatistics; use OCA\ServerInfo\SessionStatistics; use OCA\ServerInfo\ShareStatistics; @@ -40,6 +41,7 @@ class ApiControllerTest extends \Test\TestCase { private ShareStatistics&MockObject $shareStatistics; private SessionStatistics&MockObject $sessionStatistics; private UptimeFormatter&MockObject $uptimeFormatter; + private OsStatusService&MockObject $osStatus; protected function setUp(): void { parent::setUp(); @@ -56,6 +58,7 @@ protected function setUp(): void { $this->shareStatistics = $this->createMock(ShareStatistics::class); $this->sessionStatistics = $this->createMock(SessionStatistics::class); $this->uptimeFormatter = $this->createMock(UptimeFormatter::class); + $this->osStatus = $this->createMock(OsStatusService::class); } private function getController($userSession) { @@ -73,7 +76,8 @@ private function getController($userSession) { $this->databaseStatistics, $this->shareStatistics, $this->sessionStatistics, - $this->uptimeFormatter + $this->uptimeFormatter, + $this->osStatus, ); } @@ -202,6 +206,9 @@ public function testInfo() { $this->sessionStatistics->expects($this->once()) ->method('getSessionStatistics') ->willReturn(['some_user' => 'some_value']); + $this->osStatus->expects($this->once()) + ->method('monitoringSummary') + ->willReturn(['state' => 'ok', 'level' => 'warning']); $response = $this->getController($userSession)->info(); @@ -218,8 +225,22 @@ public function testInfo() { 'php' => ['some_php' => 'some_value'], 'fpm' => ['some_fpm' => 'some_value'], 'database' => ['some_database' => 'some_value'], + 'os' => ['state' => 'ok', 'level' => 'warning'], ], 'activeUsers' => ['some_user' => 'some_value'], ], $response->getData()); } + + public function testInfoWithoutTheOperatingSystem(): void { + $userSession = $this->createMock(IUserSession::class); + $user = $this->createMock(IUser::class); + $user->method('getUID')->willReturn('admin'); + $userSession->method('getUser')->willReturn($user); + $this->groupManager->method('isAdmin')->willReturn(true); + $this->osStatus->expects($this->never())->method('monitoringSummary'); + + $response = $this->getController($userSession)->info(skipOs: true); + + $this->assertArrayNotHasKey('os', $response->getData()['server']); + } } diff --git a/tests/lib/Commands/ShowOsStatusTest.php b/tests/lib/Commands/ShowOsStatusTest.php index a980cfdd2..32bbbefd0 100644 --- a/tests/lib/Commands/ShowOsStatusTest.php +++ b/tests/lib/Commands/ShowOsStatusTest.php @@ -10,6 +10,7 @@ namespace OCA\ServerInfo\Tests\Commands; use OCA\ServerInfo\Commands\ShowOsStatus; +use OCA\ServerInfo\OsStatus\Notifications\OsNotificationSender; use OCA\ServerInfo\OsStatus\OsStatus; use OCA\ServerInfo\OsStatus\OsStatusCollector; use OCA\ServerInfo\OsStatus\OsStatusService; @@ -80,7 +81,7 @@ public function testAges(int $seconds, string $formatted): void { } private function execute(OsStatusService $service, ?OutputInterface $output = null): int { - $command = new ShowOsStatus($this->createMock(OsStatusCollector::class), $service); + $command = new ShowOsStatus($this->createMock(OsStatusCollector::class), $service, $this->createMock(OsNotificationSender::class)); $input = $this->createMock(InputInterface::class); $input->method('getOption')->willReturnMap([['refresh', false], ['output', 'json']]); diff --git a/tests/lib/Jobs/UpdateOsStatusTest.php b/tests/lib/Jobs/UpdateOsStatusTest.php index 59b56d369..6ba1db12f 100644 --- a/tests/lib/Jobs/UpdateOsStatusTest.php +++ b/tests/lib/Jobs/UpdateOsStatusTest.php @@ -10,6 +10,8 @@ namespace OCA\ServerInfo\Tests\Jobs; use OCA\ServerInfo\Jobs\UpdateOsStatus; +use OCA\ServerInfo\OsStatus\Notifications\OsNotificationSender; +use OCA\ServerInfo\OsStatus\OsStatus; use OCA\ServerInfo\OsStatus\OsStatusCollector; use OCP\AppFramework\Utility\ITimeFactory; use OCP\IAppConfig; @@ -30,9 +32,11 @@ public function testRunsOnlyFromSystemCron(string $mode, int $runs): void { $appConfig->method('getValueInt')->willReturn(3600); $appConfig->method('getValueString')->with('core', 'backgroundjobs_mode', 'ajax')->willReturn($mode); $collector = $this->createMock(OsStatusCollector::class); - $collector->expects($this->exactly($runs))->method('collectAndStore'); + $collector->expects($this->exactly($runs))->method('collectAndStore')->willReturn(OsStatus::restricted(1792411200)); + $notifications = $this->createMock(OsNotificationSender::class); + $notifications->expects($this->exactly($runs))->method('sync'); - $job = new UpdateOsStatus($this->createMock(ITimeFactory::class), $collector, $appConfig); + $job = new UpdateOsStatus($this->createMock(ITimeFactory::class), $collector, $notifications, $appConfig); (new \ReflectionMethod($job, 'run'))->invoke($job, null); } } diff --git a/tests/lib/OsStatus/ConcernTextsTest.php b/tests/lib/OsStatus/ConcernTextsTest.php new file mode 100644 index 000000000..3651210a0 --- /dev/null +++ b/tests/lib/OsStatus/ConcernTextsTest.php @@ -0,0 +1,98 @@ +createMock(IDateTimeFormatter::class); + $formatter->method('formatDate')->willReturnCallback(static fn (int $time): string => gmdate('Y-m-d', $time)); + $this->texts = new ConcernTexts($formatter); + + $this->l = $this->createMock(IL10N::class); + $replace = static function (string $text, array $parameters): string { + foreach ($parameters as $key => $value) { + $text = str_replace('{' . $key . '}', (string)$value, $text); + } + + return $text; + }; + $this->l->method('t')->willReturnCallback(static fn (string $text, array $parameters = []): string => $replace($text, $parameters)); + $this->l->method('n')->willReturnCallback(static fn (string $one, string $many, int $count, array $parameters = []): string => str_replace('%n', (string)$count, $replace($count === 1 ? $one : $many, $parameters))); + + $this->status = new OsStatus(OsStatus::STATE_OK, 1, new OsRelease('ubuntu', ['debian'], 'Ubuntu', 'Ubuntu 22.04.5 LTS', '22.04', 'jammy')); + } + + public function testUpdates(): void { + $concern = new Concern(Concern::UPDATES, Concern::LEVEL_WARNING, ['classified' => true, 'security' => 7, 'other' => 2, 'stack' => 3, 'days' => 5]); + + $this->assertSame('7 security updates pending', $this->texts->title($concern, $this->status, $this->l)); + $this->assertSame( + '3 of them update software Nextcloud runs on, such as PHP or the database. The oldest has been waiting for 5 days.', + $this->texts->detail($concern, $this->l), + ); + } + + public function testUpdatesFromOutdatedPackageLists(): void { + $concern = new Concern(Concern::UPDATES, Concern::LEVEL_OK, ['classified' => true, 'security' => 0, 'other' => 0, 'listsStale' => true]); + + $this->assertSame('No security updates pending', $this->texts->title($concern, $this->status, $this->l)); + $this->assertSame('The package lists are outdated, so newer updates may be missing from this count.', $this->texts->detail($concern, $this->l)); + } + + public function testUnclassifiedUpdates(): void { + $concern = new Concern(Concern::UPDATES, Concern::LEVEL_INFO, ['classified' => false, 'security' => 0, 'other' => 1, 'days' => 0]); + + $this->assertSame('1 update pending', $this->texts->title($concern, $this->status, $this->l)); + $this->assertSame('', $this->texts->detail($concern, $this->l)); + } + + public function testReboot(): void { + $concern = new Concern(Concern::REBOOT, Concern::LEVEL_INFO, ['days' => 2, 'kinds' => ['kernel'], 'livePatched' => true]); + + $this->assertSame('Reboot required', $this->texts->title($concern, $this->status, $this->l)); + $this->assertSame( + 'A newer kernel is installed. Pending for 2 days. A live kernel patch is active, which covers the kernel until the reboot.', + $this->texts->detail($concern, $this->l), + ); + } + + public function testSupport(): void { + $extended = new Concern(Concern::SUPPORT, Concern::LEVEL_WARNING, [ + 'state' => SupportStatus::STATE_EXTENDED_AVAILABLE, 'ends' => '2027-05-31', 'extended' => 'ubuntu-pro', 'extendedEnds' => '2032-04-09', + ]); + $this->assertSame( + 'Standard support for Ubuntu 22.04.5 LTS has ended. Security updates continue only with Ubuntu Pro until 2032-04-09', + $this->texts->title($extended, $this->status, $this->l), + ); + + $ended = new Concern(Concern::SUPPORT, Concern::LEVEL_ERROR, ['state' => SupportStatus::STATE_ENDED, 'ends' => '2024-06-30']); + $this->assertSame('Ubuntu 22.04.5 LTS no longer receives security updates since 2024-06-30', $this->texts->title($ended, $this->status, $this->l)); + } + + public function testRestartPrefersThePhpMessage(): void { + $concern = new Concern(Concern::RESTART, Concern::LEVEL_WARNING, ['php' => true, 'services' => 2, 'days' => 4]); + + $this->assertSame('PHP is running outdated libraries', $this->texts->title($concern, $this->status, $this->l)); + $this->assertSame('Another 2 services need a restart too. Pending for 4 days.', $this->texts->detail($concern, $this->l)); + } +} diff --git a/tests/lib/OsStatus/EntryPointsTest.php b/tests/lib/OsStatus/EntryPointsTest.php new file mode 100644 index 000000000..6616291da --- /dev/null +++ b/tests/lib/OsStatus/EntryPointsTest.php @@ -0,0 +1,128 @@ +service = $this->createMock(OsStatusService::class); + $this->l10n = $this->createMock(IL10N::class); + $this->l10n->method('t')->willReturnArgument(0); + $this->l10n->method('n')->willReturnCallback(static fn (string $one, string $many, int $count): string => str_replace('%n', (string)$count, $count === 1 ? $one : $many)); + $this->urlGenerator = $this->createMock(IURLGenerator::class); + $this->urlGenerator->method('linkToRouteAbsolute')->willReturn('https://cloud.example.org/settings/admin/serverinfo'); + $this->urlGenerator->method('imagePath')->willReturnCallback(static fn (string $app, string $image): string => "/apps/$app/img/$image"); + $this->urlGenerator->method('getAbsoluteURL')->willReturnCallback(static fn (string $url): string => 'https://cloud.example.org' . $url); + } + + private function texts(): ConcernTexts { + return new ConcernTexts($this->createMock(IDateTimeFormatter::class)); + } + + private function notifier(OsNotificationSender&MockObject $sender): OsNotifier { + $factory = $this->createMock(IFactory::class); + $factory->method('get')->willReturn($this->l10n); + + return new OsNotifier($factory, $this->urlGenerator, $this->service, $sender, $this->texts()); + } + + private function notification(string $app, string $objectType, string $objectId): INotification&MockObject { + $notification = $this->createMock(INotification::class); + $notification->method('getApp')->willReturn($app); + $notification->method('getObjectType')->willReturn($objectType); + $notification->method('getObjectId')->willReturn($objectId); + + return $notification; + } + + public function testNotifierIgnoresOtherApps(): void { + $this->expectException(UnknownNotificationException::class); + $this->notifier($this->createMock(OsNotificationSender::class))->prepare($this->notification('files', 'os', 'updates'), 'en'); + } + + public function testNotifierDropsResolvedConcerns(): void { + $sender = $this->createMock(OsNotificationSender::class); + $sender->method('isCurrent')->willReturn(true); + $this->service->method('getStatus')->willReturn(new OsStatus(OsStatus::STATE_OK, 1)); + $this->service->method('evaluate')->willReturn(new Verdict([new Concern(Concern::UPDATES, Concern::LEVEL_OK)])); + + $this->expectException(AlreadyProcessedException::class); + $this->notifier($sender)->prepare($this->notification('serverinfo', 'os', 'updates'), 'en'); + } + + private function widget(bool $admin): ServerHealthWidget { + $user = $this->createMock(IUser::class); + $user->method('getUID')->willReturn('alice'); + $session = $this->createMock(IUserSession::class); + $session->method('getUser')->willReturn($user); + $groups = $this->createMock(IGroupManager::class); + $groups->method('isAdmin')->willReturn($admin); + + return new ServerHealthWidget($this->l10n, $this->urlGenerator, $session, $groups, $this->service, $this->texts()); + } + + public function testWidgetIsForAdminsOnly(): void { + $this->service->expects($this->never())->method('getStatus'); + + $widget = $this->widget(false); + + $this->assertFalse($widget->isEnabled()); + $this->assertSame([], $widget->getItemsV2('alice')->getItems()); + } + + public function testWidgetListsProblemsFirst(): void { + $this->service->method('getStatus')->willReturn(new OsStatus(OsStatus::STATE_OK, 1)); + $this->service->method('evaluate')->willReturn(new Verdict([ + new Concern(Concern::TIME_SYNC, Concern::LEVEL_WARNING), + new Concern(Concern::UPDATES, Concern::LEVEL_ERROR, ['classified' => true, 'security' => 3, 'other' => 0, 'days' => 20]), + new Concern(Concern::HELPER, Concern::LEVEL_WARNING, ['state' => 'insecure']), + new Concern(Concern::REBOOT, Concern::LEVEL_OK), + ])); + + $items = $this->widget(true)->getItemsV2('alice')->getItems(); + + $this->assertSame(['3 security updates pending', 'The system clock is not synchronized'], array_map(static fn ($item) => $item->getTitle(), $items)); + $this->assertStringEndsWith('status-error.svg', $items[0]->getIconUrl()); + } + + public function testWidgetWhenAllIsWell(): void { + $this->service->method('getStatus')->willReturn(new OsStatus(OsStatus::STATE_OK, 1)); + $this->service->method('evaluate')->willReturn(new Verdict([new Concern(Concern::UPDATES, Concern::LEVEL_OK)])); + + $items = $this->widget(true)->getItemsV2('alice')->getItems(); + + $this->assertSame('The system is up to date', $items[0]->getTitle()); + $this->assertStringEndsWith('status-ok.svg', $items[0]->getIconUrl()); + } +} diff --git a/tests/lib/OsStatus/NotificationsTest.php b/tests/lib/OsStatus/NotificationsTest.php new file mode 100644 index 000000000..d837ad5a9 --- /dev/null +++ b/tests/lib/OsStatus/NotificationsTest.php @@ -0,0 +1,176 @@ + */ + private array $notified = []; + /** @var list "notify:id:level:user" and "withdraw:id" */ + private array $events = []; + private OsNotificationSender $sender; + + protected function setUp(): void { + parent::setUp(); + + $this->service = $this->createMock(OsStatusService::class); + $store = $this->createMock(OsStatusStore::class); + $store->method('notifiedLevels')->willReturnCallback(fn (): array => $this->notified); + $store->method('setNotifiedLevels')->willReturnCallback(function (array $levels): void { + $this->notified = $levels; + }); + + $this->manager = $this->createMock(IManager::class); + $this->manager->method('createNotification')->willReturnCallback(fn (): INotification => $this->notification()); + $this->manager->method('notify')->willReturnCallback(function (INotification $n): void { + $this->events[] = 'notify:' . $n->getObjectId() . ':' . $n->getSubjectParameters()['level'] . ':' . $n->getUser(); + }); + $this->manager->method('markProcessed')->willReturnCallback(function (INotification $n): void { + $this->events[] = 'withdraw:' . $n->getObjectId(); + }); + + $admin = $this->createMock(IUser::class); + $admin->method('getUID')->willReturn('admin'); + $admin->method('isEnabled')->willReturn(true); + $disabled = $this->createMock(IUser::class); + $disabled->method('getUID')->willReturn('former'); + $disabled->method('isEnabled')->willReturn(false); + $group = $this->createMock(IGroup::class); + $group->method('getUsers')->willReturn([$admin, $disabled]); + $groups = $this->createMock(IGroupManager::class); + $groups->method('get')->with('admin')->willReturn($group); + + $config = $this->createMock(IAppConfig::class); + $config->method('getAppValueBool')->willReturnCallback(fn (): bool => $this->enabled); + + $time = $this->createMock(ITimeFactory::class); + $time->method('getDateTime')->willReturn(new \DateTime('@1792411200')); + + $this->sender = new OsNotificationSender($this->service, $store, $this->manager, $groups, $config, $time, $this->createMock(LoggerInterface::class)); + } + + /** + * A notification that remembers what it was given. + */ + private function notification(): INotification { + $values = []; + $notification = $this->createMock(INotification::class); + foreach (['setApp', 'setUser', 'setDateTime', 'setObject', 'setSubject'] as $setter) { + $notification->method($setter)->willReturnCallback(function (...$args) use (&$values, $setter, $notification) { + $values[$setter] = $args; + + return $notification; + }); + } + // Closures by reference: arrow functions would see $values as it was here. + $notification->method('getObjectId')->willReturnCallback(static function () use (&$values): string { + return $values['setObject'][1] ?? ''; + }); + $notification->method('getUser')->willReturnCallback(static function () use (&$values): string { + return $values['setUser'][0] ?? ''; + }); + $notification->method('getSubjectParameters')->willReturnCallback(static function () use (&$values): array { + return $values['setSubject'][1] ?? []; + }); + + return $notification; + } + + private function verdict(string $updates, string $reboot = Concern::LEVEL_OK): void { + $this->service->method('evaluate')->willReturn(new Verdict([ + new Concern(Concern::UPDATES, $updates), + new Concern(Concern::REBOOT, $reboot), + new Concern(Concern::TIME_SYNC, Concern::LEVEL_WARNING), + ])); + } + + public function testAWarningNotifiesEnabledAdminsOnce(): void { + $this->verdict(Concern::LEVEL_WARNING); + + $this->sender->sync(); + $this->sender->sync(); + + $this->assertSame(['withdraw:updates', 'notify:updates:warning:admin'], $this->events); + $this->assertSame(['updates' => 'warning'], $this->notified); + } + + public function testOnlyNotifiableConcernsNotify(): void { + $this->verdict(Concern::LEVEL_OK); + + $this->sender->sync(); + + $this->assertSame([], $this->events, 'Time sync is shown, but not worth a notification'); + } + + public function testEscalationNotifiesAgain(): void { + $this->notified = ['updates' => 'warning']; + $this->verdict(Concern::LEVEL_ERROR); + + $this->sender->sync(); + + $this->assertSame(['withdraw:updates', 'notify:updates:error:admin'], $this->events); + } + + public function testDeEscalationIsRememberedSilently(): void { + $this->notified = ['updates' => 'error']; + $this->verdict(Concern::LEVEL_WARNING); + + $this->sender->sync(); + + $this->assertSame([], $this->events); + $this->assertSame(['updates' => 'warning'], $this->notified); + } + + public function testResolvedConcernsAreWithdrawn(): void { + $this->notified = ['updates' => 'warning', 'reboot' => 'error']; + $this->verdict(Concern::LEVEL_INFO, Concern::LEVEL_OK); + + $this->sender->sync(); + + $this->assertSame(['withdraw:updates', 'withdraw:reboot'], $this->events); + $this->assertSame([], $this->notified); + } + + public function testUnknownIsNotResolved(): void { + $this->notified = ['updates' => 'warning']; + $this->verdict(Concern::LEVEL_UNKNOWN); + + $this->sender->sync(); + + $this->assertSame([], $this->events); + $this->assertSame(['updates' => 'warning'], $this->notified); + } + + public function testTurningNotificationsOffWithdrawsThem(): void { + $this->notified = ['updates' => 'warning']; + $this->enabled = false; + + $this->sender->sync(); + + $this->assertSame(['withdraw:updates'], $this->events); + } +} diff --git a/tests/lib/OsStatus/OsSetupChecksTest.php b/tests/lib/OsStatus/OsSetupChecksTest.php new file mode 100644 index 000000000..2cb320fc6 --- /dev/null +++ b/tests/lib/OsStatus/OsSetupChecksTest.php @@ -0,0 +1,115 @@ +service = $this->createMock(OsStatusService::class); + $this->l10n = $this->createMock(IL10N::class); + $this->l10n->method('t')->willReturnArgument(0); + $this->l10n->method('n')->willReturnCallback(static fn (string $one, string $many, int $count): string => str_replace('%n', (string)$count, $count === 1 ? $one : $many)); + $this->urlGenerator = $this->createMock(IURLGenerator::class); + $this->urlGenerator->method('linkToRoute')->willReturn('/settings/admin/serverinfo'); + } + + private function texts(): ConcernTexts { + return new ConcernTexts($this->createMock(IDateTimeFormatter::class)); + } + + private function given(?OsStatus $status, Concern ...$concerns): void { + $this->service->method('getStatus')->willReturn($status); + $this->service->method('evaluate')->willReturn(new Verdict($concerns)); + } + + public function testNotCheckedYet(): void { + $this->given(null); + + $result = (new OsUpdatesCheck($this->l10n, $this->urlGenerator, $this->service, $this->texts()))->run(); + + $this->assertSame(SetupResult::INFO, $result->getSeverity()); + $this->assertSame('security', (new OsUpdatesCheck($this->l10n, $this->urlGenerator, $this->service, $this->texts()))->getCategory()); + } + + public function testTheWorstConcernOfTheCheckDecides(): void { + $this->given( + new OsStatus(OsStatus::STATE_OK, 1), + new Concern(Concern::UPDATES, Concern::LEVEL_INFO, ['classified' => true, 'security' => 1, 'other' => 0]), + new Concern(Concern::PACKAGE_LISTS, Concern::LEVEL_ERROR, ['days' => 40]), + new Concern(Concern::REBOOT, Concern::LEVEL_WARNING, ['days' => 9, 'kinds' => ['kernel']]), + ); + + $result = (new OsUpdatesCheck($this->l10n, $this->urlGenerator, $this->service, $this->texts()))->run(); + + $this->assertSame(SetupResult::ERROR, $result->getSeverity()); + $this->assertStringContainsString('Package lists were last refreshed 40 days ago', (string)$result->getDescription()); + $this->assertStringEndsWith('{link}', (string)$result->getDescription()); + $this->assertSame('/settings/admin/serverinfo#os', $result->getDescriptionParameters()['link']['link'] ?? null); + } + + public function testRebootCheck(): void { + $this->given( + new OsStatus(OsStatus::STATE_OK, 1), + new Concern(Concern::REBOOT, Concern::LEVEL_WARNING, ['days' => 9, 'kinds' => ['kernel'], 'livePatched' => false]), + new Concern(Concern::RESTART, Concern::LEVEL_OK, ['php' => false, 'services' => 0]), + ); + + $result = (new OsRebootCheck($this->l10n, $this->urlGenerator, $this->service, $this->texts()))->run(); + + $this->assertSame(SetupResult::WARNING, $result->getSeverity()); + $this->assertStringStartsWith('Reboot required. A newer kernel is installed. Pending for 9 days.', (string)$result->getDescription()); + } + + public function testAllGoodHasNoLink(): void { + $this->given(new OsStatus(OsStatus::STATE_OK, 1), new Concern(Concern::REBOOT, Concern::LEVEL_OK), new Concern(Concern::RESTART, Concern::LEVEL_OK)); + + $result = (new OsRebootCheck($this->l10n, $this->urlGenerator, $this->service, $this->texts()))->run(); + + $this->assertSame(SetupResult::SUCCESS, $result->getSeverity()); + $this->assertStringNotContainsString('{link}', (string)$result->getDescription()); + } + + public function testUnknownIsInfo(): void { + $this->given(new OsStatus(OsStatus::STATE_OK, 1), new Concern(Concern::SUPPORT, Concern::LEVEL_UNKNOWN)); + + $this->assertSame(SetupResult::INFO, (new OsSupportCheck($this->l10n, $this->urlGenerator, $this->service, $this->texts()))->run()->getSeverity()); + } + + public function testRestrictedMode(): void { + $this->given(OsStatus::restricted(1)); + + $this->assertSame(SetupResult::SUCCESS, (new OsSupportCheck($this->l10n, $this->urlGenerator, $this->service, $this->texts()))->run()->getSeverity()); + } + + public function testItSamplesThePhpProcess(): void { + $this->given(null); + $this->service->expects($this->once())->method('samplePhpProcess'); + + (new OsRebootCheck($this->l10n, $this->urlGenerator, $this->service, $this->texts()))->run(); + } +} diff --git a/tests/lib/OsStatus/OsStatusServiceTest.php b/tests/lib/OsStatus/OsStatusServiceTest.php index f894cd7c4..814310e30 100644 --- a/tests/lib/OsStatus/OsStatusServiceTest.php +++ b/tests/lib/OsStatus/OsStatusServiceTest.php @@ -9,10 +9,15 @@ namespace OCA\ServerInfo\Tests\OsStatus; +use OCA\ServerInfo\OsStatus\Environment; +use OCA\ServerInfo\OsStatus\OsRelease; use OCA\ServerInfo\OsStatus\OsStatus; use OCA\ServerInfo\OsStatus\OsStatusService; use OCA\ServerInfo\OsStatus\OsStatusStore; +use OCA\ServerInfo\OsStatus\Packages\HeldPackage; +use OCA\ServerInfo\OsStatus\Packages\PendingUpdate; use OCA\ServerInfo\OsStatus\Restart\PhpProcessProbe; +use OCA\ServerInfo\OsStatus\Support\SupportStatus; use OCA\ServerInfo\OsStatus\Verdict\Concern; use OCA\ServerInfo\OsStatus\Verdict\OsVerdict; use OCP\AppFramework\Services\IAppConfig; @@ -48,6 +53,56 @@ protected function setUp(): void { $this->service = new OsStatusService($this->store, new OsVerdict(), new PhpProcessProbe($this->host), $config, $time, $this->host, $serverConfig); } + public function testTheMonitoringSummaryHasCountsButNoNames(): void { + $this->store->method('load')->willReturn(new OsStatus( + OsStatus::STATE_OK, + self::NOW - 60, + new OsRelease('ubuntu', ['debian'], 'Ubuntu', 'Ubuntu 24.04.3 LTS', '24.04', 'noble'), + '6.8.0-45-generic', + 'x86_64', + new Environment(Environment::KIND_HOST, null, 'kvm'), + new SupportStatus(false, '2029-05-31'), + 'apt', + [ + new PendingUpdate('openssl', '1', '2', true, advisories: ['USN-7012-1'], since: self::NOW - 4 * 86400), + new PendingUpdate('tzdata', '1', '2', false), + ], + OsStatus::SOURCE_SYSTEM, + true, + 5, + [new HeldPackage('mariadb-server', 'apt-mark hold')], + self::NOW - 7200, + rebootReasons: [], + restartServices: null, + failedUnits: ['nginx.service'], + timeSynchronized: true, + )); + + $summary = $this->service->monitoringSummary(); + $encoded = (string)json_encode($summary); + + $this->assertSame('warning', $summary['level']); + $this->assertSame(['classified' => true, 'security' => 1, 'total' => 7, 'pendingDays' => 4, 'held' => 1], $summary['updates']); + $this->assertFalse($summary['rebootRequired']); + $this->assertNull($summary['servicesNeedRestart'], 'Unknown is null, never 0'); + $this->assertSame(1, $summary['failedUnits']); + $this->assertSame(2, $summary['listsAgeHours']); + $this->assertSame('supported', $summary['support']['state']); + foreach (['openssl', 'USN-7012', 'mariadb', 'nginx'] as $secret) { + $this->assertStringNotContainsString($secret, $encoded); + } + } + + public function testUnclassifiedDistributionsReportNoSecurityCount(): void { + $this->store->method('load')->willReturn(new OsStatus(OsStatus::STATE_OK, self::NOW, updates: [new PendingUpdate('linux', '1', '2', false)], securityClassified: false)); + + $this->assertNull($this->service->monitoringSummary()['updates']['security']); + } + + public function testBeforeTheFirstCheck(): void { + $this->assertSame(['state' => 'pending', 'level' => 'unknown'], $this->service->monitoringSummary()); + } + public function testOutdatedWhenPackagesChangedAfterTheCheck(): void { $status = new OsStatus(OsStatus::STATE_OK, self::NOW - 600, packageDatabase: '/var/lib/dpkg/status'); diff --git a/tests/lib/OsStatus/OsStatusStoreTest.php b/tests/lib/OsStatus/OsStatusStoreTest.php index 23558cf59..dd053345d 100644 --- a/tests/lib/OsStatus/OsStatusStoreTest.php +++ b/tests/lib/OsStatus/OsStatusStoreTest.php @@ -240,4 +240,10 @@ public function testThePhpSampleLeavesTheCollectorsFirstSeenTimesAlone(): void { $this->assertSame($stored, $this->values[ConfigLexicon::CACHED_OS_FIRST_SEEN]); $this->assertSame(1120, $this->store->phpOutdatedSince(), 'Still outdated, so still counting from the first sighting'); } + + public function testNotifiedLevels(): void { + $this->store->setNotifiedLevels(['updates' => 'warning']); + + $this->assertSame(['updates' => 'warning'], $this->store->notifiedLevels()); + } } diff --git a/tests/lib/OsStatus/OsVerdictTest.php b/tests/lib/OsStatus/OsVerdictTest.php index 9d951e179..64bca5fff 100644 --- a/tests/lib/OsStatus/OsVerdictTest.php +++ b/tests/lib/OsStatus/OsVerdictTest.php @@ -61,6 +61,7 @@ public function testAllGood(): void { $verdict = $this->evaluate($this->osStatus(), new PhpProcessSample(self::NOW, 'fpm-fcgi', [], 'php8.3-fpm', null)); $this->assertSame(Concern::LEVEL_OK, $verdict->level()); + $this->assertSame([], $verdict->atLeast(Concern::LEVEL_INFO)); } public static function dataSecurityUpdates(): array { @@ -194,6 +195,7 @@ public function testHealth(): void { $this->assertSame(Concern::LEVEL_INFO, $verdict->get(Concern::AUTOMATIC_UPDATES)?->level); $this->assertSame(Concern::LEVEL_WARNING, $verdict->get(Concern::HELPER)?->level); $this->assertSame(Concern::LEVEL_WARNING, $verdict->level()); + $this->assertSame(Concern::FAILED_UNITS, $verdict->atLeast(Concern::LEVEL_WARNING)[0]->id); } public function testAHelperReportFromTheFutureIsWorthMentioning(): void { From 8de20e29271daf157bf705f184de6104fd5c1e17 Mon Sep 17 00:00:00 2001 From: Frank Karlitschek Date: Fri, 23 Oct 2026 05:41:54 +0200 Subject: [PATCH 02/16] fix(os): fill in the release and date in support sentences The support titles used {name}-style placeholders with a keyed parameter array, but IL10N::t() in PHP only runs vsprintf(). Every install showed "{name} receives security updates until {date}" in the setup check, the notification and the dashboard widget. They now use positional %1$s placeholders. The tests did not notice because their IL10N mocks replaced {key} placeholders, which the real class never does. A shared FakeL10N now formats like the server, %n and vsprintf() only, and the setup check, notifier and widget tests use it too. Assisted-by: ClaudeCode:claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 Signed-off-by: Frank Karlitschek --- lib/OsStatus/Verdict/ConcernTexts.php | 28 ++++++++------- tests/bootstrap.php | 3 +- tests/lib/OsStatus/ConcernTextsTest.php | 27 +++++++++------ tests/lib/OsStatus/EntryPointsTest.php | 6 ++-- tests/lib/OsStatus/FakeL10N.php | 44 ++++++++++++++++++++++++ tests/lib/OsStatus/OsSetupChecksTest.php | 21 ++++++++--- 6 files changed, 97 insertions(+), 32 deletions(-) create mode 100644 tests/lib/OsStatus/FakeL10N.php diff --git a/lib/OsStatus/Verdict/ConcernTexts.php b/lib/OsStatus/Verdict/ConcernTexts.php index 06757fac3..7c54eccc6 100644 --- a/lib/OsStatus/Verdict/ConcernTexts.php +++ b/lib/OsStatus/Verdict/ConcernTexts.php @@ -165,22 +165,24 @@ private function supportTitle(Concern $concern, OsStatus $status, IL10N $l): str $extendedEnds = $this->date(self::string($concern, 'extendedEnds'), $l); return match (self::string($concern, 'state')) { - SupportStatus::STATE_ROLLING => $l->t('{name} is a rolling release', ['name' => $name]), - SupportStatus::STATE_ENDED => $l->t('{name} no longer receives security updates since {date}', ['name' => $name, 'date' => $ends]), - SupportStatus::STATE_EXTENDED_AVAILABLE => $l->t('Standard support for {name} has ended. Security updates continue only with {programme} until {date}', [ - 'name' => $name, - 'programme' => $programme ?? '', - 'date' => $extendedEnds, + SupportStatus::STATE_ROLLING => $l->t('%1$s is a rolling release', [$name]), + SupportStatus::STATE_ENDED => $l->t('%1$s no longer receives security updates since %2$s', [$name, $ends]), + // TRANSLATORS: %1$s is the release, %2$s a paid support programme, e.g. "Ubuntu Pro", %3$s the date it ends + SupportStatus::STATE_EXTENDED_AVAILABLE => $l->t('Standard support for %1$s has ended. Security updates continue only with %2$s until %3$s', [ + $name, + $programme ?? '', + $extendedEnds, ]), - SupportStatus::STATE_EXTENDED => $l->t('{name} receives security updates through {programme} until {date}', [ - 'name' => $name, - 'programme' => $programme ?? '', - 'date' => $ends, + // TRANSLATORS: %1$s is the release, %2$s a paid support programme, e.g. "Ubuntu Pro", %3$s the date it ends + SupportStatus::STATE_EXTENDED => $l->t('%1$s receives security updates through %2$s until %3$s', [ + $name, + $programme ?? '', + $ends, ]), SupportStatus::STATE_SUPPORTED => $concern->level === Concern::LEVEL_OK - ? $l->t('{name} receives security updates until {date}', ['name' => $name, 'date' => $ends]) - : $l->t('Security updates for {name} end on {date}', ['name' => $name, 'date' => $ends]), - default => $l->t('The support status of {name} is unknown', ['name' => $name]), + ? $l->t('%1$s receives security updates until %2$s', [$name, $ends]) + : $l->t('Security updates for %1$s end on %2$s', [$name, $ends]), + default => $l->t('The support status of %1$s is unknown', [$name]), }; } diff --git a/tests/bootstrap.php b/tests/bootstrap.php index eb6a610f4..ddb477edc 100644 --- a/tests/bootstrap.php +++ b/tests/bootstrap.php @@ -19,5 +19,6 @@ Server::get(IAppManager::class)->loadApp('serverinfo'); -// Shared test double, not a test case, so PHPUnit does not load it on its own. +// Shared test doubles, not test cases, so PHPUnit does not load them on its own. require_once __DIR__ . '/lib/OsStatus/FakeHost.php'; +require_once __DIR__ . '/lib/OsStatus/FakeL10N.php'; diff --git a/tests/lib/OsStatus/ConcernTextsTest.php b/tests/lib/OsStatus/ConcernTextsTest.php index 3651210a0..bae83913e 100644 --- a/tests/lib/OsStatus/ConcernTextsTest.php +++ b/tests/lib/OsStatus/ConcernTextsTest.php @@ -16,6 +16,7 @@ use OCA\ServerInfo\OsStatus\Verdict\ConcernTexts; use OCP\IDateTimeFormatter; use OCP\IL10N; +use PHPUnit\Framework\Attributes\DataProvider; class ConcernTextsTest extends \Test\TestCase { private ConcernTexts $texts; @@ -28,16 +29,7 @@ protected function setUp(): void { $formatter->method('formatDate')->willReturnCallback(static fn (int $time): string => gmdate('Y-m-d', $time)); $this->texts = new ConcernTexts($formatter); - $this->l = $this->createMock(IL10N::class); - $replace = static function (string $text, array $parameters): string { - foreach ($parameters as $key => $value) { - $text = str_replace('{' . $key . '}', (string)$value, $text); - } - - return $text; - }; - $this->l->method('t')->willReturnCallback(static fn (string $text, array $parameters = []): string => $replace($text, $parameters)); - $this->l->method('n')->willReturnCallback(static fn (string $one, string $many, int $count, array $parameters = []): string => str_replace('%n', (string)$count, $replace($count === 1 ? $one : $many, $parameters))); + $this->l = new FakeL10N(); $this->status = new OsStatus(OsStatus::STATE_OK, 1, new OsRelease('ubuntu', ['debian'], 'Ubuntu', 'Ubuntu 22.04.5 LTS', '22.04', 'jammy')); } @@ -89,6 +81,21 @@ public function testSupport(): void { $this->assertSame('Ubuntu 22.04.5 LTS no longer receives security updates since 2024-06-30', $this->texts->title($ended, $this->status, $this->l)); } + public static function dataSupportTitles(): array { + return [ + 'supported' => [Concern::LEVEL_OK, ['state' => SupportStatus::STATE_SUPPORTED, 'ends' => '2027-05-31'], 'Ubuntu 22.04.5 LTS receives security updates until 2027-05-31'], + 'ending soon' => [Concern::LEVEL_WARNING, ['state' => SupportStatus::STATE_SUPPORTED, 'ends' => '2027-05-31'], 'Security updates for Ubuntu 22.04.5 LTS end on 2027-05-31'], + 'extended' => [Concern::LEVEL_OK, ['state' => SupportStatus::STATE_EXTENDED, 'ends' => '2032-04-09', 'extended' => 'ubuntu-pro'], 'Ubuntu 22.04.5 LTS receives security updates through Ubuntu Pro until 2032-04-09'], + 'rolling' => [Concern::LEVEL_OK, ['state' => SupportStatus::STATE_ROLLING], 'Ubuntu 22.04.5 LTS is a rolling release'], + 'unknown' => [Concern::LEVEL_UNKNOWN, ['state' => SupportStatus::STATE_UNKNOWN], 'The support status of Ubuntu 22.04.5 LTS is unknown'], + ]; + } + + #[DataProvider('dataSupportTitles')] + public function testEverySupportTitleIsFilledIn(string $level, array $facts, string $expected): void { + $this->assertSame($expected, $this->texts->title(new Concern(Concern::SUPPORT, $level, $facts), $this->status, $this->l)); + } + public function testRestartPrefersThePhpMessage(): void { $concern = new Concern(Concern::RESTART, Concern::LEVEL_WARNING, ['php' => true, 'services' => 2, 'days' => 4]); diff --git a/tests/lib/OsStatus/EntryPointsTest.php b/tests/lib/OsStatus/EntryPointsTest.php index 6616291da..c80ca0ebe 100644 --- a/tests/lib/OsStatus/EntryPointsTest.php +++ b/tests/lib/OsStatus/EntryPointsTest.php @@ -31,15 +31,13 @@ class EntryPointsTest extends \Test\TestCase { private OsStatusService&MockObject $service; - private IL10N&MockObject $l10n; + private IL10N $l10n; private IURLGenerator&MockObject $urlGenerator; protected function setUp(): void { parent::setUp(); $this->service = $this->createMock(OsStatusService::class); - $this->l10n = $this->createMock(IL10N::class); - $this->l10n->method('t')->willReturnArgument(0); - $this->l10n->method('n')->willReturnCallback(static fn (string $one, string $many, int $count): string => str_replace('%n', (string)$count, $count === 1 ? $one : $many)); + $this->l10n = new FakeL10N(); $this->urlGenerator = $this->createMock(IURLGenerator::class); $this->urlGenerator->method('linkToRouteAbsolute')->willReturn('https://cloud.example.org/settings/admin/serverinfo'); $this->urlGenerator->method('imagePath')->willReturnCallback(static fn (string $app, string $image): string => "/apps/$app/img/$image"); diff --git a/tests/lib/OsStatus/FakeL10N.php b/tests/lib/OsStatus/FakeL10N.php new file mode 100644 index 000000000..2e88837da --- /dev/null +++ b/tests/lib/OsStatus/FakeL10N.php @@ -0,0 +1,44 @@ +service = $this->createMock(OsStatusService::class); - $this->l10n = $this->createMock(IL10N::class); - $this->l10n->method('t')->willReturnArgument(0); - $this->l10n->method('n')->willReturnCallback(static fn (string $one, string $many, int $count): string => str_replace('%n', (string)$count, $count === 1 ? $one : $many)); + $this->l10n = new FakeL10N(); $this->urlGenerator = $this->createMock(IURLGenerator::class); $this->urlGenerator->method('linkToRoute')->willReturn('/settings/admin/serverinfo'); } @@ -100,6 +100,19 @@ public function testUnknownIsInfo(): void { $this->assertSame(SetupResult::INFO, (new OsSupportCheck($this->l10n, $this->urlGenerator, $this->service, $this->texts()))->run()->getSeverity()); } + public function testTheSupportSentenceIsFilledIn(): void { + $formatter = $this->createMock(IDateTimeFormatter::class); + $formatter->method('formatDate')->willReturn('May 31, 2029'); + $this->given( + new OsStatus(OsStatus::STATE_OK, 1, new OsRelease('ubuntu', ['debian'], 'Ubuntu', 'Ubuntu 24.04.3 LTS', '24.04', 'noble')), + new Concern(Concern::SUPPORT, Concern::LEVEL_OK, ['state' => SupportStatus::STATE_SUPPORTED, 'ends' => '2029-05-31']), + ); + + $result = (new OsSupportCheck($this->l10n, $this->urlGenerator, $this->service, new ConcernTexts($formatter)))->run(); + + $this->assertSame('Ubuntu 24.04.3 LTS receives security updates until May 31, 2029.', $result->getDescription()); + } + public function testRestrictedMode(): void { $this->given(OsStatus::restricted(1)); From 776b5ff824e0e1b83036b320bd4a4e01e3c90f7a Mon Sep 17 00:00:00 2001 From: Frank Karlitschek Date: Fri, 23 Oct 2026 05:42:19 +0200 Subject: [PATCH 03/16] fix(os): skip non-string items in list facts of a concern ConcernTexts read list facts such as the failed units or reboot kinds as they were stored and passed them to implode() and count(). A stored status is read back from JSON, so a nested array or a number in such a list raised "Array to string conversion" or miscounted. Only string items are used now. Assisted-by: ClaudeCode:claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 Signed-off-by: Frank Karlitschek --- lib/OsStatus/Verdict/ConcernTexts.php | 4 ++-- tests/lib/OsStatus/ConcernTextsTest.php | 13 +++++++++++++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/lib/OsStatus/Verdict/ConcernTexts.php b/lib/OsStatus/Verdict/ConcernTexts.php index 7c54eccc6..3517546d3 100644 --- a/lib/OsStatus/Verdict/ConcernTexts.php +++ b/lib/OsStatus/Verdict/ConcernTexts.php @@ -199,12 +199,12 @@ private static function string(Concern $concern, string $fact): ?string { } /** - * @return list + * @return list the fact's string items; anything else is skipped */ private static function strings(Concern $concern, string $fact): array { $value = $concern->facts[$fact] ?? null; - return is_array($value) ? $value : []; + return is_array($value) ? array_values(array_filter($value, is_string(...))) : []; } private function date(?string $date, IL10N $l): string { diff --git a/tests/lib/OsStatus/ConcernTextsTest.php b/tests/lib/OsStatus/ConcernTextsTest.php index bae83913e..faa989f5c 100644 --- a/tests/lib/OsStatus/ConcernTextsTest.php +++ b/tests/lib/OsStatus/ConcernTextsTest.php @@ -102,4 +102,17 @@ public function testRestartPrefersThePhpMessage(): void { $this->assertSame('PHP is running outdated libraries', $this->texts->title($concern, $this->status, $this->l)); $this->assertSame('Another 2 services need a restart too. Pending for 4 days.', $this->texts->detail($concern, $this->l)); } + + public function testFailedUnitsSkipItemsThatAreNotNames(): void { + $concern = new Concern(Concern::FAILED_UNITS, Concern::LEVEL_WARNING, ['units' => ['nginx.service', 5, ['nested'], null, 'cron.service']]); + + $this->assertSame('2 system services have failed', $this->texts->title($concern, $this->status, $this->l)); + $this->assertSame('nginx.service, cron.service', $this->texts->detail($concern, $this->l)); + } + + public function testRebootKindsSkipItemsThatAreNotKinds(): void { + $concern = new Concern(Concern::REBOOT, Concern::LEVEL_WARNING, ['kinds' => [['kernel'], 'kernel']]); + + $this->assertSame('A newer kernel is installed.', $this->texts->detail($concern, $this->l)); + } } From f7b8d8293cb53e4db4473b5d8c77f85e11c8bc5a Mon Sep 17 00:00:00 2001 From: Frank Karlitschek Date: Fri, 23 Oct 2026 05:42:37 +0200 Subject: [PATCH 04/16] fix(os): link the setup checks to the absolute page URL Setup check results are also read by occ setupchecks and the API, where a path without the host is useless. The notifier and the widget already use linkToRouteAbsolute(); the setup checks now do too. Assisted-by: ClaudeCode:claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 Signed-off-by: Frank Karlitschek --- lib/SetupChecks/AbstractOsCheck.php | 2 +- tests/lib/OsStatus/OsSetupChecksTest.php | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/SetupChecks/AbstractOsCheck.php b/lib/SetupChecks/AbstractOsCheck.php index 50c5a7ab3..cdc2b758b 100644 --- a/lib/SetupChecks/AbstractOsCheck.php +++ b/lib/SetupChecks/AbstractOsCheck.php @@ -97,7 +97,7 @@ private function linkParameter(): array { 'type' => 'highlight', 'id' => 'serverinfo-os', 'name' => $this->l10n->t('Show the operating system status…'), - 'link' => $this->urlGenerator->linkToRoute('settings.AdminSettings.index', ['section' => Application::APP_ID]) . '#os', + 'link' => $this->urlGenerator->linkToRouteAbsolute('settings.AdminSettings.index', ['section' => Application::APP_ID]) . '#os', ], ]; } diff --git a/tests/lib/OsStatus/OsSetupChecksTest.php b/tests/lib/OsStatus/OsSetupChecksTest.php index 28d204c06..9fea5f3c8 100644 --- a/tests/lib/OsStatus/OsSetupChecksTest.php +++ b/tests/lib/OsStatus/OsSetupChecksTest.php @@ -35,7 +35,7 @@ protected function setUp(): void { $this->service = $this->createMock(OsStatusService::class); $this->l10n = new FakeL10N(); $this->urlGenerator = $this->createMock(IURLGenerator::class); - $this->urlGenerator->method('linkToRoute')->willReturn('/settings/admin/serverinfo'); + $this->urlGenerator->method('linkToRouteAbsolute')->willReturn('https://cloud.example.org/settings/admin/serverinfo'); } private function texts(): ConcernTexts { @@ -69,7 +69,7 @@ public function testTheWorstConcernOfTheCheckDecides(): void { $this->assertSame(SetupResult::ERROR, $result->getSeverity()); $this->assertStringContainsString('Package lists were last refreshed 40 days ago', (string)$result->getDescription()); $this->assertStringEndsWith('{link}', (string)$result->getDescription()); - $this->assertSame('/settings/admin/serverinfo#os', $result->getDescriptionParameters()['link']['link'] ?? null); + $this->assertSame('https://cloud.example.org/settings/admin/serverinfo#os', $result->getDescriptionParameters()['link']['link'] ?? null); } public function testRebootCheck(): void { From 5c4a0c5f1f51be4520f6e1177089d8494ee9164e Mon Sep 17 00:00:00 2001 From: Frank Karlitschek Date: Fri, 23 Oct 2026 05:57:21 +0200 Subject: [PATCH 05/16] fix(os): send the operating system block in the API only on request The monitoring API added server.os to every response, while app and server updates are opt-in with skipApps and skipUpdate. Existing token holders, often third-party monitoring services, would suddenly have received the distribution, security counts and the kernel version. skipOs now defaults to true, so the block is sent with skipOs=false. The kernel string is no longer part of the block at all: its exact version is what someone matching CVEs needs, and the README promised no versions. The README now says how to request the block. Assisted-by: ClaudeCode:claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 Signed-off-by: Frank Karlitschek --- README.md | 8 ++++---- lib/Controller/ApiController.php | 2 +- lib/OsStatus/OsStatusService.php | 7 +++---- tests/lib/ApiControllerTest.php | 15 +++++++-------- tests/lib/OsStatus/OsStatusServiceTest.php | 3 ++- 5 files changed, 17 insertions(+), 18 deletions(-) diff --git a/README.md b/README.md index 6a40bfdc7..4c3a06b49 100644 --- a/README.md +++ b/README.md @@ -143,11 +143,12 @@ https:///ocs/v2.php/apps/serverinfo/api/v1/info - To request the information in JSON append the url parameter `format=json` - Use the url parameter `skipUpdate=true` to omit server updates. - Use the url parameter `skipApps=true` to omit app updates (including available app updates will send an external request to the app store). -- Use the url parameter `skipOs=true` to omit the operating system status. +- Use the url parameter `skipOs=false` to include the operating system status. The operating system status in `server.os` holds counts and flags only, never -package names or advisories, so a leaked token does not reveal what is -unpatched. Values that could not be determined are `null`: +package names, package or kernel versions, or advisories, so a leaked token +does not reveal what is unpatched. Values that could not be determined are +`null`: ```json "os": { @@ -156,7 +157,6 @@ unpatched. Values that could not be determined are `null`: "collectedAt": 1792411200, "outdated": false, "name": "Ubuntu 24.04.3 LTS", - "kernel": "6.8.0-45-generic", "environment": "host", "container": null, "updates": { "classified": true, "security": 7, "total": 23, "pendingDays": 5, "held": 0 }, diff --git a/lib/Controller/ApiController.php b/lib/Controller/ApiController.php index 75ec31c9a..568b6de5b 100644 --- a/lib/Controller/ApiController.php +++ b/lib/Controller/ApiController.php @@ -81,7 +81,7 @@ private function checkAuthorized(): bool { * @PublicPage * @BruteForceProtection(action=serverinfo) */ - public function info(bool $skipApps = true, bool $skipUpdate = true, bool $skipOs = false): DataResponse { + public function info(bool $skipApps = true, bool $skipUpdate = true, bool $skipOs = true): DataResponse { if (!$this->checkAuthorized()) { $response = new DataResponse(['message' => 'Unauthorized']); $response->throttle(); diff --git a/lib/OsStatus/OsStatusService.php b/lib/OsStatus/OsStatusService.php index dcd91305a..1796ce7af 100644 --- a/lib/OsStatus/OsStatusService.php +++ b/lib/OsStatus/OsStatusService.php @@ -99,9 +99,9 @@ public function isOutdated(OsStatus $status): bool { /** * Counts and flags for the monitoring API. Deliberately no package names, - * versions or advisories: a leaked monitoring token must not become a list - * of unpatched software. Unknown values are null, never 0 or false, so a - * monitoring system cannot mistake "unknown" for "healthy". + * package or kernel versions, or advisories: a leaked monitoring token must + * not become a list of unpatched software. Unknown values are null, never 0 + * or false, so a monitoring system cannot mistake "unknown" for "healthy". */ public function monitoringSummary(): array { $status = $this->getStatus(); @@ -123,7 +123,6 @@ public function monitoringSummary(): array { 'collectedAt' => $status->collectedAt, 'outdated' => $this->isOutdated($status), 'name' => $status->release?->prettyName, - 'kernel' => $status->kernel, 'environment' => $status->environment->kind, 'container' => $status->environment->container, 'updates' => [ diff --git a/tests/lib/ApiControllerTest.php b/tests/lib/ApiControllerTest.php index 64d06ed6e..7635d2ae3 100644 --- a/tests/lib/ApiControllerTest.php +++ b/tests/lib/ApiControllerTest.php @@ -206,9 +206,7 @@ public function testInfo() { $this->sessionStatistics->expects($this->once()) ->method('getSessionStatistics') ->willReturn(['some_user' => 'some_value']); - $this->osStatus->expects($this->once()) - ->method('monitoringSummary') - ->willReturn(['state' => 'ok', 'level' => 'warning']); + $this->osStatus->expects($this->never())->method('monitoringSummary'); $response = $this->getController($userSession)->info(); @@ -225,22 +223,23 @@ public function testInfo() { 'php' => ['some_php' => 'some_value'], 'fpm' => ['some_fpm' => 'some_value'], 'database' => ['some_database' => 'some_value'], - 'os' => ['state' => 'ok', 'level' => 'warning'], ], 'activeUsers' => ['some_user' => 'some_value'], ], $response->getData()); } - public function testInfoWithoutTheOperatingSystem(): void { + public function testInfoWithTheOperatingSystemOnRequest(): void { $userSession = $this->createMock(IUserSession::class); $user = $this->createMock(IUser::class); $user->method('getUID')->willReturn('admin'); $userSession->method('getUser')->willReturn($user); $this->groupManager->method('isAdmin')->willReturn(true); - $this->osStatus->expects($this->never())->method('monitoringSummary'); + $this->osStatus->expects($this->once()) + ->method('monitoringSummary') + ->willReturn(['state' => 'ok', 'level' => 'warning']); - $response = $this->getController($userSession)->info(skipOs: true); + $response = $this->getController($userSession)->info(skipOs: false); - $this->assertArrayNotHasKey('os', $response->getData()['server']); + $this->assertSame(['state' => 'ok', 'level' => 'warning'], $response->getData()['server']['os']); } } diff --git a/tests/lib/OsStatus/OsStatusServiceTest.php b/tests/lib/OsStatus/OsStatusServiceTest.php index 814310e30..fec114bfd 100644 --- a/tests/lib/OsStatus/OsStatusServiceTest.php +++ b/tests/lib/OsStatus/OsStatusServiceTest.php @@ -88,7 +88,8 @@ public function testTheMonitoringSummaryHasCountsButNoNames(): void { $this->assertSame(1, $summary['failedUnits']); $this->assertSame(2, $summary['listsAgeHours']); $this->assertSame('supported', $summary['support']['state']); - foreach (['openssl', 'USN-7012', 'mariadb', 'nginx'] as $secret) { + $this->assertArrayNotHasKey('kernel', $summary); + foreach (['openssl', 'USN-7012', 'mariadb', 'nginx', '6.8.0-45'] as $secret) { $this->assertStringNotContainsString($secret, $encoded); } } From 21204d1e0e8d9de07277ee7c70fe9a84bb5cb497 Mon Sep 17 00:00:00 2001 From: Frank Karlitschek Date: Fri, 23 Oct 2026 05:58:22 +0200 Subject: [PATCH 06/16] perf(os): read and judge the stored status once per request One Overview load ran the three setup checks, and each read up to 8 MB of /proc/self/maps for its PHP sample and decoded and evaluated the stored status again, about six times in all. The notifier did the same for every notification it rendered. The store already reads the status once per request. OsStatusService now also reuses the verdict for the same status object and samples the PHP process at most once per instance; a new sample clears the reused verdict, since the verdict depends on it. With that, IPreloadableNotifier would add nothing: every notification in a request already shares one verdict. Assisted-by: ClaudeCode:claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 Signed-off-by: Frank Karlitschek --- lib/OsStatus/OsStatusService.php | 22 ++++++++++++--- tests/lib/OsStatus/OsStatusServiceTest.php | 31 +++++++++++++++++++++- tests/lib/OsStatus/OsStatusStoreTest.php | 1 + 3 files changed, 50 insertions(+), 4 deletions(-) diff --git a/lib/OsStatus/OsStatusService.php b/lib/OsStatus/OsStatusService.php index 1796ce7af..3a5186941 100644 --- a/lib/OsStatus/OsStatusService.php +++ b/lib/OsStatus/OsStatusService.php @@ -28,6 +28,10 @@ * @psalm-api */ class OsStatusService { + private bool $sampled = false; + private ?OsStatus $evaluated = null; + private ?Verdict $verdictOfEvaluated = null; + public function __construct( private OsStatusStore $store, private OsVerdict $verdict, @@ -45,16 +49,18 @@ public function getStatus(): ?OsStatus { /** * Looks at the PHP process serving this request and remembers what it - * found. Cheap: one read of /proc/self/maps. + * found. Cheap: one read of /proc/self/maps, at most once per request. */ public function samplePhpProcess(): void { - if (!$this->phpProcessProbe->canSample()) { + if ($this->sampled || !$this->phpProcessProbe->canSample()) { return; } + $this->sampled = true; $sample = $this->phpProcessProbe->sample($this->store->load()?->release, $this->timeFactory->getTime()); if ($sample !== null) { $this->store->savePhpSample($sample); + $this->evaluated = null; } } @@ -66,13 +72,20 @@ public function getThresholds(): Thresholds { return Thresholds::load($this->appConfig); } + /** + * Judges the given status, or the stored one. The verdict of the same + * status is reused, because one page load asks for it several times. + */ public function evaluate(?OsStatus $status = null): ?Verdict { $status ??= $this->store->load(); if ($status === null) { return null; } + if ($status === $this->evaluated) { + return $this->verdictOfEvaluated; + } - return $this->verdict->evaluate( + $this->verdictOfEvaluated = $this->verdict->evaluate( $status, $this->store->loadPhpSample(), $this->store->phpOutdatedSince(), @@ -81,6 +94,9 @@ public function evaluate(?OsStatus $status = null): ?Verdict { UpdateOsStatus::runsFromSystemCron($this->serverConfig), UpdateOsStatus::interval($this->serverConfig), ); + $this->evaluated = $status; + + return $this->verdictOfEvaluated; } /** diff --git a/tests/lib/OsStatus/OsStatusServiceTest.php b/tests/lib/OsStatus/OsStatusServiceTest.php index fec114bfd..d11824025 100644 --- a/tests/lib/OsStatus/OsStatusServiceTest.php +++ b/tests/lib/OsStatus/OsStatusServiceTest.php @@ -129,8 +129,37 @@ public function testFreshnessFollowsTheCronModeAndTheJobInterval(): void { $this->assertTrue($fresh->facts['systemCron']); $this->mode = 'webcron'; - $stale = $this->service->evaluate($status)?->get(Concern::FRESHNESS); + $stale = $this->service->evaluate(new OsStatus(OsStatus::STATE_OK, self::NOW - 2 * 86400))?->get(Concern::FRESHNESS); $this->assertSame(Concern::LEVEL_WARNING, $stale?->level); $this->assertFalse($stale->facts['systemCron']); } + + public function testThePhpProcessIsSampledOncePerRequest(): void { + $this->host->files['/proc/self/maps'] = ''; + $this->store->expects($this->once())->method('savePhpSample'); + + $this->service->samplePhpProcess(); + $this->service->samplePhpProcess(); + } + + public function testTheVerdictOfTheSameStatusIsReused(): void { + $stored = new OsStatus(OsStatus::STATE_OK, self::NOW); + $this->store->method('load')->willReturn($stored); + $this->store->expects($this->exactly(2))->method('loadPhpSample')->willReturn(null); + + $verdict = $this->service->evaluate(); + $this->assertSame($verdict, $this->service->evaluate($stored)); + $this->assertNotSame($verdict, $this->service->evaluate(new OsStatus(OsStatus::STATE_OK, self::NOW))); + } + + public function testASampleTakenAfterwardsIsJudged(): void { + $stored = new OsStatus(OsStatus::STATE_OK, self::NOW); + $this->store->method('load')->willReturn($stored); + $this->host->files['/proc/self/maps'] = ''; + + $before = $this->service->evaluate(); + $this->service->samplePhpProcess(); + + $this->assertNotSame($before, $this->service->evaluate()); + } } diff --git a/tests/lib/OsStatus/OsStatusStoreTest.php b/tests/lib/OsStatus/OsStatusStoreTest.php index dd053345d..23610119b 100644 --- a/tests/lib/OsStatus/OsStatusStoreTest.php +++ b/tests/lib/OsStatus/OsStatusStoreTest.php @@ -246,4 +246,5 @@ public function testNotifiedLevels(): void { $this->assertSame(['updates' => 'warning'], $this->store->notifiedLevels()); } + } From 6ddd0f05bcb6e3e771d62893626e7c78e6d3a0b0 Mon Sep 17 00:00:00 2001 From: Frank Karlitschek Date: Fri, 23 Oct 2026 05:59:07 +0200 Subject: [PATCH 07/16] fix(os): keep notifications of concerns that have not cleared OsNotifier threw AlreadyProcessedException whenever the concern was not a problem at render time, including when it was unknown, missing from the verdict, or when the stored status could not be read, for example after an upgrade changed its format. The notification manager then deleted that admin's notification, while os_notified_levels still remembered the level, so it was never sent again. It is now only dropped when the concern is known and no longer a problem (Concern::isResolved()), or when the sender has withdrawn it. Otherwise it renders a generic "The operating system needs attention" with the link to the page. Assisted-by: ClaudeCode:claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 Signed-off-by: Frank Karlitschek --- lib/OsStatus/Notifications/OsNotifier.php | 16 ++++-- lib/OsStatus/Verdict/Concern.php | 8 +++ tests/lib/OsStatus/EntryPointsTest.php | 68 +++++++++++++++++++++++ 3 files changed, 87 insertions(+), 5 deletions(-) diff --git a/lib/OsStatus/Notifications/OsNotifier.php b/lib/OsStatus/Notifications/OsNotifier.php index ac722ec9c..f076df0a7 100644 --- a/lib/OsStatus/Notifications/OsNotifier.php +++ b/lib/OsStatus/Notifications/OsNotifier.php @@ -57,7 +57,7 @@ public function prepare(INotification $notification, string $languageCode): INot $status = $this->service->getStatus(); $concern = $status !== null ? $this->service->evaluate($status)?->get($notification->getObjectId()) : null; - if ($status === null || $concern === null || !$concern->isProblem()) { + if ($concern !== null && $concern->isResolved()) { throw new AlreadyProcessedException(); } @@ -65,13 +65,19 @@ public function prepare(INotification $notification, string $languageCode): INot $link = $this->urlGenerator->linkToRouteAbsolute('settings.AdminSettings.index', ['section' => Application::APP_ID]) . '#os'; $notification - ->setParsedSubject($this->texts->title($concern, $status, $l)) ->setLink($link) ->setIcon($this->urlGenerator->getAbsoluteURL($this->urlGenerator->imagePath(Application::APP_ID, 'app-dark.svg'))); - $detail = $this->texts->detail($concern, $l); - if ($detail !== '') { - $notification->setParsedMessage($detail); + // Without a judgeable concern the notification stays: deleting it here + // would lose it for good while the concern may still be live. + if ($status === null || $concern === null || !$concern->isProblem()) { + $notification->setParsedSubject($l->t('The operating system needs attention')); + } else { + $notification->setParsedSubject($this->texts->title($concern, $status, $l)); + $detail = $this->texts->detail($concern, $l); + if ($detail !== '') { + $notification->setParsedMessage($detail); + } } $action = $notification->createAction(); diff --git a/lib/OsStatus/Verdict/Concern.php b/lib/OsStatus/Verdict/Concern.php index 68cb49d07..7a42118bd 100644 --- a/lib/OsStatus/Verdict/Concern.php +++ b/lib/OsStatus/Verdict/Concern.php @@ -72,4 +72,12 @@ public static function lower(string $level): string { public function isProblem(): bool { return self::rank($this->level) >= self::rank(self::LEVEL_WARNING); } + + /** + * Known and not a problem. Unknown is never resolved: a probe failing once + * says nothing about whether the problem went away. + */ + public function isResolved(): bool { + return $this->level !== self::LEVEL_UNKNOWN && !$this->isProblem(); + } } diff --git a/tests/lib/OsStatus/EntryPointsTest.php b/tests/lib/OsStatus/EntryPointsTest.php index c80ca0ebe..0f279864e 100644 --- a/tests/lib/OsStatus/EntryPointsTest.php +++ b/tests/lib/OsStatus/EntryPointsTest.php @@ -25,8 +25,10 @@ use OCP\IUserSession; use OCP\L10N\IFactory; use OCP\Notification\AlreadyProcessedException; +use OCP\Notification\IAction; use OCP\Notification\INotification; use OCP\Notification\UnknownNotificationException; +use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\MockObject\MockObject; class EntryPointsTest extends \Test\TestCase { @@ -79,6 +81,72 @@ public function testNotifierDropsResolvedConcerns(): void { $this->notifier($sender)->prepare($this->notification('serverinfo', 'os', 'updates'), 'en'); } + /** + * A notification of a live concern that remembers its parsed subject. + */ + private function liveNotification(string $objectId, ?string &$subject): INotification&MockObject { + $notification = $this->notification('serverinfo', 'os', $objectId); + foreach (['setLink', 'setIcon', 'setParsedMessage', 'addParsedAction'] as $setter) { + $notification->method($setter)->willReturnSelf(); + } + $notification->method('setParsedSubject')->willReturnCallback(function (string $text) use (&$subject, $notification): INotification { + $subject = $text; + + return $notification; + }); + $action = $this->createMock(IAction::class); + foreach (['setParsedLabel', 'setLink', 'setPrimary'] as $setter) { + $action->method($setter)->willReturnSelf(); + } + $notification->method('createAction')->willReturn($action); + + return $notification; + } + + private function currentSender(): OsNotificationSender&MockObject { + $sender = $this->createMock(OsNotificationSender::class); + $sender->method('isCurrent')->willReturn(true); + + return $sender; + } + + public function testNotifierRendersALiveConcern(): void { + $this->service->method('getStatus')->willReturn(new OsStatus(OsStatus::STATE_OK, 1)); + $this->service->method('evaluate')->willReturn(new Verdict([ + new Concern(Concern::UPDATES, Concern::LEVEL_WARNING, ['classified' => true, 'security' => 4, 'other' => 0]), + ])); + + $this->notifier($this->currentSender())->prepare($this->liveNotification('updates', $subject), 'en'); + + $this->assertSame('4 security updates pending', $subject); + } + + public static function dataNotJudgeable(): array { + return [ + 'unknown' => [new OsStatus(OsStatus::STATE_OK, 1), [new Concern(Concern::UPDATES, Concern::LEVEL_UNKNOWN)]], + 'missing from the verdict' => [new OsStatus(OsStatus::STATE_OK, 1), []], + 'no readable status' => [null, []], + ]; + } + + #[DataProvider('dataNotJudgeable')] + public function testNotifierKeepsConcernsItCannotJudge(?OsStatus $status, array $concerns): void { + $this->service->method('getStatus')->willReturn($status); + $this->service->method('evaluate')->willReturn($status === null ? null : new Verdict($concerns)); + + $this->notifier($this->currentSender())->prepare($this->liveNotification('updates', $subject), 'en'); + + $this->assertSame('The operating system needs attention', $subject); + } + + public function testNotifierDropsNotificationsTheSenderWithdrew(): void { + $sender = $this->createMock(OsNotificationSender::class); + $sender->method('isCurrent')->willReturn(false); + + $this->expectException(AlreadyProcessedException::class); + $this->notifier($sender)->prepare($this->notification('serverinfo', 'os', 'updates'), 'en'); + } + private function widget(bool $admin): ServerHealthWidget { $user = $this->createMock(IUser::class); $user->method('getUID')->willReturn('alice'); From 4dbeb1768662e47ed7844dbc4a1e6e347128fbb4 Mon Sep 17 00:00:00 2001 From: Frank Karlitschek Date: Fri, 23 Oct 2026 06:01:28 +0200 Subject: [PATCH 08/16] fix(os): notify later admins once and never notify twice Three gaps in OsNotificationSender::sync(): - Admins added to the admin group while a concern was live never got its notification, because a concern notified only on escalation. Every run now notifies enabled admins who were not notified since the last escalation and have no notification of the concern. Who was notified is kept in os_notified_users, so an admin who dismissed the notification does not get it again until the concern escalates. - The job and occ serverinfo:os-status --refresh could sync at the same time and notify twice. sync() now holds an exclusive lock from ILockingProvider and skips when another process holds it. Each admin is also checked with IManager::getCount() before a notification, and the subject is now the level, so an escalation withdraws only the notifications of the other level instead of all of them; a process with outdated notified levels therefore re-sends nothing. - markProcessed() failures escaped and failed the job. Withdrawals are now caught and logged, and a concern whose withdrawal failed stays remembered so the next run tries again. A stored status that cannot be read no longer withdraws anything, for the same reason the notifier keeps such notifications. Assisted-by: ClaudeCode:claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 Signed-off-by: Frank Karlitschek --- lib/Config/ConfigLexicon.php | 8 + .../Notifications/OsNotificationSender.php | 125 ++++++++++---- lib/OsStatus/OsStatusStore.php | 21 +++ tests/lib/OsStatus/NotificationsTest.php | 152 ++++++++++++++++-- tests/lib/OsStatus/OsStatusStoreTest.php | 5 + 5 files changed, 268 insertions(+), 43 deletions(-) diff --git a/lib/Config/ConfigLexicon.php b/lib/Config/ConfigLexicon.php index b9f8643e7..2a181d1c5 100644 --- a/lib/Config/ConfigLexicon.php +++ b/lib/Config/ConfigLexicon.php @@ -32,6 +32,7 @@ class ConfigLexicon implements ILexicon { public const CACHED_OS_PHP_PROCESS = 'cached_os_php_process'; public const CACHED_OS_PHP_OUTDATED_SINCE = 'cached_os_php_outdated_since'; public const OS_NOTIFIED_LEVELS = 'os_notified_levels'; + public const OS_NOTIFIED_USERS = 'os_notified_users'; public const JOB_INTERVAL_OS_STATUS = 'job_interval_os_status'; public const OS_HELPER_REPORT_PATH = 'os_helper_report_path'; public const OS_NOTIFICATIONS = 'os_notifications'; @@ -116,6 +117,13 @@ public function getAppConfigs(): array { 'Alert level each operating system concern was last notified at, so admins are told once per escalation', lazy: true, ), + new Entry( + self::OS_NOTIFIED_USERS, + ValueType::ARRAY, + [], + 'Admins each operating system concern was notified to since it last escalated, so a dismissed notification is not sent again', + lazy: true, + ), new Entry( self::JOB_INTERVAL_OS_STATUS, ValueType::INT, diff --git a/lib/OsStatus/Notifications/OsNotificationSender.php b/lib/OsStatus/Notifications/OsNotificationSender.php index f8ca290e7..67e4b486b 100644 --- a/lib/OsStatus/Notifications/OsNotificationSender.php +++ b/lib/OsStatus/Notifications/OsNotificationSender.php @@ -17,6 +17,8 @@ use OCP\AppFramework\Services\IAppConfig; use OCP\AppFramework\Utility\ITimeFactory; use OCP\IGroupManager; +use OCP\Lock\ILockingProvider; +use OCP\Lock\LockedException; use OCP\Notification\IManager; use OCP\Notification\INotification; use Psr\Log\LoggerInterface; @@ -25,12 +27,16 @@ * Tells admins when an operating system concern gets worse. * * Each concern notifies once per escalation (warning, then error), not on - * every run, and its notification is withdrawn once it is resolved. + * every run, and its notification is withdrawn once it is resolved. Admins + * who join while a concern is live get it on the next run; admins who + * dismissed it do not get it again until it escalates. * * @psalm-api */ class OsNotificationSender { public const OBJECT_TYPE = 'os'; + /** Held while syncing, so the job and occ serverinfo:os-status --refresh never notify twice */ + private const LOCK = 'serverinfo/os-notifications'; public function __construct( private OsStatusService $service, @@ -39,15 +45,36 @@ public function __construct( private IGroupManager $groupManager, private IAppConfig $appConfig, private ITimeFactory $timeFactory, + private ILockingProvider $lockingProvider, private LoggerInterface $logger, ) { } public function sync(): void { + try { + $this->lockingProvider->acquireLock(self::LOCK, ILockingProvider::LOCK_EXCLUSIVE); + } catch (LockedException) { + $this->logger->debug('Operating system notifications are being synced by another process'); + return; + } + + try { + $this->syncLocked(); + } finally { + $this->lockingProvider->releaseLock(self::LOCK, ILockingProvider::LOCK_EXCLUSIVE); + } + } + + private function syncLocked(): void { $enabled = $this->appConfig->getAppValueBool(ConfigLexicon::OS_NOTIFICATIONS, true); $verdict = $enabled ? $this->service->evaluate() : null; - $notified = $this->store->notifiedLevels(); - $changed = false; + if ($enabled && $verdict === null) { + // Without a readable status nothing is known about the notified concerns. + return; + } + + $notified = $levelsBefore = $this->store->notifiedLevels(); + $recipients = $usersBefore = $this->store->notifiedUsers(); foreach (Concern::NOTIFIABLE as $id) { $concern = $verdict?->get($id); @@ -56,63 +83,105 @@ public function sync(): void { if ($concern === null || !$concern->isProblem()) { // Unknown is not resolved: a probe failing once must not // withdraw a standing warning. - if ($previous !== null && ($verdict === null || $concern === null || $concern->level !== Concern::LEVEL_UNKNOWN)) { - $this->withdraw($id); - unset($notified[$id]); - $changed = true; + if ($previous !== null && ($concern === null || $concern->isResolved()) && $this->withdraw($id)) { + unset($notified[$id], $recipients[$id]); } continue; } $level = $concern->level; - if ($previous === null || Concern::rank($level) > Concern::rank($previous)) { - $this->withdraw($id); - $this->notifyAdmins($concern); - $notified[$id] = $level; - $changed = true; - } elseif (Concern::rank($level) < Concern::rank($previous)) { - // Remember the lower level so a later escalation is announced again. - $notified[$id] = $level; - $changed = true; + $escalated = $previous === null || Concern::rank($level) > Concern::rank($previous); + if ($escalated) { + foreach ([Concern::LEVEL_WARNING, Concern::LEVEL_ERROR] as $other) { + if ($other !== $level) { + $this->withdraw($id, $other); + } + } } + $recipients[$id] = $this->notifyAdmins($concern, $escalated ? [] : ($recipients[$id] ?? [])); + // A lower level is remembered too, so a later escalation reaches + // admins who dismissed the notification in between. + $notified[$id] = $level; } - if ($changed) { + if ($notified !== $levelsBefore) { $this->store->setNotifiedLevels($notified); } + if ($recipients !== $usersBefore) { + $this->store->setNotifiedUsers($recipients); + } } - private function notifyAdmins(Concern $concern): void { + /** + * Notifies every enabled admin who has not been notified of this concern + * yet and has no notification of it. + * + * @param list $notified admins already notified + * @return list admins notified now or before + */ + private function notifyAdmins(Concern $concern, array $notified): array { $admins = $this->groupManager->get('admin'); if ($admins === null) { - return; + return $notified; } + $recipients = []; $time = $this->timeFactory->getDateTime(); foreach ($admins->getUsers() as $user) { if (!$user->isEnabled()) { continue; } + $uid = $user->getUID(); try { - $notification = $this->notificationManager->createNotification() - ->setApp(Application::APP_ID) - ->setUser($user->getUID()) - ->setDateTime($time) - ->setObject(self::OBJECT_TYPE, $concern->id) - ->setSubject($concern->id, ['level' => $concern->level]); - $this->notificationManager->notify($notification); + if (!in_array($uid, $notified, true) && !$this->hasNotification($uid, $concern->id)) { + $notification = $this->notificationManager->createNotification() + ->setApp(Application::APP_ID) + ->setUser($uid) + ->setDateTime($time) + ->setObject(self::OBJECT_TYPE, $concern->id) + ->setSubject($concern->level); + $this->notificationManager->notify($notification); + } + $recipients[] = $uid; } catch (\Throwable $e) { $this->logger->warning('Could not notify an admin about the operating system status', ['exception' => $e]); } } + + return $recipients; } - private function withdraw(string $id): void { + private function hasNotification(string $uid, string $id): bool { $notification = $this->notificationManager->createNotification() ->setApp(Application::APP_ID) + ->setUser($uid) ->setObject(self::OBJECT_TYPE, $id); - $this->notificationManager->markProcessed($notification); + + return $this->notificationManager->getCount($notification) > 0; + } + + /** + * Removes the notifications of a concern, or only those of one level. + * + * @return bool false when they could not be removed + */ + private function withdraw(string $id, ?string $level = null): bool { + try { + $notification = $this->notificationManager->createNotification() + ->setApp(Application::APP_ID) + ->setObject(self::OBJECT_TYPE, $id); + if ($level !== null) { + $notification->setSubject($level); + } + $this->notificationManager->markProcessed($notification); + + return true; + } catch (\Throwable $e) { + $this->logger->warning('Could not withdraw an operating system notification', ['exception' => $e]); + + return false; + } } /** diff --git a/lib/OsStatus/OsStatusStore.php b/lib/OsStatus/OsStatusStore.php index 59864b439..9aa26bd8f 100644 --- a/lib/OsStatus/OsStatusStore.php +++ b/lib/OsStatus/OsStatusStore.php @@ -231,6 +231,27 @@ public function setNotifiedLevels(array $levels): void { $this->appConfig->setAppValueArray(ConfigLexicon::OS_NOTIFIED_LEVELS, $levels); } + /** + * @return array> concern id => user ids notified since it last escalated + */ + public function notifiedUsers(): array { + $users = []; + foreach ($this->appConfig->getAppValueArray(ConfigLexicon::OS_NOTIFIED_USERS) as $id => $uids) { + if (is_string($id) && is_array($uids)) { + $users[$id] = array_values(array_filter($uids, is_string(...))); + } + } + + return $users; + } + + /** + * @param array> $users + */ + public function setNotifiedUsers(array $users): void { + $this->appConfig->setAppValueArray(ConfigLexicon::OS_NOTIFIED_USERS, $users); + } + /** * @param list|null $scope */ diff --git a/tests/lib/OsStatus/NotificationsTest.php b/tests/lib/OsStatus/NotificationsTest.php index d837ad5a9..8a6c0f394 100644 --- a/tests/lib/OsStatus/NotificationsTest.php +++ b/tests/lib/OsStatus/NotificationsTest.php @@ -19,6 +19,8 @@ use OCP\IGroup; use OCP\IGroupManager; use OCP\IUser; +use OCP\Lock\ILockingProvider; +use OCP\Lock\LockedException; use OCP\Notification\IManager; use OCP\Notification\INotification; use PHPUnit\Framework\MockObject\MockObject; @@ -27,11 +29,20 @@ class NotificationsTest extends \Test\TestCase { private OsStatusService&MockObject $service; private IManager&MockObject $manager; + private ILockingProvider&MockObject $locking; + private LoggerInterface&MockObject $logger; private bool $enabled = true; + private bool $withdrawFails = false; /** @var array */ private array $notified = []; - /** @var list "notify:id:level:user" and "withdraw:id" */ + /** @var array> */ + private array $notifiedUsers = []; + /** @var list what admins have in their notifications */ + private array $inbox = []; + /** @var list "notify:id:level:user", "withdraw:id" and "withdraw:id:level" */ private array $events = []; + /** @var list */ + private array $admins = []; private OsNotificationSender $sender; protected function setUp(): void { @@ -43,24 +54,29 @@ protected function setUp(): void { $store->method('setNotifiedLevels')->willReturnCallback(function (array $levels): void { $this->notified = $levels; }); + $store->method('notifiedUsers')->willReturnCallback(fn (): array => $this->notifiedUsers); + $store->method('setNotifiedUsers')->willReturnCallback(function (array $users): void { + $this->notifiedUsers = $users; + }); $this->manager = $this->createMock(IManager::class); $this->manager->method('createNotification')->willReturnCallback(fn (): INotification => $this->notification()); $this->manager->method('notify')->willReturnCallback(function (INotification $n): void { - $this->events[] = 'notify:' . $n->getObjectId() . ':' . $n->getSubjectParameters()['level'] . ':' . $n->getUser(); + $this->events[] = 'notify:' . $n->getObjectId() . ':' . $n->getSubject() . ':' . $n->getUser(); + $this->inbox[] = ['user' => $n->getUser(), 'id' => $n->getObjectId(), 'level' => $n->getSubject()]; }); $this->manager->method('markProcessed')->willReturnCallback(function (INotification $n): void { - $this->events[] = 'withdraw:' . $n->getObjectId(); + if ($this->withdrawFails) { + throw new \RuntimeException('The database is gone'); + } + $this->events[] = 'withdraw:' . $n->getObjectId() . ($n->getSubject() !== '' ? ':' . $n->getSubject() : ''); + $this->inbox = array_values(array_filter($this->inbox, static fn (array $held): bool => !self::filterMatches($held, $n))); }); + $this->manager->method('getCount')->willReturnCallback(fn (INotification $n): int => count(array_filter($this->inbox, static fn (array $held): bool => self::filterMatches($held, $n)))); - $admin = $this->createMock(IUser::class); - $admin->method('getUID')->willReturn('admin'); - $admin->method('isEnabled')->willReturn(true); - $disabled = $this->createMock(IUser::class); - $disabled->method('getUID')->willReturn('former'); - $disabled->method('isEnabled')->willReturn(false); + $this->admins = [$this->user('admin', true), $this->user('former', false)]; $group = $this->createMock(IGroup::class); - $group->method('getUsers')->willReturn([$admin, $disabled]); + $group->method('getUsers')->willReturnCallback(fn (): array => $this->admins); $groups = $this->createMock(IGroupManager::class); $groups->method('get')->with('admin')->willReturn($group); @@ -70,7 +86,30 @@ protected function setUp(): void { $time = $this->createMock(ITimeFactory::class); $time->method('getDateTime')->willReturn(new \DateTime('@1792411200')); - $this->sender = new OsNotificationSender($this->service, $store, $this->manager, $groups, $config, $time, $this->createMock(LoggerInterface::class)); + $this->locking = $this->createMock(ILockingProvider::class); + $this->logger = $this->createMock(LoggerInterface::class); + + $this->sender = new OsNotificationSender($this->service, $store, $this->manager, $groups, $config, $time, $this->locking, $this->logger); + } + + private function user(string $uid, bool $enabled): IUser { + $user = $this->createMock(IUser::class); + $user->method('getUID')->willReturn($uid); + $user->method('isEnabled')->willReturn($enabled); + + return $user; + } + + /** + * Whether a held notification matches a filter, the way the notifications + * app matches: fields left empty match anything. + * + * @param array{user: string, id: string, level: string} $held + */ + private static function filterMatches(array $held, INotification $filter): bool { + return ($filter->getUser() === '' || $filter->getUser() === $held['user']) + && $filter->getObjectId() === $held['id'] + && ($filter->getSubject() === '' || $filter->getSubject() === $held['level']); } /** @@ -93,8 +132,8 @@ private function notification(): INotification { $notification->method('getUser')->willReturnCallback(static function () use (&$values): string { return $values['setUser'][0] ?? ''; }); - $notification->method('getSubjectParameters')->willReturnCallback(static function () use (&$values): array { - return $values['setSubject'][1] ?? []; + $notification->method('getSubject')->willReturnCallback(static function () use (&$values): string { + return $values['setSubject'][0] ?? ''; }); return $notification; @@ -114,8 +153,9 @@ public function testAWarningNotifiesEnabledAdminsOnce(): void { $this->sender->sync(); $this->sender->sync(); - $this->assertSame(['withdraw:updates', 'notify:updates:warning:admin'], $this->events); + $this->assertSame(['withdraw:updates:error', 'notify:updates:warning:admin'], $this->events); $this->assertSame(['updates' => 'warning'], $this->notified); + $this->assertSame(['updates' => ['admin']], $this->notifiedUsers); } public function testOnlyNotifiableConcernsNotify(): void { @@ -128,15 +168,19 @@ public function testOnlyNotifiableConcernsNotify(): void { public function testEscalationNotifiesAgain(): void { $this->notified = ['updates' => 'warning']; + $this->notifiedUsers = ['updates' => ['admin']]; + $this->inbox = [['user' => 'admin', 'id' => 'updates', 'level' => 'warning']]; $this->verdict(Concern::LEVEL_ERROR); $this->sender->sync(); - $this->assertSame(['withdraw:updates', 'notify:updates:error:admin'], $this->events); + $this->assertSame(['withdraw:updates:warning', 'notify:updates:error:admin'], $this->events); + $this->assertSame([['user' => 'admin', 'id' => 'updates', 'level' => 'error']], $this->inbox); } public function testDeEscalationIsRememberedSilently(): void { $this->notified = ['updates' => 'error']; + $this->inbox = [['user' => 'admin', 'id' => 'updates', 'level' => 'error']]; $this->verdict(Concern::LEVEL_WARNING); $this->sender->sync(); @@ -173,4 +217,82 @@ public function testTurningNotificationsOffWithdrawsThem(): void { $this->assertSame(['withdraw:updates'], $this->events); } + + public function testAdminsWhoJoinLaterAreNotified(): void { + $this->notified = ['updates' => 'warning']; + $this->notifiedUsers = ['updates' => ['admin']]; + $this->inbox = [['user' => 'admin', 'id' => 'updates', 'level' => 'warning']]; + $this->admins[] = $this->user('newcomer', true); + $this->verdict(Concern::LEVEL_WARNING); + + $this->sender->sync(); + + $this->assertSame(['notify:updates:warning:newcomer'], $this->events); + $this->assertSame(['updates' => ['admin', 'newcomer']], $this->notifiedUsers); + } + + public function testADismissedNotificationIsNotSentAgain(): void { + $this->notified = ['updates' => 'warning']; + $this->notifiedUsers = ['updates' => ['admin']]; + $this->verdict(Concern::LEVEL_WARNING); + + $this->sender->sync(); + + $this->assertSame([], $this->events); + } + + public function testAdminsWhoHaveItAlreadyAreNotNotifiedTwice(): void { + // Another process notified after this one read the notified levels. + $this->inbox = [['user' => 'admin', 'id' => 'updates', 'level' => 'warning']]; + $this->verdict(Concern::LEVEL_WARNING); + + $this->sender->sync(); + + $this->assertSame(['withdraw:updates:error'], $this->events); + $this->assertSame(['updates' => ['admin']], $this->notifiedUsers); + } + + public function testOnlyOneProcessSyncsAtATime(): void { + $this->locking->method('acquireLock')->willThrowException(new LockedException('serverinfo/os-notifications')); + $this->locking->expects($this->never())->method('releaseLock'); + $this->verdict(Concern::LEVEL_WARNING); + + $this->sender->sync(); + + $this->assertSame([], $this->events); + $this->assertSame([], $this->notified); + } + + public function testTheLockIsReleased(): void { + $this->locking->expects($this->once())->method('acquireLock')->with('serverinfo/os-notifications', ILockingProvider::LOCK_EXCLUSIVE); + $this->locking->expects($this->once())->method('releaseLock')->with('serverinfo/os-notifications', ILockingProvider::LOCK_EXCLUSIVE); + $this->verdict(Concern::LEVEL_WARNING); + + $this->sender->sync(); + } + + public function testAFailedWithdrawalIsLoggedAndRetried(): void { + $this->notified = ['updates' => 'warning']; + $this->withdrawFails = true; + $this->logger->expects($this->once())->method('warning'); + $this->verdict(Concern::LEVEL_OK); + + $this->sender->sync(); + $this->assertSame(['updates' => 'warning'], $this->notified, 'Kept, so the next run withdraws it'); + + $this->withdrawFails = false; + $this->sender->sync(); + $this->assertSame(['withdraw:updates'], $this->events); + $this->assertSame([], $this->notified); + } + + public function testAnUnreadableStatusWithdrawsNothing(): void { + $this->notified = ['updates' => 'warning']; + $this->service->method('evaluate')->willReturn(null); + + $this->sender->sync(); + + $this->assertSame([], $this->events); + $this->assertSame(['updates' => 'warning'], $this->notified); + } } diff --git a/tests/lib/OsStatus/OsStatusStoreTest.php b/tests/lib/OsStatus/OsStatusStoreTest.php index 23610119b..7c42030e9 100644 --- a/tests/lib/OsStatus/OsStatusStoreTest.php +++ b/tests/lib/OsStatus/OsStatusStoreTest.php @@ -247,4 +247,9 @@ public function testNotifiedLevels(): void { $this->assertSame(['updates' => 'warning'], $this->store->notifiedLevels()); } + public function testNotifiedUsersSkipWhatIsNotAUserId(): void { + $this->values['os_notified_users'] = ['updates' => ['admin', 7, ['x'], 'ops'], 'reboot' => 'admin', 3 => ['admin']]; + + $this->assertSame(['updates' => ['admin', 'ops']], $this->store->notifiedUsers()); + } } From 0a8cbc4cf83bdd0f10ff9b4a8f5378f61dd79567 Mon Sep 17 00:00:00 2001 From: Frank Karlitschek Date: Fri, 23 Oct 2026 06:02:20 +0200 Subject: [PATCH 09/16] fix(os): hide the server health widget where nothing is checked On systems other than Linux and in restricted mode the widget said "The operating system has not been checked yet." forever, since the status there is never more than "unsupported" or "restricted". ServerHealthWidget::isEnabled() now also asks the new OsStatusService::isApplicable(), which reads restricted_mode and the OS family but no stored status, so the dashboard stays cheap. Should the widget still be asked for items, it shows the sentence the setup checks use; both now come from ConcernTexts::unavailable(). Assisted-by: ClaudeCode:claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 Signed-off-by: Frank Karlitschek --- lib/Dashboard/ServerHealthWidget.php | 7 +++++-- lib/OsStatus/OsStatusService.php | 9 +++++++++ lib/OsStatus/Verdict/ConcernTexts.php | 12 ++++++++++++ lib/SetupChecks/AbstractOsCheck.php | 9 +++------ tests/lib/OsStatus/EntryPointsTest.php | 22 ++++++++++++++++++++++ tests/lib/OsStatus/OsStatusServiceTest.php | 14 ++++++++++++++ 6 files changed, 65 insertions(+), 8 deletions(-) diff --git a/lib/Dashboard/ServerHealthWidget.php b/lib/Dashboard/ServerHealthWidget.php index 08d021191..558cf5507 100644 --- a/lib/Dashboard/ServerHealthWidget.php +++ b/lib/Dashboard/ServerHealthWidget.php @@ -88,7 +88,7 @@ public function load(): void { public function isEnabled(): bool { $user = $this->userSession->getUser(); - return $user !== null && $this->groupManager->isAdmin($user->getUID()); + return $user !== null && $this->groupManager->isAdmin($user->getUID()) && $this->service->isApplicable(); } #[\Override] @@ -108,9 +108,12 @@ public function getItemsV2(string $userId, ?string $since = null, int $limit = 7 } $status = $this->service->getStatus(); - if ($status === null || $status->state !== OsStatus::STATE_OK) { + if ($status === null) { return new WidgetItems([], $this->l10n->t('The operating system has not been checked yet.')); } + if ($status->state !== OsStatus::STATE_OK) { + return new WidgetItems([], $this->texts->unavailable($status, $this->l10n) ?? ''); + } $verdict = $this->service->evaluate($status); $items = []; diff --git a/lib/OsStatus/OsStatusService.php b/lib/OsStatus/OsStatusService.php index 3a5186941..93617bddd 100644 --- a/lib/OsStatus/OsStatusService.php +++ b/lib/OsStatus/OsStatusService.php @@ -43,6 +43,15 @@ public function __construct( ) { } + /** + * Whether the operating system of this server is checked at all: not in + * restricted mode, and only on Linux. Reads no stored status. + */ + public function isApplicable(): bool { + return $this->appConfig->getAppValueString('restricted_mode', 'no') !== 'yes' + && $this->host->osFamily() === 'Linux'; + } + public function getStatus(): ?OsStatus { return $this->store->load(); } diff --git a/lib/OsStatus/Verdict/ConcernTexts.php b/lib/OsStatus/Verdict/ConcernTexts.php index 3517546d3..8561598e8 100644 --- a/lib/OsStatus/Verdict/ConcernTexts.php +++ b/lib/OsStatus/Verdict/ConcernTexts.php @@ -36,6 +36,18 @@ public function __construct( ) { } + /** + * Why a stored status has no concerns: restricted mode or a system other + * than Linux. Null when it has them. + */ + public function unavailable(OsStatus $status, IL10N $l): ?string { + return match ($status->state) { + OsStatus::STATE_RESTRICTED => $l->t('Not checked, because the Monitoring app runs in restricted mode.'), + OsStatus::STATE_UNSUPPORTED => $l->t('Only Linux systems are checked.'), + default => null, + }; + } + /** * A short headline, e.g. "7 security updates pending". */ diff --git a/lib/SetupChecks/AbstractOsCheck.php b/lib/SetupChecks/AbstractOsCheck.php index cdc2b758b..123510fdd 100644 --- a/lib/SetupChecks/AbstractOsCheck.php +++ b/lib/SetupChecks/AbstractOsCheck.php @@ -10,7 +10,6 @@ namespace OCA\ServerInfo\SetupChecks; use OCA\ServerInfo\AppInfo\Application; -use OCA\ServerInfo\OsStatus\OsStatus; use OCA\ServerInfo\OsStatus\OsStatusService; use OCA\ServerInfo\OsStatus\Verdict\Concern; use OCA\ServerInfo\OsStatus\Verdict\ConcernTexts; @@ -56,11 +55,9 @@ public function run(): SetupResult { $this->l10n->t('The operating system has not been checked yet. It is checked by the background jobs when they run from system cron, or with occ serverinfo:os-status --refresh.'), ); } - if ($status->state === OsStatus::STATE_RESTRICTED) { - return SetupResult::success($this->l10n->t('Not checked, because the Monitoring app runs in restricted mode.')); - } - if ($status->state === OsStatus::STATE_UNSUPPORTED) { - return SetupResult::success($this->l10n->t('Only Linux systems are checked.')); + $unavailable = $this->texts->unavailable($status, $this->l10n); + if ($unavailable !== null) { + return SetupResult::success($unavailable); } $verdict = $this->service->evaluate($status); diff --git a/tests/lib/OsStatus/EntryPointsTest.php b/tests/lib/OsStatus/EntryPointsTest.php index 0f279864e..dfb83178f 100644 --- a/tests/lib/OsStatus/EntryPointsTest.php +++ b/tests/lib/OsStatus/EntryPointsTest.php @@ -167,6 +167,28 @@ public function testWidgetIsForAdminsOnly(): void { $this->assertSame([], $widget->getItemsV2('alice')->getItems()); } + public function testWidgetIsHiddenWhereNothingIsChecked(): void { + $this->service->method('isApplicable')->willReturn(false); + $this->service->expects($this->never())->method('getStatus'); + + $this->assertFalse($this->widget(true)->isEnabled()); + } + + public function testWidgetIsShownToAdminsOfCheckedSystems(): void { + $this->service->method('isApplicable')->willReturn(true); + + $this->assertTrue($this->widget(true)->isEnabled()); + } + + public function testWidgetSaysWhyNothingWasChecked(): void { + $this->service->method('getStatus')->willReturn(OsStatus::restricted(1)); + + $items = $this->widget(true)->getItemsV2('alice'); + + $this->assertSame([], $items->getItems()); + $this->assertSame('Not checked, because the Monitoring app runs in restricted mode.', $items->getEmptyContentMessage()); + } + public function testWidgetListsProblemsFirst(): void { $this->service->method('getStatus')->willReturn(new OsStatus(OsStatus::STATE_OK, 1)); $this->service->method('evaluate')->willReturn(new Verdict([ diff --git a/tests/lib/OsStatus/OsStatusServiceTest.php b/tests/lib/OsStatus/OsStatusServiceTest.php index d11824025..a33cdb775 100644 --- a/tests/lib/OsStatus/OsStatusServiceTest.php +++ b/tests/lib/OsStatus/OsStatusServiceTest.php @@ -32,6 +32,7 @@ class OsStatusServiceTest extends \Test\TestCase { private FakeHost $host; private string $mode = 'cron'; private OsStatusService $service; + private string $restrictedMode = 'no'; protected function setUp(): void { parent::setUp(); @@ -43,6 +44,7 @@ protected function setUp(): void { 'os_reboot_error_days' => 30, 'os_restart_warning_days' => 3, 'os_eol_warning_days' => 90, 'os_lists_warning_days' => 7, 'os_lists_error_days' => 30, default => 0, }); + $config->method('getAppValueString')->willReturnCallback(fn (string $key, string $default): string => $key === 'restricted_mode' ? $this->restrictedMode : $default); $time = $this->createMock(ITimeFactory::class); $time->method('getTime')->willReturn(self::NOW); @@ -162,4 +164,16 @@ public function testASampleTakenAfterwardsIsJudged(): void { $this->assertNotSame($before, $this->service->evaluate()); } + + public function testOnlyLinuxOutsideRestrictedModeIsChecked(): void { + $this->store->expects($this->never())->method('load'); + $this->assertTrue($this->service->isApplicable()); + + $this->restrictedMode = 'yes'; + $this->assertFalse($this->service->isApplicable()); + + $this->restrictedMode = 'no'; + $this->host->family = 'BSD'; + $this->assertFalse($this->service->isApplicable()); + } } From 11321e5d4b91a290b65a853ddb46df750a9619ff Mon Sep 17 00:00:00 2001 From: Frank Karlitschek Date: Fri, 23 Oct 2026 06:12:53 +0200 Subject: [PATCH 10/16] fix(os): say in the setup checks and the widget when the status is old The stored status is only renewed by the background job under system cron. When cron stops, or background jobs run by AJAX or webcron, the Overview and the dashboard kept presenting a weeks-old status as the current one. The freshness concern now has its own headline and sentence: how long ago the system was checked, and either "switch background jobs to Cron" or "check that the cron job runs". Each setup check appends it and is at least a warning while the status is old, since all three judge the same stored status. The widget lists it like the other concerns. It sends no notification: the job that would send one is the one that stopped. Assisted-by: ClaudeCode:claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 Signed-off-by: Frank Karlitschek --- lib/Dashboard/ServerHealthWidget.php | 1 + lib/OsStatus/Verdict/ConcernTexts.php | 24 ++++++++++++++++++++ lib/SetupChecks/AbstractOsCheck.php | 12 +++++++++- tests/lib/OsStatus/ConcernTextsTest.php | 11 ++++++++++ tests/lib/OsStatus/OsSetupChecksTest.php | 28 ++++++++++++++++++++++++ 5 files changed, 75 insertions(+), 1 deletion(-) diff --git a/lib/Dashboard/ServerHealthWidget.php b/lib/Dashboard/ServerHealthWidget.php index 558cf5507..c3e9f3e47 100644 --- a/lib/Dashboard/ServerHealthWidget.php +++ b/lib/Dashboard/ServerHealthWidget.php @@ -38,6 +38,7 @@ class ServerHealthWidget implements IAPIWidgetV2, IConditionalWidget, IIconWidge private const SHOWN = [ Concern::UPDATES, Concern::REBOOT, Concern::RESTART, Concern::SUPPORT, Concern::PACKAGE_LISTS, Concern::FAILED_UNITS, Concern::TIME_SYNC, Concern::AUTOMATIC_UPDATES, + Concern::FRESHNESS, ]; public function __construct( diff --git a/lib/OsStatus/Verdict/ConcernTexts.php b/lib/OsStatus/Verdict/ConcernTexts.php index 8561598e8..1d5717372 100644 --- a/lib/OsStatus/Verdict/ConcernTexts.php +++ b/lib/OsStatus/Verdict/ConcernTexts.php @@ -78,6 +78,7 @@ public function title(Concern $concern, OsStatus $status, IL10N $l): string { ? $l->t('The system clock is synchronized') : $l->t('The system clock is not synchronized'), Concern::HELPER => $l->t('The root helper report needs attention'), + Concern::FRESHNESS => $this->freshnessTitle($concern, $l), default => $concern->id, }; } @@ -146,11 +147,34 @@ public function detail(Concern $concern, IL10N $l): string { return $units === [] ? '' : implode(', ', $units); case Concern::TIME_SYNC: return $concern->level === Concern::LEVEL_OK ? '' : $l->t('Two-factor codes, federation and object storage requests fail when the clock drifts.'); + case Concern::FRESHNESS: + if ($concern->level !== Concern::LEVEL_WARNING) { + return ''; + } + + return ($facts['systemCron'] ?? true) === false + ? $l->t('The operating system is only checked when background jobs run from system cron, so everything shown here may be out of date. Switch background jobs to Cron in the basic settings.') + : $l->t('The background job that checks the operating system has not run for a while, so everything shown here may be out of date. Check that the system cron job for Nextcloud runs.'); default: return ''; } } + private function freshnessTitle(Concern $concern, IL10N $l): string { + if ($concern->level === Concern::LEVEL_UNKNOWN) { + return $l->t('The operating system has not been checked yet'); + } + $age = self::int($concern, 'ageSeconds') ?? 0; + if ($concern->level === Concern::LEVEL_OK) { + return $l->t('The operating system status is current'); + } + if ($age < 86400) { + return $l->n('The operating system was last checked %n hour ago', 'The operating system was last checked %n hours ago', intdiv($age, 3600)); + } + + return $l->n('The operating system was last checked %n day ago', 'The operating system was last checked %n days ago', intdiv($age, 86400)); + } + private function updatesTitle(Concern $concern, IL10N $l): string { $classified = ($concern->facts['classified'] ?? false) === true; $security = self::int($concern, 'security') ?? 0; diff --git a/lib/SetupChecks/AbstractOsCheck.php b/lib/SetupChecks/AbstractOsCheck.php index 123510fdd..d49307f28 100644 --- a/lib/SetupChecks/AbstractOsCheck.php +++ b/lib/SetupChecks/AbstractOsCheck.php @@ -73,9 +73,19 @@ public function run(): SetupResult { $worst = $concerns[0]; $text = trim($this->texts->title($worst, $status, $this->l10n) . '. ' . $this->texts->detail($worst, $this->l10n)); + $level = $worst->level; + + // Every check judges the stored status, so an old one makes each of them doubtful. + $freshness = $verdict?->get(Concern::FRESHNESS); + if ($freshness !== null && $freshness->level === Concern::LEVEL_WARNING) { + $text .= ' ' . $this->texts->title($freshness, $status, $this->l10n) . '. ' . $this->texts->detail($freshness, $this->l10n); + if (Concern::rank($level) < Concern::rank(Concern::LEVEL_WARNING)) { + $level = Concern::LEVEL_WARNING; + } + } $description = $text . ' {link}'; - return match ($worst->level) { + return match ($level) { Concern::LEVEL_ERROR => SetupResult::error($description, null, $this->linkParameter()), Concern::LEVEL_WARNING => SetupResult::warning($description, null, $this->linkParameter()), Concern::LEVEL_INFO => SetupResult::info($description, null, $this->linkParameter()), diff --git a/tests/lib/OsStatus/ConcernTextsTest.php b/tests/lib/OsStatus/ConcernTextsTest.php index faa989f5c..e7427a33a 100644 --- a/tests/lib/OsStatus/ConcernTextsTest.php +++ b/tests/lib/OsStatus/ConcernTextsTest.php @@ -96,6 +96,17 @@ public function testEverySupportTitleIsFilledIn(string $level, array $facts, str $this->assertSame($expected, $this->texts->title(new Concern(Concern::SUPPORT, $level, $facts), $this->status, $this->l)); } + public function testFreshness(): void { + $texts = $this->texts; + $status = $this->status; + + $this->assertSame('The operating system status is current', $texts->title(new Concern(Concern::FRESHNESS, Concern::LEVEL_OK, ['ageSeconds' => 60]), $status, $this->l)); + $this->assertSame('', $texts->detail(new Concern(Concern::FRESHNESS, Concern::LEVEL_OK, ['systemCron' => true]), $this->l)); + $this->assertSame('The operating system has not been checked yet', $texts->title(new Concern(Concern::FRESHNESS, Concern::LEVEL_UNKNOWN), $status, $this->l)); + $this->assertSame('The operating system was last checked 1 day ago', $texts->title(new Concern(Concern::FRESHNESS, Concern::LEVEL_WARNING, ['ageSeconds' => 86400 + 60]), $status, $this->l)); + $this->assertStringStartsWith('The operating system is only checked when background jobs run from system cron', $texts->detail(new Concern(Concern::FRESHNESS, Concern::LEVEL_WARNING, ['systemCron' => false]), $this->l)); + } + public function testRestartPrefersThePhpMessage(): void { $concern = new Concern(Concern::RESTART, Concern::LEVEL_WARNING, ['php' => true, 'services' => 2, 'days' => 4]); diff --git a/tests/lib/OsStatus/OsSetupChecksTest.php b/tests/lib/OsStatus/OsSetupChecksTest.php index 9fea5f3c8..7e8784b9e 100644 --- a/tests/lib/OsStatus/OsSetupChecksTest.php +++ b/tests/lib/OsStatus/OsSetupChecksTest.php @@ -72,6 +72,34 @@ public function testTheWorstConcernOfTheCheckDecides(): void { $this->assertSame('https://cloud.example.org/settings/admin/serverinfo#os', $result->getDescriptionParameters()['link']['link'] ?? null); } + public function testAnOldStatusMakesEveryCheckAWarning(): void { + $this->given( + new OsStatus(OsStatus::STATE_OK, 1), + new Concern(Concern::SUPPORT, Concern::LEVEL_OK, ['state' => 'rolling']), + new Concern(Concern::FRESHNESS, Concern::LEVEL_WARNING, ['collectedAt' => 1, 'ageSeconds' => 3 * 86400, 'systemCron' => true]), + ); + + $result = (new OsSupportCheck($this->l10n, $this->urlGenerator, $this->service, $this->texts()))->run(); + + $this->assertSame(SetupResult::WARNING, $result->getSeverity()); + $this->assertStringContainsString('The operating system was last checked 3 days ago', (string)$result->getDescription()); + $this->assertStringContainsString('Check that the system cron job for Nextcloud runs.', (string)$result->getDescription()); + } + + public function testWithoutSystemCronTheChecksSayHowToFixIt(): void { + $this->given( + new OsStatus(OsStatus::STATE_OK, 1), + new Concern(Concern::UPDATES, Concern::LEVEL_ERROR, ['classified' => true, 'security' => 3, 'other' => 0]), + new Concern(Concern::FRESHNESS, Concern::LEVEL_WARNING, ['collectedAt' => 1, 'ageSeconds' => 7200, 'systemCron' => false]), + ); + + $result = (new OsUpdatesCheck($this->l10n, $this->urlGenerator, $this->service, $this->texts()))->run(); + + $this->assertSame(SetupResult::ERROR, $result->getSeverity(), 'An old status never lowers a level'); + $this->assertStringContainsString('The operating system was last checked 2 hours ago', (string)$result->getDescription()); + $this->assertStringContainsString('Switch background jobs to Cron', (string)$result->getDescription()); + } + public function testRebootCheck(): void { $this->given( new OsStatus(OsStatus::STATE_OK, 1), From e85eef58c6207052a14ebe2d40a96ab0041fca03 Mon Sep 17 00:00:00 2001 From: Frank Karlitschek Date: Fri, 23 Oct 2026 06:13:22 +0200 Subject: [PATCH 11/16] fix(os): no "reboot required" headline when the reboot state is unknown ConcernTexts::title() only told the concerns' known levels apart, so a concern that could not be determined got the headline of a problem: "Reboot required" when the reboot state was unknown, "The system clock is not synchronized" when time sync could not be checked. Setup checks and the widget skip unknown concerns, but the page shows every concern with its headline. Unknown concerns now say what could not be determined, in the words the page already uses. Assisted-by: ClaudeCode:claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 Signed-off-by: Frank Karlitschek --- lib/OsStatus/Verdict/ConcernTexts.php | 15 +++++++++++++++ tests/lib/OsStatus/ConcernTextsTest.php | 7 +++++++ 2 files changed, 22 insertions(+) diff --git a/lib/OsStatus/Verdict/ConcernTexts.php b/lib/OsStatus/Verdict/ConcernTexts.php index 1d5717372..d34996963 100644 --- a/lib/OsStatus/Verdict/ConcernTexts.php +++ b/lib/OsStatus/Verdict/ConcernTexts.php @@ -53,6 +53,21 @@ public function unavailable(OsStatus $status, IL10N $l): ?string { */ public function title(Concern $concern, OsStatus $status, IL10N $l): string { $facts = $concern->facts; + if ($concern->level === Concern::LEVEL_UNKNOWN) { + $unknown = match ($concern->id) { + Concern::UPDATES => $l->t('Pending updates could not be determined'), + Concern::REBOOT => $l->t('Whether a reboot is required could not be determined'), + Concern::RESTART => $l->t('Whether services need a restart could not be determined'), + Concern::PACKAGE_LISTS => $l->t('The age of the package lists could not be determined'), + Concern::AUTOMATIC_UPDATES => $l->t('Whether automatic updates are enabled could not be determined'), + Concern::FAILED_UNITS => $l->t('System services could not be checked'), + Concern::TIME_SYNC => $l->t('Time synchronization could not be checked'), + default => null, + }; + if ($unknown !== null) { + return $unknown; + } + } return match ($concern->id) { Concern::UPDATES => $this->updatesTitle($concern, $l), diff --git a/tests/lib/OsStatus/ConcernTextsTest.php b/tests/lib/OsStatus/ConcernTextsTest.php index e7427a33a..24ff96331 100644 --- a/tests/lib/OsStatus/ConcernTextsTest.php +++ b/tests/lib/OsStatus/ConcernTextsTest.php @@ -96,6 +96,13 @@ public function testEverySupportTitleIsFilledIn(string $level, array $facts, str $this->assertSame($expected, $this->texts->title(new Concern(Concern::SUPPORT, $level, $facts), $this->status, $this->l)); } + public function testConcernsThatCouldNotBeDeterminedSaySo(): void { + $this->assertSame('Whether a reboot is required could not be determined', $this->texts->title(new Concern(Concern::REBOOT, Concern::LEVEL_UNKNOWN), $this->status, $this->l)); + $this->assertSame('Time synchronization could not be checked', $this->texts->title(new Concern(Concern::TIME_SYNC, Concern::LEVEL_UNKNOWN), $this->status, $this->l)); + $this->assertSame('Pending updates could not be determined', $this->texts->title(new Concern(Concern::UPDATES, Concern::LEVEL_UNKNOWN), $this->status, $this->l)); + $this->assertSame('The support status of Ubuntu 22.04.5 LTS is unknown', $this->texts->title(new Concern(Concern::SUPPORT, Concern::LEVEL_UNKNOWN), $this->status, $this->l)); + } + public function testFreshness(): void { $texts = $this->texts; $status = $this->status; From 8e54f786b5bb4d584c1d0662624a735fa00904ed Mon Sep 17 00:00:00 2001 From: Frank Karlitschek Date: Fri, 23 Oct 2026 06:13:40 +0200 Subject: [PATCH 12/16] docs(os): list the PHP library check where it starts to run The setup checks are the first to sample the PHP process that serves Nextcloud, so this is where the README can promise that check and name the process maps it reads. Assisted-by: ClaudeCode:claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 Signed-off-by: Frank Karlitschek --- README.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 4c3a06b49..87649c204 100644 --- a/README.md +++ b/README.md @@ -28,6 +28,8 @@ secure: runs on (PHP, the web server, the database, Redis …) marked - whether a reboot is needed, and why (a newer kernel, updated core libraries, the distribution asking for one), with live kernel patches taken into account +- whether the PHP serving Nextcloud still runs libraries that an update + replaced, and which service to restart - how long the release still receives security updates, and whether a paid extension (Ubuntu Pro or Debian ELTS) is active - automatic updates, package list age, held packages, failed system services, @@ -39,7 +41,7 @@ what can and cannot be known from inside; LXC and systemd-nspawn guests are treated like machines without a kernel of their own. Nothing is installed or changed. To find out, the app reads `/etc/os-release`, -`/proc` (uptime, `/proc/1/cgroup`), `/sys/class/dmi/id`, +`/proc` (uptime, `/proc/1/cgroup`, its own process maps), `/sys/class/dmi/id`, `/sys/kernel/livepatch`, the SELinux and AppArmor state under `/sys`, the package manager's configuration, state and logs, `/run/reboot-required` and the installed kernel modules. It runs the package manager's query commands (updates From 008275983bab0fe4de4ccb1405a0865327e9e35c Mon Sep 17 00:00:00 2001 From: Frank Karlitschek Date: Fri, 23 Oct 2026 09:55:21 +0200 Subject: [PATCH 13/16] fix(os): show the operating system setup checks to admins only Core serves GET /settings/setupcheck with #[NoAdminRequired], and the settings app's sub-admin middleware lets sub-admins through. They got the full OS setup checks: pending security updates and their age, the reboot state and the end-of-life status. The widget and notifications are for admins only, so the endpoint undercut the app's own boundary. The checks now answer a session user who is not an admin with the same neutral info result, whatever the system's state, and do not sample the PHP process for them. occ setupchecks and cron run without a session user and still get the full result. Assisted-by: ClaudeCode:claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 Signed-off-by: Frank Karlitschek --- README.md | 7 +- lib/SetupChecks/AbstractOsCheck.php | 13 ++++ tests/lib/OsStatus/OsSetupChecksTest.php | 86 +++++++++++++++++++++--- 3 files changed, 92 insertions(+), 14 deletions(-) diff --git a/README.md b/README.md index 87649c204..2ad943ab1 100644 --- a/README.md +++ b/README.md @@ -72,9 +72,10 @@ occ serverinfo:os-status [--refresh] [--output=json] The command shows how old the status is and exits with 0, 1, 2 or 3 for ok, warning, error and unknown, so it can be used as a monitoring check. Unknown means the status has not been collected yet, or nothing is wrong but the pending updates could not be -determined. The same verdict appears as setup checks on Settings → Overview, in -a notification to admins when a concern escalates, and in the **Server health** -dashboard widget. When something counts as a warning or an error is set by +determined. The same verdict appears as setup checks on Settings → Overview +(only for admins; sub-admins, who may load the setup checks too, get no +details), in a notification to admins when a concern escalates, and in the +**Server health** dashboard widget for admins. When something counts as a warning or an error is set by the `os_*_days` app config values, see `lib/Config/ConfigLexicon.php`. Release support dates are bundled with the app (see diff --git a/lib/SetupChecks/AbstractOsCheck.php b/lib/SetupChecks/AbstractOsCheck.php index d49307f28..70169b8eb 100644 --- a/lib/SetupChecks/AbstractOsCheck.php +++ b/lib/SetupChecks/AbstractOsCheck.php @@ -13,8 +13,10 @@ use OCA\ServerInfo\OsStatus\OsStatusService; use OCA\ServerInfo\OsStatus\Verdict\Concern; use OCA\ServerInfo\OsStatus\Verdict\ConcernTexts; +use OCP\IGroupManager; use OCP\IL10N; use OCP\IURLGenerator; +use OCP\IUserSession; use OCP\SetupCheck\ISetupCheck; use OCP\SetupCheck\SetupResult; @@ -24,6 +26,10 @@ * Reads the stored status only: the Overview is loaded far too often to run * the package manager, and the background job keeps the status current. * + * Core runs the setup checks for sub-admins too, so a request by anyone but an + * admin gets a result that says nothing about the system. occ and cron have no + * session user and get the full result. + * * @psalm-api */ abstract class AbstractOsCheck implements ISetupCheck { @@ -32,6 +38,8 @@ public function __construct( private IURLGenerator $urlGenerator, private OsStatusService $service, private ConcernTexts $texts, + private IUserSession $userSession, + private IGroupManager $groupManager, ) { } @@ -47,6 +55,11 @@ public function getCategory(): string { #[\Override] public function run(): SetupResult { + $user = $this->userSession->getUser(); + if ($user !== null && !$this->groupManager->isAdmin($user->getUID())) { + return SetupResult::info($this->l10n->t('Only administrators can see the operating system status.')); + } + $this->service->samplePhpProcess(); $status = $this->service->getStatus(); diff --git a/tests/lib/OsStatus/OsSetupChecksTest.php b/tests/lib/OsStatus/OsSetupChecksTest.php index 7e8784b9e..89ae0e720 100644 --- a/tests/lib/OsStatus/OsSetupChecksTest.php +++ b/tests/lib/OsStatus/OsSetupChecksTest.php @@ -16,12 +16,16 @@ use OCA\ServerInfo\OsStatus\Verdict\Concern; use OCA\ServerInfo\OsStatus\Verdict\ConcernTexts; use OCA\ServerInfo\OsStatus\Verdict\Verdict; +use OCA\ServerInfo\SetupChecks\AbstractOsCheck; use OCA\ServerInfo\SetupChecks\OsRebootCheck; use OCA\ServerInfo\SetupChecks\OsSupportCheck; use OCA\ServerInfo\SetupChecks\OsUpdatesCheck; use OCP\IDateTimeFormatter; +use OCP\IGroupManager; use OCP\IL10N; use OCP\IURLGenerator; +use OCP\IUser; +use OCP\IUserSession; use OCP\SetupCheck\SetupResult; use PHPUnit\Framework\MockObject\MockObject; @@ -29,6 +33,9 @@ class OsSetupChecksTest extends \Test\TestCase { private OsStatusService&MockObject $service; private IL10N $l10n; private IURLGenerator&MockObject $urlGenerator; + /** The uid of the session user, null without a session as in occ and cron */ + private ?string $sessionUser = 'admin'; + private bool $sessionUserIsAdmin = true; protected function setUp(): void { parent::setUp(); @@ -42,6 +49,22 @@ private function texts(): ConcernTexts { return new ConcernTexts($this->createMock(IDateTimeFormatter::class)); } + /** + * @param class-string $class + */ + private function check(string $class, ?ConcernTexts $texts = null): AbstractOsCheck { + $session = $this->createMock(IUserSession::class); + if ($this->sessionUser !== null) { + $user = $this->createMock(IUser::class); + $user->method('getUID')->willReturn($this->sessionUser); + $session->method('getUser')->willReturn($user); + } + $groups = $this->createMock(IGroupManager::class); + $groups->method('isAdmin')->willReturnCallback(fn (string $uid): bool => $uid === $this->sessionUser && $this->sessionUserIsAdmin); + + return new $class($this->l10n, $this->urlGenerator, $this->service, $texts ?? $this->texts(), $session, $groups); + } + private function given(?OsStatus $status, Concern ...$concerns): void { $this->service->method('getStatus')->willReturn($status); $this->service->method('evaluate')->willReturn(new Verdict($concerns)); @@ -50,10 +73,10 @@ private function given(?OsStatus $status, Concern ...$concerns): void { public function testNotCheckedYet(): void { $this->given(null); - $result = (new OsUpdatesCheck($this->l10n, $this->urlGenerator, $this->service, $this->texts()))->run(); + $result = $this->check(OsUpdatesCheck::class)->run(); $this->assertSame(SetupResult::INFO, $result->getSeverity()); - $this->assertSame('security', (new OsUpdatesCheck($this->l10n, $this->urlGenerator, $this->service, $this->texts()))->getCategory()); + $this->assertSame('security', $this->check(OsUpdatesCheck::class)->getCategory()); } public function testTheWorstConcernOfTheCheckDecides(): void { @@ -64,7 +87,7 @@ public function testTheWorstConcernOfTheCheckDecides(): void { new Concern(Concern::REBOOT, Concern::LEVEL_WARNING, ['days' => 9, 'kinds' => ['kernel']]), ); - $result = (new OsUpdatesCheck($this->l10n, $this->urlGenerator, $this->service, $this->texts()))->run(); + $result = $this->check(OsUpdatesCheck::class)->run(); $this->assertSame(SetupResult::ERROR, $result->getSeverity()); $this->assertStringContainsString('Package lists were last refreshed 40 days ago', (string)$result->getDescription()); @@ -79,7 +102,7 @@ public function testAnOldStatusMakesEveryCheckAWarning(): void { new Concern(Concern::FRESHNESS, Concern::LEVEL_WARNING, ['collectedAt' => 1, 'ageSeconds' => 3 * 86400, 'systemCron' => true]), ); - $result = (new OsSupportCheck($this->l10n, $this->urlGenerator, $this->service, $this->texts()))->run(); + $result = $this->check(OsSupportCheck::class)->run(); $this->assertSame(SetupResult::WARNING, $result->getSeverity()); $this->assertStringContainsString('The operating system was last checked 3 days ago', (string)$result->getDescription()); @@ -93,7 +116,7 @@ public function testWithoutSystemCronTheChecksSayHowToFixIt(): void { new Concern(Concern::FRESHNESS, Concern::LEVEL_WARNING, ['collectedAt' => 1, 'ageSeconds' => 7200, 'systemCron' => false]), ); - $result = (new OsUpdatesCheck($this->l10n, $this->urlGenerator, $this->service, $this->texts()))->run(); + $result = $this->check(OsUpdatesCheck::class)->run(); $this->assertSame(SetupResult::ERROR, $result->getSeverity(), 'An old status never lowers a level'); $this->assertStringContainsString('The operating system was last checked 2 hours ago', (string)$result->getDescription()); @@ -107,7 +130,7 @@ public function testRebootCheck(): void { new Concern(Concern::RESTART, Concern::LEVEL_OK, ['php' => false, 'services' => 0]), ); - $result = (new OsRebootCheck($this->l10n, $this->urlGenerator, $this->service, $this->texts()))->run(); + $result = $this->check(OsRebootCheck::class)->run(); $this->assertSame(SetupResult::WARNING, $result->getSeverity()); $this->assertStringStartsWith('Reboot required. A newer kernel is installed. Pending for 9 days.', (string)$result->getDescription()); @@ -116,7 +139,7 @@ public function testRebootCheck(): void { public function testAllGoodHasNoLink(): void { $this->given(new OsStatus(OsStatus::STATE_OK, 1), new Concern(Concern::REBOOT, Concern::LEVEL_OK), new Concern(Concern::RESTART, Concern::LEVEL_OK)); - $result = (new OsRebootCheck($this->l10n, $this->urlGenerator, $this->service, $this->texts()))->run(); + $result = $this->check(OsRebootCheck::class)->run(); $this->assertSame(SetupResult::SUCCESS, $result->getSeverity()); $this->assertStringNotContainsString('{link}', (string)$result->getDescription()); @@ -125,7 +148,7 @@ public function testAllGoodHasNoLink(): void { public function testUnknownIsInfo(): void { $this->given(new OsStatus(OsStatus::STATE_OK, 1), new Concern(Concern::SUPPORT, Concern::LEVEL_UNKNOWN)); - $this->assertSame(SetupResult::INFO, (new OsSupportCheck($this->l10n, $this->urlGenerator, $this->service, $this->texts()))->run()->getSeverity()); + $this->assertSame(SetupResult::INFO, $this->check(OsSupportCheck::class)->run()->getSeverity()); } public function testTheSupportSentenceIsFilledIn(): void { @@ -136,7 +159,7 @@ public function testTheSupportSentenceIsFilledIn(): void { new Concern(Concern::SUPPORT, Concern::LEVEL_OK, ['state' => SupportStatus::STATE_SUPPORTED, 'ends' => '2029-05-31']), ); - $result = (new OsSupportCheck($this->l10n, $this->urlGenerator, $this->service, new ConcernTexts($formatter)))->run(); + $result = $this->check(OsSupportCheck::class, new ConcernTexts($formatter))->run(); $this->assertSame('Ubuntu 24.04.3 LTS receives security updates until May 31, 2029.', $result->getDescription()); } @@ -144,13 +167,54 @@ public function testTheSupportSentenceIsFilledIn(): void { public function testRestrictedMode(): void { $this->given(OsStatus::restricted(1)); - $this->assertSame(SetupResult::SUCCESS, (new OsSupportCheck($this->l10n, $this->urlGenerator, $this->service, $this->texts()))->run()->getSeverity()); + $this->assertSame(SetupResult::SUCCESS, $this->check(OsSupportCheck::class)->run()->getSeverity()); } public function testItSamplesThePhpProcess(): void { $this->given(null); $this->service->expects($this->once())->method('samplePhpProcess'); - (new OsRebootCheck($this->l10n, $this->urlGenerator, $this->service, $this->texts()))->run(); + $this->check(OsRebootCheck::class)->run(); + } + + private function aPendingSecurityUpdate(): void { + $this->given( + new OsStatus(OsStatus::STATE_OK, 1), + new Concern(Concern::UPDATES, Concern::LEVEL_ERROR, ['classified' => true, 'security' => 3, 'other' => 0]), + ); + } + + public function testAdminsSeeTheStatus(): void { + $this->aPendingSecurityUpdate(); + + $result = $this->check(OsUpdatesCheck::class)->run(); + + $this->assertSame(SetupResult::ERROR, $result->getSeverity()); + $this->assertStringContainsString('3 security updates pending', (string)$result->getDescription()); + } + + public function testOccAndCronWithoutASessionSeeTheStatus(): void { + $this->sessionUser = null; + $this->aPendingSecurityUpdate(); + + $result = $this->check(OsUpdatesCheck::class)->run(); + + $this->assertSame(SetupResult::ERROR, $result->getSeverity()); + $this->assertStringContainsString('3 security updates pending', (string)$result->getDescription()); + } + + public function testSubAdminsLearnNothingAboutTheSystem(): void { + $this->sessionUser = 'subadmin'; + $this->sessionUserIsAdmin = false; + $this->aPendingSecurityUpdate(); + $this->service->expects($this->never())->method('samplePhpProcess'); + + foreach ([OsUpdatesCheck::class, OsRebootCheck::class, OsSupportCheck::class] as $class) { + $result = $this->check($class)->run(); + + $this->assertSame(SetupResult::INFO, $result->getSeverity()); + $this->assertSame('Only administrators can see the operating system status.', $result->getDescription()); + $this->assertNull($result->getDescriptionParameters()); + } } } From 0342a2822a5542be628c031a5e4b8706d7290f9b Mon Sep 17 00:00:00 2001 From: Frank Karlitschek Date: Fri, 23 Oct 2026 09:56:00 +0200 Subject: [PATCH 14/16] fix(os): escape the setup check text before the Overview renders it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Settings → Overview renders a setup check's description with v-html and escapes only the rich object parameters. The OS checks build their description from ConcernTexts, which puts strings from the system into it, such as PRETTY_NAME from /etc/os-release, which is free text. Markup there ran in an admin's browser. The checks now escape the text with htmlspecialchars() before adding the {link} placeholder, which core fills with an escaped link, and on the success path, which has no link. Quotes are left alone: the text is never put into an attribute, and occ setupchecks prints the description without decoding entities, so translations with apostrophes stay readable there. Only <, > and & show as entities in occ. Assisted-by: ClaudeCode:claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 Signed-off-by: Frank Karlitschek --- lib/SetupChecks/AbstractOsCheck.php | 13 +++++++++- tests/lib/OsStatus/OsSetupChecksTest.php | 31 ++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/lib/SetupChecks/AbstractOsCheck.php b/lib/SetupChecks/AbstractOsCheck.php index 70169b8eb..89db49b14 100644 --- a/lib/SetupChecks/AbstractOsCheck.php +++ b/lib/SetupChecks/AbstractOsCheck.php @@ -70,7 +70,7 @@ public function run(): SetupResult { } $unavailable = $this->texts->unavailable($status, $this->l10n); if ($unavailable !== null) { - return SetupResult::success($unavailable); + return SetupResult::success(self::escape($unavailable)); } $verdict = $this->service->evaluate($status); @@ -96,6 +96,7 @@ public function run(): SetupResult { $level = Concern::LEVEL_WARNING; } } + $text = self::escape($text); $description = $text . ' {link}'; return match ($level) { @@ -106,6 +107,16 @@ public function run(): SetupResult { }; } + /** + * The Overview renders the description as HTML and escapes only the rich + * object parameters. The texts carry strings from the system, such as + * PRETTY_NAME from os-release. Quotes stay: the description never goes + * into an attribute, and occ prints entities without decoding them. + */ + private static function escape(string $text): string { + return htmlspecialchars($text, ENT_NOQUOTES | ENT_SUBSTITUTE); + } + /** * The Overview only renders a link when it comes as a rich object parameter. * diff --git a/tests/lib/OsStatus/OsSetupChecksTest.php b/tests/lib/OsStatus/OsSetupChecksTest.php index 89ae0e720..84c8df659 100644 --- a/tests/lib/OsStatus/OsSetupChecksTest.php +++ b/tests/lib/OsStatus/OsSetupChecksTest.php @@ -27,6 +27,7 @@ use OCP\IUser; use OCP\IUserSession; use OCP\SetupCheck\SetupResult; +use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\MockObject\MockObject; class OsSetupChecksTest extends \Test\TestCase { @@ -217,4 +218,34 @@ public function testSubAdminsLearnNothingAboutTheSystem(): void { $this->assertNull($result->getDescriptionParameters()); } } + + public static function dataMarkupInTheReleaseName(): array { + return [ + 'supported, no link' => [Concern::LEVEL_OK, ['state' => SupportStatus::STATE_SUPPORTED, 'ends' => '2029-05-31'], SetupResult::SUCCESS], + 'ended, with a link' => [Concern::LEVEL_ERROR, ['state' => SupportStatus::STATE_ENDED, 'ends' => '2024-06-30'], SetupResult::ERROR], + ]; + } + + #[DataProvider('dataMarkupInTheReleaseName')] + public function testMarkupInTheReleaseNameIsEscaped(string $level, array $facts, string $severity): void { + $formatter = $this->createMock(IDateTimeFormatter::class); + $formatter->method('formatDate')->willReturn('May 31, 2029'); + $this->given( + new OsStatus(OsStatus::STATE_OK, 1, new OsRelease('ubuntu', ['debian'], 'Ubuntu', 'Ubuntu "Noble" & co', '24.04', 'noble')), + new Concern(Concern::SUPPORT, $level, $facts), + ); + + $result = $this->check(OsSupportCheck::class, new ConcernTexts($formatter))->run(); + $description = (string)$result->getDescription(); + + $this->assertSame($severity, $result->getSeverity()); + $this->assertStringContainsString('Ubuntu "Noble" <img src=x onerror=alert(1)> & co', $description); + $this->assertStringNotContainsString('<', $description); + if ($severity === SetupResult::SUCCESS) { + $this->assertStringNotContainsString('{link}', $description); + } else { + $this->assertStringEndsWith(' {link}', $description); + $this->assertSame('highlight', $result->getDescriptionParameters()['link']['type'] ?? null); + } + } } From de1939216462974e1ed8bc05d06d302ebb420248 Mon Sep 17 00:00:00 2001 From: Frank Karlitschek Date: Fri, 23 Oct 2026 09:56:26 +0200 Subject: [PATCH 15/16] fix(os): drop operating system notifications of users who are no longer admins OsNotifier builds the text from the current status every time the notification is fetched, and the sender withdraws a notification only when its concern resolves. An admin who was removed from the admin group kept getting live security update counts, reboot and end-of-life state through notifications sent while they were an admin. prepare() now throws AlreadyProcessedException when the recipient is not an admin, so the notification is deleted before anything from the status is read. Assisted-by: ClaudeCode:claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 Signed-off-by: Frank Karlitschek --- lib/OsStatus/Notifications/OsNotifier.php | 6 +++++- tests/lib/OsStatus/EntryPointsTest.php | 15 ++++++++++++++- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/lib/OsStatus/Notifications/OsNotifier.php b/lib/OsStatus/Notifications/OsNotifier.php index f076df0a7..3076577ca 100644 --- a/lib/OsStatus/Notifications/OsNotifier.php +++ b/lib/OsStatus/Notifications/OsNotifier.php @@ -12,6 +12,7 @@ use OCA\ServerInfo\AppInfo\Application; use OCA\ServerInfo\OsStatus\OsStatusService; use OCA\ServerInfo\OsStatus\Verdict\ConcernTexts; +use OCP\IGroupManager; use OCP\IURLGenerator; use OCP\L10N\IFactory; use OCP\Notification\AlreadyProcessedException; @@ -32,6 +33,7 @@ public function __construct( private OsStatusService $service, private OsNotificationSender $sender, private ConcernTexts $texts, + private IGroupManager $groupManager, ) { } @@ -51,7 +53,9 @@ public function prepare(INotification $notification, string $languageCode): INot throw new UnknownNotificationException(); } - if (!$this->sender->isCurrent($notification)) { + // The text comes from the live status, so it must not reach someone + // who has stopped being an admin since it was sent. + if (!$this->groupManager->isAdmin($notification->getUser()) || !$this->sender->isCurrent($notification)) { throw new AlreadyProcessedException(); } diff --git a/tests/lib/OsStatus/EntryPointsTest.php b/tests/lib/OsStatus/EntryPointsTest.php index dfb83178f..702c17168 100644 --- a/tests/lib/OsStatus/EntryPointsTest.php +++ b/tests/lib/OsStatus/EntryPointsTest.php @@ -35,6 +35,7 @@ class EntryPointsTest extends \Test\TestCase { private OsStatusService&MockObject $service; private IL10N $l10n; private IURLGenerator&MockObject $urlGenerator; + private bool $recipientIsAdmin = true; protected function setUp(): void { parent::setUp(); @@ -54,7 +55,10 @@ private function notifier(OsNotificationSender&MockObject $sender): OsNotifier { $factory = $this->createMock(IFactory::class); $factory->method('get')->willReturn($this->l10n); - return new OsNotifier($factory, $this->urlGenerator, $this->service, $sender, $this->texts()); + $groups = $this->createMock(IGroupManager::class); + $groups->method('isAdmin')->willReturnCallback(fn (string $uid): bool => $uid === 'admin' && $this->recipientIsAdmin); + + return new OsNotifier($factory, $this->urlGenerator, $this->service, $sender, $this->texts(), $groups); } private function notification(string $app, string $objectType, string $objectId): INotification&MockObject { @@ -62,6 +66,7 @@ private function notification(string $app, string $objectType, string $objectId) $notification->method('getApp')->willReturn($app); $notification->method('getObjectType')->willReturn($objectType); $notification->method('getObjectId')->willReturn($objectId); + $notification->method('getUser')->willReturn('admin'); return $notification; } @@ -147,6 +152,14 @@ public function testNotifierDropsNotificationsTheSenderWithdrew(): void { $this->notifier($sender)->prepare($this->notification('serverinfo', 'os', 'updates'), 'en'); } + public function testNotifierDropsNotificationsOfFormerAdmins(): void { + $this->recipientIsAdmin = false; + $this->service->expects($this->never())->method('getStatus'); + + $this->expectException(AlreadyProcessedException::class); + $this->notifier($this->currentSender())->prepare($this->liveNotification('updates', $subject), 'en'); + } + private function widget(bool $admin): ServerHealthWidget { $user = $this->createMock(IUser::class); $user->method('getUID')->willReturn('alice'); From 3f7891915840dfae5cd8ad4b912bbdb2ec74052a Mon Sep 17 00:00:00 2001 From: Frank Karlitschek Date: Fri, 23 Oct 2026 09:57:10 +0200 Subject: [PATCH 16/16] fix(os): do not sample the PHP process in restricted mode or off Linux PhpProcessProbe::canSample() only looks at the SAPI, so the setup checks still read /proc/self/maps, /proc/self/exe and the confinement files, and stored a sample, with restricted_mode=yes, although the collector honours the setting and the README promises that restricted mode stops reads outside the app directory. samplePhpProcess() now returns before any read or store access when isApplicable() is false: in restricted mode or on a system other than Linux. FakeHost records the paths it is asked to read, so the test can show that nothing was read. Assisted-by: ClaudeCode:claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 Signed-off-by: Frank Karlitschek --- lib/OsStatus/OsStatusService.php | 3 ++- tests/lib/OsStatus/FakeHost.php | 5 +++++ tests/lib/OsStatus/OsStatusServiceTest.php | 15 +++++++++++++++ 3 files changed, 22 insertions(+), 1 deletion(-) diff --git a/lib/OsStatus/OsStatusService.php b/lib/OsStatus/OsStatusService.php index 93617bddd..fa453f52c 100644 --- a/lib/OsStatus/OsStatusService.php +++ b/lib/OsStatus/OsStatusService.php @@ -59,9 +59,10 @@ public function getStatus(): ?OsStatus { /** * Looks at the PHP process serving this request and remembers what it * found. Cheap: one read of /proc/self/maps, at most once per request. + * Does nothing where the system is not checked at all. */ public function samplePhpProcess(): void { - if ($this->sampled || !$this->phpProcessProbe->canSample()) { + if ($this->sampled || !$this->isApplicable() || !$this->phpProcessProbe->canSample()) { return; } $this->sampled = true; diff --git a/tests/lib/OsStatus/FakeHost.php b/tests/lib/OsStatus/FakeHost.php index c0eac92cb..5da7ffe6e 100644 --- a/tests/lib/OsStatus/FakeHost.php +++ b/tests/lib/OsStatus/FakeHost.php @@ -43,6 +43,8 @@ class FakeHost implements IHost { public array $basedir = []; /** @var list commands that were run, for assertions */ public array $ran = []; + /** @var list paths that were read, for assertions */ + public array $read = []; public bool $commandsAllowed = true; public string $release = '6.8.0-45-generic'; public string $machine = 'x86_64'; @@ -83,11 +85,13 @@ public function withCommand(string $binary, array $arguments, string $stdout, in #[\Override] public function readFile(string $path, int $maxBytes = 1048576): ?string { + $this->read[] = $path; return isset($this->files[$path]) ? substr($this->files[$path], 0, $maxBytes) : null; } #[\Override] public function readFileTail(string $path, int $maxBytes = 262144): ?string { + $this->read[] = $path; return isset($this->files[$path]) ? substr($this->files[$path], -$maxBytes) : null; } @@ -113,6 +117,7 @@ public function ownership(string $path): ?array { #[\Override] public function readLink(string $path): ?string { + $this->read[] = $path; return $this->links[$path] ?? null; } diff --git a/tests/lib/OsStatus/OsStatusServiceTest.php b/tests/lib/OsStatus/OsStatusServiceTest.php index a33cdb775..88acfdc79 100644 --- a/tests/lib/OsStatus/OsStatusServiceTest.php +++ b/tests/lib/OsStatus/OsStatusServiceTest.php @@ -123,6 +123,21 @@ public function testSamplingOnlyInWebRequests(): void { $this->service->samplePhpProcess(); } + public function testSamplingOnlyWhereTheSystemIsChecked(): void { + $this->host->files['/proc/self/maps'] = ''; + $this->store->expects($this->never())->method('load'); + $this->store->expects($this->never())->method('savePhpSample'); + + $this->restrictedMode = 'yes'; + $this->service->samplePhpProcess(); + + $this->restrictedMode = 'no'; + $this->host->family = 'BSD'; + $this->service->samplePhpProcess(); + + $this->assertSame([], $this->host->read); + } + public function testFreshnessFollowsTheCronModeAndTheJobInterval(): void { $status = new OsStatus(OsStatus::STATE_OK, self::NOW - 2 * 86400);