diff --git a/Justfile b/Justfile index dec258f31..4dd50f798 100644 --- a/Justfile +++ b/Justfile @@ -9,6 +9,7 @@ test: bats tests/test_privileged_setup.bats bats tests/test_bling.bats bats tests/test_bling_sh.bats + bats tests/test_bling_preexec_rearm.bats bats tests/test_luks_tpm2.bats bats tests/test_rechunker_group_fix.bats bats tests/test_bling_fastfetch.bats diff --git a/docs/TESTING.md b/docs/TESTING.md index 98ee356a3..ab4f295fe 100644 --- a/docs/TESTING.md +++ b/docs/TESTING.md @@ -84,6 +84,7 @@ Do not add exemptions for scripts with branching logic. | `tests/test_setup_scripts.bats` | `ublue-system-setup`, `ublue-user-setup` — hook runner logic | | `tests/test_privileged_setup.bats` | `ublue-privileged-setup` — privileged hook runner logic | | `tests/test_bling.bats` | `ublue-bling` — shell config injection install/uninstall | +| `tests/test_bling_preexec_rearm.bats` | `bling/bash-preexec-rearm.sh` — DEBUG trap re-arm with array/scalar `PROMPT_COMMAND`, idempotency, degradation when bash-preexec is absent | | `tests/test_luks_tpm2.bats` | `luks-tpm2-autounlock` — UUID parsing, device resolution, cryptenroll flag construction | | `tests/test_rechunker_group_fix.bats` | `rechunker-group-fix` — group/gshadow append, duplicate detection, format | | `tests/test_bling_fastfetch.bats` | `ublue-bling-fastfetch` — all 9 accent colors, dconf/gsettings fallback chain, FASTFETCH_FORCE_THEME override | diff --git a/docs/skills/index.json b/docs/skills/index.json index 92cec3a81..05210e9a5 100644 --- a/docs/skills/index.json +++ b/docs/skills/index.json @@ -592,7 +592,7 @@ ], "description": "Shell script authoring and testability. Use when writing or testing shell scripts under system_files/, removing scripts, or adding bats tests.", "version": "1.0", - "last_updated": "2026-08-01", + "last_updated": "2026-08-06", "doc_type": "reference" }, { diff --git a/docs/skills/shell-scripts.md b/docs/skills/shell-scripts.md index 9bf0d4dd1..49486deeb 100644 --- a/docs/skills/shell-scripts.md +++ b/docs/skills/shell-scripts.md @@ -1,7 +1,7 @@ --- name: shell-scripts version: "1.0" -last_updated: "2026-08-01" +last_updated: "2026-08-06" id: shell-scripts one_line_purpose: Write and test shell scripts under system_files/. entry_point: docs/skills/shell-scripts.md @@ -438,6 +438,41 @@ grep -q "^name:!\*::" file grep -qF "name:!*::" file ``` +### Bash DEBUG traps are invisible inside functions + +Without `set -o functrace`, bash does **not** inherit the `DEBUG` trap into +shell functions. Two consequences bite when testing or writing prompt hooks: + +```bash +f() { echo "[$(trap -p DEBUG)]"; } # always prints [] — even when a trap is set +g() { trap - DEBUG; } # does NOT clear the caller's DEBUG trap +h() { trap 'cmd' DEBUG; } # DOES set the caller's DEBUG trap +``` + +So `trap -p DEBUG` is useless as a detector from inside a function, while +`trap ... DEBUG` from inside a function is a reliable way to (re-)install one. + +For bats: `PROMPT_COMMAND` entries execute at **top level** in a real shell. +Simulate a prompt cycle with a top-level loop, never a helper function — +wrapping the cycle in a function hides `trap - DEBUG` clobbers entirely and +makes the test pass vacuously. + +```bash +CYCLE='for __e in "${PROMPT_COMMAND[@]}"; do eval "$__e"; done' +``` + +See `tests/test_bling_preexec_rearm.bats` and +[#869](https://github.com/projectbluefin/common/issues/869). + +### POSIX-`sh` files cannot hold bash array code + +`system_files/**/*.sh` is shellchecked with the dialect from its shebang. +`bling.sh` is `#!/usr/bin/env sh`, so bash arrays, `BASH_SOURCE`, and `+=(...)` +trip SC3028/SC3030/SC3054 and fail CI. Put bash-only logic in a sibling +`#!/usr/bin/env bash` file and source it from inside the existing +`[ "${BLING_SHELL}" = "bash" ]` guard, with a `BLING_DIR` override so bats can +point at the repo tree instead of `/usr/share/ublue-os/bling`. + ## Red Flags - A shell script reads from a hardcoded `/proc`, `/dev`, or `/usr/share/...` path without an env-var override — untestable in CI diff --git a/system_files/shared/usr/share/ublue-os/bling/bash-preexec-rearm.sh b/system_files/shared/usr/share/ublue-os/bling/bash-preexec-rearm.sh new file mode 100644 index 000000000..b629b812e --- /dev/null +++ b/system_files/shared/usr/share/ublue-os/bling/bash-preexec-rearm.sh @@ -0,0 +1,48 @@ +#!/usr/bin/env bash +# Re-arm the bash-preexec DEBUG trap on every prompt. +# +# Fedora (bash >= 5.1) exposes PROMPT_COMMAND as an *array*. bash-preexec 0.6.0 +# defers its own installation by appending a string to PROMPT_COMMAND: +# +# __bp_trap_string="$(trap -p DEBUG)"; trap - DEBUG; __bp_install +# +# __bp_install is supposed to delete that string again, but it only ever reads +# and rewrites "${PROMPT_COMMAND}" — which expands to element [0] alone. Once +# another hook (direnv, starship, mise, zoxide, vte, systemd) has pushed the +# installer into a later array element, it is never removed and therefore runs +# on *every* prompt. Its first act is `trap - DEBUG`, and __bp_install then +# returns early because PROMPT_COMMAND already contains __bp_precmd_invoke_cmd. +# From the second prompt onward the DEBUG trap is permanently empty, so every +# preexec consumer silently stops firing — atuin loads and CTRL+R works, but no +# command is ever recorded. +# +# Re-arming the trap at the end of each prompt cycle restores the invariant +# bash-preexec assumes without patching or vendoring bash-preexec itself. +# +# See: https://github.com/projectbluefin/common/issues/869 +# https://github.com/rcaloras/bash-preexec/issues/188 +# https://github.com/rcaloras/bash-preexec/issues/186 + +# Only meaningful when bash-preexec is actually loaded, and only safe when we +# are allowed to write PROMPT_COMMAND (bash-preexec bails out in that case too). +if [[ "$(type -t __bp_preexec_invoke_exec)" == "function" ]] && + (unset PROMPT_COMMAND) 2>/dev/null; then + + # Re-install the exact trap bash-preexec installs in __bp_install. A prior, + # non-bash-preexec DEBUG trap is not lost: bash-preexec preserves it as + # __bp_original_debug_trap inside preexec_functions. + __bling_rearm_bp_debug_trap() { + trap '__bp_preexec_invoke_exec "$_"' DEBUG + } + + # Idempotent — sourcing bling.sh twice must not queue the hook twice. + if [[ "${PROMPT_COMMAND[*]-}" != *__bling_rearm_bp_debug_trap* ]]; then + if ((BASH_VERSINFO[0] > 5 || (BASH_VERSINFO[0] == 5 && BASH_VERSINFO[1] >= 1))); then + PROMPT_COMMAND+=('__bling_rearm_bp_debug_trap') + elif [[ -n "${PROMPT_COMMAND:-}" ]]; then + PROMPT_COMMAND="${PROMPT_COMMAND}"$'\n'"__bling_rearm_bp_debug_trap" + else + PROMPT_COMMAND="__bling_rearm_bp_debug_trap" + fi + fi +fi diff --git a/system_files/shared/usr/share/ublue-os/bling/bling.sh b/system_files/shared/usr/share/ublue-os/bling/bling.sh index d868b889d..85c7ef17f 100755 --- a/system_files/shared/usr/share/ublue-os/bling/bling.sh +++ b/system_files/shared/usr/share/ublue-os/bling/bling.sh @@ -67,3 +67,13 @@ if command -v mise >/dev/null 2>&1; then fi fi fi + +# Keep bash-preexec's DEBUG trap alive. Must stay last so the re-arm hook is the +# final PROMPT_COMMAND entry, after every hook above has queued its own. +# See: https://github.com/projectbluefin/common/issues/869 +if [ "${BLING_SHELL}" = "bash" ]; then + BLING_REARM="${BLING_DIR:-/usr/share/ublue-os/bling}/bash-preexec-rearm.sh" + # shellcheck source=/dev/null + [ -f "${BLING_REARM}" ] && . "${BLING_REARM}" + unset BLING_REARM +fi diff --git a/tests/test_bling_preexec_rearm.bats b/tests/test_bling_preexec_rearm.bats new file mode 100644 index 000000000..b97eaf661 --- /dev/null +++ b/tests/test_bling_preexec_rearm.bats @@ -0,0 +1,324 @@ +#!/usr/bin/env bats +# Tests for system_files/shared/usr/share/ublue-os/bling/bash-preexec-rearm.sh +# +# Regression coverage for https://github.com/projectbluefin/common/issues/869: +# with Fedora's array-valued PROMPT_COMMAND, bash-preexec 0.6.0 fails to remove +# its deferred installer from PROMPT_COMMAND whenever another hook has pushed it +# out of element [0]. The leftover installer runs `trap - DEBUG` on every prompt +# while __bp_install returns early, so the DEBUG trap stays gone and atuin (and +# every other preexec consumer) silently stops recording. +# +# These tests use a minimal stand-in for bash-preexec 0.6.0 that mirrors the +# upstream install semantics verbatim (install string, __bp_install early return, +# and the fact that __bp_install only sanitizes "${PROMPT_COMMAND}" — element +# [0]). "bash-preexec bug reproduces without the re-arm hook" pins that stand-in +# to the real failure, so the other tests cannot pass vacuously. +# +# Prompt cycles are simulated by eval-ing PROMPT_COMMAND entries at *top level*. +# That matters: bash does not inherit the DEBUG trap into shell functions, so +# running the cycle inside a helper function would hide the clobber entirely. +# +# Run: bats tests/test_bling_preexec_rearm.bats + +BLING_LIB="$BATS_TEST_DIRNAME/../system_files/shared/usr/share/ublue-os/bling" +BASH_BIN="$(command -v bash)" +WORKDIR="" +MOCKDIR="" +BASEBIN="" + +setup() { + WORKDIR="$BATS_TEST_DIRNAME/.tmp/test_bling_preexec_rearm_${BATS_TEST_NUMBER}_$$" + MOCKDIR="$WORKDIR/mockbin" + BASEBIN="$WORKDIR/basebin" + + mkdir -p "$MOCKDIR" "$BASEBIN" "$WORKDIR/home" "$WORKDIR/brew/etc/profile.d" + ln -s "$(command -v basename)" "$BASEBIN/basename" + ln -s "$(command -v readlink)" "$BASEBIN/readlink" + + write_bash_preexec_stub "$WORKDIR/brew/etc/profile.d/bash-preexec.sh" +} + +teardown() { + rm -rf "$WORKDIR" +} + +# Minimal stand-in for bash-preexec 0.6.0. Only the parts that decide whether the +# DEBUG trap survives are reproduced; they are copied from upstream 0.6.0. +write_bash_preexec_stub() { + cat > "$1" <<'STUB' +__bp_install_string=$'__bp_trap_string="$(trap -p DEBUG)"\ntrap - DEBUG\n__bp_install' +preexec_functions=() +precmd_functions=() +__bp_preexec_invoke_exec() { + local cmd="$BASH_COMMAND" f + [[ -n "${__bp_preexec_interactive_mode:-}" ]] || return 0 + __bp_preexec_interactive_mode="" + for f in "${preexec_functions[@]}"; do [[ -n "$f" ]] && "$f" "$cmd"; done + return 0 +} +__bp_interactive_mode() { __bp_preexec_interactive_mode=1; } +__bp_precmd_invoke_cmd() { + local f + for f in "${precmd_functions[@]}"; do [[ -n "$f" ]] && "$f"; done + return 0 +} +__bp_install() { + [[ "${PROMPT_COMMAND[*]:-}" == *"__bp_precmd_invoke_cmd"* ]] && return 1 + trap '__bp_preexec_invoke_exec "$_"' DEBUG + unset __bp_trap_string + local existing="${PROMPT_COMMAND:-}" + existing="${existing//$__bp_install_string/:}" + [[ "${existing:-:}" == ":" ]] && existing= + PROMPT_COMMAND='__bp_precmd_invoke_cmd' + PROMPT_COMMAND+=${existing:+$'\n'$existing} + PROMPT_COMMAND+=('__bp_interactive_mode') + __bp_precmd_invoke_cmd + __bp_interactive_mode +} +__bp_install_after_session_init() { + local s="${PROMPT_COMMAND:-}" + [[ -n "$s" ]] && PROMPT_COMMAND="$s"$'\n' + PROMPT_COMMAND+=${__bp_install_string} +} +__bp_install_after_session_init +STUB +} + +# Run a snippet in a clean bash with only the mocked PATH visible. +run_shell() { + printf '%s\n' "$1" > "$WORKDIR/script.bash" + run env -i \ + PATH="$MOCKDIR:$BASEBIN" \ + HOME="$WORKDIR/home" \ + HOMEBREW_PREFIX="$WORKDIR/brew" \ + BLING_DIR="$BLING_LIB" \ + "$BASH_BIN" --noprofile --norc "$WORKDIR/script.bash" +} + +# Prelude: array-valued PROMPT_COMMAND plus a hook queued ahead of bash-preexec's +# installer, which is what pushes the installer out of element [0] on Fedora. +ARRAY_SETUP=' +__vte_precmd() { :; } +_pre_hook() { :; } +PROMPT_COMMAND=("__vte_precmd") +source "${BLING_DIR}/bling.sh" +PROMPT_COMMAND=("_pre_hook" "${PROMPT_COMMAND[@]}") +' + +# Simulated prompt cycle. MUST stay at top level — see header note on functrace. +CYCLE='for __e in "${PROMPT_COMMAND[@]}"; do eval "$__e"; done' + +# --------------------------------------------------------------------------- +# The upstream defect itself — pins the stand-in to real behaviour +# --------------------------------------------------------------------------- + +@test "bash-preexec bug reproduces without the re-arm hook" { + run_shell ' +__vte_precmd() { :; } +_pre_hook() { :; } +PROMPT_COMMAND=("__vte_precmd") +source "${HOMEBREW_PREFIX}/etc/profile.d/bash-preexec.sh" +PROMPT_COMMAND=("_pre_hook" "${PROMPT_COMMAND[@]}") +'"$CYCLE"' +echo "one:[$(trap -p DEBUG)]" +'"$CYCLE"' +echo "two:[$(trap -p DEBUG)]" +' + [ "$status" -eq 0 ] + [[ "$output" == *"one:[trap -- '__bp_preexec_invoke_exec \"\$_\"' DEBUG]"* ]] + [[ "$output" == *"two:[]"* ]] +} + +# --------------------------------------------------------------------------- +# Array-valued PROMPT_COMMAND — the reported failure mode +# --------------------------------------------------------------------------- + +@test "array PROMPT_COMMAND: DEBUG trap survives repeated prompts" { + run_shell "$ARRAY_SETUP$CYCLE"' +echo "one:[$(trap -p DEBUG)]" +'"$CYCLE"' +echo "two:[$(trap -p DEBUG)]" +'"$CYCLE"' +echo "three:[$(trap -p DEBUG)]" +' + [ "$status" -eq 0 ] + [[ "$output" == *"one:[trap -- '__bp_preexec_invoke_exec \"\$_\"' DEBUG]"* ]] + [[ "$output" == *"two:[trap -- '__bp_preexec_invoke_exec \"\$_\"' DEBUG]"* ]] + [[ "$output" == *"three:[trap -- '__bp_preexec_invoke_exec \"\$_\"' DEBUG]"* ]] +} + +@test "array PROMPT_COMMAND: atuin-style preexec hook keeps firing" { + run_shell "$ARRAY_SETUP"' +_atuin_count=0 +_atuin_preexec() { _atuin_count=$((_atuin_count + 1)); } +preexec_functions+=(_atuin_preexec) +'"$CYCLE"' +'"$CYCLE"' +'"$CYCLE"' +echo "count=$_atuin_count" +' + [ "$status" -eq 0 ] + [[ "$output" == *"count=3"* ]] +} + +@test "array PROMPT_COMMAND: preexec stops firing without the re-arm hook" { + run_shell ' +__vte_precmd() { :; } +_pre_hook() { :; } +PROMPT_COMMAND=("__vte_precmd") +source "${HOMEBREW_PREFIX}/etc/profile.d/bash-preexec.sh" +PROMPT_COMMAND=("_pre_hook" "${PROMPT_COMMAND[@]}") +_atuin_count=0 +_atuin_preexec() { _atuin_count=$((_atuin_count + 1)); } +preexec_functions+=(_atuin_preexec) +'"$CYCLE"' +'"$CYCLE"' +'"$CYCLE"' +echo "count=$_atuin_count" +' + [ "$status" -eq 0 ] + [[ "$output" == *"count=1"* ]] +} + +@test "array PROMPT_COMMAND: re-arm hook is queued after the stale installer" { + run_shell "$ARRAY_SETUP"' +installer=-1; rearm=-1 +for i in "${!PROMPT_COMMAND[@]}"; do + [[ "${PROMPT_COMMAND[i]}" == *"trap - DEBUG"* ]] && installer=$i + [[ "${PROMPT_COMMAND[i]}" == *"__bling_rearm_bp_debug_trap"* ]] && rearm=$i +done +echo "installer=$installer rearm=$rearm" +' + [ "$status" -eq 0 ] + [[ "$output" =~ installer=([0-9]+)\ rearm=([0-9]+) ]] + [ "${BASH_REMATCH[2]}" -gt "${BASH_REMATCH[1]}" ] +} + +# --------------------------------------------------------------------------- +# Scalar PROMPT_COMMAND must not regress +# --------------------------------------------------------------------------- + +@test "scalar PROMPT_COMMAND: DEBUG trap survives repeated prompts" { + run_shell ' +_legacy_hook() { :; } +PROMPT_COMMAND="_legacy_hook" +source "${BLING_DIR}/bling.sh" +'"$CYCLE"' +echo "one:[$(trap -p DEBUG)]" +'"$CYCLE"' +echo "two:[$(trap -p DEBUG)]" +echo "kept:${PROMPT_COMMAND[*]}" +' + [ "$status" -eq 0 ] + [[ "$output" == *"one:[trap -- '__bp_preexec_invoke_exec \"\$_\"' DEBUG]"* ]] + [[ "$output" == *"two:[trap -- '__bp_preexec_invoke_exec \"\$_\"' DEBUG]"* ]] + [[ "$output" == *"kept:"*"_legacy_hook"* ]] +} + +# --------------------------------------------------------------------------- +# Idempotency +# --------------------------------------------------------------------------- + +@test "sourcing bling.sh twice queues the re-arm hook only once" { + run_shell "$ARRAY_SETUP"' +source "${BLING_DIR}/bling.sh" +count=0 +for e in "${PROMPT_COMMAND[@]}"; do + [[ "$e" == *"__bling_rearm_bp_debug_trap"* ]] && count=$((count + 1)) +done +echo "count=$count" +' + [ "$status" -eq 0 ] + [[ "$output" == *"count=1"* ]] +} + +@test "sourcing the re-arm helper twice queues the hook only once" { + run_shell ' +source "${HOMEBREW_PREFIX}/etc/profile.d/bash-preexec.sh" +source "${BLING_DIR}/bash-preexec-rearm.sh" +source "${BLING_DIR}/bash-preexec-rearm.sh" +count=0 +for e in "${PROMPT_COMMAND[@]}"; do + [[ "$e" == *"__bling_rearm_bp_debug_trap"* ]] && count=$((count + 1)) +done +echo "count=$count" +' + [ "$status" -eq 0 ] + [[ "$output" == *"count=1"* ]] +} + +# --------------------------------------------------------------------------- +# Graceful degradation +# --------------------------------------------------------------------------- + +@test "no bash-preexec installed: bling.sh sources cleanly and adds no hook" { + rm -f "$WORKDIR/brew/etc/profile.d/bash-preexec.sh" + run_shell ' +set -u +PROMPT_COMMAND=("__noop"); __noop() { :; } +source "${BLING_DIR}/bling.sh" +echo "pc:${PROMPT_COMMAND[*]}" +echo "done" +' + [ "$status" -eq 0 ] + [[ "$output" != *"__bling_rearm_bp_debug_trap"* ]] + [[ "$output" == *"done"* ]] +} + +@test "no bash-preexec installed: re-arm helper is a no-op with PROMPT_COMMAND unset" { + run_shell ' +set -u +unset PROMPT_COMMAND +source "${BLING_DIR}/bash-preexec-rearm.sh" +echo "declared:$(declare -p PROMPT_COMMAND 2>/dev/null)" +echo "done" +' + [ "$status" -eq 0 ] + [[ "$output" == *"declared:"* ]] + [[ "$output" != *"__bling_rearm_bp_debug_trap"* ]] + [[ "$output" == *"done"* ]] +} + +@test "re-arm helper does not error when PROMPT_COMMAND is readonly" { + run_shell ' +source "${HOMEBREW_PREFIX}/etc/profile.d/bash-preexec.sh" +readonly PROMPT_COMMAND +source "${BLING_DIR}/bash-preexec-rearm.sh" 2>"$HOME/err.log" +echo "stderr:[$(cat "$HOME/err.log")]" +echo "done" +' + [ "$status" -eq 0 ] + [[ "$output" == *"stderr:[]"* ]] + [[ "$output" == *"done"* ]] +} + +@test "zsh can still source bling.sh without the bash-only helper" { + command -v zsh >/dev/null 2>&1 || skip "zsh not installed" + run env -i \ + PATH="$MOCKDIR:$BASEBIN:$(dirname "$(command -v zsh)")" \ + HOME="$WORKDIR/home" \ + BLING_DIR="$BLING_LIB" \ + zsh -c 'source "${BLING_DIR}/bling.sh"; echo done' + [ "$status" -eq 0 ] + [[ "$output" == *"done"* ]] +} + +@test "helper defines no function when bash-preexec is absent" { + rm -f "$WORKDIR/brew/etc/profile.d/bash-preexec.sh" + run_shell ' +source "${BLING_DIR}/bash-preexec-rearm.sh" +echo "type:$(type -t __bling_rearm_bp_debug_trap)" +' + [ "$status" -eq 0 ] + [[ "$output" == "type:" ]] +} + +@test "bling.sh tolerates a missing re-arm helper" { + run_shell ' +BLING_DIR="$HOME/nowhere" +source "'"$BLING_LIB"'/bling.sh" +echo "done" +' + [ "$status" -eq 0 ] + [[ "$output" == *"done"* ]] +}