This commit is contained in:
Anton Bogdanovich 2026-05-15 11:29:56 +03:00 committed by GitHub
commit fbdda146dc
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
12 changed files with 586 additions and 102 deletions

View file

@ -245,7 +245,7 @@ For more complete routing and model-tier examples, see the [Routing Guide](routi
Per-agent tool declarations live in `AGENT.md` frontmatter, not in `config.json`. Per-agent tool declarations live in `AGENT.md` frontmatter, not in `config.json`.
If `tools` is omitted from frontmatter, the agent gets the normal globally enabled tool set. If `tools` is present, PicoClaw registers only the listed runtime tools for that agent. If `tools` is omitted from frontmatter, the agent gets the normal globally enabled tool set. If `tools` is present, PicoClaw applies the declared tool policy during registration.
```md ```md
--- ---
@ -261,11 +261,37 @@ You are the research agent.
Notes: Notes:
- This is an allowlist, not a preference hint. - List form is shorthand for an allow policy.
- Tool names are matched against the runtime tool name 1:1. - Tool and MCP server names can be declared either as an exact-name list or as an `allow` / `deny` policy object.
- Pattern matching uses shell-style globs against the runtime tool name or MCP server name.
- Use runtime tool names such as `web_search`, `web_fetch`, `spawn`, `subagent`, `send_file`. - Use runtime tool names such as `web_search`, `web_fetch`, `spawn`, `subagent`, `send_file`.
- Tool declarations in `AGENT.md` are used by runtime/tooling, but they are not injected into the discovery prompt. - Tool declarations in `AGENT.md` are used by runtime/tooling, but they are not injected into the discovery prompt.
Examples:
```md
---
tools:
allow:
- mcp_*
- web_fetch
deny:
- mcp_gpt_researcher_*
mcpServers:
allow:
- github
- filesystem
---
```
Policy rules:
- omitted field: no frontmatter restriction
- list form: allowlist shorthand
- `deny` is applied after `allow`
- `deny` wins on overlap
- explicit empty list blocks all values for that field
### Agent Discovery (Automatic) ### Agent Discovery (Automatic)
When an agent has spawnable peers and can call `spawn`, PicoClaw injects a structured agent registry into that agent's system prompt on every turn. No extra `list_agents` tool call is required. When an agent has spawnable peers and can call `spawn`, PicoClaw injects a structured agent registry into that agent's system prompt on every turn. No extra `list_agents` tool call is required.

View file

@ -130,7 +130,7 @@ func registerSharedTools(
if err != nil { if err != nil {
logger.ErrorCF("agent", "Failed to create web search tool", map[string]any{"error": err.Error()}) logger.ErrorCF("agent", "Failed to create web search tool", map[string]any{"error": err.Error()})
} else if searchTool != nil { } else if searchTool != nil {
agent.Tools.Register(searchTool) registerToolIfAllowed(agent, searchTool)
} }
} }
if cfg.Tools.IsToolEnabled("web_fetch") { if cfg.Tools.IsToolEnabled("web_fetch") {
@ -143,19 +143,19 @@ func registerSharedTools(
if err != nil { if err != nil {
logger.ErrorCF("agent", "Failed to create web fetch tool", map[string]any{"error": err.Error()}) logger.ErrorCF("agent", "Failed to create web fetch tool", map[string]any{"error": err.Error()})
} else { } else {
agent.Tools.Register(fetchTool) registerToolIfAllowed(agent, fetchTool)
} }
} }
// Hardware tools (I2C, SPI) - Linux only, returns error on other platforms // Hardware tools (I2C, SPI) - Linux only, returns error on other platforms
if cfg.Tools.IsToolEnabled("i2c") { if cfg.Tools.IsToolEnabled("i2c") {
agent.Tools.Register(tools.NewI2CTool()) registerToolIfAllowed(agent, tools.NewI2CTool())
} }
if cfg.Tools.IsToolEnabled("spi") { if cfg.Tools.IsToolEnabled("spi") {
agent.Tools.Register(tools.NewSPITool()) registerToolIfAllowed(agent, tools.NewSPITool())
} }
if cfg.Tools.IsToolEnabled("serial") { if cfg.Tools.IsToolEnabled("serial") {
agent.Tools.Register(tools.NewSerialTool()) registerToolIfAllowed(agent, tools.NewSerialTool())
} }
// Message tool // Message tool
@ -182,7 +182,7 @@ func registerSharedTools(
ReplyToMessageID: replyToMessageID, ReplyToMessageID: replyToMessageID,
}) })
}) })
agent.Tools.Register(messageTool) registerToolIfAllowed(agent, messageTool)
} }
if cfg.Tools.IsToolEnabled("reaction") { if cfg.Tools.IsToolEnabled("reaction") {
reactionTool := tools.NewReactionTool() reactionTool := tools.NewReactionTool()
@ -201,7 +201,7 @@ func registerSharedTools(
_, err := rc.ReactToMessage(ctx, chatID, messageID) _, err := rc.ReactToMessage(ctx, chatID, messageID)
return err return err
}) })
agent.Tools.Register(reactionTool) registerToolIfAllowed(agent, reactionTool)
} }
// Send file tool (outbound media via MediaStore — store injected later by SetMediaStore) // Send file tool (outbound media via MediaStore — store injected later by SetMediaStore)
@ -213,11 +213,11 @@ func registerSharedTools(
nil, nil,
allowReadPaths, allowReadPaths,
) )
agent.Tools.Register(sendFileTool) registerToolIfAllowed(agent, sendFileTool)
} }
if ttsProvider != nil { if ttsProvider != nil {
agent.Tools.Register(tools.NewSendTTSTool(ttsProvider, nil)) registerToolIfAllowed(agent, tools.NewSendTTSTool(ttsProvider, nil))
} }
if cfg.Tools.IsToolEnabled("load_image") { if cfg.Tools.IsToolEnabled("load_image") {
@ -228,7 +228,7 @@ func registerSharedTools(
nil, nil,
allowReadPaths, allowReadPaths,
) )
agent.Tools.Register(loadImageTool) registerToolIfAllowed(agent, loadImageTool)
} }
// Skill discovery and installation tools // Skill discovery and installation tools
@ -243,11 +243,11 @@ func registerSharedTools(
cfg.Tools.Skills.SearchCache.MaxSize, cfg.Tools.Skills.SearchCache.MaxSize,
time.Duration(cfg.Tools.Skills.SearchCache.TTLSeconds)*time.Second, time.Duration(cfg.Tools.Skills.SearchCache.TTLSeconds)*time.Second,
) )
agent.Tools.Register(tools.NewFindSkillsTool(registryMgr, searchCache)) registerToolIfAllowed(agent, tools.NewFindSkillsTool(registryMgr, searchCache))
} }
if install_skills_enable { if install_skills_enable {
agent.Tools.Register(tools.NewInstallSkillTool(registryMgr, agent.Workspace)) registerToolIfAllowed(agent, tools.NewInstallSkillTool(registryMgr, agent.Workspace))
} }
} }
@ -340,15 +340,15 @@ func registerSharedTools(
return registry.CanSpawnSubagent(currentAgentID, targetAgentID) return registry.CanSpawnSubagent(currentAgentID, targetAgentID)
}) })
agent.Tools.Register(spawnTool) registerToolIfAllowed(agent, spawnTool)
// Also register the synchronous subagent tool // Also register the synchronous subagent tool
subagentTool := tools.NewSubagentTool(subagentManager) subagentTool := tools.NewSubagentTool(subagentManager)
subagentTool.SetSpawner(NewSubTurnSpawner(al)) subagentTool.SetSpawner(NewSubTurnSpawner(al))
agent.Tools.Register(subagentTool) registerToolIfAllowed(agent, subagentTool)
} }
if spawnStatusEnabled { if spawnStatusEnabled {
agent.Tools.Register(tools.NewSpawnStatusTool(subagentManager)) registerToolIfAllowed(agent, tools.NewSpawnStatusTool(subagentManager))
} }
} else if (spawnEnabled || spawnStatusEnabled) && !cfg.Tools.IsToolEnabled("subagent") { } else if (spawnEnabled || spawnStatusEnabled) && !cfg.Tools.IsToolEnabled("subagent") {
logger.WarnCF("agent", "spawn/spawn_status tools require subagent to be enabled", nil) logger.WarnCF("agent", "spawn/spawn_status tools require subagent to be enabled", nil)
@ -366,7 +366,7 @@ func registerSharedTools(
delegateTool.SetAllowlistChecker(func(targetAgentID string) bool { delegateTool.SetAllowlistChecker(func(targetAgentID string) bool {
return registry.CanSpawnSubagent(currentAgentID, targetAgentID) return registry.CanSpawnSubagent(currentAgentID, targetAgentID)
}) })
agent.Tools.Register(delegateTool) registerToolIfAllowed(agent, delegateTool)
} }
warnOnUnknownAgentToolDeclarations(agentID, agent.Workspace, agent.Definition, agent.Tools) warnOnUnknownAgentToolDeclarations(agentID, agent.Workspace, agent.Definition, agent.Tools)

View file

@ -168,12 +168,13 @@ func (al *AgentLoop) ensureMCPInitialized(ctx context.Context) error {
mcpTool.SetMaxInlineTextRunes(al.cfg.Tools.MCP.GetMaxInlineTextChars()) mcpTool.SetMaxInlineTextRunes(al.cfg.Tools.MCP.GetMaxInlineTextChars())
mcpTool.SetEventPublisher(al.runtimeEvents) mcpTool.SetEventPublisher(al.runtimeEvents)
var registered bool
if registerAsHidden { if registerAsHidden {
agent.Tools.RegisterHidden(mcpTool) registered = registerHiddenToolIfAllowed(agent, mcpTool)
} else { } else {
agent.Tools.Register(mcpTool) registered = registerToolIfAllowed(agent, mcpTool)
} }
if !toolRegistryIncludes(agent.Tools, toolName) { if !registered || !toolRegistryIncludes(agent.Tools, toolName) {
continue continue
} }
@ -250,7 +251,7 @@ func (al *AgentLoop) ensureMCPInitialized(ctx context.Context) error {
if !ok { if !ok {
continue continue
} }
if !agentHasDiscoverableMCPServers(al.cfg, agent.MCPServerAllowlist) { if !agentHasDiscoverableMCPServers(al.cfg, agent.MCPServerPolicy) {
continue continue
} }
@ -337,12 +338,20 @@ func filterMCPConfigServers(
return filtered return filtered
} }
func agentHasDiscoverableMCPServers(cfg *config.Config, allowed map[string]struct{}) bool { func agentHasDiscoverableMCPServers(cfg *config.Config, allowed *PatternPolicy) bool {
if cfg == nil || !cfg.Tools.MCP.Enabled || !cfg.Tools.MCP.Discovery.Enabled { if cfg == nil || !cfg.Tools.MCP.Enabled || !cfg.Tools.MCP.Discovery.Enabled {
return false return false
} }
filtered := filterMCPConfigServers(cfg.Tools.MCP, allowed) filtered := cfg.Tools.MCP
if allowed != nil {
filtered.Servers = make(map[string]config.MCPServerConfig)
for serverName, serverCfg := range cfg.Tools.MCP.Servers {
if toolAllowedByPolicy(allowed, normalizeMCPServerName(serverName)) {
filtered.Servers[serverName] = serverCfg
}
}
}
for _, serverCfg := range filtered.Servers { for _, serverCfg := range filtered.Servers {
if serverCfg.Enabled && serverIsDeferred(cfg.Tools.MCP.Discovery.Enabled, serverCfg) { if serverCfg.Enabled && serverIsDeferred(cfg.Tools.MCP.Discovery.Enabled, serverCfg) {
return true return true

View file

@ -225,7 +225,7 @@ func TestAgentHasDiscoverableMCPServers(t *testing.T) {
tests := []struct { tests := []struct {
name string name string
allowed map[string]struct{} allowed *PatternPolicy
want bool want bool
}{ }{
{ {
@ -234,29 +234,28 @@ func TestAgentHasDiscoverableMCPServers(t *testing.T) {
}, },
{ {
name: "empty allowlist denies all servers", name: "empty allowlist denies all servers",
allowed: map[string]struct{}{}, allowed: &PatternPolicy{Allow: []string{}, form: patternPolicyFormList},
want: false, want: false,
}, },
{ {
name: "selected server discoverable", name: "selected server discoverable",
allowed: map[string]struct{}{ allowed: &PatternPolicy{Allow: []string{"github"}, form: patternPolicyFormList},
"github": {}, want: true,
},
want: true,
}, },
{ {
name: "selected server opted out of discovery", name: "selected server opted out of discovery",
allowed: map[string]struct{}{ allowed: &PatternPolicy{Allow: []string{"filesystem"}, form: patternPolicyFormList},
"filesystem": {}, want: false,
},
want: false,
}, },
{ {
name: "unknown allowlist server matches nothing", name: "unknown allowlist server matches nothing",
allowed: map[string]struct{}{ allowed: &PatternPolicy{Allow: []string{"slack"}, form: patternPolicyFormList},
"slack": {}, want: false,
}, },
want: false, {
name: "deny-only policy can exclude discoverable server",
allowed: &PatternPolicy{Deny: []string{"github"}, form: patternPolicyFormObject},
want: false,
}, },
} }

View file

@ -1,6 +1,7 @@
package agent package agent
import ( import (
"fmt"
"os" "os"
"path/filepath" "path/filepath"
"slices" "slices"
@ -31,13 +32,29 @@ type AgentFrontmatter struct {
Name string `json:"name"` Name string `json:"name"`
Description string `json:"description"` Description string `json:"description"`
Tools []string `json:"tools,omitempty"` Tools []string `json:"tools,omitempty"`
ToolPolicy *PatternPolicy `json:"-"`
Model string `json:"model,omitempty"` Model string `json:"model,omitempty"`
MaxTurns *int `json:"maxTurns,omitempty"` MaxTurns *int `json:"maxTurns,omitempty"`
Skills []string `json:"skills,omitempty"` Skills []string `json:"skills,omitempty"`
MCPServers []string `json:"mcpServers,omitempty"` MCPServers []string `json:"mcpServers,omitempty"`
MCPPolicy *PatternPolicy `json:"-"`
Fields map[string]any `json:"-"` Fields map[string]any `json:"-"`
} }
type PatternPolicy struct {
Allow []string
Deny []string
form patternPolicyForm
}
type patternPolicyForm int
const (
patternPolicyFormUnset patternPolicyForm = iota
patternPolicyFormList
patternPolicyFormObject
)
// AgentPromptDefinition represents the parsed AGENT.md or AGENTS.md prompt file. // AgentPromptDefinition represents the parsed AGENT.md or AGENTS.md prompt file.
type AgentPromptDefinition struct { type AgentPromptDefinition struct {
Path string `json:"path"` Path string `json:"path"`
@ -176,11 +193,9 @@ func parseAgentFrontmatter(path, frontmatter string) (AgentFrontmatter, error) {
var typed struct { var typed struct {
Name string `yaml:"name"` Name string `yaml:"name"`
Description string `yaml:"description"` Description string `yaml:"description"`
Tools []string `yaml:"tools"`
Model string `yaml:"model"` Model string `yaml:"model"`
MaxTurns *int `yaml:"maxTurns"` MaxTurns *int `yaml:"maxTurns"`
Skills []string `yaml:"skills"` Skills []string `yaml:"skills"`
MCPServers []string `yaml:"mcpServers"`
} }
if err := yaml.Unmarshal([]byte(frontmatter), &typed); err != nil { if err := yaml.Unmarshal([]byte(frontmatter), &typed); err != nil {
logger.WarnCF("agent", "Failed to decode AGENT.md frontmatter fields", map[string]any{ logger.WarnCF("agent", "Failed to decode AGENT.md frontmatter fields", map[string]any{
@ -190,18 +205,125 @@ func parseAgentFrontmatter(path, frontmatter string) (AgentFrontmatter, error) {
return AgentFrontmatter{}, err return AgentFrontmatter{}, err
} }
var (
toolPolicy *PatternPolicy
mcpPolicy *PatternPolicy
err error
)
if raw, ok := rawFields["tools"]; ok {
toolPolicy, err = parsePatternPolicy(raw, "tools")
if err != nil {
logger.WarnCF("agent", "Failed to decode AGENT.md tools policy", map[string]any{
"path": path,
"error": err.Error(),
})
return AgentFrontmatter{}, err
}
}
if raw, ok := rawFields["mcpServers"]; ok {
mcpPolicy, err = parsePatternPolicy(raw, "mcpServers")
if err != nil {
logger.WarnCF("agent", "Failed to decode AGENT.md mcpServers policy", map[string]any{
"path": path,
"error": err.Error(),
})
return AgentFrontmatter{}, err
}
}
return AgentFrontmatter{ return AgentFrontmatter{
Name: strings.TrimSpace(typed.Name), Name: strings.TrimSpace(typed.Name),
Description: strings.TrimSpace(typed.Description), Description: strings.TrimSpace(typed.Description),
Tools: append([]string(nil), typed.Tools...), Tools: policyAllowPatterns(toolPolicy),
ToolPolicy: toolPolicy,
Model: strings.TrimSpace(typed.Model), Model: strings.TrimSpace(typed.Model),
MaxTurns: typed.MaxTurns, MaxTurns: typed.MaxTurns,
Skills: append([]string(nil), typed.Skills...), Skills: append([]string(nil), typed.Skills...),
MCPServers: append([]string(nil), typed.MCPServers...), MCPServers: policyAllowPatterns(mcpPolicy),
MCPPolicy: mcpPolicy,
Fields: rawFields, Fields: rawFields,
}, nil }, nil
} }
func parsePatternPolicy(raw any, field string) (*PatternPolicy, error) {
if raw == nil {
return &PatternPolicy{Allow: []string{}, form: patternPolicyFormList}, nil
}
switch value := raw.(type) {
case []any:
allow, err := parsePatternList(value, field)
if err != nil {
return nil, err
}
return &PatternPolicy{Allow: allow, form: patternPolicyFormList}, nil
case map[string]any:
allow, err := parseNamedPatternList(value, field, "allow")
if err != nil {
return nil, err
}
deny, err := parseNamedPatternList(value, field, "deny")
if err != nil {
return nil, err
}
return &PatternPolicy{Allow: allow, Deny: deny, form: patternPolicyFormObject}, nil
case string:
trimmed := strings.TrimSpace(value)
if trimmed == "" {
return &PatternPolicy{Allow: []string{}, form: patternPolicyFormList}, nil
}
return &PatternPolicy{Allow: []string{trimmed}, form: patternPolicyFormList}, nil
default:
return nil, fmt.Errorf("%s must be a list, string, or object with allow/deny", field)
}
}
func parseNamedPatternList(raw map[string]any, field, key string) ([]string, error) {
value, ok := raw[key]
if !ok {
return nil, nil
}
if value == nil {
return []string{}, nil
}
switch list := value.(type) {
case []any:
return parsePatternList(list, field+"."+key)
case string:
trimmed := strings.TrimSpace(list)
if trimmed == "" {
return []string{}, nil
}
return []string{trimmed}, nil
default:
return nil, fmt.Errorf("%s.%s must be a list or string", field, key)
}
}
func parsePatternList(values []any, field string) ([]string, error) {
result := make([]string, 0, len(values))
for _, raw := range values {
s, ok := raw.(string)
if !ok {
return nil, fmt.Errorf("%s entries must be strings", field)
}
trimmed := strings.TrimSpace(s)
if trimmed == "" {
continue
}
result = append(result, trimmed)
}
return result, nil
}
func policyAllowPatterns(policy *PatternPolicy) []string {
if policy == nil {
return nil
}
return append([]string(nil), policy.Allow...)
}
func splitAgentFrontmatter(content string) (frontmatter, body string) { func splitAgentFrontmatter(content string) (frontmatter, body string) {
normalized := string(parser.NormalizeNewlines([]byte(content))) normalized := string(parser.NormalizeNewlines([]byte(content)))
lines := strings.Split(normalized, "\n") lines := strings.Split(normalized, "\n")

View file

@ -79,6 +79,51 @@ Act directly and use tools first.
} }
} }
func TestLoadAgentDefinitionParsesPatternPolicies(t *testing.T) {
tmpDir := setupWorkspace(t, map[string]string{
"AGENT.md": `---
tools:
allow:
- mcp_*
- web_fetch
deny:
- mcp_gpt_researcher_*
mcpServers:
allow:
- git*
deny:
- github-legacy
---
# Agent
`,
})
defer cleanupWorkspace(t, tmpDir)
definition := NewContextBuilder(tmpDir).LoadAgentDefinition()
if definition.Agent == nil {
t.Fatal("expected AGENT.md definition to be loaded")
}
if definition.Agent.Frontmatter.ToolPolicy == nil {
t.Fatal("expected tool policy to be parsed")
}
if got := definition.Agent.Frontmatter.ToolPolicy.Allow; len(got) != 2 ||
got[0] != "mcp_*" || got[1] != "web_fetch" {
t.Fatalf("tool allow = %v", got)
}
if got := definition.Agent.Frontmatter.ToolPolicy.Deny; len(got) != 1 || got[0] != "mcp_gpt_researcher_*" {
t.Fatalf("tool deny = %v", got)
}
if definition.Agent.Frontmatter.MCPPolicy == nil {
t.Fatal("expected mcp policy to be parsed")
}
if got := definition.Agent.Frontmatter.MCPPolicy.Allow; len(got) != 1 || got[0] != "git*" {
t.Fatalf("mcp allow = %v", got)
}
if got := definition.Agent.Frontmatter.MCPPolicy.Deny; len(got) != 1 || got[0] != "github-legacy" {
t.Fatalf("mcp deny = %v", got)
}
}
func TestLoadAgentDefinitionFallsBackToLegacyAgentsMarkdown(t *testing.T) { func TestLoadAgentDefinitionFallsBackToLegacyAgentsMarkdown(t *testing.T) {
tmpDir := setupWorkspace(t, map[string]string{ tmpDir := setupWorkspace(t, map[string]string{
"AGENTS.md": "# Legacy Agent\nKeep compatibility.", "AGENTS.md": "# Legacy Agent\nKeep compatibility.",

View file

@ -41,7 +41,8 @@ type AgentInstance struct {
Definition AgentContextDefinition Definition AgentContextDefinition
Subagents *config.SubagentsConfig Subagents *config.SubagentsConfig
SkillsFilter []string SkillsFilter []string
MCPServerAllowlist map[string]struct{} MCPServerPolicy *PatternPolicy
ToolPolicy *PatternPolicy
Candidates []providers.FallbackCandidate Candidates []providers.FallbackCandidate
// Router is non-nil when model routing is configured and the light model // Router is non-nil when model routing is configured and the light model
@ -87,26 +88,28 @@ func NewAgentInstance(
// Compile path whitelist patterns from config. // Compile path whitelist patterns from config.
allowReadPaths := buildAllowReadPatterns(cfg) allowReadPaths := buildAllowReadPatterns(cfg)
allowWritePaths := compilePatterns(cfg.Tools.AllowWritePaths) allowWritePaths := compilePatterns(cfg.Tools.AllowWritePaths)
agentToolAllowlist := resolveAgentToolAllowlist(definition) agentToolPolicy := resolveAgentToolPolicy(definition)
agentMCPServerAllowlist := resolveAgentMCPServerAllowlist(definition) agentMCPServerPolicy := resolveAgentMCPServerPolicy(definition)
toolsRegistry := tools.NewToolRegistry() toolsRegistry := tools.NewToolRegistry()
toolsRegistry.SetAllowlist(agentToolAllowlist) registerTool := func(tool tools.Tool) {
registerToolWithPolicies(toolsRegistry, tool, agentToolPolicy)
}
if cfg.Tools.IsToolEnabled("read_file") { if cfg.Tools.IsToolEnabled("read_file") {
maxReadFileSize := cfg.Tools.ReadFile.MaxReadFileSize maxReadFileSize := cfg.Tools.ReadFile.MaxReadFileSize
switch cfg.Tools.ReadFile.EffectiveMode() { switch cfg.Tools.ReadFile.EffectiveMode() {
case config.ReadFileModeLines: case config.ReadFileModeLines:
toolsRegistry.Register(tools.NewReadFileLinesTool(workspace, readRestrict, maxReadFileSize, allowReadPaths)) registerTool(tools.NewReadFileLinesTool(workspace, readRestrict, maxReadFileSize, allowReadPaths))
default: default:
toolsRegistry.Register(tools.NewReadFileBytesTool(workspace, readRestrict, maxReadFileSize, allowReadPaths)) registerTool(tools.NewReadFileBytesTool(workspace, readRestrict, maxReadFileSize, allowReadPaths))
} }
} }
if cfg.Tools.IsToolEnabled("write_file") { if cfg.Tools.IsToolEnabled("write_file") {
toolsRegistry.Register(tools.NewWriteFileTool(workspace, restrict, allowWritePaths)) registerTool(tools.NewWriteFileTool(workspace, restrict, allowWritePaths))
} }
if cfg.Tools.IsToolEnabled("list_dir") { if cfg.Tools.IsToolEnabled("list_dir") {
toolsRegistry.Register(tools.NewListDirTool(workspace, readRestrict, allowReadPaths)) registerTool(tools.NewListDirTool(workspace, readRestrict, allowReadPaths))
} }
if cfg.Tools.IsToolEnabled("exec") { if cfg.Tools.IsToolEnabled("exec") {
execTool, err := tools.NewExecToolWithConfig(workspace, restrict, cfg, allowReadPaths) execTool, err := tools.NewExecToolWithConfig(workspace, restrict, cfg, allowReadPaths)
@ -114,21 +117,21 @@ func NewAgentInstance(
logger.ErrorCF("agent", "Failed to initialize exec tool; continuing without exec", logger.ErrorCF("agent", "Failed to initialize exec tool; continuing without exec",
map[string]any{"error": err.Error()}) map[string]any{"error": err.Error()})
} else { } else {
toolsRegistry.Register(execTool) registerTool(execTool)
} }
} }
if cfg.Tools.IsToolEnabled("edit_file") { if cfg.Tools.IsToolEnabled("edit_file") {
toolsRegistry.Register(tools.NewEditFileTool(workspace, restrict, allowWritePaths)) registerTool(tools.NewEditFileTool(workspace, restrict, allowWritePaths))
} }
if cfg.Tools.IsToolEnabled("append_file") { if cfg.Tools.IsToolEnabled("append_file") {
toolsRegistry.Register(tools.NewAppendFileTool(workspace, restrict, allowWritePaths)) registerTool(tools.NewAppendFileTool(workspace, restrict, allowWritePaths))
} }
sessionsDir := filepath.Join(workspace, "sessions") sessionsDir := filepath.Join(workspace, "sessions")
sessions := initSessionStore(sessionsDir) sessions := initSessionStore(sessionsDir)
mcpDiscoveryActive := agentHasDiscoverableMCPServers(cfg, agentMCPServerAllowlist) mcpDiscoveryActive := agentHasDiscoverableMCPServers(cfg, agentMCPServerPolicy)
contextBuilder := NewContextBuilder(workspace). contextBuilder := NewContextBuilder(workspace).
WithToolDiscovery( WithToolDiscovery(
mcpDiscoveryActive && cfg.Tools.MCP.Discovery.UseBM25, mcpDiscoveryActive && cfg.Tools.MCP.Discovery.UseBM25,
@ -261,7 +264,8 @@ func NewAgentInstance(
Definition: definition, Definition: definition,
Subagents: subagents, Subagents: subagents,
SkillsFilter: skillsFilter, SkillsFilter: skillsFilter,
MCPServerAllowlist: agentMCPServerAllowlist, MCPServerPolicy: agentMCPServerPolicy,
ToolPolicy: agentToolPolicy,
Candidates: candidates, Candidates: candidates,
Router: router, Router: router,
LightCandidates: lightCandidates, LightCandidates: lightCandidates,
@ -399,11 +403,10 @@ func resolveAgentSkillsFilter(
} }
func (a *AgentInstance) AllowsMCPServer(serverName string) bool { func (a *AgentInstance) AllowsMCPServer(serverName string) bool {
if a == nil || a.MCPServerAllowlist == nil { if a == nil {
return true return true
} }
_, ok := a.MCPServerAllowlist[strings.ToLower(strings.TrimSpace(serverName))] return toolAllowedByPolicy(a.MCPServerPolicy, normalizeMCPServerName(serverName))
return ok
} }
func compilePatterns(patterns []string) []*regexp.Regexp { func compilePatterns(patterns []string) []*regexp.Regexp {

View file

@ -639,6 +639,16 @@ Use frontmatter identity.
ModelName: "default-model", ModelName: "default-model",
}, },
}, },
Tools: config.ToolsConfig{
MCP: config.MCPConfig{
Servers: map[string]config.MCPServerConfig{
"github": {Enabled: true},
"github-legacy": {Enabled: true},
"filesystem": {Enabled: true},
"slack": {Enabled: true},
},
},
},
} }
agent := NewAgentInstance(&config.AgentConfig{ agent := NewAgentInstance(&config.AgentConfig{
@ -899,3 +909,87 @@ func TestNewAgentInstance_ExplicitEmptyToolsFieldBlocksAllTools(t *testing.T) {
}) })
} }
} }
func TestNewAgentInstance_FrontmatterToolPolicySupportsAllowAndDenyPatterns(t *testing.T) {
workspace := setupWorkspace(t, map[string]string{
"AGENT.md": `---
tools:
allow:
- read_*
- list_*
deny:
- list_dir
---
# Agent
`,
})
defer cleanupWorkspace(t, workspace)
cfg := &config.Config{
Agents: config.AgentsConfig{
Defaults: config.AgentDefaults{
Workspace: workspace,
ModelName: "default-model",
},
},
Tools: config.ToolsConfig{
ReadFile: config.ReadFileToolConfig{Enabled: true},
ListDir: config.ToolConfig{Enabled: true},
},
}
agent := NewAgentInstance(&config.AgentConfig{
ID: "research",
Workspace: workspace,
}, &cfg.Agents.Defaults, cfg, &mockProvider{})
if _, ok := agent.Tools.Get("read_file"); !ok {
t.Fatal("expected read_file to be allowed by frontmatter policy")
}
if _, ok := agent.Tools.Get("list_dir"); ok {
t.Fatal("expected list_dir to be denied by frontmatter policy")
}
}
func TestNewAgentInstance_MCPServerPolicySupportsAllowAndDenyPatterns(t *testing.T) {
workspace := setupWorkspace(t, map[string]string{
"AGENT.md": `---
mcpServers:
allow:
- git*
- filesystem
deny:
- github-legacy
---
# Agent
`,
})
defer cleanupWorkspace(t, workspace)
cfg := &config.Config{
Agents: config.AgentsConfig{
Defaults: config.AgentDefaults{
Workspace: workspace,
ModelName: "default-model",
},
},
}
agent := NewAgentInstance(&config.AgentConfig{
ID: "research",
Workspace: workspace,
}, &cfg.Agents.Defaults, cfg, &mockProvider{})
if !agent.AllowsMCPServer("github") {
t.Fatal("expected github to be allowed by glob")
}
if !agent.AllowsMCPServer("filesystem") {
t.Fatal("expected filesystem to be allowed explicitly")
}
if agent.AllowsMCPServer("github-legacy") {
t.Fatal("expected github-legacy to be denied")
}
if agent.AllowsMCPServer("slack") {
t.Fatal("expected slack to be blocked by mcp policy")
}
}

View file

@ -97,16 +97,23 @@ func (r *AgentRegistry) allowedMCPServers() map[string]struct{} {
return nil return nil
} }
if r.cfg == nil {
return nil
}
union := make(map[string]struct{}) union := make(map[string]struct{})
serverCfgs := r.cfg.Tools.MCP.Servers
for _, agent := range r.agents { for _, agent := range r.agents {
if agent == nil { if agent == nil {
continue continue
} }
if agent.MCPServerAllowlist == nil { if agent.MCPServerPolicy == nil {
return nil return nil
} }
for serverName := range agent.MCPServerAllowlist { for serverName := range serverCfgs {
union[serverName] = struct{}{} if agent.AllowsMCPServer(serverName) {
union[normalizeMCPServerName(serverName)] = struct{}{}
}
} }
} }

View file

@ -71,15 +71,15 @@ func unknownAgentToolNames(
registry *tools.ToolRegistry, registry *tools.ToolRegistry,
definition AgentContextDefinition, definition AgentContextDefinition,
) []string { ) []string {
if definition.Agent == nil || definition.Agent.Frontmatter.Tools == nil { if definition.Agent == nil || definition.Agent.Frontmatter.ToolPolicy == nil {
return nil return nil
} }
known := registeredRuntimeToolNames(registry) known := registeredRuntimeToolNames(registry)
unknown := make(map[string]struct{}) unknown := make(map[string]struct{})
for _, raw := range definition.Agent.Frontmatter.Tools { for _, raw := range definition.Agent.Frontmatter.ToolPolicy.Allow {
name := strings.ToLower(strings.TrimSpace(raw)) name := strings.ToLower(strings.TrimSpace(raw))
if name == "" || strings.HasPrefix(name, dynamicMCPToolPrefix) { if name == "" || strings.HasPrefix(name, dynamicMCPToolPrefix) || containsGlobMeta(name) {
continue continue
} }
if _, ok := known[name]; ok { if _, ok := known[name]; ok {
@ -107,15 +107,15 @@ func registeredRuntimeToolNames(registry *tools.ToolRegistry) map[string]struct{
} }
func unknownAgentMCPServerNames(cfg *config.Config, definition AgentContextDefinition) []string { func unknownAgentMCPServerNames(cfg *config.Config, definition AgentContextDefinition) []string {
if cfg == nil || definition.Agent == nil || definition.Agent.Frontmatter.MCPServers == nil { if cfg == nil || definition.Agent == nil || definition.Agent.Frontmatter.MCPPolicy == nil {
return nil return nil
} }
knownServers := normalizedMCPServerNameSet(cfg.Tools.MCP.Servers) knownServers := normalizedMCPServerNameSet(cfg.Tools.MCP.Servers)
unknown := make(map[string]struct{}) unknown := make(map[string]struct{})
for _, raw := range definition.Agent.Frontmatter.MCPServers { for _, raw := range definition.Agent.Frontmatter.MCPPolicy.Allow {
name := normalizeMCPServerName(raw) name := normalizeMCPServerName(raw)
if name == "" { if name == "" || containsGlobMeta(name) {
continue continue
} }
if _, ok := knownServers[name]; ok { if _, ok := knownServers[name]; ok {
@ -140,48 +140,68 @@ func sortedKeys(values map[string]struct{}) []string {
return result return result
} }
func resolveAgentToolAllowlist(definition AgentContextDefinition) []string { func resolveAgentToolPolicy(definition AgentContextDefinition) *PatternPolicy {
if frontmatterParseFailed(definition) { if frontmatterParseFailed(definition) {
return []string{} return &PatternPolicy{Allow: []string{}, form: patternPolicyFormList}
} }
if definition.Agent == nil || !frontmatterDeclaresField(definition, "tools") { if definition.Agent == nil || !frontmatterDeclaresField(definition, "tools") {
return nil return nil
} }
return normalizePatternPolicy(definition.Agent.Frontmatter.ToolPolicy)
allowlist := make(map[string]struct{}, len(definition.Agent.Frontmatter.Tools))
for _, raw := range definition.Agent.Frontmatter.Tools {
trimmed := strings.ToLower(strings.TrimSpace(raw))
if trimmed == "" {
continue
}
allowlist[trimmed] = struct{}{}
}
if len(allowlist) == 0 {
return []string{}
}
return sortedKeys(allowlist)
} }
func resolveAgentMCPServerAllowlist(definition AgentContextDefinition) map[string]struct{} { func resolveAgentMCPServerPolicy(definition AgentContextDefinition) *PatternPolicy {
if frontmatterParseFailed(definition) { if frontmatterParseFailed(definition) {
return map[string]struct{}{} return &PatternPolicy{Allow: []string{}, form: patternPolicyFormList}
} }
if definition.Agent == nil || !frontmatterDeclaresField(definition, "mcpServers") { if definition.Agent == nil || !frontmatterDeclaresField(definition, "mcpServers") {
return nil return nil
} }
return normalizePatternPolicy(definition.Agent.Frontmatter.MCPPolicy)
}
allowlist := make(map[string]struct{}, len(definition.Agent.Frontmatter.MCPServers)) func normalizePatternPolicy(policy *PatternPolicy) *PatternPolicy {
for _, raw := range definition.Agent.Frontmatter.MCPServers { if policy == nil {
return nil
}
normalized := &PatternPolicy{form: policy.form}
normalized.Allow = normalizePatterns(policy.Allow)
normalized.Deny = normalizePatterns(policy.Deny)
if policy.form == patternPolicyFormList && len(normalized.Allow) == 0 {
normalized.Allow = []string{}
}
return normalized
}
func normalizePatterns(patterns []string) []string {
if len(patterns) == 0 {
return nil
}
seen := make(map[string]struct{}, len(patterns))
result := make([]string, 0, len(patterns))
for _, raw := range patterns {
trimmed := strings.ToLower(strings.TrimSpace(raw)) trimmed := strings.ToLower(strings.TrimSpace(raw))
if trimmed == "" { if trimmed == "" {
continue continue
} }
allowlist[trimmed] = struct{}{} if _, ok := seen[trimmed]; ok {
continue
}
seen[trimmed] = struct{}{}
result = append(result, trimmed)
} }
if len(result) == 0 {
return nil
}
sort.Strings(result)
return result
}
return allowlist func containsGlobMeta(value string) bool {
return strings.ContainsAny(value, "*?[")
} }
func frontmatterDeclaresField(definition AgentContextDefinition, field string) bool { func frontmatterDeclaresField(definition AgentContextDefinition, field string) bool {

View file

@ -68,7 +68,7 @@ tools: [serial, reaction, send_tts, load_image, delegate, made_up]
} }
} }
func TestResolveAgentToolAllowlistDistinguishesMissingAndEmptyToolsField(t *testing.T) { func TestResolveAgentToolPolicyDistinguishesMissingAndEmptyToolsField(t *testing.T) {
tests := []struct { tests := []struct {
name string name string
agentMD string agentMD string
@ -111,25 +111,53 @@ tools:
}) })
defer cleanupWorkspace(t, workspace) defer cleanupWorkspace(t, workspace)
allowlist := resolveAgentToolAllowlist(loadAgentDefinition(workspace)) policy := resolveAgentToolPolicy(loadAgentDefinition(workspace))
if tt.wantNil { if tt.wantNil {
if allowlist != nil { if policy != nil {
t.Fatalf("resolveAgentToolAllowlist() = %v, want nil", allowlist) t.Fatalf("resolveAgentToolPolicy() = %v, want nil", policy)
} }
return return
} }
if allowlist == nil { if policy == nil {
t.Fatal("resolveAgentToolAllowlist() = nil, want explicit empty allowlist") t.Fatal("resolveAgentToolPolicy() = nil, want explicit empty allowlist")
} }
if len(allowlist) != 0 { if len(policy.Allow) != 0 {
t.Fatalf("resolveAgentToolAllowlist() = %v, want empty allowlist", allowlist) t.Fatalf("resolveAgentToolPolicy() = %+v, want empty allowlist", policy)
} }
}) })
} }
} }
func TestResolveAgentToolPolicy_ObjectAllowDeny(t *testing.T) {
workspace := setupWorkspace(t, map[string]string{
"AGENT.md": `---
tools:
allow:
- mcp_*
- web_fetch
deny:
- mcp_gpt_researcher_*
---
# Agent
`,
})
defer cleanupWorkspace(t, workspace)
policy := resolveAgentToolPolicy(loadAgentDefinition(workspace))
if policy == nil {
t.Fatal("resolveAgentToolPolicy() = nil, want policy")
}
if got, want := policy.Allow, []string{"mcp_*", "web_fetch"}; len(got) != len(want) ||
got[0] != want[0] || got[1] != want[1] {
t.Fatalf("allow = %v, want %v", got, want)
}
if got, want := policy.Deny, []string{"mcp_gpt_researcher_*"}; len(got) != len(want) || got[0] != want[0] {
t.Fatalf("deny = %v, want %v", got, want)
}
}
func TestUnknownAgentMCPServerNames(t *testing.T) { func TestUnknownAgentMCPServerNames(t *testing.T) {
workspace := setupWorkspace(t, map[string]string{ workspace := setupWorkspace(t, map[string]string{
"AGENT.md": `--- "AGENT.md": `---
@ -182,3 +210,42 @@ mcpServers: [github, FileSystem, slak]
t.Fatalf("unknownAgentMCPServerNames() = %v, want [slak]", unknown) t.Fatalf("unknownAgentMCPServerNames() = %v, want [slak]", unknown)
} }
} }
func TestUnknownDeclarationsIgnoreGlobPatterns(t *testing.T) {
workspace := setupWorkspace(t, map[string]string{
"AGENT.md": `---
tools:
allow:
- mcp_*
- web_*
- read_file
mcpServers:
allow:
- git*
- filesystem
---
# Agent
`,
})
defer cleanupWorkspace(t, workspace)
registry := agenttools.NewToolRegistry()
registry.Register(&allowlistTestTool{name: "read_file"})
cfg := &config.Config{
Tools: config.ToolsConfig{
MCP: config.MCPConfig{
Servers: map[string]config.MCPServerConfig{
"github": {Enabled: true},
"filesystem": {Enabled: true},
},
},
},
}
if unknown := unknownAgentToolNames(registry, loadAgentDefinition(workspace)); len(unknown) != 0 {
t.Fatalf("unknownAgentToolNames() = %v, want none", unknown)
}
if unknown := unknownAgentMCPServerNames(cfg, loadAgentDefinition(workspace)); len(unknown) != 0 {
t.Fatalf("unknownAgentMCPServerNames() = %v, want none", unknown)
}
}

92
pkg/agent/tool_filter.go Normal file
View file

@ -0,0 +1,92 @@
package agent
import (
"path"
"github.com/sipeed/picoclaw/pkg/logger"
"github.com/sipeed/picoclaw/pkg/tools"
)
func agentAllowsTool(agent *AgentInstance, toolName string) bool {
if agent == nil {
return true
}
return toolAllowedByPolicy(agent.ToolPolicy, toolName)
}
func toolAllowedByPolicy(policy *PatternPolicy, toolName string) bool {
if policy == nil {
return true
}
allowed := true
if policy.form == patternPolicyFormList || len(policy.Allow) > 0 {
allowed = matchesAnyGlob(toolName, policy.Allow)
}
if !allowed {
return false
}
if len(policy.Deny) > 0 && matchesAnyGlob(toolName, policy.Deny) {
return false
}
return true
}
func registerToolWithPolicies(
registry *tools.ToolRegistry,
tool tools.Tool,
policies ...*PatternPolicy,
) bool {
if registry == nil || tool == nil {
return false
}
for _, policy := range policies {
if !toolAllowedByPolicy(policy, tool.Name()) {
return false
}
}
registry.Register(tool)
return true
}
func matchesAnyGlob(name string, patterns []string) bool {
for _, pattern := range patterns {
if pattern == "" {
continue
}
if ok, err := path.Match(pattern, name); err == nil && ok {
return true
}
}
return false
}
func registerToolIfAllowed(agent *AgentInstance, tool tools.Tool) bool {
if tool == nil {
return false
}
if !agentAllowsTool(agent, tool.Name()) {
logger.DebugCF("agent", "Skipped tool by agent filter", map[string]any{
"agent_id": agent.ID,
"tool": tool.Name(),
})
return false
}
agent.Tools.Register(tool)
return true
}
func registerHiddenToolIfAllowed(agent *AgentInstance, tool tools.Tool) bool {
if tool == nil {
return false
}
if !agentAllowsTool(agent, tool.Name()) {
logger.DebugCF("agent", "Skipped hidden tool by agent filter", map[string]any{
"agent_id": agent.ID,
"tool": tool.Name(),
})
return false
}
agent.Tools.RegisterHidden(tool)
return true
}