Repository navigation
feat(os): collect the operating system status - #1214
karlitschek wants to merge 10 commits into
Conversation
AndyScherzinger
left a comment
AndyScherzinger
left a comment
There was a problem hiding this comment.
I reviewed this for correctness, security and performance, and deployed the branch to a nextcloud-docker-dev instance (NC 36 dev, PHP 8.5, Debian 13 container) to exercise it. Inline comments carry the details; this is the summary.
Blocking
- CI "NPM build" fails: the committed
js/serverinfo-main.mjs.licenseandjs/serverinfo-main.mjs.map.licenselist@vitejs/plugin-vue 6.0.10, whilepackage-lock.jsonpins6.0.7. They were generated from anode_modulesout of sync with the lock file;npm ci && npm run buildregenerates them correctly. The version bump itself is needed for the background job and command to register. - Missing or stale package lists turn into "no security updates" (reproduced): with the image's purged
/var/lib/apt/lists,apt-get -s dist-upgradereported 0 pending updates; afterapt-get updatethe same container reported 71 pending, 35 of them security. The updates concern only becomes unknown whenupdates === null, so the headline answer is a false all-clear wherever the lists are missing or old (seeOsVerdict.php). occ serverinfo:os-statusexit codes do not follow the monitoring-plugin convention it cites: "not checked yet" exits 1 (= WARNING), and an all-unknown verdict reports"level": "ok"and exits 0. In the Docker container 3 of 4 concerns were unknown and the command said OK. Monitoring plugins use 3 for UNKNOWN.
Correctness
- The first-seen key includes the candidate version, so a superseding security update resets the pending-days counter.
CoreLibraryProbetreats an upgraded systemd binary as a reboot reason, though systemd re-executes itself on upgrade; on Debian, which has no/run/reboot-requiredby default, that is a recurring false positive.- In LXC without lxcfs,
/proc/uptimeis the host's, so a container created after the host booted reports a core-library reboot straight away. - Smaller:
Thresholds::load()does not enforce warning ≤ error;nixos-unstableis not an os-releaseID; Raspbian is mapped to Debian's support dates though it has its own archive;/dev/shm/is shadowed by/dev/. - Worth stating in the UI later: Debian ships many fixes through point releases (
libc6 2.41-12+deb13u4fromDebian:13.7/stablein my run), so "0 security updates" does not mean fully patched.
Security
CommandRunneris well done: an argument vector toproc_open()with no shell, validated binary names, a fixedPATH,LC_ALL=C, stdin from/dev/null, an output cap and a kill on timeout. All arguments are constants, so I found no injection surface, andrestricted_modeis honoured.- The risk is exposure rather than execution. In this PR the data is only visible to admins through occ, but a list of unpatched packages with exact versions is an attacker's roadmap. Once #1217 exposes this through the token-authenticated monitoring API, consider returning counts there and keeping the package list admin-only.
- Package names and origins are stored as parsed from tool output (
\S+). Nothing here renders them, but #1218 should not pass them throughv-html. - The collector reads
/proc/1/cgroup,/sys/class/dmi/id/*and queries systemd over D-Bus. Everything degrades to unknown when denied, which is right; the README could list what is read so hardened setups know what to expect.
Performance
- From system cron this is cheap. A full refresh took 0.75 s in the container; with 71 pending updates,
apt-get -s dist-upgradetook 2.3 s and the remaining apt probes 0.4 s. - Under AJAX or webcron background-job modes the job runs inside a web request. The timeouts add up to roughly four minutes in the worst case (120 s for apt-get, 3 × 30 s, plus the 10 s and 5 s probes), all holding one PHP-FPM worker. Consider skipping the job unless
backgroundjobs_modeiscron, or shortening the timeouts there. - The status is stored as a lazy app-config array capped at 500 updates, which fits the long-text column on every supported database.
| } | ||
|
|
||
| private function updates(OsStatus $status, Thresholds $thresholds, int $now): Concern { | ||
| if ($status->updates === null) { |
There was a problem hiding this comment.
This only reports unknown when updates === null. With missing package lists, apt-get -s succeeds and returns nothing. I reproduced it in a Debian 13 container: 0 pending before apt-get update, 71 pending (35 security) after. The concern then says OK with 0 security updates. Suggest treating the updates as unknown when $status->listsRefreshedAt === null, and qualifying the answer once the lists are past the warning threshold, since apt only knows what the lists on disk tell it.
|
|
||
| $status = $this->service->getStatus(); | ||
| if ($status === null) { | ||
| $output->writeln('The operating system has not been checked yet. Run with --refresh to check it now.'); |
There was a problem hiding this comment.
This exits 1, which a monitoring check reads as WARNING. "No data yet" is UNKNOWN, exit 3 in the monitoring-plugin convention the comment below refers to.
| return match ($verdict?->level()) { | ||
| Concern::LEVEL_ERROR => 2, | ||
| Concern::LEVEL_WARNING => 1, | ||
| default => 0, |
There was a problem hiding this comment.
Verdict::level() skips unknown concerns and returns unknown only when every concern is unknown. That case and null both fall through to 0 here. In a Docker container I got "level": "ok" and exit 0 with updates, reboot, failed units and time sync all unknown. Suggest mapping LEVEL_UNKNOWN (and a null verdict) to 3.
| } | ||
|
|
||
| public function key(): string { | ||
| return $this->name . ':' . $this->candidateVersion; |
There was a problem hiding this comment.
Because the key includes the candidate version, a security update that is superseded by a newer one before it is installed starts a fresh first-seen entry. Then the pending-days count, and with it the 3/14-day thresholds, starts over. On frequently updated packages (openssl, the kernel, libc) a host can stay below the error threshold indefinitely. Keying on the package name alone would keep counting from when the package first had a security fix pending.
| '/lib64/libc.so.6', '/usr/lib64/libc.so.6', '/usr/lib/libc.so.6', | ||
| '/lib/ld-musl-*.so.1', | ||
| ], | ||
| 'systemd' => ['/usr/lib/systemd/systemd', '/lib/systemd/systemd'], |
There was a problem hiding this comment.
systemd's package runs systemctl daemon-reexec on upgrade, so a changed systemd binary does not need a reboot. Debian creates no /run/reboot-required by default, so this probe runs there, and every systemd update becomes a reboot warning after 7 days and an error after 30. Ubuntu is affected too whenever that file is absent. glibc and dbus are the defensible entries here.
| IAppConfig $appConfig, | ||
| ) { | ||
| parent::__construct($time); | ||
| $this->setInterval(max(300, $appConfig->getValueInt(Application::APP_ID, ConfigLexicon::JOB_INTERVAL_OS_STATUS))); |
There was a problem hiding this comment.
With backgroundjobs_mode set to ajax or webcron, this job runs inside a web request. The command timeouts add up to roughly four minutes in the worst case (120 s for apt-get -s alone), all holding one PHP-FPM worker. Consider returning early unless background jobs run from system cron.
| foreach ($origins as $origin) { | ||
| $slash = strrpos($origin, '/'); | ||
| $suite = $slash === false ? $origin : substr($origin, $slash + 1); | ||
| if (str_ends_with($suite, '-security') || stripos($origin, 'Debian-Security') !== false) { |
There was a problem hiding this comment.
This is correct by origin. Note for the UI later in the stack: Debian ships many fixes through point releases without a DSA (in my run, libc6 2.41-12+deb13u4 came from Debian:13.7/stable). So "0 security updates" should not read as fully patched.
| const ROLLING = [ | ||
| 'arch', 'archarm', 'manjaro', 'endeavouros', 'cachyos', 'garuda', | ||
| 'opensuse-tumbleweed', 'opensuse-microos', 'opensuse-slowroll', | ||
| 'gentoo', 'void', 'nixos-unstable', |
There was a problem hiding this comment.
NixOS's os-release has ID=nixos in every channel, so nixos-unstable never matches.
| private function tableId(OsRelease $release): string { | ||
| return match (true) { | ||
| $release->id === 'centos' && stripos($release->name, 'stream') !== false => 'centos-stream', | ||
| $release->id === 'raspbian' => 'debian', |
There was a problem hiding this comment.
Raspbian is a separate archive (raspbian.org) with its own security handling, so Debian's end dates do not necessarily apply. Unknown may be the more honest answer here.
| */ | ||
| class PhpProcessProbe { | ||
| /** Mappings that are deleted by design rather than by an upgrade. */ | ||
| private const IGNORED_PREFIXES = ['/dev/', '/memfd:', '/tmp/', '/var/tmp/', '/run/', '/dev/shm/', '/SYSV', '/proc/', '/var/lib/php', '/var/cache/']; |
There was a problem hiding this comment.
Nit: /dev/shm/ is already covered by /dev/.
Collect what keeps the Linux distribution under Nextcloud secure, as far as the web server user can see it: pending updates with security fixes told apart (apt), reboots that installed updates wait for (the running kernel against /lib/modules, core libraries replaced since boot, the distribution's own flag), libraries replaced under the PHP serving Nextcloud, how long the release still receives security updates (bundled from endoflife.date by build/update-os-support.php, nothing fetched at runtime), automatic updates, held packages, failed system services, time synchronisation and mandatory access control. Containers, LXC guests and snaps are told apart, so nothing is claimed that cannot be known from inside. Commands run through proc_open() with an argument vector, a fixed search path, LC_ALL=C, a time limit and an output cap. A background job collects the status hourly. One verdict with configurable thresholds judges it, and occ serverinfo:os-status shows it, exiting 0/1/2 for monitoring checks. Assisted-by: ClaudeCode:claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Frank Karlitschek <karlitschek@users.noreply.github.com>
Without package lists apt-get -s finds no newer versions, which read as "no security updates". When the lists cannot be found and nothing is pending, the updates are now unknown, with a diagnostic. Updates found in lists past the warning age carry a listsStale fact, and occ reports the security update count as unknown when the updates are. Assisted-by: ClaudeCode:claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Frank Karlitschek <karlitschek@users.noreply.github.com>
occ serverinfo:os-status follows the monitoring-plugin exit codes, where 3 is UNKNOWN. A status that was never collected, could not be collected or has no verdict now exits 3 instead of 1 or 0. The overall level is unknown when nothing is wrong but the pending updates are unknown, so an unknown update state no longer reads as an all-clear. Assisted-by: ClaudeCode:claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Frank Karlitschek <karlitschek@users.noreply.github.com>
A security update superseded by a newer one before it was installed started a new first-seen entry, so the pending days, and with them the thresholds, started over. Hosts with frequently updated packages could stay below the error threshold indefinitely. Assisted-by: ClaudeCode:claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Frank Karlitschek <karlitschek@users.noreply.github.com>
systemd's package re-executes PID 1 on upgrade, so a newer systemd binary needs no reboot; on Debian, which has no /run/reboot-required by default, every systemd update became a reboot warning. In a system container without lxcfs the uptime is the host's, so a container created after the host booted reported a reboot straight away; the probe now skips system containers. Assisted-by: ClaudeCode:claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Frank Karlitschek <karlitschek@users.noreply.github.com>
With an error threshold below its warning threshold, a finding went straight from info to error and the warning stage disappeared. Loading the thresholds now raises each error value to its warning value. Assisted-by: ClaudeCode:claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Frank Karlitschek <karlitschek@users.noreply.github.com>
The commands can take minutes in the worst case. Under AJAX or webcron the background job runs inside a web request and would hold a web server worker that long, so it now does nothing unless background jobs run from system cron; occ serverinfo:os-status --refresh still works everywhere. Assisted-by: ClaudeCode:claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Frank Karlitschek <karlitschek@users.noreply.github.com>
Raspbian is a separate archive with its own security handling, so Debian's end dates do not necessarily apply; its support is unknown now. NixOS has ID=nixos in every channel, so "nixos-unstable" never matched and is gone from the rolling releases. /dev/shm/ was already covered by /dev/ among the ignored PHP mappings. Assisted-by: ClaudeCode:claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Frank Karlitschek <karlitschek@users.noreply.github.com>
…uirement Assisted-by: ClaudeCode:claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Frank Karlitschek <karlitschek@users.noreply.github.com>
The license files listed @vitejs/plugin-vue 6.0.10 and the lock file had follow-redirects downgraded to 1.16.0, both from a node_modules out of sync with the lock file. The lock file is master's again with only the version bump, and npm ci && npm run build regenerated the license files. Assisted-by: ClaudeCode:claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Frank Karlitschek <karlitschek@users.noreply.github.com>
b2085c1 to
2e8c497
Compare
3 participants
Part 1 of 5, stacked on master:
AI disclosure: this change was written with Claude Code (claude-opus-5-5) and reviewed by the author; every commit carries an
Assisted-bytrailer.🤖 Generated with Claude Code