OpenClaw: Process Safety - Unvalidated PID Kill via SIGKILL in Process Cleanup

@@ -36,7 +36,8 @@ describe("runCliAgent resume cleanup", () => {
         ].join("\n"),
         stderr: "",
       }) // cleanupResumeProcesses (ps)
-      .mockResolvedValueOnce({ stdout: "", stderr: "" }); // cleanupResumeProcesses (kill)
+      .mockResolvedValueOnce({ stdout: "", stderr: "" }) // cleanupResumeProcesses (kill -TERM)
+      .mockResolvedValueOnce({ stdout: "", stderr: "" }); // cleanupResumeProcesses (kill -9)
     runCommandWithTimeoutMock.mockResolvedValueOnce({
       stdout: "ok",
       stderr: "",
@@ -62,19 +63,23 @@ describe("runCliAgent resume cleanup", () => {
       return;
     }
 
-    expect(runExecMock).toHaveBeenCalledTimes(3);
+    expect(runExecMock).toHaveBeenCalledTimes(4);
 
     // Second call: cleanupResumeProcesses ps
     const psCall = runExecMock.mock.calls[1] ?? [];
     expect(psCall[0]).toBe("ps");
 
-    // Third call: kill with only the child PID
-    const killCall = runExecMock.mock.calls[2] ?? [];
+    // Third call: TERM, only the child PID
+    const termCall = runExecMock.mock.calls[2] ?? [];
+    expect(termCall[0]).toBe("kill");
+    const termArgs = termCall[1] as string[];
+    expect(termArgs).toEqual(["-TERM", String(selfPid + 1)]);
+
+    // Fourth call: KILL, only the child PID
+    const killCall = runExecMock.mock.calls[3] ?? [];
     expect(killCall[0]).toBe("kill");
     const killArgs = killCall[1] as string[];
-    expect(killArgs[0]).toBe("-9");
-    expect(killArgs[1]).toBe(String(selfPid + 1));
-    expect(killArgs).toHaveLength(2); // only -9 + one PID
+    expect(killArgs).toEqual(["-9", String(selfPid + 1)]);
   });
 
   it("falls back to per-agent workspace when workspaceDir is missing", async () => {
@@ -318,6 +323,7 @@ describe("cleanupResumeProcesses", () => {
         ].join("\n"),
         stderr: "",
       })
+      .mockResolvedValueOnce({ stdout: "", stderr: "" })
       .mockResolvedValueOnce({ stdout: "", stderr: "" });
 
     await cleanupResumeProcesses(
@@ -333,8 +339,13 @@ describe("cleanupResumeProcesses", () => {
       return;
     }
 
-    expect(runExecMock).toHaveBeenCalledTimes(2);
-    const killCall = runExecMock.mock.calls[1] ?? [];
+    expect(runExecMock).toHaveBeenCalledTimes(3);
+
+    const termCall = runExecMock.mock.calls[1] ?? [];
+    expect(termCall[0]).toBe("kill");
+    expect(termCall[1]).toEqual(["-TERM", String(selfPid + 1)]);
+
+    const killCall = runExecMock.mock.calls[2] ?? [];
     expect(killCall[0]).toBe("kill");
     expect(killCall[1]).toEqual(["-9", String(selfPid + 1)]);
   });

@@ -18,6 +18,31 @@ import { buildAgentSystemPrompt } from "../system-prompt.js";
 
 const CLI_RUN_QUEUE = new Map<string, Promise<unknown>>();
 
+function buildLooseArgOrderRegex(tokens: string[]): RegExp {
+  // Scan `ps` output lines. Keep matching flexible, but require whitespace arg boundaries
+  // to avoid substring matches like `codexx` or `/path/to/codexx`.
+  const [head, ...rest] = tokens.map((t) => String(t ?? "").trim()).filter(Boolean);
+  if (!head) {
+    return /$^/;
+  }
+
+  const headEscaped = escapeRegExp(head);
+  const headFragment = `(?:^|\\s)(?:${headEscaped}|\\S+\\/${headEscaped})(?=\\s|$)`;
+  const restFragments = rest.map((t) => `(?:^|\\s)${escapeRegExp(t)}(?=\\s|$)`);
+  return new RegExp([headFragment, ...restFragments].join(".*"));
+}
+
+async function psWithFallback(argsA: string[], argsB: string[]): Promise<string> {
+  try {
+    const { stdout } = await runExec("ps", argsA);
+    return stdout;
+  } catch {
+    // fallthrough
+  }
+  const { stdout } = await runExec("ps", argsB);
+  return stdout;
+}
+
 export async function cleanupResumeProcesses(
   backend: CliBackendConfig,
   sessionId: string,
@@ -47,9 +72,11 @@ export async function cleanupResumeProcesses(
   }
 
   try {
-    // Use wide output to reduce false negatives from argv truncation.
-    const { stdout } = await runExec("ps", ["-axww", "-o", "pid=,ppid=,command="]);
-    const patternRegex = new RegExp(pattern);
+    const stdout = await psWithFallback(
+      ["-axww", "-o", "pid=,ppid=,command="],
+      ["-ax", "-o", "pid=,ppid=,command="],
+    );
+    const patternRegex = buildLooseArgOrderRegex([commandToken, ...resumeTokens]);
     const toKill: number[] = [];
 
     for (const line of stdout.split("\n")) {
@@ -77,7 +104,18 @@ export async function cleanupResumeProcesses(
     }
 
     if (toKill.length > 0) {
-      await runExec("kill", ["-9", ...toKill.map((pid) => String(pid))]);
+      const pidArgs = toKill.map((pid) => String(pid));
+      try {
+        await runExec("kill", ["-TERM", ...pidArgs]);
+      } catch {
+        // ignore
+      }
+      await new Promise((resolve) => setTimeout(resolve, 250));
+      try {
+        await runExec("kill", ["-9", ...pidArgs]);
+      } catch {
+        // ignore
+      }
     }
   } catch {
     // ignore errors - best effort cleanup
@@ -146,8 +184,10 @@ export async function cleanupSuspendedCliProcesses(
   }
 
   try {
-    // Use wide output to reduce false negatives from argv truncation.
-    const { stdout } = await runExec("ps", ["-axww", "-o", "pid=,ppid=,stat=,command="]);
+    const stdout = await psWithFallback(
+      ["-axww", "-o", "pid=,ppid=,stat=,command="],
+      ["-ax", "-o", "pid=,ppid=,stat=,command="],
+    );
     const suspended: number[] = [];
     for (const line of stdout.split("\n")) {
       const trimmed = line.trim();

@@ -10,6 +10,7 @@ Docs: https://docs.openclaw.ai
 
 ### Fixes
 
+- Security/Agents: scope CLI process cleanup to owned child PIDs to avoid killing unrelated processes on shared hosts. Thanks @aether-ai-agent.
 - Security: fix Chutes manual OAuth login state validation (thanks @aether-ai-agent). (#16058)
 - macOS: hard-limit unkeyed `openclaw://agent` deep links and ignore `deliver` / `to` / `channel` unless a valid unattended key is provided. Thanks @Cillian-Collins.
 - Plugins: suppress false duplicate plugin id warnings when the same extension is discovered via multiple paths (config/workspace/global vs bundled), while still warning on genuine duplicates. (#16222) Thanks @shadril238.

@@ -5,7 +5,7 @@ import { beforeEach, describe, expect, it, vi } from "vitest";
 import type { OpenClawConfig } from "../config/config.js";
 import type { CliBackendConfig } from "../config/types.js";
 import { runCliAgent } from "./cli-runner.js";
-import { cleanupSuspendedCliProcesses } from "./cli-runner/helpers.js";
+import { cleanupResumeProcesses, cleanupSuspendedCliProcesses } from "./cli-runner/helpers.js";
 
 const runCommandWithTimeoutMock = vi.fn();
 const runExecMock = vi.fn();
@@ -22,12 +22,21 @@ describe("runCliAgent resume cleanup", () => {
   });
 
   it("kills stale resume processes for codex sessions", async () => {
+    const selfPid = process.pid;
+
     runExecMock
       .mockResolvedValueOnce({
-        stdout: "  1 S /bin/launchd\n",
+        stdout: "  1 999 S /bin/launchd\n",
         stderr: "",
-      }) // cleanupSuspendedCliProcesses (ps)
-      .mockResolvedValueOnce({ stdout: "", stderr: "" }); // cleanupResumeProcesses (pkill)
+      }) // cleanupSuspendedCliProcesses (ps) — ppid 999 != selfPid, no match
+      .mockResolvedValueOnce({
+        stdout: [
+          `  ${selfPid + 1} ${selfPid} codex exec resume thread-123 --color never --sandbox read-only --skip-git-repo-check`,
+          `  ${selfPid + 2} 999 codex exec resume thread-123 --color never --sandbox read-only --skip-git-repo-check`,
+        ].join("\n"),
+        stderr: "",
+      }) // cleanupResumeProcesses (ps)
+      .mockResolvedValueOnce({ stdout: "", stderr: "" }); // cleanupResumeProcesses (kill)
     runCommandWithTimeoutMock.mockResolvedValueOnce({
       stdout: "ok",
       stderr: "",
@@ -53,14 +62,19 @@ describe("runCliAgent resume cleanup", () => {
       return;
     }
 
-    expect(runExecMock).toHaveBeenCalledTimes(2);
-    const pkillCall = runExecMock.mock.calls[1] ?? [];
-    expect(pkillCall[0]).toBe("pkill");
-    const pkillArgs = pkillCall[1] as string[];
-    expect(pkillArgs[0]).toBe("-f");
-    expect(pkillArgs[1]).toContain("codex");
-    expect(pkillArgs[1]).toContain("resume");
-    expect(pkillArgs[1]).toContain("thread-123");
+    expect(runExecMock).toHaveBeenCalledTimes(3);
+
+    // Second call: cleanupResumeProcesses ps
+    const psCall = runExecMock.mock.calls[1] ?? [];
+    expect(psCall[0]).toBe("ps");
+
+    // Third call: kill with only the child PID
+    const killCall = runExecMock.mock.calls[2] ?? [];
+    expect(killCall[0]).toBe("kill");
+    const killArgs = killCall[1] as string[];
+    expect(killArgs[0]).toBe("-9");
+    expect(killArgs[1]).toBe(String(selfPid + 1));
+    expect(killArgs).toHaveLength(2); // only -9 + one PID
   });
 
   it("falls back to per-agent workspace when workspaceDir is missing", async () => {
@@ -165,11 +179,12 @@ describe("cleanupSuspendedCliProcesses", () => {
   });
 
   it("matches sessionArg-based commands", async () => {
+    const selfPid = process.pid;
     runExecMock
       .mockResolvedValueOnce({
         stdout: [
-          "  40 T+ claude --session-id thread-1 -p",
-          "  41 S  claude --session-id thread-2 -p",
+          `  40 ${selfPid} T+ claude --session-id thread-1 -p`,
+          `  41 ${selfPid} S  claude --session-id thread-2 -p`,
         ].join("\n"),
         stderr: "",
       })
@@ -195,11 +210,12 @@ describe("cleanupSuspendedCliProcesses", () => {
   });
 
   it("matches resumeArgs with positional session id", async () => {
+    const selfPid = process.pid;
     runExecMock
       .mockResolvedValueOnce({
         stdout: [
-          "  50 T  codex exec resume thread-99 --color never --sandbox read-only",
-          "  51 T  codex exec resume other --color never --sandbox read-only",
+          `  50 ${selfPid} T  codex exec resume thread-99 --color never --sandbox read-only`,
+          `  51 ${selfPid} T  codex exec resume other --color never --sandbox read-only`,
         ].join("\n"),
         stderr: "",
       })
@@ -223,4 +239,128 @@ describe("cleanupSuspendedCliProcesses", () => {
     expect(killCall[0]).toBe("kill");
     expect(killCall[1]).toEqual(["-9", "50", "51"]);
   });
+
+  it("only kills child processes of current process (ppid validation)", async () => {
+    const selfPid = process.pid;
+    const childPid = selfPid + 1;
+    const unrelatedPid = 9999;
+
+    runExecMock
+      .mockResolvedValueOnce({
+        stdout: [
+          `  ${childPid} ${selfPid} T  claude --session-id thread-1 -p`,
+          `  ${unrelatedPid} 100 T  claude --session-id thread-2 -p`,
+        ].join("\n"),
+        stderr: "",
+      })
+      .mockResolvedValueOnce({ stdout: "", stderr: "" });
+
+    await cleanupSuspendedCliProcesses(
+      {
+        command: "claude",
+        sessionArg: "--session-id",
+      } as CliBackendConfig,
+      0,
+    );
+
+    if (process.platform === "win32") {
+      expect(runExecMock).not.toHaveBeenCalled();
+      return;
+    }
+
+    expect(runExecMock).toHaveBeenCalledTimes(2);
+    const killCall = runExecMock.mock.calls[1] ?? [];
+    expect(killCall[0]).toBe("kill");
+    // Only childPid killed; unrelatedPid (ppid=100) excluded
+    expect(killCall[1]).toEqual(["-9", String(childPid)]);
+  });
+
+  it("skips all processes when none are children of current process", async () => {
+    runExecMock.mockResolvedValueOnce({
+      stdout: [
+        "  200 100 T  claude --session-id thread-1 -p",
+        "  201 100 T  claude --session-id thread-2 -p",
+      ].join("\n"),
+      stderr: "",
+    });
+
+    await cleanupSuspendedCliProcesses(
+      {
+        command: "claude",
+        sessionArg: "--session-id",
+      } as CliBackendConfig,
+      0,
+    );
+
+    if (process.platform === "win32") {
+      expect(runExecMock).not.toHaveBeenCalled();
+      return;
+    }
+
+    // Only ps called — no kill because no matching ppid
+    expect(runExecMock).toHaveBeenCalledTimes(1);
+  });
+});
+
+describe("cleanupResumeProcesses", () => {
+  beforeEach(() => {
+    runExecMock.mockReset();
+  });
+
+  it("only kills resume processes owned by current process", async () => {
+    const selfPid = process.pid;
+
+    runExecMock
+      .mockResolvedValueOnce({
+        stdout: [
+          `  ${selfPid + 1} ${selfPid} codex exec resume abc-123`,
+          `  ${selfPid + 2} 999 codex exec resume abc-123`,
+        ].join("\n"),
+        stderr: "",
+      })
+      .mockResolvedValueOnce({ stdout: "", stderr: "" });
+
+    await cleanupResumeProcesses(
+      {
+        command: "codex",
+        resumeArgs: ["exec", "resume", "{sessionId}"],
+      } as CliBackendConfig,
+      "abc-123",
+    );
+
+    if (process.platform === "win32") {
+      expect(runExecMock).not.toHaveBeenCalled();
+      return;
+    }
+
+    expect(runExecMock).toHaveBeenCalledTimes(2);
+    const killCall = runExecMock.mock.calls[1] ?? [];
+    expect(killCall[0]).toBe("kill");
+    expect(killCall[1]).toEqual(["-9", String(selfPid + 1)]);
+  });
+
+  it("skips kill when no resume processes match ppid", async () => {
+    runExecMock.mockResolvedValueOnce({
+      stdout: ["  300 100 codex exec resume abc-123", "  301 200 codex exec resume abc-123"].join(
+        "\n",
+      ),
+      stderr: "",
+    });
+
+    await cleanupResumeProcesses(
+      {
+        command: "codex",
+        resumeArgs: ["exec", "resume", "{sessionId}"],
+      } as CliBackendConfig,
+      "abc-123",
+    );
+
+    if (process.platform === "win32") {
+      expect(runExecMock).not.toHaveBeenCalled();
+      return;
+    }
+
+    // Only ps called — no kill because no matching ppid
+    expect(runExecMock).toHaveBeenCalledTimes(1);
+  });
 });

@@ -47,9 +47,40 @@ export async function cleanupResumeProcesses(
   }
 
   try {
-    await runExec("pkill", ["-f", pattern]);
+    // Use wide output to reduce false negatives from argv truncation.
+    const { stdout } = await runExec("ps", ["-axww", "-o", "pid=,ppid=,command="]);
+    const patternRegex = new RegExp(pattern);
+    const toKill: number[] = [];
+
+    for (const line of stdout.split("\n")) {
+      const trimmed = line.trim();
+      if (!trimmed) {
+        continue;
+      }
+      const match = /^(\d+)\s+(\d+)\s+(.*)$/.exec(trimmed);
+      if (!match) {
+        continue;
+      }
+      const pid = Number(match[1]);
+      const ppid = Number(match[2]);
+      const cmd = match[3] ?? "";
+      if (!Number.isFinite(pid)) {
+        continue;
+      }
+      if (ppid !== process.pid) {
+        continue;
+      }
+      if (!patternRegex.test(cmd)) {
+        continue;
+      }
+      toKill.push(pid);
+    }
+
+    if (toKill.length > 0) {
+      await runExec("kill", ["-9", ...toKill.map((pid) => String(pid))]);
+    }
   } catch {
-    // ignore missing pkill or no matches
+    // ignore errors - best effort cleanup
   }
 }
 
@@ -115,23 +146,28 @@ export async function cleanupSuspendedCliProcesses(
   }
 
   try {
-    const { stdout } = await runExec("ps", ["-ax", "-o", "pid=,stat=,command="]);
+    // Use wide output to reduce false negatives from argv truncation.
+    const { stdout } = await runExec("ps", ["-axww", "-o", "pid=,ppid=,stat=,command="]);
     const suspended: number[] = [];
     for (const line of stdout.split("\n")) {
       const trimmed = line.trim();
       if (!trimmed) {
         continue;
       }
-      const match = /^(\d+)\s+(\S+)\s+(.*)$/.exec(trimmed);
+      const match = /^(\d+)\s+(\d+)\s+(\S+)\s+(.*)$/.exec(trimmed);
       if (!match) {
         continue;
       }
       const pid = Number(match[1]);
-      const stat = match[2] ?? "";
-      const command = match[3] ?? "";
+      const ppid = Number(match[2]);
+      const stat = match[3] ?? "";
+      const command = match[4] ?? "";
       if (!Number.isFinite(pid)) {
         continue;
       }
+      if (ppid !== process.pid) {
+        continue;
+      }
       if (!stat.includes("T")) {
         continue;
       }