From 49969415133d57c1e38f629f3ddec018e20bb875 Mon Sep 17 00:00:00 2001 From: acrogenesis Date: Sat, 29 Aug 2026 20:03:46 -0600 Subject: [PATCH] Address privileged cleanup review findings --- agents/skills/migrations.md | 1 + bin/omarchy-dns | 12 +- bin/omarchy-migrate | 20 ++- migrations/1788025225.sh | 94 +++++++++---- .../arithmetic-left-shift-before-heredoc.sh | 5 + ...plain-heredoc-indented-pseudo-delimiter.sh | 5 + .../route-append-redirect.sh | 3 + .../safe-annotated-reordered-paths.sh | 8 ++ test/shell.d/migrate-scope-test.sh | 36 +++++ test/shell.d/privileged-heredoc-test.sh | 123 ++++++++++++++---- ...ired-installer-artifacts-migration-test.sh | 91 +++++++++++-- 11 files changed, 331 insertions(+), 67 deletions(-) create mode 100644 test/shell.d/fixtures/privileged-heredoc/arithmetic-left-shift-before-heredoc.sh create mode 100644 test/shell.d/fixtures/privileged-heredoc/plain-heredoc-indented-pseudo-delimiter.sh create mode 100644 test/shell.d/fixtures/privileged-heredoc/route-append-redirect.sh create mode 100644 test/shell.d/fixtures/privileged-heredoc/safe-annotated-reordered-paths.sh diff --git a/agents/skills/migrations.md b/agents/skills/migrations.md index 2c97ebde..08e3710e 100644 --- a/agents/skills/migrations.md +++ b/agents/skills/migrations.md @@ -126,6 +126,7 @@ New migration format: - Start with an `echo` describing what the migration does. - Use `$OMARCHY_PATH` to reference the Omarchy directory. - Be idempotent. Check existing state before changing it. +- Exit 75 when a temporary condition must leave the migration pending without blocking later migrations. `omarchy-migrate` continues the queue, does not write that migration's completion marker, and retries it on a later run. Other non-zero statuses still abort the migration run. - Use helper commands such as `omarchy-cmd-present`, `omarchy-cmd-missing`, `omarchy-pkg-add`, `omarchy-pkg-drop`, `omarchy-pkg-present`, and `omarchy-pkg-missing` when appropriate. diff --git a/bin/omarchy-dns b/bin/omarchy-dns index 7d790b78..19c6549e 100755 --- a/bin/omarchy-dns +++ b/bin/omarchy-dns @@ -149,9 +149,9 @@ write_networkmanager_dns() { local servers="$1" install -d -m 0755 "$(dirname "$NM_DNS_CONF")" - # omarchy:heredoc-expands paths=none -- $servers is a space-separated list of - # validated DNS server addresses, not a path; nothing user-writable is baked - # into the root-owned drop-in. + # omarchy:heredoc-expands paths=none -- $servers is a normalized, single-line + # DNS server list written as data, not a path or command; nothing user-writable + # is resolved or executed from the root-owned drop-in. cat >"$NM_DNS_CONF" </dev/null </dev/null 2>&1 || true +if ((${#deferred[@]} == 0)); then + omarchy-notification-dismiss "Omarchy Migrations" >/dev/null 2>&1 || true +fi diff --git a/migrations/1788025225.sh b/migrations/1788025225.sh index daf4abb6..3eeddb0d 100644 --- a/migrations/1788025225.sh +++ b/migrations/1788025225.sh @@ -2,6 +2,10 @@ echo "Remove privileged files left behind by retired Omarchy installers" sudoers_dir="${OMARCHY_SUDOERS_DIR:-/etc/sudoers.d}" systemd_dir="${OMARCHY_SYSTEMD_SYSTEM_DIR:-/etc/systemd/system}" +machine_marker="${OMARCHY_RETIRED_INSTALLER_ARTIFACTS_MARKER:-/var/lib/omarchy/migrations/1788025225}" +reload_needed_marker="$machine_marker.daemon-reload" + +[[ ! -e $machine_marker ]] || exit 0 as_root() { if (( EUID == 0 )); then @@ -226,9 +230,10 @@ plymouth_unit_runs_from_home() { done if [[ $word =~ $home_pattern || $word == "$HOME/.local/share/omarchy/bin/$binary" ]]; then + # Non-empty ExecStop= assignments append to the command list. Once a + # vulnerable command is present it stays live until an empty assignment + # explicitly resets the list; a later packaged command does not replace it. matched=0 - else - matched=1 fi done < <(active_lines systemd) @@ -242,39 +247,82 @@ plymouth_unit_runs_from_home() { 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 +defer_privileged_repair() { + echo "Cannot complete the privileged installer-artifact repair; omarchy-migrate will retry it later." >&2 + exit 75 +} + +# This is a machine-wide repair with per-user migration markers. A root-owned, +# readable marker lets later non-sudo users finish their own migration run after +# one privileged account has inspected and repaired the machine. Until then, +# exit 75 asks omarchy-migrate to leave this migration pending while continuing +# with every later migration instead of wedging the whole queue. +if ! as_root true 2>/dev/null; then + defer_privileged_repair 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 - as_root rm -f "$first_run_sudoers" +# Removing a unit and reloading systemd are one repair. Persist the second half +# before removing the file so a failed daemon-reload cannot be forgotten on a +# retry that now sees no unit on disk. +if [[ -e $reload_needed_marker ]]; then + if ! as_root systemctl daemon-reload >/dev/null 2>&1; then + defer_privileged_repair fi - - if as_root test -f "$tsui_sudoers" && - as_root cat "$tsui_sudoers" | tsui_sudoers_is_generated; then - as_root rm -f "$tsui_sudoers" + if ! as_root rm -f "$reload_needed_marker"; then + defer_privileged_repair fi fi +inspect_sudoers_file() { + local file="$1" predicate="$2" kind content + + # Emit an explicit state from the elevated process. A bare `sudo test -f` in + # an if-condition makes "file missing" indistinguishable from "sudo failed", + # which could mark a live grant repaired without ever reading it. + if ! kind=$(as_root bash -c 'if [[ -f $1 ]]; then printf file; elif [[ -e $1 ]]; then printf other; else printf missing; fi' bash "$file"); then + defer_privileged_repair + fi + + [[ $kind == "file" ]] || return 0 + if ! content=$(as_root cat "$file"); then + defer_privileged_repair + fi + + if "$predicate" <<<"$content"; then + if ! as_root rm -f "$file"; then + defer_privileged_repair + fi + fi +} + +inspect_sudoers_file "$first_run_sudoers" first_run_sudoers_is_generated +inspect_sudoers_file "$tsui_sudoers" tsui_sudoers_is_generated + # /etc/systemd/system is 0755, so this one needs no elevation to look at. plymouth_unit="$systemd_dir/omarchy-plymouth-shutdown.service" if [[ -f $plymouth_unit ]] && plymouth_unit_runs_from_home <"$plymouth_unit"; then # Disable, never stop. Stopping the unit is precisely what runs ExecStop, and # ExecStop is the path this migration exists to keep root away from; disabling # only drops the multi-user.target symlink. - as_root systemctl disable omarchy-plymouth-shutdown.service >/dev/null 2>&1 || true - as_root rm -f "$plymouth_unit" + if ! as_root install -Dm644 /dev/null "$reload_needed_marker"; then + defer_privileged_repair + fi + if ! as_root systemctl disable omarchy-plymouth-shutdown.service >/dev/null 2>&1; then + defer_privileged_repair + fi + if ! as_root rm -f "$plymouth_unit"; then + defer_privileged_repair + fi # systemd keeps serving the copy it already loaded until it rereads the # directory, so without this the unit is still there to run at shutdown. - as_root systemctl daemon-reload >/dev/null 2>&1 || true + if ! as_root systemctl daemon-reload >/dev/null 2>&1; then + defer_privileged_repair + fi + if ! as_root rm -f "$reload_needed_marker"; then + defer_privileged_repair + fi +fi + +if ! as_root install -Dm644 /dev/null "$machine_marker"; then + defer_privileged_repair fi diff --git a/test/shell.d/fixtures/privileged-heredoc/arithmetic-left-shift-before-heredoc.sh b/test/shell.d/fixtures/privileged-heredoc/arithmetic-left-shift-before-heredoc.sh new file mode 100644 index 00000000..d6bf7d1d --- /dev/null +++ b/test/shell.d/fixtures/privileged-heredoc/arithmetic-left-shift-before-heredoc.sh @@ -0,0 +1,5 @@ +mask=$((1 << bits)) + +cat >/etc/omarchy/agent.conf </etc/omarchy/agent.conf <>/etc/omarchy/agent.conf </etc/omarchy/mounts.conf <"$deferred_root/migrations/100-deferred.sh" <<'SH' +echo deferred >>"$TEST_CALLS" +exit 75 +SH +cat >"$deferred_root/migrations/200-after.sh" <<'SH' +echo after >>"$TEST_CALLS" +SH + +HOME="$deferred_home" \ +OMARCHY_PATH="$deferred_root" \ +TEST_CALLS="$deferred_calls" \ + "$ROOT/bin/omarchy-migrate" >"$test_tmp/deferred.out" + +grep -q '^deferred$' "$deferred_calls" || fail "migration runner starts a deferred migration" +grep -q '^after$' "$deferred_calls" || fail "migration runner continues after a deferred migration" +[[ ! -f $deferred_home/.local/state/omarchy/migrations/100-deferred.sh ]] || + fail "migration runner leaves a deferred migration pending" +[[ -f $deferred_home/.local/state/omarchy/migrations/200-after.sh ]] || + fail "migration runner records a later successful migration" +grep -q 'was deferred and will be retried later' "$test_tmp/deferred.out" || + fail "migration runner reports a deferred migration" +pass "migration runner leaves exit-75 migrations pending and continues the queue" + +HOME="$deferred_home" OMARCHY_PATH="$deferred_root" \ + "$ROOT/bin/omarchy-migrate" --pending >"$test_tmp/deferred-pending.out" +grep -q '^100-deferred\.sh$' "$test_tmp/deferred-pending.out" || + fail "migration runner still reports a deferred migration as pending" +! grep -q '^200-after\.sh$' "$test_tmp/deferred-pending.out" || + fail "migration runner does not report the completed later migration as pending" +pass "migration runner reports only the deferred migration as pending" diff --git a/test/shell.d/privileged-heredoc-test.sh b/test/shell.d/privileged-heredoc-test.sh index ce274260..f02be8c1 100755 --- a/test/shell.d/privileged-heredoc-test.sh +++ b/test/shell.d/privileged-heredoc-test.sh @@ -299,20 +299,18 @@ mentions_user_writable_root() { # while the $HOME token stands alone. classify_expansion() { local masked="$1" name="$2" head literal piece - local path_shaped=1 token_has_slash=1 + local path_shape="" head=$(literal_head "$masked") head=${head%%$'\001'*} literal=$(literal_value "$name") - [[ $masked == */* ]] && token_has_slash=0 + [[ $masked == */* ]] && path_shape+="token " + in_list "$name" "${USER_WRITABLE_VARS[@]}" && path_shape+="user-root " + [[ -v VARS_TAINTED[$name] ]] && path_shape+="tainted " + [[ -n $literal && $literal == */* ]] && path_shape+="literal " - ((token_has_slash == 0)) && path_shaped=0 - in_list "$name" "${USER_WRITABLE_VARS[@]}" && path_shaped=0 - [[ -v VARS_TAINTED[$name] ]] && path_shaped=0 - [[ -n $literal && $literal == */* ]] && path_shaped=0 - - ((path_shaped == 0)) || return 1 + [[ -n $path_shape ]] || return 1 # A path expansion anchored under a root-owned prefix cannot introduce a # user-writable location, so it does not need declaring. @@ -334,8 +332,7 @@ classify_expansion() { # A name assigned a user root anywhere in the file is never rescued: the # assignment that won may be the packaged path it was later reassigned away # from, and the rescue would then clear it on evidence it no longer holds. - if ((token_has_slash != 0)) && ! in_list "$name" "${USER_WRITABLE_VARS[@]}" && - ! [[ -v VARS_TAINTED[$name] ]] && [[ -n $literal ]]; then + if [[ $path_shape == "literal " ]]; then for piece in $literal; do piece=$(literal_head "$piece") if mentions_user_writable_root "$piece"; then @@ -361,8 +358,12 @@ command_destinations() { # contain spaces anywhere this check runs. line=${line//\"/ } line=${line//\'/ } - # Detach redirects from their targets so "> /etc/x" and ">/etc/x" agree. + # Preserve append redirects before detaching redirect operators from their + # targets, so ">> /etc/x" does not become two ">" tokens whose first target + # is the second operator. + line=${line//>>/$'\003'} line=${line//>/ > } + line=${line//$'\003'/" >> "} read -r -a tokens <<<"$line" @@ -373,7 +374,7 @@ command_destinations() { in_list "$token" "${ELEVATORS[@]}" && elevated=0 - if [[ $token == ">" ]]; then + if [[ $token == ">" || $token == ">>" ]]; then target=${tokens[index]:-} index=$((index + 1)) [[ -n $target && $target != "&"* && $target != /dev/* ]] && printf '%s\n' "$target" @@ -386,7 +387,7 @@ command_destinations() { fi if in_list "$token" "${WRITE_COMMANDS[@]}"; then - if [[ $token == "tee" || $token == "dd" ]]; then + if [[ $token == "tee" ]]; then # Every non-flag argument to tee is a destination. scan=$index while ((scan < ${#tokens[@]})); do @@ -397,7 +398,8 @@ command_destinations() { [[ $target == /dev/* ]] && continue printf '%s\n' "$target" done - else + elif [[ $token != "dd" ]]; then + # dd destinations are expressed only by of= operands, handled above. copy_like=0 fi continue @@ -485,13 +487,45 @@ count_placeholders() { printf '%s' "$count" } +normalize_path_set() { + local value="$1" + local -a names=() + + if [[ $value == "none" ]]; then + printf 'none' + return 0 + fi + + IFS=, read -ra names <<<"$value" + mapfile -t names < <(printf '%s\n' "${names[@]}" | sort -u) + ( + IFS=, + printf '%s' "${names[*]}" + ) +} + +inside_same_line_arithmetic() { + local prefix="$1" opens=0 closes=0 + + while [[ $prefix == *"(("* ]]; do + opens=$((opens + 1)) + prefix=${prefix#*"(("} + done + while [[ $prefix == *"))"* ]]; do + closes=$((closes + 1)) + prefix=${prefix#*"))"} + done + + ((opens > closes)) +} + scan_file() { local file="$1" display="${2:-$1}" local -a lines=() - local index lineno line scan rest raw guard slot delim candidate + local index lineno line scan rest raw operator match prefix guard slot delim candidate candidate_delim body_start local body_text unescaped destination body_line masked_line token name - local declared_paths annotation look shown_paths shown_plain count next slots - local hd_re='<<-?[[:space:]]*("[A-Za-z_][A-Za-z0-9_]*"|'"'"'[A-Za-z_][A-Za-z0-9_]*'"'"'|[A-Za-z_][A-Za-z0-9_]*)' + local declared_paths annotation look shown_paths shown_plain count next slots terminated + local hd_re='(<<-?)[[:space:]]*("[A-Za-z_][A-Za-z0-9_]*"|'"'"'[A-Za-z_][A-Za-z0-9_]*'"'"'|[A-Za-z_][A-Za-z0-9_]*)' mapfile -t lines <"$file" collect_vars lines @@ -512,12 +546,21 @@ scan_file() { # Collect this line's heredoc delimiters in order. Quoted ones are safe by # construction but still have to be tracked, or their bodies would be # parsed as code. - local -a delims=() quoted=() + local -a delims=() quoted=() strip_tabs=() rest=$scan guard=0 while ((guard++ < 8)) && [[ $rest =~ $hd_re ]]; do - raw=${BASH_REMATCH[1]} - rest=${rest#*"${BASH_REMATCH[0]}"} + match=${BASH_REMATCH[0]} + operator=${BASH_REMATCH[1]} + raw=${BASH_REMATCH[2]} + prefix=${rest%%"$match"*} + rest=${rest#*"$match"} + + # `(( value << shift ))` and `$(( value << shift ))` are arithmetic, not + # heredocs. Without this guard the shift count becomes a phantom delimiter + # and can consume every real heredoc below it. + inside_same_line_arithmetic "$prefix" && continue + if [[ $raw == \"*\" || $raw == \'*\' ]]; then delims+=("${raw:1:${#raw}-2}") quoted+=(0) @@ -525,6 +568,7 @@ scan_file() { delims+=("$raw") quoted+=(1) fi + [[ $operator == "<<-" ]] && strip_tabs+=(1) || strip_tabs+=(0) done ((${#delims[@]} > 0)) || continue @@ -532,14 +576,34 @@ scan_file() { for slot in "${!delims[@]}"; do delim=${delims[slot]} local -a body=() + body_start=$index + terminated=1 while ((index < ${#lines[@]})); do candidate=${lines[index]} index=$((index + 1)) - [[ ${candidate#"${candidate%%[![:space:]]*}"} == "$delim" ]] && break + candidate_delim=$candidate + if ((strip_tabs[slot] == 1)); then + while [[ $candidate_delim == $'\t'* ]]; do + candidate_delim=${candidate_delim#$'\t'} + done + fi + if [[ $candidate_delim == "$delim" ]]; then + terminated=0 + break + fi body+=("$candidate") done + # A valid shell source cannot contain an unterminated heredoc. If this + # candidate has no terminator it was syntax such as a multi-line arithmetic + # shift that the lightweight matcher could not classify; resume scanning + # below it instead of swallowing the rest of the file. + if ((terminated != 0)); then + index=$body_start + continue + fi + # A quoted delimiter cannot expand anything. ((quoted[slot] == 1)) || continue @@ -621,7 +685,7 @@ scan_file() { continue fi - if [[ $declared_paths != "$shown_paths" ]]; then + if [[ $(normalize_path_set "$declared_paths") != $(normalize_path_set "$shown_paths") ]]; then FINDINGS+=("$display:$lineno: heredoc annotation declares paths=$declared_paths but the path-shaped expansions are $shown_paths Writing to: $destination Every expansion used as a path outside a root-owned prefix has to be named, @@ -761,6 +825,19 @@ fixture_flags route-variable-path.sh \ fixture_flags route-install-hop.sh \ "flags a scratch file that install(1) later copies into /usr" fixture_flags route-dash-delimiter.sh "flags an indented <<- heredoc" +fixture_flags route-append-redirect.sh "flags an append redirect into /etc" +fixture_flags arithmetic-left-shift-before-heredoc.sh \ + "an arithmetic left shift does not swallow a later privileged heredoc" \ + "path-shaped expansions: HOME" +fixture_flags plain-heredoc-indented-pseudo-delimiter.sh \ + "an indented delimiter does not terminate a plain heredoc" \ + "path-shaped expansions: HOME" + +mapfile -t dd_destinations < <(command_destinations \ + 'sudo dd if=/tmp/input bs=4M status=none of=/etc/omarchy/image') +[[ ${dd_destinations[0]:-} == "/etc/omarchy/image" && ${dd_destinations[1]:-} == $'\002elevated' && ${#dd_destinations[@]} == 2 ]] || + fail "dd emits only its of= destination" "$(printf '%q\n' "${dd_destinations[@]:-}")" +pass "dd emits only its of= destination" # Negatives. fixture_passes safe-quoted-delimiter.sh "a quoted delimiter passes" @@ -771,6 +848,8 @@ fixture_passes safe-no-expansion.sh \ fixture_passes safe-runtime-expansion.sh \ "an escaped \\\$VAR left for a root daemon to expand passes" fixture_passes safe-annotated.sh "a declared, reasoned exemption passes" +fixture_passes safe-annotated-reordered-paths.sh \ + "path declarations compare as sets rather than traversal order" fixture_passes safe-root-anchored.sh \ "a path expansion anchored under /etc is truthfully declared paths=none" fixture_passes safe-herestring.sh "a herestring is not mistaken for a heredoc" diff --git a/test/shell.d/retired-installer-artifacts-migration-test.sh b/test/shell.d/retired-installer-artifacts-migration-test.sh index da6a3663..83f42881 100755 --- a/test/shell.d/retired-installer-artifacts-migration-test.sh +++ b/test/shell.d/retired-installer-artifacts-migration-test.sh @@ -25,6 +25,10 @@ cat >"$test_dir/bin/systemctl" <<'STUB' #!/bin/bash printf 'systemctl %s\n' "$*" >>"$CALLS" +if [[ $* == "daemon-reload" && -n ${FAIL_DAEMON_RELOAD_ONCE_MARKER:-} && ! -e $FAIL_DAEMON_RELOAD_ONCE_MARKER ]]; then + touch "$FAIL_DAEMON_RELOAD_ONCE_MARKER" + exit 1 +fi STUB chmod +x "$test_dir/bin/"* @@ -49,9 +53,11 @@ home_dir="$test_dir/home" first_run="$sudoers_dir/first-run" tsui="$sudoers_dir/tsui" plymouth_unit="$systemd_dir/omarchy-plymouth-shutdown.service" +machine_marker="$test_dir/machine-marker" +reload_needed_marker="$machine_marker.daemon-reload" reset_machine() { - rm -rf "$sudoers_dir" "$systemd_dir" "$home_dir" + rm -rf "$sudoers_dir" "$systemd_dir" "$home_dir" "$machine_marker" "$reload_needed_marker" mkdir -p "$sudoers_dir" "$systemd_dir" "$home_dir" } @@ -61,6 +67,7 @@ run_migration() { HOME="$home_dir" \ OMARCHY_SUDOERS_DIR="$sudoers_dir" \ OMARCHY_SYSTEMD_SYSTEM_DIR="$systemd_dir" \ + OMARCHY_RETIRED_INSTALLER_ARTIFACTS_MARKER="$machine_marker" \ PATH="$test_dir/bin:$PATH" \ bash -euo pipefail "$migration" >/dev/null } @@ -83,7 +90,7 @@ assert_changed_nothing() { assert_read_elevated() { local file="$1" label="$2" - grep -qF "sudo test -f $file" "$CALLS" || + grep -qF "bash $file" "$CALLS" || fail "$label" "$(cat "$CALLS")" grep -qF "sudo cat $file" "$CALLS" || fail "$label" "$(cat "$CALLS")" @@ -174,6 +181,9 @@ for body in "${first_run_variants[@]}"; do done pass "migration removes every first-run sudoers grant the installer ever wrote" +[[ -f $machine_marker ]] || fail "migration records the machine-wide repair" +pass "migration records the machine-wide repair" + grep -q '^sudo rm -f .*/sudoers\.d/first-run$' "$CALLS" || fail "migration removes the first-run grant with elevated privileges" "$(cat "$CALLS")" pass "migration removes the first-run grant with elevated privileges" @@ -322,6 +332,22 @@ run_migration [[ -e $plymouth_unit ]] || fail "migration keeps a unit whose home ExecStop is commented out" pass "migration keeps a unit whose home ExecStop is commented out" +# Non-empty ExecStop= assignments append; a packaged command after the retired +# home command does not replace it, so the vulnerable command remains live. +reset_machine +cat >"$plymouth_unit" <<'EOF' +[Service] +Type=oneshot +ExecStart=/usr/bin/true +ExecStop=/home/installer/.local/share/omarchy/bin/omarchy-plymouth-shutdown-sync +ExecStop=/usr/bin/true +EOF +run_migration + +[[ ! -e $plymouth_unit ]] || + fail "migration removes a home ExecStop followed by another command" "$(cat "$plymouth_unit")" +pass "migration removes a home ExecStop followed by another command" + reset_machine run_migration @@ -517,30 +543,71 @@ run_migration fail "migration removes a unit whose last line ends mid-continuation" pass "migration removes a unit whose last line ends mid-continuation" +# If removing the unit succeeds but daemon-reload fails, the loaded unit still +# needs to be forgotten. Persist that half of the repair so the retry reloads +# systemd even though the unit file is already gone. +reset_machine +write_plymouth_unit "/home/installer/.local/share/omarchy/bin/omarchy-plymouth-shutdown-sync" +reload_failure_seen="$test_dir/reload-failure-seen" +rm -f "$reload_failure_seen" + +set +e +FAIL_DAEMON_RELOAD_ONCE_MARKER="$reload_failure_seen" run_migration +reload_status=$? +set -e + +(( reload_status == 75 )) || + fail "migration defers after a failed daemon-reload" "status=$reload_status" +[[ ! -e $plymouth_unit && -e $reload_needed_marker && ! -e $machine_marker ]] || + fail "migration records the pending reload without marking the repair complete" + +run_migration +[[ ! -e $reload_needed_marker && -e $machine_marker ]] || + fail "migration completes a pending daemon-reload on retry" +grep -q '^systemctl daemon-reload$' "$CALLS" || + fail "migration retries daemon-reload after the unit file is gone" "$(cat "$CALLS")" +pass "migration retries daemon-reload after the unit file is gone" + # 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" +readable="$test_dir/readable-sudoers" +rm -rf "$readable" +mkdir -p "$readable" +chmod 755 "$readable" +printf '%s\n' "${first_run_variants[-1]}" >"$readable/first-run" : >"$CALLS" set +e HOME="$home_dir" \ - OMARCHY_SUDOERS_DIR="$unreadable" \ + OMARCHY_SUDOERS_DIR="$readable" \ OMARCHY_SYSTEMD_SYSTEM_DIR="$systemd_dir" \ + OMARCHY_RETIRED_INSTALLER_ARTIFACTS_MARKER="$machine_marker" \ 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" || +(( gate_status == 75 )) || + fail "migration defers when it cannot elevate to inspect the sudoers directory" "status=$gate_status$(printf '\n%s' "$(cat "$test_dir/gate.out")")" +[[ -e $readable/first-run ]] || + fail "migration keeps a live grant when elevation fails" +[[ ! -e $machine_marker ]] || + fail "migration leaves the machine repair unmarked when elevation fails" +grep -q 'will retry it later' "$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" +pass "migration defers without marking a readable sudoers directory repaired when elevation fails" + +# After one privileged account completes the machine repair, a non-sudo user +# can finish their per-user migration without probing sudo again. +touch "$machine_marker" +HOME="$home_dir" \ + OMARCHY_SUDOERS_DIR="$readable" \ + OMARCHY_SYSTEMD_SYSTEM_DIR="$systemd_dir" \ + OMARCHY_RETIRED_INSTALLER_ARTIFACTS_MARKER="$machine_marker" \ + PATH="$test_dir/failing-bin:$PATH" \ + bash -euo pipefail "$migration" >/dev/null +pass "machine marker lets a non-sudo user complete after the repair"