diff --git a/bin/omarchy-sudo-passwordless b/bin/omarchy-sudo-passwordless index 3b0af89de8..58a9b15c61 100755 --- a/bin/omarchy-sudo-passwordless +++ b/bin/omarchy-sudo-passwordless @@ -29,6 +29,7 @@ readonly BOOT_CLEANUP_FILE=/etc/tmpfiles.d/omarchy-nopasswd-sudo.conf readonly PACKAGE_HOOK=/usr/share/libalpm/hooks/05-omarchy-passwordless-revoke.hook readonly REMOVAL_BLOCKER=/run/omarchy-sudo-passwordless-package-removing readonly MIGRATION_MARKER=/var/lib/omarchy/migrations/1788163635 +readonly QUARANTINE_DIR=/var/lib/omarchy/sudoers-quarantine readonly INSTALLED_SELF=/usr/bin/omarchy-sudo-passwordless readonly STATUS_INACTIVE=3 @@ -143,6 +144,24 @@ cleanup_uid_locked() { /usr/bin/rm -f -- "$file" && [[ ! -e $file && ! -L $file ]] } +# The generated prefix is reserved: boot cleanup and the package hook already +# remove everything in it, and the legacy writer could produce a rule whose +# body differs from its filename. Nothing unrecognized may stay live there, but +# its content is kept for the administrator instead of being deleted. +quarantine_foreign_rule() { + local file=$1 target + if [[ ! -e /var/lib/omarchy && ! -L /var/lib/omarchy ]]; then + /usr/bin/install -d -o root -g root -m 0755 -- /var/lib/omarchy || return 1 + fi + omarchy_security_prepare_private_root_directory "$QUARANTINE_DIR" /var/lib/omarchy || return 1 + # A legacy filename can already be close to NAME_MAX, so the destination + # name is fixed and the original name travels beside it. + target=$(/usr/bin/mktemp -d "$QUARANTINE_DIR/XXXXXXXXXX") || return 1 + /usr/bin/printf '%s\n' "${file##*/}" >"$target/name" || return 1 + /usr/bin/mv -fT -- "$file" "$target/policy" && [[ ! -e $file && ! -L $file ]] || return 1 + echo "Moved unrecognized sudoers policy $file to $target/policy" >&2 +} + cleanup_all_locked() { local file classification failed=0 verify_root_path /etc/sudoers.d || return 1 @@ -154,7 +173,9 @@ cleanup_all_locked() { fi else classification=$? - (( classification == 1 )) || failed=1 + if (( classification != 1 )) || ! quarantine_foreign_rule "$file"; then + failed=1 + fi fi done return "$failed" diff --git a/docs/passwordless-sudo.md b/docs/passwordless-sudo.md index 1a846995f8..4aa6e31d58 100644 --- a/docs/passwordless-sudo.md +++ b/docs/passwordless-sudo.md @@ -10,7 +10,7 @@ An internal status result is `0` for an active, validated grant and `3` for conf Calendar timers clean up expired files; their liveness does not define authorization. Callbacks read the current rule and remove it only when expired. Earlier callbacks cannot shorten a renewed grant, so no timer identity needs to be persisted. Old UID-only and token-bearing callbacks remain accepted. Pending callbacks after renewal or manual disable are harmless and expire within the maximum 24-hour grant window. Boot-time tmpfiles cleanup removes the reserved generated filename namespace before users log in; routine non-boot tmpfiles maintenance leaves live grants alone. -Legacy cleanup uses a root-owned machine marker under `/var/lib/omarchy/migrations/`, written only after successful cleanup under the grant lock. Later accounts can finish their migration queues without sudo and without revoking grants created after the repair. Old grant state files are no longer consulted. A legacy grant is recognized by its exact filename and rule relationship, since the old command wrote the caller's unvalidated name into both, so accounts outside the current name policy are still cleaned up; administrator-modified policy under the owned prefix is preserved by the migration. +Legacy cleanup uses a root-owned machine marker under `/var/lib/omarchy/migrations/`, written only after successful cleanup under the grant lock. Later accounts can finish their migration queues without sudo and without revoking grants created after the repair. Old grant state files are no longer consulted. A legacy grant is recognized by its exact filename and rule relationship, since the old command wrote the caller's unvalidated name into both, so accounts outside the current name policy are still cleaned up. The generated filename prefix is reserved: boot cleanup and the package hook already remove everything under it, and the old writer could emit a rule whose body differs from its filename, so the migration moves any other file found there into a fresh root-only directory under `/var/lib/omarchy/sudoers-quarantine/`, as `policy` with the original name stored beside it, rather than leaving it live or deleting its content. ## Package ownership diff --git a/test/shell.d/passwordless-grant-lifecycle-test.sh b/test/shell.d/passwordless-grant-lifecycle-test.sh index 91a0ba895a..71f34b2409 100644 --- a/test/shell.d/passwordless-grant-lifecycle-test.sh +++ b/test/shell.d/passwordless-grant-lifecycle-test.sh @@ -4,6 +4,19 @@ set -euo pipefail source "$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd)/base-test.sh" source "$SHELL_TEST_DIR/fixtures/passwordless-sudo-test.sh" +quarantine="$test_tmp/var/lib/omarchy/sudoers-quarantine" +quarantined_policy() { + local entry + for entry in "$quarantine"/*/; do + if [[ $(cat "$entry/name") == "$1" ]]; then + cat "$entry/policy" + return 0 + fi + done + return 1 +} +# The old writer accepted any $USER, so a basename can sit just under NAME_MAX. +long_suffix=$(printf 'l%.0s' {1..223}) ( source "$library" printf 'deleteduser ALL=(ALL) NOPASSWD: ALL\n' >"$(rule_file 1000)" @@ -11,17 +24,23 @@ source "$SHELL_TEST_DIR/fixtures/passwordless-sudo-test.sh" # The legacy command never validated the account name, so a manual or NSS # account outside the current policy still has its exact old grant removed. printf 'Alice ALL=(ALL) NOPASSWD: ALL\n' >"$test_tmp/etc/sudoers.d/99-omarchy-nopasswd-Alice" + # The legacy writer produced the body with echo. Under BASH_ENV with + # xpg_echo, USER='ali\0143e' yields this filename with an 'alice' rule, so + # a suffix/body mismatch does not prove administrator authorship. + printf 'alice ALL=(ALL) NOPASSWD: ALL\n' >"$test_tmp/etc/sudoers.d/99-omarchy-nopasswd-ali\\0143e" + printf 'alice ALL=(ALL) NOPASSWD: ALL\n' >"$test_tmp/etc/sudoers.d/99-omarchy-nopasswd-$long_suffix" printf 'admin ALL=(ALL) NOPASSWD: /usr/bin/true\n' >"$test_tmp/etc/sudoers.d/99-omarchy-nopasswd-custom" - printf 'Alice ALL=(ALL) NOPASSWD: ALL\n' >"$test_tmp/etc/sudoers.d/99-omarchy-nopasswd-handwritten" TEST_DELETE_FAIL=1 assert_status 1 cleanup_all_locked [[ -e $(rule_file 1000) ]] cleanup_all_locked - [[ ! -e $(rule_file 1000) && ! -e $test_tmp/etc/sudoers.d/99-omarchy-nopasswd-Alice ]] - [[ -e $test_tmp/etc/sudoers.d/99-omarchy-nopasswd-custom && -e $test_tmp/etc/sudoers.d/99-omarchy-nopasswd-handwritten ]] - ! compgen -G "$test_tmp/etc/sudoers.d/99-omarchy-nopasswd-buildbot*" - rm "$test_tmp/etc/sudoers.d/99-omarchy-nopasswd-custom" "$test_tmp/etc/sudoers.d/99-omarchy-nopasswd-handwritten" + ! compgen -G "$test_tmp/etc/sudoers.d/99-omarchy-nopasswd-*" + [[ $(stat -c '%a' "$quarantine") == 700 ]] + [[ $(quarantined_policy '99-omarchy-nopasswd-ali\0143e') == 'alice ALL=(ALL) NOPASSWD: ALL' ]] + [[ $(quarantined_policy "99-omarchy-nopasswd-$long_suffix") == 'alice ALL=(ALL) NOPASSWD: ALL' ]] + [[ $(quarantined_policy 99-omarchy-nopasswd-custom) == 'admin ALL=(ALL) NOPASSWD: /usr/bin/true' ]] + (( $(ls -A "$quarantine" | wc -l) == 3 )) ) -pass "legacy cleanup removes generated orphan rules for any account and preserves custom policy" +pass "legacy cleanup removes generated rules for any account and quarantines everything else in the prefix" # Run the actual migration queue for separate temporary homes. Sudo only calls # the mapped helper and can be refused without requesting host authorization. @@ -36,13 +55,18 @@ run_migrations() { marker="$test_tmp/var/lib/omarchy/migrations/1788163635" ( source "$library" + # A quarantine that cannot be trusted keeps the migration pending. + printf 'alice ALL=(ALL) NOPASSWD: ALL\n' >"$test_tmp/etc/sudoers.d/99-omarchy-nopasswd-mismatch" + TEST_BAD_PATH="$test_tmp/var/lib/omarchy" assert_status 1 run_migrations first + [[ ! -e $marker && -e $test_tmp/etc/sudoers.d/99-omarchy-nopasswd-mismatch ]] printf 'audituser ALL=(ALL) NOPASSWD: ALL\n' >"$(rule_file 1000)" printf 'Alice ALL=(ALL) NOPASSWD: ALL\n' >"$test_tmp/etc/sudoers.d/99-omarchy-nopasswd-Alice" TEST_DELETE_FAIL=1 assert_status 1 run_migrations first [[ ! -e $marker && ! -e $test_tmp/first/1788163636.sh ]] run_migrations first [[ -f $marker && -f $test_tmp/first/1788163636.sh ]] - [[ ! -e $test_tmp/etc/sudoers.d/99-omarchy-nopasswd-Alice ]] + ! compgen -G "$test_tmp/etc/sudoers.d/99-omarchy-nopasswd-*" + [[ $(quarantined_policy 99-omarchy-nopasswd-mismatch) == 'alice ALL=(ALL) NOPASSWD: ALL' ]] enable_locked 1000 15 cp "$(rule_file 1000)" "$test_tmp/renewed" : >"$test_tmp/commands"