From fcb69860c4807bbe1da4ef9144fc38fbcaf49fd9 Mon Sep 17 00:00:00 2001 From: wenjie Date: Tue, 17 Mar 2026 09:44:32 +0800 Subject: [PATCH] feat(web): add configurable cron command execution settings (#1647) - add tools.cron.allow_command config with a default value of true - require command_confirm only when cron command execution is disabled - expose cron command permission and timeout settings in the config UI - add backend tests and update i18n strings --- pkg/config/config.go | 5 +- pkg/config/config_test.go | 23 +++++++ pkg/config/defaults.go | 1 + pkg/tools/cron.go | 34 +++++++---- pkg/tools/cron_test.go | 61 +++++++++++++++++-- .../src/components/config/config-page.tsx | 12 ++++ .../src/components/config/config-sections.tsx | 36 +++++++++++ .../src/components/config/form-model.ts | 13 ++++ web/frontend/src/i18n/locales/en.json | 5 ++ web/frontend/src/i18n/locales/zh.json | 5 ++ 10 files changed, 174 insertions(+), 21 deletions(-) diff --git a/pkg/config/config.go b/pkg/config/config.go index 2937c36e4..ad5618907 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -699,8 +699,9 @@ type WebToolsConfig struct { } type CronToolsConfig struct { - ToolConfig ` envPrefix:"PICOCLAW_TOOLS_CRON_"` - ExecTimeoutMinutes int ` env:"PICOCLAW_TOOLS_CRON_EXEC_TIMEOUT_MINUTES" json:"exec_timeout_minutes"` // 0 means no timeout + ToolConfig ` envPrefix:"PICOCLAW_TOOLS_CRON_"` + ExecTimeoutMinutes int ` env:"PICOCLAW_TOOLS_CRON_EXEC_TIMEOUT_MINUTES" json:"exec_timeout_minutes"` // 0 means no timeout + AllowCommand bool ` env:"PICOCLAW_TOOLS_CRON_ALLOW_COMMAND" json:"allow_command"` } type ExecConfig struct { diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index 4c4dd9421..fc835f78f 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -405,6 +405,13 @@ func TestDefaultConfig_ExecAllowRemoteEnabled(t *testing.T) { } } +func TestDefaultConfig_CronAllowCommandEnabled(t *testing.T) { + cfg := DefaultConfig() + if !cfg.Tools.Cron.AllowCommand { + t.Fatal("DefaultConfig().Tools.Cron.AllowCommand should be true") + } +} + func TestLoadConfig_OpenAIWebSearchDefaultsTrueWhenUnset(t *testing.T) { dir := t.TempDir() configPath := filepath.Join(dir, "config.json") @@ -437,6 +444,22 @@ func TestLoadConfig_ExecAllowRemoteDefaultsTrueWhenUnset(t *testing.T) { } } +func TestLoadConfig_CronAllowCommandDefaultsTrueWhenUnset(t *testing.T) { + dir := t.TempDir() + configPath := filepath.Join(dir, "config.json") + if err := os.WriteFile(configPath, []byte(`{"tools":{"cron":{"exec_timeout_minutes":5}}}`), 0o600); err != nil { + t.Fatalf("WriteFile() error: %v", err) + } + + cfg, err := LoadConfig(configPath) + if err != nil { + t.Fatalf("LoadConfig() error: %v", err) + } + if !cfg.Tools.Cron.AllowCommand { + t.Fatal("tools.cron.allow_command should remain true when unset in config file") + } +} + func TestLoadConfig_OpenAIWebSearchCanBeDisabled(t *testing.T) { dir := t.TempDir() configPath := filepath.Join(dir, "config.json") diff --git a/pkg/config/defaults.go b/pkg/config/defaults.go index dc534d852..a029eeb59 100644 --- a/pkg/config/defaults.go +++ b/pkg/config/defaults.go @@ -452,6 +452,7 @@ func DefaultConfig() *Config { Enabled: true, }, ExecTimeoutMinutes: 5, + AllowCommand: true, }, Exec: ExecConfig{ ToolConfig: ToolConfig{ diff --git a/pkg/tools/cron.go b/pkg/tools/cron.go index 25608a54c..aa22f9aa6 100644 --- a/pkg/tools/cron.go +++ b/pkg/tools/cron.go @@ -20,10 +20,11 @@ type JobExecutor interface { // CronTool provides scheduling capabilities for the agent type CronTool struct { - cronService *cron.CronService - executor JobExecutor - msgBus *bus.MessageBus - execTool *ExecTool + cronService *cron.CronService + executor JobExecutor + msgBus *bus.MessageBus + execTool *ExecTool + allowCommand bool } // NewCronTool creates a new CronTool @@ -37,12 +38,18 @@ func NewCronTool( return nil, fmt.Errorf("unable to configure exec tool: %w", err) } + allowCommand := true + if config != nil { + allowCommand = config.Tools.Cron.AllowCommand + } + execTool.SetTimeout(execTimeout) return &CronTool{ - cronService: cronService, - executor: executor, - msgBus: msgBus, - execTool: execTool, + cronService: cronService, + executor: executor, + msgBus: msgBus, + execTool: execTool, + allowCommand: allowCommand, }, nil } @@ -76,7 +83,7 @@ func (t *CronTool) Parameters() map[string]any { }, "command_confirm": map[string]any{ "type": "boolean", - "description": "Required when using command=true. Must be true to explicitly confirm scheduling a shell command.", + "description": "Optional explicit confirmation flag for scheduling a shell command. Command execution must also be enabled via tools.cron.allow_command.", }, "at_seconds": map[string]any{ "type": "integer", @@ -180,16 +187,17 @@ func (t *CronTool) addJob(ctx context.Context, args map[string]any) *ToolResult deliver = d } - // GHSA-pv8c-p6jf-3fpp: command scheduling requires internal channel + explicit confirm. - // Non-command reminders (plain messages) remain open to all channels. + // GHSA-pv8c-p6jf-3fpp: command scheduling requires internal channel. When + // allow_command is disabled, explicit confirmation is required as an override. + // Non-command reminders remain open to all channels. command, _ := args["command"].(string) commandConfirm, _ := args["command_confirm"].(bool) if command != "" { if !constants.IsInternalChannel(channel) { return ErrorResult("scheduling command execution is restricted to internal channels") } - if !commandConfirm { - return ErrorResult("command_confirm=true is required to schedule command execution") + if !t.allowCommand && !commandConfirm { + return ErrorResult("command_confirm=true is required when allow_command is disabled") } deliver = false } diff --git a/pkg/tools/cron_test.go b/pkg/tools/cron_test.go index f1e857949..e46b13b13 100644 --- a/pkg/tools/cron_test.go +++ b/pkg/tools/cron_test.go @@ -11,12 +11,11 @@ import ( "github.com/sipeed/picoclaw/pkg/cron" ) -func newTestCronTool(t *testing.T) *CronTool { +func newTestCronToolWithConfig(t *testing.T, cfg *config.Config) *CronTool { t.Helper() storePath := filepath.Join(t.TempDir(), "cron.json") cronService := cron.NewCronService(storePath, nil) msgBus := bus.NewMessageBus() - cfg := config.DefaultConfig() tool, err := NewCronTool(cronService, nil, msgBus, t.TempDir(), true, 0, cfg) if err != nil { t.Fatalf("NewCronTool() error: %v", err) @@ -24,6 +23,11 @@ func newTestCronTool(t *testing.T) *CronTool { return tool } +func newTestCronTool(t *testing.T) *CronTool { + t.Helper() + return newTestCronToolWithConfig(t, config.DefaultConfig()) +} + // TestCronTool_CommandBlockedFromRemoteChannel verifies command scheduling is restricted to internal channels func TestCronTool_CommandBlockedFromRemoteChannel(t *testing.T) { tool := newTestCronTool(t) @@ -44,8 +48,7 @@ func TestCronTool_CommandBlockedFromRemoteChannel(t *testing.T) { } } -// TestCronTool_CommandRequiresConfirm verifies command_confirm=true is required -func TestCronTool_CommandRequiresConfirm(t *testing.T) { +func TestCronTool_CommandDoesNotRequireConfirmByDefault(t *testing.T) { tool := newTestCronTool(t) ctx := WithToolContext(context.Background(), "cli", "direct") result := tool.Execute(ctx, map[string]any{ @@ -55,11 +58,57 @@ func TestCronTool_CommandRequiresConfirm(t *testing.T) { "at_seconds": float64(60), }) + if result.IsError { + t.Fatalf("expected command scheduling without confirm to succeed by default, got: %s", result.ForLLM) + } + if !strings.Contains(result.ForLLM, "Cron job added") { + t.Errorf("expected 'Cron job added', got: %s", result.ForLLM) + } +} + +func TestCronTool_CommandRequiresConfirmWhenAllowCommandDisabled(t *testing.T) { + cfg := config.DefaultConfig() + cfg.Tools.Cron.AllowCommand = false + + tool := newTestCronToolWithConfig(t, cfg) + ctx := WithToolContext(context.Background(), "cli", "direct") + 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 error when command_confirm is missing") + t.Fatal("expected command scheduling to require confirm when allow_command is disabled") } if !strings.Contains(result.ForLLM, "command_confirm=true") { - t.Errorf("expected 'command_confirm=true' message, got: %s", result.ForLLM) + t.Errorf("expected command_confirm requirement message, got: %s", result.ForLLM) + } +} + +func TestCronTool_CommandAllowedWithConfirmWhenAllowCommandDisabled(t *testing.T) { + cfg := config.DefaultConfig() + cfg.Tools.Cron.AllowCommand = false + + tool := newTestCronToolWithConfig(t, cfg) + ctx := WithToolContext(context.Background(), "cli", "direct") + 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.Fatalf( + "expected command scheduling with confirm to succeed when allow_command is disabled, got: %s", + result.ForLLM, + ) + } + if !strings.Contains(result.ForLLM, "Cron job added") { + t.Errorf("expected 'Cron job added', got: %s", result.ForLLM) } } diff --git a/web/frontend/src/components/config/config-page.tsx b/web/frontend/src/components/config/config-page.tsx index cbce7d27e..130498ba4 100644 --- a/web/frontend/src/components/config/config-page.tsx +++ b/web/frontend/src/components/config/config-page.tsx @@ -14,6 +14,7 @@ import { } from "@/api/system" import { AgentDefaultsSection, + CronSection, DevicesSection, LauncherSection, RuntimeSection, @@ -164,6 +165,11 @@ export function ConfigPage() { "Heartbeat interval", { min: 1 }, ) + const cronExecTimeoutMinutes = parseIntField( + form.cronExecTimeoutMinutes, + "Cron exec timeout", + { min: 0 }, + ) await patchAppConfig({ agents: { @@ -180,6 +186,10 @@ export function ConfigPage() { dm_scope: dmScope, }, tools: { + cron: { + allow_command: form.allowCommand, + exec_timeout_minutes: cronExecTimeoutMinutes, + }, exec: { allow_remote: form.allowRemote, }, @@ -279,6 +289,8 @@ export function ConfigPage() { + + + onFieldChange("allowCommand", checked)} + /> + + + + onFieldChange("cronExecTimeoutMinutes", e.target.value) + } + /> + + + ) +} + interface LauncherSectionProps { launcherForm: LauncherForm onFieldChange: UpdateLauncherField diff --git a/web/frontend/src/components/config/form-model.ts b/web/frontend/src/components/config/form-model.ts index d868c4bb4..8c850b2c4 100644 --- a/web/frontend/src/components/config/form-model.ts +++ b/web/frontend/src/components/config/form-model.ts @@ -4,6 +4,8 @@ export interface CoreConfigForm { workspace: string restrictToWorkspace: boolean allowRemote: boolean + allowCommand: boolean + cronExecTimeoutMinutes: string maxTokens: string maxToolIterations: string summarizeMessageThreshold: string @@ -56,6 +58,8 @@ export const EMPTY_FORM: CoreConfigForm = { workspace: "", restrictToWorkspace: true, allowRemote: true, + allowCommand: true, + cronExecTimeoutMinutes: "5", maxTokens: "32768", maxToolIterations: "50", summarizeMessageThreshold: "20", @@ -106,6 +110,7 @@ export function buildFormFromConfig(config: unknown): CoreConfigForm { const heartbeat = asRecord(root.heartbeat) const devices = asRecord(root.devices) const tools = asRecord(root.tools) + const cron = asRecord(tools.cron) const exec = asRecord(tools.exec) return { @@ -118,6 +123,14 @@ export function buildFormFromConfig(config: unknown): CoreConfigForm { exec.allow_remote === undefined ? EMPTY_FORM.allowRemote : asBool(exec.allow_remote), + allowCommand: + cron.allow_command === undefined + ? EMPTY_FORM.allowCommand + : asBool(cron.allow_command), + cronExecTimeoutMinutes: asNumberString( + cron.exec_timeout_minutes, + EMPTY_FORM.cronExecTimeoutMinutes, + ), maxTokens: asNumberString(defaults.max_tokens, EMPTY_FORM.maxTokens), maxToolIterations: asNumberString( defaults.max_tool_iterations, diff --git a/web/frontend/src/i18n/locales/en.json b/web/frontend/src/i18n/locales/en.json index b099dec13..2fa32ebb5 100644 --- a/web/frontend/src/i18n/locales/en.json +++ b/web/frontend/src/i18n/locales/en.json @@ -394,6 +394,10 @@ "restrict_workspace_hint": "Only allow file operations inside workspace.", "allow_remote": "Allow Remote Shell Execution", "allow_remote_hint": "When enabled, shell commands can also run for remote sessions or non-local contexts. When disabled, shell execution stays limited to local safe contexts.", + "allow_shell_execution": "Allow Shell Execution", + "allow_shell_execution_hint": "Enable scheduled shell commands for cron jobs by default. When disabled, users must pass command_confirm=true to schedule a cron command.", + "cron_exec_timeout": "Cron Command Timeout (minutes)", + "cron_exec_timeout_hint": "Maximum runtime for scheduled shell commands. Set to 0 to disable the timeout.", "max_tokens": "Max Tokens", "max_tokens_hint": "Upper token limit per model response.", "max_tool_iterations": "Max Tool Iterations", @@ -434,6 +438,7 @@ "sections": { "agent": "Agent", "runtime": "Runtime", + "cron": "Cron Tasks", "launcher": "Service", "devices": "Devices" }, diff --git a/web/frontend/src/i18n/locales/zh.json b/web/frontend/src/i18n/locales/zh.json index 78093e5c7..badf5bb3d 100644 --- a/web/frontend/src/i18n/locales/zh.json +++ b/web/frontend/src/i18n/locales/zh.json @@ -394,6 +394,10 @@ "restrict_workspace_hint": "仅允许在工作目录内执行文件操作。", "allow_remote": "允许远程执行 Shell 命令", "allow_remote_hint": "开启后，来自远程会话或非本地上下文的请求也可以执行 shell 命令；关闭后，仅允许本地安全上下文执行。", + "allow_shell_execution": "允许 Shell 执行", + "allow_shell_execution_hint": "开启后，cron 定时任务默认允许执行 shell 命令。关闭后，必须显式传入 command_confirm=true 才能创建 cron 命令任务。", + "cron_exec_timeout": "定时命令超时（分钟）", + "cron_exec_timeout_hint": "定时 shell 命令的最长执行时间。设置为 0 表示不限制超时。", "max_tokens": "最大 Token 数", "max_tokens_hint": "单次模型响应允许的最大 Token 数。", "max_tool_iterations": "最大工具迭代次数", @@ -434,6 +438,7 @@ "sections": { "agent": "智能体", "runtime": "运行时", + "cron": "定时任务", "launcher": "服务参数", "devices": "设备" },