diff --git a/agents/skills/migrations.md b/agents/skills/migrations.md index 08e3710e..4cae6668 100644 --- a/agents/skills/migrations.md +++ b/agents/skills/migrations.md @@ -126,7 +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. +- When a temporary condition must leave the migration pending without blocking later migrations, write `$OMARCHY_MIGRATION_DEFER_TOKEN` to `$OMARCHY_MIGRATION_DEFER_FILE`, then exit 75. `omarchy-migrate` requires both signals before it continues the queue without writing that migration's completion marker. An unmarked exit 75 is an ordinary failure and aborts the run, so a child command returning `EX_TEMPFAIL` cannot accidentally defer a migration forever. - 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-migrate b/bin/omarchy-migrate index 85f694b5..59791789 100755 --- a/bin/omarchy-migrate +++ b/bin/omarchy-migrate @@ -91,18 +91,28 @@ while IFS=$'\t' read -r name file marker; do if [[ ! -f $marker ]]; then echo -e "\e[32m\nRunning migration (${name%.sh})\e[0m" - if OMARCHY_PATH="$OMARCHY_PATH" bash -euo pipefail "$file"; then + defer_file=$(mktemp "$STATE_DIR/.defer.XXXXXX") + defer_token="$BASHPID-$RANDOM-$RANDOM" + migration_status=0 + + if OMARCHY_PATH="$OMARCHY_PATH" \ + OMARCHY_MIGRATION_DEFER_FILE="$defer_file" \ + OMARCHY_MIGRATION_DEFER_TOKEN="$defer_token" \ + bash -euo pipefail "$file"; then mkdir -p "$(dirname "$marker")" touch "$marker" else migration_status=$? - if (( migration_status == 75 )); then + if (( migration_status == 75 )) && [[ $(<"$defer_file") == "$defer_token" ]]; then deferred+=("$name") echo "Migration ${name%.sh} was deferred and will be retried later." else + rm -f "$defer_file" exit "$migration_status" fi fi + + rm -f "$defer_file" fi done < <(migration_entries) diff --git a/bin/omarchy-provision-owner b/bin/omarchy-provision-owner index fc0f219e..d300d1de 100755 --- a/bin/omarchy-provision-owner +++ b/bin/omarchy-provision-owner @@ -788,7 +788,7 @@ configure_login() { # After=) is what makes it deterministic — no sleep/race against SDDM's startup. install_autologin_once_cleanup() { local unit=omarchy-provision-autologin-once.service - cat >"/etc/systemd/system/$unit" <<'UNIT' + sed "s|@UNIT@|$unit|g" >"/etc/systemd/system/$unit" <<'UNIT' [Unit] Description=Drop the first-boot autologin before the next login Before=display-manager.service @@ -797,7 +797,7 @@ ConditionPathExists=/etc/sddm.conf.d/autologin.conf [Service] Type=oneshot ExecStart=/usr/bin/rm -f /etc/sddm.conf.d/autologin.conf -ExecStartPost=/usr/bin/rm -f /etc/systemd/system/graphical.target.wants/omarchy-provision-autologin-once.service /etc/systemd/system/omarchy-provision-autologin-once.service +ExecStartPost=/usr/bin/rm -f /etc/systemd/system/graphical.target.wants/@UNIT@ /etc/systemd/system/@UNIT@ [Install] WantedBy=graphical.target diff --git a/migrations/1787946619.sh b/migrations/1787946619.sh index 4344503a..be89acfe 100644 --- a/migrations/1787946619.sh +++ b/migrations/1787946619.sh @@ -10,9 +10,17 @@ as_root() { fi } +defer_privileged_repair() { + echo "Cannot remove the legacy privileged udev rule; omarchy-migrate will retry it later." >&2 + if [[ -n ${OMARCHY_MIGRATION_DEFER_FILE:-} && -n ${OMARCHY_MIGRATION_DEFER_TOKEN:-} ]]; then + printf '%s\n' "$OMARCHY_MIGRATION_DEFER_TOKEN" >"$OMARCHY_MIGRATION_DEFER_FILE" + fi + exit 75 +} + # Omarchy 3 generated these two rules with an unquoted heredoc, so the installing -# user's $HOME was expanded and the file on disk names -# /home//.local/share/omarchy/bin/. udev runs RUN+= as root, and +# user's $HOME was expanded and the file on disk names that absolute home path. +# udev runs RUN+= as root, and # ~/.local/share/omarchy is a symlink that same unprivileged user owns: replacing # it with a tree of their own and provoking a power_supply event runs their code # as root. Quattro ships the rules as 99-omarchy-*.rules under /usr/bin, but the @@ -32,7 +40,7 @@ as_root() { # path in a comment, and so does a legacy file already repointed at /usr/bin. rule_runs_from_home() { local file="$1" binary="$2" - local pattern="^(/home/[^/]+|/root)/\\.local/share/omarchy/bin/$binary\$" + local pattern="^/.+/\\.local/share/omarchy/bin/$binary\$" local line logical="" rest command word local -a words @@ -68,7 +76,7 @@ rule_runs_from_home() { # argument. Compare whole words so no substring stands in for the path. read -ra words <<<"$command" for word in "${words[@]}"; do - if [[ $word =~ $pattern || $word == "$HOME/.local/share/omarchy/bin/$binary" ]]; then + if [[ $word =~ $pattern ]]; then return 0 fi done @@ -84,7 +92,9 @@ for legacy_rule in "99-power-profile.rules:omarchy-powerprofiles-set" "99-wifi-p rule_file="$rules_dir/${legacy_rule%%:*}" if [[ -f $rule_file ]] && rule_runs_from_home "$rule_file" "${legacy_rule##*:}"; then - as_root rm -f "$rule_file" + if ! as_root rm -f "$rule_file"; then + defer_privileged_repair + fi removed=1 fi done diff --git a/migrations/1788025225.sh b/migrations/1788025225.sh index 3eeddb0d..d7dbb785 100644 --- a/migrations/1788025225.sh +++ b/migrations/1788025225.sh @@ -115,10 +115,10 @@ sudoers_hash_is_active() { # one line that is unmistakably this grant: its own self-cleanup. One # hand-written line anywhere in the file and it is not ours to delete. first_run_sudoers_is_generated() { - local spec_pattern='^[^[:space:]]+ ALL=\(ALL\) NOPASSWD: (.+)$' + local spec_pattern='^([^[:space:]]+) ALL=\(ALL\) NOPASSWD: (.+)$' local marker_pattern='^/bin/rm -f /home/[^/]+/\.local/state/omarchy/first-run\.mode$' - local line command - local seen_any=0 seen_marker=0 + local line user command generated_user="" + local seen_any=0 seen_marker=0 seen_spec=0 while IFS= read -r line; do seen_any=1 @@ -140,7 +140,13 @@ first_run_sudoers_is_generated() { if [[ ! $line =~ $spec_pattern ]]; then return 1 fi - command=${BASH_REMATCH[1]} + user=${BASH_REMATCH[1]} + command=${BASH_REMATCH[2]} + if [[ -n $generated_user && $user != "$generated_user" ]]; then + return 1 + fi + generated_user=$user + seen_spec=1 case "$command" in "/usr/bin/systemctl" | "/usr/bin/ufw" | "/usr/bin/ufw-docker" | \ @@ -162,7 +168,7 @@ first_run_sudoers_is_generated() { return 1 done < <(active_lines sudoers) - (( seen_any && seen_marker )) + (( seen_any && seen_marker && seen_spec )) } # bin/omarchy-install-tailscale (2025-08-22 to 2026-02-02) ran @@ -202,7 +208,7 @@ tsui_sudoers_is_generated() { plymouth_unit_runs_from_home() { local binary="omarchy-plymouth-shutdown-sync" local exec_stop_pattern='^ExecStop[[:space:]]*=[[:space:]]*(.*)$' - local home_pattern="^(/home/[^/]+|/root)/\\.local/share/omarchy/bin/$binary\$" + local home_pattern="^/.+/\\.local/share/omarchy/bin/$binary\$" local line word local -a words local matched=1 @@ -229,7 +235,7 @@ plymouth_unit_runs_from_home() { word=${word:1} done - if [[ $word =~ $home_pattern || $word == "$HOME/.local/share/omarchy/bin/$binary" ]]; then + if [[ $word =~ $home_pattern ]]; 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. @@ -249,6 +255,9 @@ tsui_sudoers="$sudoers_dir/tsui" defer_privileged_repair() { echo "Cannot complete the privileged installer-artifact repair; omarchy-migrate will retry it later." >&2 + if [[ -n ${OMARCHY_MIGRATION_DEFER_FILE:-} && -n ${OMARCHY_MIGRATION_DEFER_TOKEN:-} ]]; then + printf '%s\n' "$OMARCHY_MIGRATION_DEFER_TOKEN" >"$OMARCHY_MIGRATION_DEFER_FILE" + fi exit 75 } diff --git a/test/shell.d/fixtures/privileged-heredoc/annotated-special-parameter-before-home.sh b/test/shell.d/fixtures/privileged-heredoc/annotated-special-parameter-before-home.sh new file mode 100644 index 00000000..5f4d50a2 --- /dev/null +++ b/test/shell.d/fixtures/privileged-heredoc/annotated-special-parameter-before-home.sh @@ -0,0 +1,5 @@ +# omarchy:heredoc-expands paths=none -- the positional argument is a scalar +sudo tee /etc/omarchy/example.conf <"$tmp" <"$tmp" <"$test_dir/failing-bin/sudo" <<'STUB' +#!/bin/bash + +echo "sudo: a terminal is required to read the password" >&2 +exit 1 +STUB +chmod +x "$test_dir/failing-bin/sudo" + export CALLS="$test_dir/calls" rules_dir="$test_dir/rules.d" @@ -189,17 +198,41 @@ run_migration fail "migration keeps a legacy filename already repointed at /usr/bin" pass "migration keeps a legacy filename already repointed at /usr/bin" -# Homes are not all under /home, so the running user's own home counts too, and -# the argument the later variants passed must not hide the path. +# Homes are not all under /home, and a different account may run this +# machine-wide repair after the installer account has gone away. reset_machine cat >"$wifi_rule" <"$defer_file" + +set +e +HOME="$home_dir" \ + OMARCHY_UDEV_RULES_DIR="$rules_dir" \ + OMARCHY_MIGRATION_DEFER_FILE="$defer_file" \ + OMARCHY_MIGRATION_DEFER_TOKEN="$defer_token" \ + PATH="$test_dir/failing-bin:$PATH" \ + bash -euo pipefail "$migration" >"$test_dir/defer.out" 2>&1 +defer_status=$? +set -e + +(( defer_status == 75 )) || fail "migration defers when sudo cannot remove a vulnerable rule" "status=$defer_status" +[[ -e $wifi_rule ]] || fail "migration keeps the vulnerable rule when its elevated removal fails" +[[ $(<"$defer_file") == "$defer_token" ]] || fail "migration authenticates its deferral to the runner" +pass "migration defers instead of blocking the queue when removal cannot elevate" # Nothing named the wrong binary is ours: the same path with a different command # is a rule this migration cannot claim to know anything about. diff --git a/test/shell.d/migrate-scope-test.sh b/test/shell.d/migrate-scope-test.sh index ba499f0a..d31240b9 100644 --- a/test/shell.d/migrate-scope-test.sh +++ b/test/shell.d/migrate-scope-test.sh @@ -83,6 +83,7 @@ mkdir -p "$deferred_root/migrations" "$deferred_home" cat >"$deferred_root/migrations/100-deferred.sh" <<'SH' echo deferred >>"$TEST_CALLS" +printf '%s\n' "$OMARCHY_MIGRATION_DEFER_TOKEN" >"$OMARCHY_MIGRATION_DEFER_FILE" exit 75 SH cat >"$deferred_root/migrations/200-after.sh" <<'SH' @@ -111,3 +112,32 @@ grep -q '^100-deferred\.sh$' "$test_tmp/deferred-pending.out" || ! 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" + +raw_75_root="$test_tmp/raw-75-omarchy" +raw_75_home="$test_tmp/raw-75-home" +raw_75_calls="$test_tmp/raw-75-calls" +mkdir -p "$raw_75_root/migrations" "$raw_75_home" + +cat >"$raw_75_root/migrations/100-child-tempfail.sh" <<'SH' +echo child-tempfail >>"$TEST_CALLS" +bash -c 'exit 75' +SH +cat >"$raw_75_root/migrations/200-after.sh" <<'SH' +echo after-tempfail >>"$TEST_CALLS" +SH + +set +e +HOME="$raw_75_home" \ +OMARCHY_PATH="$raw_75_root" \ +TEST_CALLS="$raw_75_calls" \ + "$ROOT/bin/omarchy-migrate" >"$test_tmp/raw-75.out" 2>"$test_tmp/raw-75.err" +raw_75_status=$? +set -e + +(( raw_75_status == 75 )) || + fail "migration runner preserves an unmarked child exit 75" "status=$raw_75_status" +grep -q '^child-tempfail$' "$raw_75_calls" || fail "migration runner starts the exit-75 child" +! grep -q '^after-tempfail$' "$raw_75_calls" || fail "migration runner stops after an unmarked exit 75" +[[ ! -f $raw_75_home/.local/state/omarchy/migrations/100-child-tempfail.sh ]] || + fail "migration runner leaves an unmarked exit-75 migration incomplete" +pass "migration runner does not mistake a child EX_TEMPFAIL for intentional deferral" diff --git a/test/shell.d/privileged-heredoc-test.sh b/test/shell.d/privileged-heredoc-test.sh index f02be8c1..0bbe1abf 100755 --- a/test/shell.d/privileged-heredoc-test.sh +++ b/test/shell.d/privileged-heredoc-test.sh @@ -50,14 +50,14 @@ USER_WRITABLE_VARS=(HOME PWD OLDPWD TMPDIR OMARCHY_PATH OMARCHY_INSTALL WRITE_COMMANDS=(tee dd install cp mv) ELEVATORS=(sudo as_root pkexec doas run0) -# A dollar the installing user's shell would act on: $name, ${name} or $(cmd). +# A dollar the installing user's shell would act on: $name, ${name}, $1, or $(cmd). # Kept in a variable because an unquoted `(` inside a bracket expression is a # syntax error in [[ =~ ]]. -EXPANSION_RE='\$[A-Za-z_{(]' +EXPANSION_RE='\$[A-Za-z_{(0-9@*#?$!-]' # One pattern for every expansion form, shared by masking and name extraction # so the two stay in lockstep. -EXPANSION_SCAN_RE='^([^$]*)\$(\{[^}]*\}|\([^)]*\)|[A-Za-z_][A-Za-z0-9_]*(\[[^]]*\])?)(.*)$' +EXPANSION_SCAN_RE='^([^$]*)\$(\{[^}]*\}|\([^)]*\)|[A-Za-z_][A-Za-z0-9_]*(\[[^]]*\])?|[0-9@*#?$!-])(.*)$' # Stand-in name for a command substitution, which has no variable to report. COMMAND_SUBSTITUTION="command-substitution" @@ -145,11 +145,14 @@ mask_and_names() { body=${body#[\#!]} if [[ $body =~ ^([A-Za-z_][A-Za-z0-9_]*) ]]; then name=${BASH_REMATCH[1]} + elif [[ $body =~ ^[0-9@*#?$!-]$ ]]; then + name="shell-parameter" else name=$COMMAND_SUBSTITUTION fi else name=${body%%\[*} + [[ $name =~ ^[A-Za-z_] ]] || name="shell-parameter" fi names+=("$name") @@ -418,6 +421,28 @@ command_destinations() { fi } +# Does LINE carry the same resolved value as DEST? Compare resolved tokens rather +# than source spelling so $tmp, ${tmp}, and an alias assigned from either form +# all identify the same scratch file. +line_carries_destination() { + local line="$1" dest="$2" resolved token candidate + local -a tokens=() + + resolved=$(resolve_value "$dest") + line=${line//\"/ } + line=${line//\'/ } + read -r -a tokens <<<"$line" + + for token in "${tokens[@]}"; do + token=${token#[<>]} + token=${token%;} + candidate=$(resolve_value "$token") + [[ $candidate == "$resolved" ]] && return 0 + done + + return 1 +} + # Does the heredoc on this line reach a root-owned file? Either directly, or in # one hop: written to a scratch file that a later install/cp/mv carries into a # privileged directory. @@ -450,7 +475,7 @@ privileged_destination() { while ((follow < ${#scan_lines[@]})); do hop=${scan_lines[follow]} follow=$((follow + 1)) - [[ $hop == *"$dest"* ]] || continue + line_carries_destination "$hop" "$dest" || continue [[ $hop =~ (^|[[:space:]])(install|cp|mv)([[:space:]]|$) ]] || continue while IFS= read -r hop_dest; do [[ $hop_dest == $'\002elevated' ]] && continue @@ -475,6 +500,31 @@ privileged_destination() { return 1 } +# A pipeline may put the command consuming a heredoc after its terminator: +# +# cat <"$first_run" +printf '%%wheel ALL=(ALL) NOPASSWD: /usr/bin/systemctl\n' >>"$first_run" +before=$(cat "$first_run") +run_migration + +[[ -e $first_run ]] || fail "migration keeps a generated file extended for another sudoers user" +[[ $(cat "$first_run") == "$before" ]] || + fail "migration leaves a generated file extended for another user byte for byte" +pass "migration does not delete an administrator grant that uses a generated command" + # Nothing in this file ties it to Omarchy's first run: no self-cleanup line. reset_machine cat >"$first_run" <<'EOF' @@ -301,14 +314,16 @@ reload_at=$(grep -n '^systemctl daemon-reload$' "$CALLS" | cut -d: -f1) fail "migration disables before removing and reloads last" "$(cat "$CALLS")" pass "migration disables the unit, removes it, then reloads systemd in that order" -# Homes are not all under /home. +# Homes are not all under /home, and the account running this machine-wide +# repair may not be the account that installed the unit. reset_machine -write_plymouth_unit "$home_dir/.local/share/omarchy/bin/omarchy-plymouth-shutdown-sync" +write_plymouth_unit "/srv/retired-installer/.local/share/omarchy/bin/omarchy-plymouth-shutdown-sync" run_migration [[ ! -e $plymouth_unit ]] || - fail "migration removes a shutdown unit rooted in a home outside /home" -pass "migration removes a shutdown unit rooted in a home outside /home" + fail "migration removes another user's shutdown unit rooted outside /home" +[[ -e $machine_marker ]] || fail "migration marks the cross-user Plymouth repair complete" +pass "migration removes another user's shutdown unit rooted outside /home" reset_machine write_plymouth_unit "/usr/bin/omarchy-plymouth-shutdown-sync"