Run the refresh hook before its transaction and keep the inhibitor through user work
Three review findings on the update-hook boundary: The pre-refresh-pacman hook had been moved after the refresh transaction and, during a channel switch, deferred to the very end. That defeated the hook's purpose: custom repositories and IgnorePkg entries were not in place when the downgrade-capable -Syyuu ran. Run the hook where it used to run, after the package config is re-synced and before the transaction, but cold: revoke the timestamp, run it behind the no-update wrapper with the caller's original PATH, and revoke again before continuing. Every later privileged command authenticates with --no-update, so a detached child left by the hook has no reusable timestamp to wait for. Channel switching hands the caller's PATH to the refresh the same way the updater receives it, and no longer defers or re-runs the hook. Stay Awake was released before AUR builds, hooks and mise, so the machine could sleep during the longest part of an update. Releasing the inhibitor needs no privilege because the held command already dropped to the user, so stop it after mise and before the reboot prompt, as before. A packaged channel destination cannot be inspected before its package is installed, and a transaction can replace the running tree with a release that predates the command-scoped wrapper; from then on a bare sudo would resolve to /usr/bin/sudo and publish a timestamp, and the destination's own updater authenticates the same way. The switch used to abort only after the packages had changed, with generic rerun advice. Now it checks for the wrapper after each transaction before any further privileged step, completes what it safely can, and stops cold with instructions to run that release's update from a fresh session instead of launching it. Boundary tests pin the hook between the config copies and the transaction with a cold timestamp on both sides, the older-destination stop with its guidance and no launched updater, the new inhibitor position, and the post-update hook staying unreached on failures and signals. Docs, the manual and the sample hook describe the restored timing. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
1 parent
99ddf35138
commit
13a4306a8e
12 files changed
+160
-87
No files matched your search
@@ -47,8 +47,12 @@ sudo = [event for event in events if event.startswith('sudo ')]
|
||||
assert all(event in ('sudo -h', 'sudo -k') or event.startswith('sudo -N ') for event in sudo), events
|
||||
hooks = [i for i, event in enumerate(events) if event.startswith('step:omarchy-hook ')]
|
||||
assert len(hooks) == 2, events
|
||||
assert events[hooks[-1]] == 'step:omarchy-hook pre-refresh-pacman', events
|
||||
assert not any(event.startswith('sudo -N ') for event in events[hooks[0]:]), events
|
||||
assert events[hooks[0]] == 'step:omarchy-hook pre-refresh-pacman', events
|
||||
assert events[hooks[1]] == 'step:omarchy-hook post-update', events
|
||||
assert events[hooks[0] - 1] == 'sudo -k' and events[hooks[0] + 1] == 'sudo -k', events
|
||||
transaction = next(i for i, event in enumerate(events) if event.startswith('step:pacman '))
|
||||
assert hooks[0] < transaction, events
|
||||
assert not any(event.startswith('sudo -N ') for event in events[hooks[1]:]), events
|
||||
PY
|
||||
}
|
||||
|
||||
@@ -59,7 +63,7 @@ for channel in stable rc edge dev; do
|
||||
reset_boundary
|
||||
run_channel "$channel" || fail "$channel failed" "$(<"$boundary_tmp/output")"
|
||||
assert_scoped_channel "$channel"
|
||||
pass "$channel starts cold, authorizes only individual commands, defers hooks and exits cold"
|
||||
pass "$channel starts cold, authorizes only individual commands, runs the refresh hook cold before its transaction and exits cold"
|
||||
done
|
||||
|
||||
reset_boundary
|
||||
@@ -79,6 +83,59 @@ OMARCHY_PATH="$SUDO_TEST_HOME/omarchy" run_channel stable || fail "leaving dev f
|
||||
assert_scoped_channel "dev to stable"
|
||||
pass "leaving dev preserves no-update sudo through unlink and the packaged update"
|
||||
|
||||
# A packaged destination that predates the wrapper cannot be checked before its
|
||||
# package is installed. Its updater authenticates without --no-update, so the
|
||||
# switch must not launch it: it stops at a consistent point with instructions.
|
||||
reset_boundary
|
||||
mv "$SUDO_TEST_ROOT/default/omarchy/sudo-no-update/sudo" "$boundary_tmp/saved-package-wrapper"
|
||||
mv "$SUDO_TEST_ROOT/bin/omarchy-update" "$boundary_tmp/saved-package-update"
|
||||
ln -s test-step "$SUDO_TEST_ROOT/bin/omarchy-update"
|
||||
if OMARCHY_PATH="$SUDO_TEST_HOME/omarchy" run_channel stable; then fail "an older packaged destination was updated with ordinary sudo" "$(<"$SUDO_TEST_LOG")"; fi
|
||||
grep -q "predates command-scoped sudo" "$boundary_tmp/output" || fail "an older packaged destination was not reported" "$(<"$boundary_tmp/output")"
|
||||
grep -q "Run 'omarchy update' from a new terminal to finish" "$boundary_tmp/output" || fail "an older packaged destination lacks recovery guidance" "$(<"$boundary_tmp/output")"
|
||||
grep -Fxq 'step:omarchy-dev-unlink --no-reboot' "$SUDO_TEST_LOG" || fail "the package switch was not completed before stopping" "$(<"$SUDO_TEST_LOG")"
|
||||
grep -Fxq 'step:omarchy-state set reboot-required' "$SUDO_TEST_LOG" || fail "leaving dev for an older release did not mark the reboot" "$(<"$SUDO_TEST_LOG")"
|
||||
if grep -q '^step:omarchy-update ' "$SUDO_TEST_LOG"; then fail "an older packaged updater was launched from the hardened switch" "$(<"$SUDO_TEST_LOG")"; fi
|
||||
grep -q 'The channel switch did not complete' "$boundary_tmp/output" && fail "the stop was reported as an error needing a rerun" "$(<"$boundary_tmp/output")"
|
||||
assert_boundary_cold "older packaged destination"
|
||||
rm "$SUDO_TEST_ROOT/bin/omarchy-update"
|
||||
mv "$boundary_tmp/saved-package-update" "$SUDO_TEST_ROOT/bin/omarchy-update"
|
||||
mv "$boundary_tmp/saved-package-wrapper" "$SUDO_TEST_ROOT/default/omarchy/sudo-no-update/sudo"
|
||||
pass "an older packaged destination stops the switch cold with instructions instead of running its updater"
|
||||
|
||||
# A package-backed source can be downgraded by its own transaction to a release
|
||||
# without the wrapper. From then on a bare sudo would be the real one, so no
|
||||
# privileged step may follow either transaction without checking first. A decoy
|
||||
# sudo in the package bin catches any such call instead of reaching the host.
|
||||
cat >"$SUDO_TEST_ROOT/bin/sudo" <<'STUB'
|
||||
#!/bin/bash
|
||||
printf 'unwrapped-sudo %s\n' "$*" >>"$SUDO_TEST_LOG"
|
||||
exit 97
|
||||
STUB
|
||||
chmod +x "$SUDO_TEST_ROOT/bin/sudo"
|
||||
cp "$SUDO_TEST_ROOT/default/omarchy/sudo-no-update/sudo" "$boundary_tmp/saved-package-wrapper"
|
||||
for pattern in 'pacman -Syyuu*' 'pacman -S --needed*'; do
|
||||
reset_boundary
|
||||
export SUDO_TEST_REMOVE_WRAPPER_STEP=$pattern
|
||||
if run_channel rc; then fail "a downgrade during '$pattern' was not detected" "$(<"$SUDO_TEST_LOG")"; fi
|
||||
unset SUDO_TEST_REMOVE_WRAPPER_STEP
|
||||
grep -q "predates command-scoped sudo" "$boundary_tmp/output" || fail "downgrade during '$pattern' was not reported" "$(<"$boundary_tmp/output")"
|
||||
if grep -q '^unwrapped-sudo ' "$SUDO_TEST_LOG"; then fail "downgrade during '$pattern' reached ordinary sudo" "$(<"$SUDO_TEST_LOG")"; fi
|
||||
if grep -q '^step:omarchy-dev-unlink' "$SUDO_TEST_LOG"; then fail "downgrade during '$pattern' still unlinked" "$(<"$SUDO_TEST_LOG")"; fi
|
||||
if grep -q '^step:omarchy-update ' "$SUDO_TEST_LOG"; then fail "downgrade during '$pattern' still updated" "$(<"$SUDO_TEST_LOG")"; fi
|
||||
python3 - "$SUDO_TEST_LOG" "$pattern" <<'PY'
|
||||
import sys
|
||||
events = open(sys.argv[1]).read().splitlines()
|
||||
transactions = [e for e in events if e.startswith('step:pacman ')]
|
||||
assert len(transactions) == (1 if sys.argv[2].startswith('pacman -Syyuu') else 2), events
|
||||
assert all(e in ('sudo -h', 'sudo -k') or e.startswith('sudo -N ') for e in events if e.startswith('sudo ')), events
|
||||
PY
|
||||
assert_boundary_cold "downgrade during $pattern"
|
||||
cp "$boundary_tmp/saved-package-wrapper" "$SUDO_TEST_ROOT/default/omarchy/sudo-no-update/sudo"
|
||||
pass "a transaction that removes the wrapper stops the switch before any further sudo ($pattern)"
|
||||
done
|
||||
rm "$SUDO_TEST_ROOT/bin/sudo"
|
||||
|
||||
mkdir "$boundary_tmp/user tools"
|
||||
cat >"$boundary_tmp/user tools/channel-user-tool" <<'STUB'
|
||||
#!/bin/bash
|
||||
@@ -110,8 +167,8 @@ for step in pacman omarchy-update-system-pkgs omarchy-hook; do
|
||||
reset_boundary
|
||||
if SUDO_TEST_FAIL_STEP="$step" run_channel stable; then fail "$step failure was ignored"; fi
|
||||
assert_boundary_cold "$step failure"
|
||||
if grep -q '^step:omarchy-hook pre-refresh-pacman$' "$SUDO_TEST_LOG"; then fail "$step failure reached the deferred hook"; fi
|
||||
pass "$step failure exits cold without the deferred hook"
|
||||
if grep -q '^step:omarchy-hook post-update$' "$SUDO_TEST_LOG"; then fail "$step failure reached the post-update hook"; fi
|
||||
pass "$step failure exits cold without the post-update hook"
|
||||
done
|
||||
|
||||
for signal in HUP INT TERM; do
|
||||
@@ -123,7 +180,7 @@ kill -s "$SUDO_TEST_CHANNEL_SIGNAL" "$PPID"
|
||||
STUB
|
||||
if SUDO_TEST_CHANNEL_SIGNAL="$signal" run_channel stable; then fail "$signal was ignored"; fi
|
||||
assert_boundary_cold "$signal"
|
||||
if grep -q '^step:omarchy-hook ' "$SUDO_TEST_LOG"; then fail "$signal reached an update hook"; fi
|
||||
if grep -q '^step:omarchy-hook post-update$' "$SUDO_TEST_LOG"; then fail "$signal reached the post-update hook"; fi
|
||||
pass "$signal stops the channel transition and revokes authorization"
|
||||
done
|
||||
|
||||
|
||||
@@ -127,7 +127,7 @@ assert_log_line() {
|
||||
}
|
||||
|
||||
run_channel stable
|
||||
assert_log_line $'refresh\tstable\tdefer-hook' "stable refreshes the stable pacman channel"
|
||||
assert_log_line $'refresh\tstable' "stable refreshes the stable pacman channel"
|
||||
assert_log_line $'update-pacman\t-S\t--needed\t--noconfirm\t--ask\t4\tomarchy\tomarchy-settings' "stable installs stable Omarchy packages"
|
||||
assert_log_line $'unlink\t--no-reboot' "stable restores the package-backed Omarchy path without an early reboot prompt"
|
||||
assert_log_line $'update\t-y\tOMARCHY_PATH='"$package_root" "stable runs the normal update pipeline from the package-backed path"
|
||||
@@ -137,7 +137,7 @@ fi
|
||||
pass "stable does not require reboot when already package-backed"
|
||||
|
||||
run_channel rc
|
||||
assert_log_line $'refresh\trc\tdefer-hook' "rc refreshes the rc pacman channel"
|
||||
assert_log_line $'refresh\trc' "rc refreshes the rc pacman channel"
|
||||
assert_log_line $'update-pacman\t-S\t--needed\t--noconfirm\t--ask\t4\tomarchy\tomarchy-settings' "rc installs rc Omarchy packages"
|
||||
assert_log_line $'unlink\t--no-reboot' "rc restores the package-backed Omarchy path without an early reboot prompt"
|
||||
assert_log_line $'update\t-y\tOMARCHY_PATH='"$package_root" "rc runs the normal update pipeline from the package-backed path"
|
||||
@@ -145,7 +145,7 @@ assert_log_line $'update\t-y\tOMARCHY_PATH='"$package_root" "rc runs the normal
|
||||
active_checkout="$test_tmp/active-checkout"
|
||||
cp -a "$package_root" "$active_checkout"
|
||||
OMARCHY_TEST_PATH="$active_checkout" run_channel edge
|
||||
assert_log_line $'refresh\tedge\tdefer-hook' "edge refreshes the edge pacman channel"
|
||||
assert_log_line $'refresh\tedge' "edge refreshes the edge pacman channel"
|
||||
assert_log_line $'update-pacman\t-S\t--needed\t--noconfirm\t--ask\t4\tomarchy-dev\tomarchy-settings-dev' "edge installs development Omarchy packages"
|
||||
assert_log_line $'unlink\t--no-reboot' "edge unlinks dev without an early reboot prompt"
|
||||
assert_log_line $'state\tset\treboot-required' "edge marks reboot required when leaving dev"
|
||||
@@ -161,7 +161,7 @@ if run_channel dev >"$test_tmp/occupied.out" 2>"$test_tmp/occupied.err"; then
|
||||
fi
|
||||
|
||||
grep -q "already exists and is not a git checkout" "$test_tmp/occupied.err" || fail "dev explains occupied checkout paths" "$(cat "$test_tmp/occupied.err")"
|
||||
if grep -Fx $'refresh\tedge\tdefer-hook' "$log_file" >/dev/null; then
|
||||
if grep -Fx $'refresh\tedge' "$log_file" >/dev/null; then
|
||||
fail "dev validates checkout path before changing packages" "$(cat "$log_file")"
|
||||
fi
|
||||
pass "dev refuses occupied non-checkout paths before package changes"
|
||||
@@ -169,17 +169,15 @@ pass "dev refuses occupied non-checkout paths before package changes"
|
||||
rmdir "$checkout"
|
||||
run_channel dev
|
||||
assert_log_line $'gum\tconfirm\t--default=false\tSwitch to dev channel?' "dev asks for confirmation"
|
||||
assert_log_line $'refresh\tedge\tdefer-hook' "dev refreshes the edge pacman channel"
|
||||
assert_log_line $'refresh\tedge' "dev refreshes the edge pacman channel"
|
||||
assert_log_line $'update-pacman\t-S\t--needed\t--noconfirm\t--ask\t4\tomarchy-dev\tomarchy-settings-dev' "dev installs development Omarchy packages"
|
||||
assert_log_line $'git\tclone\thttps://github.com/omacom/omarchy.git\t'"$checkout" "dev clones the source checkout to ~/omarchy"
|
||||
assert_log_line $'link\t'"$checkout"$'\t--no-reboot' "dev links ~/omarchy without an early reboot prompt"
|
||||
assert_log_line $'state\tset\treboot-required' "dev defers the reboot prompt to the update pipeline"
|
||||
assert_log_line $'update\t-y\tOMARCHY_PATH='"$checkout" "dev runs the normal update pipeline from the source checkout"
|
||||
[[ $(grep -E '^(git|link|state|refresh|sudo|update)' "$log_file" | sed '/run-deferred/d') == $'git\tclone\thttps://github.com/omacom/omarchy.git\t'"$checkout"$'\nlink\t'"$checkout"$'\t--no-reboot\nstate\tset\treboot-required\nrefresh\tedge\tdefer-hook\nupdate-pacman\t-S\t--needed\t--noconfirm\t--ask\t4\tomarchy-dev\tomarchy-settings-dev\nupdate\t-y\tOMARCHY_PATH='"$checkout" ]] ||
|
||||
[[ $(grep -E '^(git|link|state|refresh|sudo|update)' "$log_file") == $'git\tclone\thttps://github.com/omacom/omarchy.git\t'"$checkout"$'\nlink\t'"$checkout"$'\t--no-reboot\nstate\tset\treboot-required\nrefresh\tedge\nupdate-pacman\t-S\t--needed\t--noconfirm\t--ask\t4\tomarchy-dev\tomarchy-settings-dev\nupdate\t-y\tOMARCHY_PATH='"$checkout" ]] ||
|
||||
fail "dev activates the checkout before changing or updating packages" "$(cat "$log_file")"
|
||||
pass "dev activates the checkout before changing or updating packages"
|
||||
[[ $(tail -1 "$log_file") == $'refresh\tedge\trun-deferred' ]] || fail "channel refresh hook must run after the complete update"
|
||||
pass "channel changes defer the refresh hook until all update work finishes"
|
||||
|
||||
OMARCHY_TEST_PATH="$checkout" run_channel stable
|
||||
assert_log_line $'unlink\t--no-reboot' "switching from dev to stable unlinks without an early reboot prompt"
|
||||
|
||||
@@ -92,6 +92,11 @@ fi
|
||||
if [[ $step == "omarchy-hook" || $step == "omarchy-update-mise" ]]; then
|
||||
[[ ! -e $SUDO_TEST_CACHE ]] || exit 91
|
||||
fi
|
||||
if [[ -n ${SUDO_TEST_REMOVE_WRAPPER_STEP:-} && "$step $*" == $SUDO_TEST_REMOVE_WRAPPER_STEP ]]; then
|
||||
# Model a package transaction replacing the running tree with a release
|
||||
# that predates the wrapper.
|
||||
/usr/bin/rm -f "$OMARCHY_PATH/default/omarchy/sudo-no-update/sudo"
|
||||
fi
|
||||
if [[ ${SUDO_TEST_FAIL_STEP:-} == "$step" ]]; then
|
||||
# Model a misbehaving child leaving state behind, then failing. Cleanup must
|
||||
# still revoke it. This never invokes real sudo or exercises a privilege flaw.
|
||||
@@ -123,7 +128,7 @@ ln -s ../bin/test-step "$SUDO_TEST_ROOT/mock/pacman"
|
||||
reset_boundary() {
|
||||
: >"$SUDO_TEST_LOG"
|
||||
/usr/bin/rm -f "$SUDO_TEST_CACHE"
|
||||
unset SUDO_TEST_FAIL_STEP SUDO_TEST_SIGNAL_STEP SUDO_TEST_SUDO_FAIL SUDO_TEST_REVOKE_FAIL SUDO_TEST_UNSUPPORTED
|
||||
unset SUDO_TEST_FAIL_STEP SUDO_TEST_SIGNAL_STEP SUDO_TEST_SUDO_FAIL SUDO_TEST_REVOKE_FAIL SUDO_TEST_UNSUPPORTED SUDO_TEST_REMOVE_WRAPPER_STEP
|
||||
}
|
||||
assert_boundary_cold() {
|
||||
[[ ! -e $SUDO_TEST_CACHE ]] || fail "$1 left cached authorization"
|
||||
|
||||
@@ -24,9 +24,9 @@ for args in '-y' ''; do
|
||||
python3 - "$SUDO_TEST_LOG" <<'PY'
|
||||
import sys
|
||||
s=open(sys.argv[1]).read().splitlines()
|
||||
positions=[next(i for i,line in enumerate(s) if line.startswith(prefix)) for prefix in ['step:omarchy-update-restart --services-only','step:omarchy-update-stay-awake stop','step:yay','step:omarchy-hook post-update','step:omarchy-update-mise','step:omarchy-update-restart --reboot-only']]
|
||||
positions=[next(i for i,line in enumerate(s) if line.startswith(prefix)) for prefix in ['step:omarchy-update-restart --services-only','step:yay','step:omarchy-hook post-update','step:omarchy-update-mise','step:omarchy-update-stay-awake stop','step:omarchy-update-restart --reboot-only']]
|
||||
assert positions==sorted(positions), s
|
||||
assert not any(line.startswith('sudo -N ') for line in s[positions[3]:]), s
|
||||
assert not any(line.startswith('sudo -N ') for line in s[positions[2]:]), s
|
||||
PY
|
||||
pass "update $args runs privileged phases before hooks and exits cold"
|
||||
done
|
||||
@@ -63,21 +63,20 @@ if run_update -y; then fail "unsupported sudo must prevent mixed-trust work"; fi
|
||||
assert_boundary_cold "unsupported sudo"
|
||||
pass "unsupported sudo fails without running update steps"
|
||||
|
||||
for mode in normal defer-hook run-deferred; do
|
||||
reset_boundary
|
||||
"$SUDO_TEST_ROOT/bin/omarchy-refresh-pacman" stable "$mode" >"$boundary_tmp/output" 2>&1 || fail "refresh $mode failed" "$(<"$boundary_tmp/output")"
|
||||
assert_boundary_cold "refresh $mode"
|
||||
python3 - "$SUDO_TEST_LOG" "$mode" <<'PY'
|
||||
reset_boundary
|
||||
"$SUDO_TEST_ROOT/bin/omarchy-refresh-pacman" stable >"$boundary_tmp/output" 2>&1 || fail "refresh failed" "$(<"$boundary_tmp/output")"
|
||||
assert_boundary_cold "refresh"
|
||||
python3 - "$SUDO_TEST_LOG" <<'PY'
|
||||
import sys
|
||||
s=open(sys.argv[1]).read().splitlines();mode=sys.argv[2]
|
||||
hooks=[i for i,l in enumerate(s) if l.startswith('step:omarchy-hook')]
|
||||
priv=[i for i,l in enumerate(s) if l.startswith('sudo -N ')]
|
||||
assert bool(hooks)==(mode!='defer-hook'), s
|
||||
assert bool(priv)==(mode!='run-deferred'), s
|
||||
if hooks: assert not any(i>hooks[0] for i in priv),s
|
||||
s=open(sys.argv[1]).read().splitlines()
|
||||
hook=next(i for i,l in enumerate(s) if l=='step:omarchy-hook pre-refresh-pacman')
|
||||
copies=[i for i,l in enumerate(s) if l.startswith('sudo -N cp ')]
|
||||
transaction=next(i for i,l in enumerate(s) if l.startswith('step:pacman '))
|
||||
assert len(copies)==4 and max(copies) < hook < transaction, s
|
||||
assert s[hook-1]=='sudo -k' and s[hook+1]=='sudo -k', s
|
||||
assert all(l in ('sudo -h','sudo -k') or l.startswith('sudo -N ') for l in s if l.startswith('sudo ')), s
|
||||
PY
|
||||
pass "refresh $mode preserves the final cold hook boundary"
|
||||
done
|
||||
pass "refresh runs the hook cold between the config re-sync and the transaction"
|
||||
|
||||
for step in pacman omarchy-hook; do
|
||||
reset_boundary
|
||||
|
||||
@@ -73,10 +73,10 @@ expected_steps() {
|
||||
omarchy-update-analyze-logs \
|
||||
omarchy-update-status \
|
||||
omarchy-update-restart \
|
||||
omarchy-update-stay-awake \
|
||||
omarchy-update-aur-pkgs \
|
||||
omarchy-hook \
|
||||
omarchy-update-mise \
|
||||
omarchy-update-stay-awake \
|
||||
omarchy-update-restart
|
||||
}
|
||||
|
||||
|
||||
Reference in new issue
Block a user