diff --git a/Cleanup). b/Cleanup). new file mode 100644 index 0000000..e69de29 diff --git a/doc/changelog/CHANGELOG.md b/doc/changelog/CHANGELOG.md index 679b23b..92f4b57 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 df4d540..2929546 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 53e6178..54d1e83 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 }