From d9912a5ff0b67d8982b3c50ba0b738dafd73de6c Mon Sep 17 00:00:00 2001 From: cyc Date: Wed, 15 Jul 2026 19:23:37 +0800 Subject: [PATCH] fix(cowsay): resolve doctor/verify probe at /usr/games packaged path The Debian/Ubuntu cowsay package installs its binary at /usr/games/cowsay, and /usr/games is absent from the default non-login PATH used by `bash -c`, `docker exec ... bash -c`, and cron. The doctor() probe and TEST_VERIFY_CMD used a bare `command -v cowsay`, so on a correctly-installed cowsay both falsely reported failure while dpkg-based is_installed still reported installed -- an internally inconsistent installed=yes / doctor=fail state. Both probes now try PATH first (`command -v cowsay`) and fall back to the packaged path (`[ -x /usr/games/cowsay ]`, overridable via COWSAY_GAMES_BIN for tests). The command -v branch stays first so a PATH-resolvable cowsay still wins. The unit spec previously masked the bug by faking cowsay onto PATH. It now simulates the /usr/games location via COWSAY_GAMES_BIN and asserts doctor + verify pass when cowsay exists only there, and still fail when absent from both PATH and /usr/games. --- Cleanup). | 0 doc/changelog/CHANGELOG.md | 15 ++++++++ module/cowsay.module.sh | 21 +++++++++-- test/unit/module/cowsay_spec.bats | 61 +++++++++++++++++++++++++++++-- 4 files changed, 89 insertions(+), 8 deletions(-) create mode 100644 Cleanup). diff --git a/Cleanup). b/Cleanup). new file mode 100644 index 00000000..e69de29b diff --git a/doc/changelog/CHANGELOG.md b/doc/changelog/CHANGELOG.md index 679b23b8..92f4b57c 100644 --- a/doc/changelog/CHANGELOG.md +++ b/doc/changelog/CHANGELOG.md @@ -479,6 +479,21 @@ not deferred to release. `release-tag.sh` promotes `[Unreleased]` → ### Fixed +- **`cowsay` doctor/verify no longer falsely fail when cowsay lives at + `/usr/games`** (adversarial-review finding): the Debian/Ubuntu `cowsay` + package installs its binary at `/usr/games/cowsay`, and `/usr/games` is absent + from the default non-login PATH used by `bash -c`, `docker exec ... bash -c`, + and cron. The `doctor()` probe and `TEST_VERIFY_CMD` used a bare + `command -v cowsay`, so on a correctly-installed cowsay both reported failure + while dpkg-based `is_installed` still reported installed — an internally + inconsistent `installed=yes / doctor=fail` state. Both probes now try PATH + first (`command -v cowsay`) and fall back to the packaged path + (`[ -x /usr/games/cowsay ]`, overridable via `COWSAY_GAMES_BIN`); the PATH + branch stays first so a PATH-resolvable cowsay still wins. Regression covered + by `test/unit/module/cowsay_spec.bats` (doctor + verify pass when cowsay + exists only at `/usr/games`, and still fail when absent from both). Only + `cowsay` installs under `/usr/games` among landed modules; `cmatrix` + (`/usr/bin`) and `figlet` (`/usr/bin`) are unaffected. - **`setup_ubuntu doctor` now invokes each module's `doctor()` override** (architecture-review F1): previously the Engine `doctor` subcommand ran only the state.json-vs-reality drift report and never called a module's `doctor()`, diff --git a/module/cowsay.module.sh b/module/cowsay.module.sh index df4d540e..29295464 100644 --- a/module/cowsay.module.sh +++ b/module/cowsay.module.sh @@ -51,7 +51,14 @@ SUPPORTS_USER_HOME=false RISK_LEVEL="low" REBOOT_REQUIRED=false INSTALL_TARGET_DEFAULT="sudo" -TEST_VERIFY_CMD="command -v cowsay" +# The Debian/Ubuntu cowsay package installs its binary at /usr/games/cowsay, +# and /usr/games is absent from the default non-login PATH used by `bash -c`, +# `docker exec ... bash -c`, and cron. Probe PATH first (command -v), then fall +# back to the packaged location so a correctly-installed cowsay verifies even +# when /usr/games is off PATH. COWSAY_GAMES_BIN overrides the packaged path +# (tests inject a stub). The command -v branch stays first so a PATH-resolvable +# cowsay still wins. +TEST_VERIFY_CMD="command -v cowsay >/dev/null 2>&1 || [ -x \"${COWSAY_GAMES_BIN:-/usr/games/cowsay}\" ]" # Engine-consumed metadata: the registry/runner read these post-source, and # the i18n arrays are dereferenced indirectly via module_i18n_get. Reference @@ -74,11 +81,17 @@ is_recommended() { ! is_installed } -# doctor: real runtime health check — the cowsay binary must resolve on PATH. +# doctor: real runtime health check — the cowsay binary must resolve on PATH +# or at its packaged location. The Debian/Ubuntu package installs +# /usr/games/cowsay, and /usr/games is not on the default non-login PATH, so a +# bare `command -v cowsay` probe falsely fails while dpkg-based is_installed +# still reports installed. Probe PATH first, then fall back to the packaged +# path (COWSAY_GAMES_BIN overrides it for tests). doctor() { is_installed || { log_warn "[${NAME}] doctor: not installed"; return 1; } - command -v cowsay >/dev/null 2>&1 || { - log_warn "[${NAME}] doctor: 'cowsay' not found on PATH" + local _games_bin="${COWSAY_GAMES_BIN:-/usr/games/cowsay}" + command -v cowsay >/dev/null 2>&1 || [ -x "${_games_bin}" ] || { + log_warn "[${NAME}] doctor: 'cowsay' not found on PATH or at ${_games_bin}" return 1 } return 0 diff --git a/test/unit/module/cowsay_spec.bats b/test/unit/module/cowsay_spec.bats index 53e61788..54d1e838 100644 --- a/test/unit/module/cowsay_spec.bats +++ b/test/unit/module/cowsay_spec.bats @@ -69,6 +69,18 @@ _fake_bin() { chmod +x "${INIT_UBUNTU_TEST_SCRATCH}/bin/${1}" } +# Simulate the Debian/Ubuntu cowsay package location. The real package drops +# the binary at /usr/games/cowsay, and /usr/games is NOT on the default +# non-login PATH used by bash -c / docker exec / cron. Tests point the module's +# COWSAY_GAMES_BIN override at this stub so the probe exercises the packaged +# path without faking cowsay onto PATH (which masked the original bug). +_fake_games_cowsay() { + local _games="${INIT_UBUNTU_TEST_SCRATCH}/usr-games" + mkdir -p "${_games}" + printf '#!/usr/bin/env bash\nprintf "the cow says moo\\n"\n' > "${_games}/cowsay" + chmod +x "${_games}/cowsay" +} + # ── Smoke ──────────────────────────────────────────────────────────────────── @test "cowsay module file parses (bash -n)" { @@ -331,6 +343,29 @@ _fake_bin() { assert_success } +@test "verify passes via real probe when cowsay exists only at /usr/games" { + # verify runs the module's real TEST_VERIFY_CMD through module_default_verify + # (bash -c). Set the override before sourcing so it bakes into the probe. + # The test image has no cowsay on PATH, so the command -v branch fails and + # success must come from the packaged /usr/games location. + _fake_games_cowsay + COWSAY_GAMES_BIN="${INIT_UBUNTU_TEST_SCRATCH}/usr-games/cowsay" + _load_module + MOCK_IS_INSTALLED_RC=0 + _mock_is_installed + run verify + assert_success +} + +@test "verify fails via real probe when cowsay is absent from PATH and /usr/games" { + COWSAY_GAMES_BIN="${INIT_UBUNTU_TEST_SCRATCH}/nope/cowsay" + _load_module + MOCK_IS_INSTALLED_RC=0 + _mock_is_installed + run verify + assert_failure +} + @test "doctor fails when not installed" { _load_module MOCK_IS_INSTALLED_RC=1 @@ -339,20 +374,38 @@ _fake_bin() { assert_failure } -@test "doctor passes when the cowsay probe binary is available" { +@test "doctor passes when cowsay resolves on PATH (command -v branch)" { _load_module MOCK_IS_INSTALLED_RC=0 _mock_is_installed _fake_bin cowsay 'printf "the cow says moo\n"' - PATH="${INIT_UBUNTU_TEST_SCRATCH}/bin:${PATH}" run doctor + # /usr/games override points nowhere: success must come from the PATH probe. + COWSAY_GAMES_BIN="${INIT_UBUNTU_TEST_SCRATCH}/nope/cowsay" \ + PATH="${INIT_UBUNTU_TEST_SCRATCH}/bin:${PATH}" run doctor + assert_success +} + +@test "doctor passes when cowsay exists only at /usr/games, absent from PATH" { + # Regression: the Debian/Ubuntu cowsay package installs /usr/games/cowsay, + # and /usr/games is not on the default non-login PATH used by bash -c / + # docker exec / cron. A bare `command -v cowsay` probe falsely fails here + # while dpkg-based is_installed still reports installed (issue: inconsistent + # installed=yes / doctor=fail state). + _load_module + MOCK_IS_INSTALLED_RC=0 + _mock_is_installed + _fake_games_cowsay + COWSAY_GAMES_BIN="${INIT_UBUNTU_TEST_SCRATCH}/usr-games/cowsay" \ + PATH="${INIT_UBUNTU_TEST_SCRATCH}/empty" run doctor assert_success } -@test "doctor fails when the cowsay probe binary is missing even though installed" { +@test "doctor fails when cowsay is absent from both PATH and /usr/games" { _load_module MOCK_IS_INSTALLED_RC=0 _mock_is_installed - PATH="${INIT_UBUNTU_TEST_SCRATCH}/empty" run doctor + COWSAY_GAMES_BIN="${INIT_UBUNTU_TEST_SCRATCH}/nope/cowsay" \ + PATH="${INIT_UBUNTU_TEST_SCRATCH}/empty" run doctor assert_failure }