[codex] Complete passwordless sudo expiry across suspend

[AFOliveira]

Temporary passwordless sudo expires at the requested wall-clock deadline, including across suspend. The generated sudoers rule carries a UTC NOTAFTER deadline, and a calendar timer cleans up the expired file. Publication validates a complete temporary rule, arms its cleanup timer, and atomically renames the rule into place. Failed revocation remains an error.

The sudoers rule is now the only grant record, removing separate state files and timer-identity bookkeeping. Expiry callbacks inspect the current deadline, so earlier callbacks cannot shorten a renewed grant. Pending callbacks after renewal or manual disable finish within the maximum 24-hour grant window. The command shrinks from 573 to 421 lines, and the two focused test suites plus their shared fixture shrink from 693 to 424 lines. The shared security library and its interface remain unchanged.

Legacy cleanup records successful repair in a trusted, root-owned machine marker. Later accounts can finish their per-user migration queues without sudo and without revoking grants created after the repair. Cleanup preserves administrator-modified policy and reports failures.

This builds on merged #9387, preserving Erik Melton's work and Adolanium's co-authorship. It addresses publication-cleanup, status-error, package-race, trailing-$ account, duplication, and multi-user migration feedback. The package-path test accepts either a checkout or pkgbuilds/.

Grant publication and settings upgrade/removal share one lock. Publication requires the exact installed boot cleanup rule and aborting ALPM hook. Settings owns the helper and its cleanup prerequisites through companion omarchy-pkgs #257. The first upgrade from packages without that hook retains the documented legacy scriptlet fallback; later transactions can abort before files are removed.

Coordinated release order: merge this source, advance both #257 release pins/checksums to the actual upstream commit containing it, then build and publish the matching runtime/settings pair together.

Validation at c46f3216: 13 expiry groups, including native sudo policy evaluation before and after the generated deadline, and 6 lifecycle groups passed. The tests exercise native visudo, flock, and boot cleanup through systemd-tmpfiles against private fixtures, alongside mapped commands and package lifecycle scripts. The CLI suite, Bash syntax checks, and git diff --check also passed.

Fresh VM validation on 2026-09-11: all four x86_64 candidate packages were rebuilt from exact source c46f3216 with companion packaging b787991e. A fresh disposable Omarchy 4.0.2 VM installed the matched pair with normal dependency checks. Installed helper/library/boot-rule/hook hashes matched the source, and all four archives preserved exclusive settings ownership and exact runtime/settings version-release dependencies.

Normal one-minute expiry, actual suspend/resume across the deadline, normal reboot before expiry, and abrupt VM reset before expiry passed. Each expiry/restart check used cold, noninteractive sudo requests for /usr/bin/true and confirmed the generated rule was removed. Real settings reinstall, stable/dev transfers in both directions, runtime removal with expiry support retained, settings removal, ALPM transaction abortion on a controlled cleanup failure, and successful recovery passed. A real second account without sudo rights completed its migration without revoking a later grant. Grants were cleaned up and the disposable VM was shut down.

These are candidate-source builds using OMARCHY_SRC; host builds used makepkg --nodeps, while guest installation checked dependencies normally. Earlier independent review covered the previous implementation; no fresh independent review is claimed for this redesign. Final declared-source release builds, native ARM validation, and a fresh ISO remain release work.

[axelfontaine]

Regardless of the security aspects, there is way too much code duplication across this entire series! This should be reworked to extract the common methods to make it much easier to review.

[AFOliveira]

Regardless of the security aspects, there is way too much code duplication across this entire series! This should be reworked to extract the common methods to make it much easier to review.

I agree. The rate at which I'm catching these vulns is higher than my bandwidth, I need a better workflow to avoid becoming a slop-machine. I 'm reworking this PR

[AFOliveira]

Addressing Axel’s review before continuing the series:

• The dependent security PRs remain drafts; I am not pushing the 46-file integration refactor as a mega-PR.
• This branch now contains the minimal shared, source-only primitives used by this finding. Each surviving draft will be rebuilt to source the shared implementation and keep only its own production delta and regression.
• Commit 2f816a8 documents the narrow #!/bin/bash -p exception required to suppress BASH_ENV before the first command. The existing regression rejects ordinary Bash with a decoy -p argument.
• I withdrew [codex] OM-SEC-02: Keep Windows VM passwords out of process argv #9458, [codex] OM-SEC-06: Keep support data in private workspaces #9462, [codex] OM-SEC-11: Confine screen recorder state to a private runtime #9466, [codex] OM-SEC-13: Remove the package-conflict root-write primitive #9468, and [codex] OM-SEC-16: Validate source roots before privileged publication #9471 after local comparison showed that they were exact duplicates, worse than narrower existing fixes, or not an independent privilege boundary.

Local validation on 2f816a8 against exact base 945af75:

• git diff --check: pass
• changed-shell bash -n: pass
• nopasswd-sudo-expiry-test.sh: all 16 security cases pass
• CLI: all 116 cases pass
• full suite: 221/222 shell files pass; launch-about-test.sh fails the same a roomy window animates assertion on both candidate and exact base 945af75, so it is not a candidate regression

I will not mark any dependent draft ready until its copied helpers are removed and its final branch passes the same local gate.

[Copilot]

🟡 Changes recommended

Unresolved critical and moderate grant-lifecycle issues can leave passwordless policy active or revoke a newer grant incorrectly.

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

Pull request overview

Hardens temporary passwordless sudo grants with suspend-safe expiry, UID-bound state, and fail-closed cleanup.

Changes:

• Adds absolute wall-clock timers and transactional grant handling.
• Introduces shared security helpers and legacy cleanup.
• Expands documentation and regression coverage.

File summaries

File	Review
test/shell.d/nopasswd-sudo-expiry-test.sh	Nit: Normalize OMARCHY_PKGS_PATH to support both checkout-root and pkgbuilds/ paths.
migrations/1788163635.sh	Removes legacy grants and state; no issues found.
manual/48-security.md	Documents wall-clock expiry; no issues found.
etc/tmpfiles.d/omarchy-nopasswd-sudo.conf	Defines boot cleanup ownership; no issues found.
bin/omarchy-sudo-passwordless	Critical: Distinguish failed revocation from inactive status; preserve expiry until policy removal succeeds; bind expiry callbacks to the current timer identity. Moderate: Validate oversized integers before arithmetic; support usernames ending in $; recheck boot-cleanup prerequisites before success.
bin/omarchy-security-functions	Adds shared security primitives; no issues found.
AGENTS.md	Documents privileged Bash startup requirements; no issues found.

Review details

• Files reviewed: 5/7 changed files
• Comments generated: 1
• Review effort level: Balanced

---

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

[ErikMelton]

A publication failure can leave passwordless sudo active with no timer or state. After publish_rule fails, the code stops the timer before removing the policy. publish_rule can fail after installing the rule—for example, if deleting its temporary file fails—and interruption or failure of the subsequent rm leaves a live sudoers rule without expiry. Remove/confirm the policy first, using the fail-closed cleanup path, before stopping its timer. bin/omarchy-sudo-passwordless:346

Failed revocation is treated as “inactive.” When status detects malformed, expired, or timerless state, any cleanup failure is collapsed into exit status 1. The public flow interprets every status 1 as inactive and offers to enable; if the user declines, it prints “Aborted. No changes made” even though the passwordless rule may remain active. Cleanup failure needs a distinct status that aborts loudly.

Package removal can race grant publication. verify_boot_cleanup runs before timer creation and publication. The companion package’s post_remove does not share this command’s lock, so it can remove the tmpfiles rule and find no grant, after which this transaction publishes one and reports success. That grant then survives reboot without cleanup. Package removal and enablement need shared serialization, plus a final prerequisite check.

Supported usernames ending in $ are rejected and skipped by migration cleanup. Omarchy provisioning explicitly permits a trailing $, but valid_account_name does not. Those users cannot use the new command, and an exact legacy grant for such a user is classified as administrator-authored and left active while the migration reports success.

[Copilot]

🟡 Changes recommended

The unresolved status fast-path and per-user migration behavior can conceal or block failed cleanup.

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

Review details

• Files reviewed: 8/10 changed files
• Comments generated: 2
• Review effort level: Balanced

[ErikMelton]

Hopefully last comment!

Legacy migration can certify success while an unrestricted sudo rule remains. The previous implementation used unvalidated $USER directly in the sudoers filename and body. The new classifier only recognizes legacy rules whose suffix satisfies the new lowercase-only account pattern.

For a manually created or NSS account such as Alice, the exact legacy rule Alice ALL=(ALL) NOPASSWD: ALL is treated as unrelated. Cleanup therefore returns success and writes the permanent machine-wide migration marker, even though the unbounded rule remains active. The migration will never retry, and the new command rejects that account, preventing normal disablement.

Please recognize the exact legacy filename/body relationship without applying the new account-name restriction, or fail the migration while any legacy-shaped unrestricted rule remains.