From 9d46547ef43c4b4c6303e3f1ba9a9e377007ac4b Mon Sep 17 00:00:00 2001 From: magges Date: Mon, 13 Jul 2026 22:10:44 +0000 Subject: [PATCH] Fix package-verify false negatives: symlink traversal + prefix collision MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit podman-verify-packages.sh/podman-update-packages.sh reported every package as "not installed" right after a genuinely successful install. Root cause: /var/log/packages is itself a symlink on Unraid (-> ../lib/pkgtools/packages), and GNU find's default -P mode doesn't descend into a symlinked starting path at all without -L — confirmed live by running the exact same find both with and without -L against a host where the packages had just installed successfully. Fixing that alone surfaced a second, previously-masked bug: "podman"'s own glob also matches podman-compose's file (a literal prefix collision), and find's unsorted output let podman-compose's record silently win podman's own check. Both fixed once via a new shared podman_find_installed_package_record() helper in podman-common.sh, rather than separately in each caller. Co-Authored-By: Claude Sonnet 5 --- CHANGELOG.md | 16 +++++++++ plugin/sbin/podman-common.sh | 49 +++++++++++++++++++++++++++ plugin/sbin/podman-update-packages.sh | 7 +++- plugin/sbin/podman-verify-packages.sh | 4 ++- 4 files changed, 74 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a723689..65a7e36 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,22 @@ see [docs/ARCHITECTURE.md](docs/ARCHITECTURE.md#52-build-strategie)). ## [Unreleased] +### Fixed +- `podman-verify-packages.sh`/`podman-update-packages.sh` reported every + single package as "not installed" right after a genuinely successful + install (confirmed live on a real v0.1.5 install: `/var/log/packages/` + had the correct records the whole time). Root cause: + `/var/log/packages` is itself a symlink on Unraid + (`-> ../lib/pkgtools/packages`), and GNU `find`'s default `-P` mode + doesn't descend into a symlinked starting path at all without `-L` — + it just returns the symlink itself and nothing below it. Fixed via a + new shared `podman_find_installed_package_record()` helper in + `podman-common.sh`, which also fixes a second bug found while testing + the first fix: "podman"'s own glob also matched podman-compose's file + (a literal prefix collision), and `find`'s unsorted output let + podman-compose's record silently win podman's own check on the same + real host. + ## [0.1.5] - 2026-07-13 ### Added diff --git a/plugin/sbin/podman-common.sh b/plugin/sbin/podman-common.sh index 0295034..06e8530 100755 --- a/plugin/sbin/podman-common.sh +++ b/plugin/sbin/podman-common.sh @@ -195,6 +195,55 @@ podman_path_has_real_mount_ancestor() { [ "$parent" != "/" ] && [ "$(stat -c %d "$parent")" != "$(stat -c %d /)" ] } +# ----------------------------------------------------------------------------- +# podman_find_installed_package_record +# +# Prints the /var/log/packages/-... record for (empty/failure +# if not installed). Two real bugs, both found live on an actual host, +# fixed here once instead of separately in every script that needs this: +# +# -L: /var/log/packages is itself a symlink on Unraid (-> +# ../lib/pkgtools/packages) — without it, GNU find's default -P mode +# doesn't descend into it AT ALL (it returns just the symlink itself and +# nothing below it), so every package was reported "not installed" +# regardless of what had actually just been installed. +# +# Prefix collision: a plain `-name "$name-*"` glob for "podman" also +# matches podman-compose's file ("podman" is a literal prefix of +# "podman-compose"), and find's output order isn't sorted, so whichever +# one happened to come back first silently won — on a real host, that +# was podman-compose's record, reported as if it were podman's own. +# Explicitly skip any match that actually belongs to a different, more +# specific name also in the given package list. +# ----------------------------------------------------------------------------- +podman_find_installed_package_record() { + local name="$1" + shift + local all_names=("$@") + local candidate other belongs_to_other base + while IFS= read -r candidate; do + [ -n "$candidate" ] || continue + base=$(basename "$candidate") + belongs_to_other=0 + for other in "${all_names[@]}"; do + # Only a LONGER (more specific) other name can steal a match — + # "podman" is itself a prefix of "podman-compose", so without the + # length check, searching for "podman-compose" would wrongly + # exclude its own, genuinely correct record too (found live: this + # exact over-correction on the very first fix attempt). + if [ "$other" != "$name" ] && [ "${#other}" -gt "${#name}" ] && [ "${base#"$other"-}" != "$base" ]; then + belongs_to_other=1 + break + fi + done + if [ "$belongs_to_other" -eq 0 ]; then + echo "$candidate" + return 0 + fi + done < <(find -L /var/log/packages -maxdepth 1 -name "${name}-*" -print 2> /dev/null) + return 1 +} + # ----------------------------------------------------------------------------- # podman_require_command # diff --git a/plugin/sbin/podman-update-packages.sh b/plugin/sbin/podman-update-packages.sh index d73348c..d63c442 100755 --- a/plugin/sbin/podman-update-packages.sh +++ b/plugin/sbin/podman-update-packages.sh @@ -68,7 +68,12 @@ for name in $targets; do continue fi - installed_record=$(find /var/log/packages -maxdepth 1 -name "${name}-*" -print 2> /dev/null | head -n1) + # See podman-common.sh's podman_find_installed_package_record() for why + # this isn't just a plain `find ... -name "$name-*" | head -n1` — note + # it's given $ALL_PACKAGES (not $targets), since prefix-collision + # detection needs the full component universe even when only updating + # a subset of it. + installed_record=$(podman_find_installed_package_record "$name" $ALL_PACKAGES) installed_basename=$(basename "${installed_record:-__none__}") case "$installed_basename" in diff --git a/plugin/sbin/podman-verify-packages.sh b/plugin/sbin/podman-verify-packages.sh index a4414d1..c3d4b9f 100755 --- a/plugin/sbin/podman-verify-packages.sh +++ b/plugin/sbin/podman-verify-packages.sh @@ -72,7 +72,9 @@ for name in $PACKAGES; do report " expected version: $expected_version" # --- Check 1: installed ----------------------------------------------- - installed_record=$(find /var/log/packages -maxdepth 1 -name "${name}-*" -print 2> /dev/null | head -n1) + # See podman-common.sh's podman_find_installed_package_record() for why + # this isn't just a plain `find ... -name "$name-*" | head -n1`. + installed_record=$(podman_find_installed_package_record "$name" $PACKAGES) if [ -z "$installed_record" ]; then report " installed: NO" podman_log_error "verify: $name is not installed (no /var/log/packages/$name-* record)"