Repository navigation
land: kill-survivors - #848
Open
aft-alfonso[bot] wants to merge 12 commits into
Open
aft-alfonso[bot] wants to merge 12 commits into
aft-alfonso[bot] wants to merge 12 commits into
Conversation
After a timeout, bash_kill or cancelled call, AFT lists the task's process group again and names any member still running (pid, owner, command) in live_descendants, the task text and the completion notice, with one line that says the process survived the kill. A null list now always carries live_descendants_unavailable with the reason the group could not be checked. - macOS listing no longer drops another user's process: proc_name fails with EPERM for it, so owner and name now come from kern.proc.pid. - Linux reports listing_failed instead of an empty list when /proc cannot be read, and records each member's owner and command line. - PTY tasks are checked after AFT kills their group. - A terminal state published while a kill was in flight no longer skips the check. Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com>
…istry Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com>
A process-group check used to read the command line and owner of every member and only then keep the first 16. It now lists members by pid and group (cheap), sorts them, describes at most LIVE_DESCENDANT_CAP and counts the rest without reading them. bounded_members_stops_describing_at_the_cap pins the bound. Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com>
Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com>
A survivor's command was its full argument list, so a lingering `client --token "$DEPLOY_TOKEN"` saved the expanded token in live_descendants, the survivor line, the task record and the completion notice. The completion masker cannot be relied on here: its patterns do not recognise argv-shaped secrets (--token X, --password=X, KEY=value), and the exact-value list only knows secret-named environment variables, not a value read from a file. The shown command is now the executable name plus a fixed '…' when the process had (or may have had) arguments; argv0 keeps only its first word, and both pass through aft_redactor. Tests that pinned 'sleep 30' now expect 'sleep …' because the arguments are no longer kept, which is the point of this change; the approved survivor line template is unchanged. Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com>
With /proc mounted hidepid, or any process whose stat cannot be read, the group probe said the group still existed while the listing found nobody, and the task reported live_descendants: [] - a clean kill. A listing now counts each /proc entry as a member, a zombie, elsewhere, or unreadable. If any entry is unreadable, or the group is alive but the listing found no member or zombie, the group is probed once more: if it has exited in the meantime the result is a clean [], otherwise the new live_descendants_unavailable reason listing_incomplete. macOS applies the same rule to an empty proc_listpgrppids answer. Test hooks hide or make unreadable a group's members to reproduce both mount modes. Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com>
BashCompletedFrame, BgCompletion and the plugin types that mirror them
described a live descendant as {pid, comm, argv0} and had no
live_descendants_unavailable. The bridge now exports a LiveDescendant type
with the optional user and command fields, and the unavailable reasons as
LIVE_DESCENDANTS_UNAVAILABLE_REASONS with a type derived from it. A unit test
reads the engine's LiveDescendantsUnavailable enum and fails if the two lists
drift.
Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com>
…830) macOS /bin/sh is bash, so the survivor's kernel name is bash while its first argument is sh. The test now finds the survivor by argv0 and expects its shown command to be its own name plus the omitted-arguments marker. Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com>
Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com>
) With /proc mounted hidepid=1, any unreadable process, even one outside the task's group, made the group check return unavailable and discard the survivors it had found, so live_descendants was null. A later bash_kill on the finished task then counted zero survivors and sent no signal, leaving the group running. The listing now returns the members it identified with an incomplete flag. The record keeps those members next to live_descendants_unavailable = listing_incomplete; only an incomplete listing that found nobody stores null, so [] still always means a clean check. The task text and completion notice add one sentence (INCOMPLETE_LISTING_NOTE) whenever the listing was incomplete. bash_kill on a finished task now signals its group when members were recorded, or when the last check was incomplete or missing and the group still answers signal 0. A group last seen empty is left alone, since an empty group never regains members and a group answering under that id later is a different one. Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com>
…its own (#830) A later bash_kill on a finished task signalled whatever group answered under its saved number whenever its last check was incomplete or missing (a PTY task, for example). Once the original group has exited, the system can give that number to an unrelated group, which would then be signalled. Each check now records the identity (pid and start time) of the members it found. A finished task's group is signalled only if its leader (the saved number, with the start time recorded at launch) or one of those members is still running in that group with the same start time. Without that evidence the group is left alone, and the record keeps its survivor and unavailable fields as they were. The previous test that signalled a fully hidden group whose leader had exited now asserts the opposite: nothing proves that group is still the task's, which is the case this change exists to refuse. The partial case, where an incomplete listing found the survivor, is still signalled. Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com>
…nly those shown (#830) group_member_identities held only the 16 members a task shows. When a finished task left more than 16 processes and the shown ones exited while a member that was only counted (+N more) kept running, bash_kill found no matching identity and sent nothing. Each check now records the identity (pid and start time) of every member it finds, separately from the 16 shown, up to GROUP_IDENTITY_CAP (1,024), and sets group_member_identities_truncated when it found more. A match on any saved identity is enough evidence to signal the group. A task record holding the most a check can store (1,024 identities at their largest values plus 16 shown members with names at their cap) measures 63,199 bytes; a test bounds it below 96 KiB. The identity cap is registered as a list surface. Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.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
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.
Accepted agent delivery; branch-keyed search descriptor included.
Summary by cubic
Reports processes that survive a bash kill. After a timeout,
bash_kill, or cancelled call, AFT now re-checks the task's process group and names any member still running inlive_descendants, the task text, and the completion notice — a process AFT cannot signal, such as one started withsudoand owned by another user, used to be reported as a clean kill.listing_incompleteand keeps the members it did find; an unreadable/procreportslisting_failed, and a null list always carries the reason. Abash_killon a finished task re-signals a group whose last check was incomplete or missing, but never one last seen empty.LiveDescendantand the unavailable reasons; docs now recommend bounding sudo commands yourself withtimeoutor a trap, since AFT cannot kill them.Written for commit 477f627. Summary will update on new commits.


Confidence Score: 5/5
The changes since the previous review appear safe to merge.Summary
AFT reports processes left running after a kill, without saving their arguments.
No new actionable issues were found in the changes since the previous review.
Diagram
%%{init: {'theme': 'neutral'}}%% flowchart TD A[Check task process group] --> B[Display up to 16 members] A --> C[Save up to 1,024 process identities] B --> D[Status and completion notice] C --> E[Later kill of finished task] E --> F{Saved process still in group?} F -->|Yes| G[Signal group and check survivors] F -->|No| H[Leave group alone]Reviews (5) · Last reviewed commit: "mason: recognise a finished task's group..." · Reviewed by Greptile