feat: Add Skills Whitelisting for enhanced agent security

This commit is contained in:
stevef 2026-04-04 07:52:18 +02:00
parent 84e42d6904
commit 62ef5ec6a3
14 changed files with 339 additions and 113 deletions

View file

@ -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
},

View file

@ -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),
}
}

View file

@ -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"},

View file

@ -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") {

View file

@ -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,

View file

@ -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))
}
}

View file

@ -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.

View file

@ -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(" <skill>"))
lines = append(lines, fmt.Sprintf(" <name>%s</name>", escapedName))
lines = append(lines, fmt.Sprintf(" <description>%s</description>", escapedDesc))
lines = append(lines, fmt.Sprintf(" <location>%s</location>", escapedPath))
lines = append(lines, fmt.Sprintf(" <source>%s</source>", s.Source))
lines = append(lines, " <skill>")
lines = append(lines, " <name>"+escapedName+"</name>")
lines = append(lines, " <description>"+escapedDesc+"</description>")
lines = append(lines, " <location>"+escapedPath+"</location>")
lines = append(lines, " <source>"+s.Source+"</source>")
lines = append(lines, " </skill>")
}
lines = append(lines, "</skills>")

View file

@ -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{

View file

@ -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 {

View file

@ -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")
})
}

View file

@ -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)

View file

@ -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")
}

View file

@ -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