Replace macOS ps exec with in-process sysctl-based Unity process discovery#1882
Merged
hatayama merged 2 commits intoJul 20, 2026
Conversation
…ocess discovery Sandboxed environments (e.g. Claude Code's default sandbox) deny exec of external binaries including /bin/ps, which broke Unity Editor discovery on macOS entirely. Replace listUnityProcessesMac's `ps -axo` invocation with kern.proc.all / kern.procargs2 sysctl reads via golang.org/x/sys/unix, which the sandbox permits. Matching/filtering logic (isUnityEditorCommand, extractProjectPath) is unchanged and now shared via matchMacUnityProcess so both the old ps-based tests and the new sysctl path exercise the same rule. Darwin-only syscall code is isolated behind a //go:build darwin file (process_darwin.go, with a process_other.go stub for other OSes) because this repo's CI has no macOS runner; the buffer parser and matcher stay in the shared, build-tag-free process.go so ubuntu-latest CI still covers them. Verified via a direct Go probe that FindRunningUnityProcess succeeds inside Claude Code's default sandbox (where ps/pgrep themselves fail with "operation not permitted"), and that `uloop launch -r` still succeeds outside the sandbox with no regression. Note: -r/-q's kill of the found process is a separate, pre-existing sandbox restriction (kill(2) against a non-child PID) that this change does not and cannot resolve. Also promotes golang.org/x/sys from an indirect to a direct dependency (go mod tidy) and refreshes cli/project-runner and cli/dispatcher's shared-inputs-stamp.json per the shared release-input trigger convention.
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughmacOS Unity process discovery now uses Darwin sysctl APIs, decodes ChangesUnity process discovery
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant listUnityProcessesMac
participant kern.proc.all
participant macProcessArgs
participant parseMacProcArgs2
participant matchMacUnityProcess
listUnityProcessesMac->>kern.proc.all: enumerate process records
listUnityProcessesMac->>macProcessArgs: read kern.procargs2 for each PID
macProcessArgs->>parseMacProcArgs2: decode raw argument buffer
parseMacProcArgs2-->>macProcessArgs: return executable path and argv
listUnityProcessesMac->>matchMacUnityProcess: match command and project path
matchMacUnityProcess-->>listUnityProcessesMac: return matched UnityProcess
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
The comment claimed ps could not see EPERM'd processes at all, but ps can list them; it only fails to read another user's argv. Corrected per advisor review to avoid misleading future readers.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cli/common/unityprocess/process.go`:
- Around line 118-145: Cap or otherwise validate the raw argc value in
parseMacProcArgs2 before passing it as the capacity to make([]string, 0, argc).
Ensure malformed values cannot trigger an enormous allocation or panic, while
preserving parsing of valid argument counts and the existing short-buffer and
missing-terminator errors.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 73e2d72a-c57e-4683-af73-c64c3cba07b9
⛔ Files ignored due to path filters (1)
cli/common/go.modis excluded by none and included by none
📒 Files selected for processing (7)
cli/common/unityprocess/process.gocli/common/unityprocess/process_darwin.gocli/common/unityprocess/process_other.gocli/common/unityprocess/process_test.gocli/common/unityprocess/timeouts.gocli/dispatcher/shared-inputs-stamp.jsoncli/project-runner/shared-inputs-stamp.json
Comment on lines
+118
to
145
| func parseMacProcArgs2(buf []byte) ([]string, error) { | ||
| if len(buf) < 4 { | ||
| return nil, fmt.Errorf("procargs2 buffer too short: %d bytes", len(buf)) | ||
| } | ||
|
|
||
| command := matches[2] | ||
| if !isUnityEditorCommand(command, macUnityExecutablePattern) { | ||
| continue | ||
| } | ||
| projectPath := extractProjectPath(command) | ||
| if projectPath == "" { | ||
| continue | ||
| } | ||
| argc := int(binary.LittleEndian.Uint32(buf[:4])) | ||
| rest := buf[4:] | ||
|
|
||
| processes = append(processes, UnityProcess{Pid: pid, projectPath: projectPath}) | ||
| execPathEnd := bytes.IndexByte(rest, 0) | ||
| if execPathEnd < 0 { | ||
| return nil, fmt.Errorf("procargs2 missing exec path terminator") | ||
| } | ||
| return processes | ||
| rest = rest[execPathEnd:] | ||
| for len(rest) > 0 && rest[0] == 0 { | ||
| rest = rest[1:] | ||
| } | ||
|
|
||
| args := make([]string, 0, argc) | ||
| for len(rest) > 0 && len(args) < argc { | ||
| argEnd := bytes.IndexByte(rest, 0) | ||
| if argEnd < 0 { | ||
| break | ||
| } | ||
| args = append(args, string(rest[:argEnd])) | ||
| rest = rest[argEnd+1:] | ||
| } | ||
| return args, nil | ||
| } |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Bound argc before using it as slice capacity.
argc is read directly from the raw sysctl buffer and used unbounded as make([]string, 0, argc). A corrupted/unexpected buffer with a very large leading value (e.g. near 0xFFFFFFFF) would attempt a huge allocation and could panic, defeating the malformed-buffer hardening this function is otherwise designed and tested for (short-buffer and missing-terminator cases are already rejected).
🛡️ Proposed fix to cap the pre-allocation
+const maxProcArgs2Argv = 4096 // sane upper bound; real argv counts are far smaller
+
func parseMacProcArgs2(buf []byte) ([]string, error) {
if len(buf) < 4 {
return nil, fmt.Errorf("procargs2 buffer too short: %d bytes", len(buf))
}
argc := int(binary.LittleEndian.Uint32(buf[:4]))
+ if argc < 0 || argc > maxProcArgs2Argv {
+ return nil, fmt.Errorf("procargs2 argc out of range: %d", argc)
+ }
rest := buf[4:]📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| func parseMacProcArgs2(buf []byte) ([]string, error) { | |
| if len(buf) < 4 { | |
| return nil, fmt.Errorf("procargs2 buffer too short: %d bytes", len(buf)) | |
| } | |
| command := matches[2] | |
| if !isUnityEditorCommand(command, macUnityExecutablePattern) { | |
| continue | |
| } | |
| projectPath := extractProjectPath(command) | |
| if projectPath == "" { | |
| continue | |
| } | |
| argc := int(binary.LittleEndian.Uint32(buf[:4])) | |
| rest := buf[4:] | |
| processes = append(processes, UnityProcess{Pid: pid, projectPath: projectPath}) | |
| execPathEnd := bytes.IndexByte(rest, 0) | |
| if execPathEnd < 0 { | |
| return nil, fmt.Errorf("procargs2 missing exec path terminator") | |
| } | |
| return processes | |
| rest = rest[execPathEnd:] | |
| for len(rest) > 0 && rest[0] == 0 { | |
| rest = rest[1:] | |
| } | |
| args := make([]string, 0, argc) | |
| for len(rest) > 0 && len(args) < argc { | |
| argEnd := bytes.IndexByte(rest, 0) | |
| if argEnd < 0 { | |
| break | |
| } | |
| args = append(args, string(rest[:argEnd])) | |
| rest = rest[argEnd+1:] | |
| } | |
| return args, nil | |
| } | |
| const maxProcArgs2Argv = 4096 // sane upper bound; real argv counts are far smaller | |
| func parseMacProcArgs2(buf []byte) ([]string, error) { | |
| if len(buf) < 4 { | |
| return nil, fmt.Errorf("procargs2 buffer too short: %d bytes", len(buf)) | |
| } | |
| argc := int(binary.LittleEndian.Uint32(buf[:4])) | |
| if argc < 0 || argc > maxProcArgs2Argv { | |
| return nil, fmt.Errorf("procargs2 argc out of range: %d", argc) | |
| } | |
| rest := buf[4:] | |
| execPathEnd := bytes.IndexByte(rest, 0) | |
| if execPathEnd < 0 { | |
| return nil, fmt.Errorf("procargs2 missing exec path terminator") | |
| } | |
| rest = rest[execPathEnd:] | |
| for len(rest) > 0 && rest[0] == 0 { | |
| rest = rest[1:] | |
| } | |
| args := make([]string, 0, argc) | |
| for len(rest) > 0 && len(args) < argc { | |
| argEnd := bytes.IndexByte(rest, 0) | |
| if argEnd < 0 { | |
| break | |
| } | |
| args = append(args, string(rest[:argEnd])) | |
| rest = rest[argEnd+1:] | |
| } | |
| return args, nil | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@cli/common/unityprocess/process.go` around lines 118 - 145, Cap or otherwise
validate the raw argc value in parseMacProcArgs2 before passing it as the
capacity to make([]string, 0, argc). Ensure malformed values cannot trigger an
enormous allocation or panic, while preserving parsing of valid argument counts
and the existing short-buffer and missing-terminator errors.
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.
Summary
/bin/ps, so it now works inside sandboxed environments (e.g. Claude Code's default sandbox) that deny exec of external binaries.uloop's plainlaunch,get-logs,compile, and other commands that need to detect an already-running Unity Editor for this project now succeed in such sandboxes; they previously failed outright because thepsinvocation itself was denied.User Impact
uloopcommand path that needed to check for a running Unity Editor on macOS shelled out tops. In sandboxes that block exec of external binaries, this made Unity process discovery fail entirely with a permission error, even though the rest of the command logic could otherwise run.sysctl(kern.proc.all/kern.procargs2), which sandboxes that blockpsstill permit. Discovery now works in both sandboxed and normal environments with identical results.launch -r/-qstill kill the found Unity process viakill(2)against a non-child PID. Sandboxes that deny process discovery also independently deny sending signals to processes they don't own, so-r/-qwill still fail inside such a sandbox even though discovery itself now succeeds. This is a separate, pre-existing sandbox restriction that no process-discovery implementation can work around.Changes
cli/common/unityprocess/process.go: removed theps -axo ... -wwexec call and its output parser; kept the existing project-path/batchmode matching rules (isUnityEditorCommand,extractProjectPath) and exposed them through a newmatchMacUnityProcess(pid, command)helper shared by both the old command-line-based tests and the new argv-based path. Added a build-tag-freeparseMacProcArgs2buffer parser for thekern.procargs2sysctl layout (leadingint32argc, NUL-terminated exec path, NUL padding, then argc NUL-terminated argv entries; both darwin architectures are little-endian).cli/common/unityprocess/process_darwin.go(new,//go:build darwin): the real sysctl-basedlistUnityProcessesMac, usinggolang.org/x/sys/unix'sSysctlKinfoProcSlice("kern.proc.all")for enumeration andSysctlRaw("kern.procargs2", pid)for argv. Processes whose argv can't be read (e.g.EPERMfor another user's process) are skipped, matchingps's own visibility limits.cli/common/unityprocess/process_other.go(new,//go:build !darwin): a stub so the shared dispatcher compiles on every target OS; never reached at runtime.cli/common/unityprocess/timeouts.go: corrected a comment now that macOS enumeration no longer uses the timed external-command path.cli/common/unityprocess/process_test.go: replaced the old ps-output-parsing test withTestMatchMacUnityProcessExtractsProjectPath(same coverage: quoted path, batchmode-skip, unquoted path) and added fixture-based tests for the buffer parser (TestParseMacProcArgs2DecodesArgv,TestParseMacProcArgs2RejectsShortBuffer,TestParseMacProcArgs2RejectsMissingExecPathTerminator).cli/common/go.mod: promotedgolang.org/x/sysfrom an indirect to a direct dependency (go mod tidy).cli/project-runner/shared-inputs-stamp.json,cli/dispatcher/shared-inputs-stamp.json: refreshed viascripts/stamp-release-inputs.shper this repo's shared release-input trigger convention, since non-testcli/common/**/*.gosources changed.Verification
sh scripts/check-go-cli.sh: 0 lint issues, all packages pass (includingcli/common/unityprocess,cli/project-runner,cli/dispatcher).sh scripts/build-go-cli.sh: rebuilt dev binaries successfully.unityprocess.FindRunningUnityProcessdirectly, run under Claude Code's default sandbox (no sandbox override), correctly found the actual running Unity Editor process for this repo. In the same sandbox, plainps/pgrepboth fail with "operation not permitted."dist/darwin-arm64/uloop launch -r --project-path <project root>still succeeds end-to-end (finds the existing process, kills it, and relaunches), confirming no behavior change for the normal (non-sandboxed) path.main/v3-beta, so repository CI workflows (filtered to[main, v3-beta]) do not fire here; the checks above are the local equivalent.