aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorCraig Jennings <c@cjennings.net>2026-07-20 15:44:29 -0500
committerCraig Jennings <c@cjennings.net>2026-07-20 15:44:29 -0500
commitc80e85545be3b1ab98b69771f502b7b36102c7f6 (patch)
tree53da27e3db108accbba0a7cae760647ed5b7549c
parent577088700e78afc9ee1d5cad352768cdf108ffe8 (diff)
downloadarchsetup-c80e85545be3b1ab98b69771f502b7b36102c7f6.tar.gz
archsetup-c80e85545be3b1ab98b69771f502b7b36102c7f6.zip
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.
-rwxr-xr-xarchsetup19
-rw-r--r--tests/installer-steps/test_replace_sudoers_pacnew.py76
2 files changed, 94 insertions, 1 deletions
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()