From c80e85545be3b1ab98b69771f502b7b36102c7f6 Mon Sep 17 00:00:00 2001
From: Craig Jennings <c@cjennings.net>
Date: Mon, 20 Jul 2026 15:44:29 -0500
Subject: fix(installer): validate sudoers.pacnew with visudo before copying

The installer copied /etc/sudoers.pacnew over /etc/sudoers with no validation. A malformed pacnew that sudo refuses to parse would replace a working sudoers with an unparseable one, locking out privilege escalation right before the NOPASSWD rule gets appended. replace_sudoers_pacnew now runs `visudo -cf` on the pacnew first and only copies a file that validates; a failure warns and leaves the current sudoers in place.

Extracted the logic into a function taking the pacnew and target as positional args defaulting to the system paths, so the guard can run against fixtures.
---
 archsetup                                          | 19 +++++-
 .../installer-steps/test_replace_sudoers_pacnew.py | 76 ++++++++++++++++++++++
 2 files changed, 94 insertions(+), 1 deletion(-)
 create mode 100644 tests/installer-steps/test_replace_sudoers_pacnew.py

diff --git a/archsetup b/archsetup
index b7541e1..646d1f9 100755
--- a/archsetup
+++ b/archsetup
@@ -1131,6 +1131,23 @@ configure_build_environment() {
 
 }
 
+# Replace /etc/sudoers with its .pacnew only after visudo validates it. A
+# malformed pacnew that sudo refuses to parse would lock out privilege
+# escalation, so a validation failure warns (non-fatal) and keeps the working
+# file. Paths are positional args defaulting to the system paths so the guard
+# can be exercised against fixtures.
+replace_sudoers_pacnew() {
+    local pacnew="${1:-/etc/sudoers.pacnew}"
+    local target="${2:-/etc/sudoers}"
+    [ -f "$pacnew" ] || return 0
+    if visudo -cf "$pacnew" >> "$logfile" 2>&1; then
+        cp "$pacnew" "$target" >> "$logfile" 2>&1 \
+            || error_warn "copying validated $pacnew to $target" "$?"
+    else
+        error_warn "$pacnew failed visudo validation; keeping current sudoers" "1"
+    fi
+}
+
 configure_package_mirrors() {
     action="Package Mirrors" && display "subtitle" "$action"
     pacman_install reflector
@@ -1168,7 +1185,7 @@ EOF
         error_warn "$action" "$?"
 
     action="replacing sudoers file if new package version exists" && display "task" "$action"
-    [ -f /etc/sudoers.pacnew ] && cp /etc/sudoers.pacnew /etc/sudoers >> "$logfile" 2>&1
+    replace_sudoers_pacnew
 
     action="creating a directory to build/install software from git/AUR."
     (mkdir -p "$source_dir") || error_fatal "creating the directory $source_dir" "$?" "check permissions and free space: df -h"
diff --git a/tests/installer-steps/test_replace_sudoers_pacnew.py b/tests/installer-steps/test_replace_sudoers_pacnew.py
new file mode 100644
index 0000000..8bfa914
--- /dev/null
+++ b/tests/installer-steps/test_replace_sudoers_pacnew.py
@@ -0,0 +1,76 @@
+"""Test the guarded sudoers.pacnew replacement.
+
+Bug (Minor, hard consequence): the installer did
+    [ -f /etc/sudoers.pacnew ] && cp /etc/sudoers.pacnew /etc/sudoers
+with no validation. A malformed pacnew replaces /etc/sudoers with a file sudo
+refuses to parse, locking out privilege escalation right before the NOPASSWD
+rule is appended. replace_sudoers_pacnew now runs `visudo -cf` on the pacnew
+first and only copies a file that validates; a failure warns (non-fatal) and
+leaves the working sudoers in place.
+
+Method: sed-extract replace_sudoers_pacnew from the real `archsetup`, pass a
+temp pacnew + temp target (the function's positional-arg testability seam), and
+drive it with a fake visudo (controlled validation result) and a fake
+error_warn recorder.
+
+Run from repo root:
+    python3 -m unittest tests.installer-steps.test_replace_sudoers_pacnew
+"""
+
+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(*, pacnew_exists, visudo_rc):
+    """Run replace_sudoers_pacnew with a temp pacnew/target and fake visudo."""
+    with tempfile.TemporaryDirectory() as d:
+        pacnew = os.path.join(d, "sudoers.pacnew")
+        target = os.path.join(d, "sudoers")
+        with open(target, "w") as f:
+            f.write("ORIGINAL\n")
+        if pacnew_exists:
+            with open(pacnew, "w") as f:
+                f.write("NEW\n")
+        script = textwrap.dedent(f"""\
+            logfile=/dev/null
+            display() {{ :; }}
+            visudo() {{ return {visudo_rc}; }}
+            error_warn() {{ echo "WARN: $1"; return 1; }}
+            source <(sed -n '/^replace_sudoers_pacnew() {{/,/^}}/p' "{ARCHSETUP}")
+            replace_sudoers_pacnew "{pacnew}" "{target}"
+            echo "RC=$?"
+            echo "TARGET=[$(cat "{target}")]"
+        """)
+        return subprocess.run(
+            ["bash", "-c", script], capture_output=True, text=True, timeout=10,
+        )
+
+
+class ReplaceSudoersPacnew(unittest.TestCase):
+    def test_valid_pacnew_is_copied(self):
+        r = run(pacnew_exists=True, visudo_rc=0)
+        self.assertIn("TARGET=[NEW]", r.stdout)
+        self.assertNotIn("WARN", r.stdout)
+
+    def test_invalid_pacnew_warns_and_keeps_original(self):
+        r = run(pacnew_exists=True, visudo_rc=1)
+        self.assertIn("WARN", r.stdout,
+                      "a pacnew that fails visudo must warn, not clobber sudoers")
+        self.assertIn("TARGET=[ORIGINAL]", r.stdout,
+                      "the working sudoers must survive a malformed pacnew")
+
+    def test_no_pacnew_is_a_noop(self):
+        r = run(pacnew_exists=False, visudo_rc=0)
+        self.assertIn("RC=0", r.stdout)
+        self.assertIn("TARGET=[ORIGINAL]", r.stdout)
+        self.assertNotIn("WARN", r.stdout)
+
+
+if __name__ == "__main__":
+    unittest.main()
-- 
cgit v1.2.3