[codex] OM-SEC-14: Run update hooks without reusable sudo authority

[AFOliveira]

Keeps update hooks, AUR builds, mise and channel hooks outside authorization created by the update workflow. It starts with canonical privileged-Bash/shared-helper validation and cold revocation, uses fixed no-update sudo dispatch, finishes privileged stages before user code, and revokes on errors, cancellation and completion. Finding: OM-SEC-14.

Reporter credit: Roger Piñol (R08) and Sean Huber reported earlier update/Pacman authorization-reuse cases; Afonso Oliveira independently disclosed OM-SEC-14 and authored this remediation. Sean also reported the separately tracked Stay Awake occurrence.

Candidate 75e58f03 builds on #9457's exact shared foundation and current Quattro. Earlier feedback about sudo option forwarding, unreachable tests, PATH preservation, hook timing and cleanup is reflected in the final implementation. The deferred pacman-refresh hook runs once after the completed channel transition.

The inhibitor identity protection is now a compatible child PR: #9467 is based on this exact candidate and preserves this updater/helper/wrapper while adding private state, full process identity and cancellation-safe launch. Order: #9457 → #9469 → #9467, then #9472/#9474. #9474 must also integrate #9463's SSH proof transaction. #9473 remains closed; its rejected 163-prompt legacy upgrade is not a successful validation or dependency.

Independent Daybreak review found no new defect in the update/channel authorization boundary. Thirteen focused suites (157 groups), CLI112, syntax and whitespace checks pass. The integrated inhibitor received separate reviews and tests in #9467.

Release gates remain: exact installed package-stack validation, measured normal update/channel prompt counts and cancellation, and #9474's migration security/UX integration. Command-scoped sudo can prompt for each privileged operation; -y does not bypass authentication. This source validation does not establish usable legacy upgrade authentication or account-wide isolation from unrelated concurrent sudo sessions.

[Copilot]

🟡 Changes recommended

Several required integrations are missing, leaving credential-reuse paths open while the relevant regression tests are unreachable.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Hardens update hooks against reusable sudo credentials by introducing cold credential boundaries and reordering user-controlled stages.

Changes:

• Adds a sudo -N wrapper and trusted source validation.
• Moves hooks and mise after privileged update stages.
• Adds security regression tests and updates documentation.

File summaries

File	Description
bin/omarchy-update	Reorders update stages and manages sudo boundaries.
bin/omarchy-refresh-pacman	Moves or defers the legacy refresh hook.
default/omarchy/sudo-no-update/sudo	Adds the no-update sudo wrapper.
test/shell.d/update-hook-security-test.sh	Adds security regression scenarios.
test/shell.d/update-sequence-test.sh	Updates expected stage ordering.
docs/update-process.md	Documents the revised update process.
default/agents/skills/omarchy/hooks.md	Documents hook timing and authorization.
config/omarchy/hooks/pre-refresh-pacman.d/add-custom-repo.sample	Updates sample hook guidance.

Review details

• Files reviewed: 6/8 changed files
• Comments generated: 4
• Review effort level: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[sayem314]

@robosayem review and audit this properly, and if you have any opinion or suggestions then share it.

[robosayem]

Full audit done — I ran the whole thing, not just read it. This is a well-constructed security boundary and I found no exploitable gap. Verdict first: no blocking findings, four non-blocking notes, one of which is an interaction worth a decision.

What the PR does: update/channel-switch entrypoints start privileged Bash cold (-p enforced, decoy -p argument rejected), scrub BASH_ENV/ENV/SHELLOPTS/exported functions, pin OMARCHY_PATH to the canonical checkout or packaged /usr/bin, revoke any pre-existing sudo ticket, then scope all privileged work to sudo -N through a PATH-shim wrapper, with sudo -k traps on every exit path. The privilege ordering matters: system packages → migrations → AUR → hooks → mise, with a sudo -k before user code so a detached malicious child has no future ticket to wait for. This directly closes the R08/OM-SEC-14 class.

Verification on head 0add43f:

• All 12 test suites pass here: security-source-root, security-entrypoint-symlink, update-hook-security, channel-sudo-boundary, update-user-path, update-lock, update-restart-phases, update-sequence, update-disk-space, channel, restart-shell, update-dev. The boundary fixture asserts real properties (no cached credential left, final sudo -k in the log, strict phase ordering, decoy -p rejection, BASH_ENV and exported-function non-propagation) — not just exit codes
• shellcheck -S warning is clean across all nine touched shell entrypoints
• Copilot's four Sep 4 findings are all resolved at this head: the wrapper no longer inserts -- so it stays transparent to sudo options; docs no longer overclaim (the migration-standalone boundary is explicitly scoped out and honest about it); the hostile-yay section reads as fixture guidance inside the rewritten 123-line test, which runs to completion (I ran it); the kernel-compare == quoting nit stands — a kernel with glob metacharacters can't occur, cosmetic only

Non-blocking notes:

1. UX consideration, not a bug: stock Omarchy grants no NOPASSWD for update commands (only dns/theme/timezone), so sudo -N + cold start means password prompts per privileged command in a fresh-run update: pkg-prune, snapshot, system-pkgs, stay-awake inhibitor, keyring steps each can prompt. That is the security posture chosen and the docs are explicit about it, but the update just went from one password to potentially several. Worth a deliberate call before merge — either accept it in docs/release notes or reconsider after feedback. One mitigating detail I verified: the sudo wrapper still honors -k/-K/-h/-V transparently, and omarchy-sudo-passwordless users can re-enable NOPASSWD during updates.

2. bin/omarchy-pkg-install/omarchy-pkg-aur-install still source omarchy-sudo-keepalive, which runs a background sudo -v refresh loop outside the update boundary. That's outside this PR's stated scope (interactive installs, not update hooks), but the same OM-SEC-14 class — worth a follow-up card so it isn't lost.

3. The stay-awake rewrite now drops privilege via setpriv --reuid after a single foreground sudo -N authentication, writes the inhibit PID + start-time from inside the child, and stop revalidates both before killing — PID-reuse spoofing by a same-UID process is bounded correctly. The 5 s/100-attempt readiness polling caps are sensible. Nice work.

4. omarchy-security-functions includes an unused-but-exported omarchy_security_sudo_supports_no_update public path (used internally by the wrapper check) — harmless; the library's "internal only, exit 64 if sourced as a program" guard is a good touch.

Ship it. This is exactly the remediation pattern the other sudo-reuse findings (OM-SEC-17, 21, 23) should follow.

[Copilot]

🟡 Changes recommended

A remaining caller exposes hook code to later sudo authentication, and grant cleanup can delete administrator-modified policy.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

• Files reviewed: 24/35 changed files
• Comments generated: 1
• Review effort level: Balanced

[ErikMelton]

LGTM, pending hook doc fix.