From cf3d69c38d9c50e75a7b35c3e51a67c7ccc38d35 Mon Sep 17 00:00:00 2001 From: Ryan Hughes Date: Sun, 30 Aug 2026 12:07:18 -0400 Subject: [PATCH] Merge pull request #9002 from acrogenesis/remove-legacy-installer-privileged-files Repair legacy paths and privileged files left by retired installers (cherry picked from commit 943d2fcbe91507e5efa1da8d80736b02032019d8) --- bin/omarchy-dns | 6 + bin/omarchy-migrate | 6 +- bin/omarchy-provision-owner | 4 +- bin/omarchy-setup-security-fingerprint | 3 + bin/omarchy-upgrade-to-quattro | 22 +- bin/omarchy-windows-vm | 4 + docs/migrations.md | 3 + migrations/1788025225.sh | 339 +++++++ migrations/1788102906.sh | 257 +++++ .../annotated-paths-none-still-fails.sh | 12 + ...annotated-special-parameter-before-home.sh | 5 + .../arithmetic-left-shift-before-heredoc.sh | 5 + .../privileged-heredoc/hop-twice-home-path.sh | 11 + .../hop-variable-home-path.sh | 13 + .../nested-parameter-default.sh | 6 + ...plain-heredoc-indented-pseudo-delimiter.sh | 5 + .../route-append-redirect.sh | 3 + .../route-continued-pipeline.sh | 4 + .../route-dash-delimiter.sh | 5 + .../route-install-hop-alias.sh | 6 + .../route-install-hop-braced.sh | 5 + .../route-install-hop-literal.sh | 7 + .../privileged-heredoc/route-install-hop.sh | 8 + .../route-noclobber-redirect.sh | 4 + .../route-prebody-escaped-pipeline.sh | 6 + .../privileged-heredoc/route-redirect.sh | 4 + .../privileged-heredoc/route-sudo-dd.sh | 3 + .../privileged-heredoc/route-variable-path.sh | 6 + .../safe-annotated-reordered-paths.sh | 8 + .../privileged-heredoc/safe-annotated.sh | 11 + .../privileged-heredoc/safe-herestring.sh | 7 + .../privileged-heredoc/safe-no-expansion.sh | 3 + .../safe-quoted-delimiter.sh | 7 + .../privileged-heredoc/safe-root-anchored.sh | 11 + .../safe-runtime-expansion.sh | 6 + .../safe-user-destination.sh | 9 + .../shadowed-assignment-home-path.sh | 13 + .../shutdown-unit-home-execstop.sh | 84 ++ .../privileged-heredoc/udev-rule-home-path.sh | 11 + .../privileged-heredoc/wifi-rule-home-path.sh | 9 + .../legacy-power-udev-rules-migration-test.sh | 651 ++++++++++++ test/shell.d/migrate-scope-test.sh | 28 + test/shell.d/privileged-heredoc-test.sh | 960 ++++++++++++++++++ ...ired-installer-artifacts-migration-test.sh | 654 ++++++++++++ test/shell.d/upgrade-to-quattro-test.sh | 53 + 45 files changed, 3279 insertions(+), 8 deletions(-) create mode 100644 migrations/1788025225.sh create mode 100644 migrations/1788102906.sh create mode 100644 test/shell.d/fixtures/privileged-heredoc/annotated-paths-none-still-fails.sh create mode 100644 test/shell.d/fixtures/privileged-heredoc/annotated-special-parameter-before-home.sh create mode 100644 test/shell.d/fixtures/privileged-heredoc/arithmetic-left-shift-before-heredoc.sh create mode 100644 test/shell.d/fixtures/privileged-heredoc/hop-twice-home-path.sh create mode 100644 test/shell.d/fixtures/privileged-heredoc/hop-variable-home-path.sh create mode 100644 test/shell.d/fixtures/privileged-heredoc/nested-parameter-default.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/route-continued-pipeline.sh create mode 100644 test/shell.d/fixtures/privileged-heredoc/route-dash-delimiter.sh create mode 100644 test/shell.d/fixtures/privileged-heredoc/route-install-hop-alias.sh create mode 100644 test/shell.d/fixtures/privileged-heredoc/route-install-hop-braced.sh create mode 100644 test/shell.d/fixtures/privileged-heredoc/route-install-hop-literal.sh create mode 100644 test/shell.d/fixtures/privileged-heredoc/route-install-hop.sh create mode 100644 test/shell.d/fixtures/privileged-heredoc/route-noclobber-redirect.sh create mode 100644 test/shell.d/fixtures/privileged-heredoc/route-prebody-escaped-pipeline.sh create mode 100644 test/shell.d/fixtures/privileged-heredoc/route-redirect.sh create mode 100644 test/shell.d/fixtures/privileged-heredoc/route-sudo-dd.sh create mode 100644 test/shell.d/fixtures/privileged-heredoc/route-variable-path.sh create mode 100644 test/shell.d/fixtures/privileged-heredoc/safe-annotated-reordered-paths.sh create mode 100644 test/shell.d/fixtures/privileged-heredoc/safe-annotated.sh create mode 100644 test/shell.d/fixtures/privileged-heredoc/safe-herestring.sh create mode 100644 test/shell.d/fixtures/privileged-heredoc/safe-no-expansion.sh create mode 100644 test/shell.d/fixtures/privileged-heredoc/safe-quoted-delimiter.sh create mode 100644 test/shell.d/fixtures/privileged-heredoc/safe-root-anchored.sh create mode 100644 test/shell.d/fixtures/privileged-heredoc/safe-runtime-expansion.sh create mode 100644 test/shell.d/fixtures/privileged-heredoc/safe-user-destination.sh create mode 100644 test/shell.d/fixtures/privileged-heredoc/shadowed-assignment-home-path.sh create mode 100644 test/shell.d/fixtures/privileged-heredoc/shutdown-unit-home-execstop.sh create mode 100644 test/shell.d/fixtures/privileged-heredoc/udev-rule-home-path.sh create mode 100644 test/shell.d/fixtures/privileged-heredoc/wifi-rule-home-path.sh create mode 100755 test/shell.d/legacy-power-udev-rules-migration-test.sh create mode 100755 test/shell.d/privileged-heredoc-test.sh create mode 100755 test/shell.d/retired-installer-artifacts-migration-test.sh diff --git a/bin/omarchy-dns b/bin/omarchy-dns index 60f60e74..19c6549e 100755 --- a/bin/omarchy-dns +++ b/bin/omarchy-dns @@ -149,6 +149,9 @@ write_networkmanager_dns() { local servers="$1" install -d -m 0755 "$(dirname "$NM_DNS_CONF")" + # 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 <"/etc/systemd/system/$unit" <"/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/$unit /etc/systemd/system/$unit +ExecStartPost=/usr/bin/rm -f /etc/systemd/system/graphical.target.wants/@UNIT@ /etc/systemd/system/@UNIT@ [Install] WantedBy=graphical.target diff --git a/bin/omarchy-setup-security-fingerprint b/bin/omarchy-setup-security-fingerprint index dd4f428f..383aa376 100755 --- a/bin/omarchy-setup-security-fingerprint +++ b/bin/omarchy-setup-security-fingerprint @@ -41,6 +41,9 @@ setup_pam_config() { fi else echo "Creating polkit configuration with fingerprint authentication..." + # omarchy:heredoc-expands paths=none -- $fprintd_gate is the literal PAM + # line defined above, shared with the two sed insertions so the gate cannot + # drift between files. The only path in it is the fixed /usr/bin one. sudo tee /etc/pam.d/polkit-1 >/dev/null </dev/null </dev/null 2>&1 || return 0 + local pending_status log "Running Omarchy migrations" if ! run_as_user_omarchy OMARCHY_UPGRADE_TO_QUATTRO_LIVE=1 omarchy-migrate; then - warn "Could not run Omarchy migrations; the user may be prompted to run them after login." + fail "Omarchy migrations did not complete. Fix the error above and rerun the upgrade before rebooting." + fi + if run_as_user_omarchy omarchy-migrate --pending >/dev/null; then + fail "Omarchy migrations are still pending. Rerun the upgrade before rebooting." + else + pending_status=$? + if (( pending_status != 1 )); then + fail "Could not verify that Omarchy migrations completed. Rerun the upgrade before rebooting." + fi fi } @@ -1395,6 +1406,9 @@ EOF autologin_user=$(as_root awk -F= '/^User=/ { print $2; exit }' /etc/sddm.conf.d/autologin.conf 2>/dev/null || true) fi [[ -n ${autologin_user:-} ]] || autologin_user="$target_user" + # omarchy:heredoc-expands paths=none -- $autologin_user is a username, read + # back from the root-owned drop-in or falling back to $target_user. Same + # mechanism as the old getty override: a name expands, no path does. cat </dev/null [Autologin] User=$autologin_user @@ -1405,6 +1419,8 @@ EOF fi as_root install -d -m 0755 -o sddm -g sddm /var/lib/sddm 2>/dev/null || as_root install -d -m 0755 /var/lib/sddm + # omarchy:heredoc-expands paths=none -- $target_user is a username, not a + # path; SDDM's state file records who logged in last. cat </dev/null [Last] Session=omarchy.desktop @@ -2358,8 +2374,8 @@ run_as_user_omarchy omarchy-bar defaults || cleanup_retired_services ensure_sleep_lock_service remove_retired_default_packages -run_post_upgrade_migrations run_final_system_package_upgrade +run_post_upgrade_migrations run_post_upgrade_update_steps refresh_current_theme_after_upgrade # Do not force-reload Hyprland in the live upgraded session. The legacy diff --git a/bin/omarchy-windows-vm b/bin/omarchy-windows-vm index f1582e2d..f672ed7e 100755 --- a/bin/omarchy-windows-vm +++ b/bin/omarchy-windows-vm @@ -691,6 +691,10 @@ write_compose_atomically() ( esc_password=${esc_password//\$/\$\$} tmp=$(mktemp "$RUNTIME_DIR/.compose.XXXXXX") || exit 1 + # omarchy:heredoc-expands paths=EXPECTED_STORAGE,EXPECTED_SHARED -- both are + # root-protected anchors derived from the authenticated caller uid and bound + # to source inodes that were opened and validated before this compose is + # written. The remaining expansions are revalidated scalar settings. cat >"$tmp" < 1 )); then + return 1 + fi + if [[ ! $line =~ $spec_pattern ]]; then + return 1 + fi + command=${BASH_REMATCH[1]} + done < <(active_lines sudoers) + + if (( count == 1 )) && [[ ${command##*/} == "tsui" ]]; then + return 0 + fi + + return 1 +} + +# install/plymouth.sh wrote this unit for two days (2025-07-05 to 2025-07-07) +# with an unquoted heredoc, so ExecStop names the installing user's home. The +# unit is enabled WantedBy=multi-user.target, so systemd runs that path as uid 0 +# on every shutdown, with no hardware event needed to reach it. +plymouth_unit_runs_from_home() { + local binary="omarchy-plymouth-shutdown-sync" + local exec_stop_pattern='^ExecStop[[:space:]]*=[[:space:]]*(.*)$' + local home_pattern="^/.+/\\.local/share/omarchy/bin/$binary\$" + local line word + local -a words + local matched=1 + + while IFS= read -r line; do + if [[ ! $line =~ $exec_stop_pattern ]]; then + continue + fi + + read -ra words <<<"${BASH_REMATCH[1]}" + if (( ! ${#words[@]} )); then + # An empty assignment resets the list, so nothing named before it still + # runs. `systemd-analyze verify` reports the missing command for a unit + # with one ExecStop=, and reports nothing once a bare ExecStop= follows it. + # An administrator who neutralised the unit this way is left alone. + matched=1 + continue + fi + + # systemd reads -, @, +, ! and : ahead of the command as flags, not as part + # of the path it runs. + word=${words[0]} + while [[ $word == [-@+!:]* ]]; do + word=${word:1} + done + + 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. + matched=0 + fi + done < <(active_lines systemd) + + return $matched +} + +# /etc/sudoers.d is 0750 root:root as shipped, and omarchy-migrate runs as the +# 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. +first_run_sudoers="$sudoers_dir/first-run" +tsui_sudoers="$sudoers_dir/tsui" + +fail_privileged_repair() { + echo "Cannot complete the privileged installer-artifact repair. An administrator must run omarchy-migrate to repair this machine." >&2 + exit 1 +} + +# 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, +# the migration fails loudly and remains pending. After an administrator repairs +# the machine, this marker lets every other account complete without using sudo. +if ! as_root true 2>/dev/null; then + fail_privileged_repair +fi + +# 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 + fail_privileged_repair + fi + if ! as_root rm -f "$reload_needed_marker"; then + fail_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 + fail_privileged_repair + fi + + [[ $kind == "file" ]] || return 0 + if ! content=$(as_root cat "$file"); then + fail_privileged_repair + fi + + if "$predicate" <<<"$content"; then + if ! as_root rm -f "$file"; then + fail_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. + if ! as_root install -Dm644 /dev/null "$reload_needed_marker"; then + fail_privileged_repair + fi + if ! as_root systemctl disable omarchy-plymouth-shutdown.service >/dev/null 2>&1; then + fail_privileged_repair + fi + if ! as_root rm -f "$plymouth_unit"; then + fail_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. + if ! as_root systemctl daemon-reload >/dev/null 2>&1; then + fail_privileged_repair + fi + if ! as_root rm -f "$reload_needed_marker"; then + fail_privileged_repair + fi +fi + +if ! as_root install -Dm644 /dev/null "$machine_marker"; then + fail_privileged_repair +fi diff --git a/migrations/1788102906.sh b/migrations/1788102906.sh new file mode 100644 index 00000000..f81c4303 --- /dev/null +++ b/migrations/1788102906.sh @@ -0,0 +1,257 @@ +echo "Repair legacy XCompose and remove vulnerable Omarchy 3 power udev rules" + +xcompose="$HOME/.XCompose" +packaged_xcompose="$OMARCHY_PATH/default/xcompose" +legacy_xcompose_pattern='^[[:space:]]*include[[:space:]]+"[^"]*/\.local/share/omarchy/default/xcompose"[[:space:]]*$' + +# Omarchy 3 pointed the user's compose file through the checkout compatibility +# link. Preserve their own sequences while moving that include to the packaged +# tree. A failed live restart is harmless: the next graphical login reads the +# repaired file. +if [[ -f $xcompose ]] && grep -Eq "$legacy_xcompose_pattern" "$xcompose"; then + xcompose_replacement=${packaged_xcompose//\\/\\\\} + xcompose_replacement=${xcompose_replacement//&/\\&} + xcompose_replacement=${xcompose_replacement//|/\\|} + sed -i -E "s|^([[:space:]]*include[[:space:]]+\")[^\"]*/\\.local/share/omarchy/default/xcompose\"[[:space:]]*$|\\1$xcompose_replacement\"|" "$xcompose" + omarchy-restart-xcompose >/dev/null 2>&1 || true +fi + +rules_dir=/etc/udev/rules.d +reload_marker_prefix=/var/lib/omarchy/migrations/1788102906-udev-reload-needed +udev_control=/run/udev/control +install_command=/usr/bin/install +mv_command=/usr/bin/mv +rm_command=/usr/bin/rm +udevadm_command=/usr/bin/udevadm + +as_root() { + if (( EUID == 0 )); then + "$@" + else + sudo "$@" + fi +} + +# Omarchy 3 generated these two rules with an unquoted heredoc, so the installing +# 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 +# one-shot migration that swept the old filenames was itself dropped, so an +# install that came up through the 3.x line keeps the old file until this +# migration removes it. +# +# Pre-4 layout work normally belongs in the Omarchy 4 upgrade command, but that +# command only runs on a machine still making the crossing, so an install that +# crossed already would never see it. The upgrade command ends by running +# omarchy-migrate, so this covers the installs still to upgrade as well. +# +# Only remove an exact two-line body that one of the retired installers wrote. +# A same-named file with no active vulnerable RUN survives; one that still has +# the vulnerable command but also contains administrator changes is quarantined +# under a non-.rules suffix for review. +rule_runs_from_home() { + local file="$1" binary="$2" + local run_pattern='RUN([[:space:]]*\{[^}]*\})?[[:space:]]*(\+|:)?=[[:space:]]*(e)?"([^"]*)"' + local line logical="" rest command + + while IFS= read -r line || [[ -n $line ]]; do + # udev tests for a comment before it joins continuations, and skipping one + # does not end a continuation already under way. Both halves verified with + # `udevadm verify`: "# disabled \" followed by a bogus key reports the error + # on line 2, so a comment's own trailing backslash swallows nothing, while + # 'SUBSYSTEM=="power_supply" \' + "# c" + ', RUN+="..."' reports its style + # warning on line 1, so the rule spans the comment. Testing the comment after + # the join would hide a live rule; clearing the pending line here would hide + # one just as well. + if [[ $line =~ ^[[:space:]]*# ]]; then + continue + fi + + # A trailing backslash continues the rule on the next line. + if [[ $line == *\\ ]]; then + logical+=${line%\\} + continue + fi + + rest=$logical$line + logical="" + + while [[ $rest =~ $run_pattern ]]; do + command=${BASH_REMATCH[4]} + rest=${rest#*"${BASH_REMATCH[0]}"} + + # This is only the fail-closed detector; wholesale deletion still requires + # an exact historical body below. Match spacing edits and wrapper arguments + # conservatively so a modified active rule is never mistaken for a safe one. + if [[ $command == *"/.local/share/omarchy/bin/$binary"* ]]; then + return 0 + fi + done + done <"$file" + + return 1 +} + +mark_reload_needed() { + if ! as_root "$install_command" -Dm644 /dev/null "$reload_needed_marker"; then + echo "Administrator privileges are required to repair the vulnerable legacy udev rule. Ask an administrator to run omarchy-migrate." >&2 + exit 1 + fi +} + +# The retired installers overwrote each file with one of five known two-line +# bodies. Only those exact bodies are safe to delete wholesale. If a vulnerable +# rule has since been edited or extended, preserve it under an inactive name +# rather than taking unrelated rules with it. +rule_is_exact_generated() { + local file="$1" binary="$2" + local prefix suffix home expected + local -a lines + + mapfile -t lines <"$file" + (( ${#lines[@]} == 2 )) || return 1 + + case "$binary" in + omarchy-powerprofiles-set) + # Initial userland helper: separate AC/battery units and arguments. + prefix='SUBSYSTEM=="power_supply", ATTR{type}=="Mains", ATTR{online}=="0", RUN+="/usr/bin/systemd-run --no-block --collect --unit=omarchy-power-profile-battery --property=After=power-profiles-daemon.service ' + suffix='/.local/share/omarchy/bin/omarchy-powerprofiles-set battery"' + if [[ ${lines[0]} == "$prefix"*"$suffix" ]]; then + home=${lines[0]#"$prefix"} + home=${home%"$suffix"} + expected='SUBSYSTEM=="power_supply", ATTR{type}=="Mains", ATTR{online}=="1", RUN+="/usr/bin/systemd-run --no-block --collect --unit=omarchy-power-profile-ac --property=After=power-profiles-daemon.service '"$home"'/.local/share/omarchy/bin/omarchy-powerprofiles-set ac"' + [[ $home == /* && $home != *'"'* && ${lines[1]} == "$expected" ]] && return 0 + fi + + # USB-C support: one fixed transient-unit name and no profile argument. + prefix='SUBSYSTEM=="power_supply", ATTR{type}=="Mains", RUN+="/usr/bin/systemd-run --no-block --collect --unit=omarchy-power-profile --property=After=power-profiles-daemon.service ' + suffix='/.local/share/omarchy/bin/omarchy-powerprofiles-set"' + if [[ ${lines[0]} == "$prefix"*"$suffix" ]]; then + home=${lines[0]#"$prefix"} + home=${home%"$suffix"} + expected='SUBSYSTEM=="power_supply", ATTR{type}=="USB", RUN+="/usr/bin/systemd-run --no-block --collect --unit=omarchy-power-profile --property=After=power-profiles-daemon.service '"$home"'/.local/share/omarchy/bin/omarchy-powerprofiles-set"' + [[ $home == /* && $home != *'"'* && ${lines[1]} == "$expected" ]] && return 0 + fi + + # The final Omarchy 3 revision dropped the fixed transient-unit name. + prefix='SUBSYSTEM=="power_supply", ATTR{type}=="Mains", RUN+="/usr/bin/systemd-run --no-block --collect --property=After=power-profiles-daemon.service ' + suffix='/.local/share/omarchy/bin/omarchy-powerprofiles-set"' + if [[ ${lines[0]} == "$prefix"*"$suffix" ]]; then + home=${lines[0]#"$prefix"} + home=${home%"$suffix"} + expected='SUBSYSTEM=="power_supply", ATTR{type}=="USB", RUN+="/usr/bin/systemd-run --no-block --collect --property=After=power-profiles-daemon.service '"$home"'/.local/share/omarchy/bin/omarchy-powerprofiles-set"' + [[ $home == /* && $home != *'"'* && ${lines[1]} == "$expected" ]] && return 0 + fi + ;; + omarchy-wifi-powersave) + # Initial Wi-Fi helper: invoke the userland command directly. + prefix='SUBSYSTEM=="power_supply", ATTR{type}=="Mains", ATTR{online}=="0", RUN+="' + suffix='/.local/share/omarchy/bin/omarchy-wifi-powersave on"' + if [[ ${lines[0]} == "$prefix"*"$suffix" ]]; then + home=${lines[0]#"$prefix"} + home=${home%"$suffix"} + expected='SUBSYSTEM=="power_supply", ATTR{type}=="Mains", ATTR{online}=="1", RUN+="'"$home"'/.local/share/omarchy/bin/omarchy-wifi-powersave off"' + [[ $home == /* && $home != *'"'* && ${lines[1]} == "$expected" ]] && return 0 + fi + + # Later revision deferred each change through its own transient unit. + prefix='SUBSYSTEM=="power_supply", ATTR{type}=="Mains", ATTR{online}=="0", RUN+="/usr/bin/systemd-run --no-block --collect --unit=omarchy-wifi-powersave-on ' + suffix='/.local/share/omarchy/bin/omarchy-wifi-powersave on"' + if [[ ${lines[0]} == "$prefix"*"$suffix" ]]; then + home=${lines[0]#"$prefix"} + home=${home%"$suffix"} + expected='SUBSYSTEM=="power_supply", ATTR{type}=="Mains", ATTR{online}=="1", RUN+="/usr/bin/systemd-run --no-block --collect --unit=omarchy-wifi-powersave-off '"$home"'/.local/share/omarchy/bin/omarchy-wifi-powersave off"' + [[ $home == /* && $home != *'"'* && ${lines[1]} == "$expected" ]] && return 0 + fi + ;; + esac + + return 1 +} + +finish_pending_reload() { + # With no control socket there is no running udevd holding the deleted rule; + # the next daemon start reads the directory from disk. If a daemon is running, + # a failed reload must keep this migration pending so the in-memory root rule + # cannot outlive the per-user completion marker. + if [[ -e $udev_control ]] && ! as_root "$udevadm_command" control --reload 2>/dev/null; then + echo "Could not reload udev after removing a vulnerable legacy rule. Ask an administrator to run omarchy-migrate." >&2 + exit 1 + fi + + if ! as_root "$rm_command" -f "$reload_needed_marker"; then + echo "Could not finish the legacy udev-rule repair. Ask an administrator to run omarchy-migrate." >&2 + exit 1 + fi +} + +quarantine_rule() { + local rule_file="$1" + local backup="$rule_file.omarchy-disabled" + local suffix=0 + + # udev only loads files ending in .rules. Preserve an administrator-modified + # file byte-for-byte under a suffix udev ignores instead of either deleting + # their additions or leaving its user-controlled command active as root. + while [[ -e $backup || -L $backup ]]; do + ((++suffix)) + backup="$rule_file.omarchy-disabled.$suffix" + done + + mark_reload_needed + if ! as_root "$mv_command" --no-clobber -- "$rule_file" "$backup"; then + echo "Administrator privileges are required to quarantine the vulnerable legacy udev rule. Ask an administrator to run omarchy-migrate." >&2 + exit 1 + fi + if [[ -e $rule_file || -L $rule_file ]]; then + echo "Could not quarantine the vulnerable legacy udev rule at $rule_file. Ask an administrator to run omarchy-migrate." >&2 + exit 1 + fi + + finish_pending_reload + echo "Quarantined the modified legacy rule as $backup so udev cannot execute it. Review the preserved file before restoring any safe custom actions." >&2 +} + +if [[ -d $rules_dir && ! -x $rules_dir ]]; then + echo "Could not inspect legacy udev rules under $rules_dir. Ask an administrator to run omarchy-migrate." >&2 + exit 1 +fi + +for legacy_rule in "99-power-profile.rules:omarchy-powerprofiles-set" "99-wifi-powersave.rules:omarchy-wifi-powersave"; do + rule_name="${legacy_rule%%:*}" + rule_file="$rules_dir/$rule_name" + binary="${legacy_rule##*:}" + reload_needed_marker="$reload_marker_prefix-$rule_name" + + # The marker is written before removal so a crash cannot lose the need to + # reload. Only consume it early when the corresponding active file is already + # gone; another user's concurrent run may still be between those two steps. + if [[ -e $reload_needed_marker && ! -e $rule_file && ! -L $rule_file ]]; then + finish_pending_reload + fi + + if [[ -f $rule_file && ! -r $rule_file ]]; then + echo "Could not inspect the legacy udev rule at $rule_file. Ask an administrator to run omarchy-migrate." >&2 + exit 1 + fi + + if [[ -f $rule_file ]] && rule_is_exact_generated "$rule_file" "$binary"; then + mark_reload_needed + if ! as_root "$rm_command" -f "$rule_file"; then + echo "Administrator privileges are required to remove the vulnerable legacy udev rule. Ask an administrator to run omarchy-migrate." >&2 + exit 1 + fi + + finish_pending_reload + elif [[ -f $rule_file ]] && rule_runs_from_home "$rule_file" "$binary"; then + quarantine_rule "$rule_file" + fi + + # If an interrupted removal was followed by an administrator installing a + # safe replacement, reload that replacement before clearing the old marker. + if [[ -e $reload_needed_marker ]]; then + finish_pending_reload + fi +done diff --git a/test/shell.d/fixtures/privileged-heredoc/annotated-paths-none-still-fails.sh b/test/shell.d/fixtures/privileged-heredoc/annotated-paths-none-still-fails.sh new file mode 100644 index 00000000..d0702af8 --- /dev/null +++ b/test/shell.d/fixtures/privileged-heredoc/annotated-paths-none-still-fails.sh @@ -0,0 +1,12 @@ +if omarchy-battery-present; then + # omarchy:heredoc-expands paths=none -- only interpolates the omarchy bin path + cat </dev/null + sudo udevadm trigger --subsystem-match=power_supply 2>/dev/null +fi 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 </etc/omarchy/agent.conf </dev/null +SUBSYSTEM=="power_supply", RUN+="$helper" +EOF diff --git a/test/shell.d/fixtures/privileged-heredoc/hop-variable-home-path.sh b/test/shell.d/fixtures/privileged-heredoc/hop-variable-home-path.sh new file mode 100644 index 00000000..916887c8 --- /dev/null +++ b/test/shell.d/fixtures/privileged-heredoc/hop-variable-home-path.sh @@ -0,0 +1,13 @@ +#!/bin/bash + +# One hop between the expansion and the home path it carries. The token in the +# heredoc has no slash and the value never resolves to a literal path, so a scan +# that rescues unresolved values would exempt a unit baking the user's home into +# /etc/systemd/system. +helper="$HOME/.local/share/omarchy/bin/omarchy-agent" + +# omarchy:heredoc-expands paths=none -- helper is just the agent command name +cat </dev/null +[Service] +ExecStart=$helper +EOF diff --git a/test/shell.d/fixtures/privileged-heredoc/nested-parameter-default.sh b/test/shell.d/fixtures/privileged-heredoc/nested-parameter-default.sh new file mode 100644 index 00000000..7cb74bc0 --- /dev/null +++ b/test/shell.d/fixtures/privileged-heredoc/nested-parameter-default.sh @@ -0,0 +1,6 @@ +#!/bin/bash + +# omarchy:heredoc-expands paths=none -- review regression fixture +sudo tee /etc/omarchy/review.conf >/dev/null </etc/omarchy/agent.conf <>/etc/omarchy/agent.conf </dev/null + helper=$HOME/.local/share/omarchy/bin/omarchy-agent + EOF +fi diff --git a/test/shell.d/fixtures/privileged-heredoc/route-install-hop-alias.sh b/test/shell.d/fixtures/privileged-heredoc/route-install-hop-alias.sh new file mode 100644 index 00000000..fbaab243 --- /dev/null +++ b/test/shell.d/fixtures/privileged-heredoc/route-install-hop-alias.sh @@ -0,0 +1,6 @@ +tmp=/tmp/omarchy-generated +copy=$tmp +cat >"$tmp" <"$tmp" </tmp/omarchy-review-unit +[Service] +ExecStart=$HOME/.local/bin/payload +EOF +sudo install -m 644 /tmp/omarchy-review-unit /etc/systemd/system/review.service diff --git a/test/shell.d/fixtures/privileged-heredoc/route-install-hop.sh b/test/shell.d/fixtures/privileged-heredoc/route-install-hop.sh new file mode 100644 index 00000000..f3894ff4 --- /dev/null +++ b/test/shell.d/fixtures/privileged-heredoc/route-install-hop.sh @@ -0,0 +1,8 @@ +tmp=$(mktemp) + +cat >"$tmp" <|` is a plain redirect with noclobber overridden, not a redirect into a pipe. +cat >|/etc/omarchy/agent.conf </etc/omarchy/agent.conf </dev/null +[Service] +ExecStart=$OMARCHY_PATH/bin/omarchy-agent +EOF diff --git a/test/shell.d/fixtures/privileged-heredoc/safe-annotated-reordered-paths.sh b/test/shell.d/fixtures/privileged-heredoc/safe-annotated-reordered-paths.sh new file mode 100644 index 00000000..b003ca58 --- /dev/null +++ b/test/shell.d/fixtures/privileged-heredoc/safe-annotated-reordered-paths.sh @@ -0,0 +1,8 @@ +storage="$HOME/storage" +shared="$HOME/shared" + +# omarchy:heredoc-expands paths=shared,storage -- both sources are validated before use +cat >/etc/omarchy/mounts.conf </dev/null +servers=$servers +EOF + +# omarchy:heredoc-expands paths=storage -- validated by valid_path and symlink-checked before use +cat </dev/null +source=$storage:/storage +EOF diff --git a/test/shell.d/fixtures/privileged-heredoc/safe-herestring.sh b/test/shell.d/fixtures/privileged-heredoc/safe-herestring.sh new file mode 100644 index 00000000..41e6355e --- /dev/null +++ b/test/shell.d/fixtures/privileged-heredoc/safe-herestring.sh @@ -0,0 +1,7 @@ +resolved="RemoteCommand none" + +grep -qvi '^remotecommand none$' <<<"$resolved" || true + +sudo tee /etc/omarchy/plain.conf >/dev/null <<'EOF' +ok=1 +EOF diff --git a/test/shell.d/fixtures/privileged-heredoc/safe-no-expansion.sh b/test/shell.d/fixtures/privileged-heredoc/safe-no-expansion.sh new file mode 100644 index 00000000..d79fa9e4 --- /dev/null +++ b/test/shell.d/fixtures/privileged-heredoc/safe-no-expansion.sh @@ -0,0 +1,3 @@ +cat </dev/null +SUBSYSTEM=="power_supply", ATTR{type}=="Mains", RUN+="/usr/bin/omarchy-powerprofiles-set" +EOF diff --git a/test/shell.d/fixtures/privileged-heredoc/safe-quoted-delimiter.sh b/test/shell.d/fixtures/privileged-heredoc/safe-quoted-delimiter.sh new file mode 100644 index 00000000..a62c627a --- /dev/null +++ b/test/shell.d/fixtures/privileged-heredoc/safe-quoted-delimiter.sh @@ -0,0 +1,7 @@ +cat <<'EOF' | sudo tee /etc/udev/rules.d/99-omarchy.rules >/dev/null +SUBSYSTEM=="power_supply", RUN+="/usr/bin/omarchy-powerprofiles-set $HOME" +EOF + +cat <<"XML" | sudo tee /etc/omarchy/agent.xml >/dev/null + +XML diff --git a/test/shell.d/fixtures/privileged-heredoc/safe-root-anchored.sh b/test/shell.d/fixtures/privileged-heredoc/safe-root-anchored.sh new file mode 100644 index 00000000..92c6c076 --- /dev/null +++ b/test/shell.d/fixtures/privileged-heredoc/safe-root-anchored.sh @@ -0,0 +1,11 @@ +unit=omarchy-agent.service + +# "/etc/systemd/system/$unit" is a path, but one anchored where root already +# owns everything, so paths=none is the truthful declaration -- the check must +# not demand paths=unit here. +# omarchy:heredoc-expands paths=none -- $unit is a unit name interpolated only into absolute /etc paths +cat >"/etc/systemd/system/$unit" </dev/null +[Service] +Environment=TERM=\$TERM +ExecStart=-/usr/bin/agetty --noclear %I \$TERM +EOF diff --git a/test/shell.d/fixtures/privileged-heredoc/safe-user-destination.sh b/test/shell.d/fixtures/privileged-heredoc/safe-user-destination.sh new file mode 100644 index 00000000..78a47bf7 --- /dev/null +++ b/test/shell.d/fixtures/privileged-heredoc/safe-user-destination.sh @@ -0,0 +1,9 @@ +mkdir -p ~/.config/omarchy + +cat >~/.config/omarchy/agent.conf <"$HOME/.local/bin/omarchy-shim" </dev/null +[Service] +ExecStart=$target +EOF diff --git a/test/shell.d/fixtures/privileged-heredoc/shutdown-unit-home-execstop.sh b/test/shell.d/fixtures/privileged-heredoc/shutdown-unit-home-execstop.sh new file mode 100644 index 00000000..54b21414 --- /dev/null +++ b/test/shell.d/fixtures/privileged-heredoc/shutdown-unit-home-execstop.sh @@ -0,0 +1,84 @@ +#!/bin/bash + +# Install Plymouth package +echo "Installing Plymouth..." +yay -S --noconfirm --needed plymouth + +# Skip if plymouth already exists for some reason +if ! grep -q "plymouth" /etc/mkinitcpio.conf; then + # Backup original mkinitcpio.conf just in case + backup_timestamp=$(date +"%Y%m%d%H%M%S") + sudo cp /etc/mkinitcpio.conf "/etc/mkinitcpio.conf.bak.${backup_timestamp}" + + # Add plymouth to HOOKS array. Should be added: + # - After 'base' and 'udev' (or 'systemd' if using systemd hook) + # - Before 'encrypt' or 'sd-encrypt' if present + + # Use sed to add plymouth in-place + if grep -q "systemd" /etc/mkinitcpio.conf; then + # Add after systemd + sudo sed -i '/^HOOKS=/s/systemd/systemd plymouth/' /etc/mkinitcpio.conf + elif grep -q "udev" /etc/mkinitcpio.conf; then + # Add after udev + sudo sed -i '/^HOOKS=/s/udev/udev plymouth/' /etc/mkinitcpio.conf + else + # Fallback: add after base + sudo sed -i '/^HOOKS=/s/base/base plymouth/' /etc/mkinitcpio.conf + fi +fi + +# Regenerate initramfs +sudo mkinitcpio -P + +# Add kernel parameters for Plymouth (systemd-boot only) +if [ -d "/boot/loader/entries" ]; then + echo "Detected systemd-boot" + + for entry in /boot/loader/entries/*.conf; do + if [ -f "$entry" ]; then + # Skip fallback entries + if [[ "$(basename "$entry")" == *"fallback"* ]]; then + echo "Skipped: $(basename "$entry") (fallback entry)" + continue + fi + + # Skip if splash it already present for some reason + if ! grep -q "splash" "$entry"; then + sudo sed -i '/^options/ s/$/ splash quiet/' "$entry" + else + echo "Skipped: $(basename "$entry") (splash already present)" + fi + fi + done +else + echo "" + echo "systemd-boot not detected. Please manually add these kernel parameters:" + echo " - splash (to see the graphical splash screen)" + echo " - quiet (for silent boot)" + echo "" +fi + +# Touch .plymouth-sync-needed to signal rebuild on shutdown / reboot +touch "$HOME/.config/omarchy/.plymouth-sync-needed" + +# Create the systemd service +sudo tee /etc/systemd/system/omarchy-plymouth-shutdown.service >/dev/null </dev/null + sudo udevadm trigger --subsystem-match=power_supply 2>/dev/null +fi diff --git a/test/shell.d/fixtures/privileged-heredoc/wifi-rule-home-path.sh b/test/shell.d/fixtures/privileged-heredoc/wifi-rule-home-path.sh new file mode 100644 index 00000000..08b8d49b --- /dev/null +++ b/test/shell.d/fixtures/privileged-heredoc/wifi-rule-home-path.sh @@ -0,0 +1,9 @@ +if omarchy-battery-present; then + cat <"$test_dir/bin/sudo" <<'STUB' +#!/bin/bash + +printf 'sudo %s\n' "$*" >>"$CALLS" +if [[ ${1:-} == "/usr/bin/udevadm" ]]; then + shift + exec "$UDEVADM_STUB" "$@" +fi +exec "$@" +STUB + +cat >"$test_dir/bin/udevadm" <<'STUB' +#!/bin/bash + +printf 'udevadm %s\n' "$*" >>"$CALLS" +if [[ -n ${FAIL_UDEV_RELOAD_ONCE_MARKER:-} && ! -e $FAIL_UDEV_RELOAD_ONCE_MARKER ]]; then + touch "$FAIL_UDEV_RELOAD_ONCE_MARKER" + exit 1 +fi +STUB + +cat >"$test_dir/bin/install" <<'STUB' +#!/bin/bash + +echo "migration resolved install through PATH" >&2 +exit 97 +STUB + +cat >"$test_dir/bin/rm" <<'STUB' +#!/bin/bash + +echo "migration resolved rm through PATH" >&2 +exit 98 +STUB + +cat >"$test_dir/bin/mv" <<'STUB' +#!/bin/bash + +echo "migration resolved mv through PATH" >&2 +exit 99 +STUB + +cat >"$test_dir/bin/omarchy-restart-xcompose" <<'STUB' +#!/bin/bash + +echo "omarchy-restart-xcompose" >>"$CALLS" +STUB + +chmod +x "$test_dir/bin/"* + +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 +chmod +x "$test_dir/failing-bin/sudo" + +export CALLS="$test_dir/calls" +export UDEVADM_STUB="$test_dir/bin/udevadm" + +rules_dir="$test_dir/rules.d" +home_dir="$test_dir/home" +omarchy_path="$test_dir/omarchy" +xcompose="$home_dir/.XCompose" +packaged_xcompose="include \"$omarchy_path/default/xcompose\"" +power_rule="$rules_dir/99-power-profile.rules" +wifi_rule="$rules_dir/99-wifi-powersave.rules" +reload_marker_prefix="$test_dir/reload-needed" +power_reload_marker="$reload_marker_prefix-99-power-profile.rules" +wifi_reload_marker="$reload_marker_prefix-99-wifi-powersave.rules" +udev_control="$test_dir/udev-control" +migration="$test_dir/migration.sh" + +# These paths become operands to privileged commands. Keep them fixed in the +# shipped migration and retarget a scratch copy for the unprivileged test; an +# environment override would let the caller choose what root removes. +grep -Fxq 'rules_dir=/etc/udev/rules.d' "$shipped_migration" || + fail "the production udev rules directory is a fixed literal" +grep -Fxq 'reload_marker_prefix=/var/lib/omarchy/migrations/1788102906-udev-reload-needed' "$shipped_migration" || + fail "the production reload marker is a fixed literal" +grep -Fxq 'udev_control=/run/udev/control' "$shipped_migration" || + fail "the production udev control path is a fixed literal" +if grep -q 'OMARCHY_UDEV_' "$shipped_migration"; then + fail "the migration does not accept caller-controlled privileged paths" +fi + +sed \ + -e "s|^rules_dir=/etc/udev/rules.d$|rules_dir=$rules_dir|" \ + -e "s|^reload_marker_prefix=/var/lib/omarchy/migrations/1788102906-udev-reload-needed$|reload_marker_prefix=$reload_marker_prefix|" \ + -e "s|^udev_control=/run/udev/control$|udev_control=$udev_control|" \ + "$shipped_migration" >"$migration" +pass "migration keeps privileged production paths caller-independent" + +reset_machine() { + rm -rf "$rules_dir" "$home_dir" "$omarchy_path" "$power_reload_marker" "$wifi_reload_marker" "$udev_control" + mkdir -p "$rules_dir" "$home_dir" "$omarchy_path/default" + touch "$omarchy_path/default/xcompose" + touch "$udev_control" +} + +run_migration() { + : >"$CALLS" + + HOME="$home_dir" \ + OMARCHY_PATH="$omarchy_path" \ + PATH="$test_dir/bin:$PATH" \ + bash -euo pipefail "$migration" >/dev/null +} + +reload_count() { + grep -cx 'udevadm control --reload' "$CALLS" || true +} + +write_xcompose() { + local include="$1" + + cat >"$xcompose" < : "Test User" + : "test@example.com" +EOF +} + +# #8175's non-udev behavior belongs here so one migration owns the whole legacy +# compatibility-link repair. Omarchy 3 emitted %H, while users may have changed +# it to ~ or its expanded value. +for legacy_include in \ + 'include "%H/.local/share/omarchy/default/xcompose"' \ + 'include "~/.local/share/omarchy/default/xcompose"' \ + "include \"$home_dir/.local/share/omarchy/default/xcompose\""; do + reset_machine + write_xcompose "$legacy_include" + run_migration + + grep -qxF "$packaged_xcompose" "$xcompose" || + fail "migration repoints $legacy_include at the active Omarchy tree" "$(cat "$xcompose")" + grep -qF ' : "test@example.com"' "$xcompose" || + fail "migration discards the user's own compose sequences" + grep -qxF 'omarchy-restart-xcompose' "$CALLS" || + fail "migration does not reload XCompose after rewriting its include" "$(cat "$CALLS")" +done +pass "migration repoints every legacy XCompose include and preserves custom sequences" + +before=$(sha256sum "$xcompose") +run_migration +[[ $(sha256sum "$xcompose") == "$before" ]] || fail "migration changes an already repaired XCompose file" +[[ ! -s $CALLS ]] || fail "migration restarts XCompose when nothing changed" "$(cat "$CALLS")" +pass "migration is idempotent on an already repaired XCompose file" + +reset_machine +run_migration +[[ ! -e $xcompose ]] || fail "migration creates a missing XCompose file" +[[ ! -s $CALLS ]] || fail "migration acts when XCompose and legacy udev rules are absent" "$(cat "$CALLS")" +pass "migration leaves a home without XCompose alone" + +# What Omarchy 3's unquoted heredoc actually left on disk: the installing user's +# home expanded into a rule root runs on every power_supply event. +write_vulnerable_power_rule() { + cat >"$power_rule" <<'RULE' +SUBSYSTEM=="power_supply", ATTR{type}=="Mains", RUN+="/usr/bin/systemd-run --no-block --collect --unit=omarchy-power-profile --property=After=power-profiles-daemon.service /home/someuser/.local/share/omarchy/bin/omarchy-powerprofiles-set" +SUBSYSTEM=="power_supply", ATTR{type}=="USB", RUN+="/usr/bin/systemd-run --no-block --collect --unit=omarchy-power-profile --property=After=power-profiles-daemon.service /home/someuser/.local/share/omarchy/bin/omarchy-powerprofiles-set" +RULE +} + +write_initial_vulnerable_power_rule() { + cat >"$power_rule" <<'RULE' +SUBSYSTEM=="power_supply", ATTR{type}=="Mains", ATTR{online}=="0", RUN+="/usr/bin/systemd-run --no-block --collect --unit=omarchy-power-profile-battery --property=After=power-profiles-daemon.service /home/someuser/.local/share/omarchy/bin/omarchy-powerprofiles-set battery" +SUBSYSTEM=="power_supply", ATTR{type}=="Mains", ATTR{online}=="1", RUN+="/usr/bin/systemd-run --no-block --collect --unit=omarchy-power-profile-ac --property=After=power-profiles-daemon.service /home/someuser/.local/share/omarchy/bin/omarchy-powerprofiles-set ac" +RULE +} + +write_final_vulnerable_power_rule() { + cat >"$power_rule" <<'RULE' +SUBSYSTEM=="power_supply", ATTR{type}=="Mains", RUN+="/usr/bin/systemd-run --no-block --collect --property=After=power-profiles-daemon.service /home/someuser/.local/share/omarchy/bin/omarchy-powerprofiles-set" +SUBSYSTEM=="power_supply", ATTR{type}=="USB", RUN+="/usr/bin/systemd-run --no-block --collect --property=After=power-profiles-daemon.service /home/someuser/.local/share/omarchy/bin/omarchy-powerprofiles-set" +RULE +} + +write_vulnerable_wifi_rule() { + cat >"$wifi_rule" <<'RULE' +SUBSYSTEM=="power_supply", ATTR{type}=="Mains", ATTR{online}=="0", RUN+="/home/someuser/.local/share/omarchy/bin/omarchy-wifi-powersave on" +SUBSYSTEM=="power_supply", ATTR{type}=="Mains", ATTR{online}=="1", RUN+="/home/someuser/.local/share/omarchy/bin/omarchy-wifi-powersave off" +RULE +} + +write_systemd_vulnerable_wifi_rule() { + cat >"$wifi_rule" <<'RULE' +SUBSYSTEM=="power_supply", ATTR{type}=="Mains", ATTR{online}=="0", RUN+="/usr/bin/systemd-run --no-block --collect --unit=omarchy-wifi-powersave-on /home/someuser/.local/share/omarchy/bin/omarchy-wifi-powersave on" +SUBSYSTEM=="power_supply", ATTR{type}=="Mains", ATTR{online}=="1", RUN+="/usr/bin/systemd-run --no-block --collect --unit=omarchy-wifi-powersave-off /home/someuser/.local/share/omarchy/bin/omarchy-wifi-powersave off" +RULE +} + +reset_machine +write_vulnerable_power_rule +run_migration + +[[ ! -e $power_rule ]] || + fail "migration removes a power profile rule that runs out of a user home" "$(cat "$power_rule")" +pass "migration removes a power profile rule that runs out of a user home" + +grep -q '^sudo /usr/bin/rm -f .*99-power-profile\.rules$' "$CALLS" || + fail "migration removes the rule with elevated privileges" "$(cat "$CALLS")" +pass "migration removes the rule with elevated privileges" + +grep -q '^sudo /usr/bin/install -Dm644 /dev/null ' "$CALLS" && + grep -q '^sudo /usr/bin/udevadm control --reload$' "$CALLS" || + fail "migration pins privileged helpers to root-owned paths" "$(cat "$CALLS")" +pass "migration pins install, rm, and udevadm to root-owned paths" + +reset_machine +write_initial_vulnerable_power_rule +run_migration + +[[ ! -e $power_rule ]] || + fail "migration removes the initial AC/battery power rule" "$(cat "$power_rule")" +(( $(reload_count) == 1 )) || + fail "migration reloads udev after removing the initial power rule" "$(cat "$CALLS")" +pass "migration removes the initial AC/battery power rule body" + +reset_machine +write_final_vulnerable_power_rule +run_migration + +[[ ! -e $power_rule ]] || + fail "migration removes the final Omarchy 3 power rule" "$(cat "$power_rule")" +(( $(reload_count) == 1 )) || + fail "migration reloads udev after removing the final power rule" "$(cat "$CALLS")" +pass "migration removes the final Omarchy 3 power rule body" + +reset_machine +write_vulnerable_wifi_rule +run_migration + +[[ ! -e $wifi_rule ]] || + fail "migration removes a Wi-Fi power save rule that runs out of a user home" "$(cat "$wifi_rule")" +pass "migration removes a Wi-Fi power save rule that runs out of a user home" + +reset_machine +write_systemd_vulnerable_wifi_rule +run_migration + +[[ ! -e $wifi_rule ]] || + fail "migration removes the systemd-run Wi-Fi rule" "$(cat "$wifi_rule")" +(( $(reload_count) == 1 )) || + fail "migration reloads udev after removing the systemd-run Wi-Fi rule" "$(cat "$CALLS")" +pass "migration removes the systemd-run Wi-Fi rule body" + +# udevd keeps running the rule it already parsed, so the file being gone from +# disk is only half the fix until it reloads. +(( $(reload_count) == 1 )) || + fail "migration reloads udev after removing a rule" "$(cat "$CALLS")" +pass "migration reloads udev after removing a rule" + +# Reload each removed rule immediately, so a later failure cannot leave an +# already-deleted rule active in udevd. +reset_machine +write_vulnerable_power_rule +write_vulnerable_wifi_rule +run_migration + +[[ ! -e $power_rule && ! -e $wifi_rule ]] || + fail "migration removes both legacy rules in one pass" +(( $(reload_count) == 2 )) || + fail "migration reloads udev after each removal" "$(cat "$CALLS")" +pass "migration removes both legacy rules and reloads after each one" + +# Removing the file and reloading the running daemon are one repair. If reload +# fails, the durable marker must keep the migration pending even though the rule +# has already disappeared from disk; a retry finishes that half before exiting. +reset_machine +write_vulnerable_wifi_rule +reload_failure_seen="$test_dir/reload-failure-seen" +rm -f "$reload_failure_seen" + +set +e +FAIL_UDEV_RELOAD_ONCE_MARKER="$reload_failure_seen" run_migration +reload_status=$? +set -e + +(( reload_status != 0 )) || fail "migration fails when a running udevd cannot reload" +[[ ! -e $wifi_rule && -e $wifi_reload_marker ]] || + fail "migration records a deleted rule whose daemon reload is still pending" +pass "migration keeps a failed udev reload pending" + +run_migration + +[[ ! -e $wifi_reload_marker ]] || fail "migration clears the reload marker after a successful retry" +(( $(reload_count) == 1 )) || + fail "migration retries the pending udev reload" "$(cat "$CALLS")" +pass "migration retries and completes a previously failed udev reload" + +# A marker is created before removal. A concurrent user who sees that phase +# must not consume it and reload while the vulnerable file is still active. +reset_machine +write_vulnerable_power_rule +touch "$power_reload_marker" +run_migration + +remove_line=$(grep -n '^sudo /usr/bin/rm -f .*99-power-profile\.rules$' "$CALLS" | head -n1 | cut -d: -f1) +reload_line=$(grep -n '^sudo /usr/bin/udevadm control --reload$' "$CALLS" | head -n1 | cut -d: -f1) +[[ -n $remove_line && -n $reload_line ]] || fail "concurrent repair records removal and reload" "$(cat "$CALLS")" +(( remove_line < reload_line )) || fail "a concurrent repair reloads before removing the active rule" "$(cat "$CALLS")" +[[ ! -e $power_reload_marker ]] || fail "concurrent repair leaves a completed power reload pending" +pass "a concurrent run cannot consume a marker before rule removal" + +# Each rule owns its reload state. Concurrent repairs of different files cannot +# clear one another's evidence that udev still needs to reload. +reset_machine +touch "$power_reload_marker" "$wifi_reload_marker" +run_migration + +(( $(reload_count) == 2 )) || fail "migration finishes both independent pending reloads" "$(cat "$CALLS")" +[[ ! -e $power_reload_marker && ! -e $wifi_reload_marker ]] || + fail "migration leaves an independent reload marker behind" +pass "power and Wi-Fi removals keep independent durable reload state" + +# A safe replacement installed after an interrupted removal still needs one +# reload to displace the vulnerable ruleset already held by udevd. +reset_machine +cat >"$power_rule" <<'RULE' +SUBSYSTEM=="power_supply", ATTR{type}=="Mains", RUN+="/usr/local/bin/admin-power-hook" +RULE +touch "$power_reload_marker" +run_migration + +[[ -e $power_rule ]] || fail "migration removes a safe replacement rule" +(( $(reload_count) == 1 )) || fail "migration does not reload a safe replacement after interruption" "$(cat "$CALLS")" +[[ ! -e $power_reload_marker ]] || fail "migration leaves the replacement reload pending" +pass "migration reloads a safe replacement after an interrupted removal" + +# A chroot or stopped daemon has no in-memory ruleset to update. An absent udev +# control socket is therefore a completed removal, not a permanent migration +# failure waiting for a daemon that is not running. +reset_machine +rm -f "$udev_control" +write_vulnerable_wifi_rule +run_migration + +[[ ! -e $wifi_rule && ! -e $wifi_reload_marker ]] || + fail "migration completes the disk-only repair when udevd is not running" +(( $(reload_count) == 0 )) || + fail "migration does not contact an absent udevd" "$(cat "$CALLS")" +pass "migration permits environments with no running udev daemon" + +# The second run is what every other account on the machine does, and what a +# user gets from running omarchy-migrate again. +run_migration + +(( $(reload_count) == 0 )) || + fail "migration does not reload udev on a second run" "$(cat "$CALLS")" +[[ ! -s $CALLS ]] || + fail "migration touches nothing on a second run" "$(cat "$CALLS")" +pass "migration is a no-op on a second run" + +reset_machine +run_migration + +[[ ! -s $CALLS ]] || + fail "migration touches nothing when the legacy rules are absent" "$(cat "$CALLS")" +pass "migration leaves a machine without the legacy rules alone" + +# An unreadable same-named file cannot safely be classified as generated or +# custom. Require an administrator rather than silently disabling their rule. +reset_machine +write_vulnerable_power_rule +chmod 000 "$power_rule" + +set +e +run_migration 2>"$test_dir/unreadable-rule.out" +unreadable_status=$? +set -e + +(( unreadable_status != 0 )) || fail "migration accepts a rule it could not inspect" +[[ -e $power_rule ]] || fail "migration disables a rule it could not inspect" +grep -q 'Ask an administrator to run omarchy-migrate' "$test_dir/unreadable-rule.out" || + fail "migration gives no administrator guidance for an unreadable rule" "$(cat "$test_dir/unreadable-rule.out")" +[[ ! -s $CALLS ]] || fail "migration escalates before classifying an unreadable rule" "$(cat "$CALLS")" +chmod 644 "$power_rule" +pass "migration fails without changing an unreadable administrator rule" + +reset_machine +write_vulnerable_power_rule +chmod 600 "$rules_dir" + +set +e +run_migration 2>"$test_dir/unsearchable-rules-dir.out" +unsearchable_status=$? +set -e + +(( unsearchable_status != 0 )) || fail "migration accepts a rules directory it could not inspect" +grep -q 'Ask an administrator to run omarchy-migrate' "$test_dir/unsearchable-rules-dir.out" || + fail "migration gives no administrator guidance for an unsearchable rules directory" "$(cat "$test_dir/unsearchable-rules-dir.out")" +[[ ! -s $CALLS ]] || fail "migration escalates before inspecting the rules directory" "$(cat "$CALLS")" +chmod 755 "$rules_dir" +pass "migration fails closed when it cannot search the rules directory" + +# A user who wrote their own rule under one of these names keeps it, even when +# the file talks about the legacy checkout. udev never runs a comment. +reset_machine +cat >"$power_rule" <<'RULE' +# Replaces the rule Omarchy used to install from +# /home/someuser/.local/share/omarchy/bin/omarchy-powerprofiles-set +#SUBSYSTEM=="power_supply", ATTR{type}=="Mains", RUN+="/home/someuser/.local/share/omarchy/bin/omarchy-powerprofiles-set" +SUBSYSTEM=="power_supply", ATTR{type}=="Mains", RUN+="/usr/local/bin/my-own-power-hook" +RULE +cat >"$wifi_rule" <<'RULE' +# Kept from the old local/share/omarchy setup, rewritten to my own script +SUBSYSTEM=="power_supply", ATTR{type}=="Mains", ATTR{online}=="1", RUN+="/usr/local/bin/my-own-wifi-hook off" +RULE +before=$(cat "$power_rule" "$wifi_rule") +run_migration + +[[ -e $power_rule && -e $wifi_rule ]] || + fail "migration keeps same-named rules that only mention the legacy path" +[[ $(cat "$power_rule" "$wifi_rule") == "$before" ]] || + fail "migration leaves the user's own rules byte for byte" +[[ ! -s $CALLS ]] || + fail "migration escalates nothing when it removes nothing" "$(cat "$CALLS")" +pass "migration keeps same-named rules that only mention the legacy path" + +# A vulnerable rule that an administrator extended is no longer the exact file +# Omarchy generated. Preserve the whole file under a suffix udev ignores rather +# than deleting their addition or leaving the vulnerable command active. +reset_machine +write_vulnerable_power_rule +cat >>"$power_rule" <<'RULE' +ACTION=="add", SUBSYSTEM=="usb", RUN+="/usr/local/sbin/admin-power-hook" +RULE +write_vulnerable_wifi_rule +cp "$power_rule" "$test_dir/mixed-power-rule.before" + +set +e +run_migration 2>"$test_dir/mixed-power-rule.out" +mixed_status=$? +set -e + +(( mixed_status == 0 )) || fail "migration fails after safely quarantining a modified vulnerable rule" "$(cat "$test_dir/mixed-power-rule.out")" +[[ ! -e $power_rule && -e $power_rule.omarchy-disabled && ! -e $wifi_rule ]] || + fail "migration leaves a modified vulnerable rule active" +cmp -s "$test_dir/mixed-power-rule.before" "$power_rule.omarchy-disabled" || + fail "migration changes a mixed rule while quarantining it" "$(cat "$power_rule.omarchy-disabled")" +(( $(reload_count) == 2 )) || fail "migration does not continue through every vulnerable rule after quarantine" "$(cat "$CALLS")" +grep -q 'Quarantined.*\.omarchy-disabled' "$test_dir/mixed-power-rule.out" || + fail "migration does not explain where it preserved a mixed rule" "$(cat "$test_dir/mixed-power-rule.out")" +[[ ! -e $power_reload_marker ]] || fail "migration leaves a completed quarantine reload pending" +grep -q '^sudo /usr/bin/mv --no-clobber -- .*99-power-profile\.rules .*99-power-profile\.rules\.omarchy-disabled$' "$CALLS" || + fail "migration does not pin quarantine moves to root-owned mv" "$(cat "$CALLS")" +pass "migration quarantines a mixed rule and continues repairing the machine" + +run_migration +[[ ! -e $power_rule && -e $power_rule.omarchy-disabled ]] || + fail "a quarantine retry does not preserve the disabled rule" +[[ ! -s $CALLS ]] || fail "a quarantine retry changes machine state" "$(cat "$CALLS")" +pass "migration is a no-op after completing a quarantine" + +# Reformatting RUN does not make the user-controlled command safe, but it does +# make the file something Omarchy cannot delete wholesale without guessing. +reset_machine +cat >"$wifi_rule" <<'RULE' +SUBSYSTEM=="power_supply", ATTR{type}=="Mains", ATTR{online}=="0", RUN += "/home/someuser/.local/share/omarchy/bin/omarchy-wifi-powersave on" +RULE +cp "$wifi_rule" "$test_dir/reformatted-wifi-rule.before" + +set +e +run_migration 2>"$test_dir/reformatted-wifi-rule.out" +reformatted_status=$? +set -e + +(( reformatted_status == 0 )) || fail "migration fails after quarantining a reformatted vulnerable rule" "$(cat "$test_dir/reformatted-wifi-rule.out")" +[[ ! -e $wifi_rule && -e $wifi_rule.omarchy-disabled ]] || + fail "migration leaves a reformatted vulnerable rule active" +cmp -s "$test_dir/reformatted-wifi-rule.before" "$wifi_rule.omarchy-disabled" || + fail "migration changes a reformatted rule while quarantining it" "$(cat "$wifi_rule.omarchy-disabled")" +(( $(reload_count) == 1 )) || fail "migration does not reload udev after quarantining a reformatted rule" "$(cat "$CALLS")" +pass "migration quarantines reformatted vulnerable rules" + +# All assignment forms udev accepts for RUN can execute the same user-home +# helper. None may evade the conservative quarantine detector. +variant_number=0 +for run_assignment in 'RUN{program}+=' 'RUN=' 'RUN:=' 'RUN+=e'; do + ((++variant_number)) + reset_machine + printf 'SUBSYSTEM=="power_supply", ATTR{type}=="Mains", %s"/home/someuser/.local/share/omarchy/bin/omarchy-wifi-powersave on"\n' "$run_assignment" >"$wifi_rule" + cp "$wifi_rule" "$test_dir/run-variant-$variant_number.before" + + set +e + run_migration 2>"$test_dir/run-variant-$variant_number.out" + variant_status=$? + set -e + + (( variant_status == 0 )) || fail "migration fails after quarantining udev assignment $run_assignment" "$(cat "$test_dir/run-variant-$variant_number.out")" + [[ ! -e $wifi_rule && -e $wifi_rule.omarchy-disabled ]] || + fail "migration leaves udev assignment $run_assignment active" + cmp -s "$test_dir/run-variant-$variant_number.before" "$wifi_rule.omarchy-disabled" || + fail "migration changes udev assignment $run_assignment while quarantining it" + (( $(reload_count) == 1 )) || fail "migration does not reload after quarantining $run_assignment" "$(cat "$CALLS")" +done +pass "migration quarantines every valid RUN assignment form" + +# Never overwrite an earlier preserved file. Choose another inactive suffix so +# the active vulnerability is still neutralized without losing either copy. +reset_machine +cat >"$wifi_rule" <<'RULE' +SUBSYSTEM=="power_supply", ATTR{type}=="Mains", RUN="/home/someuser/.local/share/omarchy/bin/omarchy-wifi-powersave on" +RULE +cp "$wifi_rule" "$test_dir/collision-active.before" +printf '%s\n' 'older preserved rule' >"$wifi_rule.omarchy-disabled" +cp "$wifi_rule.omarchy-disabled" "$test_dir/collision-backup.before" + +set +e +run_migration 2>"$test_dir/quarantine-collision.out" +collision_status=$? +set -e + +(( collision_status == 0 )) || fail "migration fails to resolve an existing quarantine" "$(cat "$test_dir/quarantine-collision.out")" +[[ ! -e $wifi_rule && -e $wifi_rule.omarchy-disabled.1 ]] || fail "migration leaves the colliding vulnerable rule active" +cmp -s "$test_dir/collision-backup.before" "$wifi_rule.omarchy-disabled" || fail "migration overwrites the existing quarantine" +cmp -s "$test_dir/collision-active.before" "$wifi_rule.omarchy-disabled.1" || fail "migration changes the new quarantine" +grep -q 'Quarantined.*\.omarchy-disabled\.1' "$test_dir/quarantine-collision.out" || + fail "migration does not report the unique quarantine path" "$(cat "$test_dir/quarantine-collision.out")" +(( $(reload_count) == 1 )) || fail "migration does not reload after resolving a quarantine collision" "$(cat "$CALLS")" +pass "migration preserves both files on a quarantine collision" + +# A comment that does not continue still hides nothing behind it: the file holds +# no active RUN+= at all and stays. +reset_machine +cat >"$power_rule" <<'RULE' +# SUBSYSTEM=="power_supply", ATTR{type}=="Mains", RUN+="/home/someuser/.local/share/omarchy/bin/omarchy-powerprofiles-set" +RULE +run_migration + +[[ -e $power_rule ]] || + fail "migration keeps a rule that is only ever mentioned in a comment" +pass "migration keeps a rule that is only ever mentioned in a comment" + +# The May 2026 rename left an intermediate variant under the old filename that +# already ran out of /usr/bin. It duplicates the packaged rule but is not the +# privilege escalation this migration exists to clear, so it is not ours to take. +reset_machine +cat >"$power_rule" <<'RULE' +SUBSYSTEM=="power_supply", ATTR{type}=="Mains", ATTR{online}=="1", RUN+="/usr/bin/systemd-run --no-block --collect --unit=omarchy-power-profile-ac --property=After=power-profiles-daemon.service /usr/bin/powerprofilesctl set performance" +RULE +run_migration + +[[ -e $power_rule ]] || + 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, and a different account may run this +# machine-wide repair after the installer account has gone away. +reset_machine +cat >"$wifi_rule" <"$test_dir/elevation-failure.out" 2>&1 +failure_status=$? +set -e + +(( failure_status != 0 )) || fail "migration fails when sudo cannot remove a vulnerable rule" +[[ -e $wifi_rule ]] || fail "migration keeps the vulnerable rule when its elevated removal fails" +grep -q 'Ask an administrator to run omarchy-migrate' "$test_dir/elevation-failure.out" || + fail "migration explains how a non-sudo user can complete the repair" "$(cat "$test_dir/elevation-failure.out")" +pass "migration fails loudly with administrator guidance when removal cannot elevate" + +# If the first removal succeeds but the second one cannot elevate, the first +# rule must already have been dropped from the running udevd. +reset_machine +write_vulnerable_power_rule +write_vulnerable_wifi_rule +cat >"$test_dir/failing-bin/sudo" <<'STUB' +#!/bin/bash + +printf 'sudo %s\n' "$*" >>"$CALLS" +if [[ ${1:-} == "/usr/bin/rm" && ${*: -1} == */rules.d/99-wifi-powersave.rules ]]; then + exit 1 +fi +if [[ ${1:-} == "/usr/bin/udevadm" ]]; then + shift + exec "$UDEVADM_STUB" "$@" +fi +exec "$@" +STUB +chmod +x "$test_dir/failing-bin/sudo" +: >"$CALLS" + +set +e +HOME="$home_dir" \ + PATH="$test_dir/failing-bin:$test_dir/bin:$PATH" \ + bash -euo pipefail "$migration" >"$test_dir/partial-failure.out" 2>&1 +partial_status=$? +set -e + +(( partial_status != 0 )) || fail "migration fails when the second rule cannot be removed" +[[ ! -e $power_rule && -e $wifi_rule ]] || fail "migration preserves the expected partial-removal state" +[[ -e $wifi_reload_marker ]] || fail "migration records the second rule removal as still pending" +(( $(reload_count) == 1 )) || + fail "migration reloads udev before a later removal failure" "$(cat "$CALLS")" +pass "a later removal failure cannot leave an already-deleted rule loaded" + +# 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. +reset_machine +cat >"$power_rule" <<'RULE' +SUBSYSTEM=="power_supply", ATTR{type}=="Mains", RUN+="/home/someuser/.local/share/omarchy/bin/omarchy-wifi-powersave on" +RULE +run_migration + +[[ -e $power_rule ]] || + fail "migration matches the binary the filename promises, not any home path" +pass "migration matches the binary the filename promises, not any home path" diff --git a/test/shell.d/migrate-scope-test.sh b/test/shell.d/migrate-scope-test.sh index f877847b..ddb7f59e 100644 --- a/test/shell.d/migrate-scope-test.sh +++ b/test/shell.d/migrate-scope-test.sh @@ -75,3 +75,31 @@ set -e grep -q '^before-fail$' "$calls" || fail "migration runner started failing migration" ! grep -q '^after-fail$' "$calls" || fail "migration runner stops failing migration under strict mode" pass "migration runner does not mark failed migrations complete" + +stdin_root="$test_tmp/stdin-omarchy" +stdin_home="$test_tmp/stdin-home" +stdin_calls="$test_tmp/stdin-calls" +mkdir -p "$stdin_root/migrations" "$stdin_home" + +cat >"$stdin_root/migrations/100-reader.sh" <<'SH' +IFS= read -r value +printf 'reader:%s\n' "$value" >>"$TEST_CALLS" +SH +cat >"$stdin_root/migrations/200-after.sh" <<'SH' +echo after-reader >>"$TEST_CALLS" +SH + +printf 'migration input\n' | \ + HOME="$stdin_home" \ + OMARCHY_PATH="$stdin_root" \ + TEST_CALLS="$stdin_calls" \ + "$ROOT/bin/omarchy-migrate" >"$test_tmp/stdin.out" + +grep -q '^reader:migration input$' "$stdin_calls" || + fail "migration runner preserves the caller's stdin for a migration" "$(cat "$stdin_calls")" +grep -q '^after-reader$' "$stdin_calls" || + fail "a migration reading stdin does not swallow later queue entries" "$(cat "$stdin_calls")" +[[ -f $stdin_home/.local/state/omarchy/migrations/100-reader.sh && + -f $stdin_home/.local/state/omarchy/migrations/200-after.sh ]] || + fail "migration runner marks both stdin-isolated migrations complete" +pass "migration queue uses a private file descriptor instead of migration stdin" diff --git a/test/shell.d/privileged-heredoc-test.sh b/test/shell.d/privileged-heredoc-test.sh new file mode 100755 index 00000000..00dcc985 --- /dev/null +++ b/test/shell.d/privileged-heredoc-test.sh @@ -0,0 +1,960 @@ +#!/bin/bash + +set -euo pipefail + +source "$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd)/base-test.sh" + +# Omarchy does not write a root-owned file through a heredoc whose delimiter is +# unquoted, and this check enforces that. +# +# With an unquoted delimiter (</.local/share/omarchy/bin/..., and ~/.local/share/omarchy is a +# symlink that same user owns. Replacing the symlink and provoking a +# power_supply event gets their code run by udev as root. Quote the delimiter +# and the rule names a literal $HOME instead, which udev never expands, so +# there is nothing to aim at. +# +# The distinction that matters throughout: install-time expansion bakes a +# literal, user-controlled value into a file root later reads or executes -- +# that is the bug. Runtime expansion (an escaped \$VAR left literal in the file +# for a root daemon that does not have the variable set) is a different +# mechanism and is not flagged. install/3-config.sh once used both in one +# heredoc on purpose: $USER expanded at install time because it is a username, +# while \$TERM stayed escaped for systemd to expand later. + +# A file under one of these is owned by root, so its content is a root-level +# input no unprivileged user should be able to influence. +PRIVILEGED_PREFIXES=(/etc /usr /opt /srv /boot /var/lib) + +# Path roots the installing user can replace outright -- by editing the +# directory, or by swapping a symlink like ~/.local/share/omarchy. An expansion +# anchored in one of these is the shape this check exists to catch. +USER_WRITABLE_VARS=(HOME PWD OLDPWD TMPDIR OMARCHY_PATH OMARCHY_INSTALL + XDG_CONFIG_HOME XDG_DATA_HOME XDG_CACHE_HOME XDG_STATE_HOME XDG_RUNTIME_DIR) + +# Commands that carry a heredoc's output to its destination, and the ones that +# do it as root. `tee` counts unelevated too: several bin/ commands re-exec +# themselves as root and then tee straight into /etc. +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}, $1, or $(cmd). +# Kept in a variable because an unquoted `(` inside a bracket expression is a +# syntax error in [[ =~ ]]. +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_]*(\[[^]]*\])?|[0-9@*#?$!-])(.*)$' + +# Stand-in name for a command substitution, which has no variable to report. +COMMAND_SUBSTITUTION="command-substitution" + +# Sites that legitimately need install-time expansion declare it in a comment +# immediately above the heredoc: +# +# # omarchy:heredoc-expands paths=none -- $servers is a validated IP list +# # omarchy:heredoc-expands paths=storage,shared -- checked by valid_path +# +# `paths=` is the machine-checked half, and is what keeps this from being a +# rubber stamp: it must name exactly the expansions that are path-shaped, so +# adding a "$HOME/..." to an already-annotated heredoc makes the declaration +# false and trips the check again instead of inheriting the old exemption. The +# reason after `--` is for the reviewer. +ANNOTATION_RE='^[[:space:]]*#[[:space:]]*omarchy:heredoc-expands[[:space:]]+paths=([A-Za-z_][A-Za-z0-9_-]*(,[A-Za-z_][A-Za-z0-9_-]*)*|none)[[:space:]]+--[[:space:]]+([^[:space:]].*)$' + +FINDINGS=() + +starts_with_privileged_prefix() { + local candidate="$1" prefix + + for prefix in "${PRIVILEGED_PREFIXES[@]}"; do + [[ $candidate == "$prefix"/* ]] && return 0 + done + + return 1 +} + +in_list() { + local needle="$1" item + shift + + for item in "$@"; do + [[ $needle == "$item" ]] && return 0 + done + + return 1 +} + +# Drop escaped dollars, backticks and backslashes so what is left is only what +# the installing user's shell would actually expand. Escaped backslashes go +# first, otherwise "\\$TERM" would read as an escaped dollar. +strip_escapes() { + local text="$1" + + text=${text//\\\\/} + text=${text//\\$/} + text=${text//\\\`/} + + printf '%s' "$text" +} + +# Replace every expansion in TEXT with \001 and list the variable names in +# order: the masked text first, then one name per line. +# +# Masking whole lines rather than whitespace-split words is what makes +# "DNS=${dns_servers//,/ }" readable. Those slashes belong to the substitution +# operator, not to a path, and the space inside the braces would otherwise +# split the expansion across two words and leave a bare "//,/" looking like a +# path. Because both halves come from one pass over one pattern, the Nth \001 +# is the Nth name, so a token can be judged against the right variable. +mask_and_names() { + local text="$1" masked="" body inner tail name guard=0 nested_masked + local -a names=() nested_scan=() nested_names=() + + # Normalize backtick substitution into $( ) so one pattern covers both. + while ((guard++ < 64)) && [[ $text =~ ^([^\`]*)\`([^\`]*)\`(.*)$ ]]; do + body=${BASH_REMATCH[2]//[()]/} + text="${BASH_REMATCH[1]}\$($body)${BASH_REMATCH[3]}" + done + + guard=0 + while ((guard++ < 128)) && [[ $text =~ $EXPANSION_SCAN_RE ]]; do + masked+="${BASH_REMATCH[1]}"$'\001' + body=${BASH_REMATCH[2]} + text=${BASH_REMATCH[4]} + nested_masked="" + nested_names=() + + if [[ $body == \(* ]]; then + name=$COMMAND_SUBSTITUTION + elif [[ $body == \{* ]]; then + inner=${body:1:${#body}-2} + # ${name}, ${name:-default}, ${name//a/b}, ${#name}, ${!name} all start + # with the name once the decorations are stripped. + inner=${inner#[\#!]} + if [[ $inner =~ ^([A-Za-z_][A-Za-z0-9_]*) ]]; then + name=${BASH_REMATCH[1]} + tail=${inner#"$name"} + elif [[ $inner =~ ^[0-9@*#?$!-] ]]; then + name="shell-parameter" + tail=${inner:1} + else + name=$COMMAND_SUBSTITUTION + tail=$inner + fi + + # The shell expands the operator payload too. Keep it as a synthetic + # adjacent token so its placeholders stay aligned with their names while + # the outer expansion remains independently classifiable. Without this, + # ${target:-$HOME/path} is consumed as only `target` and hides HOME. + if [[ $tail =~ $EXPANSION_RE || $tail == *'`'* ]]; then + mapfile -t nested_scan < <(mask_and_names "$tail") + nested_masked=${nested_scan[0]} + nested_names=("${nested_scan[@]:1}") + fi + else + name=${body%%\[*} + [[ $name =~ ^[A-Za-z_] ]] || name="shell-parameter" + fi + + names+=("$name") + if ((${#nested_names[@]} > 0)); then + masked+=" $nested_masked" + names+=("${nested_names[@]}") + fi + done + + printf '%s\n' "$masked$text" + if ((${#names[@]} > 0)); then + printf '%s\n' "${names[@]}" + fi +} + +declare -A VARS=() +declare -A VARS_TAINTED=() + +# Literal assignments in the file under scan, so a destination written as +# "$DROP_IN" or "$COMPOSE_FILE" can be judged as the path it actually is. +# First assignment wins: these scripts set a constant once, and an append like +# boot_params+=(...) is not an assignment this reads at all. +# +# VARS_TAINTED records, separately, any name that is assigned a value naming a +# root the user can replace -- at any point in the file, not just the assignment +# that won. That is what stops a name being introduced with a harmless packaged +# value and then reassigned under $HOME, which judging one assignment in +# isolation would miss in whichever direction it picked. +collect_vars() { + local -n source_lines="$1" + local line name value append + + VARS=() + VARS_TAINTED=() + for line in "${source_lines[@]}"; do + [[ $line =~ ^[[:space:]]*# ]] && continue + [[ $line =~ ^[[:space:]]*(local|declare|export|readonly|typeset)?[[:space:]]*([A-Za-z_][A-Za-z0-9_]*)(\+?)=(.*)$ ]] || continue + + name=${BASH_REMATCH[2]} + append=${BASH_REMATCH[3]} + value=${BASH_REMATCH[4]} + value=${value%%[[:space:]]#*} + value=${value%[[:space:]]} + if [[ $value == \"*\" || $value == \'*\' ]]; then + value=${value:1:${#value}-2} + fi + + mentions_user_writable_root "$value" && VARS_TAINTED["$name"]=1 + + # An append never wins the value -- an array grown across a file resolves to + # nothing useful -- but it does carry the taint, or a name could reach a user + # root through += and never be judged on it. + [[ -n $append ]] && continue + [[ -v VARS[$name] ]] || VARS["$name"]=$value + done +} + +# Expand what can be expanded from the file's own assignments. ${NAME:-default} +# falls back to the default, which is how RUNTIME_DIR reaches +# /var/lib/omarchy/windows; mktemp is unwrapped to the template it is handed, so +# a scratch file inside a privileged directory still reads as privileged. +resolve_value() { + local value="$1" outer=0 inner before name default replacement + + while ((outer++ < 8)); do + before=$value + + inner=0 + while ((inner++ < 32)) && [[ $value =~ \$\{([A-Za-z_][A-Za-z0-9_]*):?-([^}]*)\} ]]; do + name=${BASH_REMATCH[1]} + default=${BASH_REMATCH[2]} + if [[ -v VARS[$name] && ${VARS[$name]} != *"\$$name"* ]]; then + replacement=${VARS[$name]} + else + replacement=$default + fi + value=${value/"${BASH_REMATCH[0]}"/$replacement} + done + + inner=0 + while ((inner++ < 32)) && [[ $value =~ \$\{([A-Za-z_][A-Za-z0-9_]*)\}|\$([A-Za-z_][A-Za-z0-9_]*) ]]; do + name=${BASH_REMATCH[1]} + [[ -n $name ]] || name=${BASH_REMATCH[2]} + [[ -v VARS[$name] && ${VARS[$name]} != *"\$$name"* ]] || break + value=${value/"${BASH_REMATCH[0]}"/${VARS[$name]}} + done + + if [[ $value =~ \$\(mktemp[^\)]*[[:space:]]\"?([^\"\)]+)\"?\) ]]; then + value=${BASH_REMATCH[1]} + fi + + [[ $value == "$before" ]] && break + done + + printf '%s' "$value" +} + +# Strip a leading KEY= and any opening quote so a token's literal head can be +# compared against the privileged prefixes. +literal_head() { + local text="$1" + + [[ $text =~ ^[A-Za-z_][A-Za-z0-9_]*\+?= ]] && text=${text#*=} + text=${text#[\"\']} + + printf '%s' "$text" +} + +# The literal value a name is assigned, when that value is knowable. A value +# read out of a command substitution is not: resolving it would treat the +# slashes in the command itself as a path, which made $autologin_user (an awk +# over /etc/sddm.conf.d) look like a path when it holds a username. +literal_value() { + local name="$1" + + [[ -v VARS[$name] ]] || return 0 + [[ ${VARS[$name]} != *'$('* && ${VARS[$name]} != *'`'* ]] || return 0 + + resolve_value "${VARS[$name]}" +} + +# Does TEXT still reference a root the installing user can replace? Used on a +# value the scan could not fully resolve, where the remaining "$HOME" or +# "${XDG_DATA_HOME}" is the whole reason the value cannot be trusted as a +# root-owned path. A chain through a name this scan never saw assigned resolves +# to neither, and is judged on the rest of the evidence. +mentions_user_writable_root() { + local text="$1" name pattern + + for name in "${USER_WRITABLE_VARS[@]}"; do + # Whole name only. A prefix match would read ${OMARCHY_INSTALL_USER:-}, which + # holds a username, as OMARCHY_INSTALL, which holds a path. + pattern='\$\{?'"$name"'([^A-Za-z0-9_]|$)' + [[ $text =~ $pattern ]] && return 0 + done + + return 1 +} + +# Is this expansion used as a path, and if so, is that path anchored somewhere +# root already owns? MASKED is the containing whitespace token with expansions +# replaced by \001. +# +# Path-shaped means the literal text left around the expansion contains a slash +# ("$HOME/.local/...", "$storage:/storage"), or the variable names a +# user-writable root, or it is assigned a literal value containing a slash. +# Anchored means the literal text *before* the expansion is itself a privileged +# path, as in "/etc/systemd/system/$unit" -- still a path, but one root owns end +# to end. Scoping the anchor test to the containing token is what keeps a udev +# RUN+= line honest: it can name /usr/bin/systemd-run earlier on the same line +# while the $HOME token stands alone. +classify_expansion() { + local masked="$1" name="$2" head literal piece + local path_shape="" + + head=$(literal_head "$masked") + head=${head%%$'\001'*} + literal=$(literal_value "$name") + + [[ $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 " + + [[ -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. + starts_with_privileged_prefix "$head" && return 1 + + # When the token itself is not a path and the shape came only from the + # assigned value, a variable holding root-owned absolute paths is not baking + # anything user-writable in: that is how $fprintd_gate carries + # /usr/bin/omarchy-hw-laptop-closed. This rescue deliberately does not apply + # when the token is a path, so "$storage:/storage" stays flagged. + # + # It also does not apply to a value this scan could not finish resolving whose + # unresolved part reaches a root the user can replace. One hop is all it takes + # to hide the shape: helper="$HOME/.local/share/omarchy/bin/agent" followed by + # ExecStart=$helper puts no slash in the token and no literal path in the value, + # so rescuing it would exempt exactly the write this check exists to catch. A + # value that merely fails to resolve -- a kernel parameter list, an escaped + # password -- is left to the rescue, since nothing in it names a user root. + # 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 [[ $path_shape == "literal " ]]; then + for piece in $literal; do + piece=$(literal_head "$piece") + if mentions_user_writable_root "$piece"; then + return 0 + fi + if [[ $piece == /* ]] && ! starts_with_privileged_prefix "$piece"; then + return 0 + fi + done + return 1 + fi + + return 0 +} + +# Destination paths a command line hands a heredoc's output, as written. An +# "\002elevated" marker is emitted when the line runs through sudo and friends. +command_destinations() { + local line="$1" token target elevated=1 copy_like=1 last="" index scan + local -a tokens=() + + # Quotes only get in the way of splitting; the paths inside them do not + # contain spaces anywhere this check runs. + line=${line//\"/ } + line=${line//\'/ } + # `>|` overrides noclobber; the bar belongs to the operator, not to a pipe. + # Left alone it becomes the redirect's target and hides the privileged path + # behind it, so a `cat <| /etc/...` heredoc reports no destination. + line=${line//">|"/">"} + # 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" + + index=0 + while ((index < ${#tokens[@]})); do + token=${tokens[index]} + index=$((index + 1)) + + in_list "$token" "${ELEVATORS[@]}" && elevated=0 + + if [[ $token == ">" || $token == ">>" ]]; then + target=${tokens[index]:-} + index=$((index + 1)) + [[ -n $target && $target != "&"* && $target != /dev/* ]] && printf '%s\n' "$target" + continue + fi + + if [[ $token == of=* ]]; then + printf '%s\n' "${token#of=}" + continue + fi + + if in_list "$token" "${WRITE_COMMANDS[@]}"; then + if [[ $token == "tee" ]]; then + # Every non-flag argument to tee is a destination. + scan=$index + while ((scan < ${#tokens[@]})); do + target=${tokens[scan]} + scan=$((scan + 1)) + [[ $target == "|" || $target == "&&" || $target == ";" ]] && break + [[ $target == -* || $target == "<"* || $target == ">" || $target == of=* ]] && continue + [[ $target == /dev/* ]] && continue + printf '%s\n' "$target" + done + elif [[ $token != "dd" ]]; then + # dd destinations are expressed only by of= operands, handled above. + copy_like=0 + fi + continue + fi + + [[ $token != -* && $token != "|" && $token != "<"* && $token != ">" ]] && last=$token + done + + # install/cp/mv put the destination last. + if ((copy_like == 0)) && [[ -n $last ]]; then + printf '%s\n' "$last" + fi + + if ((elevated == 0)); then + printf '%s\n' $'\002elevated' + 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. +privileged_destination() { + local line="$1" start_index="$2" + local -n scan_lines="$3" + local dest resolved elevated=1 follow hop hop_dest + local -a unresolved=() + + while IFS= read -r dest; do + if [[ $dest == $'\002elevated' ]]; then + elevated=0 + continue + fi + + resolved=$(resolve_value "$dest") + resolved=${resolved#\~} + if starts_with_privileged_prefix "$resolved"; then + printf '%s' "$resolved" + return 0 + fi + + if [[ $resolved == *'$'* ]]; then + unresolved+=("$dest") + fi + + # One hop: a later copy of this same destination into a root-owned path. + # Literal scratch files need tracing just as much as variable destinations. + follow=$start_index + while ((follow < ${#scan_lines[@]})); do + hop=${scan_lines[follow]} + follow=$((follow + 1)) + [[ $hop =~ (^|[[:space:]])(install|cp|mv)([[:space:]]|$) ]] || continue + line_carries_destination "$hop" "$dest" || continue + while IFS= read -r hop_dest; do + [[ $hop_dest == $'\002elevated' ]] && continue + [[ $hop_dest == "$dest" ]] && continue + hop_dest=$(resolve_value "$hop_dest") + if starts_with_privileged_prefix "$hop_dest"; then + printf '%s' "$hop_dest" + return 0 + fi + done < <(command_destinations "$hop") + done + done < <(command_destinations "$line") + + # An elevated write whose destination cannot be resolved counts as privileged: + # sudo tee is not aimed at a user's own dotfile. + if ((elevated == 0)) && ((${#unresolved[@]} > 0)); then + printf '%s' "${unresolved[0]} (unresolved destination of an elevated write)" + return 0 + fi + + return 1 +} + +# A pipeline may put the command consuming a heredoc after its terminator: +# +# cat < closes)) +} + +scan_file() { + local file="$1" display="${2:-$1}" + local -a lines=() + local index lineno line command scan rest raw operator match prefix guard slot delim candidate candidate_delim body_start + local body_text unescaped destination destination_command body_line masked_line token name + 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 + + index=0 + while ((index < ${#lines[@]})); do + line=${lines[index]} + lineno=$((index + 1)) + index=$((index + 1)) + + [[ $line =~ ^[[:space:]]*# ]] && continue + + # A backslash-escaped newline is removed before Bash parses the command, so + # a pipeline consumer can appear on the next physical line before heredoc + # body collection begins: `cat < 0)) || continue + + 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_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 + + printf -v body_text '%s\n' "${body[@]:-}" + unescaped=$(strip_escapes "$body_text") + [[ $unescaped =~ $EXPANSION_RE || $unescaped == *'`'* ]] || continue + + destination_command=$(continued_heredoc_command "$command" "$index" lines) + destination=$(privileged_destination "$destination_command" "$index" lines) || continue + + # Sort the expansions into the ones that bake a path into the file and + # the ones that only interpolate a scalar. + local -a path_expansions=() plain_expansions=() scanned=() names=() + while IFS= read -r body_line; do + mapfile -t scanned < <(mask_and_names "$body_line") + masked_line=${scanned[0]} + names=("${scanned[@]:1}") + next=0 + + for token in $masked_line; do + count=$(count_placeholders "$token") + ((count > 0)) || continue + + for ((slots = 0; slots < count; slots++)); do + name=${names[next]:-} + next=$((next + 1)) + [[ -n $name ]] || continue + + if classify_expansion "$token" "$name"; then + in_list "$name" "${path_expansions[@]:-}" || path_expansions+=("$name") + else + in_list "$name" "${plain_expansions[@]:-}" || plain_expansions+=("$name") + fi + done + done + done <<<"$unescaped" + + declared_paths="" + annotation="" + look=$((lineno - 2)) + while ((look >= 0)) && [[ ${lines[look]} =~ ^[[:space:]]*# ]]; do + if [[ ${lines[look]} =~ $ANNOTATION_RE ]]; then + declared_paths=${BASH_REMATCH[1]} + annotation=${BASH_REMATCH[3]} + fi + look=$((look - 1)) + done + + shown_paths="none" + if ((${#path_expansions[@]} > 0)); then + shown_paths=$( + IFS=, + printf '%s' "${path_expansions[*]}" + ) + fi + shown_plain="none" + if ((${#plain_expansions[@]} > 0)); then + shown_plain=$( + IFS=, + printf '%s' "${plain_expansions[*]}" + ) + fi + + if [[ -z $annotation ]]; then + FINDINGS+=("$display:$lineno: unquoted heredoc <<$delim expands values at install time and its output reaches $destination + path-shaped expansions: $shown_paths + other expansions: $shown_plain + Whatever expands here is baked into a file root owns. If it is a path the + installing user can replace, root later reads or executes attacker-controlled + content -- that is a local privilege escalation. + Fix, in order of preference: + 1. quote the delimiter (<<'$delim') so nothing expands at install time; + 2. hardcode an absolute root-owned path instead of expanding one; + 3. if the expansion is genuinely required, declare it above the heredoc: + # omarchy:heredoc-expands paths= -- + Decide that list yourself. The scan's own reading of it is above, and + where the scan is most likely wrong is exactly here -- a path it could + not follow reads as an ordinary value -- so pasting its verdict back + signs off on the case worth checking by hand.") + continue + fi + + 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, + so adding one to an already-annotated heredoc trips this check again instead + of inheriting the old exemption. + Fix: drop the path expansion (hardcode an absolute root-owned path), or name + every path-shaped expansion in the declaration and say why root using it is + safe.") + fi + done + done +} + +# bin/, install/ and migrations/ are where privileged writes live: bin/ holds +# the setup and upgrade commands, install/ runs during install, migrations/ +# during update. default/ is scanned too even though the pattern is not +# reachable there today -- it ships bash functions and completions whose only +# "<<" uses are herestrings, with no privileged writes at all -- because the +# scan is cheap and default/ is sourced into every login shell, so a privileged +# write arriving there later should not arrive unchecked. +shell_sources() { + local file first + + while IFS= read -r -d '' file; do + # Binary files and sources with no heredoc operator have nothing this check + # can classify. Filter them before collect_vars and the line-by-line scan. + grep -Iq '<<' "$file" 2>/dev/null || continue + + case $file in + *.sh | *.hook) + printf '%s\0' "$file" + continue + ;; + esac + + IFS= read -r first <"$file" || true + if [[ $first =~ ^#!.*[[:space:]/](bash|sh)$ ]]; then + printf '%s\0' "$file" + fi + done < <(find "$ROOT/bin" "$ROOT/install" "$ROOT/migrations" "$ROOT/default" \ + -type f -print0 2>/dev/null | sort -z) +} + +require_command find +require_command grep + +sources=() +while IFS= read -r -d '' file; do + sources+=("$file") +done < <(shell_sources) + +((${#sources[@]} > 50)) || fail "the scan reaches the privileged-write scripts" \ + "only ${#sources[@]} shell sources found under bin/, install/, migrations/ and default/" +pass "the scan reaches the privileged-write scripts (${#sources[@]} files)" + +for file in "${sources[@]}"; do + scan_file "$file" "${file#"$ROOT"/}" +done + +if ((${#FINDINGS[@]} > 0)); then + fail "no privileged write embeds an install-time expansion through an unquoted heredoc" \ + "$(printf '%s\n\n' "${FINDINGS[@]}")" +fi +pass "no privileged write embeds an install-time expansion through an unquoted heredoc" + +# --- Non-vacuity ------------------------------------------------------------ +# +# A check that cannot catch the bug it was written for is worthless, so the same +# scanner runs against fixtures: installer shapes taken verbatim from this +# repository's history, the routes other than a pipe into sudo tee, and the +# shapes that must stay quiet. + +FIXTURES="$SHELL_TEST_DIR/fixtures/privileged-heredoc" + +fixture_flags() { + local fixture="$1" description="$2" expected="${3:-}" + + FINDINGS=() + scan_file "$FIXTURES/$fixture" "$fixture" + + ((${#FINDINGS[@]} > 0)) || fail "$description" "$fixture produced no finding" + if [[ -n $expected ]]; then + printf '%s\n' "${FINDINGS[@]}" | grep -qF -- "$expected" || + fail "$description" "expected \"$expected\" in:$(printf '\n%s' "${FINDINGS[@]}")" + fi + pass "$description" +} + +fixture_passes() { + local fixture="$1" description="$2" + + FINDINGS=() + scan_file "$FIXTURES/$fixture" "$fixture" + + ((${#FINDINGS[@]} == 0)) || fail "$description" "$(printf '%s\n' "${FINDINGS[@]}")" + pass "$description" +} + +# Verbatim installer shapes: two udev rules whose RUN+= resolves through a +# user's home, and a systemd unit whose ExecStop did the same. Kept as written +# rather than tidied, so the fixtures stay faithful to the real shape instead of +# a cleaned-up sketch of it. +fixture_flags udev-rule-home-path.sh \ + "flags a power-profile udev rule whose RUN+= resolves under \$HOME" \ + "path-shaped expansions: HOME" +fixture_flags wifi-rule-home-path.sh \ + "flags a wifi-powersave udev rule whose RUN+= resolves under \$HOME" \ + "path-shaped expansions: HOME" +fixture_flags shutdown-unit-home-execstop.sh \ + "flags a shutdown unit with ExecStop=\$HOME/..." \ + "path-shaped expansions: HOME" + +# The exemption must not be a rubber stamp: the same file carrying a +# plausible-looking annotation still fails, because $HOME is path-shaped and +# the declaration does not say so. +fixture_flags annotated-paths-none-still-fails.sh \ + "an annotation claiming paths=none cannot silence a baked \$HOME path" \ + "declares paths=none but the path-shaped expansions are HOME" +fixture_flags annotated-special-parameter-before-home.sh \ + "a shell special parameter cannot hide a later baked \$HOME path" \ + "declares paths=none but the path-shaped expansions are HOME" + +# A path can hide one or more hops away from the heredoc. In each of these the +# token in the body has no slash and the value never resolves to a literal path, +# so an annotation of paths=none looks plausible while the write still bakes the +# user's home into a root-owned file. The declaration has to name the expansion. +fixture_flags hop-variable-home-path.sh \ + "an annotation cannot exempt a home path carried one variable hop away" \ + "declares paths=none but the path-shaped expansions are helper" +fixture_flags hop-twice-home-path.sh \ + "an annotation cannot exempt a home path carried two variable hops away" \ + "declares paths=none but the path-shaped expansions are helper" +fixture_flags shadowed-assignment-home-path.sh \ + "a later assignment under \$HOME is judged, not the packaged value it shadowed" \ + "declares paths=none but the path-shaped expansions are target" + +# Routes other than a direct pipe into sudo tee. +fixture_flags route-redirect.sh "flags a plain redirect into /etc" +fixture_flags route-sudo-dd.sh "flags sudo dd of= into a privileged path" +fixture_flags route-variable-path.sh \ + "flags an elevated write whose destination is a variable resolving under /etc" +fixture_flags route-install-hop.sh \ + "flags a scratch file that install(1) later copies into /usr" +fixture_flags route-install-hop-literal.sh \ + "flags a literal scratch file that install(1) later copies into /etc" +fixture_flags route-install-hop-braced.sh \ + "flags a scratch-file hop whose variable uses braces at the privileged copy" +fixture_flags route-install-hop-alias.sh \ + "flags a scratch-file hop carried through an alias variable" +fixture_flags route-continued-pipeline.sh \ + "flags a privileged pipeline command continued after the heredoc terminator" +fixture_flags route-prebody-escaped-pipeline.sh \ + "flags an escaped-line pipeline consumer before the heredoc body" +fixture_flags route-dash-delimiter.sh "flags an indented <<- heredoc" +fixture_flags route-append-redirect.sh "flags an append redirect into /etc" +fixture_flags route-noclobber-redirect.sh \ + "flags a noclobber-override 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" +fixture_flags nested-parameter-default.sh \ + "a nested parameter default cannot hide a baked home path" \ + "path-shaped expansions are 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" +fixture_passes safe-user-destination.sh \ + "an unquoted heredoc expanding into the user's own ~/.config passes" +fixture_passes safe-no-expansion.sh \ + "a privileged write with no expansion in the body passes" +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 new file mode 100755 index 00000000..a5d85388 --- /dev/null +++ b/test/shell.d/retired-installer-artifacts-migration-test.sh @@ -0,0 +1,654 @@ +#!/bin/bash + +set -euo pipefail + +source "$(dirname "$0")/base-test.sh" + +migration="$ROOT/migrations/1788025225.sh" +[[ -f $migration ]] || fail "the retired installer artifact migration exists at $migration" + +test_dir=$(mktemp -d) +trap 'rm -rf "$test_dir"' EXIT + +mkdir -p "$test_dir/bin" + +# sudo runs the real command, so the removals act on the redirected directories +# below and the elevated calls land in the log beside it. +cat >"$test_dir/bin/sudo" <<'STUB' +#!/bin/bash + +printf 'sudo %s\n' "$*" >>"$CALLS" +exec "$@" +STUB + +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/"* + +# 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" +systemd_dir="$test_dir/systemd" +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" "$machine_marker" "$reload_needed_marker" + mkdir -p "$sudoers_dir" "$systemd_dir" "$home_dir" +} + +run_migration() { + : >"$CALLS" + + 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 +} + +# /etc/sudoers.d is 0750 root:root on a real machine, so the migration has to +# escalate merely to see whether either grant is there. An empty call log is +# therefore the wrong invariant: what must be absent unless a file really is +# Omarchy's is a removal, or a unit being disabled or reloaded. +assert_changed_nothing() { + local label="$1" + + ! grep -qE '^(sudo rm|systemctl disable|systemctl daemon-reload)' "$CALLS" || + fail "$label" "$(cat "$CALLS")" + pass "$label" +} + +# The reads themselves must be elevated too. A plain [[ -f ]] or cat under a +# root-only directory returns nothing as the logged-in user, which would make the +# migration report success having looked at nothing at all. +assert_read_elevated() { + local file="$1" label="$2" + + grep -qF "bash $file" "$CALLS" || + fail "$label" "$(cat "$CALLS")" + grep -qF "sudo cat $file" "$CALLS" || + fail "$label" "$(cat "$CALLS")" + pass "$label" +} + +write_plymouth_unit() { + cat >"$plymouth_unit" <"$first_run" + run_migration + + [[ ! -e $first_run ]] || + fail "migration removes first-run grant variant $variant" "$(cat "$first_run")" +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" + +# The grant is only recognisable as Omarchy's because every line in it is one the +# installer emitted. One line an administrator added and the file is theirs. +reset_machine +cat >"$first_run" <<'EOF' +Cmnd_Alias FIRST_RUN_CLEANUP = /bin/rm -f /etc/sudoers.d/first-run +Cmnd_Alias SYMLINK_RESOLVED = /usr/bin/ln -sf /run/systemd/resolve/stub-resolv.conf /etc/resolv.conf +installer ALL=(ALL) NOPASSWD: /usr/bin/systemctl +installer ALL=(ALL) NOPASSWD: /usr/local/bin/our-own-deploy-script +installer ALL=(ALL) NOPASSWD: FIRST_RUN_CLEANUP +EOF +before=$(cat "$first_run") +run_migration + +[[ -e $first_run ]] || fail "migration keeps a first-run file carrying a hand-written rule" +[[ $(cat "$first_run") == "$before" ]] || + fail "migration leaves a hand-written first-run file byte for byte" +assert_changed_nothing "migration changes nothing for a hand-written first-run file" +pass "migration keeps a first-run file carrying a hand-written rule" + +# The installer wrote one account name consistently. A whitelisted command for +# another user is an administrator extension, not part of the generated body. +reset_machine +printf '%s\n' "${first_run_variants[-1]}" >"$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" + +# The oldest installer variant expanded the same $USER into the sudoers account +# and the /home// cleanup path. A different account in the path proves the +# line was edited or hand-written and makes the whole file administrator-owned. +reset_machine +cat >"$first_run" <<'EOF' +alice ALL=(ALL) NOPASSWD: /bin/rm -f /home/bob/.local/state/omarchy/first-run.mode +EOF +before=$(cat "$first_run") +run_migration + +[[ -e $first_run ]] || fail "migration keeps a first-run cleanup path for another account" +[[ $(cat "$first_run") == "$before" ]] || + fail "migration leaves the cross-account first-run file byte for byte" +pass "migration requires the cleanup path account to match the granted account" + +# Nothing in this file ties it to Omarchy's first run: no self-cleanup line. +reset_machine +cat >"$first_run" <<'EOF' +installer ALL=(ALL) NOPASSWD: /usr/bin/ufw +installer ALL=(ALL) NOPASSWD: /usr/bin/ufw-docker +EOF +run_migration + +[[ -e $first_run ]] || + fail "migration keeps a same-named file that never cleaned itself up" +pass "migration keeps a same-named file that never cleaned itself up" + +# A spec continued onto the next line is one logical line, and a comment's own +# trailing backslash swallows nothing: `visudo -cf` on "# note \" plus a bogus +# token reports the error on line 2, so the spec below a commented line is live. +# The hand-written spec sits under the comment on purpose -- above it, the file +# is kept under either reading and the assertion cannot fail. +reset_machine +cat >"$first_run" <<'EOF' +# Retired, keeping the old grant here for reference: \ +installer ALL=(ALL) NOPASSWD: /usr/local/bin/our-own-deploy-script +Cmnd_Alias FIRST_RUN_CLEANUP = /bin/rm -f /etc/sudoers.d/first-run +installer ALL=(ALL) NOPASSWD: \ + /usr/bin/systemctl +installer ALL=(ALL) NOPASSWD: FIRST_RUN_CLEANUP +EOF +run_migration + +[[ -e $first_run ]] || + fail "migration keeps a spec left live under a commented continuation" +pass "migration keeps a spec left live under a commented continuation" + +reset_machine +printf 'installer ALL=(ALL) NOPASSWD: %s/.local/bin/tsui\n' "$home_dir" >"$tsui" +run_migration + +[[ ! -e $tsui ]] || fail "migration removes the tsui grant pointing into the user's home" +grep -q '^sudo rm -f .*/sudoers\.d/tsui$' "$CALLS" || + fail "migration removes the tsui grant with elevated privileges" "$(cat "$CALLS")" +pass "migration removes the tsui grant pointing into the user's home" + +# The feature is gone from Omarchy either way, and unrestricted NOPASSWD on a TUI +# that can shell out escalates from a root-owned path too. +reset_machine +printf 'installer ALL=(ALL) NOPASSWD: /usr/bin/tsui\n' >"$tsui" +run_migration + +[[ ! -e $tsui ]] || fail "migration removes the tsui grant wherever the path points" +pass "migration removes the tsui grant wherever the path points" + +reset_machine +cat >"$tsui" <<'EOF' +# Kept after Omarchy dropped tsui, extended for our operators +installer ALL=(ALL) NOPASSWD: /usr/bin/tsui +operator ALL=(ALL) NOPASSWD: /usr/bin/tsui +EOF +before=$(cat "$tsui") +run_migration + +[[ -e $tsui ]] || fail "migration keeps a tsui file an administrator extended" +[[ $(cat "$tsui") == "$before" ]] || fail "migration leaves an extended tsui file byte for byte" +assert_changed_nothing "migration changes nothing for an extended tsui file" +pass "migration keeps a tsui file an administrator extended" + +reset_machine +printf 'installer ALL=(ALL) NOPASSWD: /usr/bin/tailscale\n' >"$tsui" +run_migration + +[[ -e $tsui ]] || fail "migration keeps a lone grant for some other command" +pass "migration keeps a lone grant for some other command" + +reset_machine +write_plymouth_unit "/home/installer/.local/share/omarchy/bin/omarchy-plymouth-shutdown-sync" +run_migration + +[[ ! -e $plymouth_unit ]] || + fail "migration removes the shutdown unit that runs out of a user home" +pass "migration removes the shutdown unit that runs out of a user home" + +# Stopping the unit is exactly what runs ExecStop, which is the path being taken +# away from root. Disabling only drops the multi-user.target symlink. +! grep -q '^systemctl stop' "$CALLS" || + fail "migration never stops the unit, which would run ExecStop as root" "$(cat "$CALLS")" +pass "migration never stops the unit, which would run ExecStop as root" + +disable_at=$(grep -n '^systemctl disable omarchy-plymouth-shutdown\.service$' "$CALLS" | cut -d: -f1) +remove_at=$(grep -n '^sudo rm -f .*omarchy-plymouth-shutdown\.service$' "$CALLS" | cut -d: -f1) +reload_at=$(grep -n '^systemctl daemon-reload$' "$CALLS" | cut -d: -f1) +[[ -n $disable_at && -n $remove_at && -n $reload_at ]] || + fail "migration disables, removes, then reloads the unit" "$(cat "$CALLS")" +(( disable_at < remove_at && remove_at < reload_at )) || + 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, and the account running this machine-wide +# repair may not be the account that installed the unit. +reset_machine +write_plymouth_unit "/srv/retired-installer/.local/share/omarchy/bin/omarchy-plymouth-shutdown-sync" +run_migration + +[[ ! -e $plymouth_unit ]] || + 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" +run_migration + +[[ -e $plymouth_unit ]] || + fail "migration keeps a same-named unit that runs a packaged command" +assert_changed_nothing "migration changes nothing for a packaged shutdown unit" +pass "migration keeps a same-named unit that runs a packaged command" + +# systemd takes ';' as a comment too, and an ExecStop behind one runs nothing. +reset_machine +cat >"$plymouth_unit" <<'EOF' +[Service] +Type=oneshot +; ExecStop=/home/installer/.local/share/omarchy/bin/omarchy-plymouth-shutdown-sync +ExecStop=/usr/bin/true +EOF +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 + +assert_changed_nothing "migration changes nothing when no retired artifact is present" +pass "migration leaves a machine without any retired artifact alone" + +# All three at once, then the same run again: what a second account on the +# machine does, and what running omarchy-migrate twice does. +reset_machine +printf '%s\n' "${first_run_variants[-1]}" >"$first_run" +printf 'installer ALL=(ALL) NOPASSWD: /usr/bin/tsui\n' >"$tsui" +write_plymouth_unit "/home/installer/.local/share/omarchy/bin/omarchy-plymouth-shutdown-sync" +run_migration + +[[ ! -e $first_run && ! -e $tsui && ! -e $plymouth_unit ]] || + fail "migration clears all three retired artifacts in one pass" +pass "migration clears all three retired artifacts in one pass" + +run_migration + +assert_changed_nothing "migration changes nothing on a second run" +pass "migration is a no-op on a second run" + +# sudo does not treat every '#' as a comment. plugins/sudoers/toke.l matches +# ^#include and ^#includedir as directives in the INITIAL state, and its comment +# rule excludes '#' followed by a digit or -digit so those reach the ID token as a +# numeric uid user spec -- sudoers(5) says the same. Dropping such a line as a +# comment would let a file that still carries an active directive read as though +# it held only generated lines, and be deleted. +for directive in \ + '#include /etc/sudoers.local' \ + '#includedir /etc/sudoers.d.local' \ + '#1000 ALL=(ALL) NOPASSWD: ALL' \ + '#-1000 ALL=(ALL) NOPASSWD: ALL'; do + reset_machine + { + printf '%s\n' "$directive" + printf '%s\n' "${first_run_variants[-1]}" + } >"$first_run" + before=$(cat "$first_run") + run_migration + + [[ -e $first_run ]] || + fail "migration keeps a first-run file carrying the active directive $directive" + [[ $(cat "$first_run") == "$before" ]] || + fail "migration leaves a first-run file with $directive byte for byte" + pass "migration keeps a first-run file carrying the active directive $directive" + + reset_machine + { + printf '%s\n' "$directive" + printf 'installer ALL=(ALL) NOPASSWD: /usr/bin/tsui\n' + } >"$tsui" + before=$(cat "$tsui") + run_migration + + [[ -e $tsui ]] || + fail "migration keeps a tsui file carrying the active directive $directive" + [[ $(cat "$tsui") == "$before" ]] || + fail "migration leaves a tsui file with $directive byte for byte" + pass "migration keeps a tsui file carrying the active directive $directive" +done + +# A sudoers comment ending in a backslash does not swallow the line beneath it: +# toke.l's comment rule consumes to the newline and clears the continuation flag. +# Joining before testing for a comment would hide this administrator's grant. +reset_machine +cat >"$tsui" <<'EOF' +# retired, kept for reference \ +ops ALL=(ALL) NOPASSWD: /usr/bin/tsui +installer ALL=(ALL) NOPASSWD: /usr/bin/tsui +EOF +before=$(cat "$tsui") +run_migration + +[[ -e $tsui ]] || + fail "migration keeps a tsui file whose second grant survives a commented continuation" +[[ $(cat "$tsui") == "$before" ]] || + fail "migration leaves that tsui file byte for byte" +pass "migration keeps a tsui file whose second grant survives a commented continuation" + +# Both grants live under a root-only directory, so seeing them at all takes +# elevation. Pin that the migration reads them elevated rather than silently +# reading nothing. +reset_machine +printf '%s\n' "${first_run_variants[-1]}" >"$first_run" +run_migration + +assert_read_elevated "$first_run" "migration reads the first-run grant with elevated privileges" + +reset_machine +printf 'installer ALL=(ALL) NOPASSWD: /usr/bin/tsui\n' >"$tsui" +run_migration + +assert_read_elevated "$tsui" "migration reads the tsui grant with elevated privileges" + +# sudo ends a logical line at a comment and keeps what came before it: visudo -cf +# reads a spec ending in a backslash, then a comment, then a second spec as two +# live specs. Dropping the pending half would hide this administrator's grant and +# let the file read as though the installer had written all of it. +reset_machine +cat >"$first_run" <<'EOF' +Cmnd_Alias FIRST_RUN_CLEANUP = /bin/rm -f /etc/sudoers.d/first-run +operator ALL=(ALL) NOPASSWD: /usr/local/bin/deploy \ +# kept deliberately +installer ALL=(ALL) NOPASSWD: /usr/bin/systemctl +installer ALL=(ALL) NOPASSWD: FIRST_RUN_CLEANUP +EOF +before=$(cat "$first_run") +run_migration + +[[ -e $first_run ]] || + fail "migration keeps a first-run file whose hand-written spec precedes a comment" +[[ $(cat "$first_run") == "$before" ]] || + fail "migration leaves that first-run file byte for byte" +pass "migration keeps a first-run file whose hand-written spec precedes a comment" + +reset_machine +cat >"$tsui" <<'EOF' +operator ALL=(ALL) NOPASSWD: /usr/local/bin/deploy \ +# kept deliberately +installer ALL=(ALL) NOPASSWD: /usr/bin/tsui +EOF +before=$(cat "$tsui") +run_migration + +[[ -e $tsui ]] || + fail "migration keeps a tsui file whose hand-written spec precedes a comment" +[[ $(cat "$tsui") == "$before" ]] || + fail "migration leaves that tsui file byte for byte" +pass "migration keeps a tsui file whose hand-written spec precedes a comment" + +# systemd resumes a continuation across a comment: systemd-analyze verify on +# "ExecStop=\" + "; c" + a path resolves that path. The unit is live and has to go. +reset_machine +cat >"$plymouth_unit" <<'EOF' +[Service] +Type=oneshot +ExecStart=/usr/bin/true +ExecStop=\ +; still one directive +/home/installer/.local/share/omarchy/bin/omarchy-plymouth-shutdown-sync +EOF +run_migration + +[[ ! -e $plymouth_unit ]] || + fail "migration removes a unit whose ExecStop continues across a comment" "$(cat "$plymouth_unit")" +pass "migration removes a unit whose ExecStop continues across a comment" + +# An empty ExecStop= resets the list: `systemd-analyze verify` reports the missing +# command for a unit with one ExecStop=, and reports nothing once a bare +# ExecStop= follows it. An administrator who neutralised the unit that way runs +# nothing at shutdown and keeps their file. +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= +EOF +before=$(cat "$plymouth_unit") +run_migration + +[[ -e $plymouth_unit ]] || + fail "migration keeps a unit whose ExecStop list was reset to empty" +[[ $(cat "$plymouth_unit") == "$before" ]] || + fail "migration leaves that unit byte for byte" +pass "migration keeps a unit whose ExecStop list was reset to empty" + +# A reset followed by a fresh home ExecStop= is live again. +reset_machine +cat >"$plymouth_unit" <<'EOF' +[Service] +Type=oneshot +ExecStart=/usr/bin/true +ExecStop= +ExecStop=/home/installer/.local/share/omarchy/bin/omarchy-plymouth-shutdown-sync +EOF +run_migration + +[[ ! -e $plymouth_unit ]] || + fail "migration removes a unit whose ExecStop is set again after a reset" +pass "migration removes a unit whose ExecStop is set again after a reset" + +# systemd honours a directive whose line ends the file mid-continuation: +# `systemd-analyze verify` resolves an ExecStop= written that way. +reset_machine +printf '[Service]\nType=oneshot\nExecStart=/usr/bin/true\nExecStop=/home/installer/.local/share/omarchy/bin/omarchy-plymouth-shutdown-sync \\\n' >"$plymouth_unit" +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" + +# 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 != 0 )) || + fail "migration fails 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 +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="$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 + +(( gate_status != 0 )) || + fail "migration fails 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 'An administrator must run omarchy-migrate' "$test_dir/gate.out" || + fail "migration explains how the machine-wide repair can complete" "$(cat "$test_dir/gate.out")" +pass "migration fails without marking the machine repaired and names the administrator action" + +# 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" diff --git a/test/shell.d/upgrade-to-quattro-test.sh b/test/shell.d/upgrade-to-quattro-test.sh index 60e8abff..0648409d 100644 --- a/test/shell.d/upgrade-to-quattro-test.sh +++ b/test/shell.d/upgrade-to-quattro-test.sh @@ -32,6 +32,12 @@ grep -F 'run_post_upgrade_migrations' "$upgrade_to_quattro" >/dev/null grep -F 'omarchy-migrate' "$upgrade_to_quattro" >/dev/null grep -F 'dust' "$upgrade_to_quattro" >/dev/null grep -F 'satty' "$upgrade_to_quattro" >/dev/null +final_upgrade_line=$(grep -n '^run_final_system_package_upgrade$' "$upgrade_to_quattro" | cut -d: -f1) +migrations_line=$(grep -n '^run_post_upgrade_migrations$' "$upgrade_to_quattro" | cut -d: -f1) +[[ -n $final_upgrade_line && -n $migrations_line ]] || + fail "final package upgrade and migration calls exist" +(( final_upgrade_line < migrations_line )) || + fail "Omarchy migrations run after the final package upgrade" pass "Omarchy 4 upgrade applies packaged migrations" if grep -F 'skip-first-run-update-notification' "$upgrade_to_quattro" >/dev/null; then @@ -98,6 +104,53 @@ function_body() { awk -v name="$1" '$0 == name "() {" { inside = 1; next } inside && $0 == "}" { exit } inside' "$upgrade_to_quattro" } +migrations_body=$(function_body run_post_upgrade_migrations) +grep -F 'fail "Omarchy migrations did not complete.' <<<"$migrations_body" >/dev/null || + fail "Omarchy 4 upgrade fails when a migration cannot complete" +grep -F 'omarchy-migrate --pending' <<<"$migrations_body" >/dev/null || + fail "Omarchy 4 upgrade verifies that migrations actually completed" +grep -F 'fail "Omarchy migrations are still pending.' <<<"$migrations_body" >/dev/null || + fail "Omarchy 4 upgrade fails when a successful migration command leaves pending work" +grep -F 'pending_status != 1' <<<"$migrations_body" >/dev/null || + fail "Omarchy 4 upgrade distinguishes no pending work from a failed verification" +grep -F 'fail "Could not verify that Omarchy migrations completed.' <<<"$migrations_body" >/dev/null || + fail "Omarchy 4 upgrade fails when it cannot verify migration state" +if grep -F 'return 0' <<<"$migrations_body" >/dev/null || grep -F 'warn ' <<<"$migrations_body" >/dev/null; then + fail "Omarchy 4 upgrade does not continue past failed migrations" +fi + +exercise_post_upgrade_migrations() { + local stub_migration_status="$1" stub_pending_status="$2" + + ( + log() { :; } + fail() { exit 1; } + run_as_user_omarchy() { + if [[ " $* " == *" --pending "* ]]; then + return "$stub_pending_status" + else + return "$stub_migration_status" + fi + } + eval "run_post_upgrade_migrations() { $migrations_body +}" + run_post_upgrade_migrations + ) +} + +exercise_post_upgrade_migrations 0 1 >/dev/null 2>&1 || + fail "Omarchy 4 upgrade accepts a completed migration queue" +if exercise_post_upgrade_migrations 1 1 >/dev/null 2>&1; then + fail "Omarchy 4 upgrade accepts a failed migration" +fi +if exercise_post_upgrade_migrations 0 0 >/dev/null 2>&1; then + fail "Omarchy 4 upgrade accepts pending migrations" +fi +if exercise_post_upgrade_migrations 0 2 >/dev/null 2>&1; then + fail "Omarchy 4 upgrade accepts a failed pending-state check" +fi +pass "Omarchy 4 upgrade cannot finish with pending migrations" + if function_body cleanup_retired_services | grep -F 'systemctl disable iwd' >/dev/null; then fail "Omarchy 4 upgrade does not retire iwd in a step separate from the NetworkManager enable" fi