From b4e6bbf79fc4660af1b13aa6093497acd996c00c Mon Sep 17 00:00:00 2001 From: Christian Manivong Date: Sun, 20 Sep 2026 22:37:23 +0200 Subject: [PATCH] feat: purge, and a way out of `install ok unpacked` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both cases come from a fleet-wide Wazuh rollback. Of thirteen hosts carrying the agent, seven sat at `install ok unpacked` with the unit failed — an upgrade whose postinst could not reach a manager that had been decommissioned. `apt-get remove` cannot help there. apt configures a package before removing it, and configuring is precisely what was broken. On one host only `dpkg --purge --force-all` got it out. So `uninstall_package` takes `purge: bool = False`, and falls back to a forced dpkg purge **after apt has failed** — never as a routine second step. Forcing dpkg past its own consistency checks is a bigger hammer than apt, and a caller who reaches for it every time will eventually break something apt would rightly have refused. `purge` is off by default: configuration somebody may want back is not this function's to delete unless asked. It matters for more than tidiness — a package's apt source survives a plain remove, so the repository keeps being fetched on every update long after the package is gone, which is what the agent left behind on all thirteen. Found while writing the fallback test, and older than this change: the success check read apt's commonest failure as a success. `E: Sub-process /usr/bin/dpkg returned an error code (1)` contains neither "error:" nor "failed", so a removal that did not happen was reported as one that did — and the caller then records the package as gone. `_uninstall_failed` now also treats a line starting with `e: ` as failure, matched at line start because "note: " ends in "e: ". Reading success out of prose stays guesswork; the exit status is the real answer and `_sudo`'s `|| true` throws it away before anyone can read it. That is netork#267, deliberately not fixed here. apk and pacman have no separate purge. Asking for one there is not an error, it simply has nothing extra to do. --- napalm_linux/linux.py | 66 +++++++++++++++++++++++++++++++++++---- tests/test_linux.py | 72 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 132 insertions(+), 6 deletions(-) diff --git a/napalm_linux/linux.py b/napalm_linux/linux.py index f12e5fd..7460dc6 100644 --- a/napalm_linux/linux.py +++ b/napalm_linux/linux.py @@ -1045,24 +1045,78 @@ class LinuxDriver(OSDriver): success = not any(kw in low for kw in ("error:", "failed", "no packages", "not found", "unable to locate", "no match")) return {"success": success, "output": raw.strip()} - def uninstall_package(self, name: str) -> dict[str, Any]: - """Remove a package by name. Returns ``{"success": bool, "output": str}``.""" + #: Words in a package manager's output that mean it did not do the job. + _UNINSTALL_FAILED = ("error:", "failed", "not found", "is not installed", "no packages") + + def uninstall_package(self, name: str, purge: bool = False) -> dict[str, Any]: + """Remove a package by name. Returns ``{"success": bool, "output": str}``. + + ``purge`` also removes the package's configuration where the package + manager distinguishes the two. Off by default: configuration somebody + may want back is not this function's to delete unless it was asked for. + + It matters for more than tidiness. A package's apt source survives a + plain ``remove``, so the repository keeps being fetched on every + ``apt-get update`` long after the package itself is gone — which is what + the Wazuh agent left behind on thirteen hosts. + + **The dpkg fallback.** A package whose ``postinst`` failed sits at + ``install ok unpacked``, and apt cannot remove it: it configures a + package before removing it, and configuring is precisely what is broken. + Seven of those thirteen hosts were in that state after an upgrade whose + postinst could not reach a manager that had been decommissioned, and on + one of them only ``dpkg --purge --force-all`` got it out. + + So the fallback runs **only after apt has failed**, never as a routine + second step: forcing dpkg past its own consistency checks is a bigger + hammer than apt, and a caller who reaches for it every time will + eventually break something apt would have refused to. + """ from shlex import quote as _q safe = _q(name) pm = self._pkg_manager if pm == "apt": - raw = self._sudo(f"DEBIAN_FRONTEND=noninteractive apt-get remove -y {safe} 2>&1 || true") + action = "purge" if purge else "remove" + raw = self._sudo( + f"DEBIAN_FRONTEND=noninteractive apt-get {action} -y {safe} 2>&1 || true" + ) elif pm in ("dnf", "yum"): raw = self._sudo(f"{pm} remove -y {safe} 2>&1 || true") elif pm == "apk": + # apk and pacman have no separate purge; asking for one is not an + # error, it simply has nothing extra to do. raw = self._sudo(f"apk del {safe} 2>&1 || true") elif pm == "pacman": raw = self._sudo(f"pacman -R --noconfirm {safe} 2>&1 || true") else: return {"success": False, "output": f"Unsupported package manager: {pm}"} - low = raw.lower() - success = not any(kw in low for kw in ("error:", "failed", "not found", "is not installed", "no packages")) - return {"success": success, "output": raw.strip()} + + if self._uninstall_failed(raw) and pm == "apt": + forced = self._sudo(f"dpkg --purge --force-all {safe} 2>&1 || true") + if not self._uninstall_failed(forced): + return { + "success": True, + "output": f"{raw.strip()}\n--- dpkg --purge --force-all ---\n{forced.strip()}", + } + raw = f"{raw.strip()}\n--- dpkg --purge --force-all ---\n{forced.strip()}" + + return {"success": not self._uninstall_failed(raw), "output": raw.strip()} + + def _uninstall_failed(self, output: str) -> bool: + """Whether the package manager said it did not do the job. + + apt prefixes its own errors with ``E: `` at the start of a line, and + the commonest of them — ``E: Sub-process /usr/bin/dpkg returned an + error code (1)`` — contains neither "error:" nor "failed". The keyword + list alone therefore read a failed removal as a success, which is the + worst direction for this particular answer to be wrong in. + + Matched at line start rather than anywhere: "note: " ends in "e: ". + """ + low = output.lower() + if any(line.lstrip().startswith("e: ") for line in low.splitlines()): + return True + return any(kw in low for kw in self._UNINSTALL_FAILED) def get_available_updates(self) -> list[UpdateDict]: if self._pkg_manager == "apt": diff --git a/tests/test_linux.py b/tests/test_linux.py index edbf7d9..898aa6f 100644 --- a/tests/test_linux.py +++ b/tests/test_linux.py @@ -912,3 +912,75 @@ class TestDockerBinHook: assert docker_cmds for cmd in docker_cmds: assert "/opt/cs/docker" in cmd, f"unconverted call site: {cmd}" + + +# --------------------------------------------------------------------------- +# uninstall_package – purge, and getting out of `install ok unpacked` +# --------------------------------------------------------------------------- + + +class TestUninstallPackage: + """Removing a package that does not want to go. + + Both cases here were found during a fleet-wide Wazuh rollback. Of thirteen + hosts carrying the agent, seven sat at `install ok unpacked` with the unit + failed — an upgrade whose postinst could not reach a manager that no longer + existed. `apt-get remove` cannot help there: apt configures a package before + removing it, and configuring is exactly what was broken. + + And `remove` leaves the configuration behind by design, which for the Wazuh + agent means its apt source keeps being fetched on every update, long after + the package is gone. + """ + + def test_remove_is_still_the_default(self, driver): + """Callers that did not ask for a purge must not get one: configuration + somebody may want back is not this function's to delete.""" + _mock_send(driver, "Removing wazuh-agent ...") + + driver.uninstall_package("wazuh-agent") + + sent = driver._device.send_command.call_args[0][0] + assert "apt-get remove" in sent + assert "purge" not in sent + + def test_purge_is_asked_for_explicitly(self, driver): + _mock_send(driver, "Purging configuration files for wazuh-agent ...") + + driver.uninstall_package("wazuh-agent", purge=True) + + assert "apt-get purge" in driver._device.send_command.call_args[0][0] + + def test_a_half_configured_package_falls_back_to_dpkg(self, driver): + """`install ok unpacked` is the state apt cannot get out of. On one host + only `dpkg --purge --force-all` removed it.""" + driver._device.send_command.side_effect = [ + "E: Sub-process /usr/bin/dpkg returned an error code (1)", + "Removing wazuh-agent (4.14.7-1) ...", + ] + + result = driver.uninstall_package("wazuh-agent", purge=True) + + assert result["success"] is True + second = driver._device.send_command.call_args_list[1][0][0] + assert "dpkg --purge --force-all" in second + + def test_the_fallback_is_not_tried_when_the_first_pass_worked(self, driver): + """A forced dpkg purge is a bigger hammer than apt and must stay a last + resort, not a routine second step.""" + _mock_send(driver, "Removing wazuh-agent ...") + + driver.uninstall_package("wazuh-agent", purge=True) + + assert driver._device.send_command.call_count == 1 + + def test_a_package_manager_without_purge_still_removes(self, driver): + """apk and pacman have no separate purge; asking for one must not turn + into a failure or a command they do not understand.""" + driver._pkg_manager = "apk" + _mock_send(driver, "(1/1) Purging wazuh-agent") + + result = driver.uninstall_package("wazuh-agent", purge=True) + + assert result["success"] is True + assert "apk del" in driver._device.send_command.call_args[0][0]