From 8cdafe577eb2cfcc0bdf3787d489ae15cb89cbfe Mon Sep 17 00:00:00 2001 From: Kevin O'Donnell Date: Tue, 5 May 2026 10:04:15 -0400 Subject: [PATCH 1/8] fix(aliases): create ~/.local/bin before kubectl install (SUR-2330) Signed-off-by: Kevin O'Donnell --- bash/aliases | 1 + tests/aliases.bats | 50 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+) diff --git a/bash/aliases b/bash/aliases index 1d04d1f..efedf16 100644 --- a/bash/aliases +++ b/bash/aliases @@ -223,6 +223,7 @@ function update_kubectl { echo "update_kubectl: SHA-256 verification failed" >&2 return 1 fi + mkdir -p "$HOME/.local/bin" install -m 0755 "$tmpdir/kubectl" "$HOME/.local/bin/kubectl" } diff --git a/tests/aliases.bats b/tests/aliases.bats index 81da913..1e5f499 100644 --- a/tests/aliases.bats +++ b/tests/aliases.bats @@ -72,6 +72,56 @@ setup() { [[ "$output" =~ "install -m 0755" ]] } +# SUR-2330: update_kubectl must create ~/.local/bin if missing (peers +# update_eksctl/update_helm already do). Without this, install(1) hard-fails +# with "No such file or directory" on a fresh machine. +@test "update_kubectl creates ~/.local/bin before installing (SUR-2330)" { + run awk '/^function update_kubectl/,/^}$/' "$ALIASES" + [ "$status" -eq 0 ] + # Single quotes are intentional: the source contains the literal `$HOME`, + # not its expansion. + # shellcheck disable=SC2016 + [[ "$output" == *'mkdir -p "$HOME/.local/bin"'* ]] +} + +@test "update_kubectl succeeds when ~/.local/bin does not pre-exist (SUR-2330)" { + # Stub curl so the network/verify path resolves against a local fake + # "kubectl" payload, then run update_kubectl with HOME pointing at a + # tmpdir that lacks .local/bin and assert the binary is installed. + STUB_BIN=$(mktemp -d) + FAKE_KUBECTL="$STUB_BIN/kubectl-payload" + printf '#!/usr/bin/env bash\nexit 0\n' >"$FAKE_KUBECTL" + FAKE_HASH=$(sha256sum "$FAKE_KUBECTL" | awk '{print $1}') + + cat >"$STUB_BIN/curl" <"\$out" ;; + *kubectl) cp "$FAKE_KUBECTL" "\$out" ;; + *) exit 1 ;; +esac +EOF + chmod +x "$STUB_BIN/curl" + + # Confirm the isolated HOME does not already contain .local/bin. + [ ! -d "$HOME/.local/bin" ] + + run env "PATH=$STUB_BIN:$PATH" KUBECTL_VERSION=v1.32.0 \ + bash -c "source '$ALIASES'; update_kubectl" + [ "$status" -eq 0 ] + [ -x "$HOME/.local/bin/kubectl" ] + + rm -rf "$STUB_BIN" +} + # ---------- get_latest_btp_branch (SUR-1928) ---------------------------------- @test "get_latest_btp_branch declares loop vars local (SUR-1928)" { From 81b92a7d405b84b050a430941b2e293880d2fc4d Mon Sep 17 00:00:00 2001 From: Kevin O'Donnell Date: Tue, 5 May 2026 10:07:09 -0400 Subject: [PATCH 2/8] fix(commands): sanitise commands::use cache key for hyphenated names (SUR-2342) Signed-off-by: Kevin O'Donnell --- bash/commands.sh | 8 ++++- tests/commands.bats | 73 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 80 insertions(+), 1 deletion(-) diff --git a/bash/commands.sh b/bash/commands.sh index b4ba1ba..40865de 100644 --- a/bash/commands.sh +++ b/bash/commands.sh @@ -34,7 +34,13 @@ function commands::use { @doc Find the specified command on the PATH if available or error @arg _1_ the base command name to find local cmd=${1:?} - local var=_${cmd} + # Cache-key sanitisation: bash variable names cannot contain hyphens or + # dots, so commands like ssh-keygen or python3.12 must be remapped to + # _ssh_keygen / _python3_12 for the declare -g call. The lookup itself + # below keeps the original $cmd. Distinct names that collide after + # sanitisation (foo-bar vs foo_bar) would share a cache slot, but that + # is vanishingly unlikely on a real PATH. + local var=_${cmd//[^A-Za-z0-9_]/_} if [ -z "${!var}" ]; then local cmd_path cmd_path=$(command -v "$cmd") || commands::err_not_found "$cmd" diff --git a/tests/commands.bats b/tests/commands.bats index 999cfe6..f355117 100644 --- a/tests/commands.bats +++ b/tests/commands.bats @@ -32,3 +32,76 @@ setup() { " [ "$status" -ne 0 ] } + +# SUR-2342: bash variable names reject hyphens and dots, so the cache-key +# variable name must be sanitised. Pre-fix, `commands::use ssh-keygen` +# crashed with `declare: '_ssh-keygen=...': not a valid identifier`. +@test "commands::use resolves a hyphenated command name (SUR-2342)" { + STUB_BIN=$(mktemp -d) + cat >"$STUB_BIN/ssh-keygen" <<'EOF' +#!/usr/bin/env bash +exit 0 +EOF + chmod +x "$STUB_BIN/ssh-keygen" + out=$(mktemp) + run env "PATH=$STUB_BIN:$PATH" bash -c " + source '$REPO_ROOT/bash/includer.sh' + @include commands + commands::use ssh-keygen >'$out' + " + rm -rf "$STUB_BIN" + [ "$status" -eq 0 ] + resolved=$(cat "$out") + rm -f "$out" + [[ "$resolved" == */ssh-keygen ]] +} + +@test "commands::use resolves a dotted command name (SUR-2342)" { + STUB_BIN=$(mktemp -d) + cat >"$STUB_BIN/python3.12" <<'EOF' +#!/usr/bin/env bash +exit 0 +EOF + chmod +x "$STUB_BIN/python3.12" + out=$(mktemp) + run env "PATH=$STUB_BIN:$PATH" bash -c " + source '$REPO_ROOT/bash/includer.sh' + @include commands + commands::use python3.12 >'$out' + " + rm -rf "$STUB_BIN" + [ "$status" -eq 0 ] + resolved=$(cat "$out") + rm -f "$out" + [[ "$resolved" == */python3.12 ]] +} + +@test "commands::use caches under sanitised key and survives binary removal (SUR-2342)" { + # First call resolves and caches under _ssh_keygen. After the stub is + # deleted, a second call must still return the cached path rather than + # re-resolving. $() is a subshell so we redirect to a tempfile to keep + # the declare -g cache write in the same shell. + STUB_BIN=$(mktemp -d) + cat >"$STUB_BIN/ssh-keygen" <<'EOF' +#!/usr/bin/env bash +exit 0 +EOF + chmod +x "$STUB_BIN/ssh-keygen" + first="$BATS_TEST_TMPDIR/first" + second="$BATS_TEST_TMPDIR/second" + rm -f "$first" "$second" + run env "PATH=$STUB_BIN:$PATH" bash -c " + source '$REPO_ROOT/bash/includer.sh' + @include commands + commands::use ssh-keygen >'$first' + rm -f '$STUB_BIN/ssh-keygen' + commands::use ssh-keygen >'$second' + [ \"\$_ssh_keygen\" = \"\$(cat '$first')\" ] || { echo 'cache key mismatch' >&2; exit 1; } + " + rm -rf "$STUB_BIN" + [ "$status" -eq 0 ] + p1=$(cat "$first") + p2=$(cat "$second") + [ "$p1" = "$p2" ] + [[ "$p1" == */ssh-keygen ]] +} From 0d71feef8dd8e5798fbdc0a019b94cdec8836be9 Mon Sep 17 00:00:00 2001 From: Kevin O'Donnell Date: Tue, 5 May 2026 10:10:12 -0400 Subject: [PATCH 3/8] fix(log): initialise LOG_DISABLE_* per-flag at source time (SUR-2347) Signed-off-by: Kevin O'Donnell --- bash/log.sh | 29 ++++++++++++++++++++++------- tests/log.bats | 41 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+), 7 deletions(-) diff --git a/bash/log.sh b/bash/log.sh index bc8450c..29171a3 100644 --- a/bash/log.sh +++ b/bash/log.sh @@ -87,13 +87,28 @@ function log::level() { } # Initialise the LOG_DISABLE_* flags from LOG_LEVEL at source time so -# scripts that don't pass -v still get deterministic gating. Skip if a -# caller has already pre-set any of the disable flags directly (so -# explicit pre-set values survive sourcing). -if [ -z "${LOG_DISABLE_INFO+set}" ] && [ -z "${LOG_DISABLE_DEBUG+set}" ] && - [ -z "${LOG_DISABLE_WARNING+set}" ] && [ -z "${LOG_DISABLE_TRACE+set}" ]; then - log::level "$LOG_LEVEL" -fi +# scripts that don't pass -v still get deterministic gating. Each flag is +# initialised independently: SUR-2347 — pre-fix, if a caller pre-set any +# single flag (e.g. LOG_DISABLE_INFO=false) the entire block was skipped, +# leaving the other three unset (empty), and `[ "$LOG_DISABLE_TRACE" = +# "false" ]` evaluated false, silently disabling output. Snapshot the +# caller's explicit pre-sets, let log::level recompute defaults from +# LOG_LEVEL, then restore the snapshots so explicit values win. +__log_preset_trace=${LOG_DISABLE_TRACE+x} +__log_preset_debug=${LOG_DISABLE_DEBUG+x} +__log_preset_info=${LOG_DISABLE_INFO+x} +__log_preset_warning=${LOG_DISABLE_WARNING+x} +__log_value_trace=${LOG_DISABLE_TRACE-} +__log_value_debug=${LOG_DISABLE_DEBUG-} +__log_value_info=${LOG_DISABLE_INFO-} +__log_value_warning=${LOG_DISABLE_WARNING-} +log::level "$LOG_LEVEL" +[ "$__log_preset_trace" = x ] && LOG_DISABLE_TRACE=$__log_value_trace +[ "$__log_preset_debug" = x ] && LOG_DISABLE_DEBUG=$__log_value_debug +[ "$__log_preset_info" = x ] && LOG_DISABLE_INFO=$__log_value_info +[ "$__log_preset_warning" = x ] && LOG_DISABLE_WARNING=$__log_value_warning +unset __log_preset_trace __log_preset_debug __log_preset_info __log_preset_warning +unset __log_value_trace __log_value_debug __log_value_info __log_value_warning function log::level_increase() { @doc Increase the LOG_LEVEL diff --git a/tests/log.bats b/tests/log.bats index 842d8b8..e90c003 100644 --- a/tests/log.bats +++ b/tests/log.bats @@ -115,3 +115,44 @@ probe_level() { " 2>&1) [[ "$out" == *"hit-info from-shim"* ]] } + +# SUR-2347: source-time guard must initialise each LOG_DISABLE_* flag +# independently. Pre-fix: pre-setting only one flag short-circuited the +# `if [ -z ... ] && [ -z ... ] && ...` guard, leaving the other three +# unset (empty), so `[ "$LOG_DISABLE_X" = "false" ]` evaluated false and +# silently disabled those levels. +@test "log.sh initialises every LOG_DISABLE_* flag independently when caller pre-sets one (SUR-2347)" { + out=$(bash -c " + LOG_DISABLE_INFO=false + LOG_LEVEL=4 + source '$REPO_ROOT/bash/includer.sh' + @include log + echo \"TRACE=\$LOG_DISABLE_TRACE DEBUG=\$LOG_DISABLE_DEBUG INFO=\$LOG_DISABLE_INFO WARNING=\$LOG_DISABLE_WARNING\" + ") + [[ "$out" == "TRACE=false DEBUG=false INFO=false WARNING=false" ]] +} + +# Caller pre-set values must survive log::level's recompute (i.e. the +# caller's explicit pre-set wins over the LOG_LEVEL default). +@test "log.sh preserves caller pre-set LOG_DISABLE_* values (SUR-2347)" { + out=$(bash -c " + LOG_DISABLE_INFO=true + LOG_LEVEL=4 + source '$REPO_ROOT/bash/includer.sh' + @include log + echo \"INFO=\$LOG_DISABLE_INFO\" + ") + [[ "$out" == "INFO=true" ]] +} + +# When no flag is pre-set, behaviour matches the existing baseline: +# log::level "$LOG_LEVEL" populates all four. +@test "log.sh source-time init matches log::level when nothing pre-set (SUR-2347)" { + out=$(bash -c " + LOG_LEVEL=2 + source '$REPO_ROOT/bash/includer.sh' + @include log + echo \"TRACE=\$LOG_DISABLE_TRACE DEBUG=\$LOG_DISABLE_DEBUG INFO=\$LOG_DISABLE_INFO WARNING=\$LOG_DISABLE_WARNING\" + ") + [[ "$out" == "TRACE=true DEBUG=true INFO=false WARNING=false" ]] +} From 40be38c162ed1892333a099ac2c17ab8856f8a6e Mon Sep 17 00:00:00 2001 From: Kevin O'Donnell Date: Tue, 5 May 2026 10:13:02 -0400 Subject: [PATCH 4/8] fix(log): substitute message last in log::_format to prevent placeholder injection (SUR-2331) Signed-off-by: Kevin O'Donnell --- bash/log.sh | 5 ++++- tests/log.bats | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/bash/log.sh b/bash/log.sh index 29171a3..738be4c 100644 --- a/bash/log.sh +++ b/bash/log.sh @@ -227,10 +227,13 @@ function log::_format() { local date date="$(date "$LOG_DATE_FORMAT")" local formatted_log="$LOG_FORMAT" - formatted_log="${formatted_log/'%MESSAGE'/$log}" + # SUR-2331: substitute %MESSAGE last so any literal %LEVEL/%PID/%DATE + # text inside the user-supplied message does not get re-interpreted by + # the subsequent substitutions. formatted_log="${formatted_log/'%LEVEL'/$level}" formatted_log="${formatted_log/'%PID'/$pid}" formatted_log="${formatted_log/'%DATE'/$date}" + formatted_log="${formatted_log/'%MESSAGE'/$log}" printf '%s\n' "$formatted_log" } # Deprecated public name retained as a parens-only shim so the bare identifier diff --git a/tests/log.bats b/tests/log.bats index e90c003..92e779a 100644 --- a/tests/log.bats +++ b/tests/log.bats @@ -156,3 +156,39 @@ probe_level() { ") [[ "$out" == "TRACE=true DEBUG=true INFO=false WARNING=false" ]] } + +# SUR-2331: log::_format substitutes %MESSAGE last so a literal +# placeholder inside the user-supplied message survives without being +# re-interpreted by the subsequent %LEVEL/%PID/%DATE substitutions. +@test "log::_format does not re-interpret %LEVEL inside the message (SUR-2331)" { + out=$(bash -c " + source '$REPO_ROOT/bash/includer.sh' + @include log + LOG_FORMAT='%LEVEL %MESSAGE' + log::_format INFO 'literal %LEVEL' + ") + # The level is INFO and the message contains the literal token %LEVEL. + # Pre-fix the output was 'INFO INFO literal INFO' — message substituted + # first, then the %LEVEL inside the message got re-substituted. + [[ "$out" == "INFO literal %LEVEL" ]] +} + +@test "log::_format does not re-interpret %PID inside the message (SUR-2331)" { + out=$(bash -c " + source '$REPO_ROOT/bash/includer.sh' + @include log + LOG_FORMAT='%LEVEL %MESSAGE' + log::_format INFO 'pid=%PID' + ") + [[ "$out" == "INFO pid=%PID" ]] +} + +@test "log::_format does not re-interpret %DATE inside the message (SUR-2331)" { + out=$(bash -c " + source '$REPO_ROOT/bash/includer.sh' + @include log + LOG_FORMAT='%LEVEL %MESSAGE' + log::_format INFO 'date=%DATE' + ") + [[ "$out" == "INFO date=%DATE" ]] +} From 983cb818d557649d6f2a9e4439bdf8bc1365bba6 Mon Sep 17 00:00:00 2001 From: Kevin O'Donnell Date: Tue, 5 May 2026 10:15:51 -0400 Subject: [PATCH 5/8] fix(changelog): default missing -f/-t endpoint to repo root / HEAD (SUR-2327) Signed-off-by: Kevin O'Donnell --- bash/changelog | 13 +++++++++---- tests/changelog.bats | 24 ++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 4 deletions(-) diff --git a/bash/changelog b/bash/changelog index e9fb561..d960aa1 100755 --- a/bash/changelog +++ b/bash/changelog @@ -136,12 +136,17 @@ if [ -n "$FROM_DATE" ]; then fi if [ -n "$FROM_DATE" ] || [ -n "$TO_DATE" ]; then + # SUR-2327: default each missing endpoint so -f and -t can be used + # independently. Pre-fix, -f alone hit the "No change history" branch + # (to_commit empty) and -t alone tripped ::fromto's `${1:?}` because + # from_commit was empty. if [ -z "$to_commit" ]; then - echo "No change history in the requested range" - exit 0 - else - ::fromto "$from_commit" "$to_commit" + to_commit=HEAD + fi + if [ -z "$from_commit" ]; then + from_commit=$(git::cmd rev-list --max-parents=0 HEAD | tail -n 1) fi + ::fromto "$from_commit" "$to_commit" else ::full fi diff --git a/tests/changelog.bats b/tests/changelog.bats index bdad9c7..b97cdb4 100644 --- a/tests/changelog.bats +++ b/tests/changelog.bats @@ -88,6 +88,30 @@ teardown() { [ "$trailing" -eq 1 ] } +# SUR-2327: -f and -t are documented as independent. Pre-fix, -f alone +# printed "No change history in the requested range" and -t alone hit +# ::fromto's `${1:?}` because from_commit was empty. +@test "changelog -f alone produces a non-empty range up to HEAD (SUR-2327)" { + run bash -c "cd '$FIXTURE_DIR' && '$CHANGELOG' -f 1970-01-01" + [ "$status" -eq 0 ] + [[ "$output" == *"feat: post-tag change"* ]] + [[ "$output" != *"No change history in the requested range"* ]] +} + +@test "changelog -t alone produces a non-empty range from repo root (SUR-2327)" { + run bash -c "cd '$FIXTURE_DIR' && '$CHANGELOG' -t 2999-01-01" + [ "$status" -eq 0 ] + [[ "$output" == *"feat: post-tag change"* ]] + [[ "$output" != *"No change history in the requested range"* ]] +} + +@test "changelog -f and -t together produce a bounded range (SUR-2327)" { + run bash -c "cd '$FIXTURE_DIR' && '$CHANGELOG' -f 1970-01-01 -t 2999-01-01" + [ "$status" -eq 0 ] + [[ "$output" == *"feat: post-tag change"* ]] + [[ "$output" != *"No change history in the requested range"* ]] +} + # SUR-1939: count::fromto invocations to guarantee we don't run twice # per tag iteration. We instrument by sourcing `bash/changelog`'s # ::full + ::fromto definitions in a sub-shell that wraps ::fromto in From f9ab3b324d706ae617701d0e695b6ce855bacb64 Mon Sep 17 00:00:00 2001 From: Kevin O'Donnell Date: Tue, 5 May 2026 10:18:52 -0400 Subject: [PATCH 6/8] fix(pretty-labels): pass --no-headers to kubectl get (SUR-2332) Signed-off-by: Kevin O'Donnell --- bash/pretty-labels | 2 +- tests/pretty-labels.bats | 26 +++++++++++++++++++++++++- 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/bash/pretty-labels b/bash/pretty-labels index 4ea3c18..1369c8f 100755 --- a/bash/pretty-labels +++ b/bash/pretty-labels @@ -27,7 +27,7 @@ options::add -o t -d "resource type to examine" -a -m -e RESOURCE_TYPE options::parse "$@" shift $((OPTIND - 1)) -for line in $(k8s::ctl get "$RESOURCE_TYPE" --show-labels "$@" | awk '{print $1":"$NF}'); do +for line in $(k8s::ctl get "$RESOURCE_TYPE" --show-labels --no-headers "$@" | awk '{print $1":"$NF}'); do resource_name=$(echo "$line" | awk -F: '{print $1}') labels=$(echo "$line" | awk -F: '{print $NF}') col1="$resource_name" diff --git a/tests/pretty-labels.bats b/tests/pretty-labels.bats index 7370800..b014c5e 100644 --- a/tests/pretty-labels.bats +++ b/tests/pretty-labels.bats @@ -6,9 +6,18 @@ setup() { load 'helpers.bash' helpers::isolate_home STUB_BIN=$(mktemp -d) + # Stub kubectl mimics real kubectl: emits a header row unless + # --no-headers is in the args. Lets us assert pretty-labels passes + # --no-headers correctly (SUR-2332). cat >"$STUB_BIN/kubectl" <<'STUB' #!/usr/bin/env bash -printf "NAME READY STATUS RESTARTS AGE LABELS\n" +no_headers=false +for a in "$@"; do + [[ "$a" == "--no-headers" ]] && no_headers=true +done +if ! $no_headers; then + printf "NAME READY STATUS RESTARTS AGE LABELS\n" +fi printf "mypod-1 1/1 Running 0 1d app=myapp,tier=frontend\n" STUB chmod +x "$STUB_BIN/kubectl" @@ -41,3 +50,18 @@ teardown() { [[ "$output" == *"app=myapp"* ]] [[ "$output" == *"tier=frontend"* ]] } + +# SUR-2332: pre-fix, pretty-labels did not pass --no-headers to kubectl, +# so the kubectl header row "NAME ... LABELS" survived the awk extraction +# as the junk row "NAME:LABELS" and rendered one stray pair per +# invocation. +@test "pretty-labels does not emit the kubectl header row (SUR-2332)" { + run bash -c " + LOGFILE_DISABLE=true + unset _kubectl + '$REPO_ROOT/bash/pretty-labels' -t pod + " + [ "$status" -eq 0 ] + [[ "$output" != *"LABELS"* ]] + [[ "$output" != *"NAME"* ]] +} From f76fedc9fb9e41670696bef1b933f02faf172d08 Mon Sep 17 00:00:00 2001 From: Kevin O'Donnell Date: Tue, 5 May 2026 10:29:05 -0400 Subject: [PATCH 7/8] fix(secret): chain caller's existing EXIT trap before secret::clear (SUR-2324) Signed-off-by: Kevin O'Donnell --- bash/secret.sh | 41 ++++++++++++++++++++++++++++-- tests/secret.bats | 63 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 102 insertions(+), 2 deletions(-) diff --git a/bash/secret.sh b/bash/secret.sh index cc891d8..e0a621a 100644 --- a/bash/secret.sh +++ b/bash/secret.sh @@ -28,13 +28,50 @@ declare -g -A SECRETS declare -g -A SECRETS_FILES declare -g -a SECRET_TMPFILES declare -g SECRET_TRAP_INSTALLED=${SECRET_TRAP_INSTALLED:-false} +declare -g SECRET_PREV_EXIT_TRAP="" +declare -g SECRET_PREV_INT_TRAP="" +declare -g SECRET_PREV_TERM_TRAP="" + +function secret::_run_chained_trap { + @doc Internal - run secret::clear and afterwards run the caller \ + pre-existing trap captured at install time for the given signal. + @arg _1_ signal name EXIT INT or TERM + local sig=${1:?} + secret::clear + local prev= + case "$sig" in + EXIT) prev=$SECRET_PREV_EXIT_TRAP ;; + INT) prev=$SECRET_PREV_INT_TRAP ;; + TERM) prev=$SECRET_PREV_TERM_TRAP ;; + *) return ;; + esac + if [ -n "$prev" ]; then + # trap -p SIG returns the literal text: + # trap -- SIG + # Strip the wrapper, then unquote the CMD via eval-set-positional-args + # so embedded single quotes (escaped by bash as the standard + # close/escape/reopen sequence) round-trip cleanly. + local cmd_quoted=${prev#trap -- } + cmd_quoted=${cmd_quoted% "$sig"} + eval "set -- $cmd_quoted" + eval "$1" + fi +} function secret::_install_cleanup_trap { @doc Internal - install the EXIT/INT/TERM trap that calls secret::clear \ exactly once per shell. Must be invoked from the parent shell scope so \ - the trap is in the right place. The registration helpers call this. + the trap is in the right place. The registration helpers call this. \ + SUR-2324: captures any pre-existing caller trap on EXIT/INT/TERM so the \ + chained handler runs the caller pre-existing trap after secret::clear \ + instead of silently overwriting it. if [ "$SECRET_TRAP_INSTALLED" != "true" ]; then - trap 'secret::clear' EXIT INT TERM + SECRET_PREV_EXIT_TRAP=$(trap -p EXIT) + SECRET_PREV_INT_TRAP=$(trap -p INT) + SECRET_PREV_TERM_TRAP=$(trap -p TERM) + trap 'secret::_run_chained_trap EXIT' EXIT + trap 'secret::_run_chained_trap INT' INT + trap 'secret::_run_chained_trap TERM' TERM SECRET_TRAP_INSTALLED=true fi } diff --git a/tests/secret.bats b/tests/secret.bats index d64a4bf..61e9bce 100644 --- a/tests/secret.bats +++ b/tests/secret.bats @@ -97,3 +97,66 @@ setup() { " [ "$status" -ne 0 ] } + +# SUR-2324: secret::_install_cleanup_trap must chain (not clobber) any +# caller-installed EXIT trap. Pre-fix, the unconditional +# `trap 'secret::clear' EXIT INT TERM` overwrote the caller's trap and +# the caller's cleanup never fired, leaking resources. +@test "secret::register_env preserves caller's pre-existing EXIT trap (SUR-2324)" { + SENTINEL="$BATS_TEST_TMPDIR/exit-sentinel" + TMPFILE_PROBE="$BATS_TEST_TMPDIR/tmpfile-probe" + rm -f "$SENTINEL" "$TMPFILE_PROBE" + # secret::as_file must run in the outer shell (not via $()) so the + # tempfile path lands in SECRET_TMPFILES of the same shell whose EXIT + # trap will later run secret::clear. We redirect stdout to a probe file + # so the test can read the path after the subshell exits. + bash -c " + trap 'touch \"$SENTINEL\"' EXIT + MY_SECRET=hunter2 + source '$REPO_ROOT/bash/includer.sh' + @include secret + secret::register_env MY_SECRET + secret::as_file MY_SECRET 2>/dev/null >'$TMPFILE_PROBE' + " + # Caller's EXIT trap fired (sentinel created). + [ -f "$SENTINEL" ] + # secret::clear also ran (the tempfile is gone). + tf=$(cat "$TMPFILE_PROBE") + [ -n "$tf" ] + [ ! -e "$tf" ] +} + +# SUR-2324: caller-trap commands containing single quotes are escaped by +# `trap -p` as '\''; the chained-trap eval must unescape them correctly. +@test "secret::_install_cleanup_trap survives a caller trap with embedded single quotes (SUR-2324)" { + SENTINEL="$BATS_TEST_TMPDIR/quoted-sentinel" + rm -f "$SENTINEL" + bash -c " + trap 'echo '\\''quoted-trap-fired'\\'' >\"$SENTINEL\"' EXIT + MY_SECRET=hunter2 + source '$REPO_ROOT/bash/includer.sh' + @include secret + secret::register_env MY_SECRET + " + [ -f "$SENTINEL" ] + out=$(cat "$SENTINEL") + [ "$out" = "quoted-trap-fired" ] +} + +# SUR-2324: idempotency — installing twice must not stack chained calls. +@test "secret::_install_cleanup_trap is idempotent within a shell (SUR-2324)" { + SENTINEL="$BATS_TEST_TMPDIR/idempotent-sentinel" + rm -f "$SENTINEL" + bash -c " + trap 'printf x >>\"$SENTINEL\"' EXIT + MY_SECRET=hunter2 + OTHER_SECRET=passw0rd + source '$REPO_ROOT/bash/includer.sh' + @include secret + secret::register_env MY_SECRET + secret::register_env OTHER_SECRET + " + # Caller's trap should fire exactly once on shell exit. + out=$(cat "$SENTINEL") + [ "$out" = "x" ] +} From 3ece59226d2c963ab9a57c67dd272e7a6cd71ea6 Mon Sep 17 00:00:00 2001 From: Kevin O'Donnell Date: Tue, 5 May 2026 11:58:37 -0400 Subject: [PATCH 8/8] fix(changelog): bail when -f/-t resolves to empty commit (SUR-2327) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The original SUR-2327 fix defaulted any unresolved endpoint to HEAD or the repo root so -f and -t could be used independently. That conflated two cases: (a) the user did not pass the flag, and (b) the user passed the flag but the date matched no commit. Case (b) now silently expanded the range to the opposite of what was requested — e.g. -t 1900-01-01 dumped full history instead of reporting "no commits before that date". Distinguish the two before applying the default. If the user supplied the flag but the corresponding date_to_commit lookup returned empty, print "No commits before " / "No commits on or after " and exit 0. Otherwise apply the per-flag default as before. Updated the existing 2999-01-01 fixtures to 2050-01-01: git rev-list's approxidate parser silently misparses 2999, which masked the regression by making the broken-default path indistinguishable from the fixed one. Adds two regression tests covering -t with a pre-history date and -f with a post-history date; both must NOT print any commit message. Signed-off-by: Kevin O'Donnell --- bash/changelog | 12 ++++++++++++ tests/changelog.bats | 36 ++++++++++++++++++++++++++++++++++-- 2 files changed, 46 insertions(+), 2 deletions(-) diff --git a/bash/changelog b/bash/changelog index d960aa1..f4100bd 100755 --- a/bash/changelog +++ b/bash/changelog @@ -136,6 +136,18 @@ if [ -n "$FROM_DATE" ]; then fi if [ -n "$FROM_DATE" ] || [ -n "$TO_DATE" ]; then + # SUR-2327: distinguish "flag not passed" from "flag passed but date + # matched no commit". The latter must NOT silently fall through to a + # default endpoint — that would expand the range to the opposite of + # what the user asked for (e.g. -t 1900-01-01 dumping full history). + if [ -n "$TO_DATE" ] && [ -z "$to_commit" ]; then + echo "No commits before $TO_DATE" + exit 0 + fi + if [ -n "$FROM_DATE" ] && [ -z "$from_commit" ]; then + echo "No commits on or after $FROM_DATE" + exit 0 + fi # SUR-2327: default each missing endpoint so -f and -t can be used # independently. Pre-fix, -f alone hit the "No change history" branch # (to_commit empty) and -t alone tripped ::fromto's `${1:?}` because diff --git a/tests/changelog.bats b/tests/changelog.bats index b97cdb4..6aa942e 100644 --- a/tests/changelog.bats +++ b/tests/changelog.bats @@ -99,19 +99,51 @@ teardown() { } @test "changelog -t alone produces a non-empty range from repo root (SUR-2327)" { - run bash -c "cd '$FIXTURE_DIR' && '$CHANGELOG' -t 2999-01-01" + # 2050-01-01 is the future-but-parseable upper bound for git's approxidate + # parser. Pre-2026 we used 2999-01-01, but git rev-list's --before/--after + # silently misparse 2999 and return wrong sets, masking the SUR-2327 + # follow-up bug ("flag passed but no match" must NOT silently expand). + run bash -c "cd '$FIXTURE_DIR' && '$CHANGELOG' -t 2050-01-01" [ "$status" -eq 0 ] [[ "$output" == *"feat: post-tag change"* ]] [[ "$output" != *"No change history in the requested range"* ]] } @test "changelog -f and -t together produce a bounded range (SUR-2327)" { - run bash -c "cd '$FIXTURE_DIR' && '$CHANGELOG' -f 1970-01-01 -t 2999-01-01" + run bash -c "cd '$FIXTURE_DIR' && '$CHANGELOG' -f 1970-01-01 -t 2050-01-01" [ "$status" -eq 0 ] [[ "$output" == *"feat: post-tag change"* ]] [[ "$output" != *"No change history in the requested range"* ]] } +# SUR-2327 follow-up: the SUR-2327 default-endpoint fix must not silently +# expand the range when the user passes -t with a date older than every +# commit. Pre-fix this branch defaulted to_commit=HEAD and dumped the +# full history — the opposite of what the user asked for. +@test "changelog -t reports no commits, not full history (SUR-2327)" { + run bash -c "cd '$FIXTURE_DIR' && '$CHANGELOG' -t 1900-01-01" + [ "$status" -eq 0 ] + [[ "$output" != *"feat: initial"* ]] + [[ "$output" != *"feat: second"* ]] + [[ "$output" != *"feat: third"* ]] + [[ "$output" != *"feat: post-tag change"* ]] + [[ "$output" == *"No commits before 1900-01-01"* ]] +} + +# SUR-2327 follow-up: same disambiguation for -f with a date after HEAD. +# Pre-fix this branch defaulted from_commit to the repo root and dumped +# the full history. 2050-01-01 chosen instead of 2999-01-01 because git +# rev-list's date parser silently misparses 2999 and would mask the bug. +@test "changelog -f reports no commits, not full history (SUR-2327)" { + run bash -c "cd '$FIXTURE_DIR' && '$CHANGELOG' -f 2050-01-01" + [ "$status" -eq 0 ] + [[ "$output" != *"feat: initial"* ]] + [[ "$output" != *"feat: second"* ]] + [[ "$output" != *"feat: third"* ]] + [[ "$output" != *"feat: post-tag change"* ]] + [[ "$output" == *"No commits on or after 2050-01-01"* ]] +} + # SUR-1939: count::fromto invocations to guarantee we don't run twice # per tag iteration. We instrument by sourcing `bash/changelog`'s # ::full + ::fromto definitions in a sub-shell that wraps ::fromto in