From 03a9ff353721ba8fb5f37cad2546d25788011451 Mon Sep 17 00:00:00 2001
From: Craig Jennings <c@cjennings.net>
Date: Thu, 25 Jun 2026 01:07:31 -0400
Subject: fix: harden rsyncshot destination, rotation, and mount handling

Refuse to run when REMOTE_PATH or MOUNTDIR is empty or non-absolute. A blank config value otherwise resolves the destination to the filesystem root, where rotation runs rm -rf and --delete.

Run the rotation cp/mv/rm as bare command names in remote mode, resolved by the remote PATH, instead of hardcoded /usr/bin paths that broke on remotes whose binaries live elsewhere. Resolve the real binary via command -v in local mode.

Pass rsync -R so each source is stored under its full path. Two includes sharing a basename (/usr/local/bin and /usr/bin) previously both mapped to latest/bin, and the second sync's --delete wiped the first. This changes the stored layout: /usr/local/bin now lives at latest/usr/local/bin instead of latest/bin. /home and /etc are unchanged.

Add rsync --numeric-ids so /etc and /home ownership survives a restore when the destination has a different passwd/group database.

Match mount points exactly via is_mounted() instead of a substring grep of /proc/mounts, so /media/backup no longer matches /media/backup2, paths compare literally, and mount points with spaces work.

Reject a retention count below 1, which previously still created one snapshot. Drop the grep "|| echo 0" that emitted a stray second line, assemble ssh options as arrays, and quote the RSYNC_RSH identity path.

Extract derive_paths() and is_mounted() as sourceable functions and add unit, rsync-flag, and gated remote-mode test suites. The suite now runs 36 tests, and shellcheck is clean across the script and tests.
---
 tests/cases/test_validation.sh | 56 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 56 insertions(+)

(limited to 'tests/cases/test_validation.sh')

diff --git a/tests/cases/test_validation.sh b/tests/cases/test_validation.sh
index 03da676..9a89787 100755
--- a/tests/cases/test_validation.sh
+++ b/tests/cases/test_validation.sh
@@ -79,6 +79,59 @@ test_accepts_valid_snapshot_types() {
     assert_not_contains "$output" "must be alphabetic" "should accept valid type" || return 1
 }
 
+# ------------------------------------------------------------------------------
+# Test: Rejects a retention count of zero
+# ------------------------------------------------------------------------------
+test_rejects_zero_count() {
+    local output
+    output=$(sudo "$SCRIPT_PATH" "manual" "0" 2>&1)
+    local exit_code=$?
+
+    assert_exit_code 1 "$exit_code" "should reject count 0" || return 1
+    assert_contains "$output" "at least 1" "should explain minimum count" || return 1
+}
+
+# ------------------------------------------------------------------------------
+# Test: Refuses an empty MOUNTDIR (would resolve to the filesystem root)
+# ------------------------------------------------------------------------------
+test_rejects_empty_mountdir() {
+    setup_test_env
+    cat > "$TEST_CONFIG_DIR/config" <<EOF
+REMOTE_HOST=""
+MOUNTDIR=""
+EOF
+    create_test_includes >/dev/null
+    create_test_excludes >/dev/null
+
+    local output
+    output=$(sudo INSTALLHOME="$TEST_CONFIG_DIR" RSYNCSHOT_SKIP_MOUNT_CHECK=1 "$SCRIPT_PATH" manual 1 2>&1)
+    local exit_code=$?
+
+    teardown_test_env
+
+    assert_exit_code 1 "$exit_code" "empty MOUNTDIR should be rejected" || return 1
+    assert_contains "$output" "MOUNTDIR is empty" "should explain the empty-MOUNTDIR refusal" || return 1
+}
+
+# ------------------------------------------------------------------------------
+# Test: status produces no stderr noise (the grep "0\n0" integer-test bug)
+# ------------------------------------------------------------------------------
+test_status_no_stderr_noise() {
+    setup_test_env
+    create_test_config >/dev/null
+    create_test_includes >/dev/null
+    create_test_excludes >/dev/null
+
+    local stderr_file="$TEST_DIR/status.stderr"
+    sudo INSTALLHOME="$TEST_CONFIG_DIR" RSYNCSHOT_SKIP_MOUNT_CHECK=1 "$SCRIPT_PATH" status >/dev/null 2>"$stderr_file"
+    local err
+    err=$(cat "$stderr_file")
+
+    teardown_test_env
+
+    assert_no_stderr "$err" "status should emit no stderr (no 'integer expression' noise)" || return 1
+}
+
 # ------------------------------------------------------------------------------
 # Run tests
 # ------------------------------------------------------------------------------
@@ -95,6 +148,9 @@ run_validation_tests() {
     run_test "rejects alphabetic retention count" test_rejects_alpha_retention_count
     run_test "rejects mixed retention count" test_rejects_mixed_retention_count
     run_test "accepts valid snapshot types" test_accepts_valid_snapshot_types
+    run_test "rejects retention count of zero" test_rejects_zero_count
+    run_test "refuses empty MOUNTDIR" test_rejects_empty_mountdir
+    run_test "status emits no stderr noise" test_status_no_stderr_noise
 
     teardown_test_env
 }
-- 
cgit v1.2.3