[codex] OM-SEC-12: Bind update inhibitor cleanup to process identity - #9467
Draft
AFOliveira wants to merge 27 commits into
Draft
[codex] OM-SEC-12: Bind update inhibitor cleanup to process identity#9467AFOliveira wants to merge 27 commits into
AFOliveira wants to merge 27 commits into
Conversation
AFOliveira
marked this pull request as draft
September 1, 2026 09:49
…view-9469-20260906
…dex/portfolio-9457-20260907
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Cancellation can leak an inhibitor, one caller reopens reusable sudo after a user hook, and lifecycle coverage contains an unreachable security test.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Hardens updater authorization, sleep-inhibitor identity, and temporary passwordless-sudo lifecycle across the stacked security changes.
Changes:
- Adds private, identity-bound inhibitor state and cancellation handling.
- Introduces command-scoped sudo boundaries and safer update sequencing.
- Adds wall-clock passwordless-sudo expiry, cleanup hooks, documentation, and tests.
File summaries
| File | Description |
|---|---|
AGENTS.md |
Documents privileged Bash shebang exception. |
bin/omarchy-channel-set |
Secures channel transitions and deferred hooks. |
bin/omarchy-refresh-pacman |
Adds cold sudo and hook modes. |
bin/omarchy-remove-ai-hermes |
Relies on invariant gum availability. |
bin/omarchy-remove-ai-openclaw |
Relies on invariant gum availability. |
bin/omarchy-restart-shell |
Waits for notification readiness. |
bin/omarchy-security-functions |
Adds shared security-boundary helpers. |
bin/omarchy-sudo-passwordless |
Reworks bounded grant lifecycle. |
bin/omarchy-update |
Reorders update trust boundaries. |
bin/omarchy-update-aur-pkgs |
Configures no-update sudo for yay. |
bin/omarchy-update-restart |
Splits service and reboot phases. |
bin/omarchy-update-stay-awake |
Adds private identity-bound inhibitor control. |
config/omarchy/hooks/pre-refresh-pacman.d/add-custom-repo.sample |
Updates hook timing guidance. |
default/agents/skills/omarchy/hooks.md |
Documents secure hook lifecycle. |
default/libalpm/hooks/05-omarchy-passwordless-revoke.hook |
Revokes grants before package changes. |
default/omarchy/sudo-no-update/sudo |
Adds no-update sudo wrapper. |
docs/passwordless-sudo.md |
Documents grant architecture and validation. |
docs/update-process.md |
Documents revised update lifecycle. |
etc/tmpfiles.d/omarchy-nopasswd-sudo.conf |
Clarifies boot-only grant cleanup. |
manual/48-security.md |
Expands passwordless-sudo user guidance. |
migrations/1788163635.sh |
Cleans legacy temporary grants. |
test/shell.d/channel-sudo-boundary-test.sh |
Tests channel authorization boundaries. |
test/shell.d/channel-test.sh |
Updates channel sequencing tests. |
test/shell.d/fixtures/sudo-boundary-test.sh |
Adds shared safe sudo fixtures. |
test/shell.d/nopasswd-sudo-expiry-test.sh |
Expands expiry and package tests. |
test/shell.d/passwordless-grant-lifecycle-test.sh |
Tests grant failure lifecycle. |
test/shell.d/restart-shell-test.sh |
Tests notification readiness waiting. |
test/shell.d/security-entrypoint-symlink-test.sh |
Tests symlinked secure entrypoints. |
test/shell.d/security-source-root-test.sh |
Tests canonical source-root enforcement. |
test/shell.d/update-disk-space-test.sh |
Adopts shared boundary fixtures. |
test/shell.d/update-hook-security-test.sh |
Tests hook credential isolation. |
test/shell.d/update-lock-test.sh |
Expands lock and cancellation coverage. |
test/shell.d/update-restart-phases-test.sh |
Tests split restart phases. |
test/shell.d/update-sequence-test.sh |
Updates expected phase ordering. |
test/shell.d/update-stay-awake-security-test.sh |
Tests inhibitor state security. |
test/shell.d/update-user-path-test.sh |
Tests user PATH preservation. |
Review details
- Files reviewed: 25/36 changed files
- Comments generated: 2
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+17
to
+18
| ├── post-update.d/ # At the end of `omarchy update`, after privileged work | ||
| ├── pre-refresh-pacman.d/ # After `omarchy refresh pacman` finishes (legacy name) |
| namespace_capable=0 | ||
| # Cross-UID adversarial execution is intentionally excluded from this safe fixture. | ||
| # Ownership rejection is covered above with caller-owned benign files. | ||
| if false && (( ${#namespace_args[@]} > 0 )) && |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Binds update-inhibitor cleanup to a private state directory and the actual process owner, PID/start time and per-launch token. State files are validated and published atomically; cleanup only signals the matching owned inhibitor. Finding: OM-SEC-12.
This PR is now a direct child of #9469, which in turn depends on #9457. It retains their exact updater, shared helper and no-update wrapper. Its five-file delta adds the inhibitor protections and focused lifecycle documentation/tests, avoiding the previous standalone updater that referenced an absent wrapper.
A private per-launch control handshake handles cancellation before publication. The child must observe the active token while holding its control lock; rollback cancels that token before examining any published process identity. The child opens the control file read-only, so a delayed launch cannot recreate cancellation-unlinked state. The held child drops to the invoking user and execs sleep with its token retained in argv; stop needs no additional authentication.
At
0240113d, independent Daybreak review accepts the integration and cancellation fixes. Eleven lock/lifecycle groups, twelve identity/private-state groups, focused update/source/entrypoint suites, CLI112, syntax and whitespace checks pass. The fixtures cover delayed terminal and graphical publication, failure/cancellation, stale state and repeat start/stop without invoking host privilege.Order: #9457 → #9469 → #9467 → #9472/#9474. Installed TTY validation with the matching #9469 wrapper passed two cold start/stop cycles: each start used one password prompt, each stop used none, the PTY returned promptly, exactly one owned inhibitor existed, and private state and idle behavior were restored. Graphical/pkexec cancellation and complete delivered-stack update prompt validation remain release gates. #9473 stays closed; this PR does not resurrect the rejected legacy upgrade design.