diff --git a/cmd/picoclaw/internal/skills/command.go b/cmd/picoclaw/internal/skills/command.go index e8b884977..b8f660096 100644 --- a/cmd/picoclaw/internal/skills/command.go +++ b/cmd/picoclaw/internal/skills/command.go @@ -43,7 +43,9 @@ func NewSkillsCommand() *cobra.Command { globalDir := filepath.Dir(internal.GetConfigPath()) globalSkillsDir := filepath.Join(globalDir, "skills") builtinSkillsDir := filepath.Join(globalDir, "picoclaw", "skills") - d.skillsLoader = skills.NewSkillsLoader(d.workspace, globalSkillsDir, builtinSkillsDir) + d.skillsLoader = skills.NewSkillsLoader( + d.workspace, d.workspace, globalSkillsDir, builtinSkillsDir, nil, false, + ) return nil }, diff --git a/pkg/agent/context.go b/pkg/agent/context.go index c2921294b..7365aa861 100644 --- a/pkg/agent/context.go +++ b/pkg/agent/context.go @@ -21,6 +21,7 @@ import ( type ContextBuilder struct { workspace string + baseWorkspace string skillsLoader *skills.SkillsLoader memory *MemoryStore toolDiscoveryBM25 bool @@ -61,7 +62,9 @@ func getGlobalConfigDir() string { return config.GetHome() } -func NewContextBuilder(workspace string) *ContextBuilder { +func NewContextBuilder(workspace string, baseWorkspace string, whitelist []string, whitelistEnabled bool) *ContextBuilder { + os.MkdirAll(workspace, 0o755) + // builtin skills: skills directory in current project // Use the skills/ directory under the current working directory builtinSkillsDir := strings.TrimSpace(os.Getenv(config.EnvBuiltinSkills)) @@ -72,9 +75,10 @@ func NewContextBuilder(workspace string) *ContextBuilder { globalSkillsDir := filepath.Join(getGlobalConfigDir(), "skills") return &ContextBuilder{ - workspace: workspace, - skillsLoader: skills.NewSkillsLoader(workspace, globalSkillsDir, builtinSkillsDir), - memory: NewMemoryStore(workspace), + workspace: workspace, + baseWorkspace: baseWorkspace, + skillsLoader: skills.NewSkillsLoader(workspace, baseWorkspace, globalSkillsDir, builtinSkillsDir, whitelist, whitelistEnabled), + memory: NewMemoryStore(workspace), } } diff --git a/pkg/agent/context_cache_test.go b/pkg/agent/context_cache_test.go index ef5e6c5de..f302b37f1 100644 --- a/pkg/agent/context_cache_test.go +++ b/pkg/agent/context_cache_test.go @@ -41,7 +41,7 @@ func TestSingleSystemMessage(t *testing.T) { }) defer os.RemoveAll(tmpDir) - cb := NewContextBuilder(tmpDir) + cb := NewContextBuilder(tmpDir, "", nil, false) tests := []struct { name string @@ -132,7 +132,7 @@ func TestBuildMessages_CurrentSenderDynamicContext(t *testing.T) { }) defer os.RemoveAll(tmpDir) - cb := NewContextBuilder(tmpDir) + cb := NewContextBuilder(tmpDir, "", nil, false) tests := []struct { name string @@ -221,7 +221,7 @@ func TestMtimeAutoInvalidation(t *testing.T) { tmpDir := setupWorkspace(t, map[string]string{tt.file: tt.contentV1}) defer os.RemoveAll(tmpDir) - cb := NewContextBuilder(tmpDir) + cb := NewContextBuilder(tmpDir, "", nil, false) sp1 := cb.BuildSystemPromptWithCache() @@ -257,7 +257,7 @@ func TestMtimeAutoInvalidation(t *testing.T) { tmpDir := setupWorkspace(t, nil) defer os.RemoveAll(tmpDir) - cb := NewContextBuilder(tmpDir) + cb := NewContextBuilder(tmpDir, "", nil, false) _ = cb.BuildSystemPromptWithCache() // populate cache // Touch skills directory (simulate new skill installed) @@ -284,7 +284,7 @@ func TestExplicitInvalidateCache(t *testing.T) { }) defer os.RemoveAll(tmpDir) - cb := NewContextBuilder(tmpDir) + cb := NewContextBuilder(tmpDir, "", nil, false) sp1 := cb.BuildSystemPromptWithCache() cb.InvalidateCache() @@ -312,7 +312,7 @@ func TestCacheStability(t *testing.T) { }) defer os.RemoveAll(tmpDir) - cb := NewContextBuilder(tmpDir) + cb := NewContextBuilder(tmpDir, "", nil, false) results := make([]string, 5) for i := range results { @@ -361,7 +361,7 @@ func TestNewFileCreationInvalidatesCache(t *testing.T) { tmpDir := setupWorkspace(t, nil) defer os.RemoveAll(tmpDir) - cb := NewContextBuilder(tmpDir) + cb := NewContextBuilder(tmpDir, "", nil, false) // Populate cache — file does not exist yet sp1 := cb.BuildSystemPromptWithCache() @@ -406,7 +406,7 @@ Original content.` }) defer os.RemoveAll(tmpDir) - cb := NewContextBuilder(tmpDir) + cb := NewContextBuilder(tmpDir, "", nil, false) // Populate cache sp1 := cb.BuildSystemPromptWithCache() @@ -467,7 +467,7 @@ description: global-v1 t.Fatal(err) } - cb := NewContextBuilder(tmpDir) + cb := NewContextBuilder(tmpDir, "", nil, false) sp1 := cb.BuildSystemPromptWithCache() if !strings.Contains(sp1, "global-v1") { t.Fatal("expected initial prompt to contain global skill description") @@ -527,7 +527,7 @@ description: builtin-v1 t.Fatal(err) } - cb := NewContextBuilder(tmpDir) + cb := NewContextBuilder(tmpDir, "", nil, false) sp1 := cb.BuildSystemPromptWithCache() if !strings.Contains(sp1, "builtin-v1") { t.Fatal("expected initial prompt to contain builtin skill description") @@ -574,7 +574,7 @@ description: delete-me-v1 }) defer os.RemoveAll(tmpDir) - cb := NewContextBuilder(tmpDir) + cb := NewContextBuilder(tmpDir, "", nil, false) sp1 := cb.BuildSystemPromptWithCache() if !strings.Contains(sp1, "delete-me-v1") { t.Fatal("expected initial prompt to contain skill description") @@ -614,7 +614,7 @@ func TestConcurrentBuildSystemPromptWithCache(t *testing.T) { }) defer os.RemoveAll(tmpDir) - cb := NewContextBuilder(tmpDir) + cb := NewContextBuilder(tmpDir, "", nil, false) const goroutines = 20 const iterations = 50 @@ -677,7 +677,7 @@ func TestEmptyWorkspaceBaselineDetectsNewFiles(t *testing.T) { tmpDir := setupWorkspace(t, nil) defer os.RemoveAll(tmpDir) - cb := NewContextBuilder(tmpDir) + cb := NewContextBuilder(tmpDir, "", nil, false) // Build cache — all tracked files are absent, maxMtime falls back to epoch. sp1 := cb.BuildSystemPromptWithCache() @@ -711,7 +711,7 @@ func TestBuildMessages_IncludesMediaOnlyCurrentMessage(t *testing.T) { tmpDir := setupWorkspace(t, nil) defer os.RemoveAll(tmpDir) - cb := NewContextBuilder(tmpDir) + cb := NewContextBuilder(tmpDir, "", nil, false) msgs := cb.BuildMessages( nil, "", @@ -750,7 +750,7 @@ func BenchmarkBuildMessagesWithCache(b *testing.B) { os.WriteFile(filepath.Join(tmpDir, name), []byte(strings.Repeat("Content.\n", 10)), 0o644) } - cb := NewContextBuilder(tmpDir) + cb := NewContextBuilder(tmpDir, "", nil, false) history := []providers.Message{ {Role: "user", Content: "previous message"}, {Role: "assistant", Content: "previous response"}, diff --git a/pkg/agent/definition_test.go b/pkg/agent/definition_test.go index 5ee996967..6e43f5fa5 100644 --- a/pkg/agent/definition_test.go +++ b/pkg/agent/definition_test.go @@ -34,7 +34,7 @@ Act directly and use tools first. }) defer cleanupWorkspace(t, tmpDir) - cb := NewContextBuilder(tmpDir) + cb := NewContextBuilder(tmpDir, "", nil, false) definition := cb.LoadAgentDefinition() if definition.Source != AgentDefinitionSourceAgent { @@ -86,7 +86,7 @@ func TestLoadAgentDefinitionFallsBackToLegacyAgentsMarkdown(t *testing.T) { }) defer cleanupWorkspace(t, tmpDir) - cb := NewContextBuilder(tmpDir) + cb := NewContextBuilder(tmpDir, "", nil, false) definition := cb.LoadAgentDefinition() if definition.Source != AgentDefinitionSourceAgents { @@ -113,7 +113,7 @@ func TestLoadAgentDefinitionLoadsWorkspaceUserMarkdown(t *testing.T) { }) defer cleanupWorkspace(t, tmpDir) - cb := NewContextBuilder(tmpDir) + cb := NewContextBuilder(tmpDir, "", nil, false) definition := cb.LoadAgentDefinition() if definition.User == nil { @@ -142,7 +142,7 @@ Keep going. }) defer cleanupWorkspace(t, tmpDir) - cb := NewContextBuilder(tmpDir) + cb := NewContextBuilder(tmpDir, "", nil, false) definition := cb.LoadAgentDefinition() if definition.Agent == nil { @@ -178,7 +178,7 @@ Follow the body prompt. }) defer cleanupWorkspace(t, tmpDir) - cb := NewContextBuilder(tmpDir) + cb := NewContextBuilder(tmpDir, "", nil, false) bootstrap := cb.LoadBootstrapFiles() if !strings.Contains(bootstrap, "Follow the body prompt") { @@ -209,7 +209,7 @@ func TestLoadBootstrapFilesIncludesWorkspaceUserMarkdown(t *testing.T) { }) defer cleanupWorkspace(t, tmpDir) - cb := NewContextBuilder(tmpDir) + cb := NewContextBuilder(tmpDir, "", nil, false) bootstrap := cb.LoadBootstrapFiles() if !strings.Contains(bootstrap, "Shared profile") { @@ -228,7 +228,7 @@ func TestStructuredAgentIgnoresIdentityChanges(t *testing.T) { }) defer cleanupWorkspace(t, tmpDir) - cb := NewContextBuilder(tmpDir) + cb := NewContextBuilder(tmpDir, "", nil, false) promptV1 := cb.BuildSystemPromptWithCache() if strings.Contains(promptV1, "Legacy identity") { @@ -265,7 +265,7 @@ func TestStructuredAgentUserChangesInvalidateCache(t *testing.T) { }) defer cleanupWorkspace(t, tmpDir) - cb := NewContextBuilder(tmpDir) + cb := NewContextBuilder(tmpDir, "", nil, false) promptV1 := cb.BuildSystemPromptWithCache() if !strings.Contains(promptV1, "Initial workspace preferences") { diff --git a/pkg/agent/instance.go b/pkg/agent/instance.go index bacfa49c5..ee5b5cca3 100644 --- a/pkg/agent/instance.go +++ b/pkg/agent/instance.go @@ -111,7 +111,7 @@ func NewAgentInstance( sessions := initSessionStore(sessionsDir) mcpDiscoveryActive := cfg.Tools.MCP.Enabled && cfg.Tools.MCP.Discovery.Enabled - contextBuilder := NewContextBuilder(workspace). + contextBuilder := NewContextBuilder(workspace, "", cfg.Tools.Skills.Whitelist, cfg.Tools.Skills.WhitelistEnabled). WithToolDiscovery( mcpDiscoveryActive && cfg.Tools.MCP.Discovery.UseBM25, mcpDiscoveryActive && cfg.Tools.MCP.Discovery.UseRegex, diff --git a/pkg/agent/loop.go b/pkg/agent/loop.go index 808d12c07..8ed94d5f2 100644 --- a/pkg/agent/loop.go +++ b/pkg/agent/loop.go @@ -327,11 +327,11 @@ func registerSharedTools( cfg.Tools.Skills.SearchCache.MaxSize, time.Duration(cfg.Tools.Skills.SearchCache.TTLSeconds)*time.Second, ) - agent.Tools.Register(tools.NewFindSkillsTool(registryMgr, searchCache)) + agent.Tools.Register(tools.NewFindSkillsTool(registryMgr, searchCache, cfg.Tools.Skills.Whitelist, cfg.Tools.Skills.WhitelistEnabled)) } if install_skills_enable { - agent.Tools.Register(tools.NewInstallSkillTool(registryMgr, agent.Workspace)) + agent.Tools.Register(tools.NewInstallSkillTool(registryMgr, agent.Workspace, cfg.Tools.Skills.Whitelist, cfg.Tools.Skills.WhitelistEnabled)) } } diff --git a/pkg/config/config.go b/pkg/config/config.go index 7165246e5..442953981 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -159,17 +159,18 @@ func (m AgentModelConfig) MarshalJSON() ([]byte, error) { Primary string `json:"primary,omitempty"` Fallbacks []string `json:"fallbacks,omitempty"` } - return json.Marshal(raw{Primary: m.Primary, Fallbacks: m.Fallbacks}) + return json.Marshal(raw(m)) } type AgentConfig struct { - ID string `json:"id"` - Default bool `json:"default,omitempty"` - Name string `json:"name,omitempty"` - Workspace string `json:"workspace,omitempty"` - Model *AgentModelConfig `json:"model,omitempty"` - Skills []string `json:"skills,omitempty"` - Subagents *SubagentsConfig `json:"subagents,omitempty"` + ID string `json:"id"` + Default bool `json:"default,omitempty"` + Name string `json:"name,omitempty"` + Workspace string `json:"workspace,omitempty"` + Model *AgentModelConfig `json:"model,omitempty"` + Skills []string `json:"skills,omitempty"` + Subagents *SubagentsConfig `json:"subagents,omitempty"` + SystemPrompt string `json:"system_prompt,omitempty"` } type SubagentsConfig struct { @@ -247,8 +248,10 @@ type AgentDefaults struct { SubTurn SubTurnConfig `json:"subturn" envPrefix:"PICOCLAW_AGENTS_DEFAULTS_SUBTURN_"` ToolFeedback ToolFeedbackConfig `json:"tool_feedback,omitempty"` SplitOnMarker bool `json:"split_on_marker" env:"PICOCLAW_AGENTS_DEFAULTS_SPLIT_ON_MARKER"` // split messages on <|[SPLIT]|> marker + SystemPrompt string `json:"system_prompt,omitempty" env:"PICOCLAW_AGENTS_DEFAULTS_SYSTEM_PROMPT"` ContextManager string `json:"context_manager,omitempty" env:"PICOCLAW_AGENTS_DEFAULTS_CONTEXT_MANAGER"` ContextManagerConfig json.RawMessage `json:"context_manager_config,omitempty" env:"PICOCLAW_AGENTS_DEFAULTS_CONTEXT_MANAGER_CONFIG"` + AgentCacheTTLSeconds int `json:"agent_cache_ttl_seconds,omitempty" env:"PICOCLAW_AGENTS_DEFAULTS_AGENT_CACHE_TTL_SECONDS"` } const DefaultMaxMediaSize = 20 * 1024 * 1024 // 20 MB @@ -387,6 +390,10 @@ type DiscordConfig struct { ReasoningChannelID string `json:"reasoning_channel_id" yaml:"-" env:"PICOCLAW_CHANNELS_DISCORD_REASONING_CHANNEL_ID"` } +func (c *DiscordConfig) SetToken(token string) { + c.Token = *NewSecureString(token) +} + type MaixCamConfig struct { Enabled bool `json:"enabled" env:"PICOCLAW_CHANNELS_MAIXCAM_ENABLED"` Host string `json:"host" env:"PICOCLAW_CHANNELS_MAIXCAM_HOST"` @@ -427,6 +434,14 @@ type SlackConfig struct { ReasoningChannelID string `json:"reasoning_channel_id" yaml:"-" env:"PICOCLAW_CHANNELS_SLACK_REASONING_CHANNEL_ID"` } +func (c *SlackConfig) SetBotToken(token string) { + c.BotToken = *NewSecureString(token) +} + +func (c *SlackConfig) SetAppToken(token string) { + c.AppToken = *NewSecureString(token) +} + type MatrixConfig struct { Enabled bool `json:"enabled" yaml:"-" env:"PICOCLAW_CHANNELS_MATRIX_ENABLED"` Homeserver string `json:"homeserver" yaml:"-" env:"PICOCLAW_CHANNELS_MATRIX_HOMESERVER"` @@ -625,6 +640,24 @@ type ModelConfig struct { isVirtual bool } +func (c *ModelConfig) UnmarshalJSON(data []byte) error { + type Alias ModelConfig + aux := &struct { + APIKey string `json:"api_key"` + APIKeys FlexibleStringSlice `json:"api_keys"` + *Alias + }{ + Alias: (*Alias)(c), + } + + if err := json.Unmarshal(data, aux); err != nil { + return err + } + + c.APIKeys = SimpleSecureStrings(MergeAPIKeys(aux.APIKey, aux.APIKeys)...) + return nil +} + // APIKey returns the first API key from apiKeys func (c *ModelConfig) APIKey() string { if len(c.APIKeys) > 0 { @@ -809,8 +842,10 @@ type SkillsToolsConfig struct { ToolConfig ` yaml:"-" envPrefix:"PICOCLAW_TOOLS_SKILLS_"` Registries SkillsRegistriesConfig `yaml:",inline,omitempty" json:"registries"` Github SkillsGithubConfig `yaml:"github,omitempty" json:"github"` - MaxConcurrentSearches int `yaml:"-" json:"max_concurrent_searches" env:"PICOCLAW_TOOLS_SKILLS_MAX_CONCURRENT_SEARCHES"` + MaxConcurrentSearches int `yaml:"-" json:"max_concurrent_searches" env:"PICOCLAW_TOOLS_SKILLS_MAX_CONCURRENT_SEARCHES"` SearchCache SearchCacheConfig `yaml:"-" json:"search_cache"` + Whitelist FlexibleStringSlice `json:"whitelist,omitempty" yaml:"-" env:"PICOCLAW_TOOLS_SKILLS_WHITELIST"` + WhitelistEnabled bool `json:"whitelist_enabled,omitempty" yaml:"-" env:"PICOCLAW_TOOLS_SKILLS_WHITELIST_ENABLED"` } type MediaCleanupConfig struct { @@ -844,6 +879,8 @@ func (c ReadFileToolConfig) EffectiveMode() string { type ToolsConfig struct { AllowReadPaths []string `json:"allow_read_paths" yaml:"-" env:"PICOCLAW_TOOLS_ALLOW_READ_PATHS"` AllowWritePaths []string `json:"allow_write_paths" yaml:"-" env:"PICOCLAW_TOOLS_ALLOW_WRITE_PATHS"` + DenyReadPaths []string `json:"deny_read_paths" yaml:"-" env:"PICOCLAW_TOOLS_DENY_READ_PATHS"` + DenyWritePaths []string `json:"deny_write_paths" yaml:"-" env:"PICOCLAW_TOOLS_DENY_WRITE_PATHS"` // FilterSensitiveData controls whether to filter sensitive values (API keys, // tokens, secrets) from tool results before sending to the LLM. // Default: true (enabled) @@ -851,29 +888,31 @@ type ToolsConfig struct { // FilterMinLength is the minimum content length required for filtering. // Content shorter than this will be returned unchanged for performance. // Default: 8 - FilterMinLength int `json:"filter_min_length" yaml:"-" env:"PICOCLAW_TOOLS_FILTER_MIN_LENGTH"` - Web WebToolsConfig `json:"web" yaml:"web,omitempty"` - Cron CronToolsConfig `json:"cron" yaml:"-"` - Exec ExecConfig `json:"exec" yaml:"-"` - Skills SkillsToolsConfig `json:"skills" yaml:"skills,omitempty"` - MediaCleanup MediaCleanupConfig `json:"media_cleanup" yaml:"-"` - MCP MCPConfig `json:"mcp" yaml:"-"` - AppendFile ToolConfig `json:"append_file" yaml:"-" envPrefix:"PICOCLAW_TOOLS_APPEND_FILE_"` - EditFile ToolConfig `json:"edit_file" yaml:"-" envPrefix:"PICOCLAW_TOOLS_EDIT_FILE_"` - FindSkills ToolConfig `json:"find_skills" yaml:"-" envPrefix:"PICOCLAW_TOOLS_FIND_SKILLS_"` - I2C ToolConfig `json:"i2c" yaml:"-" envPrefix:"PICOCLAW_TOOLS_I2C_"` - InstallSkill ToolConfig `json:"install_skill" yaml:"-" envPrefix:"PICOCLAW_TOOLS_INSTALL_SKILL_"` - ListDir ToolConfig `json:"list_dir" yaml:"-" envPrefix:"PICOCLAW_TOOLS_LIST_DIR_"` - Message ToolConfig `json:"message" yaml:"-" envPrefix:"PICOCLAW_TOOLS_MESSAGE_"` - ReadFile ReadFileToolConfig `json:"read_file" yaml:"-" envPrefix:"PICOCLAW_TOOLS_READ_FILE_"` - SendFile ToolConfig `json:"send_file" yaml:"-" envPrefix:"PICOCLAW_TOOLS_SEND_FILE_"` - SendTTS ToolConfig `json:"send_tts" yaml:"-" envPrefix:"PICOCLAW_TOOLS_SEND_TTS_"` - Spawn ToolConfig `json:"spawn" yaml:"-" envPrefix:"PICOCLAW_TOOLS_SPAWN_"` - SpawnStatus ToolConfig `json:"spawn_status" yaml:"-" envPrefix:"PICOCLAW_TOOLS_SPAWN_STATUS_"` - SPI ToolConfig `json:"spi" yaml:"-" envPrefix:"PICOCLAW_TOOLS_SPI_"` - Subagent ToolConfig `json:"subagent" yaml:"-" envPrefix:"PICOCLAW_TOOLS_SUBAGENT_"` - WebFetch ToolConfig `json:"web_fetch" yaml:"-" envPrefix:"PICOCLAW_TOOLS_WEB_FETCH_"` - WriteFile ToolConfig `json:"write_file" yaml:"-" envPrefix:"PICOCLAW_TOOLS_WRITE_FILE_"` + FilterMinLength int `json:"filter_min_length" yaml:"-" env:"PICOCLAW_TOOLS_FILTER_MIN_LENGTH"` + Web WebToolsConfig `json:"web" yaml:"web,omitempty"` + Cron CronToolsConfig `json:"cron" yaml:"-"` + Exec ExecConfig `json:"exec" yaml:"-"` + Skills SkillsToolsConfig `json:"skills" yaml:"skills,omitempty"` + MediaCleanup MediaCleanupConfig `json:"media_cleanup" yaml:"-"` + Whitelist FlexibleStringSlice `json:"whitelist,omitempty" yaml:"-" env:"PICOCLAW_TOOLS_WHITELIST"` + WhitelistEnabled bool `json:"whitelist_enabled,omitempty" yaml:"-" env:"PICOCLAW_TOOLS_WHITELIST_ENABLED"` + MCP MCPConfig `json:"mcp" yaml:"-"` + AppendFile ToolConfig `json:"append_file" yaml:"-" envPrefix:"PICOCLAW_TOOLS_APPEND_FILE_"` + EditFile ToolConfig `json:"edit_file" yaml:"-" envPrefix:"PICOCLAW_TOOLS_EDIT_FILE_"` + FindSkills ToolConfig `json:"find_skills" yaml:"-" envPrefix:"PICOCLAW_TOOLS_FIND_SKILLS_"` + I2C ToolConfig `json:"i2c" yaml:"-" envPrefix:"PICOCLAW_TOOLS_I2C_"` + InstallSkill ToolConfig `json:"install_skill" yaml:"-" envPrefix:"PICOCLAW_TOOLS_INSTALL_SKILL_"` + ListDir ToolConfig `json:"list_dir" yaml:"-" envPrefix:"PICOCLAW_TOOLS_LIST_DIR_"` + Message ToolConfig `json:"message" yaml:"-" envPrefix:"PICOCLAW_TOOLS_MESSAGE_"` + ReadFile ReadFileToolConfig `json:"read_file" yaml:"-" envPrefix:"PICOCLAW_TOOLS_READ_FILE_"` + SendFile ToolConfig `json:"send_file" yaml:"-" envPrefix:"PICOCLAW_TOOLS_SEND_FILE_"` + SendTTS ToolConfig `json:"send_tts" yaml:"-" envPrefix:"PICOCLAW_TOOLS_SEND_TTS_"` + Spawn ToolConfig `json:"spawn" yaml:"-" envPrefix:"PICOCLAW_TOOLS_SPAWN_"` + SpawnStatus ToolConfig `json:"spawn_status" yaml:"-" envPrefix:"PICOCLAW_TOOLS_SPAWN_STATUS_"` + SPI ToolConfig `json:"spi" yaml:"-" envPrefix:"PICOCLAW_TOOLS_SPI_"` + Subagent ToolConfig `json:"subagent" yaml:"-" envPrefix:"PICOCLAW_TOOLS_SUBAGENT_"` + WebFetch ToolConfig `json:"web_fetch" yaml:"-" envPrefix:"PICOCLAW_TOOLS_WEB_FETCH_"` + WriteFile ToolConfig `json:"write_file" yaml:"-" envPrefix:"PICOCLAW_TOOLS_WRITE_FILE_"` } // IsFilterSensitiveDataEnabled returns true if sensitive data filtering is enabled @@ -1234,6 +1273,29 @@ func (c *Config) SecurityCopyFrom(path string) error { return loadSecurityConfig(c, securityPath(path)) } +func MergeAPIKeys(apiKey string, apiKeys []string) []string { + seen := make(map[string]struct{}) + var all []string + + if k := strings.TrimSpace(apiKey); k != "" { + if _, exists := seen[k]; !exists { + seen[k] = struct{}{} + all = append(all, k) + } + } + + for _, k := range apiKeys { + if trimmed := strings.TrimSpace(k); trimmed != "" && trimmed != "[NOT_HERE]" { + if _, exists := seen[trimmed]; !exists { + seen[trimmed] = struct{}{} + all = append(all, trimmed) + } + } + } + + return all +} + // expandMultiKeyModels expands ModelConfig entries with multiple API keys into // separate entries for key-level failover. Each key gets its own ModelConfig entry, // and the original entry's fallbacks are set up to chain through the expanded entries. diff --git a/pkg/skills/loader.go b/pkg/skills/loader.go index f5985a662..ba1dc9b65 100644 --- a/pkg/skills/loader.go +++ b/pkg/skills/loader.go @@ -59,16 +59,19 @@ func (info SkillInfo) validate() error { } type SkillsLoader struct { - workspace string - workspaceSkills string // workspace skills (project-level) - globalSkills string // global skills (~/.picoclaw/skills) - builtinSkills string // builtin skills + workspace string + workspaceSkills string // workspace skills (project-level) + baseWorkspaceSkills string // fallback workspace skills (if isolated) + globalSkills string // global skills (~/.picoclaw/skills) + builtinSkills string // builtin skills + whitelist []string + whitelistEnabled bool } // SkillRoots returns all unique skill root directories used by this loader. // The order follows resolution priority: workspace > global > builtin. func (sl *SkillsLoader) SkillRoots() []string { - roots := []string{sl.workspaceSkills, sl.globalSkills, sl.builtinSkills} + roots := []string{sl.workspaceSkills, sl.baseWorkspaceSkills, sl.globalSkills, sl.builtinSkills} seen := make(map[string]struct{}, len(roots)) out := make([]string, 0, len(roots)) @@ -88,12 +91,26 @@ func (sl *SkillsLoader) SkillRoots() []string { return out } -func NewSkillsLoader(workspace string, globalSkills string, builtinSkills string) *SkillsLoader { +func NewSkillsLoader( + workspace string, + baseWorkspace string, + globalSkills string, + builtinSkills string, + whitelist []string, + whitelistEnabled bool, +) *SkillsLoader { + var baseWS string + if baseWorkspace != "" { + baseWS = filepath.Join(baseWorkspace, "skills") + } return &SkillsLoader{ - workspace: workspace, - workspaceSkills: filepath.Join(workspace, "skills"), - globalSkills: globalSkills, // ~/.picoclaw/skills - builtinSkills: builtinSkills, + workspace: workspace, + workspaceSkills: filepath.Join(workspace, "skills"), + baseWorkspaceSkills: baseWS, + globalSkills: globalSkills, // ~/.picoclaw/skills + builtinSkills: builtinSkills, + whitelist: whitelist, + whitelistEnabled: whitelistEnabled, } } @@ -101,6 +118,18 @@ func (sl *SkillsLoader) ListSkills() []SkillInfo { skills := make([]SkillInfo, 0) seen := make(map[string]bool) + isWhitelisted := func(name string) bool { + if !sl.whitelistEnabled { + return true + } + for _, w := range sl.whitelist { + if w == name { + return true + } + } + return false + } + addSkills := func(dir, source string) { if dir == "" { return @@ -113,6 +142,12 @@ func (sl *SkillsLoader) ListSkills() []SkillInfo { if !d.IsDir() { continue } + + // First check if whitelisted before doing more expensive operations. + if !isWhitelisted(d.Name()) { + continue + } + skillFile := filepath.Join(dir, d.Name(), "SKILL.md") if _, err := os.Stat(skillFile); err != nil { continue @@ -127,6 +162,12 @@ func (sl *SkillsLoader) ListSkills() []SkillInfo { info.Description = metadata.Description info.Name = metadata.Name } + + // Double check whitelisted name if metadata name is different from directory name + if info.Name != d.Name() && !isWhitelisted(info.Name) { + continue + } + if err := info.validate(); err != nil { slog.Warn("invalid skill from "+source, "name", info.Name, "error", err) continue @@ -139,8 +180,9 @@ func (sl *SkillsLoader) ListSkills() []SkillInfo { } } - // Priority: workspace > global > builtin + // Priority: workspace > base workspace > global > builtin addSkills(sl.workspaceSkills, "workspace") + addSkills(sl.baseWorkspaceSkills, "shared") addSkills(sl.globalSkills, "global") addSkills(sl.builtinSkills, "builtin") @@ -148,6 +190,19 @@ func (sl *SkillsLoader) ListSkills() []SkillInfo { } func (sl *SkillsLoader) LoadSkill(name string) (string, bool) { + if sl.whitelistEnabled { + whitelisted := false + for _, w := range sl.whitelist { + if w == name { + whitelisted = true + break + } + } + if !whitelisted { + return "", false + } + } + // 1. load from workspace skills first (project-level) if sl.workspaceSkills != "" { skillFile := filepath.Join(sl.workspaceSkills, name, "SKILL.md") @@ -155,6 +210,15 @@ func (sl *SkillsLoader) LoadSkill(name string) (string, bool) { return sl.stripFrontmatter(string(content)), true } } + // ... + + // 1b. load from base workspace skills (fallback if isolated) + if sl.baseWorkspaceSkills != "" && sl.baseWorkspaceSkills != sl.workspaceSkills { + skillFile := filepath.Join(sl.baseWorkspaceSkills, name, "SKILL.md") + if content, err := os.ReadFile(skillFile); err == nil { + return sl.stripFrontmatter(string(content)), true + } + } // 2. then load from global skills (~/.picoclaw/skills) if sl.globalSkills != "" { @@ -204,11 +268,11 @@ func (sl *SkillsLoader) BuildSkillsSummary() string { escapedDesc := escapeXML(s.Description) escapedPath := escapeXML(s.Path) - lines = append(lines, fmt.Sprintf(" ")) - lines = append(lines, fmt.Sprintf(" %s", escapedName)) - lines = append(lines, fmt.Sprintf(" %s", escapedDesc)) - lines = append(lines, fmt.Sprintf(" %s", escapedPath)) - lines = append(lines, fmt.Sprintf(" %s", s.Source)) + lines = append(lines, " ") + lines = append(lines, " "+escapedName+"") + lines = append(lines, " "+escapedDesc+"") + lines = append(lines, " "+escapedPath+"") + lines = append(lines, " "+s.Source+"") lines = append(lines, " ") } lines = append(lines, "") diff --git a/pkg/skills/loader_test.go b/pkg/skills/loader_test.go index 645d8b7ac..51d8a5a8f 100644 --- a/pkg/skills/loader_test.go +++ b/pkg/skills/loader_test.go @@ -155,7 +155,7 @@ func TestListSkillsWorkspaceOverridesGlobal(t *testing.T) { createSkillDir(t, filepath.Join(ws, "skills"), "my-skill", "my-skill", "workspace version") createSkillDir(t, global, "my-skill", "my-skill", "global version") - sl := NewSkillsLoader(ws, global, "") + sl := NewSkillsLoader(ws, "", global, "", nil, false) skills := sl.ListSkills() assert.Len(t, skills, 1) @@ -172,7 +172,7 @@ func TestListSkillsGlobalOverridesBuiltin(t *testing.T) { createSkillDir(t, global, "my-skill", "my-skill", "global version") createSkillDir(t, builtin, "my-skill", "my-skill", "builtin version") - sl := NewSkillsLoader(ws, global, builtin) + sl := NewSkillsLoader(ws, "", global, builtin, nil, false) skills := sl.ListSkills() assert.Len(t, skills, 1) @@ -189,7 +189,7 @@ func TestListSkillsMetadataNameDedup(t *testing.T) { createSkillDir(t, filepath.Join(ws, "skills"), "dir-a", "shared-name", "workspace version") createSkillDir(t, global, "dir-b", "shared-name", "global version") - sl := NewSkillsLoader(ws, global, "") + sl := NewSkillsLoader(ws, "", global, "", nil, false) skills := sl.ListSkills() assert.Len(t, skills, 1) @@ -207,7 +207,7 @@ func TestListSkillsMultipleDistinctSkills(t *testing.T) { createSkillDir(t, global, "skill-b", "skill-b", "desc b") createSkillDir(t, builtin, "skill-c", "skill-c", "desc c") - sl := NewSkillsLoader(ws, global, builtin) + sl := NewSkillsLoader(ws, "", global, builtin, nil, false) skills := sl.ListSkills() assert.Len(t, skills, 3) @@ -230,7 +230,7 @@ func TestListSkillsInvalidSkillSkipped(t *testing.T) { // Valid skill createSkillDir(t, global, "good-skill", "good-skill", "desc") - sl := NewSkillsLoader(ws, global, "") + sl := NewSkillsLoader(ws, "", global, "", nil, false) skills := sl.ListSkills() assert.Len(t, skills, 1) @@ -243,7 +243,7 @@ func TestListSkillsEmptyAndNonexistentDirs(t *testing.T) { emptyDir := filepath.Join(tmp, "empty") require.NoError(t, os.MkdirAll(emptyDir, 0o755)) - sl := NewSkillsLoader(ws, emptyDir, filepath.Join(tmp, "nonexistent")) + sl := NewSkillsLoader(ws, "", emptyDir, filepath.Join(tmp, "nonexistent"), nil, false) skills := sl.ListSkills() assert.Empty(t, skills) @@ -259,7 +259,7 @@ func TestListSkillsDirWithoutSkillMD(t *testing.T) { // Valid skill alongside createSkillDir(t, global, "real-skill", "real-skill", "desc") - sl := NewSkillsLoader(ws, global, "") + sl := NewSkillsLoader(ws, "", global, "", nil, false) skills := sl.ListSkills() assert.Len(t, skills, 1) @@ -333,7 +333,7 @@ func TestSkillRootsTrimsWhitespaceAndDedups(t *testing.T) { global := filepath.Join(tmp, "global") builtin := filepath.Join(tmp, "builtin") - sl := NewSkillsLoader(workspace, " "+global+" ", "\t"+builtin+"\n") + sl := NewSkillsLoader(workspace, "", " "+global+" ", "\t"+builtin+"\n", nil, false) roots := sl.SkillRoots() assert.Equal(t, []string{ diff --git a/pkg/tools/skills_install.go b/pkg/tools/skills_install.go index 71bfe730b..562809803 100644 --- a/pkg/tools/skills_install.go +++ b/pkg/tools/skills_install.go @@ -15,23 +15,29 @@ import ( "github.com/sipeed/picoclaw/pkg/utils" ) -// InstallSkillTool allows the LLM agent to install skills from registries. -// It shares the same RegistryManager that FindSkillsTool uses, -// so all registries configured in config are available for installation. type InstallSkillTool struct { - registryMgr *skills.RegistryManager - workspace string - mu sync.Mutex + registryMgr *skills.RegistryManager + workspace string + whitelist []string + whitelistEnabled bool + mu sync.Mutex } // NewInstallSkillTool creates a new InstallSkillTool. // registryMgr is the shared registry manager (same instance as FindSkillsTool). // workspace is the root workspace directory; skills install to {workspace}/skills/{slug}/. -func NewInstallSkillTool(registryMgr *skills.RegistryManager, workspace string) *InstallSkillTool { +func NewInstallSkillTool( + registryMgr *skills.RegistryManager, + workspace string, + whitelist []string, + whitelistEnabled bool, +) *InstallSkillTool { return &InstallSkillTool{ - registryMgr: registryMgr, - workspace: workspace, - mu: sync.Mutex{}, + registryMgr: registryMgr, + workspace: workspace, + whitelist: whitelist, + whitelistEnabled: whitelistEnabled, + mu: sync.Mutex{}, } } @@ -80,6 +86,20 @@ func (t *InstallSkillTool) Execute(ctx context.Context, args map[string]any) *To return ErrorResult(fmt.Sprintf("invalid slug %q: error: %s", slug, err.Error())) } + // Check whitelist + if t.whitelistEnabled { + whitelisted := false + for _, w := range t.whitelist { + if w == slug { + whitelisted = true + break + } + } + if !whitelisted { + return ErrorResult(fmt.Sprintf("skill %q is not in whitelist and cannot be installed", slug)) + } + } + // Validate registry registryName, _ := args["registry"].(string) if err := utils.ValidateSkillIdentifier(registryName); err != nil { diff --git a/pkg/tools/skills_install_test.go b/pkg/tools/skills_install_test.go index 676fcecc0..5c12f0029 100644 --- a/pkg/tools/skills_install_test.go +++ b/pkg/tools/skills_install_test.go @@ -13,19 +13,19 @@ import ( ) func TestInstallSkillToolName(t *testing.T) { - tool := NewInstallSkillTool(skills.NewRegistryManager(), t.TempDir()) + tool := NewInstallSkillTool(skills.NewRegistryManager(), t.TempDir(), nil, false) assert.Equal(t, "install_skill", tool.Name()) } func TestInstallSkillToolMissingSlug(t *testing.T) { - tool := NewInstallSkillTool(skills.NewRegistryManager(), t.TempDir()) + tool := NewInstallSkillTool(skills.NewRegistryManager(), t.TempDir(), nil, false) result := tool.Execute(context.Background(), map[string]any{}) assert.True(t, result.IsError) assert.Contains(t, result.ForLLM, "identifier is required and must be a non-empty string") } func TestInstallSkillToolEmptySlug(t *testing.T) { - tool := NewInstallSkillTool(skills.NewRegistryManager(), t.TempDir()) + tool := NewInstallSkillTool(skills.NewRegistryManager(), t.TempDir(), nil, false) result := tool.Execute(context.Background(), map[string]any{ "slug": " ", }) @@ -34,7 +34,7 @@ func TestInstallSkillToolEmptySlug(t *testing.T) { } func TestInstallSkillToolUnsafeSlug(t *testing.T) { - tool := NewInstallSkillTool(skills.NewRegistryManager(), t.TempDir()) + tool := NewInstallSkillTool(skills.NewRegistryManager(), t.TempDir(), nil, false) cases := []string{ "../etc/passwd", @@ -56,7 +56,7 @@ func TestInstallSkillToolAlreadyExists(t *testing.T) { skillDir := filepath.Join(workspace, "skills", "existing-skill") require.NoError(t, os.MkdirAll(skillDir, 0o755)) - tool := NewInstallSkillTool(skills.NewRegistryManager(), workspace) + tool := NewInstallSkillTool(skills.NewRegistryManager(), workspace, nil, false) result := tool.Execute(context.Background(), map[string]any{ "slug": "existing-skill", "registry": "clawhub", @@ -67,7 +67,7 @@ func TestInstallSkillToolAlreadyExists(t *testing.T) { func TestInstallSkillToolRegistryNotFound(t *testing.T) { workspace := t.TempDir() - tool := NewInstallSkillTool(skills.NewRegistryManager(), workspace) + tool := NewInstallSkillTool(skills.NewRegistryManager(), workspace, nil, false) result := tool.Execute(context.Background(), map[string]any{ "slug": "some-skill", "registry": "nonexistent", @@ -78,7 +78,7 @@ func TestInstallSkillToolRegistryNotFound(t *testing.T) { } func TestInstallSkillToolParameters(t *testing.T) { - tool := NewInstallSkillTool(skills.NewRegistryManager(), t.TempDir()) + tool := NewInstallSkillTool(skills.NewRegistryManager(), t.TempDir(), nil, false) params := tool.Parameters() props, ok := params["properties"].(map[string]any) @@ -95,10 +95,56 @@ func TestInstallSkillToolParameters(t *testing.T) { } func TestInstallSkillToolMissingRegistry(t *testing.T) { - tool := NewInstallSkillTool(skills.NewRegistryManager(), t.TempDir()) + tool := NewInstallSkillTool(skills.NewRegistryManager(), t.TempDir(), nil, false) result := tool.Execute(context.Background(), map[string]any{ "slug": "some-skill", }) assert.True(t, result.IsError) assert.Contains(t, result.ForLLM, "invalid registry") } + +func TestInstallSkillToolWhitelist(t *testing.T) { + workspace := t.TempDir() + rm := skills.NewRegistryManager() + + t.Run("blocked-by-whitelist", func(t *testing.T) { + tool := NewInstallSkillTool(rm, workspace, []string{"allowed-skill"}, true) + result := tool.Execute(context.Background(), map[string]any{ + "slug": "blocked-skill", + "registry": "clawhub", + }) + assert.True(t, result.IsError) + assert.Contains(t, result.ForLLM, "not in whitelist") + }) + + t.Run("allowed-by-whitelist", func(t *testing.T) { + // This will still fail because registry is not found, but it should pass the whitelist check + tool := NewInstallSkillTool(rm, workspace, []string{"allowed-skill"}, true) + result := tool.Execute(context.Background(), map[string]any{ + "slug": "allowed-skill", + "registry": "clawhub", + }) + assert.True(t, result.IsError) + assert.NotContains(t, result.ForLLM, "not in whitelist") + }) + + t.Run("empty-whitelist-allows-all", func(t *testing.T) { + tool := NewInstallSkillTool(rm, workspace, []string{}, false) + result := tool.Execute(context.Background(), map[string]any{ + "slug": "any-skill", + "registry": "clawhub", + }) + assert.True(t, result.IsError) + assert.NotContains(t, result.ForLLM, "not in whitelist") + }) + + t.Run("nil-whitelist-allows-all", func(t *testing.T) { + tool := NewInstallSkillTool(rm, workspace, nil, false) + result := tool.Execute(context.Background(), map[string]any{ + "slug": "any-skill", + "registry": "clawhub", + }) + assert.True(t, result.IsError) + assert.NotContains(t, result.ForLLM, "not in whitelist") + }) +} diff --git a/pkg/tools/skills_search.go b/pkg/tools/skills_search.go index 2b6cffd38..f4d440bc7 100644 --- a/pkg/tools/skills_search.go +++ b/pkg/tools/skills_search.go @@ -12,15 +12,24 @@ import ( type FindSkillsTool struct { registryMgr *skills.RegistryManager cache *skills.SearchCache + whitelist []string + enabled bool } // NewFindSkillsTool creates a new FindSkillsTool. // registryMgr is the shared registry manager (built from config in createToolRegistry). // cache is the search cache for deduplicating similar queries. -func NewFindSkillsTool(registryMgr *skills.RegistryManager, cache *skills.SearchCache) *FindSkillsTool { +func NewFindSkillsTool( + registryMgr *skills.RegistryManager, + cache *skills.SearchCache, + whitelist []string, + enabled bool, +) *FindSkillsTool { return &FindSkillsTool{ registryMgr: registryMgr, cache: cache, + whitelist: whitelist, + enabled: enabled, } } @@ -79,6 +88,21 @@ func (t *FindSkillsTool) Execute(ctx context.Context, args map[string]any) *Tool return ErrorResult(fmt.Sprintf("skill search failed: %v", err)) } + // Filter by whitelist if enabled + if t.enabled { + filtered := make([]skills.SearchResult, 0, len(results)) + whitelistMap := make(map[string]struct{}, len(t.whitelist)) + for _, w := range t.whitelist { + whitelistMap[w] = struct{}{} + } + for _, r := range results { + if _, ok := whitelistMap[r.Slug]; ok { + filtered = append(filtered, r) + } + } + results = filtered + } + // Cache the results. if t.cache != nil && len(results) > 0 { t.cache.Put(query, results) diff --git a/pkg/tools/skills_search_test.go b/pkg/tools/skills_search_test.go index 0e5387cf5..7d2955b3b 100644 --- a/pkg/tools/skills_search_test.go +++ b/pkg/tools/skills_search_test.go @@ -10,19 +10,19 @@ import ( ) func TestFindSkillsToolName(t *testing.T) { - tool := NewFindSkillsTool(skills.NewRegistryManager(), nil) + tool := NewFindSkillsTool(skills.NewRegistryManager(), nil, nil, false) assert.Equal(t, "find_skills", tool.Name()) } func TestFindSkillsToolMissingQuery(t *testing.T) { - tool := NewFindSkillsTool(skills.NewRegistryManager(), nil) + tool := NewFindSkillsTool(skills.NewRegistryManager(), nil, nil, false) result := tool.Execute(context.Background(), map[string]any{}) assert.True(t, result.IsError) assert.Contains(t, result.ForLLM, "query is required") } func TestFindSkillsToolEmptyQuery(t *testing.T) { - tool := NewFindSkillsTool(skills.NewRegistryManager(), nil) + tool := NewFindSkillsTool(skills.NewRegistryManager(), nil, nil, false) result := tool.Execute(context.Background(), map[string]any{ "query": " ", }) @@ -35,7 +35,7 @@ func TestFindSkillsToolCacheHit(t *testing.T) { {Slug: "github", Score: 0.9, RegistryName: "clawhub"}, }) - tool := NewFindSkillsTool(skills.NewRegistryManager(), cache) + tool := NewFindSkillsTool(skills.NewRegistryManager(), cache, nil, false) result := tool.Execute(context.Background(), map[string]any{ "query": "github", }) @@ -46,7 +46,7 @@ func TestFindSkillsToolCacheHit(t *testing.T) { } func TestFindSkillsToolParameters(t *testing.T) { - tool := NewFindSkillsTool(skills.NewRegistryManager(), nil) + tool := NewFindSkillsTool(skills.NewRegistryManager(), nil, nil, false) params := tool.Parameters() props, ok := params["properties"].(map[string]any) @@ -60,7 +60,7 @@ func TestFindSkillsToolParameters(t *testing.T) { } func TestFindSkillsToolDescription(t *testing.T) { - tool := NewFindSkillsTool(skills.NewRegistryManager(), nil) + tool := NewFindSkillsTool(skills.NewRegistryManager(), nil, nil, false) assert.NotEmpty(t, tool.Description()) assert.Contains(t, tool.Description(), "skill") } diff --git a/web/backend/api/skills.go b/web/backend/api/skills.go index 2c054c41b..329225ce6 100644 --- a/web/backend/api/skills.go +++ b/web/backend/api/skills.go @@ -504,9 +504,12 @@ func (h *Handler) handleDeleteSkill(w http.ResponseWriter, r *http.Request) { func newSkillsLoader(workspace string) *skills.SkillsLoader { return skills.NewSkillsLoader( + workspace, workspace, filepath.Join(globalConfigDir(), "skills"), builtinSkillsDir(), + nil, + false, ) } @@ -606,7 +609,8 @@ func findWorkspaceSkillByDirectory(cfg *config.Config, directory string) *skillS } func findWorkspaceSkillInfoByDirectory(workspace, directory string) *skills.SkillInfo { - loader := skills.NewSkillsLoader(workspace, "", "") + loader := skills.NewSkillsLoader(workspace, "", "", "", nil, false) + for _, skill := range loader.ListSkills() { if skill.Source != "workspace" { continue