cask/uninstall: skip signalling other users' processes - #24082
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Copilot review overview
馃煝 Approval recommended
The ownership filtering and tests are covered; the remaining warning-wording issue is a minor nit.
Review effort: Lite
Findings: 1
What changed in this PR
Updates macOS cask uninstallation to signal only processes owned by the current user, while skipping unknown or foreign owners.
Changes:
- Adds real UID lookup via
LibProc.real_uid. - Filters process signalling by real UID.
- Adds tests for UID lookup and filtering behavior.
| File | Summary |
|---|---|
Library/鈥婬omebrew/鈥媡est/鈥媜s/鈥媘ac/鈥媐fi/鈥媗ibproc_spec.rb |
Tests real UID lookup. |
Library/鈥婬omebrew/鈥媡est/鈥媍ask/鈥媋rtifact/鈥媢ninstall_spec.rb |
Tests process filtering. |
Library/鈥婬omebrew/鈥媡est/鈥媍ask/鈥媋rtifact/鈥媠hared_examples/鈥媢ninstall_zap.rb |
Updates signal test setup. |
Library/鈥婬omebrew/鈥媜s/鈥媘ac/鈥媐fi/鈥媗ibproc.rb |
Adds real UID process inspection. |
Library/鈥婬omebrew/鈥媏xtend/鈥媜s/鈥媘ac/鈥媍ask/鈥媋rtifact/鈥媋bstract_uninstall.rb |
Exposes macOS owner lookup. |
Library/鈥婬omebrew/鈥媍ask/鈥媋rtifact/鈥媋bstract_uninstall.rb |
Filters signalling targets by UID. |
馃挕 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
MikeMcQuaid
reviewed
Sep 24, 2026
MikeMcQuaid
approved these changes
Sep 25, 2026
Member
|
Thanks @hyuraku! |
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
Sep 25, 2026
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.

uninstall signal:sends the signal to all PIDs for the bundle ID in oneProcess.killcall. If another user's PID comes first,Errno::EPERMstops the current user's app from being signalled while its files are
still removed. A
TODOasked to check the owner of each PID.Now only PIDs whose real UID matches
Process.uidare signalled, andother users' PIDs or PIDs with an unknown owner are skipped with a
warning. I used the real UID and not the effective UID because the kernel
checks the target's real (or saved) UID when it decides if a signal is
allowed.
The UID comes from
proc_bsdshortinfovia a newLibProc.real_uid, whichLibProc.parent_pidalready reads, so no extra process is spawned.Behaviour is unchanged when all PIDs are the current user's. I added tests
for other users' PIDs, for an unknown owner, and for
LibProc.real_uid.brew benchmarkresults.brewcommands to reproduce the bug?brew lgtm(style, typechecking and tests) locally?AI (Claude Code) was used to review the existing diff, and draft this PR description.
I reviewed its output and verified the changes with the targeted spec and brew lgtm.