From 8068b7dcfd039960f50b9ea446a65fbd3938f7e7 Mon Sep 17 00:00:00 2001 From: harshbansal7 Date: Wed, 18 Feb 2026 17:19:05 +0530 Subject: [PATCH] Comments resolved --- cmd/picoclaw/main.go | 10 ++++++++-- pkg/skills/clawhub_registry.go | 4 ++-- pkg/skills/registry.go | 2 +- pkg/tools/skills_install.go | 20 ++++++++++++++++++-- pkg/utils/skills.go | 5 +++-- 5 files changed, 32 insertions(+), 9 deletions(-) diff --git a/cmd/picoclaw/main.go b/cmd/picoclaw/main.go index 2d38626c5..1e6af1ed7 100644 --- a/cmd/picoclaw/main.go +++ b/cmd/picoclaw/main.go @@ -1344,13 +1344,19 @@ func skillsInstallFromRegistry(cfg *config.Config, registryName, slug string) { result, err := registry.DownloadAndInstall(ctx, slug, "", targetDir) if err != nil { - os.RemoveAll(targetDir) + rmErr := os.RemoveAll(targetDir) + if rmErr != nil { + fmt.Printf("\u2717 Failed to remove partial install: %v\n", rmErr) + } fmt.Printf("\u2717 Failed to install skill: %v\n", err) os.Exit(1) } if result.IsMalwareBlocked { - os.RemoveAll(targetDir) + rmErr := os.RemoveAll(targetDir) + if rmErr != nil { + fmt.Printf("\u2717 Failed to remove partial install: %v\n", rmErr) + } fmt.Printf("\u2717 Skill '%s' is flagged as malicious and cannot be installed.\n", slug) os.Exit(1) } diff --git a/pkg/skills/clawhub_registry.go b/pkg/skills/clawhub_registry.go index 3f5e27ba0..e2a940afd 100644 --- a/pkg/skills/clawhub_registry.go +++ b/pkg/skills/clawhub_registry.go @@ -19,7 +19,7 @@ const ( defaultMaxResponseSize = 2 * 1024 * 1024 // 2 MB ) -// ClawHubRegistry implements SkillRegistry for the ClawhHub platform. +// ClawHubRegistry implements SkillRegistry for the ClawHub platform. type ClawHubRegistry struct { baseURL string authToken string // Optional - for elevated rate limits @@ -31,7 +31,7 @@ type ClawHubRegistry struct { client *http.Client } -// NewClawHubRegistry creates a new ClawhHub registry client from config. +// NewClawHubRegistry creates a new ClawHub registry client from config. func NewClawHubRegistry(cfg ClawHubConfig) *ClawHubRegistry { baseURL := cfg.BaseURL if baseURL == "" { diff --git a/pkg/skills/registry.go b/pkg/skills/registry.go index 3be27cd11..45ae72253 100644 --- a/pkg/skills/registry.go +++ b/pkg/skills/registry.go @@ -64,7 +64,7 @@ type RegistryConfig struct { MaxConcurrentSearches int } -// ClawHubConfig configures the ClawhHub registry. +// ClawHubConfig configures the ClawHub registry. type ClawHubConfig struct { Enabled bool BaseURL string diff --git a/pkg/tools/skills_install.go b/pkg/tools/skills_install.go index b6284c8f6..6b05918ce 100644 --- a/pkg/tools/skills_install.go +++ b/pkg/tools/skills_install.go @@ -116,13 +116,29 @@ func (t *InstallSkillTool) Execute(ctx context.Context, args map[string]interfac result, err := registry.DownloadAndInstall(ctx, slug, version, targetDir) if err != nil { // Clean up partial install. - os.RemoveAll(targetDir) + rmErr := os.RemoveAll(targetDir) + if rmErr != nil { + logger.ErrorCF("tool", "Failed to remove partial install", + map[string]interface{}{ + "tool": "install_skill", + "target_dir": targetDir, + "error": rmErr.Error(), + }) + } return ErrorResult(fmt.Sprintf("failed to install %q: %v", slug, err)) } // Moderation: block malware. if result.IsMalwareBlocked { - os.RemoveAll(targetDir) + rmErr := os.RemoveAll(targetDir) + if rmErr != nil { + logger.ErrorCF("tool", "Failed to remove partial install", + map[string]interface{}{ + "tool": "install_skill", + "target_dir": targetDir, + "error": rmErr.Error(), + }) + } return ErrorResult(fmt.Sprintf("skill %q is flagged as malicious and cannot be installed", slug)) } diff --git a/pkg/utils/skills.go b/pkg/utils/skills.go index f66fa4915..1d2cfac7f 100644 --- a/pkg/utils/skills.go +++ b/pkg/utils/skills.go @@ -8,10 +8,11 @@ import ( // ValidateSkillIdentifier validates that the given skill identifier (slug or registry name) is non-empty // and does not contain path separators ("/", "\\") or ".." for security. func ValidateSkillIdentifier(identifier string) error { - if identifier == "" { + trimmed := strings.TrimSpace(identifier) + if trimmed == "" { return fmt.Errorf("identifier is required and must be a non-empty string") } - if strings.ContainsAny(identifier, "/\\") || strings.Contains(identifier, "..") { + if strings.ContainsAny(trimmed, "/\\") || strings.Contains(trimmed, "..") { return fmt.Errorf("identifier must not contain path separators or '..' to prevent directory traversal") } return nil