fix: report the source package's version, not only its name
`get_packages` named the Debian source package and never its version, so a
consumer was handed two numbers on different axes and no way to tell.
OSV states Debian ranges in *source* versions. libldb2 is
2:2.11.0+samba4.22.11+dfsg-… while its source, samba, is 2:4.22.11+dfsg-…;
comparing the first against a samba range is meaningless, and dpkg reads
ldb's 2.11.0 as older than the 2:4.17.4+dfsg-1 that fixed CVE-2022-44640.
Reporting the source without its version is worse than reporting neither,
because it looks usable.
Measured on three live Proxmox nodes: every one of their 2 349 packages
was in that state — 802 of 802, 774 of 774, 773 of 773 — while twenty
non-Proxmox hosts had both fields. It was not a parsing bug. The
dpkg-query format string never asked for ${source:Version}, so nothing
downstream could have recovered it.
Now asked for and reported, with the same fallback napalm-linux uses:
dpkg leaves the field empty when it equals Version, and an older dpkg
leaves it empty because it does not know the field at all. Neither may
produce a package without a coordinate.
tests/test_packages.py covers all of it, including that the *query* names
the field — the assertion that would have caught this.
This commit is contained in:
@@ -245,7 +245,8 @@ class ProxmoxSystemMixin:
|
||||
# Installed packages via SSH dpkg-query
|
||||
raw = self._exec_ssh_command(
|
||||
"dpkg-query -W -f="
|
||||
"'${Package}\\t${Version}\\t${db:Status-Status}\\t${Installed-Size}\\t${source:Package}\\n'"
|
||||
"'${Package}\\t${Version}\\t${db:Status-Status}\\t${Installed-Size}"
|
||||
"\\t${source:Package}\\t${source:Version}\\n'"
|
||||
" 2>/dev/null"
|
||||
)
|
||||
result: list[_JsonDict] = []
|
||||
@@ -260,6 +261,19 @@ class ProxmoxSystemMixin:
|
||||
# Debian source package (differs from the binary for split packages,
|
||||
# e.g. openssh-server → openssh). Needed for accurate OSV matching.
|
||||
source_package = parts[4] if len(parts) > 4 and parts[4] else name
|
||||
# And its version, which is a different number from this package's.
|
||||
#
|
||||
# OSV states Debian ranges in *source* versions, so a consumer given
|
||||
# the source package and only the binary version compares two
|
||||
# unrelated numbers: libldb2 is 2:2.11.0+samba4.22.11+dfsg-… while
|
||||
# its source, samba, is 2:4.22.11+dfsg-…. Reporting the source
|
||||
# without its version is worse than reporting neither, because it
|
||||
# looks usable. Every package on all three Proxmox nodes was in that
|
||||
# state — the format string never asked for the field.
|
||||
#
|
||||
# dpkg leaves it empty when it equals `Version`, and so does an
|
||||
# older dpkg that does not know the field at all.
|
||||
source_version = parts[5] if len(parts) > 5 and parts[5] else version
|
||||
if not name or status != "installed":
|
||||
continue
|
||||
size_bytes = int(size_kb) * 1024 if size_kb.isdigit() else 0
|
||||
@@ -271,6 +285,7 @@ class ProxmoxSystemMixin:
|
||||
"size": size_bytes,
|
||||
"source": "pve",
|
||||
"source_package": source_package,
|
||||
"source_version": source_version,
|
||||
"upgrade_version": upgradable.get(name, ""),
|
||||
})
|
||||
return result
|
||||
|
||||
@@ -0,0 +1,67 @@
|
||||
"""`get_packages` and the source coordinate OSV matching needs.
|
||||
|
||||
A Proxmox node is a Debian host, so its packages are matched against Debian
|
||||
advisories — and OSV states those ranges in *source* package versions. Reporting
|
||||
the source package without its version leaves a consumer holding two numbers on
|
||||
different axes: `libldb2` is `2:2.11.0+samba4.22.11+dfsg-…` while its source,
|
||||
samba, is `2:4.22.11+dfsg-…`, and comparing the first against a samba range is
|
||||
meaningless.
|
||||
|
||||
Measured before this was fixed: on three Proxmox nodes, **every one** of their
|
||||
2 349 packages carried a source package and no source version — 802 of 802,
|
||||
774 of 774, 773 of 773 — while twenty non-Proxmox hosts had both. The format
|
||||
string simply never asked for the field.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import pytest
|
||||
|
||||
# `${Package}\t${Version}\t${db:Status-Status}\t${Installed-Size}\t${source:Package}\t${source:Version}`
|
||||
DPKG = (
|
||||
"openssh-server\t1:9.2p1-2\tinstalled\t1024\topenssh\t1:9.2p1-2\n"
|
||||
"libldb2\t2:2.11.0+samba4.22.11+dfsg-0+deb13u1\tinstalled\t512\tsamba\t2:4.22.11+dfsg-0+deb13u1\n"
|
||||
"curl\t7.88.1-10\tinstalled\t256\t\t\n"
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("driver_with_exec", [DPKG], indirect=True)
|
||||
def test_the_source_version_is_reported(driver_with_exec):
|
||||
"""The field the whole fix is about."""
|
||||
packages = {p["name"]: p for p in driver_with_exec.get_packages()}
|
||||
|
||||
assert packages["libldb2"]["source_package"] == "samba"
|
||||
assert packages["libldb2"]["source_version"] == "2:4.22.11+dfsg-0+deb13u1"
|
||||
|
||||
|
||||
@pytest.mark.parametrize("driver_with_exec", [DPKG], indirect=True)
|
||||
def test_the_binary_version_is_kept_beside_it(driver_with_exec):
|
||||
"""Both are wanted: one says what is installed, the other is the axis the
|
||||
advisory's range is stated on."""
|
||||
libldb2 = {p["name"]: p for p in driver_with_exec.get_packages()}["libldb2"]
|
||||
|
||||
assert libldb2["version"] == "2:2.11.0+samba4.22.11+dfsg-0+deb13u1"
|
||||
|
||||
|
||||
@pytest.mark.parametrize("driver_with_exec", [DPKG], indirect=True)
|
||||
def test_an_empty_source_falls_back_to_the_package_itself(driver_with_exec):
|
||||
"""dpkg leaves both fields empty when the source is the package — and an
|
||||
older dpkg leaves them empty because it does not know the field at all.
|
||||
Neither may produce a package with no coordinate."""
|
||||
curl = {p["name"]: p for p in driver_with_exec.get_packages()}["curl"]
|
||||
|
||||
assert curl["source_package"] == "curl"
|
||||
assert curl["source_version"] == "7.88.1-10"
|
||||
|
||||
|
||||
def test_the_query_asks_for_the_field():
|
||||
"""The defect was in the format string, not in the parsing: the driver
|
||||
reported a source package it had asked for and a source version it had
|
||||
not, so no amount of parsing could have produced one."""
|
||||
import inspect
|
||||
|
||||
from napalm_proxmox import system_mixin
|
||||
|
||||
source = inspect.getsource(system_mixin.ProxmoxSystemMixin.get_packages)
|
||||
|
||||
assert "source:Version" in source
|
||||
Reference in New Issue
Block a user