Repository navigation
fix(security): force-stop a hung root child on timeout (#142) - #164
Merged
Merged
Conversation
Follow-up to #140. The daemon runs as User=sysknife and elevates through sudo, so it cannot signal the root child a sudo action forks; #140 detected this (ActionNotStopped) and vetoed rollback, but the work kept running. kill_and_reap now escalates: after its own SIGTERM/SIGKILL to the process group, if the group survives (root members it cannot signal) it runs the privileged reaper — - `sudo -n /usr/bin/kill -s TERM -- -<pgid>` then `-s KILL` to signal the action's own process group as root (reaching the root children via the existing /usr/bin/kill sudoers grant; -n never prompts, -- makes the negative pgid a group not a flag), or - `sudo -n /usr/bin/rpm-ostree cancel` for an rpm-ostree transaction, which runs in rpm-ostreed's own cgroup and cannot be reached by a group signal. ActionNotStopped is now a genuine last resort (the reaper failed), not the expected path for every sudo action. No new sudoers grant is required. Tests: root_group_kill_argv and is_rpm_ostree_action are unit-tested; the same-uid path still returns a plain stopped-timeout without escalating. The cross-uid root-child termination is inherently root-only and is validated on a live VM (the unit tests can only cover the same-uid path, per the issue). Closes #142 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01R9dpzc8CXrPDksbHmndT8f
…fety Three reviewers (code, silent-failure, tests): - The pid-reuse concern (root-killing a recycled pgid) is refuted: POSIX keeps a process-group id allocated for the group's whole lifetime, so a probe that reports the group alive guarantees the pgid still names THIS action's group, never a recycled unrelated process. Documented at the escalation site. - Escalate only for AliveUnreachable (EPERM) — members running as root, the case a root signal actually helps. A same-uid D-state survivor or an Unknown errno now falls through to ActionNotStopped instead of a pointless sudo round-trip. - run_sudo now logs the escalation's exit status / spawn error / timeout, so a "could not be confirmed stopped" line can be told apart from a misconfigured sudoers grant or missing sudo. - Pinned the rpm-ostree assumption in a comment: the client blocks until the transaction resolves, so the client-group probe correctly reflects it. - Extracted a pure reap_command (dispatch + argv unit-tested for both branches) and added a cfg(test) SUDO_INVOCATIONS counter so the same-uid test proves the escalation does NOT fire for a killable group. Added is_rpm_ostree edge cases. The cross-uid root kill remains VM-gated per the issue; the primitive itself is the same sudo-kill the KillProcess action already exercises. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01R9dpzc8CXrPDksbHmndT8f
vladimirrott
force-pushed
the
fix/142-privileged-reaper
branch
from
July 31, 2026 13:05
6029007 to
79758f6
Compare
vladimirrott
added a commit
that referenced
this pull request
Aug 16, 2026
* fix(security): force-stop a hung root child on timeout (#142) Follow-up to #140. The daemon runs as User=sysknife and elevates through sudo, so it cannot signal the root child a sudo action forks; #140 detected this (ActionNotStopped) and vetoed rollback, but the work kept running. kill_and_reap now escalates: after its own SIGTERM/SIGKILL to the process group, if the group survives (root members it cannot signal) it runs the privileged reaper — - `sudo -n /usr/bin/kill -s TERM -- -<pgid>` then `-s KILL` to signal the action's own process group as root (reaching the root children via the existing /usr/bin/kill sudoers grant; -n never prompts, -- makes the negative pgid a group not a flag), or - `sudo -n /usr/bin/rpm-ostree cancel` for an rpm-ostree transaction, which runs in rpm-ostreed's own cgroup and cannot be reached by a group signal. ActionNotStopped is now a genuine last resort (the reaper failed), not the expected path for every sudo action. No new sudoers grant is required. Tests: root_group_kill_argv and is_rpm_ostree_action are unit-tested; the same-uid path still returns a plain stopped-timeout without escalating. The cross-uid root-child termination is inherently root-only and is validated on a live VM (the unit tests can only cover the same-uid path, per the issue). Closes #142 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01R9dpzc8CXrPDksbHmndT8f * fix(security): address #142 review — narrow escalation, prove pgid safety Three reviewers (code, silent-failure, tests): - The pid-reuse concern (root-killing a recycled pgid) is refuted: POSIX keeps a process-group id allocated for the group's whole lifetime, so a probe that reports the group alive guarantees the pgid still names THIS action's group, never a recycled unrelated process. Documented at the escalation site. - Escalate only for AliveUnreachable (EPERM) — members running as root, the case a root signal actually helps. A same-uid D-state survivor or an Unknown errno now falls through to ActionNotStopped instead of a pointless sudo round-trip. - run_sudo now logs the escalation's exit status / spawn error / timeout, so a "could not be confirmed stopped" line can be told apart from a misconfigured sudoers grant or missing sudo. - Pinned the rpm-ostree assumption in a comment: the client blocks until the transaction resolves, so the client-group probe correctly reflects it. - Extracted a pure reap_command (dispatch + argv unit-tested for both branches) and added a cfg(test) SUDO_INVOCATIONS counter so the same-uid test proves the escalation does NOT fire for a killable group. Added is_rpm_ostree edge cases. The cross-uid root kill remains VM-gated per the issue; the primitive itself is the same sudo-kill the KillProcess action already exercises. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01R9dpzc8CXrPDksbHmndT8f --------- Co-authored-by: vladimirrott <vladimirrott@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
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
1 participant
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.
What
Follow-up to #140. The daemon runs as
User=sysknifeand elevates throughsudo, so it cannot signal the root child asudoaction forks. #140 detected this (ActionNotStopped) and vetoed rollback, but the privileged work kept running to completion. #142 is the termination half.Fix
kill_and_reapescalates: after its own SIGTERM/SIGKILL to the process group, if the group survives (root members), it invokes the privileged reaper:sudo -n /usr/bin/kill -s TERM -- -<pgid>then-s KILL— signals the action's own process group as root, reaching the root children (via the existing/usr/bin/killsudoers grant;-nnever prompts,--makes the negative pgid a group not a flag), orsudo -n /usr/bin/rpm-ostree cancel— for an rpm-ostree transaction, which runs inrpm-ostreed's own cgroup and cannot be reached by a group signal.ActionNotStoppedis now a genuine last resort (the reaper itself failed), not the expected path for every sudo action. No new sudoers grant is required.Why the sudoers-kill approach, not systemd-run scope
The issue lists both. The
systemd-run --scopewrapping would change the spawn happy-path for every privileged action; the sudoers-authorised root kill keeps the happy-path untouched and uses standard POSIX group-signal semantics on the daemon's own spawned group, which is far safer to ship. rpm-ostree's cross-cgroup case is handled explicitly withcancel.Tests / validation
root_group_kill_argvandis_rpm_ostree_actionare unit-tested.kill_and_reap_confirms_...test).Closes #142