From dd41036329e28a46a7269025657d782736c5d84d Mon Sep 17 00:00:00 2001 From: Craig Jennings Date: Mon, 20 Jul 2026 15:52:40 -0500 Subject: fix(installer): guard the cp and chmod that install helper scripts With `set -e` off, the installer copied its zfs-replicate, zfs-pre-snapshot, and hypr-live-update-guard scripts into place with bare cp/chmod. A missing or partial source failed silently, leaving a systemd service with a dead ExecStart or a pacman hook pointing at a script that never landed. The two zfs scripts now install through install_executable, which guards the copy, warns on failure, and skips the chmod when the copy did not land. The two live-update-guard chmods (the guard binary and its hook) are guarded inline with error_warn. --- archsetup | 27 +++++++--- tests/installer-steps/test_install_executable.py | 66 ++++++++++++++++++++++++ 2 files changed, 87 insertions(+), 6 deletions(-) create mode 100644 tests/installer-steps/test_install_executable.py diff --git a/archsetup b/archsetup index 28ec2c2..d7b84cb 100755 --- a/archsetup +++ b/archsetup @@ -1131,6 +1131,21 @@ configure_build_environment() { } +# Guarded install of an executable: copy to , then chmod +x. With +# set -e off, a bare cp/chmod on a missing or partial source failed silently and +# left a dead ExecStart or a pacman hook pointing at a script that never landed. +# Each step warns on failure and the chmod is skipped when the copy did not +# land. +install_executable() { + local src="$1" dest="$2" + if cp "$src" "$dest" >> "$logfile" 2>&1; then + chmod +x "$dest" >> "$logfile" 2>&1 || error_warn "chmod +x $dest" "$?" + else + error_warn "copying $src to $dest" "$?" + return 1 + fi +} + # Append to the crontab piped in on stdin, but only when no existing # line contains . Idempotent so a resume re-run of the step never stacks # a duplicate entry. Emits the resulting crontab on stdout. @@ -1902,8 +1917,7 @@ configure_zfs_snapshots() { EOF action="installing zfs-replicate script" && display "task" "$action" - cp "$user_archsetup_dir/scripts/zfs-replicate" /usr/local/bin/zfs-replicate - chmod +x /usr/local/bin/zfs-replicate + install_executable "$user_archsetup_dir/scripts/zfs-replicate" /usr/local/bin/zfs-replicate action="creating zfs-replicate systemd service" && display "task" "$action" cat << 'EOF' > /etc/systemd/system/zfs-replicate.service @@ -2032,8 +2046,7 @@ configure_pre_pacman_snapshots() { is_zfs_root || return 0 action="installing pre-pacman snapshot script" && display "task" "$action" - cp "$user_archsetup_dir/scripts/zfs-pre-snapshot" /usr/local/bin/zfs-pre-snapshot - chmod +x /usr/local/bin/zfs-pre-snapshot + install_executable "$user_archsetup_dir/scripts/zfs-pre-snapshot" /usr/local/bin/zfs-pre-snapshot action="installing pre-pacman snapshot hook" && display "task" "$action" mkdir -p /etc/pacman.d/hooks @@ -2197,7 +2210,8 @@ UDEVEOF action="Live-Update Guard" && display "subtitle" "$action" run_task "installing the live GPU/compositor update guard" \ cp "$user_archsetup_dir/scripts/hypr-live-update-guard" /usr/local/bin/hypr-live-update-guard - chmod 755 /usr/local/bin/hypr-live-update-guard + chmod 755 /usr/local/bin/hypr-live-update-guard >> "$logfile" 2>&1 \ + || error_warn "chmod 755 hypr-live-update-guard" "$?" action="installing the live-update guard pacman hook" && display "task" "$action" mkdir -p /etc/pacman.d/hooks @@ -2233,7 +2247,8 @@ Exec = /usr/local/bin/hypr-live-update-guard AbortOnFail NeedsTargets HOOKEOF - chmod 644 /etc/pacman.d/hooks/10-hypr-live-update-guard.hook + chmod 644 /etc/pacman.d/hooks/10-hypr-live-update-guard.hook >> "$logfile" 2>&1 \ + || error_warn "chmod 644 hypr-live-update-guard.hook" "$?" } ### Display Server (conditional) diff --git a/tests/installer-steps/test_install_executable.py b/tests/installer-steps/test_install_executable.py new file mode 100644 index 0000000..6a2ecfd --- /dev/null +++ b/tests/installer-steps/test_install_executable.py @@ -0,0 +1,66 @@ +"""Test the guarded executable install helper. + +With `set -e` off, the installer's bare `cp script /usr/local/bin/...` followed +by `chmod +x` failed silently when the source was missing or partial, leaving a +systemd service with a dead ExecStart or a pacman hook pointing at a script that +was never installed. install_executable guards the copy and the chmod, warns on +failure, and skips the chmod when the copy did not land. + +Method: sed-extract install_executable from the real `archsetup` and run it +against real temp files (no fakes needed) with a fake error_warn recorder. + +Run from repo root: + python3 -m unittest tests.installer-steps.test_install_executable +""" + +import os +import subprocess +import tempfile +import textwrap +import unittest + +REPO_ROOT = os.path.abspath(os.path.join(os.path.dirname(__file__), "..", "..")) +ARCHSETUP = os.path.join(REPO_ROOT, "archsetup") + + +def run(src, dest): + script = textwrap.dedent(f"""\ + logfile=/dev/null + error_warn() {{ echo "WARN: $1"; return 1; }} + source <(sed -n '/^install_executable() {{/,/^}}/p' "{ARCHSETUP}") + install_executable "{src}" "{dest}" + echo "RC=$?" + """) + return subprocess.run( + ["bash", "-c", script], capture_output=True, text=True, timeout=10, + ) + + +class InstallExecutable(unittest.TestCase): + def test_copies_and_makes_executable(self): + with tempfile.TemporaryDirectory() as d: + src = os.path.join(d, "myscript") + dest = os.path.join(d, "bin", "myscript") + os.mkdir(os.path.join(d, "bin")) + with open(src, "w") as f: + f.write("#!/bin/sh\necho hi\n") + r = run(src, dest) + self.assertIn("RC=0", r.stdout) + self.assertTrue(os.path.exists(dest)) + self.assertTrue(os.access(dest, os.X_OK), "dest must be executable") + self.assertNotIn("WARN", r.stdout) + + def test_missing_source_warns_and_skips_chmod(self): + with tempfile.TemporaryDirectory() as d: + src = os.path.join(d, "does-not-exist") + dest = os.path.join(d, "bin", "myscript") + os.mkdir(os.path.join(d, "bin")) + r = run(src, dest) + self.assertIn("WARN: copying", r.stdout, + "a failed copy must warn, not silently continue") + self.assertNotIn("RC=0", r.stdout, "helper returns non-zero on copy failure") + self.assertFalse(os.path.exists(dest)) + + +if __name__ == "__main__": + unittest.main() -- cgit v1.2.3