fix(security): kill timed out exec process trees

src/process/child-process-tree.ts

@@ -0,0 +1,17 @@
+import type { ChildProcess } from "node:child_process";
+import { signalProcessTree } from "./kill-tree.js";
+
+export function shouldDetachChildForProcessTree(): boolean {
+  return process.platform !== "win32";
+}
+
+export function forceKillChildProcessTree(child: Pick<ChildProcess, "kill" | "pid">): void {
+  if (typeof child.pid === "number" && child.pid > 0) {
+    signalProcessTree(child.pid, "SIGKILL", {
+      detached: shouldDetachChildForProcessTree(),
+    });
+    return;
+  }
+
+  child.kill("SIGKILL");
+}

src/secrets/resolve.test.ts

@@ -5,6 +5,12 @@ import path from "node:path";
 import { afterAll, beforeAll, describe, expect, it, vi } from "vitest";
 import type { OpenClawConfig } from "../config/config.js";
 import { MAX_TIMER_TIMEOUT_MS } from "../shared/number-coercion.js";
+import {
+  killPidIfAlive,
+  readPidFile,
+  waitForPidToExit,
+  writeForkingNoOutputScript,
+} from "../test-utils/process-tree.js";
 import { INVALID_EXEC_SECRET_REF_IDS } from "../test-utils/secret-ref-test-vectors.js";
 import { withMockedWindowsPlatform } from "../test-utils/vitest-spies.js";
 import {
@@ -55,6 +61,7 @@ describe("secret ref resolver", () => {
     jsonOnly?: boolean;
     allowSymlinkCommand?: boolean;
     trustedDirs?: string[];
+    env?: Record<string, string>;
     args?: string[];
     timeoutMs?: number;
     noOutputTimeoutMs?: number;
@@ -255,6 +262,28 @@ describe("secret ref resolver", () => {
     expect(value).toBe("ok");
   });
 
+  itPosix("kills forked exec provider children on no-output timeout", async () => {
+    const root = await createCaseDir("exec-fork-timeout");
+    const scriptPath = await writeForkingNoOutputScript(root);
+    const pidPath = path.join(root, "forked.pid");
+    let childPid: number | undefined;
+
+    try {
+      await expect(
+        resolveExecSecret(scriptPath, {
+          env: { NODE_BINARY: process.execPath, PID_FILE: pidPath },
+          noOutputTimeoutMs: 150,
+          timeoutMs: 2000,
+        }),
+      ).rejects.toThrow('Exec provider "execmain" produced no output');
+
+      childPid = await readPidFile(pidPath);
+      expect(await waitForPidToExit(childPid)).toBe(true);
+    } finally {
+      killPidIfAlive(childPid);
+    }
+  });
+
   itPosix("supports non-JSON single-value exec output when jsonOnly is false", async () => {
     const value = await resolveExecSecret(execPlainScriptPath, { jsonOnly: false });
     expect(value).toBe("plain-secret");

src/secrets/resolve.ts

@@ -18,6 +18,10 @@ import {
   loadPluginManifestRegistry,
   type PluginManifestRegistry,
 } from "../plugins/manifest-registry.js";
+import {
+  forceKillChildProcessTree,
+  shouldDetachChildForProcessTree,
+} from "../process/child-process-tree.js";
 import { inspectPathPermissions, safeStat } from "../security/audit-fs.js";
 import { isPathInside } from "../security/scan-paths.js";
 import { resolveUserPath } from "../utils.js";
@@ -497,6 +501,7 @@ async function runExecResolver(params: {
       stdio: ["pipe", "pipe", "pipe"],
       shell: false,
       windowsHide: true,
+      detached: shouldDetachChildForProcessTree(),
     });
 
     let settled = false;
@@ -508,7 +513,7 @@ async function runExecResolver(params: {
     let noOutputTimer: NodeJS.Timeout | null = null;
     const timeoutTimer = setTimeout(() => {
       timedOut = true;
-      child.kill("SIGKILL");
+      forceKillChildProcessTree(child);
     }, params.timeoutMs);
 
     const clearTimers = () => {
@@ -525,7 +530,7 @@ async function runExecResolver(params: {
       }
       noOutputTimer = setTimeout(() => {
         noOutputTimedOut = true;
-        child.kill("SIGKILL");
+        forceKillChildProcessTree(child);
       }, params.noOutputTimeoutMs);
     };
 
@@ -533,7 +538,7 @@ async function runExecResolver(params: {
       const text = typeof chunk === "string" ? chunk : chunk.toString("utf8");
       outputBytes += Buffer.byteLength(text, "utf8");
       if (outputBytes > params.maxOutputBytes) {
-        child.kill("SIGKILL");
+        forceKillChildProcessTree(child);
         if (!settled) {
           settled = true;
           clearTimers();

src/security/install-policy.test.ts

@@ -4,6 +4,12 @@ import os from "node:os";
 import path from "node:path";
 import { afterEach, beforeEach, describe, expect, it } from "vitest";
 import type { OpenClawConfig } from "../config/types.openclaw.js";
+import {
+  killPidIfAlive,
+  readPidFile,
+  waitForPidToExit,
+  writeForkingNoOutputScript,
+} from "../test-utils/process-tree.js";
 import {
   runInstallPolicy,
   validateInstallPolicyStatic,
@@ -211,6 +217,42 @@ describe("runInstallPolicy", () => {
     expect(result).toEqual({});
   });
 
+  it.runIf(process.platform !== "win32")(
+    "kills forked policy command children on no-output timeout",
+    async () => {
+      const forkScriptPath = await writeForkingNoOutputScript(sourceDir);
+      const pidPath = path.join(sourceDir, "forked.pid");
+      let childPid: number | undefined;
+
+      try {
+        const result = await runInstallPolicy({
+          config: {
+            security: {
+              installPolicy: {
+                enabled: true,
+                exec: {
+                  source: "exec",
+                  command: forkScriptPath,
+                  env: { NODE_BINARY: process.execPath, PID_FILE: pidPath },
+                  allowInsecurePath: true,
+                  noOutputTimeoutMs: 150,
+                  timeoutMs: 2000,
+                },
+              },
+            },
+          },
+          request: baseRequest(sourceDir),
+        });
+
+        expect(result?.blocked?.reason).toContain("policy command produced no output");
+        childPid = await readPidFile(pidPath);
+        expect(await waitForPidToExit(childPid)).toBe(true);
+      } finally {
+        killPidIfAlive(childPid);
+      }
+    },
+  );
+
   it("does not inherit PATH unless passEnv includes it", async () => {
     const envPath = path.join(sourceDir, "env.json");
     const response = JSON.stringify({ protocolVersion: 1, decision: "allow" });

src/security/install-policy.ts

@@ -4,6 +4,10 @@ import fs from "node:fs/promises";
 import path from "node:path";
 import type { OpenClawConfig, SecurityConfig } from "../config/types.openclaw.js";
 import { formatErrorMessage } from "../infra/errors.js";
+import {
+  forceKillChildProcessTree,
+  shouldDetachChildForProcessTree,
+} from "../process/child-process-tree.js";
 import { normalizePositiveInt, normalizePositiveTimerMs } from "../secrets/shared.js";
 import { resolveUserPath } from "../utils.js";
 import { resolveRuntimeServiceVersion } from "../version.js";
@@ -534,6 +538,7 @@ async function runPolicyCommand(params: {
       stdio: ["pipe", "pipe", "pipe"],
       shell: false,
       windowsHide: true,
+      detached: shouldDetachChildForProcessTree(),
     });
 
     let settled = false;
@@ -545,7 +550,7 @@ async function runPolicyCommand(params: {
     let noOutputTimer: NodeJS.Timeout | null = null;
     const timeoutTimer = setTimeout(() => {
       timedOut = true;
-      child.kill("SIGKILL");
+      forceKillChildProcessTree(child);
     }, params.timeoutMs);
 
     const clearTimers = () => {
@@ -562,7 +567,7 @@ async function runPolicyCommand(params: {
       }
       noOutputTimer = setTimeout(() => {
         noOutputTimedOut = true;
-        child.kill("SIGKILL");
+        forceKillChildProcessTree(child);
       }, params.noOutputTimeoutMs);
     };
 
@@ -570,7 +575,7 @@ async function runPolicyCommand(params: {
       const text = typeof chunk === "string" ? chunk : chunk.toString("utf8");
       outputBytes += Buffer.byteLength(text, "utf8");
       if (outputBytes > params.maxOutputBytes) {
-        child.kill("SIGKILL");
+        forceKillChildProcessTree(child);
         if (!settled) {
           settled = true;
           clearTimers();

src/test-utils/process-tree.ts

@@ -0,0 +1,51 @@
+import fs from "node:fs/promises";
+import path from "node:path";
+
+export async function writeForkingNoOutputScript(dir: string): Promise<string> {
+  const scriptPath = path.join(dir, "fork-no-output.sh");
+  await fs.writeFile(
+    scriptPath,
+    [
+      "#!/bin/sh",
+      '"$NODE_BINARY" -e "setInterval(() => {}, 1000)" &',
+      'printf "%s" "$!" > "$PID_FILE"',
+      "sleep 30",
+    ].join("\n"),
+    "utf8",
+  );
+  await fs.chmod(scriptPath, 0o700);
+  return scriptPath;
+}
+
+export function isPidAlive(pid: number): boolean {
+  try {
+    process.kill(pid, 0);
+    return true;
+  } catch {
+    return false;
+  }
+}
+
+export async function waitForPidToExit(pid: number, timeoutMs = 2000): Promise<boolean> {
+  const deadline = Date.now() + timeoutMs;
+  while (Date.now() < deadline) {
+    if (!isPidAlive(pid)) {
+      return true;
+    }
+    await new Promise<void>((resolve) => {
+      setTimeout(resolve, 25);
+    });
+  }
+  return !isPidAlive(pid);
+}
+
+export async function readPidFile(pidPath: string): Promise<number> {
+  return Number((await fs.readFile(pidPath, "utf8")).trim());
+}
+
+export function killPidIfAlive(pid: number | undefined): void {
+  if (pid === undefined || !isPidAlive(pid)) {
+    return;
+  }
+  process.kill(pid, "SIGKILL");
+}