From 80e7c25b3761323bffe03404ed88a0b25ab231ef Mon Sep 17 00:00:00 2001 From: acrogenesis Date: Sat, 29 Aug 2026 15:50:33 -0600 Subject: [PATCH] Fail the sudoers cleanup when it cannot elevate to look Running the migration on a real machine with no cached sudo credentials printed sudo's "a terminal is required to read the password" and still exited 0. bin/omarchy-migrate writes the completion marker on a zero exit, so the cleanup would have been recorded as done on every install that runs migrations without a terminal, and never tried again. Probe for elevation before the combined existence check and exit non-zero when it fails, so the marker stays unwritten and the next run retries. The probe is skipped when the directory is readable as-is, which is the case when migrations run as root. --- migrations/1788025225.sh | 14 ++++++- ...ired-installer-artifacts-migration-test.sh | 40 +++++++++++++++++++ 2 files changed, 52 insertions(+), 2 deletions(-) diff --git a/migrations/1788025225.sh b/migrations/1788025225.sh index b131d046..daf4abb6 100644 --- a/migrations/1788025225.sh +++ b/migrations/1788025225.sh @@ -239,11 +239,21 @@ plymouth_unit_runs_from_home() { # logged-in user, so an unelevated [[ -f ]] on a file in there is false whether or # not the file exists and an unelevated read returns nothing. Both tests and both # reads have to be elevated or this migration reports success having done nothing. -# One combined probe first, so the common case of neither file being present costs -# a single sudo call rather than one per file. first_run_sudoers="$sudoers_dir/first-run" tsui_sudoers="$sudoers_dir/tsui" +# Sudo cannot prompt without a terminal, and omarchy-migrate runs from places that +# have none. Failing the elevation probe there is indistinguishable from finding +# no files, and since bin/omarchy-migrate writes the completion marker on a zero +# exit, a silent skip would mark this migration done forever. Exit non-zero +# instead so the marker stays unwritten and the next run tries again. +if [[ ! -r $sudoers_dir ]] && ! as_root true 2>/dev/null; then + echo "Cannot inspect $sudoers_dir without elevation; leaving it for the next run." >&2 + exit 1 +fi + +# One combined probe, so the common case of neither file being present costs a +# single elevated call rather than one per file. if as_root test -e "$first_run_sudoers" -o -e "$tsui_sudoers"; then if as_root test -f "$first_run_sudoers" && as_root cat "$first_run_sudoers" | first_run_sudoers_is_generated; then diff --git a/test/shell.d/retired-installer-artifacts-migration-test.sh b/test/shell.d/retired-installer-artifacts-migration-test.sh index d89ab09c..da6a3663 100755 --- a/test/shell.d/retired-installer-artifacts-migration-test.sh +++ b/test/shell.d/retired-installer-artifacts-migration-test.sh @@ -29,6 +29,18 @@ STUB chmod +x "$test_dir/bin/"* +# A second stub directory where sudo cannot elevate, standing in for a run with +# no terminal to read a password from. +mkdir -p "$test_dir/failing-bin" +cat >"$test_dir/failing-bin/sudo" <<'STUB' +#!/bin/bash + +echo "sudo: a terminal is required to read the password" >&2 +exit 1 +STUB +cp "$test_dir/bin/systemctl" "$test_dir/failing-bin/systemctl" +chmod +x "$test_dir/failing-bin/"* + export CALLS="$test_dir/calls" sudoers_dir="$test_dir/sudoers.d" @@ -504,3 +516,31 @@ run_migration [[ ! -e $plymouth_unit ]] || fail "migration removes a unit whose last line ends mid-continuation" pass "migration removes a unit whose last line ends mid-continuation" + +# sudo cannot prompt without a terminal, and omarchy-migrate runs from places that +# have none. bin/omarchy-migrate writes the completion marker on a zero exit, so +# reporting success after failing to look would mark this migration done for good. +# Observed on a real machine before this guard existed: the run printed sudo's +# "a terminal is required" and still exited 0. +reset_machine +unreadable="$test_dir/unreadable-sudoers" +rm -rf "$unreadable" +mkdir -p "$unreadable" +chmod 000 "$unreadable" + +: >"$CALLS" +set +e +HOME="$home_dir" \ + OMARCHY_SUDOERS_DIR="$unreadable" \ + OMARCHY_SYSTEMD_SYSTEM_DIR="$systemd_dir" \ + PATH="$test_dir/failing-bin:$PATH" \ + bash -euo pipefail "$migration" >"$test_dir/gate.out" 2>&1 +gate_status=$? +set -e +chmod 755 "$unreadable" + +(( gate_status != 0 )) || + fail "migration fails when it cannot elevate to inspect the sudoers directory" "$(cat "$test_dir/gate.out")" +grep -q 'without elevation' "$test_dir/gate.out" || + fail "migration says why it could not inspect the directory" "$(cat "$test_dir/gate.out")" +pass "migration fails when it cannot elevate to inspect the sudoers directory"