diff --git a/docs/skills.md b/docs/skills.md index 2172ccf..7fa3f03 100644 --- a/docs/skills.md +++ b/docs/skills.md @@ -403,7 +403,7 @@ This registers eight tools: | `glob_search` | Find files by name pattern | | `directory_tree` | Show project directory tree | -The skill uses **denied tools** (`bash_execute`, `file_write`, `file_edit`, `file_patch`, `file_read`, `schedule_*`) to ensure the LLM uses the skill's own tool wrappers instead of raw builtins. All file operations are confined to the agent's working directory via `PathValidator`. +The skill uses **denied tools** (`file_write`, `file_edit`, `file_patch`, `file_read`, `schedule_*`) to ensure the LLM uses the skill's own tool wrappers instead of raw builtins. All file operations are confined to the agent's working directory via `PathValidator`. Requires: `bash`, `jq`. Egress: `registry.npmjs.org`, `cdn.tailwindcss.com`, `pypi.org`, `files.pythonhosted.org`, `proxy.golang.org`, `sum.golang.org`, `storage.googleapis.com`, `repo.maven.apache.org`, `repo1.maven.org`. diff --git a/docs/tools.md b/docs/tools.md index d35cdbe..ea0b997 100644 --- a/docs/tools.md +++ b/docs/tools.md @@ -44,7 +44,6 @@ All code-agent tools use a `PathValidator` that confines resolved paths within t | Tool | Description | |------|-------------| -| `bash_execute` | Execute bash commands with pipes, redirection, and shell features | | `file_read` | Read file contents with optional line offset/limit, or list directory entries | | `file_write` | Create or overwrite files in the project directory | | `file_edit` | Edit files by exact string matching with unified diff output | @@ -61,18 +60,9 @@ Code-agent tools are registered in layered groups, allowing skills to request on |-------|-------|---------| | `CodeAgentSearchTools` | `grep_search`, `glob_search`, `directory_tree` | Read-only exploration | | `CodeAgentReadTools` | `file_read` + search tools | Safe reading | -| `CodeAgentWriteTools` | `file_write`, `file_edit`, `file_patch`, `bash_execute` | Modification + execution | +| `CodeAgentWriteTools` | `file_write`, `file_edit`, `file_patch` | Modification | | `CodeAgentTools` | All read + write tools | Full code-agent capability | -### bash_execute Security - -| Layer | Detail | -|-------|--------| -| **Dangerous command denylist** | Blocks `rm -rf /`, `mkfs`, `dd`, fork bombs, and similar destructive patterns | -| **sudo/su blocked** | Privilege escalation prefixes are rejected | -| **Timeout** | Default 120s, configurable per invocation | -| **Output cap** | Maximum 1MB output to prevent memory exhaustion | - ### Path Validation All file tools use `PathValidator` (from `pathutil.go`): diff --git a/forge-core/runtime/loop.go b/forge-core/runtime/loop.go index b976cdd..8ee0787 100644 --- a/forge-core/runtime/loop.go +++ b/forge-core/runtime/loop.go @@ -593,7 +593,7 @@ func isWriteActionTool(name string) bool { case "code_agent_edit", "code_agent_write", "code_agent_patch", "github_commit", "github_push", "github_create_pr", "github_checkout", "github_create_issue", - "file_create", "bash_execute", "code_agent_run": + "file_create", "code_agent_run": return true } // Catch any tool with "edit", "write", "commit", "push" in the name. @@ -674,7 +674,7 @@ func toolPhase(name string) workflowPhase { return phaseSetup case "code_agent_read", "grep_search", "glob_search", "directory_tree", "read_skill", "github_status": return phaseExplore - case "code_agent_edit", "code_agent_write", "code_agent_patch", "bash_execute", "file_create", "code_agent_run": + case "code_agent_edit", "code_agent_write", "code_agent_patch", "file_create", "code_agent_run": return phaseEdit case "github_commit", "github_push", "github_create_pr": return phaseGitOps diff --git a/forge-core/runtime/loop_test.go b/forge-core/runtime/loop_test.go index 9765eaf..a641ec9 100644 --- a/forge-core/runtime/loop_test.go +++ b/forge-core/runtime/loop_test.go @@ -371,7 +371,6 @@ func TestToolPhaseClassification(t *testing.T) { {"code_agent_edit", phaseEdit}, {"code_agent_write", phaseEdit}, {"code_agent_patch", phaseEdit}, - {"bash_execute", phaseEdit}, {"file_create", phaseEdit}, {"code_agent_run", phaseEdit}, {"github_commit", phaseGitOps}, diff --git a/forge-core/tools/builtins/bash_execute.go b/forge-core/tools/builtins/bash_execute.go deleted file mode 100644 index f2d8fdf..0000000 --- a/forge-core/tools/builtins/bash_execute.go +++ /dev/null @@ -1,219 +0,0 @@ -package builtins - -import ( - "bytes" - "context" - "encoding/json" - "fmt" - "os" - "os/exec" - "runtime" - "strings" - "time" - - "github.com/initializ/forge/forge-core/tools" -) - -const ( - bashDefaultTimeout = 120 * time.Second - bashMaxOutputBytes = 1 * 1024 * 1024 // 1 MB -) - -// dangerousCommands is a deny-list of commands/patterns that are blocked. -var dangerousCommands = []string{ - "rm -rf /", - "rm -rf /*", - "mkfs.", - "dd if=", - ":(){ :|:& };:", // fork bomb - "> /dev/sda", // disk overwrite - "chmod -R 777 /", // recursive world-writable root - "shutdown", // system shutdown - "reboot", // system reboot - "init 0", // system halt - "halt", // system halt - "poweroff", // system poweroff -} - -// blockedPrefixes are command prefixes that are always blocked. -var blockedPrefixes = []string{ - "sudo ", - "su ", - "su\n", -} - -type bashExecuteTool struct { - workDir string - proxyURL string -} - -func (t *bashExecuteTool) Name() string { return "bash_execute" } -func (t *bashExecuteTool) Description() string { - return "Execute a bash command in the project directory. Supports pipes, redirection, and shell features. Commands run with a timeout (default 120s) and output is capped at 1MB. Dangerous commands (sudo, rm -rf /, etc.) are blocked." -} -func (t *bashExecuteTool) Category() tools.Category { return tools.CategoryBuiltin } - -func (t *bashExecuteTool) InputSchema() json.RawMessage { - return json.RawMessage(`{ - "type": "object", - "properties": { - "command": { - "type": "string", - "description": "The bash command to execute" - }, - "timeout": { - "type": "integer", - "description": "Timeout in seconds. Default: 120, Max: 600" - } - }, - "required": ["command"] - }`) -} - -func (t *bashExecuteTool) Execute(ctx context.Context, args json.RawMessage) (string, error) { - var input struct { - Command string `json:"command"` - Timeout int `json:"timeout"` - } - if err := json.Unmarshal(args, &input); err != nil { - return "", fmt.Errorf("invalid arguments: %w", err) - } - - if strings.TrimSpace(input.Command) == "" { - return "", fmt.Errorf("command is required") - } - - // Check deny-list. - if err := t.validateCommand(input.Command); err != nil { - return "", err - } - - // Determine timeout. - timeout := bashDefaultTimeout - if input.Timeout > 0 { - timeout = time.Duration(min(input.Timeout, 600)) * time.Second - } - - cmdCtx, cancel := context.WithTimeout(ctx, timeout) - defer cancel() - - cmd := exec.CommandContext(cmdCtx, "bash", "-c", input.Command) - cmd.Dir = t.workDir - cmd.Env = t.buildEnv() - - stdoutWriter := newBashLimitedWriter(bashMaxOutputBytes) - stderrWriter := newBashLimitedWriter(bashMaxOutputBytes) - cmd.Stdout = stdoutWriter - cmd.Stderr = stderrWriter - - err := cmd.Run() - - exitCode := 0 - if err != nil { - if exitErr, ok := err.(*exec.ExitError); ok { - exitCode = exitErr.ExitCode() - } else if cmdCtx.Err() == context.DeadlineExceeded { - exitCode = 124 // timeout exit code - } else { - exitCode = 1 - } - } - - result := map[string]any{ - "stdout": strings.TrimRight(stdoutWriter.String(), "\n"), - "stderr": strings.TrimRight(stderrWriter.String(), "\n"), - "exit_code": exitCode, - "truncated": stdoutWriter.overflow || stderrWriter.overflow, - } - - if cmdCtx.Err() == context.DeadlineExceeded { - result["error"] = "command timed out" - } - - out, _ := json.Marshal(result) - return string(out), nil -} - -func (t *bashExecuteTool) validateCommand(cmd string) error { - lower := strings.ToLower(strings.TrimSpace(cmd)) - - for _, prefix := range blockedPrefixes { - if strings.HasPrefix(lower, prefix) { - return fmt.Errorf("command blocked: %q is not allowed", strings.TrimSpace(prefix)) - } - } - - for _, pattern := range dangerousCommands { - if strings.Contains(lower, strings.ToLower(pattern)) { - return fmt.Errorf("command blocked: contains dangerous pattern %q", pattern) - } - } - - return nil -} - -func (t *bashExecuteTool) buildEnv() []string { - home := os.Getenv("HOME") - if home == "" { - home = t.workDir - } - - env := []string{ - "PATH=" + os.Getenv("PATH"), - "HOME=" + home, - "LANG=" + os.Getenv("LANG"), - "TERM=xterm-256color", - "USER=" + os.Getenv("USER"), - } - - // Pass through DISPLAY for Linux GUI apps (browser opening). - if runtime.GOOS == "linux" { - if display := os.Getenv("DISPLAY"); display != "" { - env = append(env, "DISPLAY="+display) - } - if xauth := os.Getenv("XAUTHORITY"); xauth != "" { - env = append(env, "XAUTHORITY="+xauth) - } - } - - if t.proxyURL != "" { - env = append(env, - "HTTP_PROXY="+t.proxyURL, - "HTTPS_PROXY="+t.proxyURL, - "http_proxy="+t.proxyURL, - "https_proxy="+t.proxyURL, - ) - } - - return env -} - -// bashLimitedWriter caps output at a byte limit. -type bashLimitedWriter struct { - buf bytes.Buffer - limit int - overflow bool -} - -func newBashLimitedWriter(limit int) *bashLimitedWriter { - return &bashLimitedWriter{limit: limit} -} - -func (w *bashLimitedWriter) Write(p []byte) (int, error) { - remaining := w.limit - w.buf.Len() - if remaining <= 0 { - w.overflow = true - return len(p), nil - } - if len(p) > remaining { - w.buf.Write(p[:remaining]) - w.overflow = true - return len(p), nil - } - w.buf.Write(p) - return len(p), nil -} - -func (w *bashLimitedWriter) String() string { - return w.buf.String() -} diff --git a/forge-core/tools/builtins/code_agent_tools_test.go b/forge-core/tools/builtins/code_agent_tools_test.go index 3d70d49..a22c6bb 100644 --- a/forge-core/tools/builtins/code_agent_tools_test.go +++ b/forge-core/tools/builtins/code_agent_tools_test.go @@ -26,7 +26,6 @@ func TestRegisterCodeAgentTools(t *testing.T) { "file_write", "file_edit", "file_patch", - "bash_execute", "grep_search", "glob_search", "directory_tree", @@ -41,8 +40,8 @@ func TestRegisterCodeAgentTools(t *testing.T) { func TestCodeAgentToolsCount(t *testing.T) { toolList := CodeAgentTools(t.TempDir()) - if len(toolList) != 8 { - t.Errorf("expected 8 tools, got %d", len(toolList)) + if len(toolList) != 7 { + t.Errorf("expected 7 tools, got %d", len(toolList)) } } @@ -321,120 +320,6 @@ func TestFilePatch_PathTraversal(t *testing.T) { } } -// --- bash_execute tests --- - -func TestBashExecute_HappyPath(t *testing.T) { - workDir := t.TempDir() - tool := &bashExecuteTool{workDir: workDir} - - result, err := tool.Execute(context.Background(), toJSON(t, map[string]any{ - "command": "echo hello", - })) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - - var out map[string]any - if err := json.Unmarshal([]byte(result), &out); err != nil { - t.Fatalf("unmarshal: %v", err) - } - if out["stdout"] != "hello" { - t.Errorf("expected stdout 'hello', got %q", out["stdout"]) - } - if out["exit_code"] != float64(0) { - t.Errorf("expected exit_code 0, got %v", out["exit_code"]) - } -} - -func TestBashExecute_ExitCode(t *testing.T) { - workDir := t.TempDir() - tool := &bashExecuteTool{workDir: workDir} - - result, err := tool.Execute(context.Background(), toJSON(t, map[string]any{ - "command": "exit 42", - })) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - - var out map[string]any - if err := json.Unmarshal([]byte(result), &out); err != nil { - t.Fatalf("unmarshal: %v", err) - } - if out["exit_code"] != float64(42) { - t.Errorf("expected exit_code 42, got %v", out["exit_code"]) - } -} - -func TestBashExecute_DangerousCommandBlocked(t *testing.T) { - workDir := t.TempDir() - tool := &bashExecuteTool{workDir: workDir} - - tests := []struct { - name string - command string - }{ - {"sudo", "sudo rm -rf /"}, - {"rm_rf_root", "rm -rf /"}, - {"fork_bomb", ":(){ :|:& };:"}, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - _, err := tool.Execute(context.Background(), toJSON(t, map[string]any{ - "command": tt.command, - })) - if err == nil { - t.Fatal("expected error for dangerous command") - } - }) - } -} - -func TestBashExecute_Timeout(t *testing.T) { - workDir := t.TempDir() - tool := &bashExecuteTool{workDir: workDir} - - result, err := tool.Execute(context.Background(), toJSON(t, map[string]any{ - "command": "sleep 10", - "timeout": 1, - })) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - - var out map[string]any - if err := json.Unmarshal([]byte(result), &out); err != nil { - t.Fatalf("unmarshal: %v", err) - } - if out["exit_code"] != float64(124) { - // Process may be killed with different exit codes, just verify it's non-zero - if out["exit_code"] == float64(0) { - t.Error("expected non-zero exit code for timed out command") - } - } -} - -func TestBashExecute_Pipes(t *testing.T) { - workDir := t.TempDir() - tool := &bashExecuteTool{workDir: workDir} - - result, err := tool.Execute(context.Background(), toJSON(t, map[string]any{ - "command": "echo 'hello world' | tr ' ' '\\n' | wc -l", - })) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - - var out map[string]any - if err := json.Unmarshal([]byte(result), &out); err != nil { - t.Fatalf("unmarshal: %v", err) - } - if out["exit_code"] != float64(0) { - t.Errorf("expected exit_code 0, got %v", out["exit_code"]) - } -} - // --- grep_search tests --- func TestGrepSearch_HappyPath(t *testing.T) { diff --git a/forge-core/tools/builtins/register.go b/forge-core/tools/builtins/register.go index 0e060ac..d750aa4 100644 --- a/forge-core/tools/builtins/register.go +++ b/forge-core/tools/builtins/register.go @@ -57,16 +57,6 @@ func RegisterCodeAgentSearchTools(reg *tools.Registry, workDir string) error { return nil } -// CodeAgentBashTool returns the bash execution tool. -func CodeAgentBashTool(workDir string) tools.Tool { - return &bashExecuteTool{workDir: workDir} -} - -// RegisterCodeAgentBashTool registers the bash execution tool. -func RegisterCodeAgentBashTool(reg *tools.Registry, workDir string) error { - return reg.Register(CodeAgentBashTool(workDir)) -} - // CodeAgentReadTools returns read-only coding tools (file_read + search). func CodeAgentReadTools(workDir string) []tools.Tool { pv := NewPathValidator(workDir) @@ -85,7 +75,6 @@ func CodeAgentWriteTools(workDir string) []tools.Tool { &fileWriteTool{pathValidator: pv}, &fileEditTool{pathValidator: pv}, &filePatchTool{pathValidator: pv}, - &bashExecuteTool{workDir: workDir}, } } diff --git a/forge-skills/local/embedded/code-agent/SKILL.md b/forge-skills/local/embedded/code-agent/SKILL.md index 81992b7..91c441b 100644 --- a/forge-skills/local/embedded/code-agent/SKILL.md +++ b/forge-skills/local/embedded/code-agent/SKILL.md @@ -35,7 +35,6 @@ metadata: - repo.maven.apache.org - repo1.maven.org denied_tools: - - bash_execute - file_write - file_edit - file_patch diff --git a/skills/code-agent/SKILL.md b/skills/code-agent/SKILL.md index e716a35..819a909 100644 --- a/skills/code-agent/SKILL.md +++ b/skills/code-agent/SKILL.md @@ -34,7 +34,6 @@ metadata: - repo.maven.apache.org - repo1.maven.org denied_tools: - - bash_execute - file_write - file_edit - file_patch