diff --git a/config/config.example.json b/config/config.example.json index 3c9158e9c..660c580cc 100644 --- a/config/config.example.json +++ b/config/config.example.json @@ -114,6 +114,15 @@ } }, "tools": { + "exec": { + "enabled": false + }, + "i2c": { + "enabled": false + }, + "spi": { + "enabled": false + }, "web": { "search": { "api_key": "YOUR_BRAVE_API_KEY", diff --git a/pkg/agent/loop.go b/pkg/agent/loop.go index cd4276155..cac2de7a8 100644 --- a/pkg/agent/loop.go +++ b/pkg/agent/loop.go @@ -70,8 +70,10 @@ func createToolRegistry(workspace string, restrict bool, cfg *config.Config, msg registry.Register(tools.NewEditFileTool(workspace, restrict)) registry.Register(tools.NewAppendFileTool(workspace, restrict)) - // Shell execution - registry.Register(tools.NewExecTool(workspace, restrict)) + // Shell execution (disabled by default for security) + if cfg.Tools.Exec.Enabled { + registry.Register(tools.NewExecTool(workspace, restrict)) + } if searchTool := tools.NewWebSearchTool(tools.WebSearchToolOptions{ BraveAPIKey: cfg.Tools.Web.Brave.APIKey, @@ -84,9 +86,13 @@ func createToolRegistry(workspace string, restrict bool, cfg *config.Config, msg } registry.Register(tools.NewWebFetchTool(50000)) - // Hardware tools (I2C, SPI) - Linux only, returns error on other platforms - registry.Register(tools.NewI2CTool()) - registry.Register(tools.NewSPITool()) + // Hardware tools (I2C, SPI) - disabled by default for security + if cfg.Tools.I2C.Enabled { + registry.Register(tools.NewI2CTool()) + } + if cfg.Tools.SPI.Enabled { + registry.Register(tools.NewSPITool()) + } // Message tool - available to both agent and subagent // Subagent uses it to communicate directly with user diff --git a/pkg/agent/loop_test.go b/pkg/agent/loop_test.go index 0bd38abf4..e9b26f5ee 100644 --- a/pkg/agent/loop_test.go +++ b/pkg/agent/loop_test.go @@ -323,6 +323,247 @@ func TestAgentLoop_GetStartupInfo(t *testing.T) { } } +// TestCreateToolRegistry_ExecDisabled verifies exec tool is NOT registered when disabled +func TestCreateToolRegistry_ExecDisabled(t *testing.T) { + tmpDir, err := os.MkdirTemp("", "agent-test-*") + if err != nil { + t.Fatalf("Failed to create temp dir: %v", err) + } + defer os.RemoveAll(tmpDir) + + cfg := &config.Config{ + Agents: config.AgentsConfig{ + Defaults: config.AgentDefaults{ + Workspace: tmpDir, + Model: "test-model", + MaxTokens: 4096, + MaxToolIterations: 10, + }, + }, + Tools: config.ToolsConfig{ + Exec: config.ExecToolsConfig{ + Enabled: false, + }, + }, + } + + msgBus := bus.NewMessageBus() + provider := &mockProvider{} + al := NewAgentLoop(cfg, msgBus, provider) + + info := al.GetStartupInfo() + toolsInfo := info["tools"].(map[string]interface{}) + toolsList := toolsInfo["names"].([]string) + + for _, name := range toolsList { + if name == "exec" { + t.Error("exec tool should NOT be registered when Exec.Enabled is false") + } + } +} + +// TestCreateToolRegistry_ExecEnabled verifies exec tool IS registered when enabled +func TestCreateToolRegistry_ExecEnabled(t *testing.T) { + tmpDir, err := os.MkdirTemp("", "agent-test-*") + if err != nil { + t.Fatalf("Failed to create temp dir: %v", err) + } + defer os.RemoveAll(tmpDir) + + cfg := &config.Config{ + Agents: config.AgentsConfig{ + Defaults: config.AgentDefaults{ + Workspace: tmpDir, + Model: "test-model", + MaxTokens: 4096, + MaxToolIterations: 10, + }, + }, + Tools: config.ToolsConfig{ + Exec: config.ExecToolsConfig{ + Enabled: true, + }, + }, + } + + msgBus := bus.NewMessageBus() + provider := &mockProvider{} + al := NewAgentLoop(cfg, msgBus, provider) + + info := al.GetStartupInfo() + toolsInfo := info["tools"].(map[string]interface{}) + toolsList := toolsInfo["names"].([]string) + + found := false + for _, name := range toolsList { + if name == "exec" { + found = true + break + } + } + if !found { + t.Error("exec tool should be registered when Exec.Enabled is true") + } +} + +// TestCreateToolRegistry_I2CDisabled verifies I2C tool is NOT registered when disabled +func TestCreateToolRegistry_I2CDisabled(t *testing.T) { + tmpDir, err := os.MkdirTemp("", "agent-test-*") + if err != nil { + t.Fatalf("Failed to create temp dir: %v", err) + } + defer os.RemoveAll(tmpDir) + + cfg := &config.Config{ + Agents: config.AgentsConfig{ + Defaults: config.AgentDefaults{ + Workspace: tmpDir, + Model: "test-model", + MaxTokens: 4096, + MaxToolIterations: 10, + }, + }, + Tools: config.ToolsConfig{ + I2C: config.I2CToolsConfig{Enabled: false}, + }, + } + + msgBus := bus.NewMessageBus() + provider := &mockProvider{} + al := NewAgentLoop(cfg, msgBus, provider) + + info := al.GetStartupInfo() + toolsInfo := info["tools"].(map[string]interface{}) + toolsList := toolsInfo["names"].([]string) + + for _, name := range toolsList { + if name == "i2c" { + t.Error("i2c tool should NOT be registered when I2C.Enabled is false") + } + } +} + +// TestCreateToolRegistry_I2CEnabled verifies I2C tool IS registered when enabled +func TestCreateToolRegistry_I2CEnabled(t *testing.T) { + tmpDir, err := os.MkdirTemp("", "agent-test-*") + if err != nil { + t.Fatalf("Failed to create temp dir: %v", err) + } + defer os.RemoveAll(tmpDir) + + cfg := &config.Config{ + Agents: config.AgentsConfig{ + Defaults: config.AgentDefaults{ + Workspace: tmpDir, + Model: "test-model", + MaxTokens: 4096, + MaxToolIterations: 10, + }, + }, + Tools: config.ToolsConfig{ + I2C: config.I2CToolsConfig{Enabled: true}, + }, + } + + msgBus := bus.NewMessageBus() + provider := &mockProvider{} + al := NewAgentLoop(cfg, msgBus, provider) + + info := al.GetStartupInfo() + toolsInfo := info["tools"].(map[string]interface{}) + toolsList := toolsInfo["names"].([]string) + + found := false + for _, name := range toolsList { + if name == "i2c" { + found = true + break + } + } + if !found { + t.Error("i2c tool should be registered when I2C.Enabled is true") + } +} + +// TestCreateToolRegistry_SPIDisabled verifies SPI tool is NOT registered when disabled +func TestCreateToolRegistry_SPIDisabled(t *testing.T) { + tmpDir, err := os.MkdirTemp("", "agent-test-*") + if err != nil { + t.Fatalf("Failed to create temp dir: %v", err) + } + defer os.RemoveAll(tmpDir) + + cfg := &config.Config{ + Agents: config.AgentsConfig{ + Defaults: config.AgentDefaults{ + Workspace: tmpDir, + Model: "test-model", + MaxTokens: 4096, + MaxToolIterations: 10, + }, + }, + Tools: config.ToolsConfig{ + SPI: config.SPIToolsConfig{Enabled: false}, + }, + } + + msgBus := bus.NewMessageBus() + provider := &mockProvider{} + al := NewAgentLoop(cfg, msgBus, provider) + + info := al.GetStartupInfo() + toolsInfo := info["tools"].(map[string]interface{}) + toolsList := toolsInfo["names"].([]string) + + for _, name := range toolsList { + if name == "spi" { + t.Error("spi tool should NOT be registered when SPI.Enabled is false") + } + } +} + +// TestCreateToolRegistry_SPIEnabled verifies SPI tool IS registered when enabled +func TestCreateToolRegistry_SPIEnabled(t *testing.T) { + tmpDir, err := os.MkdirTemp("", "agent-test-*") + if err != nil { + t.Fatalf("Failed to create temp dir: %v", err) + } + defer os.RemoveAll(tmpDir) + + cfg := &config.Config{ + Agents: config.AgentsConfig{ + Defaults: config.AgentDefaults{ + Workspace: tmpDir, + Model: "test-model", + MaxTokens: 4096, + MaxToolIterations: 10, + }, + }, + Tools: config.ToolsConfig{ + SPI: config.SPIToolsConfig{Enabled: true}, + }, + } + + msgBus := bus.NewMessageBus() + provider := &mockProvider{} + al := NewAgentLoop(cfg, msgBus, provider) + + info := al.GetStartupInfo() + toolsInfo := info["tools"].(map[string]interface{}) + toolsList := toolsInfo["names"].([]string) + + found := false + for _, name := range toolsList { + if name == "spi" { + found = true + break + } + } + if !found { + t.Error("spi tool should be registered when SPI.Enabled is true") + } +} + // TestAgentLoop_Stop verifies Stop() sets running to false func TestAgentLoop_Stop(t *testing.T) { tmpDir, err := os.MkdirTemp("", "agent-test-*") diff --git a/pkg/config/config.go b/pkg/config/config.go index d189ff00b..4f7e0028e 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -211,8 +211,23 @@ type WebToolsConfig struct { DuckDuckGo DuckDuckGoConfig `json:"duckduckgo"` } +type ExecToolsConfig struct { + Enabled bool `json:"enabled" env:"PICOCLAW_TOOLS_EXEC_ENABLED"` +} + +type I2CToolsConfig struct { + Enabled bool `json:"enabled" env:"PICOCLAW_TOOLS_I2C_ENABLED"` +} + +type SPIToolsConfig struct { + Enabled bool `json:"enabled" env:"PICOCLAW_TOOLS_SPI_ENABLED"` +} + type ToolsConfig struct { - Web WebToolsConfig `json:"web"` + Web WebToolsConfig `json:"web"` + Exec ExecToolsConfig `json:"exec"` + I2C I2CToolsConfig `json:"i2c"` + SPI SPIToolsConfig `json:"spi"` } func DefaultConfig() *Config { @@ -311,6 +326,15 @@ func DefaultConfig() *Config { Port: 18790, }, Tools: ToolsConfig{ + Exec: ExecToolsConfig{ + Enabled: false, + }, + I2C: I2CToolsConfig{ + Enabled: false, + }, + SPI: SPIToolsConfig{ + Enabled: false, + }, Web: WebToolsConfig{ Brave: BraveConfig{ Enabled: false, diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index febfd0456..b7df307a8 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -134,6 +134,30 @@ func TestDefaultConfig_Channels(t *testing.T) { } } +// TestDefaultConfig_ExecToolDisabled verifies exec tool is disabled by default +func TestDefaultConfig_ExecToolDisabled(t *testing.T) { + cfg := DefaultConfig() + if cfg.Tools.Exec.Enabled { + t.Error("Exec tool should be disabled by default") + } +} + +// TestDefaultConfig_I2CToolDisabled verifies I2C tool is disabled by default +func TestDefaultConfig_I2CToolDisabled(t *testing.T) { + cfg := DefaultConfig() + if cfg.Tools.I2C.Enabled { + t.Error("I2C tool should be disabled by default") + } +} + +// TestDefaultConfig_SPIToolDisabled verifies SPI tool is disabled by default +func TestDefaultConfig_SPIToolDisabled(t *testing.T) { + cfg := DefaultConfig() + if cfg.Tools.SPI.Enabled { + t.Error("SPI tool should be disabled by default") + } +} + // TestDefaultConfig_WebTools verifies web tools config func TestDefaultConfig_WebTools(t *testing.T) { cfg := DefaultConfig()