diff options
| -rwxr-xr-x | archsetup | 19 | ||||
| -rw-r--r-- | tests/installer-steps/test_replace_sudoers_pacnew.py | 76 |
2 files changed, 94 insertions, 1 deletions
@@ -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() |