From 133c3cef14df74f8a885af201e1c8250d7f002e0 Mon Sep 17 00:00:00 2001 From: Anton Bogdanovich <27antonb@gmail.com> Date: Sat, 9 May 2026 12:22:16 -0700 Subject: [PATCH] feat(agent): support frontmatter tool policy filters --- docs/guides/configuration.md | 32 +++++++- pkg/agent/agent_mcp.go | 21 +++-- pkg/agent/agent_mcp_test.go | 33 ++++---- pkg/agent/definition.go | 130 ++++++++++++++++++++++++++++++- pkg/agent/definition_test.go | 45 +++++++++++ pkg/agent/instance.go | 35 +++++---- pkg/agent/instance_test.go | 94 ++++++++++++++++++++++ pkg/agent/registry.go | 13 +++- pkg/agent/tool_allowlist.go | 78 ++++++++++++------- pkg/agent/tool_allowlist_test.go | 83 ++++++++++++++++++-- pkg/agent/tool_filter.go | 92 ++++++++++++++++++++++ 11 files changed, 570 insertions(+), 86 deletions(-) create mode 100644 pkg/agent/tool_filter.go diff --git a/docs/guides/configuration.md b/docs/guides/configuration.md index 3bec847ba..869ca896f 100644 --- a/docs/guides/configuration.md +++ b/docs/guides/configuration.md @@ -215,7 +215,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`. -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 --- @@ -231,11 +231,37 @@ You are the research agent. Notes: -- This is an allowlist, not a preference hint. -- Tool names are matched against the runtime tool name 1:1. +- List form is shorthand for an allow policy. +- 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`. - 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) 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. diff --git a/pkg/agent/agent_mcp.go b/pkg/agent/agent_mcp.go index e8cdf81c8..c056ac006 100644 --- a/pkg/agent/agent_mcp.go +++ b/pkg/agent/agent_mcp.go @@ -168,12 +168,13 @@ func (al *AgentLoop) ensureMCPInitialized(ctx context.Context) error { mcpTool.SetMaxInlineTextRunes(al.cfg.Tools.MCP.GetMaxInlineTextChars()) mcpTool.SetEventPublisher(al.runtimeEvents) + var registered bool if registerAsHidden { - agent.Tools.RegisterHidden(mcpTool) + registered = registerHiddenToolIfAllowed(agent, mcpTool) } else { - agent.Tools.Register(mcpTool) + registered = registerToolIfAllowed(agent, mcpTool) } - if !toolRegistryIncludes(agent.Tools, toolName) { + if !registered || !toolRegistryIncludes(agent.Tools, toolName) { continue } @@ -250,7 +251,7 @@ func (al *AgentLoop) ensureMCPInitialized(ctx context.Context) error { if !ok { continue } - if !agentHasDiscoverableMCPServers(al.cfg, agent.MCPServerAllowlist) { + if !agentHasDiscoverableMCPServers(al.cfg, agent.MCPServerPolicy) { continue } @@ -337,12 +338,20 @@ func filterMCPConfigServers( 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 { 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 { if serverCfg.Enabled && serverIsDeferred(cfg.Tools.MCP.Discovery.Enabled, serverCfg) { return true diff --git a/pkg/agent/agent_mcp_test.go b/pkg/agent/agent_mcp_test.go index f85861146..1715cc085 100644 --- a/pkg/agent/agent_mcp_test.go +++ b/pkg/agent/agent_mcp_test.go @@ -225,7 +225,7 @@ func TestAgentHasDiscoverableMCPServers(t *testing.T) { tests := []struct { name string - allowed map[string]struct{} + allowed *PatternPolicy want bool }{ { @@ -234,29 +234,28 @@ func TestAgentHasDiscoverableMCPServers(t *testing.T) { }, { name: "empty allowlist denies all servers", - allowed: map[string]struct{}{}, + allowed: &PatternPolicy{Allow: []string{}, form: patternPolicyFormList}, want: false, }, { - name: "selected server discoverable", - allowed: map[string]struct{}{ - "github": {}, - }, - want: true, + name: "selected server discoverable", + allowed: &PatternPolicy{Allow: []string{"github"}, form: patternPolicyFormList}, + want: true, }, { - name: "selected server opted out of discovery", - allowed: map[string]struct{}{ - "filesystem": {}, - }, - want: false, + name: "selected server opted out of discovery", + allowed: &PatternPolicy{Allow: []string{"filesystem"}, form: patternPolicyFormList}, + want: false, }, { - name: "unknown allowlist server matches nothing", - allowed: map[string]struct{}{ - "slack": {}, - }, - want: false, + name: "unknown allowlist server matches nothing", + allowed: &PatternPolicy{Allow: []string{"slack"}, form: patternPolicyFormList}, + want: false, + }, + { + name: "deny-only policy can exclude discoverable server", + allowed: &PatternPolicy{Deny: []string{"github"}, form: patternPolicyFormObject}, + want: false, }, } diff --git a/pkg/agent/definition.go b/pkg/agent/definition.go index 5b0e29137..aa8e5002a 100644 --- a/pkg/agent/definition.go +++ b/pkg/agent/definition.go @@ -1,6 +1,7 @@ package agent import ( + "fmt" "os" "path/filepath" "slices" @@ -31,13 +32,29 @@ type AgentFrontmatter struct { Name string `json:"name"` Description string `json:"description"` Tools []string `json:"tools,omitempty"` + ToolPolicy *PatternPolicy `json:"-"` Model string `json:"model,omitempty"` MaxTurns *int `json:"maxTurns,omitempty"` Skills []string `json:"skills,omitempty"` MCPServers []string `json:"mcpServers,omitempty"` + MCPPolicy *PatternPolicy `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. type AgentPromptDefinition struct { Path string `json:"path"` @@ -176,11 +193,9 @@ func parseAgentFrontmatter(path, frontmatter string) (AgentFrontmatter, error) { var typed struct { Name string `yaml:"name"` Description string `yaml:"description"` - Tools []string `yaml:"tools"` Model string `yaml:"model"` MaxTurns *int `yaml:"maxTurns"` Skills []string `yaml:"skills"` - MCPServers []string `yaml:"mcpServers"` } if err := yaml.Unmarshal([]byte(frontmatter), &typed); err != nil { 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 } + 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{ Name: strings.TrimSpace(typed.Name), Description: strings.TrimSpace(typed.Description), - Tools: append([]string(nil), typed.Tools...), + Tools: policyAllowPatterns(toolPolicy), + ToolPolicy: toolPolicy, Model: strings.TrimSpace(typed.Model), MaxTurns: typed.MaxTurns, Skills: append([]string(nil), typed.Skills...), - MCPServers: append([]string(nil), typed.MCPServers...), + MCPServers: policyAllowPatterns(mcpPolicy), + MCPPolicy: mcpPolicy, Fields: rawFields, }, 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) { normalized := string(parser.NormalizeNewlines([]byte(content))) lines := strings.Split(normalized, "\n") diff --git a/pkg/agent/definition_test.go b/pkg/agent/definition_test.go index 5ee996967..b18a32424 100644 --- a/pkg/agent/definition_test.go +++ b/pkg/agent/definition_test.go @@ -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) { tmpDir := setupWorkspace(t, map[string]string{ "AGENTS.md": "# Legacy Agent\nKeep compatibility.", diff --git a/pkg/agent/instance.go b/pkg/agent/instance.go index 4ed713035..c665c741a 100644 --- a/pkg/agent/instance.go +++ b/pkg/agent/instance.go @@ -41,7 +41,8 @@ type AgentInstance struct { Definition AgentContextDefinition Subagents *config.SubagentsConfig SkillsFilter []string - MCPServerAllowlist map[string]struct{} + MCPServerPolicy *PatternPolicy + ToolPolicy *PatternPolicy Candidates []providers.FallbackCandidate // 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. allowReadPaths := buildAllowReadPatterns(cfg) allowWritePaths := compilePatterns(cfg.Tools.AllowWritePaths) - agentToolAllowlist := resolveAgentToolAllowlist(definition) - agentMCPServerAllowlist := resolveAgentMCPServerAllowlist(definition) + agentToolPolicy := resolveAgentToolPolicy(definition) + agentMCPServerPolicy := resolveAgentMCPServerPolicy(definition) toolsRegistry := tools.NewToolRegistry() - toolsRegistry.SetAllowlist(agentToolAllowlist) + registerTool := func(tool tools.Tool) { + registerToolWithPolicies(toolsRegistry, tool, agentToolPolicy) + } if cfg.Tools.IsToolEnabled("read_file") { maxReadFileSize := cfg.Tools.ReadFile.MaxReadFileSize switch cfg.Tools.ReadFile.EffectiveMode() { case config.ReadFileModeLines: - toolsRegistry.Register(tools.NewReadFileLinesTool(workspace, readRestrict, maxReadFileSize, allowReadPaths)) + registerTool(tools.NewReadFileLinesTool(workspace, readRestrict, maxReadFileSize, allowReadPaths)) default: - toolsRegistry.Register(tools.NewReadFileBytesTool(workspace, readRestrict, maxReadFileSize, allowReadPaths)) + registerTool(tools.NewReadFileBytesTool(workspace, readRestrict, maxReadFileSize, allowReadPaths)) } } 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") { - toolsRegistry.Register(tools.NewListDirTool(workspace, readRestrict, allowReadPaths)) + registerTool(tools.NewListDirTool(workspace, readRestrict, allowReadPaths)) } if cfg.Tools.IsToolEnabled("exec") { 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", map[string]any{"error": err.Error()}) } else { - toolsRegistry.Register(execTool) + registerTool(execTool) } } 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") { - toolsRegistry.Register(tools.NewAppendFileTool(workspace, restrict, allowWritePaths)) + registerTool(tools.NewAppendFileTool(workspace, restrict, allowWritePaths)) } sessionsDir := filepath.Join(workspace, "sessions") sessions := initSessionStore(sessionsDir) - mcpDiscoveryActive := agentHasDiscoverableMCPServers(cfg, agentMCPServerAllowlist) + mcpDiscoveryActive := agentHasDiscoverableMCPServers(cfg, agentMCPServerPolicy) contextBuilder := NewContextBuilder(workspace). WithToolDiscovery( mcpDiscoveryActive && cfg.Tools.MCP.Discovery.UseBM25, @@ -261,7 +264,8 @@ func NewAgentInstance( Definition: definition, Subagents: subagents, SkillsFilter: skillsFilter, - MCPServerAllowlist: agentMCPServerAllowlist, + MCPServerPolicy: agentMCPServerPolicy, + ToolPolicy: agentToolPolicy, Candidates: candidates, Router: router, LightCandidates: lightCandidates, @@ -399,11 +403,10 @@ func resolveAgentSkillsFilter( } func (a *AgentInstance) AllowsMCPServer(serverName string) bool { - if a == nil || a.MCPServerAllowlist == nil { + if a == nil { return true } - _, ok := a.MCPServerAllowlist[strings.ToLower(strings.TrimSpace(serverName))] - return ok + return toolAllowedByPolicy(a.MCPServerPolicy, normalizeMCPServerName(serverName)) } func compilePatterns(patterns []string) []*regexp.Regexp { diff --git a/pkg/agent/instance_test.go b/pkg/agent/instance_test.go index dff2c0f2f..e298059d9 100644 --- a/pkg/agent/instance_test.go +++ b/pkg/agent/instance_test.go @@ -639,6 +639,16 @@ Use frontmatter identity. 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{ @@ -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") + } +} diff --git a/pkg/agent/registry.go b/pkg/agent/registry.go index 821ad4187..4711b1124 100644 --- a/pkg/agent/registry.go +++ b/pkg/agent/registry.go @@ -97,16 +97,23 @@ func (r *AgentRegistry) allowedMCPServers() map[string]struct{} { return nil } + if r.cfg == nil { + return nil + } + union := make(map[string]struct{}) + serverCfgs := r.cfg.Tools.MCP.Servers for _, agent := range r.agents { if agent == nil { continue } - if agent.MCPServerAllowlist == nil { + if agent.MCPServerPolicy == nil { return nil } - for serverName := range agent.MCPServerAllowlist { - union[serverName] = struct{}{} + for serverName := range serverCfgs { + if agent.AllowsMCPServer(serverName) { + union[normalizeMCPServerName(serverName)] = struct{}{} + } } } diff --git a/pkg/agent/tool_allowlist.go b/pkg/agent/tool_allowlist.go index 962f7ec05..ad1c60742 100644 --- a/pkg/agent/tool_allowlist.go +++ b/pkg/agent/tool_allowlist.go @@ -71,15 +71,15 @@ func unknownAgentToolNames( registry *tools.ToolRegistry, definition AgentContextDefinition, ) []string { - if definition.Agent == nil || definition.Agent.Frontmatter.Tools == nil { + if definition.Agent == nil || definition.Agent.Frontmatter.ToolPolicy == nil { return nil } known := registeredRuntimeToolNames(registry) 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)) - if name == "" || strings.HasPrefix(name, dynamicMCPToolPrefix) { + if name == "" || strings.HasPrefix(name, dynamicMCPToolPrefix) || containsGlobMeta(name) { continue } 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 { - if cfg == nil || definition.Agent == nil || definition.Agent.Frontmatter.MCPServers == nil { + if cfg == nil || definition.Agent == nil || definition.Agent.Frontmatter.MCPPolicy == nil { return nil } knownServers := normalizedMCPServerNameSet(cfg.Tools.MCP.Servers) unknown := make(map[string]struct{}) - for _, raw := range definition.Agent.Frontmatter.MCPServers { + for _, raw := range definition.Agent.Frontmatter.MCPPolicy.Allow { name := normalizeMCPServerName(raw) - if name == "" { + if name == "" || containsGlobMeta(name) { continue } if _, ok := knownServers[name]; ok { @@ -140,48 +140,68 @@ func sortedKeys(values map[string]struct{}) []string { return result } -func resolveAgentToolAllowlist(definition AgentContextDefinition) []string { +func resolveAgentToolPolicy(definition AgentContextDefinition) *PatternPolicy { if frontmatterParseFailed(definition) { - return []string{} + return &PatternPolicy{Allow: []string{}, form: patternPolicyFormList} } if definition.Agent == nil || !frontmatterDeclaresField(definition, "tools") { return nil } - - 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) + return normalizePatternPolicy(definition.Agent.Frontmatter.ToolPolicy) } -func resolveAgentMCPServerAllowlist(definition AgentContextDefinition) map[string]struct{} { +func resolveAgentMCPServerPolicy(definition AgentContextDefinition) *PatternPolicy { if frontmatterParseFailed(definition) { - return map[string]struct{}{} + return &PatternPolicy{Allow: []string{}, form: patternPolicyFormList} } if definition.Agent == nil || !frontmatterDeclaresField(definition, "mcpServers") { return nil } + return normalizePatternPolicy(definition.Agent.Frontmatter.MCPPolicy) +} - allowlist := make(map[string]struct{}, len(definition.Agent.Frontmatter.MCPServers)) - for _, raw := range definition.Agent.Frontmatter.MCPServers { +func normalizePatternPolicy(policy *PatternPolicy) *PatternPolicy { + 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)) if trimmed == "" { 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 { diff --git a/pkg/agent/tool_allowlist_test.go b/pkg/agent/tool_allowlist_test.go index 5ed35d4c6..05771d2a3 100644 --- a/pkg/agent/tool_allowlist_test.go +++ b/pkg/agent/tool_allowlist_test.go @@ -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 { name string agentMD string @@ -111,25 +111,53 @@ tools: }) defer cleanupWorkspace(t, workspace) - allowlist := resolveAgentToolAllowlist(loadAgentDefinition(workspace)) + policy := resolveAgentToolPolicy(loadAgentDefinition(workspace)) if tt.wantNil { - if allowlist != nil { - t.Fatalf("resolveAgentToolAllowlist() = %v, want nil", allowlist) + if policy != nil { + t.Fatalf("resolveAgentToolPolicy() = %v, want nil", policy) } return } - if allowlist == nil { - t.Fatal("resolveAgentToolAllowlist() = nil, want explicit empty allowlist") + if policy == nil { + t.Fatal("resolveAgentToolPolicy() = nil, want explicit empty allowlist") } - if len(allowlist) != 0 { - t.Fatalf("resolveAgentToolAllowlist() = %v, want empty allowlist", allowlist) + if len(policy.Allow) != 0 { + 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) { workspace := setupWorkspace(t, map[string]string{ "AGENT.md": `--- @@ -182,3 +210,42 @@ mcpServers: [github, FileSystem, slak] 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) + } +} diff --git a/pkg/agent/tool_filter.go b/pkg/agent/tool_filter.go new file mode 100644 index 000000000..4fdfe43d8 --- /dev/null +++ b/pkg/agent/tool_filter.go @@ -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 +}