From 092bdeae29532c3560df00161a073e014fca7a80 Mon Sep 17 00:00:00 2001 From: flan Date: Mon, 13 Jul 2026 16:30:50 +0000 Subject: [PATCH] v0.4.1: fix three real bugs in update.sh found by auditing it Release-candidate tags would have been installed as stable ----------------------------------------------------------- git's version sort ranks v0.5.0-rc1 ABOVE v0.5.0 (verified empirically), and the release workflow deliberately supports rc/beta/alpha tags. update.sh would have offered an RC as "the newest release". Tag selection is now filtered to plain vX.Y.Z. update.sh would have died mid-update on an untracked file ---------------------------------------------------------- The dirty-tree guard uses --untracked-files=no, so an untracked file that the TARGET tracks slips past it -- and `git checkout` then aborts. Under set -e the script died with a raw git error, after already recording the rollback point. Not hypothetical: a hand-copied patch/wait_restart.sh blocked a pull on a real box in exactly this way. It is now detected up front, by name. Gitignored files are correctly not treated as blockers, since git overwrites those silently. Special case: if update.sh ITSELF is the blocker, it was hand-copied in to bootstrap -- and "delete update.sh, then re-run update.sh" is impossible. It now says so and prints the git commands that bootstrap it properly. --rollback skipped that check entirely and would have hit the identical failure. The check is now a shared function used by both paths, and rollback also validates that the recorded revision still exists. Also: install.sh's chmod aborted under set -e if a listed file was missing (the file set changes between versions, so --rollback must not be killed by a name this version happens to know about), and --to with no value was silently ignored. Verified end to end in a throwaway clone: forward v0.4.1 -> v0.4.2 and rollback back, with files appearing and disappearing correctly; both guards fire. 132 tests, ruff and shellcheck -S style clean. --- CHANGELOG.md | 32 ++++++++++++++++++ install.sh | 12 +++++-- patch/apply.sh | 2 +- patch/create_task.py | 2 +- recover.sh | 2 +- uninstall.sh | 2 +- update.sh | 79 +++++++++++++++++++++++++++++++++++++++++--- 7 files changed, 120 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b36fe41..066a6e3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,37 @@ # Changelog +## v0.4.1 — 2026-07-13 + +### Fixed + +- **`update.sh` would have picked a release candidate as "the newest release".** + Git's version sort ranks `v0.5.0-rc1` *above* `v0.5.0` (verified), and the + release workflow deliberately supports rc/beta tags — so an RC would have been + installed as though it were the latest stable. Tag selection is now filtered to + plain `vX.Y.Z`. + +- **`update.sh` would have died mid-update on an untracked file.** The dirty-tree + guard uses `--untracked-files=no`, so an untracked file that the *target* tracks + slipped past it — and `git checkout` then aborts. Under `set -e` the script died + with a raw git error, *after* recording the rollback point. This is exactly what + blocked a pull on a real box (a hand-copied `patch/wait_restart.sh`). It now + detects the collision up front and names the files. Gitignored files are + correctly *not* treated as blockers — git overwrites those silently. + + Special case: if `update.sh` *itself* is the blocker, you hand-copied it in to + bootstrap — and "delete `update.sh`, then re-run `update.sh`" is impossible. It + now says so and prints the git commands that bootstrap it properly. + +- **`--rollback` skipped that check entirely**, so it would have hit the identical + failure. The check is now a shared function used by both paths, and rollback also + validates that the recorded revision still exists (history can be rewritten). + +- `install.sh`'s `chmod` aborted under `set -e` if any listed file was missing. The + file set changes between versions, so `update.sh --rollback` to an older revision + must not be killed by a filename this version happens to know about. + +- `--to` with no value was silently ignored and fell back to the default target. + ## v0.4.0 — 2026-07-13 ### Added diff --git a/install.sh b/install.sh index 55da8e1..36b66ef 100755 --- a/install.sh +++ b/install.sh @@ -18,7 +18,7 @@ set -euo pipefail -VERSION="0.4.0" +VERSION="0.4.1" # The directory containing install.sh is the permanent install location. PATCH_DIR="$(cd "$(dirname "$0")" && pwd)" @@ -95,8 +95,14 @@ fi # ── Set permissions ─────────────────────────────────────────────────────────── echo "Setting permissions ..." -chmod +x "$PATCH_DIR/patch/apply.sh" "$PATCH_DIR/patch/create_task.py" \ - "$PATCH_DIR/recover.sh" "$PATCH_DIR/uninstall.sh" "$PATCH_DIR/update.sh" +# Guard each path: under `set -e` a chmod on a missing file aborts the install. +# The file set changes between versions, so `update.sh --rollback` to an older +# revision must not be killed by a name this version happens to know about. +for _exe in patch/apply.sh patch/create_task.py recover.sh uninstall.sh update.sh; do + if [ -f "$PATCH_DIR/$_exe" ]; then + chmod +x "$PATCH_DIR/$_exe" + fi +done echo "Done." echo "" diff --git a/patch/apply.sh b/patch/apply.sh index bb0135b..27ce09e 100755 --- a/patch/apply.sh +++ b/patch/apply.sh @@ -32,7 +32,7 @@ # Derive PATCH_DIR from this script's location (parent of the patch/ directory). PATCH_DIR="$(cd "$(dirname "$0")/.." && pwd)" LOG="$PATCH_DIR/apply.log" -VERSION="0.4.0" +VERSION="0.4.1" # Rotate log at 512 KB to avoid unbounded growth on a system volume. # Keep two prior generations (.1 and .2) so the last three boots are always available. diff --git a/patch/create_task.py b/patch/create_task.py index 6962391..c499d3c 100755 --- a/patch/create_task.py +++ b/patch/create_task.py @@ -52,7 +52,7 @@ import subprocess import sys import time -__version__ = "0.4.0" +__version__ = "0.4.1" _PATCH_DIR = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) _STATUS_FILE = os.path.join(_PATCH_DIR, "hook_status.json") diff --git a/recover.sh b/recover.sh index 7e92a23..838088e 100755 --- a/recover.sh +++ b/recover.sh @@ -17,7 +17,7 @@ # bash /mnt/tank/truenas-truecloud-patch/patch/apply.sh # systemctl restart middlewared -VERSION="0.4.0" +VERSION="0.4.1" PATCH_DIR="$(cd "$(dirname "$0")" && pwd)" diff --git a/uninstall.sh b/uninstall.sh index f52ad9e..52e0984 100755 --- a/uninstall.sh +++ b/uninstall.sh @@ -3,7 +3,7 @@ set -euo pipefail -VERSION="0.4.0" +VERSION="0.4.1" PATCH_DIR="$(cd "$(dirname "$0")" && pwd)" _HOOK_COMMENT='TrueCloud provider patch (S3/B2)' diff --git a/update.sh b/update.sh index 47e5f3d..ded0926 100644 --- a/update.sh +++ b/update.sh @@ -19,7 +19,7 @@ set -euo pipefail -VERSION="0.4.0" +VERSION="0.4.1" PATCH_DIR="$(cd "$(dirname "$0")" && pwd)" _PREV_FILE="$PATCH_DIR/.update_previous" @@ -46,9 +46,59 @@ Updating preserves your nested-snapshot opt-in setting either way. USAGE } +# An UNTRACKED file that the target tracks makes `git checkout` abort. The dirty- +# tree check deliberately ignores untracked files, so this slips past it and the +# checkout then dies mid-operation. Not hypothetical: a hand-copied +# patch/wait_restart.sh blocked a pull on a real box exactly this way. +# +# Used by BOTH the update and the rollback path -- rolling back moves the tree too, +# and would hit the identical failure. +_abort_if_untracked_blockers() { + local ref="$1" blocking + + # Set intersection of {untracked, not ignored} and {tracked by the target}. Two + # git calls, not one `ls-files --error-unmatch` per file in the target tree. + # --exclude-standard is deliberate: git silently overwrites *ignored* files on + # checkout, so those are not blockers — only untracked-and-not-ignored ones are. + blocking="$(comm -12 \ + <(git ls-files --others --exclude-standard | sort) \ + <(git ls-tree -r --name-only "$ref" | sort) \ + | sed 's/^/ /')" + + [ -n "$blocking" ] || return 0 + + echo "ERROR: these untracked files would be overwritten:" >&2 + printf '%s\n\n' "$blocking" >&2 + echo " They exist here but git does not track them — most likely hand-copied" >&2 + echo " or scp'd in. Move or delete them, then re-run." >&2 + + # "Delete update.sh, then re-run update.sh" is impossible. If the script itself + # is a blocker, it was hand-copied in to bootstrap; the honest answer is to + # bootstrap with git instead, which installs it properly. + case "$blocking" in + *update.sh*) + echo "" >&2 + echo " update.sh itself is untracked here — you copied it in to bootstrap." >&2 + echo " Do that with git instead, once; it installs update.sh properly:" >&2 + echo "" >&2 + echo " rm -f $PATCH_DIR/update.sh" >&2 + echo " git -C $PATCH_DIR checkout $ref" >&2 + echo " bash $PATCH_DIR/install.sh" >&2 + echo "" >&2 + echo " Every later update is then just: bash update.sh" >&2 + ;; + esac + exit 1 +} + while [ $# -gt 0 ]; do case "$1" in - --to) _target="${2:-}"; shift ;; + --to) + if [ -z "${2:-}" ]; then + echo "ERROR: --to needs a tag, branch, or commit." >&2 + exit 1 + fi + _target="$2"; shift ;; --main) _use_main=1 ;; --check) _check_only=1 ;; --rollback) _rollback=1 ;; @@ -103,8 +153,15 @@ if [ "$_rollback" -eq 1 ]; then exit 1 fi _prev="$(cat "$_PREV_FILE")" + if ! git rev-parse --verify --quiet "${_prev}^{commit}" >/dev/null; then + echo "ERROR: recorded revision '$_prev' is not a valid commit." >&2 + echo " The history may have been rewritten. Pick a target explicitly:" >&2 + echo " bash update.sh --to " >&2 + exit 1 + fi + _abort_if_untracked_blockers "$_prev" echo "Rolling back to $_prev ..." - git checkout -q "$_prev" + git checkout -q --detach "$_prev" echo "Reverted. Re-applying ..." echo "" bash "$PATCH_DIR/install.sh" @@ -128,7 +185,13 @@ else # correct while tags are created in ascending version order; it breaks the # moment a hotfix is tagged out of band (a v0.3.6 released after v0.4.0 would # sort as "newest" by date and silently downgrade the box). - _target="$(git tag -l 'v*' --sort=-version:refname | head -1)" + # + # Filter to PLAIN vX.Y.Z: git's version sort ranks `v0.5.0-rc1` ABOVE `v0.5.0` + # (verified), so without this a release candidate would be installed as though + # it were the newest release. The release workflow deliberately supports + # rc/beta/alpha tags, so they will exist. + _target="$(git tag -l 'v*' --sort=-version:refname \ + | grep -E '^v[0-9]+\.[0-9]+\.[0-9]+$' | head -1)" if [ -z "$_target" ]; then echo "ERROR: no release tags found; use --main to track unreleased code." >&2 exit 1 @@ -149,6 +212,8 @@ if [ "$_current" = "$_target_sha" ]; then exit 0 fi +_abort_if_untracked_blockers "$_target_sha" + # ── Show what is coming ─────────────────────────────────────────────────────── echo "Commits you do not have yet:" @@ -217,6 +282,12 @@ echo "" echo "=== Update complete ===" echo " $_current_desc -> $(git describe --tags --always)" echo "" +if ! git symbolic-ref -q HEAD >/dev/null; then + echo "NOTE: the checkout is now pinned to a release tag (detached HEAD), which is" + echo " what you want for a deployment. Plain \`git pull\` will not work here —" + echo " use \`bash update.sh\` from now on." + echo "" +fi echo "If anything looks wrong:" echo " bash $PATCH_DIR/update.sh --rollback # back to $_current_desc" echo " bash $PATCH_DIR/recover.sh # kill switch + restart"