feat: 🎸 add allow patterns to config file#377
feat: 🎸 add allow patterns to config file#377candywater wants to merge 2 commits intosipeed:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds configurable allow patterns for the ExecTool to provide fine-grained control over which shell commands the agent can execute. This enhances security by allowing administrators to restrict the agent to only execute specific whitelisted commands while still maintaining control. The feature addresses issue #373.
Changes:
- Added
allow_patternsconfiguration field to agent defaults, supporting regex-based command whitelisting - Updated ExecTool and CronTool constructors to accept and compile allow patterns
- Added migration logic to preserve allow_patterns during config conversions
- Added comprehensive documentation in English, Chinese, and Japanese READMEs
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/tools/shell.go | Modified NewExecTool to accept optional allowPatterns parameter and compile them into regex |
| pkg/tools/cron.go | Updated NewCronTool to forward allowPatterns to ExecTool |
| pkg/config/config.go | Added AllowPatterns field to AgentDefaults struct with JSON and env tag support |
| pkg/agent/loop.go | Passed configured allow patterns from config to ExecTool during tool registry creation |
| cmd/picoclaw/main.go | Updated setupCronTool signature and call to include allowPatterns from config |
| pkg/migrate/config.go | Added logic to convert and merge allow_patterns during config migration |
| pkg/migrate/migrate_test.go | Added tests for allow_patterns conversion and merging behavior |
| config/config.example.json | Added allow_patterns example and fixed missing provider/duckduckgo fields |
| README.md | Added Security Sandbox section documenting allow_patterns configuration and usage |
| README.zh.md | Added Chinese documentation for Security Sandbox with allow_patterns |
| README.ja.md | Added Japanese documentation for Security Sandbox with allow_patterns |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if len(allowPatterns) > 0 { | ||
| allowRegex = make([]*regexp.Regexp, 0, len(allowPatterns)) | ||
| for _, p := range allowPatterns { | ||
| allowRegex = append(allowRegex, regexp.MustCompile(p)) |
There was a problem hiding this comment.
Invalid regex patterns in allowPatterns will cause a panic when regexp.MustCompile is called with an invalid pattern. The existing SetAllowPatterns method (lines 230-240) handles this properly by returning an error, but the constructor does not. Consider handling compilation errors gracefully, either by logging and skipping invalid patterns or returning an error from the constructor.
| allowRegex = append(allowRegex, regexp.MustCompile(p)) | |
| re, err := regexp.Compile(p) | |
| if err != nil { | |
| fmt.Fprintf(os.Stderr, "NewExecTool: invalid allow pattern %q: %v\n", p, err) | |
| continue | |
| } | |
| allowRegex = append(allowRegex, re) |
| var allowRegex []*regexp.Regexp | ||
| if len(allowPatterns) > 0 { | ||
| allowRegex = make([]*regexp.Regexp, 0, len(allowPatterns)) | ||
| for _, p := range allowPatterns { | ||
| allowRegex = append(allowRegex, regexp.MustCompile(p)) | ||
| } | ||
| } |
There was a problem hiding this comment.
The allow_patterns feature lacks test coverage in pkg/tools/shell_test.go. The existing test file has comprehensive tests for other shell tool features (timeout, workspace restriction, dangerous commands, etc.), but there are no tests verifying that allow_patterns work correctly. Consider adding tests for: (1) commands matching allowlist patterns should execute, (2) commands not matching any allowlist pattern should be blocked, and (3) empty allowlist should allow all commands.
| ### 心跳 / 周期性任务 (Heartbeat) | ||
|
|
||
| ### 🔒 安全沙箱 (Security Sandbox) | ||
|
|
||
| 默认情况下，PicoClaw 在沙箱中运行，Agent 仅能在工作区内访问文件与执行命令。 | ||
|
|
||
| ```json | ||
| { | ||
| "agents": { | ||
| "defaults": { | ||
| "workspace": "~/.picoclaw/workspace", | ||
| "restrict_to_workspace": true, | ||
| "allow_patterns": [] | ||
| } | ||
| } | ||
| } | ||
| ``` | ||
|
|
||
| | 选项 | 默认值 | 描述 | | ||
| | --- | --- | --- | | ||
| | `workspace` | `~/.picoclaw/workspace` | Agent 工作目录 | | ||
| | `restrict_to_workspace` | `true` | 限制文件/命令访问仅在工作区内 | | ||
| | `allow_patterns` | `[]` | 可选：对 `exec` 命令内容做正则白名单匹配；设置后命令必须命中至少一条规则 | | ||
|
|
||
| `allow_patterns` 会对完整命令进行匹配（不区分大小写）。为空数组时，不启用白名单过滤。 | ||
|
|
||
| 示例： | ||
|
|
||
| ```json | ||
| { | ||
| "agents": { | ||
| "defaults": { | ||
| "allow_patterns": [ | ||
| "^ls(\\s|$)", | ||
| "^pwd$", | ||
| "^cat\\s+README\\.md$" | ||
| ] | ||
| } | ||
| } | ||
| } | ||
| ``` | ||
|
|
||
| > ⚠️ 说明：`allow_patterns` 只影响 `exec` 工具；仍受 `restrict_to_workspace` 约束。 | ||
|
|
There was a problem hiding this comment.
The Security Sandbox section is inserted between the Heartbeat heading (line 464) and its content (line 508), which breaks the document structure. The Heartbeat section starts at line 464 but its actual content doesn't begin until line 508. This makes the documentation confusing. Consider moving the Security Sandbox section to appear before or after the complete Heartbeat section.
1 participant
📝 Description
🗣️ Type of Change
🤖 AI Code Generation
🔗 Linked Issue
📚 Technical Context (Skip for Docs)
PS: should set
restrict_to_workspace= false , otherwise it would get erroreg.
🧪 Test Environment & Hardware
📸 Proof of Work (Optional for Docs)
log
☑️ Checklist