fix(coding-agent): reject oversized bash timeouts

closes #6181
This commit is contained in:
Vegard Stikbakke
2026-06-30 15:01:02 +02:00
parent 0ac3cfe09b
commit cbcf4e04c3
4 changed files with 50 additions and 7 deletions
+1
View File
@@ -10,6 +10,7 @@
### Fixed ### Fixed
- Fixed oversized harness shell execution timeouts to fail with a clear validation error instead of being clamped to an immediate timeout ([#6181](https://github.com/earendil-works/pi/issues/6181)).
- Fixed `Agent.prepareNextTurn` to keep receiving the run abort signal instead of the next-turn context. - Fixed `Agent.prepareNextTurn` to keep receiving the run abort signal instead of the next-turn context.
## [0.80.2] - 2026-06-23 ## [0.80.2] - 2026-06-23
+22 -2
View File
@@ -28,6 +28,23 @@ import {
toError, toError,
} from "../types.ts"; } from "../types.ts";
const MAX_TIMEOUT_MS = 2_147_483_647;
const MAX_TIMEOUT_SECONDS = MAX_TIMEOUT_MS / 1000;
function resolveTimeoutMs(timeout: number | undefined): Result<number | undefined, ExecutionError> {
if (timeout === undefined) return ok(undefined);
if (!Number.isFinite(timeout)) {
return err(new ExecutionError("timeout", "Invalid timeout: must be a finite number of seconds"));
}
if (timeout <= 0) return ok(undefined);
const timeoutMs = timeout * 1000;
if (timeoutMs > MAX_TIMEOUT_MS) {
return err(new ExecutionError("timeout", `Invalid timeout: maximum is ${MAX_TIMEOUT_SECONDS} seconds`));
}
return ok(timeoutMs);
}
function resolvePath(cwd: string, path: string): string { function resolvePath(cwd: string, path: string): string {
return isAbsolute(path) ? path : resolve(cwd, path); return isAbsolute(path) ? path : resolve(cwd, path);
} }
@@ -258,6 +275,9 @@ export class NodeExecutionEnv implements ExecutionEnv {
}, },
): Promise<Result<{ stdout: string; stderr: string; exitCode: number }, ExecutionError>> { ): Promise<Result<{ stdout: string; stderr: string; exitCode: number }, ExecutionError>> {
if (options?.abortSignal?.aborted) return err(new ExecutionError("aborted", "aborted")); if (options?.abortSignal?.aborted) return err(new ExecutionError("aborted", "aborted"));
const timeoutMsResult = resolveTimeoutMs(options?.timeout);
if (!timeoutMsResult.ok) return err(timeoutMsResult.error);
const timeoutMs = timeoutMsResult.value;
const cwd = options?.cwd ? resolvePath(this.cwd, options.cwd) : this.cwd; const cwd = options?.cwd ? resolvePath(this.cwd, options.cwd) : this.cwd;
const shellConfig = await getShellConfig(this.shellPath); const shellConfig = await getShellConfig(this.shellPath);
@@ -310,13 +330,13 @@ export class NodeExecutionEnv implements ExecutionEnv {
} }
timeoutId = timeoutId =
typeof options?.timeout === "number" timeoutMs !== undefined
? setTimeout(() => { ? setTimeout(() => {
timedOut = true; timedOut = true;
if (child?.pid) { if (child?.pid) {
killProcessTree(child.pid); killProcessTree(child.pid);
} }
}, options.timeout * 1000) }, timeoutMs)
: undefined; : undefined;
if (options?.abortSignal) { if (options?.abortSignal) {
+4
View File
@@ -2,6 +2,10 @@
## [Unreleased] ## [Unreleased]
### Fixed
- Fixed oversized bash tool timeouts to fail with a clear validation error instead of being clamped to an immediate timeout ([#6181](https://github.com/earendil-works/pi/issues/6181)).
## [0.80.3] - 2026-06-30 ## [0.80.3] - 2026-06-30
### New Features ### New Features
+23 -5
View File
@@ -21,6 +21,23 @@ import { getTextOutput, invalidArgText, str } from "./render-utils.ts";
import { wrapToolDefinition } from "./tool-definition-wrapper.ts"; import { wrapToolDefinition } from "./tool-definition-wrapper.ts";
import { DEFAULT_MAX_BYTES, DEFAULT_MAX_LINES, formatSize, type TruncationResult } from "./truncate.ts"; import { DEFAULT_MAX_BYTES, DEFAULT_MAX_LINES, formatSize, type TruncationResult } from "./truncate.ts";
const MAX_TIMEOUT_MS = 2_147_483_647;
const MAX_TIMEOUT_SECONDS = MAX_TIMEOUT_MS / 1000;
function resolveTimeoutMs(timeout: number | undefined): number | undefined {
if (timeout === undefined) return undefined;
if (!Number.isFinite(timeout)) {
throw new Error("Invalid timeout: must be a finite number of seconds");
}
if (timeout <= 0) return undefined;
const timeoutMs = timeout * 1000;
if (timeoutMs > MAX_TIMEOUT_MS) {
throw new Error(`Invalid timeout: maximum is ${MAX_TIMEOUT_SECONDS} seconds`);
}
return timeoutMs;
}
const bashSchema = Type.Object({ const bashSchema = Type.Object({
command: Type.String({ description: "Bash command to execute" }), command: Type.String({ description: "Bash command to execute" }),
timeout: Type.Optional(Type.Number({ description: "Timeout in seconds (optional, no default timeout)" })), timeout: Type.Optional(Type.Number({ description: "Timeout in seconds (optional, no default timeout)" })),
@@ -66,15 +83,16 @@ export interface BashOperations {
export function createLocalBashOperations(options?: { shellPath?: string }): BashOperations { export function createLocalBashOperations(options?: { shellPath?: string }): BashOperations {
return { return {
exec: async (command, cwd, { onData, signal, timeout, env }) => { exec: async (command, cwd, { onData, signal, timeout, env }) => {
const timeoutMs = resolveTimeoutMs(timeout);
if (signal?.aborted) {
throw new Error("aborted");
}
const shellConfig = getShellConfig(options?.shellPath); const shellConfig = getShellConfig(options?.shellPath);
try { try {
await fsAccess(cwd, constants.F_OK); await fsAccess(cwd, constants.F_OK);
} catch { } catch {
throw new Error(`Working directory does not exist: ${cwd}\nCannot execute bash commands.`); throw new Error(`Working directory does not exist: ${cwd}\nCannot execute bash commands.`);
} }
if (signal?.aborted) {
throw new Error("aborted");
}
const commandFromStdin = shellConfig.commandTransport === "stdin"; const commandFromStdin = shellConfig.commandTransport === "stdin";
const child = spawn(shellConfig.shell, commandFromStdin ? shellConfig.args : [...shellConfig.args, command], { const child = spawn(shellConfig.shell, commandFromStdin ? shellConfig.args : [...shellConfig.args, command], {
@@ -97,11 +115,11 @@ export function createLocalBashOperations(options?: { shellPath?: string }): Bas
try { try {
// Set timeout if provided. // Set timeout if provided.
if (timeout !== undefined && timeout > 0) { if (timeoutMs !== undefined) {
timeoutHandle = setTimeout(() => { timeoutHandle = setTimeout(() => {
timedOut = true; timedOut = true;
if (child.pid) killProcessTree(child.pid); if (child.pid) killProcessTree(child.pid);
}, timeout * 1000); }, timeoutMs);
} }
// Stream stdout and stderr. // Stream stdout and stderr.
child.stdout?.on("data", onData); child.stdout?.on("data", onData);