Skip to content

feat: add self-update command with passive version checking#12

Merged
codesoda merged 1 commit intomainfrom
add-update-command
Apr 6, 2026
Merged

feat: add self-update command with passive version checking#12
codesoda merged 1 commit intomainfrom
add-update-command

Conversation

@codesoda
Copy link
Copy Markdown
Owner

@codesoda codesoda commented Apr 4, 2026

Summary

  • Adds bugatti update command for manual self-update (download, SHA256 verify, extract, replace)
  • Adds bugatti update --check for version-only checking
  • Passive background version check after successful bugatti test runs (rate-limited to 8h, TTY-only, opt-out via BUGATTI_NO_UPDATE_CHECK=1)
  • Uses the GitHub releases redirect trick (single HTTP request, no API token, no rate limits)
  • macOS binary security: quarantine removal + ad-hoc codesign after update
  • Updates install.sh with secure_binary() function and redirect-based tag fetch

Test plan

  • cargo test — all 200 tests pass (12 new in update module)
  • cargo clippy — clean
  • cargo fmt --check — clean
  • bugatti update --check — correctly reports "up to date" against live GitHub
  • bugatti update — end-to-end test against a real release (publish a test release first)
  • bugatti update -y — non-interactive update
  • Verify passive check fires after bugatti test when interval has elapsed
  • Verify BUGATTI_NO_UPDATE_CHECK=1 suppresses passive check
  • Verify install.sh secure_binary and redirect trick work on fresh install

Add `bugatti update` for manual updates and `bugatti update --check`
for version checking. After successful test runs, a passive background
check notifies users of available updates (rate-limited to 8h, TTY-only,
opt-out via BUGATTI_NO_UPDATE_CHECK=1).

Uses the GitHub releases redirect trick to avoid API rate limits.
Downloads are verified via SHA256 checksums. On macOS, removes
quarantine attributes and applies ad-hoc code signing after update.

Also updates install.sh with secure_binary() and the redirect trick.
@codesoda codesoda marked this pull request as ready for review April 5, 2026 05:52
@codesoda
Copy link
Copy Markdown
Owner Author

codesoda commented Apr 6, 2026

Code review

Found 5 issues:

  1. spawn_passive_check() blocks the main thread for up to 3 seconds -- not actually a background check. The function spawns a thread then immediately calls handle.join(), which blocks the calling thread until the spawned thread finishes. Every successful bugatti test run will be delayed by up to 3 seconds (the PASSIVE_CHECK_TIMEOUT). The function name, doc comment, and commit message all promise non-blocking "background" behavior, but the implementation is synchronous. Either drop the join() call (let the thread be abandoned on process exit) or remove the thread entirely if blocking is intentional.

bugatti-cli/src/update.rs

Lines 566 to 570 in a4e435c

/// the thread is abandoned (it dies when the process exits).
pub fn spawn_passive_check() {
let handle = std::thread::spawn(passive_version_check);
let _ = handle.join(); // join will wait; the HTTP timeout (3s) is the real bound
}

  1. install.sh curl redirect technique will silently fail -- %{redirect_url} is empty when curl follows the redirect. The command curl -sSf -o /dev/null -w '%{redirect_url}' does not disable redirect following. By default, curl follows the 302 redirect to the final page, which makes %{redirect_url} empty (it only populates when curl stops at the redirect). The tag extraction then gets an empty string, triggering the die on line 178. The Rust code in update.rs handles this correctly by using redirect::Policy::none(). The shell equivalent needs --max-redirs 0 to stop at the 302, or should use -w '%{url_effective}' with -L to capture the final URL instead.

bugatti-cli/install.sh

Lines 175 to 179 in a4e435c

latest_url="https://github.com/$REPO_OWNER/$REPO_NAME/releases/latest"
tag="$(curl -sSf -o /dev/null -w '%{redirect_url}' "$latest_url" | grep -oE '[^/]+$')"
if [ -z "$tag" ]; then
die "Could not determine latest release — check https://github.com/$REPO_OWNER/$REPO_NAME/releases"
fi

  1. Update command errors return EXIT_CONFIG_ERROR (code 2), which is semantically wrong. EXIT_CONFIG_ERROR is defined as "Configuration, parse, cycle, or validation error before execution." Network failures, download errors, checksum mismatches, and permission errors during bugatti update are not configuration errors. This could mislead CI scripts that parse exit codes to determine failure categories. Consider adding a dedicated exit code for update failures, or using a more general error code.

bugatti-cli/src/main.rs

Lines 84 to 90 in a4e435c

Commands::Update { check, yes } => match bugatti::update::run_update(check, yes) {
Ok(()) => EXIT_OK,
Err(e) => {
eprintln!("ERROR: {e}");
EXIT_CONFIG_ERROR
}
},

  1. install.sh has no SHA256 checksum verification, unlike the bugatti update path. The self-update code in update.rs downloads checksums-sha256.txt and verifies the artifact before extraction. The initial install script downloads and extracts the binary with no integrity check. This creates an inconsistent security model where first-time installs (the most common attack surface via curl | sh) have no verification while self-updates do.

bugatti-cli/install.sh

Lines 186 to 198 in a4e435c

info "Downloading $asset_name..."
if ! curl -sSfL "$asset_url" -o "$TMP_DIR/$asset_name"; then
die "Failed to download $asset_url"
fi
ok "Downloaded"
tar xzf "$TMP_DIR/$asset_name" -C "$TMP_DIR"
downloaded_binary="$TMP_DIR/bugatti-${tag}-${target}/bugatti"
if [ ! -f "$downloaded_binary" ]; then
die "Archive does not contain expected binary"
fi
install_binary "$downloaded_binary"

  1. artifact_name fallback in unwrap_or("artifact.tar.gz") is unreachable -- empty URLs produce an empty string, not None. rsplit('/').next() on an empty string returns Some(""), not None, so the unwrap_or fallback never fires. If artifact_url were somehow empty or malformed, checksum lookup for "" would fail with a confusing "checksum entry not found" error rather than a clear "malformed artifact URL" message. Consider adding an explicit empty-string check.

bugatti-cli/src/update.rs

Lines 433 to 438 in a4e435c

let artifact_name = release
.artifact_url
.rsplit('/')
.next()
.unwrap_or("artifact.tar.gz");
let artifact_path = download_to_file(&release.artifact_url, tmp_dir.path(), artifact_name)

@codesoda
Copy link
Copy Markdown
Owner Author

codesoda commented Apr 6, 2026

All 5 issues addressed in bc5334a:

  1. spawn_passive_check() blocking — Removed handle.join(); thread is now detached. The 3-second HTTP timeout prevents it from hanging.

  2. install.sh curl redirect — Added --max-redirs 0 and removed -f so the 302 isn't treated as a failure and %{redirect_url} is populated.

  3. Wrong exit code — Added EXIT_UPDATE_ERROR = 7 and used it in the update error path instead of EXIT_CONFIG_ERROR. Updated docs, describe_exit_code(), and CLI help.

  4. No checksum in install.sh — Script now downloads checksums-sha256.txt, computes SHA256 (via sha256sum or shasum -a 256), and verifies before extracting.

  5. Unreachable unwrap_or fallback — Added .filter(|s| !s.is_empty()) with .ok_or_else() that returns a clear "malformed artifact URL" error.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant