← Back to PRs

#16157: fix(security): OC-31 scope process cleanup to owned PIDs

by aether-ai-agent open 2026-02-14 10:46 View on GitHub →
agents size: M trusted-contributor
## Summary Add parent PID (ppid) filtering to `cleanupSuspendedCliProcesses` and `cleanupResumeProcesses` in `src/agents/cli-runner/helpers.ts` to prevent accidentally killing unrelated processes on shared hosts. **Suggested by Aether AI Agent** — Process Safety Improvement (OC-31) **Severity**: Low **CWE**: CWE-283 (Unverified Ownership) ## Problem Both cleanup functions enumerate system processes and send SIGKILL to pattern-matched PIDs without verifying they belong to the current OpenClaw process tree. On shared hosts, unrelated processes could incidentally match the cleanup regex and be terminated. ## Solution - **`cleanupSuspendedCliProcesses`**: Added ppid column to `ps` output, parse ppid from regex, filter `ppid !== process.pid` before kill — only direct children of the current process are targeted - **`cleanupResumeProcesses`**: Replaced `pkill -f pattern` with `ps -ax -o pid=,ppid=,command=` + ppid validation + explicit `kill` — eliminates system-wide blind pattern matching ## Changes - **`src/agents/cli-runner/helpers.ts`**: ppid validation added to both cleanup functions - **`src/agents/cli-runner.e2e.test.ts`**: - Updated existing test mocks for new ppid column format - Updated resume cleanup test for ps+kill flow (was pkill) - Added: "only kills child processes of current process" test - Added: "skips all processes when none are children" test - Added: "only kills resume processes owned by current process" test - Added: "skips kill when no resume processes match ppid" test ## Test Plan - [x] All existing tests updated and passing (10/10) - [x] New ppid ownership validation tests for `cleanupSuspendedCliProcesses` - [x] New ppid ownership validation tests for `cleanupResumeProcesses` - [x] `npx vitest run --config vitest.e2e.config.ts src/agents/cli-runner.e2e.test.ts` — 10/10 pass Co-authored-by: Aether AI Agent <github@tryaether.ai> <!-- greptile_comment --> <h3>Greptile Summary</h3> Added parent PID validation to process cleanup functions to prevent accidentally killing unrelated processes on shared hosts. Both `cleanupSuspendedCliProcesses` and `cleanupResumeProcesses` now verify `ppid === process.pid` before sending SIGKILL, ensuring only direct child processes are targeted. - `cleanupResumeProcesses` now uses `ps` + explicit `kill` instead of `pkill -f` for precise ppid validation - `cleanupSuspendedCliProcesses` adds ppid column to `ps` output and filters by parent ownership - Test coverage expanded with 4 new ppid ownership validation tests - All existing tests updated for new ps output format <h3>Confidence Score: 5/5</h3> - This PR is safe to merge with minimal risk - The changes are narrowly scoped security improvements with comprehensive test coverage. The ppid validation logic is straightforward and correctly implemented in both cleanup functions. All 10 tests pass, including 4 new tests specifically validating ppid ownership. The fallback error handling ensures graceful degradation if process enumeration fails. - No files require special attention <sub>Last reviewed commit: 895125c</sub> <!-- greptile_other_comments_section --> <!-- /greptile_comment -->

Most Similar PRs