Repository navigation
feat(os): support dnf, zypper, pacman and apk - #1216
karlitschek wants to merge 14 commits into
Conversation
27b6c04 to
6a62a2d
Compare
b90d8ac to
d16a601
Compare
6a62a2d to
a7044bb
Compare
d16a601 to
f9455a7
Compare
a7044bb to
f0d69b6
Compare
AndyScherzinger
left a comment
There was a problem hiding this comment.
I reviewed this for correctness, security and performance. I checked in particular whether the new package managers repeat the three apt mistakes from #1214: an empty result read as an all-clear, a day counter that resets, and systemd treated as a reboot reason. Inline comments carry the details. My dev instance is Debian, so I could not run these managers live; the parsers were checked against the fixtures and the code.
Repeats of the apt issues
- Empty result read as an all-clear:
- zypper accepts exit 106 (repositories skipped) as a complete answer.
- With arch-audit installed, pacman counts only what arch-audit flags as security. That covers few Arch fixes, so the verdict stays OK while security fixes are pending.
- systemd as a reboot reason: it comes back through dnf's
needs-restarting -r. Its reboot list includes systemd, andhasOwnRebootCheck()keeps the generic probes from correcting it. - Day counter reset: zypper rows switch between package and patch keys when the patch lookup fails once, which restarts the counter.
Security and performance
- Commands are run safely: argv-only,
LC_ALL=C, patch names validated before they go into argv, XML parsed withLIBXML_NONET, and read-only queries use--no-refresh/--cacheonly/--no-network. os_allow_networkdefaults to true. On Arch, every hourly run then contacts the mirrors and security.archlinux.org as the web server user, and writes$TMPDIR/checkup-db-<uid>. That contradicts the README's statement that nothing is looked up online.- Worst-case command time per run is several minutes for dnf and zypper. That is acceptable from system cron, but there is no overall budget.
Sound
- dnf: exit 100 handling, dnf 4 line wrapping, advisory columns of dnf 4 and 5, epoch handling, and dnf 5 never running as the web user.
- zypper: the informational exit-code set and the patch-to-package mapping.
- pacman: the exit codes of checkupdates and
-Qu. - apk: world-constraint filtering.
- First-seen keys are the package name now, and pacman joined
LISTS_AGE_MATTERS.
| $xmlError = preg_match('#<message type="error">([^<]*)</message>#', $result->stdout, $matches) === 1 | ||
| ? html_entity_decode($matches[1], ENT_QUOTES | ENT_XML1) | ||
| : null; | ||
| $skipped = $result->exitCode === self::EXIT_REPOS_SKIPPED; |
There was a problem hiding this comment.
Exit 106 is accepted as a complete answer, with only a diagnostic. As the web user with --no-refresh, a repository without a usable cache gets skipped, e.g. one root added but never refreshed. If that is the update repository, list-patches --category security returns nothing, and OsVerdict::updates() says OK. The diagnostic does not change the level, and the other repositories' cookies keep the lists age fresh. This is the same all-clear problem #1214 fixed for apt. Suggest making the updates unknown on 106 (or at least classifiesSecurity() false for that run).
|
|
||
| #[\Override] | ||
| public function classifiesSecurity(): bool { | ||
| return !$this->auditFailed && $this->networkAllowed() && $this->host->hasBinary('arch-audit'); |
There was a problem hiding this comment.
With arch-audit installed, only updates it flags count as security, and only those escalate. Its data (the security.archlinux.org advisories) covers only a small share of the security fixes Arch ships, so most of them become "other" and the verdict stays OK while they are pending. Suggest treating pacman as unclassified, so every update counts as it does for apk and Tumbleweed, and using arch-audit only to mark rows as known-vulnerable.
|
|
||
| return []; | ||
| } | ||
| if ($result->exitCode !== 1 || preg_match('/^Reboot is required/m', $result->stdout) !== 1) { |
There was a problem hiding this comment.
needs-restarting -r (dnf-plugins-core) has systemd and dbus on its reboot list, next to kernel, glibc and linux-firmware. So a routine systemd update on RHEL or Alma raises a reboot reason that escalates by days. That is the false positive #1214 removed for the core-library probe, coming back through the distribution's tool. Because hasOwnRebootCheck() is true, nothing corrects it. Suggest dropping the reason when every listed package is on an allowlist (systemd at least).
| return []; | ||
| } | ||
|
|
||
| $result = $this->host->run('zypper', ['--non-interactive', '--no-refresh', 'info', '-t', 'patch', ...array_slice($names, 0, self::MAX_PATCH_LOOKUP)], 180); |
There was a problem hiding this comment.
When this info -t patch call fails or times out once, the coverage map is empty. The security updates then come back as patch rows keyed by patch id instead of package rows keyed by package name. OsStatusStore::firstSeen() forgets the package keys, because they are in scope but absent, so the day counter restarts on the next good run; the failing run itself also reads 0 days. Suggest keying zypper's security rows by advisory id, keeping the previous coverage on failure, or making the updates unknown for that run.
| new Entry(self::OS_LISTS_WARNING_DAYS, ValueType::INT, 7, 'Days without a package list refresh before a warning', lazy: true), | ||
| new Entry(self::OS_LISTS_ERROR_DAYS, ValueType::INT, 30, 'Days without a package list refresh before an error', lazy: true), | ||
| new Entry( | ||
| self::OS_ALLOW_NETWORK, |
There was a problem hiding this comment.
Defaulting this to true means every hourly cron run on Arch makes outbound connections as the web server user. checkupdates syncs the core/extra databases, several MB whenever they changed, and arch-audit fetches its advisory list. checkupdates also writes $TMPDIR/checkup-db-<uid>. Admins with egress filtering get an hourly diagnostic, mirrors get about 24 database pulls a day per instance, and the README says the app never looks anything up online. Suggest defaulting to false (opt-in), or at least throttling these to about once a day.
|
|
||
| // Without --cacheonly, dnf 4 downloads the full repository metadata | ||
| // into a per-user cache just to list the locks. | ||
| $result = $this->dnf(['-q', '--cacheonly', 'versionlock', 'list'], 60); |
There was a problem hiding this comment.
Nit: on stock RHEL and Alma the versionlock plugin is not installed. versionlock list then fails, so held packages are permanently unknown instead of none. exclude= in dnf.conf, a common way to pin PHP or the kernel, is never reported. Suggest returning [] when /etc/dnf/plugins/versionlock.conf is absent, and optionally parsing exclude=.
|
|
||
| $yast = $this->host->readFile('/etc/sysconfig/automatic_online_update', 65536); | ||
| if ($yast !== null && preg_match('/^AOU_ENABLE_CRONJOB="?(true|yes)"?/mi', $yast) === 1) { | ||
| $securityOnly = preg_match('/^AOU_PATCH_CATEGORIES="?security"?/mi', $yast) === 1; |
There was a problem hiding this comment.
Nit: not anchored at the end, so AOU_PATCH_CATEGORIES="security recommended" is reported as security-only. /^AOU_PATCH_CATEGORIES="?security"?\s*$/mi fixes it.
| public function getRebootReasons(): array { | ||
| $result = $this->host->run('zypper', ['--non-interactive', 'needs-rebooting'], 60); | ||
| if (($result !== null && $result->exitCode === self::EXIT_REBOOT_NEEDED) | ||
| || ($result === null && $this->host->fileExists('/run/reboot-needed'))) { |
There was a problem hiding this comment.
Nit: /run/reboot-needed is only consulted when zypper could not be run at all. On a zypper without needs-rebooting, which exits non-zero with an unknown-command error, a pending reboot is silently missed. Suggest falling back to the file whenever the exit code is neither 0 nor 102.
| public function getPendingUpdates(): array { | ||
| $this->diagnostics = []; | ||
| if ($this->isRolling()) { | ||
| return self::parsePackageUpdates($this->zypperXml(['list-updates'])); |
There was a problem hiding this comment.
Nit: on Tumbleweed and MicroOS the supported upgrade is zypper dup. list-updates leaves out what only dup would do (renames, vendor changes), so pending updates are under-reported there. Worth documenting, or leaving rolling releases to the root helper's zypper dup --dry-run.
Pending updates, holds, automatic updates and reboot hints for RHEL and its rebuilds, Fedora, openSUSE and SLES, Arch and Alpine, checked against output captured on each distribution, which is kept as test fixtures. dnf 5 is never run as the web server: it needs a writable home and first copies its whole cache there, so the root helper reports pending updates on those systems. SUSE security patches mark the package updates they cover. 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>
dnf, zypper and apk each had their own loop taking the newest modification time of a set of globbed files, and their own way of cutting command output down for a diagnostic. Both now live in ProbeHelper. dnf and zypper ran `rpm -qa` twice per collection: once for the installed versions and once for the newest install time. One query now returns both. 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 parsers for dnf check-update, advisory NEVRAs and zypper patch conflicts only knew a fixed list of architectures. On AlmaLinux 10 for x86_64_v2, and on ppc64 or s390 systems, every line failed to match and the system reported no pending updates at all. Any token after the last dot now counts as an architecture when it starts with a letter, so versions such as the ".22" of "go1.22" stay part of the name. The pattern is one constant, Rpm::ARCH. dnf 4's versionlock patterns carry no architecture, and a release such as "1.el9" ends like one, so they are split without stripping one. 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>
dnf 5 must never run as the web server: it copies root's whole cache into the web server's home first. dnf 5 was recognised only by its dnf5 binary, and everything else ran plain `dnf`. A distribution that ships dnf 5 only as `dnf` would have had it run as the web server. dnf 4's real binary is dnf-3 on RHEL 8 to 10, their rebuilds and Amazon Linux 2023; on AlmaLinux 9 `dnf`, `dnf4` and `yum` are links to it. dnf is now run only as dnf-3, and only when dnf5 is not installed next to it, because then dnf 5 keeps the system cache and dnf 4's would be stale. Any other dnf is left to the root helper, like dnf 5. The "only root can ask" diagnostic is dropped when the helper report supplies the pending updates, since nothing is missing then. 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>
`dnf needs-restarting -r` exits 1 when a reboot is needed, but dnf exits 1 for its own errors as well. On an AlmaLinux 9 container without the plugin module, it printed "No such command: needs-restarting" to stderr and exited 1, which read as a permanent reboot reason. A reboot now also needs "Reboot is required" in the output, which LC_ALL=C keeps in English. When needs-restarting gives no answer, hasOwnRebootCheck() is false, so the generic core library check runs instead. hasOwnRebootCheck() reuses the answer of getRebootReasons() rather than running dnf a second time. The stderr fixtures were captured on almalinux:9 (dnf 4.14) as an unprivileged user. There, needs-restarting with --cacheonly and no cache at all still answered with exit 0; the no-cache error is tested anyway. 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 a terminal, dnf 4 lays the check-update columns out in 80 characters. When a column is wider than its share, as with long package names next to RHEL repository ids such as rhel-9-for-x86_64-appstream-rpms, dnf ends the line after that column and continues the row on the next line, indented to the next column (fmtColumns in dnf/cli/output.py). Only complete one-line rows were parsed, so those updates were silently lost. Indented lines now continue the row before them until it has its three columns. The test follows dnf's layout rules; capturing real output with a long repository id on AlmaLinux 9 failed because the mirrors could not be reached from the test host. 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>
dnf always claimed to tell security fixes apart. CentOS Stream publishes no updateinfo, and neither do many third-party repositories, so an openssl CVE fix there showed as an ordinary update and the verdict stayed green. And when `dnf updateinfo` failed, the whole update list was thrown away. classifiesSecurity() is now false for CentOS (ID=centos), when no cached repository of dnf 4 or dnf 5 has updateinfo metadata, and when the advisories could not be listed in this collection. The updates are then kept with security=false and an unclassified status, which the verdict already treats as "every update counts", so it never claims there are no security fixes. A failed advisory listing also adds a diagnostic. Package managers report such partial answers through the new IReportsDiagnostics, which the collector appends. The factory hands dnf the os-release it already parsed. 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>
…s busy zypper exits with 100 to 106 to say something beyond success: updates or a reboot are needed, or repositories were skipped. Every non-zero exit was treated as a failure, so a single never-refreshed SLES module (exit 106) would have cost the whole update list. Exit codes 0, 100 to 104 and 106 are now accepted when zypper finished and its XML parses; 106 adds a diagnostic that the list may be incomplete, with zypper's own message. 105 means zypper was interrupted and stays a failure. Exit 7, the package manager lock held by another process, now says zypper is busy instead of failing with the lock message. `zypper needs-rebooting` exiting 0 turned the core library check off, but it only knows what /etc/zypp/needreboot lists, which is nothing on the Leap 16 image. hasOwnRebootCheck() is now false, so the core library check runs, and needs-rebooting runs once per collection instead of twice. Patch names passed to `zypper info` must start with a letter or digit, so none can be read as an option. isRolling() uses the os-release the factory already parsed instead of reading the file three times. Exit 106 with --no-refresh on a never-refreshed repository could not be reproduced: the test host could not reach the mirrors. 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>
Three ways an Arch system read as up to date when it was not: - `pacman -Qu` errors that only went to stderr, such as a failed alpm initialisation or a held database lock, returned an empty list. "Nothing to update" now needs exit 1 with no output at all; every other failure is a diagnostic and the updates stay unknown. - When checkupdates could not sync, the fallback answered from root's sync databases without saying how old they are. The lists age is now the newest of the sync databases and their directory, pacman counts for the lists-age rules, and a diagnostic says the fallback was used. After a successful checkupdates the lists age is the time it synced. - A failed arch-audit threw away every update. The updates are now kept, unclassified, with a diagnostic. Adding pacman to OsVerdict::LISTS_AGE_MATTERS means its package lists concern is unknown, not ok, when no age can be found. 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>
Every other query of the operating system status reads only what is on the machine, but on Arch checkupdates syncs its own copy of the package databases and arch-audit fetches the advisories from security.archlinux.org, both as the web server user, on every run. The README now says so, and the new os_allow_network app config value (on by default, as before) turns both off. Pending updates then come from root's sync databases, whose age counts as the package list age, and they are not classified. 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>
Alpine was recognised by ID=alpine only; postmarketOS and other derivatives declare ID_LIKE=alpine and got no package manager at all. `apk list --upgradable` lists every newer version apk knows of, also those /etc/apk/world keeps it from installing. Only "=" pins were taken out. The world entries are now parsed as name[@tag][operator version], and an update is left out when its candidate does not meet the constraint: "=", "<", "<=" and the fuzzy "~" can hold one back. A repository tag alone ("curl@edge") is not a hold: it lets the package also come from the tagged repository, and apk upgrades it from there, so such an update stays pending and the package is no longer listed as held. Versions compare like apk's, closely enough for these constraints: upstream version, then the -rN release. apk runs with --no-network. apk-tools 3.0.8 on Alpine 3.24 accepts it before or after the applet and still lists from its cached indexes. 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 factory picks the package manager from os-release and then checks that the tool is installed. Nothing tested that choice. The new test runs it for Debian, RHEL 9 with dnf 4, Rocky, Oracle, Amazon Linux 2023 and Amazon Linux 2 (yum only, so none), CentOS Stream, Fedora with dnf 5, Leap, Tumbleweed, SLES, Arch, Manjaro, Alpine, postmarketOS and an unsupported distribution, together with whether security fixes can be told apart. It also checks that RHEL 9 runs dnf-3 and Fedora with dnf 5 runs nothing. 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>
…ibution apt already says "timed out after N s" or "output too large" when a command fails for those reasons, and it is tried when open_basedir hides apt-get. dnf, zypper, pacman and apk still quoted the first output line, which is often a progress line after a timeout, and were not tried at all when open_basedir hid their binary. They now use CommandResult::failure() for the failure text and treat an unknown binary state as "try it", like apt. 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 README and the os_allow_network setting said that checkupdates and arch-audit use the network on Arch, but not where to. They now name the pacman mirrors and security.archlinux.org, and the README says that these are the only outbound connections of the operating system status, so an admin can allow exactly those or turn them off. 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>
f0d69b6 to
e9df3f3
Compare
f9455a7 to
cede2f8
Compare
Part 3 of 5, stacked on #1215:
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