diff --git a/CHANGELOG.md b/CHANGELOG.md index 5321a3f..70c900a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Changed + +- **Shell-first exploration.** The `list_files` and `search_files` tools are no longer offered to the model; it now searches and lists with `rg`, `find`, `ls` and `git` through `execute_command`, the way Claude Code does, and the system prompt teaches the common patterns. To keep this from becoming a prompt on every search, read-only commands (`rg`, `grep`, `find` without `-exec`/`-delete`, `ls`, `cat`, `head`, `wc`, `git status/diff/log/show/grep`, and pipes or `&&` chains of these, with no redirects or command substitution) skip the approval prompt and run in parallel. Anything unrecognised still asks. Old sessions that called the removed tools still resume. + +- **`execute_command` is now `Bash`, and it really runs bash.** The tool is renamed to match Claude Code. Commands now run in bash instead of whatever `$SHELL` is (fish and csh choke on the `rg … | head` / `&&` syntax the model writes): bash on macOS/Linux (zsh if it is your shell and bash is missing, then `/bin/sh`), and Git Bash on Windows, falling back to `cmd.exe` only when Git for Windows isn't installed. The system prompt states the shell, and warns the model when it is stuck on `cmd.exe`. Hook commands use the same shell. Hook matchers written as `execute_command` still match, and sessions saved with the old tool name still resume. + ## [6.9.1] - 2026-09-30 ### Added diff --git a/README.md b/README.md index 8d04ca0..f9998f6 100644 --- a/README.md +++ b/README.md @@ -349,7 +349,7 @@ tools prompt first: - **File edits** (`file_edit`, `multi_file_edit`, `file_write`) — prompt shows the target; `y` allow once, `n` deny, `a` allow for the rest of the session. -- **Commands** (`execute_command`) — prompt shows the exact command line. The +- **Commands** (`Bash`) — prompt shows the exact command line. The model classifies commands with an `isDangerous` flag; dangerous commands (deletes, force-pushes, system changes…) can **never** be auto-approved — no `a` option, and `--yolo`/session-approval don't apply. @@ -466,7 +466,7 @@ shell commands from a repo — see [Security](https://github.com/MatterAIOrg/Orb "hooks": { "PreToolUse": [ { - "matcher": "execute_command", + "matcher": "Bash", "hooks": [ { "type": "command", @@ -492,7 +492,7 @@ shell commands from a repo — see [Security](https://github.com/MatterAIOrg/Orb Start OrbCode normally — that's all. Each event maps to a list of matchers; a matcher has an optional `matcher` regex (omit, or use `"*"`, to match -everything; the regex is auto-anchored so `"execute_command"` matches exactly +everything; the regex is auto-anchored so `"Bash"` matches exactly that tool name) and a list of `command` hooks (`timeout` is per-command seconds, default 10). @@ -979,9 +979,7 @@ Active in the CLI (aligned with the extension's native tools, with CLI-specific | `file_edit` | single replacement; unique-match enforcement; `replace_all`; empty `old_string` = whole file | | `multi_file_edit` | batched edits grouped per file, per-edit OK/FAILED results | | `file_write` | creates parent dirs, full-content writes | -| `list_files` | optional recursive, ignores node_modules/.git/build dirs, 800-entry cap | -| `search_files` | FFF-first Rust-regex search, compact pagination, and bundled/system ripgrep fallback | -| `execute_command` | user's shell, 120s timeout, 30k output cap, optional cwd | +| `Bash` | user's shell, 120s timeout, 30k output cap; also used for search/listing (`rg`, `find`, `ls`) — read-only commands skip approval | | `web_search` / `web_fetch` | proxied through the MatterAI backend with your token | | `update_todo_list` | drives the TUI todo panel | | `use_skill` | loads standalone or namespaced plugin skill instructions | diff --git a/docs/HOOKS.md b/docs/HOOKS.md index 39e92f8..4d7a2ee 100644 --- a/docs/HOOKS.md +++ b/docs/HOOKS.md @@ -8,7 +8,7 @@ condition is met. OrbCode's hooks follow the **same contract as Claude Code's hooks**, so scripts written for Claude Code work here with two tweaks: use `$MATTERAI_PROJECT_DIR` -(not `$CLAUDE_PROJECT_DIR`) and use OrbCode's tool names (`execute_command`, +(not `$CLAUDE_PROJECT_DIR`) and use OrbCode's tool names (`Bash`, `file_edit`, …) in your matchers. See [Differences from Claude Code](#differences-from-claude-code). @@ -67,7 +67,7 @@ chmod +x ~/.orbcode/hooks/guard.sh "hooks": { "PreToolUse": [ { - "matcher": "execute_command", + "matcher": "Bash", "hooks": [{ "type": "command", "command": "~/.orbcode/hooks/guard.sh" }] } ], @@ -136,8 +136,8 @@ configuration you own. - **`matcher`** — a JavaScript regex tested against one field of the event (the tool name for `PreToolUse`/`PostToolUse`, `source` for `SessionStart`, etc.; see the per-event tables). The regex is **auto-anchored** (`^…$`), so - `"execute_command"` matches exactly that tool name, not - `"execute_command_extra"`; use `"a|b"` for alternation. Omit it, or use + `"Bash"` matches exactly that tool name, not + `"BashExtra"`; use `"a|b"` for alternation. Omit it, or use `"*"`, to match everything. An invalid regex falls back to an exact-string comparison. - **`hooks`** — the commands to run when the matcher matches. You can list @@ -468,7 +468,7 @@ exit 0 "hooks": { "PreToolUse": [ { - "matcher": "execute_command", + "matcher": "Bash", "hooks": [{ "type": "command", "command": "~/.orbcode/hooks/guard.sh" }] } ] @@ -491,14 +491,14 @@ exit 0 ### Auto-approve a safe, read-only tool -Skip the approval prompt for `read_file` and `list_files`: +Skip the approval prompt for `read_file` (searching and listing go through `Bash`, and read-only commands such as `rg`, `find` and `ls` already skip approval): ```json { "hooks": { "PreToolUse": [ { - "matcher": "read_file|list_files|search_files", + "matcher": "read_file", "hooks": [ { "type": "command", "command": "echo '{\"hookSpecificOutput\":{\"hookEventName\":\"PreToolUse\",\"permissionDecision\":\"allow\"}}'" } @@ -538,7 +538,7 @@ Force `ls` to always be `ls -la`: "hooks": { "PreToolUse": [ { - "matcher": "execute_command", + "matcher": "Bash", "hooks": [{ "type": "command", "command": "~/.orbcode/hooks/rewrite.sh" }] } ] @@ -698,7 +698,7 @@ exit 0 **Run a hook by hand** — pipe it a fake payload and inspect the exit code: ```bash -echo '{"hook_event_name":"PreToolUse","tool_name":"execute_command","tool_input":{"command":"rm -rf /"}}' \ +echo '{"hook_event_name":"PreToolUse","tool_name":"Bash","tool_input":{"command":"rm -rf /"}}' \ | ~/.orbcode/hooks/guard.sh echo "exit=$?" ``` @@ -723,8 +723,8 @@ file=$(node -e 'let s="";process.stdin.on("data",d=>s+=d).on("end",()=>console.l with the workspace as the working directory. - **`matcher` on a no-match-field event** (e.g. a `matcher` on `Stop`) — it will never match. Omit the matcher for those events. -- **Wrong tool names** — OrbCode uses `execute_command`, `file_edit`, - `file_write`, `multi_file_edit`, `read_file`, `list_files`, `search_files`, +- **Wrong tool names** — OrbCode uses `Bash`, `file_edit`, + `file_write`, `multi_file_edit`, `read_file`, `web_fetch`, `web_search`, `update_todo_list` (not Claude Code's `Bash`, `Edit`, `Write`, …). @@ -743,7 +743,7 @@ schema, matcher regexes, parallel execution, per-command timeout). Differences: | Settings file | `~/.claude/settings.json` | `~/.orbcode/settings.json` | | Project file | `.claude/settings.json` | `.orbcode/settings.json` | | Project dir env var | `$CLAUDE_PROJECT_DIR` | `$MATTERAI_PROJECT_DIR` | -| Tool names in matchers | `Bash`, `Edit`, `Write`, `Read`, … | `execute_command`, `file_edit`, `file_write`, `read_file`, … | +| Tool names in matchers | `Bash`, `Edit`, `Write`, `Read`, … | `Bash` (same; the old name `execute_command` still matches), `file_edit`, `file_write`, `read_file`, … | | Hook types | `command`, plus newer MCP/HTTP/prompt hooks | `command` only | | Events | full internal SDK set | the documented set: `SessionStart`, `UserPromptSubmit`, `PreToolUse`, `PostToolUse`, `Notification`, `Stop`, `PreCompact`, `SessionEnd` (+ `SubagentStop`, reserved) | diff --git a/package.json b/package.json index e1f5ac4..3b9652c 100644 --- a/package.json +++ b/package.json @@ -38,6 +38,7 @@ "test:plugins": "node --import tsx --test test/plugins.test.ts", "test:metrics": "node --import tsx --test test/metrics.test.ts", "test:json-repair": "node --import tsx --test test/json-repair.test.ts", + "test:readonly": "node --import tsx --test test/read-only-command.test.ts", "test:search": "node --import tsx --test test/search-files.test.ts", "test:ui": "bun test test/ui-viewport.test.tsx test/session-picker.test.tsx", "prepublishOnly": "npm run typecheck && npm run build" diff --git a/src/core/agent.ts b/src/core/agent.ts index bad8389..e2ea10c 100644 --- a/src/core/agent.ts +++ b/src/core/agent.ts @@ -24,6 +24,7 @@ import { walkFiles } from "../tools/executors/listFiles.js" import { previewFileChange } from "../tools/executors/files.js" import { extractFigmaUrls, figmaFetch } from "../tools/executors/figma.js" import { stripSearchPageMetadataForDisplay } from "../tools/executors/searchFiles/format.js" +import { isReadOnlyCommand } from "../tools/readOnlyCommand.js" import type { AgentCallbacks, AgentEvent, ApprovalDecision } from "./events.js" import { getSessionFilePath, @@ -85,7 +86,7 @@ const PRUNE_BATCH = 6 /** Results shorter than this are not worth stubbing. */ const PRUNE_MIN_CHARS = 1500 /** Tools whose output is bulky and can simply be re-fetched. */ -const PRUNABLE_TOOLS = new Set(["read_file", "search_files", "list_files", "execute_command", "web_fetch", "web_search"]) +const PRUNABLE_TOOLS = new Set(["read_file", "search_files", "list_files", "Bash", "execute_command", "web_fetch", "web_search"]) /** Summarize the history before a step once context passes this fraction of the window. */ const AUTO_COMPACT_FRACTION = 0.8 /** Warn the model when the same call returns the same output this many times in a row. */ @@ -183,6 +184,18 @@ interface PendingToolCall { arguments: string } +/** Read-only tools, plus shell commands that only observe (rg, find, ls, git diff, ...). */ +function isParallelReadOnlyCall(toolCall: PendingToolCall): boolean { + if (PARALLEL_READ_ONLY_TOOLS.has(toolCall.name)) return true + if (toolCall.name !== "Bash" && toolCall.name !== "execute_command") return false + try { + const args = JSON.parse(toolCall.arguments) as { command?: unknown; isDangerous?: unknown } + return typeof args.command === "string" && !args.isDangerous && isReadOnlyCommand(args.command) + } catch { + return false + } +} + function getGitSummary(cwd: string): string { try { const branch = execSync("git rev-parse --abbrev-ref HEAD", { cwd, stdio: ["ignore", "pipe", "ignore"] }) @@ -1372,7 +1385,7 @@ User time zone: ${timeZone}, UTC${timeZoneOffsetStr}` // committed in model order so tool_call/tool_result pairing stays intact; // mutating and interactive calls remain on the serialized path. let batchEnd = 0 - while (batchEnd < toolCalls.length && PARALLEL_READ_ONLY_TOOLS.has(toolCalls[batchEnd].name)) { + while (batchEnd < toolCalls.length && isParallelReadOnlyCall(toolCalls[batchEnd])) { batchEnd++ } @@ -1502,11 +1515,12 @@ User time zone: ${timeZone}, UTC${timeZoneOffsetStr}` const approvalKind = getApprovalKind(toolCall.name, args) const diff = approvalKind === "edit" ? previewFileChange(toolCall.name, args, this.options.cwd) : undefined - const isDangerous = toolCall.name === "execute_command" && Boolean(args.isDangerous) + const isDangerous = (toolCall.name === "Bash" || toolCall.name === "execute_command") && Boolean(args.isDangerous) let needsApproval = false if (approvalKind === "edit" && !this.sessionApproveEdits) needsApproval = true if (approvalKind === "command") { - needsApproval = isDangerous || !(this.sessionApproveCommands || this.options.autoApproveSafeCommands) + const readOnly = !isDangerous && isReadOnlyCommand(String(args.command ?? "")) + needsApproval = isDangerous || !(readOnly || this.sessionApproveCommands || this.options.autoApproveSafeCommands) } // A PreToolUse hook can force the approval prompt ("ask") or skip it ("allow"). if (forceApproval) needsApproval = true diff --git a/src/core/hooks.ts b/src/core/hooks.ts index c993997..5b12206 100644 --- a/src/core/hooks.ts +++ b/src/core/hooks.ts @@ -283,9 +283,11 @@ function getMatchQuery(event: HookEvent, fields: Record): strin function matcherMatches(matcher: string | undefined, query: string | undefined): boolean { if (!matcher || matcher === "*") return true if (query === undefined) return false + // Hooks written before the tool was renamed still match "execute_command". + if (query === "Bash" && matcherMatches(matcher, "execute_command")) return true try { - // Auto-anchor so "execute_command" matches exactly that tool name, not - // "execute_command_extra". Alternation ("a|b") still works because the + // Auto-anchor so "Bash" matches exactly that tool name, not + // "BashExtra". Alternation ("a|b") still works because the // anchors wrap a non-capturing group: ^(?:a|b)$. return new RegExp(`^(?:${matcher})$`).test(query) } catch { diff --git a/src/prompts/system.ts b/src/prompts/system.ts index c64fbe6..3ae904a 100644 --- a/src/prompts/system.ts +++ b/src/prompts/system.ts @@ -1,6 +1,6 @@ import * as os from "node:os"; -import { getShell } from "../utils/shell.js"; +import { getShell, isCmdShell } from "../utils/shell.js"; import type { MemoryFile } from "../memory/types.js"; import { renderMemorySection } from "../memory/loader.js"; import type { Skill } from "../skills/types.js"; @@ -10,6 +10,13 @@ import { renderSkillCatalog } from "../skills/loader.js"; // (agent mode roleDefinition + applyDiffToolDescription). Only the system // information section is adapted from the IDE to the CLI environment. +function shellDescription(): string { + const shell = getShell(); + return isCmdShell(shell) + ? `${shell} (bash is not installed, so bash syntax, rg/find/ls/grep pipelines and POSIX paths may not work; use cmd.exe-compatible commands and check a tool exists before relying on it)` + : `${shell} (write commands in bash syntax)`; +} + const roleDefinition = `You are OrbCode, AI coding assistant, by MatterAI. You operate in OrbCode CLI. You are pair programming with a USER to solve their coding task. Each time the USER sends a message, we may automatically attach some information about their current state, such as their working directory, project file structure, git status, and more. This information may or may not be relevant to the coding task, it is up for you to decide. @@ -31,7 +38,7 @@ Some tool results and user messages may contain blocks wrapped in = 1 and \`limit\` must be between 200 and 1000 when specified. Omitting both reads from the top up to the 1000-line cap. To inspect line N in a large file, use an offset that includes enough context for the complete surrounding function or logical region. -When you don't know line numbers: use \`search_files\` to locate the code, note the line number from the results, then \`read_file\` that region with surrounding context. +When you don't know line numbers: use \`rg -n\` via \`Bash\` to locate the code, note the line number from the results, then \`read_file\` that region with surrounding context. ### Reading Strategy @@ -168,55 +175,40 @@ When you don't know line numbers: use \`search_files\` to locate the code, note - For code reviews, first use a compact change inventory such as \`git status --short\`, \`git diff --stat\`, and \`git diff --unified=20\`. Do not dump an unbounded repository diff and then request the same per-file diffs again. -# list_files +# Bash -The \`list_files\` tool lists files and directories within a given directory. Use it to explore directory structure when you need to understand the project layout or find files by location rather than content. +The \`Bash\` tool runs bash commands on the user's system. It is your primary tool for exploring the codebase and for system operations: searching, listing, inspecting git state, installing dependencies, building, testing, and running scripts. ## Parameters -- \`path\` (required): Directory path to inspect, relative to the workspace. -- \`recursive\` (required, default false): Set true to list contents recursively; false or null for top-level only. Must always be provided (boolean or null) per strict mode. - -## Guidance - -- Use \`list_files\` for directory exploration and file discovery by location. Use \`search_files\` for finding content by regex. -- For generic directories where you don't need the nested structure (like the Desktop), use non-recursive mode. -- Do not use this tool to confirm file creation; rely on user confirmation instead. - -# execute_command - -The \`execute_command\` tool runs CLI commands on the user's system. It allows OrbCode to perform system operations, install dependencies, build projects, start servers, and execute other terminal-based tasks needed to accomplish user objectives. - -## Parameters - -The tool accepts these parameters: - -- \`command\` (required): The CLI command to execute. Must be valid for the user's operating system. -- \`cwd\` (optional): The working directory to execute the command in. If not provided, the current working directory is used. Ensure this is always an absolute path (starting with \`/\`, or a drive letter like \`C:\\\` on Windows). If you are running the command in the root directly, skip this parameter. The command executor is defaulted to run in the root directory. You already have the Current Workspace Directory in the Environment Details section. +- \`command\` (required): The shell command to execute. Must be valid for the user's operating system and shell. +- \`cwd\` (required, string or null): Absolute working directory, or null for the workspace directory. You already have the Current Workspace Directory in the Environment Details section. +- \`message\` (required): One-line description shown to the user. +- \`isDangerous\` (required): true only for destructive or irreversible commands. CRITICAL: If the command is a very long running process, prefer to let the user know so they can run it manually in their terminal. If the user specifically requests to run a long running command, you may proceed. -Command validity rules: a command is never empty, never just \`:\`, never a bare single word with no arguments, and never contains tool-call markup tokens or angle-bracket tags of any kind. Commands must be valid for the user's operating system, shell, and current working directory. +Command validity rules: a command is never empty, never just \`:\`, never a bare single word with no arguments (except \`ls\` or \`pwd\`), and never contains tool-call markup tokens or angle-bracket tags of any kind. -## search_files - -Search file contents using a Rust-compatible regex. Results are compact and bounded to the first 100 matches; refine the query instead of paginating. - -### Parameters +## Exploring with the shell -1. **path** (string, required): Directory to search recursively, relative to workspace -2. **regex** (string, required): Rust-compatible regular expression pattern -3. **file_pattern** (string or null, required): Glob pattern to filter files OR null -4. **max_results** (integer or null, required): Target 1-100 results; null defaults to 100 -5. **context_lines** (integer or null, required): 0-2 surrounding lines; null defaults to 0 +There are no dedicated search or list tools. Use the shell, the way an engineer at a terminal would. Read-only commands (\`rg\`, \`grep\`, \`find\`, \`ls\`, \`cat\`, \`head\`, \`wc\`, \`git status/diff/log/show/grep\`, and pipes of these) run without an approval prompt, so use them freely and in parallel. -Use zero context for discovery, then read the relevant file region. If results are capped, refine the path, regex, or file pattern. +- **Search contents:** \`rg -n "pattern" src/\`. Prefer \`rg\` (respects .gitignore, fast); fall back to \`grep -rn\` if it is missing. Useful flags: \`-g '*.ts'\` to filter files, \`-i\` case-insensitive, \`-w\` whole word, \`-F\` literal string, \`-l\` file names only, \`-c\` counts, \`-C 2\` context, \`-t py\` by language. +- **Find files by name:** \`rg --files -g '*auth*'\`, \`fd auth\`, or \`find . -name '*auth*' -not -path '*/node_modules/*'\`. +- **List a directory:** \`ls -la src/\`, or \`rg --files src | head -100\` for a recursive, gitignore-aware listing. \`tree -L 2 -I node_modules\` if available. +- **Structure of a file:** \`rg -n "^(export |class |function |def )" path/to/file\`. +- **Git state:** \`git status --short\`, \`git diff --stat\`, \`git log --oneline -20\`, \`git grep -n "pattern"\`. +- **Peek at a file:** \`head -50 file\`, \`wc -l file\`. Use \`read_file\` when you need real content for editing. -### Search Hygiene +### Shell hygiene -- Exclude test, spec, and mock paths from discovery searches by default (\`__tests__\`, \`*.spec.*\`, \`*.test.*\`, \`__mocks__\`) unless the task itself is about tests. They pollute results and bury the implementation you are looking for. -- Scope \`path\` to the narrowest plausible directory instead of searching from the repository root. -- If a search returns hundreds of hits, tighten the regex or \`file_pattern\` and search again. Do not scan through the dump. +- Bound the output: pipe through \`| head -50\` or use \`-l\`/\`-c\` first when a search may match widely. Output beyond 30k characters is truncated. +- Scope searches to the narrowest plausible directory, never \`/\` or the home directory. Commands are killed after 120 seconds. +- Exclude test, spec, and mock paths from discovery searches by default (\`-g '!**/*.test.*' -g '!**/__tests__/**'\`) unless the task is about tests. +- Combine independent lookups into one call (\`rg -n foo src/ ; rg -n bar src/\`) or issue several calls in parallel. +- If a search returns hundreds of hits, tighten the pattern or path and search again. Do not scan through the dump. +- Never use \`cat\`, \`sed -n\`, or \`head\`/\`tail\` to read code you are about to edit; use \`read_file\`. Never use \`echo\`, heredocs, or \`sed -i\` to write files; use the edit tools. ## Working style @@ -247,11 +239,11 @@ function getSystemInfoSection(cwd: string): string { return `# System Information - Operating System: ${process.platform === "darwin" ? `macOS ${os.release()}` : `${process.platform} ${os.release()}`} -- Default Shell: ${getShell()} +- Default Shell: ${shellDescription()} - Home Directory: ${os.homedir()} - Current Workspace Directory: ${cwd} -The Current Workspace Directory is the directory the user launched OrbCode CLI from, and is therefore the default directory for all tool operations. Commands run in the current workspace directory unless a different cwd is passed; changing directories inside a command does not modify the workspace directory. When the user initially gives you a task, a listing of filepaths in the current workspace directory will be included in the Environment Details section. This provides an overview of the project's file structure, offering key insights into the project from directory/file names (how developers conceptualize and organize their code) and file extensions (the language used). This can also guide decision-making on which files to explore further. If you need to further explore directories such as outside the current workspace directory, you can use the list_files tool. If you pass 'true' for the recursive parameter, it will list files recursively. Otherwise, it will list files at the top level, which is better suited for generic directories where you don't necessarily need the nested structure, like the Desktop.`; +The Current Workspace Directory is the directory the user launched OrbCode CLI from, and is therefore the default directory for all tool operations. Commands run in the current workspace directory unless a different cwd is passed; changing directories inside a command does not modify the workspace directory. When the user initially gives you a task, a listing of filepaths in the current workspace directory will be included in the Environment Details section. This provides an overview of the project's file structure, offering key insights into the project from directory/file names (how developers conceptualize and organize their code) and file extensions (the language used). This can also guide decision-making on which files to explore further. If you need to further explore directories such as outside the current workspace directory, you can use \`ls\` or \`find\` through Bash. Prefer a non-recursive \`ls\` for generic directories where you don't need the nested structure, like the Desktop.`; } export interface SystemPromptOptions { diff --git a/src/tools/executors/executeCommand.ts b/src/tools/executors/executeCommand.ts index a42daf6..7885b5e 100644 --- a/src/tools/executors/executeCommand.ts +++ b/src/tools/executors/executeCommand.ts @@ -1,6 +1,6 @@ import { spawn } from "node:child_process" -import { getShell, getShellRunArgs } from "../../utils/shell.js" +import { getShell, getShellRunArgs, isCmdShell } from "../../utils/shell.js" import { type ToolContext, type ToolResult, resolveWorkspacePath } from "../types.js" const COMMAND_TIMEOUT_MS = 120_000 @@ -26,7 +26,7 @@ export async function executeCommand(args: Record, context: Too // keep the output pipes open). detached: !isWindows, // cmd.exe parses the command string itself; pre-quoting would corrupt it. - windowsVerbatimArguments: isWindows, + windowsVerbatimArguments: isWindows && isCmdShell(), }) let output = "" diff --git a/src/tools/index.ts b/src/tools/index.ts index 2224e83..079392c 100644 --- a/src/tools/index.ts +++ b/src/tools/index.ts @@ -24,7 +24,8 @@ export function getApprovalKind(toolName: string, args: Record) case "multi_file_edit": case "file_write": return "edit" - case "execute_command": + case "Bash": + case "execute_command": // name used by sessions saved before the rename return "command" default: return "none" @@ -54,6 +55,7 @@ export function describeToolCall(toolName: string, args: Record const files = [...new Set(edits.map((e: { file_path?: string }) => e.file_path ?? ""))] return `${edits.length} edits in ${files.length} file${files.length === 1 ? "" : "s"}` } + case "Bash": case "execute_command": return String(args.command ?? "") case "list_files": @@ -93,6 +95,7 @@ const executors: Record, context: ToolCon multi_file_edit: multiFileEdit, list_files: listFiles, search_files: searchFiles, + Bash: executeCommand, execute_command: executeCommand, web_search: webSearch, web_fetch: webFetch, diff --git a/src/tools/readOnlyCommand.ts b/src/tools/readOnlyCommand.ts new file mode 100644 index 0000000..082035c --- /dev/null +++ b/src/tools/readOnlyCommand.ts @@ -0,0 +1,109 @@ +/** + * Conservative detector for shell commands that only observe the workspace + * (rg, grep, find, ls, cat, git status, ...). Such commands skip the approval + * prompt and can run in parallel, which is what lets the Bash tool stand in + * for dedicated search/list tools. A false negative just means a prompt; a false + * positive would run something unreviewed, so anything unrecognised is rejected. + */ + +const READ_ONLY_COMMANDS = new Set([ + "rg", "grep", "egrep", "fgrep", "find", "fd", "ls", "tree", "cat", "head", "tail", "wc", "file", "stat", + "pwd", "echo", "sort", "uniq", "cut", "tr", "nl", "du", "df", "which", "basename", "dirname", "realpath", + "diff", "jq", "column", "cd", "true", "git", +]) + +const READ_ONLY_GIT = new Set([ + "status", "diff", "log", "show", "blame", "ls-files", "ls-tree", "grep", "rev-parse", "rev-list", "describe", + "shortlog", "cat-file", "diff-tree", "merge-base", "name-rev", "check-ignore", +]) + +/** Flags that make an otherwise read-only command run code or write files. */ +const UNSAFE_FLAGS: Record = { + find: /^-(exec|execdir|ok|okdir|delete|fprint|fprint0|fprintf|fls)$/, + fd: /^(-x|-X|--exec|--exec-batch)$/, + rg: /^(--pre|--pre-glob|--hostname-bin)(=|$)/, + sort: /^(-o|--output)(=|$)/, + tree: /^-o$/, + git: /^(--output|--ext-diff|--textconv|-O|--open-files-in-pager)(=|$)/, +} + +/** Split on unquoted shell operators; null if the command uses anything that could hide a write or a nested command. */ +function splitSegments(command: string): string[] | null { + const segments: string[] = [] + let current = "" + let quote: "'" | '"' | null = null + for (let i = 0; i < command.length; i++) { + const ch = command[i] + if (quote === "'") { + if (ch === "'") quote = null + current += ch + continue + } + if (ch === "\\") { + current += ch + (command[i + 1] ?? "") + i++ + continue + } + if (ch === "`" || (ch === "$" && command[i + 1] === "(")) return null + if (quote === '"') { + if (ch === '"') quote = null + current += ch + continue + } + if (ch === "'" || ch === '"') { + quote = ch + current += ch + continue + } + if (ch === "\n" || ch === "<" || ch === "(" || ch === ")" || ch === "{" || ch === "}") return null + if (ch === ">") return null + if (ch === "|" || ch === ";" || ch === "&") { + // `||`, `&&`, `|&`: swallow the doubled operator. + if (command[i + 1] === ch || (ch === "|" && command[i + 1] === "&")) i++ + segments.push(current) + current = "" + continue + } + current += ch + } + if (quote) return null + segments.push(current) + return segments +} + +/** Words of a single segment with quotes removed (enough to inspect the command name and flags). */ +function words(segment: string): string[] { + const out: string[] = [] + for (const match of segment.matchAll(/"((?:[^"\\]|\\.)*)"|'([^']*)'|((?:\\.|[^\s"'\\])+)/g)) { + out.push(match[1] ?? match[2] ?? match[3] ?? "") + } + return out +} + +export function isReadOnlyCommand(command: string): boolean { + // Harmless redirections the model adds constantly; removed before looking for real ones. + const cleaned = command + .replace(/\s\d?>\s*\/dev\/null/g, " ") + .replace(/\s2>&1/g, " ") + .trim() + if (!cleaned) return false + const segments = splitSegments(cleaned) + if (!segments) return false + let sawCommand = false + for (const segment of segments) { + const w = words(segment) + if (w.length === 0) continue + const [name, ...rest] = w + if (!READ_ONLY_COMMANDS.has(name)) return false + sawCommand = true + if (name === "git") { + const sub = rest.find((arg) => !arg.startsWith("-")) + if (!sub || !READ_ONLY_GIT.has(sub)) return false + // `git -c core.pager=...` / `--exec-path` global options can run code. + if (rest.some((arg) => arg === "-c" || arg.startsWith("--exec-path"))) return false + } + const unsafe = UNSAFE_FLAGS[name] + if (unsafe && rest.some((arg) => unsafe.test(arg))) return false + } + return sawCommand +} diff --git a/src/tools/schemas/execute_command.ts b/src/tools/schemas/bash.ts similarity index 85% rename from src/tools/schemas/execute_command.ts rename to src/tools/schemas/bash.ts index a6cd0b5..218e33e 100644 --- a/src/tools/schemas/execute_command.ts +++ b/src/tools/schemas/bash.ts @@ -3,9 +3,9 @@ import type OpenAI from "openai" export default { type: "function", function: { - name: "execute_command", + name: "Bash", description: - "Run one CLI command. Provide a short user-facing message and explicitly classify whether it may modify or delete data. Prefer commands scoped to the workspace.", + "Run one bash command. Provide a short user-facing message and explicitly classify whether it may modify or delete data. Prefer commands scoped to the workspace.", strict: true, parameters: { type: "object", diff --git a/src/tools/schemas/index.ts b/src/tools/schemas/index.ts index 28b937f..454dd3e 100644 --- a/src/tools/schemas/index.ts +++ b/src/tools/schemas/index.ts @@ -5,17 +5,18 @@ import multiFileEdit from "./multi_file_edit.js" import fileWrite from "./file_write.js" import askFollowupQuestion from "./ask_followup_question.js" import attemptCompletion from "./attempt_completion.js" -import executeCommand from "./execute_command.js" -import listFiles from "./list_files.js" +import bash from "./bash.js" import read_file from "./read_file.js" -import searchFiles from "./search_files.js" import updateTodoList from "./update_todo_list.js" import useSkill from "./use_skill.js" import figmaFetch from "./figma_fetch.js" import webFetch from "./web_fetch.js" import webSearch from "./web_search.js" -// Native tool schemas ported from the Orbital extension. IDE-only tools +// Native tool schemas ported from the Orbital extension. File discovery and +// content search (list_files, search_files) are deliberately not exposed: the +// model uses rg/find/ls through the Bash tool, and read-only commands skip +// the approval prompt (see tools/readOnlyCommand.ts). IDE-only tools // (codebase_search, lsp, check_past_chat_memories, browser_action, …) are not // active in the CLI. use_skill is now active: standalone and installed-plugin // skills are loaded by src/skills/loader.ts. @@ -25,10 +26,8 @@ export const nativeTools = [ fileWrite, askFollowupQuestion, attemptCompletion, - executeCommand, - listFiles, + bash, read_file, - searchFiles, updateTodoList, useSkill, figmaFetch, diff --git a/src/utils/shell.ts b/src/utils/shell.ts index 397b1ed..7f28938 100644 --- a/src/utils/shell.ts +++ b/src/utils/shell.ts @@ -1,18 +1,61 @@ +import * as fs from "node:fs" +import * as path from "node:path" + /** - * Cross-platform shell selection. On Windows, $SHELL is normally unset and - * unix paths like /bin/sh do not exist, so we use ComSpec (cmd.exe); on - * POSIX systems we honor the user's shell with a /bin/sh fallback. + * Cross-platform shell selection. Commands are written in bash syntax (the + * model is told so, and the read-only classifier assumes it), so we run bash + * whenever it exists instead of trusting $SHELL, which may be fish or csh: + * - POSIX: bash, else zsh when that is the user's shell, else /bin/sh. + * - Windows: Git Bash when installed, else ComSpec (cmd.exe). */ -export function getShell(): string { +let cached: string | undefined + +function firstExisting(candidates: string[]): string | undefined { + return candidates.find((candidate) => { + try { + return fs.statSync(candidate).isFile() + } catch { + return false + } + }) +} + +function findOnPath(binary: string, skip: (dir: string) => boolean = () => false): string | undefined { + const dirs = (process.env.PATH ?? "").split(path.delimiter).filter((dir) => dir && !skip(dir)) + return firstExisting(dirs.map((dir) => path.join(dir, binary))) +} + +function findWindowsBash(): string | undefined { + const roots = [process.env.ProgramFiles, process.env["ProgramFiles(x86)"], process.env.LOCALAPPDATA && path.join(process.env.LOCALAPPDATA, "Programs")] + const known = firstExisting(roots.filter(Boolean).flatMap((root) => [path.join(root!, "Git", "bin", "bash.exe"), path.join(root!, "Git", "usr", "bin", "bash.exe")])) + // System32\bash.exe is the WSL launcher, which runs a different filesystem. + return known ?? findOnPath("bash.exe", (dir) => /[\\/]system32$/i.test(dir) || /[\\/]windowsapps$/i.test(dir)) +} + +function resolveShell(): string { if (process.platform === "win32") { - return process.env.ComSpec || "cmd.exe" + return findWindowsBash() ?? (process.env.ComSpec || "cmd.exe") } - return process.env.SHELL || "/bin/sh" + const userShell = process.env.SHELL + if (userShell && path.basename(userShell) === "bash") return userShell + const bash = firstExisting(["/bin/bash", "/usr/bin/bash", "/usr/local/bin/bash", "/opt/homebrew/bin/bash"]) ?? findOnPath("bash") + if (bash) return bash + if (userShell && path.basename(userShell) === "zsh") return userShell + return "/bin/sh" +} + +export function getShell(): string { + return (cached ??= resolveShell()) +} + +/** True when the selected shell is cmd.exe, i.e. bash syntax will not work. */ +export function isCmdShell(shell: string = getShell()): boolean { + return /(^|[\\/])cmd(\.exe)?$/i.test(shell) } /** Arguments that make the shell run a single command string. */ export function getShellRunArgs(command: string): string[] { - if (process.platform === "win32") { + if (isCmdShell()) { // /d skips AutoRun, /s preserves quotes in the command string. return ["/d", "/s", "/c", command] } diff --git a/test/read-only-command.test.ts b/test/read-only-command.test.ts new file mode 100644 index 0000000..a64e372 --- /dev/null +++ b/test/read-only-command.test.ts @@ -0,0 +1,55 @@ +import assert from "node:assert/strict" +import test from "node:test" + +import { isReadOnlyCommand } from "../src/tools/readOnlyCommand.js" + +test("search, list and inspect commands are read-only", () => { + for (const command of [ + `rg -n "foo" src/`, + `rg -n "=>" src -g '*.ts'`, + `rg --files src | head -50`, + `grep -rn "a|b" . --include='*.ts' 2>/dev/null`, + `find . -name '*.ts' -not -path '*/node_modules/*'`, + `ls -la src/ && pwd`, + `cd src && rg -l TODO | wc -l`, + `git status --short`, + `git diff --stat; git log --oneline -20`, + `cat package.json | jq .version`, + ]) { + assert.equal(isReadOnlyCommand(command), true, command) + } +}) + +test("anything that writes, runs code or is unrecognised is not read-only", () => { + for (const command of [ + ``, + `rm -rf node_modules`, + `echo hi > file.txt`, + `cat a >> b`, + `rg foo | xargs rm`, + `find . -name '*.log' -delete`, + `find . -exec rm {} \;`, + `rg --pre ./evil.sh foo`, + `sort -o out.txt in.txt`, + `ls $(rm -rf /)`, + "ls `whoami`", + `git commit -m x`, + `git -c core.pager=evil log`, + `git diff --output=out.patch`, + `ls; rm file`, + `ls && npm install`, + `sed -i s/a/b/ file`, + `FOO=1 rg x`, + `cat < { + const { getShell, isCmdShell } = await import("../src/utils/shell.js") + if (process.platform !== "win32") assert.match(getShell(), /(bash|zsh|sh)$/) + assert.equal(isCmdShell("C:\\Windows\\System32\\cmd.exe"), true) + assert.equal(isCmdShell("/bin/bash"), false) +})