Skip to content

upgrade overwrites and then deletes the installed binary when the download fails #1407

Closed

Description

@Chemaclass

Problem

bashunit upgrade downloads the new release straight onto the installed binary. On a failed download it removes that file, so the user ends with no bashunit at all. A download that stops halfway leaves a truncated binary in place until the next step removes it.

Reproduced offline by stubbing the tag lookup to return a version that has no release:

mkdir -p lib && printf '#!/bin/bash\necho old\n' > lib/bashunit && chmod +x lib/bashunit
BASHUNIT_INSTALL_DIR=$PWD/lib bashunit upgrade   # with get_latest_tag returning 9.9.9
ls lib

Output: Failed to download bashunit 9.9.9 from ... with curl: (56) The requested URL returned error: 404, exit 1, lib/ empty.

Where

  • src/cli/upgrade.sh:29: bashunit::io::download_to "$url" "$target" with target being the installed binary.
  • src/cli/upgrade.sh:37 and :44: rm -f "$target" on a failed or empty download.
  • src/system/io.sh:37-47: download_to writes to the output path directly.

Why it matters

The same shape as the install.sh finding, but triggered by the command users run to stay current. GitHub returning a 5xx for a minute is enough to lose the binary from a project's lib/.

Suggested change

Download to a temporary file next to the target, check it is non-empty, chmod u+x, then mv over the target. Never remove the target on failure.

Acceptance criteria

  • A failed or empty download leaves the installed binary untouched and exits 1 with the current message.
  • A successful upgrade replaces it in one rename.
  • A unit test with a stubbed bashunit::io::download_to covers the failure and fails on the current code.

Proposed fix

In bashunit::upgrade::upgrade, src/cli/upgrade.sh:25-51: download to a temporary file next to the target and move it into place only once it is non-empty and executable. Never remove $target.

local tmp="$target.download.$$"
bashunit::io::download_to "$url" "$tmp" 2>"$err_file" || download_status=$?
if [ "$download_status" -ne 0 ] || [ ! -s "$tmp" ]; then
  echo "Failed to download bashunit $latest_tag from $url" >&2
  # keep the Reason block as today
  rm -f "$err_file" "$tmp"
  return 1
fi
rm -f "$err_file"
chmod u+x "$tmp" || { echo "Failed to make $tmp executable" >&2; rm -f "$tmp"; return 1; }
mv -f "$tmp" "$target"

Bash 3.0: $$, [ -s ], mv -f only. Public behaviour that must not change: the three messages the unit tests assert (Failed to download bashunit <tag> from <url>, the indented Reason: block, upgraded successfully to latest version <tag>), exit 1 on failure, You are already on latest version when up to date. No docs change; add a CHANGELOG line under Unreleased ("upgrade keeps the installed binary when the download fails").

Agent brief

  • Branch: fix/1407-upgrade-keeps-installed-binary from fresh origin/main. Commit: fix(cli): keep the installed binary when upgrade cannot download with Closes #1407 in the body.
  • Files: src/cli/upgrade.sh, CHANGELOG.md. Test: extend tests/unit/cli/upgrade_test.sh, next to test_upgrade_aborts_when_download_fails (upgrade_test.sh:44), reusing fake_get_latest_tag and fake_download_fail.
  • RED first:
function test_upgrade_keeps_the_installed_binary_when_download_fails() {
  printf '#!/bin/bash\necho previous\n' >"$WORK_DIR/bashunit"
  chmod +x "$WORK_DIR/bashunit"
  bashunit::mock bashunit::helper::get_latest_tag fake_get_latest_tag
  bashunit::mock bashunit::io::download_to fake_download_fail

  local exit_code=0
  bashunit::upgrade::upgrade >/dev/null 2>&1 || exit_code=$?

  assert_equals "1" "$exit_code"
  assert_same "previous" "$("$WORK_DIR/bashunit")"
}

Run only that file: ./bashunit tests/unit/cli/upgrade_test.sh. The existing assert_file_not_exists "$WORK_DIR/bashunit" in the neighbour stays true because that test starts with no binary.

  • Non-goals: do not change bashunit::io::download_to (src/system/io.sh:37), the tag lookup, or how the install directory is resolved; install.sh is install.sh removes the installed binary before downloading, so a failed download leaves nothing #1406.
  • Gate, once, when the change is complete: ./bashunit tests/, ./bashunit --parallel --simple --strict tests/, make sa, make lint. make lint is the formatting authority; never shfmt -w; 2-space indent.
  • Done when: every acceptance box is checked, the gate is green, CHANGELOG is updated, and a PR is open with the pr skill.

Activity

  1. added
    p1Data loss, a failure hidden behind exit 0, or a vacuous pass
    area:installCode under src or scripts for install
    area:cliCode under src or scripts for cli
    agent-readyScoped, reproduced and testable; an agent can take it without more questions
    on Oct 28, 2026
  2. self-assigned this
    on Oct 29, 2026
  3. added a commit that references this issue on Oct 30, 2026
    5f5ed30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

agent-readyScoped, reproduced and testable; an agent can take it without more questionsarea:cliCode under src or scripts for cliarea:installCode under src or scripts for installbugSomething isn't workinggood first issueGood for newcomersp1Data loss, a failure hidden behind exit 0, or a vacuous pass

Type

No type

Projects

  • Status
    Done

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions