From 4067d78a4c1d43577b50e40bc46aa8059654392c Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Mon, 27 Apr 2026 11:15:29 +0100 Subject: [PATCH] fix(exec): enforce default timeout on node runs --- CHANGELOG.md | 2 +- docs/gateway/background-process.md | 3 +- docs/tools/exec.md | 6 ++- .../bash-tools.exec-host-node-phases.ts | 18 +++---- src/agents/bash-tools.exec-host-node.test.ts | 53 ++++++++++++++++++- src/agents/bash-tools.exec-host-node.ts | 2 - 6 files changed, 67 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4cb4e02e505..d9dec264bc1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,7 +22,7 @@ Docs: https://docs.openclaw.ai - Git hooks: skip ignored staged paths when formatting and restaging pre-commit files, so merge commits no longer abort when `.gitignore` newly ignores staged merged content. Fixes #72744. Thanks @100yenadmin. - Memory-core/dreaming: add a supported `dreaming.model` knob for Dream Diary narrative subagents, wired through phase config and the existing plugin subagent model-override trust gate. Refs #65963. Thanks @esqandil and @mjamiv. - Memory-core/dreaming: treat request-scoped narrative fallback as expected, skip session cleanup when no subagent run was created, and remove duplicate phase-level cleanup so fallback no longer emits warning noise. Fixes #67152. Thanks @jsompis. -- Agents/exec: apply configured `tools.exec.timeoutSec` to background and `yieldMs` commands when no per-call timeout is set, preventing auto-backgrounded commands from running indefinitely. Fixes #67600; supersedes #67603. Thanks @dlmpx and @kagura-agent. +- Agents/exec: apply configured `tools.exec.timeoutSec` to background, `yieldMs`, and node `system.run` commands when no per-call timeout is set, preventing auto-backgrounded and remote node commands from running indefinitely. Fixes #67600; supersedes #67603. Thanks @dlmpx and @kagura-agent. - Config/doctor: stop masking unknown-key validation diagnostics such as `agents.defaults.llm`, and have `openclaw doctor --fix` remove the retired `agents.defaults.llm` timeout block. Thanks @aidiffuser. - CLI/plugins: preserve unversioned ClawHub install specs so `plugins update` can follow newer ClawHub releases instead of pinning to the initially resolved version. Fixes #63010; supersedes #58426. Thanks @kangsen1234 and @robinspt. - Memory-core/subagents: tag plugin-created subagent sessions with their plugin owner so dreaming narrative cleanup can delete its own ephemeral sessions without granting broad admin session deletion. Fixes #72712. Thanks @BSG2000. diff --git a/docs/gateway/background-process.md b/docs/gateway/background-process.md index f5d42d78475..03a0811d077 100644 --- a/docs/gateway/background-process.md +++ b/docs/gateway/background-process.md @@ -17,7 +17,7 @@ Key parameters: - `command` (required) - `yieldMs` (default 10000): auto‑background after this delay - `background` (bool): background immediately -- `timeout` (seconds, default 1800): kill the process after this timeout +- `timeout` (seconds, default `tools.exec.timeoutSec`): kill the process after this timeout; set `timeout: 0` only to disable the exec process timeout for that call - `elevated` (bool): run outside the sandbox if elevated mode is enabled/allowed (`gateway` by default, or `node` when the exec target is `node`) - Need a real TTY? Set `pty: true`. - `workdir`, `env` @@ -26,6 +26,7 @@ Behavior: - Foreground runs return output directly. - When backgrounded (explicit or timeout), the tool returns `status: "running"` + `sessionId` and a short tail. +- Background and `yieldMs` runs inherit `tools.exec.timeoutSec` unless the call provides an explicit `timeout`. - Output is kept in memory until the session is polled or cleared. - If the `process` tool is disallowed, `exec` runs synchronously and ignores `yieldMs`/`background`. - Spawned exec commands receive `OPENCLAW_SHELL=exec` for context-aware shell/profile rules. diff --git a/docs/tools/exec.md b/docs/tools/exec.md index 6f0a06504e2..69bf097ba36 100644 --- a/docs/tools/exec.md +++ b/docs/tools/exec.md @@ -32,8 +32,8 @@ Auto-background the command after this delay (ms). Background the command immediately instead of waiting for `yieldMs`. - -Kill the command after this many seconds. + +Override the configured exec timeout for this call. Set `timeout: 0` only when the command should run without the exec process timeout. @@ -70,6 +70,7 @@ Notes: - `node` requires a paired node (companion app or headless node host). - If multiple nodes are available, set `exec.node` or `tools.exec.node` to select one. - `exec host=node` is the only shell-execution path for nodes; the legacy `nodes.run` wrapper has been removed. +- `timeout` applies to foreground, background, `yieldMs`, gateway, sandbox, and node `system.run` execution. If omitted, OpenClaw uses `tools.exec.timeoutSec`; explicit `timeout: 0` disables the exec process timeout for that call. - On non-Windows hosts, exec uses `SHELL` when set; if `SHELL` is `fish`, it prefers `bash` (or `sh`) from `PATH` to avoid fish-incompatible scripts, then falls back to `SHELL` if neither exists. - On Windows hosts, exec prefers PowerShell 7 (`pwsh`) discovery (Program Files, ProgramW6432, then PATH), @@ -94,6 +95,7 @@ Notes: - `tools.exec.notifyOnExit` (default: true): when true, backgrounded exec sessions enqueue a system event and request a heartbeat on exit. - `tools.exec.approvalRunningNoticeMs` (default: 10000): emit a single “running” notice when an approval-gated exec runs longer than this (0 disables). +- `tools.exec.timeoutSec` (default: 1800): default per-command exec timeout in seconds. Per-call `timeout` overrides it; per-call `timeout: 0` disables the exec process timeout. - `tools.exec.host` (default: `auto`; resolves to `sandbox` when sandbox runtime is active, `gateway` otherwise) - `tools.exec.security` (default: `deny` for sandbox, `full` for gateway + node when unset) - `tools.exec.ask` (default: `off`) diff --git a/src/agents/bash-tools.exec-host-node-phases.ts b/src/agents/bash-tools.exec-host-node-phases.ts index 6adadacb4b7..0053cb9a5a6 100644 --- a/src/agents/bash-tools.exec-host-node-phases.ts +++ b/src/agents/bash-tools.exec-host-node-phases.ts @@ -28,6 +28,7 @@ export type NodeExecutionTarget = { argv: string[]; env: Record | undefined; invokeTimeoutMs: number; + runTimeoutSec: number; supportsSystemRunPrepare: boolean; }; @@ -124,17 +125,16 @@ export async function resolveNodeExecutionTarget( ); } + const runTimeoutSec = + typeof params.timeoutSec === "number" ? params.timeoutSec : params.defaultTimeoutSec; + const invokeBaseTimeoutSec = runTimeoutSec > 0 ? runTimeoutSec : params.defaultTimeoutSec; return { nodeId, platform: nodeInfo?.platform, argv: buildNodeShellCommand(params.command, nodeInfo?.platform), env: params.requestedEnv ? { ...params.requestedEnv } : undefined, - invokeTimeoutMs: Math.max( - 10_000, - (typeof params.timeoutSec === "number" ? params.timeoutSec : params.defaultTimeoutSec) * - 1000 + - 5_000, - ), + invokeTimeoutMs: Math.max(10_000, invokeBaseTimeoutSec * 1000 + 5_000), + runTimeoutSec, supportsSystemRunPrepare: declaredCommands.includes("system.run.prepare"), }; } @@ -144,7 +144,6 @@ export function buildNodeSystemRunInvoke(params: { command: string[]; rawCommand: string; cwd: string | undefined; - timeoutSec: number | undefined; agentId: string | undefined; sessionKey: string | undefined; approved?: boolean; @@ -154,6 +153,8 @@ export function buildNodeSystemRunInvoke(params: { notifyOnExit?: boolean; systemRunPlan?: SystemRunApprovalPlan; }): Record { + const timeoutMs = + params.target.runTimeoutSec > 0 ? Math.floor(params.target.runTimeoutSec * 1000) : 0; return { nodeId: params.target.nodeId, command: "system.run", @@ -163,7 +164,7 @@ export function buildNodeSystemRunInvoke(params: { ...(params.systemRunPlan ? { systemRunPlan: params.systemRunPlan } : {}), ...(params.cwd != null ? { cwd: params.cwd } : {}), env: params.target.env, - timeoutMs: typeof params.timeoutSec === "number" ? params.timeoutSec * 1000 : undefined, + timeoutMs, agentId: params.agentId, sessionKey: params.sessionKey, approved: params.approved, @@ -189,7 +190,6 @@ export async function invokeNodeSystemRunDirect(params: { command: params.target.argv, rawCommand: params.request.command, cwd: params.request.workdir, - timeoutSec: params.request.timeoutSec, agentId: params.request.agentId, sessionKey: params.request.sessionKey, notifyOnExit: params.request.notifyOnExit, diff --git a/src/agents/bash-tools.exec-host-node.test.ts b/src/agents/bash-tools.exec-host-node.test.ts index ef45f35ffbd..80485226c9e 100644 --- a/src/agents/bash-tools.exec-host-node.test.ts +++ b/src/agents/bash-tools.exec-host-node.test.ts @@ -151,6 +151,17 @@ type MockNodeInvokeParams = { params?: Record; }; +function expectSystemRunInvoke(params: { invokeTimeoutMs: number; runTimeoutMs: number }) { + expect(callGatewayToolMock).toHaveBeenCalledWith( + "node.invoke", + expect.objectContaining({ timeoutMs: params.invokeTimeoutMs }), + expect.objectContaining({ + command: "system.run", + params: expect.objectContaining({ timeoutMs: params.runTimeoutMs }), + }), + ); +} + describe("executeNodeHostCommand", () => { beforeAll(async () => { ({ executeNodeHostCommand } = await import("./bash-tools.exec-host-node.js")); @@ -276,13 +287,14 @@ describe("executeNodeHostCommand", () => { expect(callGatewayToolMock).toHaveBeenNthCalledWith( 3, "node.invoke", - expect.anything(), + expect.objectContaining({ timeoutMs: 35_000 }), expect.objectContaining({ command: "system.run", params: expect.objectContaining({ approved: true, approvalDecision: "allow-once", systemRunPlan: preparedPlan, + timeoutMs: 30_000, }), }), ); @@ -365,13 +377,14 @@ describe("executeNodeHostCommand", () => { expect(callGatewayToolMock).toHaveBeenCalledTimes(1); expect(callGatewayToolMock).toHaveBeenCalledWith( "node.invoke", - expect.anything(), + expect.objectContaining({ timeoutMs: 35_000 }), expect.objectContaining({ command: "system.run", params: expect.objectContaining({ command: ["bash", "-lc", "bun ./script.ts"], rawCommand: "bun ./script.ts", suppressNotifyOnExit: true, + timeoutMs: 30_000, }), }), ); @@ -386,6 +399,42 @@ describe("executeNodeHostCommand", () => { ); }); + it("forwards explicit timeouts to node system.run", async () => { + await executeNodeHostCommand({ + command: "bun ./script.ts", + workdir: "/tmp/work", + env: {}, + security: "full", + ask: "off", + timeoutSec: 12, + defaultTimeoutSec: 30, + approvalRunningNoticeMs: 0, + warnings: [], + agentId: "requested-agent", + sessionKey: "requested-session", + }); + + expectSystemRunInvoke({ invokeTimeoutMs: 17_000, runTimeoutMs: 12_000 }); + }); + + it("forwards timeout zero to node system.run and keeps the invoke wait bounded", async () => { + await executeNodeHostCommand({ + command: "bun ./script.ts", + workdir: "/tmp/work", + env: {}, + security: "full", + ask: "off", + timeoutSec: 0, + defaultTimeoutSec: 30, + approvalRunningNoticeMs: 0, + warnings: [], + agentId: "requested-agent", + sessionKey: "requested-session", + }); + + expectSystemRunInvoke({ invokeTimeoutMs: 35_000, runTimeoutMs: 0 }); + }); + it("denies timed-out inline-eval requests instead of invoking the node", async () => { detectInterpreterInlineEvalArgvMock.mockReturnValue(INLINE_EVAL_HIT); resolveApprovalDecisionOrUndefinedMock.mockResolvedValue(null); diff --git a/src/agents/bash-tools.exec-host-node.ts b/src/agents/bash-tools.exec-host-node.ts index 9d216bea328..52ddd97fcb2 100644 --- a/src/agents/bash-tools.exec-host-node.ts +++ b/src/agents/bash-tools.exec-host-node.ts @@ -213,7 +213,6 @@ export async function executeNodeHostCommand( command: prepared.argv, rawCommand: prepared.rawCommand, cwd: prepared.cwd, - timeoutSec: params.timeoutSec, agentId: prepared.agentId, sessionKey: prepared.sessionKey, approved: approvedByAsk, @@ -280,7 +279,6 @@ export async function executeNodeHostCommand( command: prepared.argv, rawCommand: prepared.rawCommand, cwd: prepared.cwd, - timeoutSec: params.timeoutSec, agentId: prepared.agentId, sessionKey: prepared.sessionKey, approved: inlineApprovedByAsk,