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

fix skills config compatibility and legacy security overlays

Browse source
This commit is contained in:
lxowalle 2026-04-13 18:19:08 +08:00
parent 24794d31e4
commit 2ac8c3f5fb
21 changed files with 623 additions and 56 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
cmd/picoclaw/internal/skills/command.go
View file

@ -61,7 +61,7 @@ func NewSkillsCommand() *cobra.Command {
newInstallCommand(),
newInstallBuiltinCommand(workspaceFn),
newListBuiltinCommand(),
newRemoveCommand(workspaceFn),
newRemoveCommand(),
newSearchCommand(),
newShowCommand(loaderFn),
)

12
cmd/picoclaw/internal/skills/helpers.go
View file

@ -153,15 +153,21 @@ func workspaceHasValidSkillDirectory(workspace, directory string) bool {
return false
}
func skillsRemoveFromWorkspace(workspace, skillName string) error {
func skillsRemoveFromWorkspace(workspace string, toolsConfig config.SkillsToolsConfig, skillName string) error {
name := strings.TrimSpace(skillName)
name = strings.Trim(name, "/")
if name == "" {
return fmt.Errorf("skill name is required")
}
if strings.Contains(name, "/") {
parts := strings.Split(name, "/")
name = parts[len(parts)-1]
dirName, err := skills.GitHubInstallDirNameFromToolsConfig(toolsConfig, name)
if err != nil || dirName == "" {
return fmt.Errorf("invalid skill name %q", skillName)
}
name = dirName
}
if name == "." || name == ".." {
return fmt.Errorf("invalid skill name %q", skillName)
}
skillDir := filepath.Join(workspace, "skills", name)
if _, err := os.Stat(skillDir); os.IsNotExist(err) {

92
cmd/picoclaw/internal/skills/helpers_test.go
View file

@ -97,3 +97,95 @@ func TestSkillsInstallFromRegistryRejectsInvalidSkillArchive(t *testing.T) {
_, statErr := os.Stat(filepath.Join(workspace, "skills", "pr-review"))
assert.True(t, os.IsNotExist(statErr))
}
func TestSkillsRemoveFromWorkspaceRejectsDotTarget(t *testing.T) {
workspace := t.TempDir()
skillsDir := filepath.Join(workspace, "skills")
require.NoError(t, os.MkdirAll(skillsDir, 0o755))
require.NoError(t, os.WriteFile(filepath.Join(skillsDir, "keep.txt"), []byte("keep"), 0o644))
err := skillsRemoveFromWorkspace(workspace, config.DefaultConfig().Tools.Skills, ".")
require.Error(t, err)
assert.Contains(t, err.Error(), "invalid skill name")
_, statErr := os.Stat(skillsDir)
assert.NoError(t, statErr)
_, fileErr := os.Stat(filepath.Join(skillsDir, "keep.txt"))
assert.NoError(t, fileErr)
}
func TestSkillsRemoveFromWorkspaceUsesLastPathSegment(t *testing.T) {
workspace := t.TempDir()
targetDir := filepath.Join(workspace, "skills", "pr-review")
require.NoError(t, os.MkdirAll(targetDir, 0o755))
err := skillsRemoveFromWorkspace(
workspace,
config.DefaultConfig().Tools.Skills,
"https://github.com/foo/bar/tree/main/.agents/skills/pr-review",
)
require.NoError(t, err)
_, statErr := os.Stat(targetDir)
assert.True(t, os.IsNotExist(statErr))
}
func TestSkillsRemoveFromWorkspaceSupportsRepoRootGitHubBlobURL(t *testing.T) {
workspace := t.TempDir()
targetDir := filepath.Join(workspace, "skills", "bar")
require.NoError(t, os.MkdirAll(targetDir, 0o755))
err := skillsRemoveFromWorkspace(
workspace,
config.DefaultConfig().Tools.Skills,
"https://github.com/foo/bar/blob/feature/skills-registry/SKILL.md",
)
require.NoError(t, err)
_, statErr := os.Stat(targetDir)
assert.True(t, os.IsNotExist(statErr))
}
func TestSkillsRemoveFromWorkspaceSupportsGitHubEnterpriseURL(t *testing.T) {
workspace := t.TempDir()
targetDir := filepath.Join(workspace, "skills", "pr-review")
require.NoError(t, os.MkdirAll(targetDir, 0o755))
cfg := config.DefaultConfig()
githubRegistry, ok := cfg.Tools.Skills.Registries.Get("github")
require.True(t, ok)
githubRegistry.BaseURL = "https://ghe.example.com/git"
cfg.Tools.Skills.Registries.Set("github", githubRegistry)
err := skillsRemoveFromWorkspace(
workspace,
cfg.Tools.Skills,
"https://ghe.example.com/git/foo/bar/tree/main/.agents/skills/pr-review",
)
require.NoError(t, err)
_, statErr := os.Stat(targetDir)
assert.True(t, os.IsNotExist(statErr))
}
func TestSkillsRemoveFromWorkspaceDoesNotRequireEnabledGitHubRegistry(t *testing.T) {
workspace := t.TempDir()
targetDir := filepath.Join(workspace, "skills", "pr-review")
require.NoError(t, os.MkdirAll(targetDir, 0o755))
cfg := config.DefaultConfig()
githubRegistry, ok := cfg.Tools.Skills.Registries.Get("github")
require.True(t, ok)
githubRegistry.Enabled = false
cfg.Tools.Skills.Registries.Set("github", githubRegistry)
err := skillsRemoveFromWorkspace(
workspace,
cfg.Tools.Skills,
"https://github.com/foo/bar/tree/main/.agents/skills/pr-review",
)
require.NoError(t, err)
_, statErr := os.Stat(targetDir)
assert.True(t, os.IsNotExist(statErr))
}

8
cmd/picoclaw/internal/skills/remove.go
View file

@ -2,9 +2,11 @@ package skills
import (
"github.com/spf13/cobra"
"github.com/sipeed/picoclaw/cmd/picoclaw/internal"
)
func newRemoveCommand(workspaceFn func() (string, error)) *cobra.Command {
func newRemoveCommand() *cobra.Command {
cmd := &cobra.Command{
Use: "remove",
Aliases: []string{"rm", "uninstall"},
@ -12,11 +14,11 @@ func newRemoveCommand(workspaceFn func() (string, error)) *cobra.Command {
Args: cobra.ExactArgs(1),
Example: `picoclaw skills remove weather`,
RunE: func(_ *cobra.Command, args []string) error {
workspace, err := workspaceFn()
cfg, err := internal.LoadConfig()
if err != nil {
return err
}
return skillsRemoveFromWorkspace(workspace, args[0])
return skillsRemoveFromWorkspace(cfg.WorkspacePath(), cfg.Tools.Skills, args[0])
},
}

2
cmd/picoclaw/internal/skills/remove_test.go
View file

@ -8,7 +8,7 @@ import (
)
func TestNewRemoveSubcommand(t *testing.T) {
cmd := newRemoveCommand(nil)
cmd := newRemoveCommand()
require.NotNil(t, cmd)

23
pkg/agent/hooks_test.go
View file

@ -819,9 +819,26 @@ func TestAgentLoop_HookRespond_SteeringSkipsRemaining(t *testing.T) {
resultCh <- result{resp: resp, err: err}
}()
time.Sleep(50 * time.Millisecond)
collectedEvents := make([]Event, 0, 8)
steered := false
deadline := time.After(3 * time.Second)
for !steered {
select {
case evt := <-sub.C:
collectedEvents = append(collectedEvents, evt)
if evt.Kind != EventKindToolExecEnd {
continue
}
payload, ok := evt.Payload.(ToolExecEndPayload)
if !ok || payload.Tool != "tool_one" {
continue
}
al.Steer(providers.Message{Role: "user", Content: "change direction"})
steered = true
case <-deadline:
t.Fatal("timeout waiting for tool_one to finish before steering")
}
}
select {
case r := <-resultCh:
@ -832,7 +849,7 @@ func TestAgentLoop_HookRespond_SteeringSkipsRemaining(t *testing.T) {
t.Fatal("timeout waiting for result")
}
events := collectEventStream(sub.C)
events := append(collectedEvents, collectEventStream(sub.C)...)
skippedEvts := filterEvents(events, EventKindToolExecSkipped)
if len(skippedEvts) < 1 {

54
pkg/config/config.go
View file

@ -989,6 +989,10 @@ const (
envSkillsClawHubTimeout = "PICOCLAW_SKILLS_REGISTRIES_CLAWHUB_TIMEOUT"
envSkillsClawHubMaxZipSize = "PICOCLAW_SKILLS_REGISTRIES_CLAWHUB_MAX_ZIP_SIZE"
envSkillsClawHubMaxResponseSize = "PICOCLAW_SKILLS_REGISTRIES_CLAWHUB_MAX_RESPONSE_SIZE"
envSkillsGitHubEnabled = "PICOCLAW_SKILLS_REGISTRIES_GITHUB_ENABLED"
envSkillsGitHubBaseURL = "PICOCLAW_SKILLS_REGISTRIES_GITHUB_BASE_URL"
envSkillsGitHubAuthToken = "PICOCLAW_SKILLS_REGISTRIES_GITHUB_AUTH_TOKEN"
envSkillsGitHubProxy = "PICOCLAW_SKILLS_REGISTRIES_GITHUB_PROXY"
)
func (c *SkillRegistryConfig) DecodeParam(target any) error {
@ -1201,8 +1205,8 @@ func applySkillsRegistryEnvCompat(cfg *Config) {
return
}
registryCfg, ok := cfg.Tools.Skills.Registries.Get("clawhub")
if !ok {
registryCfg, foundClawHub := cfg.Tools.Skills.Registries.Get("clawhub")
if !foundClawHub {
registryCfg = SkillRegistryConfig{
Name: "clawhub",
Param: map[string]any{},
@ -1212,43 +1216,71 @@ func applySkillsRegistryEnvCompat(cfg *Config) {
registryCfg.Param = map[string]any{}
}
if raw, ok := os.LookupEnv(envSkillsClawHubEnabled); ok {
if raw, envSet := os.LookupEnv(envSkillsClawHubEnabled); envSet {
if value, err := strconv.ParseBool(strings.TrimSpace(raw)); err == nil {
registryCfg.Enabled = value
}
}
if value, ok := os.LookupEnv(envSkillsClawHubBaseURL); ok {
if value, envSet := os.LookupEnv(envSkillsClawHubBaseURL); envSet {
registryCfg.BaseURL = value
}
if value, ok := os.LookupEnv(envSkillsClawHubAuthToken); ok {
if value, envSet := os.LookupEnv(envSkillsClawHubAuthToken); envSet {
registryCfg.AuthToken = *NewSecureString(value)
}
if value, ok := os.LookupEnv(envSkillsClawHubSearchPath); ok {
if value, envSet := os.LookupEnv(envSkillsClawHubSearchPath); envSet {
registryCfg.Param["search_path"] = value
}
if value, ok := os.LookupEnv(envSkillsClawHubSkillsPath); ok {
if value, envSet := os.LookupEnv(envSkillsClawHubSkillsPath); envSet {
registryCfg.Param["skills_path"] = value
}
if value, ok := os.LookupEnv(envSkillsClawHubDownloadPath); ok {
if value, envSet := os.LookupEnv(envSkillsClawHubDownloadPath); envSet {
registryCfg.Param["download_path"] = value
}
if raw, ok := os.LookupEnv(envSkillsClawHubTimeout); ok {
if raw, envSet := os.LookupEnv(envSkillsClawHubTimeout); envSet {
if value, err := strconv.Atoi(strings.TrimSpace(raw)); err == nil {
registryCfg.Param["timeout"] = value
}
}
if raw, ok := os.LookupEnv(envSkillsClawHubMaxZipSize); ok {
if raw, envSet := os.LookupEnv(envSkillsClawHubMaxZipSize); envSet {
if value, err := strconv.Atoi(strings.TrimSpace(raw)); err == nil {
registryCfg.Param["max_zip_size"] = value
}
}
if raw, ok := os.LookupEnv(envSkillsClawHubMaxResponseSize); ok {
if raw, envSet := os.LookupEnv(envSkillsClawHubMaxResponseSize); envSet {
if value, err := strconv.Atoi(strings.TrimSpace(raw)); err == nil {
registryCfg.Param["max_response_size"] = value
}
}
cfg.Tools.Skills.Registries.Set("clawhub", registryCfg)
githubCfg, foundGitHub := cfg.Tools.Skills.Registries.Get("github")
if !foundGitHub {
githubCfg = SkillRegistryConfig{
Name: "github",
Param: map[string]any{},
}
}
if githubCfg.Param == nil {
githubCfg.Param = map[string]any{}
}
if raw, envSet := os.LookupEnv(envSkillsGitHubEnabled); envSet {
if value, err := strconv.ParseBool(strings.TrimSpace(raw)); err == nil {
githubCfg.Enabled = value
}
}
if value, envSet := os.LookupEnv(envSkillsGitHubBaseURL); envSet {
githubCfg.BaseURL = value
}
if value, envSet := os.LookupEnv(envSkillsGitHubAuthToken); envSet {
githubCfg.AuthToken = *NewSecureString(value)
}
if value, envSet := os.LookupEnv(envSkillsGitHubProxy); envSet {
githubCfg.Param["proxy"] = value
}
cfg.Tools.Skills.Registries.Set("github", githubCfg)
}
func makeBackup(path string) error {

12
pkg/config/config_struct.go
View file

@ -454,6 +454,10 @@ func (c *SkillRegistryConfig) UnmarshalJSON(data []byte) error {
switch key {
case "name", "enabled", "base_url", "auth_token", "param":
continue
case "_auth_token":
// UI/API shadow secret fields should hydrate SecureString only and must
// never be persisted as arbitrary registry params.
continue
default:
var decoded any
if err := json.Unmarshal(value, &decoded); err != nil {
@ -475,7 +479,7 @@ func (c SkillRegistryConfig) MarshalJSON() ([]byte, error) {
m["auth_token"] = c.AuthToken
}
for key, value := range c.Param {
if key == "" || key == "param" {
if key == "" || key == "param" || strings.HasPrefix(key, "_") {
continue
}
if _, exists := m[key]; exists {
@ -522,6 +526,10 @@ func (c *SkillRegistryConfig) UnmarshalYAML(value *yaml.Node) error {
if err := yaml.Unmarshal(data, &c.AuthToken); err != nil {
return err
}
case "_auth_token":
// UI/API shadow secret fields should hydrate SecureString only and must
// never be persisted as arbitrary registry params.
continue
case "param":
continue
default:
@ -542,7 +550,7 @@ func (c SkillRegistryConfig) MarshalYAML() (any, error) {
}
keys := make([]string, 0, len(c.Param))
for key := range c.Param {
if key == "" || key == "param" {
if key == "" || key == "param" || strings.HasPrefix(key, "_") {
continue
}
keys = append(keys, key)

41
pkg/config/config_struct_test.go
View file

@ -181,6 +181,47 @@ func TestSkillRegistryConfigJSONFlattensParam(t *testing.T) {
assert.Equal(t, "http://127.0.0.1:7890", loaded.Param["proxy"])
}
func TestSkillRegistryConfigJSONIgnoresShadowSecretFields(t *testing.T) {
var registry SkillRegistryConfig
err := json.Unmarshal([]byte(`{
"enabled": true,
"base_url": "https://github.com",
"_auth_token": "shadow-secret",
"proxy": "http://127.0.0.1:7890"
}`), &registry)
assert.NoError(t, err)
assert.Equal(t, "https://github.com", registry.BaseURL)
assert.Equal(t, "http://127.0.0.1:7890", registry.Param["proxy"])
_, exists := registry.Param["_auth_token"]
assert.False(t, exists)
registry.Param["_auth_token"] = "should-not-round-trip"
data, err := json.Marshal(registry)
assert.NoError(t, err)
assert.NotContains(t, string(data), "_auth_token")
assert.Contains(t, string(data), `"proxy":"http://127.0.0.1:7890"`)
yamlData, err := yaml.Marshal(registry)
assert.NoError(t, err)
assert.NotContains(t, string(yamlData), "_auth_token")
assert.Contains(t, string(yamlData), "proxy: http://127.0.0.1:7890")
}
func TestSkillRegistryConfigYAMLIgnoresShadowSecretFields(t *testing.T) {
var registry SkillRegistryConfig
err := yaml.Unmarshal([]byte(`
enabled: true
base_url: https://github.com
_auth_token: shadow-secret
proxy: http://127.0.0.1:7890
`), &registry)
assert.NoError(t, err)
assert.Equal(t, "https://github.com", registry.BaseURL)
assert.Equal(t, "http://127.0.0.1:7890", registry.Param["proxy"])
_, exists := registry.Param["_auth_token"]
assert.False(t, exists)
}
func TestSkillsRegistriesConfigMarshalYAMLIncludesRegistryToken(t *testing.T) {
registries := SkillsRegistriesConfig{
&SkillRegistryConfig{

36
pkg/config/config_test.go
View file

@ -1567,6 +1567,42 @@ func TestLoadConfig_AppliesLegacyClawHubRegistryEnvOverrides(t *testing.T) {
}
}
func TestLoadConfig_AppliesGitHubRegistryEnvOverrides(t *testing.T) {
dir := t.TempDir()
cfgPath := filepath.Join(dir, "config.json")
data := `{"version":2,"tools":{"skills":{"registries":{"github":{"enabled":true,"base_url":"https://github.com"}}}}}`
if err := os.WriteFile(cfgPath, []byte(data), 0o600); err != nil {
t.Fatalf("setup: %v", err)
}
t.Setenv(envSkillsGitHubBaseURL, "https://ghe.example.com/git")
t.Setenv(envSkillsGitHubAuthToken, "github-token-from-env")
t.Setenv(envSkillsGitHubEnabled, "false")
t.Setenv(envSkillsGitHubProxy, "http://127.0.0.1:7890")
cfg, err := LoadConfig(cfgPath)
if err != nil {
t.Fatalf("LoadConfig: %v", err)
}
github, ok := cfg.Tools.Skills.Registries.Get("github")
if !ok {
t.Fatal("github registry missing")
}
if github.BaseURL != "https://ghe.example.com/git" {
t.Fatalf("BaseURL = %q, want %q", github.BaseURL, "https://ghe.example.com/git")
}
if github.AuthToken.String() != "github-token-from-env" {
t.Fatalf("AuthToken = %q, want %q", github.AuthToken.String(), "github-token-from-env")
}
if github.Enabled {
t.Fatal("Enabled = true, want false")
}
if got := github.Param["proxy"]; got != "http://127.0.0.1:7890" {
t.Fatalf("proxy = %v, want %q", got, "http://127.0.0.1:7890")
}
}
func TestModelConfig_ExtraBodyRoundTrip(t *testing.T) {
dir := t.TempDir()
cfgPath := filepath.Join(dir, "config.json")

96
pkg/config/security.go
View file

@ -46,6 +46,102 @@ func loadSecurityConfig(cfg *Config, securityPath string) error {
if err := yaml.Unmarshal(data, cfg); err != nil {
return fmt.Errorf("failed to parse security config: %w", err)
}
if err := applyLegacySkillsSecurityConfig(cfg, data); err != nil {
return fmt.Errorf("failed to parse legacy skills security config: %w", err)
}
return nil
}
func applyLegacySkillsSecurityConfig(cfg *Config, data []byte) error {
var root yaml.Node
if err := yaml.Unmarshal(data, &root); err != nil {
return err
}
if len(root.Content) == 0 {
return nil
}
rootMap := root.Content[0]
if rootMap == nil || rootMap.Kind != yaml.MappingNode {
return nil
}
for i := 0; i+1 < len(rootMap.Content); i += 2 {
keyNode := rootMap.Content[i]
valueNode := rootMap.Content[i+1]
if keyNode == nil || valueNode == nil || strings.TrimSpace(keyNode.Value) != "skills" {
continue
}
return applyLegacySkillsSecurityNode(cfg, valueNode)
}
return nil
}
func applyLegacySkillsSecurityNode(cfg *Config, skillsNode *yaml.Node) error {
if cfg == nil || skillsNode == nil || skillsNode.Kind != yaml.MappingNode {
return nil
}
for i := 0; i+1 < len(skillsNode.Content); i += 2 {
nameNode := skillsNode.Content[i]
valueNode := skillsNode.Content[i+1]
if nameNode == nil || valueNode == nil {
continue
}
name := strings.TrimSpace(nameNode.Value)
if name == "" || name == "registries" {
continue
}
if name == "github" {
var legacyGitHub SkillsGithubConfig
if err := valueNode.Decode(&legacyGitHub); err != nil {
return err
}
if cfg.Tools.Skills.Github.Token.String() == "" && legacyGitHub.Token.String() != "" {
cfg.Tools.Skills.Github.Token = legacyGitHub.Token
}
}
var legacyRegistry SkillRegistryConfig
if err := valueNode.Decode(&legacyRegistry); err != nil {
return err
}
legacyRegistry.Name = name
if legacyRegistry.AuthToken.String() == "" {
if name == "github" && cfg.Tools.Skills.Github.Token.String() != "" {
legacyRegistry.AuthToken = cfg.Tools.Skills.Github.Token
} else {
continue
}
}
registryCfg, ok := cfg.Tools.Skills.Registries.Get(name)
if !ok {
registryCfg = SkillRegistryConfig{
Name: name,
Param: map[string]any{},
}
}
if registryCfg.Param == nil {
registryCfg.Param = map[string]any{}
}
if registryCfg.AuthToken.String() == "" {
registryCfg.AuthToken = legacyRegistry.AuthToken
}
if registryCfg.BaseURL == "" && legacyRegistry.BaseURL != "" {
registryCfg.BaseURL = legacyRegistry.BaseURL
}
for key, value := range legacyRegistry.Param {
if _, exists := registryCfg.Param[key]; !exists {
registryCfg.Param[key] = value
}
}
cfg.Tools.Skills.Registries.Set(name, registryCfg)
}
return nil
}

73
pkg/config/security_integration_test.go
View file

@ -527,4 +527,77 @@ skills:
require.True(t, ok)
assert.Equal(t, "https://github.com", githubRegistry.BaseURL)
})
t.Run("Legacy direct registry security entries remain supported", func(t *testing.T) {
tmpDir := t.TempDir()
configPath := filepath.Join(tmpDir, "config.json")
configContent := `{
"version": 1,
"tools": {
"skills": {
"registries": {
"clawhub": {
"enabled": true,
"base_url": "https://clawhub.ai"
}
}
}
}
}`
err := os.WriteFile(configPath, []byte(configContent), 0o644)
require.NoError(t, err)
securityPath := filepath.Join(tmpDir, SecurityConfigFile)
securityContent := `skills:
clawhub:
auth_token: "legacy-clawhub-token"
`
err = os.WriteFile(securityPath, []byte(securityContent), 0o600)
require.NoError(t, err)
cfg, err := LoadConfig(configPath)
require.NoError(t, err)
registry, ok := cfg.Tools.Skills.Registries.Get("clawhub")
require.True(t, ok)
assert.Equal(t, "legacy-clawhub-token", registry.AuthToken.String())
})
t.Run("Legacy github security token populates github registry", func(t *testing.T) {
tmpDir := t.TempDir()
configPath := filepath.Join(tmpDir, "config.json")
configContent := `{
"version": 1,
"tools": {
"skills": {
"registries": {
"github": {
"enabled": true,
"base_url": "https://github.com"
}
}
}
}
}`
err := os.WriteFile(configPath, []byte(configContent), 0o644)
require.NoError(t, err)
securityPath := filepath.Join(tmpDir, SecurityConfigFile)
securityContent := `skills:
github:
token: "legacy-github-token"
`
err = os.WriteFile(securityPath, []byte(securityContent), 0o600)
require.NoError(t, err)
cfg, err := LoadConfig(configPath)
require.NoError(t, err)
registry, ok := cfg.Tools.Skills.Registries.Get("github")
require.True(t, ok)
assert.Equal(t, "legacy-github-token", cfg.Tools.Skills.Github.Token.String())
assert.Equal(t, "legacy-github-token", registry.AuthToken.String())
})
}

9
pkg/skills/config_bridge.go
View file

@ -95,6 +95,15 @@ func LookupRegistryFromToolsConfig(cfg config.SkillsToolsConfig, name string) Sk
return nil
}
func GitHubInstallDirNameFromToolsConfig(cfg config.SkillsToolsConfig, target string) (string, error) {
registryCfg, ok := cfg.Registries.Get("github")
if ok {
registryCfg = applyLegacyGithubRegistryCompatibility(cfg, registryCfg)
return githubInstallDirNameWithBaseURL(target, registryCfg.BaseURL)
}
return githubInstallDirNameWithBaseURL(target, cfg.Github.BaseURL)
}
func NormalizeInstallTargetForRegistry(cfg config.SkillsToolsConfig, registryName, target string) string {
if registryName == "" || target == "" {
return target

9
pkg/skills/github_registry.go
View file

@ -90,16 +90,19 @@ func (r *GitHubRegistry) SkillURL(target, version string) string {
ref := parsedTarget.Ref
base := strings.TrimRight(parsedTarget.Endpoints.WebBaseURL, "/")
urlPath := path.Join(ref.Owner, ref.RepoName)
if ref.Ref == "" {
return fmt.Sprintf("%s/%s", base, urlPath)
}
if ref.SubPath != "" {
if ref.Ref == "" {
return ""
}
viewKind := "tree"
if isSkillMarkdownPath(ref.SubPath) {
viewKind = "blob"
}
return fmt.Sprintf("%s/%s/%s/%s/%s", base, urlPath, viewKind, ref.Ref, ref.SubPath)
}
if ref.Ref == "" {
return fmt.Sprintf("%s/%s", base, urlPath)
}
if ref.Ref != "main" {
return fmt.Sprintf("%s/%s/tree/%s", base, urlPath, ref.Ref)
}

1
pkg/skills/github_registry_test.go
View file

@ -186,6 +186,7 @@ func TestGitHubRegistrySkillURLUsesProvidedVersionAndBasePath(t *testing.T) {
"https://github.com/org/repo/tree/main/.agents/skills/pr-review",
registry.SkillURL("https://github.com/org/repo/tree/main/.agents/skills/pr-review", ""),
)
assert.Empty(t, registry.SkillURL("org/repo/.agents/skills/pr-review", ""))
}
func TestGitHubRegistryResolveInstallDirNameSupportsFullURLs(t *testing.T) {

41
pkg/skills/installer.go
View file

@ -210,32 +210,23 @@ func splitGitHubTreeOrBlobRefPath(parts []string, defaultRef string) (string, st
if len(parts) == 0 {
return defaultRef, ""
}
for i := 1; i < len(parts); i++ {
candidateRef := strings.Join(parts[:i], "/")
candidateSubPath := strings.Join(parts[i:], "/")
if looksLikeSkillSubPath(candidateSubPath) {
return candidateRef, candidateSubPath
if anchor := knownSkillSubPathAnchor(parts); anchor > 0 {
return strings.Join(parts[:anchor], "/"), strings.Join(parts[anchor:], "/")
}
if parts[len(parts)-1] == "SKILL.md" {
return strings.Join(parts[:len(parts)-1], "/"), "SKILL.md"
}
return parts[0], strings.Join(parts[1:], "/")
}
func looksLikeSkillSubPath(subPath string) bool {
subPath = strings.Trim(strings.TrimSpace(subPath), "/")
if subPath == "" {
return false
func knownSkillSubPathAnchor(parts []string) int {
for i := 1; i < len(parts); i++ {
candidateSubPath := strings.Join(parts[i:], "/")
if strings.HasPrefix(candidateSubPath, ".agents/skills/") || strings.HasPrefix(candidateSubPath, "skills/") {
return i
}
if isSkillMarkdownPath(subPath) {
return true
}
parts := strings.Split(subPath, "/")
if len(parts) >= 2 && parts[0] == "skills" {
return true
}
if len(parts) >= 3 && parts[0] == ".agents" && parts[1] == "skills" {
return true
}
return false
return -1
}
func isSkillMarkdownPath(subPath string) bool {
@ -437,11 +428,19 @@ func (si *SkillInstaller) InstallFromGitHubToDir(
return nil, err
}
ref := target.Ref
apiSubPath := strings.Trim(ref.SubPath, "/")
if isSkillMarkdownPath(apiSubPath) {
if dir := path.Dir(apiSubPath); dir == "." {
apiSubPath = ""
} else {
apiSubPath = dir
}
}
// Build GitHub API URL
apiPath := path.Join(ref.Owner, ref.RepoName, "contents")
if ref.SubPath != "" {
apiPath = path.Join(apiPath, ref.SubPath)
if apiSubPath != "" {
apiPath = path.Join(apiPath, apiSubPath)
}
apiURL := fmt.Sprintf("%s/repos/%s?ref=%s", target.Endpoints.APIBaseURL, apiPath, url.QueryEscape(ref.Ref))

36
pkg/skills/installer_test.go
View file

@ -289,6 +289,23 @@ func TestParseGitHubTargetWithBaseURLPreservesSourceEndpoints(t *testing.T) {
}
}
func TestParseGitHubTargetWithBaseURLPreservesSlashBranchForRepoRootBlobSkill(t *testing.T) {
target, err := parseGitHubTargetWithBaseURL(
"https://github.com/org/repo/blob/feature/skills-registry/SKILL.md",
"",
"",
)
if err != nil {
t.Fatalf("parseGitHubTargetWithBaseURL() unexpected error = %v", err)
}
if target.Ref.Ref != "feature/skills-registry" {
t.Fatalf("ref = %q, want feature/skills-registry", target.Ref.Ref)
}
if target.Ref.SubPath != "SKILL.md" {
t.Fatalf("subPath = %q, want SKILL.md", target.Ref.SubPath)
}
}
func TestSkillInstallerResolveGitHubRefUsesDefaultBranch(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
switch r.URL.Path {
@ -324,11 +341,21 @@ func TestSkillInstallerInstallFromGitHubToDirSupportsBlobSkillURL(t *testing.T)
var server *httptest.Server
server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
switch r.URL.Path {
case "/api/v3/repos/org/repo/contents/.agents/skills/pr-review/SKILL.md":
case "/api/v3/repos/org/repo/contents/.agents/skills/pr-review":
w.Header().Set("Content-Type", "application/json")
_, _ = w.Write([]byte(`{"type":"file","name":"SKILL.md"}`))
_, _ = w.Write([]byte(`[
{"type":"file","name":"SKILL.md","download_url":"` + server.URL + `/raw/org/repo/main/.agents/skills/pr-review/SKILL.md"},
{"type":"dir","name":"scripts","url":"` + server.URL + `/api/v3/repos/org/repo/contents/.agents/skills/pr-review/scripts?ref=main"}
]`))
case "/api/v3/repos/org/repo/contents/.agents/skills/pr-review/scripts":
w.Header().Set("Content-Type", "application/json")
_, _ = w.Write([]byte(`[
{"type":"file","name":"check.sh","download_url":"` + server.URL + `/raw/org/repo/main/.agents/skills/pr-review/scripts/check.sh"}
]`))
case "/raw/org/repo/main/.agents/skills/pr-review/SKILL.md":
_, _ = w.Write([]byte("---\nname: pr-review\ndescription: PR review skill\n---\n# PR Review\n"))
case "/raw/org/repo/main/.agents/skills/pr-review/scripts/check.sh":
_, _ = w.Write([]byte("#!/bin/sh\nexit 0\n"))
default:
t.Fatalf("unexpected path: %s", r.URL.Path)
}
@ -361,6 +388,11 @@ func TestSkillInstallerInstallFromGitHubToDirSupportsBlobSkillURL(t *testing.T)
if !strings.Contains(string(content), "name: pr-review") {
t.Fatalf("SKILL.md content = %q, want skill metadata", string(content))
}
scriptPath := filepath.Join(targetDir, "scripts", "check.sh")
if _, err := os.Stat(scriptPath); err != nil {
t.Fatalf("Stat(scripts/check.sh) error = %v", err)
}
}
func TestShouldDownload(t *testing.T) {

52
web/backend/api/config_test.go
View file

@ -6,6 +6,7 @@ import (
"net/http/httptest"
"os"
"path/filepath"
"strings"
"testing"
"github.com/sipeed/picoclaw/pkg/config"
@ -379,6 +380,57 @@ func TestHandlePatchConfig_SavesDiscordTokenFromPayload(t *testing.T) {
}
}
func TestHandlePatchConfig_DoesNotPersistShadowRegistryAuthTokenField(t *testing.T) {
configPath, cleanup := setupOAuthTestEnv(t)
defer cleanup()
h := NewHandler(configPath)
mux := http.NewServeMux()
h.RegisterRoutes(mux)
req := httptest.NewRequest(http.MethodPatch, "/api/config", bytes.NewBufferString(`{
"tools": {
"skills": {
"registries": {
"github": {
"_auth_token": "ghp-shadow-token"
}
}
}
}
}`))
req.Header.Set("Content-Type", "application/json")
rec := httptest.NewRecorder()
mux.ServeHTTP(rec, req)
if rec.Code != http.StatusOK {
t.Fatalf("PATCH /api/config status = %d, want %d, body=%s", rec.Code, http.StatusOK, rec.Body.String())
}
cfg, err := config.LoadConfig(configPath)
if err != nil {
t.Fatalf("LoadConfig() error = %v", err)
}
githubRegistry, ok := cfg.Tools.Skills.Registries.Get("github")
if !ok {
t.Fatal("github registry missing after PATCH")
}
if got := githubRegistry.AuthToken.String(); got != "ghp-shadow-token" {
t.Fatalf("github registry auth token = %q, want %q", got, "ghp-shadow-token")
}
if got := githubRegistry.BaseURL; got != "https://github.com" {
t.Fatalf("github registry base_url = %q, want %q", got, "https://github.com")
}
rawConfig, err := os.ReadFile(configPath)
if err != nil {
t.Fatalf("ReadFile(configPath) error = %v", err)
}
if strings.Contains(string(rawConfig), "_auth_token") {
t.Fatalf("config.json should not persist _auth_token shadow field, got:\n%s", string(rawConfig))
}
}
func TestHandlePatchConfig_AllowsInvalidDenyRegexPatternsWhenDenyPatternsDisabled(t *testing.T) {
configPath, cleanup := setupOAuthTestEnv(t)
defer cleanup()

6
web/backend/api/pico_test.go
View file

@ -567,7 +567,11 @@ func TestHandleWebSocketProxyLoadsPidDataOnDemand(t *testing.T) {
}
func TestHandleWebSocketProxyRejectsStalePidDataAfterProcessExit(t *testing.T) {
configPath := filepath.Join(t.TempDir(), "config.json")
tmpDir := t.TempDir()
t.Setenv("HOME", tmpDir)
t.Setenv("PICOCLAW_HOME", filepath.Join(tmpDir, ".picoclaw"))
configPath := filepath.Join(tmpDir, "config.json")
h := NewHandler(configPath)
handler := h.handleWebSocketProxy()

9
web/backend/api/skills.go
View file

@ -493,13 +493,14 @@ func (h *Handler) handleDeleteSkill(w http.ResponseWriter, r *http.Request) {
workspaceSkillWriteMu.Lock()
defer workspaceSkillWriteMu.Unlock()
var matchedNonWorkspace bool
for _, skill := range loader.ListSkills() {
if skill.Name != name {
continue
}
if skill.Source != "workspace" {
http.Error(w, "only workspace skills can be deleted", http.StatusBadRequest)
return
matchedNonWorkspace = true
continue
}
if err := os.RemoveAll(filepath.Dir(skill.Path)); err != nil {
http.Error(w, fmt.Sprintf("Failed to delete skill: %v", err), http.StatusInternalServerError)
@ -509,6 +510,10 @@ func (h *Handler) handleDeleteSkill(w http.ResponseWriter, r *http.Request) {
json.NewEncoder(w).Encode(map[string]string{"status": "ok"})
return
}
if matchedNonWorkspace {
http.Error(w, "only workspace skills can be deleted", http.StatusBadRequest)
return
}
http.Error(w, "Skill not found", http.StatusNotFound)
}

59
web/backend/api/skills_test.go
View file

@ -549,6 +549,65 @@ func TestHandleDeleteSkill(t *testing.T) {
}
}
func TestHandleDeleteSkillPrefersWorkspaceMatch(t *testing.T) {
configPath, cleanup := setupOAuthTestEnv(t)
defer cleanup()
cfg, err := config.LoadConfig(configPath)
if err != nil {
t.Fatalf("LoadConfig() error = %v", err)
}
homeDir := t.TempDir()
t.Setenv(config.EnvHome, homeDir)
workspace := filepath.Join(t.TempDir(), "workspace")
cfg.Agents.Defaults.Workspace = workspace
if err := config.SaveConfig(configPath, cfg); err != nil {
t.Fatalf("SaveConfig() error = %v", err)
}
workspaceSkillDir := filepath.Join(workspace, "skills", "delete-me-workspace")
if err := os.MkdirAll(workspaceSkillDir, 0o755); err != nil {
t.Fatalf("MkdirAll(workspace) error = %v", err)
}
if err := os.WriteFile(
filepath.Join(workspaceSkillDir, "SKILL.md"),
[]byte("---\nname: delete-me\ndescription: workspace delete me\n---\n"),
0o644,
); err != nil {
t.Fatalf("WriteFile(workspace) error = %v", err)
}
globalSkillDir := filepath.Join(homeDir, "skills", "delete-me-global")
if err := os.MkdirAll(globalSkillDir, 0o755); err != nil {
t.Fatalf("MkdirAll(global) error = %v", err)
}
if err := os.WriteFile(
filepath.Join(globalSkillDir, "SKILL.md"),
[]byte("---\nname: delete-me\ndescription: global delete me\n---\n"),
0o644,
); err != nil {
t.Fatalf("WriteFile(global) error = %v", err)
}
h := NewHandler(configPath)
mux := http.NewServeMux()
h.RegisterRoutes(mux)
rec := httptest.NewRecorder()
req := httptest.NewRequest(http.MethodDelete, "/api/skills/delete-me", nil)
mux.ServeHTTP(rec, req)
if rec.Code != http.StatusOK {
t.Fatalf("status = %d, want %d, body=%s", rec.Code, http.StatusOK, rec.Body.String())
}
if _, err := os.Stat(workspaceSkillDir); !os.IsNotExist(err) {
t.Fatalf("workspace skill directory should be removed, stat err=%v", err)
}
if _, err := os.Stat(globalSkillDir); err != nil {
t.Fatalf("global skill directory should remain, stat err=%v", err)
}
}
func TestHandleSearchSkills(t *testing.T) {
configPath, cleanup := setupOAuthTestEnv(t)
defer cleanup()