Skip to content

Refuse omarchy-update when invoked as root - #13377

Closed
Chessing234 wants to merge 1 commit into
omacom:quattrofrom
Chessing234:fix/13329-update-refuse-root
Closed

Chessing234 wants to merge 1 commit into
omacom:quattrofrom
Chessing234:fix/13329-update-refuse-root

Conversation

@Chessing234

@Chessing234 Chessing234 commented Sep 27, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • sudo omarchy update runs mise and post-update hooks as root, leaving root-owned installs under the user's home and breaking later mise use.
  • Exit early when EUID is 0; privileged package steps still escalate via sudo from an unprivileged caller.

Fixes #13329

Test plan

  • bash test/shell.d/update-refuse-root-test.sh
  • Confirm omarchy update still works when run as the user (menu path)

sudo omarchy update runs mise and post-update hooks as root, so tool
installs land root-owned under the user's home and break later mise use.
Keep the usual per-step sudo escalation and exit early when EUID is 0.
@omarchybot omarchybot added the bug Something isn't working label Sep 27, 2026
@llstrk

llstrk commented Sep 29, 2026

Copy link
Copy Markdown

Automated AI review

Community review: Independent automated community review, unaffiliated with the Omarchy team, intended to help prepare PRs for their review.

Outcome: Plain sudo omarchy update (#13329) never reaches the new message, and the new test passes with the guard's exit 1 removed.

Plain sudo exits before the guard

sudo's default env_reset drops OMARCHY_PATH, so the source-root check (line 19) exits with OMARCHY_PATH does not match this Omarchy command. before the guard (line 24). The output is unchanged from the base.

Suggested change: move the block above omarchy_security_require_source_root and test without OMARCHY_PATH.

Reproducer (fails at a9a22c9)

Reproducer: plain sudo never reaches the new refusal

Finding: with Arch's default sudo (env_reset), OMARCHY_PATH is not passed
through, so omarchy_security_require_source_root (line 19) exits before the
new EUID == 0 block (line 24) and prints OMARCHY_PATH does not match this Omarchy command. instead of the new guidance.

Test (fails at the PR head, a9a22c9)

Save as test/shell.d/update-refuse-root-reset-env-test.sh and run
bash test/shell.d/update-refuse-root-reset-env-test.sh as a normal user.
It uses the repository's sudo-boundary fixture, so sudo and every update step
are stand-ins; nothing real is updated, even on a regression.

#!/bin/bash

set -euo pipefail

source "$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd)/base-test.sh"
source "$SHELL_TEST_DIR/fixtures/sudo-boundary-test.sh"
copy_boundary_file bin/omarchy-update

if ! unshare --user --map-root-user true 2>/dev/null; then
  skip "no unprivileged user namespace; skipping the root refusal"
  exit 0
fi

# Every update step is a stand-in that records itself; sudo is the fixture's.
steps="$boundary_tmp/steps"
for step in omarchy-update-lock omarchy-update-requires-free-space omarchy-update-pkg-prune \
  omarchy-snapshot omarchy-update-stay-awake omarchy-update-dev omarchy-update-keyring \
  omarchy-update-system-pkgs omarchy-migrate omarchy-hook omarchy-update-mise \
  omarchy-update-aur-pkgs omarchy-update-orphan-pkgs omarchy-update-analyze-logs \
  omarchy-update-status omarchy-update-restart; do
  rm -f "$SUDO_TEST_ROOT/bin/$step"
  printf '#!/bin/bash\necho "${0##*/} euid=$EUID" >>"%s"\n' "$steps" >"$SUDO_TEST_ROOT/bin/$step"
  chmod +x "$SUDO_TEST_ROOT/bin/$step"
done

run_as_root() {
  : >"$steps"
  : >"$SUDO_TEST_LOG"
  status=0
  output=$(unshare --user --map-root-user env "$@" OMARCHY_UPDATE_LOGGED=1 \
    "$SUDO_TEST_ROOT/bin/omarchy-update" -y 2>&1) || status=$?
}

# sudo -E, sudo -i and su keep OMARCHY_PATH.
run_as_root
(( status != 0 )) || fail "root with OMARCHY_PATH is refused" "$output"
[[ ! -s $steps && ! -s $SUDO_TEST_LOG ]] || fail "root with OMARCHY_PATH runs no step and no sudo" "$(cat "$steps" "$SUDO_TEST_LOG")"
[[ $output == *"not under sudo"* ]] || fail "root with OMARCHY_PATH is told to run as the user" "$output"
pass "root with OMARCHY_PATH is refused before any step"

# Plain sudo resets the environment (env_reset), so OMARCHY_PATH is gone.
run_as_root -u OMARCHY_PATH
(( status != 0 )) || fail "root under sudo's reset environment is refused" "$output"
[[ ! -s $steps ]] || fail "root under sudo's reset environment runs no step" "$(cat "$steps")"
[[ $output == *"not under sudo"* ]] || fail "root under sudo's reset environment is told to run as the user" "$output"
pass "root under sudo's reset environment is told to run as the user"

Actual output at the PR head (a9a22c9), exit status 1

stdout:

ok - root with OMARCHY_PATH is refused before any step

stderr:

OMARCHY_PATH does not match this Omarchy command.
not ok - root under sudo's reset environment is told to run as the user

Actual output with the same 9-line block moved above omarchy_security_require_source_root "$0", exit status 0

stdout:

ok - root with OMARCHY_PATH is refused before any step
ok - root under sudo's reset environment is told to run as the user

stderr: empty.

Same result through the dispatcher

From the root of the PR head checkout, with the environment sudo's
env_reset leaves (no OMARCHY_PATH), inside a user namespace mapped to root:

unshare --user --map-root-user env -i PATH=/usr/local/sbin:/usr/local/bin:/usr/bin HOME=/root TERM=dumb \
  LOGNAME=root USER=root SHELL=/bin/bash SUDO_USER=user SUDO_UID=1000 SUDO_GID=1000 \
  "SUDO_COMMAND=/usr/bin/omarchy update -y" "$PWD/bin/omarchy" update -y

Actual output (stderr), exit status 1, identical at the PR head and at its
base c5b4db7:

OMARCHY_PATH does not match this Omarchy command.

With OMARCHY_PATH="$PWD" added (the sudo -E / sudo -i / su shape), the
PR head prints:

Error: run omarchy-update as your user, not under sudo.
It asks for a password when a step needs privilege.

Environment: bubblewrap sandbox, bash 5.3.15, util-linux 2.42.3 unshare.

Test passes when the guard does not stop the update

With exit 1 replaced by :, the test passes as a normal user: the update continues and the real sudo -k fails with a nonzero status. On a regression it runs the real update script, not the sudo stand-ins.

Suggested change: build the test on fixtures/sudo-boundary-test.sh and assert that no step or sudo call ran.

Reproducer (passes with the guard disabled)

Reproducer: the new test still passes when the guard does not stop the update

Finding: the namespace branch of test/shell.d/update-refuse-root-test.sh
only checks for a nonzero status and the message text. With the guard's
exit 1 removed, the update prints the message and carries on; the real
/usr/bin/sudo -k then fails inside the user namespace, which supplies the
nonzero status, so the test still reports ok.

Caution: run this only as a normal user. With the guard disabled, the test
executes the real update script (not the repository's sudo-boundary stand-ins),
so as real root it would go on to a real update.

Steps (from the root of a scratch copy of the PR head, a9a22c9)

sed -i '/not under sudo/,/^fi$/ s/^  exit 1$/  :/' bin/omarchy-update
bash test/shell.d/update-refuse-root-test.sh

The sed changes exactly one line:

 if (( EUID == 0 )); then
   echo "Error: run omarchy-update as your user, not under sudo." >&2
   echo "It asks for a password when a step needs privilege." >&2
-  exit 1
+  :
 fi

Actual output, exit status 0

ok - omarchy-update refuses to run as root so mise stays user-owned

Why it passes

The command the test runs, against the mutated script:

unshare --user --map-root-user env OMARCHY_PATH="$PWD" OMARCHY_UPDATE_LOGGED=1 \
  /usr/bin/bash -p "$PWD/bin/omarchy-update" -y

Actual stderr, exit status 1:

Error: run omarchy-update as your user, not under sudo.
It asks for a password when a step needs privilege.
sudo: /etc/sudo.conf is owned by uid 65534, should be 0
sudo: /etc/sudo.conf is owned by uid 65534, should be 0
sudo: PERM_SUDOERS: setresuid(-1, 1, -1): Invalid argument
sudo: unable to open /etc/sudoers: Invalid argument
sudo: error initializing audit plugin sudoers_audit
sudo: /etc/sudo.conf is owned by uid 65534, should be 0
sudo: /etc/sudo.conf is owned by uid 65534, should be 0
sudo: PERM_SUDOERS: setresuid(-1, 1, -1): Invalid argument
sudo: unable to open /etc/sudoers: Invalid argument
sudo: error initializing audit plugin sudoers_audit
Could not invalidate cached sudo authorization.

The sudo-boundary version catches it

The test in the other reproducer (update-refuse-root-reset-env-test.sh,
built on test/shell.d/fixtures/sudo-boundary-test.sh) run against the same
mutated script, exit status 1:

Error: run omarchy-update as your user, not under sudo.
It asks for a password when a step needs privilege.
not ok - root with OMARCHY_PATH is refused

Environment: bubblewrap sandbox, bash 5.3.15, util-linux 2.42.3 unshare.

Other open PRs for this issue
  • Refuse to run omarchy-update as root #13660: same guard, also after the source-root check, test is grep-only; over 260 commits behind quattro, bundles an unrelated Windows VM change, and adds the same test file path (add/add conflict with this PR).

Related open PRs that reference other issues:

Details

Verified (sandbox, root simulated with unshare --user --map-root-user):

  • Root with OMARCHY_PATH kept is refused before any sudo call, trap, log or update step; at the base, all update steps ran with euid 0. From source, sudo -E, sudo -i and su keep OMARCHY_PATH.
  • As a normal user, all update steps still run. From source, the menu and first-run callers launch the update as the session user.
  • omarchy commands --check passes.

Optional notes:

  • omarchy-channel-set ends by running omarchy-update, so as root with OMARCHY_PATH kept it switches packages, then fails and suggests a rerun that fails the same way. A root refusal at its start, as in omarchy-plymouth-set, avoids the half-finished switch.
  • requires-sudo=true on omarchy-update is what led to the sudo call in omarchy-update is annotated omarchy:requires-sudo=true, but running it as root makes the mise step write root-owned files into the user home #13329. Dropping it, or clarifying in agents/skills/command-metadata.md that it means the command prompts for sudo itself, avoids the contradiction with the new refusal.
  • Run as root (namespace), six existing update and sudo-boundary tests fail at this head but pass at the base; this matters only if the suite is run as root.

Review information

Test scope: Source and sandbox tests at a9a22c9, root simulated in a user namespace; sudo never ran as root.

AI process: Opus 5.5 Medium coordination and synthesis, Opus 5.5 Xhigh technical review and final fact check, GPT 6 Sol Xhigh search for related issues, Opus 5.5 Medium editorial check.

Opt out: To stop receiving these reviews, reply to this comment saying so.

@omarchybot

Copy link
Copy Markdown
Collaborator

Reviewed at a9a22c9. The bug is real: sudo omarchy update runs mise and the user hooks as root, which is what #13329 reports, and refusing root at the entry point is the right kind of fix. Three other open pull requests add the same refusal: #11479, #13660 and #12957. Only one of them should be brought forward, and on reading all four I would take #11479 rather than this one.

The difference that matters is where the guard sits. Here it runs after omarchy_security_require_source_root (bin/omarchy-update:19). Plain sudo resets the environment, so on a stock install root arrives without OMARCHY_PATH, and the source-root check exits first with OMARCHY_PATH does not match this Omarchy command. That means the usual sudo omarchy update from #13329 never reaches the new message. Only sudo -E, sudo -i or su do. #11479 refuses root right after environment sanitising, before the source-root check, the cleanup traps and sudo -k, so plain sudo gets the explanation.

The tests differ too. test/shell.d/update-refuse-root-test.sh always passes OMARCHY_PATH and checks only the exit status and the message. With exit 1 removed, the update goes on and a later sudo call fails inside the namespace, so the test still passes. #11479's test runs the real script through the sudo-boundary fixture, with and without OMARCHY_PATH. It asserts that nothing is logged and sudo is never touched, and it checks that a normal user still enters the update.

Codex Medium was given all four pull requests as a second opinion, without being told which one I picked, and it also chose #11479 for the same two reasons. Its independence from this review is not guaranteed. It also noted that #13660 and #12957 place their guards later still, after the trap and sudo -k setup. Nothing was run on a worker for this pull request: running a fix that is not the one going forward would cost a worker and verify nothing anyone needs. #11479 was verified on a worker at an earlier head.

Nothing is needed from you. Choosing between these is the maintainer's call, so this waits on them.

@omarchybot

Copy link
Copy Markdown
Collaborator

Thank you for this, @Chessing234. #11479 got there first with a fix for the same problem, so this is closed in favour of it, and the credit stays with the first fix. If it does not cover your case, please say so there.

Closed at the maintainer's request. The review comment above has the details; it was done by Claude Opus 5.5 with Codex Medium as a second opinion, whose agreement is not independent.

@omarchybot omarchybot closed this Oct 3, 2026
@jandrusk jandrusk mentioned this pull request Oct 7, 2026
1 task done
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

omarchy-update is annotated omarchy:requires-sudo=true, but running it as root makes the mise step write root-owned files into the user home

3 participants