fix(installer): guard constructed-path rm -rf deletes

Three rm -rf sites in archsetup delete paths built from variables: $state_dir for --fresh, and $source_dir/$prog_name for the git and AUR clone-retry cleanups. If a path variable were empty or malformed (preflight skipped, a degenerate git URL), the delete could expand to a top-level or otherwise unintended directory.

I added a safe_rm_rf <path> <allowed_prefix> helper that refuses to run unless the target is absolute, free of '..', deeper than a bare top-level dir, strictly inside the allowed prefix, and a real directory rather than a symlink. On the happy path it delegates to rm -rf, so successful installs are unchanged. The helper is self-contained and defined before the top-level --fresh handler, which runs before the logging helpers exist.

I covered the guard with unit tests under tests/safe-rm-rf/ that source the real function and exercise normal, boundary, and error cases against temp directories.

2 files changed, 229 insertions, 3 deletions

diff --git a/archsetup b/archsetup
index 6c17232..6f828f5 100755
--- a/archsetup
+++ b/archsetup
@@ -295,6 +295,61 @@ show_status() {
     exit 0
 }
 
+### Safe Removal
+
+# safe_rm_rf <path> <allowed_prefix>
+# Guarded recursive delete. Refuses to run unless <path>:
+#   - is non-empty (and <allowed_prefix> is non-empty),
+#   - is an absolute path with no '..' segment,
+#   - is not '/' or a bare top-level dir (must be at least 2 levels deep),
+#   - lies strictly inside <allowed_prefix> (and is not the prefix itself),
+#   - exists and is a real directory (not a symlink).
+# Prints its own refusal reason to stderr and returns non-zero on any failed
+# check (deleting nothing). Returns 0 on a successful delete or if the path
+# is already gone. Self-contained — no dependency on error_warn — so it is
+# safe to call from the top-level --fresh handler that runs before the
+# logging helpers are defined.
+safe_rm_rf() {
+    local target="$1"
+    local prefix="$2"
+
+    if [ -z "$target" ] || [ -z "$prefix" ]; then
+        echo "safe_rm_rf: empty target or prefix" >&2
+        return 1
+    fi
+    case "$target" in
+        /*) : ;;
+        *) echo "safe_rm_rf: refusing non-absolute path '$target'" >&2; return 1 ;;
+    esac
+    case "$target" in
+        *..*) echo "safe_rm_rf: refusing path with '..' '$target'" >&2; return 1 ;;
+    esac
+    # Reject '/' and bare top-level dirs (e.g. '/home'): require >= 2 slashes.
+    case "$target" in
+        /*/?*) : ;;
+        *) echo "safe_rm_rf: refusing top-level path '$target'" >&2; return 1 ;;
+    esac
+    # Must be strictly inside the allowed prefix, not the prefix itself.
+    case "$target/" in
+        "$prefix"/*) : ;;
+        *) echo "safe_rm_rf: '$target' is outside '$prefix'" >&2; return 1 ;;
+    esac
+    if [ "$target" = "$prefix" ]; then
+        echo "safe_rm_rf: refusing to delete the prefix root '$prefix'" >&2
+        return 1
+    fi
+    # Already gone is fine.
+    if [ ! -e "$target" ]; then
+        return 0
+    fi
+    # Must be a real directory, not a symlink to one.
+    if [ -L "$target" ] || [ ! -d "$target" ]; then
+        echo "safe_rm_rf: '$target' is not a real directory" >&2
+        return 1
+    fi
+    rm -rf "$target"
+}
+
 # Handle --status flag (must be after state_dir is defined)
 if $show_status_only; then
     show_status
@@ -303,7 +358,7 @@ fi
 # Handle --fresh flag
 if $fresh_install; then
     echo "Starting fresh installation (removing previous state)..."
-    rm -rf "$state_dir"
+    safe_rm_rf "$state_dir" "/var/lib/archsetup"
 fi
 
 ### Pre-flight Checks
@@ -530,7 +585,7 @@ git_install() {
 
     if ! (sudo -u "$username" git clone --depth 1 "$1" "$build_dir" >> "$logfile" 2>&1); then
         error_warn "cloning $prog_name - directory may exist, removing and retrying" "$?"
-        rm -rf "$build_dir" >> "$logfile" 2>&1 || \
+        safe_rm_rf "$build_dir" "$source_dir" >> "$logfile" 2>&1 || \
             error_warn "removing existing directory for $prog_name" "$?"
         sudo -u "$username" git clone --depth 1 "$1" "$build_dir" >> "$logfile" 2>&1 || \
             error_fatal "re-cloning $prog_name after cleanup" "$?"
@@ -1010,7 +1065,7 @@ aur_installer() {
     display "task" "fetching source code for yay"
     if ! (sudo -u "$username" git clone --depth 1 "$yay_repo" "$build_dir" >> "$logfile" 2>&1); then
         error_warn "cloning source code for yay - directory may exist, removing and retrying" "$?"
-        (rm -rf "$build_dir" >> "$logfile" 2>&1) || \
+        (safe_rm_rf "$build_dir" "$source_dir" >> "$logfile" 2>&1) || \
             error_fatal "removing existing directory for yay" "$?"
         (sudo -u "$username" git clone --depth 1 "$yay_repo" "$build_dir" >> "$logfile" 2>&1) || \
             error_fatal "re-cloning source code for yay after cleanup" "$?"
diff --git a/tests/safe-rm-rf/test_safe_rm_rf.py b/tests/safe-rm-rf/test_safe_rm_rf.py
new file mode 100644
index 0000000..0cf23c6
--- /dev/null
+++ b/tests/safe-rm-rf/test_safe_rm_rf.py
@@ -0,0 +1,171 @@
+"""Tests for the safe_rm_rf guard helper in the archsetup installer.
+
+safe_rm_rf is a defensive wrapper around `rm -rf`: it refuses to delete a
+path unless the path is absolute, free of '..', deeper than a bare
+top-level dir, strictly inside a caller-supplied allowed prefix, and a real
+directory (not a symlink). On the happy path it delegates to `rm -rf`.
+
+These tests exercise the REAL function body, extracted from the `archsetup`
+script at run time (not a copy), with a stub `error_warn` standing in for
+the installer's logger. The delete runs against real temp dirs the test
+creates and tears down, so the rm path is genuinely exercised.
+
+Run from repo root:
+    python3 -m unittest tests.safe-rm-rf.test_safe_rm_rf
+"""
+
+import os
+import shutil
+import subprocess
+import tempfile
+import unittest
+
+
+REPO_ROOT = os.path.abspath(os.path.join(os.path.dirname(__file__), "..", ".."))
+ARCHSETUP = os.path.join(REPO_ROOT, "archsetup")
+
+
+class SafeRmRfHarness(unittest.TestCase):
+    """Source safe_rm_rf out of the real archsetup script and invoke it."""
+
+    def setUp(self):
+        self.tmp = tempfile.mkdtemp(prefix="safe-rm-rf-test-")
+        # A bash wrapper that extracts just the safe_rm_rf function from the
+        # real installer and invokes it with the test's args. Sourcing the
+        # sed-extracted function means we test the production code path, not a
+        # reimplementation. The helper is self-contained (it prints its own
+        # refusal reasons), so no logger stub is needed.
+        self.wrapper = os.path.join(self.tmp, "run.sh")
+        with open(self.wrapper, "w") as f:
+            f.write(
+                "#!/bin/bash\n"
+                'ARCHSETUP="$1"; shift\n'
+                "source <(sed -n '/^safe_rm_rf() {/,/^}/p' \"$ARCHSETUP\")\n"
+                'safe_rm_rf "$@"\n'
+            )
+        os.chmod(self.wrapper, 0o755)
+
+    def tearDown(self):
+        shutil.rmtree(self.tmp, ignore_errors=True)
+
+    def run_guard(self, target, prefix):
+        return subprocess.run(
+            ["bash", self.wrapper, ARCHSETUP, target, prefix],
+            capture_output=True, text=True, timeout=10,
+        )
+
+    def make_dir(self, *parts):
+        path = os.path.join(self.tmp, *parts)
+        os.makedirs(path, exist_ok=True)
+        return path
+
+
+# -----------------------------------------------------------------------------
+# Normal cases
+# -----------------------------------------------------------------------------
+
+class TestSafeRmRfNormal(SafeRmRfHarness):
+
+    def test_real_dir_under_prefix_is_deleted(self):
+        prefix = self.make_dir("src")
+        target = self.make_dir("src", "repo")
+        result = self.run_guard(target, prefix)
+        self.assertEqual(result.returncode, 0, msg=result.stderr)
+        self.assertFalse(os.path.exists(target), "target should be deleted")
+        self.assertTrue(os.path.isdir(prefix), "prefix must survive")
+
+    def test_nested_dir_with_contents_is_deleted(self):
+        prefix = self.make_dir("src")
+        target = self.make_dir("src", "repo", "sub")
+        with open(os.path.join(target, "file.txt"), "w") as f:
+            f.write("data")
+        result = self.run_guard(os.path.join(prefix, "repo"), prefix)
+        self.assertEqual(result.returncode, 0, msg=result.stderr)
+        self.assertFalse(os.path.exists(os.path.join(prefix, "repo")))
+
+
+# -----------------------------------------------------------------------------
+# Boundary cases
+# -----------------------------------------------------------------------------
+
+class TestSafeRmRfBoundary(SafeRmRfHarness):
+
+    def test_target_already_absent_succeeds_quietly(self):
+        prefix = self.make_dir("src")
+        target = os.path.join(prefix, "never-existed")
+        result = self.run_guard(target, prefix)
+        self.assertEqual(result.returncode, 0, msg=result.stderr)
+
+    def test_target_equal_to_prefix_is_refused(self):
+        prefix = self.make_dir("src")
+        result = self.run_guard(prefix, prefix)
+        self.assertNotEqual(result.returncode, 0)
+        self.assertTrue(os.path.isdir(prefix), "prefix must not be deleted")
+
+    def test_symlink_to_dir_under_prefix_is_refused(self):
+        prefix = self.make_dir("src")
+        real = self.make_dir("src", "real")
+        link = os.path.join(prefix, "link")
+        os.symlink(real, link)
+        result = self.run_guard(link, prefix)
+        self.assertNotEqual(result.returncode, 0)
+        self.assertTrue(os.path.isdir(real), "symlink target must survive")
+        self.assertTrue(os.path.islink(link), "symlink itself must survive")
+
+    def test_prefix_lookalike_is_refused(self):
+        # target shares a textual prefix but is not inside it:
+        # /.../srcX is NOT under /.../src
+        prefix = self.make_dir("src")
+        sibling = self.make_dir("srcX")
+        result = self.run_guard(sibling, prefix)
+        self.assertNotEqual(result.returncode, 0)
+        self.assertTrue(os.path.isdir(sibling), "lookalike sibling must survive")
+
+
+# -----------------------------------------------------------------------------
+# Error cases
+# -----------------------------------------------------------------------------
+
+class TestSafeRmRfErrors(SafeRmRfHarness):
+
+    def test_empty_target_is_refused(self):
+        prefix = self.make_dir("src")
+        result = self.run_guard("", prefix)
+        self.assertNotEqual(result.returncode, 0)
+
+    def test_empty_prefix_is_refused(self):
+        target = self.make_dir("src", "repo")
+        result = self.run_guard(target, "")
+        self.assertNotEqual(result.returncode, 0)
+        self.assertTrue(os.path.isdir(target), "target must survive a bad call")
+
+    def test_relative_path_is_refused(self):
+        result = self.run_guard("foo/bar", "/var/lib/archsetup")
+        self.assertNotEqual(result.returncode, 0)
+
+    def test_root_is_refused(self):
+        result = self.run_guard("/", "/var/lib/archsetup")
+        self.assertNotEqual(result.returncode, 0)
+
+    def test_bare_top_level_dir_is_refused(self):
+        result = self.run_guard("/home", "/home")
+        self.assertNotEqual(result.returncode, 0)
+        self.assertTrue(os.path.isdir("/home"), "/home must never be touched")
+
+    def test_path_with_dotdot_is_refused(self):
+        prefix = self.make_dir("src")
+        # textually inside the prefix but contains a traversal segment
+        sneaky = os.path.join(prefix, "..", "src", "repo")
+        result = self.run_guard(sneaky, prefix)
+        self.assertNotEqual(result.returncode, 0)
+
+    def test_path_outside_prefix_is_refused(self):
+        prefix = self.make_dir("src")
+        outside = self.make_dir("other")
+        result = self.run_guard(outside, prefix)
+        self.assertNotEqual(result.returncode, 0)
+        self.assertTrue(os.path.isdir(outside), "outside dir must survive")
+
+
+if __name__ == "__main__":
+    unittest.main()