Merge pull request #122 from bnema/require-bash-timeout-seconds
fix(tools): require bash timeouts in seconds
13 files · +399 -57
M README.md +2 -2
| @@ -249,7 +249,7 @@ Print-mode stats contain `provider`, `model`, `prompt_tokens`, `reasoning_tokens | |||
| 249 | 249 | - `read`: read text files, or inline images (PNG, JPEG, GIF, WebP). | |
| 250 | 250 | - `write`: create or overwrite files, making parent directories as needed. | |
| 251 | 251 | - `edit`: one or more exact-match replacements in an existing file. | |
| 252 | - | - `bash`: run a command in the session cwd with merged stdout/stderr and a timeout. On Unix, zut uses `/bin/bash -c` when available, then `bash -c` from `PATH`, and falls back to POSIX `/bin/sh -c` when Bash is unavailable. On Windows, it uses `cmd /C`. macOS ships Bash 3.2 by default, so newer Bash features may be unavailable. | |
| 252 | + | - `bash`: run a command in the session cwd with merged stdout/stderr. Every call must include a positive integer `timeout` in seconds; calls without it or with `timeout <= 0` are rejected, and the deadline is always enforced. On Unix, zut uses `/bin/bash -c` when available, then `bash -c` from `PATH`, and falls back to POSIX `/bin/sh -c` when Bash is unavailable. On Windows, it uses `cmd /C`. macOS ships Bash 3.2 by default, so newer Bash features may be unavailable. | |
| 253 | 253 | - `create_worktree`: create a new branch from the current `HEAD` and check it out persistently. A repository with no configured root and no `.worktrees` directory first returns bootstrap guidance without making changes; the agent asks whether to use the repository default or an external root, then retries with `bootstrap_root`. The choice is saved privately in local Git config. Choosing `.worktrees` creates `<repository-root>/.worktrees/<branch>` and adds `/.worktrees/` to the root `.gitignore`; an absolute external root leaves tracked repository files and the root `.gitignore` unchanged. The tool never copies uncommitted or ignored files, and it refuses an existing branch or worktree path. | |
| 254 | 254 | - `lsp`: query configured language servers and linters for diagnostics, definitions, references, hover information, symbols, renames, code actions, capabilities, and raw protocol requests. LSP servers use stdio JSON-RPC; project `lsp.json` files can add or override servers and CLI linters. Diagnostics are bounded, sorted, deduplicated by path/severity/code/start position, and repeated issues are grouped before they reach the model. See [docs/lsp.md](docs/lsp.md). | |
| 255 | 255 | - `web_search`: search the public web through DuckDuckGo HTML and return bounded source titles, URLs, and snippets. Enabled by default for normal CLI sessions; it is unavailable to bot, default SDK, and packaged-agent runs. See [docs/web-search.md](docs/web-search.md). | |
| @@ -340,7 +340,7 @@ A branch that copies the complete transcript inherits the goal. Forking from an | |||
| 340 | 340 | ||
| 341 | 341 | ### Shell escape (`!command`) | |
| 342 | 342 | ||
| 343 | - | Type `!` followed by a command to run it directly without going through the model. Everything after the `!` is passed to the same shell the `bash` tool uses (`/bin/bash -c` when available on Unix, then `bash -c` from `PATH`, with POSIX `/bin/sh -c` as a fallback; `cmd /C` on Windows), runs in the session working directory, and honors the `/jail` sandbox. The command, merged output, and exit status are appended to the transcript as user context, so the model can use them on the next turn. Running the command does not itself start a model turn. A running `!command` shares the busy state with the agent: `esc` cancels it, and you cannot start one while a turn (or another shell escape) is in flight. | |
| 343 | + | Type `!` followed by a command to run it directly without going through the model. Everything after the `!` is passed to the same shell the `bash` tool uses (`/bin/bash -c` when available on Unix, then `bash -c` from `PATH`, with POSIX `/bin/sh -c` as a fallback; `cmd /C` on Windows), runs in the session working directory, and honors the `/jail` sandbox. Interactive shell escapes always use zut's built-in 600-second (10-minute) timeout. The command, merged output, and exit status are appended to the transcript as user context, so the model can use them on the next turn. Running the command does not itself start a model turn. A running `!command` shares the busy state with the agent: `esc` cancels it, and you cannot start one while a turn (or another shell escape) is in flight. | |
| 344 | 344 | ||
| 345 | 345 | ### `/sessions` | |
| 346 | 346 | ||
| @@ -249,7 +249,7 @@ Print-mode stats contain `provider`, `model`, `prompt_tokens`, `reasoning_tokens | |||||
| 249 | - `read`: read text files, or inline images (PNG, JPEG, GIF, WebP). | 249 | - `read`: read text files, or inline images (PNG, JPEG, GIF, WebP). | ||
| 250 | - `write`: create or overwrite files, making parent directories as needed. | 250 | - `write`: create or overwrite files, making parent directories as needed. | ||
| 251 | - `edit`: one or more exact-match replacements in an existing file. | 251 | - `edit`: one or more exact-match replacements in an existing file. | ||
| 252 | - | - `bash`: run a command in the session cwd with merged stdout/stderr and a timeout. On Unix, zut uses `/bin/bash -c` when available, then `bash -c` from `PATH`, and falls back to POSIX `/bin/sh -c` when Bash is unavailable. On Windows, it uses `cmd /C`. macOS ships Bash 3.2 by default, so newer Bash features may be unavailable. | 252 | + | - `bash`: run a command in the session cwd with merged stdout/stderr. Every call must include a positive integer `timeout` in seconds; calls without it or with `timeout <= 0` are rejected, and the deadline is always enforced. On Unix, zut uses `/bin/bash -c` when available, then `bash -c` from `PATH`, and falls back to POSIX `/bin/sh -c` when Bash is unavailable. On Windows, it uses `cmd /C`. macOS ships Bash 3.2 by default, so newer Bash features may be unavailable. |
| 253 | - `create_worktree`: create a new branch from the current `HEAD` and check it out persistently. A repository with no configured root and no `.worktrees` directory first returns bootstrap guidance without making changes; the agent asks whether to use the repository default or an external root, then retries with `bootstrap_root`. The choice is saved privately in local Git config. Choosing `.worktrees` creates `<repository-root>/.worktrees/<branch>` and adds `/.worktrees/` to the root `.gitignore`; an absolute external root leaves tracked repository files and the root `.gitignore` unchanged. The tool never copies uncommitted or ignored files, and it refuses an existing branch or worktree path. | 253 | - `create_worktree`: create a new branch from the current `HEAD` and check it out persistently. A repository with no configured root and no `.worktrees` directory first returns bootstrap guidance without making changes; the agent asks whether to use the repository default or an external root, then retries with `bootstrap_root`. The choice is saved privately in local Git config. Choosing `.worktrees` creates `<repository-root>/.worktrees/<branch>` and adds `/.worktrees/` to the root `.gitignore`; an absolute external root leaves tracked repository files and the root `.gitignore` unchanged. The tool never copies uncommitted or ignored files, and it refuses an existing branch or worktree path. | ||
| 254 | - `lsp`: query configured language servers and linters for diagnostics, definitions, references, hover information, symbols, renames, code actions, capabilities, and raw protocol requests. LSP servers use stdio JSON-RPC; project `lsp.json` files can add or override servers and CLI linters. Diagnostics are bounded, sorted, deduplicated by path/severity/code/start position, and repeated issues are grouped before they reach the model. See [docs/lsp.md](docs/lsp.md). | 254 | - `lsp`: query configured language servers and linters for diagnostics, definitions, references, hover information, symbols, renames, code actions, capabilities, and raw protocol requests. LSP servers use stdio JSON-RPC; project `lsp.json` files can add or override servers and CLI linters. Diagnostics are bounded, sorted, deduplicated by path/severity/code/start position, and repeated issues are grouped before they reach the model. See [docs/lsp.md](docs/lsp.md). | ||
| 255 | - `web_search`: search the public web through DuckDuckGo HTML and return bounded source titles, URLs, and snippets. Enabled by default for normal CLI sessions; it is unavailable to bot, default SDK, and packaged-agent runs. See [docs/web-search.md](docs/web-search.md). | 255 | - `web_search`: search the public web through DuckDuckGo HTML and return bounded source titles, URLs, and snippets. Enabled by default for normal CLI sessions; it is unavailable to bot, default SDK, and packaged-agent runs. See [docs/web-search.md](docs/web-search.md). | ||
| @@ -340,7 +340,7 @@ A branch that copies the complete transcript inherits the goal. Forking from an | |||||
| 340 | 340 | ||||
| 341 | ### Shell escape (`!command`) | 341 | ### Shell escape (`!command`) | ||
| 342 | 342 | ||||
| 343 | - | Type `!` followed by a command to run it directly without going through the model. Everything after the `!` is passed to the same shell the `bash` tool uses (`/bin/bash -c` when available on Unix, then `bash -c` from `PATH`, with POSIX `/bin/sh -c` as a fallback; `cmd /C` on Windows), runs in the session working directory, and honors the `/jail` sandbox. The command, merged output, and exit status are appended to the transcript as user context, so the model can use them on the next turn. Running the command does not itself start a model turn. A running `!command` shares the busy state with the agent: `esc` cancels it, and you cannot start one while a turn (or another shell escape) is in flight. | 343 | + | Type `!` followed by a command to run it directly without going through the model. Everything after the `!` is passed to the same shell the `bash` tool uses (`/bin/bash -c` when available on Unix, then `bash -c` from `PATH`, with POSIX `/bin/sh -c` as a fallback; `cmd /C` on Windows), runs in the session working directory, and honors the `/jail` sandbox. Interactive shell escapes always use zut's built-in 600-second (10-minute) timeout. The command, merged output, and exit status are appended to the transcript as user context, so the model can use them on the next turn. Running the command does not itself start a model turn. A running `!command` shares the busy state with the agent: `esc` cancels it, and you cannot start one while a turn (or another shell escape) is in flight. |
| 344 | 344 | ||||
| 345 | ### `/sessions` | 345 | ### `/sessions` | ||
| 346 | 346 | ||||
M docs/extensions.md +4 -4
| @@ -318,7 +318,7 @@ Reply to an `event_intercept` from the host. All fields default to | |||
| 318 | 318 | |---|---| | |
| 319 | 319 | | `block` | `true` refuses the action. For `tool_call`, `reason` is shown to the model; for `turn_start` / `assistant_message`, `reason` is shown to the user. | | |
| 320 | 320 | | `reason` | refusal text (on block) or pass-through note. | | |
| 321 | - | | `modified_args` | for `tool_call`: rewritten JSON args the tool will actually see. Must be a valid JSON object. Ignored when `block` is true. | | |
| 321 | + | | `modified_args` | for `tool_call`: rewritten JSON args the tool will actually see. Must be a valid JSON object. Ignored when `block` is true. Rewrites replace the complete argument object; for `bash`, preserve the required positive integer `timeout` when changing `command`. | | |
| 322 | 322 | | `replace_text` | for `assistant_message`: replaces the user-visible text. The model's original output still lives in the transcript. Ignored when `block` is true. | | |
| 323 | 323 | | `context` | for `turn_start`: bounded hidden context sent only in the upcoming provider request. It is not a user message and is not persisted in the transcript. | | |
| 324 | 324 | ||
| @@ -333,7 +333,7 @@ subsequent interceptor sees the previous one's output. | |||
| 333 | 333 | "block":true,"reason":"refused: matches danger pattern \"rm -rf\""} | |
| 334 | 334 | ||
| 335 | 335 | {"type":"event_intercept_response","id":"...", | |
| 336 | - | "modified_args":{"command":"echo GUARDED: ls"}} | |
| 336 | + | "modified_args":{"command":"echo GUARDED: ls","timeout":30}} | |
| 337 | 337 | ||
| 338 | 338 | {"type":"event_intercept_response","id":"...", | |
| 339 | 339 | "replace_text":"[redacted]"} | |
| @@ -579,7 +579,7 @@ Payload fields depend on the event: | |||
| 579 | 579 | // tool_call: includes the tool id, name, and parsed args | |
| 580 | 580 | {"type":"event_intercept","id":"...","event":"tool_call", | |
| 581 | 581 | "tool_id":"...","tool_name":"bash", | |
| 582 | - | "tool_args":{"command":"rm -rf /tmp/foo"}} | |
| 582 | + | "tool_args":{"command":"rm -rf /tmp/foo","timeout":30}} | |
| 583 | 583 | ||
| 584 | 584 | // turn_start: includes the step number | |
| 585 | 585 | {"type":"event_intercept","id":"...","event":"turn_start", | |
| @@ -743,7 +743,7 @@ e.InterceptToolCall(func(tool string, args json.RawMessage) (bool, string) { | |||
| 743 | 743 | // args via ModifiedArgs. | |
| 744 | 744 | e.InterceptToolCallX(func(tool string, args json.RawMessage) ext.ToolCallDecision { | |
| 745 | 745 | return ext.ToolCallDecision{ | |
| 746 | - | ModifiedArgs: json.RawMessage(`{"command":"echo GUARDED"}`), | |
| 746 | + | ModifiedArgs: json.RawMessage(`{"command":"echo GUARDED","timeout":30}`), | |
| 747 | 747 | } | |
| 748 | 748 | }) | |
| 749 | 749 | ||
| @@ -318,7 +318,7 @@ Reply to an `event_intercept` from the host. All fields default to | |||||
| 318 | |---|---| | 318 | |---|---| | ||
| 319 | | `block` | `true` refuses the action. For `tool_call`, `reason` is shown to the model; for `turn_start` / `assistant_message`, `reason` is shown to the user. | | 319 | | `block` | `true` refuses the action. For `tool_call`, `reason` is shown to the model; for `turn_start` / `assistant_message`, `reason` is shown to the user. | | ||
| 320 | | `reason` | refusal text (on block) or pass-through note. | | 320 | | `reason` | refusal text (on block) or pass-through note. | | ||
| 321 | - | | `modified_args` | for `tool_call`: rewritten JSON args the tool will actually see. Must be a valid JSON object. Ignored when `block` is true. | | 321 | + | | `modified_args` | for `tool_call`: rewritten JSON args the tool will actually see. Must be a valid JSON object. Ignored when `block` is true. Rewrites replace the complete argument object; for `bash`, preserve the required positive integer `timeout` when changing `command`. | |
| 322 | | `replace_text` | for `assistant_message`: replaces the user-visible text. The model's original output still lives in the transcript. Ignored when `block` is true. | | 322 | | `replace_text` | for `assistant_message`: replaces the user-visible text. The model's original output still lives in the transcript. Ignored when `block` is true. | | ||
| 323 | | `context` | for `turn_start`: bounded hidden context sent only in the upcoming provider request. It is not a user message and is not persisted in the transcript. | | 323 | | `context` | for `turn_start`: bounded hidden context sent only in the upcoming provider request. It is not a user message and is not persisted in the transcript. | | ||
| 324 | 324 | ||||
| @@ -333,7 +333,7 @@ subsequent interceptor sees the previous one's output. | |||||
| 333 | "block":true,"reason":"refused: matches danger pattern \"rm -rf\""} | 333 | "block":true,"reason":"refused: matches danger pattern \"rm -rf\""} | ||
| 334 | 334 | ||||
| 335 | {"type":"event_intercept_response","id":"...", | 335 | {"type":"event_intercept_response","id":"...", | ||
| 336 | - | "modified_args":{"command":"echo GUARDED: ls"}} | 336 | + | "modified_args":{"command":"echo GUARDED: ls","timeout":30}} |
| 337 | 337 | ||||
| 338 | {"type":"event_intercept_response","id":"...", | 338 | {"type":"event_intercept_response","id":"...", | ||
| 339 | "replace_text":"[redacted]"} | 339 | "replace_text":"[redacted]"} | ||
| @@ -579,7 +579,7 @@ Payload fields depend on the event: | |||||
| 579 | // tool_call: includes the tool id, name, and parsed args | 579 | // tool_call: includes the tool id, name, and parsed args | ||
| 580 | {"type":"event_intercept","id":"...","event":"tool_call", | 580 | {"type":"event_intercept","id":"...","event":"tool_call", | ||
| 581 | "tool_id":"...","tool_name":"bash", | 581 | "tool_id":"...","tool_name":"bash", | ||
| 582 | - | "tool_args":{"command":"rm -rf /tmp/foo"}} | 582 | + | "tool_args":{"command":"rm -rf /tmp/foo","timeout":30}} |
| 583 | 583 | ||||
| 584 | // turn_start: includes the step number | 584 | // turn_start: includes the step number | ||
| 585 | {"type":"event_intercept","id":"...","event":"turn_start", | 585 | {"type":"event_intercept","id":"...","event":"turn_start", | ||
| @@ -743,7 +743,7 @@ e.InterceptToolCall(func(tool string, args json.RawMessage) (bool, string) { | |||||
| 743 | // args via ModifiedArgs. | 743 | // args via ModifiedArgs. | ||
| 744 | e.InterceptToolCallX(func(tool string, args json.RawMessage) ext.ToolCallDecision { | 744 | e.InterceptToolCallX(func(tool string, args json.RawMessage) ext.ToolCallDecision { | ||
| 745 | return ext.ToolCallDecision{ | 745 | return ext.ToolCallDecision{ | ||
| 746 | - | ModifiedArgs: json.RawMessage(`{"command":"echo GUARDED"}`), | 746 | + | ModifiedArgs: json.RawMessage(`{"command":"echo GUARDED","timeout":30}`), |
| 747 | } | 747 | } | ||
| 748 | }) | 748 | }) | ||
| 749 | 749 | ||||
M docs/zutfiles.md +1 -1
| @@ -275,7 +275,7 @@ Zut checks these requirements before requesting consent. These zutfiles do not h | |||
| 275 | 275 | ||
| 276 | 276 | ### Entry fields | |
| 277 | 277 | ||
| 278 | - | `entry.pre` is auto-submitted once when `zut run` starts, before the initial prompt is applied. Values that begin with `!` use the interactive shell-escape path (or `BashTool` in `--print` / `--stream` / `--json`). Shell output streams live to the TUI (interactive) or stderr (non-interactive). Other values are sent as a normal user turn; in `--stream` mode that turn's assistant text also streams to stdout. After `pre` finishes, zut reloads extensions and rediscovers skills so anything installed by that command is available for the following turn. | |
| 278 | + | `entry.pre` is auto-submitted once when `zut run` starts, before the initial prompt is applied. Values that begin with `!` use the interactive shell-escape path (or `BashTool` in `--print` / `--stream` / `--json`) and always receive the built-in 600-second (10-minute) timeout. Shell output streams live to the TUI (interactive) or stderr (non-interactive). Other values are sent as a normal user turn; in `--stream` mode that turn's assistant text also streams to stdout. After `pre` finishes, zut reloads extensions and rediscovers skills so anything installed by that command is available for the following turn. | |
| 279 | 279 | ||
| 280 | 280 | ```json | |
| 281 | 281 | { | |
| @@ -275,7 +275,7 @@ Zut checks these requirements before requesting consent. These zutfiles do not h | |||||
| 275 | 275 | ||||
| 276 | ### Entry fields | 276 | ### Entry fields | ||
| 277 | 277 | ||||
| 278 | - | `entry.pre` is auto-submitted once when `zut run` starts, before the initial prompt is applied. Values that begin with `!` use the interactive shell-escape path (or `BashTool` in `--print` / `--stream` / `--json`). Shell output streams live to the TUI (interactive) or stderr (non-interactive). Other values are sent as a normal user turn; in `--stream` mode that turn's assistant text also streams to stdout. After `pre` finishes, zut reloads extensions and rediscovers skills so anything installed by that command is available for the following turn. | 278 | + | `entry.pre` is auto-submitted once when `zut run` starts, before the initial prompt is applied. Values that begin with `!` use the interactive shell-escape path (or `BashTool` in `--print` / `--stream` / `--json`) and always receive the built-in 600-second (10-minute) timeout. Shell output streams live to the TUI (interactive) or stderr (non-interactive). Other values are sent as a normal user turn; in `--stream` mode that turn's assistant text also streams to stdout. After `pre` finishes, zut reloads extensions and rediscovers skills so anything installed by that command is available for the following turn. |
| 279 | 279 | ||||
| 280 | ```json | 280 | ```json | ||
| 281 | { | 281 | { | ||
M packages/agent/cli.go +4 -1
| @@ -1267,7 +1267,10 @@ func runZutfileStartupPre(ctx context.Context, pre, cwd string, sandbox *tools.S | |||
| 1267 | 1267 | return nil | |
| 1268 | 1268 | } | |
| 1269 | 1269 | if cmd, ok := modes.ShellEscapeCommand(pre); ok { | |
| 1270 | - | raw, err := json.Marshal(map[string]any{"command": cmd}) | |
| 1270 | + | raw, err := json.Marshal(map[string]any{ | |
| 1271 | + | "command": cmd, | |
| 1272 | + | "timeout": modes.ShellEscapeTimeoutSeconds, | |
| 1273 | + | }) | |
| 1271 | 1274 | if err != nil { | |
| 1272 | 1275 | return err | |
| 1273 | 1276 | } | |
| @@ -1267,7 +1267,10 @@ func runZutfileStartupPre(ctx context.Context, pre, cwd string, sandbox *tools.S | |||||
| 1267 | return nil | 1267 | return nil | ||
| 1268 | } | 1268 | } | ||
| 1269 | if cmd, ok := modes.ShellEscapeCommand(pre); ok { | 1269 | if cmd, ok := modes.ShellEscapeCommand(pre); ok { | ||
| 1270 | - | raw, err := json.Marshal(map[string]any{"command": cmd}) | 1270 | + | raw, err := json.Marshal(map[string]any{ |
| 1271 | + | "command": cmd, | |||
| 1272 | + | "timeout": modes.ShellEscapeTimeoutSeconds, | |||
| 1273 | + | }) | |||
| 1271 | if err != nil { | 1274 | if err != nil { | ||
| 1272 | return err | 1275 | return err | ||
| 1273 | } | 1276 | } | ||
M packages/agent/extensions/intercept_test.go +15 -6
| @@ -56,7 +56,8 @@ while IFS= read -r line; do | |||
| 56 | 56 | if [[ "$name" == "bash" && "$cmd" == *"rm -rf"* ]]; then | |
| 57 | 57 | emit "{\"type\":\"event_intercept_response\",\"id\":\"$id\",\"block\":true,\"reason\":\"refused: rm -rf\"}" | |
| 58 | 58 | elif [[ "$name" == "bash" && -n "$cmd" ]]; then | |
| 59 | - | new=$(python3 -c "import json,sys;print(json.dumps({'command':'echo GUARDED: '+sys.argv[1]}))" "$cmd") | |
| 59 | + | timeout=$(printf '%s' "$args" | python3 -c 'import sys,json;print(json.load(sys.stdin).get("timeout",0))') | |
| 60 | + | new=$(python3 -c "import json,sys;print(json.dumps({'command':'echo GUARDED: '+sys.argv[1], 'timeout': int(sys.argv[2])}))" "$cmd" "$timeout") | |
| 60 | 61 | emit "{\"type\":\"event_intercept_response\",\"id\":\"$id\",\"modified_args\":$new}" | |
| 61 | 62 | else | |
| 62 | 63 | emit "{\"type\":\"event_intercept_response\",\"id\":\"$id\"}" | |
| @@ -95,24 +96,32 @@ done | |||
| 95 | 96 | m.WaitForReady(3 * time.Second) | |
| 96 | 97 | ||
| 97 | 98 | // tool_call: rm -rf is blocked | |
| 98 | - | res := m.InterceptToolCall(ctx, "T1", "bash", json.RawMessage(`{"command":"rm -rf /tmp/foo"}`)) | |
| 99 | + | res := m.InterceptToolCall(ctx, "T1", "bash", json.RawMessage(`{"command":"rm -rf /tmp/foo","timeout":30}`)) | |
| 99 | 100 | if !res.Block || !strings.Contains(res.Reason, "refused") { | |
| 100 | 101 | t.Errorf("rm -rf: want block+reason, got %+v", res) | |
| 101 | 102 | } | |
| 102 | 103 | ||
| 103 | 104 | // tool_call: non-dangerous bash gets args rewritten | |
| 104 | - | res = m.InterceptToolCall(ctx, "T2", "bash", json.RawMessage(`{"command":"ls -la"}`)) | |
| 105 | + | res = m.InterceptToolCall(ctx, "T2", "bash", json.RawMessage(`{"command":"ls -la","timeout":30}`)) | |
| 105 | 106 | if res.Block { | |
| 106 | 107 | t.Errorf("ls -la: want allow, got block %q", res.Reason) | |
| 107 | 108 | } | |
| 108 | 109 | if len(res.ModifiedArgs) == 0 { | |
| 109 | 110 | t.Errorf("ls -la: want modified_args, got nothing") | |
| 110 | 111 | } else { | |
| 111 | - | var obj map[string]string | |
| 112 | + | var obj struct { | |
| 113 | + | Command string `json:"command"` | |
| 114 | + | Timeout int `json:"timeout"` | |
| 115 | + | } | |
| 112 | 116 | if err := json.Unmarshal(res.ModifiedArgs, &obj); err != nil { | |
| 113 | 117 | t.Errorf("modified_args: %v", err) | |
| 114 | - | } else if !strings.HasPrefix(obj["command"], "echo GUARDED") { | |
| 115 | - | t.Errorf("modified_args command=%q, want echo GUARDED prefix", obj["command"]) | |
| 118 | + | } else { | |
| 119 | + | if !strings.HasPrefix(obj.Command, "echo GUARDED") { | |
| 120 | + | t.Errorf("modified_args command=%q, want echo GUARDED prefix", obj.Command) | |
| 121 | + | } | |
| 122 | + | if obj.Timeout != 30 { | |
| 123 | + | t.Errorf("modified_args timeout=%d, want 30", obj.Timeout) | |
| 124 | + | } | |
| 116 | 125 | } | |
| 117 | 126 | } | |
| 118 | 127 | ||
| @@ -56,7 +56,8 @@ while IFS= read -r line; do | |||||
| 56 | if [[ "$name" == "bash" && "$cmd" == *"rm -rf"* ]]; then | 56 | if [[ "$name" == "bash" && "$cmd" == *"rm -rf"* ]]; then | ||
| 57 | emit "{\"type\":\"event_intercept_response\",\"id\":\"$id\",\"block\":true,\"reason\":\"refused: rm -rf\"}" | 57 | emit "{\"type\":\"event_intercept_response\",\"id\":\"$id\",\"block\":true,\"reason\":\"refused: rm -rf\"}" | ||
| 58 | elif [[ "$name" == "bash" && -n "$cmd" ]]; then | 58 | elif [[ "$name" == "bash" && -n "$cmd" ]]; then | ||
| 59 | - | new=$(python3 -c "import json,sys;print(json.dumps({'command':'echo GUARDED: '+sys.argv[1]}))" "$cmd") | 59 | + | timeout=$(printf '%s' "$args" | python3 -c 'import sys,json;print(json.load(sys.stdin).get("timeout",0))') |
| 60 | + | new=$(python3 -c "import json,sys;print(json.dumps({'command':'echo GUARDED: '+sys.argv[1], 'timeout': int(sys.argv[2])}))" "$cmd" "$timeout") | |||
| 60 | emit "{\"type\":\"event_intercept_response\",\"id\":\"$id\",\"modified_args\":$new}" | 61 | emit "{\"type\":\"event_intercept_response\",\"id\":\"$id\",\"modified_args\":$new}" | ||
| 61 | else | 62 | else | ||
| 62 | emit "{\"type\":\"event_intercept_response\",\"id\":\"$id\"}" | 63 | emit "{\"type\":\"event_intercept_response\",\"id\":\"$id\"}" | ||
| @@ -95,24 +96,32 @@ done | |||||
| 95 | m.WaitForReady(3 * time.Second) | 96 | m.WaitForReady(3 * time.Second) | ||
| 96 | 97 | ||||
| 97 | // tool_call: rm -rf is blocked | 98 | // tool_call: rm -rf is blocked | ||
| 98 | - | res := m.InterceptToolCall(ctx, "T1", "bash", json.RawMessage(`{"command":"rm -rf /tmp/foo"}`)) | 99 | + | res := m.InterceptToolCall(ctx, "T1", "bash", json.RawMessage(`{"command":"rm -rf /tmp/foo","timeout":30}`)) |
| 99 | if !res.Block || !strings.Contains(res.Reason, "refused") { | 100 | if !res.Block || !strings.Contains(res.Reason, "refused") { | ||
| 100 | t.Errorf("rm -rf: want block+reason, got %+v", res) | 101 | t.Errorf("rm -rf: want block+reason, got %+v", res) | ||
| 101 | } | 102 | } | ||
| 102 | 103 | ||||
| 103 | // tool_call: non-dangerous bash gets args rewritten | 104 | // tool_call: non-dangerous bash gets args rewritten | ||
| 104 | - | res = m.InterceptToolCall(ctx, "T2", "bash", json.RawMessage(`{"command":"ls -la"}`)) | 105 | + | res = m.InterceptToolCall(ctx, "T2", "bash", json.RawMessage(`{"command":"ls -la","timeout":30}`)) |
| 105 | if res.Block { | 106 | if res.Block { | ||
| 106 | t.Errorf("ls -la: want allow, got block %q", res.Reason) | 107 | t.Errorf("ls -la: want allow, got block %q", res.Reason) | ||
| 107 | } | 108 | } | ||
| 108 | if len(res.ModifiedArgs) == 0 { | 109 | if len(res.ModifiedArgs) == 0 { | ||
| 109 | t.Errorf("ls -la: want modified_args, got nothing") | 110 | t.Errorf("ls -la: want modified_args, got nothing") | ||
| 110 | } else { | 111 | } else { | ||
| 111 | - | var obj map[string]string | 112 | + | var obj struct { |
| 113 | + | Command string `json:"command"` | |||
| 114 | + | Timeout int `json:"timeout"` | |||
| 115 | + | } | |||
| 112 | if err := json.Unmarshal(res.ModifiedArgs, &obj); err != nil { | 116 | if err := json.Unmarshal(res.ModifiedArgs, &obj); err != nil { | ||
| 113 | t.Errorf("modified_args: %v", err) | 117 | t.Errorf("modified_args: %v", err) | ||
| 114 | - | } else if !strings.HasPrefix(obj["command"], "echo GUARDED") { | 118 | + | } else { |
| 115 | - | t.Errorf("modified_args command=%q, want echo GUARDED prefix", obj["command"]) | 119 | + | if !strings.HasPrefix(obj.Command, "echo GUARDED") { |
| 120 | + | t.Errorf("modified_args command=%q, want echo GUARDED prefix", obj.Command) | |||
| 121 | + | } | |||
| 122 | + | if obj.Timeout != 30 { | |||
| 123 | + | t.Errorf("modified_args timeout=%d, want 30", obj.Timeout) | |||
| 124 | + | } | |||
| 116 | } | 125 | } | ||
| 117 | } | 126 | } | ||
| 118 | 127 | ||||
M packages/agent/modes/interactive.go +6 -1
| @@ -7542,6 +7542,8 @@ func shellEscapeCommand(text string) (string, bool) { | |||
| 7542 | 7542 | // surrounding whitespace) stripped. A bare "!" with no command is | |
| 7543 | 7543 | // treated as not an escape so it falls through to the normal prompt | |
| 7544 | 7544 | // path rather than running an empty shell. | |
| 7545 | + | const ShellEscapeTimeoutSeconds = 600 | |
| 7546 | + | ||
| 7545 | 7547 | func ShellEscapeCommand(text string) (string, bool) { | |
| 7546 | 7548 | trimmed := strings.TrimLeft(text, " \t") | |
| 7547 | 7549 | if !strings.HasPrefix(trimmed, "!") { | |
| @@ -7598,7 +7600,10 @@ func (i *Interactive) startShellEscape(parent context.Context, cmd string) { | |||
| 7598 | 7600 | ||
| 7599 | 7601 | go func() { | |
| 7600 | 7602 | defer cancel() | |
| 7601 | - | raw, _ := json.Marshal(map[string]any{"command": cmd}) | |
| 7603 | + | raw, _ := json.Marshal(map[string]any{ | |
| 7604 | + | "command": cmd, | |
| 7605 | + | "timeout": ShellEscapeTimeoutSeconds, | |
| 7606 | + | }) | |
| 7602 | 7607 | bash := &tools.BashTool{CWD: cwd, Sandbox: sandbox} | |
| 7603 | 7608 | progress := func(chunk string) { | |
| 7604 | 7609 | i.mu.Lock() | |
| @@ -7542,6 +7542,8 @@ func shellEscapeCommand(text string) (string, bool) { | |||||
| 7542 | // surrounding whitespace) stripped. A bare "!" with no command is | 7542 | // surrounding whitespace) stripped. A bare "!" with no command is | ||
| 7543 | // treated as not an escape so it falls through to the normal prompt | 7543 | // treated as not an escape so it falls through to the normal prompt | ||
| 7544 | // path rather than running an empty shell. | 7544 | // path rather than running an empty shell. | ||
| 7545 | + | const ShellEscapeTimeoutSeconds = 600 | |||
| 7546 | + | ||||
| 7545 | func ShellEscapeCommand(text string) (string, bool) { | 7547 | func ShellEscapeCommand(text string) (string, bool) { | ||
| 7546 | trimmed := strings.TrimLeft(text, " \t") | 7548 | trimmed := strings.TrimLeft(text, " \t") | ||
| 7547 | if !strings.HasPrefix(trimmed, "!") { | 7549 | if !strings.HasPrefix(trimmed, "!") { | ||
| @@ -7598,7 +7600,10 @@ func (i *Interactive) startShellEscape(parent context.Context, cmd string) { | |||||
| 7598 | 7600 | ||||
| 7599 | go func() { | 7601 | go func() { | ||
| 7600 | defer cancel() | 7602 | defer cancel() | ||
| 7601 | - | raw, _ := json.Marshal(map[string]any{"command": cmd}) | 7603 | + | raw, _ := json.Marshal(map[string]any{ |
| 7604 | + | "command": cmd, | |||
| 7605 | + | "timeout": ShellEscapeTimeoutSeconds, | |||
| 7606 | + | }) | |||
| 7602 | bash := &tools.BashTool{CWD: cwd, Sandbox: sandbox} | 7607 | bash := &tools.BashTool{CWD: cwd, Sandbox: sandbox} | ||
| 7603 | progress := func(chunk string) { | 7608 | progress := func(chunk string) { | ||
| 7604 | i.mu.Lock() | 7609 | i.mu.Lock() | ||
M packages/agent/skills/builtin/write-zut-extension/SKILL.md +1 -1
| @@ -140,7 +140,7 @@ process itself runs from the extension directory, so do not use | |||
| 140 | 140 | {"type":"tool_call","id":"def","name":"weather","args":{"city":"Berlin"}} | |
| 141 | 141 | {"type":"event","event":"turn_start","step":1} | |
| 142 | 142 | {"type":"event_intercept","id":"ghi","event":"tool_call", | |
| 143 | - | "tool_name":"bash","tool_args":{"command":"rm -rf /tmp/foo"}} | |
| 143 | + | "tool_name":"bash","tool_args":{"command":"rm -rf /tmp/foo","timeout":30}} | |
| 144 | 144 | {"type":"shutdown"} | |
| 145 | 145 | ``` | |
| 146 | 146 | ||
| @@ -140,7 +140,7 @@ process itself runs from the extension directory, so do not use | |||||
| 140 | {"type":"tool_call","id":"def","name":"weather","args":{"city":"Berlin"}} | 140 | {"type":"tool_call","id":"def","name":"weather","args":{"city":"Berlin"}} | ||
| 141 | {"type":"event","event":"turn_start","step":1} | 141 | {"type":"event","event":"turn_start","step":1} | ||
| 142 | {"type":"event_intercept","id":"ghi","event":"tool_call", | 142 | {"type":"event_intercept","id":"ghi","event":"tool_call", | ||
| 143 | - | "tool_name":"bash","tool_args":{"command":"rm -rf /tmp/foo"}} | 143 | + | "tool_name":"bash","tool_args":{"command":"rm -rf /tmp/foo","timeout":30}} |
| 144 | {"type":"shutdown"} | 144 | {"type":"shutdown"} | ||
| 145 | ``` | 145 | ``` | ||
| 146 | 146 | ||||
M packages/agent/tools/bash.go +37 -28
| @@ -6,6 +6,7 @@ import ( | |||
| 6 | 6 | "crypto/rand" | |
| 7 | 7 | "encoding/hex" | |
| 8 | 8 | "encoding/json" | |
| 9 | + | "errors" | |
| 9 | 10 | "fmt" | |
| 10 | 11 | "io" | |
| 11 | 12 | "os" | |
| @@ -22,6 +23,10 @@ import ( | |||
| 22 | 23 | const ( | |
| 23 | 24 | maxBashLines = 2000 | |
| 24 | 25 | maxBashBytes = 50 * 1024 | |
| 26 | + | ||
| 27 | + | // maxBashTimeoutSeconds is the largest whole-second timeout that can be | |
| 28 | + | // converted to a time.Duration without overflowing it. | |
| 29 | + | maxBashTimeoutSeconds int64 = int64(time.Duration(1<<63-1) / time.Second) | |
| 25 | 30 | ) | |
| 26 | 31 | ||
| 27 | 32 | // BashTool runs a shell command in the agent's cwd. | |
| @@ -32,15 +37,22 @@ type BashTool struct { | |||
| 32 | 37 | ||
| 33 | 38 | type bashArgs struct { | |
| 34 | 39 | Command string `json:"command"` | |
| 35 | - | Timeout int `json:"timeout,omitempty"` | |
| 40 | + | Timeout *int64 `json:"timeout"` | |
| 36 | 41 | } | |
| 37 | 42 | ||
| 38 | - | const bashSchema = `{"type":"object","properties":{"command":{"type":"string"},"timeout":{"type":"integer"}},"required":["command"]}` | |
| 43 | + | var bashSchema = fmt.Sprintf(`{"type":"object","properties":{"command":{"type":"string"},"timeout":{"type":"integer","minimum":1,"maximum":%d,"description":"Maximum command runtime in seconds."}},"required":["command","timeout"]}`, maxBashTimeoutSeconds) | |
| 39 | 44 | ||
| 40 | 45 | func (t *BashTool) Name() string { return "bash" } | |
| 41 | 46 | func (t *BashTool) Description() string { return shellDescription(currentShell()) } | |
| 42 | 47 | func (t *BashTool) Schema() json.RawMessage { return json.RawMessage(bashSchema) } | |
| 43 | 48 | ||
| 49 | + | func bashTimeoutDuration(seconds int64) (time.Duration, error) { | |
| 50 | + | if seconds < 1 || seconds > maxBashTimeoutSeconds { | |
| 51 | + | return 0, fmt.Errorf("timeout must be between 1 and %d seconds", maxBashTimeoutSeconds) | |
| 52 | + | } | |
| 53 | + | return time.Duration(seconds) * time.Second, nil | |
| 54 | + | } | |
| 55 | + | ||
| 44 | 56 | func (t *BashTool) Execute(ctx context.Context, raw json.RawMessage, progress func(string)) (core.ToolResult, error) { | |
| 45 | 57 | var a bashArgs | |
| 46 | 58 | if err := json.Unmarshal(raw, &a); err != nil { | |
| @@ -49,6 +61,13 @@ func (t *BashTool) Execute(ctx context.Context, raw json.RawMessage, progress fu | |||
| 49 | 61 | if strings.TrimSpace(a.Command) == "" { | |
| 50 | 62 | return core.ToolResult{}, fmt.Errorf("command is required") | |
| 51 | 63 | } | |
| 64 | + | if a.Timeout == nil { | |
| 65 | + | return core.ToolResult{}, fmt.Errorf("timeout is required") | |
| 66 | + | } | |
| 67 | + | timeout, err := bashTimeoutDuration(*a.Timeout) | |
| 68 | + | if err != nil { | |
| 69 | + | return core.ToolResult{}, err | |
| 70 | + | } | |
| 52 | 71 | if err := t.Sandbox.CheckCommand(a.Command); err != nil { | |
| 53 | 72 | return core.ToolResult{}, err | |
| 54 | 73 | } | |
| @@ -60,46 +79,30 @@ func (t *BashTool) Execute(ctx context.Context, raw json.RawMessage, progress fu | |||
| 60 | 79 | cwd, _ = os.Getwd() | |
| 61 | 80 | } | |
| 62 | 81 | ||
| 63 | - | runCtx := ctx | |
| 64 | - | var cancel context.CancelFunc | |
| 65 | - | if a.Timeout > 0 { | |
| 66 | - | runCtx, cancel = context.WithTimeout(ctx, time.Duration(a.Timeout)*time.Second) | |
| 67 | - | defer cancel() | |
| 68 | - | } | |
| 82 | + | runCtx, cancel := context.WithTimeout(ctx, timeout) | |
| 83 | + | defer cancel() | |
| 69 | 84 | ||
| 70 | 85 | cmd := newShellCmd(runCtx, a.Command) | |
| 71 | 86 | cmd.Dir = cwd | |
| 72 | 87 | cmd.Env = os.Environ() | |
| 73 | - | setProcessGroup(cmd) | |
| 74 | 88 | ||
| 75 | 89 | // Capture merged stdout+stderr with line-by-line streaming. | |
| 76 | 90 | pr, pw := io.Pipe() | |
| 77 | 91 | cmd.Stdout = pw | |
| 78 | 92 | cmd.Stderr = pw | |
| 93 | + | closeOutput := configureBashProcess(cmd, pw) | |
| 79 | 94 | ||
| 80 | 95 | if err := cmd.Start(); err != nil { | |
| 96 | + | closeOutput() | |
| 81 | 97 | return core.ToolResult{}, fmt.Errorf("start: %w", err) | |
| 82 | 98 | } | |
| 83 | 99 | ||
| 84 | 100 | // Writer to both the buffer (trimmed) and progress callback. | |
| 85 | 101 | captured := &bytes.Buffer{} | |
| 86 | - | done := make(chan struct{}) | |
| 102 | + | readerDone := make(chan struct{}) | |
| 87 | 103 | ||
| 88 | - | // Watch for context cancellation and kill the entire process | |
| 89 | - | // group immediately. exec.CommandContext only kills the direct | |
| 90 | - | // process, but child processes (e.g. grep spawned by the shell) | |
| 91 | - | // keep the output pipe open and block cmd.Wait() indefinitely. | |
| 92 | - | go func() { | |
| 93 | - | select { | |
| 94 | - | case <-runCtx.Done(): | |
| 95 | - | killProcessGroup(cmd) | |
| 96 | - | // Close the write end so the reader goroutine unblocks. | |
| 97 | - | pw.Close() | |
| 98 | - | case <-done: | |
| 99 | - | } | |
| 100 | - | }() | |
| 101 | 104 | go func() { | |
| 102 | - | defer close(done) | |
| 105 | + | defer close(readerDone) | |
| 103 | 106 | buf := make([]byte, 4096) | |
| 104 | 107 | for { | |
| 105 | 108 | n, err := pr.Read(buf) | |
| @@ -124,8 +127,8 @@ func (t *BashTool) Execute(ctx context.Context, raw json.RawMessage, progress fu | |||
| 124 | 127 | }() | |
| 125 | 128 | ||
| 126 | 129 | waitErr := cmd.Wait() | |
| 127 | - | pw.Close() | |
| 128 | - | <-done | |
| 130 | + | closeOutput() | |
| 131 | + | <-readerDone | |
| 129 | 132 | ||
| 130 | 133 | output := captured.String() | |
| 131 | 134 | truncBytes := captured.Len() >= maxBashBytes | |
| @@ -166,7 +169,13 @@ func (t *BashTool) Execute(ctx context.Context, raw json.RawMessage, progress fu | |||
| 166 | 169 | fmt.Fprintf(&sb, "... [truncated at %d bytes]\n", maxBashBytes) | |
| 167 | 170 | } | |
| 168 | 171 | sb.WriteString("\n") | |
| 169 | - | if exitCode == 0 { | |
| 172 | + | if errors.Is(runCtx.Err(), context.DeadlineExceeded) && ctx.Err() == nil { | |
| 173 | + | fmt.Fprintf(&sb, "[timed out after %d second", *a.Timeout) | |
| 174 | + | if *a.Timeout != 1 { | |
| 175 | + | sb.WriteByte('s') | |
| 176 | + | } | |
| 177 | + | sb.WriteByte(']') | |
| 178 | + | } else if exitCode == 0 { | |
| 170 | 179 | fmt.Fprintf(&sb, "[exit 0]") | |
| 171 | 180 | } else { | |
| 172 | 181 | fmt.Fprintf(&sb, "[exit %d]", exitCode) | |
| @@ -180,7 +189,7 @@ func (t *BashTool) Execute(ctx context.Context, raw json.RawMessage, progress fu | |||
| 180 | 189 | } | |
| 181 | 190 | } | |
| 182 | 191 | ||
| 183 | - | isErr := exitCode != 0 || ctx.Err() != nil | |
| 192 | + | isErr := exitCode != 0 || runCtx.Err() != nil | |
| 184 | 193 | return core.ToolResult{ | |
| 185 | 194 | Content: []provider.Content{provider.TextBlock{Text: sb.String()}}, | |
| 186 | 195 | IsError: isErr, | |
| @@ -6,6 +6,7 @@ import ( | |||||
| 6 | "crypto/rand" | 6 | "crypto/rand" | ||
| 7 | "encoding/hex" | 7 | "encoding/hex" | ||
| 8 | "encoding/json" | 8 | "encoding/json" | ||
| 9 | + | "errors" | |||
| 9 | "fmt" | 10 | "fmt" | ||
| 10 | "io" | 11 | "io" | ||
| 11 | "os" | 12 | "os" | ||
| @@ -22,6 +23,10 @@ import ( | |||||
| 22 | const ( | 23 | const ( | ||
| 23 | maxBashLines = 2000 | 24 | maxBashLines = 2000 | ||
| 24 | maxBashBytes = 50 * 1024 | 25 | maxBashBytes = 50 * 1024 | ||
| 26 | + | ||||
| 27 | + | // maxBashTimeoutSeconds is the largest whole-second timeout that can be | |||
| 28 | + | // converted to a time.Duration without overflowing it. | |||
| 29 | + | maxBashTimeoutSeconds int64 = int64(time.Duration(1<<63-1) / time.Second) | |||
| 25 | ) | 30 | ) | ||
| 26 | 31 | ||||
| 27 | // BashTool runs a shell command in the agent's cwd. | 32 | // BashTool runs a shell command in the agent's cwd. | ||
| @@ -32,15 +37,22 @@ type BashTool struct { | |||||
| 32 | 37 | ||||
| 33 | type bashArgs struct { | 38 | type bashArgs struct { | ||
| 34 | Command string `json:"command"` | 39 | Command string `json:"command"` | ||
| 35 | - | Timeout int `json:"timeout,omitempty"` | 40 | + | Timeout *int64 `json:"timeout"` |
| 36 | } | 41 | } | ||
| 37 | 42 | ||||
| 38 | - | const bashSchema = `{"type":"object","properties":{"command":{"type":"string"},"timeout":{"type":"integer"}},"required":["command"]}` | 43 | + | var bashSchema = fmt.Sprintf(`{"type":"object","properties":{"command":{"type":"string"},"timeout":{"type":"integer","minimum":1,"maximum":%d,"description":"Maximum command runtime in seconds."}},"required":["command","timeout"]}`, maxBashTimeoutSeconds) |
| 39 | 44 | ||||
| 40 | func (t *BashTool) Name() string { return "bash" } | 45 | func (t *BashTool) Name() string { return "bash" } | ||
| 41 | func (t *BashTool) Description() string { return shellDescription(currentShell()) } | 46 | func (t *BashTool) Description() string { return shellDescription(currentShell()) } | ||
| 42 | func (t *BashTool) Schema() json.RawMessage { return json.RawMessage(bashSchema) } | 47 | func (t *BashTool) Schema() json.RawMessage { return json.RawMessage(bashSchema) } | ||
| 43 | 48 | ||||
| 49 | + | func bashTimeoutDuration(seconds int64) (time.Duration, error) { | |||
| 50 | + | if seconds < 1 || seconds > maxBashTimeoutSeconds { | |||
| 51 | + | return 0, fmt.Errorf("timeout must be between 1 and %d seconds", maxBashTimeoutSeconds) | |||
| 52 | + | } | |||
| 53 | + | return time.Duration(seconds) * time.Second, nil | |||
| 54 | + | } | |||
| 55 | + | ||||
| 44 | func (t *BashTool) Execute(ctx context.Context, raw json.RawMessage, progress func(string)) (core.ToolResult, error) { | 56 | func (t *BashTool) Execute(ctx context.Context, raw json.RawMessage, progress func(string)) (core.ToolResult, error) { | ||
| 45 | var a bashArgs | 57 | var a bashArgs | ||
| 46 | if err := json.Unmarshal(raw, &a); err != nil { | 58 | if err := json.Unmarshal(raw, &a); err != nil { | ||
| @@ -49,6 +61,13 @@ func (t *BashTool) Execute(ctx context.Context, raw json.RawMessage, progress fu | |||||
| 49 | if strings.TrimSpace(a.Command) == "" { | 61 | if strings.TrimSpace(a.Command) == "" { | ||
| 50 | return core.ToolResult{}, fmt.Errorf("command is required") | 62 | return core.ToolResult{}, fmt.Errorf("command is required") | ||
| 51 | } | 63 | } | ||
| 64 | + | if a.Timeout == nil { | |||
| 65 | + | return core.ToolResult{}, fmt.Errorf("timeout is required") | |||
| 66 | + | } | |||
| 67 | + | timeout, err := bashTimeoutDuration(*a.Timeout) | |||
| 68 | + | if err != nil { | |||
| 69 | + | return core.ToolResult{}, err | |||
| 70 | + | } | |||
| 52 | if err := t.Sandbox.CheckCommand(a.Command); err != nil { | 71 | if err := t.Sandbox.CheckCommand(a.Command); err != nil { | ||
| 53 | return core.ToolResult{}, err | 72 | return core.ToolResult{}, err | ||
| 54 | } | 73 | } | ||
| @@ -60,46 +79,30 @@ func (t *BashTool) Execute(ctx context.Context, raw json.RawMessage, progress fu | |||||
| 60 | cwd, _ = os.Getwd() | 79 | cwd, _ = os.Getwd() | ||
| 61 | } | 80 | } | ||
| 62 | 81 | ||||
| 63 | - | runCtx := ctx | 82 | + | runCtx, cancel := context.WithTimeout(ctx, timeout) |
| 64 | - | var cancel context.CancelFunc | 83 | + | defer cancel() |
| 65 | - | if a.Timeout > 0 { | |||
| 66 | - | runCtx, cancel = context.WithTimeout(ctx, time.Duration(a.Timeout)*time.Second) | |||
| 67 | - | defer cancel() | |||
| 68 | - | } | |||
| 69 | 84 | ||||
| 70 | cmd := newShellCmd(runCtx, a.Command) | 85 | cmd := newShellCmd(runCtx, a.Command) | ||
| 71 | cmd.Dir = cwd | 86 | cmd.Dir = cwd | ||
| 72 | cmd.Env = os.Environ() | 87 | cmd.Env = os.Environ() | ||
| 73 | - | setProcessGroup(cmd) | |||
| 74 | 88 | ||||
| 75 | // Capture merged stdout+stderr with line-by-line streaming. | 89 | // Capture merged stdout+stderr with line-by-line streaming. | ||
| 76 | pr, pw := io.Pipe() | 90 | pr, pw := io.Pipe() | ||
| 77 | cmd.Stdout = pw | 91 | cmd.Stdout = pw | ||
| 78 | cmd.Stderr = pw | 92 | cmd.Stderr = pw | ||
| 93 | + | closeOutput := configureBashProcess(cmd, pw) | |||
| 79 | 94 | ||||
| 80 | if err := cmd.Start(); err != nil { | 95 | if err := cmd.Start(); err != nil { | ||
| 96 | + | closeOutput() | |||
| 81 | return core.ToolResult{}, fmt.Errorf("start: %w", err) | 97 | return core.ToolResult{}, fmt.Errorf("start: %w", err) | ||
| 82 | } | 98 | } | ||
| 83 | 99 | ||||
| 84 | // Writer to both the buffer (trimmed) and progress callback. | 100 | // Writer to both the buffer (trimmed) and progress callback. | ||
| 85 | captured := &bytes.Buffer{} | 101 | captured := &bytes.Buffer{} | ||
| 86 | - | done := make(chan struct{}) | 102 | + | readerDone := make(chan struct{}) |
| 87 | 103 | ||||
| 88 | - | // Watch for context cancellation and kill the entire process | |||
| 89 | - | // group immediately. exec.CommandContext only kills the direct | |||
| 90 | - | // process, but child processes (e.g. grep spawned by the shell) | |||
| 91 | - | // keep the output pipe open and block cmd.Wait() indefinitely. | |||
| 92 | - | go func() { | |||
| 93 | - | select { | |||
| 94 | - | case <-runCtx.Done(): | |||
| 95 | - | killProcessGroup(cmd) | |||
| 96 | - | // Close the write end so the reader goroutine unblocks. | |||
| 97 | - | pw.Close() | |||
| 98 | - | case <-done: | |||
| 99 | - | } | |||
| 100 | - | }() | |||
| 101 | go func() { | 104 | go func() { | ||
| 102 | - | defer close(done) | 105 | + | defer close(readerDone) |
| 103 | buf := make([]byte, 4096) | 106 | buf := make([]byte, 4096) | ||
| 104 | for { | 107 | for { | ||
| 105 | n, err := pr.Read(buf) | 108 | n, err := pr.Read(buf) | ||
| @@ -124,8 +127,8 @@ func (t *BashTool) Execute(ctx context.Context, raw json.RawMessage, progress fu | |||||
| 124 | }() | 127 | }() | ||
| 125 | 128 | ||||
| 126 | waitErr := cmd.Wait() | 129 | waitErr := cmd.Wait() | ||
| 127 | - | pw.Close() | 130 | + | closeOutput() |
| 128 | - | <-done | 131 | + | <-readerDone |
| 129 | 132 | ||||
| 130 | output := captured.String() | 133 | output := captured.String() | ||
| 131 | truncBytes := captured.Len() >= maxBashBytes | 134 | truncBytes := captured.Len() >= maxBashBytes | ||
| @@ -166,7 +169,13 @@ func (t *BashTool) Execute(ctx context.Context, raw json.RawMessage, progress fu | |||||
| 166 | fmt.Fprintf(&sb, "... [truncated at %d bytes]\n", maxBashBytes) | 169 | fmt.Fprintf(&sb, "... [truncated at %d bytes]\n", maxBashBytes) | ||
| 167 | } | 170 | } | ||
| 168 | sb.WriteString("\n") | 171 | sb.WriteString("\n") | ||
| 169 | - | if exitCode == 0 { | 172 | + | if errors.Is(runCtx.Err(), context.DeadlineExceeded) && ctx.Err() == nil { |
| 173 | + | fmt.Fprintf(&sb, "[timed out after %d second", *a.Timeout) | |||
| 174 | + | if *a.Timeout != 1 { | |||
| 175 | + | sb.WriteByte('s') | |||
| 176 | + | } | |||
| 177 | + | sb.WriteByte(']') | |||
| 178 | + | } else if exitCode == 0 { | |||
| 170 | fmt.Fprintf(&sb, "[exit 0]") | 179 | fmt.Fprintf(&sb, "[exit 0]") | ||
| 171 | } else { | 180 | } else { | ||
| 172 | fmt.Fprintf(&sb, "[exit %d]", exitCode) | 181 | fmt.Fprintf(&sb, "[exit %d]", exitCode) | ||
| @@ -180,7 +189,7 @@ func (t *BashTool) Execute(ctx context.Context, raw json.RawMessage, progress fu | |||||
| 180 | } | 189 | } | ||
| 181 | } | 190 | } | ||
| 182 | 191 | ||||
| 183 | - | isErr := exitCode != 0 || ctx.Err() != nil | 192 | + | isErr := exitCode != 0 || runCtx.Err() != nil |
| 184 | return core.ToolResult{ | 193 | return core.ToolResult{ | ||
| 185 | Content: []provider.Content{provider.TextBlock{Text: sb.String()}}, | 194 | Content: []provider.Content{provider.TextBlock{Text: sb.String()}}, | ||
| 186 | IsError: isErr, | 195 | IsError: isErr, | ||
M packages/agent/tools/bash_unix.go +45 -3
| @@ -3,11 +3,37 @@ | |||
| 3 | 3 | package tools | |
| 4 | 4 | ||
| 5 | 5 | import ( | |
| 6 | + | "io" | |
| 6 | 7 | "os/exec" | |
| 8 | + | "sync" | |
| 7 | 9 | "syscall" | |
| 8 | 10 | "time" | |
| 9 | 11 | ) | |
| 10 | 12 | ||
| 13 | + | const bashProcessWaitDelay = 5 * time.Second | |
| 14 | + | ||
| 15 | + | // configureBashProcess makes context cancellation terminate the shell's | |
| 16 | + | // process group and closes the output pipe exactly once. CommandContext calls | |
| 17 | + | // cmd.Cancel from its context watcher, so no second cancellation watcher is | |
| 18 | + | // needed here. | |
| 19 | + | func configureBashProcess(cmd *exec.Cmd, output io.Closer) func() { | |
| 20 | + | setProcessGroup(cmd) | |
| 21 | + | cmd.WaitDelay = bashProcessWaitDelay | |
| 22 | + | var closeOutput sync.Once | |
| 23 | + | closePipe := func() { | |
| 24 | + | closeOutput.Do(func() { | |
| 25 | + | if output != nil { | |
| 26 | + | _ = output.Close() | |
| 27 | + | } | |
| 28 | + | }) | |
| 29 | + | } | |
| 30 | + | cmd.Cancel = func() error { | |
| 31 | + | closePipe() | |
| 32 | + | return killBashProcessGroup(cmd) | |
| 33 | + | } | |
| 34 | + | return closePipe | |
| 35 | + | } | |
| 36 | + | ||
| 11 | 37 | // setProcessGroup puts the command in its own process group so | |
| 12 | 38 | // killProcessGroup can target the entire tree including background | |
| 13 | 39 | // children spawned with &. | |
| @@ -15,10 +41,26 @@ func setProcessGroup(cmd *exec.Cmd) { | |||
| 15 | 41 | cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} | |
| 16 | 42 | } | |
| 17 | 43 | ||
| 18 | - | // killProcessGroup sends SIGTERM then SIGKILL to the entire process | |
| 19 | - | // group so backgrounded children (cmd &) are also cleaned up. | |
| 44 | + | // killBashProcessGroup forcefully terminates the entire process group so | |
| 45 | + | // backgrounded children (cmd &) cannot survive cancellation or retain an | |
| 46 | + | // output pipe. | |
| 47 | + | func killBashProcessGroup(cmd *exec.Cmd) error { | |
| 48 | + | if cmd == nil || cmd.Process == nil { | |
| 49 | + | return nil | |
| 50 | + | } | |
| 51 | + | pgid := cmd.Process.Pid | |
| 52 | + | err := syscall.Kill(-pgid, syscall.SIGKILL) | |
| 53 | + | if err == syscall.ESRCH { | |
| 54 | + | return nil | |
| 55 | + | } | |
| 56 | + | return err | |
| 57 | + | } | |
| 58 | + | ||
| 59 | + | // killProcessGroup sends SIGTERM then SIGKILL to the entire process group. | |
| 60 | + | // It is also used by the create_worktree helper, which keeps its existing | |
| 61 | + | // graceful cancellation behavior. | |
| 20 | 62 | func killProcessGroup(cmd *exec.Cmd) { | |
| 21 | - | if cmd.Process == nil { | |
| 63 | + | if cmd == nil || cmd.Process == nil { | |
| 22 | 64 | return | |
| 23 | 65 | } | |
| 24 | 66 | pgid := cmd.Process.Pid | |
| @@ -3,11 +3,37 @@ | |||||
| 3 | package tools | 3 | package tools | ||
| 4 | 4 | ||||
| 5 | import ( | 5 | import ( | ||
| 6 | + | "io" | |||
| 6 | "os/exec" | 7 | "os/exec" | ||
| 8 | + | "sync" | |||
| 7 | "syscall" | 9 | "syscall" | ||
| 8 | "time" | 10 | "time" | ||
| 9 | ) | 11 | ) | ||
| 10 | 12 | ||||
| 13 | + | const bashProcessWaitDelay = 5 * time.Second | |||
| 14 | + | ||||
| 15 | + | // configureBashProcess makes context cancellation terminate the shell's | |||
| 16 | + | // process group and closes the output pipe exactly once. CommandContext calls | |||
| 17 | + | // cmd.Cancel from its context watcher, so no second cancellation watcher is | |||
| 18 | + | // needed here. | |||
| 19 | + | func configureBashProcess(cmd *exec.Cmd, output io.Closer) func() { | |||
| 20 | + | setProcessGroup(cmd) | |||
| 21 | + | cmd.WaitDelay = bashProcessWaitDelay | |||
| 22 | + | var closeOutput sync.Once | |||
| 23 | + | closePipe := func() { | |||
| 24 | + | closeOutput.Do(func() { | |||
| 25 | + | if output != nil { | |||
| 26 | + | _ = output.Close() | |||
| 27 | + | } | |||
| 28 | + | }) | |||
| 29 | + | } | |||
| 30 | + | cmd.Cancel = func() error { | |||
| 31 | + | closePipe() | |||
| 32 | + | return killBashProcessGroup(cmd) | |||
| 33 | + | } | |||
| 34 | + | return closePipe | |||
| 35 | + | } | |||
| 36 | + | ||||
| 11 | // setProcessGroup puts the command in its own process group so | 37 | // setProcessGroup puts the command in its own process group so | ||
| 12 | // killProcessGroup can target the entire tree including background | 38 | // killProcessGroup can target the entire tree including background | ||
| 13 | // children spawned with &. | 39 | // children spawned with &. | ||
| @@ -15,10 +41,26 @@ func setProcessGroup(cmd *exec.Cmd) { | |||||
| 15 | cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} | 41 | cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} | ||
| 16 | } | 42 | } | ||
| 17 | 43 | ||||
| 18 | - | // killProcessGroup sends SIGTERM then SIGKILL to the entire process | 44 | + | // killBashProcessGroup forcefully terminates the entire process group so |
| 19 | - | // group so backgrounded children (cmd &) are also cleaned up. | 45 | + | // backgrounded children (cmd &) cannot survive cancellation or retain an |
| 46 | + | // output pipe. | |||
| 47 | + | func killBashProcessGroup(cmd *exec.Cmd) error { | |||
| 48 | + | if cmd == nil || cmd.Process == nil { | |||
| 49 | + | return nil | |||
| 50 | + | } | |||
| 51 | + | pgid := cmd.Process.Pid | |||
| 52 | + | err := syscall.Kill(-pgid, syscall.SIGKILL) | |||
| 53 | + | if err == syscall.ESRCH { | |||
| 54 | + | return nil | |||
| 55 | + | } | |||
| 56 | + | return err | |||
| 57 | + | } | |||
| 58 | + | ||||
| 59 | + | // killProcessGroup sends SIGTERM then SIGKILL to the entire process group. | |||
| 60 | + | // It is also used by the create_worktree helper, which keeps its existing | |||
| 61 | + | // graceful cancellation behavior. | |||
| 20 | func killProcessGroup(cmd *exec.Cmd) { | 62 | func killProcessGroup(cmd *exec.Cmd) { | ||
| 21 | - | if cmd.Process == nil { | 63 | + | if cmd == nil || cmd.Process == nil { |
| 22 | return | 64 | return | ||
| 23 | } | 65 | } | ||
| 24 | pgid := cmd.Process.Pid | 66 | pgid := cmd.Process.Pid | ||
A packages/agent/tools/bash_unix_test.go +54 -0
| @@ -0,0 +1,54 @@ | |||
| 1 | + | //go:build !windows | |
| 2 | + | ||
| 3 | + | package tools | |
| 4 | + | ||
| 5 | + | import ( | |
| 6 | + | "context" | |
| 7 | + | "errors" | |
| 8 | + | "fmt" | |
| 9 | + | "os" | |
| 10 | + | "path/filepath" | |
| 11 | + | "strconv" | |
| 12 | + | "syscall" | |
| 13 | + | "testing" | |
| 14 | + | "time" | |
| 15 | + | ) | |
| 16 | + | ||
| 17 | + | func TestBashTimeoutKillsDescendant(t *testing.T) { | |
| 18 | + | pidPath := filepath.Join(t.TempDir(), "child.pid") | |
| 19 | + | command := fmt.Sprintf("sleep 30 & child=$!; printf '%%s' \"$child\" > %s; wait", shellQuoteForTest(pidPath)) | |
| 20 | + | tool := &BashTool{CWD: t.TempDir()} | |
| 21 | + | ||
| 22 | + | res, err := tool.Execute(context.Background(), mustJSON(t, map[string]any{ | |
| 23 | + | "command": command, | |
| 24 | + | "timeout": 1, | |
| 25 | + | }), nil) | |
| 26 | + | if err != nil { | |
| 27 | + | t.Fatal(err) | |
| 28 | + | } | |
| 29 | + | if !res.IsError { | |
| 30 | + | t.Fatal("timed-out command should be an error") | |
| 31 | + | } | |
| 32 | + | ||
| 33 | + | data, err := os.ReadFile(pidPath) | |
| 34 | + | if err != nil { | |
| 35 | + | t.Fatalf("read child pid: %v", err) | |
| 36 | + | } | |
| 37 | + | childPID, err := strconv.Atoi(string(data)) | |
| 38 | + | if err != nil || childPID <= 0 { | |
| 39 | + | t.Fatalf("child pid = %q: %v", data, err) | |
| 40 | + | } | |
| 41 | + | ||
| 42 | + | deadline := time.Now().Add(2 * time.Second) | |
| 43 | + | for time.Now().Before(deadline) && bashProcessExists(childPID) { | |
| 44 | + | time.Sleep(10 * time.Millisecond) | |
| 45 | + | } | |
| 46 | + | if bashProcessExists(childPID) { | |
| 47 | + | t.Fatalf("bash descendant pid %d survived timeout cancellation", childPID) | |
| 48 | + | } | |
| 49 | + | } | |
| 50 | + | ||
| 51 | + | func bashProcessExists(pid int) bool { | |
| 52 | + | err := syscall.Kill(pid, 0) | |
| 53 | + | return err == nil || errors.Is(err, syscall.EPERM) | |
| 54 | + | } | |
| @@ -0,0 +1,54 @@ | |||||
| 1 | + | //go:build !windows | |||
| 2 | + | ||||
| 3 | + | package tools | |||
| 4 | + | ||||
| 5 | + | import ( | |||
| 6 | + | "context" | |||
| 7 | + | "errors" | |||
| 8 | + | "fmt" | |||
| 9 | + | "os" | |||
| 10 | + | "path/filepath" | |||
| 11 | + | "strconv" | |||
| 12 | + | "syscall" | |||
| 13 | + | "testing" | |||
| 14 | + | "time" | |||
| 15 | + | ) | |||
| 16 | + | ||||
| 17 | + | func TestBashTimeoutKillsDescendant(t *testing.T) { | |||
| 18 | + | pidPath := filepath.Join(t.TempDir(), "child.pid") | |||
| 19 | + | command := fmt.Sprintf("sleep 30 & child=$!; printf '%%s' \"$child\" > %s; wait", shellQuoteForTest(pidPath)) | |||
| 20 | + | tool := &BashTool{CWD: t.TempDir()} | |||
| 21 | + | ||||
| 22 | + | res, err := tool.Execute(context.Background(), mustJSON(t, map[string]any{ | |||
| 23 | + | "command": command, | |||
| 24 | + | "timeout": 1, | |||
| 25 | + | }), nil) | |||
| 26 | + | if err != nil { | |||
| 27 | + | t.Fatal(err) | |||
| 28 | + | } | |||
| 29 | + | if !res.IsError { | |||
| 30 | + | t.Fatal("timed-out command should be an error") | |||
| 31 | + | } | |||
| 32 | + | ||||
| 33 | + | data, err := os.ReadFile(pidPath) | |||
| 34 | + | if err != nil { | |||
| 35 | + | t.Fatalf("read child pid: %v", err) | |||
| 36 | + | } | |||
| 37 | + | childPID, err := strconv.Atoi(string(data)) | |||
| 38 | + | if err != nil || childPID <= 0 { | |||
| 39 | + | t.Fatalf("child pid = %q: %v", data, err) | |||
| 40 | + | } | |||
| 41 | + | ||||
| 42 | + | deadline := time.Now().Add(2 * time.Second) | |||
| 43 | + | for time.Now().Before(deadline) && bashProcessExists(childPID) { | |||
| 44 | + | time.Sleep(10 * time.Millisecond) | |||
| 45 | + | } | |||
| 46 | + | if bashProcessExists(childPID) { | |||
| 47 | + | t.Fatalf("bash descendant pid %d survived timeout cancellation", childPID) | |||
| 48 | + | } | |||
| 49 | + | } | |||
| 50 | + | ||||
| 51 | + | func bashProcessExists(pid int) bool { | |||
| 52 | + | err := syscall.Kill(pid, 0) | |||
| 53 | + | return err == nil || errors.Is(err, syscall.EPERM) | |||
| 54 | + | } | |||
M packages/agent/tools/bash_windows.go +91 -7
| @@ -3,18 +3,102 @@ | |||
| 3 | 3 | package tools | |
| 4 | 4 | ||
| 5 | 5 | import ( | |
| 6 | + | "errors" | |
| 7 | + | "fmt" | |
| 8 | + | "io" | |
| 9 | + | "os" | |
| 6 | 10 | "os/exec" | |
| 11 | + | "strconv" | |
| 12 | + | "strings" | |
| 13 | + | "sync" | |
| 14 | + | "syscall" | |
| 7 | 15 | "time" | |
| 8 | 16 | ) | |
| 9 | 17 | ||
| 10 | - | func setProcessGroup(_ *exec.Cmd) {} | |
| 18 | + | const bashProcessWaitDelay = 5 * time.Second | |
| 11 | 19 | ||
| 20 | + | // configureBashProcess makes CommandContext cancellation terminate the | |
| 21 | + | // shell and its descendants on Windows. Windows does not provide a Unix-like | |
| 22 | + | // process-group kill operation, so killProcessGroup uses taskkill's tree mode. | |
| 23 | + | func configureBashProcess(cmd *exec.Cmd, output io.Closer) func() { | |
| 24 | + | setProcessGroup(cmd) | |
| 25 | + | cmd.WaitDelay = bashProcessWaitDelay | |
| 26 | + | var closeOutput sync.Once | |
| 27 | + | closePipe := func() { | |
| 28 | + | closeOutput.Do(func() { | |
| 29 | + | if output != nil { | |
| 30 | + | _ = output.Close() | |
| 31 | + | } | |
| 32 | + | }) | |
| 33 | + | } | |
| 34 | + | cmd.Cancel = func() error { | |
| 35 | + | closePipe() | |
| 36 | + | return killBashProcessTree(cmd) | |
| 37 | + | } | |
| 38 | + | return closePipe | |
| 39 | + | } | |
| 40 | + | ||
| 41 | + | // setProcessGroup gives the shell its own Windows process group. The group | |
| 42 | + | // flag prevents it from being attached to the supervisor's console group; | |
| 43 | + | // taskkill /T below is still needed because Windows group creation alone does | |
| 44 | + | // not terminate descendants. | |
| 45 | + | func setProcessGroup(cmd *exec.Cmd) { | |
| 46 | + | cmd.SysProcAttr = &syscall.SysProcAttr{ | |
| 47 | + | CreationFlags: syscall.CREATE_NEW_PROCESS_GROUP, | |
| 48 | + | } | |
| 49 | + | } | |
| 50 | + | ||
| 51 | + | // killBashProcessTree terminates the shell process tree. taskkill is part of | |
| 52 | + | // Windows and /T follows the parent-child relationships to include processes | |
| 53 | + | // spawned by the shell. | |
| 54 | + | func killBashProcessTree(cmd *exec.Cmd) error { | |
| 55 | + | if cmd == nil || cmd.Process == nil { | |
| 56 | + | return nil | |
| 57 | + | } | |
| 58 | + | err := forceKillProcess(cmd.Process.Pid) | |
| 59 | + | var taskkillErr *taskkillError | |
| 60 | + | if errors.As(err, &taskkillErr) && taskkillErr.processDone() { | |
| 61 | + | return os.ErrProcessDone | |
| 62 | + | } | |
| 63 | + | return err | |
| 64 | + | } | |
| 65 | + | ||
| 66 | + | // killProcessGroup preserves the process-tree cancellation helper used by | |
| 67 | + | // other tools while the Bash path uses the error-returning Cancel hook above. | |
| 12 | 68 | func killProcessGroup(cmd *exec.Cmd) { | |
| 13 | - | if cmd.Process == nil { | |
| 14 | - | return | |
| 69 | + | _ = killBashProcessTree(cmd) | |
| 70 | + | } | |
| 71 | + | ||
| 72 | + | func forceKillProcess(pid int) error { | |
| 73 | + | if pid <= 0 { | |
| 74 | + | return nil | |
| 75 | + | } | |
| 76 | + | output, err := exec.Command("taskkill", "/PID", strconv.Itoa(pid), "/T", "/F").CombinedOutput() | |
| 77 | + | if err == nil { | |
| 78 | + | return nil | |
| 15 | 79 | } | |
| 16 | - | _ = cmd.Process.Kill() | |
| 17 | - | time.AfterFunc(3*time.Second, func() { | |
| 18 | - | _ = cmd.Process.Kill() | |
| 19 | - | }) | |
| 80 | + | return &taskkillError{pid: pid, err: err, diagnostics: string(output)} | |
| 81 | + | } | |
| 82 | + | ||
| 83 | + | type taskkillError struct { | |
| 84 | + | pid int | |
| 85 | + | err error | |
| 86 | + | diagnostics string | |
| 87 | + | } | |
| 88 | + | ||
| 89 | + | func (e *taskkillError) Error() string { | |
| 90 | + | if diagnostics := strings.TrimSpace(e.diagnostics); diagnostics != "" { | |
| 91 | + | return fmt.Sprintf("taskkill process tree for PID %d: %v: %s", e.pid, e.err, diagnostics) | |
| 92 | + | } | |
| 93 | + | return fmt.Sprintf("taskkill process tree for PID %d: %v", e.pid, e.err) | |
| 94 | + | } | |
| 95 | + | ||
| 96 | + | func (e *taskkillError) Unwrap() error { return e.err } | |
| 97 | + | ||
| 98 | + | // taskkill returns exit code 128 when the requested process no longer exists. | |
| 99 | + | // Other taskkill failures remain errors so callers can distinguish a | |
| 100 | + | // cancellation race from a real process-control failure. | |
| 101 | + | func (e *taskkillError) processDone() bool { | |
| 102 | + | var exitErr *exec.ExitError | |
| 103 | + | return errors.As(e.err, &exitErr) && exitErr.ExitCode() == 128 | |
| 20 | 104 | } | |
| @@ -3,18 +3,102 @@ | |||||
| 3 | package tools | 3 | package tools | ||
| 4 | 4 | ||||
| 5 | import ( | 5 | import ( | ||
| 6 | + | "errors" | |||
| 7 | + | "fmt" | |||
| 8 | + | "io" | |||
| 9 | + | "os" | |||
| 6 | "os/exec" | 10 | "os/exec" | ||
| 11 | + | "strconv" | |||
| 12 | + | "strings" | |||
| 13 | + | "sync" | |||
| 14 | + | "syscall" | |||
| 7 | "time" | 15 | "time" | ||
| 8 | ) | 16 | ) | ||
| 9 | 17 | ||||
| 10 | - | func setProcessGroup(_ *exec.Cmd) {} | 18 | + | const bashProcessWaitDelay = 5 * time.Second |
| 11 | 19 | ||||
| 20 | + | // configureBashProcess makes CommandContext cancellation terminate the | |||
| 21 | + | // shell and its descendants on Windows. Windows does not provide a Unix-like | |||
| 22 | + | // process-group kill operation, so killProcessGroup uses taskkill's tree mode. | |||
| 23 | + | func configureBashProcess(cmd *exec.Cmd, output io.Closer) func() { | |||
| 24 | + | setProcessGroup(cmd) | |||
| 25 | + | cmd.WaitDelay = bashProcessWaitDelay | |||
| 26 | + | var closeOutput sync.Once | |||
| 27 | + | closePipe := func() { | |||
| 28 | + | closeOutput.Do(func() { | |||
| 29 | + | if output != nil { | |||
| 30 | + | _ = output.Close() | |||
| 31 | + | } | |||
| 32 | + | }) | |||
| 33 | + | } | |||
| 34 | + | cmd.Cancel = func() error { | |||
| 35 | + | closePipe() | |||
| 36 | + | return killBashProcessTree(cmd) | |||
| 37 | + | } | |||
| 38 | + | return closePipe | |||
| 39 | + | } | |||
| 40 | + | ||||
| 41 | + | // setProcessGroup gives the shell its own Windows process group. The group | |||
| 42 | + | // flag prevents it from being attached to the supervisor's console group; | |||
| 43 | + | // taskkill /T below is still needed because Windows group creation alone does | |||
| 44 | + | // not terminate descendants. | |||
| 45 | + | func setProcessGroup(cmd *exec.Cmd) { | |||
| 46 | + | cmd.SysProcAttr = &syscall.SysProcAttr{ | |||
| 47 | + | CreationFlags: syscall.CREATE_NEW_PROCESS_GROUP, | |||
| 48 | + | } | |||
| 49 | + | } | |||
| 50 | + | ||||
| 51 | + | // killBashProcessTree terminates the shell process tree. taskkill is part of | |||
| 52 | + | // Windows and /T follows the parent-child relationships to include processes | |||
| 53 | + | // spawned by the shell. | |||
| 54 | + | func killBashProcessTree(cmd *exec.Cmd) error { | |||
| 55 | + | if cmd == nil || cmd.Process == nil { | |||
| 56 | + | return nil | |||
| 57 | + | } | |||
| 58 | + | err := forceKillProcess(cmd.Process.Pid) | |||
| 59 | + | var taskkillErr *taskkillError | |||
| 60 | + | if errors.As(err, &taskkillErr) && taskkillErr.processDone() { | |||
| 61 | + | return os.ErrProcessDone | |||
| 62 | + | } | |||
| 63 | + | return err | |||
| 64 | + | } | |||
| 65 | + | ||||
| 66 | + | // killProcessGroup preserves the process-tree cancellation helper used by | |||
| 67 | + | // other tools while the Bash path uses the error-returning Cancel hook above. | |||
| 12 | func killProcessGroup(cmd *exec.Cmd) { | 68 | func killProcessGroup(cmd *exec.Cmd) { | ||
| 13 | - | if cmd.Process == nil { | 69 | + | _ = killBashProcessTree(cmd) |
| 14 | - | return | 70 | + | } |
| 71 | + | ||||
| 72 | + | func forceKillProcess(pid int) error { | |||
| 73 | + | if pid <= 0 { | |||
| 74 | + | return nil | |||
| 75 | + | } | |||
| 76 | + | output, err := exec.Command("taskkill", "/PID", strconv.Itoa(pid), "/T", "/F").CombinedOutput() | |||
| 77 | + | if err == nil { | |||
| 78 | + | return nil | |||
| 15 | } | 79 | } | ||
| 16 | - | _ = cmd.Process.Kill() | 80 | + | return &taskkillError{pid: pid, err: err, diagnostics: string(output)} |
| 17 | - | time.AfterFunc(3*time.Second, func() { | 81 | + | } |
| 18 | - | _ = cmd.Process.Kill() | 82 | + | |
| 19 | - | }) | 83 | + | type taskkillError struct { |
| 84 | + | pid int | |||
| 85 | + | err error | |||
| 86 | + | diagnostics string | |||
| 87 | + | } | |||
| 88 | + | ||||
| 89 | + | func (e *taskkillError) Error() string { | |||
| 90 | + | if diagnostics := strings.TrimSpace(e.diagnostics); diagnostics != "" { | |||
| 91 | + | return fmt.Sprintf("taskkill process tree for PID %d: %v: %s", e.pid, e.err, diagnostics) | |||
| 92 | + | } | |||
| 93 | + | return fmt.Sprintf("taskkill process tree for PID %d: %v", e.pid, e.err) | |||
| 94 | + | } | |||
| 95 | + | ||||
| 96 | + | func (e *taskkillError) Unwrap() error { return e.err } | |||
| 97 | + | ||||
| 98 | + | // taskkill returns exit code 128 when the requested process no longer exists. | |||
| 99 | + | // Other taskkill failures remain errors so callers can distinguish a | |||
| 100 | + | // cancellation race from a real process-control failure. | |||
| 101 | + | func (e *taskkillError) processDone() bool { | |||
| 102 | + | var exitErr *exec.ExitError | |||
| 103 | + | return errors.As(e.err, &exitErr) && exitErr.ExitCode() == 128 | |||
| 20 | } | 104 | } | ||
M packages/agent/tools/sandbox_test.go +1 -1
| @@ -69,7 +69,7 @@ func TestBashToolRejectsOutsidePathWhenLocked(t *testing.T) { | |||
| 69 | 69 | sb.Lock() | |
| 70 | 70 | tool := &BashTool{CWD: root, Sandbox: sb} | |
| 71 | 71 | ||
| 72 | - | _, err := tool.Execute(context.Background(), mustJSONRaw(t, map[string]any{"command": "cat " + outsideFile}), nil) | |
| 72 | + | _, err := tool.Execute(context.Background(), mustJSONRaw(t, map[string]any{"command": "cat " + outsideFile, "timeout": 1}), nil) | |
| 73 | 73 | if err == nil { | |
| 74 | 74 | t.Fatal("expected jailed bash to reject outside path") | |
| 75 | 75 | } | |
| @@ -69,7 +69,7 @@ func TestBashToolRejectsOutsidePathWhenLocked(t *testing.T) { | |||||
| 69 | sb.Lock() | 69 | sb.Lock() | ||
| 70 | tool := &BashTool{CWD: root, Sandbox: sb} | 70 | tool := &BashTool{CWD: root, Sandbox: sb} | ||
| 71 | 71 | ||||
| 72 | - | _, err := tool.Execute(context.Background(), mustJSONRaw(t, map[string]any{"command": "cat " + outsideFile}), nil) | 72 | + | _, err := tool.Execute(context.Background(), mustJSONRaw(t, map[string]any{"command": "cat " + outsideFile, "timeout": 1}), nil) |
| 73 | if err == nil { | 73 | if err == nil { | ||
| 74 | t.Fatal("expected jailed bash to reject outside path") | 74 | t.Fatal("expected jailed bash to reject outside path") | ||
| 75 | } | 75 | } | ||
M packages/agent/tools/tools_test.go +138 -2
| @@ -9,6 +9,7 @@ import ( | |||
| 9 | 9 | "runtime" | |
| 10 | 10 | "strings" | |
| 11 | 11 | "testing" | |
| 12 | + | "time" | |
| 12 | 13 | ||
| 13 | 14 | "github.com/bnema/zut/packages/provider" | |
| 14 | 15 | ) | |
| @@ -337,12 +338,90 @@ func TestShellDescription(t *testing.T) { | |||
| 337 | 338 | } | |
| 338 | 339 | } | |
| 339 | 340 | ||
| 341 | + | func TestBashSchemaRequiresTimeoutInSeconds(t *testing.T) { | |
| 342 | + | var schema struct { | |
| 343 | + | Properties map[string]struct { | |
| 344 | + | Type string `json:"type"` | |
| 345 | + | Minimum *int64 `json:"minimum"` | |
| 346 | + | Maximum *int64 `json:"maximum"` | |
| 347 | + | Description string `json:"description"` | |
| 348 | + | } `json:"properties"` | |
| 349 | + | Required []string `json:"required"` | |
| 350 | + | } | |
| 351 | + | if err := json.Unmarshal((&BashTool{}).Schema(), &schema); err != nil { | |
| 352 | + | t.Fatal(err) | |
| 353 | + | } | |
| 354 | + | ||
| 355 | + | timeout, ok := schema.Properties["timeout"] | |
| 356 | + | if !ok { | |
| 357 | + | t.Fatal("timeout property missing from schema") | |
| 358 | + | } | |
| 359 | + | if timeout.Type != "integer" { | |
| 360 | + | t.Fatalf("timeout type = %q, want integer", timeout.Type) | |
| 361 | + | } | |
| 362 | + | if timeout.Minimum == nil || *timeout.Minimum != 1 { | |
| 363 | + | if timeout.Minimum == nil { | |
| 364 | + | t.Fatal("timeout minimum missing from schema") | |
| 365 | + | } | |
| 366 | + | t.Fatalf("timeout minimum = %d, want 1", *timeout.Minimum) | |
| 367 | + | } | |
| 368 | + | if timeout.Maximum == nil { | |
| 369 | + | t.Fatal("timeout maximum missing from schema") | |
| 370 | + | } | |
| 371 | + | if *timeout.Maximum != maxBashTimeoutSeconds { | |
| 372 | + | t.Fatalf("timeout maximum = %d, want %d", *timeout.Maximum, maxBashTimeoutSeconds) | |
| 373 | + | } | |
| 374 | + | if !strings.Contains(strings.ToLower(timeout.Description), "second") { | |
| 375 | + | t.Fatalf("timeout description = %q, want seconds wording", timeout.Description) | |
| 376 | + | } | |
| 377 | + | ||
| 378 | + | required := make(map[string]bool, len(schema.Required)) | |
| 379 | + | for _, name := range schema.Required { | |
| 380 | + | required[name] = true | |
| 381 | + | } | |
| 382 | + | for _, name := range []string{"command", "timeout"} { | |
| 383 | + | if !required[name] { | |
| 384 | + | t.Errorf("required fields missing %q: %v", name, schema.Required) | |
| 385 | + | } | |
| 386 | + | } | |
| 387 | + | } | |
| 388 | + | ||
| 389 | + | func TestBashTimeoutDurationBounds(t *testing.T) { | |
| 390 | + | tests := []struct { | |
| 391 | + | name string | |
| 392 | + | seconds int64 | |
| 393 | + | want time.Duration | |
| 394 | + | wantErr bool | |
| 395 | + | }{ | |
| 396 | + | {name: "minimum", seconds: 1, want: time.Second}, | |
| 397 | + | {name: "maximum", seconds: maxBashTimeoutSeconds, want: time.Duration(maxBashTimeoutSeconds) * time.Second}, | |
| 398 | + | {name: "one over maximum", seconds: maxBashTimeoutSeconds + 1, wantErr: true}, | |
| 399 | + | } | |
| 400 | + | for _, tt := range tests { | |
| 401 | + | t.Run(tt.name, func(t *testing.T) { | |
| 402 | + | got, err := bashTimeoutDuration(tt.seconds) | |
| 403 | + | if tt.wantErr { | |
| 404 | + | if err == nil { | |
| 405 | + | t.Fatalf("bashTimeoutDuration(%d) returned nil error", tt.seconds) | |
| 406 | + | } | |
| 407 | + | return | |
| 408 | + | } | |
| 409 | + | if err != nil { | |
| 410 | + | t.Fatalf("bashTimeoutDuration(%d): %v", tt.seconds, err) | |
| 411 | + | } | |
| 412 | + | if got != tt.want { | |
| 413 | + | t.Fatalf("bashTimeoutDuration(%d) = %s, want %s", tt.seconds, got, tt.want) | |
| 414 | + | } | |
| 415 | + | }) | |
| 416 | + | } | |
| 417 | + | } | |
| 418 | + | ||
| 340 | 419 | func TestBashSuccess(t *testing.T) { | |
| 341 | 420 | if runtime.GOOS == "windows" { | |
| 342 | 421 | t.Skip("posix shell only") | |
| 343 | 422 | } | |
| 344 | 423 | tool := &BashTool{CWD: t.TempDir()} | |
| 345 | - | res, err := tool.Execute(context.Background(), mustJSON(t, map[string]any{"command": "echo hi"}), nil) | |
| 424 | + | res, err := tool.Execute(context.Background(), mustJSON(t, map[string]any{"command": "echo hi", "timeout": 1}), nil) | |
| 346 | 425 | if err != nil { | |
| 347 | 426 | t.Fatal(err) | |
| 348 | 427 | } | |
| @@ -375,6 +454,7 @@ func TestBashSyntax(t *testing.T) { | |||
| 375 | 454 | tool := &BashTool{CWD: t.TempDir()} | |
| 376 | 455 | res, err := tool.Execute(context.Background(), mustJSON(t, map[string]any{ | |
| 377 | 456 | "command": `items=(one two); [[ ${#items[@]} -eq 2 ]] && printf 'bash syntax works\n'`, | |
| 457 | + | "timeout": 1, | |
| 378 | 458 | }), nil) | |
| 379 | 459 | if err != nil { | |
| 380 | 460 | t.Fatal(err) | |
| @@ -390,7 +470,7 @@ func TestBashFailure(t *testing.T) { | |||
| 390 | 470 | t.Skip("posix shell only") | |
| 391 | 471 | } | |
| 392 | 472 | tool := &BashTool{CWD: t.TempDir()} | |
| 393 | - | res, _ := tool.Execute(context.Background(), mustJSON(t, map[string]any{"command": "false"}), nil) | |
| 473 | + | res, _ := tool.Execute(context.Background(), mustJSON(t, map[string]any{"command": "false", "timeout": 1}), nil) | |
| 394 | 474 | if !res.IsError { | |
| 395 | 475 | t.Fatal("want error") | |
| 396 | 476 | } | |
| @@ -399,3 +479,59 @@ func TestBashFailure(t *testing.T) { | |||
| 399 | 479 | t.Fatalf("got %q", got) | |
| 400 | 480 | } | |
| 401 | 481 | } | |
| 482 | + | ||
| 483 | + | func TestBashRejectsInvalidTimeout(t *testing.T) { | |
| 484 | + | tool := &BashTool{CWD: t.TempDir()} | |
| 485 | + | tests := []struct { | |
| 486 | + | name string | |
| 487 | + | args map[string]any | |
| 488 | + | want string | |
| 489 | + | }{ | |
| 490 | + | {name: "missing", args: map[string]any{"command": "echo hi"}, want: "timeout is required"}, | |
| 491 | + | {name: "null", args: map[string]any{"command": "echo hi", "timeout": nil}, want: "timeout is required"}, | |
| 492 | + | {name: "zero", args: map[string]any{"command": "echo hi", "timeout": 0}, want: "timeout must be between 1 and"}, | |
| 493 | + | {name: "negative", args: map[string]any{"command": "echo hi", "timeout": -1}, want: "timeout must be between 1 and"}, | |
| 494 | + | {name: "one over maximum", args: map[string]any{"command": "echo hi", "timeout": maxBashTimeoutSeconds + 1}, want: "timeout must be between 1 and"}, | |
| 495 | + | } | |
| 496 | + | for _, tt := range tests { | |
| 497 | + | t.Run(tt.name, func(t *testing.T) { | |
| 498 | + | _, err := tool.Execute(context.Background(), mustJSON(t, tt.args), nil) | |
| 499 | + | if err == nil || !strings.Contains(err.Error(), tt.want) { | |
| 500 | + | t.Fatalf("error = %v, want substring %q", err, tt.want) | |
| 501 | + | } | |
| 502 | + | }) | |
| 503 | + | } | |
| 504 | + | ||
| 505 | + | _, err := tool.Execute(context.Background(), mustJSON(t, map[string]any{ | |
| 506 | + | "command": "echo hi", | |
| 507 | + | "timeout": "1", | |
| 508 | + | }), nil) | |
| 509 | + | if err == nil || !strings.Contains(err.Error(), "invalid args") { | |
| 510 | + | t.Fatalf("wrong-type error = %v, want invalid args", err) | |
| 511 | + | } | |
| 512 | + | } | |
| 513 | + | ||
| 514 | + | func TestBashTimeoutCancelsCommand(t *testing.T) { | |
| 515 | + | if runtime.GOOS == "windows" { | |
| 516 | + | t.Skip("posix shell only") | |
| 517 | + | } | |
| 518 | + | tool := &BashTool{CWD: t.TempDir()} | |
| 519 | + | start := time.Now() | |
| 520 | + | res, err := tool.Execute(context.Background(), mustJSON(t, map[string]any{ | |
| 521 | + | "command": "sleep 5", | |
| 522 | + | "timeout": 1, | |
| 523 | + | }), nil) | |
| 524 | + | elapsed := time.Since(start) | |
| 525 | + | if err != nil { | |
| 526 | + | t.Fatal(err) | |
| 527 | + | } | |
| 528 | + | if !res.IsError { | |
| 529 | + | t.Fatal("timed-out command should be an error") | |
| 530 | + | } | |
| 531 | + | if got := res.Content[0].(provider.TextBlock).Text; !strings.Contains(got, "[timed out after 1 second]") { | |
| 532 | + | t.Fatalf("timeout result = %q, want timeout diagnostic", got) | |
| 533 | + | } | |
| 534 | + | if elapsed >= 3*time.Second { | |
| 535 | + | t.Fatalf("timeout took %s; command was not cancelled promptly", elapsed) | |
| 536 | + | } | |
| 537 | + | } | |
| @@ -9,6 +9,7 @@ import ( | |||||
| 9 | "runtime" | 9 | "runtime" | ||
| 10 | "strings" | 10 | "strings" | ||
| 11 | "testing" | 11 | "testing" | ||
| 12 | + | "time" | |||
| 12 | 13 | ||||
| 13 | "github.com/bnema/zut/packages/provider" | 14 | "github.com/bnema/zut/packages/provider" | ||
| 14 | ) | 15 | ) | ||
| @@ -337,12 +338,90 @@ func TestShellDescription(t *testing.T) { | |||||
| 337 | } | 338 | } | ||
| 338 | } | 339 | } | ||
| 339 | 340 | ||||
| 341 | + | func TestBashSchemaRequiresTimeoutInSeconds(t *testing.T) { | |||
| 342 | + | var schema struct { | |||
| 343 | + | Properties map[string]struct { | |||
| 344 | + | Type string `json:"type"` | |||
| 345 | + | Minimum *int64 `json:"minimum"` | |||
| 346 | + | Maximum *int64 `json:"maximum"` | |||
| 347 | + | Description string `json:"description"` | |||
| 348 | + | } `json:"properties"` | |||
| 349 | + | Required []string `json:"required"` | |||
| 350 | + | } | |||
| 351 | + | if err := json.Unmarshal((&BashTool{}).Schema(), &schema); err != nil { | |||
| 352 | + | t.Fatal(err) | |||
| 353 | + | } | |||
| 354 | + | ||||
| 355 | + | timeout, ok := schema.Properties["timeout"] | |||
| 356 | + | if !ok { | |||
| 357 | + | t.Fatal("timeout property missing from schema") | |||
| 358 | + | } | |||
| 359 | + | if timeout.Type != "integer" { | |||
| 360 | + | t.Fatalf("timeout type = %q, want integer", timeout.Type) | |||
| 361 | + | } | |||
| 362 | + | if timeout.Minimum == nil || *timeout.Minimum != 1 { | |||
| 363 | + | if timeout.Minimum == nil { | |||
| 364 | + | t.Fatal("timeout minimum missing from schema") | |||
| 365 | + | } | |||
| 366 | + | t.Fatalf("timeout minimum = %d, want 1", *timeout.Minimum) | |||
| 367 | + | } | |||
| 368 | + | if timeout.Maximum == nil { | |||
| 369 | + | t.Fatal("timeout maximum missing from schema") | |||
| 370 | + | } | |||
| 371 | + | if *timeout.Maximum != maxBashTimeoutSeconds { | |||
| 372 | + | t.Fatalf("timeout maximum = %d, want %d", *timeout.Maximum, maxBashTimeoutSeconds) | |||
| 373 | + | } | |||
| 374 | + | if !strings.Contains(strings.ToLower(timeout.Description), "second") { | |||
| 375 | + | t.Fatalf("timeout description = %q, want seconds wording", timeout.Description) | |||
| 376 | + | } | |||
| 377 | + | ||||
| 378 | + | required := make(map[string]bool, len(schema.Required)) | |||
| 379 | + | for _, name := range schema.Required { | |||
| 380 | + | required[name] = true | |||
| 381 | + | } | |||
| 382 | + | for _, name := range []string{"command", "timeout"} { | |||
| 383 | + | if !required[name] { | |||
| 384 | + | t.Errorf("required fields missing %q: %v", name, schema.Required) | |||
| 385 | + | } | |||
| 386 | + | } | |||
| 387 | + | } | |||
| 388 | + | ||||
| 389 | + | func TestBashTimeoutDurationBounds(t *testing.T) { | |||
| 390 | + | tests := []struct { | |||
| 391 | + | name string | |||
| 392 | + | seconds int64 | |||
| 393 | + | want time.Duration | |||
| 394 | + | wantErr bool | |||
| 395 | + | }{ | |||
| 396 | + | {name: "minimum", seconds: 1, want: time.Second}, | |||
| 397 | + | {name: "maximum", seconds: maxBashTimeoutSeconds, want: time.Duration(maxBashTimeoutSeconds) * time.Second}, | |||
| 398 | + | {name: "one over maximum", seconds: maxBashTimeoutSeconds + 1, wantErr: true}, | |||
| 399 | + | } | |||
| 400 | + | for _, tt := range tests { | |||
| 401 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 402 | + | got, err := bashTimeoutDuration(tt.seconds) | |||
| 403 | + | if tt.wantErr { | |||
| 404 | + | if err == nil { | |||
| 405 | + | t.Fatalf("bashTimeoutDuration(%d) returned nil error", tt.seconds) | |||
| 406 | + | } | |||
| 407 | + | return | |||
| 408 | + | } | |||
| 409 | + | if err != nil { | |||
| 410 | + | t.Fatalf("bashTimeoutDuration(%d): %v", tt.seconds, err) | |||
| 411 | + | } | |||
| 412 | + | if got != tt.want { | |||
| 413 | + | t.Fatalf("bashTimeoutDuration(%d) = %s, want %s", tt.seconds, got, tt.want) | |||
| 414 | + | } | |||
| 415 | + | }) | |||
| 416 | + | } | |||
| 417 | + | } | |||
| 418 | + | ||||
| 340 | func TestBashSuccess(t *testing.T) { | 419 | func TestBashSuccess(t *testing.T) { | ||
| 341 | if runtime.GOOS == "windows" { | 420 | if runtime.GOOS == "windows" { | ||
| 342 | t.Skip("posix shell only") | 421 | t.Skip("posix shell only") | ||
| 343 | } | 422 | } | ||
| 344 | tool := &BashTool{CWD: t.TempDir()} | 423 | tool := &BashTool{CWD: t.TempDir()} | ||
| 345 | - | res, err := tool.Execute(context.Background(), mustJSON(t, map[string]any{"command": "echo hi"}), nil) | 424 | + | res, err := tool.Execute(context.Background(), mustJSON(t, map[string]any{"command": "echo hi", "timeout": 1}), nil) |
| 346 | if err != nil { | 425 | if err != nil { | ||
| 347 | t.Fatal(err) | 426 | t.Fatal(err) | ||
| 348 | } | 427 | } | ||
| @@ -375,6 +454,7 @@ func TestBashSyntax(t *testing.T) { | |||||
| 375 | tool := &BashTool{CWD: t.TempDir()} | 454 | tool := &BashTool{CWD: t.TempDir()} | ||
| 376 | res, err := tool.Execute(context.Background(), mustJSON(t, map[string]any{ | 455 | res, err := tool.Execute(context.Background(), mustJSON(t, map[string]any{ | ||
| 377 | "command": `items=(one two); [[ ${#items[@]} -eq 2 ]] && printf 'bash syntax works\n'`, | 456 | "command": `items=(one two); [[ ${#items[@]} -eq 2 ]] && printf 'bash syntax works\n'`, | ||
| 457 | + | "timeout": 1, | |||
| 378 | }), nil) | 458 | }), nil) | ||
| 379 | if err != nil { | 459 | if err != nil { | ||
| 380 | t.Fatal(err) | 460 | t.Fatal(err) | ||
| @@ -390,7 +470,7 @@ func TestBashFailure(t *testing.T) { | |||||
| 390 | t.Skip("posix shell only") | 470 | t.Skip("posix shell only") | ||
| 391 | } | 471 | } | ||
| 392 | tool := &BashTool{CWD: t.TempDir()} | 472 | tool := &BashTool{CWD: t.TempDir()} | ||
| 393 | - | res, _ := tool.Execute(context.Background(), mustJSON(t, map[string]any{"command": "false"}), nil) | 473 | + | res, _ := tool.Execute(context.Background(), mustJSON(t, map[string]any{"command": "false", "timeout": 1}), nil) |
| 394 | if !res.IsError { | 474 | if !res.IsError { | ||
| 395 | t.Fatal("want error") | 475 | t.Fatal("want error") | ||
| 396 | } | 476 | } | ||
| @@ -399,3 +479,59 @@ func TestBashFailure(t *testing.T) { | |||||
| 399 | t.Fatalf("got %q", got) | 479 | t.Fatalf("got %q", got) | ||
| 400 | } | 480 | } | ||
| 401 | } | 481 | } | ||
| 482 | + | ||||
| 483 | + | func TestBashRejectsInvalidTimeout(t *testing.T) { | |||
| 484 | + | tool := &BashTool{CWD: t.TempDir()} | |||
| 485 | + | tests := []struct { | |||
| 486 | + | name string | |||
| 487 | + | args map[string]any | |||
| 488 | + | want string | |||
| 489 | + | }{ | |||
| 490 | + | {name: "missing", args: map[string]any{"command": "echo hi"}, want: "timeout is required"}, | |||
| 491 | + | {name: "null", args: map[string]any{"command": "echo hi", "timeout": nil}, want: "timeout is required"}, | |||
| 492 | + | {name: "zero", args: map[string]any{"command": "echo hi", "timeout": 0}, want: "timeout must be between 1 and"}, | |||
| 493 | + | {name: "negative", args: map[string]any{"command": "echo hi", "timeout": -1}, want: "timeout must be between 1 and"}, | |||
| 494 | + | {name: "one over maximum", args: map[string]any{"command": "echo hi", "timeout": maxBashTimeoutSeconds + 1}, want: "timeout must be between 1 and"}, | |||
| 495 | + | } | |||
| 496 | + | for _, tt := range tests { | |||
| 497 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 498 | + | _, err := tool.Execute(context.Background(), mustJSON(t, tt.args), nil) | |||
| 499 | + | if err == nil || !strings.Contains(err.Error(), tt.want) { | |||
| 500 | + | t.Fatalf("error = %v, want substring %q", err, tt.want) | |||
| 501 | + | } | |||
| 502 | + | }) | |||
| 503 | + | } | |||
| 504 | + | ||||
| 505 | + | _, err := tool.Execute(context.Background(), mustJSON(t, map[string]any{ | |||
| 506 | + | "command": "echo hi", | |||
| 507 | + | "timeout": "1", | |||
| 508 | + | }), nil) | |||
| 509 | + | if err == nil || !strings.Contains(err.Error(), "invalid args") { | |||
| 510 | + | t.Fatalf("wrong-type error = %v, want invalid args", err) | |||
| 511 | + | } | |||
| 512 | + | } | |||
| 513 | + | ||||
| 514 | + | func TestBashTimeoutCancelsCommand(t *testing.T) { | |||
| 515 | + | if runtime.GOOS == "windows" { | |||
| 516 | + | t.Skip("posix shell only") | |||
| 517 | + | } | |||
| 518 | + | tool := &BashTool{CWD: t.TempDir()} | |||
| 519 | + | start := time.Now() | |||
| 520 | + | res, err := tool.Execute(context.Background(), mustJSON(t, map[string]any{ | |||
| 521 | + | "command": "sleep 5", | |||
| 522 | + | "timeout": 1, | |||
| 523 | + | }), nil) | |||
| 524 | + | elapsed := time.Since(start) | |||
| 525 | + | if err != nil { | |||
| 526 | + | t.Fatal(err) | |||
| 527 | + | } | |||
| 528 | + | if !res.IsError { | |||
| 529 | + | t.Fatal("timed-out command should be an error") | |||
| 530 | + | } | |||
| 531 | + | if got := res.Content[0].(provider.TextBlock).Text; !strings.Contains(got, "[timed out after 1 second]") { | |||
| 532 | + | t.Fatalf("timeout result = %q, want timeout diagnostic", got) | |||
| 533 | + | } | |||
| 534 | + | if elapsed >= 3*time.Second { | |||
| 535 | + | t.Fatalf("timeout took %s; command was not cancelled promptly", elapsed) | |||
| 536 | + | } | |||
| 537 | + | } | |||