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.

8 files changed, 422 insertions, 29 deletions

diff --git a/tests/cases/test_backup.sh b/tests/cases/test_backup.sh
index 1ab4fbd..af01221 100755
--- a/tests/cases/test_backup.sh
+++ b/tests/cases/test_backup.sh
@@ -15,9 +15,7 @@ test_creates_backup_structure() {
     create_test_includes
     create_test_excludes
 
-    local output
-    output=$(sudo INSTALLHOME="$TEST_CONFIG_DIR" RSYNCSHOT_SKIP_MOUNT_CHECK=1 "$SCRIPT_PATH" manual 1 2>&1)
-    local exit_code=$?
+    sudo INSTALLHOME="$TEST_CONFIG_DIR" RSYNCSHOT_SKIP_MOUNT_CHECK=1 "$SCRIPT_PATH" manual 1 >/dev/null 2>&1
 
     # Check directory structure
     assert_dir_exists "$TEST_BACKUP_DIR/$HOSTNAME" "should create hostname dir" || {
@@ -46,15 +44,14 @@ test_copies_files() {
     create_test_includes
     create_test_excludes
 
-    local output
-    output=$(sudo INSTALLHOME="$TEST_CONFIG_DIR" RSYNCSHOT_SKIP_MOUNT_CHECK=1 "$SCRIPT_PATH" manual 1 2>&1)
+    sudo INSTALLHOME="$TEST_CONFIG_DIR" RSYNCSHOT_SKIP_MOUNT_CHECK=1 "$SCRIPT_PATH" manual 1 >/dev/null 2>&1
 
     # Check files were copied
-    assert_file_exists "$TEST_BACKUP_DIR/$HOSTNAME/latest/home/testuser/file1.txt" "should copy file1.txt" || {
+    assert_file_exists "$TEST_BACKUP_DIR/$HOSTNAME/latest$TEST_SOURCE_DIR/home/testuser/file1.txt" "should copy file1.txt" || {
         teardown_test_env
         return 1
     }
-    assert_file_exists "$TEST_BACKUP_DIR/$HOSTNAME/latest/etc/test.conf" "should copy test.conf" || {
+    assert_file_exists "$TEST_BACKUP_DIR/$HOSTNAME/latest$TEST_SOURCE_DIR/etc/test.conf" "should copy test.conf" || {
         teardown_test_env
         return 1
     }
@@ -72,8 +69,7 @@ test_snapshot_readonly() {
     create_test_includes
     create_test_excludes
 
-    local output
-    output=$(sudo INSTALLHOME="$TEST_CONFIG_DIR" RSYNCSHOT_SKIP_MOUNT_CHECK=1 "$SCRIPT_PATH" manual 1 2>&1)
+    sudo INSTALLHOME="$TEST_CONFIG_DIR" RSYNCSHOT_SKIP_MOUNT_CHECK=1 "$SCRIPT_PATH" manual 1 >/dev/null 2>&1
 
     # Check snapshot directory has no write permission
     # Note: We use stat to check actual permissions because -w always returns true for root
@@ -100,8 +96,8 @@ test_rotation() {
     create_test_excludes
 
     # Run backup twice
-    sudo INSTALLHOME="$TEST_CONFIG_DIR" RSYNCSHOT_SKIP_MOUNT_CHECK=1 "$SCRIPT_PATH" manual 3 2>&1 >/dev/null
-    sudo INSTALLHOME="$TEST_CONFIG_DIR" RSYNCSHOT_SKIP_MOUNT_CHECK=1 "$SCRIPT_PATH" manual 3 2>&1 >/dev/null
+    sudo INSTALLHOME="$TEST_CONFIG_DIR" RSYNCSHOT_SKIP_MOUNT_CHECK=1 "$SCRIPT_PATH" manual 3 >/dev/null 2>&1
+    sudo INSTALLHOME="$TEST_CONFIG_DIR" RSYNCSHOT_SKIP_MOUNT_CHECK=1 "$SCRIPT_PATH" manual 3 >/dev/null 2>&1
 
     # Should have MANUAL.0 and MANUAL.1
     assert_dir_exists "$TEST_BACKUP_DIR/$HOSTNAME/MANUAL.0" "should have MANUAL.0" || {
@@ -127,8 +123,8 @@ test_retention_limit() {
     create_test_excludes
 
     # Run backup 4 times with retention of 3
-    for i in 1 2 3 4; do
-        sudo INSTALLHOME="$TEST_CONFIG_DIR" RSYNCSHOT_SKIP_MOUNT_CHECK=1 "$SCRIPT_PATH" manual 3 2>&1 >/dev/null
+    for _ in 1 2 3 4; do
+        sudo INSTALLHOME="$TEST_CONFIG_DIR" RSYNCSHOT_SKIP_MOUNT_CHECK=1 "$SCRIPT_PATH" manual 3 >/dev/null 2>&1
     done
 
     # Should have MANUAL.0, MANUAL.1, MANUAL.2 but NOT MANUAL.3
@@ -162,9 +158,7 @@ test_backup_command() {
     create_test_includes
     create_test_excludes
 
-    local output
-    output=$(sudo INSTALLHOME="$TEST_CONFIG_DIR" RSYNCSHOT_SKIP_MOUNT_CHECK=1 "$SCRIPT_PATH" backup 2>&1)
-    local exit_code=$?
+    sudo INSTALLHOME="$TEST_CONFIG_DIR" RSYNCSHOT_SKIP_MOUNT_CHECK=1 "$SCRIPT_PATH" backup >/dev/null 2>&1
 
     # Should create MANUAL.0 (backup is alias for manual 1)
     assert_dir_exists "$TEST_BACKUP_DIR/$HOSTNAME/MANUAL.0" "backup should create MANUAL.0" || {
@@ -194,20 +188,56 @@ EOF
     echo "temp" > "$TEST_SOURCE_DIR/home/testuser/temp.tmp"
     echo "log" > "$TEST_SOURCE_DIR/home/testuser/app.log"
 
-    local output
-    output=$(sudo INSTALLHOME="$TEST_CONFIG_DIR" RSYNCSHOT_SKIP_MOUNT_CHECK=1 "$SCRIPT_PATH" manual 1 2>&1)
+    sudo INSTALLHOME="$TEST_CONFIG_DIR" RSYNCSHOT_SKIP_MOUNT_CHECK=1 "$SCRIPT_PATH" manual 1 >/dev/null 2>&1
 
     # Excluded files should not exist in backup
-    assert_file_not_exists "$TEST_BACKUP_DIR/$HOSTNAME/latest/home/testuser/temp.tmp" "should exclude .tmp files" || {
+    assert_file_not_exists "$TEST_BACKUP_DIR/$HOSTNAME/latest$TEST_SOURCE_DIR/home/testuser/temp.tmp" "should exclude .tmp files" || {
         teardown_test_env
         return 1
     }
-    assert_file_not_exists "$TEST_BACKUP_DIR/$HOSTNAME/latest/home/testuser/app.log" "should exclude .log files" || {
+    assert_file_not_exists "$TEST_BACKUP_DIR/$HOSTNAME/latest$TEST_SOURCE_DIR/home/testuser/app.log" "should exclude .log files" || {
         teardown_test_env
         return 1
     }
     # Regular files should still exist
-    assert_file_exists "$TEST_BACKUP_DIR/$HOSTNAME/latest/home/testuser/file1.txt" "should include regular files" || {
+    assert_file_exists "$TEST_BACKUP_DIR/$HOSTNAME/latest$TEST_SOURCE_DIR/home/testuser/file1.txt" "should include regular files" || {
+        teardown_test_env
+        return 1
+    }
+
+    teardown_test_env
+}
+
+# ------------------------------------------------------------------------------
+# Test: Same-basename sources don't collide (relative paths)
+# ------------------------------------------------------------------------------
+# Two includes with the same basename ("bin") previously both mapped to
+# latest/bin, and the second sync's --delete wiped the first. With -R each is
+# stored under its full path, so both survive.
+test_no_basename_collision() {
+    setup_test_env
+
+    mkdir -p "$TEST_SOURCE_DIR/opt/bin" "$TEST_SOURCE_DIR/usr/bin"
+    echo "from-opt" > "$TEST_SOURCE_DIR/opt/bin/optfile"
+    echo "from-usr" > "$TEST_SOURCE_DIR/usr/bin/usrfile"
+
+    cat > "$TEST_CONFIG_DIR/config" <<EOF
+REMOTE_HOST=""
+MOUNTDIR="$TEST_BACKUP_DIR"
+EOF
+    cat > "$TEST_CONFIG_DIR/include.txt" <<EOF
+$TEST_SOURCE_DIR/opt/bin
+$TEST_SOURCE_DIR/usr/bin
+EOF
+    create_test_excludes >/dev/null
+
+    sudo INSTALLHOME="$TEST_CONFIG_DIR" RSYNCSHOT_SKIP_MOUNT_CHECK=1 "$SCRIPT_PATH" manual 1 >/dev/null 2>&1
+
+    assert_file_exists "$TEST_BACKUP_DIR/$HOSTNAME/latest$TEST_SOURCE_DIR/opt/bin/optfile" "opt/bin should survive" || {
+        teardown_test_env
+        return 1
+    }
+    assert_file_exists "$TEST_BACKUP_DIR/$HOSTNAME/latest$TEST_SOURCE_DIR/usr/bin/usrfile" "usr/bin should survive (no collision)" || {
         teardown_test_env
         return 1
     }
@@ -230,6 +260,7 @@ run_backup_tests() {
     run_test "respects retention limit" test_retention_limit
     run_test "backup command works as alias" test_backup_command
     run_test "excludes files matching patterns" test_excludes_patterns
+    run_test "same-basename sources don't collide" test_no_basename_collision
 }
 
 # Run if executed directly
diff --git a/tests/cases/test_cron.sh b/tests/cases/test_cron.sh
index 2fdb6f5..5c9bbfa 100755
--- a/tests/cases/test_cron.sh
+++ b/tests/cases/test_cron.sh
@@ -39,7 +39,7 @@ EOF
     create_test_excludes
 
     # Run setup (will fail on some checks but should still add cron)
-    sudo INSTALLHOME="$TEST_CONFIG_DIR" SCRIPTLOC="/tmp/rsyncshot-test" RSYNCSHOT_SKIP_MOUNT_CHECK=1 "$SCRIPT_PATH" setup 2>&1 >/dev/null || true
+    sudo INSTALLHOME="$TEST_CONFIG_DIR" SCRIPTLOC="/tmp/rsyncshot-test" RSYNCSHOT_SKIP_MOUNT_CHECK=1 "$SCRIPT_PATH" setup >/dev/null 2>&1 || true
 
     # Check crontab contains rsyncshot entries
     local crontab_content
@@ -72,8 +72,8 @@ EOF
     create_test_excludes
 
     # Run setup twice
-    sudo INSTALLHOME="$TEST_CONFIG_DIR" SCRIPTLOC="/tmp/rsyncshot-test" RSYNCSHOT_SKIP_MOUNT_CHECK=1 "$SCRIPT_PATH" setup 2>&1 >/dev/null || true
-    sudo INSTALLHOME="$TEST_CONFIG_DIR" SCRIPTLOC="/tmp/rsyncshot-test" RSYNCSHOT_SKIP_MOUNT_CHECK=1 "$SCRIPT_PATH" setup 2>&1 >/dev/null || true
+    sudo INSTALLHOME="$TEST_CONFIG_DIR" SCRIPTLOC="/tmp/rsyncshot-test" RSYNCSHOT_SKIP_MOUNT_CHECK=1 "$SCRIPT_PATH" setup >/dev/null 2>&1 || true
+    sudo INSTALLHOME="$TEST_CONFIG_DIR" SCRIPTLOC="/tmp/rsyncshot-test" RSYNCSHOT_SKIP_MOUNT_CHECK=1 "$SCRIPT_PATH" setup >/dev/null 2>&1 || true
 
     # Count rsyncshot entries
     local crontab_content hourly_count
@@ -105,7 +105,7 @@ EOF
     create_test_excludes
 
     # Run setup
-    sudo INSTALLHOME="$TEST_CONFIG_DIR" SCRIPTLOC="/tmp/rsyncshot-test" RSYNCSHOT_SKIP_MOUNT_CHECK=1 "$SCRIPT_PATH" setup 2>&1 >/dev/null || true
+    sudo INSTALLHOME="$TEST_CONFIG_DIR" SCRIPTLOC="/tmp/rsyncshot-test" RSYNCSHOT_SKIP_MOUNT_CHECK=1 "$SCRIPT_PATH" setup >/dev/null 2>&1 || true
 
     # Check custom job still exists
     local crontab_content
diff --git a/tests/cases/test_functions.sh b/tests/cases/test_functions.sh
new file mode 100644
index 0000000..cd607ab
--- /dev/null
+++ b/tests/cases/test_functions.sh
@@ -0,0 +1,146 @@
+#!/usr/bin/env bash
+# ==============================================================================
+# Unit Tests for Internal Functions (is_mounted, derive_paths)
+# ==============================================================================
+# These source the script with RSYNCSHOT_SOURCE_ONLY=1 so the functions are
+# defined without running the main flow, then exercise them directly. Each test
+# runs the source + assertions in a subshell so the derived globals and config
+# variables don't leak into other suites.
+
+source "$(dirname "${BASH_SOURCE[0]}")/../lib/test_helpers.sh"
+
+# ------------------------------------------------------------------------------
+# is_mounted: exact mount point matches
+# ------------------------------------------------------------------------------
+test_is_mounted_exact_match() {
+    setup_test_env
+    local mounts="$TEST_DIR/mounts"
+    cat > "$mounts" <<EOF
+/dev/sda1 /media/backup ext4 rw,relatime 0 0
+/dev/sdb1 /home ext4 rw,relatime 0 0
+EOF
+    (
+        INSTALLHOME="$TEST_CONFIG_DIR" RSYNCSHOT_SOURCE_ONLY=1 source "$SCRIPT_PATH"
+        is_mounted "/media/backup" "$mounts"
+    )
+    local rc=$?
+    teardown_test_env
+    assert_exit_code 0 "$rc" "exact mount point should match" || return 1
+}
+
+# ------------------------------------------------------------------------------
+# is_mounted: a prefix is NOT a match (the substring-grep bug)
+# ------------------------------------------------------------------------------
+test_is_mounted_no_substring_match() {
+    setup_test_env
+    local mounts="$TEST_DIR/mounts"
+    cat > "$mounts" <<EOF
+/dev/sda1 /media/backup2 ext4 rw,relatime 0 0
+EOF
+    (
+        INSTALLHOME="$TEST_CONFIG_DIR" RSYNCSHOT_SOURCE_ONLY=1 source "$SCRIPT_PATH"
+        is_mounted "/media/backup" "$mounts"
+    )
+    local rc=$?
+    teardown_test_env
+    assert_exit_code 1 "$rc" "/media/backup must NOT match /media/backup2" || return 1
+}
+
+# ------------------------------------------------------------------------------
+# is_mounted: mount points with spaces (encoded as \040 in /proc/mounts)
+# ------------------------------------------------------------------------------
+test_is_mounted_handles_spaces() {
+    setup_test_env
+    local mounts="$TEST_DIR/mounts"
+    printf '/dev/sda1 /media/my\\040backup ext4 rw 0 0\n' > "$mounts"
+    (
+        INSTALLHOME="$TEST_CONFIG_DIR" RSYNCSHOT_SOURCE_ONLY=1 source "$SCRIPT_PATH"
+        is_mounted "/media/my backup" "$mounts"
+    )
+    local rc=$?
+    teardown_test_env
+    assert_exit_code 0 "$rc" "mount point with spaces should match" || return 1
+}
+
+# ------------------------------------------------------------------------------
+# is_mounted: target is treated literally, not as a regex
+# ------------------------------------------------------------------------------
+test_is_mounted_target_is_literal() {
+    setup_test_env
+    local mounts="$TEST_DIR/mounts"
+    cat > "$mounts" <<EOF
+/dev/sda1 /media/backup ext4 rw 0 0
+EOF
+    (
+        INSTALLHOME="$TEST_CONFIG_DIR" RSYNCSHOT_SOURCE_ONLY=1 source "$SCRIPT_PATH"
+        is_mounted "/media/b.ckup" "$mounts"
+    )
+    local rc=$?
+    teardown_test_env
+    assert_exit_code 1 "$rc" "regex metacharacters in target must be literal" || return 1
+}
+
+# ------------------------------------------------------------------------------
+# derive_paths: remote mode uses bare cp/mv/rm (resolved by remote PATH)
+# ------------------------------------------------------------------------------
+test_derive_paths_remote() {
+    setup_test_env
+    cat > "$TEST_CONFIG_DIR/config" <<EOF
+REMOTE_HOST="truenas"
+REMOTE_PATH="/mnt/vault/Backups"
+EOF
+    (
+        # The script sources the config and runs derive_paths during load.
+        INSTALLHOME="$TEST_CONFIG_DIR" RSYNCSHOT_SOURCE_ONLY=1 source "$SCRIPT_PATH"
+        assert_equals "remote" "$MODE" "should be remote mode" &&
+        assert_equals "truenas:/mnt/vault/Backups/$HOSTNAME" "$DESTINATION" "remote destination" &&
+        assert_equals "cp" "$CP" "remote cp is bare (remote PATH resolves it)" &&
+        assert_equals "mv" "$MV" "remote mv is bare" &&
+        assert_equals "rm" "$RM" "remote rm is bare"
+    )
+    local rc=$?
+    teardown_test_env
+    return $rc
+}
+
+# ------------------------------------------------------------------------------
+# derive_paths: local mode resolves cp/mv/rm to real absolute binaries
+# ------------------------------------------------------------------------------
+test_derive_paths_local() {
+    setup_test_env
+    cat > "$TEST_CONFIG_DIR/config" <<EOF
+REMOTE_HOST=""
+MOUNTDIR="/media/backup"
+EOF
+    (
+        INSTALLHOME="$TEST_CONFIG_DIR" RSYNCSHOT_SOURCE_ONLY=1 source "$SCRIPT_PATH"
+        assert_equals "local" "$MODE" "should be local mode" &&
+        assert_equals "/media/backup/$HOSTNAME" "$DESTINATION" "local destination" &&
+        assert_contains "$CP" "/" "local cp resolves to an absolute path, not a bare name"
+    )
+    local rc=$?
+    teardown_test_env
+    return $rc
+}
+
+# ------------------------------------------------------------------------------
+# Run tests
+# ------------------------------------------------------------------------------
+run_functions_tests() {
+    echo ""
+    echo "Running function unit tests..."
+    echo "------------------------------------------------------------"
+
+    run_test "is_mounted exact match" test_is_mounted_exact_match
+    run_test "is_mounted rejects prefix (no substring match)" test_is_mounted_no_substring_match
+    run_test "is_mounted handles spaces" test_is_mounted_handles_spaces
+    run_test "is_mounted treats target literally" test_is_mounted_target_is_literal
+    run_test "derive_paths remote mode" test_derive_paths_remote
+    run_test "derive_paths local mode" test_derive_paths_local
+}
+
+# Run if executed directly
+if [[ "${BASH_SOURCE[0]}" == "${0}" ]]; then
+    run_functions_tests
+    print_summary
+fi
diff --git a/tests/cases/test_remote.sh b/tests/cases/test_remote.sh
new file mode 100644
index 0000000..c2ee75e
--- /dev/null
+++ b/tests/cases/test_remote.sh
@@ -0,0 +1,60 @@
+#!/usr/bin/env bash
+# ==============================================================================
+# Remote (SSH) Mode Tests
+# ==============================================================================
+# First coverage of the remote path — backup + rotation run over SSH. The whole
+# transfer and rotation happen via ssh to localhost, which exercises run_cmd's
+# remote branch and the bare cp/mv/rm rotation commands. Gated: if root can't
+# ssh to localhost with key auth, the suite skips rather than fails, so machines
+# without loopback SSH still pass.
+
+source "$(dirname "${BASH_SOURCE[0]}")/../lib/test_helpers.sh"
+
+remote_available() {
+    ssh -o BatchMode=yes -o ConnectTimeout=5 localhost true 2>/dev/null
+}
+
+# ------------------------------------------------------------------------------
+# Backup + rotation over SSH-to-localhost
+# ------------------------------------------------------------------------------
+test_remote_backup_and_rotation() {
+    setup_test_env
+    cat > "$TEST_CONFIG_DIR/config" <<EOF
+REMOTE_HOST="localhost"
+REMOTE_PATH="$TEST_BACKUP_DIR"
+EOF
+    create_test_includes >/dev/null
+    create_test_excludes >/dev/null
+
+    # Two runs to exercise rotation over SSH (MANUAL.0 rotates to MANUAL.1)
+    INSTALLHOME="$TEST_CONFIG_DIR" "$SCRIPT_PATH" manual 2 >/dev/null 2>&1
+    INSTALLHOME="$TEST_CONFIG_DIR" "$SCRIPT_PATH" manual 2 >/dev/null 2>&1
+
+    assert_dir_exists "$TEST_BACKUP_DIR/$HOSTNAME/MANUAL.0" "remote backup should create MANUAL.0" || { teardown_test_env; return 1; }
+    assert_dir_exists "$TEST_BACKUP_DIR/$HOSTNAME/MANUAL.1" "remote rotation should produce MANUAL.1" || { teardown_test_env; return 1; }
+    assert_file_exists "$TEST_BACKUP_DIR/$HOSTNAME/latest$TEST_SOURCE_DIR/home/testuser/file1.txt" "remote backup should copy files" || { teardown_test_env; return 1; }
+
+    teardown_test_env
+}
+
+# ------------------------------------------------------------------------------
+# Run tests
+# ------------------------------------------------------------------------------
+run_remote_tests() {
+    echo ""
+    echo "Running remote (SSH) tests..."
+    echo "------------------------------------------------------------"
+
+    if ! remote_available; then
+        echo "  SKIP: root cannot ssh to localhost with key auth — skipping remote tests"
+        return 0
+    fi
+
+    run_test "remote backup + rotation over SSH" test_remote_backup_and_rotation
+}
+
+# Run if executed directly
+if [[ "${BASH_SOURCE[0]}" == "${0}" ]]; then
+    run_remote_tests
+    print_summary
+fi
diff --git a/tests/cases/test_rsync_flags.sh b/tests/cases/test_rsync_flags.sh
new file mode 100644
index 0000000..768b432
--- /dev/null
+++ b/tests/cases/test_rsync_flags.sh
@@ -0,0 +1,53 @@
+#!/usr/bin/env bash
+# ==============================================================================
+# rsync Invocation Flag Tests
+# ==============================================================================
+# Uses the command-capture shim to intercept rsync and assert on the flags the
+# script passes, without performing a real transfer. Runs a dryrun so only rsync
+# is invoked (no rotation, no cp/mv/rm).
+
+source "$(dirname "${BASH_SOURCE[0]}")/../lib/test_helpers.sh"
+
+# ------------------------------------------------------------------------------
+# rsync is called with --numeric-ids (ownership fidelity) and -R (relative paths)
+# ------------------------------------------------------------------------------
+test_rsync_flags_numeric_ids_and_relative() {
+    setup_test_env
+    create_test_config >/dev/null
+    create_test_includes >/dev/null
+    create_test_excludes >/dev/null
+
+    local shim
+    shim=$(make_command_shim rsync)
+
+    PATH="$shim:$PATH" INSTALLHOME="$TEST_CONFIG_DIR" RSYNCSHOT_SKIP_MOUNT_CHECK=1 \
+        "$SCRIPT_PATH" dryrun manual 1 >/dev/null 2>&1
+
+    local args
+    args=$(cat "$shim/rsync.args" 2>/dev/null)
+
+    assert_file_exists "$shim/rsync.args" "rsync shim should have been invoked" || { rm -rf "$shim"; teardown_test_env; return 1; }
+    assert_contains "$args" "--numeric-ids" "rsync should be called with --numeric-ids" || { rm -rf "$shim"; teardown_test_env; return 1; }
+    assert_contains "$args" "-avhR" "rsync should be called with -R (relative paths, in -avhR)" || { rm -rf "$shim"; teardown_test_env; return 1; }
+    assert_contains "$args" "--dry-run" "dryrun should pass --dry-run" || { rm -rf "$shim"; teardown_test_env; return 1; }
+
+    rm -rf "$shim"
+    teardown_test_env
+}
+
+# ------------------------------------------------------------------------------
+# Run tests
+# ------------------------------------------------------------------------------
+run_rsync_flags_tests() {
+    echo ""
+    echo "Running rsync flag tests..."
+    echo "------------------------------------------------------------"
+
+    run_test "rsync uses --numeric-ids and -R" test_rsync_flags_numeric_ids_and_relative
+}
+
+# Run if executed directly
+if [[ "${BASH_SOURCE[0]}" == "${0}" ]]; then
+    run_rsync_flags_tests
+    print_summary
+fi
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
@@ -80,6 +80,59 @@ test_accepts_valid_snapshot_types() {
 }
 
 # ------------------------------------------------------------------------------
+# 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
 # ------------------------------------------------------------------------------
 run_validation_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
 }
diff --git a/tests/lib/test_helpers.sh b/tests/lib/test_helpers.sh
index 726d788..d58895b 100755
--- a/tests/lib/test_helpers.sh
+++ b/tests/lib/test_helpers.sh
@@ -6,6 +6,7 @@
 # Colors for output
 RED='\033[0;31m'
 GREEN='\033[0;32m'
+# shellcheck disable=SC2034  # reserved for future use, kept alongside RED/GREEN
 YELLOW='\033[0;33m'
 NC='\033[0m' # No Color
 
@@ -131,9 +132,9 @@ assert_exit_code() {
 assert_contains() {
     local haystack="$1"
     local needle="$2"
-    local message="${3:-Output should contain '$needle'}"
+    local message="${3:-Output should contain: $needle}"
 
-    if echo "$haystack" | grep -q "$needle"; then
+    if echo "$haystack" | grep -qF -- "$needle"; then
         return 0
     else
         echo -e "${RED}FAIL${NC}: $message"
@@ -146,9 +147,9 @@ assert_contains() {
 assert_not_contains() {
     local haystack="$1"
     local needle="$2"
-    local message="${3:-Output should not contain '$needle'}"
+    local message="${3:-Output should not contain: $needle}"
 
-    if ! echo "$haystack" | grep -q "$needle"; then
+    if ! echo "$haystack" | grep -qF -- "$needle"; then
         return 0
     else
         echo -e "${RED}FAIL${NC}: $message"
@@ -206,6 +207,48 @@ assert_dir_not_exists() {
     fi
 }
 
+assert_no_stderr() {
+    local stderr_content="$1"
+    local message="${2:-Should produce no stderr}"
+
+    if [ -z "$stderr_content" ]; then
+        return 0
+    else
+        echo -e "${RED}FAIL${NC}: $message"
+        echo "  stderr was: $stderr_content"
+        return 1
+    fi
+}
+
+# ------------------------------------------------------------------------------
+# Command-capture shim
+# ------------------------------------------------------------------------------
+# make_command_shim CMD [CMD...] — create a directory of fake commands that
+# record their argv (one argument per line, "---" between invocations) to
+# <dir>/<cmd>.args and exit 0. Put the dir at the front of PATH to intercept
+# those commands and assert on HOW they were called, without real I/O, network,
+# or transfers. Echoes the shim directory path; the caller removes it.
+#
+# Usage:
+#   shim=$(make_command_shim rsync)
+#   PATH="$shim:$PATH" "$SCRIPT_PATH" dryrun manual 1
+#   assert_contains "$(cat "$shim/rsync.args")" "--numeric-ids" ...
+#   rm -rf "$shim"
+make_command_shim() {
+    local shim_dir
+    shim_dir=$(mktemp -d)
+    local cmd
+    for cmd in "$@"; do
+        cat > "$shim_dir/$cmd" <<SHIM
+#!/usr/bin/env bash
+{ printf '%s\n' "\$@"; echo "---"; } >> "$shim_dir/${cmd}.args"
+exit 0
+SHIM
+        chmod +x "$shim_dir/$cmd"
+    done
+    echo "$shim_dir"
+}
+
 # ------------------------------------------------------------------------------
 # Test Runner
 # ------------------------------------------------------------------------------
diff --git a/tests/test_rsyncshot.sh b/tests/test_rsyncshot.sh
index 15f2a99..95a0032 100755
--- a/tests/test_rsyncshot.sh
+++ b/tests/test_rsyncshot.sh
@@ -22,6 +22,7 @@ QUICK=false
 while [[ $# -gt 0 ]]; do
     case $1 in
         -v|--verbose)
+            # shellcheck disable=SC2034  # reserved flag; verbose output not yet wired
             VERBOSE=true
             shift
             ;;
@@ -90,12 +91,15 @@ run_test_file() {
 
 # Run test suites
 run_test_file "$SCRIPT_DIR/cases/test_validation.sh" "validation"
+run_test_file "$SCRIPT_DIR/cases/test_functions.sh" "functions"
 run_test_file "$SCRIPT_DIR/cases/test_includes.sh" "includes"
 run_test_file "$SCRIPT_DIR/cases/test_dryrun.sh" "dryrun"
+run_test_file "$SCRIPT_DIR/cases/test_rsync_flags.sh" "rsync_flags"
 
 if [ "$QUICK" = false ]; then
     run_test_file "$SCRIPT_DIR/cases/test_backup.sh" "backup"
     run_test_file "$SCRIPT_DIR/cases/test_cron.sh" "cron"
+    run_test_file "$SCRIPT_DIR/cases/test_remote.sh" "remote"
 else
     echo ""
     echo "Skipping slow tests (backup, cron) - use without --quick to run all"