feat(os): add an optional root helper by karlitschek · Pull Request #1215 · nextcloud/serverinfo

karlitschek · 2026-10-19T21:02:37Z

Part 2 of 5, stacked on #1214:

feat(os): collect the operating system status #1214 – collect the operating system status
feat(os): add an optional root helper #1215 – optional root helper ← this PR
feat(os): support dnf, zypper, pacman and apk #1216 – dnf, zypper, pacman and apk
feat(os): report operating system concerns where admins look #1217 – setup checks, notifications, monitoring API, dashboard widget
feat(os): add the Operating system page #1218 – the Operating system page

AI disclosure: this change was written with Claude Code (claude-opus-5-5) and reviewed by the author; every commit carries an Assisted-by trailer.

🤖 Generated with Claude Code

AndyScherzinger

I reviewed this for correctness, security and performance. I also ran the helper as root in the nextcloud-docker-dev container (Debian 13), with the whole stack deployed. Inline comments carry the details; this is the summary.

Security

I found no path from the web server user to root.

The helper takes no input the web user controls: no app config and no app-data paths. It reads only root-owned files, /proc and cgroups.
PATH, LC_ALL, umask, set -f and cd / are pinned, and --output is validated strictly.
The needrestart configuration forces interpscan=0 and sendnotify=0, which closes the CVE-2024-48990 family. -r l never restarts anything.
Every value in the report is a whitelisted token or escaped, the directory chain is checked for ownership, permissions and symlinks, and the write is mktemp plus rename in a root-only directory.
On the PHP side the report is treated as untrusted: root-owned, no group/other write anywhere on the path, size, depth and item caps, and host, boot, age and clock checks.

The weak spot is availability. The web user can silently stop the helper from running: reproduced live, details inline on the lock.

Correctness

apt-get update exits 0 when a mirror cannot be reached (reproduced). The helper then reports the lists as freshly refreshed, which hides the package-lists concern.
The SUSEConnect match can span two JSON objects.
Pending updates and restarts from the report outlive an upgrade the admin ran in between.
Smaller items: the /proc fallback misses replaced executables, units with escaped names are dropped, the report is world-readable, and there are no removal instructions.

Performance

The helper runs from root's cron every six hours with time limits on every package-manager call, so it is well inside its interval. Reading the report happens only in the cron collector, never in a web request.

AndyScherzinger · 2026-10-23T10:42:18Z

+take_lock() {
+	command -v flock >/dev/null 2>&1 || return 0
+	[ -d "$(dirname "$LOCK")" ] || return 0
+	exec 9>>"$LOCK"


Reproduced. The lock file is created 0644 under umask 022, and flock(2) does not need write access, so any local user can hold the lock. Steps in the container:

As www-data, run flock /run/serverinfo-os-report.lock sleep 60.

Run the helper as root: it exits 0, prints nothing (the message only goes to a tty), and the report's mtime does not change.

Release the lock and run the helper again: it rewrites the report.

A compromised web server can keep this up indefinitely. After 26 hours the app only shows the helper as stale, and the root-only findings (service restarts, reboot reasons, dnf 5 updates) quietly turn unknown.

Suggested fix: create the lock 0600, e.g. (umask 077; : >>"$LOCK") before the exec, or lock a file inside a root-only directory. Also log a skipped run through logger or stderr even without a tty.

AndyScherzinger · 2026-10-23T10:42:18Z

+	if [ "$REFRESH" -eq 1 ]; then
+		refreshed=0
+		case "$FAMILY" in
+			apt) run_limited 900 apt-get update -qq -o DPkg::Lock::Timeout=300 >/dev/null 2>&1 && refreshed=1 ;;


Reproduced: with an unreachable source added, apt-get update -qq exits 0 and only prints W: Failed to fetch … and W: Some index files failed to download. They have been ignored, or old ones used instead. The helper then sets listsRefreshedAt to now. OsStatusCollector takes the newer of this and its own estimate, so a host whose mirror has been down for weeks shows fresh lists.

apt's own update-success-stamp, which the Apt probe prefers, is only touched on Post-Invoke-Success, so it does not have this problem. Suggest treating any W:/E: line on stderr as a failed refresh, or not reporting listsRefreshedAt for the apt family at all.

AndyScherzinger · 2026-10-23T10:42:18Z

+	if [ "$FAMILY" = zypper ] && command -v SUSEConnect >/dev/null 2>&1; then
+		status=$(run_limited 120 SUSEConnect --status 2>/dev/null | tr -d ' \n')
+		case "$status" in
+			*'"identifier":"SLES-LTSS"'*'"status":"Registered"'*) SUBSCRIPTION='{"kind":"suse-ltss","active":true}' ;;


This glob runs over the whole flattened JSON array, so it can match across objects. Example: an SLES-LTSS entry with "status":"Not Registered" followed by any registered module reports LTSS as active, which suppresses the end-of-support concern on an unpatched SLES. Suggest splitting per object first (e.g. tr '}' '\n') and requiring both strings on the same line.

AndyScherzinger · 2026-10-23T10:42:18Z

 				$diagnostics[] = 'Pending updates could not be determined; see the Nextcloud log.';
 			}
 		}
+		$reportedUpdates = $report?->pendingUpdates;


The report's pending updates (and the restart services at line 151) are used for up to 26 hours, with nothing comparing them to what happened since. Suppose the admin runs dnf upgrade or restarts the services right after a helper run. The app then keeps reporting the old updates and restarts until the next run, up to about 6.5 hours with the suggested cron line and random delay. With first-seen ages, that can even escalate on a freshly patched host. Suggest dropping the report's update and restart lists when the package database (or lastUpgradeAt) changed after generatedAt, with a diagnostic.

AndyScherzinger · 2026-10-23T10:42:18Z

+
+# Fallback for any systemd distribution: processes that map a library which
+# has been deleted since, attributed to their systemd unit.
+from_proc() {


Nit: this fallback only looks for deleted .so mappings. A service whose own executable was replaced (/usr/sbin/nginx (deleted)) with unchanged libraries is missed, although the header promises services still running code an upgrade replaced. Checking /proc/PID/exe for (deleted), with the same path exclusions, would cover it.

AndyScherzinger · 2026-10-23T10:42:19Z

+
+# Package, unit and version tokens are emitted only when they consist of
+# characters that need no escaping in JSON; anything else is dropped.
+is_token() {


Nit: ServiceRestart on the PHP side accepts backslashes, as in escaped systemd unit names like systemd-cryptsetup@luks\x2d….service. is_token here drops them, so such units silently go missing from restartServices. Either allow and JSON-escape them here, or accept the gap and say so.

AndyScherzinger · 2026-10-23T10:42:19Z

+
+	TEMPORARY=$(mktemp "$directory/.os-report.XXXXXX") || die "cannot write to $directory"
+	printf '%s\n' "$1" > "$TEMPORARY" || die "cannot write $TEMPORARY"
+	chmod 0644 "$TEMPORARY"


Nit: the report is world-readable in a 0755 directory. Any local user can read the pending security advisories, and which root services still map replaced libraries. Other users' /proc/PID/maps would not otherwise tell them that. The reader only rejects group/other write, so an optional chgrp <web server group> plus 0640 would work. Either document that, or note the exposure in the README.

AndyScherzinger · 2026-10-23T10:42:19Z

+tar -xjf nextcloud-<version>.tar.bz2 -C "$d" nextcloud/apps/serverinfo/bin/serverinfo-os-report &&
+sudo install -o root -g root -m 0755 "$d"/nextcloud/apps/serverinfo/bin/serverinfo-os-report /usr/local/sbin/ &&
+rm -r "$d"
+echo '17 */6 * * * root /usr/local/sbin/serverinfo-os-report --refresh --random-delay' | sudo tee /etc/cron.d/serverinfo-os-report


Nit: there are no removal instructions. If the cron file and the script are removed but /var/lib/serverinfo/os-report.json stays, the app shows the helper as stale forever. Three lines would do: remove /etc/cron.d/serverinfo-os-report, the installed script, and the report directory.

bin/serverinfo-os-report, run by root from cron, adds what the web server cannot see: services that still run code an update replaced (needrestart, dnf needs-restarting, zypper ps, or a scan of /proc), fresh package lists, and pending dnf updates. It is installed outside the web root and only reads the system. Its report is used only when root owns it and its directory, it is recent, and it was written on this host since the last boot. Assisted-by: ClaudeCode:claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 <[email protected]> Signed-off-by: Frank Karlitschek <[email protected]>

The helper ran `needrestart -b -r l` as root every six hours with the default configuration, which scans interpreters. That scan runs Python, Ruby and Perl with settings taken from the inspected process, which its owner controls; before needrestart 3.8 this is a local root exploit (CVE-2024-48990, -48991, -48992, -10224, -11003). A compromised web server could start such a process and wait for the next cron run. needrestart now gets its configuration through `-c /dev/stdin`: the configuration evaluates the system needrestart.conf (and so conf.d), then sets interpscan and sendnotify to 0. -c replaces the configuration file needrestart evaluates as Perl, and the -b and -r l options still take precedence over it. A pipe leaves no file that could be changed between writing and reading it, and needrestart exits with an error rather than falling back to its defaults when it cannot read or evaluate the configuration; the helper then uses the /proc scan instead. The parsing of the batch output moves into its own function so it can be tested. Assisted-by: ClaudeCode:claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 <[email protected]> Signed-off-by: Frank Karlitschek <[email protected]>

dnf_updates_json emits only values that are plain tokens, so nothing needs escaping in JSON, but the installed version taken from `rpm -qa` skipped that check. A quote or backslash in a version string made every report invalid JSON, and the app then ignored the whole report. Assisted-by: ClaudeCode:claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 <[email protected]> Signed-off-by: Frank Karlitschek <[email protected]>

dnf 5 loads the repositories for needs-restarting, so the helper could download metadata as root outside of --refresh. It now passes --cacheonly, which dnf 5 takes as a global option for every command (the captured Fedora 44 output uses it with check-update, advisory and versionlock). dnf 4 accepts it too, and its needs-restarting loads no repositories at all (see the verbose AlmaLinux 9 and 10 captures). The check-update parser only knew a fixed list of architectures, so x86_64_v2 (AlmaLinux 10), ppc64 and s390 updates were dropped without notice. Whatever follows the last dot is now the architecture. dnf 4 continues a row on the next line, indented to the next column, when a name or version is wider than its column, which long names with RHEL repository ids reach. Such rows were dropped; they are now joined before parsing. Assisted-by: ClaudeCode:claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 <[email protected]> Signed-off-by: Frank Karlitschek <[email protected]>

The /proc scan took the last *.service component of any cgroup path, so a process in a user's own systemd unit, e.g. a user unit named php8.3-fpm.service under user.slice, was reported as a system service that needs a restart. Any local user could make the app warn about it. Only units under system.slice are taken now, in the cgroup v2 form and in the systemd hierarchy of cgroup v1. Only root can move a process there. The extraction is a function of its own so it can be tested. Assisted-by: ClaudeCode:claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 <[email protected]> Signed-off-by: Frank Karlitschek <[email protected]>

The checks that keep root from writing through a symlink or into a directory someone else controls had no tests, because they ran only inside main as root. They move unchanged into secure_directory and write_report, which tests call with stat(1) replaced by a shell function, so ownership and modes can be faked without root. Assisted-by: ClaudeCode:claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 <[email protected]> Signed-off-by: Frank Karlitschek <[email protected]>

A helper run can take up to about 40 minutes, so runs could overlap. A package manager that ignored SIGTERM outlived its timeout, a signal left the temporary report behind under dash, and a directory named like the report made mv move the report into it. - A run takes flock on /run/serverinfo-os-report.lock and exits quietly when another run holds it. busybox builds without flock run unlocked; each run still writes its own temporary file and renames it. - Commands get `timeout -k 30` where timeout supports -k (coreutils, busybox 1.30 and later), plain timeout where it does not, and no limit with the old busybox syntax. They do not inherit the lock. - HUP, INT and TERM remove the temporary report and exit with 1. - The report is refused when its path is a directory, or a symlink to one, because busybox mv has no -T. - --random-delay waits up to 30 minutes first. The documented cron line uses it, so servers sharing it do not reach the mirrors at the same minute, while runs by hand start at once. Assisted-by: ClaudeCode:claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 <[email protected]> Signed-off-by: Frank Karlitschek <[email protected]>

The instructions copied the helper from apps/serverinfo/bin, which the web server can write to. A compromised web server could replace the file and wait for the admin to copy it, and the "helper outdated" notice invites a new copy after every update. The script header and the README now say to take the helper from a freshly extracted release archive, checked against the published SHA-256 sum or signature, and state what a copy out of the installed app does and does not protect against. The header carries the full text because .nextcloudignore leaves the README out of the release. Assisted-by: ClaudeCode:claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 <[email protected]> Signed-off-by: Frank Karlitschek <[email protected]>

The report reader had a few edge cases that gave the wrong answer: - A report directory only root can enter (0700) hides the report from the web server, which then saw "absent" and suggested setting the helper up. A missing report in a root directory that neither group nor others may enter is now "unreadable". A report or directory whose ownership cannot be read is "unreadable" too, not "insecure". - open_basedir entries are directories whether or not they end in a slash, as in PHP itself, and an entry may name the report file. Such an entry made the report count as outside open_basedir. - A report dated more than five minutes ahead was called stale. It is now the new state "clock-skew", which the verdict treats like the other unusable reports (info). - Strings in pending updates and reboot reasons were taken at any length. Fields longer than 255 bytes now count as missing, nested lists such as advisories are cut to 100 entries, and overlong host names or version numbers make the report invalid. Assisted-by: ClaudeCode:claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 <[email protected]> Signed-off-by: Frank Karlitschek <[email protected]>

The install steps extracted the helper into the current directory and installed it from there. Run from /var/www, nextcloud/ is the live installation, which the web server owns: it could swap the extracted script between tar and sudo install, and root would then run its copy from cron. The README and the script header now extract into a new directory from mktemp -d, which only the admin can enter, install from there and remove the directory afterwards. Assisted-by: ClaudeCode:claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 <[email protected]> Signed-off-by: Frank Karlitschek <[email protected]>

The helper checked only the report's directory and its parent. With --output below a directory the web server can write to, the web server could swap a directory further up for a symbolic link and have root write a file named like the report elsewhere. The reader had the same partial check. The helper now walks from the report's directory up to /: every existing directory must be owned by root, writable by no one else and no symbolic link. World-writable directories with the sticky bit, such as /tmp, are refused as well, since anyone can create entries there. Existing directories are checked before mkdir -p creates the missing ones, so nothing is created below one that fails, and all of them again afterwards. HelperReportReader applies the same rules to the report and every directory above it and refuses symbolic links on the way, which the helper never creates. Directories that open_basedir hides from PHP cannot be inspected; they are left to the helper's check, so a setup that allows only the report's directory keeps working. The README says which directories the helper accepts. Assisted-by: ClaudeCode:claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 <[email protected]> Signed-off-by: Frank Karlitschek <[email protected]>

This was referenced Oct 19, 2026

feat(os): collect the operating system status #1214

Open

feat(os): support dnf, zypper, pacman and apk #1216

Open

feat(os): report operating system concerns where admins look #1217

Open

feat(os): add the Operating system page #1218

Open

karlitschek force-pushed the feat/os-status-root-helper branch from d6c84ac to b90d8ac Compare October 22, 2026 21:32

karlitschek force-pushed the feat/os-status-collector branch from b2085c1 to 2e8c497 Compare October 22, 2026 21:32

karlitschek force-pushed the feat/os-status-root-helper branch from b90d8ac to d16a601 Compare October 23, 2026 04:41

karlitschek force-pushed the feat/os-status-collector branch from 2e8c497 to eb88b3f Compare October 23, 2026 04:41

karlitschek marked this pull request as ready for review October 23, 2026 04:41

karlitschek requested review from benjaminfrueh and kesselb as code owners October 23, 2026 04:41

karlitschek force-pushed the feat/os-status-root-helper branch from d16a601 to f9455a7 Compare October 23, 2026 08:49

karlitschek force-pushed the feat/os-status-collector branch from eb88b3f to 60f5960 Compare October 23, 2026 08:49

AndyScherzinger reviewed Oct 23, 2026

View reviewed changes

karlitschek and others added 11 commits October 23, 2026 19:40

karlitschek force-pushed the feat/os-status-root-helper branch from f9455a7 to cede2f8 Compare October 23, 2026 17:41

karlitschek force-pushed the feat/os-status-collector branch from 60f5960 to 934a33d Compare October 23, 2026 17:41

Navigation Menu

Repository navigation

feat(os): add an optional root helper - #1215

feat(os): add an optional root helper#1215
karlitschek wants to merge 11 commits into
feat/os-status-collectorfrom
feat/os-status-root-helper

karlitschek commented Oct 19, 2026 •

edited

Loading

Uh oh!

AndyScherzinger left a comment

Uh oh!

AndyScherzinger Oct 23, 2026

Uh oh!

AndyScherzinger Oct 23, 2026

Uh oh!

AndyScherzinger Oct 23, 2026

Uh oh!

AndyScherzinger Oct 23, 2026

Uh oh!

AndyScherzinger Oct 23, 2026

Uh oh!

AndyScherzinger Oct 23, 2026

Uh oh!

AndyScherzinger Oct 23, 2026

Uh oh!

AndyScherzinger Oct 23, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

karlitschek commented Oct 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AndyScherzinger left a comment

Choose a reason for hiding this comment

Security

Correctness

Performance

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

karlitschek commented Oct 19, 2026 •

edited

Loading