opencode/picoclaw
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

closed gap in skills adding

Browse source
This commit is contained in:
stevef 2026-04-17 19:30:46 +02:00
parent 01b1b008a0
commit c9c61dfc06
3 changed files with 69 additions and 12 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
pkg/agent/loop.go
View file

@ -193,6 +193,7 @@ func registerSharedTools(
) {
allowReadPaths := buildAllowReadPatterns(cfg)
denyReadPaths := compilePatterns(cfg.Tools.DenyReadPaths)
denyWritePaths := compilePatterns(cfg.Tools.DenyWritePaths)
var ttsProvider tts.TTSProvider
if cfg.Tools.IsToolEnabled("send_tts") {
ttsProvider = tts.DetectTTS(cfg)
@ -376,6 +377,7 @@ func registerSharedTools(
agent.Workspace,
cfg.Tools.Skills.Whitelist,
cfg.Tools.Skills.WhitelistEnabled,
denyWritePaths,
),
)
}

17
pkg/tools/skills_install.go
View file

@ -6,6 +6,7 @@ import (
"fmt"
"os"
"path/filepath"
"regexp"
"sync"
"time"
@ -20,23 +21,27 @@ type InstallSkillTool struct {
workspace string
whitelist []string
whitelistEnabled bool
denyWritePaths []*regexp.Regexp
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}/.
// denyWritePaths is a list of regex patterns to check before allowing installation.
func NewInstallSkillTool(
registryMgr *skills.RegistryManager,
workspace string,
whitelist []string,
whitelistEnabled bool,
denyWritePaths []*regexp.Regexp,
) *InstallSkillTool {
return &InstallSkillTool{
registryMgr: registryMgr,
workspace: workspace,
whitelist: whitelist,
whitelistEnabled: whitelistEnabled,
denyWritePaths: denyWritePaths,
mu: sync.Mutex{},
}
}
@ -109,6 +114,18 @@ func (t *InstallSkillTool) Execute(ctx context.Context, args map[string]any) *To
version, _ := args["version"].(string)
force, _ := args["force"].(bool)
// Check deny write paths before proceeding with installation.
// Patterns are expected to match relative paths (e.g., "skills", "skills/foo"),
// so we check against the relative path from workspace.
if len(t.denyWritePaths) > 0 {
relativePath := "skills"
for _, pattern := range t.denyWritePaths {
if pattern.MatchString(relativePath) {
return ErrorResult("access denied: cannot write to skills directory")
}
}
}
// Check if already installed.
skillsDir := filepath.Join(t.workspace, "skills")
targetDir := filepath.Join(skillsDir, slug)

62
pkg/tools/skills_install_test.go
View file

@ -4,6 +4,7 @@ import (
"context"
"os"
"path/filepath"
"regexp"
"testing"
"github.com/stretchr/testify/assert"
@ -13,19 +14,19 @@ import (
)
func TestInstallSkillToolName(t *testing.T) {
tool := NewInstallSkillTool(skills.NewRegistryManager(), t.TempDir(), nil, false)
tool := NewInstallSkillTool(skills.NewRegistryManager(), t.TempDir(), nil, false, nil)
assert.Equal(t, "install_skill", tool.Name())
}
func TestInstallSkillToolMissingSlug(t *testing.T) {
tool := NewInstallSkillTool(skills.NewRegistryManager(), t.TempDir(), nil, false)
tool := NewInstallSkillTool(skills.NewRegistryManager(), t.TempDir(), nil, false, nil)
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(), nil, false)
tool := NewInstallSkillTool(skills.NewRegistryManager(), t.TempDir(), nil, false, nil)
result := tool.Execute(context.Background(), map[string]any{
"slug": " ",
})
@ -34,7 +35,7 @@ func TestInstallSkillToolEmptySlug(t *testing.T) {
}
func TestInstallSkillToolUnsafeSlug(t *testing.T) {
tool := NewInstallSkillTool(skills.NewRegistryManager(), t.TempDir(), nil, false)
tool := NewInstallSkillTool(skills.NewRegistryManager(), t.TempDir(), nil, false, nil)
cases := []string{
"../etc/passwd",
@ -56,7 +57,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, nil, false)
tool := NewInstallSkillTool(skills.NewRegistryManager(), workspace, nil, false, nil)
result := tool.Execute(context.Background(), map[string]any{
"slug": "existing-skill",
"registry": "clawhub",
@ -67,7 +68,7 @@ func TestInstallSkillToolAlreadyExists(t *testing.T) {
func TestInstallSkillToolRegistryNotFound(t *testing.T) {
workspace := t.TempDir()
tool := NewInstallSkillTool(skills.NewRegistryManager(), workspace, nil, false)
tool := NewInstallSkillTool(skills.NewRegistryManager(), workspace, nil, false, nil)
result := tool.Execute(context.Background(), map[string]any{
"slug": "some-skill",
"registry": "nonexistent",
@ -78,7 +79,7 @@ func TestInstallSkillToolRegistryNotFound(t *testing.T) {
}
func TestInstallSkillToolParameters(t *testing.T) {
tool := NewInstallSkillTool(skills.NewRegistryManager(), t.TempDir(), nil, false)
tool := NewInstallSkillTool(skills.NewRegistryManager(), t.TempDir(), nil, false, nil)
params := tool.Parameters()
props, ok := params["properties"].(map[string]any)
@ -95,7 +96,7 @@ func TestInstallSkillToolParameters(t *testing.T) {
}
func TestInstallSkillToolMissingRegistry(t *testing.T) {
tool := NewInstallSkillTool(skills.NewRegistryManager(), t.TempDir(), nil, false)
tool := NewInstallSkillTool(skills.NewRegistryManager(), t.TempDir(), nil, false, nil)
result := tool.Execute(context.Background(), map[string]any{
"slug": "some-skill",
})
@ -108,7 +109,7 @@ func TestInstallSkillToolWhitelist(t *testing.T) {
rm := skills.NewRegistryManager()
t.Run("blocked-by-whitelist", func(t *testing.T) {
tool := NewInstallSkillTool(rm, workspace, []string{"allowed-skill"}, true)
tool := NewInstallSkillTool(rm, workspace, []string{"allowed-skill"}, true, nil)
result := tool.Execute(context.Background(), map[string]any{
"slug": "blocked-skill",
"registry": "clawhub",
@ -119,7 +120,7 @@ func TestInstallSkillToolWhitelist(t *testing.T) {
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)
tool := NewInstallSkillTool(rm, workspace, []string{"allowed-skill"}, true, nil)
result := tool.Execute(context.Background(), map[string]any{
"slug": "allowed-skill",
"registry": "clawhub",
@ -129,7 +130,7 @@ func TestInstallSkillToolWhitelist(t *testing.T) {
})
t.Run("empty-whitelist-allows-all", func(t *testing.T) {
tool := NewInstallSkillTool(rm, workspace, []string{}, false)
tool := NewInstallSkillTool(rm, workspace, []string{}, false, nil)
result := tool.Execute(context.Background(), map[string]any{
"slug": "any-skill",
"registry": "clawhub",
@ -139,7 +140,7 @@ func TestInstallSkillToolWhitelist(t *testing.T) {
})
t.Run("nil-whitelist-allows-all", func(t *testing.T) {
tool := NewInstallSkillTool(rm, workspace, nil, false)
tool := NewInstallSkillTool(rm, workspace, nil, false, nil)
result := tool.Execute(context.Background(), map[string]any{
"slug": "any-skill",
"registry": "clawhub",
@ -148,3 +149,40 @@ func TestInstallSkillToolWhitelist(t *testing.T) {
assert.NotContains(t, result.ForLLM, "not in whitelist")
})
}
func TestInstallSkillToolDenyWritePaths(t *testing.T) {
workspace := t.TempDir()
rm := skills.NewRegistryManager()
t.Run("blocked-by-deny-write-paths", func(t *testing.T) {
denyPatterns := []*regexp.Regexp{regexp.MustCompile(`^skills(/.*)?$`)}
tool := NewInstallSkillTool(rm, workspace, nil, false, denyPatterns)
result := tool.Execute(context.Background(), map[string]any{
"slug": "some-skill",
"registry": "clawhub",
})
assert.True(t, result.IsError)
assert.Contains(t, result.ForLLM, "access denied")
})
t.Run("allowed-without-deny-paths", func(t *testing.T) {
tool := NewInstallSkillTool(rm, workspace, nil, false, nil)
result := tool.Execute(context.Background(), map[string]any{
"slug": "some-skill",
"registry": "clawhub",
})
assert.True(t, result.IsError)
assert.NotContains(t, result.ForLLM, "access denied")
})
t.Run("non-matching-deny-pattern-allows", func(t *testing.T) {
denyPatterns := []*regexp.Regexp{regexp.MustCompile(`^restricted(/.*)?$`)}
tool := NewInstallSkillTool(rm, workspace, nil, false, denyPatterns)
result := tool.Execute(context.Background(), map[string]any{
"slug": "some-skill",
"registry": "clawhub",
})
assert.True(t, result.IsError)
assert.NotContains(t, result.ForLLM, "access denied")
})
}