Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Empty file added Cleanup).
Empty file.
15 changes: 15 additions & 0 deletions doc/changelog/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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()`,
Expand Down
21 changes: 17 additions & 4 deletions module/cowsay.module.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
61 changes: 57 additions & 4 deletions test/unit/module/cowsay_spec.bats
Original file line number Diff line number Diff line change
Expand Up @@ -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)" {
Expand Down Expand Up @@ -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
Expand All @@ -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
}

Expand Down
Loading