core: add cwd parameter to bash tool for better directory control #59
Conversation
Users can now specify a working directory for bash commands using the cwd parameter, allowing commands to run in specific subdirectories without needing cd commands. This provides more precise control over where commands execute while maintaining security by restricting access to within the project directory.
…ommand-paths fix: resolve bash tool path validation relative to cwd
WalkthroughAdds an optional Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant BashTool
participant RealpathValidator as PathValidation
participant Spawn as ProcessSpawner
User->>BashTool: run(command, cwd?)
BashTool->>PathValidation: resolve & validate cwd via realpath
alt cwd valid & inside project
PathValidation-->>BashTool: resolved workingDirectory
BashTool->>PathValidation: resolve argument paths relative to workingDirectory
BashTool->>Spawn: spawn process (cwd = workingDirectory)
Spawn-->>BashTool: exit code & output
BashTool-->>User: return result
else cwd missing -> default to project root
PathValidation-->>BashTool: project root
BashTool->>Spawn: spawn process (cwd = project root)
Spawn-->>BashTool: result
BashTool-->>User: return result
else cwd invalid or outside project
PathValidation-->>BashTool: error (out-of-bounds or not found)
BashTool-->>User: throw/reject with error
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used🧬 Code graph analysis (1)packages/opencode/src/tool/bash.ts (1)
🔇 Additional comments (4)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/opencode/src/tool/bash.ts (1)
60-82: Consider using filesystem APIs instead of shell command for better security.While Bun's
$template literal provides escaping, using a shell command likerealpathwith user-supplied input (params.cwd) introduces potential security risks. Using native filesystem APIs would be safer and more direct.Consider this refactor using Node.js filesystem APIs:
- // Validate and resolve cwd parameter let workingDirectory = Instance.directory if (params.cwd) { - const resolvedCwd = await $`realpath ${params.cwd}` - .cwd(Instance.directory) - .quiet() - .nothrow() - .text() - .then((x) => x.trim()) + const { realpathSync } = await import("fs") + const { resolve, isAbsolute } = await import("path") + + let resolvedCwd: string + try { + const cwdPath = isAbsolute(params.cwd) + ? params.cwd + : resolve(Instance.directory, params.cwd) + resolvedCwd = realpathSync(cwdPath) + } catch (error) { + throw new Error( + `Working directory does not exist or is not accessible: ${params.cwd}` + ) + } - if (!resolvedCwd) { - throw new Error(`Invalid working directory: ${params.cwd}`) - } - if (!Filesystem.contains(Instance.directory, resolvedCwd)) { throw new Error( `Working directory ${resolvedCwd} is outside of project directory ${Instance.directory}`, ) } workingDirectory = resolvedCwd }This approach:
- Eliminates shell command execution with user input
- Provides clearer error messages (distinguishing between non-existent paths and permission issues)
- Is more performant (no process spawning)
- Maintains the same security guarantees
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
docs/notes/2025.10.30.13.09.02.md(1 hunks)packages/opencode/src/tool/bash.ts(4 hunks)packages/opencode/src/tool/bash.txt(1 hunks)packages/opencode/test/tool/bash.test.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
packages/opencode/src/tool/bash.ts (1)
packages/opencode/src/project/instance.ts (1)
Instance(14-61)
packages/opencode/test/tool/bash.test.ts (1)
packages/opencode/src/project/instance.ts (1)
Instance(14-61)
🔇 Additional comments (9)
packages/opencode/src/tool/bash.txt (3)
22-22: LGTM! Clear documentation of the cwd parameter.The documentation accurately describes the parameter's constraints and default behavior.
28-28: LGTM! Appropriate integration of cwd into usage guidance.The updated guidance correctly positions the
cwdparameter as an alternative tocdcommands.
32-34: LGTM! Helpful example demonstrating cwd usage.The example clearly shows how to use the
cwdparameter with a realistic command.packages/opencode/test/tool/bash.test.ts (3)
53-69: LGTM! Valid test for cwd with absolute path.The test correctly validates that commands execute in the specified working directory.
71-87: LGTM! Important security test for directory containment.The test correctly validates that the tool rejects working directories outside the project boundary.
107-122: LGTM! Good test for default behavior.The test correctly validates backward compatibility when the
cwdparameter is not provided.packages/opencode/src/tool/bash.ts (3)
46-46: LGTM! Well-documented parameter definition.The schema correctly defines the
cwdparameter as optional with a clear description of its constraints and behavior.
114-114: LGTM! Correct use of workingDirectory for path resolution.The change correctly resolves command arguments relative to the specified working directory, maintaining proper security validation.
172-172: LGTM! Core implementation of cwd feature.The spawned process now correctly executes in the validated working directory.
| [4:08 PM]AKTK: And now I've just started getting this too, did you find a cause/solution at all? | ||
| [4:10 PM]shuv: what was the prompt/session that triggered it? based on the token counts, it looks like it was trying to print the entire previous context back to you again | ||
| [4:13 PM]AKTK: Sent out via a subagent. It was running fine for a while, then just randomly goes 💩 and errors. Which then means it delegates the task back out and leaves the repo in an awful state (That /undo didn't fix the first time) | ||
| Image | ||
| Image |
There was a problem hiding this comment.
Clarify the relevance of this file to the PR.
This file contains a chat transcript that appears unrelated to the PR's stated objective of adding a cwd parameter to the bash tool. Was this file accidentally included in this PR?
| test("cwd parameter with relative path", async () => { | ||
| await Instance.provide({ | ||
| directory: projectRoot, | ||
| fn: async () => { | ||
| const result = await bash.execute( | ||
| { | ||
| command: "pwd", | ||
| cwd: path.join(projectRoot, "src"), | ||
| description: "Use absolute path for cwd", | ||
| }, | ||
| ctx, | ||
| ) | ||
| expect(result.metadata.exit).toBe(0) | ||
| expect(result.metadata.output).toContain("/src") | ||
| }, | ||
| }) | ||
| }) |
There was a problem hiding this comment.
Fix test naming and add coverage for actual relative paths.
The test is named "cwd parameter with relative path" but uses an absolute path (path.join(projectRoot, "src")). The description also contradicts the test name by saying "Use absolute path for cwd".
Additionally, there's no test coverage for actual relative paths like "src" or "./src".
Apply this diff to fix the test name:
- test("cwd parameter with relative path", async () => {
+ test("cwd parameter with subdirectory path", async () => {
await Instance.provide({
directory: projectRoot,
fn: async () => {
const result = await bash.execute(
{
command: "pwd",
cwd: path.join(projectRoot, "src"),
- description: "Use absolute path for cwd",
+ description: "Use subdirectory path for cwd",
},
ctx,
)Consider adding a test for actual relative paths:
test("cwd parameter with relative path", async () => {
await Instance.provide({
directory: projectRoot,
fn: async () => {
const result = await bash.execute(
{
command: "pwd",
cwd: "./src",
description: "Use relative path for cwd",
},
ctx,
)
expect(result.metadata.exit).toBe(0)
expect(result.metadata.output).toContain("/src")
},
})
})🤖 Prompt for AI Agents
In packages/opencode/test/tool/bash.test.ts around lines 89 to 105, the test is
named "cwd parameter with relative path" but uses an absolute path and the
description says "Use absolute path for cwd"; rename this test to indicate it's
testing an absolute path (e.g., "cwd parameter with absolute path") and update
the description accordingly while keeping cwd: path.join(projectRoot, "src").
Then add a new test that actually exercises a relative cwd (e.g., cwd: "./src"
or "src") with description "Use relative path for cwd", asserting exit is 0 and
output contains "/src" to provide coverage for relative paths.
Summary
cwdparameter to bash tool allowing users to specify working directory for commandscdcommandsWhy this change
Users previously had to use
cdcommands or absolute paths to run commands in specific directories, which was cumbersome and could lead to working directory confusion. The newcwdparameter provides precise control over command execution location while maintaining the existing security model.Changes
cwdparameter to bash tool schema with validationMirrored from sst/opencode PR anomalyco#3588
Summary by CodeRabbit
New Features
Documentation
Tests