From 3056a540bffba4cc0dc2e70ac035ee94f33d9ecf Mon Sep 17 00:00:00 2001 From: jp39 Date: Tue, 16 Jun 2026 15:25:30 +0200 Subject: [PATCH 1/3] feat: allow configured remote cron commands Add tools.cron.command_allowed_remotes so remote channels can be explicitly allowlisted for command-executing cron jobs. Entries support channel names, channel:chat_id pairs, and a literal * to allow every non-empty channel while preserving the default deny posture. The existing allow_command and command_confirm checks still apply after the channel gate. Update tests, config examples, and cron documentation for the new option. --- config/config.example.json | 4 +- docs/guides/configuration.md | 6 +- docs/guides/providers.md | 4 +- docs/reference/cron.md | 26 ++++- docs/reference/tools_configuration.md | 11 +- pkg/config/config.go | 8 +- pkg/config/config_test.go | 36 +++++++ pkg/tools/cron.go | 65 +++++++++--- pkg/tools/cron_test.go | 146 +++++++++++++++++++++++++- 9 files changed, 271 insertions(+), 35 deletions(-) diff --git a/config/config.example.json b/config/config.example.json index 129b5bff..37487901 100644 --- a/config/config.example.json +++ b/config/config.example.json @@ -380,7 +380,9 @@ }, "cron": { "enabled": true, - "exec_timeout_minutes": 5 + "exec_timeout_minutes": 5, + "allow_command": true, + "command_allowed_remotes": [] }, "mcp": { "enabled": false, diff --git a/docs/guides/configuration.md b/docs/guides/configuration.md index 86a9fb3a..c030206c 100644 --- a/docs/guides/configuration.md +++ b/docs/guides/configuration.md @@ -1154,7 +1154,9 @@ PicoClaw supports cron-style scheduled tasks via the `cron` tool. The agent can "tools": { "cron": { "enabled": true, - "exec_timeout_minutes": 5 + "exec_timeout_minutes": 5, + "allow_command": true, + "command_allowed_remotes": [] } } } @@ -1162,6 +1164,8 @@ PicoClaw supports cron-style scheduled tasks via the `cron` tool. The agent can Scheduled tasks persist across restarts and are stored in `~/.picoclaw/workspace/cron/`. +Command cron jobs can execute shell commands. By default, remote channels cannot schedule command jobs. To allow specific remote channels, set `command_allowed_remotes` to entries such as `"telegram"` or `"telegram:1234567890"`; use `"*"` only if every non-empty channel should be allowed. This does not bypass `allow_command`, `command_confirm`, or exec safety checks. + ### Advanced Topics | Topic | Description | diff --git a/docs/guides/providers.md b/docs/guides/providers.md index 0276a1e5..9d4c4ea7 100644 --- a/docs/guides/providers.md +++ b/docs/guides/providers.md @@ -647,7 +647,9 @@ picoclaw agent -m "Hello" } }, "cron": { - "exec_timeout_minutes": 5 + "exec_timeout_minutes": 5, + "allow_command": true, + "command_allowed_remotes": [] } }, "heartbeat": { diff --git a/docs/reference/cron.md b/docs/reference/cron.md index 6808f577..1ac36a52 100644 --- a/docs/reference/cron.md +++ b/docs/reference/cron.md @@ -43,7 +43,8 @@ the original prompt, delivery target, or command payload. Remote channel access is scoped to the current `channel/chat_id`: remote callers can only list, get, or update jobs whose saved `payload.channel` and `payload.to` match the current conversation. Command jobs include a shell command payload, so -they can only be listed, inspected, or updated from internal channels. +they can only be listed, inspected, or updated from internal channels or remote +channels allowed by `tools.cron.command_allowed_remotes`. Example tool calls: @@ -59,7 +60,7 @@ Example tool calls: (`at_seconds`, `every_seconds`, or `cron_expr`). Omit `command` to preserve it, set `command` to a non-empty string to replace it, or set `command` to `""` to clear it. Command updates require the same -internal channel and confirmation gates as command creation. +channel allowlist and confirmation gates as command creation. ## Execution Modes @@ -104,7 +105,7 @@ If `tools.exec.enabled` is `false`: - new command jobs are rejected by the cron tool - existing command jobs publish a `command execution is disabled` error when they fire -`tools.exec.allow_remote` is still enforced by the exec tool, but cron command scheduling already requires an internal channel when the job is created. In practice, reminder jobs can be scheduled from remote channels, while scheduled command jobs are limited to internal channels. +`tools.exec.allow_remote` is still enforced by the exec tool, but cron command scheduling has its own channel gate when the job is created. In practice, reminder jobs can be scheduled from remote channels, while scheduled command jobs are limited to internal channels and configured remote channels. ### `allow_command` @@ -112,7 +113,19 @@ If `tools.exec.enabled` is `false`: This is not a hard disable switch. If you set `allow_command` to `false`, PicoClaw still allows a command job when the caller explicitly passes `command_confirm: true`. -Command jobs also require an internal channel. Non-command reminders do not have that restriction. +Command jobs also require either an internal channel or a remote channel allowed by `tools.cron.command_allowed_remotes`. Non-command reminders do not have that restriction. + +### `command_allowed_remotes` + +`tools.cron.command_allowed_remotes` defaults to an empty list. With the default empty list, remote channels cannot schedule command jobs. + +Entries can be either a channel name or a channel plus chat id: + +- `telegram` allows command jobs from any Telegram chat. +- `telegram:1234567890` allows command jobs only from that exact Telegram chat id. +- `*` allows command jobs from every non-empty channel. + +This setting only controls the remote-channel gate. It does not bypass `tools.cron.allow_command`, `command_confirm`, `tools.exec.enabled`, or the exec tool's command safety checks. Example: @@ -122,7 +135,10 @@ Example: "cron": { "enabled": true, "exec_timeout_minutes": 5, - "allow_command": true + "allow_command": true, + "command_allowed_remotes": [ + "telegram:1234567890" + ] }, "exec": { "enabled": true diff --git a/docs/reference/tools_configuration.md b/docs/reference/tools_configuration.md index 602fcb13..1be05b08 100644 --- a/docs/reference/tools_configuration.md +++ b/docs/reference/tools_configuration.md @@ -311,11 +311,12 @@ as containers, VMs, or an approval flow around build-and-run commands. The cron tool is used for scheduling periodic tasks. -| Config | Type | Default | Description | -|------------------------|------|---------|------------------------------------------------| -| `enabled` | bool | true | Register the agent-facing cron tool | -| `allow_command` | bool | true | Allow command jobs without extra confirmation | -| `exec_timeout_minutes` | int | 5 | Execution timeout in minutes, 0 means no limit | +| Config | Type | Default | Description | +|---------------------------|----------|---------|----------------------------------------------------------------| +| `enabled` | bool | true | Register the agent-facing cron tool | +| `allow_command` | bool | true | Allow command jobs without extra confirmation | +| `command_allowed_remotes` | string[] | [] | Remote channels or `channel:chat_id` values allowed for command jobs; `*` allows every channel | +| `exec_timeout_minutes` | int | 5 | Execution timeout in minutes, 0 means no limit | For schedule types, execution modes (`deliver`, agent turn, and command jobs), persistence, and the current command-security gates, see [Scheduled Tasks and Cron Jobs](cron.md). diff --git a/pkg/config/config.go b/pkg/config/config.go index 655e39aa..aa643d0a 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -1028,9 +1028,11 @@ type WebToolsConfig struct { } type CronToolsConfig struct { - ToolConfig ` envPrefix:"PICOCLAW_TOOLS_CRON_"` - ExecTimeoutMinutes int ` json:"exec_timeout_minutes" env:"PICOCLAW_TOOLS_CRON_EXEC_TIMEOUT_MINUTES"` // 0 means no timeout - AllowCommand bool ` json:"allow_command" env:"PICOCLAW_TOOLS_CRON_ALLOW_COMMAND"` + ToolConfig `envPrefix:"PICOCLAW_TOOLS_CRON_"` + // 0 means no timeout. + ExecTimeoutMinutes int `json:"exec_timeout_minutes" env:"PICOCLAW_TOOLS_CRON_EXEC_TIMEOUT_MINUTES"` + AllowCommand bool `json:"allow_command" env:"PICOCLAW_TOOLS_CRON_ALLOW_COMMAND"` + CommandAllowedRemotes []string `json:"command_allowed_remotes" env:"PICOCLAW_TOOLS_CRON_COMMAND_ALLOWED_REMOTES"` } type ExecConfig struct { diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index 52f8f478..94f76983 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -1540,6 +1540,16 @@ func TestDefaultConfig_CronAllowCommandEnabled(t *testing.T) { } } +func TestDefaultConfig_CronCommandAllowedRemotesEmpty(t *testing.T) { + cfg := DefaultConfig() + if len(cfg.Tools.Cron.CommandAllowedRemotes) != 0 { + t.Fatalf( + "DefaultConfig().Tools.Cron.CommandAllowedRemotes = %#v, want empty", + cfg.Tools.Cron.CommandAllowedRemotes, + ) + } +} + func TestDefaultConfig_HooksDefaults(t *testing.T) { cfg := DefaultConfig() if !cfg.Hooks.Enabled { @@ -1600,6 +1610,32 @@ func TestLoadConfig_CronAllowCommandDefaultsTrueWhenUnset(t *testing.T) { } } +func TestLoadConfig_CronCommandAllowedRemotes(t *testing.T) { + dir := t.TempDir() + configPath := filepath.Join(dir, "config.json") + if err := os.WriteFile( + configPath, + []byte(`{"version":1,"tools":{"cron":{"command_allowed_remotes":["telegram:1234567890","discord"]}}}`), + 0o600, + ); err != nil { + t.Fatalf("WriteFile() error: %v", err) + } + + cfg, err := LoadConfig(configPath) + if err != nil { + t.Fatalf("LoadConfig() error: %v", err) + } + want := []string{"telegram:1234567890", "discord"} + if len(cfg.Tools.Cron.CommandAllowedRemotes) != len(want) { + t.Fatalf("CommandAllowedRemotes = %#v, want %#v", cfg.Tools.Cron.CommandAllowedRemotes, want) + } + for i := range want { + if cfg.Tools.Cron.CommandAllowedRemotes[i] != want[i] { + t.Fatalf("CommandAllowedRemotes = %#v, want %#v", cfg.Tools.Cron.CommandAllowedRemotes, want) + } + } +} + func TestLoadConfig_WebToolsProxy(t *testing.T) { tmpDir := t.TempDir() configPath := filepath.Join(tmpDir, "config.json") diff --git a/pkg/tools/cron.go b/pkg/tools/cron.go index b3b14dc4..404dead7 100644 --- a/pkg/tools/cron.go +++ b/pkg/tools/cron.go @@ -26,12 +26,13 @@ type JobExecutor interface { // CronTool provides scheduling capabilities for the agent type CronTool struct { - cronService *cron.CronService - executor JobExecutor - msgBus *bus.MessageBus - execTool *ExecTool - allowCommand bool - execEnabled bool + cronService *cron.CronService + executor JobExecutor + msgBus *bus.MessageBus + execTool *ExecTool + allowCommand bool + execEnabled bool + commandAllowedRemotes []string } // NewCronTool creates a new CronTool @@ -42,9 +43,11 @@ func NewCronTool( ) (*CronTool, error) { allowCommand := true execEnabled := true + var commandAllowedRemotes []string if config != nil { allowCommand = config.Tools.Cron.AllowCommand execEnabled = config.Tools.Exec.Enabled + commandAllowedRemotes = config.Tools.Cron.CommandAllowedRemotes } var execTool *ExecTool @@ -60,12 +63,13 @@ func NewCronTool( execTool.SetTimeout(execTimeout) } return &CronTool{ - cronService: cronService, - executor: executor, - msgBus: msgBus, - execTool: execTool, - allowCommand: allowCommand, - execEnabled: execEnabled, + cronService: cronService, + executor: executor, + msgBus: msgBus, + execTool: execTool, + allowCommand: allowCommand, + execEnabled: execEnabled, + commandAllowedRemotes: commandAllowedRemotes, }, nil } @@ -217,8 +221,10 @@ func (t *CronTool) addJob(ctx context.Context, args map[string]any) *ToolResult if !t.execEnabled { return ErrorResult("command execution is disabled") } - if !constants.IsInternalChannel(channel) { - return ErrorResult("scheduling command execution is restricted to internal channels") + if !constants.IsInternalChannel(channel) && !isCommandAllowedRemote(channel, chatID, t.commandAllowedRemotes) { + return ErrorResult( + "scheduling command execution is restricted to internal channels or configured remote channels", + ) } if !t.allowCommand && !commandConfirm { return ErrorResult("command_confirm=true is required when allow_command is disabled") @@ -483,11 +489,15 @@ func positiveSeconds(args map[string]any, key string) (int64, *ToolResult) { } func (t *CronTool) validateCommandMutation(ctx context.Context, args map[string]any) *ToolResult { + channel := ToolChannel(ctx) + chatID := ToolChatID(ctx) if !t.execEnabled { return ErrorResult("command execution is disabled") } - if !constants.IsInternalChannel(ToolChannel(ctx)) { - return ErrorResult("updating command execution is restricted to internal channels") + if !constants.IsInternalChannel(channel) && !isCommandAllowedRemote(channel, chatID, t.commandAllowedRemotes) { + return ErrorResult( + "updating command execution is restricted to internal channels or configured remote channels", + ) } commandConfirm, _ := args["command_confirm"].(bool) if !t.allowCommand && !commandConfirm { @@ -496,6 +506,29 @@ func (t *CronTool) validateCommandMutation(ctx context.Context, args map[string] return nil } +func isCommandAllowedRemote(channel, chatID string, allowed []string) bool { + if channel == "" { + return false + } + + target := channel + if chatID != "" { + target = channel + ":" + chatID + } + + for _, entry := range allowed { + entry = strings.TrimSpace(entry) + if entry == "" { + continue + } + if entry == "*" || entry == channel || entry == target { + return true + } + } + + return false +} + func (t *CronTool) canAccessJob(ctx context.Context, job *cron.CronJob) bool { channel := ToolChannel(ctx) if constants.IsInternalChannel(channel) { diff --git a/pkg/tools/cron_test.go b/pkg/tools/cron_test.go index 41048c7f..7118ca28 100644 --- a/pkg/tools/cron_test.go +++ b/pkg/tools/cron_test.go @@ -87,7 +87,7 @@ func parseCronJobResult(t *testing.T, result *ToolResult) cron.CronJob { return job } -// TestCronTool_CommandBlockedFromRemoteChannel verifies command scheduling is restricted to internal channels +// TestCronTool_CommandBlockedFromRemoteChannel verifies command scheduling is restricted by default. func TestCronTool_CommandBlockedFromRemoteChannel(t *testing.T) { tool := newTestCronTool(t) ctx := WithToolContext(context.Background(), "telegram", "chat-1") @@ -102,8 +102,148 @@ func TestCronTool_CommandBlockedFromRemoteChannel(t *testing.T) { if !result.IsError { t.Fatal("expected command scheduling to be blocked from remote channel") } - if !strings.Contains(result.ForLLM, "restricted to internal channels") { - t.Errorf("expected 'restricted to internal channels', got: %s", result.ForLLM) + if !strings.Contains(result.ForLLM, "restricted to internal channels or configured remote channels") { + t.Errorf("expected remote restriction message, got: %s", result.ForLLM) + } +} + +func TestCronTool_CommandAllowedFromRemoteChannelAllowlist(t *testing.T) { + cfg := config.DefaultConfig() + cfg.Tools.Cron.CommandAllowedRemotes = []string{"telegram"} + + tool := newTestCronToolWithConfig(t, cfg) + ctx := WithToolContext(context.Background(), "telegram", "chat-1") + result := tool.Execute(ctx, map[string]any{ + "action": "add", + "message": "check disk", + "command": "df -h", + "at_seconds": float64(60), + }) + + if result.IsError { + t.Fatalf("expected command scheduling from allowed remote channel to succeed, got: %s", result.ForLLM) + } +} + +func TestCronTool_CommandAllowedFromRemoteChatIDAllowlist(t *testing.T) { + cfg := config.DefaultConfig() + cfg.Tools.Cron.CommandAllowedRemotes = []string{" telegram:1234567890 "} + + tool := newTestCronToolWithConfig(t, cfg) + ctx := WithToolContext(context.Background(), "telegram", "1234567890") + result := tool.Execute(ctx, map[string]any{ + "action": "add", + "message": "check disk", + "command": "df -h", + "at_seconds": float64(60), + }) + + if result.IsError { + t.Fatalf("expected command scheduling from allowed remote chat to succeed, got: %s", result.ForLLM) + } +} + +func TestCronTool_CommandAllowedFromRemoteWildcardAllowlist(t *testing.T) { + cfg := config.DefaultConfig() + cfg.Tools.Cron.CommandAllowedRemotes = []string{"*"} + + tool := newTestCronToolWithConfig(t, cfg) + ctx := WithToolContext(context.Background(), "telegram", "chat-1") + result := tool.Execute(ctx, map[string]any{ + "action": "add", + "message": "check disk", + "command": "df -h", + "at_seconds": float64(60), + }) + + if result.IsError { + t.Fatalf("expected wildcard allowlist to allow remote command scheduling, got: %s", result.ForLLM) + } +} + +func TestCronTool_CommandAllowedRemoteWildcardRequiresNonEmptyChannel(t *testing.T) { + cfg := config.DefaultConfig() + cfg.Tools.Cron.CommandAllowedRemotes = []string{"*"} + + tool := newTestCronToolWithConfig(t, cfg) + ctx := WithToolContext(context.Background(), "", "chat-1") + result := tool.Execute(ctx, map[string]any{ + "action": "add", + "message": "check disk", + "command": "df -h", + "at_seconds": float64(60), + }) + + if !result.IsError { + t.Fatal("expected missing channel to remain blocked even with wildcard allowlist") + } + if !strings.Contains(result.ForLLM, "no session context") { + t.Errorf("expected session context error, got: %s", result.ForLLM) + } +} + +func TestCronTool_CommandBlockedFromDifferentRemoteChatID(t *testing.T) { + cfg := config.DefaultConfig() + cfg.Tools.Cron.CommandAllowedRemotes = []string{"telegram:1234567890"} + + tool := newTestCronToolWithConfig(t, cfg) + ctx := WithToolContext(context.Background(), "telegram", "other-chat") + result := tool.Execute(ctx, map[string]any{ + "action": "add", + "message": "check disk", + "command": "df -h", + "command_confirm": true, + "at_seconds": float64(60), + }) + + if !result.IsError { + t.Fatal("expected command scheduling from non-allowlisted remote chat to fail") + } + if !strings.Contains(result.ForLLM, "restricted to internal channels or configured remote channels") { + t.Errorf("expected remote restriction message, got: %s", result.ForLLM) + } +} + +func TestCronTool_CommandAllowedRemoteRequiresConfirmWhenAllowCommandDisabled(t *testing.T) { + cfg := config.DefaultConfig() + cfg.Tools.Cron.AllowCommand = false + cfg.Tools.Cron.CommandAllowedRemotes = []string{"telegram"} + + tool := newTestCronToolWithConfig(t, cfg) + ctx := WithToolContext(context.Background(), "telegram", "chat-1") + result := tool.Execute(ctx, map[string]any{ + "action": "add", + "message": "check disk", + "command": "df -h", + "at_seconds": float64(60), + }) + + if !result.IsError { + t.Fatal("expected allowlisted remote command scheduling to require confirm when allow_command is disabled") + } + if !strings.Contains(result.ForLLM, "command_confirm=true") { + t.Errorf("expected command_confirm requirement message, got: %s", result.ForLLM) + } +} + +func TestCronTool_AllowCommandDoesNotBypassRemoteAllowlist(t *testing.T) { + cfg := config.DefaultConfig() + cfg.Tools.Cron.AllowCommand = true + + tool := newTestCronToolWithConfig(t, cfg) + ctx := WithToolContext(context.Background(), "telegram", "chat-1") + result := tool.Execute(ctx, map[string]any{ + "action": "add", + "message": "check disk", + "command": "df -h", + "at_seconds": float64(60), + }) + + if !result.IsError { + t.Fatal("expected allow_command=true not to bypass remote allowlist") + } + if !strings.Contains(result.ForLLM, "restricted to internal channels or configured remote channels") { + t.Errorf("expected remote restriction message, got: %s", result.ForLLM) } } From 981ab3affec0c1dab185a2039f025be81da79f4d Mon Sep 17 00:00:00 2001 From: jp39 Date: Tue, 16 Jun 2026 16:58:32 +0200 Subject: [PATCH 2/3] fix: scope remote cron command access Allow allowlisted remote channels to list, get, and update their own command cron jobs while preserving exact channel/chat ownership checks. Apply the same access control to remove, enable, and disable so remote callers cannot mutate jobs outside their channel/chat scope. Keep wildcard remote allowlist behavior constrained by exact job ownership. Add regression tests for allowlisted, non-allowlisted, wildcard, internal, and non-command reminder access paths. --- pkg/tools/cron.go | 40 ++++-- pkg/tools/cron_test.go | 312 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 342 insertions(+), 10 deletions(-) diff --git a/pkg/tools/cron.go b/pkg/tools/cron.go index 404dead7..a8ccdfdf 100644 --- a/pkg/tools/cron.go +++ b/pkg/tools/cron.go @@ -154,11 +154,11 @@ func (t *CronTool) Execute(ctx context.Context, args map[string]any) *ToolResult case "update": return t.updateJob(ctx, args) case "remove": - return t.removeJob(args) + return t.removeJob(ctx, args) case "enable": - return t.enableJob(args, true) + return t.enableJob(ctx, args, true) case "disable": - return t.enableJob(args, false) + return t.enableJob(ctx, args, false) default: return ErrorResult(fmt.Sprintf("unknown action: %s", action)) } @@ -377,12 +377,20 @@ func (t *CronTool) updateJob(ctx context.Context, args map[string]any) *ToolResu return SilentResult(fmt.Sprintf("Cron job updated:\n%s", formatCronJobJSON(updated))) } -func (t *CronTool) removeJob(args map[string]any) *ToolResult { +func (t *CronTool) removeJob(ctx context.Context, args map[string]any) *ToolResult { jobID, ok := args["job_id"].(string) if !ok || jobID == "" { return ErrorResult("job_id is required for remove") } + job, ok := t.cronService.GetJob(jobID) + if !ok { + return ErrorResult(fmt.Sprintf("Job %s not found", jobID)) + } + if !t.canAccessJob(ctx, job) { + return ErrorResult(fmt.Sprintf("Job %s is not accessible from this channel", jobID)) + } + if t.cronService.RemoveJob(jobID) { return SilentResult(fmt.Sprintf("Cron job removed: %s", jobID)) } @@ -534,14 +542,18 @@ func (t *CronTool) canAccessJob(ctx context.Context, job *cron.CronJob) bool { if constants.IsInternalChannel(channel) { return true } + chatID := ToolChatID(ctx) if channel == "" || chatID == "" { return false } - if job.Payload.Command != "" { + if job.Payload.Channel != channel || job.Payload.To != chatID { return false } - return job.Payload.Channel == channel && job.Payload.To == chatID + if job.Payload.Command != "" { + return isCommandAllowedRemote(channel, chatID, t.commandAllowedRemotes) + } + return true } func formatCronJobJSON(job *cron.CronJob) string { @@ -552,14 +564,22 @@ func formatCronJobJSON(job *cron.CronJob) string { return string(data) } -func (t *CronTool) enableJob(args map[string]any, enable bool) *ToolResult { +func (t *CronTool) enableJob(ctx context.Context, args map[string]any, enable bool) *ToolResult { jobID, ok := args["job_id"].(string) if !ok || jobID == "" { return ErrorResult("job_id is required for enable/disable") } - job := t.cronService.EnableJob(jobID, enable) - if job == nil { + job, ok := t.cronService.GetJob(jobID) + if !ok { + return ErrorResult(fmt.Sprintf("Job %s not found", jobID)) + } + if !t.canAccessJob(ctx, job) { + return ErrorResult(fmt.Sprintf("Job %s is not accessible from this channel", jobID)) + } + + updatedJob := t.cronService.EnableJob(jobID, enable) + if updatedJob == nil { return ErrorResult(fmt.Sprintf("Job %s not found", jobID)) } @@ -567,7 +587,7 @@ func (t *CronTool) enableJob(args map[string]any, enable bool) *ToolResult { if !enable { status = "disabled" } - return SilentResult(fmt.Sprintf("Cron job '%s' %s", job.Name, status)) + return SilentResult(fmt.Sprintf("Cron job '%s' %s", updatedJob.Name, status)) } // ExecuteJob executes a cron job through the agent diff --git a/pkg/tools/cron_test.go b/pkg/tools/cron_test.go index 7118ca28..7a9971d8 100644 --- a/pkg/tools/cron_test.go +++ b/pkg/tools/cron_test.go @@ -87,6 +87,28 @@ func parseCronJobResult(t *testing.T, result *ToolResult) cron.CronJob { return job } +func addTestCronJob(t *testing.T, tool *CronTool, name, channel, chatID, command string) *cron.CronJob { + t.Helper() + everyMS := int64(60_000) + job, err := tool.cronService.AddJob( + name, + cron.CronSchedule{Kind: "every", EveryMS: &everyMS}, + name+" message", + channel, + chatID, + ) + if err != nil { + t.Fatalf("AddJob() error: %v", err) + } + if command != "" { + job.Payload.Command = command + if err := tool.cronService.UpdateJob(job); err != nil { + t.Fatalf("UpdateJob() error: %v", err) + } + } + return job +} + // TestCronTool_CommandBlockedFromRemoteChannel verifies command scheduling is restricted by default. func TestCronTool_CommandBlockedFromRemoteChannel(t *testing.T) { tool := newTestCronTool(t) @@ -694,6 +716,296 @@ func TestCronTool_RemoteCannotAccessCommandJob(t *testing.T) { } } +func TestCronTool_AllowlistedRemoteCanAccessOwnCommandJob(t *testing.T) { + cfg := config.DefaultConfig() + cfg.Tools.Cron.CommandAllowedRemotes = []string{"telegram:chat-1"} + tool := newTestCronToolWithConfig(t, cfg) + job := addTestCronJob(t, tool, "command", "telegram", "chat-1", "df -h") + ctx := WithToolContext(context.Background(), "telegram", "chat-1") + + listResult := tool.Execute(ctx, map[string]any{"action": "list"}) + if listResult.IsError || !strings.Contains(listResult.ForLLM, job.ID) { + t.Fatalf("expected list to include own command job, got: %+v", listResult) + } + + getResult := tool.Execute(ctx, map[string]any{"action": "get", "job_id": job.ID}) + if getResult.IsError { + t.Fatalf("expected get to access own command job, got: %s", getResult.ForLLM) + } + got := parseCronJobResult(t, getResult) + if got.ID != job.ID || got.Payload.Command != "df -h" { + t.Fatalf("get returned wrong command job: %+v", got) + } + + updateResult := tool.Execute(ctx, map[string]any{ + "action": "update", + "job_id": job.ID, + "message": "updated command description", + }) + if updateResult.IsError { + t.Fatalf("expected update to access own command job, got: %s", updateResult.ForLLM) + } + updated, _ := tool.cronService.GetJob(job.ID) + if updated.Payload.Message != "updated command description" || updated.Payload.Command != "df -h" { + t.Fatalf("update returned wrong command payload: %+v", updated.Payload) + } +} + +func TestCronTool_AllowlistedRemoteCannotAccessOtherChatCommandJob(t *testing.T) { + cfg := config.DefaultConfig() + cfg.Tools.Cron.CommandAllowedRemotes = []string{"telegram"} + tool := newTestCronToolWithConfig(t, cfg) + job := addTestCronJob(t, tool, "command", "telegram", "chat-2", "df -h") + ctx := WithToolContext(context.Background(), "telegram", "chat-1") + + listResult := tool.Execute(ctx, map[string]any{"action": "list"}) + if listResult.IsError || strings.Contains(listResult.ForLLM, job.ID) { + t.Fatalf("expected list to hide other chat command job, got: %+v", listResult) + } + + for _, action := range []string{"get", "update"} { + args := map[string]any{"action": action, "job_id": job.ID} + if action == "update" { + args["message"] = "changed" + } + result := tool.Execute(ctx, args) + if !result.IsError || !strings.Contains(result.ForLLM, "not accessible") { + t.Fatalf("expected %s to reject other chat command job, got: %+v", action, result) + } + } +} + +func TestCronTool_NonAllowlistedRemoteCannotAccessOwnCommandJob(t *testing.T) { + tool := newTestCronTool(t) + job := addTestCronJob(t, tool, "command", "telegram", "chat-1", "df -h") + ctx := WithToolContext(context.Background(), "telegram", "chat-1") + + listResult := tool.Execute(ctx, map[string]any{"action": "list"}) + if listResult.IsError || strings.Contains(listResult.ForLLM, job.ID) { + t.Fatalf("expected list to hide non-allowlisted command job, got: %+v", listResult) + } + + for _, action := range []string{"get", "update"} { + args := map[string]any{"action": action, "job_id": job.ID} + if action == "update" { + args["message"] = "changed" + } + result := tool.Execute(ctx, args) + if !result.IsError || !strings.Contains(result.ForLLM, "not accessible") { + t.Fatalf("expected %s to reject non-allowlisted command job, got: %+v", action, result) + } + } +} + +func TestCronTool_WildcardRemoteCanAccessOwnCommandJob(t *testing.T) { + cfg := config.DefaultConfig() + cfg.Tools.Cron.CommandAllowedRemotes = []string{"*"} + tool := newTestCronToolWithConfig(t, cfg) + job := addTestCronJob(t, tool, "command", "telegram", "chat-1", "df -h") + other := addTestCronJob(t, tool, "other", "telegram", "chat-2", "uptime") + ctx := WithToolContext(context.Background(), "telegram", "chat-1") + + listResult := tool.Execute(ctx, map[string]any{"action": "list"}) + if listResult.IsError || !strings.Contains(listResult.ForLLM, job.ID) { + t.Fatalf("expected wildcard list to include own command job, got: %+v", listResult) + } + if strings.Contains(listResult.ForLLM, other.ID) { + t.Fatalf("wildcard list should still hide other chat job, got: %s", listResult.ForLLM) + } + + getResult := tool.Execute(ctx, map[string]any{"action": "get", "job_id": job.ID}) + if getResult.IsError { + t.Fatalf("expected wildcard get to access own command job, got: %s", getResult.ForLLM) + } +} + +func TestCronTool_InternalChannelCanAccessAllCommandJobs(t *testing.T) { + tool := newTestCronTool(t) + job := addTestCronJob(t, tool, "command", "telegram", "chat-1", "df -h") + ctx := WithToolContext(context.Background(), "cli", "direct") + + listResult := tool.Execute(ctx, map[string]any{"action": "list"}) + if listResult.IsError || !strings.Contains(listResult.ForLLM, job.ID) { + t.Fatalf("expected internal list to include command job, got: %+v", listResult) + } + + getResult := tool.Execute(ctx, map[string]any{"action": "get", "job_id": job.ID}) + if getResult.IsError { + t.Fatalf("expected internal get to access command job, got: %s", getResult.ForLLM) + } + + updateResult := tool.Execute(ctx, map[string]any{ + "action": "update", + "job_id": job.ID, + "message": "internal update", + }) + if updateResult.IsError { + t.Fatalf("expected internal update to access command job, got: %s", updateResult.ForLLM) + } +} + +func TestCronTool_AllowlistedRemoteCanManageOwnCommandJob(t *testing.T) { + for _, action := range []string{"remove", "enable", "disable"} { + t.Run(action, func(t *testing.T) { + cfg := config.DefaultConfig() + cfg.Tools.Cron.CommandAllowedRemotes = []string{"telegram:chat-1"} + tool := newTestCronToolWithConfig(t, cfg) + job := addTestCronJob(t, tool, "command", "telegram", "chat-1", "df -h") + if action == "enable" { + tool.cronService.EnableJob(job.ID, false) + } + ctx := WithToolContext(context.Background(), "telegram", "chat-1") + + result := tool.Execute(ctx, map[string]any{"action": action, "job_id": job.ID}) + if result.IsError { + t.Fatalf("expected %s to access own command job, got: %s", action, result.ForLLM) + } + + saved, ok := tool.cronService.GetJob(job.ID) + switch action { + case "remove": + if ok { + t.Fatalf("remove should delete own command job: %+v", saved) + } + case "enable": + if !ok || !saved.Enabled { + t.Fatalf("enable should enable own command job: %+v", saved) + } + case "disable": + if !ok || saved.Enabled { + t.Fatalf("disable should disable own command job: %+v", saved) + } + } + }) + } +} + +func TestCronTool_RemoteCannotManageOtherChatJob(t *testing.T) { + for _, action := range []string{"remove", "enable", "disable"} { + t.Run(action, func(t *testing.T) { + cfg := config.DefaultConfig() + cfg.Tools.Cron.CommandAllowedRemotes = []string{"telegram"} + tool := newTestCronToolWithConfig(t, cfg) + job := addTestCronJob(t, tool, "command", "telegram", "chat-2", "df -h") + ctx := WithToolContext(context.Background(), "telegram", "chat-1") + + result := tool.Execute(ctx, map[string]any{"action": action, "job_id": job.ID}) + if !result.IsError || !strings.Contains(result.ForLLM, "not accessible") { + t.Fatalf("expected %s to reject other chat job, got: %+v", action, result) + } + + saved, ok := tool.cronService.GetJob(job.ID) + if !ok { + t.Fatalf("%s should not remove other chat job", action) + } + if !saved.Enabled { + t.Fatalf("%s should not disable other chat job: %+v", action, saved) + } + }) + } +} + +func TestCronTool_RemoteCannotManageCommandJobUnlessAllowlisted(t *testing.T) { + for _, action := range []string{"remove", "enable", "disable"} { + t.Run(action, func(t *testing.T) { + tool := newTestCronTool(t) + job := addTestCronJob(t, tool, "command", "telegram", "chat-1", "df -h") + ctx := WithToolContext(context.Background(), "telegram", "chat-1") + + result := tool.Execute(ctx, map[string]any{"action": action, "job_id": job.ID}) + if !result.IsError || !strings.Contains(result.ForLLM, "not accessible") { + t.Fatalf("expected %s to reject non-allowlisted command job, got: %+v", action, result) + } + + saved, ok := tool.cronService.GetJob(job.ID) + if !ok { + t.Fatalf("%s should not remove non-allowlisted command job", action) + } + if !saved.Enabled { + t.Fatalf("%s should not disable non-allowlisted command job: %+v", action, saved) + } + }) + } +} + +func TestCronTool_InternalChannelCanManageAllJobs(t *testing.T) { + for _, action := range []string{"remove", "enable", "disable"} { + t.Run(action, func(t *testing.T) { + tool := newTestCronTool(t) + job := addTestCronJob(t, tool, "command", "telegram", "chat-1", "df -h") + if action == "enable" { + tool.cronService.EnableJob(job.ID, false) + } + ctx := WithToolContext(context.Background(), "cli", "direct") + + result := tool.Execute(ctx, map[string]any{"action": action, "job_id": job.ID}) + if result.IsError { + t.Fatalf("expected internal %s to access command job, got: %s", action, result.ForLLM) + } + + saved, ok := tool.cronService.GetJob(job.ID) + switch action { + case "remove": + if ok { + t.Fatalf("internal remove should delete command job: %+v", saved) + } + case "enable": + if !ok || !saved.Enabled { + t.Fatalf("internal enable should enable command job: %+v", saved) + } + case "disable": + if !ok || saved.Enabled { + t.Fatalf("internal disable should disable command job: %+v", saved) + } + } + }) + } +} + +func TestCronTool_RemoteCanManageOwnNonCommandJob(t *testing.T) { + for _, action := range []string{"remove", "enable", "disable"} { + t.Run(action, func(t *testing.T) { + tool := newTestCronTool(t) + job := addTestCronJob(t, tool, "reminder", "telegram", "chat-1", "") + if action == "enable" { + tool.cronService.EnableJob(job.ID, false) + } + ctx := WithToolContext(context.Background(), "telegram", "chat-1") + + result := tool.Execute(ctx, map[string]any{"action": action, "job_id": job.ID}) + if result.IsError { + t.Fatalf("expected %s to access own non-command job, got: %s", action, result.ForLLM) + } + }) + } +} + +func TestCronTool_WildcardRemoteCanManageOwnCommandJob(t *testing.T) { + for _, action := range []string{"remove", "enable", "disable"} { + t.Run(action, func(t *testing.T) { + cfg := config.DefaultConfig() + cfg.Tools.Cron.CommandAllowedRemotes = []string{"*"} + tool := newTestCronToolWithConfig(t, cfg) + job := addTestCronJob(t, tool, "command", "telegram", "chat-1", "df -h") + if action == "enable" { + tool.cronService.EnableJob(job.ID, false) + } + other := addTestCronJob(t, tool, "other", "telegram", "chat-2", "uptime") + ctx := WithToolContext(context.Background(), "telegram", "chat-1") + + result := tool.Execute(ctx, map[string]any{"action": action, "job_id": job.ID}) + if result.IsError { + t.Fatalf("expected wildcard %s to access own command job, got: %s", action, result.ForLLM) + } + + otherResult := tool.Execute(ctx, map[string]any{"action": action, "job_id": other.ID}) + if !otherResult.IsError || !strings.Contains(otherResult.ForLLM, "not accessible") { + t.Fatalf("wildcard %s should still reject other chat job, got: %+v", action, otherResult) + } + }) + } +} + func TestCronTool_CommandUpdateSafetyGates(t *testing.T) { t.Run("exec disabled", func(t *testing.T) { cfg := config.DefaultConfig() From 0c1e6b5279071dfad165709753111d85516d94b3 Mon Sep 17 00:00:00 2001 From: jp39 Date: Tue, 16 Jun 2026 17:02:25 +0200 Subject: [PATCH 3/3] docs: warn about cron remote wildcard Document that command_allowed_remotes = ["*"] is potentially dangerous because any remote channel that can talk to PicoClaw can schedule shell commands. Clarify that it should only be used when all enabled remote channels and chats are trusted. --- docs/guides/configuration.md | 2 +- docs/reference/cron.md | 4 ++++ docs/reference/tools_configuration.md | 2 +- 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/docs/guides/configuration.md b/docs/guides/configuration.md index c030206c..8ac86688 100644 --- a/docs/guides/configuration.md +++ b/docs/guides/configuration.md @@ -1164,7 +1164,7 @@ PicoClaw supports cron-style scheduled tasks via the `cron` tool. The agent can Scheduled tasks persist across restarts and are stored in `~/.picoclaw/workspace/cron/`. -Command cron jobs can execute shell commands. By default, remote channels cannot schedule command jobs. To allow specific remote channels, set `command_allowed_remotes` to entries such as `"telegram"` or `"telegram:1234567890"`; use `"*"` only if every non-empty channel should be allowed. This does not bypass `allow_command`, `command_confirm`, or exec safety checks. +Command cron jobs can execute shell commands. By default, remote channels cannot schedule command jobs. To allow specific remote channels, set `command_allowed_remotes` to entries such as `"telegram"` or `"telegram:1234567890"`; use `"*"` only if every non-empty channel should be allowed. The `"*"` wildcard is potentially dangerous because any remote channel that can talk to PicoClaw can schedule shell commands. This does not bypass `allow_command`, `command_confirm`, or exec safety checks. ### Advanced Topics diff --git a/docs/reference/cron.md b/docs/reference/cron.md index 1ac36a52..e421b37e 100644 --- a/docs/reference/cron.md +++ b/docs/reference/cron.md @@ -125,6 +125,10 @@ Entries can be either a channel name or a channel plus chat id: - `telegram:1234567890` allows command jobs only from that exact Telegram chat id. - `*` allows command jobs from every non-empty channel. +Warning: `*` is potentially dangerous because any remote channel that can talk +to PicoClaw can schedule shell commands. Use it only when every enabled remote +channel and chat is trusted to request command execution. + This setting only controls the remote-channel gate. It does not bypass `tools.cron.allow_command`, `command_confirm`, `tools.exec.enabled`, or the exec tool's command safety checks. Example: diff --git a/docs/reference/tools_configuration.md b/docs/reference/tools_configuration.md index 1be05b08..c3a9140b 100644 --- a/docs/reference/tools_configuration.md +++ b/docs/reference/tools_configuration.md @@ -315,7 +315,7 @@ The cron tool is used for scheduling periodic tasks. |---------------------------|----------|---------|----------------------------------------------------------------| | `enabled` | bool | true | Register the agent-facing cron tool | | `allow_command` | bool | true | Allow command jobs without extra confirmation | -| `command_allowed_remotes` | string[] | [] | Remote channels or `channel:chat_id` values allowed for command jobs; `*` allows every channel | +| `command_allowed_remotes` | string[] | [] | Remote channels or `channel:chat_id` values allowed for command jobs; `*` allows every channel and is dangerous unless all remote channels are trusted | | `exec_timeout_minutes` | int | 5 | Execution timeout in minutes, 0 means no limit | For schedule types, execution modes (`deliver`, agent turn, and command jobs), persistence, and the current command-security gates, see [Scheduled Tasks and Cron Jobs](cron.md).