Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion cmd/picoclaw/internal/gateway/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"errors"
"fmt"
"log"
"net/http"
"os"
"os/signal"
Expand Down Expand Up @@ -232,7 +233,10 @@ func setupCronTool(
cronService := cron.NewCronService(cronStorePath, nil)

// Create and register CronTool
cronTool := tools.NewCronTool(cronService, agentLoop, msgBus, workspace, restrict, execTimeout, cfg)
cronTool, err := tools.NewCronTool(cronService, agentLoop, msgBus, workspace, restrict, execTimeout, cfg)
if err != nil {
log.Fatalf("Critical error during CronTool initialization: %v", err)
}
agentLoop.RegisterTool(cronTool)

// Set the onJob handler
Expand Down
8 changes: 7 additions & 1 deletion pkg/agent/instance.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package agent

import (
"log"
"os"
"path/filepath"
"strings"
Expand Down Expand Up @@ -51,7 +52,12 @@ func NewAgentInstance(
toolsRegistry.Register(tools.NewReadFileTool(workspace, restrict))
toolsRegistry.Register(tools.NewWriteFileTool(workspace, restrict))
toolsRegistry.Register(tools.NewListDirTool(workspace, restrict))
toolsRegistry.Register(tools.NewExecToolWithConfig(workspace, restrict, cfg))
execTool, err := tools.NewExecToolWithConfig(workspace, restrict, cfg)
if err != nil {
log.Fatalf("Critical error: unable to initialize exec tool: %v", err)
}
toolsRegistry.Register(execTool)

toolsRegistry.Register(tools.NewEditFileTool(workspace, restrict))
toolsRegistry.Register(tools.NewAppendFileTool(workspace, restrict))

Expand Down
10 changes: 7 additions & 3 deletions pkg/tools/cron.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,15 +33,19 @@ type CronTool struct {
func NewCronTool(
cronService *cron.CronService, executor JobExecutor, msgBus *bus.MessageBus, workspace string, restrict bool,
execTimeout time.Duration, config *config.Config,
) *CronTool {
execTool := NewExecToolWithConfig(workspace, restrict, config)
) (*CronTool, error) {
execTool, err := NewExecToolWithConfig(workspace, restrict, config)
if err != nil {
return nil, fmt.Errorf("unable to configure exec tool: %w", err)
}

execTool.SetTimeout(execTimeout)
return &CronTool{
cronService: cronService,
executor: executor,
msgBus: msgBus,
execTool: execTool,
}
}, nil
}

// Name returns the tool name
Expand Down
9 changes: 4 additions & 5 deletions pkg/tools/shell.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,11 +69,11 @@ var defaultDenyPatterns = []*regexp.Regexp{
regexp.MustCompile(`\bsource\s+.*\.sh\b`),
}

func NewExecTool(workingDir string, restrict bool) *ExecTool {
func NewExecTool(workingDir string, restrict bool) (*ExecTool, error) {
return NewExecToolWithConfig(workingDir, restrict, nil)
}

func NewExecToolWithConfig(workingDir string, restrict bool, config *config.Config) *ExecTool {
func NewExecToolWithConfig(workingDir string, restrict bool, config *config.Config) (*ExecTool, error) {
denyPatterns := make([]*regexp.Regexp, 0)

if config != nil {
Expand All @@ -86,8 +86,7 @@ func NewExecToolWithConfig(workingDir string, restrict bool, config *config.Conf
for _, pattern := range execConfig.CustomDenyPatterns {
re, err := regexp.Compile(pattern)
if err != nil {
fmt.Printf("Invalid custom deny pattern %q: %v\n", pattern, err)
continue
return nil, fmt.Errorf("invalid custom deny pattern %q: %w", pattern, err)
}
denyPatterns = append(denyPatterns, re)
}
Expand All @@ -106,7 +105,7 @@ func NewExecToolWithConfig(workingDir string, restrict bool, config *config.Conf
denyPatterns: denyPatterns,
allowPatterns: nil,
restrictToWorkspace: restrict,
}
}, nil
}

func (t *ExecTool) Name() string {
Expand Down
59 changes: 48 additions & 11 deletions pkg/tools/shell_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,10 @@ import (

// TestShellTool_Success verifies successful command execution
func TestShellTool_Success(t *testing.T) {
tool := NewExecTool("", false)
tool, err := NewExecTool("", false)
if err != nil {
t.Errorf("unable to configure exec tool: %s", err)
}

ctx := context.Background()
args := map[string]any{
Expand All @@ -38,7 +41,10 @@ func TestShellTool_Success(t *testing.T) {

// TestShellTool_Failure verifies failed command execution
func TestShellTool_Failure(t *testing.T) {
tool := NewExecTool("", false)
tool, err := NewExecTool("", false)
if err != nil {
t.Errorf("unable to configure exec tool: %s", err)
}

ctx := context.Background()
args := map[string]any{
Expand All @@ -65,7 +71,11 @@ func TestShellTool_Failure(t *testing.T) {

// TestShellTool_Timeout verifies command timeout handling
func TestShellTool_Timeout(t *testing.T) {
tool := NewExecTool("", false)
tool, err := NewExecTool("", false)
if err != nil {
t.Errorf("unable to configure exec tool: %s", err)
}

tool.SetTimeout(100 * time.Millisecond)

ctx := context.Background()
Expand Down Expand Up @@ -93,7 +103,10 @@ func TestShellTool_WorkingDir(t *testing.T) {
testFile := filepath.Join(tmpDir, "test.txt")
os.WriteFile(testFile, []byte("test content"), 0o644)

tool := NewExecTool("", false)
tool, err := NewExecTool("", false)
if err != nil {
t.Errorf("unable to configure exec tool: %s", err)
}

ctx := context.Background()
args := map[string]any{
Expand All @@ -114,7 +127,10 @@ func TestShellTool_WorkingDir(t *testing.T) {

// TestShellTool_DangerousCommand verifies safety guard blocks dangerous commands
func TestShellTool_DangerousCommand(t *testing.T) {
tool := NewExecTool("", false)
tool, err := NewExecTool("", false)
if err != nil {
t.Errorf("unable to configure exec tool: %s", err)
}

ctx := context.Background()
args := map[string]any{
Expand All @@ -135,7 +151,10 @@ func TestShellTool_DangerousCommand(t *testing.T) {

// TestShellTool_MissingCommand verifies error handling for missing command
func TestShellTool_MissingCommand(t *testing.T) {
tool := NewExecTool("", false)
tool, err := NewExecTool("", false)
if err != nil {
t.Errorf("unable to configure exec tool: %s", err)
}

ctx := context.Background()
args := map[string]any{}
Expand All @@ -150,7 +169,10 @@ func TestShellTool_MissingCommand(t *testing.T) {

// TestShellTool_StderrCapture verifies stderr is captured and included
func TestShellTool_StderrCapture(t *testing.T) {
tool := NewExecTool("", false)
tool, err := NewExecTool("", false)
if err != nil {
t.Errorf("unable to configure exec tool: %s", err)
}

ctx := context.Background()
args := map[string]any{
Expand All @@ -170,7 +192,10 @@ func TestShellTool_StderrCapture(t *testing.T) {

// TestShellTool_OutputTruncation verifies long output is truncated
func TestShellTool_OutputTruncation(t *testing.T) {
tool := NewExecTool("", false)
tool, err := NewExecTool("", false)
if err != nil {
t.Errorf("unable to configure exec tool: %s", err)
}

ctx := context.Background()
// Generate long output (>10000 chars)
Expand Down Expand Up @@ -198,7 +223,11 @@ func TestShellTool_WorkingDir_OutsideWorkspace(t *testing.T) {
t.Fatalf("failed to create outside dir: %v", err)
}

tool := NewExecTool(workspace, true)
tool, err := NewExecTool(workspace, true)
if err != nil {
t.Errorf("unable to configure exec tool: %s", err)
}

result := tool.Execute(context.Background(), map[string]any{
"command": "pwd",
"working_dir": outsideDir,
Expand Down Expand Up @@ -232,7 +261,11 @@ func TestShellTool_WorkingDir_SymlinkEscape(t *testing.T) {
t.Skipf("symlinks not supported in this environment: %v", err)
}

tool := NewExecTool(workspace, true)
tool, err := NewExecTool(workspace, true)
if err != nil {
t.Errorf("unable to configure exec tool: %s", err)
}

result := tool.Execute(context.Background(), map[string]any{
"command": "cat secret.txt",
"working_dir": link,
Expand All @@ -249,7 +282,11 @@ func TestShellTool_WorkingDir_SymlinkEscape(t *testing.T) {
// TestShellTool_RestrictToWorkspace verifies workspace restriction
func TestShellTool_RestrictToWorkspace(t *testing.T) {
tmpDir := t.TempDir()
tool := NewExecTool(tmpDir, false)
tool, err := NewExecTool(tmpDir, false)
if err != nil {
t.Errorf("unable to configure exec tool: %s", err)
}

tool.SetRestrictToWorkspace(true)

ctx := context.Background()
Expand Down
6 changes: 5 additions & 1 deletion pkg/tools/shell_timeout_unix_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,11 @@ func processExists(pid int) bool {
}

func TestShellTool_TimeoutKillsChildProcess(t *testing.T) {
tool := NewExecTool(t.TempDir(), false)
tool, err := NewExecTool(t.TempDir(), false)
if err != nil {
t.Errorf("unable to configure exec tool: %s", err)
}

tool.SetTimeout(500 * time.Millisecond)

args := map[string]any{
Expand Down