Add default shell timeout policy

src/tools/catalog.rs

@@ -68,7 +68,7 @@ pub fn builtin_tools_enabled(enable_write_tools: bool, enable_shell_tool: bool)
     if enable_shell_tool {
         tools.push(ToolDef {
             name: "shell".to_string(),
-            description: "Run a shell command with optional args, cwd, and timeout_ms. timeout_ms bounds the command's wall-clock runtime in milliseconds; omit or use 0 for no timeout.".to_string(),
+            description: "Run a shell command with optional args, cwd, and timeout_ms. timeout_ms bounds the command's wall-clock runtime in milliseconds. If omitted, a safe default timeout (120000 ms) is applied on the host target; pass 0 to opt out and run unbounded.".to_string(),
             parameters: json!({
                 "type":"object",
                 "properties":{

src/tools/exec_shell.rs

@@ -6,6 +6,31 @@ use crate::types::SideEffects;
 use super::exec_support::{failed_exec, ToolExecution};
 use super::{minimal_builtin_example, ToolErrorCode, ToolErrorDetail, ToolRuntime};
 
+/// Default wall-clock timeout (ms) applied to a shell command on the host target
+/// when the caller omits `timeout_ms`. This keeps unattended/autonomous runs
+/// from hanging forever on a runaway command. Adjust here to change the policy
+/// globally; callers can always override per-call, and `timeout_ms: 0` opts out
+/// entirely (unbounded).
+pub(super) const DEFAULT_SHELL_TIMEOUT_MS: u64 = 120_000;
+
+/// Resolve the effective shell timeout from the request args and execution
+/// target.
+///
+/// - Explicit `timeout_ms` (including `0` = unbounded opt-out): honored as-is.
+/// - Missing `timeout_ms`: the host target applies [`DEFAULT_SHELL_TIMEOUT_MS`].
+///   The docker target instead resolves to `0` (unbounded) because it cannot
+///   enforce timeouts (`DockerTarget::exec_shell` rejects `timeout_ms > 0`), so a
+///   *missing* timeout must not silently turn into a rejection.
+pub(super) fn resolve_shell_timeout_ms(args: &Value, target: ExecTargetKind) -> u64 {
+    match args.get("timeout_ms").and_then(|v| v.as_u64()) {
+        Some(explicit) => explicit,
+        None => match target {
+            ExecTargetKind::Host => DEFAULT_SHELL_TIMEOUT_MS,
+            ExecTargetKind::Docker => 0,
+        },
+    }
+}
+
 pub(super) async fn run_shell(rt: &ToolRuntime, args: &Value) -> ToolExecution {
     let shell_allowed =
         rt.allow_shell || (rt.allow_shell_in_workdir_only && shell_cwd_is_workdir_scoped(args));
@@ -39,9 +64,11 @@ pub(super) async fn run_shell(rt: &ToolRuntime, args: &Value) -> ToolExecution {
         .get("cwd")
         .and_then(|v| v.as_str())
         .map(ToString::to_string);
-    // `timeout_ms` is optional; absent or 0 preserves unbounded behavior. The
-    // `as_u64` parse makes negative values unrepresentable.
-    let timeout_ms = args.get("timeout_ms").and_then(|v| v.as_u64()).unwrap_or(0);
+    // `timeout_ms` is optional. When omitted, the host target applies a safe
+    // default so unattended runs cannot hang forever; an explicit `0` opts out
+    // (unbounded). The `as_u64` parse makes negative values unrepresentable.
+    // Target-aware so a missing timeout never turns into a docker rejection.
+    let timeout_ms = resolve_shell_timeout_ms(args, rt.exec_target_kind);
     let req = ShellReq {
         workdir: rt.workdir.clone(),
         cmd: cmd.to_string(),
@@ -322,3 +349,43 @@ fn shell_cwd_is_workdir_scoped(args: &Value) -> bool {
         )
     })
 }
+
+#[cfg(test)]
+mod timeout_policy_tests {
+    use super::{resolve_shell_timeout_ms, DEFAULT_SHELL_TIMEOUT_MS};
+    use crate::target::ExecTargetKind;
+    use serde_json::json;
+
+    #[test]
+    fn missing_timeout_uses_host_default() {
+        let args = json!({ "cmd": "echo", "args": ["hi"] });
+        assert_eq!(
+            resolve_shell_timeout_ms(&args, ExecTargetKind::Host),
+            DEFAULT_SHELL_TIMEOUT_MS
+        );
+        assert_eq!(DEFAULT_SHELL_TIMEOUT_MS, 120_000);
+    }
+
+    #[test]
+    fn explicit_positive_timeout_overrides_default() {
+        let args = json!({ "cmd": "echo", "timeout_ms": 500 });
+        assert_eq!(resolve_shell_timeout_ms(&args, ExecTargetKind::Host), 500);
+        // Explicit values are honored on docker too (DockerTarget rejects > 0).
+        assert_eq!(resolve_shell_timeout_ms(&args, ExecTargetKind::Docker), 500);
+    }
+
+    #[test]
+    fn explicit_zero_timeout_stays_unbounded() {
+        let args = json!({ "cmd": "echo", "timeout_ms": 0 });
+        assert_eq!(resolve_shell_timeout_ms(&args, ExecTargetKind::Host), 0);
+        assert_eq!(resolve_shell_timeout_ms(&args, ExecTargetKind::Docker), 0);
+    }
+
+    #[test]
+    fn missing_timeout_on_docker_stays_unbounded_not_rejected() {
+        // Critical: a missing timeout must not become a docker timeout
+        // rejection. Docker resolves missing -> 0 (unbounded), unlike host.
+        let args = json!({ "cmd": "echo" });
+        assert_eq!(resolve_shell_timeout_ms(&args, ExecTargetKind::Docker), 0);
+    }
+}

src/tools/tests.rs

@@ -852,6 +852,52 @@ async fn shell_auto_repair_uses_sh_lc_for_embedded_command() {
     );
 }
 
+#[tokio::test]
+async fn shell_without_timeout_still_runs_fast_command_under_default_policy() {
+    // A host shell call that omits timeout_ms now gets the 120s default. A fast
+    // command must still complete normally (regression guard: the default must
+    // not break ordinary usage), proving missing -> default is wired into the
+    // execute_tool path without turning fast commands into timeouts.
+    let tmp = tempdir().expect("tempdir");
+    let rt = ToolRuntime {
+        workdir: tmp.path().to_path_buf(),
+        allow_shell: true,
+        allow_shell_in_workdir_only: false,
+        allow_write: false,
+        max_tool_output_bytes: 200_000,
+        max_read_bytes: 200_000,
+        unsafe_bypass_allow_flags: false,
+        tool_args_strict: ToolArgsStrict::On,
+        exec_target_kind: ExecTargetKind::Host,
+        exec_target: std::sync::Arc::new(HostTarget),
+    };
+    let arguments = if cfg!(windows) {
+        json!({"cmd":"cmd","args":["/C","echo default-policy-ok"]})
+    } else {
+        json!({"cmd":"sh","args":["-c","echo default-policy-ok"]})
+    };
+    let tc = ToolCall {
+        id: "tc_shell_default_timeout".to_string(),
+        name: "shell".to_string(),
+        arguments,
+    };
+    let msg = execute_tool(&rt, &tc).await;
+    let envelope: Value = serde_json::from_str(&msg.content.expect("content")).expect("json");
+    assert_eq!(envelope.get("ok").and_then(|v| v.as_bool()), Some(true));
+    let inner = envelope
+        .get("content")
+        .and_then(|v| v.as_str())
+        .and_then(|s| serde_json::from_str::<Value>(s).ok())
+        .expect("inner shell json");
+    let stdout = inner
+        .get("stdout")
+        .and_then(|v| v.as_str())
+        .unwrap_or_default();
+    assert!(stdout.contains("default-policy-ok"));
+    // Fast command must not be reported as timed out.
+    assert!(inner.get("timed_out").is_none());
+}
+
 #[tokio::test]
 async fn read_file_rejects_path_traversal() {
     let tmp = tempdir().expect("tempdir");