From 520b9541cc2791e9e2c3e27e64ecc665bc87ce1c Mon Sep 17 00:00:00 2001
From: Chemaclass <chemaclass@outlook.es>
Date: Mon, 20 Jul 2026 01:53:55 +0200
Subject: [PATCH] fix(install): fail fast and guard the beta build path

install.sh ran without set -e and with SC2164 suppressed, so a failed
`git clone` in the beta path cascaded: the cd failed, ./build.sh ran in
the caller's directory, the copy failed, and the script still printed
the success message with exit 0.

- run under set -euo pipefail; downloads keep their explicit file-check
  error messages via `|| true` guards
- guard the clone explicitly so build.sh can never execute outside the
  fresh temp clone; clean up and exit 1 with a clear error
- replace GNU-only `sed -i -e` (BSD sed reads `-e` as a backup suffix
  and leaves a stray `bashunit-e` file) with a portable tmp+mv edit,
  restoring the exec bit the rewrite would otherwise drop
- acceptance regression test: a failing clone exits non-zero, prints no
  success message, and creates nothing in the caller's directory

Closes #840

Claude-Session: https://claude.ai/code/session_01JSeB8UzLCpqst55hAK6dED
---
 CHANGELOG.md                     |  1 +
 install.sh                       | 39 +++++++++++++++++++++++---------
 tests/acceptance/install_test.sh | 20 ++++++++++++++++
 3 files changed, 49 insertions(+), 11 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 5fb61f9c..0c7ab166 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -3,6 +3,7 @@
 ## Unreleased
 
 ### Fixed
+- `install.sh` fails fast (`set -euo pipefail`): a failed download, clone, build or copy now aborts with a clear error instead of reaching the success message — previously a failed beta clone cascaded into running `build.sh` in the caller's directory. Also drops a GNU-only `sed -i -e` that left a stray backup file on BSD/macOS (#840)
 - A failing `set_up_before_script`/`set_up` is now reported consistently: previously the same failure could be attributed (plain failing command), silently ignored (failing `cmd && var=x` guard on Bash 3.2 — the hook's real exit status was discarded), or silently counted with an off-by-one total and no message (Bash ≥ 4, where the ERR trap re-fired in the runner's scope). Every test in the affected file is now marked failed with the hook message, totals match the declared count, and the suite continues — a strict test file can no longer abort the whole run mid-suite via its top-level `set -euo pipefail` (#836)
 - `bashunit::env::is_diff_enabled` and `bashunit::cleanup_testcase_temp_files` no longer break callers under `set -u`/`set -e` (unbound read after `unset BASHUNIT_NO_DIFF`; skip path returned 1) (#836)
 - No run-mode flag leaks into nested bashunit runs via the environment anymore: after #834 fixed five flags, the remaining ~40 (`--parallel`, `--simple`, `--strict`, `--filter`, `--retry`, `--seed`, `--watch`, …) are now also this-process-only, so a script under test that itself calls bashunit gets default behavior instead of silently inheriting the parent's output mode, strictness, parallelism, timeouts or report paths. Env-var configuration (`BASHUNIT_*=… bashunit`) is unchanged (#834, #837)
diff --git a/install.sh b/install.sh
index 4a57a872..4730246c 100755
--- a/install.sh
+++ b/install.sh
@@ -1,6 +1,7 @@
 #!/usr/bin/env bash
-# shellcheck disable=SC2155
-# shellcheck disable=SC2164
+# Fail fast: no step after a failed download/clone/build/copy may run, and the
+# success message must be unreachable on any failure path (#840).
+set -euo pipefail
 
 # Helper function for regex matching (Bash 3.0+ compatible)
 function regex_match() {
@@ -49,11 +50,13 @@ function verify_checksum() {
     return
   fi
 
+  # `|| true`: a failed download must reach the cannot_verify fallback below,
+  # not die on set -e / pipefail.
   local expected
   if command -v curl >/dev/null 2>&1; then
-    expected=$(curl -fsSL --retry 3 --retry-delay 2 "$checksum_url" 2>/dev/null | awk '{print $1}')
+    expected=$(curl -fsSL --retry 3 --retry-delay 2 "$checksum_url" 2>/dev/null | awk '{print $1}') || true
   else
-    expected=$(wget -qO- "$checksum_url" 2>/dev/null | awk '{print $1}')
+    expected=$(wget -qO- "$checksum_url" 2>/dev/null | awk '{print $1}') || true
   fi
 
   if [ -z "$expected" ]; then
@@ -80,20 +83,32 @@ function build_and_install_beta() {
     exit 1
   fi
 
-  git clone --depth 1 --no-tags "$BASHUNIT_GIT_REPO" temp_bashunit 2>/dev/null
+  # Explicit guard even under set -e: a failed clone must never cascade into
+  # `cd` failing and build.sh executing in the caller's directory (#840).
+  if ! git clone --depth 1 --no-tags "$BASHUNIT_GIT_REPO" temp_bashunit 2>/dev/null; then
+    echo "Error: failed to clone $BASHUNIT_GIT_REPO" >&2
+    rm -rf temp_bashunit
+    exit 1
+  fi
   cd temp_bashunit
   ./build.sh bin >/dev/null
-  local latest_commit=$(git rev-parse --short=7 HEAD)
-  # shellcheck disable=SC2103
+  local latest_commit
+  latest_commit=$(git rev-parse --short=7 HEAD)
   cd ..
 
-  local beta_version=$(printf "(non-stable) beta after %s [%s] 🐍 #%s" \
+  local beta_version
+  beta_version=$(printf "(non-stable) beta after %s [%s] 🐍 #%s" \
     "$LATEST_BASHUNIT_VERSION" \
     "$(date +'%Y-%m-%d')" \
     "$latest_commit")
 
-  sed -i -e 's/BASHUNIT_VERSION=".*"/BASHUNIT_VERSION="'"$beta_version"'"/g' temp_bashunit/bin/bashunit
+  # Portable in-place edit: BSD sed treats the argument after -i as a backup
+  # suffix, so `sed -i -e` leaves a stray `bashunit-e` file behind.
+  sed -e 's/BASHUNIT_VERSION=".*"/BASHUNIT_VERSION="'"$beta_version"'"/g' \
+    temp_bashunit/bin/bashunit >temp_bashunit/bin/bashunit.tmp
+  mv temp_bashunit/bin/bashunit.tmp temp_bashunit/bin/bashunit
   cp temp_bashunit/bin/bashunit ./
+  chmod u+x ./bashunit
   rm -rf temp_bashunit
 }
 
@@ -105,11 +120,13 @@ function install() {
     echo "> Downloading the latest version: '$TAG'"
   fi
 
+  # `|| true`: a failed download must reach the explicit file check below with
+  # its clear error message, not die silently on set -e.
   local url="$BASHUNIT_GIT_REPO/releases/download/$TAG/bashunit"
   if command -v curl >/dev/null 2>&1; then
-    curl -fL --retry 3 --retry-delay 2 -O -J "$url" 2>/dev/null
+    curl -fL --retry 3 --retry-delay 2 -O -J "$url" 2>/dev/null || true
   elif command -v wget >/dev/null 2>&1; then
-    wget --tries=3 "$url" 2>/dev/null || wget "$url" 2>/dev/null
+    wget --tries=3 "$url" 2>/dev/null || wget "$url" 2>/dev/null || true
   else
     echo "Cannot download bashunit: curl or wget not found." >&2
     exit 1
diff --git a/tests/acceptance/install_test.sh b/tests/acceptance/install_test.sh
index c9bdcb78..b6c36c0b 100644
--- a/tests/acceptance/install_test.sh
+++ b/tests/acceptance/install_test.sh
@@ -284,3 +284,23 @@ function test_install_downloads_the_non_stable_beta_version() {
   assert_same "$file_count_of_tmp_deps_directory" "1"
   assert_same "$(find ./tmp_deps -name 'bashunit')" "./tmp_deps/bashunit"
 }
+
+# Regression guard for #840: a failed `git clone` in the beta path must abort
+# with a non-zero exit — it used to cascade (failed cd, build.sh executed in
+# the caller's directory, missing copy) and still print the success message.
+function test_install_beta_aborts_when_clone_fails() {
+  local shim_dir
+  shim_dir="$(bashunit::temp_dir)"
+  printf '#!/usr/bin/env bash\nexit 128\n' >"$shim_dir/git"
+  chmod +x "$shim_dir/git"
+
+  local output
+  local exit_code=0
+  output="$(PATH="$shim_dir:$PATH" ./install.sh tmp_install beta 2>&1)" || exit_code=$?
+
+  assert_not_equals 0 "$exit_code"
+  assert_not_contains "has been installed" "$output"
+  assert_file_not_exists "./tmp_install/bashunit"
+  assert_directory_not_exists "./tmp_install/bin"
+  assert_directory_not_exists "./tmp_install/temp_bashunit"
+}