Remove the command-line fallback from socket owner resolution - #417
Merged
yummybomb merged 1 commit intoAug 17, 2026
Merged
Conversation
A process matching the socket path by command line only can never authorize teardown, so the fallback's sole effect was distinguishing provable death from ambiguity. The fd scan runs with CAP_SYS_PTRACE (hypeman runs as root or with full caps), so it cannot miss a live owner and a missing listener already proves the hypervisor is gone. The fallback was also actively harmful: a debug client holding the socket path in its argv (ch-remote, socat) resolved as an unconfirmed live match and wedged stop/delete until it exited. Resolution now trusts the listener scan alone: a confirmed owner is returned, no owner classifies as provable death, and only a failed scan fails closed.
Contributor
Author
|
folding this into #363 directly — same review scope |
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.
Summary
Stacked on #363. Deletes
pidByCmdlineand theconfirmedflag from socket owner resolution, collapsingclassifyResolvedHypervisorOwnerfrom an 8-branch matrix to 4 cases.A command-line-only match could never authorize teardown — destructive paths already refused unconfirmed PIDs — so the fallback's only effect was distinguishing provable death from ambiguity. That distinction only matters if the fd scan can miss a live owner, which requires the caller to lack
CAP_SYS_PTRACE. Hypeman runs as root (dev) or with a full capability set (prod), so the listener scan is authoritative: a confirmed owner is returned, a missing listener is proof the hypervisor is gone, and only a failed scan fails closed.The fallback was also actively harmful: a debug client carrying the socket path in its argv (
ch-remote,socat) resolved as a live unconfirmed match and wedged stop/delete with a 500 until the client exited. That case now correctly classifies as provable death and the bystander is never signaled.Assumption made explicit: resolution correctness now depends on hypeman retaining ptrace-equivalent capabilities (root or
CAP_SYS_PTRACE). This is documented onResolveProcessPID. If the service is ever de-privileged below that, the fd scan can miss owners and this needs revisiting.Testing
go vetclean;go test -race ./lib/hypervisor/ ./lib/instances/targeted identity/kill/shutdown/backfill suites pass locally (TestSocketCacheKeyChangesWhenSocketIsRecreatedfails in this sandbox on the unmodified base too — inode reuse in /tmp)/proc: all passErrNoOwningProcessinstead of failing closedTestResolveProcessPIDIgnoresCommandLineBystander,TestKillHypervisorIgnoresCommandLineBystander,TestShutdownHypervisorIgnoresCommandLineBystander, reworkedTestClassifyResolvedHypervisorOwnertable